One potential issue of shadow fault emulation

One potential issue of shadow fault emulation

[Jiang, Yunhong]

Currently shadow fault handler try to emulate up to four extra
instruction for PAE guest, to reduce vmexit times.

But there is a potential issue here: Consider the second instruction is
a change to virtual TPR register. In physical environment, if the TPR
acceleration is enabled, the cpu will try to access the
VIRTUAL_APIC_PAGE_ADDR set in the VMCS. However, when we do emulation,
we didn't cope with this situation, and will access the APIC_ACCESS_ADDR
page pointed by the shadow. This is sure cause problem to guest, usually
blue screen, and this issue will happen randomly depends on the content
in the  apic access page.

So how should we cope with such situation? Stop emulation or, continue
emulate , but access the virtual APIC page? Or any better idea?

Thanks
-- Yunhong Jiang

Re: One potential issue of shadow fault emulation

[Tim Deegan]

Hi,

At 22:58 +0800 on 21 Dec (1198277929), Jiang, Yunhong wrote:
> Currently shadow fault handler try to emulate up to four extra
> instruction for PAE guest, to reduce vmexit times.
> 
> But there is a potential issue here: Consider the second instruction is
> a change to virtual TPR register. In physical environment, if the TPR
> acceleration is enabled, the cpu will try to access the
> VIRTUAL_APIC_PAGE_ADDR set in the VMCS. However, when we do emulation,
> we didn't cope with this situation, and will access the APIC_ACCESS_ADDR
> page pointed by the shadow. This is sure cause problem to guest, usually
> blue screen, and this issue will happen randomly depends on the content
> in the  apic access page.
> 
> So how should we cope with such situation? Stop emulation or, continue
> emulate , but access the virtual APIC page? Or any better idea?

We should stop emulation.  Probably nobody writes the TPR between the
two halves of a PTE write. :) 

Cheers,

Tim.

-- 
Tim Deegan <Tim.Deegan@citrix.com>
Principal Software Engineer, Citrix Systems.
[Company #5334508: XenSource UK Ltd, reg'd c/o EC2Y 5EB, UK.]

RE: One potential issue of shadow fault emulation

[Jiang, Yunhong]

Tim Deegan <mailto:Tim.Deegan@citrix.com> wrote:
> Hi,
> 
> At 22:58 +0800 on 21 Dec (1198277929), Jiang, Yunhong wrote:
>> Currently shadow fault handler try to emulate up to four extra
>> instruction for PAE guest, to reduce vmexit times.
>> 
>> But there is a potential issue here: Consider the second instruction
is
>> a change to virtual TPR register. In physical environment, if the TPR
>> acceleration is enabled, the cpu will try to access the
>> VIRTUAL_APIC_PAGE_ADDR set in the VMCS. However, when we do
emulation,
>> we didn't cope with this situation, and will access the
APIC_ACCESS_ADDR
>> page pointed by the shadow. This is sure cause problem to guest,
usually
>> blue screen, and this issue will happen randomly depends on the
content in
>> the  apic access page. 
>> 
>> So how should we cope with such situation? Stop emulation or,
continue
>> emulate , but access the virtual APIC page? Or any better idea?
> 
> We should stop emulation.  Probably nobody writes the TPR between the
two
> halves of a PTE write. :) 

This did happen :)

> 
> Cheers,
> 
> Tim.
> 
> --
> Tim Deegan <Tim.Deegan@citrix.com>
> Principal Software Engineer, Citrix Systems.
> [Company #5334508: XenSource UK Ltd, reg'd c/o EC2Y 5EB, UK.]

Re: One potential issue of shadow fault emulation

[Tim Deegan]

At 00:05 +0800 on 22 Dec (1198281908), Jiang, Yunhong wrote:
> >> So how should we cope with such situation? Stop emulation or,
> continue
> >> emulate , but access the virtual APIC page? Or any better idea?
> > 
> > We should stop emulation.  Probably nobody writes the TPR between the
> two
> > halves of a PTE write. :) 
> 
> This did happen :)

OK.  I've learned something new today. :)

I think it's cleanest to just fail the emulation if we find we're
writing to the vlapic, rather than adding special-case code to detect
and handle TPR writes within the page.  Unless this is a particularly
common path and would have a measurable performance impact for the extra
VMEXIT.

Cheers,

Tim.

