Re: [PATCH v1 03/17] xen/riscv: add missing APLIC register offsets, masks to asm/aplic.h

On 2026-08-10 16:45:23+02:00, Oleksii Kurochko wrote:
> On 8/10/26 3:45 PM, Baptiste Le Duc wrote:
> 
> >> These definitions are required for correct decoding of APLIC MMIO
> > 
> > _REG stands for memory-mapped control "region" as explained in spec? If
> > yes, it'd be better to add a comment.
> 
> Yes, it is a mask that allows us to get the offsets for the registers of 
> an interrupt domain’s memory-mapped control region.
> 
> IMO, if the problem is with the name of the macro, it would be better to 
> use a clearer name instead of adding a comment. For example, 
> `APLIC_CTRL_REGION_OFFSET_MASK` sounds self-explanatory to me.

I think the rename of the macro is enough.
Lets keep this:

#define APLIC_CTRL_REGION_OFFSET_MASK   0x3fff

> 
> If you’re still not happy with the suggested name and it isn't 
> self-explanatory, then:
> 
> /*
>   * Offsets of the registers of an interrupt domain's memory-mapped control
>   * region, which is APLIC_MIN_SIZE bytes large.
>   */
> #define APLIC_CTRL_REGION_OFFSET_MASK   0x3fff
> 
> Alternatively, I could keep the old name (APLIC_REG_OFFSET_MASK) and use 
> the suggested comment.
> 
> Which option do you prefer?
> 
> ~ Oleksii




-- 
Baptiste Le Duc | Vates XCP-ng Intern

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech