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

[Chuck Zmudzinski <brchuckz@xxxxxxxxxxxx>]

On 3/30/22 1:27 PM, Andrew Cooper wrote:
> On 30/03/2022 18:15, Anthony PERARD wrote:
> > Some more though on that, looking at QEMU, it seems that there's already
> > a call to xc_domain_iomem_permission(), in igd_write_opregion().
> This has been discussed before, but noone's done anything about it.
> It's a massive layering violation for QEMU to issue
> xc_domain_iomem_permission()/etc hypercalls.
>
> It should be the toolstack, and only the toolstack, which makes
> permissions hypercalls, which in turn will fix a slew of "QEMU doesn't
> work when it doesn't have dom0 superpowers" bugs with stubdomains.
How much say does the Xen project have over the code
in Qemu under hw/xen? I would not be against having libxl
do the permissions hypercalls in this case instead of Qemu
doing it. My test with Qemu traditional and this patch proves
the permission can be granted by libxl instead of the device
model.
> In this case specifically, the opregion is a magic Intel graphics
> specific bodge.  The i915 driver in the guest really does need to access
> part of the real PCH during init, which (in Xen's permission model)
> really does require permitting access to the MMIO range (8k iirc) so it
> can be mapped as a BAR in QEMU's emulated PCH.
That is exactly what my testing confirmed, but in my
tests only Linux guests need access to the magic Intel
opregion. The proprietary Windows Intel graphics
drivers are apparently able to work around lack of
access to the opregion, at least on my system, and
Windows guests with Intel IGD passthrough function
very well without needing this patch. So the problem
could be fixed in the Linux i915 kernel driver, but Intel
has not contributed the magic sauce to the Linux kernel
that would enable Linux guests to work without access
to the Intel opregion.

And you are correct, the opregion is 8k (2 pages) in size,
but it is not always page aligned, as mentioned earlier,
so the IGD_OPREGION_PAGES constant is set to 3 instead
of 2.

Chuck