-- 
Tim Deegan <Tim.Deegan@citrix.com>
Principal Software Engineer, Citrix Systems.
[Company #5334508: XenSource UK Ltd, reg'd c/o EC2Y 5EB, UK.]

Re: One potential issue of shadow fault emulation

[Kurt Hackel]

Hi,

On Fri, Dec 21, 2007 at 10:58:49PM +0800, Jiang, Yunhong wrote:
> Currently shadow fault handler try to emulate up to four extra
> instruction for PAE guest, to reduce vmexit times.
> 
> But there is a potential issue here: Consider the second instruction is
> a change to virtual TPR register. In physical environment, if the TPR
> acceleration is enabled, the cpu will try to access the
> VIRTUAL_APIC_PAGE_ADDR set in the VMCS. However, when we do emulation,
> we didn't cope with this situation, and will access the APIC_ACCESS_ADDR
> page pointed by the shadow. This is sure cause problem to guest, usually
> blue screen, and this issue will happen randomly depends on the content
> in the  apic access page.
> 
> So how should we cope with such situation? Stop emulation or, continue
> emulate , but access the virtual APIC page? Or any better idea?
> 
> Thanks
> -- Yunhong Jiang

I don't know if I'm hitting the same problem, but I also have a pretty
serious issue with changeset 15199.  In our case we see RHEL3U8 and
RHEL4U4 guests with at least 2 vcpus will consistently run into errors
on boot when fscking like so:
=====
EXT3-fs error (device ide0(3,3)): ext3_free_blocks: bit already cleared for block 66191
EXT3-fs error (device ide0(3,3)): ext3_free_inode: bit already cleared for inode 29267
EXT3-fs error (device ide0(3,3)): ext3_free_blocks: bit already cleared for block 66147
EXT3-fs error (device ide0(3,1)): ext3_free_inode: bit already cleared for inode 32388 
=====
Dropping back to 1 vcpu or a uniprocessor kernel alleviates the problem.

Bisecting we found that this was caused by cs 15199, and this was
serious enough for us with 32bit PAE hvm guests that we ended up backing
that changeset out for our release.  Was in the middle of trying the
same test out with 3.1.3-rc1-pre when I saw this thread :)  Looks like
3.2 has a different behavior and ends up spinning at 100% cpu when
fscking.

The performance impact of dropping this patch is severe (pagefaults
about 7x more expensive than the same 64bit kernel) so I'd like to help
where I can on this.

Thanks
kurt

-- 

Re: One potential issue of shadow fault emulation

[Keir Fraser]

On 21/12/07 17:21, "Tim Deegan" <Tim.Deegan@citrix.com> wrote:

> I think it's cleanest to just fail the emulation if we find we're
> writing to the vlapic, rather than adding special-case code to detect
> and handle TPR writes within the page.  Unless this is a particularly
> common path and would have a measurable performance impact for the extra
> VMEXIT.

