Skip to content

xenopsd: set xen-platform-pci-bar-uc key in xenstore #6566

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

AnthoineB
Copy link
Contributor

This patch add a new parameter named 'xen-platform-pci-bar-uc' in xenopsd config file who has a default value of 'true' to keep the default behavior of hvmloader. Putting 'false' to this parameter will tell xenopsd to add a xenstore key of '0' in:
'/local/domain//hvmloader/pci/xen-platform-pci-bar-uc'. Only this key set to 0 will change the behavior of hvmloader. Otherwise, xenstore is set with '1' by default.

This changeset is link to this xen commit:
https://xenbits.xen.org/gitweb/?p=xen.git;a=commit;h=22650d6054625be10172fe0c78b9cadd1a39bd63

This is a new and much more simpler version of: #6555

This patch add a new parameter named 'xen-platform-pci-bar-uc' in
xenopsd config file who has a default value of 'true' to keep the
default behavior of hvmloader.  Putting 'false' to this parameter will
tell xenopsd to add a xenstore key of '0' in:
'/local/domain/<domid>/hvmloader/pci/xen-platform-pci-bar-uc'.
Only this key set to 0 will change the behavior of hvmloader.

This changeset is link to this xen commit:
https://xenbits.xen.org/gitweb/?p=xen.git;a=commit;h=22650d6054625be10172fe0c78b9cadd1a39bd63

Signed-off-by: Anthoine Bourgeois <[email protected]>
@AnthoineB AnthoineB force-pushed the xenopsd-AMD-xen-pci-MMIO-cache branch from cc83e10 to 83a4888 Compare July 2, 2025 12:27
Comment on lines +217 to +218
(WB, the workaround). WB mapping could improve performance of devices \
using grant tables. This is useful on AMD platform only."
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I understood Roger's explanation, the current behaviour behaviour of both hvmloader and guest kernels is that they set the uncacheable bit for for the foreign mapping for AMD systems.
In the future, the hvmloader will add a shortcut to allow guest kernels to use write-back to the foreign mappings, (this is xen_platform_pci_bar_uc=0) but only some of the guest kernels will benefit from it (linux kernel when using memremap).

What I don't quite grasp is:

  • Does setting this to 1 have negative effects? This includes
    • Intel systems where this setting, according to the description, shouldn't change behaviour.
    • AMD systems with unpatched linux kernels in guests; or Windows guests

I'm asking this because it looks to me this is a per-guest setting, and logically it should be overrdable by the platform data coming from the guest template. Having this setting be per-host can be dangerous if the setting has negative side-effects since not all guest have the same kernel.

Alternatively, if this workaround doesn't have negative side-effects, it might be worthwhile to simply set it to 0 and have the free perf boost on some of the VMs.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your understanding is good.

I didn't see any regression or negative effect in my tests (Intel or AMD with windows guests or linux guest without memremap patch). There is not effect on those configurations. But, of course, there are configurations I didn't test.

I agree with you. I'll work on the platform data to set this parameter.

Copy link
Contributor

@edwintorok edwintorok Jul 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From my testing there is a massive performance improvement on AMD when this is set to 0.
Setting it to 1 is a noop, because that is what Xen already sets it to.

On Intel this is mostly a noop because Xen already sets it to WB (UC=0) via another mechanism.
I've tested Intel and UC=0 has no measurable effect, so it is better to just set it to UC=0 to eliminate differences between how different CPU vendors are handled in Xen (Intel has the iPAT mechanism, but then that won't be needed anymore).

Copy link
Contributor

@lindig lindig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The patch by itself is simple enough from a code perspective. But I can't review the wider implications.

@edwintorok edwintorok added this pull request to the merge queue Jul 15, 2025
Merged via the queue into xapi-project:master with commit 8378a9d Jul 15, 2025
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants