| Message ID | SN4PR2101MB08808404302E2E1AB6A27DD6C0AF9@SN4PR2101MB0880.namprd21.prod.outlook.com |
|---|---|
| State | New |
| Headers | show |
| Series | Hyper-V: pci: x64: Generalize irq/msi set-up and handling | expand |
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
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>
> ...
> 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
> 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.
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.
> 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.
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");