Xen devel - Mar 2009 - [PATCH] re-work MCA telemetry internals; use common code for Intel/AMD MCA

The following patch reworks the MCA error telemetry handling inside Xen, 
  and shares code between the Intel and AMD implementations as much as 
possible.

I''ve had this patch sitting around for a while, but it wasn''t
ported to
-unstable yet. I finished porting and testing it, and am submitting it 
now, because the Intel folks want to go ahead and submit their new 
changes, so we agreed that I should push our changes first.

Brief explanation of the telemetry part: previously, the telemetry was 
accessed in a global array, with index variables used to access it. 
There were some issues with that: race conditions with regard to new 
machine checks (or CMCIs) coming in while handling the telemetry, and 
interaction with domains having been notified or not, which was a bit 
hairy. Our changes (I should say: Gavin Maltby''s changes, as he did the
bulk of this work for our 3.1 based tree, I merely ported/extended it to 
3.3 and beyond) make telemetry access transactional (think of a 
database). Also, the internal database updates are atomic, since the 
final commit is done by a pointer swap. There is a brief explanation of 
the mechanism in mctelem.h.This patch also removes dom0->domU 
notification, which is ok, since Intel''s upcoming changes will replace 
domU notification with a vMCE mechanism anyway.

The common code part is pretty much what it says. It defines a common 
MCE handler, with a few hooks for the special needs of the specific CPUs.

I''ve been told that Intel''s upcoming patch will need to make
some parts
of the common code specific to the Intel CPU again, but we''ll work 
together to use as much common code as possible.

- Frank


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

xen-devel-bounces@lists.xensource.com <> wrote:
> The following patch reworks the MCA error telemetry handling
> inside Xen,
>  and shares code between the Intel and AMD implementations as much as
> possible. 
> 
> I''ve had this patch sitting around for a while, but it
wasn''t
> ported to
> -unstable yet. I finished porting and testing it, and am submitting it
> now, because the Intel folks want to go ahead and submit their new
> changes, so we agreed that I should push our changes first.
> 
> Brief explanation of the telemetry part: previously, the telemetry was
> accessed in a global array, with index variables used to access it.
> There were some issues with that: race conditions with regard to new
> machine checks (or CMCIs) coming in while handling the telemetry, and
> interaction with domains having been notified or not, which was a bit
> hairy. Our changes (I should say: Gavin Maltby''s changes, as
> he did the
> bulk of this work for our 3.1 based tree, I merely
> ported/extended it to
> 3.3 and beyond) make telemetry access transactional (think of a
> database). Also, the internal database updates are atomic, since the
> final commit is done by a pointer swap. There is a brief
> explanation of
> the mechanism in mctelem.h.This patch also removes dom0->domU
> notification, which is ok, since Intel''s upcoming changes will
replace
> domU notification with a vMCE mechanism anyway.
> 
> The common code part is pretty much what it says. It defines a common
> MCE handler, with a few hooks for the special needs of the
> specific CPUs.
> 
> I''ve been told that Intel''s upcoming patch will need to
make
> some parts
> of the common code specific to the Intel CPU again, but we''ll work
> together to use as much common code as possible.
Yes, as shown in our previous patch, we do change the current MCA handler, the
main changes are followed:

1) Most importantly, we implement a softIRQ mechanism for post MCE handler. 
	The reason is, the #MC can happen in any time, that means: Firstly it is
spin-lock unsafe, some code like vcpu_schedule_lock_irq(v)  in current MCA
handler is sure to cause hang if that lock is already hold by a ISR; Secondly,
the execution context is uncertain, the "current " value in current
MCA handler maybe incorrect  (if set_current is interrupted by #MC), the page
ownership maybe wrong (if still in change under heap_lock protection) etc. I
remember this
	So our patch handling #MC is in two step. The MCA handler, which depends on the
execution context when MCA happen (like checking if it is in Xen context) and
especially it will bring all CPU to softIRQ context. The softIRQ handler (i.e.
post handler), which will be spin_lock safe, and all CPU is redenzvous, so it
can take more actions.

2) We implement a mechanism to handle the shared MSR resources similar to what
we have done in CMCI handler. As the Intel SDM stated, some MC resource is
shared by multiple logical CPU, we implement a ownership check.

3) As stated in linux  MCA handler, on Intel platforms machine check exceptions
are always broadcast to all CPUs, we add such support also.

