In this patch, we make pv_cpuid() and emulate_forced_invalid_op()
public to be used by PVH. Also put vmx functions like vmr(),
get_instruction_length(), inlined in header file to be used by PVH. No
real code change.
Signed-off-by: Mukesh Rathor <mukesh.rathor@oracle.com>
diff -r 0339f85f6068 -r fadb3f2f5d97 xen/arch/x86/hvm/vmx/vmcs.c
--- a/xen/arch/x86/hvm/vmx/vmcs.c Fri Jan 11 16:24:00 2013 -0800
+++ b/xen/arch/x86/hvm/vmx/vmcs.c Fri Jan 11 16:25:27 2013 -0800
@@ -1170,14 +1170,6 @@ void vmx_do_resume(struct vcpu *v)
reset_stack_and_jump(vmx_asm_do_vmentry);
}
-static unsigned long vmr(unsigned long field)
-{
- int rc;
- unsigned long val;
- val = __vmread_safe(field, &rc);
- return rc ? 0 : val;
-}
-
static void vmx_dump_sel(char *name, uint32_t selector)
{
uint32_t sel, attr, limit;
diff -r 0339f85f6068 -r fadb3f2f5d97 xen/arch/x86/hvm/vmx/vmx.c
--- a/xen/arch/x86/hvm/vmx/vmx.c Fri Jan 11 16:24:00 2013 -0800
+++ b/xen/arch/x86/hvm/vmx/vmx.c Fri Jan 11 16:25:27 2013 -0800
@@ -593,7 +593,7 @@ static int vmx_load_vmcs_ctxt(struct vcp
return 0;
}
-static void vmx_fpu_enter(struct vcpu *v)
+void vmx_fpu_enter(struct vcpu *v)
{
vcpu_restore_fpu_lazy(v);
v->arch.hvm_vmx.exception_bitmap &= ~(1u << TRAP_no_device);
@@ -1543,19 +1543,7 @@ struct hvm_function_table * __init start
return &vmx_function_table;
}
-/*
- * Not all cases receive valid value in the VM-exit instruction length field.
- * Callers must know what they''re doing!
- */
-static int get_instruction_length(void)
-{
- int len;
- len = __vmread(VM_EXIT_INSTRUCTION_LEN); /* Safe: callers audited */
- BUG_ON((len < 1) || (len > 15));
- return len;
-}
-
-static void update_guest_eip(void)
+void update_guest_eip(void)
{
struct cpu_user_regs *regs = guest_cpu_user_regs();
unsigned long x;
@@ -1633,8 +1621,8 @@ static void vmx_do_cpuid(struct cpu_user
regs->edx = edx;
}
-static void vmx_dr_access(unsigned long exit_qualification,
- struct cpu_user_regs *regs)
+void vmx_dr_access(unsigned long exit_qualification,
+ struct cpu_user_regs *regs)
{
struct vcpu *v = current;
@@ -2009,7 +1997,7 @@ gp_fault:
return X86EMUL_EXCEPTION;
}
-static void vmx_do_extint(struct cpu_user_regs *regs)
+void vmx_do_extint(struct cpu_user_regs *regs)
{
unsigned int vector;
diff -r 0339f85f6068 -r fadb3f2f5d97 xen/arch/x86/traps.c
--- a/xen/arch/x86/traps.c Fri Jan 11 16:24:00 2013 -0800
+++ b/xen/arch/x86/traps.c Fri Jan 11 16:25:27 2013 -0800
@@ -723,7 +723,7 @@ int cpuid_hypervisor_leaves( uint32_t id
return 1;
}
-static void pv_cpuid(struct cpu_user_regs *regs)
+void pv_cpuid(struct cpu_user_regs *regs)
{
uint32_t a, b, c, d;
@@ -900,7 +900,7 @@ static int emulate_invalid_rdtscp(struct
return EXCRET_fault_fixed;
}
-static int emulate_forced_invalid_op(struct cpu_user_regs *regs)
+int emulate_forced_invalid_op(struct cpu_user_regs *regs)
{
char sig[5], instr[2];
unsigned long eip, rc;
diff -r 0339f85f6068 -r fadb3f2f5d97 xen/include/asm-x86/hvm/vmx/vmcs.h
--- a/xen/include/asm-x86/hvm/vmx/vmcs.h Fri Jan 11 16:24:00 2013 -0800
+++ b/xen/include/asm-x86/hvm/vmx/vmcs.h Fri Jan 11 16:25:27 2013 -0800
@@ -415,6 +415,7 @@ int vmx_add_host_load_msr(u32 msr);
void vmx_vmcs_switch(struct vmcs_struct *from, struct vmcs_struct *to);
void vmx_set_eoi_exit_bitmap(struct vcpu *v, u8 vector);
void vmx_clear_eoi_exit_bitmap(struct vcpu *v, u8 vector);
+void vmx_fpu_enter(struct vcpu *v);
#endif /* ASM_X86_HVM_VMX_VMCS_H__ */
diff -r 0339f85f6068 -r fadb3f2f5d97 xen/include/asm-x86/hvm/vmx/vmx.h
--- a/xen/include/asm-x86/hvm/vmx/vmx.h Fri Jan 11 16:24:00 2013 -0800
+++ b/xen/include/asm-x86/hvm/vmx/vmx.h Fri Jan 11 16:25:27 2013 -0800
@@ -389,6 +389,26 @@ static inline int __vmxon(u64 addr)
return rc;
}
+static inline unsigned long vmr(unsigned long field)
+{
+ int rc;
+ unsigned long val;
+ val = __vmread_safe(field, &rc);
+ return rc ? 0 : val;
+}
+
+/*
+ * Not all cases receive valid value in the VM-exit instruction length field.
+ * Callers must know what they''re doing!
+ */
+static inline int get_instruction_length(void)
+{
+ int len;
+ len = __vmread(VM_EXIT_INSTRUCTION_LEN); /* Safe: callers audited */
+ BUG_ON((len < 1) || (len > 15));
+ return len;
+}
+
void vmx_get_segment_register(struct vcpu *, enum x86_segment,
struct segment_register *);
void vmx_inject_extint(int trap);
@@ -397,6 +417,9 @@ void vmx_inject_nmi(void);
void ept_p2m_init(struct p2m_domain *p2m);
void ept_walk_table(struct domain *d, unsigned long gfn);
void setup_ept_dump(void);
+void update_guest_eip(void);
+void vmx_dr_access(unsigned long exit_qualification,struct cpu_user_regs
*regs);
+void vmx_do_extint(struct cpu_user_regs *regs);
/* EPT violation qualifications definitions */
#define _EPT_READ_VIOLATION 0
diff -r 0339f85f6068 -r fadb3f2f5d97 xen/include/asm-x86/processor.h
--- a/xen/include/asm-x86/processor.h Fri Jan 11 16:24:00 2013 -0800
+++ b/xen/include/asm-x86/processor.h Fri Jan 11 16:25:27 2013 -0800
@@ -551,6 +551,8 @@ int wrmsr_hypervisor_regs(uint32_t idx,
void microcode_set_module(unsigned int);
int microcode_update(XEN_GUEST_HANDLE_PARAM(const_void), unsigned long len);
int microcode_resume_cpu(int cpu);
+void pv_cpuid(struct cpu_user_regs *regs);
+int emulate_forced_invalid_op(struct cpu_user_regs *regs);
#endif /* !__ASSEMBLY__ */
>>> On 12.01.13 at 02:48, Mukesh Rathor <mukesh.rathor@oracle.com> wrote: > In this patch, we make pv_cpuid() and emulate_forced_invalid_op() > public to be used by PVH. Also put vmx functions like vmr(), > get_instruction_length(), inlined in header file to be used by PVH. No > real code change.As these functions are VMX-specific, I think they ought to get renamed to express that. However, the mere fact that you intend to use VMX functions outside of VMX code looks wrong. Jan> Signed-off-by: Mukesh Rathor <mukesh.rathor@oracle.com> > > > diff -r 0339f85f6068 -r fadb3f2f5d97 xen/arch/x86/hvm/vmx/vmcs.c > --- a/xen/arch/x86/hvm/vmx/vmcs.c Fri Jan 11 16:24:00 2013 -0800 > +++ b/xen/arch/x86/hvm/vmx/vmcs.c Fri Jan 11 16:25:27 2013 -0800 > @@ -1170,14 +1170,6 @@ void vmx_do_resume(struct vcpu *v) > reset_stack_and_jump(vmx_asm_do_vmentry); > } > > -static unsigned long vmr(unsigned long field) > -{ > - int rc; > - unsigned long val; > - val = __vmread_safe(field, &rc); > - return rc ? 0 : val; > -} > - > static void vmx_dump_sel(char *name, uint32_t selector) > { > uint32_t sel, attr, limit; > diff -r 0339f85f6068 -r fadb3f2f5d97 xen/arch/x86/hvm/vmx/vmx.c > --- a/xen/arch/x86/hvm/vmx/vmx.c Fri Jan 11 16:24:00 2013 -0800 > +++ b/xen/arch/x86/hvm/vmx/vmx.c Fri Jan 11 16:25:27 2013 -0800 > @@ -593,7 +593,7 @@ static int vmx_load_vmcs_ctxt(struct vcp > return 0; > } > > -static void vmx_fpu_enter(struct vcpu *v) > +void vmx_fpu_enter(struct vcpu *v) > { > vcpu_restore_fpu_lazy(v); > v->arch.hvm_vmx.exception_bitmap &= ~(1u << TRAP_no_device); > @@ -1543,19 +1543,7 @@ struct hvm_function_table * __init start > return &vmx_function_table; > } > > -/* > - * Not all cases receive valid value in the VM-exit instruction length field. > - * Callers must know what they''re doing! > - */ > -static int get_instruction_length(void) > -{ > - int len; > - len = __vmread(VM_EXIT_INSTRUCTION_LEN); /* Safe: callers audited */ > - BUG_ON((len < 1) || (len > 15)); > - return len; > -} > - > -static void update_guest_eip(void) > +void update_guest_eip(void) > { > struct cpu_user_regs *regs = guest_cpu_user_regs(); > unsigned long x; > @@ -1633,8 +1621,8 @@ static void vmx_do_cpuid(struct cpu_user > regs->edx = edx; > } > > -static void vmx_dr_access(unsigned long exit_qualification, > - struct cpu_user_regs *regs) > +void vmx_dr_access(unsigned long exit_qualification, > + struct cpu_user_regs *regs) > { > struct vcpu *v = current; > > @@ -2009,7 +1997,7 @@ gp_fault: > return X86EMUL_EXCEPTION; > } > > -static void vmx_do_extint(struct cpu_user_regs *regs) > +void vmx_do_extint(struct cpu_user_regs *regs) > { > unsigned int vector; > > diff -r 0339f85f6068 -r fadb3f2f5d97 xen/arch/x86/traps.c > --- a/xen/arch/x86/traps.c Fri Jan 11 16:24:00 2013 -0800 > +++ b/xen/arch/x86/traps.c Fri Jan 11 16:25:27 2013 -0800 > @@ -723,7 +723,7 @@ int cpuid_hypervisor_leaves( uint32_t id > return 1; > } > > -static void pv_cpuid(struct cpu_user_regs *regs) > +void pv_cpuid(struct cpu_user_regs *regs) > { > uint32_t a, b, c, d; > > @@ -900,7 +900,7 @@ static int emulate_invalid_rdtscp(struct > return EXCRET_fault_fixed; > } > > -static int emulate_forced_invalid_op(struct cpu_user_regs *regs) > +int emulate_forced_invalid_op(struct cpu_user_regs *regs) > { > char sig[5], instr[2]; > unsigned long eip, rc; > diff -r 0339f85f6068 -r fadb3f2f5d97 xen/include/asm-x86/hvm/vmx/vmcs.h > --- a/xen/include/asm-x86/hvm/vmx/vmcs.h Fri Jan 11 16:24:00 2013 -0800 > +++ b/xen/include/asm-x86/hvm/vmx/vmcs.h Fri Jan 11 16:25:27 2013 -0800 > @@ -415,6 +415,7 @@ int vmx_add_host_load_msr(u32 msr); > void vmx_vmcs_switch(struct vmcs_struct *from, struct vmcs_struct *to); > void vmx_set_eoi_exit_bitmap(struct vcpu *v, u8 vector); > void vmx_clear_eoi_exit_bitmap(struct vcpu *v, u8 vector); > +void vmx_fpu_enter(struct vcpu *v); > > #endif /* ASM_X86_HVM_VMX_VMCS_H__ */ > > diff -r 0339f85f6068 -r fadb3f2f5d97 xen/include/asm-x86/hvm/vmx/vmx.h > --- a/xen/include/asm-x86/hvm/vmx/vmx.h Fri Jan 11 16:24:00 2013 -0800 > +++ b/xen/include/asm-x86/hvm/vmx/vmx.h Fri Jan 11 16:25:27 2013 -0800 > @@ -389,6 +389,26 @@ static inline int __vmxon(u64 addr) > return rc; > } > > +static inline unsigned long vmr(unsigned long field) > +{ > + int rc; > + unsigned long val; > + val = __vmread_safe(field, &rc); > + return rc ? 0 : val; > +} > + > +/* > + * Not all cases receive valid value in the VM-exit instruction length > field. > + * Callers must know what they''re doing! > + */ > +static inline int get_instruction_length(void) > +{ > + int len; > + len = __vmread(VM_EXIT_INSTRUCTION_LEN); /* Safe: callers audited */ > + BUG_ON((len < 1) || (len > 15)); > + return len; > +} > + > void vmx_get_segment_register(struct vcpu *, enum x86_segment, > struct segment_register *); > void vmx_inject_extint(int trap); > @@ -397,6 +417,9 @@ void vmx_inject_nmi(void); > void ept_p2m_init(struct p2m_domain *p2m); > void ept_walk_table(struct domain *d, unsigned long gfn); > void setup_ept_dump(void); > +void update_guest_eip(void); > +void vmx_dr_access(unsigned long exit_qualification,struct cpu_user_regs > *regs); > +void vmx_do_extint(struct cpu_user_regs *regs); > > /* EPT violation qualifications definitions */ > #define _EPT_READ_VIOLATION 0 > diff -r 0339f85f6068 -r fadb3f2f5d97 xen/include/asm-x86/processor.h > --- a/xen/include/asm-x86/processor.h Fri Jan 11 16:24:00 2013 -0800 > +++ b/xen/include/asm-x86/processor.h Fri Jan 11 16:25:27 2013 -0800 > @@ -551,6 +551,8 @@ int wrmsr_hypervisor_regs(uint32_t idx, > void microcode_set_module(unsigned int); > int microcode_update(XEN_GUEST_HANDLE_PARAM(const_void), unsigned long > len); > int microcode_resume_cpu(int cpu); > +void pv_cpuid(struct cpu_user_regs *regs); > +int emulate_forced_invalid_op(struct cpu_user_regs *regs); > > #endif /* !__ASSEMBLY__ */ > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel
On Mon, 14 Jan 2013 11:27:15 +0000 "Jan Beulich" <JBeulich@suse.com> wrote:> >>> On 12.01.13 at 02:48, Mukesh Rathor <mukesh.rathor@oracle.com> > >>> wrote: > > In this patch, we make pv_cpuid() and emulate_forced_invalid_op() > > public to be used by PVH. Also put vmx functions like vmr(), > > get_instruction_length(), inlined in header file to be used by PVH. > > No real code change. > > As these functions are VMX-specific, I think they ought to get > renamed to express that. However, the mere fact that you > intend to use VMX functions outside of VMX code looks wrong.Not sure what you are referring to. The functions vmr, get_instruction_length(), etc.. are called from VMX code, ie, vmx_pvh.c. I''ll rename them with vmx_ prefix. The two functions, pv_cpuid and emulate_forced_invalid_op are not vmx specific, they are called from non vmx code, ie, PV also. So renaming them doesn''t make sense IMO. Recall, PVH is a PV guest. thanks, Mukesh
>>> On 16.01.13 at 00:41, Mukesh Rathor <mukesh.rathor@oracle.com> wrote: > On Mon, 14 Jan 2013 11:27:15 +0000 "Jan Beulich" <JBeulich@suse.com> wrote: >> >>> On 12.01.13 at 02:48, Mukesh Rathor <mukesh.rathor@oracle.com> wrote: >> > In this patch, we make pv_cpuid() and emulate_forced_invalid_op() >> > public to be used by PVH. Also put vmx functions like vmr(), >> > get_instruction_length(), inlined in header file to be used by PVH. >> > No real code change. >> >> As these functions are VMX-specific, I think they ought to get >> renamed to express that. However, the mere fact that you >> intend to use VMX functions outside of VMX code looks wrong. > > Not sure what you are referring to. The functions vmr, > get_instruction_length(), etc.. are called from VMX code, ie, > vmx_pvh.c. I''ll rename them with vmx_ prefix.That''s fine then (perhaps even without the rename if the declarations sit in a header _only_ included by VMX code).> The two functions, pv_cpuid and emulate_forced_invalid_op are not vmx > specific, they are called from non vmx code, ie, PV also. So renaming > them doesn''t make sense IMO. Recall, PVH is a PV guest.Sure - these ones I didn''t have in mind. Jan
At 17:48 -0800 on 11 Jan (1357926531), Mukesh Rathor wrote:> In this patch, we make pv_cpuid() and emulate_forced_invalid_op() > public to be used by PVH. Also put vmx functions like vmr(), > get_instruction_length(), inlined in header file to be used by PVH. No > real code change.Please don''t export vmr(). It silently suppresses errors, which is OK for its current use (as a helper function in the state dump) but not for actual guest state manipulations. Tim.
On Thu, 24 Jan 2013 15:22:21 +0000 Tim Deegan <tim@xen.org> wrote:> At 17:48 -0800 on 11 Jan (1357926531), Mukesh Rathor wrote: > > In this patch, we make pv_cpuid() and emulate_forced_invalid_op() > > public to be used by PVH. Also put vmx functions like vmr(), > > get_instruction_length(), inlined in header file to be used by PVH. > > No real code change. > > Please don''t export vmr(). It silently suppresses errors, which is OK > for its current use (as a helper function in the state dump) but not > for actual guest state manipulations.Ok, done. I''ll assueme it''s ok to export get_instruction_length().
>>> On 25.01.13 at 02:44, Mukesh Rathor <mukesh.rathor@oracle.com> wrote: > On Thu, 24 Jan 2013 15:22:21 +0000 > Tim Deegan <tim@xen.org> wrote: > >> At 17:48 -0800 on 11 Jan (1357926531), Mukesh Rathor wrote: >> > In this patch, we make pv_cpuid() and emulate_forced_invalid_op() >> > public to be used by PVH. Also put vmx functions like vmr(), >> > get_instruction_length(), inlined in header file to be used by PVH. >> > No real code change. >> >> Please don''t export vmr(). It silently suppresses errors, which is OK >> for its current use (as a helper function in the state dump) but not >> for actual guest state manipulations. > > Ok, done. I''ll assueme it''s ok to export get_instruction_length().With a properly qualified name that''s fine I think. Jan
At 17:44 -0800 on 24 Jan (1359049481), Mukesh Rathor wrote:> On Thu, 24 Jan 2013 15:22:21 +0000 > Tim Deegan <tim@xen.org> wrote: > > > At 17:48 -0800 on 11 Jan (1357926531), Mukesh Rathor wrote: > > > In this patch, we make pv_cpuid() and emulate_forced_invalid_op() > > > public to be used by PVH. Also put vmx functions like vmr(), > > > get_instruction_length(), inlined in header file to be used by PVH. > > > No real code change. > > > > Please don''t export vmr(). It silently suppresses errors, which is OK > > for its current use (as a helper function in the state dump) but not > > for actual guest state manipulations. > > Ok, done.Thanks.> I''ll assueme it''s ok to export get_instruction_length().Yes, that''s fine as long as the warnings about when it''s safe to use are preserved! And, as Jan suggested, it should be renamed to vmx_get_instruction_length(). Cheers, Tim.