Re: [PATCH v7 01/14] xen/riscv: detect and store supported hypervisor CSR bits at boot

[Jan Beulich <jbeulich@xxxxxxxx>]

On 10.03.2026 17:00, Oleksii Kurochko wrote:
> On 3/10/26 9:11 AM, Jan Beulich wrote:
>> On 06.03.2026 17:33, Oleksii Kurochko wrote:
>>> --- a/xen/arch/riscv/domain.c
>>> +++ b/xen/arch/riscv/domain.c
>>> @@ -2,9 +2,66 @@
>>>   
>>>   #include <xen/init.h>
>>>   #include <xen/mm.h>
>>> +#include <xen/sections.h>
>>>   #include <xen/sched.h>
>>>   #include <xen/vmap.h>
>>>   
>>> +#include <asm/cpufeature.h>
>>> +#include <asm/csr.h>
>>> +
>>> +struct csr_masks {
>>> +    register_t hedeleg;
>>> +    register_t henvcfg;
>>> +    register_t hideleg;
>>> +    register_t hstateen0;
>>> +
>>> +    struct {
>>> +        register_t hstateen0;
>>> +    } ro_one;
>>> +};
>>> +
>>> +static struct csr_masks __ro_after_init csr_masks;
>>> +
>>> +#define HEDELEG_AVAIL_MASK ULONG_MAX
>>> +#define HIDELEG_AVAIL_MASK ULONG_MAX
>>> +#define HENVCFG_AVAIL_MASK _UL(0xE0000003000000FF)
>>> +#define HSTATEEN0_AVAIL_MASK _UL(0xDE00000000000007)
>> It's not quite clear to me what AVAIL in here is to signal.
> 
> It signal that these bits are potentially available for s/w to be set.
> If you want to suggest the better naming and can change that in the
> follow-up patch.

I'd either omit the infix altogether ("avail" after all often means
"available for software use"), or use "valid" or (less desirable)
"defined".

>>   It's also not
>> quite clear to me why you would use _UL() in #define-s sitting in a C file
>> (and hence not possibly being used in assembly code; even for asm() I'd
>> expect constants to be properly passed in as C operands).
> 
> I thought it is always be good to use _UL() for such type of constants as
> ULONG_MAX also uses UL, but not in form of _UL() macros. If it would be
> better to drop, I can do that in follow-up patch.

The suffixes want to be there, at the very least for Misra's sake. But
you can just write 0xabcdUL, there's no need to involve a macro there.
That's only needed when the appending of the suffix needs to be
conditional upon is being C or assembler code that includes a header.

Jan