2022-07-26 01:38:15

by Michael Kelley (LINUX)

[permalink] [raw]
Subject: [PATCH 1/1] iommu/hyper-v: Use helper instead of directly accessing affinity

Recent changes to solve inconsistencies in handling IRQ masks #ifdef
out the affinity field in irq_common_data for non-SMP configurations.
The current code in hyperv_irq_remapping_alloc() gets a compiler error
in that case.

Fix this by using the new irq_data_update_affinity() helper, which
handles the non-SMP case correctly.

Signed-off-by: Michael Kelley <[email protected]>
---
drivers/iommu/hyperv-iommu.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/iommu/hyperv-iommu.c b/drivers/iommu/hyperv-iommu.c
index 51bd66a..e190bb8 100644
--- a/drivers/iommu/hyperv-iommu.c
+++ b/drivers/iommu/hyperv-iommu.c
@@ -68,7 +68,6 @@ static int hyperv_irq_remapping_alloc(struct irq_domain *domain,
{
struct irq_alloc_info *info = arg;
struct irq_data *irq_data;
- struct irq_desc *desc;
int ret = 0;

if (!info || info->type != X86_IRQ_ALLOC_TYPE_IOAPIC || nr_irqs > 1)
@@ -90,8 +89,7 @@ static int hyperv_irq_remapping_alloc(struct irq_domain *domain,
* Hypver-V IO APIC irq affinity should be in the scope of
* ioapic_max_cpumask because no irq remapping support.
*/
- desc = irq_data_to_desc(irq_data);
- cpumask_copy(desc->irq_common_data.affinity, &ioapic_max_cpumask);
+ irq_data_update_affinity(irq_data, &ioapic_max_cpumask);

return 0;
}
--
1.8.3.1


2022-07-26 03:03:46

by Randy Dunlap

[permalink] [raw]
Subject: Re: [PATCH 1/1] iommu/hyper-v: Use helper instead of directly accessing affinity



On 7/25/22 17:53, Michael Kelley wrote:
> Recent changes to solve inconsistencies in handling IRQ masks #ifdef
> out the affinity field in irq_common_data for non-SMP configurations.
> The current code in hyperv_irq_remapping_alloc() gets a compiler error
> in that case.
>
> Fix this by using the new irq_data_update_affinity() helper, which
> handles the non-SMP case correctly.
>

Reported-by: Randy Dunlap <[email protected]>
> Signed-off-by: Michael Kelley <[email protected]>
Tested-by: Randy Dunlap <[email protected]>
Acked-by: Randy Dunlap <[email protected]>

Thanks.

> ---
> drivers/iommu/hyperv-iommu.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/iommu/hyperv-iommu.c b/drivers/iommu/hyperv-iommu.c
> index 51bd66a..e190bb8 100644
> --- a/drivers/iommu/hyperv-iommu.c
> +++ b/drivers/iommu/hyperv-iommu.c
> @@ -68,7 +68,6 @@ static int hyperv_irq_remapping_alloc(struct irq_domain *domain,
> {
> struct irq_alloc_info *info = arg;
> struct irq_data *irq_data;
> - struct irq_desc *desc;
> int ret = 0;
>
> if (!info || info->type != X86_IRQ_ALLOC_TYPE_IOAPIC || nr_irqs > 1)
> @@ -90,8 +89,7 @@ static int hyperv_irq_remapping_alloc(struct irq_domain *domain,
> * Hypver-V IO APIC irq affinity should be in the scope of
> * ioapic_max_cpumask because no irq remapping support.
> */
> - desc = irq_data_to_desc(irq_data);
> - cpumask_copy(desc->irq_common_data.affinity, &ioapic_max_cpumask);
> + irq_data_update_affinity(irq_data, &ioapic_max_cpumask);
>
> return 0;
> }

--
~Randy

2022-07-26 03:35:50

by Michael Kelley (LINUX)

[permalink] [raw]
Subject: RE: [PATCH 1/1] iommu/hyper-v: Use helper instead of directly accessing affinity

From: Randy Dunlap <[email protected]> Sent: Monday, July 25, 2022 7:39 PM
>
> On 7/25/22 17:53, Michael Kelley wrote:
> > Recent changes to solve inconsistencies in handling IRQ masks #ifdef
> > out the affinity field in irq_common_data for non-SMP configurations.
> > The current code in hyperv_irq_remapping_alloc() gets a compiler error
> > in that case.
> >
> > Fix this by using the new irq_data_update_affinity() helper, which
> > handles the non-SMP case correctly.
> >
>
> Reported-by: Randy Dunlap <[email protected]>

Indeed. Apologies for omitting this originally.

> > Signed-off-by: Michael Kelley <[email protected]>
> Tested-by: Randy Dunlap <[email protected]>
> Acked-by: Randy Dunlap <[email protected]>
>
> Thanks.

This patch needs to go through the same tree as Samuel Holland's earlier
patch set to solve inconsistencies in handling IRQ masks on SMP and
non-SMP configs. From what I can tell, that's the irq/irqchip-next tree.

FWIW, it appears there's another occurrence of this same kind of
problem in arch/mips/sgi-ip27/ip27-irq.c where the "affinity" field
is referenced without an #ifdef CONFIG_SMP. Copying the MIPS maintainer
and mailing list -- this one is out of my scope to fix.

Michael

>
> > ---
> > drivers/iommu/hyperv-iommu.c | 4 +---
> > 1 file changed, 1 insertion(+), 3 deletions(-)
> >
> > diff --git a/drivers/iommu/hyperv-iommu.c b/drivers/iommu/hyperv-iommu.c
> > index 51bd66a..e190bb8 100644
> > --- a/drivers/iommu/hyperv-iommu.c
> > +++ b/drivers/iommu/hyperv-iommu.c
> > @@ -68,7 +68,6 @@ static int hyperv_irq_remapping_alloc(struct irq_domain
> *domain,
> > {
> > struct irq_alloc_info *info = arg;
> > struct irq_data *irq_data;
> > - struct irq_desc *desc;
> > int ret = 0;
> >
> > if (!info || info->type != X86_IRQ_ALLOC_TYPE_IOAPIC || nr_irqs > 1)
> > @@ -90,8 +89,7 @@ static int hyperv_irq_remapping_alloc(struct irq_domain
> *domain,
> > * Hypver-V IO APIC irq affinity should be in the scope of
> > * ioapic_max_cpumask because no irq remapping support.
> > */
> > - desc = irq_data_to_desc(irq_data);
> > - cpumask_copy(desc->irq_common_data.affinity, &ioapic_max_cpumask);
> > + irq_data_update_affinity(irq_data, &ioapic_max_cpumask);
> >
> > return 0;
> > }
>
> --
> ~Randy

2022-07-26 07:21:27

by Joerg Roedel

[permalink] [raw]
Subject: Re: [PATCH 1/1] iommu/hyper-v: Use helper instead of directly accessing affinity

Hi Michael,

On Mon, Jul 25, 2022 at 05:53:40PM -0700, Michael Kelley wrote:
> Recent changes to solve inconsistencies in handling IRQ masks #ifdef
> out the affinity field in irq_common_data for non-SMP configurations.
> The current code in hyperv_irq_remapping_alloc() gets a compiler error
> in that case.
>
> Fix this by using the new irq_data_update_affinity() helper, which
> handles the non-SMP case correctly.
>
> Signed-off-by: Michael Kelley <[email protected]>

Please add a fixes tag.

Where is the change which breaks this currently, in some subsystem tree
or already upstream?

In case it is still in a maintainers tree, this patch should be applied
there. Here is my

Acked-by: Joerg Roedel <[email protected]>

for that.

2022-07-26 13:13:26

by Wei Liu

[permalink] [raw]
Subject: Re: [PATCH 1/1] iommu/hyper-v: Use helper instead of directly accessing affinity

On Tue, Jul 26, 2022 at 09:15:58AM +0200, Joerg Roedel wrote:
> Hi Michael,
>
> On Mon, Jul 25, 2022 at 05:53:40PM -0700, Michael Kelley wrote:
> > Recent changes to solve inconsistencies in handling IRQ masks #ifdef
> > out the affinity field in irq_common_data for non-SMP configurations.
> > The current code in hyperv_irq_remapping_alloc() gets a compiler error
> > in that case.
> >
> > Fix this by using the new irq_data_update_affinity() helper, which
> > handles the non-SMP case correctly.
> >
> > Signed-off-by: Michael Kelley <[email protected]>
>
> Please add a fixes tag.
>
> Where is the change which breaks this currently, in some subsystem tree
> or already upstream?
>

The offending patch aa081358 is in linux-next.

> In case it is still in a maintainers tree, this patch should be applied
> there. Here is my
>
> Acked-by: Joerg Roedel <[email protected]>

I can take this patch via hyperv-next. This is a good improvement
anyway.

Thanks,
Wei.

>
> for that.
>

2022-07-26 13:52:35

by Michael Kelley (LINUX)

[permalink] [raw]
Subject: RE: [PATCH 1/1] iommu/hyper-v: Use helper instead of directly accessing affinity

From: Wei Liu <[email protected]> Sent: Tuesday, July 26, 2022 6:09 AM
>
> On Tue, Jul 26, 2022 at 09:15:58AM +0200, Joerg Roedel wrote:
> > Hi Michael,
> >
> > On Mon, Jul 25, 2022 at 05:53:40PM -0700, Michael Kelley wrote:
> > > Recent changes to solve inconsistencies in handling IRQ masks #ifdef
> > > out the affinity field in irq_common_data for non-SMP configurations.
> > > The current code in hyperv_irq_remapping_alloc() gets a compiler error
> > > in that case.
> > >
> > > Fix this by using the new irq_data_update_affinity() helper, which
> > > handles the non-SMP case correctly.
> > >
> > > Signed-off-by: Michael Kelley <[email protected]>
> >
> > Please add a fixes tag.
> >
> > Where is the change which breaks this currently, in some subsystem tree
> > or already upstream?
> >
>
> The offending patch aa081358 is in linux-next.
>
> > In case it is still in a maintainers tree, this patch should be applied
> > there. Here is my
> >
> > Acked-by: Joerg Roedel <[email protected]>
>
> I can take this patch via hyperv-next. This is a good improvement
> anyway.

I don't think this patch should go via hyperv-next. The helper
function is introduced in the linux-next patch in the irq/irqchip-next tree,
so this patch should go through irq/irqchip-next to avoid creating an
interdependency.

Since the original breaking change is not upstream, do I need
a Fixes: tag? Won't using a linux-next commitID in a Fixes:
tag be confusing? There's nothing to backport to stable.

Michael

2022-07-26 15:07:05

by Wei Liu

[permalink] [raw]
Subject: Re: [PATCH 1/1] iommu/hyper-v: Use helper instead of directly accessing affinity

On Tue, Jul 26, 2022 at 01:48:35PM +0000, Michael Kelley (LINUX) wrote:
> From: Wei Liu <[email protected]> Sent: Tuesday, July 26, 2022 6:09 AM
> >
> > On Tue, Jul 26, 2022 at 09:15:58AM +0200, Joerg Roedel wrote:
> > > Hi Michael,
> > >
> > > On Mon, Jul 25, 2022 at 05:53:40PM -0700, Michael Kelley wrote:
> > > > Recent changes to solve inconsistencies in handling IRQ masks #ifdef
> > > > out the affinity field in irq_common_data for non-SMP configurations.
> > > > The current code in hyperv_irq_remapping_alloc() gets a compiler error
> > > > in that case.
> > > >
> > > > Fix this by using the new irq_data_update_affinity() helper, which
> > > > handles the non-SMP case correctly.
> > > >
> > > > Signed-off-by: Michael Kelley <[email protected]>
> > >
> > > Please add a fixes tag.
> > >
> > > Where is the change which breaks this currently, in some subsystem tree
> > > or already upstream?
> > >
> >
> > The offending patch aa081358 is in linux-next.
> >
> > > In case it is still in a maintainers tree, this patch should be applied
> > > there. Here is my
> > >
> > > Acked-by: Joerg Roedel <[email protected]>
> >
> > I can take this patch via hyperv-next. This is a good improvement
> > anyway.
>
> I don't think this patch should go via hyperv-next. The helper
> function is introduced in the linux-next patch in the irq/irqchip-next tree,
> so this patch should go through irq/irqchip-next to avoid creating an
> interdependency.

This is fine too.

Wei.

2022-08-03 16:42:25

by Michael Kelley (LINUX)

[permalink] [raw]
Subject: RE: [PATCH 1/1] iommu/hyper-v: Use helper instead of directly accessing affinity

From: Randy Dunlap <[email protected]> Sent: Monday, July 25, 2022 7:39 PM
>
> On 7/25/22 17:53, Michael Kelley wrote:
> > Recent changes to solve inconsistencies in handling IRQ masks #ifdef
> > out the affinity field in irq_common_data for non-SMP configurations.
> > The current code in hyperv_irq_remapping_alloc() gets a compiler error
> > in that case.
> >
> > Fix this by using the new irq_data_update_affinity() helper, which
> > handles the non-SMP case correctly.
> >
>
> Reported-by: Randy Dunlap <[email protected]>
> > Signed-off-by: Michael Kelley <[email protected]>
> Tested-by: Randy Dunlap <[email protected]>
> Acked-by: Randy Dunlap <[email protected]>
>
> Thanks.
>
> > ---
> > drivers/iommu/hyperv-iommu.c | 4 +---
> > 1 file changed, 1 insertion(+), 3 deletions(-)
> >
> > diff --git a/drivers/iommu/hyperv-iommu.c b/drivers/iommu/hyperv-iommu.c
> > index 51bd66a..e190bb8 100644
> > --- a/drivers/iommu/hyperv-iommu.c
> > +++ b/drivers/iommu/hyperv-iommu.c
> > @@ -68,7 +68,6 @@ static int hyperv_irq_remapping_alloc(struct irq_domain
> *domain,
> > {
> > struct irq_alloc_info *info = arg;
> > struct irq_data *irq_data;
> > - struct irq_desc *desc;
> > int ret = 0;
> >
> > if (!info || info->type != X86_IRQ_ALLOC_TYPE_IOAPIC || nr_irqs > 1)
> > @@ -90,8 +89,7 @@ static int hyperv_irq_remapping_alloc(struct irq_domain *domain,
> > * Hypver-V IO APIC irq affinity should be in the scope of
> > * ioapic_max_cpumask because no irq remapping support.
> > */
> > - desc = irq_data_to_desc(irq_data);
> > - cpumask_copy(desc->irq_common_data.affinity, &ioapic_max_cpumask);
> > + irq_data_update_affinity(irq_data, &ioapic_max_cpumask);
> >
> > return 0;
> > }
>
> --

Marc --

Could you pick up this patch for the 6.0 merge window? It's a fix to a
side effect of Samuel Holland's series to unify SMP and UP handling of
affinity masks that went through your irq/irqchip-next tree.

Michael




2022-08-04 09:42:24

by Marc Zyngier

[permalink] [raw]
Subject: Re: [PATCH 1/1] iommu/hyper-v: Use helper instead of directly accessing affinity

On Wed, 03 Aug 2022 17:26:10 +0100,
"Michael Kelley (LINUX)" <[email protected]> wrote:
>
> From: Randy Dunlap <[email protected]> Sent: Monday, July 25, 2022 7:39 PM
> >
> > On 7/25/22 17:53, Michael Kelley wrote:
> > > Recent changes to solve inconsistencies in handling IRQ masks #ifdef
> > > out the affinity field in irq_common_data for non-SMP configurations.
> > > The current code in hyperv_irq_remapping_alloc() gets a compiler error
> > > in that case.
> > >
> > > Fix this by using the new irq_data_update_affinity() helper, which
> > > handles the non-SMP case correctly.
> > >
> >
> > Reported-by: Randy Dunlap <[email protected]>
> > > Signed-off-by: Michael Kelley <[email protected]>
> > Tested-by: Randy Dunlap <[email protected]>
> > Acked-by: Randy Dunlap <[email protected]>
> >
> > Thanks.
> >
> > > ---
> > > drivers/iommu/hyperv-iommu.c | 4 +---
> > > 1 file changed, 1 insertion(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/iommu/hyperv-iommu.c b/drivers/iommu/hyperv-iommu.c
> > > index 51bd66a..e190bb8 100644
> > > --- a/drivers/iommu/hyperv-iommu.c
> > > +++ b/drivers/iommu/hyperv-iommu.c
> > > @@ -68,7 +68,6 @@ static int hyperv_irq_remapping_alloc(struct irq_domain
> > *domain,
> > > {
> > > struct irq_alloc_info *info = arg;
> > > struct irq_data *irq_data;
> > > - struct irq_desc *desc;
> > > int ret = 0;
> > >
> > > if (!info || info->type != X86_IRQ_ALLOC_TYPE_IOAPIC || nr_irqs > 1)
> > > @@ -90,8 +89,7 @@ static int hyperv_irq_remapping_alloc(struct irq_domain *domain,
> > > * Hypver-V IO APIC irq affinity should be in the scope of
> > > * ioapic_max_cpumask because no irq remapping support.
> > > */
> > > - desc = irq_data_to_desc(irq_data);
> > > - cpumask_copy(desc->irq_common_data.affinity, &ioapic_max_cpumask);
> > > + irq_data_update_affinity(irq_data, &ioapic_max_cpumask);
> > >
> > > return 0;
> > > }
> >
> > --
>
> Marc --
>
> Could you pick up this patch for the 6.0 merge window? It's a fix to a
> side effect of Samuel Holland's series to unify SMP and UP handling of
> affinity masks that went through your irq/irqchip-next tree.

Applied with a:

Fixes: aa0813581b8d ("genirq: Provide an IRQ affinity mask in non-SMP configs")

Thanks,

M.

--
Without deviation from the norm, progress is not possible.

Subject: [irqchip: irq/irqchip-fixes] iommu/hyper-v: Use helper instead of directly accessing affinity

The following commit has been merged into the irq/irqchip-fixes branch of irqchip:

Commit-ID: b0a4ab7ca4ce993d1cc51cbc85e9f341c729a3d4
Gitweb: https://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms/b0a4ab7ca4ce993d1cc51cbc85e9f341c729a3d4
Author: Michael Kelley <[email protected]>
AuthorDate: Mon, 25 Jul 2022 17:53:40 -07:00
Committer: Marc Zyngier <[email protected]>
CommitterDate: Thu, 04 Aug 2022 10:02:09 +01:00

iommu/hyper-v: Use helper instead of directly accessing affinity

Recent changes to solve inconsistencies in handling IRQ masks #ifdef
out the affinity field in irq_common_data for non-SMP configurations.
The current code in hyperv_irq_remapping_alloc() gets a compiler error
in that case.

Fix this by using the new irq_data_update_affinity() helper, which
handles the non-SMP case correctly.

Signed-off-by: Michael Kelley <[email protected]>
Reported-by: Randy Dunlap <[email protected]>
Tested-by: Randy Dunlap <[email protected]>
Acked-by: Randy Dunlap <[email protected]>
Acked-by: Joerg Roedel <[email protected]>
Signed-off-by: Marc Zyngier <[email protected]>
Fixes: aa0813581b8d ("genirq: Provide an IRQ affinity mask in non-SMP configs")
Link: https://lore.kernel.org/r/[email protected]
---
drivers/iommu/hyperv-iommu.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/iommu/hyperv-iommu.c b/drivers/iommu/hyperv-iommu.c
index 51bd66a..e190bb8 100644
--- a/drivers/iommu/hyperv-iommu.c
+++ b/drivers/iommu/hyperv-iommu.c
@@ -68,7 +68,6 @@ static int hyperv_irq_remapping_alloc(struct irq_domain *domain,
{
struct irq_alloc_info *info = arg;
struct irq_data *irq_data;
- struct irq_desc *desc;
int ret = 0;

if (!info || info->type != X86_IRQ_ALLOC_TYPE_IOAPIC || nr_irqs > 1)
@@ -90,8 +89,7 @@ static int hyperv_irq_remapping_alloc(struct irq_domain *domain,
* Hypver-V IO APIC irq affinity should be in the scope of
* ioapic_max_cpumask because no irq remapping support.
*/
- desc = irq_data_to_desc(irq_data);
- cpumask_copy(desc->irq_common_data.affinity, &ioapic_max_cpumask);
+ irq_data_update_affinity(irq_data, &ioapic_max_cpumask);

return 0;
}