| Message ID | 581A1E7E020000780011BB7B@prv-mh.provo.novell.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 02/11/16 16:12, Jan Beulich wrote: > This affects not only the layout of the data (always 2+8 bytes), but > also the contents (no truncation to 24 bits occurs). > > Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>, although wouldn't it be cleaner to set op_bytes = def_op_bytes, than to keep referring back to mode_64()? ~Andrew > --- > This will only apply cleanly on top of > https://lists.xenproject.org/archives/html/xen-devel/2016-11/msg00170.html. > > --- a/xen/arch/x86/x86_emulate/x86_emulate.c > +++ b/xen/arch/x86/x86_emulate/x86_emulate.c > @@ -4424,7 +4424,7 @@ x86_emulate( > fail_if(ops->read_segment == NULL); > if ( (rc = ops->read_segment(seg, &sreg, ctxt)) ) > goto done; > - if ( op_bytes == 2 ) > + if ( !mode_64bit() && op_bytes == 2 ) > sreg.base &= 0xffffff; > if ( (rc = ops->write(ea.mem.seg, ea.mem.off+0, > &sreg.limit, 2, ctxt)) || > @@ -4447,7 +4447,7 @@ x86_emulate( > !is_canonical_address(base), EXC_GP, 0); > sreg.base = base; > sreg.limit = limit; > - if ( op_bytes == 2 ) > + if ( !mode_64bit() && op_bytes == 2 ) > sreg.base &= 0xffffff; > if ( (rc = ops->write_segment(seg, &sreg, ctxt)) ) > goto done; > > >
>>> On 02.11.16 at 18:42, <andrew.cooper3@citrix.com> wrote: > On 02/11/16 16:12, Jan Beulich wrote: >> This affects not only the layout of the data (always 2+8 bytes), but >> also the contents (no truncation to 24 bits occurs). >> >> Signed-off-by: Jan Beulich <jbeulich@suse.com> > > Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>, although > wouldn't it be cleaner to set op_bytes = def_op_bytes, than to keep > referring back to mode_64()? That would still mean if ( mode_64bit() ) op_bytes = def_op_bytes; and wouldn't eliminate the uses in the second ops->write() / read_ulong() either. What we could do in the S{G,I}DT case is if ( mode_64bit() ) op_bytes = 8; else if ( op_bytes == 2 ) { sreg.base &= 0xffffff; op_bytes = 4; } if ( (rc = ops->write(ea.mem.seg, ea.mem.off+0, &sreg.limit, 2, ctxt)) || (rc = ops->write(ea.mem.seg, ea.mem.off+2, &sreg.base, op_bytes, ctxt)) ) But the same can't be done in the L{G,I}DT case. Let me know. Jan
On Wed, Nov 02, 2016 at 05:42:53PM +0000, Andrew Cooper wrote: > On 02/11/16 16:12, Jan Beulich wrote: > > This affects not only the layout of the data (always 2+8 bytes), but > > also the contents (no truncation to 24 bits occurs). > > > > Signed-off-by: Jan Beulich <jbeulich@suse.com> > > Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>, although > wouldn't it be cleaner to set op_bytes = def_op_bytes, than to keep > referring back to mode_64()? > Release-acked-by: Wei Liu <wei.liu2@citrix.com>
On 03/11/16 08:06, Jan Beulich wrote: >>>> On 02.11.16 at 18:42, <andrew.cooper3@citrix.com> wrote: >> On 02/11/16 16:12, Jan Beulich wrote: >>> This affects not only the layout of the data (always 2+8 bytes), but >>> also the contents (no truncation to 24 bits occurs). >>> >>> Signed-off-by: Jan Beulich <jbeulich@suse.com> >> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>, although >> wouldn't it be cleaner to set op_bytes = def_op_bytes, than to keep >> referring back to mode_64()? > That would still mean > > if ( mode_64bit() ) > op_bytes = def_op_bytes; Oh true. > > and wouldn't eliminate the uses in the second ops->write() / > read_ulong() either. > > What we could do in the S{G,I}DT case is > > if ( mode_64bit() ) > op_bytes = 8; > else if ( op_bytes == 2 ) > { > sreg.base &= 0xffffff; > op_bytes = 4; > } > if ( (rc = ops->write(ea.mem.seg, ea.mem.off+0, > &sreg.limit, 2, ctxt)) || > (rc = ops->write(ea.mem.seg, ea.mem.off+2, > &sreg.base, op_bytes, ctxt)) ) > > But the same can't be done in the L{G,I}DT case. Let me know. This does look a little cleaner, even if it is only for the sgdt/sidt case. ~Andrew
--- a/xen/arch/x86/x86_emulate/x86_emulate.c +++ b/xen/arch/x86/x86_emulate/x86_emulate.c @@ -4424,7 +4424,7 @@ x86_emulate( fail_if(ops->read_segment == NULL); if ( (rc = ops->read_segment(seg, &sreg, ctxt)) ) goto done; - if ( op_bytes == 2 ) + if ( !mode_64bit() && op_bytes == 2 ) sreg.base &= 0xffffff; if ( (rc = ops->write(ea.mem.seg, ea.mem.off+0, &sreg.limit, 2, ctxt)) || @@ -4447,7 +4447,7 @@ x86_emulate( !is_canonical_address(base), EXC_GP, 0); sreg.base = base; sreg.limit = limit; - if ( op_bytes == 2 ) + if ( !mode_64bit() && op_bytes == 2 ) sreg.base &= 0xffffff; if ( (rc = ops->write_segment(seg, &sreg, ctxt)) ) goto done;