RE: [Linuxarm] Re: [RFC PATCH 2/3] vfio/hisilicon: register the driver to vfio

[Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>]

> -----Original Message-----
> From: Jason Gunthorpe [mailto:jgg@nvidia.com]
> Sent: 13 May 2021 14:44
> To: liulongfang <liulongfang@huawei.com>
> Cc: Alex Williamson <alex.williamson@redhat.com>; cohuck@redhat.com;
> linux-kernel@vger.kernel.org; linuxarm@openeuler.org
> Subject: [Linuxarm] Re: [RFC PATCH 2/3] vfio/hisilicon: register the driver to
> vfio
> 
> On Thu, May 13, 2021 at 10:08:28AM +0800, liulongfang wrote:
> > On 2021/5/12 20:10, Jason Gunthorpe wrote:
> > > On Wed, May 12, 2021 at 04:39:43PM +0800, liulongfang wrote:
> > >
> > >> Therefore, this method of limiting the length of the BAR
> > >> configuration space can prevent unsafe operations of the memory.
> > >
> > > The issue is DMA controlled by the guest accessing the secure BAR
> > > area, not the guest CPU.
> > >
> > > Jason
> > > .
> > >
> > This secure BAR area is not presented to the Guest,
> > which makes it impossible for the Guest to obtain the secure BAR area
> > when establishing the DMA mapping of the configuration space.
> > If the DMA controller accesses the secure BAR area, the access will
> > be blocked by the SMMU.
> 
> There are scenarios where this is not true.
> 
> At a minimum the mdev driver should refuse to work in those cases.
> 

Hi,

I think the idea here is not a generic solution, but a quirk for this specific dev.

Something like, 

--- a/drivers/vfio/pci/vfio_pci.c
+++ b/drivers/vfio/pci/vfio_pci.c
@@ -866,7 +866,12 @@ static long vfio_pci_ioctl(struct vfio_device *core_vdev,
                        break;
                case VFIO_PCI_BAR0_REGION_INDEX ... VFIO_PCI_BAR5_REGION_INDEX:
                        info.offset = VFIO_PCI_INDEX_TO_OFFSET(info.index);
-                       info.size = pci_resource_len(pdev, info.index);
+
+                       if (check_hisi_acc_quirk(pdev, info))
+                               info.size = new_size;// BAR is limited without migration region.
+                       else
+                               info.size = pci_resource_len(pdev, info.index);
+
                        if (!info.size) {
                                info.flags = 0;
                                break;

Is this an acceptable/workable solution here?

Thanks,
Shameer