[PATCH hyperv-next v6 11/11] PCI: hv: Get vPCI MSI IRQ domain from DeviceTree

Roman Kisel posted 11 patches 9 months, 1 week ago
There is a newer version of this series
[PATCH hyperv-next v6 11/11] PCI: hv: Get vPCI MSI IRQ domain from DeviceTree
Posted by Roman Kisel 9 months, 1 week ago
The hyperv-pci driver uses ACPI for MSI IRQ domain configuration on
arm64. It won't be able to do that in the VTL mode where only DeviceTree
can be used.

Update the hyperv-pci driver to get vPCI MSI IRQ domain in the DeviceTree
case, too.

Signed-off-by: Roman Kisel <romank@linux.microsoft.com>
---
 drivers/pci/controller/pci-hyperv.c | 73 ++++++++++++++++++++++++++---
 1 file changed, 67 insertions(+), 6 deletions(-)

diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci-hyperv.c
index 6084b38bdda1..cbff19e8a07c 100644
--- a/drivers/pci/controller/pci-hyperv.c
+++ b/drivers/pci/controller/pci-hyperv.c
@@ -50,6 +50,7 @@
 #include <linux/irqdomain.h>
 #include <linux/acpi.h>
 #include <linux/sizes.h>
+#include <linux/of_irq.h>
 #include <asm/mshyperv.h>
 
 /*
@@ -817,9 +818,17 @@ static int hv_pci_vec_irq_gic_domain_alloc(struct irq_domain *domain,
 	int ret;
 
 	fwspec.fwnode = domain->parent->fwnode;
-	fwspec.param_count = 2;
-	fwspec.param[0] = hwirq;
-	fwspec.param[1] = IRQ_TYPE_EDGE_RISING;
+	if (is_of_node(fwspec.fwnode)) {
+		/* SPI lines for OF translations start at offset 32 */
+		fwspec.param_count = 3;
+		fwspec.param[0] = 0;
+		fwspec.param[1] = hwirq - 32;
+		fwspec.param[2] = IRQ_TYPE_EDGE_RISING;
+	} else {
+		fwspec.param_count = 2;
+		fwspec.param[0] = hwirq;
+		fwspec.param[1] = IRQ_TYPE_EDGE_RISING;
+	}
 
 	ret = irq_domain_alloc_irqs_parent(domain, virq, 1, &fwspec);
 	if (ret)
@@ -887,10 +896,47 @@ static const struct irq_domain_ops hv_pci_domain_ops = {
 	.activate = hv_pci_vec_irq_domain_activate,
 };
 
+#ifdef CONFIG_OF
+
+static struct irq_domain *hv_pci_of_irq_domain_parent(void)
+{
+	struct device_node *parent;
+	struct irq_domain *domain;
+
+	parent = of_irq_find_parent(hv_get_vmbus_root_device()->of_node);
+	if (!parent)
+		return NULL;
+	domain = irq_find_host(parent);
+	of_node_put(parent);
+
+	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;
+	return irq_find_matching_fwnode(gsi_domain_disp_fn(0),
+				     DOMAIN_BUS_ANY);
+}
+
+#endif
+
 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);
@@ -907,9 +953,24 @@ static int hv_pci_irqchip_init(void)
 	 * way to ensure that all the corresponding devices are also gone and
 	 * no interrupts will be generated.
 	 */
