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

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

[Michael Kelley]

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 <mikelley@microsoft.com>
---
 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

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

[Randy Dunlap]

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 <rdunlap@infradead.org>
> Signed-off-by: Michael Kelley <mikelley@microsoft.com>
Tested-by: Randy Dunlap <rdunlap@infradead.org>
Acked-by: Randy Dunlap <rdunlap@infradead.org>

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

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

[Michael Kelley (LINUX)]

From: Randy Dunlap <rdunlap@infradead.org> 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 <rdunlap@infradead.org>

Indeed.  Apologies for omitting this originally.

> > Signed-off-by: Michael Kelley <mikelley@microsoft.com>
> Tested-by: Randy Dunlap <rdunlap@infradead.org>
> Acked-by: Randy Dunlap <rdunlap@infradead.org>
> 
> 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

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

[Michael Kelley (LINUX)]

From: Randy Dunlap <rdunlap@infradead.org> 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 <rdunlap@infradead.org>
> > Signed-off-by: Michael Kelley <mikelley@microsoft.com>
> Tested-by: Randy Dunlap <rdunlap@infradead.org>
> Acked-by: Randy Dunlap <rdunlap@infradead.org>
> 
> 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

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

[Marc Zyngier]

On Wed, 03 Aug 2022 17:26:10 +0100,
"Michael Kelley (LINUX)" <mikelley@microsoft.com> wrote:
> 
> From: Randy Dunlap <rdunlap@infradead.org> 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 <rdunlap@infradead.org>
> > > Signed-off-by: Michael Kelley <mikelley@microsoft.com>
> > Tested-by: Randy Dunlap <rdunlap@infradead.org>
> > Acked-by: Randy Dunlap <rdunlap@infradead.org>
> > 
> > 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.

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

[Joerg Roedel]

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 <mikelley@microsoft.com>

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 <jroedel@suse.de>

for that.

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

[Wei Liu]

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 <mikelley@microsoft.com>
> 
> 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 <jroedel@suse.de>

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

Thanks,
Wei.

> 
> for that.
> 

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

[Michael Kelley (LINUX)]

From: Wei Liu <wei.liu@kernel.org> 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 <mikelley@microsoft.com>
> >
> > 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 <jroedel@suse.de>
> 
> 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

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

[Wei Liu]

On Tue, Jul 26, 2022 at 01:48:35PM +0000, Michael Kelley (LINUX) wrote:
> From: Wei Liu <wei.liu@kernel.org> 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 <mikelley@microsoft.com>
> > >
> > > 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 <jroedel@suse.de>
> > 
> > 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.

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

[irqchip-bot for Michael Kelley]

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 <mikelley@microsoft.com>
AuthorDate:    Mon, 25 Jul 2022 17:53:40 -07:00
Committer:     Marc Zyngier <maz@kernel.org>
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 <mikelley@microsoft.com>
Reported-by: Randy Dunlap <rdunlap@infradead.org>
Tested-by: Randy Dunlap <rdunlap@infradead.org>
Acked-by: Randy Dunlap <rdunlap@infradead.org>
Acked-by: Joerg Roedel <jroedel@suse.de>
Signed-off-by: Marc Zyngier <maz@kernel.org>
Fixes: aa0813581b8d ("genirq: Provide an IRQ affinity mask in non-SMP configs")
Link: https://lore.kernel.org/r/1658796820-2261-1-git-send-email-mikelley@microsoft.com
---
 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;
 }