[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v3 4/5] x86emul: make write and cmpxchg hooks optional
On 06/12/16 11:15, Jan Beulich wrote: > While the read and fetch hooks are basically unavoidable, write and > cmpxchg aren't really needed by that many insns. > > Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx> As a corollary, please can we gain ASSERT(ops->read && ops->fetch && ops->cpuid) at the start of x86_emulate/decode to make it rather more obvious that these are required. This bit me while developing the AFL harness. > > --- a/xen/arch/x86/x86_emulate/x86_emulate.c > +++ b/xen/arch/x86/x86_emulate/x86_emulate.c > @@ -1492,6 +1492,8 @@ protmode_load_seg( > { > uint32_t new_desc_b = desc.b | a_flag; > > + if ( !ops->cmpxchg ) > + return X86EMUL_UNHANDLEABLE; Any reason this isn't a fail_if() ? > switch ( (rc = ops->cmpxchg(sel_seg, (sel & 0xfff8) + 4, &desc.b, > &new_desc_b, sizeof(desc.b), ctxt)) ) > { > @@ -2624,13 +2626,18 @@ x86_emulate( > } > else if ( !(d & Mov) ) /* optimisation - avoid slow emulated read */ > { > + fail_if(lock_prefix ? !ops->cmpxchg : !ops->write); > if ( (rc = read_ulong(dst.mem.seg, dst.mem.off, > &dst.val, dst.bytes, ctxt, ops)) ) > goto done; > dst.orig_val = dst.val; > } > - else /* Lock prefix is allowed only on RMW instructions. */ > + else > + { > + /* Lock prefix is allowed only on RMW instructions. */ > generate_exception_if(lock_prefix, EXC_UD); > + fail_if(!ops->write); I am not sure that these two new fail_if()'s are sensibly placed here, remote from the use of the hooks they are protecting against. > + } > break; > } > > @@ -3334,6 +3343,7 @@ x86_emulate( > uint8_t depth = imm2 & 31; > int i; > > + fail_if(!ops->write); This would be slighly better moved down by 3 lines to be adjacent to the first ->write call. > dst.type = OP_REG; > dst.bytes = (mode_64bit() && (op_bytes == 4)) ? 8 : op_bytes; > dst.reg = (unsigned long *)&_regs.ebp; > @@ -4707,6 +4724,8 @@ x86_emulate( > if ( !(b & 1) ) > rc = ops->read(ea.mem.seg, ea.mem.off+0, mmvalp, > ea.bytes, ctxt); > + else > + fail_if(!ops->write); Again, this wants moving closer to the ->write call. I don't think we need to worry about partially-emulated instructions which fail due to a lack of write. Anything we get wrong like that will be obvious. ~Andrew _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx https://lists.xen.org/xen-devel
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |