[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH] tools/xen-mceinj: support AMD



> -----Original Message-----
> From: Ian Campbell [mailto:Ian.Campbell@xxxxxxxxxx]
> Sent: Tuesday, November 13, 2012 12:26 AM
> To: Jan Beulich
> Cc: Ian Jackson; Christoph Egger; xen-devel@xxxxxxxxxxxxx; Keir (Xen.org); 
> Liu,
> Jinsong; Liu, Jinsong; Jiang, Yunhong; Li, Haicheng; Hao, Xudong
> Subject: Re: [Xen-devel] [PATCH] tools/xen-mceinj: support AMD
> 
> On Fri, 2012-10-19 at 16:05 +0100, Jan Beulich wrote:
> > >>> On 19.10.12 at 17:01, Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx> wrote:
> > > Jan Beulich writes ("Re: [Xen-devel] [PATCH] tools/xen-mceinj: support
> AMD"):
> > >> >>> On 19.10.12 at 15:10, Christoph Egger <Christoph.Egger@xxxxxxx>
> wrote:
> > >> > Ping?
> > >>
> > >> I'm afraid it's not really clear who should commit this - it's tools
> > >> side code, so IanJ or IanC would normally be the ones, but otoh
> > >> it's code requiring low level hardware knowledge to review the
> > >> patch, so both of them might want to rather not do the review.
> > >> In the past it was usually Keir who eventually committed such
> > >> patches, but I don't know whether he put this on his to-look-at-
> > >> and-eventually-commit list.
> > >
> > > My view is that I would like an ack from someone who understands
> > > what's going on ...
> >
> > Which would ideally be those who introduced the code, i.e.
> > Intel folks if I'm not mistaken...
> 
> Lets CC some of them then.
> 
> Intel folks -- any opinion on the patch below from Christoph?
> 

It's ok for me except for one comments. See below.
Of course I did not review the AMD only code.

> +    if (strstr(cpu_brand, "AMD"))
> +        cpu_is_amd = 1;
> +    else
> +        cpu_is_intel = 1;
> +
> +    if (cpu_is_intel)
> +        type = INTEL_MCE_SRAO_MEM;
> +

Isn't necessary to set a default AMD type? The "-t" parameter is required for 
amd but not always for Intel for users, it's better to give a unified usage for 
all users.
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.