Re: [PATCH v10 7/9] x86emul: support FNSTENV and FNSAVE

[Jan Beulich <jbeulich@xxxxxxxx>]

On 29.05.2020 16:08, Andrew Cooper wrote:
> On 25/05/2020 15:29, Jan Beulich wrote:
>> To avoid introducing another boolean into emulator state, the
>> rex_prefix field gets (ab)used to convey the real/VM86 vs protected mode
>> info (affecting structure layout, albeit not size) to x86_emul_blk().
>>
>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> 
> Acked-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>,

Thanks.

> with one suggestion.
> 
>> --- a/xen/arch/x86/x86_emulate/x86_emulate.c
>> +++ b/xen/arch/x86/x86_emulate/x86_emulate.c
>> @@ -856,6 +856,9 @@ struct x86_emulate_state {
>>      enum {
>>          blk_NONE,
>>          blk_enqcmd,
>> +#ifndef X86EMUL_NO_FPU
>> +        blk_fst, /* FNSTENV, FNSAVE */
>> +#endif
>>          blk_movdir,
>>      } blk;
>>      uint8_t modrm, modrm_mod, modrm_reg, modrm_rm;
>> @@ -897,6 +900,50 @@ struct x86_emulate_state {
>>  #define PTR_POISON NULL /* 32-bit builds are for user-space, so NULL is OK. 
>> */
>>  #endif
>>  
>> +#ifndef X86EMUL_NO_FPU
>> +struct x87_env16 {
>> +    uint16_t fcw;
>> +    uint16_t fsw;
>> +    uint16_t ftw;
>> +    union {
>> +        struct {
>> +            uint16_t fip_lo;
>> +            uint16_t fop:11, :1, fip_hi:4;
>> +            uint16_t fdp_lo;
>> +            uint16_t :12, fdp_hi:4;
>> +        } real;
>> +        struct {
>> +            uint16_t fip;
>> +            uint16_t fcs;
>> +            uint16_t fdp;
>> +            uint16_t fds;
>> +        } prot;
>> +    } mode;
>> +};
>> +
>> +struct x87_env32 {
>> +    uint32_t fcw:16, :16;
>> +    uint32_t fsw:16, :16;
>> +    uint32_t ftw:16, :16;
> 
> uint16_t fcw, :16;
> uint16_t fsw, :16;
> uint16_t ftw, :16;
> 
> ?

You had suggested this before, and I did reply that my intention
was to have x87_env16 use uint16_t throughout, and x87_env32
uint32_t respectively for all its pieces. In the end it doesn't
make a difference, and hence this cosmetic aspect is what made
me pick one of the various possible options.

Jan