Re: [EXT] Re: [PATCH v9 2/9] memory: Add interface to set iommu page size mask
Hi Bharat,
On 3/27/20 6:33 AM, Bharat Bhushan wrote:
> Hi Eric,
>
>> -Original Message-
>> From: Auger Eric
>> Sent: Thursday, March 26, 2020 9:36 PM
>> To: Bharat Bhushan ; peter.mayd...@linaro.org;
>> pet...@redhat.com; eric.auger@gmail.com; alex.william...@redhat.com;
>> kevin.t...@intel.com; m...@redhat.com; Tomasz Nowicki [C]
>> ; drjo...@redhat.com; linuc.dec...@gmail.com; qemu-
>> de...@nongnu.org; qemu-...@nongnu.org; bharatb.li...@gmail.com; jean-
>> phili...@linaro.org; yang.zh...@intel.com
>> Subject: [EXT] Re: [PATCH v9 2/9] memory: Add interface to set iommu page
>> size
>> mask
>>
>> External Email
>>
>> --
>> Hi Bharat,
>> On 3/23/20 9:46 AM, Bharat Bhushan wrote:
>>> Allow to set page size mask to be supported by iommu.
>> by iommu memory region. I mean this is not global to the IOMMU.
>
> Yes.
>
>>> This is required to expose page size mask compatible with host with
>>> virtio-iommu.
>>>
>>> Signed-off-by: Bharat Bhushan
>>> ---
>>> include/exec/memory.h | 20
>>> memory.c | 10 ++
>>> 2 files changed, 30 insertions(+)
>>>
>>> diff --git a/include/exec/memory.h b/include/exec/memory.h index
>>> e85b7de99a..063c424854 100644
>>> --- a/include/exec/memory.h
>>> +++ b/include/exec/memory.h
>>> @@ -355,6 +355,16 @@ typedef struct IOMMUMemoryRegionClass {
>>> * @iommu: the IOMMUMemoryRegion
>>> */
>>> int (*num_indexes)(IOMMUMemoryRegion *iommu);
>>> +
>>> +/*
>>> + * Set supported IOMMU page size
>>> + *
>>> + * Optional method: if this is supported then set page size that
>>> + * can be supported by IOMMU. This is called to set supported page
>>> + * size as per host Linux.
>> What about: If supported, allows to restrict the page size mask that can be
>> supported with a given IOMMU memory region. For example, this allows to
>> propagate host physical IOMMU page size mask limitations to the virtual IOMMU
>> (vfio assignment with virtual iommu).
>
> Much better
>
>>> + */
>>> + void (*iommu_set_page_size_mask)(IOMMUMemoryRegion *iommu,
>>> + uint64_t page_size_mask);
>>> } IOMMUMemoryRegionClass;
>>>
>>> typedef struct CoalescedMemoryRange CoalescedMemoryRange; @@ -1363,6
>>> +1373,16 @@ int
>> memory_region_iommu_attrs_to_index(IOMMUMemoryRegion *iommu_mr,
>>> */
>>> int memory_region_iommu_num_indexes(IOMMUMemoryRegion *iommu_mr);
>>>
>>> +/**
>>> + * memory_region_iommu_set_page_size_mask: set the supported pages
>>> + * size by iommu.
>> supported page sizes for a given IOMMU memory region
>>> + *
>>> + * @iommu_mr: the memory region
>> IOMMU memory region
>>> + * @page_size_mask: supported page size mask */ void
>>> +memory_region_iommu_set_page_size_mask(IOMMUMemoryRegion
>> *iommu_mr,
>>> +uint64_t page_size_mask);
>>> +
>>> /**
>>> * memory_region_name: get a memory region's name
>>> *
>>> diff --git a/memory.c b/memory.c
>>> index aeaa8dcc9e..14c8783084 100644
>>> --- a/memory.c
>>> +++ b/memory.c
>>> @@ -1833,6 +1833,16 @@ static int
>> memory_region_update_iommu_notify_flags(IOMMUMemoryRegion
>> *iommu_mr,
>>> return ret;
>>> }
>>>
>>> +void memory_region_iommu_set_page_size_mask(IOMMUMemoryRegion
>> *iommu_mr,
>>> +uint64_t page_size_mask)
>>> +{
>>> +IOMMUMemoryRegionClass *imrc =
>>> +IOMMU_MEMORY_REGION_GET_CLASS(iommu_mr);
>>> +
>>> +if (imrc->iommu_set_page_size_mask) {
>>> +imrc->iommu_set_page_size_mask(iommu_mr, page_size_mask);
>> Shouldn't it return an int in case the setting cannot be applied?
>
> iommu_set_page_size_mask() is setting page-size-mask for endpoint. Below
> function from code
>
> static void virtio_iommu_set_page_size_mask(IOMMUMemoryRegion *mr,
> uint64_t page_size_mask)
> {
> IOMMUDevice *sdev = container_of(mr, IOMMUDevice, iommu_mr);
>
> sdev->page_size_mask = page_size_mask;
> }
>
> Do you see any reason it cannot be applied, am I missing something?
maybe the function can be called several times and one setting is
already in place. In other contexts, maybe the setting we try to apply
is not compatible with other settings enforced?
Thanks
Eric
>
> Thanks
> -Bharat
>
>>> +}
>>> +}
>>> +
>>> int memory_region_register_iommu_notifier(MemoryRegion *mr,
>>>IOMMUNotifier *n, Error
>>> **errp) {
>>>
>> Thanks
>> Eric
>
RE: [EXT] Re: [PATCH v9 2/9] memory: Add interface to set iommu page size mask
Hi Eric,
> -Original Message-
> From: Auger Eric
> Sent: Thursday, March 26, 2020 9:36 PM
> To: Bharat Bhushan ; peter.mayd...@linaro.org;
> pet...@redhat.com; eric.auger@gmail.com; alex.william...@redhat.com;
> kevin.t...@intel.com; m...@redhat.com; Tomasz Nowicki [C]
> ; drjo...@redhat.com; linuc.dec...@gmail.com; qemu-
> de...@nongnu.org; qemu-...@nongnu.org; bharatb.li...@gmail.com; jean-
> phili...@linaro.org; yang.zh...@intel.com
> Subject: [EXT] Re: [PATCH v9 2/9] memory: Add interface to set iommu page size
> mask
>
> External Email
>
> --
> Hi Bharat,
> On 3/23/20 9:46 AM, Bharat Bhushan wrote:
> > Allow to set page size mask to be supported by iommu.
> by iommu memory region. I mean this is not global to the IOMMU.
Yes.
> > This is required to expose page size mask compatible with host with
> > virtio-iommu.
> >
> > Signed-off-by: Bharat Bhushan
> > ---
> > include/exec/memory.h | 20
> > memory.c | 10 ++
> > 2 files changed, 30 insertions(+)
> >
> > diff --git a/include/exec/memory.h b/include/exec/memory.h index
> > e85b7de99a..063c424854 100644
> > --- a/include/exec/memory.h
> > +++ b/include/exec/memory.h
> > @@ -355,6 +355,16 @@ typedef struct IOMMUMemoryRegionClass {
> > * @iommu: the IOMMUMemoryRegion
> > */
> > int (*num_indexes)(IOMMUMemoryRegion *iommu);
> > +
> > +/*
> > + * Set supported IOMMU page size
> > + *
> > + * Optional method: if this is supported then set page size that
> > + * can be supported by IOMMU. This is called to set supported page
> > + * size as per host Linux.
> What about: If supported, allows to restrict the page size mask that can be
> supported with a given IOMMU memory region. For example, this allows to
> propagate host physical IOMMU page size mask limitations to the virtual IOMMU
> (vfio assignment with virtual iommu).
Much better
> > + */
> > + void (*iommu_set_page_size_mask)(IOMMUMemoryRegion *iommu,
> > + uint64_t page_size_mask);
> > } IOMMUMemoryRegionClass;
> >
> > typedef struct CoalescedMemoryRange CoalescedMemoryRange; @@ -1363,6
> > +1373,16 @@ int
> memory_region_iommu_attrs_to_index(IOMMUMemoryRegion *iommu_mr,
> > */
> > int memory_region_iommu_num_indexes(IOMMUMemoryRegion *iommu_mr);
> >
> > +/**
> > + * memory_region_iommu_set_page_size_mask: set the supported pages
> > + * size by iommu.
> supported page sizes for a given IOMMU memory region
> > + *
> > + * @iommu_mr: the memory region
> IOMMU memory region
> > + * @page_size_mask: supported page size mask */ void
> > +memory_region_iommu_set_page_size_mask(IOMMUMemoryRegion
> *iommu_mr,
> > +uint64_t page_size_mask);
> > +
> > /**
> > * memory_region_name: get a memory region's name
> > *
> > diff --git a/memory.c b/memory.c
> > index aeaa8dcc9e..14c8783084 100644
> > --- a/memory.c
> > +++ b/memory.c
> > @@ -1833,6 +1833,16 @@ static int
> memory_region_update_iommu_notify_flags(IOMMUMemoryRegion
> *iommu_mr,
> > return ret;
> > }
> >
> > +void memory_region_iommu_set_page_size_mask(IOMMUMemoryRegion
> *iommu_mr,
> > +uint64_t page_size_mask)
> > +{
> > +IOMMUMemoryRegionClass *imrc =
> > +IOMMU_MEMORY_REGION_GET_CLASS(iommu_mr);
> > +
> > +if (imrc->iommu_set_page_size_mask) {
> > +imrc->iommu_set_page_size_mask(iommu_mr, page_size_mask);
> Shouldn't it return an int in case the setting cannot be applied?
iommu_set_page_size_mask() is setting page-size-mask for endpoint. Below
function from code
static void virtio_iommu_set_page_size_mask(IOMMUMemoryRegion *mr,
uint64_t page_size_mask)
{
IOMMUDevice *sdev = container_of(mr, IOMMUDevice, iommu_mr);
sdev->page_size_mask = page_size_mask;
}
Do you see any reason it cannot be applied, am I missing something?
Thanks
-Bharat
> > +}
> > +}
> > +
> > int memory_region_register_iommu_notifier(MemoryRegion *mr,
> >IOMMUNotifier *n, Error
> > **errp) {
> >
> Thanks
> Eric
Re: [PATCH v9 2/9] memory: Add interface to set iommu page size mask
Hi Bharat,
On 3/23/20 9:46 AM, Bharat Bhushan wrote:
> Allow to set page size mask to be supported by iommu.
by iommu memory region. I mean this is not global to the IOMMU.
> This is required to expose page size mask compatible with
> host with virtio-iommu.
>
> Signed-off-by: Bharat Bhushan
> ---
> include/exec/memory.h | 20
> memory.c | 10 ++
> 2 files changed, 30 insertions(+)
>
> diff --git a/include/exec/memory.h b/include/exec/memory.h
> index e85b7de99a..063c424854 100644
> --- a/include/exec/memory.h
> +++ b/include/exec/memory.h
> @@ -355,6 +355,16 @@ typedef struct IOMMUMemoryRegionClass {
> * @iommu: the IOMMUMemoryRegion
> */
> int (*num_indexes)(IOMMUMemoryRegion *iommu);
> +
> +/*
> + * Set supported IOMMU page size
> + *
> + * Optional method: if this is supported then set page size that
> + * can be supported by IOMMU. This is called to set supported page
> + * size as per host Linux.
What about: If supported, allows to restrict the page size mask that can
be supported with a given IOMMU memory region. For example, this allows
to propagate host physical IOMMU page size mask limitations to the
virtual IOMMU (vfio assignment with virtual iommu).
> + */
> + void (*iommu_set_page_size_mask)(IOMMUMemoryRegion *iommu,
> + uint64_t page_size_mask);
> } IOMMUMemoryRegionClass;
>
> typedef struct CoalescedMemoryRange CoalescedMemoryRange;
> @@ -1363,6 +1373,16 @@ int
> memory_region_iommu_attrs_to_index(IOMMUMemoryRegion *iommu_mr,
> */
> int memory_region_iommu_num_indexes(IOMMUMemoryRegion *iommu_mr);
>
> +/**
> + * memory_region_iommu_set_page_size_mask: set the supported pages
> + * size by iommu.
supported page sizes for a given IOMMU memory region
> + *
> + * @iommu_mr: the memory region
IOMMU memory region
> + * @page_size_mask: supported page size mask
> + */
> +void memory_region_iommu_set_page_size_mask(IOMMUMemoryRegion *iommu_mr,
> +uint64_t page_size_mask);
> +
> /**
> * memory_region_name: get a memory region's name
> *
> diff --git a/memory.c b/memory.c
> index aeaa8dcc9e..14c8783084 100644
> --- a/memory.c
> +++ b/memory.c
> @@ -1833,6 +1833,16 @@ static int
> memory_region_update_iommu_notify_flags(IOMMUMemoryRegion *iommu_mr,
> return ret;
> }
>
> +void memory_region_iommu_set_page_size_mask(IOMMUMemoryRegion *iommu_mr,
> +uint64_t page_size_mask)
> +{
> +IOMMUMemoryRegionClass *imrc = IOMMU_MEMORY_REGION_GET_CLASS(iommu_mr);
> +
> +if (imrc->iommu_set_page_size_mask) {
> +imrc->iommu_set_page_size_mask(iommu_mr, page_size_mask);
Shouldn't it return an int in case the setting cannot be applied?
> +}
> +}
> +
> int memory_region_register_iommu_notifier(MemoryRegion *mr,
>IOMMUNotifier *n, Error **errp)
> {
>
Thanks
Eric
[PATCH v9 2/9] memory: Add interface to set iommu page size mask
Allow to set page size mask to be supported by iommu.
This is required to expose page size mask compatible with
host with virtio-iommu.
Signed-off-by: Bharat Bhushan
---
include/exec/memory.h | 20
memory.c | 10 ++
2 files changed, 30 insertions(+)
diff --git a/include/exec/memory.h b/include/exec/memory.h
index e85b7de99a..063c424854 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -355,6 +355,16 @@ typedef struct IOMMUMemoryRegionClass {
* @iommu: the IOMMUMemoryRegion
*/
int (*num_indexes)(IOMMUMemoryRegion *iommu);
+
+/*
+ * Set supported IOMMU page size
+ *
+ * Optional method: if this is supported then set page size that
+ * can be supported by IOMMU. This is called to set supported page
+ * size as per host Linux.
+ */
+ void (*iommu_set_page_size_mask)(IOMMUMemoryRegion *iommu,
+ uint64_t page_size_mask);
} IOMMUMemoryRegionClass;
typedef struct CoalescedMemoryRange CoalescedMemoryRange;
@@ -1363,6 +1373,16 @@ int memory_region_iommu_attrs_to_index(IOMMUMemoryRegion
*iommu_mr,
*/
int memory_region_iommu_num_indexes(IOMMUMemoryRegion *iommu_mr);
+/**
+ * memory_region_iommu_set_page_size_mask: set the supported pages
+ * size by iommu.
+ *
+ * @iommu_mr: the memory region
+ * @page_size_mask: supported page size mask
+ */
+void memory_region_iommu_set_page_size_mask(IOMMUMemoryRegion *iommu_mr,
+uint64_t page_size_mask);
+
/**
* memory_region_name: get a memory region's name
*
diff --git a/memory.c b/memory.c
index aeaa8dcc9e..14c8783084 100644
--- a/memory.c
+++ b/memory.c
@@ -1833,6 +1833,16 @@ static int
memory_region_update_iommu_notify_flags(IOMMUMemoryRegion *iommu_mr,
return ret;
}
+void memory_region_iommu_set_page_size_mask(IOMMUMemoryRegion *iommu_mr,
+uint64_t page_size_mask)
+{
+IOMMUMemoryRegionClass *imrc = IOMMU_MEMORY_REGION_GET_CLASS(iommu_mr);
+
+if (imrc->iommu_set_page_size_mask) {
+imrc->iommu_set_page_size_mask(iommu_mr, page_size_mask);
+}
+}
+
int memory_region_register_iommu_notifier(MemoryRegion *mr,
IOMMUNotifier *n, Error **errp)
{
--
2.17.1
