Re: [PULL 28/35] target/i386/mshv: Implement mshv_vcpu_run()

[Peter Maydell <peter.maydell@linaro.org>]

On Thu, 9 Oct 2025 at 08:56, Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> From: Magnus Kulke <magnuskulke@linux.microsoft.com>
>
> Add the main vCPU execution loop for MSHV using the MSHV_RUN_VP ioctl.
>
> The execution loop handles guest entry and VM exits. There are handlers for
> memory r/w, PIO and MMIO to which the exit events are dispatched.
>
> In case of MMIO the i386 instruction decoder/emulator is invoked to
> perform the operation in user space.

Hi; Coverity complains about this code (CID 1641395):

> +static int handle_pio_str(CPUState *cpu, hv_x64_io_port_intercept_message *info)
> +{
> +    uint8_t access_type = info->header.intercept_access_type;
> +    uint16_t port = info->port_number;
> +    bool repop = info->access_info.rep_prefix == 1;
> +    size_t repeat = repop ? info->rcx : 1;
> +    size_t insn_len = info->header.instruction_length;
> +    bool direction_flag;
> +    uint32_t reg_names[3];
> +    uint64_t reg_values[3];
> +    int ret;
> +    X86CPU *x86_cpu = X86_CPU(cpu);
> +    CPUX86State *env = &x86_cpu->env;
> +
> +    ret = fetch_guest_state(cpu);
> +    if (ret < 0) {
> +        error_report("Failed to fetch guest state");
> +        return -1;
> +    }
> +
> +    direction_flag = (env->eflags & DESC_E_MASK) != 0;
> +
> +    if (access_type == HV_X64_INTERCEPT_ACCESS_TYPE_WRITE) {
> +        ret = handle_pio_str_write(cpu, info, repeat, port, direction_flag);
> +        if (ret < 0) {
> +            error_report("Failed to handle pio str write");
> +            return -1;
> +        }
> +        reg_names[0] = HV_X64_REGISTER_RSI;
> +        reg_values[0] = info->rsi;
> +    } else {
> +        ret = handle_pio_str_read(cpu, info, repeat, port, direction_flag);

We set ret to the return value here, but there's no error check.
Should there be one here?

Coverity complains because we assign to 'ret' here but
then never read it again before we overwrite it with
the call to set_x64_registers().

> +        reg_names[0] = HV_X64_REGISTER_RDI;
> +        reg_values[0] = info->rdi;
> +    }
> +
> +    reg_names[1] = HV_X64_REGISTER_RIP;
> +    reg_values[1] = info->header.rip + insn_len;
> +    reg_names[2] = HV_X64_REGISTER_RAX;
> +    reg_values[2] = info->rax;
> +
> +    ret = set_x64_registers(cpu, reg_names, reg_values);
> +    if (ret < 0) {
> +        error_report("Failed to set x64 registers");
> +        return -1;
> +    }
> +
> +    cpu->accel->dirty = false;
> +
> +    return 0;
> +}


thanks
-- PMM