Re: [PATCH] xen/arm: smmuv1: Fixed stream matching register allocation

[Rahul Singh <Rahul.Singh@xxxxxxx>]

Hi Julien,

> On 30 Jun 2021, at 10:09 am, Julien Grall <julien@xxxxxxx> wrote:
> 
> Hi Rahul,
> 
> On 25/06/2021 17:37, Rahul Singh wrote:
>> SMR allocation should be based on the number of supported stream
>> matching register for each SMMU device.
>> Issue introduced by commit 5e08586afbb90b2e2d56c175c07db77a4afa873c
>> when backported the patches from Linux to XEN to fix the stream match
>> conflict issue when two devices have the same stream-id.
>> Acked-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>
>> Tested-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>
>> Signed-off-by: Rahul Singh <rahul.singh@xxxxxxx>
>> ---
>>  xen/drivers/passthrough/arm/smmu.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>> diff --git a/xen/drivers/passthrough/arm/smmu.c 
>> b/xen/drivers/passthrough/arm/smmu.c
>> index d9a3a0cbf6..da2cd457d7 100644
>> --- a/xen/drivers/passthrough/arm/smmu.c
>> +++ b/xen/drivers/passthrough/arm/smmu.c
>> @@ -149,6 +149,7 @@ typedef enum irqreturn irqreturn_t;
>>  #define kzalloc(size, flags)                _xzalloc(size, sizeof(void *))
>>  #define devm_kzalloc(dev, size, flags)      _xzalloc(size, sizeof(void *))
>>  #define kmalloc_array(size, n, flags)       _xmalloc_array(size, 
>> sizeof(void *), n)
>> +#define kzalloc_array(size, n, flags)       _xzalloc_array(size, 
>> sizeof(void *), n)
>>    static void __iomem *devm_ioremap_resource(struct device *dev,
>>                                         struct resource *res)
>> @@ -2221,7 +2222,7 @@ static int arm_smmu_device_cfg_probe(struct 
>> arm_smmu_device *smmu)
>>              smmu->smr_mask_mask = smr >> SMR_MASK_SHIFT;
>>              /* Zero-initialised to mark as invalid */
>> -            smmu->smrs = devm_kzalloc(smmu->dev, sizeof(*smmu->smrs), 
>> GFP_KERNEL);
>> +            smmu->smrs = kzalloc_array(sizeof(*smmu->smrs), size, 
>> GFP_KERNEL);
> 
> I noticed this is already in... However, I am a bit puzzled into why this was 
> switched devm_kzalloc() to kzalloc_array(). This doesn't matter for Xen as 
> they are just wrappers to x*alloc() but a mention in the commit message would 
> have been useful.

Yes we can use the devm_kzalloc(..) but then we have to pass 
(sizeof(*smmu->smrs) * size ) as size argument to devm_kzalloc(..)
I thought for better code readability I will use kzalloc_array() as the 
function name suggests we are allocating memory for an array.

> 
> Also, when sending series, please remember to create a cover letter and 
> number each patch.
> 

Ok.

Regards,
Rahul
> Cheers,
> 
> -- 
> Julien Grall