Re: [Qemu-devel] [Xen-devel] [PATCH 2/2] xen:i386:pc_piix: create isa bridge specific to IGD passthrough

["Chen, Tiejun" <tiejun.chen@intel.com>]

On 2014/8/22 0:16, Konrad Rzeszutek Wilk wrote:
> On Thu, Aug 21, 2014 at 09:28:28AM +0800, Tiejun Chen wrote:
>> Currenjly this ISA bridge should be fixed at 1f.0, and pass the
>
> Currently

Fixed.

>
>> real vendor/device ids as the driver expect.
>
> Could you add a bit more description to this patch please? Explain
> the rationale, etc.

So rephrase as follows:

xen:i386:pc_piix: create isa bridge specific to IGD passthrough

Currently IGD drivers always need to access PCH by 1f.0, and
identify PCH type with its own real vendor/device ids. This type
value help driver initialize correctly.

>>
>> Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
>> ---
>>   hw/i386/pc_piix.c | 24 +++++++++++++++++++++++-
>>   1 file changed, 23 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
>> index 7710724..b131fa3 100644
>> --- a/hw/i386/pc_piix.c
>> +++ b/hw/i386/pc_piix.c
>> @@ -50,7 +50,8 @@
>>   #include "cpu.h"
>>   #include "qemu/error-report.h"
>>   #ifdef CONFIG_XEN
>> -#  include <xen/hvm/hvm_info_table.h>
>> +#include <xen/hvm/hvm_info_table.h>
>> +#include <hw/xen/xen-host-pci-device.h>
>>   #endif
>>
>>   #define MAX_IDE_BUS 2
>> @@ -463,6 +464,26 @@ static void pc_xen_hvm_init(MachineState *machine)
>>       }
>>   }
>>
>> +static void xen_igd_passthrough_isa_bridge_create(PCIBus *bus)
>> +{
>> +    struct PCIDevice *dev;
>> +    XenHostPCIDevice hdev;
>> +    int r = 0;
>> +
>> +    /* This shoudl be fixed at 1f.0 then pass vendor/device ids.
>
> should
>
> However I would remove the comment as it does not add anything extra
> to the function. It is pretty clear what it is doing.
>
> What would help is if you said:
>
> Must be fixed at 1f.0 because .. bla blah

Like the patch description, so what about this,

     /* Currently IGD drivers always need to access PCH by 1f.0, and
      * identify PCH type with its own real vendor/device ids.
      */

Thanks
Tiejun

>
>> +     */
>> +    dev = pci_create_simple(bus, PCI_DEVFN(0x1f, 0),
>> +                            "xen-igd-passthrough-isa-bridge");
>> +    if (dev) {
>> +        r = xen_host_pci_device_get(&hdev, 0, 0, PCI_DEVFN(0x1f, 0), 0);
>> +        if (!r) {
>> +            pci_config_set_vendor_id(dev->config, hdev.vendor_id);
>> +            pci_config_set_device_id(dev->config, hdev.device_id);
>> +        } else
>> +            fprintf(stderr, "xen set xen-igd-passthrough-isa-bridge failed\n");
>> +    }
>> +}
>> +
>>   static void xen_igd_passthrough_pc_hvm_init(MachineState *machine)
>>   {
>>       PCIBus *bus;
>> @@ -472,6 +493,7 @@ static void xen_igd_passthrough_pc_hvm_init(MachineState *machine)
>>       bus = pci_find_primary_bus();
>>       if (bus != NULL) {
>>           pci_create_simple(bus, -1, "xen-platform");
>> +        xen_igd_passthrough_isa_bridge_create(bus);
>>       }
>>   }
>>   #endif
>> --
>> 1.9.1
>>
>>
>> _______________________________________________
>> Xen-devel mailing list
>> Xen-devel@lists.xen.org
>> http://lists.xen.org/xen-devel
>