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

[Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>]

On 8/10/26 3:45 PM, Baptiste Le Duc wrote:
> > These definitions are required for correct decoding of APLIC MMIO
> > accesses and target configuration, and will be used by both the
> > physical and virtual APLIC implementations.
> >
> > No functional change is intended by this patch; it only centralises
> > hardware definitions that were previously missing.
> >
> > Co-developed-by: Romain Caritey <Romain.Caritey@xxxxxxxxxxxxx>
> > Signed-off-by: Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>
> >
> > diff --git a/xen/arch/riscv/include/asm/aplic.h 
> > b/xen/arch/riscv/include/asm/aplic.h
> > index 07318aaac2..f22622b9a2 100644
> > --- a/xen/arch/riscv/include/asm/aplic.h
> > +++ b/xen/arch/riscv/include/asm/aplic.h
> > @@ -15,6 +15,8 @@
> >   
> >   #include <asm/imsic.h>
> >   
> > +#define APLIC_REG_OFFSET_MASK   0x3fff
> _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.
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