Li, Xin
2009-Oct-23 07:26 UTC
[Xen-devel] [PATCH] ignore guest writes to read only memory regions or memory holes in EPT
ignore guest writes to read only memory regions or memory holes in EPT.
This patch prevents domain crash when running memtest86 with EPT.
Signed-off-by: Xin Li <xin.li@intel.com>
diff -r 37829fd7c1e3 xen/arch/x86/hvm/vmx/vmx.c
--- a/xen/arch/x86/hvm/vmx/vmx.c Wed Oct 21 16:08:28 2009 +0100
+++ b/xen/arch/x86/hvm/vmx/vmx.c Fri Oct 23 23:21:27 2009 +0800
@@ -2184,6 +2184,17 @@
return;
}
+ /* Ignore writes to:
+ * 1. read only memory regions;
+ * 2. memory holes. */
+ if ( (qualification & EPT_WRITE_VIOLATION)
+ && (((gla_validity == EPT_GLA_VALIDITY_MATCH) && (t ==
p2m_ram_ro))
+ || (mfn_x(mfn) == INVALID_MFN)) ) {
+ int inst_len = __get_instruction_length();
+ __update_guest_eip(inst_len);
+ return;
+ }
+
/* Everything else is an error. */
gla = __vmread(GUEST_LINEAR_ADDRESS);
gdprintk(XENLOG_ERR, "EPT violation %#lx (%c%c%c/%c%c%c), "
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Jan Beulich
2009-Oct-26 08:27 UTC
Re: [Xen-devel] [PATCH] ignore guest writes to read only memory regions or memory holes in EPT
After looking at the documentation I don''t think using __get_instruction_length() here is valid, i.e. you need to decode the instruction in order to be able to skip it. Otherwise, could you have your doc folks update the documentation (24.2.4) accordingly? Jan>>> "Li, Xin" <xin.li@intel.com> 23.10.09 09:26 >>>ignore guest writes to read only memory regions or memory holes in EPT. This patch prevents domain crash when running memtest86 with EPT. Signed-off-by: Xin Li <xin.li@intel.com> diff -r 37829fd7c1e3 xen/arch/x86/hvm/vmx/vmx.c --- a/xen/arch/x86/hvm/vmx/vmx.c Wed Oct 21 16:08:28 2009 +0100 +++ b/xen/arch/x86/hvm/vmx/vmx.c Fri Oct 23 23:21:27 2009 +0800 @@ -2184,6 +2184,17 @@ return; } + /* Ignore writes to: + * 1. read only memory regions; + * 2. memory holes. */ + if ( (qualification & EPT_WRITE_VIOLATION) + && (((gla_validity == EPT_GLA_VALIDITY_MATCH) && (t == p2m_ram_ro)) + || (mfn_x(mfn) == INVALID_MFN)) ) { + int inst_len = __get_instruction_length(); + __update_guest_eip(inst_len); + return; + } + /* Everything else is an error. */ gla = __vmread(GUEST_LINEAR_ADDRESS); gdprintk(XENLOG_ERR, "EPT violation %#lx (%c%c%c/%c%c%c), " _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Li, Xin
2009-Oct-26 08:45 UTC
RE: [Xen-devel] [PATCH] ignore guest writes to read only memory regions or memory holes in EPT
>ignore guest writes to read only memory regions or memory holes in EPT.Keir, For reads from memory holes, hw returns a value with all bits set. Xen should emulate this behavior, and this needs to change Xen x86 instruction emulation code. do we already have such logic in Xen? In early days, such reads were sent to Qemu, which returns 0xFFFFFFFF, 0xFFFF or 0xFF. Thanks! -Xin _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Keir Fraser
2009-Oct-26 09:05 UTC
Re: [Xen-devel] [PATCH] ignore guest writes to read only memory regions or memory holes in EPT
Also, shouldn''t writes to memory holes be already caught by the
handle_mmio() case, as we identify memory holes as having type p2m_mmio_dm?
I would think something like:
if ( (gla_validity == EPT_GLA_VALIDITY_MATCH) ||
(gla_validity == EPT_GLA_VALIDITY_GPT_WALK) )
{
if ( p2m_is_mmio(t) || (t == p2m_ram_ro) )
{
/* MMIO and writes to read-only areas. */
/* We can be quite generous in what we catch with this case. */
if ( !handle_mmio() )
vmx_inject_exception(TRAP_gp_fault, 0);
}
else if ( p2m_is_ram(t) && paging_mode_log_dirty(d) )
{
/* Faster non-emulation handling for log-dirty and PoD. */
paging_mark_dirty(), ...
}
return;
}
...would be a suitably wide-ranging catch-all, without catching genuine EPT
implementation bugs that you want to print an error message for. It''s
also
less complicated than the existing ept_handle_violation() implementation, so
I would like to switch to it.
-- Keir
On 26/10/2009 08:27, "Jan Beulich" <JBeulich@novell.com> wrote:
> After looking at the documentation I don''t think using
> __get_instruction_length() here is valid, i.e. you need to decode the
> instruction in order to be able to skip it. Otherwise, could you have
> your doc folks update the documentation (24.2.4) accordingly?
>
> Jan
>
>>>> "Li, Xin" <xin.li@intel.com> 23.10.09 09:26
>>>
> ignore guest writes to read only memory regions or memory holes in EPT.
>
> This patch prevents domain crash when running memtest86 with EPT.
>
> Signed-off-by: Xin Li <xin.li@intel.com>
>
> diff -r 37829fd7c1e3 xen/arch/x86/hvm/vmx/vmx.c
> --- a/xen/arch/x86/hvm/vmx/vmx.c Wed Oct 21 16:08:28 2009 +0100
> +++ b/xen/arch/x86/hvm/vmx/vmx.c Fri Oct 23 23:21:27 2009 +0800
> @@ -2184,6 +2184,17 @@
> return;
> }
>
> + /* Ignore writes to:
> + * 1. read only memory regions;
> + * 2. memory holes. */
> + if ( (qualification & EPT_WRITE_VIOLATION)
> + && (((gla_validity == EPT_GLA_VALIDITY_MATCH) &&
(t == p2m_ram_ro))
> + || (mfn_x(mfn) == INVALID_MFN)) ) {
> + int inst_len = __get_instruction_length();
> + __update_guest_eip(inst_len);
> + return;
> + }
> +
> /* Everything else is an error. */
> gla = __vmread(GUEST_LINEAR_ADDRESS);
> gdprintk(XENLOG_ERR, "EPT violation %#lx (%c%c%c/%c%c%c), "
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Keir Fraser
2009-Oct-26 09:08 UTC
Re: [Xen-devel] [PATCH] ignore guest writes to read only memory regions or memory holes in EPT
On 26/10/2009 08:45, "Li, Xin" <xin.li@intel.com> wrote:>> ignore guest writes to read only memory regions or memory holes in EPT. > > Keir, > For reads from memory holes, hw returns a value with all bits set. Xen should > emulate this behavior, and this needs to change Xen x86 instruction emulation > code. do we already have such logic in Xen? > > In early days, such reads were sent to Qemu, which returns 0xFFFFFFFF, 0xFFFF > or 0xFF.I think we should already handle this correctly with the existing handle_mmio() case in ept_handle_violation(). The read should indeed go to qemu which should do the right thing. But also see my previous email about simplifying and robustifying ept_handle_violation(). -- Keir _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Keir Fraser
2009-Oct-26 09:26 UTC
Re: [Xen-devel] [PATCH] ignore guest writes to read only memory regions or memory holes in EPT
Oh, another reason to reject this approach is that, just because an instruction''s write to read-only memory should be thrown away, doesn''t mean it doesn''t have other necessary side effects. For example, STOS or MOVS must be emulated to have their required effect on the register file. So please comment on my suggested alternative sketched-out patch. -- Keir On 26/10/2009 08:27, "Jan Beulich" <JBeulich@novell.com> wrote:> After looking at the documentation I don''t think using > __get_instruction_length() here is valid, i.e. you need to decode the > instruction in order to be able to skip it. Otherwise, could you have > your doc folks update the documentation (24.2.4) accordingly? > > Jan > >>>> "Li, Xin" <xin.li@intel.com> 23.10.09 09:26 >>> > ignore guest writes to read only memory regions or memory holes in EPT. > > This patch prevents domain crash when running memtest86 with EPT. > > Signed-off-by: Xin Li <xin.li@intel.com> > > diff -r 37829fd7c1e3 xen/arch/x86/hvm/vmx/vmx.c > --- a/xen/arch/x86/hvm/vmx/vmx.c Wed Oct 21 16:08:28 2009 +0100 > +++ b/xen/arch/x86/hvm/vmx/vmx.c Fri Oct 23 23:21:27 2009 +0800 > @@ -2184,6 +2184,17 @@ > return; > } > > + /* Ignore writes to: > + * 1. read only memory regions; > + * 2. memory holes. */ > + if ( (qualification & EPT_WRITE_VIOLATION) > + && (((gla_validity == EPT_GLA_VALIDITY_MATCH) && (t == p2m_ram_ro)) > + || (mfn_x(mfn) == INVALID_MFN)) ) { > + int inst_len = __get_instruction_length(); > + __update_guest_eip(inst_len); > + return; > + } > + > /* Everything else is an error. */ > gla = __vmread(GUEST_LINEAR_ADDRESS); > gdprintk(XENLOG_ERR, "EPT violation %#lx (%c%c%c/%c%c%c), " > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xensource.com > http://lists.xensource.com/xen-devel_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Li, Xin
2009-Oct-26 09:57 UTC
RE: [Xen-devel] [PATCH] ignore guest writes to read only memory regions or memory holes in EPT
>Also, shouldn''t writes to memory holes be already caught by the >handle_mmio() case, as we identify memory holes as having type >p2m_mmio_dm?Much better, I don''t notice memory holes are having type p2m_mmio_dm. Thanks! -Xin _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Keir Fraser
2009-Oct-26 13:28 UTC
Re: [Xen-devel] [PATCH] ignore guest writes to read only memory regions or memory holes in EPT
On 26/10/2009 09:57, "Li, Xin" <xin.li@intel.com> wrote:>> Also, shouldn''t writes to memory holes be already caught by the >> handle_mmio() case, as we identify memory holes as having type >> p2m_mmio_dm? > > Much better, I don''t notice memory holes are having type p2m_mmio_dm.Please give c/s 20368 a look. It shares most of the logic with the AMD case. I think it would be a good idea to backport this for 3.4.2, if everyone is happy with it. -- Keir _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel