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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 16.08.2026 18:38, 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 --
>>>>> +    /*
>>>>> +     * Read the value the device model is initialized with.
>>>>> +     * If the device model supports OpRegion 2, it will
>>>>> +     * return the host IGD OpRegion address. If not, it
>>>>> +     * will return 0. If the device model does not support
>>>>> +     * OpRegion 2, the device model expects us to give it
>>>>> +     * the address to which it will map the OpRegion in the
>>>>> +     * guest and then expects us to do nothing more to setup
>>>>> +     * the OpRegion, so that is all we will do in that case.
>>>>> +     */
>>>>
>>>> Hmm, exposing the host opregion to a guest certainly feels like an issue.
>>>
>>> Well, that is how it is now. I am only retaining it to maintain backward
>>> compatiblily with DM versions that do not support the extended VBT and
>>> OpRegion 2+. My previous comment about backward compatibilty and DM
>>> compatibility also applies here. If we don't worry about that, we can do
>>> away with any cases where we are permanently mapping the host opregion to
>>> the guest and implement this new approach of always exposing a copy of
>>> the OpRegion and VBT to the guest instead.
>>
>> How does "permanently mapping" matter? hvmloader runs inside the guest, so
>> exposure just to copy the data isn't any better in terms of this being a
>> layering violation. The more correct thing to do might be for the DM to
>> put in place a copy before the guest (i.e. hvmloader) even gains control.
>> (How in turn the DM would learn of the contents of the opregion is a
>> separate question then.)
> 
> I am working on v3 of this patch and I want v3 to address this problem of a
> "layering violation" that you mentioned here, but I do not understand exactly
> what you mean. Do you mean to say that the current code we have in place and
> have had in place for over the past 10 years [1] in the Qemu DM that traps and
> maps the OpRegion into the guest is a "layering violation?"
> 
> [1] https://xenbits.xen.org/gitweb/?p=qemu-xen.git;a=commitdiff;h=5cec8aa38cc
> ("xen, gfx passthrough: add opregion mapping")

All I can say is that this at least smells like a layering violation. It maybe
wouldn't have if, in your patch, you didn't demonstrate that the machine page
range doesn't really need mapping, as copying the data and providing that to
the guest is sufficient. In such a case, the machine range should (imo) never
have been exposed. After all the guest then can fiddle with it, potentially
breaking later guests that are to also use the region.

>>>>> +    /*
>>>>> +     * Write rvda_host as 2 successive 32-bit values
>>>>> +     * to communicate location of the VBT to the device
>>>>> +     * model. If rvda_host is not 0, The device model
>>>>> +     * unmaps the OpRegion and eventually maps the VBT
>>>>> +     * after we also write the guest address where the
>>>>> +     * VBT will be mapped.
>>>>> +     *
>>>>> +     * If we send rvda_host = 0 to the device model, it
>>>>> +     * will assume we do not need OpRegion 2 support and
>>>>> +     * it will not unmap the OpRegion.
>>>>> +     */
>>>>> +    pci_writel(vga_devfn, PCI_INTEL_OPREGION,
>>>>> +               (uint32_t)(rvda_host & 0xfffffffful));
>>>>> +    unsigned long rvda_host_upper_32 = (uint64_t)rvda_host >> 32;
>>>>> +    pci_writel(vga_devfn, PCI_INTEL_OPREGION,
>>>>> +               (uint32_t)rvda_host_upper_32);
>>>>
>>>> Why would you need to communicate a host property to the DM?
>>>
>>> The DM cannot access the host rvda value because it is only accessible
>>> from the host kernel, and the DM is only a user-space process on the host.
>>
>> I don't follow this: Anything the guest can access should also be accessible
>> by its DM.
> 
> I also don't follow your comment here so permit me to comment and ask some
> questions for clarification.
> 
> I was thinking it is enough for the domain the DM is running in to have
> access to the resource for it to be legitimate for the DM to map the resource
> into the guest. So I also think that whether or not the DM itself can access
> the resource is irrelevant to the question. But you seem to be saying, no,
> that is not enough, the DM itself should be able to access the resource
> before it can be allowed to map the resource to its guest. Is that what you
> are saying?

That depends on what you mean by "access": The prereq is that the DM have
permission to access the pages. It may not have an active mapping thereof.

> Perhaps your comment here is related to the concept of a "layering violation"
> mentioned above. Are you saying it is a layering violation for the DM to
> map an MMIO resource to its guest unless it actually has access to that
> resource? If so, what kind of access to those device resources should the
> DM have? Read access? Read/Write access?

No, I'm trying to bring across that (as said above) access to machine pages
should not be granted when that isn't necessary. As in here: A copy of the
pages looks to suffice, so simply give the guest access to a copy.

> If the specs only say the DM "should" have access to the resources it maps
> into its guest, then I would think it would not be a layering violation.
> But if the specs say the DM "must" have access before it asks the hypervisor
> to map the resource to the guest, then I would admit that yes, we have a
> layering violation because the DM is mapping the OpRegion to the guest
> even though it does not have access to the OpRegion.
> 
> So, where are the specs for what the DM can and cannot do? Are they publicly
> available, or are they proprietary or only available to members of the Linux
> Foundation and/or the Xen Project?

Sadly the source code (of Xen and/or qemu) is the spec.

Jan