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
|
Home