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
