Re: [PATCH v22 11/17] i386: split misc helper into user and sysemu parts

[Richard Henderson]

On 2/24/21 5:34 AM, Claudio Fontana wrote:
> +void helper_outb(CPUX86State *env, uint32_t port, uint32_t data)
> +{
> +    fprintf(stderr, "outb: port=0x%04x, data=%02x\n", port, data);
> +}
> +
> +target_ulong helper_inb(CPUX86State *env, uint32_t port)
> +{
> +    fprintf(stderr, "inb: port=0x%04x\n", port);
> +    return 0;
> +}
> +
> +void helper_outw(CPUX86State *env, uint32_t port, uint32_t data)
> +{
> +    fprintf(stderr, "outw: port=0x%04x, data=%04x\n", port, data);
> +}
> +
> +target_ulong helper_inw(CPUX86State *env, uint32_t port)
> +{
> +    fprintf(stderr, "inw: port=0x%04x\n", port);
> +    return 0;
> +}
> +
> +void helper_outl(CPUX86State *env, uint32_t port, uint32_t data)
> +{
> +    fprintf(stderr, "outl: port=0x%04x, data=%08x\n", port, data);
> +}
> +
> +target_ulong helper_inl(CPUX86State *env, uint32_t port)
> +{
> +    fprintf(stderr, "inl: port=0x%04x\n", port);
> +    return 0;
> +}
> +
> +target_ulong helper_read_crN(CPUX86State *env, int reg)
> +{
> +    return 0;
> +}
> +
> +void helper_write_crN(CPUX86State *env, int reg, target_ulong t0)
> +{
> +}
> +
> +void helper_wrmsr(CPUX86State *env)
> +{
> +}
> +
> +void helper_rdmsr(CPUX86State *env)
> +{
> +}

It is not obvious, but all of these are not reachable.

For in/out, there's the IOPL check.  When running on hardware, there's an
ioperm(2) syscall that can change adjust the TSS to allow userland access to
specific ports.  This is used by setuid applications like the xserver.  But we
don't (and can't) implement this syscall in qemu linux-user, so the check
within check_io in seg_helper.c always fails and raises EXCP0D_GPF.

For crN and msr, the cpl check is there in the switch:

    case 0x120: /* mov reg, crN */
    case 0x122: /* mov crN, reg */
        if (s->cpl != 0) {
            gen_exception(s, EXCP0D_GPF, pc_start - s->cs_base);
...
    case 0x130: /* wrmsr */
    case 0x132: /* rdmsr */
        if (s->cpl != 0) {
            gen_exception(s, EXCP0D_GPF, pc_start - s->cs_base);

I think we can improve the compiler eliminating some of this code with

#ifdef CONFIG_USER_ONLY
#define CPL(S)  3
#else
#define CPL(S)  ((S)->cpl)
#endif

then change all of the tests to

    if (CPL(s) != 0) {

and then something akin to

#ifdef CONFIG_USER_ONLY
static inline void gen_helper_rdmsr(TCGv_env unused)
{
    qemu_build_not_reached()
}
#endif

For in/out, lots more cleanup would be required, since that needs to propagate
up through ins/outs, and gen_check_io would want changing as well.  I don't see
an obvious way to avoid the stubs, really.  But we can g_assert_not_reached()
within them, rather than fprintf to stderr.


r~