We have no idea how the issues for item 2 and 3 are handled on other platform,
so we have no idea on how to do the common handler for it, hope Christoph can
provide more suggestion, or we can just keep them different for different
platform.

But I think for item 1, it is software related, so it can be a enhancement to
the common handler, the only thing I''m not sure is, if we need bring
all CPU to softIRQ context in all platform, maybe Christoph can give more idea.

Since we have get most consensus on the high level idea of MCA handler (i.e. Xen
take action instead of dom0, use vMCE for guest MCA etc, check discussion with
subject " [RFC] RAS(Part II)--MCA enalbing in XEN", the only thing
left is the detail method of how to pass the recover action information to
dom0), maybe we can turn to this second level discussion of how to enhance the
(common) MCA handler.

Thanks
-- Yunhong Jiang
> 
> - Frank
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Hi, Frank

I am now doing some tests based on latest Intel platform for this patch 
since CMCI needs some owner_checking and only the owned CPU will report 
the error.

Without the patch, when CMCI happened, since CPU0 is the owner of bank8, 
so when do checking, Only CPU0 will report the error.

Below is the correct log
(XEN) CMCI: cmci_intr happen on CPU3
[root@lke-ep inject]# (XEN) CMCI: cmci_intr happen on CPU2
(XEN) CMCI: cmci_intr happen on CPU0
(XEN) CMCI: cmci_intr happen on CPU1
(XEN) mcheck_poll: bank8 CPU0 status[cc0000800001009f]
(XEN) mcheck_poll: CPU0, SOCKET0, CORE0, APICID[0], thread[0]
(XEN) MCE: The hardware reports a non fatal, correctable incident occured on CPU
 0.

After applied your patch, I found all CPUs will report the error. 
Below is the log
(XEN) MCE: The hardware reports a non fatal, correctable i
ncident occured on CPU 0.
(XEN) MCE: The hardware reports a non fatal, correctable incident occured on CPU
 2.
(XEN) MCE: The hardware reports a non fatal, correctable incident occured on CPU
 3.
(XEN) MCE: The hardware reports a non fatal, correctable incident occured on CPU
 1.
(XEN) Bank 8: cc0000c00001009f<1>Bank 8: 8c0000400001009f<1>Bank 8:
cc0001c00001
009f<1>MCE: The hardware reports a non fatal, correctable incident occured
on CP
U 0.

I noticed your patch has passed in the cmci_owner mask, I can't see the
reason since
this is really a big patch. I need some time to figure it out. 


Also we found the polling mechanism has some changes. My feeling is that this
patch is really too big.
We can't easily figured out the impaction to our checked-in codes right now.
Just wonder whether you could split this big patch into two parts :-)
part1: mce log telem mechanism and required mce_intel interfaces changes. So
that we can verify easily
whether the new interfaces works fine for our CMCI as well as non-fatal polling.
I guess this should not be a big work,
you can just modify the new telem interfaces machine_check_poll?
part2: common handler part. (including both CMCI parts and non-fatal polling
parts).

How do you think about it :-)

Thanks a lot for your help!
Criping



-----Original Message-----
From: xen-devel-bounces@lists.xensource.com
[mailto:xen-devel-bounces@lists.xensource.com] On Behalf Of Frank van der Linden
Sent: 2009年3月17日 7:28
To: xen-devel@lists.xensource.com
Subject: [Xen-devel] [PATCH] re-work MCA telemetry internals; use common code
for Intel/AMD MCA

The following patch reworks the MCA error telemetry handling inside Xen, 
  and shares code between the Intel and AMD implementations as much as 
possible.

I've had this patch sitting around for a while, but it wasn't ported to 
-unstable yet. I finished porting and testing it, and am submitting it 
now, because the Intel folks want to go ahead and submit their new 
changes, so we agreed that I should push our changes first.

Brief explanation of the telemetry part: previously, the telemetry was 
accessed in a global array, with index variables used to access it. 
There were some issues with that: race conditions with regard to new 
machine checks (or CMCIs) coming in while handling the telemetry, and 
interaction with domains having been notified or not, which was a bit 
hairy. Our changes (I should say: Gavin Maltby's changes, as he did the 
bulk of this work for our 3.1 based tree, I merely ported/extended it to 
3.3 and beyond) make telemetry access transactional (think of a 
database). Also, the internal database updates are atomic, since the 
final commit is done by a pointer swap. There is a brief explanation of 
the mechanism in mctelem.h.This patch also removes dom0->domU 
notification, which is ok, since Intel's upcoming changes will replace 
domU notification with a vMCE mechanism anyway.

