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

Robin Murphy
2022-03-22T18:27:43Z

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 <shameerali.kolothum.thodi@huawei.com>
> ---
> 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_head list;
> @@ -140,6 +146,9 @@ struct iommu_resv_region {
> size_t length;
> int prot;
> enum iommu_resv_type type;
> + union {
> + struct iommu_iort_rmr_data rmr;
> + } fw_data;
> };
>
> /**