RE: [Xen-devel] [PATCH] vmx: fix debugctl handling

["Shan, Haitao" <haitao.shan@xxxxxxxxx>]

Hi, Jan,

For IA32_DEBUGCTRL:
You can use VMX's generic MSR save/load mechanism for loading host value
for this MSR. Otherwise, the value in this MSR remains unchanged across
VMX transitions between root and non-root mode.
I would rather think that this dedicate MSR field is an optimization in
addition to the generic save/load mechanism. There may not be needs for
a dedicate host field to make them in pairs.

Best Regards
Haitao Shan

Jan Beulich wrote:
> I recently realized that the original way of dealing with the DebugCtl
> MSR on VMX failed to make use of the dedicated guest VMCS field. This
> is being fixed with this patch.
> 
> What is puzzling me to a certain degree is that while there is a guest
> VMCS field for this MSR, there's no equivalent host load field, but
> there's also no indication that the MSR would be cleared during a
> vmexit. Can someone at Intel perhaps give a statement on this?
> 
> I would really be possible to avoid the intercept on DebugCtl reads
> altogether, but that would require new support functions to
> individually disable read and write intercepts (which currently are
> always handled as a pair). I'm not sure such a change is warranted
> for a debugging only feature.
> 
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxxxx>
> 
> Index: 2008-03-05/xen/arch/x86/hvm/vmx/vmcs.c
> ===================================================================
> --- 2008-03-05.orig/xen/arch/x86/hvm/vmx/vmcs.c       2008-02-18
> 10:07:45.000000000 +0100 +++
> 2008-03-05/xen/arch/x86/hvm/vmx/vmcs.c        2008-03-14
14:08:11.000000000
>      +0100 @@ -870,7 +870,7 @@ void vmcs_dump_vcpu(struct vcpu *v) x 
>      = (unsigned long long)vmr(TSC_OFFSET_HIGH) << 32; x |=
>      (uint32_t)vmr(TSC_OFFSET); printk("TSC Offset = %016llx\n", x);
> -    x  = (unsigned long long)vmr(GUEST_IA32_DEBUGCTL) << 32;
> +    x  = (unsigned long long)vmr(GUEST_IA32_DEBUGCTL_HIGH) << 32;
>      x |= (uint32_t)vmr(GUEST_IA32_DEBUGCTL);
>      printk("DebugCtl=%016llx DebugExceptions=%016llx\n", x,
>             (unsigned long long)vmr(GUEST_PENDING_DBG_EXCEPTIONS));
> Index: 2008-03-05/xen/arch/x86/hvm/vmx/vmx.c
> ===================================================================
> --- 2008-03-05.orig/xen/arch/x86/hvm/vmx/vmx.c        2008-02-26
> 10:43:52.000000000 +0100 +++
> 2008-03-05/xen/arch/x86/hvm/vmx/vmx.c 2008-03-14 14:14:40.000000000
>          +0100 @@ -1512,8 +1512,10 @@ static int
>          vmx_msr_read_intercept(struct msr_content =
>      var_range_base[index]; break; case MSR_IA32_DEBUGCTLMSR:
> -        if ( vmx_read_guest_msr(v, ecx, &msr_content) != 0 )
> -            msr_content = 0;
> +        msr_content = __vmread(GUEST_IA32_DEBUGCTL);
> +#ifdef __i386__
> +        msr_content |= (u64)__vmread(GUEST_IA32_DEBUGCTL_HIGH) << 32;
> +#endif
>          break;
>      case MSR_IA32_VMX_BASIC...MSR_IA32_VMX_PROCBASED_CTLS2:
>          goto gp_fault;
> @@ -1732,11 +1734,15 @@ static int vmx_msr_write_intercept(struc
>          }
> 
>          if ( (rc < 0) ||
> -             (vmx_add_guest_msr(v, ecx) < 0) ||
>               (vmx_add_host_load_msr(v, ecx) < 0) )
>              vmx_inject_hw_exception(v, TRAP_machine_check, 0);
>          else
> -            vmx_write_guest_msr(v, ecx, msr_content);
> +        {
> +            __vmwrite(GUEST_IA32_DEBUGCTL, msr_content);
> +#ifdef __i386__
> +            __vmwrite(GUEST_IA32_DEBUGCTL_HIGH, msr_content >> 32);
> +#endif
> +        }
> 
>          break;
>      }
> 
> 
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxxxxxxxx
> http://lists.xensource.com/xen-devel

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel