| Message ID | 58345C9E0200007800120CD9@prv-mh.provo.novell.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 22/11/16 13:56, Jan Beulich wrote: > Whether to write 32 or just 16 bits depends on the D bit of the target > CS. The width of the stack pointer to use depends on the B bit of the > target SS. > > Also avoid using the no-fault copying routine. > > Finally avoid using yet another struct segment_register variable here. > > Signed-off-by: Jan Beulich <jbeulich@suse.com> > > --- a/xen/arch/x86/hvm/hvm.c > +++ b/xen/arch/x86/hvm/hvm.c > @@ -3033,9 +3033,6 @@ void hvm_task_switch( > goto out; > } > > - if ( (tss.trace & 1) && !exn_raised ) > - hvm_inject_hw_exception(TRAP_debug, HVM_DELIVER_NO_ERROR_CODE); > - > tr.attr.fields.type = 0xb; /* busy 32-bit tss */ > hvm_set_segment_register(v, x86_seg_tr, &tr); > > @@ -3051,17 +3048,32 @@ void hvm_task_switch( > > if ( errcode >= 0 ) > { > - struct segment_register reg; > unsigned long linear_addr; > - regs->esp -= 4; > - hvm_get_segment_register(current, x86_seg_ss, ®); > - /* Todo: do not ignore access faults here. */ > - if ( hvm_virtual_to_linear_addr(x86_seg_ss, ®, regs->esp, > - 4, hvm_access_write, 32, > + unsigned int opsz, sp; > + > + hvm_get_segment_register(current, x86_seg_cs, &segr); You already have current latched in v at this point. Otherwise, Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> > + opsz = segr.attr.fields.db ? 4 : 2; > + hvm_get_segment_register(current, x86_seg_ss, &segr); > + if ( segr.attr.fields.db ) > + sp = regs->_esp -= opsz; > + else > + sp = *(uint16_t *)®s->esp -= opsz; > + if ( hvm_virtual_to_linear_addr(x86_seg_ss, &segr, sp, opsz, > + hvm_access_write, > + 16 << segr.attr.fields.db, > &linear_addr) ) > - hvm_copy_to_guest_virt_nofault(linear_addr, &errcode, 4, 0); > + { > + rc = hvm_copy_to_guest_virt(linear_addr, &errcode, opsz, 0); > + if ( rc == HVMCOPY_bad_gva_to_gfn ) > + exn_raised = 1; > + else if ( rc != HVMCOPY_okay ) > + goto out; > + } > } > > + if ( (tss.trace & 1) && !exn_raised ) > + hvm_inject_hw_exception(TRAP_debug, HVM_DELIVER_NO_ERROR_CODE); > + > out: > hvm_unmap_entry(optss_desc); > hvm_unmap_entry(nptss_desc); > > >
>>> On 22.11.16 at 17:58, <andrew.cooper3@citrix.com> wrote: > On 22/11/16 13:56, Jan Beulich wrote: >> Whether to write 32 or just 16 bits depends on the D bit of the target >> CS. The width of the stack pointer to use depends on the B bit of the >> target SS. >> >> Also avoid using the no-fault copying routine. >> >> Finally avoid using yet another struct segment_register variable here. >> >> Signed-off-by: Jan Beulich <jbeulich@suse.com> >> >> --- a/xen/arch/x86/hvm/hvm.c >> +++ b/xen/arch/x86/hvm/hvm.c >> @@ -3033,9 +3033,6 @@ void hvm_task_switch( >> goto out; >> } >> >> - if ( (tss.trace & 1) && !exn_raised ) >> - hvm_inject_hw_exception(TRAP_debug, HVM_DELIVER_NO_ERROR_CODE); >> - >> tr.attr.fields.type = 0xb; /* busy 32-bit tss */ >> hvm_set_segment_register(v, x86_seg_tr, &tr); >> >> @@ -3051,17 +3048,32 @@ void hvm_task_switch( >> >> if ( errcode >= 0 ) >> { >> - struct segment_register reg; >> unsigned long linear_addr; >> - regs->esp -= 4; >> - hvm_get_segment_register(current, x86_seg_ss, ®); >> - /* Todo: do not ignore access faults here. */ >> - if ( hvm_virtual_to_linear_addr(x86_seg_ss, ®, regs->esp, >> - 4, hvm_access_write, 32, >> + unsigned int opsz, sp; >> + >> + hvm_get_segment_register(current, x86_seg_cs, &segr); > > You already have current latched in v at this point. Oh, right - I've blindly modified the old function invocation. > Otherwise, Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> Thanks, Jan
--- a/xen/arch/x86/hvm/hvm.c +++ b/xen/arch/x86/hvm/hvm.c @@ -3033,9 +3033,6 @@ void hvm_task_switch( goto out; } - if ( (tss.trace & 1) && !exn_raised ) - hvm_inject_hw_exception(TRAP_debug, HVM_DELIVER_NO_ERROR_CODE); - tr.attr.fields.type = 0xb; /* busy 32-bit tss */ hvm_set_segment_register(v, x86_seg_tr, &tr); @@ -3051,17 +3048,32 @@ void hvm_task_switch( if ( errcode >= 0 ) { - struct segment_register reg; unsigned long linear_addr; - regs->esp -= 4; - hvm_get_segment_register(current, x86_seg_ss, ®); - /* Todo: do not ignore access faults here. */ - if ( hvm_virtual_to_linear_addr(x86_seg_ss, ®, regs->esp, - 4, hvm_access_write, 32, + unsigned int opsz, sp; + + hvm_get_segment_register(current, x86_seg_cs, &segr); + opsz = segr.attr.fields.db ? 4 : 2; + hvm_get_segment_register(current, x86_seg_ss, &segr); + if ( segr.attr.fields.db ) + sp = regs->_esp -= opsz; + else + sp = *(uint16_t *)®s->esp -= opsz; + if ( hvm_virtual_to_linear_addr(x86_seg_ss, &segr, sp, opsz, + hvm_access_write, + 16 << segr.attr.fields.db, &linear_addr) ) - hvm_copy_to_guest_virt_nofault(linear_addr, &errcode, 4, 0); + { + rc = hvm_copy_to_guest_virt(linear_addr, &errcode, opsz, 0); + if ( rc == HVMCOPY_bad_gva_to_gfn ) + exn_raised = 1; + else if ( rc != HVMCOPY_okay ) + goto out; + } } + if ( (tss.trace & 1) && !exn_raised ) + hvm_inject_hw_exception(TRAP_debug, HVM_DELIVER_NO_ERROR_CODE); + out: hvm_unmap_entry(optss_desc); hvm_unmap_entry(nptss_desc);