The common code part is pretty much what it says. It defines a common 
MCE handler, with a few hooks for the special needs of the specific CPUs.

I've been told that Intel's upcoming patch will need to make some parts 
of the common code specific to the Intel CPU again, but we'll work 
together to use as much common code as possible.

- Frank

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Hi, Frank

We did some small test here, found the CMCI problem is caused by the
"mce_banks_owned" bitmap param passing.
When CMCI happened, we print the bitmap (in smp_cmci_interrupt) value, it's
correct (For cpu0, 16c).
When passing into mcheck_mca_logout, it turned to be "0xFFFF~~FFF"
which is wrong.
Only for your info -:)

Still, I suggest split the patch since this patch is realy big  -:)

Thanks a lot for your help!
Criping

-----Original Message-----
From: Ke, Liping 
Sent: 2009年3月17日 14:26
To: 'Frank van der Linden'; xen-devel@lists.xensource.com
Subject: RE: [Xen-devel] [PATCH] re-work MCA telemetry internals; use common
code for Intel/AMD MCA

Hi, Frank

I am now doing some tests based on latest Intel platform for this patch 
since CMCI needs some owner_checking and only the owned CPU will report 
the error.

Without the patch, when CMCI happened, since CPU0 is the owner of bank8, 
so when do checking, Only CPU0 will report the error.

Below is the correct log
(XEN) CMCI: cmci_intr happen on CPU3
[root@lke-ep inject]# (XEN) CMCI: cmci_intr happen on CPU2
(XEN) CMCI: cmci_intr happen on CPU0
(XEN) CMCI: cmci_intr happen on CPU1
(XEN) mcheck_poll: bank8 CPU0 status[cc0000800001009f]
(XEN) mcheck_poll: CPU0, SOCKET0, CORE0, APICID[0], thread[0]
(XEN) MCE: The hardware reports a non fatal, correctable incident occured on CPU
 0.

After applied your patch, I found all CPUs will report the error. 
Below is the log
(XEN) MCE: The hardware reports a non fatal, correctable i
ncident occured on CPU 0.
(XEN) MCE: The hardware reports a non fatal, correctable incident occured on CPU
 2.
(XEN) MCE: The hardware reports a non fatal, correctable incident occured on CPU
 3.
(XEN) MCE: The hardware reports a non fatal, correctable incident occured on CPU
 1.
(XEN) Bank 8: cc0000c00001009f<1>Bank 8: 8c0000400001009f<1>Bank 8:
cc0001c00001
009f<1>MCE: The hardware reports a non fatal, correctable incident occured
on CP
U 0.

I noticed your patch has passed in the cmci_owner mask, I can't see the
reason since
this is really a big patch. I need some time to figure it out. 


Also we found the polling mechanism has some changes. My feeling is that this
patch is really too big.
We can't easily figured out the impaction to our checked-in codes right now.
Just wonder whether you could split this big patch into two parts :-)
part1: mce log telem mechanism and required mce_intel interfaces changes. So
that we can verify easily
whether the new interfaces works fine for our CMCI as well as non-fatal polling.
I guess this should not be a big work,
you can just modify the new telem interfaces machine_check_poll?
part2: common handler part. (including both CMCI parts and non-fatal polling
parts).

How do you think about it :-)

Thanks a lot for your help!
Criping



-----Original Message-----
From: xen-devel-bounces@lists.xensource.com
[mailto:xen-devel-bounces@lists.xensource.com] On Behalf Of Frank van der Linden
Sent: 2009年3月17日 7:28
To: xen-devel@lists.xensource.com
Subject: [Xen-devel] [PATCH] re-work MCA telemetry internals; use common code
for Intel/AMD MCA

The following patch reworks the MCA error telemetry handling inside Xen, 
  and shares code between the Intel and AMD implementations as much as 
possible.

I've had this patch sitting around for a while, but it wasn't ported to 
-unstable yet. I finished porting and testing it, and am submitting it 
now, because the Intel folks want to go ahead and submit their new 
changes, so we agreed that I should push our changes first.

