Re: [PATCH v2] target/i386: seg_helper: Correct segement selector nullification in the RET/IRET helper

[Paolo Bonzini <pbonzini@redhat.com>]

On 13/11/20 10:56, Bin Meng wrote:
> From: Bin Meng <bin.meng@windriver.com>
> 
> Per the SDM, when returning to outer privilege level, for segment
> registers (ES, FS, GS, and DS) if the check fails, the segment
> selector becomes null, but QEMU clears the base/limit/flags as well
> as nullifying the segment selector, which should be a spec violation.
> 
> Real hardware seems to be compliant with the spec, at least on one
> Coffee Lake board I tested.
> 
> Signed-off-by: Bin Meng <bin.meng@windriver.com>
> 
> ---
> 
> Changes in v2:
> - clearing the DESC_P bit in the segment descriptor
> 
>   target/i386/seg_helper.c | 5 ++++-
>   1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/target/i386/seg_helper.c b/target/i386/seg_helper.c
> index be88938..d539573 100644
> --- a/target/i386/seg_helper.c
> +++ b/target/i386/seg_helper.c
> @@ -2108,7 +2108,10 @@ static inline void validate_seg(CPUX86State *env, int seg_reg, int cpl)
>       if (!(e2 & DESC_CS_MASK) || !(e2 & DESC_C_MASK)) {
>           /* data or non conforming code segment */
>           if (dpl < cpl) {
> -            cpu_x86_load_seg_cache(env, seg_reg, 0, 0, 0, 0);
> +            cpu_x86_load_seg_cache(env, seg_reg, 0,
> +                                   env->segs[seg_reg].base,
> +                                   env->segs[seg_reg].limit,
> +                                   env->segs[seg_reg].flags & ~DESC_P_MASK);
>           }
>       }
>   }
> 

Queued, thanks.  It would be nicer if the commit message explained how 
the guest can notice the difference.

Paolo