Re: [PATCH v2 20/26] xen/riscv: add missing APLIC register offsets, masks to asm/aplic.h.

[Jan Beulich <jbeulich@suse.com>]

On 05.06.2026 10:48, Oleksii Kurochko wrote:
> On 6/3/26 5:36 PM, Jan Beulich wrote:
>> On 08.05.2026 16:43, Oleksii Kurochko wrote:
>>> --- a/xen/arch/riscv/include/asm/aplic.h
>>> +++ b/xen/arch/riscv/include/asm/aplic.h
>>> @@ -15,6 +15,11 @@
>>>   
>>>   #include <asm/imsic.h>
>>>   
>>> +#define APLIC_REG_OFFSET_MASK   0x3fff
>>
>> This I can see this as wanting to live separately. Yet ...
>>
>>> +#define APLIC_TARGET_IPRIO_MASK 0xff
> 
> This could be dropped as I don't use this mask anymore.
> 
>>> +#define APLIC_TARGET_GUEST_IDX_SHIFT 12
>>> +#define APLIC_TARGET_EIID_MASK  0x7ff
>>
>> ... what (set of) register(s) do these apply to? Perhaps ...
>>
>>> @@ -26,6 +31,36 @@
>>>   #define APLIC_SOURCECFG_SM_LEVEL_HIGH   0x6
>>>   #define APLIC_SOURCECFG_SM_LEVEL_LOW    0x7
>>>   
>>> +#define APLIC_DOMAINCFG         0x0000
>>> +#define APLIC_SOURCECFG_BASE    0x0004
>>> +#define APLIC_SOURCECFG_LAST    0x0ffc
>>> +
>>> +#define APLIC_SMSICFGADDR       0x1bc8
>>> +#define APLIC_SMSICFGADDRH      0x1bcc
>>> +
>>> +#define APLIC_SETIP_BASE        0x1c00
>>> +#define APLIC_SETIP_LAST        0x1c7c
>>> +#define APLIC_SETIPNUM          0x1cdc
>>> +
>>> +#define APLIC_CLRIP_BASE        0x1d00
>>> +#define APLIC_CLRIP_LAST        0x1d7c
>>> +#define APLIC_CLRIPNUM          0x1ddc
>>> +
>>> +#define APLIC_SETIE_BASE        0x1e00
>>> +#define APLIC_SETIE_LAST        0x1e7c
>>> +#define APLIC_SETIENUM          0x1edc
>>> +
>>> +#define APLIC_CLRIE_BASE        0x1f00
>>> +#define APLIC_CLRIE_LAST        0x1f7c
>>> +#define APLIC_CLRIENUM          0x1fdc
>>> +
>>> +#define APLIC_SETIPNUM_LE       0x2000
>>> +
>>> +#define APLIC_GENMSI            0x3000
>>> +
>>> +#define APLIC_TARGET_BASE       0x3004
>>> +#define APLIC_TARGET_LAST       0x3ffc
>>
>> ... these? And then is ...
> 
> Yes, it is applied to target register.
> 
>>
>>>   #define APLIC_TARGET_HART_IDX_SHIFT 18
>>
>> ... this also covering some part of them? Can't they (a) live together and (b)
>> have some kind of connection to what they apply to?
> 
> Agreed, they could live together. The intent was simply to keep all MMIO 
> definitions in sequence,

That would also be fine, but then please add a comment to the register field
definition groups.

Jan

> but I'm okay with grouping macros related to a 
> specific register together.