Re: [PATCH v2 02/12] VT-d: tidy error handling of RMRR parsing

[Jan Beulich <jbeulich@xxxxxxxx>]

On 06.05.2024 11:12, Roger Pau Monné wrote:
> On Thu, Feb 15, 2024 at 11:14:02AM +0100, Jan Beulich wrote:
>> It's acpi_parse_one_rmrr() where the allocation is coming from (by way
>> of invoking acpi_parse_dev_scope()), or in add_one_user_rmrr()'s case
>> allocation is even open-coded there, so freeing would better also happen
>> there. Care needs to be taken to preserve acpi_parse_one_rmrr()'s
>> ultimate return value.
>>
>> While fiddling with callers also move scope_devices_free() to .init and
>> have it use XFREE() instead of open-coding it.
>>
>> In register_one_rmrr() also have the "ignore" path take the main
>> function return path.
>>
>> Suggested-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> 
> Reviewed-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>

Thanks.

>> --- a/xen/drivers/passthrough/vtd/dmar.c
>> +++ b/xen/drivers/passthrough/vtd/dmar.c
>> @@ -82,14 +82,13 @@ static int __init acpi_register_rmrr_uni
>>      return 0;
>>  }
>>  
>> -static void scope_devices_free(struct dmar_scope *scope)
>> +static void __init scope_devices_free(struct dmar_scope *scope)
>>  {
>>      if ( !scope )
>>          return;
>>  
>>      scope->devices_cnt = 0;
>> -    xfree(scope->devices);
>> -    scope->devices = NULL;
>> +    XFREE(scope->devices);
>>  }
>>  
>>  static void __init disable_all_dmar_units(void)
>> @@ -595,17 +594,13 @@ static int register_one_rmrr(struct acpi
> 
> register_one_rmrr() could also be made __init AFAICT? (even before
> this patch)

Indeed, all the more when it calls acpi_register_rmrr_unit(), which is
__init. With scope_devices_free() becoming __init here, it would seem
quite logical to fold that adjustment right into here. I'll do so,
unless you'd indicate that this would then invalidate your R-b.

Jan