Re: [XEN PATCH] tools/libs/light/libxl_pci.c: explicitly grant access to Intel IGD opregion

[Chuck Zmudzinski <brchuckz@xxxxxxxxxxxx>]

On 3/31/22 2:32 PM, Chuck Zmudzinski wrote:
> On 3/30/22 1:15 PM, Anthony PERARD wrote:
> > Hi Chuck,
> >
> > On Sun, Mar 13, 2022 at 11:41:37PM -0400, Chuck Zmudzinski wrote:
> > > When gfx_passthru is enabled for the Intel IGD, hvmloader maps the IGD
> > > opregion to the guest but libxl does not grant the guest permission to
> > I'm not reading the same thing when looking at code in hvmloader. It
> > seems that hvmloader allocate some memory for the IGD opregion rather
> > than mapping it.
> >
> >
> > tools/firmware/hvmloader/pci.c:184
> >      if ( vendor_id == 0x8086 )
> >      {
> >          igd_opregion_pgbase = mem_hole_alloc(IGD_OPREGION_PAGES);
> >          /*
> >           * Write the the OpRegion offset to give the opregion
> >           * address to the device model. The device model will trap
> >           * and map the OpRegion at the give address.
> >           */
> >          pci_writel(vga_devfn, PCI_INTEL_OPREGION,
> >                     igd_opregion_pgbase << PAGE_SHIFT);
> >      }
> >
> > I think this write would go through QEMU, does it do something with it?
> > (I kind of replying to this question at the end of the mail.)
> >
> > Is this code in hvmloader actually run in your case?
> Hi Anthony,
> ...
> So the conclusion is that hvmloader does allocate the three pages in the
> guest for the opregion and writes a value for the opregion address, but
> it appears it is overwritten later with the error value when the guest
> cannot access the opregion and with the correct value when the guest can
> access the opregion.
>
> So I agree that I should understand what is going on here better. I
> tentatively think the call to pci_writel in hvmloader can be safely
> removed because that value seems to be changed later on somewhere.
After discovering how the device model recovers the offset
of the opregion from the page boundary, I am now certain
that what we currently have in hvmloader is necessary. We
do need to call pci_writel in hvmolader because that is where
we write the mapped value of the page-aligned address of
the opregion in the guest, and then the device model reads
that value, recovers the offset of the opregion to the page
boundary, and writes the address of the beginning of the
opregion, not the page-aligned address that hvmloader wrote,
into the config attribute of the Intel IGD that is passed
through to the guest. That is why it seemed to me that the
address was changed somewhere. The device model modifies
it so it is the actual address of the opregion and not the address
of the page boundary that immediately precedes the opregion.
I hope this is an acceptable explanation of what we currently
have in hvmloader.

Regards,

Chuck