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

[Chuck Zmudzinski <brchuckz@xxxxxxx>]

On 8/14/2026 12:18 PM, Chuck Zmudzinski wrote:
> On 8/14/2026 11:23 AM, Chuck Zmudzinski wrote:
>> On 8/14/2026 9:46 AM, Jan Beulich wrote:
>>> On 14.08.2026 15:18, Chuck Zmudzinski wrote:
>>>> On 8/14/2026 3:35 AM, Jan Beulich wrote:
>>>>> On 14.08.2026 02:45, Chuck Zmudzinski wrote:
>>>>>> On 8/13/2026 6:35 AM, Jan Beulich wrote:
>>>>>>> On 02.08.2026 07:08, Chuck Zmudzinski wrote:
>>>>>>>> -- snip --
>>>>>
>>>>> I don't follow this: Anything the guest can access should also be 
>>>>> accessible
>>>>> by its DM.
>>>> 
>>>> I think the host OpRegion is not currently accessible by the DM.
>>> 
>>> Can you explain to me how the region becomes accessible to the guest?
>>> That would then (hopefully) help me understand why the DM would not have
>>> access. Fundamentally any MMIO and any I/O ports that are assigned to a
>>> guest are also assigned to its DM.
>> 
>> Currently, in the device model (Qemu) we have:
>> 
>>     ret = xc_domain_memory_mapping(xen_xc, xen_domid,
>>             (unsigned long)(igd_guest_opregion >> XC_PAGE_SHIFT),
>>             (unsigned long)(igd_host_opregion >> XC_PAGE_SHIFT),
>>             XEN_PCI_INTEL_OPREGION_PAGES,
>>             DPCI_ADD_MAPPING);
>> 
>> That statement is in the igd_write_opregion(...) function in the
>> hw/xen/xen_pt_graphics.c file of the upstream Qemu source.
> 
> I forgot to mention: In our current implementation, this statement is
> executed in the DM when hvmloader executes this statement, currently in
> hvmloader/pci:
> 
>                     pci_writel(vga_devfn, PCI_INTEL_OPREGION,
>                                igd_opregion_pgbase << PAGE_SHIFT);
> 
> 
> 
>> 
>> If I understand our current implementation correctly, this statement
>> is what gives the guest access to the host OpRegion (3 pages as defined
>> by XEN_PCI_INTEL_OPREGION_PAGES, and in agreement with IGD_OPREGION_PAGES
>> in hvmloader code). I don't think this statement makes the host OpRegion
>> accessible to the device model, though, so I think, if I understand your
>> comment in an earlier about my patch resulting in what you called a "layering
>> violation" correctly, that our current implementation is also guilty of this
>> same kind of "layering violation."
>> 
>> So, how do you suggest we fix that?

Well, that is a difficult question to answer, and if no one gives an answer
then I ask, what is the harm in making the unorthodox mapping of the OpRegion
from the host to the guest temporary for the purpose of allowing hvmloader
to setup the OpRegion properly for newer devices with new and updated specs
for the OpRegion and VBT when our current implementation permanently maps
the host OpRegion into the guest in the same unorthodox way also, that is,
without following the normal PCI MMIO interfaces?

I think the fundamental problem is the fact that the Intel IGD is an
unorthodox PCI device that does not follow the normal PCI specs and
requires adherence to Intel's proprietary specs instead.

Would that be a fair description of your problem with this patch? Are
the unorthodox requirements of the Intel IGD at the root of your issue
with this patch?

I think the reason this was allowed in the Xen codebase many years ago, I think
over 10 years ago now, is simply because the Intel IGD is such an ubiquitous
device that an exception for it was allowed.

So, to summarize what I am being asked to do in this thread, I propose the
next version of this patch should:

1. Fix style problems in this version.
2. provide a public header to define two protocols for providing
   Intel IGD support via interaction between the DM and hvmloader.
   The first protocol is the legacy protocol version, and it
   is the version that our current implementation follows. The second
   version is the new proposed protocol that is able to allow
   support for an extended VBT, which is required for newer Intel
   IGD devices.

3. For now, since only hvmloader currently has access to the host
   OpRegion in both our current implementation and the proposed new
   protocol, hvmloader will drive the decision about which protocol
   version to use for setting up the guest OpRegion. First, if the
   device model lacks support for the new protocol proposed here that
   supports the extended VBT, then hvmloader has no choice but to
   implement the current legacy protocol. Even in that case, instead
   of just printing a scary or confusing message about lack of support
   for extended VBT and continuing, which is what this version of this
   patch does, we can read the OpRegion and then print an error message
   and BUG() (or just a WARN?) only in the case when extended VBT
   support is needed for this hardware but such support is not available
   in the device model. The message could say something like:

   IGD: error: This device requires extended VBT support in the device model.
   Please upgrade the device model to a version with extended VBT support
   and try again.

   If the device does not require extended VBT support, we silently continue
   and can expect the guest will operate correctly if all else is also good.

   Now for the case when the device model does support extended VBT but the
   device is a legacy device that does not need an extended VBT. In that
   case, I think it is better to, instead of implementing the current
   legacy protocol which unconditionally maps 3 host pages into the guest
   when only 2 pages are actually needed, so an extra page from the host
   of unknown content is being exposed to the guest, we implement the new
   protocol proposed here that will reserve only two pages for the OpRegion
   in the E820 map and use a copy of the two-page OpRegion in the guest instead.
   This will be a change from this v2 of this patch which just uses the 
three-page
   mapped region in this case.

   Then there is the fourth case when the device model supports extended VBT
   and the device needs such support.

   To understand the approach to this problem that I have implemented in this
   patch and plan to implement in future versions until a better alternative
   is proposed, please refer to these Linux kernel commits which added support
   for extended VBT for KVM/vfio guests and which explain why this patch is
   needed for the newer Intel IGD devices that need an extended VBT:

   git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 
bab2c1990b78 ("vfio/pci: Add support for opregion v2.1+")
   git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 
49ba1a2976c8 ("vfio/pci: Add OpRegion 2.0+ Extended VBT support.")

   So, in this case, we have to implement some means for exposing both the
   OpRegion and the VBT to the guest, and we may need to also modify the
   OpRegion in some cases. Specifically, the value of the rvda field in the
   OpRegion needs to be modified in at least two cases:

   A) Host OpRegion version is 2.0. In this case, rvda is the absolute address
      of the VBT and will need to have a different value in the guest than its
      value in the host.

   B) OpRegion version is 2.1 or higher. In this case, rvda is the VBT address
      relative to the OpRegion base but if our memory map does not allow us to
      maintain the same relative offset of the VBT from the OpRegion base on the
      host, rvda will need to have a different value in the guest than its value
      in the host.
     
   For now, until a better way is proposed to expose the OpRegion and VBT to the
   guest in a way that allows the guest OpRegion to be modified as described 
above,
   I plan to propose the same approach of temporarily mapping the host IGD 
OpRegion
   and VBT so that hvmloader can obtain a copy of each region and configure the
   OpRegion and VBT appropriately for the guest that I have use in this patch,
   despite Jan's objections which, as far as I can tell, also apply to our 
current
   implementation.

Thanks,

Chuck