Re: [PATCH v2 5/9] IOMMU: add common API for device reserved memory

[Jan Beulich <jbeulich@xxxxxxxx>]

On 06.07.2022 17:32, Marek Marczykowski-Górecki wrote:
> Add API similar to rmrr= and ivmd= arguments, but in a common code. This
> will allow drivers to register reserved memory regardless of the IOMMU
> vendor.
> The direct reason for this API is xhci-dbc console driver (aka xue),
> that needs to use DMA. But future change may unify command line
> arguments for user-supplied reserved memory, and it may be useful for
> other drivers in the future too.

I take it that I'll understand the purpose of this when making it to
later patches.

> --- a/xen/drivers/passthrough/iommu.c
> +++ b/xen/drivers/passthrough/iommu.c
> @@ -651,6 +651,46 @@ bool_t iommu_has_feature(struct domain *d, enum 
> iommu_feature feature)
>      return is_iommu_enabled(d) && test_bit(feature, dom_iommu(d)->features);
>  }
>  
> +#define MAX_EXTRA_RESERVED_RANGES 20
> +struct extra_reserved_range {
> +    xen_pfn_t start;

Why not unsigned long?

> +    xen_ulong_t nr;

Why not unsigned int or unsigned long?

> +    u32 sbdf;

uint32_t please.

All these type related remarks apply elsewhere as well; others
below also apply wherever else applicable.

> +};
> +static unsigned int __initdata nr_extra_reserved_ranges;
> +static struct extra_reserved_range __initdata 
> extra_reserved_ranges[MAX_EXTRA_RESERVED_RANGES];

Overly long line.

> +int iommu_add_extra_reserved_device_memory(xen_pfn_t start, xen_ulong_t nr, 
> u32 sbdf)
> +{
> +    unsigned int idx;
> +
> +    if ( nr_extra_reserved_ranges >= MAX_EXTRA_RESERVED_RANGES )
> +        return -ENOMEM;
> +
> +    idx = nr_extra_reserved_ranges++;
> +    extra_reserved_ranges[idx].start = start;
> +    extra_reserved_ranges[idx].nr = nr;
> +    extra_reserved_ranges[idx].sbdf = sbdf;
> +    return 0;
> +}

Blank line please before final return statement.

Jan