On Tue, Apr 08, 2025 at 04:11:36PM +0200, Jan Beulich wrote:
> On 08.04.2025 15:23, Marek Marczykowski-Górecki wrote:
> > --- a/tools/firmware/hvmloader/pci.c
> > +++ b/tools/firmware/hvmloader/pci.c
> > @@ -173,6 +173,7 @@ void pci_setup(void)
> > switch ( class )
> > {
> > case 0x0300:
> > + case 0x0380:
> > /* If emulated VGA is found, preserve it as primary VGA. */
> > if ( (vendor_id == 0x1234) && (device_id == 0x1111) )
> > {
>
> Unlike here, where vendor IDs are subsequently checked (and the sole question
> that arises is whether any of the combinations can actually come as Display
> rather than VGA), ...
>
> > --- a/tools/libs/light/libxl_pci.c
> > +++ b/tools/libs/light/libxl_pci.c
> > @@ -2575,7 +2575,8 @@ int libxl__grant_vga_iomem_permission(libxl__gc *gc,
> > const uint32_t domid,
> >
> > if (sysfs_dev_get_class(gc, pci, &pci_device_class))
> > continue;
> > - if (pci_device_class != 0x030000) /* VGA class */
> > + if (pci_device_class != 0x030000 && /* VGA class */
> > + pci_device_class != 0x038000) /* Display class */
> > continue;
>
> ... there's no such checking here, and instead very much VGA-specific things
> are being done then. Is that really in line with permitting Display class
> devices here as well?Well, it was necessary to get IGD passthrough working. But I think upstream Xen still doesn't have all the pieces here. Specifically, Qubes's version includes (a variant of) https://lore.kernel.org/xen-devel/87d74a21bde95cfc7c53fad56bf8f0e47724953e.1592171394.git.gorba...@gmail.com/T/#m8644141760ee36d691434dfaf55031110492929b and that adds here access not only to the video memory, but also vbios, which was needed to get it working. Anyway, this code is reachable only if gfx_passthrou is enabled and currently this option is specific to IGD. -- Best Regards, Marek Marczykowski-Górecki Invisible Things Lab
signature.asc
Description: PGP signature
