qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Emanuele
Subject: Re: [Qemu-devel] [PATCH v2 03/34] tests/qgraph: pci-pc driver and interface nodes
Date: Thu, 9 Aug 2018 14:33:29 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1



On 08/09/2018 02:29 PM, Laurent Vivier wrote:
On 09/08/2018 14:17, Emanuele wrote:
+    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.
What I mean is you don't need the qpci_device_available().

The 0xffff case is detected by "g_assert(vendor_id == addr->vendor_id)".

The functions could be:

static void qpci_device_set(QPCIDevice *dev, QPCIBus *bus, int devfn)
{
      dev->bus = bus;
      dev->devfn = devfn;
}

void qpci_device_init(QPCIDevice *dev, QPCIBus *bus, QPCIAddress *addr)
{
     uint16_t vendor_id, device_id;

     qpci_device_set(dev, bus, addr->devfn);

     vendor_id = qpci_config_readw(dev, PCI_VENDOR_ID);
     device_id = qpci_config_readw(dev, PCI_DEVICE_ID);
     g_assert(vendor_id == addr->vendor_id);
     g_assert(device_id == addr->device_id);
}
Ok, makes sense for qpci_device_init, but I still need to perform that check for qpci_device_find.
Emanuele



reply via email to

[Prev in Thread] Current Thread [Next in Thread]