Skip to content

Conversation

@AnthoineB
Copy link
Contributor

This patch add a new parameter named 'xen-platform-pci-bar-uc' 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.

This changeset is link to this xen commit:
x86/hvmloader: select xen platform pci MMIO BAR UC or WB MTRR cache attribute

@AnthoineB AnthoineB force-pushed the AMD-xen-pci-MMIO-cache branch from 34242ed to f441583 Compare June 25, 2025 15:34
~allowed_roles:_R_VM_ADMIN ~doc_tags:[Windows] ()

let set_xen_platform_pci_bar_uc =
call ~name:"set_xen_platform_pci_bar_uc" ~lifecycle: []
Copy link
Member

Choose a reason for hiding this comment

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

I think this is too low level of a setting to be exposed as-is from xapi.

What's preventing the setting to be simply always disabled so the perf improvement is always present?

  • The version of xen? xenopsd should be aware of the xen version in use and select the option.
  • The guest kernel? Then a field in the VM object should be added (no need for new calls to set it) (maybe Hvm_mmio_bar)
  • ...

Copy link
Contributor

Choose a reason for hiding this comment

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

This is enabled by default in new versions of hvmloader. Xapi change is to allow disabling this optimization in case something is wrong

Copy link
Contributor

Choose a reason for hiding this comment

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

I tend to agree with @psafont that this should not be exposed in the VM data model. Otherwise all kind of low-level and Xen-specific parameters end up here.

Copy link
Member

Choose a reason for hiding this comment

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

This is enabled by default in new versions of hvmloader. Xapi change is to allow disabling this optimization in case something is wrong.

Then I'm not convinced this setting should be per-vm, nor that it should be exposed in the API.

In first instance, if there's a problem, it should be treated as a bug bug. If we want to have a workaround / failsafe while the issue is being fixed and release, a configuration option in xenopsd.conf should be enough to enable a workaround for the bug

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for the feedbacks. I will close this PR to work in this new direction. I will create a new PR with the changes on xenopsd only.

@last-genius
Copy link
Contributor

@robhoes suggested yesterday that perhaps this might be too low-level for a separate call and it might be worth putting this option into platform, which is undocumented but still available if something needs toggling. The only difficulty I could see is that unlike the rest of platformdata, which is written into ~/platform, this particular key is expected by the upstream in ~/hvmloader/pci/xen-platform-pci-bar-uc, which would need some additional handling

This patch add a new parameter named 'xen-platform-pci-bar-uc' 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 AMD-xen-pci-MMIO-cache branch from 4581df5 to d9f43a3 Compare June 26, 2025 08:54
@AnthoineB AnthoineB closed this Jun 26, 2025
@AnthoineB AnthoineB deleted the AMD-xen-pci-MMIO-cache branch July 2, 2025 12:02
github-merge-queue bot pushed a commit that referenced this pull request Jul 15, 2025
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. 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
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