[PATCH v1 0/2] Fix ARM Generic Timer interrupt parsing

Andrei Cherechesu (OSS) posted 2 patches 1 year, 1 month ago
Patches applied successfully (tree, apply log)
git fetch https://gitlab.com/xen-project/patchew/xen tags/patchew/20230307100949.9231-1-andrei.cherechesu@oss.nxp.com
Test gitlab-ci failed
There is a newer version of this series
xen/arch/arm/include/asm/irq.h        |  2 ++
xen/arch/arm/include/asm/time.h       |  3 ++-
xen/arch/arm/irq.c                    | 14 +++++++++++
xen/arch/arm/time.c                   | 26 +++++++++++++++++---
xen/drivers/passthrough/arm/smmu-v3.c | 35 +++++----------------------
5 files changed, 46 insertions(+), 34 deletions(-)
[PATCH v1 0/2] Fix ARM Generic Timer interrupt parsing
Posted by Andrei Cherechesu (OSS) 1 year, 1 month ago
From: Andrei Cherechesu <andrei.cherechesu@nxp.com>

This 2-patch series fixes the parsing of the ARM Generic Timer
interrupts from the device tree.

If the generic timer interrupts order in the DT was different than
the expected order in Xen code, these interrupts would no longer be
correctly parsed and registered by Xen, and would result in boot failure.

This method with using "interrupt-names" for the generic timer interrupts
instead of having them hardcoded in the DTB in a specific order is the newer
approach already implemented in Linux. Xen did not have the necessary code for
this approach, and it has been implemented by the means of this patch series.

Functionality should remain the same if "interrupt-names" is not present in the
Generic Timer DTB node of the platform, but the interrupts should then still be
present in the expected "sec-phys", "phys", "virt", "hyp-phys", "hyp-virt" order.
If "interrupt-names" is present, now it is also correctly handled.

Andrei Cherechesu (2):
  arch/arm: irq: Add platform_get_irq_byname() implementation
  arch/arm: time: Add support for parsing interrupts by names

 xen/arch/arm/include/asm/irq.h        |  2 ++
 xen/arch/arm/include/asm/time.h       |  3 ++-
 xen/arch/arm/irq.c                    | 14 +++++++++++
 xen/arch/arm/time.c                   | 26 +++++++++++++++++---
 xen/drivers/passthrough/arm/smmu-v3.c | 35 +++++----------------------
 5 files changed, 46 insertions(+), 34 deletions(-)

-- 
2.35.1
Re: [PATCH v1 0/2] Fix ARM Generic Timer interrupt parsing
Posted by Bertrand Marquis 1 year, 1 month ago
Hi Andrei,

When submitting patches, please use the add_maintainer.pl script so that maintainers of the code
modified are added in CC.

> On 7 Mar 2023, at 11:09, Andrei Cherechesu (OSS) <andrei.cherechesu@oss.nxp.com> wrote:
> 
> From: Andrei Cherechesu <andrei.cherechesu@nxp.com>
> 
> This 2-patch series fixes the parsing of the ARM Generic Timer
> interrupts from the device tree.
> 
> If the generic timer interrupts order in the DT was different than
> the expected order in Xen code, these interrupts would no longer be
> correctly parsed and registered by Xen, and would result in boot failure.
> 
> This method with using "interrupt-names" for the generic timer interrupts
> instead of having them hardcoded in the DTB in a specific order is the newer
> approach already implemented in Linux. Xen did not have the necessary code for
> this approach, and it has been implemented by the means of this patch series.

Would mind giving a link to an example or the Linux documentation if there is one ?

Cheers
Bertrand

> 
> Functionality should remain the same if "interrupt-names" is not present in the
> Generic Timer DTB node of the platform, but the interrupts should then still be
> present in the expected "sec-phys", "phys", "virt", "hyp-phys", "hyp-virt" order.
> If "interrupt-names" is present, now it is also correctly handled.
> 
> Andrei Cherechesu (2):
>  arch/arm: irq: Add platform_get_irq_byname() implementation
>  arch/arm: time: Add support for parsing interrupts by names
> 
> xen/arch/arm/include/asm/irq.h        |  2 ++
> xen/arch/arm/include/asm/time.h       |  3 ++-
> xen/arch/arm/irq.c                    | 14 +++++++++++
> xen/arch/arm/time.c                   | 26 +++++++++++++++++---
> xen/drivers/passthrough/arm/smmu-v3.c | 35 +++++----------------------
> 5 files changed, 46 insertions(+), 34 deletions(-)
> 
> -- 
> 2.35.1
> 
> 
Re: [PATCH v1 0/2] Fix ARM Generic Timer interrupt parsing
Posted by Andrei Cherechesu 1 year, 1 month ago

On 07/03/2023 17:27, Bertrand Marquis wrote:
> Hi Andrei,
> 
> When submitting patches, please use the add_maintainer.pl script so that maintainers of the code
> modified are added in CC.

Hi Bertrand,

Thank you for reviewing the patches. I apologize for not adding the
maintainers in CC. I added them now.

> 
>> On 7 Mar 2023, at 11:09, Andrei Cherechesu (OSS) <andrei.cherechesu@oss.nxp.com> wrote:
>>
>> From: Andrei Cherechesu <andrei.cherechesu@nxp.com>
>>
>> This 2-patch series fixes the parsing of the ARM Generic Timer
>> interrupts from the device tree.
>>
>> If the generic timer interrupts order in the DT was different than
>> the expected order in Xen code, these interrupts would no longer be
>> correctly parsed and registered by Xen, and would result in boot failure.
>>
>> This method with using "interrupt-names" for the generic timer interrupts
>> instead of having them hardcoded in the DTB in a specific order is the newer
>> approach already implemented in Linux. Xen did not have the necessary code for
>> this approach, and it has been implemented by the means of this patch series.
> 
> Would mind giving a link to an example or the Linux documentation if there is one ?
> 

The bindings [0] for the ARM Generic Timer DT node were changed around
Linux 5.13, when the interrupt-names property was added, along with the
implementation for handling it [1].


[0]
https://elixir.bootlin.com/linux/v6.3-rc1/source/Documentation/devicetree/bindings/timer/arm,arch_timer.yaml#L44
[1]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/clocksource/arm_arch_timer.c?id=86332e9e3477af8f31c9d5f3e81e57e0fd2118e7


Regards,
Andrei


> Cheers
> Bertrand
> 
>>
>> Functionality should remain the same if "interrupt-names" is not present in the
>> Generic Timer DTB node of the platform, but the interrupts should then still be
>> present in the expected "sec-phys", "phys", "virt", "hyp-phys", "hyp-virt" order.
>> If "interrupt-names" is present, now it is also correctly handled.
>>
>> Andrei Cherechesu (2):
>>  arch/arm: irq: Add platform_get_irq_byname() implementation
>>  arch/arm: time: Add support for parsing interrupts by names
>>
>> xen/arch/arm/include/asm/irq.h        |  2 ++
>> xen/arch/arm/include/asm/time.h       |  3 ++-
>> xen/arch/arm/irq.c                    | 14 +++++++++++
>> xen/arch/arm/time.c                   | 26 +++++++++++++++++---
>> xen/drivers/passthrough/arm/smmu-v3.c | 35 +++++----------------------
>> 5 files changed, 46 insertions(+), 34 deletions(-)
>>
>> -- 
>> 2.35.1
>>
>>
>