Re: [PATCH v8 4/8] virtio-iommu: set supported page size mask
Hi Eric/Jean,
On Wed, Mar 18, 2020 at 8:05 PM Bharat Bhushan wrote:
>
> Hi Eric,
>
> On Wed, Mar 18, 2020 at 4:58 PM Auger Eric 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
> > > ---
> > > 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?
Just sent Linux RFC patch to use page-size-mask per endpoint.
QEMU changes are also ready, will share soon.
Thanks
-Bharat
>
> 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 = {
> > >
> >
Re: [PATCH v8 4/8] virtio-iommu: set supported page size mask
Hi Eric,
On Wed, Mar 18, 2020 at 4:58 PM Auger Eric 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
> > ---
> > 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 = {
> >
>
Re: [PATCH v8 4/8] virtio-iommu: set supported page size mask
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
> ---
> 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. 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?
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 = {
>
[PATCH v8 4/8] virtio-iommu: set supported page size mask
Add optional interface to set page size mask.
Currently this is set global configuration and not
per endpoint.
Signed-off-by: Bharat Bhushan
---
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;
+}
+
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 = {
--
2.17.1