-	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)
+		irq_domain_parent = hv_pci_acpi_irq_domain_parent();
+#endif
+#if defined(CONFIG_OF)
+	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");
-- 
2.43.0
Re: [PATCH hyperv-next v6 11/11] PCI: hv: Get vPCI MSI IRQ domain from DeviceTree
Posted by Rafael J. Wysocki 8 months, 3 weeks ago
On Sat, Mar 15, 2025 at 1:19 AM Roman Kisel <romank@linux.microsoft.com> wrote:
>
> The hyperv-pci driver uses ACPI for MSI IRQ domain configuration on
> arm64. It won't be able to do that in the VTL mode where only DeviceTree
> can be used.
>
> Update the hyperv-pci driver to get vPCI MSI IRQ domain in the DeviceTree
> case, too.
>
> Signed-off-by: Roman Kisel <romank@linux.microsoft.com>
> ---
>  drivers/pci/controller/pci-hyperv.c | 73 ++++++++++++++++++++++++++---
>  1 file changed, 67 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci-hyperv.c
> index 6084b38bdda1..cbff19e8a07c 100644
> --- a/drivers/pci/controller/pci-hyperv.c
> +++ b/drivers/pci/controller/pci-hyperv.c
> @@ -50,6 +50,7 @@
>  #include <linux/irqdomain.h>
>  #include <linux/acpi.h>
>  #include <linux/sizes.h>
> +#include <linux/of_irq.h>
>  #include <asm/mshyperv.h>
>
>  /*
> @@ -817,9 +818,17 @@ static int hv_pci_vec_irq_gic_domain_alloc(struct irq_domain *domain,
>         int ret;
>
>         fwspec.fwnode = domain->parent->fwnode;
> -       fwspec.param_count = 2;
> -       fwspec.param[0] = hwirq;
> -       fwspec.param[1] = IRQ_TYPE_EDGE_RISING;
> +       if (is_of_node(fwspec.fwnode)) {
> +               /* SPI lines for OF translations start at offset 32 */
> +               fwspec.param_count = 3;
> +               fwspec.param[0] = 0;
> +               fwspec.param[1] = hwirq - 32;
> +               fwspec.param[2] = IRQ_TYPE_EDGE_RISING;
> +       } else {
> +               fwspec.param_count = 2;
> +               fwspec.param[0] = hwirq;
> +               fwspec.param[1] = IRQ_TYPE_EDGE_RISING;
> +       }
>
>         ret = irq_domain_alloc_irqs_parent(domain, virq, 1, &fwspec);
>         if (ret)
> @@ -887,10 +896,47 @@ static const struct irq_domain_ops hv_pci_domain_ops = {
>         .activate = hv_pci_vec_irq_domain_activate,
>  };
>
> +#ifdef CONFIG_OF
> +
> +static struct irq_domain *hv_pci_of_irq_domain_parent(void)
> +{
> +       struct device_node *parent;
> +       struct irq_domain *domain;
> +
> +       parent = of_irq_find_parent(hv_get_vmbus_root_device()->of_node);
> +       if (!parent)
> +               return NULL;
> +       domain = irq_find_host(parent);
> +       of_node_put(parent);
> +
> +       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;
> +       return irq_find_matching_fwnode(gsi_domain_disp_fn(0),
> +                                    DOMAIN_BUS_ANY);
> +}
> +
> +#endif
> +
>  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);
> @@ -907,9 +953,24 @@ static int hv_pci_irqchip_init(void)
>          * way to ensure that all the corresponding devices are also gone and
>          * no interrupts will be generated.
>          */
> -       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)
> +               irq_domain_parent = hv_pci_acpi_irq_domain_parent();
> +#endif
> +#if defined(CONFIG_OF)

Why don't you do

#ifdef CONFIG_OF

here for consistency?

> +       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");
> --
Re: [PATCH hyperv-next v6 11/11] PCI: hv: Get vPCI MSI IRQ domain from DeviceTree
Posted by Roman Kisel 8 months, 3 weeks ago

On 3/26/2025 7:56 AM, Rafael J. Wysocki wrote:
> On Sat, Mar 15, 2025 at 1:19 AM Roman Kisel <romank@linux.microsoft.com> wrote:

[...]

>> -                                                         chip_data);
>> +#ifdef CONFIG_ACPI
>> +       if (!acpi_disabled)
>> +               irq_domain_parent = hv_pci_acpi_irq_domain_parent();
>> +#endif
>> +#if defined(CONFIG_OF)
> 
> Why don't you do
> 
> #ifdef CONFIG_OF
> 
> here for consistency?
> 

Agree, that'd be easier on the eyes :) Will fix in the next version,
thanks for the suggestion!

>> +       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");
>> --

-- 
Thank you,
Roman

