On Wed, Nov 18, 2020 at 09:58:37AM +0100, Paolo Bonzini wrote:
> On 16/11/20 21:04, Jessica Clarke wrote:
> > The Requested Privilege Level field is 2 bits, the Table Indicator field
> > is 1 bit and the Index field is the remaining 15 bits, with TI=0 meaning
> > GDT and TI=1 meaning LDT.
> >
> > Signed-off-by: Jessica Clarke <jrt...@jrtc27.com>
> > ---
> > target/i386/hvf/x86.h | 8 ++++----
> > 1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/target/i386/hvf/x86.h b/target/i386/hvf/x86.h
> > index bacade7b65..ea3e1b86b3 100644
> > --- a/target/i386/hvf/x86.h
> > +++ b/target/i386/hvf/x86.h
> > @@ -214,16 +214,16 @@ static inline uint32_t
> > x86_call_gate_offset(x86_call_gate *gate)
> > return (uint32_t)((gate->offset1 << 16) | gate->offset0);
> > }
> > -#define LDT_SEL 0
> > -#define GDT_SEL 1
> > +#define GDT_SEL 0
> > +#define LDT_SEL 1
> > typedef struct x68_segment_selector {
> > union {
> > uint16_t sel;
> > struct {
> > - uint16_t rpl:3;
> > + uint16_t rpl:2;
> > uint16_t ti:1;
> > - uint16_t index:12;
> > + uint16_t index:13;
> > };
> > };
> > } __attribute__ ((__packed__)) x68_segment_selector;
> >
>
> I queued the patch, thanks.
>
> On further look, though, the bitfield part of the struct is almost never
> used, and therefore most uses of the struct itself are more or less
> superfluous (apart from some typechecking). In particular,
> vmx_read_segment_selector and vmx_write_segment_selector only use the 16-bit
> .self field, and the code would be simpler if it was changed to just use a
> uint16_t.
>
IIRC, that's because vmx_handle_task_switch is incomplete and needs
improvement. Certain task switches aren't implemented.
> The only place that "needs" the struct is in vmx_handle_task_switch's calls
> to x86_read_segment_descriptor and x86_write_segment_descriptor. Those are
> also the places that benefit from this patch. But even then, for the sake
> of consistency it would make sense for x86_segment_selector to be used only
> inside those two functions; the arguments could be just an uint16_t.
>
Reusing some bits of TCG for task switching would be the most helpful
from functional perspective and to avoid code duplication.
Thanks,
Roman
