Re: [Qemu-devel] [PATCH v3 2/2] remove function during multi-function hot-add

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

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.fnst@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? 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