Re: [PATCH v2] tools/hvmloader: implement Intel IGD extended VBT support

[Chuck Zmudzinski <brchuckz@xxxxxxx>]

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.

Actually, in hvmloader code, this value of 0xfc for the OpRegion ASLS register
is added to the header file where all the other registers defined by the
PCI spec live: tools/firmware/hvmloader/pci_regs.h, but in that file it is
defined by the PCI_INTEL_OPREGION macro.

So this is another cleanup of this I could do along with this patch: Remove
the define of PCI_INTEL_OPREGION from pci_regs.h (after all, it is not part
of the PCI spec anyways) and pci.c (or the new intel-opregion.c file if some
version of it survives until later versions of this patch) can get the value
for the ASLS register from a header where specs for the IGD OpRegion are
located instead, or it can just be added to config.h where the other IGD related
definitions currently are in hvmloader code.

Chuck