Re: [Qemu-devel] [PATCH v7] s390x/pci: add common function measurement block

[David Hildenbrand <david@redhat.com>]

On 14.01.19 10:18, Pierre Morel wrote:
> On 09/01/2019 12:43, David Hildenbrand wrote:
>> On 09.01.19 12:29, David Hildenbrand wrote:
>>> On 08.01.19 18:37, Pierre Morel wrote:
>>>> From: Yi Min Zhao <zyimin@linux.ibm.com>
>>>>
>>>> Common function measurement block is used to report zPCI internal
>>>> counters of successful pcilg/stg/stb and rpcit instructions to
>>>> a memory location provided by the program.
>>>>
>>>> This patch introduces a new ZpciFmb structure and schedules a timer
>>>> callback to copy the zPCI measures to the FMB in the guest memory
>>>> at an interval time set to 4s.
>>>>
>>>> An error while attemping to update the FMB, would generate an error
>>>> event to the guest.
>>>>
>>>> The pcilg/stg/stb and rpcit interception handlers increase the
>>>> related counter on a successful call.
>>>> The guest shall pass a null FMBA (FMB address) in the FIB (Function
>>>> Information Block) when it issues a Modify PCI Function Control
>>>> instruction to switch off FMB and stop the corresponding timer.
>>>>
>>>> Signed-off-by: Yi Min Zhao <zyimin@linux.ibm.com>
>>>> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
>>>> ---
>>>>   hw/s390x/s390-pci-bus.c  |   4 +-
>>>>   hw/s390x/s390-pci-bus.h  |  29 +++++++++++
>>>>   hw/s390x/s390-pci-inst.c | 133 +++++++++++++++++++++++++++++++++++++++++++++--
>>>>   hw/s390x/s390-pci-inst.h |   1 +
>>>>   4 files changed, 163 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
>>>> index 060ff06..f0d34dd 100644
>>>> --- a/hw/s390x/s390-pci-bus.c
>>>> +++ b/hw/s390x/s390-pci-bus.c
>>>> @@ -989,6 +989,7 @@ static void s390_pcihost_hot_unplug(HotplugHandler *hotplug_dev,
>>>>       bus = pci_get_bus(pci_dev);
>>>>       devfn = pci_dev->devfn;
>>>>       object_unparent(OBJECT(pci_dev));
>>>> +    fmb_timer_free(pbdev);
>>>
>>> I still think this is the wrong place. it has nothing to do with
>>> hotplug/unplug. This belongs into unrealize/finalize.
>>>
>>>
>>
>> ... but I see the issue. It boils down to the bad design of zPCI. The
>> PCI device has no clue about the pbdev. So this has to stay here because
>> in unrealize of pbdev it would be wrong and into unrealize of pci_dev,
>> we can't move it.
>>
> 
> OK, thanks.
> So is it a ack from you?

Acked-by: David Hildenbrand <david@redhat.com>

:)

> 
> regards,
> Pierre
> 


-- 

Thanks,

David / dhildenb