Re: [Qemu-devel] [PATCH v2 03/34] tests/qgraph: pci-pc driver and interface nodes

[Emanuele <e.emanuelegiuseppe@gmail.com>]

>> +    return TRUE;
>> +}
>> +
>> +QPCIDevice *qpci_device_find(QPCIBus *bus, int devfn)
>> +{
>> +    QPCIDevice *dev;
>> +
>> +    dev = g_malloc0(sizeof(*dev));
>> +
>> +    if (!qpci_device_set(dev, bus, devfn)) {
>>           g_free(dev);
>>           return NULL;
>>       }
>> @@ -66,6 +83,21 @@ QPCIDevice *qpci_device_find(QPCIBus *bus, int devfn)
>>       return dev;
>>   }
>>   
>> +void qpci_device_init(QPCIDevice *dev, QPCIBus *bus, QPCIAddress *addr)
>> +{
>> +    uint16_t vendor_id, device_id;
>> +
>> +    if (!qpci_device_set(dev, bus, addr->devfn)) {
>> +        printf("PCI Device not found\n");
>> +        abort();
>> +    }
> so, here, you should use qpci_device_set() and qpci_device_available()...
I changed qpci_device_set to just set the fields, while 
qpci_device_available simply is doing
{
     return qpci_config_readw(...) != 0xFFFF
}
Now both qpci_device_init and qpci_device_find call qpci_device_set, and 
check that  qpci_device_available is not false, otherwise it will 
trigger g_assert_not_reached().
>
>> +
>> +    vendor_id = qpci_config_readw(dev, PCI_VENDOR_ID);
> or you can only check vendor_id to see it is 0xffff or not...
>
>> +    device_id = qpci_config_readw(dev, PCI_DEVICE_ID);
>> +    g_assert(vendor_id == addr->vendor_id);
> that should be in fact detected by this g_assert().
since this case is covered by qpci_device_available, I don't think 
there's the need to insert the assertion.

Thank you,
Emanuele