Re: [PATCH v8 4/8] virtio-iommu: set supported page size mask

[Bharat Bhushan]

Hi Eric,

On Wed, Mar 18, 2020 at 4:58 PM Auger Eric <address@hidden> wrote:
>
> Hi Bharat,
>
> On 3/18/20 11:11 AM, Bharat Bhushan wrote:
> > Add optional interface to set page size mask.
> > Currently this is set global configuration and not
> > per endpoint.
> >
> > Signed-off-by: Bharat Bhushan <address@hidden>
> > ---
> > v7->v8:
> >  - new patch
> >
> >  hw/virtio/virtio-iommu.c | 10 ++++++++++
> >  1 file changed, 10 insertions(+)
> >
> > diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c
> > index 4cee8083bc..c00a55348d 100644
> > --- a/hw/virtio/virtio-iommu.c
> > +++ b/hw/virtio/virtio-iommu.c
> > @@ -650,6 +650,15 @@ static gint int_cmp(gconstpointer a, gconstpointer b, 
> > gpointer user_data)
> >      return (ua > ub) - (ua < ub);
> >  }
> >
> > +static void virtio_iommu_set_page_size_mask(IOMMUMemoryRegion *mr,
> > +                                            uint64_t page_size_mask)
> > +{
> > +    IOMMUDevice *sdev = container_of(mr, IOMMUDevice, iommu_mr);
> > +    VirtIOIOMMU *s = sdev->viommu;
> > +
> > +    s->config.page_size_mask = page_size_mask;
> The problem is page_size_mask is global to the VIRTIO-IOMMU.
>
> - Can't different VFIO containers impose different/inconsistent settings?
> - VFIO devices can be hotplugged.

This is possible if we different iommu's, which we support. correct?

> So we may start with some default
> page_size_mask which is latter overriden by a host imposed one. Assume
> you first launch the VM with a virtio NIC. This uses 64K. Then you
> hotplug a VFIO device behind a physical IOMMU which only supports 4K
> pages. Isn't it a valid scenario?

So we need to expose page_size_mask per endpoint?

Thanks
-Bharat

>
> Thanks
>
> Eric
>
> > +}
> > +
> >  static void virtio_iommu_device_realize(DeviceState *dev, Error **errp)
> >  {
> >      VirtIODevice *vdev = VIRTIO_DEVICE(dev);
> > @@ -865,6 +874,7 @@ static void 
> > virtio_iommu_memory_region_class_init(ObjectClass *klass,
> >      IOMMUMemoryRegionClass *imrc = IOMMU_MEMORY_REGION_CLASS(klass);
> >
> >      imrc->translate = virtio_iommu_translate;
> > +    imrc->iommu_set_page_size_mask = virtio_iommu_set_page_size_mask;
> >  }
> >
> >  static const TypeInfo virtio_iommu_info = {
> >
>