Re: [PATCH 2/2] iommu: add rmrr Xen command line option for misc rmrrs

[Elena Ufimtseva <elena.ufimtseva@oracle.com>]

----- JBeulich@suse.com wrote:

> >>> On 12.03.15 at 21:52, <elena.ufimtseva@oracle.com> wrote:
> > On Mon, Mar 09, 2015 at 05:16:18PM +0000, Andrew Cooper wrote:
> >> On 09/03/15 14:42, elena.ufimtseva@oracle.com wrote:
> >> > --- a/xen/drivers/passthrough/iommu.c
> >> > +++ b/xen/drivers/passthrough/iommu.c
> >> > @@ -55,6 +55,9 @@ bool_t __read_mostly iommu_hap_pt_share = 1;
> >> >  bool_t __read_mostly iommu_debug;
> >> >  bool_t __read_mostly amd_iommu_perdev_intremap = 1;
> >> >  
> >> > +static char __initdata misc_rmrr[100];
> >> > +string_param("rmrr", misc_rmrr);
> >> 
> >> This wants to be a custom_param, not a string_param.
> > 
> > Andrew, can you please explain why its preferred?
> 
> Consistency.

Makes sense.
> 
> > Also, I wont be able to use xen dynamic allocator at this early
> stage, 
> > correct?
> 
> Yes. But by having a static limit on the string size you had already
> limited the number of entries, so having a static array instead won't
> make things worse.

Ok, will do it.

> 
> >> Furthermore, I think it would be better to match the existing
> ivrs_hpet[
> >> and ivrs_ioapic[ parameters as closely as possible, seeing as it is
> the
> >> same kind of overrides being added to ACPI tables.
> > 
> > Thanks Andrew, looks like preferred way looks like this:
> > rmrr=start<:end>[sbdf1, sbdf2, ...], ...
> 
> With the slight adjustment that
> 
> 	rmrr=start:<end:>sbdf1[,sbdf2[, ...]][; ...]
> 
> or even (considering that sbdf necessarily include : too)
> 
> 	rmrr=start<:end>=sbdf1[,sbdf2[, ...]][; ...]
> 
> will likely make it easier to parse unambiguously.

Ok, I will just need a bit of change for this.

> 
> Jan