Re: [PATCH hyperv-next v6 11/11] PCI: hv: Get vPCI MSI IRQ domain from DeviceTree
Posted by Bjorn Helgaas 9 months, 1 week ago
On Fri, Mar 14, 2025 at 05:19:31PM -0700, Roman Kisel wrote:
> The hyperv-pci driver uses ACPI for MSI IRQ domain configuration on
> arm64. It won't be able to do that in the VTL mode where only DeviceTree
> can be used.
> 
> Update the hyperv-pci driver to get vPCI MSI IRQ domain in the DeviceTree
> case, too.
> 
> Signed-off-by: Roman Kisel <romank@linux.microsoft.com>

Acked-by: Bjorn Helgaas <bhelgaas@google.com>

Looks good to me; trivial whitespace comment below.

> ---
>  drivers/pci/controller/pci-hyperv.c | 73 ++++++++++++++++++++++++++---
>  1 file changed, 67 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci-hyperv.c
> index 6084b38bdda1..cbff19e8a07c 100644
> --- a/drivers/pci/controller/pci-hyperv.c
> +++ b/drivers/pci/controller/pci-hyperv.c
> @@ -50,6 +50,7 @@
>  #include <linux/irqdomain.h>
>  #include <linux/acpi.h>
>  #include <linux/sizes.h>
> +#include <linux/of_irq.h>
>  #include <asm/mshyperv.h>
>  
>  /*
> @@ -817,9 +818,17 @@ static int hv_pci_vec_irq_gic_domain_alloc(struct irq_domain *domain,
>  	int ret;
>  
>  	fwspec.fwnode = domain->parent->fwnode;
> -	fwspec.param_count = 2;
> -	fwspec.param[0] = hwirq;
> -	fwspec.param[1] = IRQ_TYPE_EDGE_RISING;
> +	if (is_of_node(fwspec.fwnode)) {
> +		/* SPI lines for OF translations start at offset 32 */
> +		fwspec.param_count = 3;
> +		fwspec.param[0] = 0;
> +		fwspec.param[1] = hwirq - 32;
> +		fwspec.param[2] = IRQ_TYPE_EDGE_RISING;
> +	} else {
> +		fwspec.param_count = 2;
> +		fwspec.param[0] = hwirq;
> +		fwspec.param[1] = IRQ_TYPE_EDGE_RISING;
> +	}
>  
>  	ret = irq_domain_alloc_irqs_parent(domain, virq, 1, &fwspec);
>  	if (ret)
> @@ -887,10 +896,47 @@ static const struct irq_domain_ops hv_pci_domain_ops = {
>  	.activate = hv_pci_vec_irq_domain_activate,
>  };
>  
> +#ifdef CONFIG_OF
> +
> +static struct irq_domain *hv_pci_of_irq_domain_parent(void)
> +{
> +	struct device_node *parent;
> +	struct irq_domain *domain;
> +
> +	parent = of_irq_find_parent(hv_get_vmbus_root_device()->of_node);
> +	if (!parent)
> +		return NULL;
> +	domain = irq_find_host(parent);
> +	of_node_put(parent);
> +
> +	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;
> +	return irq_find_matching_fwnode(gsi_domain_disp_fn(0),
> +				     DOMAIN_BUS_ANY);
> +}
> +
> +#endif
> +
>  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);
> @@ -907,9 +953,24 @@ static int hv_pci_irqchip_init(void)
>  	 * way to ensure that all the corresponding devices are also gone and
>  	 * no interrupts will be generated.
>  	 */
> -	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)
> +		irq_domain_parent = hv_pci_acpi_irq_domain_parent();
> +#endif
> +#if defined(CONFIG_OF)
> +	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);

This is a different style of indenting the parameters than other
similar cases in this file, which line up parameters on subsequent
lines under the open parenthesis.

