[PATCH] Hyper-V: pci: x64: Generalize irq/msi set-up and handling

[PATCH] Hyper-V: pci: x64: Generalize irq/msi set-up and handling

[Sunil Muthuswamy]

Currently, operations related to irq/msi in Hyper-V vPCI are
x86-specific code. In order to support virtual PCI on Hyper-V for
other architectures, introduce generic interfaces to replace the
x86-specific ones. There are no functional changes in this patch.

Signed-off-by: Sunil Muthuswamy <sunilmut@microsoft.com>
Signed-off-by: Boqun Feng (Microsoft) <boqun.feng@gmail.com>
---
 arch/x86/include/asm/mshyperv.h     | 24 +++++++++++++++++++++
 drivers/pci/controller/pci-hyperv.c | 33 +++++++++++++++++------------
 2 files changed, 44 insertions(+), 13 deletions(-)

diff --git a/arch/x86/include/asm/mshyperv.h b/arch/x86/include/asm/mshyperv.h
index ffc289992d1b..05b32ef57e34 100644
--- a/arch/x86/include/asm/mshyperv.h
+++ b/arch/x86/include/asm/mshyperv.h
@@ -245,6 +245,30 @@ bool hv_vcpu_is_preempted(int vcpu);
 static inline void hv_apic_init(void) {}
 #endif
 
+#define hv_msi_handler		handle_edge_irq
+#define hv_msi_handler_name	"edge"
+#define hv_msi_prepare		pci_msi_prepare
+
+/* Returns the Hyper-V PCI parent MSI vector domain. */
+static inline struct irq_domain *hv_msi_parent_vector_domain(void)
+{
+	return x86_vector_domain;
+}
+
+/* Returns the interrupt vector mapped to the given IRQ. */
+static inline unsigned int hv_msi_get_int_vector(struct irq_data *data)
+{
+	struct irq_cfg *cfg = irqd_cfg(data);
+
+	return cfg->vector;
+}
+
+/* Get the IRQ delivery mode. */
+static inline u8 hv_msi_irq_delivery_mode(void)
+{
+	return APIC_DELIVERY_MODE_FIXED;
+}
+
 static inline void hv_set_msi_entry_from_desc(union hv_msi_entry *msi_entry,
 					      struct msi_desc *msi_desc)
 {
diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci-hyperv.c
index 6db8d96a78eb..9ca740d275d7 100644
--- a/drivers/pci/controller/pci-hyperv.c
+++ b/drivers/pci/controller/pci-hyperv.c
@@ -43,12 +43,11 @@
 #include <linux/delay.h>
 #include <linux/semaphore.h>
 #include <linux/irqdomain.h>
-#include <asm/irqdomain.h>
-#include <asm/apic.h>
 #include <linux/irq.h>
 #include <linux/msi.h>
 #include <linux/hyperv.h>
 #include <linux/refcount.h>
+#include <linux/pci.h>
 #include <asm/mshyperv.h>
 
 /*
@@ -1194,7 +1193,6 @@ static void hv_irq_mask(struct irq_data *data)
 static void hv_irq_unmask(struct irq_data *data)
 {
 	struct msi_desc *msi_desc = irq_data_get_msi_desc(data);
-	struct irq_cfg *cfg = irqd_cfg(data);
 	struct hv_retarget_device_interrupt *params;
 	struct hv_pcibus_device *hbus;
 	struct cpumask *dest;
@@ -1223,7 +1221,7 @@ static void hv_irq_unmask(struct irq_data *data)
 			   (hbus->hdev->dev_instance.b[7] << 8) |
 			   (hbus->hdev->dev_instance.b[6] & 0xf8) |
 			   PCI_FUNC(pdev->devfn);
-	params->int_target.vector = cfg->vector;
+	params->int_target.vector = hv_msi_get_int_vector(data);
 
 	/*
 	 * Honoring apic->delivery_mode set to APIC_DELIVERY_MODE_FIXED by
@@ -1324,7 +1322,7 @@ static u32 hv_compose_msi_req_v1(
 	int_pkt->wslot.slot = slot;
 	int_pkt->int_desc.vector = vector;
 	int_pkt->int_desc.vector_count = 1;
-	int_pkt->int_desc.delivery_mode = APIC_DELIVERY_MODE_FIXED;
+	int_pkt->int_desc.delivery_mode = hv_msi_irq_delivery_mode();
 
 	/*
 	 * Create MSI w/ dummy vCPU set, overwritten by subsequent retarget in
@@ -1345,7 +1343,7 @@ static u32 hv_compose_msi_req_v2(
 	int_pkt->wslot.slot = slot;
 	int_pkt->int_desc.vector = vector;
 	int_pkt->int_desc.vector_count = 1;
-	int_pkt->int_desc.delivery_mode = APIC_DELIVERY_MODE_FIXED;
+	int_pkt->int_desc.delivery_mode = hv_msi_irq_delivery_mode();
 
 	/*
 	 * Create MSI w/ dummy vCPU set targeting just one vCPU, overwritten
@@ -1372,7 +1370,6 @@ static u32 hv_compose_msi_req_v2(
  */
 static void hv_compose_msi_msg(struct irq_data *data, struct msi_msg *msg)
 {
-	struct irq_cfg *cfg = irqd_cfg(data);
 	struct hv_pcibus_device *hbus;
 	struct vmbus_channel *channel;
 	struct hv_pci_dev *hpdev;
@@ -1422,7 +1419,7 @@ static void hv_compose_msi_msg(struct irq_data *data, struct msi_msg *msg)
 		size = hv_compose_msi_req_v1(&ctxt.int_pkts.v1,
 					dest,
 					hpdev->desc.win_slot.slot,
-					cfg->vector);
+					hv_msi_get_int_vector(data));
 		break;
 
 	case PCI_PROTOCOL_VERSION_1_2:
@@ -1430,7 +1427,7 @@ static void hv_compose_msi_msg(struct irq_data *data, struct msi_msg *msg)
 		size = hv_compose_msi_req_v2(&ctxt.int_pkts.v2,
 					dest,
 					hpdev->desc.win_slot.slot,
-					cfg->vector);
+					hv_msi_get_int_vector(data));
 		break;
 
 	default:
