Re: [Xen-devel] [PATCH] VMX: XSA-60 workaround

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 20.08.13 at 08:51, "Zhang, Yang Z" <yang.z.zhang@xxxxxxxxx> 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@xxxxxxxx>
>> 
>> --- 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


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