Brief explanation of the telemetry part: previously, the telemetry was 
accessed in a global array, with index variables used to access it. 
There were some issues with that: race conditions with regard to new 
machine checks (or CMCIs) coming in while handling the telemetry, and 
interaction with domains having been notified or not, which was a bit 
hairy. Our changes (I should say: Gavin Maltby's changes, as he did the 
bulk of this work for our 3.1 based tree, I merely ported/extended it to 
3.3 and beyond) make telemetry access transactional (think of a 
database). Also, the internal database updates are atomic, since the 
final commit is done by a pointer swap. There is a brief explanation of 
the mechanism in mctelem.h.This patch also removes dom0->domU 
notification, which is ok, since Intel's upcoming changes will replace 
domU notification with a vMCE mechanism anyway.

The common code part is pretty much what it says. It defines a common 
MCE handler, with a few hooks for the special needs of the specific CPUs.

I've been told that Intel's upcoming patch will need to make some parts 
of the common code specific to the Intel CPU again, but we'll work 
together to use as much common code as possible.

- Frank

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On Tuesday 17 March 2009 04:24:35 Jiang, Yunhong wrote:
> xen-devel-bounces@lists.xensource.com <> wrote:
> > The following patch reworks the MCA error telemetry handling
> > inside Xen,
> >  and shares code between the Intel and AMD implementations as much as
> > possible.
> >
> > I''ve had this patch sitting around for a while, but it
wasn''t
> > ported to
> > -unstable yet. I finished porting and testing it, and am submitting it
> > now, because the Intel folks want to go ahead and submit their new
> > changes, so we agreed that I should push our changes first.
> >
> > Brief explanation of the telemetry part: previously, the telemetry was
> > accessed in a global array, with index variables used to access it.
> > There were some issues with that: race conditions with regard to new
> > machine checks (or CMCIs) coming in while handling the telemetry, and
> > interaction with domains having been notified or not, which was a bit
> > hairy. Our changes (I should say: Gavin Maltby''s changes, as
> > he did the
> > bulk of this work for our 3.1 based tree, I merely
> > ported/extended it to
> > 3.3 and beyond) make telemetry access transactional (think of a
> > database). Also, the internal database updates are atomic, since the
> > final commit is done by a pointer swap. There is a brief
> > explanation of
> > the mechanism in mctelem.h.This patch also removes dom0->domU
> > notification, which is ok, since Intel''s upcoming changes
will replace
> > domU notification with a vMCE mechanism anyway.
> >
> > The common code part is pretty much what it says. It defines a common
> > MCE handler, with a few hooks for the special needs of the
> > specific CPUs.
> >
> > I''ve been told that Intel''s upcoming patch will need
to make
> > some parts
> > of the common code specific to the Intel CPU again, but we''ll
work
> > together to use as much common code as possible.
>
> Yes, as shown in our previous patch, we do change the current MCA handler,
> the main changes are followed:
>
> 1) Most importantly, we implement a softIRQ mechanism for post MCE handler.
> 	The reason is, the #MC can happen in any time, that means: Firstly it is
> spin-lock unsafe, some code like vcpu_schedule_lock_irq(v)  in current MCA
> handler is sure to cause hang if that lock is already hold by a ISR;
> Secondly, the execution context is uncertain, the "current "
value in
> current MCA handler maybe incorrect  (if set_current is interrupted by
> #MC), the page ownership maybe wrong (if still in change under heap_lock
> protection) etc. I remember this So our patch handling #MC is in two step.
> The MCA handler, which depends on the execution context when MCA happen
> (like checking if it is in Xen context) and especially it will bring all
> CPU to softIRQ context. The softIRQ handler (i.e. post handler), which will
> be spin_lock safe, and all CPU is redenzvous, so it can take more actions.
>
> 2) We implement a mechanism to handle the shared MSR resources similar to
> what we have done in CMCI handler. As the Intel SDM stated, some MC
> resource is shared by multiple logical CPU, we implement a ownership check.
>
> 3) As stated in linux  MCA handler, on Intel platforms machine check
> exceptions are always broadcast to all CPUs, we add such support also.
>
> We have no idea how the issues for item 2 and 3 are handled on other
> platform, so we have no idea on how to do the common handler for it, hope
> Christoph can provide more suggestion, or we can just keep them different
> for different platform.
>
> But I think for item 1, it is software related, so it can be a enhancement
> to the common handler, the only thing I''m not sure is, if we need
bring all
> CPU to softIRQ context in all platform, maybe Christoph can give more idea.
The featureset of AMD Athlon K7 and Intel Pentium III are the common
denominator on x86. This is what can go into the common code.
In order to utilize features from newer cpus, allow to register function 
pointers and call them from the common code. Look into the amd_k8.c
and amd_f10.c for example code. I register a function pointer to read
the new MSRs. It can be easily extended to utilize features of coming CPUs.

