WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

Re: [Xen-devel] Re: Possible regression in "x86-64: reduce range spanned

At 11:56 +0000 on 11 Dec (1260532578), Simon Horman wrote:
> > Hah.  That explains why we're emulating.  The guest has CR0.WP clear,
> > and this was a write fault that wouldn't have happened on real hardware,
> > so we need to emulate it because we can't actually disable CR0.WP or the
> > shadow pagetables stop working altogether.
> > 
> > In fact in this case we don't need to emulate it because retrying would
> > be good enough, but in the general case we might.
> 
> I'm not sure that I understand why that is true.

Which part?  We need to emulate writes when the guest's CR0.WP is 0 because 
- we can't just set CR0.WP=0 or our write-protection of shadowed
  pagetables goes away. 
- we can't just use writeable shadow PTEs for read-only guest PTEs 
  because (a) other vcpus mihgt have CR0.WP==1 (this does happen 
  with virus scanner software on Windows) and (b) you can't 
  properly express "read-only in userspace, read-write in kernel".

All we need to do is retry because in this particular case although 
WP=0 the PTE is in fact read/write; it was just missing from the
shadows.

> Although I'm not really that familiar with the code in question,
> I think that I have a preference for both guarding the goto emulate
> and adding an ASSERT to sh_remove_shadows(). That is:
> 
> Index: xen-unstable.hg/xen/arch/x86/mm/shadow/common.c
> ===================================================================
> --- xen-unstable.hg.orig/xen/arch/x86/mm/shadow/common.c      2009-12-11 
> 20:31:48.000000000 +0900
> +++ xen-unstable.hg/xen/arch/x86/mm/shadow/common.c   2009-12-11 
> 20:48:48.000000000 +0900
> @@ -2752,6 +2752,7 @@ void sh_remove_shadows(struct vcpu *v, m
>      };
>  
>      ASSERT(!(all && fast));
> +    ASSERT(mfn_valid(gmfn));
>  
>      /* Although this is an externally visible function, we do not know
>       * whether the shadow lock will be held when it is called (since it
> Index: xen-unstable.hg/xen/arch/x86/mm/shadow/multi.c
> ===================================================================
> --- xen-unstable.hg.orig/xen/arch/x86/mm/shadow/multi.c       2009-12-11 
> 20:47:17.000000000 +0900
> +++ xen-unstable.hg/xen/arch/x86/mm/shadow/multi.c    2009-12-11 
> 20:48:17.000000000 +0900
> @@ -3305,7 +3305,8 @@ static int sh_page_fault(struct vcpu *v,
>       * fault was a non-user write to a present page.  */
>      if ( is_hvm_domain(d) 
>           && unlikely(!hvm_wp_enabled(v)) 
> -         && regs->error_code == (PFEC_write_access|PFEC_page_present) )
> +         && regs->error_code == (PFEC_write_access|PFEC_page_present)
> +         && mfn_valid(gmfn) )
>      {
>          perfc_incr(shadow_fault_emulate_wp);
>          goto emulate;
> 

Yes, that seems good to me.

Signed-off-by: Tim Deegan <Tim.Deegan@xxxxxxxxxx>

Cheers,

Tim

-- 
Tim Deegan <Tim.Deegan@xxxxxxxxxx>
Principal Software Engineer, Citrix Systems (R&D) Ltd.
[Company #02300071, SL9 0DZ, UK.]

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

<Prev in Thread] Current Thread [Next in Thread>