On Tue, Oct 13, 2015 at 05:54:34PM +0800, Cao jin wrote:
> Hi Michael
>
> On 10/13/2015 04:51 PM, Michael S. Tsirkin wrote:
> >On Tue, Oct 13, 2015 at 04:41:35PM +0800, Cao jin wrote:
> >>In case user regret when hot-adding multi-function, should roll back,
> >>device_del the function added but not exposed to the guest.
> >>
> >>Signed-off-by: Cao jin <caoj.f...@cn.fujitsu.com>
> >
> >I think this patch should come first, before we enable the
> >functionality that depends on it.
> >
> Do you mean, the function should be removed individually in any condition?
I just mean the patches in the series should be reordered.
> Because as you know, device_del pci_dev will remove all the functions in the
> slot that are fulled exposed to the guest, Alex also mentioned this
> limitation before.
> >
> >>---
> >> hw/pci/pci_host.c | 6 +++++-
> >> hw/pci/pcie.c | 22 +++++++++++++++++-----
> >> 2 files changed, 22 insertions(+), 6 deletions(-)
> >>
> >>diff --git a/hw/pci/pci_host.c b/hw/pci/pci_host.c
> >>index 3e26f92..35e5cf3 100644
> >>--- a/hw/pci/pci_host.c
> >>+++ b/hw/pci/pci_host.c
> >>@@ -20,6 +20,7 @@
> >>
> >> #include "hw/pci/pci.h"
> >> #include "hw/pci/pci_host.h"
> >>+#include "hw/pci/pci_bus.h"
> >> #include "trace.h"
> >>
> >> /* debug PCI */
> >>@@ -88,10 +89,13 @@ void pci_data_write(PCIBus *s, uint32_t addr, uint32_t
> >>val, int len)
> >> uint32_t pci_data_read(PCIBus *s, uint32_t addr, int len)
> >> {
> >> PCIDevice *pci_dev = pci_dev_find_by_addr(s, addr);
> >>+ PCIDevice *f0 = NULL;
> >> uint32_t config_addr = addr & (PCI_CONFIG_SPACE_SIZE - 1);
> >> uint32_t val;
> >>+ uint8_t slot = (addr >> 11) & 0x1F;
> >>
> >>- if (!pci_dev) {
> >>+ f0 = s->devices[PCI_DEVFN(slot, 0)];
> >>+ if (!pci_dev || (!f0 && pci_dev)) {
> >> return ~0x0;
> >> }
> >>
> >>diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
> >>index 89bf61b..58d2153 100644
> >>--- a/hw/pci/pcie.c
> >>+++ b/hw/pci/pcie.c
> >>@@ -261,13 +261,30 @@ void pcie_cap_slot_hotplug_cb(HotplugHandler
> >>*hotplug_dev, DeviceState *dev,
> >> }
> >> }
> >>
> >>+static void pcie_unplug_device(PCIBus *bus, PCIDevice *dev, void *opaque)
> >>+{
> >>+ object_unparent(OBJECT(dev));
> >>+}
> >>+
> >> void pcie_cap_slot_hot_unplug_request_cb(HotplugHandler *hotplug_dev,
> >> DeviceState *dev, Error **errp)
> >> {
> >> uint8_t *exp_cap;
> >>+ PCIDevice *pci_dev = PCI_DEVICE(dev);
> >>+ PCIBus *bus = pci_dev->bus;
> >>
> >> pcie_cap_slot_hotplug_common(PCI_DEVICE(hotplug_dev), dev, &exp_cap,
> >> errp);
> >>
> >>+ /* In case user regret when hot-adding multi function, remove the
> >>function
> >>+ * that is unexposed to guest individually, without interaction with
> >>guest.
> >>+ */
> >>+ if (PCI_FUNC(pci_dev->devfn) > 0 &&
> >>+ bus->devices[PCI_DEVFN(PCI_SLOT(pci_dev->devfn), 0)] == NULL) {
> >>+ pcie_unplug_device(bus, pci_dev, NULL);
> >>+
> >>+ return;
> >>+ }
> >>+
> >> pcie_cap_slot_push_attention_button(PCI_DEVICE(hotplug_dev));
> >> }
> >>
> >>@@ -378,11 +395,6 @@ void pcie_cap_slot_reset(PCIDevice *dev)
> >> hotplug_event_update_event_status(dev);
> >> }
> >>
> >>-static void pcie_unplug_device(PCIBus *bus, PCIDevice *dev, void *opaque)
> >>-{
> >>- object_unparent(OBJECT(dev));
> >>-}
> >>-
> >> void pcie_cap_slot_write_config(PCIDevice *dev,
> >> uint32_t addr, uint32_t val, int len)
> >> {
> >>--
> >>2.1.0
> >.
> >
>
> --
> Yours Sincerely,
>
> Cao Jin
