Re: [Xen-devel] [vfio-users] [PATCH v3 00/11] igd passthrough chipset tweaks

[Alex Williamson <alex.williamson@xxxxxxxxxx>]

On Fri, 2016-01-29 at 08:09 +0100, Gerd Hoffmann wrote:
> Â Hi,
>Â
> > 1) The OpRegion MemoryRegion is mapped into system_memory through
> > programming of the 0xFC config space register.
> > Âa) vfio-pci could pick an address to do this as it is realized.
> > Âb) SeaBIOS/OVMF could program this.
> >Â
> > Discussion: 1.a) Avoids any BIOS dependency, but vfio-pci would need to
> > pick an address and mark it as e820 reserved.ÂÂI'm not sure how to pick
> > that address.
>Â
> Because of that I'd let the firmware pick the address and program 0xfc
> accordingly, i.e. (b).ÂÂseabios can simply malloc two pages and be done
> with it (any ram allocated by seabios will be tagged as e820 reserved).

Thanks for the tip that seabios allocated pages automatically become
e820 reserved, that simplifies things a bit.

> > 2) Read-only mappings version of 1)
> >Â
> > Discussion: Really nothing changes from the issues above, just prevents
> > any possibility of the guest modifying anything in the host.ÂÂXen
> > apparently allows write access to the host page already.
>Â
> I think read-only is out.ÂÂProbably xen allows write access because
> guest drivers expect they have write access to the opregion, so the
> question is ...
>Â
> > 3) Copy OpRegion contents into buffer and do either 1) or 2) above.
>Â
> whenever we give the guest a copy of the host opregion or direct access.
>Â
> > 4) Copy contents into a guest RAM location, mark it reserved, point to
> > it via 0xFC config as scratch register.
> > Âa) Done by QEMU (vfio-pci)
> > Âb) Done by SeaBIOS/OVMF
> >Â
> > Discussion: This is the most like real hardware.ÂÂ4.a) has the usual
> > issue of how to pick an address, but the benefit of not requiring BIOS
> > changes (simply mark the RAM reserved via existing methods).ÂÂ4.b) would
> > require passing a buffer containing the contents of the OpRegion via
> > fw_cfg and letting the BIOS do the setup.ÂÂThe latter of course requires
> > modifying each BIOS for this support.
>Â
> Maybe we should define the interface as "guest writes 0xfc to pick
> address, qemu takes care to place opregion there".ÂÂThat gives us the
> freedom to change the qemu implementation (either copy host opregion or
> map the host opregion) without breaking things.

Ok, so seabios allocates two pages, writes the base address of those
pages to 0xfc and looks to see whether the signature appears at that
address due to qemu mapping.ÂÂIt verifies the size and does a
free/realloc if not the right size.ÂÂIf the graphics signature does not
appear, free those pages and assume no opregion support.ÂÂIf we later
decide to use a copy, we'd need to disable the 0xfc automagic mapping
and probably pass the data via fw_cfg.ÂÂSound right?

Do guest drivers depend on IGD appearing at 00:02.0?ÂÂI'm currently
testing for any Intel VGA device, but I wonder if I should only be
enabling anything opregion if it also appears at a specific address.

> > Of course none of these support hotplug nor really can they since
> > reserved memory regions are not dynamic in the architecture.
>Â
> igd is chipset graphics and therefore not hotpluggable anyway (on
> physical hardware), I'd be very surprised if the guest drivers are
> prepared to handle hotplug.
>Â
> > Another thing I notice in this series is the access to PCI config space
> > of both the host bridge and the LPC bridge.ÂÂThis prevents unprivileged
> > use cases
>Â
> lpc bridge is no problem, only pci id fields are copied over and
> unprivileged access is allowed for them.
>Â
> Copying the gfx registers of the host bridge is a problem indeed.

I would argue that both are really a problem, libvirt wants to put QEMU
in a container that prevents access to any host system files other than
those explicitly allowed.ÂÂTherefore libvirt needs to grant the process
access to the lpc sysfs config file even though it only needs user
visible register values.

> > Should vfio add
> > additional device specific regions to expose the config space of these
> > other devices?
>Â
> That is an option.ÂÂIt is not clear yet which route we have to take
> though.ÂÂTesting shows that newer linux drivers work fine even without
> igd-passthru=on tweaks, whereas older linux kernels and windows drivers
> don't work even with this series applied and igd-passthru=on.ÂÂI'll go
> look at this as soon as I have test hardware (getting some is wip atm).

Ok, well we certainly don't need to necessarily tie config space of
those two devices together with opregion access, they can be added
later, but we should revisit before we make QEMU grab those config space
values itself, if we can make that functionality add value.ÂÂThanks,

Alex


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel