| Message ID | 1482337930-16744-2-git-send-email-andrew.cooper3@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
>>> On 21.12.16 at 17:32, <andrew.cooper3@citrix.com> wrote: > vmfunc can in principle modify register state, so should operate on the shadow > register state rather than the starting state of emulation. > > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> While in principle this is fine, I'd rather see the register state constified for now, to demonstrate it is not being modified. I'll submit my two remaining follow-up patches in a minute, and we can then discuss which of the two to take. Jan
On 22/12/16 08:27, Jan Beulich wrote: >>>> On 21.12.16 at 17:32, <andrew.cooper3@citrix.com> wrote: >> vmfunc can in principle modify register state, so should operate on the shadow >> register state rather than the starting state of emulation. >> >> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> > While in principle this is fine, I'd rather see the register state > constified for now, to demonstrate it is not being modified. I'll > submit my two remaining follow-up patches in a minute, and > we can then discuss which of the two to take. The question here is how likely it is that new functionality for VMFUNC will be defined, which starts mutating the values. I am not aware of anything new, so lets go with the const version for now (as it is one fewer parameters). If this changes in the future, we can easily switch back to passing the shadow register block. ~Andrew
diff --git a/xen/arch/x86/hvm/emulate.c b/xen/arch/x86/hvm/emulate.c index aa1b716..fae666a 100644 --- a/xen/arch/x86/hvm/emulate.c +++ b/xen/arch/x86/hvm/emulate.c @@ -1646,13 +1646,14 @@ static int hvmemul_invlpg( } static int hvmemul_vmfunc( + struct cpu_user_regs *regs, struct x86_emulate_ctxt *ctxt) { int rc; if ( !hvm_funcs.altp2m_vcpu_emulate_vmfunc ) return X86EMUL_UNHANDLEABLE; - rc = hvm_funcs.altp2m_vcpu_emulate_vmfunc(ctxt->regs); + rc = hvm_funcs.altp2m_vcpu_emulate_vmfunc(regs); if ( rc == X86EMUL_EXCEPTION ) x86_emul_hw_exception(TRAP_invalid_op, X86_EVENT_NO_EC, ctxt); diff --git a/xen/arch/x86/x86_emulate/x86_emulate.c b/xen/arch/x86/x86_emulate/x86_emulate.c index 3076c0c..c9ffc56 100644 --- a/xen/arch/x86/x86_emulate/x86_emulate.c +++ b/xen/arch/x86/x86_emulate/x86_emulate.c @@ -4463,7 +4463,7 @@ x86_emulate( generate_exception_if(lock_prefix | rep_prefix() | (vex.pfx == vex_66), EXC_UD); fail_if(!ops->vmfunc); - if ( (rc = ops->vmfunc(ctxt)) != X86EMUL_OKAY ) + if ( (rc = ops->vmfunc(&_regs, ctxt)) != X86EMUL_OKAY ) goto done; goto no_writeback; diff --git a/xen/arch/x86/x86_emulate/x86_emulate.h b/xen/arch/x86/x86_emulate/x86_emulate.h index 75f57ba..d70b534 100644 --- a/xen/arch/x86/x86_emulate/x86_emulate.h +++ b/xen/arch/x86/x86_emulate/x86_emulate.h @@ -448,6 +448,7 @@ struct x86_emulate_ops /* vmfunc: Emulate VMFUNC via given set of EAX ECX inputs */ int (*vmfunc)( + struct cpu_user_regs *regs, struct x86_emulate_ctxt *ctxt); };
vmfunc can in principle modify register state, so should operate on the shadow register state rather than the starting state of emulation. Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> --- CC: Jan Beulich <JBeulich@suse.com> CC: Paul Durrant <paul.durrant@citrix.com> --- xen/arch/x86/hvm/emulate.c | 3 ++- xen/arch/x86/x86_emulate/x86_emulate.c | 2 +- xen/arch/x86/x86_emulate/x86_emulate.h | 1 + 3 files changed, 4 insertions(+), 2 deletions(-)