[PATCH] VMX: XSA-60 workaround

[PATCH] VMX: XSA-60 workaround

[Jan Beulich]

[-- Attachment #1: Type: text/plain, Size: 1571 bytes --]

Considering that there's still no real progress towards a resolution
for XSA-60, I'd like to propose turning off the probelamtic code by
default, allowing it to be turned back on via command line option.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -57,6 +57,14 @@
 #include <asm/hvm/nestedhvm.h>
 #include <asm/event.h>
 
+/*
+ * Option to allow VMX guests to run with caches disabled. This is exposing
+ * the host to DoS attacks (due to the way vmx_set_uc_mode() works), and hence
+ * needs to be disabled by default.
+ */
+static bool_t __read_mostly opt_permit_cache_disable;
+boolean_param("vmx-permit-cache-disable", opt_permit_cache_disable);
+
 enum handler_return { HNDL_done, HNDL_unhandled, HNDL_exception_raised };
 
 static void vmx_ctxt_switch_from(struct vcpu *v);
@@ -1133,6 +1141,8 @@ static void vmx_update_guest_cr(struct v
 
         v->arch.hvm_vcpu.hw_cr[0] =
             v->arch.hvm_vcpu.guest_cr[0] | hw_cr0_mask;
+        if ( !opt_permit_cache_disable )
+            v->arch.hvm_vcpu.hw_cr[0] &= ~(X86_CR0_CD | X86_CR0_NW);
         __vmwrite(GUEST_CR0, v->arch.hvm_vcpu.hw_cr[0]);
         __vmwrite(CR0_READ_SHADOW, v->arch.hvm_vcpu.guest_cr[0]);
 
@@ -1603,6 +1613,9 @@ const struct hvm_function_table * __init
         vmx_function_table.sync_pir_to_irr = NULL;
     }
 
+    if ( !opt_permit_cache_disable )
+        vmx_function_table.set_uc_mode = NULL;
+
     setup_vmcs_dump();
 
     return &vmx_function_table;




[-- Attachment #2: xsa60.patch --]
[-- Type: text/plain, Size: 1591 bytes --]

VMX: XSA-60 workaround

Considering that there's still no real progress towards a resolution
for XSA-60, I'd like to propose turning off the probelamtic code by
default, allowing it to be turned back on via command line option.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -57,6 +57,14 @@
 #include <asm/hvm/nestedhvm.h>
 #include <asm/event.h>
 
+/*
+ * Option to allow VMX guests to run with caches disabled. This is exposing
+ * the host to DoS attacks (due to the way vmx_set_uc_mode() works), and hence
+ * needs to be disabled by default.
+ */
+static bool_t __read_mostly opt_permit_cache_disable;
+boolean_param("vmx-permit-cache-disable", opt_permit_cache_disable);
+
 enum handler_return { HNDL_done, HNDL_unhandled, HNDL_exception_raised };
 
 static void vmx_ctxt_switch_from(struct vcpu *v);
@@ -1133,6 +1141,8 @@ static void vmx_update_guest_cr(struct v
 
         v->arch.hvm_vcpu.hw_cr[0] =
             v->arch.hvm_vcpu.guest_cr[0] | hw_cr0_mask;
+        if ( !opt_permit_cache_disable )
+            v->arch.hvm_vcpu.hw_cr[0] &= ~(X86_CR0_CD | X86_CR0_NW);
         __vmwrite(GUEST_CR0, v->arch.hvm_vcpu.hw_cr[0]);
         __vmwrite(CR0_READ_SHADOW, v->arch.hvm_vcpu.guest_cr[0]);
 
@@ -1603,6 +1613,9 @@ const struct hvm_function_table * __init
         vmx_function_table.sync_pir_to_irr = NULL;
     }
 
+    if ( !opt_permit_cache_disable )
+        vmx_function_table.set_uc_mode = NULL;
+
     setup_vmcs_dump();
 
     return &vmx_function_table;

[-- Attachment #3: Type: text/plain, Size: 126 bytes --]

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

Re: [PATCH] VMX: XSA-60 workaround

[Andrew Cooper]

[-- Attachment #1.1: Type: text/plain, Size: 2036 bytes --]

On 13/08/13 17:36, Jan Beulich wrote:
> Considering that there's still no real progress towards a resolution
> for XSA-60, I'd like to propose turning off the probelamtic code by
> default, allowing it to be turned back on via command line option.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

In principle, ok, but can I suggest that this initially goes in with a
per domain warn once, (and perhaps gdprintk afterwards), so guests which
actually try to use this can at least be identified if they suddenly
start behaving weirdly?

~Andrew

>
> --- a/xen/arch/x86/hvm/vmx/vmx.c
> +++ b/xen/arch/x86/hvm/vmx/vmx.c
> @@ -57,6 +57,14 @@
>  #include <asm/hvm/nestedhvm.h>
>  #include <asm/event.h>
>  
> +/*
> + * Option to allow VMX guests to run with caches disabled. This is exposing
> + * the host to DoS attacks (due to the way vmx_set_uc_mode() works), and hence
> + * needs to be disabled by default.
> + */
> +static bool_t __read_mostly opt_permit_cache_disable;
> +boolean_param("vmx-permit-cache-disable", opt_permit_cache_disable);
> +
>  enum handler_return { HNDL_done, HNDL_unhandled, HNDL_exception_raised };
>  
>  static void vmx_ctxt_switch_from(struct vcpu *v);
> @@ -1133,6 +1141,8 @@ static void vmx_update_guest_cr(struct v
>  
>          v->arch.hvm_vcpu.hw_cr[0] =
>              v->arch.hvm_vcpu.guest_cr[0] | hw_cr0_mask;
> +        if ( !opt_permit_cache_disable )
> +            v->arch.hvm_vcpu.hw_cr[0] &= ~(X86_CR0_CD | X86_CR0_NW);
>          __vmwrite(GUEST_CR0, v->arch.hvm_vcpu.hw_cr[0]);
>          __vmwrite(CR0_READ_SHADOW, v->arch.hvm_vcpu.guest_cr[0]);
>  
> @@ -1603,6 +1613,9 @@ const struct hvm_function_table * __init
>          vmx_function_table.sync_pir_to_irr = NULL;
>      }
>  
> +    if ( !opt_permit_cache_disable )
> +        vmx_function_table.set_uc_mode = NULL;
> +
>      setup_vmcs_dump();
>  
>      return &vmx_function_table;
>
>
>
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel


[-- Attachment #1.2: Type: text/html, Size: 2856 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

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

Re: [PATCH] VMX: XSA-60 workaround

[Jan Beulich]

>>> On 13.08.13 at 18:48, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
> On 13/08/13 17:36, Jan Beulich wrote:
>> Considering that there's still no real progress towards a resolution
>> for XSA-60, I'd like to propose turning off the probelamtic code by
>> default, allowing it to be turned back on via command line option.
>>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> In principle, ok, but can I suggest that this initially goes in with a
> per domain warn once, (and perhaps gdprintk afterwards), so guests which
> actually try to use this can at least be identified if they suddenly
> start behaving weirdly?

No, that's pointless: Various (if not all) Linux versions set CR0.CD
in the course of fiddling with th MTRRs, i.e. we'd issue this warning
for most if not all HVM Linux guests that also have some PCI device
assigned, even though in the vast majority of cases this would be
benign to them. The one case where I'm told that broken code is
needed for guest stability is when a graphics device gets assigned
to it (proof of that is yet to be seen though), yet at the point
where the warning would need to get issued we shouldn't go as
far as looking for specific device types (even more so when there
might be other device classes where the cache disabling would
also be needed).

Jan

Re: [PATCH] VMX: XSA-60 workaround

[Andrew Cooper]

On 14/08/13 10:02, Jan Beulich wrote:
>>>> On 13.08.13 at 18:48, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
>> On 13/08/13 17:36, Jan Beulich wrote:
>>> Considering that there's still no real progress towards a resolution
>>> for XSA-60, I'd like to propose turning off the probelamtic code by
>>> default, allowing it to be turned back on via command line option.
>>>
>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>> In principle, ok, but can I suggest that this initially goes in with a
>> per domain warn once, (and perhaps gdprintk afterwards), so guests which
>> actually try to use this can at least be identified if they suddenly
>> start behaving weirdly?
> No, that's pointless: Various (if not all) Linux versions set CR0.CD
> in the course of fiddling with th MTRRs, i.e. we'd issue this warning
> for most if not all HVM Linux guests that also have some PCI device
> assigned, even though in the vast majority of cases this would be
> benign to them. The one case where I'm told that broken code is
> needed for guest stability is when a graphics device gets assigned
> to it (proof of that is yet to be seen though), yet at the point
> where the warning would need to get issued we shouldn't go as
> far as looking for specific device types (even more so when there
> might be other device classes where the cache disabling would
> also be needed).
>
> Jan
>

Hmm.  At the very least there should be a boot time message indicating
whether playing with the CD bit is enabled or not.  What can't happen is
that we silently change the behaviour of something like this, which does
have suspected impact in certain usecases.

~Andrew

Re: [PATCH] VMX: XSA-60 workaround

[Jan Beulich]

>>> On 14.08.13 at 12:12, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
> On 14/08/13 10:02, Jan Beulich wrote:
>>>>> On 13.08.13 at 18:48, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
>>> On 13/08/13 17:36, Jan Beulich wrote:
>>>> Considering that there's still no real progress towards a resolution
>>>> for XSA-60, I'd like to propose turning off the probelamtic code by
>>>> default, allowing it to be turned back on via command line option.
>>>>
>>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>> In principle, ok, but can I suggest that this initially goes in with a
>>> per domain warn once, (and perhaps gdprintk afterwards), so guests which
>>> actually try to use this can at least be identified if they suddenly
>>> start behaving weirdly?
>> No, that's pointless: Various (if not all) Linux versions set CR0.CD
>> in the course of fiddling with th MTRRs, i.e. we'd issue this warning
>> for most if not all HVM Linux guests that also have some PCI device
>> assigned, even though in the vast majority of cases this would be
>> benign to them. The one case where I'm told that broken code is
>> needed for guest stability is when a graphics device gets assigned
>> to it (proof of that is yet to be seen though), yet at the point
>> where the warning would need to get issued we shouldn't go as
>> far as looking for specific device types (even more so when there
>> might be other device classes where the cache disabling would
>> also be needed).
> 
> Hmm.  At the very least there should be a boot time message indicating
> whether playing with the CD bit is enabled or not.  What can't happen is
> that we silently change the behaviour of something like this, which does
> have suspected impact in certain usecases.

Well, I can add this, but don't really see much point in doing so.

Jan

Re: [PATCH] VMX: XSA-60 workaround

[Matt Wilson]

On Tue, Aug 13, 2013 at 05:36:17PM +0100, Jan Beulich wrote:
> Considering that there's still no real progress towards a resolution
> for XSA-60, I'd like to propose turning off the probelamtic code by
> default, allowing it to be turned back on via command line option.

Apologies for a late reply, I've been on holiday for the past week.

I think it'd be really handy to make this a per-domain configuration,
perhaps with a system-wide default set by boot command line. If not
per-domain, it would be helpful to be configurable at boot time.

--msw

> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> --- a/xen/arch/x86/hvm/vmx/vmx.c
> +++ b/xen/arch/x86/hvm/vmx/vmx.c
> @@ -57,6 +57,14 @@
>  #include <asm/hvm/nestedhvm.h>
>  #include <asm/event.h>
>  
> +/*
> + * Option to allow VMX guests to run with caches disabled. This is exposing
> + * the host to DoS attacks (due to the way vmx_set_uc_mode() works), and hence
> + * needs to be disabled by default.
> + */
> +static bool_t __read_mostly opt_permit_cache_disable;
> +boolean_param("vmx-permit-cache-disable", opt_permit_cache_disable);
> +
>  enum handler_return { HNDL_done, HNDL_unhandled, HNDL_exception_raised };
>  
>  static void vmx_ctxt_switch_from(struct vcpu *v);
> @@ -1133,6 +1141,8 @@ static void vmx_update_guest_cr(struct v
>  
>          v->arch.hvm_vcpu.hw_cr[0] =
>              v->arch.hvm_vcpu.guest_cr[0] | hw_cr0_mask;
> +        if ( !opt_permit_cache_disable )
> +            v->arch.hvm_vcpu.hw_cr[0] &= ~(X86_CR0_CD | X86_CR0_NW);
>          __vmwrite(GUEST_CR0, v->arch.hvm_vcpu.hw_cr[0]);
>          __vmwrite(CR0_READ_SHADOW, v->arch.hvm_vcpu.guest_cr[0]);
>  
> @@ -1603,6 +1613,9 @@ const struct hvm_function_table * __init
>          vmx_function_table.sync_pir_to_irr = NULL;
>      }
>  
> +    if ( !opt_permit_cache_disable )
> +        vmx_function_table.set_uc_mode = NULL;
> +
>      setup_vmcs_dump();
>  
>      return &vmx_function_table;
> 
> 
> 

> VMX: XSA-60 workaround
> 
> Considering that there's still no real progress towards a resolution
> for XSA-60, I'd like to propose turning off the probelamtic code by
> default, allowing it to be turned back on via command line option.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> --- a/xen/arch/x86/hvm/vmx/vmx.c
> +++ b/xen/arch/x86/hvm/vmx/vmx.c
> @@ -57,6 +57,14 @@
>  #include <asm/hvm/nestedhvm.h>
>  #include <asm/event.h>
>  
> +/*
> + * Option to allow VMX guests to run with caches disabled. This is exposing
> + * the host to DoS attacks (due to the way vmx_set_uc_mode() works), and hence
> + * needs to be disabled by default.
> + */
> +static bool_t __read_mostly opt_permit_cache_disable;
> +boolean_param("vmx-permit-cache-disable", opt_permit_cache_disable);
> +
>  enum handler_return { HNDL_done, HNDL_unhandled, HNDL_exception_raised };
>  
>  static void vmx_ctxt_switch_from(struct vcpu *v);
> @@ -1133,6 +1141,8 @@ static void vmx_update_guest_cr(struct v
>  
>          v->arch.hvm_vcpu.hw_cr[0] =
>              v->arch.hvm_vcpu.guest_cr[0] | hw_cr0_mask;
> +        if ( !opt_permit_cache_disable )
> +            v->arch.hvm_vcpu.hw_cr[0] &= ~(X86_CR0_CD | X86_CR0_NW);
>          __vmwrite(GUEST_CR0, v->arch.hvm_vcpu.hw_cr[0]);
>          __vmwrite(CR0_READ_SHADOW, v->arch.hvm_vcpu.guest_cr[0]);
>  
> @@ -1603,6 +1613,9 @@ const struct hvm_function_table * __init
>          vmx_function_table.sync_pir_to_irr = NULL;
>      }
>  
> +    if ( !opt_permit_cache_disable )
> +        vmx_function_table.set_uc_mode = NULL;
> +
>      setup_vmcs_dump();
>  
>      return &vmx_function_table;

> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

Re: [PATCH] VMX: XSA-60 workaround

[Jan Beulich]

>>> On 19.08.13 at 20:27, Matt Wilson <msw@amazon.com> wrote:
> On Tue, Aug 13, 2013 at 05:36:17PM +0100, Jan Beulich wrote:
>> Considering that there's still no real progress towards a resolution
>> for XSA-60, I'd like to propose turning off the probelamtic code by
>> default, allowing it to be turned back on via command line option.
> 
> Apologies for a late reply, I've been on holiday for the past week.
> 
> I think it'd be really handy to make this a per-domain configuration,
> perhaps with a system-wide default set by boot command line.

Remember this is a temporary workaround, not a solution. I'm
not going to waste time trying to make the logic more sophisticated,
but I wouldn't mind if someone else took the patch further.

> If not per-domain, it would be helpful to be configurable at boot time.

Which it is - did you overlook the addition of the
"vmx-permit-cache-disable" command line option?

Jan

Re: [PATCH] VMX: XSA-60 workaround

[Matt Wilson]

On Tue, Aug 20, 2013 at 08:22:41AM +0100, Jan Beulich wrote:
> >>> On 19.08.13 at 20:27, Matt Wilson <msw@amazon.com> wrote:
> > On Tue, Aug 13, 2013 at 05:36:17PM +0100, Jan Beulich wrote:
> >> Considering that there's still no real progress towards a resolution
> >> for XSA-60, I'd like to propose turning off the probelamtic code by
> >> default, allowing it to be turned back on via command line option.
> > 
> > Apologies for a late reply, I've been on holiday for the past week.
> > 
> > I think it'd be really handy to make this a per-domain configuration,
> > perhaps with a system-wide default set by boot command line.
> 
> Remember this is a temporary workaround, not a solution. I'm
> not going to waste time trying to make the logic more sophisticated,
> but I wouldn't mind if someone else took the patch further.

I'd like to help with a solution. What's the current thinking on the
best way to handle the problem?

> > If not per-domain, it would be helpful to be configurable at boot time.
> 
> Which it is - did you overlook the addition of the
> "vmx-permit-cache-disable" command line option?

Sorry, I dropped some words out. Of course this is currently a boot
time parameter now. I meant to ask if it could be made *run time*
configurable with a boot time default.

--msw

Re: [PATCH] VMX: XSA-60 workaround

[Jan Beulich]

>>> On 20.08.13 at 16:27, Matt Wilson <msw@amazon.com> wrote:
> On Tue, Aug 20, 2013 at 08:22:41AM +0100, Jan Beulich wrote:
>> >>> On 19.08.13 at 20:27, Matt Wilson <msw@amazon.com> wrote:
>> > On Tue, Aug 13, 2013 at 05:36:17PM +0100, Jan Beulich wrote:
>> >> Considering that there's still no real progress towards a resolution
>> >> for XSA-60, I'd like to propose turning off the probelamtic code by
>> >> default, allowing it to be turned back on via command line option.
>> > 
>> > Apologies for a late reply, I've been on holiday for the past week.
>> > 
>> > I think it'd be really handy to make this a per-domain configuration,
>> > perhaps with a system-wide default set by boot command line.
>> 
>> Remember this is a temporary workaround, not a solution. I'm
>> not going to waste time trying to make the logic more sophisticated,
>> but I wouldn't mind if someone else took the patch further.
> 
> I'd like to help with a solution. What's the current thinking on the
> best way to handle the problem?

All that we seem to agree about with the Intel engineers is that the
current model of going through the entire EPT tables is wrong.
There hasn't been a good proposal for a solution yet; the one
route I proposed was confirmed to be disallowed in hyper-
threaded environments. In fact, it's not even clear whether the
problematic code wasn't in fact added "just in case", without
actually knowing of an environment where it would be needed
(GFX passthrough is the most likely candidate of a user of all this).

Jan

Re: [PATCH] VMX: XSA-60 workaround

[Zhang, Yang Z]

Jan Beulich wrote on 2013-08-14:
> Considering that there's still no real progress towards a resolution
> for XSA-60, I'd like to propose turning off the probelamtic code by
> default, allowing it to be turned back on via command line option.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> --- a/xen/arch/x86/hvm/vmx/vmx.c
> +++ b/xen/arch/x86/hvm/vmx/vmx.c
> @@ -57,6 +57,14 @@
>  #include <asm/hvm/nestedhvm.h>
>  #include <asm/event.h>
> +/* + * Option to allow VMX guests to run with caches disabled. This is
> exposing + * the host to DoS attacks (due to the way vmx_set_uc_mode()
> works), and hence + * needs to be disabled by default. + */ +static
> bool_t __read_mostly opt_permit_cache_disable;
> +boolean_param("vmx-permit-cache-disable", opt_permit_cache_disable); +
>  enum handler_return { HNDL_done, HNDL_unhandled,
> HNDL_exception_raised };
> 
>  static void vmx_ctxt_switch_from(struct vcpu *v);
> @@ -1133,6 +1141,8 @@ static void vmx_update_guest_cr(struct v
> 
>          v->arch.hvm_vcpu.hw_cr[0] =
>              v->arch.hvm_vcpu.guest_cr[0] | hw_cr0_mask;
> +        if ( !opt_permit_cache_disable )
> +            v->arch.hvm_vcpu.hw_cr[0] &= ~(X86_CR0_CD |
> X86_CR0_NW);
>          __vmwrite(GUEST_CR0, v->arch.hvm_vcpu.hw_cr[0]);
>          __vmwrite(CR0_READ_SHADOW, v->arch.hvm_vcpu.guest_cr[0]);
> @@ -1603,6 +1613,9 @@ const struct hvm_function_table * __init
>          vmx_function_table.sync_pir_to_irr = NULL;
>      }
> +    if ( !opt_permit_cache_disable )
> +        vmx_function_table.set_uc_mode = NULL;
> +
>      setup_vmcs_dump();
>      
>      return &vmx_function_table;
>
Shouldn't it cause problem with DMA of pass-through device? Guest modified the content of DMA buffer without flushing cache if he set CR0.CD, but the modification still caching in hardware since the CD bit isn't really set in CR0. Then device may read from memory directly and it will get the wrong data.

Best regards,
Yang

Re: [PATCH] VMX: XSA-60 workaround

[Jan Beulich]

>>> On 20.08.13 at 08:51, "Zhang, Yang Z" <yang.z.zhang@intel.com> wrote:
> Jan Beulich wrote on 2013-08-14:
>> Considering that there's still no real progress towards a resolution
>> for XSA-60, I'd like to propose turning off the probelamtic code by
>> default, allowing it to be turned back on via command line option.
>> 
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>> 
>> --- a/xen/arch/x86/hvm/vmx/vmx.c
>> +++ b/xen/arch/x86/hvm/vmx/vmx.c
>> @@ -57,6 +57,14 @@
>>  #include <asm/hvm/nestedhvm.h>
>>  #include <asm/event.h>
>> +/* + * Option to allow VMX guests to run with caches disabled. This is
>> exposing + * the host to DoS attacks (due to the way vmx_set_uc_mode()
>> works), and hence + * needs to be disabled by default. + */ +static
>> bool_t __read_mostly opt_permit_cache_disable;
>> +boolean_param("vmx-permit-cache-disable", opt_permit_cache_disable); +
>>  enum handler_return { HNDL_done, HNDL_unhandled,
>> HNDL_exception_raised };
>> 
>>  static void vmx_ctxt_switch_from(struct vcpu *v);
>> @@ -1133,6 +1141,8 @@ static void vmx_update_guest_cr(struct v
>> 
>>          v->arch.hvm_vcpu.hw_cr[0] =
>>              v->arch.hvm_vcpu.guest_cr[0] | hw_cr0_mask;
>> +        if ( !opt_permit_cache_disable )
>> +            v->arch.hvm_vcpu.hw_cr[0] &= ~(X86_CR0_CD |
>> X86_CR0_NW);
>>          __vmwrite(GUEST_CR0, v->arch.hvm_vcpu.hw_cr[0]);
>>          __vmwrite(CR0_READ_SHADOW, v->arch.hvm_vcpu.guest_cr[0]);
>> @@ -1603,6 +1613,9 @@ const struct hvm_function_table * __init
>>          vmx_function_table.sync_pir_to_irr = NULL;
>>      }
>> +    if ( !opt_permit_cache_disable )
>> +        vmx_function_table.set_uc_mode = NULL;
>> +
>>      setup_vmcs_dump();
>>      
>>      return &vmx_function_table;
>>
> Shouldn't it cause problem with DMA of pass-through device? Guest modified 
> the content of DMA buffer without flushing cache if he set CR0.CD, but the 
> modification still caching in hardware since the CD bit isn't really set in 
> CR0. Then device may read from memory directly and it will get the wrong 
> data.

Yes, of course that's a possibility. But we know of no guests that
do this (the only known use of setting CR0.CD is for playing with
the MTRRs). And considering that guests usually will run with
CR0.CD clear anyway, the non-caching behavior for the described
scenario needs to be handled via EPT cachability settings anyway.
And once the EPT entries are properly set to UC, the value of
CR0.CD becomes meaningless. With the bottom line being that the
current way of handling guests setting CR0.CD is pointless if
everything else works as intended.

Jan

Re: [PATCH] VMX: XSA-60 workaround

[Zhang, Yang Z]

Jan Beulich wrote on 2013-08-20:
>>>> On 20.08.13 at 08:51, "Zhang, Yang Z" <yang.z.zhang@intel.com> wrote:
>> Jan Beulich wrote on 2013-08-14:
>>> Considering that there's still no real progress towards a
>>> resolution for XSA-60, I'd like to propose turning off the
>>> probelamtic code by default, allowing it to be turned back on via command line option.
>>> 
>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>> 
>>> --- a/xen/arch/x86/hvm/vmx/vmx.c
>>> +++ b/xen/arch/x86/hvm/vmx/vmx.c
>>> @@ -57,6 +57,14 @@
>>>  #include <asm/hvm/nestedhvm.h>
>>>  #include <asm/event.h>
>>> +/* + * Option to allow VMX guests to run with caches disabled.
>>> +This is
>>> exposing + * the host to DoS attacks (due to the way
>>> vmx_set_uc_mode() works), and hence + * needs to be disabled by
>>> default. + */ +static bool_t __read_mostly
>>> opt_permit_cache_disable;
>>> +boolean_param("vmx-permit-cache-disable",
>>> +opt_permit_cache_disable);
>>> ++
>>>  enum handler_return { HNDL_done, HNDL_unhandled,
>>> HNDL_exception_raised };
>>> 
>>>  static void vmx_ctxt_switch_from(struct vcpu *v); @@ -1133,6
>>> +1141,8 @@ static void vmx_update_guest_cr(struct v
>>> 
>>>          v->arch.hvm_vcpu.hw_cr[0] =
>>>              v->arch.hvm_vcpu.guest_cr[0] | hw_cr0_mask;
>>> +        if ( !opt_permit_cache_disable )
>>> +            v->arch.hvm_vcpu.hw_cr[0] &= ~(X86_CR0_CD |
>>> X86_CR0_NW);
>>>          __vmwrite(GUEST_CR0, v->arch.hvm_vcpu.hw_cr[0]);
>>>          __vmwrite(CR0_READ_SHADOW,
> v->arch.hvm_vcpu.guest_cr[0]); @@
>>> -1603,6 +1613,9 @@ const struct hvm_function_table * __init
>>>          vmx_function_table.sync_pir_to_irr = NULL;
>>>      }
>>> +    if ( !opt_permit_cache_disable )
>>> +        vmx_function_table.set_uc_mode = NULL;
>>> +
>>>      setup_vmcs_dump();
>>>      
>>>      return &vmx_function_table;
>> Shouldn't it cause problem with DMA of pass-through device? Guest
>> modified the content of DMA buffer without flushing cache if he set
>> CR0.CD, but the modification still caching in hardware since the CD
>> bit isn't really set in CR0. Then device may read from memory
>> directly and it will get the wrong data.
> 
> Yes, of course that's a possibility. But we know of no guests that do
> this (the only known use of setting CR0.CD is for playing with the
> MTRRs). And considering that guests usually will run with CR0.CD clear
> anyway, the non-caching behavior for the described scenario needs to
> be handled via EPT cachability settings anyway.
> And once the EPT entries are properly set to UC, the value of CR0.CD
> becomes meaningless. With the bottom line being that the current way
> of handling guests setting CR0.CD is pointless if everything else works as intended.
> 
> Jan
Yes. Another solution is that to prepare a new EPT table with setting UC in all entries. When guest set CD, then just change the EPTP to point to the new table. And back to old EPT table when guest clear it.

Best regards,
Yang

Re: [PATCH] VMX: XSA-60 workaround

[Jan Beulich]

>>> On 20.08.13 at 09:34, "Zhang, Yang Z" <yang.z.zhang@intel.com> wrote:
> Yes. Another solution is that to prepare a new EPT table with setting UC in 
> all entries. When guest set CD, then just change the EPTP to point to the new 
> table. And back to old EPT table when guest clear it.

Consuming how much extra memory for e.g. a 1Tb guest?

Jan

Re: [PATCH] VMX: XSA-60 workaround

[Zhang, Yang Z]

Jan Beulich wrote on 2013-08-20:
>>>> On 20.08.13 at 09:34, "Zhang, Yang Z" <yang.z.zhang@intel.com> wrote:
>> Yes. Another solution is that to prepare a new EPT table with
>> setting UC in all entries. When guest set CD, then just change the
>> EPTP to point to the new table. And back to old EPT table when guest clear it.
> 
> Consuming how much extra memory for e.g. a 1Tb guest?
Memory shouldn't be a problem in such platform. Also, modern CPUs already support the 1G EPT page size.

> 
> Jan


Best regards,
Yang

Re: [PATCH] VMX: XSA-60 workaround

[Jan Beulich]

>>> On 22.08.13 at 08:21, "Zhang, Yang Z" <yang.z.zhang@intel.com> wrote:
> Jan Beulich wrote on 2013-08-20:
>>>>> On 20.08.13 at 09:34, "Zhang, Yang Z" <yang.z.zhang@intel.com> wrote:
>>> Yes. Another solution is that to prepare a new EPT table with
>>> setting UC in all entries. When guest set CD, then just change the
>>> EPTP to point to the new table. And back to old EPT table when guest clear 
> it.
>> 
>> Consuming how much extra memory for e.g. a 1Tb guest?
> Memory shouldn't be a problem in such platform. Also, modern CPUs already 
> support the 1G EPT page size.

Wasting memory is always a problem, and 1Gb pages aren't
guaranteed to be available at domain creation time. I view this
dual table approach only as a last resort thing (and if we were to
go this route we ought to evaluate whether this or the non-
sharing approach [EPT/VT-d, with the EPT one having the option
of getting re-populated on demand] are preferable, which among
other things part depends on whether all vCPU-s have to be
running on the same EPT table).

Jan