Re: [Xen-devel] [PATCH RESEND v1 6/7] x86: Implement Intel Processor Trace MSRs read/write

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 15.01.18 at 19:12, <luwei.kang@xxxxxxxxx> wrote:
> --- a/xen/arch/x86/cpu/intel_pt.c
> +++ b/xen/arch/x86/cpu/intel_pt.c
> @@ -28,6 +28,107 @@
>  bool_t __read_mostly opt_intel_pt = 1;
>  boolean_param("intel_pt", opt_intel_pt);
>  
> +
> +static void intel_pt_disable_intercept_for_msr(u32 addr_num)
> +{
> +    struct vcpu *v = current;
> +    int i;

unsigned int

> +    vmx_clear_msr_intercept(v, MSR_IA32_RTIT_STATUS, VMX_MSR_RW);
> +    vmx_clear_msr_intercept(v, MSR_IA32_RTIT_OUTPUT_BASE, VMX_MSR_RW);
> +    vmx_clear_msr_intercept(v, MSR_IA32_RTIT_OUTPUT_MASK, VMX_MSR_RW);
> +    vmx_clear_msr_intercept(v, MSR_IA32_RTIT_CR3_MATCH, VMX_MSR_RW);
> +    for ( i = 0; i < addr_num; i++ )
> +        vmx_clear_msr_intercept(v, MSR_IA32_RTIT_ADDR0_A + i, VMX_MSR_RW);

Are all of these expected to be frequently written? If not, keeping the 
intercept
enabled would allow to avoid the MSR reads right after a VM exit.

> +void pt_set_rtit_ctl(struct pt_desc *pt_desc, uint64_t msr_content)
> +{
> +    if (msr_content & MSR_IA32_RTIT_CTL_TRACEEN)

Style (missing blanks).

> +int pt_do_rdmsr(unsigned int msr, uint64_t *msr_content)
> +{
> +    struct pt_desc *pt_desc = &current->arch.hvm_vmx.pt_desc;
> +
> +    if ( !opt_intel_pt )
> +        return 1;
> +
> +    switch ( msr ) {

Style (brace placement).

But on the whole - I'm not sure we want any new MSR handling like this.
Properly integrating this with guest_{rd,wr}msr() would be much preferred.

And then - what about migrating a guest with the feature enabled? Even
if you choose to not support this initially, you'd at least need to fail
migration cleanly when the feature is in use.

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel