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

[Cao jin <caoj.fnst@cn.fujitsu.com>]

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 <caoj.fnst@cn.fujitsu.com> 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 <caoj.fnst@cn.fujitsu.com>
>> ---
>>   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