Re: [PATCH v7 12/14] target/riscv: rvk: add CSR support for Zkr

[Richard Henderson]

On 2/28/22 04:48, Weiwei Li wrote:
> +/* Crypto Extension */
> +static RISCVException rmw_seed(CPURISCVState *env, int csrno,
> +                              target_ulong *ret_value,
> +                              target_ulong new_value, target_ulong write_mask)
> +{
> +    if (!write_mask) {
> +        return RISCV_EXCP_ILLEGAL_INST;
> +    }

This is incorrect.  The error should only be with a write-mask of the actual x0 register, 
not another register which happens to contain 0.  There is in fact no way to diagnose 
exactly what you want here, which IIRC has an existing fixme comment somewhere.

> +    uint32_t return_status =  SEED_OPST_ES16;
> +
> +    *ret_value = return_status;
> +    if (return_status == SEED_OPST_ES16) {
> +        uint16_t random_number;
> +        qemu_guest_getrandom_nofail(&random_number, sizeof(random_number));
> +        *ret_value = (*ret_value) | random_number;
> +    } else if (return_status == SEED_OPST_BIST) {
> +        /* Do nothing */
> +    } else if (return_status == SEED_OPST_WAIT) {
> +        /* Do nothing */
> +    } else if (return_status == SEED_OPST_DEAD) {
> +        /* Do nothing */
> +    }

This is also incorrect.  This should be

    uint32_t result;
    uint16_t random_v;
    Error *random_e = NULL;
    int random_r;

    random_r = guest_getrandom(&random_v, 2, &random_e);
    if (unlikely(random_r < 0)) {
        /*
         * Failed, for unknown reasons in the crypto subsystem.
         * The best we can do is log the reason and return a
         * failure indication to the guest.  There is no reason
         * we know to expect the failure to be transitory, so
         * indicate DEAD to avoid having the guest spin on WAIT.
         */
        qemu_log_mask(LOG_UNIMP, "%s: Crypto failure: %s",
                      __func__, error_get_pretty(random_e));
        error_free(random_e);
        result = SEED_OPST_DEAD;
    } else {
        result = random_v | SEED_OPST_ES16;
    }

C.f. target/arm/helper.c, rndr_readfn.


r~