> Since we have get most consensus on the high level idea of MCA handler
> (i.e. Xen take action instead of dom0, use vMCE for guest MCA etc, check
> discussion with subject " [RFC] RAS(Part II)--MCA enalbing in
XEN", the
> only thing left is the detail method of how to pass the recover action
> information to dom0), maybe we can turn to this second level discussion of
> how to enhance the (common) MCA handler.
>
> Thanks
> -- Yunhong Jiang
>
> > - Frank

-- 
---to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Karl-Hammerschmidt-Str. 34, 85609 Dornach b. Muenchen
Geschaeftsfuehrer: Jochen Polster, Thomas M. McCoy, Giuliano Meroni
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Ke, Liping wrote:
> Hi, Frank
> 
> We did some small test here, found the CMCI problem is caused by the
"mce_banks_owned" bitmap param passing.
> When CMCI happened, we print the bitmap (in smp_cmci_interrupt) value,
it''s correct (For cpu0, 16c).
> When passing into mcheck_mca_logout, it turned to be
"0xFFFF~~FFF" which is wrong.
> Only for your info -:)
> 
> Still, I suggest split the patch since this patch is realy big  -:)
Ok, I''ll split up the patch.. the common code part will probably be 
still fairly big, though. Hopefully I can post it by the end of the day.

- Frank

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Ke, Liping wrote:
> Hi, Frank
> 
> We did some small test here, found the CMCI problem is caused by the
"mce_banks_owned" bitmap param passing.
> When CMCI happened, we print the bitmap (in smp_cmci_interrupt) value,
it''s correct (For cpu0, 16c).
> When passing into mcheck_mca_logout, it turned to be
"0xFFFF~~FFF" which is wrong.
> Only for your info -:)
> 
> Still, I suggest split the patch since this patch is realy big  -:)
> 
> Thanks a lot for your help!
> Criping
Ok, since the patch is already in (thanks Keir!), let''s work out any 
issues you may see with it.

When you say that the mce_banks_owned value is wrong in 
mcheck_mca_logout, do you mean that the parameter passing somehow went 
wrong? Is the value ok right before the call, but not in 
mcheck_mca_logout itself? That would be strange.

You mentioned the polling code: it didn''t actually change much, in
fact,
the general polling code now is pretty much the same as the polling code 
that was originally in mce_intel.c. That code is very similar. Some AMD 
code contained AMD-specific things, and has not been touched (it still 
has its own poll function).

In general, the change is:

* machine_check_poll (going over the banks to look for non-fatal errors) 
and the loops inside the mcheck handlers that also walk the MC banks, 
have been combined in one function, which is mcheck_mca_logout. 
mcheck_mca_logout still takes a context argument, as machine_check_poll did.

* The mcheck handlers themselves mostly just call mcheck_cmn_handler, 
which calls mcheck_mca_logout, looks at the context, and then decides 
what to do.

* The mctelem structure as returned by mcheck_mca_logout, contains the 
relevant telemetry for the current error. If it should be put in the 
"database" for dom0 retrieval, use mctelem_commit to put it there. 
Otherwise, the usual course of action is to print the values and uses 
mctelem_dismiss to discard the information.

There is one thing that needs to be addressed: in your Intel-specific 
code, you do not reset the per-bank status in the case of a fatal error, 
so that the BIOS may look at the MSRs. mcheck_mca_logout does always 
reset the values. The best way to fix this is probably to pass in a flag 
to mcheck_mca_logout to tell it whether it should reset the status MSRs 
or not. The caller can then decide if they should be reset later.

Let me know if you have any more issues, I''ll be happy to help merge 
your code and test it.

