Re: [Qemu-devel] [PATCH 4/4] s390x/pci: add iommu replay callback

[Yi Min Zhao <zyimin@linux.vnet.ibm.com>]

在 2017/8/29 下午5:33, Cornelia Huck 写道:
> On Tue, 29 Aug 2017 16:26:10 +0800
> Yi Min Zhao <zyimin@linux.vnet.ibm.com> wrote:
>
>> 在 2017/8/29 下午4:07, Cornelia Huck 写道:
>>> [Restored cc:s. Please remember to do reply-all.]
>>>
>>> On Tue, 29 Aug 2017 12:46:51 +0800
>>> Yi Min Zhao <zyimin@linux.vnet.ibm.com> wrote:
>>>   
>>>> 在 2017/8/28 下午11:57, Cornelia Huck 写道:
>>>>> On Mon, 28 Aug 2017 10:04:47 +0200
>>>>> Yi Min Zhao <zyimin@linux.vnet.ibm.com> wrote:
>>>>>   
>>>>>> Let's introduce iommu replay callback for s390 pci iommu memory region.
>>>>>> Currently we don't need any dma mapping replay. So let it return
>>>>>> directly. This implementation will avoid meaningless loops calling
>>>>>> translation callback.
>>>>>>
>>>>>> Reviewed-by: Pierre Morel <pmorel@linux.vnet.ibm.com>
>>>>>> Reviewed-by: Halil Pasic <pasic@linux.vnet.ibm.com>
>>>>>> Signed-off-by: Yi Min Zhao <zyimin@linux.vnet.ibm.com>
>>>>>> ---
>>>>>>     hw/s390x/s390-pci-bus.c | 8 ++++++++
>>>>>>     1 file changed, 8 insertions(+)
>>>>>>
>>>>>> diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
>>>>>> index 9e1f7ff5c5..359509ccea 100644
>>>>>> --- a/hw/s390x/s390-pci-bus.c
>>>>>> +++ b/hw/s390x/s390-pci-bus.c
>>>>>> @@ -407,6 +407,13 @@ static IOMMUTLBEntry s390_translate_iommu(IOMMUMemoryRegion *mr, hwaddr addr,
>>>>>>         return ret;
>>>>>>     }
>>>>>>     
>>>>>> +static void s390_pci_iommu_replay(IOMMUMemoryRegion *iommu,
>>>>>> +                                  IOMMUNotifier *notifier)
>>>>>> +{
>>>>>> +    /* we don't need iommu replay currently */
>>>>> This really needs to be heavier on the _why_. My guess is that anything
>>>>> which would require replay goes through the rpcit instruction?
>>>> My understanding is:
>>>> Our arch is different from others. Each pci device has its own iommu, not
>>>> like other archs' implementation. So currently there must be no existing
>>>> mappings belonging to any newly plugged pci device whose iommu doesn't
>>>> have any mapping at the time.
>>> So please put that explanation into the function. (Also, "currently"?
>>> Are we expecting it to change?)
>> The iommu replay function is originally introduced for vfio. I think
>> they want to re-build
>> the existing mappings because vfio has a copy of mappings in kernel. For
>> our case,
>> the mappings would be cleanup when a pci device unplugged, and new mappings
>> would be created when a pci device plugged. I think replay only can
>> happen during
>> vfio-pci device migration.
> So, the base reason is that it is impossible to plug a pci device on
> s390x that already has iommu mappings which need to be replayed, which
> is due to the "one iommu per zpci device" construct (and independent of
> which devices need replay on other architectures)?
Yes.
>
> Then this should go into the explanation above. (And I'd still like to
> know what "currently" refers to. I don't like to rely on some kind of
> implicit assumptions - are we expecting this to break?)
As our discussion during internal review, we don't need to replay 
currently because vfio-pci device
doesn't support migration. 'currently' refers to the time of vfio-pci 
device migration block.
Only when vfio-pci supports migration in future, we should fill the 
replaying code.
>
>>>   
>>>> In addition, it's also due to vfio blocking migration. If vfio-pci supports
>>>> migration in future, we could implement iommu replay by that time.
>>> That's not an argument: This is the base zpci support, it should not
>>> depend on the supported devices and what they do. (What's the status of
>>> virtio-pci, btw? Does it work with your patches applied, or is there
>>> still more to be done?)
>>>
>>>   
>> My understanding is virtio-pci doesn't need replay. All mappings of any
>> pci device are existing in
>> guest memory. Once the mappings is built, all of them could be migrated
>> along with the guest
>> system. But I might misunderstand it. Please correct me.
> My question was whether virtio-pci works with your patches on top at
> all - last time I checked on master, virtio-pci devices failed to
> realize with a "msi-x is mandatory" message.
>
>
I tested. virtio-pci works fine. The message "msix is mandatory" means 
we only support
msix interrupt. If virtio-pci device (like virtio-rng) doesn't support 
msix, we don't allow it
to plug. I thinik this is not related to iommu replay.