[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [qemu-s390x] [PATCH v1 2/4] s390x/zpci: use hotplug_dev instead of l
From: |
Christian Borntraeger |
Subject: |
Re: [qemu-s390x] [PATCH v1 2/4] s390x/zpci: use hotplug_dev instead of looking up the host bridge |
Date: |
Mon, 5 Nov 2018 12:40:16 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 |
On 11/05/2018 12:37 PM, David Hildenbrand wrote:
> On 05.11.18 12:21, Cornelia Huck wrote:
>> On Mon, 5 Nov 2018 12:03:11 +0100
>> David Hildenbrand <address@hidden> wrote:
>>
>>> We directly have it in our hands.
>>>
>>> Signed-off-by: David Hildenbrand <address@hidden>
>>> ---
>>> hw/s390x/s390-pci-bus.c | 4 ++--
>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
>>> index 1eaae3aca6..68660eac74 100644
>>> --- a/hw/s390x/s390-pci-bus.c
>>> +++ b/hw/s390x/s390-pci-bus.c
>>> @@ -814,9 +814,9 @@ static bool s390_pci_alloc_idx(S390pciState *s,
>>> S390PCIBusDevice *pbdev)
>>> static void s390_pcihost_plug(HotplugHandler *hotplug_dev, DeviceState
>>> *dev,
>>> Error **errp)
>>> {
>>> + S390pciState *s = S390_PCI_HOST_BRIDGE(hotplug_dev);
>>> PCIDevice *pdev = NULL;
>>> S390PCIBusDevice *pbdev = NULL;
>>> - S390pciState *s = s390_get_phb();
>>>
>>> if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_BRIDGE)) {
>>> BusState *bus;
>>> @@ -924,11 +924,11 @@ static void s390_pcihost_timer_cb(void *opaque)
>>> static void s390_pcihost_unplug(HotplugHandler *hotplug_dev, DeviceState
>>> *dev,
>>> Error **errp)
>>> {
>>> + S390pciState *s = S390_PCI_HOST_BRIDGE(hotplug_dev);
>>> PCIDevice *pci_dev = NULL;
>>> PCIBus *bus;
>>> int32_t devfn;
>>> S390PCIBusDevice *pbdev = NULL;
>>> - S390pciState *s = s390_get_phb();
>>>
>>> if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_BRIDGE)) {
>>> error_setg(errp, "PCI bridge hot unplug currently not supported");
>>
>> Not sure whether that is an improvement (s390_get_phb() caches the
>> value, and is called from multiple other places as well.)
>>
>
> Looking up a variable that is directly passed as an argument doesn't
> look clean to me.
I think there was a reason for this caching, namely that qom resolution can
be quite expensive. For the hotplug case this obviously does not matter but
for all the other cases it might. So do we really want to have different
places use different methods?
- Re: [qemu-s390x] [PATCH v1 1/4] s390x/zpci: drop msix.available, (continued)
[qemu-s390x] [PATCH v1 2/4] s390x/zpci: use hotplug_dev instead of looking up the host bridge, David Hildenbrand, 2018/11/05
- Re: [qemu-s390x] [PATCH v1 2/4] s390x/zpci: use hotplug_dev instead of looking up the host bridge, Cornelia Huck, 2018/11/05
- Re: [qemu-s390x] [PATCH v1 2/4] s390x/zpci: use hotplug_dev instead of looking up the host bridge, David Hildenbrand, 2018/11/05
- Re: [qemu-s390x] [PATCH v1 2/4] s390x/zpci: use hotplug_dev instead of looking up the host bridge,
Christian Borntraeger <=
- Re: [qemu-s390x] [PATCH v1 2/4] s390x/zpci: use hotplug_dev instead of looking up the host bridge, David Hildenbrand, 2018/11/05
- Re: [qemu-s390x] [Qemu-devel] [PATCH v1 2/4] s390x/zpci: use hotplug_dev instead of looking up the host bridge, Collin Walling, 2018/11/07
- Re: [qemu-s390x] [Qemu-devel] [PATCH v1 2/4] s390x/zpci: use hotplug_dev instead of looking up the host bridge, Cornelia Huck, 2018/11/08
- Re: [qemu-s390x] [Qemu-devel] [PATCH v1 2/4] s390x/zpci: use hotplug_dev instead of looking up the host bridge, David Hildenbrand, 2018/11/08
Re: [qemu-s390x] [PATCH v1 0/4] s390x/zpci: some hotplug handler cleanups, Cornelia Huck, 2018/11/12