Re: [PATCH v5 07/12] xen/arm: gicv3: modify ICH_LR_PHYSICAL_MASK to allow eSPI processing

[Leonid Komarianskyi <Leonid_Komarianskyi@xxxxxxxx>]

Hi Julien,

Thank you for your review.

On 02.09.25 16:53, Julien Grall wrote:
> Hi,
> 
> On 29/08/2025 17:06, Leonid Komarianskyi wrote:
>> To properly deactivate guest interrupts and allow them to be retriggered
>> after the initial trigger, the LR needs to be updated. The current
> 
> Why guest specifically? Isn't the problem the same if a physical eSPI is 
> routed to dom0? IOW, shouldn't the explaination be:
> 
> "To properly deactivate physical eSPI routed to a domain and ..."
>

I will update the commit in V6.

>> implementation ignores interrupts outside the range specified by the mask
>> 0x3FF, which only covers IRQ numbers up to 1023. To enable processing of
>> eSPI interrupts, this patch updates the mask to 0x13FF.
>>
>> Signed-off-by: Leonid Komarianskyi <leonid_komarianskyi@xxxxxxxx>
>> Reviewed-by: Volodymyr Babchuk <volodymyr_babchuk@xxxxxxxx>
>>
>> ---
>> Changes in V5:
>> - no changes
>>
>> Changes in V4:
>> - added reviewed-by from Volodymyr Babchuk
>>
>> Changes in V3:
>> - no changes
>>
>> Changes in V2:
>> - remove unnecessary CONFIG_GICV3_ESPI ifdef guard
>> ---
>>   xen/arch/arm/include/asm/gic_v3_defs.h | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/xen/arch/arm/include/asm/gic_v3_defs.h b/xen/arch/arm/ 
>> include/asm/gic_v3_defs.h
>> index 3370b4cd52..e70c1a5675 100644
>> --- a/xen/arch/arm/include/asm/gic_v3_defs.h
>> +++ b/xen/arch/arm/include/asm/gic_v3_defs.h
>> @@ -211,7 +211,7 @@
>>   #define ICH_LR_VIRTUAL_SHIFT         0
>>   #define ICH_LR_CPUID_MASK            0x7
>>   #define ICH_LR_CPUID_SHIFT           10
>> -#define ICH_LR_PHYSICAL_MASK         0x3ff
>> +#define ICH_LR_PHYSICAL_MASK         0x13ff
> 
> It took me a while to understand why we are using 0x13ff rather than 
> 0x1fff. It is because eSPI range is 4096 - 5519. So in theory, it would 
> be ok to just add '0x1000'. But I think this is more confusion that it 
> is worth. So I would rather prefer if we use 0x1fff as this matches the 
> specification.
> 
> Cheers,
> 

Yes, I agree with that - it will be clearer, so I will update the mask 
to 0x1fff.

Best regards,
Leonid