|
From: | Chen Fan |
Subject: | Re: [Qemu-devel] [PATCH v2 06/11] pci: add a is_valid_func callback to check device if complete |
Date: | Thu, 10 Mar 2016 10:00:41 +0800 |
User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.3.0 |
On 03/10/2016 01:14 AM, Michael S. Tsirkin wrote:
On Wed, Mar 09, 2016 at 09:50:31AM -0700, Alex Williamson wrote:On Wed, 9 Mar 2016 18:22:24 +0200 "Michael S. Tsirkin" <address@hidden> wrote:On Mon, Mar 07, 2016 at 11:22:59AM +0800, Cao jin wrote:From: Chen Fan <address@hidden> Signed-off-by: Chen Fan <address@hidden>So if you create a mess, you discover it when you later add function 0. Why not call this when function is added? O(N^2) on # of functions, but that # is up to 256 so maybe that is not too bad.Because the configuration isn't valid until the slot is closed. Take a dual port NIC example again, after we add the first function, the configuration is invalid because we have an AER indicated device that can't do a bus reset because the second function, which may be in a separate IOMMU group, hasn't been added yet. Therefore we can only check the configuration when the slot is complete. Thanks, AlexI see. The name mislead me. So what you want to do is validate a multi-function device, not the bus.--- hw/pci/pci.c | 39 +++++++++++++++++++++++++++++++++++++++ include/hw/pci/pci.h | 1 + 2 files changed, 40 insertions(+) diff --git a/hw/pci/pci.c b/hw/pci/pci.c index d940f79..72650c5 100644 --- a/hw/pci/pci.c +++ b/hw/pci/pci.c @@ -1836,6 +1836,31 @@ PCIDevice *pci_find_device(PCIBus *bus, int bus_num, uint8_t devfn) return bus->devices[devfn]; }+static void pci_bus_check_device(PCIDevice *pdev, Error **errp)Is not it true that what you are really after is validating functions of the given device? Pls rename this pci_check_valid_functions or something like this, and change it to only scan functions of the device, not all devices on the bus.
thanks, will do that. Chen
+{ + PCIBus *bus = pdev->bus; + PCIDeviceClass *pc; + int i; + Error *local_err = NULL; + + for (i = 0; i < ARRAY_SIZE(bus->devices); ++i) { + if (!bus->devices[i]) { + continue; + } + + pc = PCI_DEVICE_GET_CLASS(bus->devices[i]); + if (!pc->is_valid_func) { + continue; + } + + pc->is_valid_func(bus->devices[i], &local_err); + if (local_err) { + error_propagate(errp, local_err); + return; + } + } +} + static void pci_qdev_realize(DeviceState *qdev, Error **errp) { PCIDevice *pci_dev = (PCIDevice *)qdev; @@ -1878,6 +1903,20 @@ static void pci_qdev_realize(DeviceState *qdev, Error **errp) pci_qdev_unrealize(DEVICE(pci_dev), NULL); return; } + + /* + * If the function is func 0, indicate the closure of the slot. + * then we get the chance to check all functions on same device + * if valid. + */ + if (pci_get_function_0(pci_dev) == pci_dev) { + pci_bus_check_device(pci_dev, &local_err); + if (local_err) { + error_propagate(errp, local_err); + pci_qdev_unrealize(DEVICE(pci_dev), NULL); + return; + } + } }static void pci_default_realize(PCIDevice *dev, Error **errp)diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h index dedf277..4e56256 100644 --- a/include/hw/pci/pci.h +++ b/include/hw/pci/pci.h @@ -191,6 +191,7 @@ typedef struct PCIDeviceClass {void (*realize)(PCIDevice *dev, Error **errp);int (*init)(PCIDevice *dev);/* TODO convert to realize() and remove */ + void (*is_valid_func)(PCIDevice *dev, Error **errp); PCIUnregisterFunc *exit; PCIConfigReadFunc *config_read; PCIConfigWriteFunc *config_write; -- 1.9.3.
[Prev in Thread] | Current Thread | [Next in Thread] |