@@ -1541,12 +1538,13 @@ static struct irq_chip hv_msi_irq_chip = {
 	.irq_compose_msi_msg	= hv_compose_msi_msg,
 	.irq_set_affinity	= hv_set_affinity,
 	.irq_ack		= irq_chip_ack_parent,
+	.irq_eoi		= irq_chip_eoi_parent,
 	.irq_mask		= hv_irq_mask,
 	.irq_unmask		= hv_irq_unmask,
 };
 
 static struct msi_domain_ops hv_msi_ops = {
-	.msi_prepare	= pci_msi_prepare,
+	.msi_prepare	= hv_msi_prepare,
 	.msi_free	= hv_msi_free,
 };
 
@@ -1565,17 +1563,26 @@ static struct msi_domain_ops hv_msi_ops = {
  */
 static int hv_pcie_init_irq_domain(struct hv_pcibus_device *hbus)
 {
+	struct irq_domain *parent_domain;
+
+	parent_domain = hv_msi_parent_vector_domain();
+	if (!parent_domain) {
+		dev_err(&hbus->hdev->device,
+			"Failed to get parent MSI domain\n");
+		return -ENODEV;
+	}
+
 	hbus->msi_info.chip = &hv_msi_irq_chip;
 	hbus->msi_info.ops = &hv_msi_ops;
 	hbus->msi_info.flags = (MSI_FLAG_USE_DEF_DOM_OPS |
 		MSI_FLAG_USE_DEF_CHIP_OPS | MSI_FLAG_MULTI_PCI_MSI |
 		MSI_FLAG_PCI_MSIX);
-	hbus->msi_info.handler = handle_edge_irq;
-	hbus->msi_info.handler_name = "edge";
+	hbus->msi_info.handler = hv_msi_handler;
+	hbus->msi_info.handler_name = hv_msi_handler_name;
 	hbus->msi_info.data = hbus;
 	hbus->irq_domain = pci_msi_create_irq_domain(hbus->sysdata.fwnode,
 						     &hbus->msi_info,
-						     x86_vector_domain);
+						     parent_domain);
 	if (!hbus->irq_domain) {
 		dev_err(&hbus->hdev->device,
 			"Failed to build an MSI IRQ domain\n");
-- 
2.17.1

Re: [PATCH] Hyper-V: pci: x64: Generalize irq/msi set-up and handling

[Borislav Petkov]

On Thu, Jan 07, 2021 at 05:05:36AM +0000, Sunil Muthuswamy wrote:
> Currently, operations related to irq/msi in Hyper-V vPCI are
> x86-specific code. In order to support virtual PCI on Hyper-V for
> other architectures, introduce generic interfaces to replace the
> x86-specific ones. There are no functional changes in this patch.
> 
> Signed-off-by: Sunil Muthuswamy <sunilmut@microsoft.com>
> Signed-off-by: Boqun Feng (Microsoft) <boqun.feng@gmail.com>

What is this SoB chain supposed to say?

See:

  11) Sign your work - the Developer's Certificate of Origin
  12) When to use Acked-by:, Cc:, and Co-developed-by:
  13) Using Reported-by:, Tested-by:, Reviewed-by:, Suggested-by: and Fixes:

sections in

https://www.kernel.org/doc/html/latest/process/submitting-patches.html

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCH] Hyper-V: pci: x64: Generalize irq/msi set-up and handling

[Bjorn Helgaas]

