From: Roman Kisel <romank@linux.microsoft.com>
To: Bjorn Helgaas <helgaas@kernel.org>,
rafael@kernel.org, lenb@kernel.org, linux-acpi@vger.kernel.org
Cc: arnd@arndb.de, bhelgaas@google.com, bp@alien8.de,
catalin.marinas@arm.com, conor+dt@kernel.org,
dave.hansen@linux.intel.com, decui@microsoft.com,
haiyangz@microsoft.com, hpa@zytor.com, krzk+dt@kernel.org,
kw@linux.com, kys@microsoft.com, lpieralisi@kernel.org,
manivannan.sadhasivam@linaro.org, mingo@redhat.com,
robh@kernel.org, ssengar@linux.microsoft.com, tglx@linutronix.de,
wei.liu@kernel.org, will@kernel.org, devicetree@vger.kernel.org,
linux-arch@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
linux-hyperv@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-pci@vger.kernel.org, x86@kernel.org, benhill@microsoft.com,
bperkins@microsoft.com, sunilmut@microsoft.com
Subject: Re: [PATCH hyperv-next v4 6/6] PCI: hv: Get vPCI MSI IRQ domain from DeviceTree
Date: Wed, 19 Feb 2025 15:51:59 -0800 [thread overview]
Message-ID: <5cd71475-b107-4269-829f-2c492f625aae@linux.microsoft.com> (raw)
In-Reply-To: <20250212174203.GA81135@bhelgaas>
On 2/12/2025 9:42 AM, Bjorn Helgaas wrote:
> On Tue, Feb 11, 2025 at 05:43:21PM -0800, Roman Kisel wrote:
[...]
>> */
>> - hv_msi_gic_irq_domain = acpi_irq_create_hierarchy(0, HV_PCI_MSI_SPI_NR,
>> - fn, &hv_pci_domain_ops,
>> - chip_data);
>> +#ifdef CONFIG_ACPI
>> + if (!acpi_disabled)
>> + hv_msi_gic_irq_domain = acpi_irq_create_hierarchy(0, HV_PCI_MSI_SPI_NR,
>> + fn, &hv_pci_domain_ops,
>> + chip_data);
>> +#endif
>> +#if defined(CONFIG_OF)
>> + if (!hv_msi_gic_irq_domain)
>> + hv_msi_gic_irq_domain = irq_domain_create_hierarchy(
>> + hv_pci_of_irq_domain_parent(), 0, HV_PCI_MSI_SPI_NR,
>> + fn, &hv_pci_domain_ops,
>> + chip_data);
>> +#endif
>
> I don't know if acpi_irq_create_hierarchy() is helping or hurting
> here. It obscures the fact that the only difference is the first
> argument to irq_domain_create_hierarchy(). If we could open-code or
> have a helper to figure out that irq_domain "parent" argument for the
> ACPI case, then we'd only have one call of
> irq_domain_create_hierarchy() here and it seems like it might be
> simpler.
>
Hey Bjorn, folks,
I've added few ACPI maintainers and the ACPI list as we're discussing
making a small change to the ACPI subsystem to make one static variable
available to make the code above less messy.
Change [1] makes the GSI dispatcher function available to
the outside world. Would you suggest going in that direction or there
is a better approach to converge the code above that deals with IRQ
domains both in the ACPI and DT cases?
[1]
From c6fb8bda21d6c00a308b1febc201a3a7e704c5a9 Mon Sep 17 00:00:00 2001
From: Roman Kisel <romank@linux.microsoft.com>
Date: Wed, 19 Feb 2025 15:04:06 -0800
Subject: [PATCH] Refactor the ACPI GIC case
---
drivers/acpi/irq.c | 14 ++++++-
drivers/pci/controller/pci-hyperv.c | 62 +++++++++++++++++------------
include/linux/acpi.h | 5 ++-
3 files changed, 52 insertions(+), 29 deletions(-)
diff --git a/drivers/acpi/irq.c b/drivers/acpi/irq.c
index 1687483ff319..6243db610137 100644
--- a/drivers/acpi/irq.c
+++ b/drivers/acpi/irq.c
@@ -12,7 +12,7 @@
enum acpi_irq_model_id acpi_irq_model;
-static struct fwnode_handle *(*acpi_get_gsi_domain_id)(u32 gsi);
+static acpi_gsi_domain_disp_fn acpi_get_gsi_domain_id;
static u32 (*acpi_gsi_to_irq_fallback)(u32 gsi);
/**
@@ -307,12 +307,22 @@ EXPORT_SYMBOL_GPL(acpi_irq_get);
* for a given GSI
*/
void __init acpi_set_irq_model(enum acpi_irq_model_id model,
- struct fwnode_handle *(*fn)(u32))
+ acpi_gsi_domain_disp_fn fn)
{
acpi_irq_model = model;
acpi_get_gsi_domain_id = fn;
}
+/**
+ * acpi_get_gsi_dispatcher - Returns dispatcher function that
+ * computes the domain fwnode for a
+ * given GSI.
+ */
+acpi_gsi_domain_disp_fn acpi_get_gsi_dispatcher(void)
+{
+ return acpi_get_gsi_domain_id;
+}
+
/**
* acpi_set_gsi_to_irq_fallback - Register a GSI transfer
* callback to fallback to arch specified implementation.
diff --git a/drivers/pci/controller/pci-hyperv.c
b/drivers/pci/controller/pci-hyperv.c
index 24725bea9ef1..59e670e1cb6e 100644
--- a/drivers/pci/controller/pci-hyperv.c
+++ b/drivers/pci/controller/pci-hyperv.c
@@ -910,16 +910,29 @@ static struct irq_domain
*hv_pci_of_irq_domain_parent(void)
of_node_put(parent);
}
- /*
- * `domain == NULL` shouldn't happen.
- *
- * If somehow the code does end up in that state, treat this as a
configuration
- * issue rather than a hard error, emit a warning, and let the code
proceed.
- * The NULL parent domain is an acceptable option for the
`irq_domain_create_hierarchy`
- * function called later.
- */
+ return domain;
+}
+
+#endif
+
+#ifdef CONFIG_ACPI
+
+static struct irq_domain *hv_pci_acpi_irq_domain_parent(void)
+{
+ struct irq_domain *domain;
+ acpi_gsi_domain_disp_fn gsi_domain_disp_fn;
+
+ if (acpi_irq_model != ACPI_IRQ_MODEL_GIC)
+ return NULL;
+ gsi_domain_disp_fn = acpi_get_gsi_dispatcher();
+ if (!gsi_domain_disp_fn)
+ return NULL;
+ domain = irq_find_matching_fwnode(gsi_domain_disp_fn(0),
+ DOMAIN_BUS_ANY);
+
if (!domain)
- WARN_ONCE(1, "No interrupt-parent found, check the DeviceTree data.\n");
+ return NULL;
+
return domain;
}
@@ -929,6 +942,7 @@ static int hv_pci_irqchip_init(void)
{
static struct hv_pci_chip_data *chip_data;
struct fwnode_handle *fn = NULL;
+ struct irq_domain *irq_domain_parent = NULL;
int ret = -ENOMEM;
chip_data = kzalloc(sizeof(*chip_data), GFP_KERNEL);
@@ -944,29 +958,25 @@ static int hv_pci_irqchip_init(void)
* IRQ domain once enabled, should not be removed since there is no
* way to ensure that all the corresponding devices are also gone and
* no interrupts will be generated.
- *
- * In the ACPI case, the parent IRQ domain is supplied by the ACPI
- * subsystem, and it is the default GSI domain pointing to the GIC.
- * Neither is available outside of the ACPI subsystem, cannot avoid
- * the messy ifdef below.
- * There is apparently no such default in the OF subsystem, and
- * `hv_pci_of_irq_domain_parent` finds the parent IRQ domain that
- * points to the GIC as well.
- * None of these two cases reaches for the MSI parent domain.
*/
#ifdef CONFIG_ACPI
if (!acpi_disabled)
- hv_msi_gic_irq_domain = acpi_irq_create_hierarchy(0, HV_PCI_MSI_SPI_NR,
- fn, &hv_pci_domain_ops,
- chip_data);
+ irq_domain_parent = hv_pci_acpi_irq_domain_parent();
#endif
#if defined(CONFIG_OF)
- if (!hv_msi_gic_irq_domain)
- hv_msi_gic_irq_domain = irq_domain_create_hierarchy(
- hv_pci_of_irq_domain_parent(), 0, HV_PCI_MSI_SPI_NR,
- fn, &hv_pci_domain_ops,
- chip_data);
+ if (!irq_domain_parent)
+ irq_domain_parent = hv_pci_of_irq_domain_parent();
#endif
+ if (!irq_domain_parent) {
+ WARN_ONCE(1, "Invalid firmware configuration for VMBus interrupts\n");
+ ret = -EINVAL;
+ goto free_chip;
+ }
+
+ hv_msi_gic_irq_domain = irq_domain_create_hierarchy(
+ irq_domain_parent, 0, HV_PCI_MSI_SPI_NR,
+ fn, &hv_pci_domain_ops,
+ chip_data);
if (!hv_msi_gic_irq_domain) {
pr_err("Failed to create Hyper-V arm64 vPCI MSI IRQ domain\n");
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 6adcd1b92b20..cd70a72c7073 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -336,8 +336,11 @@ int acpi_register_gsi (struct device *dev, u32 gsi,
int triggering, int polarity
int acpi_gsi_to_irq (u32 gsi, unsigned int *irq);
int acpi_isa_irq_to_gsi (unsigned isa_irq, u32 *gsi);
+typedef struct fwnode_handle *(*acpi_gsi_domain_disp_fn)(u32);
+
void acpi_set_irq_model(enum acpi_irq_model_id model,
- struct fwnode_handle *(*)(u32));
+ acpi_gsi_domain_disp_fn fn);
+acpi_gsi_domain_disp_fn acpi_get_gsi_dispatcher(void);
void acpi_set_gsi_to_irq_fallback(u32 (*)(u32));
struct irq_domain *acpi_irq_create_hierarchy(unsigned int flags,
--
2.43.0
--
Thank you,
Roman
next prev parent reply other threads:[~2025-02-19 23:52 UTC|newest]
Thread overview: 29+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-02-12 1:43 [PATCH hyperv-next v4 0/6] arm64: hyperv: Support Virtual Trust Level Boot Roman Kisel
2025-02-12 1:43 ` [PATCH hyperv-next v4 1/6] arm64: hyperv: Use SMCCC to detect hypervisor presence Roman Kisel
2025-02-12 6:54 ` Arnd Bergmann
2025-02-13 23:23 ` Roman Kisel
2025-02-14 8:05 ` Arnd Bergmann
2025-02-14 16:47 ` Roman Kisel
2025-02-24 23:22 ` Roman Kisel
2025-02-25 7:24 ` Arnd Bergmann
2025-02-25 22:25 ` Roman Kisel
2025-02-26 13:57 ` Arnd Bergmann
2025-02-19 23:13 ` Michael Kelley
2025-02-20 16:34 ` Roman Kisel
2025-02-12 1:43 ` [PATCH hyperv-next v4 2/6] Drivers: hv: Enable VTL mode for arm64 Roman Kisel
2025-02-19 23:14 ` Michael Kelley
2025-02-20 16:36 ` Roman Kisel
2025-02-12 1:43 ` [PATCH hyperv-next v4 3/6] Drivers: hv: Provide arch-neutral implementation of get_vtl() Roman Kisel
2025-02-19 23:17 ` Michael Kelley
2025-02-12 1:43 ` [PATCH hyperv-next v4 4/6] dt-bindings: microsoft,vmbus: Add GIC and DMA coherence to the example Roman Kisel
2025-02-12 6:42 ` Krzysztof Kozlowski
2025-02-12 23:57 ` Roman Kisel
2025-02-13 20:50 ` Roman Kisel
2025-02-12 1:43 ` [PATCH hyperv-next v4 5/6] Drivers: hv: vmbus: Get the IRQ number from DeviceTree Roman Kisel
2025-02-19 23:20 ` Michael Kelley
2025-02-12 1:43 ` [PATCH hyperv-next v4 6/6] PCI: hv: Get vPCI MSI IRQ domain " Roman Kisel
2025-02-12 17:42 ` Bjorn Helgaas
2025-02-18 22:32 ` Roman Kisel
2025-02-19 23:51 ` Roman Kisel [this message]
2025-02-19 23:29 ` Michael Kelley
2025-02-20 16:41 ` Roman Kisel
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=5cd71475-b107-4269-829f-2c492f625aae@linux.microsoft.com \
--to=romank@linux.microsoft.com \
--cc=arnd@arndb.de \
--cc=benhill@microsoft.com \
--cc=bhelgaas@google.com \
--cc=bp@alien8.de \
--cc=bperkins@microsoft.com \
--cc=catalin.marinas@arm.com \
--cc=conor+dt@kernel.org \
--cc=dave.hansen@linux.intel.com \
--cc=decui@microsoft.com \
--cc=devicetree@vger.kernel.org \
--cc=haiyangz@microsoft.com \
--cc=helgaas@kernel.org \
--cc=hpa@zytor.com \
--cc=krzk+dt@kernel.org \
--cc=kw@linux.com \
--cc=kys@microsoft.com \
--cc=lenb@kernel.org \
--cc=linux-acpi@vger.kernel.org \
--cc=linux-arch@vger.kernel.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-hyperv@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=lpieralisi@kernel.org \
--cc=manivannan.sadhasivam@linaro.org \
--cc=mingo@redhat.com \
--cc=rafael@kernel.org \
--cc=robh@kernel.org \
--cc=ssengar@linux.microsoft.com \
--cc=sunilmut@microsoft.com \
--cc=tglx@linutronix.de \
--cc=wei.liu@kernel.org \
--cc=will@kernel.org \
--cc=x86@kernel.org \
/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.