- Frank



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Christoph Egger <mailto:Christoph.Egger@amd.com>
wrote:
> On Tuesday 17 March 2009 04:24:35 Jiang, Yunhong wrote:
>> xen-devel-bounces@lists.xensource.com <> wrote:
>>> The following patch reworks the MCA error telemetry handling inside
Xen,
>>>  and shares code between the Intel and AMD implementations as much
as
>>> possible. 
>>> 
>>> I''ve had this patch sitting around for a while, but it
wasn''t ported to
>>> -unstable yet. I finished porting and testing it, and am submitting
it
>>> now, because the Intel folks want to go ahead and submit their new
>>> changes, so we agreed that I should push our changes first.
>>> 
>>> Brief explanation of the telemetry part: previously, the telemetry
was
>>> accessed in a global array, with index variables used to access it.
>>> There were some issues with that: race conditions with regard to
new
>>> machine checks (or CMCIs) coming in while handling the telemetry,
and
>>> interaction with domains having been notified or not, which was a
bit
>>> hairy. Our changes (I should say: Gavin Maltby''s changes,
as
>>> he did the
>>> bulk of this work for our 3.1 based tree, I merely
>>> ported/extended it to
>>> 3.3 and beyond) make telemetry access transactional (think of a
>>> database). Also, the internal database updates are atomic, since
the
>>> final commit is done by a pointer swap. There is a brief
explanation of
>>> the mechanism in mctelem.h.This patch also removes dom0->domU
>>> notification, which is ok, since Intel''s upcoming changes
will replace
>>> domU notification with a vMCE mechanism anyway.
>>> 
>>> The common code part is pretty much what it says. It defines a
common
>>> MCE handler, with a few hooks for the special needs of the specific
CPUs.
>>> 
>>> I''ve been told that Intel''s upcoming patch will
need to make
>>> some parts
>>> of the common code specific to the Intel CPU again, but
we''ll work
>>> together to use as much common code as possible.
>> 
>> Yes, as shown in our previous patch, we do change the current MCA
handler,
>> the main changes are followed:
>> 
>> 1) Most importantly, we implement a softIRQ mechanism for post MCE
handler.
>> 	The reason is, the #MC can happen in any time, that means: Firstly it
is
>> spin-lock unsafe, some code like vcpu_schedule_lock_irq(v) in current
MCA
>> handler is sure to cause hang if that lock is already hold by a ISR;
>> Secondly, the execution context is uncertain, the "current "
value in
>> current MCA handler maybe incorrect  (if set_current is interrupted by
>> #MC), the page ownership maybe wrong (if still in change under
heap_lock
>> protection) etc. I remember this So our patch handling #MC is in two
step.
>> The MCA handler, which depends on the execution context when MCA happen
>> (like checking if it is in Xen context) and especially it will bring
all
>> CPU to softIRQ context. The softIRQ handler (i.e. post handler), which
will
>> be spin_lock safe, and all CPU is redenzvous, so it can take more
actions.
>> 
>> 2) We implement a mechanism to handle the shared MSR resources similar
to
>> what we have done in CMCI handler. As the Intel SDM stated, some MC
>> resource is shared by multiple logical CPU, we implement a ownership
check.
>> 
>> 3) As stated in linux  MCA handler, on Intel platforms machine check
>> exceptions are always broadcast to all CPUs, we add such support also.
>> 
>> We have no idea how the issues for item 2 and 3 are handled on other
>> platform, so we have no idea on how to do the common handler for it,
hope
>> Christoph can provide more suggestion, or we can just keep them
different
>> for different platform. 
>> 
>> But I think for item 1, it is software related, so it can be a
enhancement
>> to the common handler, the only thing I''m not sure is, if we
need bring all
>> CPU to softIRQ context in all platform, maybe Christoph can give more
idea.
> 
> The featureset of AMD Athlon K7 and Intel Pentium III are the common
> denominator on x86. This is what can go into the common code.
> In order to utilize features from newer cpus, allow to
> register function
> pointers and call them from the common code. Look into the amd_k8.c
> and amd_f10.c for example code. I register a function pointer to read
> the new MSRs. It can be easily extended to utilize features of coming CPUs.
I suspect the difference between the mce_intel.c and amd_xxx.c will be far more
than the difference between amd_k8.c and amd_f10.c. For example, the issue 2/3
listed above (will it exists on your side?)

And how about the softIRQ mechanism? How do you think about apply it to both
side?

