On Fri, Dec 11, 2009 at 12:15:06PM +0000, Tim Deegan wrote:
> 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;
> >
Oops, forgot
Signed-off-by: Simon Horman <horms@xxxxxxxxxxxx>
> Yes, that seems good to me.
>
> Signed-off-by: Tim Deegan <Tim.Deegan@xxxxxxxxxx>
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
Home