From mboxrd@z Thu Jan  1 00:00:00 1970
From: Elena Ufimtseva <elena.ufimtseva@oracle.com>
Subject: Re: [PATCH 2/2] iommu: add rmrr Xen command line option
 for misc rmrrs
Date: Fri, 13 Mar 2015 05:19:52 -0700 (PDT)
Message-ID: <0cb6532c-724d-4904-a7c6-67857a09f5da@default>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
Content-Disposition: inline
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: JBeulich@suse.com
Cc: yang.z.zhang@intel.com, andrew.cooper3@citrix.com, boris.ostrovsky@oracle.com, kevin.tian@intel.com, xen-devel@lists.xen.org
List-Id: xen-devel@lists.xenproject.org


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