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

["Zhang, Yang Z" <yang.z.zhang@xxxxxxxxx>]

Jan Beulich wrote on 2013-08-20:
>>>> 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
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



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