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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 13.11.12 at 02:40, "Hao, Xudong" <xudong.hao@xxxxxxxxx> wrote:
>>  -----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.

Yes, I had pointed this out in an earlier reply already, as much as
I had asked (in the context of a matching hypervisor side patch)
about the reason for only dealing with bank 4 registers here.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel