RE: [PATCH v8 02/11] iommu: Introduce a union to struct iommu_resv_region

2022-03-23 Thread Shameerali Kolothum Thodi via iommu



> -Original Message-
> From: Robin Murphy [mailto:robin.mur...@arm.com]
> Sent: 22 March 2022 18:27
> To: Shameerali Kolothum Thodi ;
> linux-arm-ker...@lists.infradead.org; linux-a...@vger.kernel.org;
> iommu@lists.linux-foundation.org
> Cc: Linuxarm ; lorenzo.pieral...@arm.com;
> j...@8bytes.org; w...@kernel.org; wanghuiqiang
> ; Guohanjun (Hanjun Guo)
> ; steven.pr...@arm.com; sami.muja...@arm.com;
> j...@solid-run.com; eric.au...@redhat.com; yangyicong
> 
> Subject: Re: [PATCH v8 02/11] iommu: Introduce a union to struct
> iommu_resv_region
> 
> On 2022-02-21 15:43, Shameer Kolothum wrote:
> > A union is introduced to struct iommu_resv_region to hold
> > any firmware specific data. This is in preparation to add
> > support for IORT RMR reserve regions and the union now holds
> > the RMR specific information.
> >
> > Signed-off-by: Shameer Kolothum
> 
> > ---
> >   include/linux/iommu.h | 9 +
> >   1 file changed, 9 insertions(+)
> >
> > diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> > index de0c57a567c8..b06952a75f95 100644
> > --- a/include/linux/iommu.h
> > +++ b/include/linux/iommu.h
> > @@ -126,6 +126,11 @@ enum iommu_resv_type {
> > IOMMU_RESV_SW_MSI,
> >   };
> >
> > +struct iommu_iort_rmr_data {
> > +   u32 *sids;  /* Stream Ids associated with IORT RMR entry */
> 
> Please make this const.
> 
> Further nit: capitalisation of "IDs" in the comment, otherwise I might
> worry about the possibility of Stream Egos too :P

True :). Will do that.

Thanks,
Shameer 

> 
> > +   u32 num_sids;
> > +};
> > +
> >   /**
> >* struct iommu_resv_region - descriptor for a reserved memory region
> >* @list: Linked list pointers
> > @@ -133,6 +138,7 @@ enum iommu_resv_type {
> >* @length: Length of the region in bytes
> >* @prot: IOMMU Protection flags (READ/WRITE/...)
> >* @type: Type of the reserved region
> > + * @fw_data: FW specific reserved region data
> 
> Nit: we've got plenty of room to spell out "Firmware-specific", and it
> never hurts to make documentation as easy to read as possible.
> 
> Thanks,
> Robin.
> 
> >*/
> >   struct iommu_resv_region {
> > struct list_headlist;
> > @@ -140,6 +146,9 @@ struct iommu_resv_region {
> > size_t  length;
> > int prot;
> > enum iommu_resv_typetype;
> > +   union {
> > +   struct iommu_iort_rmr_data rmr;
> > +   } fw_data;
> >   };
> >
> >   /**
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v8 02/11] iommu: Introduce a union to struct iommu_resv_region

2022-03-22 Thread Robin Murphy

On 2022-02-21 15:43, Shameer Kolothum wrote:

A union is introduced to struct iommu_resv_region to hold
any firmware specific data. This is in preparation to add
support for IORT RMR reserve regions and the union now holds
the RMR specific information.

Signed-off-by: Shameer Kolothum 
---
  include/linux/iommu.h | 9 +
  1 file changed, 9 insertions(+)

diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index de0c57a567c8..b06952a75f95 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -126,6 +126,11 @@ enum iommu_resv_type {
IOMMU_RESV_SW_MSI,
  };
  
+struct iommu_iort_rmr_data {

+   u32 *sids;  /* Stream Ids associated with IORT RMR entry */


Please make this const.

Further nit: capitalisation of "IDs" in the comment, otherwise I might 
worry about the possibility of Stream Egos too :P



+   u32 num_sids;
+};
+
  /**
   * struct iommu_resv_region - descriptor for a reserved memory region
   * @list: Linked list pointers
@@ -133,6 +138,7 @@ enum iommu_resv_type {
   * @length: Length of the region in bytes
   * @prot: IOMMU Protection flags (READ/WRITE/...)
   * @type: Type of the reserved region
+ * @fw_data: FW specific reserved region data


Nit: we've got plenty of room to spell out "Firmware-specific", and it 
never hurts to make documentation as easy to read as possible.


Thanks,
Robin.


   */
  struct iommu_resv_region {
struct list_headlist;
@@ -140,6 +146,9 @@ struct iommu_resv_region {
size_t  length;
int prot;
enum iommu_resv_typetype;
+   union {
+   struct iommu_iort_rmr_data rmr;
+   } fw_data;
  };
  
  /**

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu