Re: [PATCH v2 04/39] xen/riscv: introduce csr_read64()

On 27/08/2026 4:20 pm, Oleksii Kurochko wrote:
> diff --git a/xen/arch/riscv/include/asm/csr.h b/xen/arch/riscv/include/asm/csr.h
> index 888d6a2a86d6..a5cdd6f99c8e 100644
> --- a/xen/arch/riscv/include/asm/csr.h
> +++ b/xen/arch/riscv/include/asm/csr.h
> @@ -39,12 +39,36 @@
>      csr_write(csr, v_);             \
>      csr_write(csr ## H, v_ >> 32);  \
>  })
> +
> +/*
> + * The two halves are read by separate instructions, so a CSR which hardware
> + * increments can carry from the low half into the high one in between,
> + * yielding a value the CSR never held. Re-read the high half and retry the
> + * sequence if it changed.
> + */
> +#define csr_read64(csr)                         \
> +({                                              \
> +    uint32_t hi_, lo_;                          \
> +                                                \
> +    do {                                        \
> +        hi_ = csr_read(csr ## H);               \
> +        lo_ = csr_read(csr);                    \
> +    } while ( hi_ != csr_read(csr ## H) );      \
> +                                                \
> +    ((uint64_t)hi_ << 32) | lo_;                \
> +})

This double reads H in the looping case.  You want something more like:

hi = csr_read();
do {
    old = hi;
    lo = csr_read();
} while ( (hi = csr_read()) != old );


Still, this only matters for volatile CSRs, and is unnecessary in the
general case.  I'd suggest naming it csr_volatile_read64().  Most CSRs
can use a simple split access.

~Andrew