[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v2 07/10] x86: Add Intel Processor Trace MSRs read/write emulation
>>> On 30.05.18 at 15:28, <luwei.kang@xxxxxxxxx> wrote: > @@ -106,6 +114,105 @@ static int __init parse_ipt_params(const char *str) > return 0; > } > > +int ipt_do_rdmsr(unsigned int msr, uint64_t *msr_content) > +{ > + const struct ipt_desc *ipt_desc = current->arch.hvm_vmx.ipt_desc; > + const struct cpuid_policy *p = current->domain->arch.cpuid; > + unsigned int index; > + > + if ( !ipt_desc ) > + return 1; > + > + switch ( msr ) > + { > + case MSR_IA32_RTIT_CTL: > + *msr_content = ipt_desc->ipt_guest.ctl; > + break; > + case MSR_IA32_RTIT_STATUS: > + *msr_content = ipt_desc->ipt_guest.status; > + break; > + case MSR_IA32_RTIT_OUTPUT_BASE: > + if ( !ipt_cap(p->ipt.raw, IPT_CAP_single_range_output) && > + !ipt_cap(p->ipt.raw, IPT_CAP_topa_output) ) > + return 1; > + *msr_content = ipt_desc->ipt_guest.output_base; > + break; > + case MSR_IA32_RTIT_OUTPUT_MASK: > + if ( !ipt_cap(p->ipt.raw, IPT_CAP_single_range_output) && > + !ipt_cap(p->ipt.raw, IPT_CAP_topa_output) ) > + return 1; > + *msr_content = ipt_desc->ipt_guest.output_mask | > + RTIT_OUTPUT_MASK_DEFAULT; > + break; > + case MSR_IA32_RTIT_CR3_MATCH: > + if ( !ipt_cap(p->ipt.raw, IPT_CAP_cr3_filter) ) > + return 1; > + *msr_content = ipt_desc->ipt_guest.cr3_match; > + break; > + default: > + index = msr - MSR_IA32_RTIT_ADDR_A(0); Hard tab. Also throughout both functions' switch() statements please add blank lines between case blocks. > +int ipt_do_wrmsr(unsigned int msr, uint64_t msr_content) > +{ > + struct ipt_desc *ipt_desc = current->arch.hvm_vmx.ipt_desc; > + const struct cpuid_policy *p = current->domain->arch.cpuid; > + unsigned int index; > + > + if ( !ipt_desc ) > + return 1; > + > + switch ( msr ) > + { > + case MSR_IA32_RTIT_CTL: > + ipt_desc->ipt_guest.ctl = msr_content; > + __vmwrite(GUEST_IA32_RTIT_CTL, msr_content); > + break; > + case MSR_IA32_RTIT_STATUS: > + if ( (ipt_desc->ipt_guest.ctl & RTIT_CTL_TRACEEN) || > + (msr_content & MSR_IA32_RTIT_STATUS_MASK) ) > + return 1; > + ipt_desc->ipt_guest.status = msr_content; > + break; > + case MSR_IA32_RTIT_OUTPUT_BASE: > + if ( (ipt_desc->ipt_guest.ctl & RTIT_CTL_TRACEEN) || > + (msr_content & > + MSR_IA32_RTIT_OUTPUT_BASE_MASK(p->extd.maxphysaddr)) || Indentation. > + (!ipt_cap(p->ipt.raw, IPT_CAP_single_range_output) && > + !ipt_cap(p->ipt.raw, IPT_CAP_topa_output)) ) > + return 1; > + ipt_desc->ipt_guest.output_base = msr_content; > + break; > + case MSR_IA32_RTIT_OUTPUT_MASK: > + if ( (ipt_desc->ipt_guest.ctl & RTIT_CTL_TRACEEN) || > + (!ipt_cap(p->ipt.raw, IPT_CAP_single_range_output) && > + !ipt_cap(p->ipt.raw, IPT_CAP_topa_output)) ) > + return 1; > + ipt_desc->ipt_guest.output_mask = msr_content | > + RTIT_OUTPUT_MASK_DEFAULT; Again. > + break; > + case MSR_IA32_RTIT_CR3_MATCH: > + if ( (ipt_desc->ipt_guest.ctl & RTIT_CTL_TRACEEN) || > + !ipt_cap(p->ipt.raw, IPT_CAP_cr3_filter) ) > + return 1; > + ipt_desc->ipt_guest.cr3_match = msr_content; > + break; > + default: > + index = msr - MSR_IA32_RTIT_ADDR_A(0); > + if ( index >= ipt_cap(p->ipt.raw, IPT_CAP_addr_range) * 2 ) > + return 1; > + ipt_desc->ipt_guest.addr[index] = msr_content; > + } Please don't omit the "break;" above here (same in the other function). > + return 0; > +} Both functions only ever return 0 or 1 - did you mean their return type to be bool? And the perhaps better use true for success and false for failure? > @@ -204,3 +311,4 @@ void ipt_destroy(struct vcpu *v) > v->arch.hvm_vmx.ipt_desc = NULL; > } > } > + Stray addition of a blank line. > --- a/xen/arch/x86/hvm/vmx/vmx.c > +++ b/xen/arch/x86/hvm/vmx/vmx.c > @@ -2898,6 +2898,15 @@ static int vmx_msr_read_intercept(unsigned int msr, > uint64_t *msr_content) > if ( vpmu_do_rdmsr(msr, msr_content) ) > goto gp_fault; > break; > + case MSR_IA32_RTIT_CTL: > + case MSR_IA32_RTIT_STATUS: > + case MSR_IA32_RTIT_OUTPUT_BASE: > + case MSR_IA32_RTIT_OUTPUT_MASK: > + case MSR_IA32_RTIT_CR3_MATCH: > + case MSR_IA32_RTIT_ADDR_A(0) ... MSR_IA32_RTIT_ADDR_B(3): Is the 3 here an architectural limit? Otherwise you want to use a higher number here and rely on the callee to do the full checking. Jan _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxxxxxxxxx https://lists.xenproject.org/mailman/listinfo/xen-devel
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |