Re: [PATCH v11 00/13] SMMUv3 Nested Stage Setup (IOMMU part)

[Auger Eric]

Hi Shameer,

On 5/7/20 8:59 AM, Shameerali Kolothum Thodi wrote:
> Hi Eric,
> 
>> -----Original Message-----
>> From: Shameerali Kolothum Thodi
>> Sent: 30 April 2020 10:38
>> To: 'Auger Eric' <eric.auger@xxxxxxxxxx>; Zhangfei Gao
>> <zhangfei.gao@xxxxxxxxxx>; eric.auger.pro@xxxxxxxxx;
>> iommu@xxxxxxxxxxxxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx;
>> kvm@xxxxxxxxxxxxxxx; kvmarm@xxxxxxxxxxxxxxxxxxxxx; will@xxxxxxxxxx;
>> joro@xxxxxxxxxx; maz@xxxxxxxxxx; robin.murphy@xxxxxxx
>> Cc: jean-philippe@xxxxxxxxxx; alex.williamson@xxxxxxxxxx;
>> jacob.jun.pan@xxxxxxxxxxxxxxx; yi.l.liu@xxxxxxxxx; peter.maydell@xxxxxxxxxx;
>> tn@xxxxxxxxxxxx; bbhushan2@xxxxxxxxxxx
>> Subject: RE: [PATCH v11 00/13] SMMUv3 Nested Stage Setup (IOMMU part)
>>
>> Hi Eric,
>>
>>> -----Original Message-----
>>> From: Auger Eric [mailto:eric.auger@xxxxxxxxxx]
>>> Sent: 16 April 2020 08:45
>>> To: Zhangfei Gao <zhangfei.gao@xxxxxxxxxx>; eric.auger.pro@xxxxxxxxx;
>>> iommu@xxxxxxxxxxxxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx;
>>> kvm@xxxxxxxxxxxxxxx; kvmarm@xxxxxxxxxxxxxxxxxxxxx; will@xxxxxxxxxx;
>>> joro@xxxxxxxxxx; maz@xxxxxxxxxx; robin.murphy@xxxxxxx
>>> Cc: jean-philippe@xxxxxxxxxx; Shameerali Kolothum Thodi
>>> <shameerali.kolothum.thodi@xxxxxxxxxx>; alex.williamson@xxxxxxxxxx;
>>> jacob.jun.pan@xxxxxxxxxxxxxxx; yi.l.liu@xxxxxxxxx; peter.maydell@xxxxxxxxxx;
>>> tn@xxxxxxxxxxxx; bbhushan2@xxxxxxxxxxx
>>> Subject: Re: [PATCH v11 00/13] SMMUv3 Nested Stage Setup (IOMMU part)
>>>
>>> Hi Zhangfei,
>>>
>>> On 4/16/20 6:25 AM, Zhangfei Gao wrote:
>>>>
>>>>
>>>> On 2020/4/14 äå11:05, Eric Auger wrote:
>>>>> This version fixes an issue observed by Shameer on an SMMU 3.2,
>>>>> when moving from dual stage config to stage 1 only config.
>>>>> The 2 high 64b of the STE now get reset. Otherwise, leaving the
>>>>> S2TTB set may cause a C_BAD_STE error.
>>>>>
>>>>> This series can be found at:
>>>>> https://github.com/eauger/linux/tree/v5.6-2stage-v11_10.1
>>>>> (including the VFIO part)
>>>>> The QEMU fellow series still can be found at:
>>>>> https://github.com/eauger/qemu/tree/v4.2.0-2stage-rfcv6
>>>>>
>>>>> Users have expressed interest in that work and tested v9/v10:
>>>>> - https://patchwork.kernel.org/cover/11039995/#23012381
>>>>> - https://patchwork.kernel.org/cover/11039995/#23197235
>>>>>
>>>>> Background:
>>>>>
>>>>> This series brings the IOMMU part of HW nested paging support
>>>>> in the SMMUv3. The VFIO part is submitted separately.
>>>>>
>>>>> The IOMMU API is extended to support 2 new API functionalities:
>>>>> 1) pass the guest stage 1 configuration
>>>>> 2) pass stage 1 MSI bindings
>>>>>
>>>>> Then those capabilities gets implemented in the SMMUv3 driver.
>>>>>
>>>>> The virtualizer passes information through the VFIO user API
>>>>> which cascades them to the iommu subsystem. This allows the guest
>>>>> to own stage 1 tables and context descriptors (so-called PASID
>>>>> table) while the host owns stage 2 tables and main configuration
>>>>> structures (STE).
>>>>>
>>>>>
>>>>
>>>> Thanks Eric
>>>>
>>>> Tested v11 on Hisilicon kunpeng920 board via hardware zip accelerator.
>>>> 1. no-sva works, where guest app directly use physical address via ioctl.
>>> Thank you for the testing. Glad it works for you.
>>>> 2. vSVA still not work, same as v10,
>>> Yes that's normal this series is not meant to support vSVM at this stage.
>>>
>>> I intend to add the missing pieces during the next weeks.
>>
>> Thanks for that. I have made an attempt to add the vSVA based on
>> your v10 + JPBs sva patches. The host kernel and Qemu changes can
>> be found here[1][2].
>>
>> This basically adds multiple pasid support on top of your changes.
>> I have done some basic sanity testing and we have some initial success
>> with the zip vf dev on our D06 platform. Please note that the STALL event is
>> not yet supported though, but works fine if we mlock() guest usr mem.
> 
> I have added STALL support for our vSVA prototype and it seems to be
> working(on our hardware). I have updated the kernel and qemu branches with
> the same[1][2]. I should warn you though that these are prototype code and I am pretty
> much re-using the VFIO_IOMMU_SET_PASID_TABLE interface for almost everything.
> But thought of sharing, in case if it is useful somehow!.

Thank you again for sharing the POC. I looked at the kernel and QEMU
branches.

Here are some preliminary comments:
- "arm-smmu-v3: Reset S2TTB while switching back from nested stage":  as
you mentionned S2TTB reset now is featured in v11
- "arm-smmu-v3: Add support for multiple pasid in nested mode": I could
easily integrate this into my series. Update the iommu api first and
pass multiple CD info in a separate patch
- "arm-smmu-v3: Add support to Invalidate CD": CD invalidation should be
cascaded to host through the PASID cache invalidation uapi (no pb you
warned us for the POC you simply used VFIO_IOMMU_SET_PASID_TABLE). I
think I should add this support in my original series although it does
not seem to trigger any issue up to now.
- "arm-smmu-v3: Remove duplication of fault propagation". I understand
the transcode is done somewhere else with SVA but we still need to do it
if a single CD is used, right? I will review the SVA code to better
understand.
- for the STALL response injection I would tend to use a new VFIO region
for responses. At the moment there is a single VFIO region for reporting
the fault.

On QEMU side:
- I am currently working on 3.2 range invalidation support which is
needed for DPDK/VFIO
- While at it I will look at how to incrementally introduce some of the
features you need in this series.

Thanks

Eric



> 
> Thanks,
> Shameer
> 
> [1]https://github.com/hisilicon/kernel-dev/commits/vsva-prototype-host-v1
> 
> [2]https://github.com/hisilicon/qemu/tree/v4.2.0-2stage-rfcv6-vsva-prototype-v1
>