There seems to be a long tradition of dreaming up random formats for
the subject lines of Hyper-V-related patches.  Look at all the
different ways these are spelled, hyphenated, and capitalized:

  $ git log --oneline arch/x86/include/asm/mshyperv.h
  626b901f6044 ("Drivers: hv: vmbus: Add parsing of VMbus interrupt in ACPI DSDT")
  b9d8cf2eb3ce ("x86/hyperv: Make hv_setup_sched_clock inline")
  a16be368dd3f ("x86/entry: Convert various hypervisor vectors to IDTENTRY_SYSVEC")
  2ddddd0b4e89 ("Drivers: hv: Move AEOI determination to architecture dependent code")
  1cf106d93245 ("PCI: hv: Introduce hv_msi_entry")
  b95a8a27c300 ("x86/vdso: Use generic VDSO clock mode storage")
  eec399dd8627 ("x86/vdso: Move VDSO clocksource state tracking to callback")
  fa36dcdf8b20 ("x86: hv: Add function to allocate zeroed page for Hyper-V")
  8c3e44bde7fd ("x86/hyperv: Add functions to allocate/deallocate page for Hyper-V")
  765e33f5211a ("Drivers: hv: vmbus: Break out ISA independent parts of mshyperv.h")
  dd2cb348613b ("clocksource/drivers: Continue making Hyper-V clocksource ISA agnostic")
  cc4edae4b924 ("x86/hyper-v: Add HvFlushGuestAddressList hypercall support")
  b42967dcac1d ("x86/hyper-v: Fix indentation in hv_do_fast_hypercall16()")
  3a025de64bf8 ("x86/hyperv: Enable PV qspinlock for Hyper-V")
  eb914cfe72f4 ("X86/Hyper-V: Add flush HvFlushGuestPhysicalAddressSpace hypercall support")
  ...

On Thu, Jan 07, 2021 at 05:05:36AM +0000, Sunil Muthuswamy wrote:
> Currently, operations related to irq/msi in Hyper-V vPCI are

In comments in the patch, you use "IRQ" and "MSI".  I don't know
whether "vPCI" means something or is a typo.  I suppose it probably
means "virtual PCI" as below.

> x86-specific code. In order to support virtual PCI on Hyper-V for
> other architectures, introduce generic interfaces to replace the
> x86-specific ones. There are no functional changes in this patch.
>
> Signed-off-by: Sunil Muthuswamy <sunilmut@microsoft.com>
> Signed-off-by: Boqun Feng (Microsoft) <boqun.feng@gmail.com>
> ...

RE: [EXTERNAL] Re: [PATCH] Hyper-V: pci: x64: Generalize irq/msi set-up and handling

[Sunil Muthuswamy]

> What is this SoB chain supposed to say?

Quoting from the link you shared:
"The Signed-off-by: tag indicates that the signer was involved in the development of
the patch, or that he/she was in the patch's delivery path."

My intent to include Boqun in the Signed-off by tag was to indicate that he was involved
in the development of the patch here.

- Sunil

RE: [EXTERNAL] Re: [PATCH] Hyper-V: pci: x64: Generalize irq/msi set-up and handling

[Sunil Muthuswamy]

> There seems to be a long tradition of dreaming up random formats for
> the subject lines of Hyper-V-related patches.  Look at all the
> different ways these are spelled, hyphenated, and capitalized:
> 
I am reading this as a suggestion to unify the format of the subject of
the Hyper-V patches. Wei and other maintainers of the Hyper-V branch; do
you have any suggestions? If we have already defined a format and it is me
who is not following it, please forward the document my way.

> On Thu, Jan 07, 2021 at 05:05:36AM +0000, Sunil Muthuswamy wrote:
> > Currently, operations related to irq/msi in Hyper-V vPCI are
> 
> In comments in the patch, you use "IRQ" and "MSI".  I don't know
> whether "vPCI" means something or is a typo.  I suppose it probably
> means "virtual PCI" as below.
> 

Yes, vPCI here means virtual PCI.

Re: [EXTERNAL] Re: [PATCH] Hyper-V: pci: x64: Generalize irq/msi set-up and handling

[Borislav Petkov]

On Thu, Jan 07, 2021 at 08:16:28PM +0000, Sunil Muthuswamy wrote:
> > What is this SoB chain supposed to say?
> 
> Quoting from the link you shared:
> "The Signed-off-by: tag indicates that the signer was involved in the development of
> the patch, or that he/she was in the patch's delivery path."
> 
> My intent to include Boqun in the Signed-off by tag was to indicate that he was involved
> in the development of the patch here.

Do you see "Co-developed-by" in the title of that section? This is how
you express co-authorship.

As it is now:

Signed-off-by: Sunil Muthuswamy <sunilmut@microsoft.com>
Signed-off-by: Boqun Feng (Microsoft) <boqun.feng@gmail.com>

it says that you authored the patch and Boqun handled it further, i.e.,
he's in the patch's delivery path. But he isn't - you're sending it.

HTH.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

RE: [EXTERNAL] Re: [PATCH] Hyper-V: pci: x64: Generalize irq/msi set-up and handling

[Sunil Muthuswamy]

> Do you see "Co-developed-by" in the title of that section? This is how
> you express co-authorship.
Yes, I do now.

> 
> As it is now:
> 
> Signed-off-by: Sunil Muthuswamy <sunilmut@microsoft.com>
> Signed-off-by: Boqun Feng (Microsoft) <boqun.feng@gmail.com>
> 
> it says that you authored the patch and Boqun handled it further, i.e.,
> he's in the patch's delivery path. But he isn't - you're sending it.

thanks, I will send out a new patch with this updated.