diff mbox series

[RFC] ACPI/irq: Apply ACPI_IRQCHIP_FWSPEC_ARG0 only for GSI domain

Message ID 20230428165825.699910-1-sdonthineni@nvidia.com (mailing list archive)
State New, archived
Headers show
Series [RFC] ACPI/irq: Apply ACPI_IRQCHIP_FWSPEC_ARG0 only for GSI domain | expand

Commit Message

Shanker Donthineni April 28, 2023, 4:58 p.m. UTC
The implementation of the updated GICv3 parameter parsing code in
pack_fwspec() is not compatible for the non-GSI drivers. It uses
the offset 1 for hardware IRQ, but the unmodified GPIO driver
expects the offset value to be 0, leading to a regression.

This patch applies new mapping to the GSI driver only to fix
the issue.

Signed-off-by: Shanker Donthineni <sdonthineni@nvidia.com>
---

 drivers/acpi/irq.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

Comments

Shanker Donthineni June 1, 2023, 2:33 a.m. UTC | #1
Hi James

When the IRQ combining feature is enabled, the following modification becomes
necessary. Without implementing this change. The MPAM feature is not working on
NVIDIA platforms.

Please review the code and provide feedback.


On 4/28/23 11:58, Shanker Donthineni wrote:
> The implementation of the updated GICv3 parameter parsing code in
> pack_fwspec() is not compatible for the non-GSI drivers. It uses
> the offset 1 for hardware IRQ, but the unmodified GPIO driver
> expects the offset value to be 0, leading to a regression.
> 
> This patch applies new mapping to the GSI driver only to fix
> the issue.
> 
> Signed-off-by: Shanker Donthineni <sdonthineni@nvidia.com>
> ---
> 
>   drivers/acpi/irq.c | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/acpi/irq.c b/drivers/acpi/irq.c
> index d4225d0b45d0..3d7f0643ed14 100644
> --- a/drivers/acpi/irq.c
> +++ b/drivers/acpi/irq.c
> @@ -53,7 +53,8 @@ static void pack_fwspec(struct irq_fwspec *fwspec, u32 gsi, int trigger,
>   {
>   	unsigned int offset = 0;
>   
> -	if (IS_ENABLED(CONFIG_ACPI_IRQCHIP_FWSPEC_ARG0)) {
> +	if (IS_ENABLED(CONFIG_ACPI_IRQCHIP_FWSPEC_ARG0) &&
> +	    (fwspec->fwnode == acpi_get_gsi_domain_id(gsi))) {
>   		fwspec->param[0] = ACPI_IRQCHIP_FWSPEC_ARG0;
>   		offset = 1;
>   	}
> @@ -216,8 +217,8 @@ static inline void acpi_irq_parse_one_match(struct fwnode_handle *fwnode,
>   	ctx->rc = 0;
>   	*ctx->res_flags = acpi_dev_irq_flags(triggering, polarity, shareable, wake_capable);
>   
> -	pack_fwspec(ctx->fwspec, hwirq, triggering, polarity);
>   	ctx->fwspec->fwnode = fwnode;
> +	pack_fwspec(ctx->fwspec, hwirq, triggering, polarity);
>   }
>   
>   /**
Shanker Donthineni June 23, 2023, 8:28 p.m. UTC | #2
Hi James,

The usage of an uninitialized variable in the PPTT code has resulted in another problem.
The MPAM driver probe fails when the variable "num_levels" contains a random value. The
code modification provided below resolves the issue. Please review it.

$ git diff drivers/acpi/pptt.c
diff --git a/drivers/acpi/pptt.c b/drivers/acpi/pptt.c
index 200a5f806d89..773334379fa4 100644
--- a/drivers/acpi/pptt.c
+++ b/drivers/acpi/pptt.c
@@ -1003,6 +1003,8 @@ int find_acpi_cache_level_from_id(u32 cache_id)
                 cpu_node = acpi_find_processor_node(table, acpi_cpu_id);
                 if (!cpu_node)
                         break;
+
+               num_levels = 0;
                 acpi_count_levels(table, cpu_node, &num_levels, NULL);

                 for (level = 0; level <= num_levels; level++) {
@@ -1073,6 +1075,8 @@ int acpi_pptt_get_cpumask_from_cache_id(u32 cache_id, cpumask_t *cpus)
                 cpu_node = acpi_find_processor_node(table, acpi_cpu_id);
                 if (!cpu_node)
                         break;
+
+               num_levels = 0;
                 acpi_count_levels(table, cpu_node, &num_levels, NULL);

                 for (level = 0; level <= num_levels; level++) {

-Shanker

On 5/31/23 21:33, Shanker Donthineni wrote:
> Hi James
> 
> When the IRQ combining feature is enabled, the following modification becomes
> necessary. Without implementing this change. The MPAM feature is not working on
> NVIDIA platforms.
> 
> Please review the code and provide feedback.
> 
> 
> On 4/28/23 11:58, Shanker Donthineni wrote:
>> The implementation of the updated GICv3 parameter parsing code in
>> pack_fwspec() is not compatible for the non-GSI drivers. It uses
>> the offset 1 for hardware IRQ, but the unmodified GPIO driver
>> expects the offset value to be 0, leading to a regression.
>>
>> This patch applies new mapping to the GSI driver only to fix
>> the issue.
>>
>> Signed-off-by: Shanker Donthineni <sdonthineni@nvidia.com>
>> ---
>>
>>   drivers/acpi/irq.c | 5 +++--
>>   1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/acpi/irq.c b/drivers/acpi/irq.c
>> index d4225d0b45d0..3d7f0643ed14 100644
>> --- a/drivers/acpi/irq.c
>> +++ b/drivers/acpi/irq.c
>> @@ -53,7 +53,8 @@ static void pack_fwspec(struct irq_fwspec *fwspec, u32 gsi, int trigger,
>>   {
>>       unsigned int offset = 0;
>> -    if (IS_ENABLED(CONFIG_ACPI_IRQCHIP_FWSPEC_ARG0)) {
>> +    if (IS_ENABLED(CONFIG_ACPI_IRQCHIP_FWSPEC_ARG0) &&
>> +        (fwspec->fwnode == acpi_get_gsi_domain_id(gsi))) {
>>           fwspec->param[0] = ACPI_IRQCHIP_FWSPEC_ARG0;
>>           offset = 1;
>>       }
>> @@ -216,8 +217,8 @@ static inline void acpi_irq_parse_one_match(struct fwnode_handle *fwnode,
>>       ctx->rc = 0;
>>       *ctx->res_flags = acpi_dev_irq_flags(triggering, polarity, shareable, wake_capable);
>> -    pack_fwspec(ctx->fwspec, hwirq, triggering, polarity);
>>       ctx->fwspec->fwnode = fwnode;
>> +    pack_fwspec(ctx->fwspec, hwirq, triggering, polarity);
>>   }
>>   /**
diff mbox series

Patch

diff --git a/drivers/acpi/irq.c b/drivers/acpi/irq.c
index d4225d0b45d0..3d7f0643ed14 100644
--- a/drivers/acpi/irq.c
+++ b/drivers/acpi/irq.c
@@ -53,7 +53,8 @@  static void pack_fwspec(struct irq_fwspec *fwspec, u32 gsi, int trigger,
 {
 	unsigned int offset = 0;
 
-	if (IS_ENABLED(CONFIG_ACPI_IRQCHIP_FWSPEC_ARG0)) {
+	if (IS_ENABLED(CONFIG_ACPI_IRQCHIP_FWSPEC_ARG0) &&
+	    (fwspec->fwnode == acpi_get_gsi_domain_id(gsi))) {
 		fwspec->param[0] = ACPI_IRQCHIP_FWSPEC_ARG0;
 		offset = 1;
 	}
@@ -216,8 +217,8 @@  static inline void acpi_irq_parse_one_match(struct fwnode_handle *fwnode,
 	ctx->rc = 0;
 	*ctx->res_flags = acpi_dev_irq_flags(triggering, polarity, shareable, wake_capable);
 
-	pack_fwspec(ctx->fwspec, hwirq, triggering, polarity);
 	ctx->fwspec->fwnode = fwnode;
+	pack_fwspec(ctx->fwspec, hwirq, triggering, polarity);
 }
 
 /**