Thanks
Yunhong Jiang
> 
> 
>> Since we have get most consensus on the high level idea of MCA handler
>> (i.e. Xen take action instead of dom0, use vMCE for guest MCA etc,
check
>> discussion with subject " [RFC] RAS(Part II)--MCA enalbing in
XEN", the
>> only thing left is the detail method of how to pass the recover action
>> information to dom0), maybe we can turn to this second level discussion
of
>> how to enhance the (common) MCA handler.
>> 
>> Thanks
>> -- Yunhong Jiang
>> 
>>> - Frank
> 
> 
> --
> ---to satisfy European Law for business letters:
> Advanced Micro Devices GmbH
> Karl-Hammerschmidt-Str. 34, 85609 Dornach b. Muenchen
> Geschaeftsfuehrer: Jochen Polster, Thomas M. McCoy, Giuliano Meroni
> Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
> Registergericht Muenchen, HRB Nr. 43632
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

xen-devel-bounces@lists.xensource.com <> wrote:
> Ke, Liping wrote:
>> Hi, Frank
>> 
>> We did some small test here, found the CMCI problem is
> caused by the "mce_banks_owned" bitmap param passing.
>> When CMCI happened, we print the bitmap (in
> smp_cmci_interrupt) value, it''s correct (For cpu0, 16c).
>> When passing into mcheck_mca_logout, it turned to be
"0xFFFF~~FFF" which
>> is wrong. Only for your info -:) 
>> 
>> Still, I suggest split the patch since this patch is realy big  -:)
>> 
>> Thanks a lot for your help!
>> Criping
> 
> Ok, since the patch is already in (thanks Keir!), let''s work out
any issues
> you may see with it. 
> 
> When you say that the mce_banks_owned value is wrong in
> mcheck_mca_logout, do you mean that the parameter passing somehow went
> wrong? Is the value ok right before the call, but not in
> mcheck_mca_logout itself? That would be strange.
> 
> You mentioned the polling code: it didn''t actually change
> much, in fact,
> the general polling code now is pretty much the same as the
> polling code
> that was originally in mce_intel.c. That code is very similar.
> Some AMD
> code contained AMD-specific things, and has not been touched (it still has
> its own poll function). 
> 
> In general, the change is:
> 
> * machine_check_poll (going over the banks to look for
> non-fatal errors)
> and the loops inside the mcheck handlers that also walk the MC banks,
> have been combined in one function, which is mcheck_mca_logout.
> mcheck_mca_logout still takes a context argument, as machine_check_poll
did.
> 
> * The mcheck handlers themselves mostly just call mcheck_cmn_handler,
> which calls mcheck_mca_logout, looks at the context, and then decides what
> to do. 
> 
> * The mctelem structure as returned by mcheck_mca_logout, contains the
> relevant telemetry for the current error. If it should be put in the
> "database" for dom0 retrieval, use mctelem_commit to put it
there.
> Otherwise, the usual course of action is to print the values and uses
> mctelem_dismiss to discard the information.
> 
> There is one thing that needs to be addressed: in your Intel-specific
> code, you do not reset the per-bank status in the case of a
> fatal error,
> so that the BIOS may look at the MSRs. mcheck_mca_logout does always
> reset the values. The best way to fix this is probably to pass
> in a flag
> to mcheck_mca_logout to tell it whether it should reset the
> status MSRs
> or not. The caller can then decide if they should be reset later.
> 
> Let me know if you have any more issues, I''ll be happy to help
merge your
> code and test it. 
We will do more test for it and also check the per-bank status. Originally we
hope:

a) settle down the interface in 3.4 release for MCA, so that it will not be
dramatically changed anymore, including the telemetry interface for CE and the
interface betwwen MCA handler and softIRQ handler (i.e. what we get consensused
in the mailing list), and your telemetry changes that already in. since usually
people based their work on some stable release.

b) If we can get consensus, finished the softIRQ mechanism so that it will get a
solid base. The spin_lock() and "current" usage in MCA handler is
really confusing.

But feature freeze now :-(

Or maybe someone can lobby Keir to accept these changes if Ke Liping and I can
finish these patches this week. Hope it should not be big if we can remove the
vMCE implementation cleanly.
> 
> - Frank
> 
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On 18/03/2009 16:07, "Jiang, Yunhong" <yunhong.jiang@intel.com>
wrote:
> But feature freeze now :-(
> 
> Or maybe someone can lobby Keir to accept these changes if Ke Liping and I
can
> finish these patches this week. Hope it should not be big if we can remove
the
> vMCE implementation cleanly.
If there''s a strong argument for it, we can stretch the feature window.
Particularly for something which is, frankly for now at least, a niche
feature.

 -- Keir



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel