[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH V2 1/3] xen/vm_access: Support for memory-content hiding
>>> On 15.06.15 at 11:03, <rcojocaru@xxxxxxxxxxxxxxx> wrote: > --- a/xen/arch/x86/hvm/emulate.c > +++ b/xen/arch/x86/hvm/emulate.c > @@ -578,6 +578,28 @@ static int hvmemul_read( > container_of(ctxt, struct hvm_emulate_ctxt, ctxt)); > } > > +static int hvmemul_read_set_context( > + enum x86_segment seg, > + unsigned long offset, > + void *p_data, > + unsigned int bytes, > + struct x86_emulate_ctxt *ctxt) > +{ > + struct vcpu *curr = current; > + unsigned int len; > + > + if ( !curr->arch.vm_event.emul_read_data ) > + return X86EMUL_UNHANDLEABLE; > + > + len = min_t(unsigned int, > + bytes, curr->arch.vm_event.emul_read_data->size); > + > + if ( len ) > + memcpy(p_data, curr->arch.vm_event.emul_read_data->data, len); And the rest of the destination simply remains unmodified / uninitialized? > @@ -891,14 +934,37 @@ static int hvmemul_rep_outs( > !!(ctxt->regs->eflags & X86_EFLAGS_DF), NULL); > } > > -static int hvmemul_rep_movs( > +static int hvmemul_rep_outs_set_context( > + enum x86_segment src_seg, > + unsigned long src_offset, > + uint16_t dst_port, > + unsigned int bytes_per_rep, > + unsigned long *reps, > + struct x86_emulate_ctxt *ctxt) > +{ > + struct vcpu *curr = current; > + unsigned int safe_bytes; > + > + if ( !curr->arch.vm_event.emul_read_data ) > + return X86EMUL_UNHANDLEABLE; > + > + safe_bytes = min_t(unsigned int, bytes_per_rep, > + curr->arch.vm_event.emul_read_data->size); > + > + return hvmemul_do_pio(dst_port, reps, safe_bytes, 0, IOREQ_WRITE, > + !!(ctxt->regs->eflags & X86_EFLAGS_DF), > + curr->arch.vm_event.emul_read_data->data); This isn't consistent with e.g. rep_movs below - you shouldn't reduce the width of the operation. Also - did I overlook where *reps gets forced to 1? If it's being done elsewhere, perhaps worth an ASSERT()? > @@ -981,7 +1047,19 @@ static int hvmemul_rep_movs( > */ > rc = hvm_copy_from_guest_phys(buf, sgpa, bytes); > if ( rc == HVMCOPY_okay ) > + { > + struct vcpu *curr = current; > + > + if ( unlikely(set_context) && curr->arch.vm_event.emul_read_data ) > + { > + unsigned long safe_bytes = min_t(unsigned long, bytes, > + curr->arch.vm_event.emul_read_data->size); The variable doesn't need to be unsigned long. > @@ -1000,13 +1078,40 @@ static int hvmemul_rep_movs( > return X86EMUL_OKAY; > } > > -static int hvmemul_rep_stos( > +static int hvmemul_rep_movs( > + enum x86_segment src_seg, > + unsigned long src_offset, > + enum x86_segment dst_seg, > + unsigned long dst_offset, > + unsigned int bytes_per_rep, > + unsigned long *reps, > + struct x86_emulate_ctxt *ctxt) > +{ > + return _hvmemul_rep_movs(src_seg, src_offset, dst_seg, dst_offset, > + bytes_per_rep, reps, ctxt, 0); > +} > + > +static int hvmemul_rep_movs_set_context( > + enum x86_segment src_seg, > + unsigned long src_offset, > + enum x86_segment dst_seg, > + unsigned long dst_offset, > + unsigned int bytes_per_rep, > + unsigned long *reps, > + struct x86_emulate_ctxt *ctxt) > +{ > + return _hvmemul_rep_movs(src_seg, src_offset, dst_seg, dst_offset, > + bytes_per_rep, reps, ctxt, 1); > +} Perhaps putting a flag hvmemul_ctxt would be better? > @@ -1408,6 +1569,32 @@ static const struct x86_emulate_ops > hvm_emulate_ops_no_write = { > .invlpg = hvmemul_invlpg > }; > > +static const struct x86_emulate_ops hvm_emulate_ops_set_context = { > + .read = hvmemul_read_set_context, > + .insn_fetch = hvmemul_insn_fetch, > + .write = hvmemul_write, > + .cmpxchg = hvmemul_cmpxchg_set_context, > + .rep_ins = hvmemul_rep_ins, > + .rep_outs = hvmemul_rep_outs_set_context, > + .rep_movs = hvmemul_rep_movs_set_context, > + .rep_stos = hvmemul_rep_stos_set_context, If you don't override .write, why would you override .rep_stos? > @@ -1528,18 +1715,31 @@ int hvm_emulate_one_no_write( > return _hvm_emulate_one(hvmemul_ctxt, &hvm_emulate_ops_no_write); > } > > -void hvm_mem_access_emulate_one(bool_t nowrite, unsigned int trapnr, > +int hvm_emulate_one_set_context( static? > +void hvm_mem_access_emulate_one(enum emul_kind kind, unsigned int trapnr, > unsigned int errcode) > { > struct hvm_emulate_ctxt ctx = {{ 0 }}; > - int rc; > + int rc = X86EMUL_UNHANDLEABLE; > > hvm_emulate_prepare(&ctx, guest_cpu_user_regs()); > > - if ( nowrite ) > - rc = hvm_emulate_one_no_write(&ctx); > - else > + switch ( kind ) { > + case EMUL_KIND_NORMAL: > rc = hvm_emulate_one(&ctx); Perhaps this should be the default case? If not, the initialization of rc would better be put in the default case instead of at the top of the function. > @@ -63,6 +64,21 @@ static int vm_event_enable( > vm_event_ring_lock_init(ved); > vm_event_ring_lock(ved); > > + for_each_vcpu( d, v ) > + { > + if ( v->arch.vm_event.emul_read_data ) > + break; continue? Jan _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |