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

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Mon, May 06, 2024 at 11:21:06AM +0200, Jan Beulich wrote:
> 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.

Sure, feel free to fold here.

Thanks, Roger.