>>> On 11.10.11 at 10:15, "Liu, Jinsong" <jinsong.liu@xxxxxxxxx> wrote:
> Jan Beulich wrote:
>>>>> On 08.10.11 at 10:29, "Jiang, Yunhong" <yunhong.jiang@xxxxxxxxx>
>>>>> wrote:
>>
>>>
>>>> -----Original Message-----
>>>> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
>>>> Sent: Friday, September 30, 2011 3:25 PM
>>>> To: Liu, Jinsong; Jiang, Yunhong
>>>> Cc: keir.xen@xxxxxxxxx; xen-devel@xxxxxxxxxxxxxxxxxxx
>>>> Subject: RE: [Xen-devel] [PATCH] X86 MCE: Add SRAR handler
>>>>
>>>>>>> On 30.09.11 at 04:51, "Jiang, Yunhong" <yunhong.jiang@xxxxxxxxx>
>>>>>>> wrote:
>>>>
>>>>>
>>>>>> -----Original Message-----
>>>>>> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
>>>>>> This made me look at the current source, and there I see in
>>>>>> mce_urgent_action()
>>>>>>
>>>>>> if ( !(gstatus & MCG_STATUS_RIPV) && !guest_mode(regs))
>>>>>> return -1;
>>>>>>
>>>>>> which I think should say ... _EIPV and use || instead. Thoughts?
>>>>>
>>>>> I think this code means, if the error happens in hypervisor mode
>>>>> (i.e. !guest_mode()), and RIPV indicate the RIP in stack can't be
>>>>> restarted, we have to panic.
>>>>
>>>> Then the guest_mode() check still lacks an extra check of EIPV, like
>>>>
>>>> if ( !(gstatus & MCG_STATUS_RIPV) &&
>>>> (!(gstatus & MCG_STATUS_EIPV) || !guest_mode(regs)))
>>>> return -1;
>>>>
>>>
>>> The RIPV is not related to the EIPV. RIPV means the context saved in
>>> the stack can't be restarted anymore. According to the SDM, RIPV
>>> means "execution can be restarted reliably at the instruction
>>> pointed to by the instruction pointer pushed on the stack". It's not
>>> about error happened synchronously or asynchronously. The point is,
>>> if the program is running in hypervisor context, and RIPV tells us
>>> that the program can't be restarted, we can't do anything but panic,
>>> because we can't switch context while we are in xen. So this code
>>> have nothing to do with EIPV.
>>
>> I continue to disagree (including the statement in your other
>> response): RIPV only tells us whether we can resume, not in which
>> context the error occurred. EIPV tells us whether, by looking at the
>> saved registers, we can determine the context that the error occurred
>> in. Since with !RIPV
>> we have to determine in what context the error occurred in order to
>> decide whether to panic or just kill a guest, we can't ignore EIPV
>> (and if it's not set we have to assume the worst case, since even if
>> the registers indicate guest mode the error may have occurred in
>> hypervisor context or accessing hypervisor structures [consider e.g. a
>> data load error during a GDT access]).
>>
>> Jan
>
> Yes, I agree EIPV=0 may indicate async error, but I think your solution
> *overkilled* most cases (i.e. the real guest instruction fetch error).
>
> Our idea is,
> * xen mce would flush prefetched instruction so we can delay handle it
> until if real need;
> * a h/w error will not disappear, but if it was not being *consumed*, it's
> OK for system keep going (like SRAO error which do not need s/w handle
> immediately);
>
> Suppose an async instruction fetch error (RIVP=EIVP=0), triggered at guest
> context but instruction prefetch hypervisor context. The scenario is,
> * at xen mce, the prefetched instruction has been flushed. xen mce handler
> needn't panic, instead it mark the page as broken page, then trigger vmce to
> guest;
If the prefetch was from Xen space (only in guest context), delivering a
vMCE to the guest is pointless (and perhaps confusing to the guest).
> * guest may kill app, kernel thread, guest itself, or whatever;
>
> The error is still an error, w/ 2 possibilities in the future:
> 1. it may not be consumed as an SRAR error, system keep going, h/w
> mechanism may detect a SRAO error (i.e. memroy scrub) at some time point and
> handled then;
> 2. it may be consumed at some time point and a SRAR error triggered again.
> At this time,
> 1). if srar occurred at hypervisor context, xen will panic. or,
> 2). if srar occurred at guest context, xen kill the guest as a malicious
> one (as what the 2nd patch do), and move the page to broken page list;
>
> Considering the rare possibility of the above case, I think it's acceptable
> to handle it in this way.
> Thoughts?
You're only discussing instruction fetches (which can be discarded), but
you're not covering the other example I gave (GDT access from guest
context - just like this is a ring-0 operations from the paging unit's pov,
this ought to be an out-of-context operation from MCE's perspective).
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
Home