[v5] Introduce eSPI support

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

Posted by Leonid Komarianskyi 2 months ago

To properly deactivate guest interrupts and allow them to be retriggered
after the initial trigger, the LR needs to be updated. The current
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@epam.com>
Reviewed-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>

---
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
 #define ICH_LR_PHYSICAL_SHIFT        32
 #define ICH_LR_STATE_MASK            0x3
 #define ICH_LR_STATE_SHIFT           62
-- 
2.34.1

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

Posted by Julien Grall 1 month, 4 weeks ago

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 ..."

> 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@epam.com>
> Reviewed-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>
> 
> ---
> 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,

-- 
Julien Grall

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

Posted by Leonid Komarianskyi 1 month, 4 weeks ago

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@epam.com>
>> Reviewed-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>
>>
>> ---
>> 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