All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andrew Jones <ajones@ventanamicro.com>
To: Thomas Gleixner <tglx@linutronix.de>
Cc: iommu@lists.linux.dev, kvm-riscv@lists.infradead.org,
	 kvm@vger.kernel.org, linux-riscv@lists.infradead.org,
	linux-kernel@vger.kernel.org,  tjeznach@rivosinc.com,
	zong.li@sifive.com, joro@8bytes.org, will@kernel.org,
	 robin.murphy@arm.com, anup@brainfault.org,
	atishp@atishpatra.org,  alex.williamson@redhat.com,
	paul.walmsley@sifive.com, palmer@dabbelt.com,
	 aou@eecs.berkeley.edu
Subject: Re: [RFC PATCH 01/15] irqchip/riscv-imsic: Use hierarchy to reach irq_set_affinity
Date: Thu, 5 Dec 2024 17:12:16 +0100	[thread overview]
Message-ID: <20241205-2ed14db745f00a0ee9be444b@orel> (raw)
In-Reply-To: <87a5dcu2wq.ffs@tglx>

On Tue, Dec 03, 2024 at 05:50:13PM +0100, Thomas Gleixner wrote:
> On Tue, Dec 03 2024 at 17:27, Andrew Jones wrote:
> > On Tue, Dec 03, 2024 at 02:53:45PM +0100, Thomas Gleixner wrote:
> >> On Thu, Nov 14 2024 at 17:18, Andrew Jones wrote:
> >> The whole IMSIC MSI support can be moved over to MSI LIB which makes all
> >> of this indirection go away and your intermediate domain will just fit
> >> in.
> >> 
> >> Uncompiled patch below. If that works, it needs to be split up properly.
> >
> > Thanks Thomas. I gave your patch below a go, but we now fail to have an
> > msi domain set up when probing devices which go through aplic_msi_setup(),
> > resulting in an immediate NULL deference in
> > msi_create_device_irq_domain(). I'll look closer tomorrow.
> 
> Duh! I forgot to update the .select callback. I don't know how you fixed that
> compile fail up. Delta patch below.
> 
> Thanks,
> 
>         tglx
> ---
> --- a/drivers/irqchip/irq-riscv-imsic-platform.c
> +++ b/drivers/irqchip/irq-riscv-imsic-platform.c
> @@ -180,7 +180,7 @@ static void imsic_irq_debug_show(struct
>  static const struct irq_domain_ops imsic_base_domain_ops = {
>  	.alloc		= imsic_irq_domain_alloc,
>  	.free		= imsic_irq_domain_free,
> -	.select		= imsic_irq_domain_select,
> +	.select		= msi_lib_irq_domain_select,
>  #ifdef CONFIG_GENERIC_IRQ_DEBUGFS
>  	.debug_show	= imsic_irq_debug_show,
>  #endif

Hi Thomas,

With this select fix and replacing patch 03/15 of this series with the
following diff, this irqbypass PoC still works.

Based on what Anup said, I kept imsic_msi_update_msg(), which means I
kept this entire patch (01/15) as is. Anup is working on a series to fix
the non-atomic MSI message writes to the device and will likely pick this
patch up along with your changes to convert IMSIC to msi-lib.

I'd like to know your opinion on patch 02/15 of this series and the diff
below. afaict, x86 does something similar with the DOMAIN_BUS_DMAR and
DOMAIN_BUS_AMDVI tokens in x86_init_dev_msi_info().

Thanks,
drew

diff --git a/drivers/irqchip/irq-msi-lib.c b/drivers/irqchip/irq-msi-lib.c
index 51464c6257f3..cc18516a4e82 100644
--- a/drivers/irqchip/irq-msi-lib.c
+++ b/drivers/irqchip/irq-msi-lib.c
@@ -36,14 +36,14 @@ bool msi_lib_init_dev_msi_info(struct device *dev, struct irq_domain *domain,
                return false;
 
        /*
-        * MSI parent domain specific settings. For now there is only the
-        * root parent domain, e.g. NEXUS, acting as a MSI parent, but it is
-        * possible to stack MSI parents. See x86 vector -> irq remapping
+        * MSI parent domain specific settings. There may be only the root
+        * parent domain, e.g. NEXUS, acting as a MSI parent, or there may
+        * be stacked MSI parents, typically used for remapping.
         */
        if (domain->bus_token == pops->bus_select_token) {
                if (WARN_ON_ONCE(domain != real_parent))
                        return false;
-       } else {
+       } else if (real_parent->bus_token != DOMAIN_BUS_MSI_REMAP) {
                WARN_ON_ONCE(1);
                return false;
        }


-- 
kvm-riscv mailing list
kvm-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kvm-riscv

WARNING: multiple messages have this Message-ID (diff)
From: Andrew Jones <ajones@ventanamicro.com>
To: Thomas Gleixner <tglx@linutronix.de>
Cc: iommu@lists.linux.dev, kvm-riscv@lists.infradead.org,
	 kvm@vger.kernel.org, linux-riscv@lists.infradead.org,
	linux-kernel@vger.kernel.org,  tjeznach@rivosinc.com,
	zong.li@sifive.com, joro@8bytes.org, will@kernel.org,
	 robin.murphy@arm.com, anup@brainfault.org,
	atishp@atishpatra.org,  alex.williamson@redhat.com,
	paul.walmsley@sifive.com, palmer@dabbelt.com,
	 aou@eecs.berkeley.edu
Subject: Re: [RFC PATCH 01/15] irqchip/riscv-imsic: Use hierarchy to reach irq_set_affinity
Date: Thu, 5 Dec 2024 17:12:16 +0100	[thread overview]
Message-ID: <20241205-2ed14db745f00a0ee9be444b@orel> (raw)
In-Reply-To: <87a5dcu2wq.ffs@tglx>

On Tue, Dec 03, 2024 at 05:50:13PM +0100, Thomas Gleixner wrote:
> On Tue, Dec 03 2024 at 17:27, Andrew Jones wrote:
> > On Tue, Dec 03, 2024 at 02:53:45PM +0100, Thomas Gleixner wrote:
> >> On Thu, Nov 14 2024 at 17:18, Andrew Jones wrote:
> >> The whole IMSIC MSI support can be moved over to MSI LIB which makes all
> >> of this indirection go away and your intermediate domain will just fit
> >> in.
> >> 
> >> Uncompiled patch below. If that works, it needs to be split up properly.
> >
> > Thanks Thomas. I gave your patch below a go, but we now fail to have an
> > msi domain set up when probing devices which go through aplic_msi_setup(),
> > resulting in an immediate NULL deference in
> > msi_create_device_irq_domain(). I'll look closer tomorrow.
> 
> Duh! I forgot to update the .select callback. I don't know how you fixed that
> compile fail up. Delta patch below.
> 
> Thanks,
> 
>         tglx
> ---
> --- a/drivers/irqchip/irq-riscv-imsic-platform.c
> +++ b/drivers/irqchip/irq-riscv-imsic-platform.c
> @@ -180,7 +180,7 @@ static void imsic_irq_debug_show(struct
>  static const struct irq_domain_ops imsic_base_domain_ops = {
>  	.alloc		= imsic_irq_domain_alloc,
>  	.free		= imsic_irq_domain_free,
> -	.select		= imsic_irq_domain_select,
> +	.select		= msi_lib_irq_domain_select,
>  #ifdef CONFIG_GENERIC_IRQ_DEBUGFS
>  	.debug_show	= imsic_irq_debug_show,
>  #endif

Hi Thomas,

With this select fix and replacing patch 03/15 of this series with the
following diff, this irqbypass PoC still works.

Based on what Anup said, I kept imsic_msi_update_msg(), which means I
kept this entire patch (01/15) as is. Anup is working on a series to fix
the non-atomic MSI message writes to the device and will likely pick this
patch up along with your changes to convert IMSIC to msi-lib.

I'd like to know your opinion on patch 02/15 of this series and the diff
below. afaict, x86 does something similar with the DOMAIN_BUS_DMAR and
DOMAIN_BUS_AMDVI tokens in x86_init_dev_msi_info().

Thanks,
drew

diff --git a/drivers/irqchip/irq-msi-lib.c b/drivers/irqchip/irq-msi-lib.c
index 51464c6257f3..cc18516a4e82 100644
--- a/drivers/irqchip/irq-msi-lib.c
+++ b/drivers/irqchip/irq-msi-lib.c
@@ -36,14 +36,14 @@ bool msi_lib_init_dev_msi_info(struct device *dev, struct irq_domain *domain,
                return false;
 
        /*
-        * MSI parent domain specific settings. For now there is only the
-        * root parent domain, e.g. NEXUS, acting as a MSI parent, but it is
-        * possible to stack MSI parents. See x86 vector -> irq remapping
+        * MSI parent domain specific settings. There may be only the root
+        * parent domain, e.g. NEXUS, acting as a MSI parent, or there may
+        * be stacked MSI parents, typically used for remapping.
         */
        if (domain->bus_token == pops->bus_select_token) {
                if (WARN_ON_ONCE(domain != real_parent))
                        return false;
-       } else {
+       } else if (real_parent->bus_token != DOMAIN_BUS_MSI_REMAP) {
                WARN_ON_ONCE(1);
                return false;
        }


WARNING: multiple messages have this Message-ID (diff)
From: Andrew Jones <ajones@ventanamicro.com>
To: Thomas Gleixner <tglx@linutronix.de>
Cc: iommu@lists.linux.dev, kvm-riscv@lists.infradead.org,
	 kvm@vger.kernel.org, linux-riscv@lists.infradead.org,
	linux-kernel@vger.kernel.org,  tjeznach@rivosinc.com,
	zong.li@sifive.com, joro@8bytes.org, will@kernel.org,
	 robin.murphy@arm.com, anup@brainfault.org,
	atishp@atishpatra.org,  alex.williamson@redhat.com,
	paul.walmsley@sifive.com, palmer@dabbelt.com,
	 aou@eecs.berkeley.edu
Subject: Re: [RFC PATCH 01/15] irqchip/riscv-imsic: Use hierarchy to reach irq_set_affinity
Date: Thu, 5 Dec 2024 17:12:16 +0100	[thread overview]
Message-ID: <20241205-2ed14db745f00a0ee9be444b@orel> (raw)
In-Reply-To: <87a5dcu2wq.ffs@tglx>

On Tue, Dec 03, 2024 at 05:50:13PM +0100, Thomas Gleixner wrote:
> On Tue, Dec 03 2024 at 17:27, Andrew Jones wrote:
> > On Tue, Dec 03, 2024 at 02:53:45PM +0100, Thomas Gleixner wrote:
> >> On Thu, Nov 14 2024 at 17:18, Andrew Jones wrote:
> >> The whole IMSIC MSI support can be moved over to MSI LIB which makes all
> >> of this indirection go away and your intermediate domain will just fit
> >> in.
> >> 
> >> Uncompiled patch below. If that works, it needs to be split up properly.
> >
> > Thanks Thomas. I gave your patch below a go, but we now fail to have an
> > msi domain set up when probing devices which go through aplic_msi_setup(),
> > resulting in an immediate NULL deference in
> > msi_create_device_irq_domain(). I'll look closer tomorrow.
> 
> Duh! I forgot to update the .select callback. I don't know how you fixed that
> compile fail up. Delta patch below.
> 
> Thanks,
> 
>         tglx
> ---
> --- a/drivers/irqchip/irq-riscv-imsic-platform.c
> +++ b/drivers/irqchip/irq-riscv-imsic-platform.c
> @@ -180,7 +180,7 @@ static void imsic_irq_debug_show(struct
>  static const struct irq_domain_ops imsic_base_domain_ops = {
>  	.alloc		= imsic_irq_domain_alloc,
>  	.free		= imsic_irq_domain_free,
> -	.select		= imsic_irq_domain_select,
> +	.select		= msi_lib_irq_domain_select,
>  #ifdef CONFIG_GENERIC_IRQ_DEBUGFS
>  	.debug_show	= imsic_irq_debug_show,
>  #endif

Hi Thomas,

With this select fix and replacing patch 03/15 of this series with the
following diff, this irqbypass PoC still works.

Based on what Anup said, I kept imsic_msi_update_msg(), which means I
kept this entire patch (01/15) as is. Anup is working on a series to fix
the non-atomic MSI message writes to the device and will likely pick this
patch up along with your changes to convert IMSIC to msi-lib.

I'd like to know your opinion on patch 02/15 of this series and the diff
below. afaict, x86 does something similar with the DOMAIN_BUS_DMAR and
DOMAIN_BUS_AMDVI tokens in x86_init_dev_msi_info().

Thanks,
drew

diff --git a/drivers/irqchip/irq-msi-lib.c b/drivers/irqchip/irq-msi-lib.c
index 51464c6257f3..cc18516a4e82 100644
--- a/drivers/irqchip/irq-msi-lib.c
+++ b/drivers/irqchip/irq-msi-lib.c
@@ -36,14 +36,14 @@ bool msi_lib_init_dev_msi_info(struct device *dev, struct irq_domain *domain,
                return false;
 
        /*
-        * MSI parent domain specific settings. For now there is only the
-        * root parent domain, e.g. NEXUS, acting as a MSI parent, but it is
-        * possible to stack MSI parents. See x86 vector -> irq remapping
+        * MSI parent domain specific settings. There may be only the root
+        * parent domain, e.g. NEXUS, acting as a MSI parent, or there may
+        * be stacked MSI parents, typically used for remapping.
         */
        if (domain->bus_token == pops->bus_select_token) {
                if (WARN_ON_ONCE(domain != real_parent))
                        return false;
-       } else {
+       } else if (real_parent->bus_token != DOMAIN_BUS_MSI_REMAP) {
                WARN_ON_ONCE(1);
                return false;
        }


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

  reply	other threads:[~2024-12-05 16:12 UTC|newest]

Thread overview: 102+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-11-14 16:18 [RFC PATCH 00/15] iommu/riscv: Add irqbypass support Andrew Jones
2024-11-14 16:18 ` Andrew Jones
2024-11-14 16:18 ` Andrew Jones
2024-11-14 16:18 ` [RFC PATCH 01/15] irqchip/riscv-imsic: Use hierarchy to reach irq_set_affinity Andrew Jones
2024-11-14 16:18   ` Andrew Jones
2024-11-14 16:18   ` Andrew Jones
2024-12-03 13:53   ` Thomas Gleixner
2024-12-03 13:53     ` Thomas Gleixner
2024-12-03 16:27     ` Andrew Jones
2024-12-03 16:27       ` Andrew Jones
2024-12-03 16:50       ` Thomas Gleixner
2024-12-03 16:50         ` Thomas Gleixner
2024-12-05 16:12         ` Andrew Jones [this message]
2024-12-05 16:12           ` Andrew Jones
2024-12-05 16:12           ` Andrew Jones
2024-12-03 16:37     ` Anup Patel
2024-12-03 16:37       ` Anup Patel
2024-12-03 20:55       ` Thomas Gleixner
2024-12-03 20:55         ` Thomas Gleixner
2024-12-03 22:59         ` Thomas Gleixner
2024-12-03 22:59           ` Thomas Gleixner
2024-12-04  3:43           ` Anup Patel
2024-12-04  3:43             ` Anup Patel
2024-12-04  3:43             ` Anup Patel
2024-12-04 13:05             ` Thomas Gleixner
2024-12-04 13:05               ` Thomas Gleixner
2024-12-04 13:05               ` Thomas Gleixner
2024-11-14 16:18 ` [RFC PATCH 02/15] genirq/msi: Provide DOMAIN_BUS_MSI_REMAP Andrew Jones
2024-11-14 16:18   ` Andrew Jones
2024-11-14 16:18   ` Andrew Jones
2024-11-14 16:18 ` [RFC PATCH 03/15] irqchip/riscv-imsic: Add support for DOMAIN_BUS_MSI_REMAP Andrew Jones
2024-11-14 16:18   ` Andrew Jones
2024-11-14 16:18   ` Andrew Jones
2024-11-14 16:18 ` [RFC PATCH 04/15] iommu/riscv: report iommu capabilities Andrew Jones
2024-11-14 16:18   ` Andrew Jones
2024-11-14 16:18   ` Andrew Jones
2024-11-15 15:20   ` Robin Murphy
2024-11-15 15:20     ` Robin Murphy
2024-11-15 15:20     ` Robin Murphy
2024-11-19  8:28     ` Andrew Jones
2024-11-19  8:28       ` Andrew Jones
2024-11-19  8:28       ` Andrew Jones
2024-11-14 16:18 ` [RFC PATCH 05/15] iommu/riscv: use data structure instead of individual values Andrew Jones
2024-11-14 16:18   ` Andrew Jones
2024-11-14 16:18   ` Andrew Jones
2024-11-14 16:18 ` [RFC PATCH 06/15] iommu/riscv: support GSCID and GVMA invalidation command Andrew Jones
2024-11-14 16:18   ` Andrew Jones
2024-11-14 16:18   ` Andrew Jones
2024-11-14 16:18 ` [RFC PATCH 07/15] iommu/riscv: Move definitions to iommu.h Andrew Jones
2024-11-14 16:18   ` Andrew Jones
2024-11-14 16:18   ` Andrew Jones
2024-11-14 16:18 ` [RFC PATCH 08/15] iommu/riscv: Add IRQ domain for interrupt remapping Andrew Jones
2024-11-14 16:18   ` Andrew Jones
2024-11-14 16:18   ` Andrew Jones
2024-11-18 18:43   ` Jason Gunthorpe
2024-11-18 18:43     ` Jason Gunthorpe
2024-11-18 18:43     ` Jason Gunthorpe
2024-11-19  7:49     ` Andrew Jones
2024-11-19  7:49       ` Andrew Jones
2024-11-19  7:49       ` Andrew Jones
2024-11-19 14:00       ` Jason Gunthorpe
2024-11-19 14:00         ` Jason Gunthorpe
2024-11-19 14:00         ` Jason Gunthorpe
2024-11-19 15:03         ` Andrew Jones
2024-11-19 15:03           ` Andrew Jones
2024-11-19 15:03           ` Andrew Jones
2024-11-19 15:36           ` Jason Gunthorpe
2024-11-19 15:36             ` Jason Gunthorpe
2024-11-19 15:36             ` Jason Gunthorpe
2024-11-22 15:11             ` Andrew Jones
2024-11-22 15:11               ` Andrew Jones
2024-11-22 15:11               ` Andrew Jones
2024-11-22 15:33               ` Jason Gunthorpe
2024-11-22 15:33                 ` Jason Gunthorpe
2024-11-22 15:33                 ` Jason Gunthorpe
2024-11-22 17:07                 ` Andrew Jones
2024-11-22 17:07                   ` Andrew Jones
2024-11-22 17:07                   ` Andrew Jones
2024-11-25 15:07                   ` Jason Gunthorpe
2024-11-25 15:07                     ` Jason Gunthorpe
2024-11-25 15:07                     ` Jason Gunthorpe
2024-11-14 16:18 ` [RFC PATCH 09/15] RISC-V: KVM: Enable KVM_VFIO interfaces on RISC-V arch Andrew Jones
2024-11-14 16:18   ` Andrew Jones
2024-11-14 16:18   ` Andrew Jones
2024-11-14 16:18 ` [RFC PATCH 10/15] RISC-V: KVM: Add irqbypass skeleton Andrew Jones
2024-11-14 16:18   ` Andrew Jones
2024-11-14 16:18   ` Andrew Jones
2024-11-14 16:18 ` [RFC PATCH 11/15] RISC-V: Define irqbypass vcpu_info Andrew Jones
2024-11-14 16:18   ` Andrew Jones
2024-11-14 16:18   ` Andrew Jones
2024-11-14 16:18 ` [RFC PATCH 12/15] iommu/riscv: Add guest file irqbypass support Andrew Jones
2024-11-14 16:18   ` Andrew Jones
2024-11-14 16:18   ` Andrew Jones
2024-11-14 16:18 ` [RFC PATCH 13/15] RISC-V: KVM: " Andrew Jones
2024-11-14 16:18   ` Andrew Jones
2024-11-14 16:18   ` Andrew Jones
2024-11-14 16:18 ` [RFC PATCH 14/15] vfio: enable IOMMU_TYPE1 for RISC-V Andrew Jones
2024-11-14 16:18   ` Andrew Jones
2024-11-14 16:18   ` Andrew Jones
2024-11-14 16:19 ` [RFC PATCH 15/15] RISC-V: defconfig: Add VFIO modules Andrew Jones
2024-11-14 16:19   ` Andrew Jones
2024-11-14 16:19   ` Andrew Jones

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20241205-2ed14db745f00a0ee9be444b@orel \
    --to=ajones@ventanamicro.com \
    --cc=alex.williamson@redhat.com \
    --cc=anup@brainfault.org \
    --cc=aou@eecs.berkeley.edu \
    --cc=atishp@atishpatra.org \
    --cc=iommu@lists.linux.dev \
    --cc=joro@8bytes.org \
    --cc=kvm-riscv@lists.infradead.org \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-riscv@lists.infradead.org \
    --cc=palmer@dabbelt.com \
    --cc=paul.walmsley@sifive.com \
    --cc=robin.murphy@arm.com \
    --cc=tglx@linutronix.de \
    --cc=tjeznach@rivosinc.com \
    --cc=will@kernel.org \
    --cc=zong.li@sifive.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.