Re: [PATCH v8 08/12] x86emul: support FLDENV and FRSTOR

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Mon, May 11, 2020 at 09:29:27AM +0200, Jan Beulich wrote:
> On 08.05.2020 18:21, Roger Pau Monné wrote:
> > On Fri, May 08, 2020 at 05:04:02PM +0200, Jan Beulich wrote:
> >> On 08.05.2020 15:37, Roger Pau Monné wrote:
> >>> On Tue, May 05, 2020 at 10:16:20AM +0200, Jan Beulich wrote:
> >>>> --- a/tools/tests/x86_emulator/test_x86_emulator.c
> >>>> +++ b/tools/tests/x86_emulator/test_x86_emulator.c
> >>>> @@ -11648,6 +11651,89 @@ int x86_emul_blk(
> >>>>  
> >>>>  #ifndef X86EMUL_NO_FPU
> >>>>  
> >>>> +    case blk_fld:
> >>>> +        ASSERT(!data);
> >>>> +
> >>>> +        /* state->rex_prefix carries CR0.PE && !EFLAGS.VM setting */
> >>>> +        switch ( bytes )
> >>>> +        {
> >>>> +        case sizeof(fpstate.env):
> >>>> +        case sizeof(fpstate):
> >>>> +            memcpy(&fpstate.env, ptr, sizeof(fpstate.env));
> >>>> +            if ( !state->rex_prefix )
> >>>> +            {
> >>>> +                unsigned int fip = fpstate.env.mode.real.fip_lo +
> >>>> +                                   (fpstate.env.mode.real.fip_hi << 16);
> >>>> +                unsigned int fdp = fpstate.env.mode.real.fdp_lo +
> >>>> +                                   (fpstate.env.mode.real.fdp_hi << 16);
> >>>> +                unsigned int fop = fpstate.env.mode.real.fop;
> >>>> +
> >>>> +                fpstate.env.mode.prot.fip = fip & 0xf;
> >>>> +                fpstate.env.mode.prot.fcs = fip >> 4;
> >>>> +                fpstate.env.mode.prot.fop = fop;
> >>>> +                fpstate.env.mode.prot.fdp = fdp & 0xf;
> >>>> +                fpstate.env.mode.prot.fds = fdp >> 4;
> >>>
> >>> I've found the layouts in the SDM vol. 1, but I haven't been able to
> >>> found the translation mechanism from real to protected. Could you
> >>> maybe add a reference here?
> >>
> >> A reference to some piece of documentation? I don't think this
> >> is spelled out anywhere. It's also only one of various possible
> >> ways of doing the translation, but among them the most flexible
> >> one for possible consumers of the data (because of using the
> >> smallest possible offsets into the segments).
> > 
> > Having this written down as a comment would help, but maybe that's
> > just because I'm not familiar at all with all this stuff.
> > 
> > Again, likely a very stupid question, but I would expect:
> > 
> > fpstate.env.mode.prot.fip = fip;
> > 
> > Without the mask.
> 
> How that? A linear address has many ways of decomposing into a
> real/vm86 mode ssss:oooo pair, but what you suggest is not one
> of them. The other extreme to the one chosen would be
> 
>                 fpstate.env.mode.prot.fip = fip & 0xffff;
>                 fpstate.env.mode.prot.fcs = (fip >> 4) & 0xf000;
> 
> Except that when doing it this way, even the full insn (or for
> fcs:fdp the full operand) may not be accessible through the
> resulting ssss, due to segment wraparound.

Thanks for the explanation. I see it's better to split the offset into
the lower 4 bytes only in order to prevent overflow.

Roger.