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

["Jan Beulich" <JBeulich@suse.com>]

>>> 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.

> 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.

>> 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.

Jan