Re: [Qemu-devel] [v5][PATCH 0/5] xen: add Intel IGD passthrough support

[Michael S. Tsirkin]

On Wed, Jun 25, 2014 at 05:21:04PM +0800, Chen, Tiejun wrote:
> On 2014/6/25 17:09, Michael S. Tsirkin wrote:
> >On Wed, Jun 25, 2014 at 04:55:25PM +0800, Chen, Tiejun wrote:
> >>On 2014/6/25 16:48, Michael S. Tsirkin wrote:
> >>>On Wed, Jun 25, 2014 at 10:39:43AM +0200, Paolo Bonzini wrote:
> >>>>Il 25/06/2014 10:31, Michael S. Tsirkin ha scritto:
> >>>>>It might be possible to move the Q35 bridge elsewhere.
> >>>>>seabios doesn't care where the host bridge is.
> >>>>>ACPI tables in QEMU can be adjusted.
> >>>>
> >>>>But why?  It's always in 00:1f.0 on real hardware.  If the i915 driver 
> >>>>wants
> >>>>to run under virtual machines, it should stop acting as if it knows what 
> >>>>the
> >>>>whole machine looks like.
> >>>>
> >>>>Paolo
> >>>
> >>>If guest driver can be fixed that seems ideal.
> >>>I don't know how it works but presumably you guys do?
> >>
> >>Paolo prefer we need to fix this in the driver like:
> >>
> >>"
> >>>>>The right way could be to make QEMU add a vendor-specific capability to
> >>>>>the video device. The driver can probe for that capability before
> >>>>
> >>>>Do you mean we can pick two unused offsets in the configuration space of
> >>>>the video device as a vendor-specific capability to hold the
> >>>>vendor/device ids of the PCH?
> >>>
> >>>Yes, either that or add a new capability (which lets you choose the
> >>>offsets more freely).
> >>>
> >>>If the IGD driver needs config space fields of the MCH, those fields
> >>>could also be mirrored in the new capability.  QEMU would forward them
> >>>automatically.
> >>>
> >>>It could even be a new BAR, which gives even more freedom to allocate
> >>>the fields.
> >>"
> >>
> >>Thanks
> >>Tiejun
> >
> >Adding a vendor-specific capability or BAR
> >in an existing device is painful - hard to find
> >free space for it.
> 
> Yes, this is a potential risk as well since we can't guarantee current free
> space is always valid for ever.
> 
> >We could add a dummy device in PCI or ACPI, but
> >driver should really look for it using device/vendor id,
> >not at a specific address.
> >What exactly is it that the driver wants to know?
> >
> 
> The i915 driver need to use the vendor/device ids to get what pch type is
> running, then go different paths.
> 
> Thanks
> Tiejun

That's all? Just the IDs? You seem to emulate a bunch of other registers, why?
If that's all, a clean way would be to set subsystem vendor ID to be one
specific to Xen, set subsystem device id to whatever you want.  Get an extra
virtual vendor ID for intel from pci sig, or ask citrix to get one.


However, looking at drivers/gpu/drm/i915/i915_drv.c,
I don't see it poking at a specific address:


         * The reason to probe ISA bridge instead of Dev31:Fun0 is to
         * make graphics device passthrough work easy for VMM, that only
         * need to expose ISA bridge to let driver know the real hardware
         * underneath. This is a requirement from virtualization team.
         *
         * In some virtualized environments (e.g. XEN), there is irrelevant
         * ISA bridge in the system. To work reliably, we should scan trhough
         * all the ISA bridge devices and check for the first match, instead
         * of only checking the first one.
         */
        while ((pch = pci_get_class(PCI_CLASS_BRIDGE_ISA << 8, pch))) {


while this is quite ugly as an extra bridge might
confuse non linux guests, as a shortcut I don't see
why you can't simply stick your fake bridge at
a random address.


-- 
MST