| Submitter | Will Auld |
|---|---|
| Date | Nov. 27, 2012, 1:35 a.m. |
| Message ID | <1353980107.8162.3.camel@WillAuldHomeLinux> |
| Download | mbox | patch |
| Permalink | /patch/202063/ |
| State | New |
| Headers | show |
Comments
On Mon, Nov 26, 2012 at 05:35:07PM -0800, Will Auld wrote: > In order to track who initiated the call (host or guest) to modify an msr > value I have changed function call parameters along the call path. The > specific change is to add a struct pointer parameter that points to (index, > data, caller) information rather than having this information passed as > individual parameters. > > The initial use for this capability is for updating the IA32_TSC_ADJUST > msr while setting the tsc value. It is anticipated that this capability > is useful other tasks. > > Signed-off-by: Will Auld <will.auld@intel.com> > --- > arch/x86/include/asm/kvm_host.h | 12 +++++++++--- > arch/x86/kvm/svm.c | 21 +++++++++++++++------ > arch/x86/kvm/vmx.c | 24 +++++++++++++++++------- > arch/x86/kvm/x86.c | 23 +++++++++++++++-------- > arch/x86/kvm/x86.h | 2 +- > 5 files changed, 57 insertions(+), 25 deletions(-) > > diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h > index 09155d6..da34027 100644 > --- a/arch/x86/include/asm/kvm_host.h > +++ b/arch/x86/include/asm/kvm_host.h > @@ -598,6 +598,12 @@ struct kvm_vcpu_stat { > > struct x86_instruction_info; > > +struct msr_data { > + bool host_initiated; > + u32 index; > + u64 data; > +}; > + > struct kvm_x86_ops { > int (*cpu_has_kvm_support)(void); /* __init */ > int (*disabled_by_bios)(void); /* __init */ > @@ -621,7 +627,7 @@ struct kvm_x86_ops { > void (*set_guest_debug)(struct kvm_vcpu *vcpu, > struct kvm_guest_debug *dbg); > int (*get_msr)(struct kvm_vcpu *vcpu, u32 msr_index, u64 *pdata); > - int (*set_msr)(struct kvm_vcpu *vcpu, u32 msr_index, u64 data); > + int (*set_msr)(struct kvm_vcpu *vcpu, struct msr_data *msr); Emulator still calls the function pointer with msr_index & data. It would be better to leave set_msr pointer as is and construct msr_data emulator_set_msr() like in V3. Just drop this set_msr signature change. > u64 (*get_segment_base)(struct kvm_vcpu *vcpu, int seg); > void (*get_segment)(struct kvm_vcpu *vcpu, > struct kvm_segment *var, int seg); > @@ -772,7 +778,7 @@ static inline int emulate_instruction(struct kvm_vcpu *vcpu, > > void kvm_enable_efer_bits(u64); > int kvm_get_msr(struct kvm_vcpu *vcpu, u32 msr_index, u64 *data); > -int kvm_set_msr(struct kvm_vcpu *vcpu, u32 msr_index, u64 data); > +int kvm_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr); > > struct x86_emulate_ctxt; > > @@ -799,7 +805,7 @@ void kvm_get_cs_db_l_bits(struct kvm_vcpu *vcpu, int *db, int *l); > int kvm_set_xcr(struct kvm_vcpu *vcpu, u32 index, u64 xcr); > > int kvm_get_msr_common(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata); > -int kvm_set_msr_common(struct kvm_vcpu *vcpu, u32 msr, u64 data); > +int kvm_set_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr); > > unsigned long kvm_get_rflags(struct kvm_vcpu *vcpu); > void kvm_set_rflags(struct kvm_vcpu *vcpu, unsigned long rflags); > diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c > index baead95..5ac11f0 100644 > --- a/arch/x86/kvm/svm.c > +++ b/arch/x86/kvm/svm.c > @@ -1211,6 +1211,7 @@ static struct kvm_vcpu *svm_create_vcpu(struct kvm *kvm, unsigned int id) > struct page *msrpm_pages; > struct page *hsave_page; > struct page *nested_msrpm_pages; > + struct msr_data msr; > int err; > > svm = kmem_cache_zalloc(kvm_vcpu_cache, GFP_KERNEL); > @@ -1255,7 +1256,10 @@ static struct kvm_vcpu *svm_create_vcpu(struct kvm *kvm, unsigned int id) > svm->vmcb_pa = page_to_pfn(page) << PAGE_SHIFT; > svm->asid_generation = 0; > init_vmcb(svm); > - kvm_write_tsc(&svm->vcpu, 0); > + msr.data = 0x0; > + msr.index = MSR_IA32_TSC; > + msr.host_initiated = true; > + kvm_write_tsc(&svm->vcpu, &msr); > > err = fx_init(&svm->vcpu); > if (err) > @@ -3147,13 +3151,15 @@ static int svm_set_vm_cr(struct kvm_vcpu *vcpu, u64 data) > return 0; > } > > -static int svm_set_msr(struct kvm_vcpu *vcpu, unsigned ecx, u64 data) > +static int svm_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr) > { > struct vcpu_svm *svm = to_svm(vcpu); > > + u32 ecx = msr->index; > + u64 data = msr->data; > switch (ecx) { > case MSR_IA32_TSC: > - kvm_write_tsc(vcpu, data); > + kvm_write_tsc(vcpu, msr); > break; > case MSR_STAR: > svm->vmcb->save.star = data; > @@ -3208,20 +3214,23 @@ static int svm_set_msr(struct kvm_vcpu *vcpu, unsigned ecx, u64 data) > vcpu_unimpl(vcpu, "unimplemented wrmsr: 0x%x data 0x%llx\n", ecx, data); > break; > default: > - return kvm_set_msr_common(vcpu, ecx, data); > + return kvm_set_msr_common(vcpu, msr); > } > return 0; > } > > static int wrmsr_interception(struct vcpu_svm *svm) > { > + struct msr_data msr; > u32 ecx = svm->vcpu.arch.regs[VCPU_REGS_RCX]; > u64 data = (svm->vcpu.arch.regs[VCPU_REGS_RAX] & -1u) > | ((u64)(svm->vcpu.arch.regs[VCPU_REGS_RDX] & -1u) << 32); > > - > + msr.data = data; > + msr.index = ecx; > + msr.host_initiated = false; > svm->next_rip = kvm_rip_read(&svm->vcpu) + 2; > - if (svm_set_msr(&svm->vcpu, ecx, data)) { > + if (svm_set_msr(&svm->vcpu, &msr)) { > trace_kvm_msr_write_ex(ecx, data); > kvm_inject_gp(&svm->vcpu, 0); > } else { > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c > index c00f03d..819970f 100644 > --- a/arch/x86/kvm/vmx.c > +++ b/arch/x86/kvm/vmx.c > @@ -2197,15 +2197,17 @@ static int vmx_get_msr(struct kvm_vcpu *vcpu, u32 msr_index, u64 *pdata) > * Returns 0 on success, non-0 otherwise. > * Assumes vcpu_load() was already called. > */ > -static int vmx_set_msr(struct kvm_vcpu *vcpu, u32 msr_index, u64 data) > +static int vmx_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info) > { > struct vcpu_vmx *vmx = to_vmx(vcpu); > struct shared_msr_entry *msr; > int ret = 0; > + u32 msr_index = msr_info->index; > + u64 data = msr_info->data; > > switch (msr_index) { > case MSR_EFER: > - ret = kvm_set_msr_common(vcpu, msr_index, data); > + ret = kvm_set_msr_common(vcpu, msr_info); > break; > #ifdef CONFIG_X86_64 > case MSR_FS_BASE: > @@ -2231,7 +2233,7 @@ static int vmx_set_msr(struct kvm_vcpu *vcpu, u32 msr_index, u64 data) > vmcs_writel(GUEST_SYSENTER_ESP, data); > break; > case MSR_IA32_TSC: > - kvm_write_tsc(vcpu, data); > + kvm_write_tsc(vcpu, msr_info); > break; > case MSR_IA32_CR_PAT: > if (vmcs_config.vmentry_ctrl & VM_ENTRY_LOAD_IA32_PAT) { > @@ -2239,7 +2241,7 @@ static int vmx_set_msr(struct kvm_vcpu *vcpu, u32 msr_index, u64 data) > vcpu->arch.pat = data; > break; > } > - ret = kvm_set_msr_common(vcpu, msr_index, data); > + ret = kvm_set_msr_common(vcpu, msr_info); > break; > case MSR_TSC_AUX: > if (!vmx->rdtscp_enabled) > @@ -2262,7 +2264,7 @@ static int vmx_set_msr(struct kvm_vcpu *vcpu, u32 msr_index, u64 data) > } > break; > } > - ret = kvm_set_msr_common(vcpu, msr_index, data); > + ret = kvm_set_msr_common(vcpu, msr_info); > } > > return ret; > @@ -3835,6 +3837,7 @@ static int vmx_vcpu_setup(struct vcpu_vmx *vmx) > unsigned long a; > #endif > int i; > + struct msr_data msr; > > /* I/O */ > vmcs_write64(IO_BITMAP_A, __pa(vmx_io_bitmap_a)); > @@ -3918,7 +3921,10 @@ static int vmx_vcpu_setup(struct vcpu_vmx *vmx) > vmcs_writel(CR0_GUEST_HOST_MASK, ~0UL); > set_cr4_guest_host_mask(vmx); > > - kvm_write_tsc(&vmx->vcpu, 0); > + msr.data = 0x0; > + msr.index = MSR_IA32_TSC; > + msr.host_initiated = true; > + kvm_write_tsc(&vmx->vcpu, &msr); > > return 0; > } > @@ -4649,11 +4655,15 @@ static int handle_rdmsr(struct kvm_vcpu *vcpu) > > static int handle_wrmsr(struct kvm_vcpu *vcpu) > { > + struct msr_data msr; > u32 ecx = vcpu->arch.regs[VCPU_REGS_RCX]; > u64 data = (vcpu->arch.regs[VCPU_REGS_RAX] & -1u) > | ((u64)(vcpu->arch.regs[VCPU_REGS_RDX] & -1u) << 32); > > - if (vmx_set_msr(vcpu, ecx, data) != 0) { > + msr.data = data; > + msr.index = ecx; > + msr.host_initiated = false; > + if (vmx_set_msr(vcpu, &msr) != 0) { > trace_kvm_msr_write_ex(ecx, data); > kvm_inject_gp(vcpu, 0); > return 1; > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index 42bce48..edafa29 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -883,9 +883,9 @@ EXPORT_SYMBOL_GPL(kvm_enable_efer_bits); > * Returns 0 on success, non-0 otherwise. > * Assumes vcpu_load() was already called. > */ > -int kvm_set_msr(struct kvm_vcpu *vcpu, u32 msr_index, u64 data) > +int kvm_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr) > { > - return kvm_x86_ops->set_msr(vcpu, msr_index, data); > + return kvm_x86_ops->set_msr(vcpu, msr); > } > > /* > @@ -893,7 +893,11 @@ int kvm_set_msr(struct kvm_vcpu *vcpu, u32 msr_index, u64 data) > */ > static int do_set_msr(struct kvm_vcpu *vcpu, unsigned index, u64 *data) > { > - return kvm_set_msr(vcpu, index, *data); > + struct msr_data msr; > + msr.data = *data; > + msr.index = index; > + msr.host_initiated = true; > + return kvm_set_msr(vcpu, &msr); > } > > static void kvm_write_wall_clock(struct kvm *kvm, gpa_t wall_clock) > @@ -1043,12 +1047,13 @@ static u64 compute_guest_tsc(struct kvm_vcpu *vcpu, s64 kernel_ns) > return tsc; > } > > -void kvm_write_tsc(struct kvm_vcpu *vcpu, u64 data) > +void kvm_write_tsc(struct kvm_vcpu *vcpu, struct msr_data *msr) > { > struct kvm *kvm = vcpu->kvm; > u64 offset, ns, elapsed; > unsigned long flags; > s64 usdiff; > + u64 data = msr->data; > > raw_spin_lock_irqsave(&kvm->arch.tsc_write_lock, flags); > offset = kvm_x86_ops->compute_tsc_offset(vcpu, data); > @@ -1561,9 +1566,11 @@ static void record_steal_time(struct kvm_vcpu *vcpu) > &vcpu->arch.st.steal, sizeof(struct kvm_steal_time)); > } > > -int kvm_set_msr_common(struct kvm_vcpu *vcpu, u32 msr, u64 data) > +int kvm_set_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info) > { > bool pr = false; > + u32 msr = msr_info->index; > + u64 data = msr_info->data; > > switch (msr) { > case MSR_EFER: > @@ -4283,10 +4290,10 @@ static int emulator_get_msr(struct x86_emulate_ctxt *ctxt, > return kvm_get_msr(emul_to_vcpu(ctxt), msr_index, pdata); > } > > -static int emulator_set_msr(struct x86_emulate_ctxt *ctxt, > - u32 msr_index, u64 data) > +static int emulator_set_msr(struct x86_emulate_ctxt *ctxt, > + struct msr_data * msr) > { > - return kvm_set_msr(emul_to_vcpu(ctxt), msr_index, data); > + return kvm_set_msr(emul_to_vcpu(ctxt), msr); > } > > static int emulator_read_pmc(struct x86_emulate_ctxt *ctxt, > diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h > index 3d1134d..1dcb8e3 100644 > --- a/arch/x86/kvm/x86.h > +++ b/arch/x86/kvm/x86.h > @@ -112,7 +112,7 @@ void kvm_before_handle_nmi(struct kvm_vcpu *vcpu); > void kvm_after_handle_nmi(struct kvm_vcpu *vcpu); > int kvm_inject_realmode_interrupt(struct kvm_vcpu *vcpu, int irq, int inc_eip); > > -void kvm_write_tsc(struct kvm_vcpu *vcpu, u64 data); > +void kvm_write_tsc(struct kvm_vcpu *vcpu, struct msr_data *msr); > > int kvm_read_guest_virt(struct x86_emulate_ctxt *ctxt, > gva_t addr, void *val, unsigned int bytes, > -- > 1.8.0.rc0 > > > -- Gleb.
Gleb,
This last change to emulator_set_msr() was wrong as you point out. I will change it back to what it was in V3 with the exception of fixing the bool that Marcelo pointed out.
However, the change of:
struct kvm_x86_ops {...
int (*set_msr)(struct kvm_vcpu *vcpu, struct msr_data *msr);
...
};
To match emulator_set_msr() prototype seems wrong because emulator_set_msr() is used with the following structure:
struct x86_emulate_ops {...
int (*set_msr)(struct x86_emulate_ctxt *ctxt, u32 msr_index, u64 data);
...
};
Rather than with "struct kvm_x86_ops" and I don't see that these two need to agree. In fact, they don't agree in the first parameter which proceeds my mucking.
If this is correct I will not make any changes to "struct kvm_x86_ops" set_msr() prototype (remain as is in V4).
Does this make since? Do you agree?
Will
> -----Original Message-----
> From: Gleb Natapov [mailto:gleb@redhat.com]
> Sent: Monday, November 26, 2012 10:59 PM
> To: Auld, Will
> Cc: qemu-devel; mtosatti@redhat.com; kvm@vger.kernel.org; Dugger,
> Donald D; Liu, Jinsong; Zhang, Xiantao; avi@redhat.com
> Subject: Re: [PATCH V4 1/2] Add code to track call origin for msr
> assignment.
>
> On Mon, Nov 26, 2012 at 05:35:07PM -0800, Will Auld wrote:
> > In order to track who initiated the call (host or guest) to modify an
> > msr value I have changed function call parameters along the call
> path.
> > The specific change is to add a struct pointer parameter that points
> > to (index, data, caller) information rather than having this
> > information passed as individual parameters.
> >
> > The initial use for this capability is for updating the
> > IA32_TSC_ADJUST msr while setting the tsc value. It is anticipated
> > that this capability is useful other tasks.
> >
> > Signed-off-by: Will Auld <will.auld@intel.com>
> > ---
> > arch/x86/include/asm/kvm_host.h | 12 +++++++++---
> > arch/x86/kvm/svm.c | 21 +++++++++++++++------
> > arch/x86/kvm/vmx.c | 24 +++++++++++++++++-------
> > arch/x86/kvm/x86.c | 23 +++++++++++++++--------
> > arch/x86/kvm/x86.h | 2 +-
> > 5 files changed, 57 insertions(+), 25 deletions(-)
> >
> > diff --git a/arch/x86/include/asm/kvm_host.h
> > b/arch/x86/include/asm/kvm_host.h index 09155d6..da34027 100644
> > --- a/arch/x86/include/asm/kvm_host.h
> > +++ b/arch/x86/include/asm/kvm_host.h
> > @@ -598,6 +598,12 @@ struct kvm_vcpu_stat {
> >
> > struct x86_instruction_info;
> >
> > +struct msr_data {
> > + bool host_initiated;
> > + u32 index;
> > + u64 data;
> > +};
> > +
> > struct kvm_x86_ops {
> > int (*cpu_has_kvm_support)(void); /* __init */
> > int (*disabled_by_bios)(void); /* __init */
> > @@ -621,7 +627,7 @@ struct kvm_x86_ops {
> > void (*set_guest_debug)(struct kvm_vcpu *vcpu,
> > struct kvm_guest_debug *dbg);
> > int (*get_msr)(struct kvm_vcpu *vcpu, u32 msr_index, u64 *pdata);
> > - int (*set_msr)(struct kvm_vcpu *vcpu, u32 msr_index, u64 data);
> > + int (*set_msr)(struct kvm_vcpu *vcpu, struct msr_data *msr);
> Emulator still calls the function pointer with msr_index & data. It
> would be better to leave set_msr pointer as is and construct msr_data
> emulator_set_msr() like in V3. Just drop this set_msr signature change.
>
> > u64 (*get_segment_base)(struct kvm_vcpu *vcpu, int seg);
> > void (*get_segment)(struct kvm_vcpu *vcpu,
> > struct kvm_segment *var, int seg); @@ -772,7
> +778,7 @@ static
> > inline int emulate_instruction(struct kvm_vcpu *vcpu,
> >
> > void kvm_enable_efer_bits(u64);
> > int kvm_get_msr(struct kvm_vcpu *vcpu, u32 msr_index, u64 *data);
> > -int kvm_set_msr(struct kvm_vcpu *vcpu, u32 msr_index, u64 data);
> > +int kvm_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr);
> >
> > struct x86_emulate_ctxt;
> >
> > @@ -799,7 +805,7 @@ void kvm_get_cs_db_l_bits(struct kvm_vcpu *vcpu,
> > int *db, int *l); int kvm_set_xcr(struct kvm_vcpu *vcpu, u32 index,
> > u64 xcr);
> >
> > int kvm_get_msr_common(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata);
> > -int kvm_set_msr_common(struct kvm_vcpu *vcpu, u32 msr, u64 data);
> > +int kvm_set_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr);
> >
> > unsigned long kvm_get_rflags(struct kvm_vcpu *vcpu); void
> > kvm_set_rflags(struct kvm_vcpu *vcpu, unsigned long rflags); diff
> > --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c index
> baead95..5ac11f0
> > 100644
> > --- a/arch/x86/kvm/svm.c
> > +++ b/arch/x86/kvm/svm.c
> > @@ -1211,6 +1211,7 @@ static struct kvm_vcpu *svm_create_vcpu(struct
> kvm *kvm, unsigned int id)
> > struct page *msrpm_pages;
> > struct page *hsave_page;
> > struct page *nested_msrpm_pages;
> > + struct msr_data msr;
> > int err;
> >
> > svm = kmem_cache_zalloc(kvm_vcpu_cache, GFP_KERNEL); @@ -1255,7
> > +1256,10 @@ static struct kvm_vcpu *svm_create_vcpu(struct kvm *kvm,
> unsigned int id)
> > svm->vmcb_pa = page_to_pfn(page) << PAGE_SHIFT;
> > svm->asid_generation = 0;
> > init_vmcb(svm);
> > - kvm_write_tsc(&svm->vcpu, 0);
> > + msr.data = 0x0;
> > + msr.index = MSR_IA32_TSC;
> > + msr.host_initiated = true;
> > + kvm_write_tsc(&svm->vcpu, &msr);
> >
> > err = fx_init(&svm->vcpu);
> > if (err)
> > @@ -3147,13 +3151,15 @@ static int svm_set_vm_cr(struct kvm_vcpu
> *vcpu, u64 data)
> > return 0;
> > }
> >
> > -static int svm_set_msr(struct kvm_vcpu *vcpu, unsigned ecx, u64
> data)
> > +static int svm_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr)
> > {
> > struct vcpu_svm *svm = to_svm(vcpu);
> >
> > + u32 ecx = msr->index;
> > + u64 data = msr->data;
> > switch (ecx) {
> > case MSR_IA32_TSC:
> > - kvm_write_tsc(vcpu, data);
> > + kvm_write_tsc(vcpu, msr);
> > break;
> > case MSR_STAR:
> > svm->vmcb->save.star = data;
> > @@ -3208,20 +3214,23 @@ static int svm_set_msr(struct kvm_vcpu *vcpu,
> unsigned ecx, u64 data)
> > vcpu_unimpl(vcpu, "unimplemented wrmsr: 0x%x data
> 0x%llx\n", ecx, data);
> > break;
> > default:
> > - return kvm_set_msr_common(vcpu, ecx, data);
> > + return kvm_set_msr_common(vcpu, msr);
> > }
> > return 0;
> > }
> >
> > static int wrmsr_interception(struct vcpu_svm *svm) {
> > + struct msr_data msr;
> > u32 ecx = svm->vcpu.arch.regs[VCPU_REGS_RCX];
> > u64 data = (svm->vcpu.arch.regs[VCPU_REGS_RAX] & -1u)
> > | ((u64)(svm->vcpu.arch.regs[VCPU_REGS_RDX] & -1u) << 32);
> >
> > -
> > + msr.data = data;
> > + msr.index = ecx;
> > + msr.host_initiated = false;
> > svm->next_rip = kvm_rip_read(&svm->vcpu) + 2;
> > - if (svm_set_msr(&svm->vcpu, ecx, data)) {
> > + if (svm_set_msr(&svm->vcpu, &msr)) {
> > trace_kvm_msr_write_ex(ecx, data);
> > kvm_inject_gp(&svm->vcpu, 0);
> > } else {
> > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c index
> > c00f03d..819970f 100644
> > --- a/arch/x86/kvm/vmx.c
> > +++ b/arch/x86/kvm/vmx.c
> > @@ -2197,15 +2197,17 @@ static int vmx_get_msr(struct kvm_vcpu *vcpu,
> u32 msr_index, u64 *pdata)
> > * Returns 0 on success, non-0 otherwise.
> > * Assumes vcpu_load() was already called.
> > */
> > -static int vmx_set_msr(struct kvm_vcpu *vcpu, u32 msr_index, u64
> > data)
> > +static int vmx_set_msr(struct kvm_vcpu *vcpu, struct msr_data
> > +*msr_info)
> > {
> > struct vcpu_vmx *vmx = to_vmx(vcpu);
> > struct shared_msr_entry *msr;
> > int ret = 0;
> > + u32 msr_index = msr_info->index;
> > + u64 data = msr_info->data;
> >
> > switch (msr_index) {
> > case MSR_EFER:
> > - ret = kvm_set_msr_common(vcpu, msr_index, data);
> > + ret = kvm_set_msr_common(vcpu, msr_info);
> > break;
> > #ifdef CONFIG_X86_64
> > case MSR_FS_BASE:
> > @@ -2231,7 +2233,7 @@ static int vmx_set_msr(struct kvm_vcpu *vcpu,
> u32 msr_index, u64 data)
> > vmcs_writel(GUEST_SYSENTER_ESP, data);
> > break;
> > case MSR_IA32_TSC:
> > - kvm_write_tsc(vcpu, data);
> > + kvm_write_tsc(vcpu, msr_info);
> > break;
> > case MSR_IA32_CR_PAT:
> > if (vmcs_config.vmentry_ctrl & VM_ENTRY_LOAD_IA32_PAT) { @@
> -2239,7
> > +2241,7 @@ static int vmx_set_msr(struct kvm_vcpu *vcpu, u32
> msr_index, u64 data)
> > vcpu->arch.pat = data;
> > break;
> > }
> > - ret = kvm_set_msr_common(vcpu, msr_index, data);
> > + ret = kvm_set_msr_common(vcpu, msr_info);
> > break;
> > case MSR_TSC_AUX:
> > if (!vmx->rdtscp_enabled)
> > @@ -2262,7 +2264,7 @@ static int vmx_set_msr(struct kvm_vcpu *vcpu,
> u32 msr_index, u64 data)
> > }
> > break;
> > }
> > - ret = kvm_set_msr_common(vcpu, msr_index, data);
> > + ret = kvm_set_msr_common(vcpu, msr_info);
> > }
> >
> > return ret;
> > @@ -3835,6 +3837,7 @@ static int vmx_vcpu_setup(struct vcpu_vmx *vmx)
> > unsigned long a;
> > #endif
> > int i;
> > + struct msr_data msr;
> >
> > /* I/O */
> > vmcs_write64(IO_BITMAP_A, __pa(vmx_io_bitmap_a)); @@ -3918,7
> > +3921,10 @@ static int vmx_vcpu_setup(struct vcpu_vmx *vmx)
> > vmcs_writel(CR0_GUEST_HOST_MASK, ~0UL);
> > set_cr4_guest_host_mask(vmx);
> >
> > - kvm_write_tsc(&vmx->vcpu, 0);
> > + msr.data = 0x0;
> > + msr.index = MSR_IA32_TSC;
> > + msr.host_initiated = true;
> > + kvm_write_tsc(&vmx->vcpu, &msr);
> >
> > return 0;
> > }
> > @@ -4649,11 +4655,15 @@ static int handle_rdmsr(struct kvm_vcpu
> *vcpu)
> >
> > static int handle_wrmsr(struct kvm_vcpu *vcpu) {
> > + struct msr_data msr;
> > u32 ecx = vcpu->arch.regs[VCPU_REGS_RCX];
> > u64 data = (vcpu->arch.regs[VCPU_REGS_RAX] & -1u)
> > | ((u64)(vcpu->arch.regs[VCPU_REGS_RDX] & -1u) << 32);
> >
> > - if (vmx_set_msr(vcpu, ecx, data) != 0) {
> > + msr.data = data;
> > + msr.index = ecx;
> > + msr.host_initiated = false;
> > + if (vmx_set_msr(vcpu, &msr) != 0) {
> > trace_kvm_msr_write_ex(ecx, data);
> > kvm_inject_gp(vcpu, 0);
> > return 1;
> > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index
> > 42bce48..edafa29 100644
> > --- a/arch/x86/kvm/x86.c
> > +++ b/arch/x86/kvm/x86.c
> > @@ -883,9 +883,9 @@ EXPORT_SYMBOL_GPL(kvm_enable_efer_bits);
> > * Returns 0 on success, non-0 otherwise.
> > * Assumes vcpu_load() was already called.
> > */
> > -int kvm_set_msr(struct kvm_vcpu *vcpu, u32 msr_index, u64 data)
> > +int kvm_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr)
> > {
> > - return kvm_x86_ops->set_msr(vcpu, msr_index, data);
> > + return kvm_x86_ops->set_msr(vcpu, msr);
> > }
> >
> > /*
> > @@ -893,7 +893,11 @@ int kvm_set_msr(struct kvm_vcpu *vcpu, u32
> msr_index, u64 data)
> > */
> > static int do_set_msr(struct kvm_vcpu *vcpu, unsigned index, u64
> > *data) {
> > - return kvm_set_msr(vcpu, index, *data);
> > + struct msr_data msr;
> > + msr.data = *data;
> > + msr.index = index;
> > + msr.host_initiated = true;
> > + return kvm_set_msr(vcpu, &msr);
> > }
> >
> > static void kvm_write_wall_clock(struct kvm *kvm, gpa_t wall_clock)
> > @@ -1043,12 +1047,13 @@ static u64 compute_guest_tsc(struct kvm_vcpu
> *vcpu, s64 kernel_ns)
> > return tsc;
> > }
> >
> > -void kvm_write_tsc(struct kvm_vcpu *vcpu, u64 data)
> > +void kvm_write_tsc(struct kvm_vcpu *vcpu, struct msr_data *msr)
> > {
> > struct kvm *kvm = vcpu->kvm;
> > u64 offset, ns, elapsed;
> > unsigned long flags;
> > s64 usdiff;
> > + u64 data = msr->data;
> >
> > raw_spin_lock_irqsave(&kvm->arch.tsc_write_lock, flags);
> > offset = kvm_x86_ops->compute_tsc_offset(vcpu, data); @@ -1561,9
> > +1566,11 @@ static void record_steal_time(struct kvm_vcpu *vcpu)
> > &vcpu->arch.st.steal, sizeof(struct kvm_steal_time)); }
> >
> > -int kvm_set_msr_common(struct kvm_vcpu *vcpu, u32 msr, u64 data)
> > +int kvm_set_msr_common(struct kvm_vcpu *vcpu, struct msr_data
> > +*msr_info)
> > {
> > bool pr = false;
> > + u32 msr = msr_info->index;
> > + u64 data = msr_info->data;
> >
> > switch (msr) {
> > case MSR_EFER:
> > @@ -4283,10 +4290,10 @@ static int emulator_get_msr(struct
> x86_emulate_ctxt *ctxt,
> > return kvm_get_msr(emul_to_vcpu(ctxt), msr_index, pdata); }
> >
> > -static int emulator_set_msr(struct x86_emulate_ctxt *ctxt,
> > - u32 msr_index, u64 data)
> > +static int emulator_set_msr(struct x86_emulate_ctxt *ctxt,
> > + struct msr_data * msr)
> > {
> > - return kvm_set_msr(emul_to_vcpu(ctxt), msr_index, data);
> > + return kvm_set_msr(emul_to_vcpu(ctxt), msr);
> > }
> >
> > static int emulator_read_pmc(struct x86_emulate_ctxt *ctxt, diff
> > --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h index
> 3d1134d..1dcb8e3
> > 100644
> > --- a/arch/x86/kvm/x86.h
> > +++ b/arch/x86/kvm/x86.h
> > @@ -112,7 +112,7 @@ void kvm_before_handle_nmi(struct kvm_vcpu
> *vcpu);
> > void kvm_after_handle_nmi(struct kvm_vcpu *vcpu); int
> > kvm_inject_realmode_interrupt(struct kvm_vcpu *vcpu, int irq, int
> > inc_eip);
> >
> > -void kvm_write_tsc(struct kvm_vcpu *vcpu, u64 data);
> > +void kvm_write_tsc(struct kvm_vcpu *vcpu, struct msr_data *msr);
> >
> > int kvm_read_guest_virt(struct x86_emulate_ctxt *ctxt,
> > gva_t addr, void *val, unsigned int bytes,
> > --
> > 1.8.0.rc0
> >
> >
> >
>
> --
> Gleb.
On Tue, Nov 27, 2012 at 06:19:21PM +0000, Auld, Will wrote: > Gleb, > > This last change to emulator_set_msr() was wrong as you point out. I will change it back to what it was in V3 with the exception of fixing the bool that Marcelo pointed out. > > However, the change of: > > struct kvm_x86_ops {... > int (*set_msr)(struct kvm_vcpu *vcpu, struct msr_data *msr); > ... > }; > > To match emulator_set_msr() prototype seems wrong because emulator_set_msr() is used with the following structure: > > struct x86_emulate_ops {... > int (*set_msr)(struct x86_emulate_ctxt *ctxt, u32 msr_index, u64 data); > ... > }; > > Rather than with "struct kvm_x86_ops" and I don't see that these two need to agree. In fact, they don't agree in the first parameter which proceeds my mucking. > > If this is correct I will not make any changes to "struct kvm_x86_ops" set_msr() prototype (remain as is in V4). > > Does this make since? Do you agree? > Ugh, yeah. I looked at the wrong structure. Disregard this. Change true to false in V3. Sorry about the confusion. > Will > > > -----Original Message----- > > From: Gleb Natapov [mailto:gleb@redhat.com] > > Sent: Monday, November 26, 2012 10:59 PM > > To: Auld, Will > > Cc: qemu-devel; mtosatti@redhat.com; kvm@vger.kernel.org; Dugger, > > Donald D; Liu, Jinsong; Zhang, Xiantao; avi@redhat.com > > Subject: Re: [PATCH V4 1/2] Add code to track call origin for msr > > assignment. > > > > On Mon, Nov 26, 2012 at 05:35:07PM -0800, Will Auld wrote: > > > In order to track who initiated the call (host or guest) to modify an > > > msr value I have changed function call parameters along the call > > path. > > > The specific change is to add a struct pointer parameter that points > > > to (index, data, caller) information rather than having this > > > information passed as individual parameters. > > > > > > The initial use for this capability is for updating the > > > IA32_TSC_ADJUST msr while setting the tsc value. It is anticipated > > > that this capability is useful other tasks. > > > > > > Signed-off-by: Will Auld <will.auld@intel.com> > > > --- > > > arch/x86/include/asm/kvm_host.h | 12 +++++++++--- > > > arch/x86/kvm/svm.c | 21 +++++++++++++++------ > > > arch/x86/kvm/vmx.c | 24 +++++++++++++++++------- > > > arch/x86/kvm/x86.c | 23 +++++++++++++++-------- > > > arch/x86/kvm/x86.h | 2 +- > > > 5 files changed, 57 insertions(+), 25 deletions(-) > > > > > > diff --git a/arch/x86/include/asm/kvm_host.h > > > b/arch/x86/include/asm/kvm_host.h index 09155d6..da34027 100644 > > > --- a/arch/x86/include/asm/kvm_host.h > > > +++ b/arch/x86/include/asm/kvm_host.h > > > @@ -598,6 +598,12 @@ struct kvm_vcpu_stat { > > > > > > struct x86_instruction_info; > > > > > > +struct msr_data { > > > + bool host_initiated; > > > + u32 index; > > > + u64 data; > > > +}; > > > + > > > struct kvm_x86_ops { > > > int (*cpu_has_kvm_support)(void); /* __init */ > > > int (*disabled_by_bios)(void); /* __init */ > > > @@ -621,7 +627,7 @@ struct kvm_x86_ops { > > > void (*set_guest_debug)(struct kvm_vcpu *vcpu, > > > struct kvm_guest_debug *dbg); > > > int (*get_msr)(struct kvm_vcpu *vcpu, u32 msr_index, u64 *pdata); > > > - int (*set_msr)(struct kvm_vcpu *vcpu, u32 msr_index, u64 data); > > > + int (*set_msr)(struct kvm_vcpu *vcpu, struct msr_data *msr); > > Emulator still calls the function pointer with msr_index & data. It > > would be better to leave set_msr pointer as is and construct msr_data > > emulator_set_msr() like in V3. Just drop this set_msr signature change. > > > > > u64 (*get_segment_base)(struct kvm_vcpu *vcpu, int seg); > > > void (*get_segment)(struct kvm_vcpu *vcpu, > > > struct kvm_segment *var, int seg); @@ -772,7 > > +778,7 @@ static > > > inline int emulate_instruction(struct kvm_vcpu *vcpu, > > > > > > void kvm_enable_efer_bits(u64); > > > int kvm_get_msr(struct kvm_vcpu *vcpu, u32 msr_index, u64 *data); > > > -int kvm_set_msr(struct kvm_vcpu *vcpu, u32 msr_index, u64 data); > > > +int kvm_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr); > > > > > > struct x86_emulate_ctxt; > > > > > > @@ -799,7 +805,7 @@ void kvm_get_cs_db_l_bits(struct kvm_vcpu *vcpu, > > > int *db, int *l); int kvm_set_xcr(struct kvm_vcpu *vcpu, u32 index, > > > u64 xcr); > > > > > > int kvm_get_msr_common(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata); > > > -int kvm_set_msr_common(struct kvm_vcpu *vcpu, u32 msr, u64 data); > > > +int kvm_set_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr); > > > > > > unsigned long kvm_get_rflags(struct kvm_vcpu *vcpu); void > > > kvm_set_rflags(struct kvm_vcpu *vcpu, unsigned long rflags); diff > > > --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c index > > baead95..5ac11f0 > > > 100644 > > > --- a/arch/x86/kvm/svm.c > > > +++ b/arch/x86/kvm/svm.c > > > @@ -1211,6 +1211,7 @@ static struct kvm_vcpu *svm_create_vcpu(struct > > kvm *kvm, unsigned int id) > > > struct page *msrpm_pages; > > > struct page *hsave_page; > > > struct page *nested_msrpm_pages; > > > + struct msr_data msr; > > > int err; > > > > > > svm = kmem_cache_zalloc(kvm_vcpu_cache, GFP_KERNEL); @@ -1255,7 > > > +1256,10 @@ static struct kvm_vcpu *svm_create_vcpu(struct kvm *kvm, > > unsigned int id) > > > svm->vmcb_pa = page_to_pfn(page) << PAGE_SHIFT; > > > svm->asid_generation = 0; > > > init_vmcb(svm); > > > - kvm_write_tsc(&svm->vcpu, 0); > > > + msr.data = 0x0; > > > + msr.index = MSR_IA32_TSC; > > > + msr.host_initiated = true; > > > + kvm_write_tsc(&svm->vcpu, &msr); > > > > > > err = fx_init(&svm->vcpu); > > > if (err) > > > @@ -3147,13 +3151,15 @@ static int svm_set_vm_cr(struct kvm_vcpu > > *vcpu, u64 data) > > > return 0; > > > } > > > > > > -static int svm_set_msr(struct kvm_vcpu *vcpu, unsigned ecx, u64 > > data) > > > +static int svm_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr) > > > { > > > struct vcpu_svm *svm = to_svm(vcpu); > > > > > > + u32 ecx = msr->index; > > > + u64 data = msr->data; > > > switch (ecx) { > > > case MSR_IA32_TSC: > > > - kvm_write_tsc(vcpu, data); > > > + kvm_write_tsc(vcpu, msr); > > > break; > > > case MSR_STAR: > > > svm->vmcb->save.star = data; > > > @@ -3208,20 +3214,23 @@ static int svm_set_msr(struct kvm_vcpu *vcpu, > > unsigned ecx, u64 data) > > > vcpu_unimpl(vcpu, "unimplemented wrmsr: 0x%x data > > 0x%llx\n", ecx, data); > > > break; > > > default: > > > - return kvm_set_msr_common(vcpu, ecx, data); > > > + return kvm_set_msr_common(vcpu, msr); > > > } > > > return 0; > > > } > > > > > > static int wrmsr_interception(struct vcpu_svm *svm) { > > > + struct msr_data msr; > > > u32 ecx = svm->vcpu.arch.regs[VCPU_REGS_RCX]; > > > u64 data = (svm->vcpu.arch.regs[VCPU_REGS_RAX] & -1u) > > > | ((u64)(svm->vcpu.arch.regs[VCPU_REGS_RDX] & -1u) << 32); > > > > > > - > > > + msr.data = data; > > > + msr.index = ecx; > > > + msr.host_initiated = false; > > > svm->next_rip = kvm_rip_read(&svm->vcpu) + 2; > > > - if (svm_set_msr(&svm->vcpu, ecx, data)) { > > > + if (svm_set_msr(&svm->vcpu, &msr)) { > > > trace_kvm_msr_write_ex(ecx, data); > > > kvm_inject_gp(&svm->vcpu, 0); > > > } else { > > > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c index > > > c00f03d..819970f 100644 > > > --- a/arch/x86/kvm/vmx.c > > > +++ b/arch/x86/kvm/vmx.c > > > @@ -2197,15 +2197,17 @@ static int vmx_get_msr(struct kvm_vcpu *vcpu, > > u32 msr_index, u64 *pdata) > > > * Returns 0 on success, non-0 otherwise. > > > * Assumes vcpu_load() was already called. > > > */ > > > -static int vmx_set_msr(struct kvm_vcpu *vcpu, u32 msr_index, u64 > > > data) > > > +static int vmx_set_msr(struct kvm_vcpu *vcpu, struct msr_data > > > +*msr_info) > > > { > > > struct vcpu_vmx *vmx = to_vmx(vcpu); > > > struct shared_msr_entry *msr; > > > int ret = 0; > > > + u32 msr_index = msr_info->index; > > > + u64 data = msr_info->data; > > > > > > switch (msr_index) { > > > case MSR_EFER: > > > - ret = kvm_set_msr_common(vcpu, msr_index, data); > > > + ret = kvm_set_msr_common(vcpu, msr_info); > > > break; > > > #ifdef CONFIG_X86_64 > > > case MSR_FS_BASE: > > > @@ -2231,7 +2233,7 @@ static int vmx_set_msr(struct kvm_vcpu *vcpu, > > u32 msr_index, u64 data) > > > vmcs_writel(GUEST_SYSENTER_ESP, data); > > > break; > > > case MSR_IA32_TSC: > > > - kvm_write_tsc(vcpu, data); > > > + kvm_write_tsc(vcpu, msr_info); > > > break; > > > case MSR_IA32_CR_PAT: > > > if (vmcs_config.vmentry_ctrl & VM_ENTRY_LOAD_IA32_PAT) { @@ > > -2239,7 > > > +2241,7 @@ static int vmx_set_msr(struct kvm_vcpu *vcpu, u32 > > msr_index, u64 data) > > > vcpu->arch.pat = data; > > > break; > > > } > > > - ret = kvm_set_msr_common(vcpu, msr_index, data); > > > + ret = kvm_set_msr_common(vcpu, msr_info); > > > break; > > > case MSR_TSC_AUX: > > > if (!vmx->rdtscp_enabled) > > > @@ -2262,7 +2264,7 @@ static int vmx_set_msr(struct kvm_vcpu *vcpu, > > u32 msr_index, u64 data) > > > } > > > break; > > > } > > > - ret = kvm_set_msr_common(vcpu, msr_index, data); > > > + ret = kvm_set_msr_common(vcpu, msr_info); > > > } > > > > > > return ret; > > > @@ -3835,6 +3837,7 @@ static int vmx_vcpu_setup(struct vcpu_vmx *vmx) > > > unsigned long a; > > > #endif > > > int i; > > > + struct msr_data msr; > > > > > > /* I/O */ > > > vmcs_write64(IO_BITMAP_A, __pa(vmx_io_bitmap_a)); @@ -3918,7 > > > +3921,10 @@ static int vmx_vcpu_setup(struct vcpu_vmx *vmx) > > > vmcs_writel(CR0_GUEST_HOST_MASK, ~0UL); > > > set_cr4_guest_host_mask(vmx); > > > > > > - kvm_write_tsc(&vmx->vcpu, 0); > > > + msr.data = 0x0; > > > + msr.index = MSR_IA32_TSC; > > > + msr.host_initiated = true; > > > + kvm_write_tsc(&vmx->vcpu, &msr); > > > > > > return 0; > > > } > > > @@ -4649,11 +4655,15 @@ static int handle_rdmsr(struct kvm_vcpu > > *vcpu) > > > > > > static int handle_wrmsr(struct kvm_vcpu *vcpu) { > > > + struct msr_data msr; > > > u32 ecx = vcpu->arch.regs[VCPU_REGS_RCX]; > > > u64 data = (vcpu->arch.regs[VCPU_REGS_RAX] & -1u) > > > | ((u64)(vcpu->arch.regs[VCPU_REGS_RDX] & -1u) << 32); > > > > > > - if (vmx_set_msr(vcpu, ecx, data) != 0) { > > > + msr.data = data; > > > + msr.index = ecx; > > > + msr.host_initiated = false; > > > + if (vmx_set_msr(vcpu, &msr) != 0) { > > > trace_kvm_msr_write_ex(ecx, data); > > > kvm_inject_gp(vcpu, 0); > > > return 1; > > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index > > > 42bce48..edafa29 100644 > > > --- a/arch/x86/kvm/x86.c > > > +++ b/arch/x86/kvm/x86.c > > > @@ -883,9 +883,9 @@ EXPORT_SYMBOL_GPL(kvm_enable_efer_bits); > > > * Returns 0 on success, non-0 otherwise. > > > * Assumes vcpu_load() was already called. > > > */ > > > -int kvm_set_msr(struct kvm_vcpu *vcpu, u32 msr_index, u64 data) > > > +int kvm_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr) > > > { > > > - return kvm_x86_ops->set_msr(vcpu, msr_index, data); > > > + return kvm_x86_ops->set_msr(vcpu, msr); > > > } > > > > > > /* > > > @@ -893,7 +893,11 @@ int kvm_set_msr(struct kvm_vcpu *vcpu, u32 > > msr_index, u64 data) > > > */ > > > static int do_set_msr(struct kvm_vcpu *vcpu, unsigned index, u64 > > > *data) { > > > - return kvm_set_msr(vcpu, index, *data); > > > + struct msr_data msr; > > > + msr.data = *data; > > > + msr.index = index; > > > + msr.host_initiated = true; > > > + return kvm_set_msr(vcpu, &msr); > > > } > > > > > > static void kvm_write_wall_clock(struct kvm *kvm, gpa_t wall_clock) > > > @@ -1043,12 +1047,13 @@ static u64 compute_guest_tsc(struct kvm_vcpu > > *vcpu, s64 kernel_ns) > > > return tsc; > > > } > > > > > > -void kvm_write_tsc(struct kvm_vcpu *vcpu, u64 data) > > > +void kvm_write_tsc(struct kvm_vcpu *vcpu, struct msr_data *msr) > > > { > > > struct kvm *kvm = vcpu->kvm; > > > u64 offset, ns, elapsed; > > > unsigned long flags; > > > s64 usdiff; > > > + u64 data = msr->data; > > > > > > raw_spin_lock_irqsave(&kvm->arch.tsc_write_lock, flags); > > > offset = kvm_x86_ops->compute_tsc_offset(vcpu, data); @@ -1561,9 > > > +1566,11 @@ static void record_steal_time(struct kvm_vcpu *vcpu) > > > &vcpu->arch.st.steal, sizeof(struct kvm_steal_time)); } > > > > > > -int kvm_set_msr_common(struct kvm_vcpu *vcpu, u32 msr, u64 data) > > > +int kvm_set_msr_common(struct kvm_vcpu *vcpu, struct msr_data > > > +*msr_info) > > > { > > > bool pr = false; > > > + u32 msr = msr_info->index; > > > + u64 data = msr_info->data; > > > > > > switch (msr) { > > > case MSR_EFER: > > > @@ -4283,10 +4290,10 @@ static int emulator_get_msr(struct > > x86_emulate_ctxt *ctxt, > > > return kvm_get_msr(emul_to_vcpu(ctxt), msr_index, pdata); } > > > > > > -static int emulator_set_msr(struct x86_emulate_ctxt *ctxt, > > > - u32 msr_index, u64 data) > > > +static int emulator_set_msr(struct x86_emulate_ctxt *ctxt, > > > + struct msr_data * msr) > > > { > > > - return kvm_set_msr(emul_to_vcpu(ctxt), msr_index, data); > > > + return kvm_set_msr(emul_to_vcpu(ctxt), msr); > > > } > > > > > > static int emulator_read_pmc(struct x86_emulate_ctxt *ctxt, diff > > > --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h index > > 3d1134d..1dcb8e3 > > > 100644 > > > --- a/arch/x86/kvm/x86.h > > > +++ b/arch/x86/kvm/x86.h > > > @@ -112,7 +112,7 @@ void kvm_before_handle_nmi(struct kvm_vcpu > > *vcpu); > > > void kvm_after_handle_nmi(struct kvm_vcpu *vcpu); int > > > kvm_inject_realmode_interrupt(struct kvm_vcpu *vcpu, int irq, int > > > inc_eip); > > > > > > -void kvm_write_tsc(struct kvm_vcpu *vcpu, u64 data); > > > +void kvm_write_tsc(struct kvm_vcpu *vcpu, struct msr_data *msr); > > > > > > int kvm_read_guest_virt(struct x86_emulate_ctxt *ctxt, > > > gva_t addr, void *val, unsigned int bytes, > > > -- > > > 1.8.0.rc0 > > > > > > > > > > > > > -- > > Gleb. -- Gleb.
Patch
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h index 09155d6..da34027 100644 --- a/arch/x86/include/asm/kvm_host.h +++ b/arch/x86/include/asm/kvm_host.h @@ -598,6 +598,12 @@ struct kvm_vcpu_stat { struct x86_instruction_info; +struct msr_data { + bool host_initiated; + u32 index; + u64 data; +}; + struct kvm_x86_ops { int (*cpu_has_kvm_support)(void); /* __init */ int (*disabled_by_bios)(void); /* __init */ @@ -621,7 +627,7 @@ struct kvm_x86_ops { void (*set_guest_debug)(struct kvm_vcpu *vcpu, struct kvm_guest_debug *dbg); int (*get_msr)(struct kvm_vcpu *vcpu, u32 msr_index, u64 *pdata); - int (*set_msr)(struct kvm_vcpu *vcpu, u32 msr_index, u64 data); + int (*set_msr)(struct kvm_vcpu *vcpu, struct msr_data *msr); u64 (*get_segment_base)(struct kvm_vcpu *vcpu, int seg); void (*get_segment)(struct kvm_vcpu *vcpu, struct kvm_segment *var, int seg); @@ -772,7 +778,7 @@ static inline int emulate_instruction(struct kvm_vcpu *vcpu, void kvm_enable_efer_bits(u64); int kvm_get_msr(struct kvm_vcpu *vcpu, u32 msr_index, u64 *data); -int kvm_set_msr(struct kvm_vcpu *vcpu, u32 msr_index, u64 data); +int kvm_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr); struct x86_emulate_ctxt; @@ -799,7 +805,7 @@ void kvm_get_cs_db_l_bits(struct kvm_vcpu *vcpu, int *db, int *l); int kvm_set_xcr(struct kvm_vcpu *vcpu, u32 index, u64 xcr); int kvm_get_msr_common(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata); -int kvm_set_msr_common(struct kvm_vcpu *vcpu, u32 msr, u64 data); +int kvm_set_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr); unsigned long kvm_get_rflags(struct kvm_vcpu *vcpu); void kvm_set_rflags(struct kvm_vcpu *vcpu, unsigned long rflags); diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c index baead95..5ac11f0 100644 --- a/arch/x86/kvm/svm.c +++ b/arch/x86/kvm/svm.c @@ -1211,6 +1211,7 @@ static struct kvm_vcpu *svm_create_vcpu(struct kvm *kvm, unsigned int id) struct page *msrpm_pages; struct page *hsave_page; struct page *nested_msrpm_pages; + struct msr_data msr; int err; svm = kmem_cache_zalloc(kvm_vcpu_cache, GFP_KERNEL); @@ -1255,7 +1256,10 @@ static struct kvm_vcpu *svm_create_vcpu(struct kvm *kvm, unsigned int id) svm->vmcb_pa = page_to_pfn(page) << PAGE_SHIFT; svm->asid_generation = 0; init_vmcb(svm); - kvm_write_tsc(&svm->vcpu, 0); + msr.data = 0x0; + msr.index = MSR_IA32_TSC; + msr.host_initiated = true; + kvm_write_tsc(&svm->vcpu, &msr); err = fx_init(&svm->vcpu); if (err) @@ -3147,13 +3151,15 @@ static int svm_set_vm_cr(struct kvm_vcpu *vcpu, u64 data) return 0; } -static int svm_set_msr(struct kvm_vcpu *vcpu, unsigned ecx, u64 data) +static int svm_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr) { struct vcpu_svm *svm = to_svm(vcpu); + u32 ecx = msr->index; + u64 data = msr->data; switch (ecx) { case MSR_IA32_TSC: - kvm_write_tsc(vcpu, data); + kvm_write_tsc(vcpu, msr); break; case MSR_STAR: svm->vmcb->save.star = data; @@ -3208,20 +3214,23 @@ static int svm_set_msr(struct kvm_vcpu *vcpu, unsigned ecx, u64 data) vcpu_unimpl(vcpu, "unimplemented wrmsr: 0x%x data 0x%llx\n", ecx, data); break; default: - return kvm_set_msr_common(vcpu, ecx, data); + return kvm_set_msr_common(vcpu, msr); } return 0; } static int wrmsr_interception(struct vcpu_svm *svm) { + struct msr_data msr; u32 ecx = svm->vcpu.arch.regs[VCPU_REGS_RCX]; u64 data = (svm->vcpu.arch.regs[VCPU_REGS_RAX] & -1u) | ((u64)(svm->vcpu.arch.regs[VCPU_REGS_RDX] & -1u) << 32); - + msr.data = data; + msr.index = ecx; + msr.host_initiated = false; svm->next_rip = kvm_rip_read(&svm->vcpu) + 2; - if (svm_set_msr(&svm->vcpu, ecx, data)) { + if (svm_set_msr(&svm->vcpu, &msr)) { trace_kvm_msr_write_ex(ecx, data); kvm_inject_gp(&svm->vcpu, 0); } else { diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c index c00f03d..819970f 100644 --- a/arch/x86/kvm/vmx.c +++ b/arch/x86/kvm/vmx.c @@ -2197,15 +2197,17 @@ static int vmx_get_msr(struct kvm_vcpu *vcpu, u32 msr_index, u64 *pdata) * Returns 0 on success, non-0 otherwise. * Assumes vcpu_load() was already called. */ -static int vmx_set_msr(struct kvm_vcpu *vcpu, u32 msr_index, u64 data) +static int vmx_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info) { struct vcpu_vmx *vmx = to_vmx(vcpu); struct shared_msr_entry *msr; int ret = 0; + u32 msr_index = msr_info->index; + u64 data = msr_info->data; switch (msr_index) { case MSR_EFER: - ret = kvm_set_msr_common(vcpu, msr_index, data); + ret = kvm_set_msr_common(vcpu, msr_info); break; #ifdef CONFIG_X86_64 case MSR_FS_BASE: @@ -2231,7 +2233,7 @@ static int vmx_set_msr(struct kvm_vcpu *vcpu, u32 msr_index, u64 data) vmcs_writel(GUEST_SYSENTER_ESP, data); break; case MSR_IA32_TSC: - kvm_write_tsc(vcpu, data); + kvm_write_tsc(vcpu, msr_info); break; case MSR_IA32_CR_PAT: if (vmcs_config.vmentry_ctrl & VM_ENTRY_LOAD_IA32_PAT) { @@ -2239,7 +2241,7 @@ static int vmx_set_msr(struct kvm_vcpu *vcpu, u32 msr_index, u64 data) vcpu->arch.pat = data; break; } - ret = kvm_set_msr_common(vcpu, msr_index, data); + ret = kvm_set_msr_common(vcpu, msr_info); break; case MSR_TSC_AUX: if (!vmx->rdtscp_enabled) @@ -2262,7 +2264,7 @@ static int vmx_set_msr(struct kvm_vcpu *vcpu, u32 msr_index, u64 data) } break; } - ret = kvm_set_msr_common(vcpu, msr_index, data); + ret = kvm_set_msr_common(vcpu, msr_info); } return ret; @@ -3835,6 +3837,7 @@ static int vmx_vcpu_setup(struct vcpu_vmx *vmx) unsigned long a; #endif int i; + struct msr_data msr; /* I/O */ vmcs_write64(IO_BITMAP_A, __pa(vmx_io_bitmap_a)); @@ -3918,7 +3921,10 @@ static int vmx_vcpu_setup(struct vcpu_vmx *vmx) vmcs_writel(CR0_GUEST_HOST_MASK, ~0UL); set_cr4_guest_host_mask(vmx); - kvm_write_tsc(&vmx->vcpu, 0); + msr.data = 0x0; + msr.index = MSR_IA32_TSC; + msr.host_initiated = true; + kvm_write_tsc(&vmx->vcpu, &msr); return 0; } @@ -4649,11 +4655,15 @@ static int handle_rdmsr(struct kvm_vcpu *vcpu) static int handle_wrmsr(struct kvm_vcpu *vcpu) { + struct msr_data msr; u32 ecx = vcpu->arch.regs[VCPU_REGS_RCX]; u64 data = (vcpu->arch.regs[VCPU_REGS_RAX] & -1u) | ((u64)(vcpu->arch.regs[VCPU_REGS_RDX] & -1u) << 32); - if (vmx_set_msr(vcpu, ecx, data) != 0) { + msr.data = data; + msr.index = ecx; + msr.host_initiated = false; + if (vmx_set_msr(vcpu, &msr) != 0) { trace_kvm_msr_write_ex(ecx, data); kvm_inject_gp(vcpu, 0); return 1; diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index 42bce48..edafa29 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -883,9 +883,9 @@ EXPORT_SYMBOL_GPL(kvm_enable_efer_bits); * Returns 0 on success, non-0 otherwise. * Assumes vcpu_load() was already called. */ -int kvm_set_msr(struct kvm_vcpu *vcpu, u32 msr_index, u64 data) +int kvm_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr) { - return kvm_x86_ops->set_msr(vcpu, msr_index, data); + return kvm_x86_ops->set_msr(vcpu, msr); } /* @@ -893,7 +893,11 @@ int kvm_set_msr(struct kvm_vcpu *vcpu, u32 msr_index, u64 data) */ static int do_set_msr(struct kvm_vcpu *vcpu, unsigned index, u64 *data) { - return kvm_set_msr(vcpu, index, *data); + struct msr_data msr; + msr.data = *data; + msr.index = index; + msr.host_initiated = true; + return kvm_set_msr(vcpu, &msr); } static void kvm_write_wall_clock(struct kvm *kvm, gpa_t wall_clock) @@ -1043,12 +1047,13 @@ static u64 compute_guest_tsc(struct kvm_vcpu *vcpu, s64 kernel_ns) return tsc; } -void kvm_write_tsc(struct kvm_vcpu *vcpu, u64 data) +void kvm_write_tsc(struct kvm_vcpu *vcpu, struct msr_data *msr) { struct kvm *kvm = vcpu->kvm; u64 offset, ns, elapsed; unsigned long flags; s64 usdiff; + u64 data = msr->data; raw_spin_lock_irqsave(&kvm->arch.tsc_write_lock, flags); offset = kvm_x86_ops->compute_tsc_offset(vcpu, data); @@ -1561,9 +1566,11 @@ static void record_steal_time(struct kvm_vcpu *vcpu) &vcpu->arch.st.steal, sizeof(struct kvm_steal_time)); } -int kvm_set_msr_common(struct kvm_vcpu *vcpu, u32 msr, u64 data) +int kvm_set_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info) { bool pr = false; + u32 msr = msr_info->index; + u64 data = msr_info->data; switch (msr) { case MSR_EFER: @@ -4283,10 +4290,10 @@ static int emulator_get_msr(struct x86_emulate_ctxt *ctxt, return kvm_get_msr(emul_to_vcpu(ctxt), msr_index, pdata); } -static int emulator_set_msr(struct x86_emulate_ctxt *ctxt, - u32 msr_index, u64 data) +static int emulator_set_msr(struct x86_emulate_ctxt *ctxt, + struct msr_data * msr) { - return kvm_set_msr(emul_to_vcpu(ctxt), msr_index, data); + return kvm_set_msr(emul_to_vcpu(ctxt), msr); } static int emulator_read_pmc(struct x86_emulate_ctxt *ctxt, diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h index 3d1134d..1dcb8e3 100644 --- a/arch/x86/kvm/x86.h +++ b/arch/x86/kvm/x86.h @@ -112,7 +112,7 @@ void kvm_before_handle_nmi(struct kvm_vcpu *vcpu); void kvm_after_handle_nmi(struct kvm_vcpu *vcpu); int kvm_inject_realmode_interrupt(struct kvm_vcpu *vcpu, int irq, int inc_eip); -void kvm_write_tsc(struct kvm_vcpu *vcpu, u64 data); +void kvm_write_tsc(struct kvm_vcpu *vcpu, struct msr_data *msr); int kvm_read_guest_virt(struct x86_emulate_ctxt *ctxt, gva_t addr, void *val, unsigned int bytes,
In order to track who initiated the call (host or guest) to modify an msr value I have changed function call parameters along the call path. The specific change is to add a struct pointer parameter that points to (index, data, caller) information rather than having this information passed as individual parameters. The initial use for this capability is for updating the IA32_TSC_ADJUST msr while setting the tsc value. It is anticipated that this capability is useful other tasks. Signed-off-by: Will Auld <will.auld@intel.com> --- arch/x86/include/asm/kvm_host.h | 12 +++++++++--- arch/x86/kvm/svm.c | 21 +++++++++++++++------ arch/x86/kvm/vmx.c | 24 +++++++++++++++++------- arch/x86/kvm/x86.c | 23 +++++++++++++++-------- arch/x86/kvm/x86.h | 2 +- 5 files changed, 57 insertions(+), 25 deletions(-)