Re: [Qemu-devel] [PATCH 2/2] PCI-e device multi-function hot-add support

[Cao jin]

Hi Igor,
    sorry I missed you mail.

On 09/11/2015 10:35 PM, Igor Mammedov wrote:
> On Thu, 10 Sep 2015 20:12:23 +0800
> Cao jin <address@hidden> wrote:
>
> > In case user regret when hot-add multi-function, we should roll back,
> > device_del the function added but still not worked.
> >
> > Signed-off-by: Cao jin <address@hidden>
> > ---
> >   hw/pci/pcie.c | 28 +++++++++++++++++++++++-----
> >   1 file changed, 23 insertions(+), 5 deletions(-)
> >
> > diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
> > index 61ebefd..b83a244 100644
> > --- a/hw/pci/pcie.c
> > +++ b/hw/pci/pcie.c
> > @@ -265,10 +265,33 @@ 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;
> > +
> > +    /* handle the condition: user want to hot-add multi function, but regret
> > +     * before finish it, and want to delete the added but not worked function.
> > +     */
> sorry, I couldn't parse this comment and commit message as well. Please 
> rephrase it.

Ok, I send v2 before saw your mail, sorry. Maybe I can rephrase it in v3.

> > +    if (PCI_FUNC(pci_dev->devfn) > 0 &&
> > +        bus->devices[PCI_DEVFN(0,0)] == NULL) {
> > +        pci_for_each_device(bus, pci_bus_num(bus),
> > +                            pcie_unplug_device, NULL);
> *_unplug_request_cb is a way to communicate to guest that it should free
> and eject device. So you are not allowed to destroy device from this path,
> it's upto guest to decide what do on this request.
>
> Look where pcie_unplug_device() is actually used, that's the place where
> guest voluntary ejects device.

Agree. In v2 patch, I changed the delete process, fake the condition, 
let the Guest voluntary ejects device

> > +
> > +        pci_word_test_and_clear_mask(exp_cap + PCI_EXP_SLTSTA,
> > +                                     PCI_EXP_SLTSTA_PDS);
> > +        pci_word_test_and_set_mask(exp_cap + PCI_EXP_SLTSTA,
> > +                                   PCI_EXP_SLTSTA_PDC);
> > +
> > +        return;
> > +    }
> >
> >       pcie_cap_slot_hotplug_common(PCI_DEVICE(hotplug_dev), dev, &exp_cap, 
> > errp);
> >
> > @@ -382,11 +405,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)
> >   {
>
> .

--
Yours Sincerely,

Cao Jin