Probably gfn_to_mfn() should explicitly check for vmx_apic_access_mfn and
return INVALID_MFN instead. That will fix all emulation routines by causing
them to take their mmio path (or bail if they don't have them) which is the
correct behaviour for this case.

This is certainly important enough to fix for 3.2.0 (and 3.1.3).

 -- Keir

RE: Re: One potential issue of shadow fault emulation

[Cui, Dexuan]

I believe this emulation issue accounts for several weird vTPR-related
bugs.

-- Dexuan

-----Original Message-----
From: xen-devel-bounces@lists.xensource.com
[mailto:xen-devel-bounces@lists.xensource.com] On Behalf Of Keir Fraser
Sent: Saturday, December 22, 2007 5:54 AM
To: Tim Deegan; Jiang, Yunhong
Cc: xen-devel@lists.xensource.com
Subject: [Xen-devel] Re: One potential issue of shadow fault emulation

On 21/12/07 17:21, "Tim Deegan" <Tim.Deegan@citrix.com> wrote:

> I think it's cleanest to just fail the emulation if we find we're
> writing to the vlapic, rather than adding special-case code to detect
> and handle TPR writes within the page.  Unless this is a particularly
> common path and would have a measurable performance impact for the
extra
> VMEXIT.

Probably gfn_to_mfn() should explicitly check for vmx_apic_access_mfn
and
return INVALID_MFN instead. That will fix all emulation routines by
causing
them to take their mmio path (or bail if they don't have them) which is
the
correct behaviour for this case.

This is certainly important enough to fix for 3.2.0 (and 3.1.3).

 -- Keir

Re: One potential issue of shadow fault emulation

[Tim Deegan]

At 21:54 +0000 on 21 Dec (1198274059), Keir Fraser wrote:
> Probably gfn_to_mfn() should explicitly check for vmx_apic_access_mfn and
> return INVALID_MFN instead. That will fix all emulation routines by causing
> them to take their mmio path (or bail if they don't have them) which is the
> correct behaviour for this case.

That will disable the VMX TPR optimization too -- the p2m lookup needs
to return this magic page when the shadow propagation code looks it up
but not when other callers look it up.  Maybe the existing scheme for
mapping MMIO pages directly would do; mark the page as direct MMIO
instead of as RAM?  Otherwise at least one of the shadow propagation
code and the emulator needs to special-case the vlapic page.

Cheers,

Tim.

-- 
Tim Deegan <Tim.Deegan@citrix.com>
Principal Software Engineer, Citrix Systems.
[Company #5334508: XenSource UK Ltd, reg'd c/o EC2Y 5EB, UK.]

Re: One potential issue of shadow fault emulation

[Keir Fraser]

On 24/12/07 17:57, "Tim Deegan" <Tim.Deegan@citrix.com> wrote:

> At 21:54 +0000 on 21 Dec (1198274059), Keir Fraser wrote:
>> Probably gfn_to_mfn() should explicitly check for vmx_apic_access_mfn and
>> return INVALID_MFN instead. That will fix all emulation routines by causing
>> them to take their mmio path (or bail if they don't have them) which is the
>> correct behaviour for this case.
> 
> That will disable the VMX TPR optimization too -- the p2m lookup needs
> to return this magic page when the shadow propagation code looks it up
> but not when other callers look it up.  Maybe the existing scheme for
> mapping MMIO pages directly would do; mark the page as direct MMIO
> instead of as RAM?  Otherwise at least one of the shadow propagation
> code and the emulator needs to special-case the vlapic page.

There are quite a few users of gfn-to-mfn functionality and only one of them
wants the magic page. So I think it'd be cleaner to push the vlapic check a
bit deeper than just the shadow emulation callbacks.

Can we wrap gfn_to_mfn for export outside the shadow code, get all
non-shadow propagation callers to use the wrapper, and stick the vlapic page
check in that? Or equivalently, move the current gfn_to_mfn code into a new
shadow-internal function that only the shadow-propagation code calls
directly?

 -- Keir

Re: Re: One potential issue of shadow fault emulation

[Keir Fraser]

On 24/12/07 18:27, "Keir Fraser" <Keir.Fraser@cl.cam.ac.uk> wrote:

>> That will disable the VMX TPR optimization too -- the p2m lookup needs
>> to return this magic page when the shadow propagation code looks it up
>> but not when other callers look it up.  Maybe the existing scheme for
>> mapping MMIO pages directly would do; mark the page as direct MMIO
>> instead of as RAM?  Otherwise at least one of the shadow propagation
>> code and the emulator needs to special-case the vlapic page.
> 
> There are quite a few users of gfn-to-mfn functionality and only one of them
> wants the magic page. So I think it'd be cleaner to push the vlapic check a
> bit deeper than just the shadow emulation callbacks.

I should have read your email a bit more closely -- fixing this via the
typed p2m table in 3.2 would be neat. Or even not including the magic page
in the p2m at all and explicitly special-casing inside the shadow
propagation code would be okay.

 -- Keir

Re: One potential issue of shadow fault emulation

[Keir Fraser]

Can you please try xen-unstable c/s 16663. This implements Tim's good idea
of mapping the apic page in the p2m with type mmio-direct rather than type
ram. I had to do some rejigging in the changeset before that so that
gfn->mfn lookup failures do not inject #PF into the guest. I've done enough
testing around this code to be confident it should work, but I don't
actually have a test machine to hand with this feature to do a proper
full-confidence test.

We'll need a different patch for xen-3.1-testing, which is going to be
awkward since it doesn't have a typed p2m table. Probably __hvm_copy() and
emulate_map_dest() will need to explicitly check for the apic access page.
That, plus avoidance of injecting #PF in this case, should work okay.

 -- Keir

On 21/12/07 14:58, "Jiang, Yunhong" <yunhong.jiang@intel.com> wrote:

> Currently shadow fault handler try to emulate up to four extra
> instruction for PAE guest, to reduce vmexit times.
> 
> But there is a potential issue here: Consider the second instruction is
> a change to virtual TPR register. In physical environment, if the TPR
> acceleration is enabled, the cpu will try to access the
> VIRTUAL_APIC_PAGE_ADDR set in the VMCS. However, when we do emulation,
> we didn't cope with this situation, and will access the APIC_ACCESS_ADDR
> page pointed by the shadow. This is sure cause problem to guest, usually
> blue screen, and this issue will happen randomly depends on the content
> in the  apic access page.
> 
> So how should we cope with such situation? Stop emulation or, continue
> emulate , but access the virtual APIC page? Or any better idea?
> 
> Thanks
> -- Yunhong Jiang

RE: Re: One potential issue of shadow fault emulation

[Cui, Dexuan]

Hi Keir,
I made tests on 2 kinds of platforms, and the previous weird
vTPR-related issues disappear using the c/s 16663; I believe the issues
have been fixed by the c/s. 

Thanks!

-- Dexuan

-----Original Message-----
From: xen-devel-bounces@lists.xensource.com
[mailto:xen-devel-bounces@lists.xensource.com] On Behalf Of Keir Fraser
Sent: Thursday, December 27, 2007 8:24 PM
To: Jiang, Yunhong; Tim Deegan
Cc: xen-devel@lists.xensource.com
Subject: [Xen-devel] Re: One potential issue of shadow fault emulation 

Can you please try xen-unstable c/s 16663. This implements Tim's good
idea
of mapping the apic page in the p2m with type mmio-direct rather than
type
ram. I had to do some rejigging in the changeset before that so that
gfn->mfn lookup failures do not inject #PF into the guest. I've done
enough
testing around this code to be confident it should work, but I don't
actually have a test machine to hand with this feature to do a proper
full-confidence test.

We'll need a different patch for xen-3.1-testing, which is going to be
awkward since it doesn't have a typed p2m table. Probably __hvm_copy()
and
emulate_map_dest() will need to explicitly check for the apic access
page.
That, plus avoidance of injecting #PF in this case, should work okay.

 -- Keir

On 21/12/07 14:58, "Jiang, Yunhong" <yunhong.jiang@intel.com> wrote:

> Currently shadow fault handler try to emulate up to four extra
> instruction for PAE guest, to reduce vmexit times.
> 
> But there is a potential issue here: Consider the second instruction
is
> a change to virtual TPR register. In physical environment, if the TPR
> acceleration is enabled, the cpu will try to access the
> VIRTUAL_APIC_PAGE_ADDR set in the VMCS. However, when we do emulation,
> we didn't cope with this situation, and will access the
APIC_ACCESS_ADDR
> page pointed by the shadow. This is sure cause problem to guest,
usually
> blue screen, and this issue will happen randomly depends on the
content
> in the  apic access page.
> 
> So how should we cope with such situation? Stop emulation or, continue
> emulate , but access the virtual APIC page? Or any better idea?
> 
> Thanks
> -- Yunhong Jiang



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

Re: Re: One potential issue of shadow fault emulation

[Keir Fraser]

Great! Can you guys please also test xen-3.1-testing c/s 15572. This is the
equivalent, but rather different, fix for 3.1 branch.

 Thanks,
 Keir

On 27/12/07 15:30, "Cui, Dexuan" <dexuan.cui@intel.com> wrote:

> Hi Keir,
> I made tests on 2 kinds of platforms, and the previous weird
> vTPR-related issues disappear using the c/s 16663; I believe the issues
> have been fixed by the c/s.
> 
> Thanks!
> 
> -- Dexuan
> 
> -----Original Message-----
> From: xen-devel-bounces@lists.xensource.com
> [mailto:xen-devel-bounces@lists.xensource.com] On Behalf Of Keir Fraser
> Sent: Thursday, December 27, 2007 8:24 PM
> To: Jiang, Yunhong; Tim Deegan
> Cc: xen-devel@lists.xensource.com
> Subject: [Xen-devel] Re: One potential issue of shadow fault emulation
> 
> Can you please try xen-unstable c/s 16663. This implements Tim's good
> idea
> of mapping the apic page in the p2m with type mmio-direct rather than
> type
> ram. I had to do some rejigging in the changeset before that so that
> gfn->mfn lookup failures do not inject #PF into the guest. I've done
> enough
> testing around this code to be confident it should work, but I don't
> actually have a test machine to hand with this feature to do a proper
> full-confidence test.
> 
> We'll need a different patch for xen-3.1-testing, which is going to be
> awkward since it doesn't have a typed p2m table. Probably __hvm_copy()
> and
> emulate_map_dest() will need to explicitly check for the apic access
> page.
> That, plus avoidance of injecting #PF in this case, should work okay.
> 
>  -- Keir
> 
> On 21/12/07 14:58, "Jiang, Yunhong" <yunhong.jiang@intel.com> wrote:
> 
>> Currently shadow fault handler try to emulate up to four extra
>> instruction for PAE guest, to reduce vmexit times.
>> 
>> But there is a potential issue here: Consider the second instruction
> is
>> a change to virtual TPR register. In physical environment, if the TPR
>> acceleration is enabled, the cpu will try to access the
>> VIRTUAL_APIC_PAGE_ADDR set in the VMCS. However, when we do emulation,
>> we didn't cope with this situation, and will access the
> APIC_ACCESS_ADDR
>> page pointed by the shadow. This is sure cause problem to guest,
> usually
>> blue screen, and this issue will happen randomly depends on the
> content
>> in the  apic access page.
>> 
>> So how should we cope with such situation? Stop emulation or, continue
>> emulate , but access the virtual APIC page? Or any better idea?
>> 
>> Thanks
>> -- Yunhong Jiang
> 
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel

RE: Re: One potential issue of shadow fault emulation

[Cui, Dexuan]

Hi Keir,
I tested xen-3.1-testing c/s 15572. It also fixes the vTPR issues. 

-- Dexuan

Keir Fraser wrote:
> Great! Can you guys please also test xen-3.1-testing c/s 15572. This
> is the equivalent, but rather different, fix for 3.1 branch.
> 
>  Thanks,
>  Keir
> 
> On 27/12/07 15:30, "Cui, Dexuan" <dexuan.cui@intel.com> wrote:
> 
>> Hi Keir,
>> I made tests on 2 kinds of platforms, and the previous weird
>> vTPR-related issues disappear using the c/s 16663; I believe the
>> issues have been fixed by the c/s. 
>> 
>> Thanks!
>> 
>> -- Dexuan
>> 
>> -----Original Message-----
>> From: xen-devel-bounces@lists.xensource.com
>> [mailto:xen-devel-bounces@lists.xensource.com] On Behalf Of Keir
>> Fraser Sent: Thursday, December 27, 2007 8:24 PM
>> To: Jiang, Yunhong; Tim Deegan
>> Cc: xen-devel@lists.xensource.com
>> Subject: [Xen-devel] Re: One potential issue of shadow fault
>> emulation 
>> 
>> Can you please try xen-unstable c/s 16663. This implements Tim's
>> good idea of mapping the apic page in the p2m with type mmio-direct
>> rather than type ram. I had to do some rejigging in the changeset
>> before that so that gfn->mfn lookup failures do not inject #PF into
>> the guest. I've done enough testing around this code to be confident
>> it should work, but I don't actually have a test machine to hand
>> with this feature to do a proper full-confidence test. 
>> 
>> We'll need a different patch for xen-3.1-testing, which is going to
>> be awkward since it doesn't have a typed p2m table. Probably
>> __hvm_copy() and emulate_map_dest() will need to explicitly check
>> for the apic access page. That, plus avoidance of injecting #PF in
>> this case, should work okay. 
>> 
>>  -- Keir
>> 
>> On 21/12/07 14:58, "Jiang, Yunhong" <yunhong.jiang@intel.com> wrote:
>> 
>>> Currently shadow fault handler try to emulate up to four extra
>>> instruction for PAE guest, to reduce vmexit times.
>>> 
>>> But there is a potential issue here: Consider the second
>>> instruction is a change to virtual TPR register. In physical
>>> environment, if the TPR acceleration is enabled, the cpu will try
>>> to access the VIRTUAL_APIC_PAGE_ADDR set in the VMCS. However, when
>>> we do emulation, we didn't cope with this situation, and will
>>> access the APIC_ACCESS_ADDR page pointed by the shadow. This is
>>> sure cause problem to guest, usually blue screen, and this issue
>>> will happen randomly depends on the content in the  apic access
>>> page.  
>>> 
>>> So how should we cope with such situation? Stop emulation or,
>>> continue emulate , but access the virtual APIC page? Or any better
>>> idea? 
>>> 
>>> Thanks
>>> -- Yunhong Jiang
>> 
>> 
>> 
>> _______________________________________________
>> Xen-devel mailing list
>> Xen-devel@lists.xensource.com
>> http://lists.xensource.com/xen-devel



-- Dexuan