Re: [PATCH v12 03/13] vfio: VFIO_IOMMU_SET_MSI_BINDING

[Auger Eric]

Hi Jean,

On 3/5/21 11:45 AM, Jean-Philippe Brucker wrote:
> Hi,
> 
> On Tue, Feb 23, 2021 at 10:06:15PM +0100, Eric Auger wrote:
>> This patch adds the VFIO_IOMMU_SET_MSI_BINDING ioctl which aim
>> to (un)register the guest MSI binding to the host. This latter
>> then can use those stage 1 bindings to build a nested stage
>> binding targeting the physical MSIs.
> 
> Now that RMR is in the IORT spec, could it be used for the nested MSI
> problem?  For virtio-iommu tables I was planning to do it like this:
> 
> MSI is mapped at stage-2 with an arbitrary IPA->doorbell PA. We report
> this IPA to userspace through iommu_groups/X/reserved_regions. No change
> there. Then to the guest we report a reserved identity mapping at IPA
> (using RMR, an equivalent DT binding, or probed RESV_MEM for
> virtio-iommu).

Is there any DT binding equivalent?

 The guest creates that mapping at stage-1, and that's it.
> Unless I overlooked something we'd only reuse existing infrastructure and
> avoid the SET_MSI_BINDING interface.

Yes at first glance I think this should work. The guest SMMU driver will
continue allocating IOVA for MSIs but I think that's not an issue as
they won't be used.

For the SMMU case this makes the guest behavior different from the
baremetal one though. Typically you will never get any S1 fault. Also
the S1 mapping is static and direct.

I will prototype this too.

Thanks

Eric
> 
> Thanks,
> Jean
> 

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH v12 03/13] vfio: VFIO_IOMMU_SET_MSI_BINDING

[Jean-Philippe Brucker]

Hi,

On Tue, Feb 23, 2021 at 10:06:15PM +0100, Eric Auger wrote:
> This patch adds the VFIO_IOMMU_SET_MSI_BINDING ioctl which aim
> to (un)register the guest MSI binding to the host. This latter
> then can use those stage 1 bindings to build a nested stage
> binding targeting the physical MSIs.

Now that RMR is in the IORT spec, could it be used for the nested MSI
problem?  For virtio-iommu tables I was planning to do it like this:

MSI is mapped at stage-2 with an arbitrary IPA->doorbell PA. We report
this IPA to userspace through iommu_groups/X/reserved_regions. No change
there. Then to the guest we report a reserved identity mapping at IPA
(using RMR, an equivalent DT binding, or probed RESV_MEM for
virtio-iommu). The guest creates that mapping at stage-1, and that's it.
Unless I overlooked something we'd only reuse existing infrastructure and
avoid the SET_MSI_BINDING interface.

Thanks,
Jean
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH v12 03/13] vfio: VFIO_IOMMU_SET_MSI_BINDING

[kernel test robot]

Hi Eric,

I love your patch! Yet something to improve:

[auto build test ERROR on v5.11]
[cannot apply to vfio/next next-20210223]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:
https://github.com/0day-ci/linux/commits/Eric-Auger/SMMUv3-Nested-Stage-Setup-VFIO-part/20210224-051641
base:f40ddce88593482919761f74910f42f4b84c004b
config: arm64-randconfig-r003-20210223 (attached as .config)
compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project 
c9439ca36342fb6013187d0a69aef92736951476)
reproduce (this is a W=1 build):
wget 
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
# install arm64 cross compiling tool for clang build
# apt-get install binutils-aarch64-linux-gnu
# 
https://github.com/0day-ci/linux/commit/ec823a68d862693dc787422f168409996f43b10a
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review 
Eric-Auger/SMMUv3-Nested-Stage-Setup-VFIO-part/20210224-051641
git checkout ec823a68d862693dc787422f168409996f43b10a
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=arm64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot 

All errors (new ones prefixed by >>):

   In file included from drivers/vfio/vfio_iommu_type1.c:36:
   In file included from include/linux/vfio.h:16:
   include/uapi/linux/vfio.h:1198:34: error: field has incomplete type 'struct 
iommu_pasid_table_config'
   struct iommu_pasid_table_config config; /* used on SET */
   ^
   include/uapi/linux/vfio.h:1198:9: note: forward declaration of 'struct 
iommu_pasid_table_config'
   struct iommu_pasid_table_config config; /* used on SET */
  ^
   drivers/vfio/vfio_iommu_type1.c:2625:3: error: implicit declaration of 
function 'iommu_detach_pasid_table' [-Werror,-Wimplicit-function-declaration]
   iommu_detach_pasid_table(d->domain);
   ^
   drivers/vfio/vfio_iommu_type1.c:2625:3: note: did you mean 
'vfio_detach_pasid_table'?
   drivers/vfio/vfio_iommu_type1.c:2619:1: note: 'vfio_detach_pasid_table' 
declared here
   vfio_detach_pasid_table(struct vfio_iommu *iommu)
   ^
   drivers/vfio/vfio_iommu_type1.c:2639:9: error: implicit declaration of 
function 'iommu_uapi_attach_pasid_table' 
[-Werror,-Wimplicit-function-declaration]
   ret = iommu_uapi_attach_pasid_table(d->domain, (void __user 
*)arg);
 ^
   drivers/vfio/vfio_iommu_type1.c:2642:5: error: implicit declaration of 
function 'iommu_detach_pasid_table' [-Werror,-Wimplicit-function-declaration]
   iommu_detach_pasid_table(d->domain);
   ^
>> drivers/vfio/vfio_iommu_type1.c:2668:9: error: implicit declaration of 
>> function 'iommu_bind_guest_msi' [-Werror,-Wimplicit-function-declaration]
   ret = iommu_bind_guest_msi(d->domain, giova, gpa, size);
 ^
>> drivers/vfio/vfio_iommu_type1.c:2675:3: error: implicit declaration of 
>> function 'iommu_unbind_guest_msi' [-Werror,-Wimplicit-function-declaration]
   iommu_unbind_guest_msi(d->domain, giova);
   ^
   drivers/vfio/vfio_iommu_type1.c:2689:3: error: implicit declaration of 
function 'iommu_unbind_guest_msi' [-Werror,-Wimplicit-function-declaration]
   iommu_unbind_guest_msi(d->domain, giova);
   ^
   7 errors generated.


vim +/iommu_bind_guest_msi +2668 drivers/vfio/vfio_iommu_type1.c

  2617  
  2618  static void
> 2619  vfio_detach_pasid_table(struct vfio_iommu *iommu)
  2620  {
  2621  struct vfio_domain *d;
  2622  
  2623  mutex_lock(>lock);
  2624  list_for_each_entry(d, >domain_list, next)
  2625  iommu_detach_pasid_table(d->domain);
  2626  
  2627  mutex_unlock(>lock);
  2628  }
  2629  
  2630  static int
  2631  vfio_attach_pasid_table(struct vfio_iommu *iommu, unsigned long arg)
  2632  {
  2633  struct vfio_domain *d;
  2634  int ret = 0;
  2635  
  2636  mutex_lock(>lock);
  2637  
  2638  list_for_each_entry(d, >domain_list, next) {
> 2639  ret = iommu_uapi_attach_pasid_table(d->domain, (void 
> __user *)arg);
  2640  if (ret) {
  2641  list_for_each_entry_continue_reverse(d, 
>domain_list, next)
  2642  iommu_detach_pasid_table(d->domain);
  2643  break;
  2644  }
  2645  }
  2646  
  2647  mutex_unlock(>lock);
  2648  return ret;
  2649