Re: [PATCH v22 14/17] i386: split seg_helper into user-only and sysemu parts

[Richard Henderson]

On 2/24/21 5:34 AM, Claudio Fontana wrote:
> Signed-off-by: Claudio Fontana <cfontana@suse.de>
> ---
>  target/i386/tcg/helper-tcg.h        |   5 +
>  target/i386/tcg/seg_helper.h        |  66 ++++++++
>  target/i386/tcg/seg_helper.c        | 233 +---------------------------
>  target/i386/tcg/sysemu/seg_helper.c | 125 +++++++++++++++
>  target/i386/tcg/user/seg_helper.c   | 109 +++++++++++++
>  target/i386/tcg/sysemu/meson.build  |   1 +
>  target/i386/tcg/user/meson.build    |   1 +
>  7 files changed, 311 insertions(+), 229 deletions(-)
>  create mode 100644 target/i386/tcg/seg_helper.h
>  create mode 100644 target/i386/tcg/sysemu/seg_helper.c
>  create mode 100644 target/i386/tcg/user/seg_helper.c

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

Not your bug, but for the record:

> +void cpu_x86_load_seg(CPUX86State *env, X86Seg seg_reg, int selector)
> +{
> +    if (!(env->cr[0] & CR0_PE_MASK) || (env->eflags & VM_MASK)) {

This first test is always false for user-only.  The second can be true via the
vm86(2) syscall.

> +        int dpl = (env->eflags & VM_MASK) ? 3 : 0;

so this second test becomes redundant.

> +        selector &= 0xffff;
> +        cpu_x86_load_seg_cache(env, seg_reg, selector,
> +                               (selector << 4), 0xffff,
> +                               DESC_P_MASK | DESC_S_MASK | DESC_W_MASK |
> +                               DESC_A_MASK | (dpl << DESC_DPL_SHIFT));
> +    } else {
> +        helper_load_seg(env, seg_reg, selector);
> +    }

And helper_load_seg calls GETPC(), so suffers from the same problem as the fpu
helpers.


r~