Re: [PATCH v2] tools/hvmloader: implement Intel IGD extended VBT support
Jan Beulich <[email protected]>
| Newsgroups | org.xenproject.lists.xen-devel,org.nongnu.qemu-devel |
|---|---|
| Message-ID | <[email protected]> |
On 20.08.2026 18:53, Chuck Zmudzinski wrote: > On 8/20/2026 11:17 AM, Jan Beulich wrote: >> On 20.08.2026 13:47, Chuck Zmudzinski wrote: >>> On 8/20/2026 3:51 AM, Jan Beulich wrote: >>>> On 19.08.2026 19:13, Chuck Zmudzinski wrote: >>>>> On 8/19/2026 9:51 AM, Jan Beulich wrote: >>>>>> On 19.08.2026 14:36, Chuck Zmudzinski wrote: >>>>>>> On 8/19/2026 8:16 AM, Chuck Zmudzinski wrote: >>>>>>>> Yes, I agree that v3 of the patch to hvmloader should presume that the DM can get >>>>>>>> a copy of the OpRegion and read its contents so most of this can be done in the >>>>>>>> DM instead of in hvmloader. So from hvmloader's perspective, the patch will be more >>>>>>>> about avoiding the layering violation than anything else. >>>>>>> >>>>>>> However, there is one advantage, from the viewpoint of the Xen virtualization platform >>>>>>> as a whole, to do the patching of the OpRegion in hvmloader instead of in the DM. >>>>>>> >>>>>>> If we patch the OpRegion in hvmloader as v2 of this patch does, we provide a common >>>>>>> solution for extended VBT support for Intel IGD devices that would be compatible with >>>>>>> all DM implementations, not just with Qemu. So why not do the patching of the OpRegion >>>>>>> in hvmloader? >>>>>> >>>>>> As indicated before: If the OpRegion holds data that is needed to drive the >>>>>> device, and if the OpRegion is exposed writable to guests, then guest can >>>>>> screw up that data such that subsequent guests won't work anymore. Hence >>>>>> exposing to guests (which includes hvmloader) needs to be stopped, or at >>>>>> least be limited to r/o. That, in fact, includes exposing to any privilege- >>>>>> restricted DM as well. >>>>>> >>>>>> Exposing r/o may be entirely okay (i.e. may not be a layering violation), >>>>>> depending how exactly an OpRegion surfaces for a device (on the host). Aiui >>>>>> it's not addressed by any of the BARs, yet it looks like it needs similar >>>>>> treatment. >>>>> >>>>> Yes, the OpRegion is not one of the BARs as specified by the PCI specs, but >>>>> it functions more or less like a BAR region with the devices's ASLS register >>>>> at offset 0xfc in the PCI device config space of the device acting like the >>>>> BAR for that region. >>>> >>>> That is, on real hardware a write to that register moves the OpRegion? That >>>> would need following by the DM then, i.e. the DM would need to indicate the >>>> original position in the register, and the guest (incl hvmloader) would >>>> then be free to relocate it. >>> >>> Why would that "need following by the DM" when the register in the guest is >>> fully emulated, [1] which means that when the guest (incl hvmloader) writes to the >>> register, the register on the real hardware is not touched, nor is the OpRegion >>> in the host address space moved? >> >> You said it's BAR-like. If the guest writes to a BAR, the referenced MMIO >> region moves accordingly. > > It's BAR-like, but it is not actually a BAR (and the OpRegion is not exactly > an MMIO region either (it is actually and ACPI thing), so that is not relevant > to this patch. > > Also, it is fully emulated so when the guest writes to it, the real register on the > real device is not touched, as I have said multiple times in my responses to your > question. No matter how often you said that, I never put that under question. I was asking about the behavior of writes (where the behavior on bare hardware would need to be reflected in the behavior of the emulated register). >>> Here is how I understand how this works in the current implementation and how >>> this should be done: >> >> I'm sorry, but this is getting out of hand, at least as far as I'm concerned. >> I've been trying to help, but even just reading your replies has already been >> taking way more time than I would have wanted to spend here. > > Fair enough. Thank you for the time you have spent on this patch, and also thank > you for clearly stating that you don't want to spend any more time on it. So > I consider this patch dead unless and until another maintainer shows some interest > in it. I didn't say I would not look at future versions of the patch. However, for me to (usefully) do so, things need to be presented in a way that I can understand without knowing all the details of IGD. Jan