>  	if (!hv_msi_gic_irq_domain) {
>  		pr_err("Failed to create Hyper-V arm64 vPCI MSI IRQ domain\n");
> -- 
> 2.43.0
>
Re: [PATCH hyperv-next v6 11/11] PCI: hv: Get vPCI MSI IRQ domain from DeviceTree
Posted by Roman Kisel 9 months ago

On 3/15/2025 11:49 AM, Bjorn Helgaas wrote:
> On Fri, Mar 14, 2025 at 05:19:31PM -0700, Roman Kisel wrote:
>> The hyperv-pci driver uses ACPI for MSI IRQ domain configuration on
>> arm64. It won't be able to do that in the VTL mode where only DeviceTree
>> can be used.
>>
>> Update the hyperv-pci driver to get vPCI MSI IRQ domain in the DeviceTree
>> case, too.
>>
>> Signed-off-by: Roman Kisel <romank@linux.microsoft.com>
> 
> Acked-by: Bjorn Helgaas <bhelgaas@google.com>
> 
> Looks good to me; trivial whitespace comment below.
> 

Thanks a bunch!! I'll fix that the whitespace (and remove the unused
variable the robot noticed).

>> ---
>>   drivers/pci/controller/pci-hyperv.c | 73 ++++++++++++++++++++++++++---
>>   1 file changed, 67 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci-hyperv.c
>> index 6084b38bdda1..cbff19e8a07c 100644
>> --- a/drivers/pci/controller/pci-hyperv.c
>> +++ b/drivers/pci/controller/pci-hyperv.c
>> @@ -50,6 +50,7 @@
>>   #include <linux/irqdomain.h>
>>   #include <linux/acpi.h>
>>   #include <linux/sizes.h>
>> +#include <linux/of_irq.h>
>>   #include <asm/mshyperv.h>
>>   
>>   /*
>> @@ -817,9 +818,17 @@ static int hv_pci_vec_irq_gic_domain_alloc(struct irq_domain *domain,
>>   	int ret;
>>   
>>   	fwspec.fwnode = domain->parent->fwnode;
>> -	fwspec.param_count = 2;
>> -	fwspec.param[0] = hwirq;
>> -	fwspec.param[1] = IRQ_TYPE_EDGE_RISING;
>> +	if (is_of_node(fwspec.fwnode)) {
>> +		/* SPI lines for OF translations start at offset 32 */
>> +		fwspec.param_count = 3;
>> +		fwspec.param[0] = 0;
>> +		fwspec.param[1] = hwirq - 32;
>> +		fwspec.param[2] = IRQ_TYPE_EDGE_RISING;
>> +	} else {
>> +		fwspec.param_count = 2;
>> +		fwspec.param[0] = hwirq;
>> +		fwspec.param[1] = IRQ_TYPE_EDGE_RISING;
>> +	}
>>   
>>   	ret = irq_domain_alloc_irqs_parent(domain, virq, 1, &fwspec);
>>   	if (ret)
>> @@ -887,10 +896,47 @@ static const struct irq_domain_ops hv_pci_domain_ops = {
>>   	.activate = hv_pci_vec_irq_domain_activate,
>>   };
>>   
>> +#ifdef CONFIG_OF
>> +
>> +static struct irq_domain *hv_pci_of_irq_domain_parent(void)
>> +{
>> +	struct device_node *parent;
>> +	struct irq_domain *domain;
>> +
>> +	parent = of_irq_find_parent(hv_get_vmbus_root_device()->of_node);
>> +	if (!parent)
>> +		return NULL;
>> +	domain = irq_find_host(parent);
>> +	of_node_put(parent);
>> +
>> +	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;
>> +	return irq_find_matching_fwnode(gsi_domain_disp_fn(0),
>> +				     DOMAIN_BUS_ANY);
>> +}
>> +
>> +#endif
>> +
>>   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);
>> @@ -907,9 +953,24 @@ static int hv_pci_irqchip_init(void)
>>   	 * way to ensure that all the corresponding devices are also gone and
>>   	 * no interrupts will be generated.
>>   	 */
>> -	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)
>> +		irq_domain_parent = hv_pci_acpi_irq_domain_parent();
>> +#endif
>> +#if defined(CONFIG_OF)
>> +	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);
> 
> This is a different style of indenting the parameters than other
> similar cases in this file, which line up parameters on subsequent
> lines under the open parenthesis.
> 
>>   	if (!hv_msi_gic_irq_domain) {
>>   		pr_err("Failed to create Hyper-V arm64 vPCI MSI IRQ domain\n");
>> -- 
>> 2.43.0
>>

-- 
Thank you,
Roman