Re: [Qemu-devel] [v4][PATCH 2/5] xen, gfx passthrough: create intel isa bridge

[Paolo Bonzini <pbonzini@redhat.com>]

Il 06/06/2014 05:06, Zhang, Yang Z ha scritto:
> Paolo Bonzini wrote on 2014-06-03:
>> Il 30/05/2014 10:59, Tiejun Chen ha scritto:
>>> +static int create_pch_isa_bridge(PCIBus *bus, XenHostPCIDevice *hdev)
>>> +{
>>> +    struct PCIDevice *dev;
>>> +
>>> +    char rid;
>>> +
>>> +    dev = pci_create(bus, PCI_DEVFN(0x1f, 0), "intel-pch-isa-bridge");
>>
>> This is really a huge hack.  You're going to have two ISA bridge devices
>> in the machine, with the BIOS imagining that the "good" one is at 1f.0
>
> Definitely. So how about expose a fake device at 00:1f.0 but not Isa Bridge? I have discussion with gfx driver developer, it is ok for them to check the device on 00:1f.0 no matter what device it is.

That would be slightly better.

>> and the ACPI tables actually describing the other one.  But the PCI
>> device at 1f.0 remains there and a driver in the OS could catch it---not
>> just intel_detect_pch---and if you want to add such a hack it should be
>> done in the Xen management layers.
>>
>> If possible, the host bridge patches are even worse.  If you change the
>> vendor and device ID while keeping the registers of the i440FX you're
>> going to get conflicts or break firmware badly; TianoCore and SeaBIOS
>> both expect the normal i440FX vendor and device IDs, for example.
>
> I only see the class id is changed but not vendor and device id.

Yes, and the class ID is a typo probably.

But when the guest reads the vendor and device ID, igd_pci_read passes 
it through.  So effectively it changes, if I read the code correctly.

Paolo

>>
>> The hardcoded list of offsets is also not acceptable.  It is also not
>> clear who is accessing the registers, whether the BIOS or the driver.
>> For Linux, a cursory look at the driver shows that it only accesses
>> 0x50/0x52 of the listed offsets, but also 0x44/0x48 ("MCH BAR"), what
>> happens if that code path is encountered?
>
> Will have a double check.
>
>>
>> The main problem with IGD passthrough is the incestuous (and that's a
>> euphemism) relationship between the MCH, PCH and graphics driver.  It
>> may make sense at the hardware level, but for virtualization it doesn't.
>>   A virt-specific driver for GPU command passthrough (with aid from the
>> kernel driver, but abstracting all the MCH/PCH-dependent details) would
>> make much more sense.
>
> Agree. But it is too hard.
>
>>
>> It's really not your fault, there's not much you can do given the
>> hardware architecture.  But I don't think this code can be accepted
>> upstream, sorry.
>>
>> Paolo
>
>
> Best regards,
> Yang
>
>
>
>