On 10.03.2026 09:11, Jan Beulich wrote:
> On 06.03.2026 17:33, Oleksii Kurochko wrote:
>> +void __init init_csr_masks(void)
>> +{
>> + /*
>> + * The mask specifies the bits that may be safely modified without
>> + * causing side effects.
>> + *
>> + * For example, registers such as henvcfg or hstateen0 contain WPRI
>> + * fields that must be preserved. Any write to the full register must
>> + * therefore retain the original values of those fields.
>> + */
>> +#define INIT_CSR_MASK(csr, field, mask) do { \
>> + register_t old = csr_read_set(CSR_##csr, mask); \
>> + csr_masks.field = csr_swap(CSR_##csr, old); \
>> + } while (0)
>> +
>> +#define INIT_RO_ONE_MASK(csr, field, mask) do { \
>> + register_t old = csr_read_clear(CSR_HSTATEEN0, mask); \
>> + csr_masks.ro_one.field = csr_swap(CSR_##csr, old) & mask; \
>> + } while (0)
>> +
>> + INIT_CSR_MASK(HEDELEG, hedeleg, HEDELEG_AVAIL_MASK);
>> + INIT_CSR_MASK(HIDELEG, hideleg, HIDELEG_AVAIL_MASK);
>> +
>> + INIT_CSR_MASK(HENVCFG, henvcfg, HENVCFG_AVAIL_MASK);
>> +
>> + if ( riscv_isa_extension_available(NULL, RISCV_ISA_EXT_smstateen) )
>> + {
>> + INIT_CSR_MASK(HSTATEEN0, hstateen0, HSTATEEN0_AVAIL_MASK);
>> + INIT_RO_ONE_MASK(HSTATEEN0, hstateen0, HSTATEEN0_AVAIL_MASK);
>> + }
>
> The 3rd macro parameters are now redundant. At the example of INIT_CSR_MASK(),
> you could now have
>
> #define INIT_CSR_MASK(csr, field) do { \
> register_t old = csr_read_set(CSR_ ## csr, csr ## _AVAIL_MASK); \
> csr_masks.field = csr_swap(CSR_ ## csr, old); \
> } while (0)
>
> This would reduce the risk of incomplete editing after copy-and-paste, or
> other typo-ing.
>
> Note also that ## being a binary operator, ./CODING_STYLE wants us to put
> blanks around it just like for non-pre-processor binary operators. I'll
> try to remember to make that adjustment when committing.
Oh, I'm also going to replace the bogus CSR_HSTATEEN0 inside the
INIT_RO_ONE_MASK() macro definition.
Jan
