* [PATCH V1 0/3] PCI passthru on Hyper-V (Part II)
From: Mukesh R @ 2026-05-12 2:12 UTC (permalink / raw)
To: hpa, robin.murphy, robh, linux-hyperv, linux-kernel, iommu,
linux-pci, linux-arch
This patch series implements interrupt remapping part of the PCI
passthru feature on Hyper-V when Linux is running as a privileged VM.
These patches complement Part I of the feature at:
https://lore.kernel.org/linux-hyperv/20260512020259.1678627-1-mrathor@linux.microsoft.com/T/#t
Testing and other details are listed there.
Changes in V1:
o rebase to above V3 of Part I
o check for NULL irqdata->parent_data->chip before calling
irq_chip_unmask_parent().
Thanks,
-Mukesh
Mukesh R (3):
mshv: Import declarations for irq remap and add irqbypass support
hyperv: Implement irq remap for passthru devices
mshv: Implement guest irq migration for passthru devices
arch/x86/hyperv/irqdomain.c | 18 +-
drivers/hv/Kconfig | 1 +
drivers/hv/mshv_eventfd.c | 501 +++++++++++++++++++++++++++-
drivers/hv/mshv_eventfd.h | 3 +
drivers/iommu/hyperv-iommu-root.c | 14 +
drivers/pci/controller/pci-hyperv.c | 10 +
include/asm-generic/mshyperv.h | 3 +
include/hyperv/hvgdk_mini.h | 3 +
include/hyperv/hvhdk.h | 17 +
9 files changed, 564 insertions(+), 6 deletions(-)
--
2.51.2.vfs.0.1
^ permalink raw reply
* [PATCH V1 1/3] mshv: Import declarations for irq remap and add irqbypass support
From: Mukesh R @ 2026-05-12 2:12 UTC (permalink / raw)
To: hpa, robin.murphy, robh, linux-hyperv, linux-kernel, iommu,
linux-pci, linux-arch
In-Reply-To: <20260512021242.1679786-1-mrathor@linux.microsoft.com>
For the irq map/remap hypercalls, copy relevant data structures from
hypervisor public headers into Linux equivalents. Also, update Kconfig and
mshv_irqfd for irqbypass. Please note, irqbypass is required for doing
passthru on MSHV. This because there is really no way of knowing the linux
irq in the mshv_irqfd_assign and mshv_irqfd_update paths without it. The
linux irq is setup upfront by VFIO before irqfd assign/update happens.
Signed-off-by: Mukesh R <mrathor@linux.microsoft.com>
---
drivers/hv/Kconfig | 1 +
drivers/hv/mshv_eventfd.h | 3 +++
include/hyperv/hvgdk_mini.h | 3 +++
include/hyperv/hvhdk.h | 17 +++++++++++++++++
4 files changed, 24 insertions(+)
diff --git a/drivers/hv/Kconfig b/drivers/hv/Kconfig
index 7937ac0cbd0f..c831fe25ca2b 100644
--- a/drivers/hv/Kconfig
+++ b/drivers/hv/Kconfig
@@ -75,6 +75,7 @@ config MSHV_ROOT
# no particular order, making it impossible to reassemble larger pages
depends on PAGE_SIZE_4KB
select EVENTFD
+ select IRQ_BYPASS_MANAGER
select VIRT_XFER_TO_GUEST_WORK
select HMM_MIRROR
select MMU_NOTIFIER
diff --git a/drivers/hv/mshv_eventfd.h b/drivers/hv/mshv_eventfd.h
index 464c6b81ab33..ff4dd24b8ad4 100644
--- a/drivers/hv/mshv_eventfd.h
+++ b/drivers/hv/mshv_eventfd.h
@@ -9,6 +9,7 @@
#define __LINUX_MSHV_EVENTFD_H
#include <linux/poll.h>
+#include <linux/irqbypass.h>
#include "mshv.h"
#include "mshv_root.h"
@@ -37,6 +38,8 @@ struct mshv_irqfd {
struct mshv_irqfd_resampler *irqfd_resampler;
struct eventfd_ctx *irqfd_resamplefd;
struct hlist_node irqfd_resampler_hnode;
+ struct irq_bypass_consumer irqfd_bypass_cons;
+ struct irq_bypass_producer *irqfd_bypass_prod;
};
void mshv_eventfd_init(struct mshv_partition *partition);
diff --git a/include/hyperv/hvgdk_mini.h b/include/hyperv/hvgdk_mini.h
index da622fb06440..1ef480825705 100644
--- a/include/hyperv/hvgdk_mini.h
+++ b/include/hyperv/hvgdk_mini.h
@@ -59,6 +59,8 @@ struct hv_u128 {
#define HV_PARTITION_ID_INVALID ((u64)0)
#define HV_PARTITION_ID_SELF ((u64)-1)
+#define HV_MAX_VPS 256 /* HV_MAXIMUM_PROCESSORS */
+
/* Hyper-V specific model specific registers (MSRs) */
#if defined(CONFIG_X86)
@@ -508,6 +510,7 @@ union hv_vp_assist_msr_contents { /* HV_REGISTER_VP_ASSIST_PAGE */
#define HVCALL_UNMAP_VP_STATE_PAGE 0x00e2
#define HVCALL_GET_VP_STATE 0x00e3
#define HVCALL_SET_VP_STATE 0x00e4
+#define HVCALL_GET_VPSET_FROM_MDA 0x00e5
#define HVCALL_GET_VP_CPUID_VALUES 0x00f4
#define HVCALL_GET_PARTITION_PROPERTY_EX 0x0101
#define HVCALL_MMIO_READ 0x0106
diff --git a/include/hyperv/hvhdk.h b/include/hyperv/hvhdk.h
index 5e83d3714966..d0a892347ab1 100644
--- a/include/hyperv/hvhdk.h
+++ b/include/hyperv/hvhdk.h
@@ -952,4 +952,21 @@ struct hv_input_modify_sparse_spa_page_host_access {
#define HV_MODIFY_SPA_PAGE_HOST_ACCESS_LARGE_PAGE 0x4
#define HV_MODIFY_SPA_PAGE_HOST_ACCESS_HUGE_PAGE 0x8
+#ifdef CONFIG_X86
+
+struct hv_input_get_vp_set_from_mda { /* HV_OUTPUT_GET_VP_SET_FROM_MDA */
+ u64 target_partid;
+ u64 dest_address;
+ u8 input_vtl;
+ u8 destmode_logical; /* true => mode is logical */
+ u16 reserved0; /* mbz */
+ u32 reserved1; /* mbz */
+} __packed;
+
+union hv_output_get_vp_set_from_mda { /* HV_OUTPUT_GET_VP_SET_FROM_MDA */
+ struct hv_vpset target_vpset;
+ u64 bitset_buffer[HV_GENERIC_SET_QWORD_COUNT(HV_MAX_VPS)];
+} __packed;
+
+#endif /* CONFIG_X86 */
#endif /* _HV_HVHDK_H */
--
2.51.2.vfs.0.1
^ permalink raw reply related
* [PATCH V1 2/3] hyperv: Implement irq remap for passthru devices
From: Mukesh R @ 2026-05-12 2:12 UTC (permalink / raw)
To: hpa, robin.murphy, robh, linux-hyperv, linux-kernel, iommu,
linux-pci, linux-arch
In-Reply-To: <20260512021242.1679786-1-mrathor@linux.microsoft.com>
Implement interrupt remapping for direct attached and domain attached
devices on Hyper-V.
Please note there are few constraints when it comes to mapping device
interrupts on Hyper-V. For example, the hypervisor will not allow mapping
device interrupts to root if the device is a direct attached device. Since
the target guest cpu and vector info is not available during the initial
VFIO irq setup, we work around by skipping this initial map. Then later
during irqbypass trigger, when both guest target cpu vector are available,
we do the map in the hypervisor, update the device, and enable the
interrupt vector on the device. Rather than special case direct attached,
we do same for domain attached also. This implies irqbypass is required
for MSHV pci device passthru. Also noteworthy is that the hypervisor
will automatically setup any direct hw injection like posted interrupts.
Signed-off-by: Mukesh R <mrathor@linux.microsoft.com>
---
arch/x86/hyperv/irqdomain.c | 18 +-
drivers/hv/mshv_eventfd.c | 423 +++++++++++++++++++++++++++-
drivers/iommu/hyperv-iommu-root.c | 14 +
drivers/pci/controller/pci-hyperv.c | 10 +
include/asm-generic/mshyperv.h | 3 +
5 files changed, 464 insertions(+), 4 deletions(-)
diff --git a/arch/x86/hyperv/irqdomain.c b/arch/x86/hyperv/irqdomain.c
index 8780573a4332..02f9a889c014 100644
--- a/arch/x86/hyperv/irqdomain.c
+++ b/arch/x86/hyperv/irqdomain.c
@@ -197,7 +197,7 @@ int hv_map_msi_interrupt(struct irq_data *data,
msidesc = irq_data_get_msi_desc(data);
pdev = msi_desc_to_pci_dev(msidesc);
- hv_devid.as_uint64 = hv_build_devid_type_pci(pdev);
+ hv_devid.as_uint64 = hv_devid_from_pdev(pdev);
cpu = cpumask_first(irq_data_get_effective_affinity_mask(data));
return hv_map_interrupt(hv_devid, false, cpu, cfg->vector,
@@ -233,6 +233,20 @@ static void hv_irq_compose_msi_msg(struct irq_data *data, struct msi_msg *msg)
return;
}
+ /*
+ * For direct attached devices, we cannot map interrupts in the
+ * hypervisor because it will not allow it until we have guest target
+ * vcpu and vector. So defer it until irqbypass. Also, do the same
+ * for domain attached devices for simplicity.
+ */
+ if (hv_pcidev_is_pthru_dev(pdev)) {
+ if (data->chip_data)
+ entry_to_msi_msg(data->chip_data, msg);
+ else
+ memset(msg, 0, sizeof(struct msi_msg));
+ return;
+ }
+
if (data->chip_data) {
/*
* This interrupt is already mapped. Let's unmap first.
@@ -272,7 +286,7 @@ static int hv_unmap_msi_interrupt(struct pci_dev *pdev,
{
union hv_device_id hv_devid;
- hv_devid.as_uint64 = hv_build_devid_type_pci(pdev);
+ hv_devid.as_uint64 = hv_devid_from_pdev(pdev);
return hv_unmap_interrupt(hv_devid.as_uint64, irq_entry);
}
diff --git a/drivers/hv/mshv_eventfd.c b/drivers/hv/mshv_eventfd.c
index 90959f639dc3..1f5c1e9ee9b7 100644
--- a/drivers/hv/mshv_eventfd.c
+++ b/drivers/hv/mshv_eventfd.c
@@ -7,7 +7,6 @@
*
* All credits to kvm developers.
*/
-
#include <linux/syscalls.h>
#include <linux/wait.h>
#include <linux/poll.h>
@@ -15,7 +14,8 @@
#include <linux/list.h>
#include <linux/workqueue.h>
#include <linux/eventfd.h>
-
+#include <linux/pci.h>
+#include <linux/vfio_pci_core.h>
#if IS_ENABLED(CONFIG_X86_64)
#include <asm/apic.h>
#endif
@@ -27,6 +27,377 @@
static struct workqueue_struct *irqfd_cleanup_wq;
+#if IS_ENABLED(CONFIG_X86_64)
+
+static int mshv_parse_mshv_irqfd(struct mshv_irqfd *irqfd,
+ struct pci_dev **out_pdev,
+ struct irq_data **out_irqdata)
+{
+ struct irq_bypass_producer *prod;
+ struct msi_desc *msidesc;
+ struct irq_data *irqdata;
+
+ if (irqfd == NULL || irqfd->irqfd_bypass_prod == NULL)
+ return -ENODEV;
+
+ prod = irqfd->irqfd_bypass_prod;
+
+ irqdata = irq_get_irq_data(prod->irq);
+ if (irqdata == NULL) {
+ pr_err("Hyper-V: irqbypass fail, no irqdata. irq:0x%x\n",
+ prod->irq);
+ return -EINVAL;
+ }
+ *out_irqdata = irqdata;
+
+ msidesc = irq_data_get_msi_desc(irqdata);
+ if (msidesc == NULL) {
+ pr_err("Hyper-V: irqbypass msi fail. irq:0x%x\n", prod->irq);
+ return -EINVAL;
+ }
+
+ *out_pdev = msi_desc_to_pci_dev(msidesc);
+ if (*out_pdev == NULL) {
+ pr_err("Hyper-V: mshv_irqfd parse fail. irq:0x%x\n", prod->irq);
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+/* Must be called with interrupts disabled */
+static int hv_vpset_from_hyp_disabled(
+ struct hv_input_get_vp_set_from_mda *input,
+ union hv_output_get_vp_set_from_mda *output,
+ struct mshv_lapic_irq *lapic_irq, u64 partid)
+{
+ u64 status;
+
+ memset(input, 0, sizeof(*input));
+ input->target_partid = partid;
+ input->dest_address = lapic_irq->lapic_apic_id;
+ input->input_vtl = 0;
+ input->destmode_logical = lapic_irq->lapic_control.logical_dest_mode;
+
+ status = hv_do_hypercall(HVCALL_GET_VPSET_FROM_MDA, input, output);
+ if (!hv_result_success(status)) {
+ hv_status_err(status, "apicid:0x%llx dest:0x%x\n",
+ lapic_irq->lapic_apic_id,
+ lapic_irq->lapic_control.logical_dest_mode);
+ }
+
+ return hv_result_to_errno(status);
+}
+
+/* Returns number of banks copied, -errno in case of error */
+static int hv_copy_vpset(struct hv_vpset *dest, struct hv_vpset *src)
+{
+ u64 bank_mask;
+ int banks, tot_banks = hv_max_vp_index / HV_VCPUS_PER_SPARSE_BANK;
+
+ if (tot_banks >= HV_MAX_SPARSE_VCPU_BANKS)
+ return -EINVAL;
+
+ dest->format = src->format;
+ dest->valid_bank_mask = src->valid_bank_mask;
+ bank_mask = src->valid_bank_mask;
+ for (banks = 0; banks <= tot_banks; banks++) {
+ if (bank_mask == 0)
+ break;
+
+ if (bank_mask & 1)
+ dest->bank_contents[banks] = src->bank_contents[banks];
+ bank_mask = bank_mask >> 1;
+ }
+
+ return banks;
+}
+
+static int mshv_map_device_interrupt(u64 ptid, union hv_device_id hv_devid,
+ struct mshv_lapic_irq *ginfo,
+ struct hv_interrupt_entry *ret_entry,
+ u64 *ret_status)
+{
+ struct hv_input_map_device_interrupt *irq_input;
+ struct hv_output_map_device_interrupt *irq_output;
+ struct hv_device_interrupt_descriptor *intdesc;
+ struct hv_input_get_vp_set_from_mda *mda_input;
+ union hv_output_get_vp_set_from_mda *mda_output;
+ ulong flags;
+ u64 status;
+ int rc, var_size;
+
+ *ret_status = U64_MAX;
+ local_irq_save(flags);
+
+ mda_input = *this_cpu_ptr(hyperv_pcpu_input_arg);
+ mda_output = *this_cpu_ptr(hyperv_pcpu_output_arg);
+
+ /*
+ * Map Device Interrupt hcall needs vp set based on vp indexes used
+ * during vp creation. Here we have lapic-id of the vp only. Easiest
+ * is to just ask the hypervisor for the vp set matching the lapic-id.
+ */
+ rc = hv_vpset_from_hyp_disabled(mda_input, mda_output, ginfo, ptid);
+ if (rc)
+ goto out; /* error already printed */
+
+ irq_input = *this_cpu_ptr(hyperv_pcpu_input_arg);
+ irq_output = *this_cpu_ptr(hyperv_pcpu_output_arg);
+ memset(irq_input, 0, sizeof(*irq_input));
+
+ irq_input->partition_id = ptid;
+ irq_input->device_id = hv_devid.as_uint64;
+
+ intdesc = &irq_input->interrupt_descriptor;
+ intdesc->interrupt_type = HV_X64_INTERRUPT_TYPE_FIXED;
+ intdesc->vector_count = 1;
+ intdesc->target.vector = ginfo->lapic_vector;
+ intdesc->trigger_mode = HV_INTERRUPT_TRIGGER_MODE_EDGE;
+
+ intdesc->target.vp_set.valid_bank_mask = 0;
+ intdesc->target.vp_set.format = HV_GENERIC_SET_SPARSE_4K;
+ intdesc->target.flags = HV_DEVICE_INTERRUPT_TARGET_PROCESSOR_SET;
+ rc = hv_copy_vpset(&intdesc->target.vp_set, &mda_output->target_vpset);
+ if (rc <= 0) {
+ pr_err("Hyper-V: ptid %lld - (irq)vpset copy failed (%d)\n",
+ ptid, rc);
+ goto out;
+ }
+
+ /*
+ * var-sized hcall: var-size starts after vp_mask (thus vp_set.format
+ * does not count, but vp_set.valid_bank_mask does).
+ */
+ var_size = rc + 1;
+ status = hv_do_rep_hypercall(HVCALL_MAP_DEVICE_INTERRUPT, 0, var_size,
+ irq_input, irq_output);
+ *ret_entry = irq_output->interrupt_entry;
+ local_irq_restore(flags);
+
+ rc = 0;
+ if (!hv_result_success(status)) {
+ if (hv_result(status) != HV_STATUS_INSUFFICIENT_MEMORY)
+ hv_status_err(status, "pt:%lld vec:%d lapic-id:%lld\n",
+ ptid, ginfo->lapic_vector, ginfo->lapic_apic_id);
+ *ret_status = status;
+ rc = hv_result_to_errno(status);
+ }
+
+ return rc;
+
+out:
+ local_irq_restore(flags);
+ return rc;
+
+}
+
+static int mshv_unmap_device_interrupt(union hv_device_id hv_devid,
+ struct hv_interrupt_entry *irq_entry)
+{
+ unsigned long flags;
+ struct hv_input_unmap_device_interrupt *input;
+ u64 status;
+
+ local_irq_save(flags);
+ input = *this_cpu_ptr(hyperv_pcpu_input_arg);
+ memset(input, 0, sizeof(*input));
+
+ if (hv_devid.device_type == HV_DEVICE_TYPE_LOGICAL)
+ input->partition_id = hv_get_current_partid();
+ else
+ input->partition_id = hv_current_partition_id;
+
+ input->device_id = hv_devid.as_uint64;
+ input->interrupt_entry = *irq_entry;
+
+ status = hv_do_hypercall(HVCALL_UNMAP_DEVICE_INTERRUPT, input, NULL);
+ local_irq_restore(flags);
+
+ if (!hv_result_success(status))
+ hv_status_err(status, "\n");
+
+ return hv_result_to_errno(status);
+}
+
+static int mshv_chk_unmap_irq(union hv_device_id hv_devid,
+ struct irq_data *irqdata)
+{
+ int rc;
+
+ if (irqdata->chip_data == NULL)
+ return 0;
+
+ rc = mshv_unmap_device_interrupt(hv_devid, irqdata->chip_data);
+ if (rc)
+ return rc;
+
+ kfree(irqdata->chip_data);
+ irqdata->chip_data = NULL;
+
+ return 0;
+}
+
+/*
+ * Synchronize device update with VFIO.
+ * See: vfio_pci_memory_lock_and_enable()
+ */
+static u16 mshv_pci_memory_lock_and_enable(struct vfio_pci_core_device *cdev)
+{
+ u16 cmd;
+
+ down_write(&cdev->memory_lock);
+ pci_read_config_word(cdev->pdev, PCI_COMMAND, &cmd);
+ if (!(cmd & PCI_COMMAND_MEMORY))
+ pci_write_config_word(cdev->pdev, PCI_COMMAND,
+ cmd | PCI_COMMAND_MEMORY);
+ return cmd;
+}
+
+static void mshv_pci_memory_unlock_and_restore(
+ struct vfio_pci_core_device *cdev,
+ u16 cmd)
+{
+ pci_write_config_word(cdev->pdev, PCI_COMMAND, cmd);
+ up_write(&cdev->memory_lock);
+}
+
+static void mshv_make_device_usable(struct pci_dev *pdev, int vector,
+ struct hv_interrupt_entry *hv_entry)
+{
+ int lirq;
+ struct msi_msg msimsg;
+ struct irq_data *irqdata, *parent;
+ u16 pcicmd;
+ struct vfio_pci_core_device *coredev = dev_get_drvdata(&pdev->dev);
+
+ if (pdev->dev.driver == NULL ||
+ strcmp(pdev->dev.driver->name, "vfio-pci") != 0) {
+ pr_err("Hyper-V: irqbypass: non vfio device %s\n",
+ pci_name(pdev));
+ return;
+ }
+ if (coredev == NULL) {
+ pr_err("Hyper-V: irqbypass: null vfio device for %s\n",
+ pci_name(pdev));
+ return;
+ }
+
+ if (hv_entry->source != HV_INTERRUPT_SOURCE_MSI) {
+ pr_err("Hyper-V: %s irq source not msi\n", pci_name(pdev));
+ return;
+ }
+
+ lirq = pci_irq_vector(pdev, vector);
+ irqdata = irq_get_irq_data(lirq);
+ if (irqdata == NULL) {
+ pr_err("Hyper-V: null irq_data for write msimsg. lirq:0x%x\n",
+ lirq);
+ return;
+ }
+
+ msimsg.address_hi = 0;
+ msimsg.address_lo = hv_entry->msi_entry.address.as_uint32;
+ msimsg.data = hv_entry->msi_entry.data.as_uint32;
+
+ pcicmd = mshv_pci_memory_lock_and_enable(coredev);
+ pci_write_msi_msg(lirq, &msimsg);
+ mshv_pci_memory_unlock_and_restore(coredev, pcicmd);
+
+ pci_msi_unmask_irq(irqdata);
+
+ parent = irqdata->parent_data;
+ if (parent && parent->chip && parent->chip->irq_unmask)
+ irq_chip_unmask_parent(irqdata);
+}
+
+/*
+ * This guest has a device passthru'd to it. VFIO did the initial setup of
+ * the device interrupts, but we left them unmapped in the hypervisor
+ * because we didn't have the guest target cpu and vector (required by
+ * hypervisor). We have them now, so do the map hypercall.
+ * Also, when here, it is expected that the device global mask is unset
+ * but individual MSI/x masks are set. Goal here is to map the interrupt in
+ * the hypervisor, update the corresponding device MSI/x entry, and enable it.
+ */
+static void mshv_pthru_dev_irq_remap(struct mshv_irqfd *irqfd)
+{
+ u64 ptid, status;
+ struct pci_dev *pdev;
+ int rc, deposit_pgs = 16;
+ struct mshv_lapic_irq *ginfo = &irqfd->irqfd_lapic_irq;
+ union hv_device_id hv_devid;
+ struct hv_interrupt_entry *new_entry;
+ struct irq_data *irqdata;
+
+ if (!irqfd->irqfd_girq_ent.girq_entry_valid ||
+ irqfd->irqfd_bypass_prod == NULL)
+ return;
+
+ rc = mshv_parse_mshv_irqfd(irqfd, &pdev, &irqdata);
+ if (rc)
+ return;
+
+ hv_devid.as_uint64 = hv_devid_from_pdev(pdev);
+
+ rc = mshv_chk_unmap_irq(hv_devid, irqdata);
+ if (rc)
+ return;
+
+ new_entry = kmalloc(sizeof(*new_entry), GFP_ATOMIC);
+ if (new_entry == NULL)
+ return;
+
+ ptid = irqfd->irqfd_partn->pt_id;
+
+ while (deposit_pgs--) {
+ rc = mshv_map_device_interrupt(ptid, hv_devid, ginfo, new_entry,
+ &status);
+ if (rc == 0)
+ break;
+ if (hv_result(status) != HV_STATUS_INSUFFICIENT_MEMORY)
+ break;
+
+ rc = hv_call_deposit_pages(NUMA_NO_NODE, ptid, 1);
+ if (rc)
+ break;
+ }
+ if (rc) {
+ kfree(new_entry);
+ return;
+ }
+
+ irqdata->chip_data = new_entry;
+
+ mshv_make_device_usable(pdev, irqdata->hwirq, new_entry);
+}
+
+static void mshv_pthru_dev_irq_undo(struct mshv_irqfd *irqfd)
+{
+ struct pci_dev *pdev;
+ union hv_device_id hv_devid;
+ struct irq_data *irqdata;
+ int rc;
+
+ if (!irqfd->irqfd_girq_ent.girq_entry_valid ||
+ irqfd->irqfd_bypass_prod == NULL)
+ return;
+
+ rc = mshv_parse_mshv_irqfd(irqfd, &pdev, &irqdata);
+ if (rc)
+ return;
+
+ hv_devid.as_uint64 = hv_devid_from_pdev(pdev);
+ mshv_chk_unmap_irq(hv_devid, irqdata);
+}
+
+#else /* IS_ENABLED(CONFIG_X86_64) */
+
+static void mshv_pthru_dev_irq_remap(struct mshv_irqfd *irqfd) { }
+static void mshv_pthru_dev_irq_undo(struct mshv_irqfd *irqfd) { }
+
+#endif /* IS_ENABLED(CONFIG_X86_64) */
+
void mshv_register_irq_ack_notifier(struct mshv_partition *partition,
struct mshv_irq_ack_notifier *mian)
{
@@ -264,6 +635,7 @@ static void mshv_irqfd_shutdown(struct work_struct *work)
/*
* It is now safe to release the object's resources
*/
+ irq_bypass_unregister_consumer(&irqfd->irqfd_bypass_cons);
eventfd_ctx_put(irqfd->irqfd_eventfd_ctx);
kfree(irqfd);
}
@@ -286,6 +658,12 @@ static void mshv_irqfd_deactivate(struct mshv_irqfd *irqfd)
hlist_del(&irqfd->irqfd_hnode);
+ /*
+ * Cleanup interrupt map (kfree chip_data) while in a VMM thread as
+ * unmap needs partition id. mshv_irqfd_shutdown() runs in a kthread.
+ */
+ mshv_pthru_dev_irq_undo(irqfd);
+
queue_work(irqfd_cleanup_wq, &irqfd->irqfd_shutdown);
}
@@ -383,6 +761,45 @@ static void mshv_irqfd_queue_proc(struct file *file, wait_queue_head_t *wqh,
add_wait_queue_priority(wqh, &irqfd->irqfd_wait);
}
+static int mshv_irq_bypass_add_producer(struct irq_bypass_consumer *cons,
+ struct irq_bypass_producer *prod)
+{
+ struct mshv_irqfd *irqfd;
+
+ irqfd = container_of(cons, struct mshv_irqfd, irqfd_bypass_cons);
+ irqfd->irqfd_bypass_prod = prod;
+
+ mshv_pthru_dev_irq_remap(irqfd);
+
+ return 0;
+}
+
+static void mshv_irq_bypass_del_producer(struct irq_bypass_consumer *cons,
+ struct irq_bypass_producer *prod)
+{
+ struct mshv_irqfd *irqfd;
+
+ irqfd = container_of(cons, struct mshv_irqfd, irqfd_bypass_cons);
+
+ WARN_ON(irqfd->irqfd_bypass_prod != prod);
+ irqfd->irqfd_bypass_prod = NULL;
+
+}
+
+static void mshv_setup_irq_bypass(struct mshv_irqfd *irqfd,
+ struct eventfd_ctx *eventfd)
+{
+ struct irq_bypass_consumer *consumer = &irqfd->irqfd_bypass_cons;
+ int rc;
+
+ consumer->add_producer = mshv_irq_bypass_add_producer;
+ consumer->del_producer = mshv_irq_bypass_del_producer;
+ rc = irq_bypass_register_consumer(&irqfd->irqfd_bypass_cons, eventfd);
+ if (rc)
+ pr_err("Hyper-V: irq bypass consumer registration failed: %d\n",
+ rc);
+}
+
static int mshv_irqfd_assign(struct mshv_partition *pt,
struct mshv_user_irqfd *args)
{
@@ -509,6 +926,8 @@ static int mshv_irqfd_assign(struct mshv_partition *pt,
if (events & EPOLLIN)
mshv_assert_irq_slow(irqfd);
+ mshv_setup_irq_bypass(irqfd, eventfd);
+
srcu_read_unlock(&pt->pt_irq_srcu, idx);
return 0;
diff --git a/drivers/iommu/hyperv-iommu-root.c b/drivers/iommu/hyperv-iommu-root.c
index a2e0f6cc78e6..dc270b0a80d9 100644
--- a/drivers/iommu/hyperv-iommu-root.c
+++ b/drivers/iommu/hyperv-iommu-root.c
@@ -217,6 +217,20 @@ u64 hv_build_devid_oftype(struct pci_dev *pdev, enum hv_device_type type)
}
EXPORT_SYMBOL_GPL(hv_build_devid_oftype);
+/* Build device id for the interrupt path */
+u64 hv_devid_from_pdev(struct pci_dev *pdev)
+{
+ enum hv_device_type dev_type;
+
+ if (hv_pcidev_is_attached_dev(pdev))
+ dev_type = HV_DEVICE_TYPE_LOGICAL;
+ else
+ dev_type = HV_DEVICE_TYPE_PCI;
+
+ return hv_build_devid_oftype(pdev, dev_type);
+}
+EXPORT_SYMBOL_GPL(hv_devid_from_pdev);
+
/* Create a new device domain in the hypervisor */
static int hv_iommu_create_hyp_devdom(struct hv_domain *hvdom)
{
diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci-hyperv.c
index 50d793ca8f31..702a8005651b 100644
--- a/drivers/pci/controller/pci-hyperv.c
+++ b/drivers/pci/controller/pci-hyperv.c
@@ -1744,6 +1744,16 @@ static void hv_irq_mask(struct irq_data *data)
static void hv_irq_unmask(struct irq_data *data)
{
+ struct pci_dev *pdev;
+ struct msi_desc *msi_desc;
+
+ msi_desc = irq_data_get_msi_desc(data);
+ pdev = msi_desc_to_pci_dev(msi_desc);
+
+ /* Done during bypass setup in mshv_eventfd.c: mshv_irqfd_assign() */
+ if (hv_pcidev_is_pthru_dev(pdev))
+ return;
+
hv_arch_irq_unmask(data);
if (data->parent_data->chip->irq_unmask)
diff --git a/include/asm-generic/mshyperv.h b/include/asm-generic/mshyperv.h
index 8d5c610da99a..88b3aba6691c 100644
--- a/include/asm-generic/mshyperv.h
+++ b/include/asm-generic/mshyperv.h
@@ -332,6 +332,7 @@ u64 hv_get_current_partid(void);
bool hv_pcidev_is_attached_dev(struct pci_dev *pdev);
bool hv_pcidev_is_pthru_dev(struct pci_dev *pdev);
u64 hv_build_devid_oftype(struct pci_dev *pdev, enum hv_device_type type);
+u64 hv_devid_from_pdev(struct pci_dev *pdev);
#else
static inline bool hv_pcidev_is_attached_dev(struct pci_dev *pdev)
{ return false; }
@@ -340,6 +341,8 @@ static inline bool hv_pcidev_is_pthru_dev(struct pci_dev *pdev)
static inline u64 hv_build_devid_oftype(struct pci_dev *pdev,
enum hv_device_type type)
{ return 0; }
+static inline u64 hv_devid_from_pdev(struct pci_dev *pdev)
+{ return 0; }
static inline u64 hv_get_current_partid(void)
{ return HV_PARTITION_ID_INVALID; }
#endif /* IS_ENABLED(CONFIG_HYPERV_IOMMU) */
--
2.51.2.vfs.0.1
^ permalink raw reply related
* [PATCH V1 3/3] mshv: Implement guest irq migration for passthru devices
From: Mukesh R @ 2026-05-12 2:12 UTC (permalink / raw)
To: hpa, robin.murphy, robh, linux-hyperv, linux-kernel, iommu,
linux-pci, linux-arch
In-Reply-To: <20260512021242.1679786-1-mrathor@linux.microsoft.com>
Ask the hypervisor to retarget interrupts to new guest cpu or vector
upon guest irq migration. This happens in the irqfd update path.
Signed-off-by: Mukesh R <mrathor@linux.microsoft.com>
---
drivers/hv/mshv_eventfd.c | 78 ++++++++++++++++++++++++++++++++++++++-
1 file changed, 76 insertions(+), 2 deletions(-)
diff --git a/drivers/hv/mshv_eventfd.c b/drivers/hv/mshv_eventfd.c
index 1f5c1e9ee9b7..c05201d857fd 100644
--- a/drivers/hv/mshv_eventfd.c
+++ b/drivers/hv/mshv_eventfd.c
@@ -192,6 +192,77 @@ static int mshv_map_device_interrupt(u64 ptid, union hv_device_id hv_devid,
}
+/* NOTE: caller does spin_lock_irq on pt_irqfds_lock, hence no disable here */
+static void mshv_do_guest_irq_retarget(u64 partid, struct mshv_irqfd *irqfd)
+{
+ int rc, var_size;
+ u64 status;
+ union hv_device_id hv_devid;
+ struct hv_input_get_vp_set_from_mda *mda_input;
+ union hv_output_get_vp_set_from_mda *mda_output;
+ struct hv_retarget_device_interrupt *remap_inp;
+ struct pci_dev *pdev;
+ struct irq_data *irqdata;
+ struct mshv_lapic_irq *lapic_irq = &irqfd->irqfd_lapic_irq;
+ struct hv_interrupt_entry *inte = NULL;
+
+ if (!irqfd->irqfd_girq_ent.girq_entry_valid ||
+ irqfd->irqfd_bypass_prod == NULL)
+ return;
+
+ rc = mshv_parse_mshv_irqfd(irqfd, &pdev, &irqdata);
+ if (rc)
+ return;
+
+ inte = irqdata->chip_data;
+ if (inte == NULL)
+ return;
+
+ hv_devid.as_uint64 = hv_devid_from_pdev(pdev);
+
+
+ mda_input = *this_cpu_ptr(hyperv_pcpu_input_arg);
+ mda_output = *this_cpu_ptr(hyperv_pcpu_output_arg);
+
+ rc = hv_vpset_from_hyp_disabled(mda_input, mda_output, lapic_irq,
+ partid);
+ if (rc)
+ return;
+
+ remap_inp = *this_cpu_ptr(hyperv_pcpu_input_arg);
+ memset(remap_inp, 0, sizeof(*remap_inp));
+
+ rc = hv_copy_vpset(&remap_inp->int_target.vp_set,
+ &mda_output->target_vpset);
+ if (rc <= 0) {
+ pr_err("Hyper-V: ptid %lld - vpset copy failed (%d)\n",
+ partid, rc);
+ return;
+ }
+
+ /*
+ * var-sized hcall: var-size starts after vp_mask (thus vp_set.format
+ * does not count, but vp_set.valid_bank_mask does).
+ */
+ var_size = rc + 1;
+
+ remap_inp->partition_id = partid;
+ remap_inp->device_id = hv_devid.as_uint64;
+ remap_inp->int_target.vector = lapic_irq->lapic_vector;
+ remap_inp->int_target.flags = HV_DEVICE_INTERRUPT_TARGET_PROCESSOR_SET;
+
+ remap_inp->int_entry.source = inte->source;
+ remap_inp->int_entry.msi_entry.as_uint64 = inte->msi_entry.as_uint64;
+
+ status = hv_do_rep_hypercall(HVCALL_RETARGET_INTERRUPT, 0, var_size,
+ remap_inp, NULL);
+
+ if (!hv_result_success(status))
+ hv_status_err(status, "pt:%lld vec:%d lapic-id:%lld\n",
+ partid, lapic_irq->lapic_vector,
+ lapic_irq->lapic_apic_id);
+}
+
static int mshv_unmap_device_interrupt(union hv_device_id hv_devid,
struct hv_interrupt_entry *irq_entry)
{
@@ -729,9 +800,12 @@ static void mshv_irqfd_update(struct mshv_partition *pt,
struct mshv_irqfd *irqfd)
{
write_seqcount_begin(&irqfd->irqfd_irqe_sc);
- irqfd->irqfd_girq_ent = mshv_ret_girq_entry(pt,
- irqfd->irqfd_irqnum);
+ irqfd->irqfd_girq_ent = mshv_ret_girq_entry(pt, irqfd->irqfd_irqnum);
mshv_copy_girq_info(&irqfd->irqfd_girq_ent, &irqfd->irqfd_lapic_irq);
+
+#if IS_ENABLED(CONFIG_X86_64)
+ mshv_do_guest_irq_retarget(pt->pt_id, irqfd);
+#endif
write_seqcount_end(&irqfd->irqfd_irqe_sc);
}
--
2.51.2.vfs.0.1
^ permalink raw reply related
* Re: [PATCH net-next v7 1/6] net: mana: Create separate EQs for each vPort
From: Jakub Kicinski @ 2026-05-12 2:13 UTC (permalink / raw)
To: longli
Cc: Jakub Kicinski, kotaranov, davem, pabeni, edumazet, andrew+netdev,
jgg, leon, haiyangz, kys, wei.liu, decui, shradhagupta, horms,
netdev, linux-rdma, linux-hyperv, linux-kernel
In-Reply-To: <20260507191237.438671-2-longli@microsoft.com>
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
net: mana: Create separate EQs for each vPort
This change removes EQ sharing across vPorts and moves EQ ownership
from struct mana_context to struct mana_port_context. It exports
mana_create_eq() and mana_destroy_eq() so the MANA RDMA driver can
create EQs when a raw QP configures the vport, and also rearranges
mana_alloc_queues()/mana_dealloc_queues() to own the netdev-side
EQ lifecycle.
A few questions on the resulting ownership model below.
> diff --git a/drivers/infiniband/hw/mana/main.c b/drivers/infiniband/hw/mana/main.c
> index ac5e75dd3494..8000ab6e8beb 100644
> --- a/drivers/infiniband/hw/mana/main.c
> +++ b/drivers/infiniband/hw/mana/main.c
> @@ -20,8 +20,10 @@ void mana_ib_uncfg_vport(struct mana_ib_dev *dev, struct mana_ib_pd *pd,
> pd->vport_use_count--;
> WARN_ON(pd->vport_use_count < 0);
>
> - if (!pd->vport_use_count)
> + if (!pd->vport_use_count) {
> + mana_destroy_eq(mpc);
> mana_uncfg_vport(mpc);
> + }
>
> mutex_unlock(&pd->vport_mutex);
> }
[High]
After this change, mpc->eqs has two distinct owners: the RDMA path
frees it here under pd->vport_mutex, and the netdev path frees it under
rtnl_lock via mana_dealloc_queues() -> mana_destroy_eq(). Neither lock
is held by the readers in qp.c. See the comment on the RSS QP path
below.
> diff --git a/drivers/infiniband/hw/mana/qp.c b/drivers/infiniband/hw/mana/qp.c
> index 645581359cee..6f1043383e8c 100644
> --- a/drivers/infiniband/hw/mana/qp.c
> +++ b/drivers/infiniband/hw/mana/qp.c
> @@ -168,7 +168,15 @@ static int mana_ib_create_qp_rss(struct ib_qp *ibqp, struct ib_pd *pd,
> cq_spec.gdma_region = cq->queue.gdma_region;
> cq_spec.queue_size = cq->cqe * COMP_ENTRY_SIZE;
> cq_spec.modr_ctx_id = 0;
> - eq = &mpc->ac->eqs[cq->comp_vector];
> + /* EQs are created when a raw QP configures the vport.
> + * A raw QP must be created before creating rwq_ind_tbl.
> + */
> + if (!mpc->eqs) {
> + ret = -EINVAL;
> + i--;
> + goto fail;
> + }
> + eq = &mpc->eqs[cq->comp_vector % mpc->num_queues];
> cq_spec.attached_eq = eq->eq->id;
[High]
Is the NULL check on mpc->eqs safe here without any synchronization?
Before this patch, ac->eqs was allocated in mana_probe() and freed in
mana_remove(), so its lifetime spanned the whole driver. With this
patch, apc->eqs can be freed concurrently by either owner:
mana_ib_uncfg_vport() (pd->vport_mutex)
mana_destroy_eq(mpc)
kfree(apc->eqs);
apc->eqs = NULL;
mana_dealloc_queues() (rtnl_lock, ndo_close)
mana_destroy_eq(apc)
kfree(apc->eqs);
apc->eqs = NULL;
This path holds neither lock. Can the 'if (!mpc->eqs)' observation
survive through the subsequent '&mpc->eqs[...]' and 'eq->eq->id'
dereference, or can a concurrent destroyer free apc->eqs between the
check and the deref and turn this into a read of freed memory?
There is also an initialization window in the writer:
apc->eqs = kzalloc_objs(struct mana_eq, apc->num_queues);
...
for (i = 0; i < apc->num_queues; i++) {
...
err = mana_gd_create_mana_eq(gd, &spec, &apc->eqs[i].eq);
apc->eqs is published before apc->eqs[i].eq is filled in. Can a
concurrent reader here see apc->eqs non-NULL but apc->eqs[i].eq still
NULL, and fault on eq->eq->id?
[Medium]
mana_ib advertises ib_dev.num_comp_vectors as gc->max_num_queues in
mana_ib_probe(), but this patch sizes the per-port pool as
apc->num_queues, which defaults to min(gc->max_num_queues,
MANA_DEF_NUM_QUEUES) and can be reduced further via ethtool -L.
eq = &mpc->eqs[cq->comp_vector % mpc->num_queues];
When gc->max_num_queues > apc->num_queues, does this modulo silently
collapse distinct advertised completion vectors onto shared EQs, so
CQs created by userspace with different comp_vector values end up on
the same notification stream? Would it be better to cap
ib_dev.num_comp_vectors to apc->num_queues, or reject comp_vector >=
apc->num_queues, instead of wrapping?
[Medium]
The added comment codifies a new ordering constraint:
/* EQs are created when a raw QP configures the vport.
* A raw QP must be created before creating rwq_ind_tbl.
*/
Before this patch, ac->eqs existed from mana_probe() onward, so
mana_ib_create_qp_rss() succeeded whether or not the netdev was up and
whether or not any raw QP had been created on the port. After this
patch, if the netdev is administratively down and no raw QP has
configured the vport, mpc->eqs is NULL and this path returns -EINVAL.
Is that ABI-visible change to RSS QP creation intentional, and worth
calling out in the commit message?
The comment is also slightly incomplete: EQs are created on netdev open
via mana_alloc_queues() -> mana_create_eq() as well, not only when a
raw QP configures the vport. Could the wording be adjusted?
>
> ret = mana_create_wq_obj(mpc, mpc->port_handle, GDMA_RQ,
> diff --git a/drivers/net/ethernet/microsoft/mana/mana_en.c b/drivers/net/ethernet/microsoft/mana/mana_en.c
[ ... ]
> @@ -3343,6 +3367,9 @@ static int mana_dealloc_queues(struct net_device *ndev)
> mana_fence_rqs(apc);
>
> /* Even in err case, still need to cleanup the vPort */
> + mana_destroy_rxqs(apc);
> + mana_destroy_txq(apc);
> + mana_destroy_eq(apc);
> mana_destroy_vport(apc);
>
> return 0;
--
pw-bot: cr
^ permalink raw reply
* Re: [PATCH net-next v7 2/6] net: mana: Query device capabilities and configure MSI-X sharing for EQs
From: Jakub Kicinski @ 2026-05-12 2:14 UTC (permalink / raw)
To: longli
Cc: Jakub Kicinski, kotaranov, davem, pabeni, edumazet, andrew+netdev,
jgg, leon, haiyangz, kys, wei.liu, decui, shradhagupta, horms,
netdev, linux-rdma, linux-hyperv, linux-kernel
In-Reply-To: <20260507191237.438671-3-longli@microsoft.com>
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
net: mana: Query device capabilities and configure MSI-X sharing for EQs
This change queries device capabilities earlier during GDMA setup,
renames mana_query_device_cfg() to mana_gd_query_device_cfg(), and adds
a new gc->msi_sharing flag plus a gc->msi_bitmap to manage dedicated
vs. shared MSI-X vectors for EQs across vPorts.
A few questions below, mostly about msi_sharing state across resume and
about some wording in the commit message and in the code.
> diff --git a/drivers/net/ethernet/microsoft/mana/gdma_main.c b/drivers/net/ethernet/microsoft/mana/gdma_main.c
> index f3316e929175..3aa96329f359 100644
> --- a/drivers/net/ethernet/microsoft/mana/gdma_main.c
> +++ b/drivers/net/ethernet/microsoft/mana/gdma_main.c
> @@ -149,6 +149,9 @@ static int mana_gd_query_max_resources(struct pci_dev *pdev)
> struct gdma_context *gc = pci_get_drvdata(pdev);
> struct gdma_query_max_resources_resp resp = {};
> struct gdma_general_req req = {};
> + unsigned int max_num_queues;
> + u8 bm_hostmode;
> + u16 num_ports;
> int err;
>
> mana_gd_init_req_hdr(&req.hdr, GDMA_QUERY_MAX_RESOURCES,
> @@ -197,6 +200,40 @@ static int mana_gd_query_max_resources(struct pci_dev *pdev)
> if (gc->max_num_queues == 0)
> return -ENOSPC;
>
> + err = mana_gd_query_device_cfg(gc, MANA_MAJOR_VERSION, MANA_MINOR_VERSION,
> + MANA_MICRO_VERSION, &num_ports, &bm_hostmode);
> + if (err)
> + return err;
> +
> + if (!num_ports)
> + return -EINVAL;
[Low]
This is the first of two back-to-back mana_gd_query_device_cfg() calls
per probe cycle. The second one happens in mana_probe() (further down
in this same patch) and also fetches num_ports and bm_hostmode from
firmware. The first caller here discards bm_hostmode; the second one
uses both.
Would it be reasonable to cache num_ports and bm_hostmode on
struct gdma_context during the first call and have mana_probe() read
them from there, instead of doing a second HWC round-trip that must
return the same values for the per-vPort math in
mana_gd_query_max_resources() to match ac->num_ports in mana_probe()?
> +
> + /*
> + * Adjust gc->max_num_queues returned from the SOC to allow dedicated
> + * MSIx for each vPort. Clamp to no less than MANA_DEF_NUM_QUEUES.
> + */
> + max_num_queues = (gc->num_msix_usable - 1) / num_ports;
> + max_num_queues = rounddown_pow_of_two(max(max_num_queues, 1U));
> + if (max_num_queues < MANA_DEF_NUM_QUEUES)
> + max_num_queues = MANA_DEF_NUM_QUEUES;
> +
> + /*
> + * Use dedicated MSIx for EQs whenever possible, use MSIx sharing for
> + * Ethernet EQs when (max_num_queues * num_ports > num_msix_usable - 1)
> + */
> + max_num_queues = min(gc->max_num_queues, max_num_queues);
[Low]
The commit message says "The number of queues per vPort is clamped to
no less than MANA_DEF_NUM_QUEUES". After the clamp-up above, this
min() with gc->max_num_queues can bring the result back below
MANA_DEF_NUM_QUEUES whenever gc->max_num_queues is below
MANA_DEF_NUM_QUEUES (which is 16).
gc->max_num_queues was previously clamped by num_online_cpus(),
resp.max_eq/cq/sq/rq, and gc->num_msix_usable - 1, so a VM with
fewer than 16 online CPUs will end up with gc->max_num_queues_vport
below MANA_DEF_NUM_QUEUES in the non-sharing branch.
Could the commit message be reworded to describe the actual behaviour
(the per-vPort count is clamped towards MANA_DEF_NUM_QUEUES but never
exceeds the hardware maximum)?
> + if (max_num_queues * num_ports > gc->num_msix_usable - 1)
> + gc->msi_sharing = true;
[Medium]
Is gc->msi_sharing ever cleared?
Walking the write sites for this flag in the series: it is set to true
unconditionally in the non-dyn branch of mana_gd_setup_hwc_irqs() (see
the hunk below), and conditionally true here. I could not find any
path that writes false, and neither mana_gd_remove_irqs() nor
mana_gd_cleanup() reset it.
The same gdma_context allocated in mana_gd_probe() survives a suspend
/ resume cycle via mana_gd_suspend() -> mana_gd_cleanup() ->
mana_gd_resume() -> mana_gd_setup(). In the dynamic MSI-X path
(pci_msix_can_alloc_dyn() true, i.e. the PF), gc->num_msix_usable is
recomputed as min(resp.max_msix, num_online_cpus() + 1), so it can
legitimately grow across suspend/resume if CPUs came online while the
guest was suspended.
If the first probe set msi_sharing=true because MSI-X was tight, but
on resume there are enough vectors for dedicated allocation, does the
flag stay stuck at true? If so:
- gc->max_num_queues_vport is computed via the sharing branch
min(gc->num_msix_usable - 1, gc->max_num_queues) rather than the
dedicated max_num_queues value;
- the if (!gc->msi_sharing) branch at the end of mana_gd_setup()
is skipped and gc->msi_bitmap stays NULL;
- later patches in this series that consult !gc->msi_sharing as the
"use bitmap" predicate keep taking the shared path.
Would it be appropriate to reset gc->msi_sharing = false at the top of
mana_gd_query_max_resources() (and/or in mana_gd_setup_hwc_irqs()'s
other branch) before the conditions that may set it to true are
evaluated? This would also line up with the commit message's claim
that "MSI-X sharing among vPorts is disabled by default and is only
enabled when there are not enough MSI-X vectors for dedicated
allocation."
> +
> + /* If MSI is shared, use max allowed value */
> + if (gc->msi_sharing)
> + gc->max_num_queues_vport = min(gc->num_msix_usable - 1, gc->max_num_queues);
> + else
> + gc->max_num_queues_vport = max_num_queues;
> +
> + dev_info(gc->dev, "MSI sharing mode %d max queues %d\n",
> + gc->msi_sharing, gc->max_num_queues);
[Low, Low]
Two small wording questions on this block:
The comment above reads "Adjust gc->max_num_queues returned from the
SOC to allow dedicated MSIx for each vPort", but the code never
modifies gc->max_num_queues; it updates a local and assigns to
gc->max_num_queues_vport. Should the comment say
gc->max_num_queues_vport instead?
For the dev_info, should the printed field be gc->max_num_queues_vport
rather than gc->max_num_queues? As written, the log always shows the
hardware maximum that was already decided earlier in the function,
not the per-vPort value that the preceding logic just chose, so the
number never reflects the effect of toggling msi_sharing.
> +
> return 0;
> }
>
> @@ -1859,6 +1896,7 @@ static int mana_gd_setup_hwc_irqs(struct pci_dev *pdev)
> /* Need 1 interrupt for HWC */
> max_irqs = min(num_online_cpus(), MANA_MAX_NUM_QUEUES) + 1;
> min_irqs = 2;
> + gc->msi_sharing = true;
> }
>
[ ... ]
^ permalink raw reply
* Re: [PATCH net-next v7 4/6] net: mana: Use GIC functions to allocate global EQs
From: Jakub Kicinski @ 2026-05-12 2:14 UTC (permalink / raw)
To: longli
Cc: Jakub Kicinski, kotaranov, davem, pabeni, edumazet, andrew+netdev,
jgg, leon, haiyangz, kys, wei.liu, decui, shradhagupta, horms,
netdev, linux-rdma, linux-hyperv, linux-kernel
In-Reply-To: <20260507191237.438671-5-longli@microsoft.com>
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
net: mana: Use GIC functions to allocate global EQs
This patch replaces the inline GDMA global interrupt setup in
mana_gd_setup_dyn_irqs(), mana_gd_setup_irqs(), and mana_gd_remove_irqs()
with calls to the new mana_gd_get_gic()/mana_gd_put_gic() helpers
introduced in the preceding patch.
> diff --git a/drivers/net/ethernet/microsoft/mana/gdma_main.c b/drivers/net/ethernet/microsoft/mana/gdma_main.c
> index 5930ab817056..850afdbd9c4e 100644
> --- a/drivers/net/ethernet/microsoft/mana/gdma_main.c
> +++ b/drivers/net/ethernet/microsoft/mana/gdma_main.c
> @@ -1888,30 +1888,13 @@ static int mana_gd_setup_dyn_irqs(struct pci_dev *pdev, int nvec)
> * further used in irq_setup()
> */
> for (i = 1; i <= nvec; i++) {
> - gic = kzalloc_obj(*gic);
> + gic = mana_gd_get_gic(gc, false, &i);
> if (!gic) {
> err = -ENOMEM;
> goto free_irq;
> }
> - gic->handler = mana_gd_process_eq_events;
> - INIT_LIST_HEAD(&gic->eq_list);
> - spin_lock_init(&gic->lock);
> -
> - snprintf(gic->name, MANA_IRQ_NAME_SZ, "mana_q%d@pci:%s",
> - i - 1, pci_name(pdev));
[Low]
Before this patch the per-queue interrupts in mana_gd_setup_dyn_irqs()
and mana_gd_setup_irqs() were named as:
"mana_q%d@pci:%s", i - 1, pci_name(pdev)
so they appeared as mana_q0, mana_q1, ... in /proc/interrupts.
After this patch, allocation goes through mana_gd_get_gic(), which
names them using gic->msi (equal to i, not i - 1):
snprintf(gic->name, MANA_IRQ_NAME_SZ, "mana_msi%d@pci:%s",
gic->msi, pci_name(dev));
so the same vectors now appear as mana_msi1, mana_msi2, ... The
mana_hwc@pci:%s name is preserved, but every per-queue vector is both
renamed (mana_q -> mana_msi) and renumbered (0-based -> 1-based).
Is this rename intentional? The commit message describes the change
as replacing "the GDMA global interrupt setup code with the new GIC
allocation and release functions", which reads as a pure refactor. If
the rename is intended, could it be mentioned in the changelog so that
operators relying on the mana_q prefix in /proc/interrupts are not
surprised?
> -
> - /* one pci vector is already allocated for HWC */
> - irqs[i - 1] = pci_irq_vector(pdev, i);
> - if (irqs[i - 1] < 0) {
> - err = irqs[i - 1];
> - goto free_current_gic;
> - }
> -
> - err = request_irq(irqs[i - 1], mana_gd_intr, 0, gic->name, gic);
> - if (err)
> - goto free_current_gic;
>
> - xa_store(&gc->irq_contexts, i, gic, GFP_KERNEL);
> + irqs[i - 1] = gic->irq;
> }
>
> /*
> @@ -1933,19 +1916,11 @@ static int mana_gd_setup_dyn_irqs(struct pci_dev *pdev, int nvec)
> kfree(irqs);
> return 0;
>
> -free_current_gic:
> - kfree(gic);
> free_irq:
> for (i -= 1; i > 0; i--) {
> irq = pci_irq_vector(pdev, i);
> - gic = xa_load(&gc->irq_contexts, i);
> - if (WARN_ON(!gic))
> - continue;
> -
> irq_update_affinity_hint(irq, NULL);
> - free_irq(irq, gic);
> - xa_erase(&gc->irq_contexts, i);
> - kfree(gic);
> + mana_gd_put_gic(gc, false, i);
> }
[Low]
Is the irq_update_affinity_hint(irq, NULL) call here still needed?
Looking at mana_gd_put_gic():
if (!refcount_dec_and_test(&gic->refcount))
goto out;
irq = gic->irq;
irq_update_affinity_hint(irq, NULL);
free_irq(irq, gic);
When the last reference is dropped (which is the case on every caller in
this patch, since the refcount is 1 here), the helper already clears the
affinity hint before calling free_irq(). The explicit call in the caller
duplicates that work.
The duplication is harmless today because irq_update_affinity_hint() is
idempotent, but could the clearing of the affinity hint be left entirely
to mana_gd_put_gic()? Otherwise, if these helpers are ever reused in a
context where the refcount is greater than one, the caller will clear an
affinity hint that the helper would correctly have left intact.
> @@ -1966,34 +1941,13 @@ static int mana_gd_setup_irqs(struct pci_dev *pdev, int nvec)
> start_irqs = irqs;
>
> for (i = 0; i < nvec; i++) {
> - gic = kzalloc_obj(*gic);
> + gic = mana_gd_get_gic(gc, false, &i);
> if (!gic) {
> err = -ENOMEM;
> goto free_irq;
> }
>
> - gic->handler = mana_gd_process_eq_events;
> - INIT_LIST_HEAD(&gic->eq_list);
> - spin_lock_init(&gic->lock);
> -
> - if (!i)
> - snprintf(gic->name, MANA_IRQ_NAME_SZ, "mana_hwc@pci:%s",
> - pci_name(pdev));
> - else
> - snprintf(gic->name, MANA_IRQ_NAME_SZ, "mana_q%d@pci:%s",
> - i - 1, pci_name(pdev));
[Low]
The same rename applies here for the non-HWC vectors (mana_q%d with
i - 1 becomes mana_msi%d with i inside mana_gd_get_gic()). Could the
commit message note this user-visible change in /proc/interrupts?
[ ... ]
> @@ -2022,19 +1976,11 @@ static int mana_gd_setup_irqs(struct pci_dev *pdev, int nvec)
> kfree(start_irqs);
> return 0;
>
> -free_current_gic:
> - kfree(gic);
> free_irq:
> for (i -= 1; i >= 0; i--) {
> irq = pci_irq_vector(pdev, i);
> - gic = xa_load(&gc->irq_contexts, i);
> - if (WARN_ON(!gic))
> - continue;
> -
> irq_update_affinity_hint(irq, NULL);
> - free_irq(irq, gic);
> - xa_erase(&gc->irq_contexts, i);
> - kfree(gic);
> + mana_gd_put_gic(gc, false, i);
> }
[Low]
Same observation as in mana_gd_setup_dyn_irqs(): the explicit
irq_update_affinity_hint(irq, NULL) duplicates what
mana_gd_put_gic() already does when it drops the last reference. Could
this call be dropped from the caller?
> @@ -2109,26 +2055,20 @@ static int mana_gd_setup_remaining_irqs(struct pci_dev *pdev)
> static void mana_gd_remove_irqs(struct pci_dev *pdev)
> {
> struct gdma_context *gc = pci_get_drvdata(pdev);
> - struct gdma_irq_context *gic;
> int irq, i;
>
> if (gc->max_num_msix < 1)
> return;
>
> for (i = 0; i < gc->max_num_msix; i++) {
> - irq = pci_irq_vector(pdev, i);
> - if (irq < 0)
> - continue;
> -
> - gic = xa_load(&gc->irq_contexts, i);
> - if (WARN_ON(!gic))
> + if (!xa_load(&gc->irq_contexts, i))
> continue;
>
> /* Need to clear the hint before free_irq */
> + irq = pci_irq_vector(pdev, i);
> irq_update_affinity_hint(irq, NULL);
> - free_irq(irq, gic);
> - xa_erase(&gc->irq_contexts, i);
> - kfree(gic);
> +
> + mana_gd_put_gic(gc, false, i);
> }
[Low]
Same pattern in mana_gd_remove_irqs(): the comment says "Need to clear
the hint before free_irq", but the actual free_irq() now happens inside
mana_gd_put_gic(), which itself calls irq_update_affinity_hint(irq, NULL)
right before free_irq(). Could the explicit call (and comment) be
removed so that the affinity-hint clearing lives in one place?
^ permalink raw reply
* Re: [PATCH net-next v7 5/6] net: mana: Allocate interrupt context for each EQ when creating vPort
From: Jakub Kicinski @ 2026-05-12 2:14 UTC (permalink / raw)
To: longli
Cc: Jakub Kicinski, kotaranov, davem, pabeni, edumazet, andrew+netdev,
jgg, leon, haiyangz, kys, wei.liu, decui, shradhagupta, horms,
netdev, linux-rdma, linux-hyperv, linux-kernel
In-Reply-To: <20260507191237.438671-6-longli@microsoft.com>
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
Subject: net: mana: Allocate interrupt context for each EQ when creating vPort
This commit switches the vPort EQ creation path to acquire a GIC (either
dedicated or shared) for each EQ via mana_gd_get_gic()/mana_gd_put_gic(),
adds a new irq field to struct gdma_queue's EQ union and a debugfs entry
for it, and moves the msix_index invalidation out of mana_gd_deregister_irq()
into the mana_gd_create_eq() error path.
[Low]
The changelog is a single sentence, but the patch makes several coupled
changes beyond acquiring a GIC per EQ:
- it adds a new per-EQ irq field on the shared struct gdma_queue,
- it adds a new debugfs entry for that field,
- it changes an invariant in gdma_main.c (see the comment on
mana_gd_deregister_irq() below),
- it splits the GIC lifecycle across two source files — mana_en.c now
owns get_gic/put_gic around mana_gd_create_mana_eq(), while
mana_gd_register_irq() is still called internally from the create path.
Could the commit message describe the resulting ownership contract
(caller holds a GIC reference across create/destroy, register_irq does
not) and justify the scope of the new field?
> diff --git a/drivers/net/ethernet/microsoft/mana/gdma_main.c b/drivers/net/ethernet/microsoft/mana/gdma_main.c
> index 850afdbd9c4e..54127c385240 100644
> --- a/drivers/net/ethernet/microsoft/mana/gdma_main.c
> +++ b/drivers/net/ethernet/microsoft/mana/gdma_main.c
> @@ -854,7 +854,6 @@ static void mana_gd_deregister_irq(struct gdma_queue *queue)
> }
> spin_unlock_irqrestore(&gic->lock, flags);
>
> - queue->eq.msix_index = INVALID_PCI_MSIX_INDEX;
> synchronize_rcu();
> }
[Low]
Removing the msix_index reset from mana_gd_deregister_irq() and relocating
it to the mana_gd_create_eq() error path is the prerequisite that lets the
new mana_destroy_eq() read eq->eq.msix_index before calling
mana_gd_destroy_queue():
msi = eq->eq.msix_index;
mana_gd_destroy_queue(gc, eq);
mana_gd_put_gic(gc, !gc->msi_sharing, msi);
Could the changelog mention this intentional invariant change and the
ownership contract it implies? Any out-of-tree caller that relied on
mana_gd_deregister_irq() to invalidate msix_index will now silently observe
stale values after a normal destroy.
> @@ -969,6 +968,7 @@ static int mana_gd_create_eq(struct gdma_dev *gd,
> out:
> dev_err(dev, "Failed to create EQ: %d\n", err);
> mana_gd_destroy_eq(gc, false, queue);
> + queue->eq.msix_index = INVALID_PCI_MSIX_INDEX;
> return err;
> }
>
> diff --git a/drivers/net/ethernet/microsoft/mana/mana_en.c b/drivers/net/ethernet/microsoft/mana/mana_en.c
> index 2f106d6f5be4..b3684fa3eb4a 100644
> --- a/drivers/net/ethernet/microsoft/mana/mana_en.c
> +++ b/drivers/net/ethernet/microsoft/mana/mana_en.c
[ ... ]
> @@ -1664,6 +1668,8 @@ int mana_create_eq(struct mana_port_context *apc)
> struct gdma_queue_spec spec = {};
> int err;
> int i;
> + int msi;
> + struct gdma_irq_context *gic;
>
> WARN_ON(apc->eqs);
> apc->eqs = kzalloc_objs(struct mana_eq, apc->num_queues);
> @@ -1681,12 +1687,24 @@ int mana_create_eq(struct mana_port_context *apc)
> apc->mana_port_debugfs);
>
> for (i = 0; i < apc->num_queues; i++) {
> - spec.eq.msix_index = (i + 1) % gc->num_msix_usable;
> + msi = (i + 1) % gc->num_msix_usable;
> + if (gc->msi_sharing)
> + spec.eq.msix_index = msi;
> +
> + gic = mana_gd_get_gic(gc, !gc->msi_sharing, &msi);
> + if (!gic) {
> + err = -ENOMEM;
> + goto out;
> + }
> + spec.eq.msix_index = msi;
[Low]
Is the conditional write to spec.eq.msix_index dead?
if (gc->msi_sharing)
spec.eq.msix_index = msi;
gic = mana_gd_get_gic(gc, !gc->msi_sharing, &msi);
...
spec.eq.msix_index = msi;
The unconditional assignment after mana_gd_get_gic() always overwrites the
conditional one. In the non-sharing case, mana_gd_get_gic() also reassigns
*msi_requested via find_first_zero_bit(), so the pre-computed
(i + 1) % gc->num_msix_usable is only consumed in the sharing branch.
Would splitting the sharing and non-sharing paths, or dropping the
conditional write entirely, make the intent clearer and avoid the
inverted-boolean fallthrough (!gc->msi_sharing)?
> +
> err = mana_gd_create_mana_eq(gd, &spec, &apc->eqs[i].eq);
> if (err) {
> dev_err(gc->dev, "Failed to create EQ %d : %d\n", i, err);
> + mana_gd_put_gic(gc, !gc->msi_sharing, msi);
> goto out;
> }
> + apc->eqs[i].eq->eq.irq = gic->irq;
> mana_create_eq_debugfs(apc, i);
> }
[Low]
The new struct gdma_queue.eq.irq field is only assigned here on the vPort
path:
apc->eqs[i].eq->eq.irq = gic->irq;
The other in-tree caller of mana_gd_create_mana_eq(), mana_ib_create_eqs()
in drivers/infiniband/hw/mana/main.c, creates EQs using the same struct
but never assigns eq.irq, leaving it zero. No debugfs is wired up for ib
EQs today, so the zero is not user-visible, but any future consumer that
reads this field on an ib EQ will see 0 rather than the real IRQ number.
Should mana_gd_create_mana_eq() (or its callers) populate eq.irq
uniformly so the field has one well-defined meaning across all EQs?
> diff --git a/include/net/mana/gdma.h b/include/net/mana/gdma.h
> index fbe3c1427b45..6c138cc77407 100644
> --- a/include/net/mana/gdma.h
> +++ b/include/net/mana/gdma.h
> @@ -342,6 +342,7 @@ struct gdma_queue {
> void *context;
>
> unsigned int msix_index;
> + unsigned int irq;
>
> u32 log2_throttle_limit;
> } eq;
^ permalink raw reply
* Re: [PATCH net-next v7 6/6] RDMA/mana_ib: Allocate interrupt contexts on EQs
From: Jakub Kicinski @ 2026-05-12 2:14 UTC (permalink / raw)
To: longli
Cc: Jakub Kicinski, kotaranov, davem, pabeni, edumazet, andrew+netdev,
jgg, leon, haiyangz, kys, wei.liu, decui, shradhagupta, horms,
netdev, linux-rdma, linux-hyperv, linux-kernel
In-Reply-To: <20260507191237.438671-7-longli@microsoft.com>
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
Subject: RDMA/mana_ib: Allocate interrupt contexts on EQs
This patch converts RDMA/mana_ib EQ creation to use the shared GIC
allocator (mana_gd_get_gic/mana_gd_put_gic), allowing the RDMA EQs
to share interrupt contexts with the Ethernet driver's EQs. It also
adds a new driver capability bit and sets it in the advertised flags.
> diff --git a/drivers/infiniband/hw/mana/main.c b/drivers/infiniband/hw/mana/main.c
> index 8000ab6e8beb..0dc47540fb6f 100644
> --- a/drivers/infiniband/hw/mana/main.c
> +++ b/drivers/infiniband/hw/mana/main.c
> @@ -749,7 +749,8 @@ int mana_ib_create_eqs(struct mana_ib_dev *mdev)
> {
> struct gdma_context *gc = mdev_to_gc(mdev);
> struct gdma_queue_spec spec = {};
> - int err, i;
> + struct gdma_irq_context *gic;
> + int err, i, msi;
>
> spec.type = GDMA_EQ;
> spec.monitor_avl_buf = false;
> @@ -757,11 +758,18 @@ int mana_ib_create_eqs(struct mana_ib_dev *mdev)
> spec.eq.callback = mana_ib_event_handler;
> spec.eq.context = mdev;
> spec.eq.log2_throttle_limit = LOG2_EQ_THROTTLE;
> - spec.eq.msix_index = 0;
> +
> + msi = 0;
> + gic = mana_gd_get_gic(gc, false, &msi);
[Low]
The commit message states "The driver now supports allocating dedicated
MSI-X for each EQ", and the new capability bit added below is named
GDMA_DRV_CAP_FLAG_1_EQ_MSI_UNSHARE_MULTI_VPORT. Does that description
match what this code does?
The calls here pass use_msi_bitmap=false, which appears to share an
existing gic slot (incrementing its refcount) rather than allocating a
fresh dedicated MSI-X from the bitmap:
msi = 0;
gic = mana_gd_get_gic(gc, false, &msi);
...
msi = (i + 1) % gc->num_msix_usable;
gic = mana_gd_get_gic(gc, false, &msi);
The commit body also says "These interrupt contexts may be shared with
Ethernet EQs when MSI-X vectors are limited", which describes
sharing-with-fallback, while the flag name says "UNSHARE". Could the
changelog and flag naming be reconciled so the semantics are clear (for
example, whether the bit advertises "driver can handle unshared EQs" vs
"driver will always be unshared")?
Should the flag also be set only when the driver actually ends up
dedicated rather than advertised unconditionally?
[ ... ]
> diff --git a/include/net/mana/gdma.h b/include/net/mana/gdma.h
> index 6c138cc77407..d84e474309a3 100644
> --- a/include/net/mana/gdma.h
> +++ b/include/net/mana/gdma.h
> @@ -615,6 +615,7 @@ enum {
> #define GDMA_DRV_CAP_FLAG_1_HWC_TIMEOUT_RECONFIG BIT(3)
> #define GDMA_DRV_CAP_FLAG_1_GDMA_PAGES_4MB_1GB_2GB BIT(4)
> #define GDMA_DRV_CAP_FLAG_1_VARIABLE_INDIRECTION_TABLE_SUPPORT BIT(5)
> +#define GDMA_DRV_CAP_FLAG_1_HW_VPORT_LINK_AWARE BIT(6)
>
> /* Driver can handle holes (zeros) in the device list */
> #define GDMA_DRV_CAP_FLAG_1_DEV_LIST_HOLES_SUP BIT(11)
> @@ -631,7 +632,8 @@ enum {
> /* Driver detects stalled send queues and recovers them */
> #define GDMA_DRV_CAP_FLAG_1_HANDLE_STALL_SQ_RECOVERY BIT(18)
>
> -#define GDMA_DRV_CAP_FLAG_1_HW_VPORT_LINK_AWARE BIT(6)
> +/* Driver supports separate EQ/MSIs for each vPort */
> +#define GDMA_DRV_CAP_FLAG_1_EQ_MSI_UNSHARE_MULTI_VPORT BIT(19)
>
> /* Driver supports linearizing the skb when num_sge exceeds hardware limit */
> #define GDMA_DRV_CAP_FLAG_1_SKB_LINEARIZE BIT(20)
[Low]
The subject line scopes this commit to RDMA/mana_ib, but this hunk
changes the shared header include/net/mana/gdma.h in two ways that
affect the Ethernet driver and firmware interface:
- it introduces the new GDMA_DRV_CAP_FLAG_1_EQ_MSI_UNSHARE_MULTI_VPORT
BIT(19) advertised to firmware, and
- it silently moves GDMA_DRV_CAP_FLAG_1_HW_VPORT_LINK_AWARE BIT(6) from
after BIT(18) back to its numeric slot after BIT(5).
Neither change is mentioned in the changelog. Would it be clearer to
split the capability-bit addition into its own patch (or use a subject
not scoped to mana_ib), and to note the BIT(6) relocation as a separate
cleanup so bisecting firmware/Ethernet behavior changes is easier?
> @@ -659,7 +661,8 @@ enum {
> GDMA_DRV_CAP_FLAG_1_SKB_LINEARIZE | \
> GDMA_DRV_CAP_FLAG_1_PROBE_RECOVERY | \
> GDMA_DRV_CAP_FLAG_1_HANDLE_STALL_SQ_RECOVERY | \
> - GDMA_DRV_CAP_FLAG_1_HWC_TIMEOUT_RECOVERY)
> + GDMA_DRV_CAP_FLAG_1_HWC_TIMEOUT_RECOVERY | \
> + GDMA_DRV_CAP_FLAG_1_EQ_MSI_UNSHARE_MULTI_VPORT)
>
> #define GDMA_DRV_CAP_FLAGS2 0
^ permalink raw reply
* Re: [PATCH net-next v7 1/6] net: mana: Create separate EQs for each vPort
From: Jakub Kicinski @ 2026-05-12 2:14 UTC (permalink / raw)
To: longli
Cc: Jakub Kicinski, kotaranov, davem, pabeni, edumazet, andrew+netdev,
jgg, leon, haiyangz, kys, wei.liu, decui, shradhagupta, horms,
netdev, linux-rdma, linux-hyperv, linux-kernel
In-Reply-To: <20260507191237.438671-2-longli@microsoft.com>
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
Subject: net: mana: Create separate EQs for each vPort
This commit prepares for assigning vPorts to dedicated MSI-X vectors by
removing EQ sharing among the vPorts and creating dedicated EQs for each
vPort. It moves the EQ definition from struct mana_context to struct
mana_port_context and exports related functions for the RDMA driver.
> diff --git a/drivers/infiniband/hw/mana/main.c b/drivers/infiniband/hw/mana/main.c
> index ac5e75dd3494d..8000ab6e8beb9 100644
> --- a/drivers/infiniband/hw/mana/main.c
> +++ b/drivers/infiniband/hw/mana/main.c
> @@ -55,15 +57,22 @@ int mana_ib_cfg_vport(struct mana_ib_dev *dev, u32 port, struct mana_ib_pd *pd,
> return err;
> }
>
> - mutex_unlock(&pd->vport_mutex);
>
> pd->tx_shortform_allowed = mpc->tx_shortform_allowed;
> pd->tx_vp_offset = mpc->tx_vp_offset;
> + err = mana_create_eq(mpc);
> + if (err) {
> + mana_uncfg_vport(mpc);
> + pd->vport_use_count--;
> + }
>
> - ibdev_dbg(&dev->ib_dev, "vport handle %llx pdid %x doorbell_id %x\n",
> - mpc->port_handle, pd->pdn, doorbell_id);
> + mutex_unlock(&pd->vport_mutex);
How does this handle a Protection Domain shared across multiple physical
ports?
Looking at the earlier part of mana_ib_cfg_vport(), if a PD is already
in use, the code increments pd->vport_use_count and returns early:
pd->vport_use_count++;
if (pd->vport_use_count > 1) {
ibdev_dbg(&dev->ib_dev,
"Skip as this PD is already configured vport\n");
mutex_unlock(&pd->vport_mutex);
return 0;
}
If a second port is configured with the same PD, it skips calling
mana_create_eq() for the new port's mpc. Will this cause mpc->eqs to
remain NULL for the second port and fail subsequent queue creations?
[ ... ]
> diff --git a/drivers/infiniband/hw/mana/qp.c b/drivers/infiniband/hw/mana/qp.c
> index 0fbcf449c134b..6167742df9d29 100644
> --- a/drivers/infiniband/hw/mana/qp.c
> +++ b/drivers/infiniband/hw/mana/qp.c
> @@ -171,7 +171,15 @@ static int mana_ib_create_qp_rss(struct ib_qp *ibqp, struct ib_pd *pd,
> cq_spec.gdma_region = cq->queue.gdma_region;
> cq_spec.queue_size = cq->cqe * COMP_ENTRY_SIZE;
> cq_spec.modr_ctx_id = 0;
> - eq = &mpc->ac->eqs[cq->comp_vector];
> + /* EQs are created when a raw QP configures the vport.
> + * A raw QP must be created before creating rwq_ind_tbl.
> + */
> + if (!mpc->eqs) {
> + ret = -EINVAL;
> + i--;
> + goto fail;
> + }
Does this explicit i-- cause a resource leak in the error path?
The fail block cleans up previously created WQ objects using a post-decrement
while loop:
fail:
while (i-- > 0) {
ibwq = ind_tbl->ind_tbl[i];
ibcq = ibwq->cq;
...
mana_destroy_wq_obj(mpc, GDMA_RQ, wq->rx_object);
}
If we manually decrement i before jumping to the fail block, the while loop
will decrement it again prior to evaluation, completely skipping the cleanup
of the WQ object that was just successfully created at the original index i.
> + eq = &mpc->eqs[cq->comp_vector % mpc->num_queues];
What guarantees the lifetime of the mpc->eqs array for active RSS QPs?
Since mana_ib_create_qp_rss() doesn't take a reference count or hold a lock
that prevents mana_destroy_eq() from running, if the Ethernet interface is
brought down or the RAW QP is destroyed, could mana_destroy_eq() free the
array while RSS QPs are still actively using it?
Additionally, can this out-of-bounds read if an ethtool channel update
changes mpc->num_queues while the interface is down?
If a RAW QP is created while the interface is down, mpc->eqs is sized to the
old channel count. If the channel count is then increased, mpc->num_queues
updates but the array isn't reallocated because the interface is down and
queue deallocation was skipped. This could lead to accessing past the end of
the allocation both here and in mana_destroy_eq().
[ ... ]
> diff --git a/drivers/net/ethernet/microsoft/mana/mana_en.c b/drivers/net/ethernet/microsoft/mana/mana_en.c
> index b2faa7cf398f3..cef86c9310543 100644
> --- a/drivers/net/ethernet/microsoft/mana/mana_en.c
> +++ b/drivers/net/ethernet/microsoft/mana/mana_en.c
[ ... ]
> -static void mana_create_eq_debugfs(struct mana_context *ac, int i)
> +static void mana_create_eq_debugfs(struct mana_port_context *apc, int i)
> {
> - struct mana_eq eq = ac->eqs[i];
> + struct mana_eq eq = apc->eqs[i];
> char eqnum[32];
>
> sprintf(eqnum, "eq%d", i);
> - eq.mana_eq_debugfs = debugfs_create_dir(eqnum, ac->mana_eqs_debugfs);
> + eq.mana_eq_debugfs = debugfs_create_dir(eqnum, apc->mana_eqs_debugfs);
> }
Does this update the actual debugfs pointer in the array?
By using struct mana_eq eq = apc->eqs[i];, it creates a local by-value copy
on the stack. The debugfs directory is created, but the pointer in the actual
apc->eqs array remains NULL. Would it be better to use a pointer reference
like struct mana_eq *eq = &apc->eqs[i]; here?
[ ... ]
> -static int mana_create_eq(struct mana_context *ac)
> +int mana_create_eq(struct mana_port_context *apc)
> {
> - struct gdma_dev *gd = ac->gdma_dev;
> + struct gdma_dev *gd = apc->ac->gdma_dev;
> struct gdma_context *gc = gd->gdma_context;
> struct gdma_queue_spec spec = {};
> int err;
> int i;
>
> - ac->eqs = kzalloc_objs(struct mana_eq, gc->max_num_queues);
> - if (!ac->eqs)
> + WARN_ON(apc->eqs);
> + apc->eqs = kzalloc_objs(struct mana_eq, apc->num_queues);
> + if (!apc->eqs)
> return -ENOMEM;
>
> spec.type = GDMA_EQ;
> spec.monitor_avl_buf = false;
> spec.queue_size = EQ_SIZE;
> spec.eq.callback = NULL;
> - spec.eq.context = ac->eqs;
> + spec.eq.context = apc->eqs;
> spec.eq.log2_throttle_limit = LOG2_EQ_THROTTLE;
>
> - ac->mana_eqs_debugfs = debugfs_create_dir("EQs", gc->mana_pci_debugfs);
> + apc->mana_eqs_debugfs = debugfs_create_dir("EQs",
> + apc->mana_port_debugfs);
>
> - for (i = 0; i < gc->max_num_queues; i++) {
> + for (i = 0; i < apc->num_queues; i++) {
> spec.eq.msix_index = (i + 1) % gc->num_msix_usable;
> - err = mana_gd_create_mana_eq(gd, &spec, &ac->eqs[i].eq);
> + err = mana_gd_create_mana_eq(gd, &spec, &apc->eqs[i].eq);
Could there be a race condition between mana_create_eq() and
mana_ib_create_qp_rss() here?
apc->eqs is assigned immediately after allocation, but before the individual
eq structures are populated by mana_gd_create_mana_eq() in this loop.
If an RDMA application concurrently calls mana_ib_create_qp_rss(), it checks
if (!mpc->eqs). That check will pass because the array is allocated, but
accessing eq->eq->id could result in a NULL pointer dereference because the
element has not been fully initialized yet.
^ permalink raw reply
* Re: [PATCH net-next v7 4/6] net: mana: Use GIC functions to allocate global EQs
From: Jakub Kicinski @ 2026-05-12 2:14 UTC (permalink / raw)
To: longli
Cc: Jakub Kicinski, kotaranov, davem, pabeni, edumazet, andrew+netdev,
jgg, leon, haiyangz, kys, wei.liu, decui, shradhagupta, horms,
netdev, linux-rdma, linux-hyperv, linux-kernel
In-Reply-To: <20260507191237.438671-5-longli@microsoft.com>
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
net: mana: Use GIC functions to allocate global EQs
This commit replaces the GDMA global interrupt setup code with the new GIC
allocation and release functions to manage interrupt contexts via refcounting.
> diff --git a/drivers/net/ethernet/microsoft/mana/gdma_main.c b/drivers/net/ethernet/microsoft/mana/gdma_main.c
> index 81bf379283663..72a67b0b2b5d4 100644
> --- a/drivers/net/ethernet/microsoft/mana/gdma_main.c
> +++ b/drivers/net/ethernet/microsoft/mana/gdma_main.c
> @@ -1918,30 +1918,13 @@ static int mana_gd_setup_dyn_irqs(struct pci_dev *pdev, int nvec)
> * further used in irq_setup()
> */
> for (i = 1; i <= nvec; i++) {
> - gic = kzalloc_obj(*gic);
> + gic = mana_gd_get_gic(gc, false, &i);
> if (!gic) {
> err = -ENOMEM;
> goto free_irq;
> }
> - gic->handler = mana_gd_process_eq_events;
> - INIT_LIST_HEAD(&gic->eq_list);
> - spin_lock_init(&gic->lock);
> -
> - snprintf(gic->name, MANA_IRQ_NAME_SZ, "mana_q%d@pci:%s",
> - i - 1, pci_name(pdev));
Does moving the name generation into mana_gd_get_gic() unintentionally change
the IRQ names exposed in /proc/interrupts?
In mana_gd_get_gic(), the names are now generated as "mana_msi%d" using the
raw MSI index:
if (!gic->msi)
snprintf(gic->name, MANA_IRQ_NAME_SZ, "mana_hwc@pci:%s",
pci_name(dev));
else
snprintf(gic->name, MANA_IRQ_NAME_SZ, "mana_msi%d@pci:%s",
gic->msi, pci_name(dev));
This could break userspace tooling (like irqbalance or monitoring scripts) that
might rely on parsing the "mana_q*" pattern to bind network queues.
[ ... ]
> @@ -1963,19 +1946,11 @@ static int mana_gd_setup_dyn_irqs(struct pci_dev *pdev, int nvec)
> kfree(irqs);
> return 0;
>
> -free_current_gic:
> - kfree(gic);
> free_irq:
> for (i -= 1; i > 0; i--) {
> irq = pci_irq_vector(pdev, i);
> - gic = xa_load(&gc->irq_contexts, i);
> - if (WARN_ON(!gic))
> - continue;
> -
> irq_update_affinity_hint(irq, NULL);
> - free_irq(irq, gic);
> - xa_erase(&gc->irq_contexts, i);
> - kfree(gic);
> + mana_gd_put_gic(gc, false, i);
> }
> kfree(irqs);
> return err;
[ ... ]
> @@ -2139,26 +2085,20 @@ static int mana_gd_setup_remaining_irqs(struct pci_dev *pdev)
> static void mana_gd_remove_irqs(struct pci_dev *pdev)
> {
> struct gdma_context *gc = pci_get_drvdata(pdev);
> - struct gdma_irq_context *gic;
> int irq, i;
>
> if (gc->max_num_msix < 1)
> return;
>
> for (i = 0; i < gc->max_num_msix; i++) {
> - irq = pci_irq_vector(pdev, i);
> - if (irq < 0)
> - continue;
> -
> - gic = xa_load(&gc->irq_contexts, i);
> - if (WARN_ON(!gic))
> + if (!xa_load(&gc->irq_contexts, i))
> continue;
>
> /* Need to clear the hint before free_irq */
> + irq = pci_irq_vector(pdev, i);
> irq_update_affinity_hint(irq, NULL);
> - free_irq(irq, gic);
> - xa_erase(&gc->irq_contexts, i);
> - kfree(gic);
> +
> + mana_gd_put_gic(gc, false, i);
> }
Is it safe to unconditionally call irq_update_affinity_hint() in these cleanup
paths?
Now that IRQs can be shared and are reference counted via mana_gd_put_gic(),
unconditionally clearing the affinity hint here could disrupt the routing for
any remaining active users sharing this IRQ. mana_gd_put_gic() already clears
the affinity hint internally when the refcount reaches zero.
Also, since the "if (irq < 0)" check was removed in mana_gd_remove_irqs(), if
pci_irq_vector() returns a negative error like -EINVAL, we would pass a
negative value to irq_update_affinity_hint(), which expects an unsigned int.
^ permalink raw reply
* Re: [PATCH net-next v7 0/6] net: mana: Per-vPort EQ and MSI-X interrupt management
From: Jakub Kicinski @ 2026-05-12 2:15 UTC (permalink / raw)
To: Long Li
Cc: Konstantin Taranov, David S . Miller, Paolo Abeni, Eric Dumazet,
Andrew Lunn, Jason Gunthorpe, Leon Romanovsky, Haiyang Zhang,
K . Y . Srinivasan, Wei Liu, Dexuan Cui, shradhagupta,
Simon Horman, netdev, linux-rdma, linux-hyperv, linux-kernel
In-Reply-To: <20260507191237.438671-1-longli@microsoft.com>
On Thu, 7 May 2026 12:12:31 -0700 Long Li wrote:
> This series adds per-vPort Event Queue (EQ) allocation and MSI-X interrupt
> management for the MANA driver. Previously, all vPorts shared a single set
> of EQs. This change enables dedicated EQs per vPort with support for both
> dedicated and shared MSI-X vector allocation modes.
Once all the AI review comments are address / only false positives
remain - could you pop these patches on a branch and add PR info
to the cover letter so that both RDMA and netdev can pull this?
^ permalink raw reply
* Re: [PATCH v8 2/2] net: mana: force full-page RX buffers via ethtool private flag
From: Jakub Kicinski @ 2026-05-12 2:21 UTC (permalink / raw)
To: dipayanroy
Cc: Jakub Kicinski, kys, haiyangz, wei.liu, decui, andrew+netdev,
davem, edumazet, pabeni, leon, longli, kotaranov, horms,
shradhagupta, ssengar, ernis, shirazsaleem, linux-hyperv, netdev,
linux-kernel, linux-rdma, stephen, jacob.e.keller, dipayanroy,
leitao, kees, john.fastabend, hawk, bpf, daniel, ast, sdf,
yury.norov
In-Reply-To: <20260508115100.488506-3-dipayanroy@linux.microsoft.com>
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
net: mana: force full-page RX buffers via ethtool private flag
This patch adds an ethtool private flag "full-page-rx" that forces one
RX buffer per page, bypassing the page_pool fragment path, to address
a 15-20% throughput regression seen on some 4K PAGE_SIZE ARM64
platforms under high connection counts. Toggling the flag triggers
mana_detach() + mana_attach() to apply the new RX buffer layout.
> diff --git a/drivers/net/ethernet/microsoft/mana/mana_ethtool.c b/drivers/net/ethernet/microsoft/mana/mana_ethtool.c
> index a28ca461c135..dfd222de11d0 100644
> --- a/drivers/net/ethernet/microsoft/mana/mana_ethtool.c
> +++ b/drivers/net/ethernet/microsoft/mana/mana_ethtool.c
[ ... ]
> @@ -590,6 +609,88 @@ static int mana_get_link_ksettings(struct net_device *ndev,
> return 0;
> }
>
[ ... ]
> +static int mana_set_priv_flags(struct net_device *ndev, u32 priv_flags)
> +{
> + struct mana_port_context *apc = netdev_priv(ndev);
> + u32 changed = apc->priv_flags ^ priv_flags;
> + u32 old_priv_flags = apc->priv_flags;
> + bool schedule_port_reset = false;
> + int err = 0;
> +
> + if (!changed)
> + return 0;
> +
> + /* Reject unknown bits */
> + if (priv_flags & ~GENMASK(MANA_PRIV_FLAG_MAX - 1, 0))
> + return -EINVAL;
> +
> + if (changed & BIT(MANA_PRIV_FLAG_USE_FULL_PAGE_RXBUF)) {
> + apc->priv_flags = priv_flags;
> +
> + if (!apc->port_is_up) {
> + /* Port is down, flag updated to apply on next up
> + * so just return.
> + */
> + return 0;
> + }
> +
> + /* Pre-allocate buffers to prevent failure in mana_attach
> + * later
> + */
> + err = mana_pre_alloc_rxbufs(apc, ndev->mtu, apc->num_queues);
> + if (err) {
> + netdev_err(ndev,
> + "Insufficient memory for new allocations\n");
> + apc->priv_flags = old_priv_flags;
> + return err;
> + }
> +
> + err = mana_detach(ndev, false);
> + if (err) {
> + netdev_err(ndev, "mana_detach failed: %d\n", err);
> + apc->priv_flags = old_priv_flags;
> +
> + /* Port is in an inconsistent state. Restore
> + * 'port_is_up' so that queue reset work handler
> + * can properly detach and re-attach.
> + */
> + apc->port_is_up = true;
> + schedule_port_reset = true;
> + goto out;
> + }
> +
> + err = mana_attach(ndev);
> + if (err) {
> + netdev_err(ndev, "mana_attach failed: %d\n", err);
> + apc->priv_flags = old_priv_flags;
> +
> + /* Restore 'port_is_up' so the reset work handler
> + * can properly detach/attach. Without this,
> + * the handler sees port_is_up=false and skips
> + * queue allocation, leaving the port dead.
> + */
> + apc->port_is_up = true;
> + schedule_port_reset = true;
> + }
> + }
> +
> +out:
> + mana_pre_dealloc_rxbufs(apc);
> +
> + if (schedule_port_reset)
> + queue_work(apc->ac->per_port_queue_reset_wq,
> + &apc->queue_reset_work);
> +
> + return err;
> +}
[High]
Can this failure path oops inside the reset work handler?
When mana_attach() fails after mana_detach() has already torn the vport
down, apc->tx_qp has been set to NULL by mana_destroy_txq() and apc->rxqs
has been cleared by mana_cleanup_port_context(). This code then forces
apc->port_is_up back to true and queues apc->queue_reset_work:
apc->port_is_up = true;
schedule_port_reset = true;
When mana_per_port_queue_reset_work_handler runs it calls
mana_detach(ndev, false), which latches port_st_save = port_is_up = true
and, because port_st_save is true, invokes mana_dealloc_queues(). The TX
drain loop there does:
txq = &apc->tx_qp[i].txq;
atomic_read(&txq->pending_sends);
with no NULL check on apc->tx_qp, and mana_destroy_vport further down
iterates apc->rxqs[rxq_idx] while rxqs is NULL. Would that produce a
NULL pointer dereference in the workqueue worker whenever mana_attach()
fails here (for example on an allocation failure in
mana_init_port_context or mana_create_txq, or a transient HW command
failure in mana_query_vport_cfg / mana_cfg_vport)?
The mana_detach() failure path above (goto out) reaches
mana_dealloc_queues() through the same chain and looks to have the same
exposure.
For comparison, mana_change_mtu() handles a mana_attach() failure by
returning the error without scheduling a reset. Would a similar
treatment here avoid the asynchronous oops, or is there a reason the
reset must be scheduled in this specific failure case?
--
pw-bot: cr
^ permalink raw reply
* Re: [PATCH V1 1/3] mshv: Import declarations for irq remap and add irqbypass support
From: Souradeep Chakrabarti @ 2026-05-12 4:59 UTC (permalink / raw)
To: Mukesh R
Cc: hpa, robin.murphy, robh, linux-hyperv, linux-kernel, iommu,
linux-pci, linux-arch
In-Reply-To: <20260512021242.1679786-2-mrathor@linux.microsoft.com>
On Mon, May 11, 2026 at 07:12:40PM -0700, Mukesh R wrote:
> For the irq map/remap hypercalls, copy relevant data structures from
> hypervisor public headers into Linux equivalents. Also, update Kconfig and
> mshv_irqfd for irqbypass. Please note, irqbypass is required for doing
> passthru on MSHV. This because there is really no way of knowing the linux
> irq in the mshv_irqfd_assign and mshv_irqfd_update paths without it. The
> linux irq is setup upfront by VFIO before irqfd assign/update happens.
>
Reviewed-by: Souradeep Chakrabarti <schakrabarti@linux.microsoft.com>
> Signed-off-by: Mukesh R <mrathor@linux.microsoft.com>
> ---
> drivers/hv/Kconfig | 1 +
> drivers/hv/mshv_eventfd.h | 3 +++
> include/hyperv/hvgdk_mini.h | 3 +++
> include/hyperv/hvhdk.h | 17 +++++++++++++++++
> 4 files changed, 24 insertions(+)
>
> diff --git a/drivers/hv/Kconfig b/drivers/hv/Kconfig
> index 7937ac0cbd0f..c831fe25ca2b 100644
> --- a/drivers/hv/Kconfig
> +++ b/drivers/hv/Kconfig
> @@ -75,6 +75,7 @@ config MSHV_ROOT
> # no particular order, making it impossible to reassemble larger pages
> depends on PAGE_SIZE_4KB
> select EVENTFD
> + select IRQ_BYPASS_MANAGER
> select VIRT_XFER_TO_GUEST_WORK
> select HMM_MIRROR
> select MMU_NOTIFIER
> diff --git a/drivers/hv/mshv_eventfd.h b/drivers/hv/mshv_eventfd.h
> index 464c6b81ab33..ff4dd24b8ad4 100644
> --- a/drivers/hv/mshv_eventfd.h
> +++ b/drivers/hv/mshv_eventfd.h
> @@ -9,6 +9,7 @@
> #define __LINUX_MSHV_EVENTFD_H
>
> #include <linux/poll.h>
> +#include <linux/irqbypass.h>
>
> #include "mshv.h"
> #include "mshv_root.h"
> @@ -37,6 +38,8 @@ struct mshv_irqfd {
> struct mshv_irqfd_resampler *irqfd_resampler;
> struct eventfd_ctx *irqfd_resamplefd;
> struct hlist_node irqfd_resampler_hnode;
> + struct irq_bypass_consumer irqfd_bypass_cons;
> + struct irq_bypass_producer *irqfd_bypass_prod;
> };
>
> void mshv_eventfd_init(struct mshv_partition *partition);
> diff --git a/include/hyperv/hvgdk_mini.h b/include/hyperv/hvgdk_mini.h
> index da622fb06440..1ef480825705 100644
> --- a/include/hyperv/hvgdk_mini.h
> +++ b/include/hyperv/hvgdk_mini.h
> @@ -59,6 +59,8 @@ struct hv_u128 {
> #define HV_PARTITION_ID_INVALID ((u64)0)
> #define HV_PARTITION_ID_SELF ((u64)-1)
>
> +#define HV_MAX_VPS 256 /* HV_MAXIMUM_PROCESSORS */
> +
> /* Hyper-V specific model specific registers (MSRs) */
>
> #if defined(CONFIG_X86)
> @@ -508,6 +510,7 @@ union hv_vp_assist_msr_contents { /* HV_REGISTER_VP_ASSIST_PAGE */
> #define HVCALL_UNMAP_VP_STATE_PAGE 0x00e2
> #define HVCALL_GET_VP_STATE 0x00e3
> #define HVCALL_SET_VP_STATE 0x00e4
> +#define HVCALL_GET_VPSET_FROM_MDA 0x00e5
> #define HVCALL_GET_VP_CPUID_VALUES 0x00f4
> #define HVCALL_GET_PARTITION_PROPERTY_EX 0x0101
> #define HVCALL_MMIO_READ 0x0106
> diff --git a/include/hyperv/hvhdk.h b/include/hyperv/hvhdk.h
> index 5e83d3714966..d0a892347ab1 100644
> --- a/include/hyperv/hvhdk.h
> +++ b/include/hyperv/hvhdk.h
> @@ -952,4 +952,21 @@ struct hv_input_modify_sparse_spa_page_host_access {
> #define HV_MODIFY_SPA_PAGE_HOST_ACCESS_LARGE_PAGE 0x4
> #define HV_MODIFY_SPA_PAGE_HOST_ACCESS_HUGE_PAGE 0x8
>
> +#ifdef CONFIG_X86
> +
> +struct hv_input_get_vp_set_from_mda { /* HV_OUTPUT_GET_VP_SET_FROM_MDA */
> + u64 target_partid;
> + u64 dest_address;
> + u8 input_vtl;
> + u8 destmode_logical; /* true => mode is logical */
> + u16 reserved0; /* mbz */
> + u32 reserved1; /* mbz */
> +} __packed;
> +
> +union hv_output_get_vp_set_from_mda { /* HV_OUTPUT_GET_VP_SET_FROM_MDA */
> + struct hv_vpset target_vpset;
> + u64 bitset_buffer[HV_GENERIC_SET_QWORD_COUNT(HV_MAX_VPS)];
> +} __packed;
> +
> +#endif /* CONFIG_X86 */
> #endif /* _HV_HVHDK_H */
> --
> 2.51.2.vfs.0.1
>
^ permalink raw reply
* Re: [PATCH 1/3] mm/hmm: Add hmm_range_fault_unlockable() for mmap lock-drop support
From: David Hildenbrand (Arm) @ 2026-05-12 8:42 UTC (permalink / raw)
To: Stanislav Kinsburskii, kys, Liam.Howlett, akpm, decui, haiyangz,
jgg, corbet, leon, longli, ljs, mhocko, rppt, shuah, skhan,
surenb, vbabka, wei.liu
Cc: linux-doc, linux-hyperv, linux-kernel, linux-kselftest, linux-mm
In-Reply-To: <177759840859.221039.13065406062747296947.stgit@skinsburskii-cloud-desktop.internal.cloudapp.net>
> + for (; addr < end; addr += PAGE_SIZE) {
> + vm_fault_t ret;
> +
> + ret = handle_mm_fault(vma, addr, fault_flags, NULL);
> +
> + if (ret & (VM_FAULT_RETRY | VM_FAULT_COMPLETED)) {
> + /*
> + * The mmap lock has been dropped by the fault handler.
> + * Record the failing address and signal lock-drop to
> + * the caller.
> + */
> + *hmm_vma_walk->locked = 0;
> + hmm_vma_walk->last = addr;
> + return -EAGAIN;
Okay, so we'll return straight from hmm_vma_fault() to
hmm_vma_handle_pte()/hmm_vma_walk_pmd() -> walk_page_range() machinery.
Hopefully we don't refer to the MM/VMA on any path there? It would be nicer if
the hmm_vma_fault() could be called by the caller of walk_page_range(), but
that's tricky I guess, as hmm_vma_fault() consumes the walk structure and
requires the vma in there.
Note: am I wrong, or is hmm_vma_fault() really always called with
required_fault=true?
> + }
> +
> + if (ret & VM_FAULT_ERROR)
> return -EFAULT;
> + }
> return -EBUSY;
> }
>
> @@ -566,6 +585,17 @@ static int hmm_vma_walk_hugetlb_entry(pte_t *pte, unsigned long hmask,
> if (required_fault) {
> int ret;
>
> + /*
> + * Faulting hugetlb pages on the unlockable path is not
> + * supported. The walk framework holds hugetlb_vma_lock_read
> + * which must be dropped before handle_mm_fault, but if the
> + * mmap lock is also dropped (VM_FAULT_RETRY), the vma may
> + * be freed and the walk framework's unconditional unlock
> + * becomes a use-after-free.
> + */
> + if (hmm_vma_walk->locked)
> + return -EFAULT;
Just because it's unlockable doesn't mean that you must unlock. Can't this be
kept working as is, just simulating here as if it would not be unlockable?
--
Cheers,
David
^ permalink raw reply
* Re: [PATCH V1 2/3] hyperv: Implement irq remap for passthru devices
From: Souradeep Chakrabarti @ 2026-05-12 9:29 UTC (permalink / raw)
To: Mukesh R
Cc: hpa, robin.murphy, robh, linux-hyperv, linux-kernel, iommu,
linux-pci, linux-arch
In-Reply-To: <20260512021242.1679786-3-mrathor@linux.microsoft.com>
On Mon, May 11, 2026 at 07:12:41PM -0700, Mukesh R wrote:
> Implement interrupt remapping for direct attached and domain attached
> devices on Hyper-V.
>
> Please note there are few constraints when it comes to mapping device
> interrupts on Hyper-V. For example, the hypervisor will not allow mapping
> device interrupts to root if the device is a direct attached device. Since
> the target guest cpu and vector info is not available during the initial
> VFIO irq setup, we work around by skipping this initial map. Then later
> during irqbypass trigger, when both guest target cpu vector are available,
> we do the map in the hypervisor, update the device, and enable the
> interrupt vector on the device. Rather than special case direct attached,
> we do same for domain attached also. This implies irqbypass is required
> for MSHV pci device passthru. Also noteworthy is that the hypervisor
> will automatically setup any direct hw injection like posted interrupts.
>
Reviewed-by: Souradeep Chakrabarti <schakrabarti@linux.microsoft.com>
> Signed-off-by: Mukesh R <mrathor@linux.microsoft.com>
> ---
> arch/x86/hyperv/irqdomain.c | 18 +-
> drivers/hv/mshv_eventfd.c | 423 +++++++++++++++++++++++++++-
> drivers/iommu/hyperv-iommu-root.c | 14 +
> drivers/pci/controller/pci-hyperv.c | 10 +
> include/asm-generic/mshyperv.h | 3 +
> 5 files changed, 464 insertions(+), 4 deletions(-)
>
> diff --git a/arch/x86/hyperv/irqdomain.c b/arch/x86/hyperv/irqdomain.c
> index 8780573a4332..02f9a889c014 100644
> --- a/arch/x86/hyperv/irqdomain.c
> +++ b/arch/x86/hyperv/irqdomain.c
> @@ -197,7 +197,7 @@ int hv_map_msi_interrupt(struct irq_data *data,
>
> msidesc = irq_data_get_msi_desc(data);
> pdev = msi_desc_to_pci_dev(msidesc);
> - hv_devid.as_uint64 = hv_build_devid_type_pci(pdev);
> + hv_devid.as_uint64 = hv_devid_from_pdev(pdev);
> cpu = cpumask_first(irq_data_get_effective_affinity_mask(data));
>
> return hv_map_interrupt(hv_devid, false, cpu, cfg->vector,
> @@ -233,6 +233,20 @@ static void hv_irq_compose_msi_msg(struct irq_data *data, struct msi_msg *msg)
> return;
> }
>
> + /*
> + * For direct attached devices, we cannot map interrupts in the
> + * hypervisor because it will not allow it until we have guest target
> + * vcpu and vector. So defer it until irqbypass. Also, do the same
> + * for domain attached devices for simplicity.
> + */
> + if (hv_pcidev_is_pthru_dev(pdev)) {
> + if (data->chip_data)
> + entry_to_msi_msg(data->chip_data, msg);
> + else
> + memset(msg, 0, sizeof(struct msi_msg));
> + return;
> + }
> +
> if (data->chip_data) {
> /*
> * This interrupt is already mapped. Let's unmap first.
> @@ -272,7 +286,7 @@ static int hv_unmap_msi_interrupt(struct pci_dev *pdev,
> {
> union hv_device_id hv_devid;
>
> - hv_devid.as_uint64 = hv_build_devid_type_pci(pdev);
> + hv_devid.as_uint64 = hv_devid_from_pdev(pdev);
> return hv_unmap_interrupt(hv_devid.as_uint64, irq_entry);
> }
>
> diff --git a/drivers/hv/mshv_eventfd.c b/drivers/hv/mshv_eventfd.c
> index 90959f639dc3..1f5c1e9ee9b7 100644
> --- a/drivers/hv/mshv_eventfd.c
> +++ b/drivers/hv/mshv_eventfd.c
> @@ -7,7 +7,6 @@
> *
> * All credits to kvm developers.
> */
> -
> #include <linux/syscalls.h>
> #include <linux/wait.h>
> #include <linux/poll.h>
> @@ -15,7 +14,8 @@
> #include <linux/list.h>
> #include <linux/workqueue.h>
> #include <linux/eventfd.h>
> -
> +#include <linux/pci.h>
> +#include <linux/vfio_pci_core.h>
> #if IS_ENABLED(CONFIG_X86_64)
> #include <asm/apic.h>
> #endif
> @@ -27,6 +27,377 @@
>
> static struct workqueue_struct *irqfd_cleanup_wq;
>
> +#if IS_ENABLED(CONFIG_X86_64)
> +
> +static int mshv_parse_mshv_irqfd(struct mshv_irqfd *irqfd,
> + struct pci_dev **out_pdev,
> + struct irq_data **out_irqdata)
> +{
> + struct irq_bypass_producer *prod;
> + struct msi_desc *msidesc;
> + struct irq_data *irqdata;
> +
> + if (irqfd == NULL || irqfd->irqfd_bypass_prod == NULL)
> + return -ENODEV;
> +
> + prod = irqfd->irqfd_bypass_prod;
> +
> + irqdata = irq_get_irq_data(prod->irq);
> + if (irqdata == NULL) {
> + pr_err("Hyper-V: irqbypass fail, no irqdata. irq:0x%x\n",
> + prod->irq);
> + return -EINVAL;
> + }
> + *out_irqdata = irqdata;
> +
> + msidesc = irq_data_get_msi_desc(irqdata);
> + if (msidesc == NULL) {
> + pr_err("Hyper-V: irqbypass msi fail. irq:0x%x\n", prod->irq);
> + return -EINVAL;
> + }
> +
> + *out_pdev = msi_desc_to_pci_dev(msidesc);
> + if (*out_pdev == NULL) {
> + pr_err("Hyper-V: mshv_irqfd parse fail. irq:0x%x\n", prod->irq);
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> +/* Must be called with interrupts disabled */
> +static int hv_vpset_from_hyp_disabled(
> + struct hv_input_get_vp_set_from_mda *input,
> + union hv_output_get_vp_set_from_mda *output,
> + struct mshv_lapic_irq *lapic_irq, u64 partid)
> +{
> + u64 status;
> +
> + memset(input, 0, sizeof(*input));
> + input->target_partid = partid;
> + input->dest_address = lapic_irq->lapic_apic_id;
> + input->input_vtl = 0;
> + input->destmode_logical = lapic_irq->lapic_control.logical_dest_mode;
> +
> + status = hv_do_hypercall(HVCALL_GET_VPSET_FROM_MDA, input, output);
> + if (!hv_result_success(status)) {
> + hv_status_err(status, "apicid:0x%llx dest:0x%x\n",
> + lapic_irq->lapic_apic_id,
> + lapic_irq->lapic_control.logical_dest_mode);
> + }
> +
> + return hv_result_to_errno(status);
> +}
> +
> +/* Returns number of banks copied, -errno in case of error */
> +static int hv_copy_vpset(struct hv_vpset *dest, struct hv_vpset *src)
> +{
> + u64 bank_mask;
> + int banks, tot_banks = hv_max_vp_index / HV_VCPUS_PER_SPARSE_BANK;
> +
> + if (tot_banks >= HV_MAX_SPARSE_VCPU_BANKS)
> + return -EINVAL;
> +
> + dest->format = src->format;
> + dest->valid_bank_mask = src->valid_bank_mask;
> + bank_mask = src->valid_bank_mask;
> + for (banks = 0; banks <= tot_banks; banks++) {
> + if (bank_mask == 0)
> + break;
> +
> + if (bank_mask & 1)
> + dest->bank_contents[banks] = src->bank_contents[banks];
> + bank_mask = bank_mask >> 1;
> + }
> +
> + return banks;
> +}
> +
> +static int mshv_map_device_interrupt(u64 ptid, union hv_device_id hv_devid,
> + struct mshv_lapic_irq *ginfo,
> + struct hv_interrupt_entry *ret_entry,
> + u64 *ret_status)
> +{
> + struct hv_input_map_device_interrupt *irq_input;
> + struct hv_output_map_device_interrupt *irq_output;
> + struct hv_device_interrupt_descriptor *intdesc;
> + struct hv_input_get_vp_set_from_mda *mda_input;
> + union hv_output_get_vp_set_from_mda *mda_output;
> + ulong flags;
> + u64 status;
> + int rc, var_size;
> +
> + *ret_status = U64_MAX;
> + local_irq_save(flags);
> +
> + mda_input = *this_cpu_ptr(hyperv_pcpu_input_arg);
> + mda_output = *this_cpu_ptr(hyperv_pcpu_output_arg);
> +
> + /*
> + * Map Device Interrupt hcall needs vp set based on vp indexes used
> + * during vp creation. Here we have lapic-id of the vp only. Easiest
> + * is to just ask the hypervisor for the vp set matching the lapic-id.
> + */
> + rc = hv_vpset_from_hyp_disabled(mda_input, mda_output, ginfo, ptid);
> + if (rc)
> + goto out; /* error already printed */
> +
> + irq_input = *this_cpu_ptr(hyperv_pcpu_input_arg);
> + irq_output = *this_cpu_ptr(hyperv_pcpu_output_arg);
> + memset(irq_input, 0, sizeof(*irq_input));
> +
> + irq_input->partition_id = ptid;
> + irq_input->device_id = hv_devid.as_uint64;
> +
> + intdesc = &irq_input->interrupt_descriptor;
> + intdesc->interrupt_type = HV_X64_INTERRUPT_TYPE_FIXED;
> + intdesc->vector_count = 1;
> + intdesc->target.vector = ginfo->lapic_vector;
> + intdesc->trigger_mode = HV_INTERRUPT_TRIGGER_MODE_EDGE;
> +
> + intdesc->target.vp_set.valid_bank_mask = 0;
> + intdesc->target.vp_set.format = HV_GENERIC_SET_SPARSE_4K;
> + intdesc->target.flags = HV_DEVICE_INTERRUPT_TARGET_PROCESSOR_SET;
> + rc = hv_copy_vpset(&intdesc->target.vp_set, &mda_output->target_vpset);
> + if (rc <= 0) {
> + pr_err("Hyper-V: ptid %lld - (irq)vpset copy failed (%d)\n",
> + ptid, rc);
> + goto out;
> + }
> +
> + /*
> + * var-sized hcall: var-size starts after vp_mask (thus vp_set.format
> + * does not count, but vp_set.valid_bank_mask does).
> + */
> + var_size = rc + 1;
> + status = hv_do_rep_hypercall(HVCALL_MAP_DEVICE_INTERRUPT, 0, var_size,
> + irq_input, irq_output);
> + *ret_entry = irq_output->interrupt_entry;
> + local_irq_restore(flags);
> +
> + rc = 0;
> + if (!hv_result_success(status)) {
> + if (hv_result(status) != HV_STATUS_INSUFFICIENT_MEMORY)
> + hv_status_err(status, "pt:%lld vec:%d lapic-id:%lld\n",
> + ptid, ginfo->lapic_vector, ginfo->lapic_apic_id);
> + *ret_status = status;
> + rc = hv_result_to_errno(status);
> + }
> +
> + return rc;
> +
> +out:
> + local_irq_restore(flags);
> + return rc;
> +
> +}
> +
> +static int mshv_unmap_device_interrupt(union hv_device_id hv_devid,
> + struct hv_interrupt_entry *irq_entry)
> +{
> + unsigned long flags;
> + struct hv_input_unmap_device_interrupt *input;
> + u64 status;
> +
> + local_irq_save(flags);
> + input = *this_cpu_ptr(hyperv_pcpu_input_arg);
> + memset(input, 0, sizeof(*input));
> +
> + if (hv_devid.device_type == HV_DEVICE_TYPE_LOGICAL)
> + input->partition_id = hv_get_current_partid();
> + else
> + input->partition_id = hv_current_partition_id;
> +
> + input->device_id = hv_devid.as_uint64;
> + input->interrupt_entry = *irq_entry;
> +
> + status = hv_do_hypercall(HVCALL_UNMAP_DEVICE_INTERRUPT, input, NULL);
> + local_irq_restore(flags);
> +
> + if (!hv_result_success(status))
> + hv_status_err(status, "\n");
> +
> + return hv_result_to_errno(status);
> +}
> +
> +static int mshv_chk_unmap_irq(union hv_device_id hv_devid,
> + struct irq_data *irqdata)
> +{
> + int rc;
> +
> + if (irqdata->chip_data == NULL)
> + return 0;
> +
> + rc = mshv_unmap_device_interrupt(hv_devid, irqdata->chip_data);
> + if (rc)
> + return rc;
> +
> + kfree(irqdata->chip_data);
> + irqdata->chip_data = NULL;
> +
> + return 0;
> +}
> +
> +/*
> + * Synchronize device update with VFIO.
> + * See: vfio_pci_memory_lock_and_enable()
> + */
> +static u16 mshv_pci_memory_lock_and_enable(struct vfio_pci_core_device *cdev)
> +{
> + u16 cmd;
> +
> + down_write(&cdev->memory_lock);
> + pci_read_config_word(cdev->pdev, PCI_COMMAND, &cmd);
> + if (!(cmd & PCI_COMMAND_MEMORY))
> + pci_write_config_word(cdev->pdev, PCI_COMMAND,
> + cmd | PCI_COMMAND_MEMORY);
> + return cmd;
> +}
> +
> +static void mshv_pci_memory_unlock_and_restore(
> + struct vfio_pci_core_device *cdev,
> + u16 cmd)
> +{
> + pci_write_config_word(cdev->pdev, PCI_COMMAND, cmd);
> + up_write(&cdev->memory_lock);
> +}
> +
> +static void mshv_make_device_usable(struct pci_dev *pdev, int vector,
> + struct hv_interrupt_entry *hv_entry)
> +{
> + int lirq;
> + struct msi_msg msimsg;
> + struct irq_data *irqdata, *parent;
> + u16 pcicmd;
> + struct vfio_pci_core_device *coredev = dev_get_drvdata(&pdev->dev);
> +
> + if (pdev->dev.driver == NULL ||
> + strcmp(pdev->dev.driver->name, "vfio-pci") != 0) {
> + pr_err("Hyper-V: irqbypass: non vfio device %s\n",
> + pci_name(pdev));
> + return;
> + }
> + if (coredev == NULL) {
> + pr_err("Hyper-V: irqbypass: null vfio device for %s\n",
> + pci_name(pdev));
> + return;
> + }
> +
> + if (hv_entry->source != HV_INTERRUPT_SOURCE_MSI) {
> + pr_err("Hyper-V: %s irq source not msi\n", pci_name(pdev));
> + return;
> + }
> +
> + lirq = pci_irq_vector(pdev, vector);
> + irqdata = irq_get_irq_data(lirq);
> + if (irqdata == NULL) {
> + pr_err("Hyper-V: null irq_data for write msimsg. lirq:0x%x\n",
> + lirq);
> + return;
> + }
> +
> + msimsg.address_hi = 0;
> + msimsg.address_lo = hv_entry->msi_entry.address.as_uint32;
> + msimsg.data = hv_entry->msi_entry.data.as_uint32;
> +
> + pcicmd = mshv_pci_memory_lock_and_enable(coredev);
> + pci_write_msi_msg(lirq, &msimsg);
> + mshv_pci_memory_unlock_and_restore(coredev, pcicmd);
> +
> + pci_msi_unmask_irq(irqdata);
> +
> + parent = irqdata->parent_data;
> + if (parent && parent->chip && parent->chip->irq_unmask)
> + irq_chip_unmask_parent(irqdata);
> +}
> +
> +/*
> + * This guest has a device passthru'd to it. VFIO did the initial setup of
> + * the device interrupts, but we left them unmapped in the hypervisor
> + * because we didn't have the guest target cpu and vector (required by
> + * hypervisor). We have them now, so do the map hypercall.
> + * Also, when here, it is expected that the device global mask is unset
> + * but individual MSI/x masks are set. Goal here is to map the interrupt in
> + * the hypervisor, update the corresponding device MSI/x entry, and enable it.
> + */
> +static void mshv_pthru_dev_irq_remap(struct mshv_irqfd *irqfd)
> +{
> + u64 ptid, status;
> + struct pci_dev *pdev;
> + int rc, deposit_pgs = 16;
> + struct mshv_lapic_irq *ginfo = &irqfd->irqfd_lapic_irq;
> + union hv_device_id hv_devid;
> + struct hv_interrupt_entry *new_entry;
> + struct irq_data *irqdata;
> +
> + if (!irqfd->irqfd_girq_ent.girq_entry_valid ||
> + irqfd->irqfd_bypass_prod == NULL)
> + return;
> +
> + rc = mshv_parse_mshv_irqfd(irqfd, &pdev, &irqdata);
> + if (rc)
> + return;
> +
> + hv_devid.as_uint64 = hv_devid_from_pdev(pdev);
> +
> + rc = mshv_chk_unmap_irq(hv_devid, irqdata);
> + if (rc)
> + return;
> +
> + new_entry = kmalloc(sizeof(*new_entry), GFP_ATOMIC);
> + if (new_entry == NULL)
> + return;
> +
> + ptid = irqfd->irqfd_partn->pt_id;
> +
> + while (deposit_pgs--) {
> + rc = mshv_map_device_interrupt(ptid, hv_devid, ginfo, new_entry,
> + &status);
> + if (rc == 0)
> + break;
> + if (hv_result(status) != HV_STATUS_INSUFFICIENT_MEMORY)
> + break;
> +
> + rc = hv_call_deposit_pages(NUMA_NO_NODE, ptid, 1);
> + if (rc)
> + break;
> + }
> + if (rc) {
> + kfree(new_entry);
> + return;
> + }
> +
> + irqdata->chip_data = new_entry;
> +
> + mshv_make_device_usable(pdev, irqdata->hwirq, new_entry);
> +}
> +
> +static void mshv_pthru_dev_irq_undo(struct mshv_irqfd *irqfd)
> +{
> + struct pci_dev *pdev;
> + union hv_device_id hv_devid;
> + struct irq_data *irqdata;
> + int rc;
> +
> + if (!irqfd->irqfd_girq_ent.girq_entry_valid ||
> + irqfd->irqfd_bypass_prod == NULL)
> + return;
> +
> + rc = mshv_parse_mshv_irqfd(irqfd, &pdev, &irqdata);
> + if (rc)
> + return;
> +
> + hv_devid.as_uint64 = hv_devid_from_pdev(pdev);
> + mshv_chk_unmap_irq(hv_devid, irqdata);
> +}
> +
> +#else /* IS_ENABLED(CONFIG_X86_64) */
> +
> +static void mshv_pthru_dev_irq_remap(struct mshv_irqfd *irqfd) { }
> +static void mshv_pthru_dev_irq_undo(struct mshv_irqfd *irqfd) { }
> +
> +#endif /* IS_ENABLED(CONFIG_X86_64) */
> +
> void mshv_register_irq_ack_notifier(struct mshv_partition *partition,
> struct mshv_irq_ack_notifier *mian)
> {
> @@ -264,6 +635,7 @@ static void mshv_irqfd_shutdown(struct work_struct *work)
> /*
> * It is now safe to release the object's resources
> */
> + irq_bypass_unregister_consumer(&irqfd->irqfd_bypass_cons);
> eventfd_ctx_put(irqfd->irqfd_eventfd_ctx);
> kfree(irqfd);
> }
> @@ -286,6 +658,12 @@ static void mshv_irqfd_deactivate(struct mshv_irqfd *irqfd)
>
> hlist_del(&irqfd->irqfd_hnode);
>
> + /*
> + * Cleanup interrupt map (kfree chip_data) while in a VMM thread as
> + * unmap needs partition id. mshv_irqfd_shutdown() runs in a kthread.
> + */
> + mshv_pthru_dev_irq_undo(irqfd);
> +
> queue_work(irqfd_cleanup_wq, &irqfd->irqfd_shutdown);
> }
>
> @@ -383,6 +761,45 @@ static void mshv_irqfd_queue_proc(struct file *file, wait_queue_head_t *wqh,
> add_wait_queue_priority(wqh, &irqfd->irqfd_wait);
> }
>
> +static int mshv_irq_bypass_add_producer(struct irq_bypass_consumer *cons,
> + struct irq_bypass_producer *prod)
> +{
> + struct mshv_irqfd *irqfd;
> +
> + irqfd = container_of(cons, struct mshv_irqfd, irqfd_bypass_cons);
> + irqfd->irqfd_bypass_prod = prod;
> +
> + mshv_pthru_dev_irq_remap(irqfd);
> +
> + return 0;
> +}
> +
> +static void mshv_irq_bypass_del_producer(struct irq_bypass_consumer *cons,
> + struct irq_bypass_producer *prod)
> +{
> + struct mshv_irqfd *irqfd;
> +
> + irqfd = container_of(cons, struct mshv_irqfd, irqfd_bypass_cons);
> +
> + WARN_ON(irqfd->irqfd_bypass_prod != prod);
> + irqfd->irqfd_bypass_prod = NULL;
> +
> +}
> +
> +static void mshv_setup_irq_bypass(struct mshv_irqfd *irqfd,
> + struct eventfd_ctx *eventfd)
> +{
> + struct irq_bypass_consumer *consumer = &irqfd->irqfd_bypass_cons;
> + int rc;
> +
> + consumer->add_producer = mshv_irq_bypass_add_producer;
> + consumer->del_producer = mshv_irq_bypass_del_producer;
> + rc = irq_bypass_register_consumer(&irqfd->irqfd_bypass_cons, eventfd);
> + if (rc)
> + pr_err("Hyper-V: irq bypass consumer registration failed: %d\n",
> + rc);
> +}
> +
> static int mshv_irqfd_assign(struct mshv_partition *pt,
> struct mshv_user_irqfd *args)
> {
> @@ -509,6 +926,8 @@ static int mshv_irqfd_assign(struct mshv_partition *pt,
> if (events & EPOLLIN)
> mshv_assert_irq_slow(irqfd);
>
> + mshv_setup_irq_bypass(irqfd, eventfd);
> +
> srcu_read_unlock(&pt->pt_irq_srcu, idx);
> return 0;
>
> diff --git a/drivers/iommu/hyperv-iommu-root.c b/drivers/iommu/hyperv-iommu-root.c
> index a2e0f6cc78e6..dc270b0a80d9 100644
> --- a/drivers/iommu/hyperv-iommu-root.c
> +++ b/drivers/iommu/hyperv-iommu-root.c
> @@ -217,6 +217,20 @@ u64 hv_build_devid_oftype(struct pci_dev *pdev, enum hv_device_type type)
> }
> EXPORT_SYMBOL_GPL(hv_build_devid_oftype);
>
> +/* Build device id for the interrupt path */
> +u64 hv_devid_from_pdev(struct pci_dev *pdev)
> +{
> + enum hv_device_type dev_type;
> +
> + if (hv_pcidev_is_attached_dev(pdev))
> + dev_type = HV_DEVICE_TYPE_LOGICAL;
> + else
> + dev_type = HV_DEVICE_TYPE_PCI;
> +
> + return hv_build_devid_oftype(pdev, dev_type);
> +}
> +EXPORT_SYMBOL_GPL(hv_devid_from_pdev);
> +
> /* Create a new device domain in the hypervisor */
> static int hv_iommu_create_hyp_devdom(struct hv_domain *hvdom)
> {
> diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci-hyperv.c
> index 50d793ca8f31..702a8005651b 100644
> --- a/drivers/pci/controller/pci-hyperv.c
> +++ b/drivers/pci/controller/pci-hyperv.c
> @@ -1744,6 +1744,16 @@ static void hv_irq_mask(struct irq_data *data)
>
> static void hv_irq_unmask(struct irq_data *data)
> {
> + struct pci_dev *pdev;
> + struct msi_desc *msi_desc;
> +
> + msi_desc = irq_data_get_msi_desc(data);
> + pdev = msi_desc_to_pci_dev(msi_desc);
> +
> + /* Done during bypass setup in mshv_eventfd.c: mshv_irqfd_assign() */
> + if (hv_pcidev_is_pthru_dev(pdev))
> + return;
> +
> hv_arch_irq_unmask(data);
>
> if (data->parent_data->chip->irq_unmask)
> diff --git a/include/asm-generic/mshyperv.h b/include/asm-generic/mshyperv.h
> index 8d5c610da99a..88b3aba6691c 100644
> --- a/include/asm-generic/mshyperv.h
> +++ b/include/asm-generic/mshyperv.h
> @@ -332,6 +332,7 @@ u64 hv_get_current_partid(void);
> bool hv_pcidev_is_attached_dev(struct pci_dev *pdev);
> bool hv_pcidev_is_pthru_dev(struct pci_dev *pdev);
> u64 hv_build_devid_oftype(struct pci_dev *pdev, enum hv_device_type type);
> +u64 hv_devid_from_pdev(struct pci_dev *pdev);
> #else
> static inline bool hv_pcidev_is_attached_dev(struct pci_dev *pdev)
> { return false; }
> @@ -340,6 +341,8 @@ static inline bool hv_pcidev_is_pthru_dev(struct pci_dev *pdev)
> static inline u64 hv_build_devid_oftype(struct pci_dev *pdev,
> enum hv_device_type type)
> { return 0; }
> +static inline u64 hv_devid_from_pdev(struct pci_dev *pdev)
> +{ return 0; }
> static inline u64 hv_get_current_partid(void)
> { return HV_PARTITION_ID_INVALID; }
> #endif /* IS_ENABLED(CONFIG_HYPERV_IOMMU) */
> --
> 2.51.2.vfs.0.1
>
^ permalink raw reply
* Re: [PATCH V3 01/11] iommu/hyperv: Rename hyperv-iommu.c to hyperv-irq.c
From: Souradeep Chakrabarti @ 2026-05-12 10:26 UTC (permalink / raw)
To: Mukesh R
Cc: hpa, robin.murphy, robh, wei.liu, mhklinux, muislam, namjain,
magnuskulke, anbelski, linux-kernel, linux-hyperv, iommu,
linux-pci, linux-arch, kys, haiyangz, decui, longli, tglx, mingo,
bp, dave.hansen, x86, joro, will, lpieralisi, kwilczynski,
bhelgaas, arnd, jacob.pan
In-Reply-To: <20260512020259.1678627-2-mrathor@linux.microsoft.com>
On Mon, May 11, 2026 at 07:02:49PM -0700, Mukesh R wrote:
> This file actually implements irq remapping, so rename to more appropriate
> hyperv-irq.c. A new file to implement hyperv iommu will be introduced
> later. Also, it should not be tied to HYPERV_IOMMU, but to CONFIG_HYPERV
> and IRQ_REMAP. The file already has #ifdef CONFIG_IRQ_REMAP.
>
> Reviewed-by: Anirudh Rayabharam (Microsoft) <anirudh@anirudhrb.com>
Reviewed-by: Souradeep Chakrabarti <schakrabarti@linux.microsoft.com>
> Signed-off-by: Mukesh R <mrathor@linux.microsoft.com>
> ---
> MAINTAINERS | 2 +-
> drivers/iommu/Makefile | 2 +-
> drivers/iommu/{hyperv-iommu.c => hyperv-irq.c} | 6 +++---
> drivers/iommu/irq_remapping.c | 2 +-
> 4 files changed, 6 insertions(+), 6 deletions(-)
> rename drivers/iommu/{hyperv-iommu.c => hyperv-irq.c} (99%)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index d1cc0e12fe1f..f803a6a38fee 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -11914,7 +11914,7 @@ F: drivers/clocksource/hyperv_timer.c
> F: drivers/hid/hid-hyperv.c
> F: drivers/hv/
> F: drivers/input/serio/hyperv-keyboard.c
> -F: drivers/iommu/hyperv-iommu.c
> +F: drivers/iommu/hyperv-irq.c
> F: drivers/net/ethernet/microsoft/
> F: drivers/net/hyperv/
> F: drivers/pci/controller/pci-hyperv-intf.c
> diff --git a/drivers/iommu/Makefile b/drivers/iommu/Makefile
> index 0275821f4ef9..335ea77cced6 100644
> --- a/drivers/iommu/Makefile
> +++ b/drivers/iommu/Makefile
> @@ -30,7 +30,7 @@ obj-$(CONFIG_TEGRA_IOMMU_SMMU) += tegra-smmu.o
> obj-$(CONFIG_EXYNOS_IOMMU) += exynos-iommu.o
> obj-$(CONFIG_FSL_PAMU) += fsl_pamu.o fsl_pamu_domain.o
> obj-$(CONFIG_S390_IOMMU) += s390-iommu.o
> -obj-$(CONFIG_HYPERV_IOMMU) += hyperv-iommu.o
> +obj-$(CONFIG_HYPERV) += hyperv-irq.o
> obj-$(CONFIG_VIRTIO_IOMMU) += virtio-iommu.o
> obj-$(CONFIG_IOMMU_SVA) += iommu-sva.o
> obj-$(CONFIG_IOMMU_IOPF) += io-pgfault.o
> diff --git a/drivers/iommu/hyperv-iommu.c b/drivers/iommu/hyperv-irq.c
> similarity index 99%
> rename from drivers/iommu/hyperv-iommu.c
> rename to drivers/iommu/hyperv-irq.c
> index 479103261ae6..d11076f906fb 100644
> --- a/drivers/iommu/hyperv-iommu.c
> +++ b/drivers/iommu/hyperv-irq.c
> @@ -8,6 +8,8 @@
> * Author : Lan Tianyu <Tianyu.Lan@microsoft.com>
> */
>
> +#ifdef CONFIG_IRQ_REMAP
> +
> #include <linux/types.h>
> #include <linux/interrupt.h>
> #include <linux/irq.h>
> @@ -24,8 +26,6 @@
>
> #include "irq_remapping.h"
>
> -#ifdef CONFIG_IRQ_REMAP
> -
> /*
> * According 82093AA IO-APIC spec , IO APIC has a 24-entry Interrupt
> * Redirection Table. Hyper-V exposes one single IO-APIC and so define
> @@ -331,4 +331,4 @@ static const struct irq_domain_ops hyperv_root_ir_domain_ops = {
> .free = hyperv_root_irq_remapping_free,
> };
>
> -#endif
> +#endif /* CONFIG_IRQ_REMAP */
> diff --git a/drivers/iommu/irq_remapping.c b/drivers/iommu/irq_remapping.c
> index c2443659812a..41bf65e4ea88 100644
> --- a/drivers/iommu/irq_remapping.c
> +++ b/drivers/iommu/irq_remapping.c
> @@ -108,7 +108,7 @@ int __init irq_remapping_prepare(void)
> else if (IS_ENABLED(CONFIG_AMD_IOMMU) &&
> amd_iommu_irq_ops.prepare() == 0)
> remap_ops = &amd_iommu_irq_ops;
> - else if (IS_ENABLED(CONFIG_HYPERV_IOMMU) &&
> + else if (IS_ENABLED(CONFIG_HYPERV) &&
> hyperv_irq_remap_ops.prepare() == 0)
> remap_ops = &hyperv_irq_remap_ops;
> else
> --
> 2.51.2.vfs.0.1
>
^ permalink raw reply
* Re: [PATCH V3 04/11] mshv: Declarations and definitions for VFIO-MSHV bridge device
From: Souradeep Chakrabarti @ 2026-05-12 10:26 UTC (permalink / raw)
To: Mukesh R
Cc: hpa, robin.murphy, robh, wei.liu, mhklinux, muislam, namjain,
magnuskulke, anbelski, linux-kernel, linux-hyperv, iommu,
linux-pci, linux-arch, kys, haiyangz, decui, longli, tglx, mingo,
bp, dave.hansen, x86, joro, will, lpieralisi, kwilczynski,
bhelgaas, arnd, jacob.pan
In-Reply-To: <20260512020259.1678627-5-mrathor@linux.microsoft.com>
On Mon, May 11, 2026 at 07:02:52PM -0700, Mukesh R wrote:
> Add data structs needed by the subsequent patch that introduces a new
> module to implement VFIO-MSHV pseudo device.
>
Reviewed-by: Souradeep Chakrabarti <schakrabarti@linux.microsoft.com>
> Signed-off-by: Mukesh R <mrathor@linux.microsoft.com>
> ---
> drivers/hv/mshv_root.h | 19 +++++++++++++++++++
> include/uapi/linux/mshv.h | 30 ++++++++++++++++++++++++++++++
> 2 files changed, 49 insertions(+)
>
> diff --git a/drivers/hv/mshv_root.h b/drivers/hv/mshv_root.h
> index a85c24dcc701..b9880d0bdc4d 100644
> --- a/drivers/hv/mshv_root.h
> +++ b/drivers/hv/mshv_root.h
> @@ -227,6 +227,25 @@ struct port_table_info {
> };
> };
>
> +struct mshv_device {
> + const struct mshv_device_ops *device_ops;
> + struct mshv_partition *device_pt;
> + void *device_private;
> + struct hlist_node device_ptnode;
> +};
> +
> +struct mshv_device_ops {
> + const char *device_name;
> + long (*device_create)(struct mshv_device *dev);
> + void (*device_release)(struct mshv_device *dev);
> + long (*device_set_attr)(struct mshv_device *dev,
> + struct mshv_device_attr *attr);
> + long (*device_has_attr)(struct mshv_device *dev,
> + struct mshv_device_attr *attr);
> +};
> +
> +extern struct mshv_device_ops mshv_vfio_device_ops;
> +
> int mshv_update_routing_table(struct mshv_partition *partition,
> const struct mshv_user_irq_entry *entries,
> unsigned int numents);
> diff --git a/include/uapi/linux/mshv.h b/include/uapi/linux/mshv.h
> index 32ff92b6342b..be6fe3ee8707 100644
> --- a/include/uapi/linux/mshv.h
> +++ b/include/uapi/linux/mshv.h
> @@ -404,4 +404,34 @@ struct mshv_sint_mask {
> /* hv_hvcall device */
> #define MSHV_HVCALL_SETUP _IOW(MSHV_IOCTL, 0x1E, struct mshv_vtl_hvcall_setup)
> #define MSHV_HVCALL _IOWR(MSHV_IOCTL, 0x1F, struct mshv_vtl_hvcall)
> +
> +/* Device passhthru */
> +#define MSHV_CREATE_DEVICE_TEST 1
> +
> +enum {
> + MSHV_DEV_TYPE_VFIO,
> + MSHV_DEV_TYPE_MAX,
> +};
> +
> +struct mshv_create_device {
> + __u32 type; /* in: MSHV_DEV_TYPE_xxx */
> + __u32 fd; /* out: device handle */
> + __u32 flags; /* in: MSHV_CREATE_DEVICE_xxx */
> +};
> +
> +#define MSHV_DEV_VFIO_FILE 1
> +#define MSHV_DEV_VFIO_FILE_ADD 1
> +#define MSHV_DEV_VFIO_FILE_DEL 2
> +
> +struct mshv_device_attr {
> + __u32 flags; /* no flags currently defined */
> + __u32 group; /* device-defined */
> + __u64 attr; /* group-defined */
> + __u64 addr; /* userspace address of attr data */
> +};
> +
> +/* Device fds created with MSHV_CREATE_DEVICE */
> +#define MSHV_SET_DEVICE_ATTR _IOW(MSHV_IOCTL, 0x00, struct mshv_device_attr)
> +#define MSHV_HAS_DEVICE_ATTR _IOW(MSHV_IOCTL, 0x01, struct mshv_device_attr)
> +
> #endif
> --
> 2.51.2.vfs.0.1
>
^ permalink raw reply
* Re: [PATCH V3 02/11] x86/hyperv: Cosmetic changes in irqdomain.c for readability
From: Souradeep Chakrabarti @ 2026-05-12 10:27 UTC (permalink / raw)
To: Mukesh R
Cc: hpa, robin.murphy, robh, wei.liu, mhklinux, muislam, namjain,
magnuskulke, anbelski, linux-kernel, linux-hyperv, iommu,
linux-pci, linux-arch, kys, haiyangz, decui, longli, tglx, mingo,
bp, dave.hansen, x86, joro, will, lpieralisi, kwilczynski,
bhelgaas, arnd, jacob.pan
In-Reply-To: <20260512020259.1678627-3-mrathor@linux.microsoft.com>
On Mon, May 11, 2026 at 07:02:50PM -0700, Mukesh R wrote:
> Make cosmetic changes:
> o Rename struct pci_dev *dev to *pdev since there are cases of
> struct device *dev in the file and all over the kernel
> o Rename hv_build_pci_dev_id to hv_build_devid_type_pci in anticipation
> of building different types of device IDs
> o Fix checkpatch.pl issues with return and extraneous printk
> o Replace spaces with tabs
> o Rename struct hv_devid *xxx to struct hv_devid *hv_devid given code
> paths involve many types of device IDs
> o Fix indentation in a large if block by using goto.
>
> There are no functional changes.
>
> Reviewed-by: Anirudh Rayabharam (Microsoft) <anirudh@anirudhrb.com>
Reviewed-by: Souradeep Chakrabarti <schakrabarti@linux.microsoft.com>
> Signed-off-by: Mukesh R <mrathor@linux.microsoft.com>
> ---
> arch/x86/hyperv/irqdomain.c | 198 +++++++++++++++++++-----------------
> 1 file changed, 104 insertions(+), 94 deletions(-)
>
> diff --git a/arch/x86/hyperv/irqdomain.c b/arch/x86/hyperv/irqdomain.c
> index 365e364268d9..b3ad50a874dc 100644
> --- a/arch/x86/hyperv/irqdomain.c
> +++ b/arch/x86/hyperv/irqdomain.c
> @@ -1,5 +1,4 @@
> // SPDX-License-Identifier: GPL-2.0
> -
> /*
> * Irqdomain for Linux to run as the root partition on Microsoft Hypervisor.
> *
> @@ -14,8 +13,8 @@
> #include <linux/irqchip/irq-msi-lib.h>
> #include <asm/mshyperv.h>
>
> -static int hv_map_interrupt(union hv_device_id device_id, bool level,
> - int cpu, int vector, struct hv_interrupt_entry *entry)
> +static int hv_map_interrupt(union hv_device_id hv_devid, bool level,
> + int cpu, int vector, struct hv_interrupt_entry *ret_entry)
> {
> struct hv_input_map_device_interrupt *input;
> struct hv_output_map_device_interrupt *output;
> @@ -32,7 +31,7 @@ static int hv_map_interrupt(union hv_device_id device_id, bool level,
> intr_desc = &input->interrupt_descriptor;
> memset(input, 0, sizeof(*input));
> input->partition_id = hv_current_partition_id;
> - input->device_id = device_id.as_uint64;
> + input->device_id = hv_devid.as_uint64;
> intr_desc->interrupt_type = HV_X64_INTERRUPT_TYPE_FIXED;
> intr_desc->vector_count = 1;
> intr_desc->target.vector = vector;
> @@ -44,7 +43,7 @@ static int hv_map_interrupt(union hv_device_id device_id, bool level,
>
> intr_desc->target.vp_set.valid_bank_mask = 0;
> intr_desc->target.vp_set.format = HV_GENERIC_SET_SPARSE_4K;
> - nr_bank = cpumask_to_vpset(&(intr_desc->target.vp_set), cpumask_of(cpu));
> + nr_bank = cpumask_to_vpset(&intr_desc->target.vp_set, cpumask_of(cpu));
> if (nr_bank < 0) {
> local_irq_restore(flags);
> pr_err("%s: unable to generate VP set\n", __func__);
> @@ -61,7 +60,7 @@ static int hv_map_interrupt(union hv_device_id device_id, bool level,
>
> status = hv_do_rep_hypercall(HVCALL_MAP_DEVICE_INTERRUPT, 0, var_size,
> input, output);
> - *entry = output->interrupt_entry;
> + *ret_entry = output->interrupt_entry;
>
> local_irq_restore(flags);
>
> @@ -71,21 +70,19 @@ static int hv_map_interrupt(union hv_device_id device_id, bool level,
> return hv_result_to_errno(status);
> }
>
> -static int hv_unmap_interrupt(u64 id, struct hv_interrupt_entry *old_entry)
> +static int hv_unmap_interrupt(u64 id, struct hv_interrupt_entry *irq_entry)
> {
> unsigned long flags;
> struct hv_input_unmap_device_interrupt *input;
> - struct hv_interrupt_entry *intr_entry;
> u64 status;
>
> local_irq_save(flags);
> input = *this_cpu_ptr(hyperv_pcpu_input_arg);
>
> memset(input, 0, sizeof(*input));
> - intr_entry = &input->interrupt_entry;
> input->partition_id = hv_current_partition_id;
> input->device_id = id;
> - *intr_entry = *old_entry;
> + input->interrupt_entry = *irq_entry;
>
> status = hv_do_hypercall(HVCALL_UNMAP_DEVICE_INTERRUPT, input, NULL);
> local_irq_restore(flags);
> @@ -115,67 +112,71 @@ static int get_rid_cb(struct pci_dev *pdev, u16 alias, void *data)
> return 0;
> }
>
> -static union hv_device_id hv_build_pci_dev_id(struct pci_dev *dev)
> +static union hv_device_id hv_build_devid_type_pci(struct pci_dev *pdev)
> {
> - union hv_device_id dev_id;
> + int pos;
> + union hv_device_id hv_devid;
> struct rid_data data = {
> .bridge = NULL,
> - .rid = PCI_DEVID(dev->bus->number, dev->devfn)
> + .rid = PCI_DEVID(pdev->bus->number, pdev->devfn)
> };
>
> - pci_for_each_dma_alias(dev, get_rid_cb, &data);
> + pci_for_each_dma_alias(pdev, get_rid_cb, &data);
>
> - dev_id.as_uint64 = 0;
> - dev_id.device_type = HV_DEVICE_TYPE_PCI;
> - dev_id.pci.segment = pci_domain_nr(dev->bus);
> + hv_devid.as_uint64 = 0;
> + hv_devid.device_type = HV_DEVICE_TYPE_PCI;
> + hv_devid.pci.segment = pci_domain_nr(pdev->bus);
>
> - dev_id.pci.bdf.bus = PCI_BUS_NUM(data.rid);
> - dev_id.pci.bdf.device = PCI_SLOT(data.rid);
> - dev_id.pci.bdf.function = PCI_FUNC(data.rid);
> - dev_id.pci.source_shadow = HV_SOURCE_SHADOW_NONE;
> + hv_devid.pci.bdf.bus = PCI_BUS_NUM(data.rid);
> + hv_devid.pci.bdf.device = PCI_SLOT(data.rid);
> + hv_devid.pci.bdf.function = PCI_FUNC(data.rid);
> + hv_devid.pci.source_shadow = HV_SOURCE_SHADOW_NONE;
>
> - if (data.bridge) {
> - int pos;
> + if (data.bridge == NULL)
> + goto out;
>
> - /*
> - * Microsoft Hypervisor requires a bus range when the bridge is
> - * running in PCI-X mode.
> - *
> - * To distinguish conventional vs PCI-X bridge, we can check
> - * the bridge's PCI-X Secondary Status Register, Secondary Bus
> - * Mode and Frequency bits. See PCI Express to PCI/PCI-X Bridge
> - * Specification Revision 1.0 5.2.2.1.3.
> - *
> - * Value zero means it is in conventional mode, otherwise it is
> - * in PCI-X mode.
> - */
> + /*
> + * Microsoft Hypervisor requires a bus range when the bridge is
> + * running in PCI-X mode.
> + *
> + * To distinguish conventional vs PCI-X bridge, we can check
> + * the bridge's PCI-X Secondary Status Register, Secondary Bus
> + * Mode and Frequency bits. See PCI Express to PCI/PCI-X Bridge
> + * Specification Revision 1.0 5.2.2.1.3.
> + *
> + * Value zero means it is in conventional mode, otherwise it is
> + * in PCI-X mode.
> + */
>
> - pos = pci_find_capability(data.bridge, PCI_CAP_ID_PCIX);
> - if (pos) {
> - u16 status;
> + pos = pci_find_capability(data.bridge, PCI_CAP_ID_PCIX);
> + if (pos) {
> + u16 status;
>
> - pci_read_config_word(data.bridge, pos +
> - PCI_X_BRIDGE_SSTATUS, &status);
> + pci_read_config_word(data.bridge, pos + PCI_X_BRIDGE_SSTATUS,
> + &status);
>
> - if (status & PCI_X_SSTATUS_FREQ) {
> - /* Non-zero, PCI-X mode */
> - u8 sec_bus, sub_bus;
> + if (status & PCI_X_SSTATUS_FREQ) {
> + /* Non-zero, PCI-X mode */
> + u8 sec_bus, sub_bus;
>
> - dev_id.pci.source_shadow = HV_SOURCE_SHADOW_BRIDGE_BUS_RANGE;
> + hv_devid.pci.source_shadow =
> + HV_SOURCE_SHADOW_BRIDGE_BUS_RANGE;
>
> - pci_read_config_byte(data.bridge, PCI_SECONDARY_BUS, &sec_bus);
> - dev_id.pci.shadow_bus_range.secondary_bus = sec_bus;
> - pci_read_config_byte(data.bridge, PCI_SUBORDINATE_BUS, &sub_bus);
> - dev_id.pci.shadow_bus_range.subordinate_bus = sub_bus;
> - }
> + pci_read_config_byte(data.bridge, PCI_SECONDARY_BUS,
> + &sec_bus);
> + hv_devid.pci.shadow_bus_range.secondary_bus = sec_bus;
> + pci_read_config_byte(data.bridge, PCI_SUBORDINATE_BUS,
> + &sub_bus);
> + hv_devid.pci.shadow_bus_range.subordinate_bus = sub_bus;
> }
> }
>
> - return dev_id;
> +out:
> + return hv_devid;
> }
>
> -/**
> - * hv_map_msi_interrupt() - "Map" the MSI IRQ in the hypervisor.
> +/*
> + * hv_map_msi_interrupt() - Map the MSI IRQ in the hypervisor.
> * @data: Describes the IRQ
> * @out_entry: Hypervisor (MSI) interrupt entry (can be NULL)
> *
> @@ -188,22 +189,23 @@ int hv_map_msi_interrupt(struct irq_data *data,
> {
> struct irq_cfg *cfg = irqd_cfg(data);
> struct hv_interrupt_entry dummy;
> - union hv_device_id device_id;
> + union hv_device_id hv_devid;
> struct msi_desc *msidesc;
> - struct pci_dev *dev;
> + struct pci_dev *pdev;
> int cpu;
>
> msidesc = irq_data_get_msi_desc(data);
> - dev = msi_desc_to_pci_dev(msidesc);
> - device_id = hv_build_pci_dev_id(dev);
> + pdev = msi_desc_to_pci_dev(msidesc);
> + hv_devid = hv_build_devid_type_pci(pdev);
> cpu = cpumask_first(irq_data_get_effective_affinity_mask(data));
>
> - return hv_map_interrupt(device_id, false, cpu, cfg->vector,
> + return hv_map_interrupt(hv_devid, false, cpu, cfg->vector,
> out_entry ? out_entry : &dummy);
> }
> EXPORT_SYMBOL_GPL(hv_map_msi_interrupt);
>
> -static inline void entry_to_msi_msg(struct hv_interrupt_entry *entry, struct msi_msg *msg)
> +static void entry_to_msi_msg(struct hv_interrupt_entry *entry,
> + struct msi_msg *msg)
> {
> /* High address is always 0 */
> msg->address_hi = 0;
> @@ -211,17 +213,19 @@ static inline void entry_to_msi_msg(struct hv_interrupt_entry *entry, struct msi
> msg->data = entry->msi_entry.data.as_uint32;
> }
>
> -static int hv_unmap_msi_interrupt(struct pci_dev *dev, struct hv_interrupt_entry *old_entry);
> +static int hv_unmap_msi_interrupt(struct pci_dev *pdev,
> + struct hv_interrupt_entry *irq_entry);
> +
> static void hv_irq_compose_msi_msg(struct irq_data *data, struct msi_msg *msg)
> {
> struct hv_interrupt_entry *stored_entry;
> struct irq_cfg *cfg = irqd_cfg(data);
> struct msi_desc *msidesc;
> - struct pci_dev *dev;
> + struct pci_dev *pdev;
> int ret;
>
> msidesc = irq_data_get_msi_desc(data);
> - dev = msi_desc_to_pci_dev(msidesc);
> + pdev = msi_desc_to_pci_dev(msidesc);
>
> if (!cfg) {
> pr_debug("%s: cfg is NULL", __func__);
> @@ -240,7 +244,7 @@ static void hv_irq_compose_msi_msg(struct irq_data *data, struct msi_msg *msg)
> stored_entry = data->chip_data;
> data->chip_data = NULL;
>
> - ret = hv_unmap_msi_interrupt(dev, stored_entry);
> + ret = hv_unmap_msi_interrupt(pdev, stored_entry);
>
> kfree(stored_entry);
>
> @@ -249,10 +253,8 @@ static void hv_irq_compose_msi_msg(struct irq_data *data, struct msi_msg *msg)
> }
>
> stored_entry = kzalloc_obj(*stored_entry, GFP_ATOMIC);
> - if (!stored_entry) {
> - pr_debug("%s: failed to allocate chip data\n", __func__);
> + if (!stored_entry)
> return;
> - }
>
> ret = hv_map_msi_interrupt(data, stored_entry);
> if (ret) {
> @@ -262,18 +264,21 @@ static void hv_irq_compose_msi_msg(struct irq_data *data, struct msi_msg *msg)
>
> data->chip_data = stored_entry;
> entry_to_msi_msg(data->chip_data, msg);
> -
> - return;
> }
>
> -static int hv_unmap_msi_interrupt(struct pci_dev *dev, struct hv_interrupt_entry *old_entry)
> +static int hv_unmap_msi_interrupt(struct pci_dev *pdev,
> + struct hv_interrupt_entry *irq_entry)
> {
> - return hv_unmap_interrupt(hv_build_pci_dev_id(dev).as_uint64, old_entry);
> + union hv_device_id hv_devid;
> +
> + hv_devid = hv_build_devid_type_pci(pdev);
> + return hv_unmap_interrupt(hv_devid.as_uint64, irq_entry);
> }
>
> -static void hv_teardown_msi_irq(struct pci_dev *dev, struct irq_data *irqd)
> +/* NB: during map, hv_interrupt_entry is saved via data->chip_data */
> +static void hv_teardown_msi_irq(struct pci_dev *pdev, struct irq_data *irqd)
> {
> - struct hv_interrupt_entry old_entry;
> + struct hv_interrupt_entry irq_entry;
> struct msi_msg msg;
>
> if (!irqd->chip_data) {
> @@ -281,13 +286,13 @@ static void hv_teardown_msi_irq(struct pci_dev *dev, struct irq_data *irqd)
> return;
> }
>
> - old_entry = *(struct hv_interrupt_entry *)irqd->chip_data;
> - entry_to_msi_msg(&old_entry, &msg);
> + irq_entry = *(struct hv_interrupt_entry *)irqd->chip_data;
> + entry_to_msi_msg(&irq_entry, &msg);
>
> kfree(irqd->chip_data);
> irqd->chip_data = NULL;
>
> - (void)hv_unmap_msi_interrupt(dev, &old_entry);
> + (void)hv_unmap_msi_interrupt(pdev, &irq_entry);
> }
>
> /*
> @@ -302,7 +307,8 @@ static struct irq_chip hv_pci_msi_controller = {
> };
>
> static bool hv_init_dev_msi_info(struct device *dev, struct irq_domain *domain,
> - struct irq_domain *real_parent, struct msi_domain_info *info)
> + struct irq_domain *real_parent,
> + struct msi_domain_info *info)
> {
> struct irq_chip *chip = info->chip;
>
> @@ -317,7 +323,8 @@ static bool hv_init_dev_msi_info(struct device *dev, struct irq_domain *domain,
> }
>
> #define HV_MSI_FLAGS_SUPPORTED (MSI_GENERIC_FLAGS_MASK | MSI_FLAG_PCI_MSIX)
> -#define HV_MSI_FLAGS_REQUIRED (MSI_FLAG_USE_DEF_DOM_OPS | MSI_FLAG_USE_DEF_CHIP_OPS)
> +#define HV_MSI_FLAGS_REQUIRED (MSI_FLAG_USE_DEF_DOM_OPS | \
> + MSI_FLAG_USE_DEF_CHIP_OPS)
>
> static struct msi_parent_ops hv_msi_parent_ops = {
> .supported_flags = HV_MSI_FLAGS_SUPPORTED,
> @@ -329,14 +336,14 @@ static struct msi_parent_ops hv_msi_parent_ops = {
> .init_dev_msi_info = hv_init_dev_msi_info,
> };
>
> -static int hv_msi_domain_alloc(struct irq_domain *d, unsigned int virq, unsigned int nr_irqs,
> - void *arg)
> +/* Allocate nr_irqs IRQs for the given irq domain */
> +static int hv_msi_domain_alloc(struct irq_domain *d, unsigned int virq,
> + unsigned int nr_irqs, void *arg)
> {
> /*
> - * TODO: The allocation bits of hv_irq_compose_msi_msg(), i.e. everything except
> - * entry_to_msi_msg() should be in here.
> + * TODO: The allocation bits of hv_irq_compose_msi_msg(), i.e.
> + * everything except entry_to_msi_msg() should be in here.
> */
> -
> int ret;
>
> ret = irq_domain_alloc_irqs_parent(d, virq, nr_irqs, arg);
> @@ -344,13 +351,15 @@ static int hv_msi_domain_alloc(struct irq_domain *d, unsigned int virq, unsigned
> return ret;
>
> for (int i = 0; i < nr_irqs; ++i) {
> - irq_domain_set_info(d, virq + i, 0, &hv_pci_msi_controller, NULL,
> - handle_edge_irq, NULL, "edge");
> + irq_domain_set_info(d, virq + i, 0, &hv_pci_msi_controller,
> + NULL, handle_edge_irq, NULL, "edge");
> }
> +
> return 0;
> }
>
> -static void hv_msi_domain_free(struct irq_domain *d, unsigned int virq, unsigned int nr_irqs)
> +static void hv_msi_domain_free(struct irq_domain *d, unsigned int virq,
> + unsigned int nr_irqs)
> {
> for (int i = 0; i < nr_irqs; ++i) {
> struct irq_data *irqd = irq_domain_get_irq_data(d, virq);
> @@ -362,6 +371,7 @@ static void hv_msi_domain_free(struct irq_domain *d, unsigned int virq, unsigned
>
> hv_teardown_msi_irq(to_pci_dev(desc->dev), irqd);
> }
> +
> irq_domain_free_irqs_top(d, virq, nr_irqs);
> }
>
> @@ -394,25 +404,25 @@ struct irq_domain * __init hv_create_pci_msi_domain(void)
>
> int hv_unmap_ioapic_interrupt(int ioapic_id, struct hv_interrupt_entry *entry)
> {
> - union hv_device_id device_id;
> + union hv_device_id hv_devid;
>
> - device_id.as_uint64 = 0;
> - device_id.device_type = HV_DEVICE_TYPE_IOAPIC;
> - device_id.ioapic.ioapic_id = (u8)ioapic_id;
> + hv_devid.as_uint64 = 0;
> + hv_devid.device_type = HV_DEVICE_TYPE_IOAPIC;
> + hv_devid.ioapic.ioapic_id = (u8)ioapic_id;
>
> - return hv_unmap_interrupt(device_id.as_uint64, entry);
> + return hv_unmap_interrupt(hv_devid.as_uint64, entry);
> }
> EXPORT_SYMBOL_GPL(hv_unmap_ioapic_interrupt);
>
> int hv_map_ioapic_interrupt(int ioapic_id, bool level, int cpu, int vector,
> struct hv_interrupt_entry *entry)
> {
> - union hv_device_id device_id;
> + union hv_device_id hv_devid;
>
> - device_id.as_uint64 = 0;
> - device_id.device_type = HV_DEVICE_TYPE_IOAPIC;
> - device_id.ioapic.ioapic_id = (u8)ioapic_id;
> + hv_devid.as_uint64 = 0;
> + hv_devid.device_type = HV_DEVICE_TYPE_IOAPIC;
> + hv_devid.ioapic.ioapic_id = (u8)ioapic_id;
>
> - return hv_map_interrupt(device_id, level, cpu, vector, entry);
> + return hv_map_interrupt(hv_devid, level, cpu, vector, entry);
> }
> EXPORT_SYMBOL_GPL(hv_map_ioapic_interrupt);
> --
> 2.51.2.vfs.0.1
>
^ permalink raw reply
* Re: [PATCH net-next v8 1/6] net: mana: Create separate EQs for each vPort
From: Paolo Abeni @ 2026-05-12 11:34 UTC (permalink / raw)
To: longli
Cc: kotaranov, kuba, davem, pabeni, edumazet, andrew+netdev, jgg,
leon, haiyangz, kys, wei.liu, decui, shradhagupta, horms, netdev,
linux-rdma, linux-hyperv, linux-kernel
In-Reply-To: <20260508221202.15725-2-longli@microsoft.com>
From: AI Reviewer <ai@example.com>
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at: https://netdev-ai.bots.linux.dev/sashiko/
---
Subject: net: mana: Create separate EQs for each vPort
This patch moves EQ ownership from struct mana_context to
struct mana_port_context, creates per-vPort EQs in mana_alloc_queues()
and mana_ib_cfg_vport(), and exports mana_create_eq()/mana_destroy_eq()
for use by the MANA RDMA driver.
A few questions below about the new lifetime and ordering rules for
mpc->eqs.
> diff --git a/drivers/infiniband/hw/mana/main.c b/drivers/infiniband/hw/mana/main.c
> index ac5e75dd3494..8000ab6e8beb 100644
> --- a/drivers/infiniband/hw/mana/main.c
> +++ b/drivers/infiniband/hw/mana/main.c
> @@ -20,8 +20,10 @@ void mana_ib_uncfg_vport(struct mana_ib_dev *dev, struct mana_ib_pd *pd,
> pd->vport_use_count--;
> WARN_ON(pd->vport_use_count < 0);
>
> - if (!pd->vport_use_count)
> + if (!pd->vport_use_count) {
> + mana_destroy_eq(mpc);
> mana_uncfg_vport(mpc);
> + }
>
> mutex_unlock(&pd->vport_mutex);
> }
[High]
After this change, mpc->eqs lifetime is governed by pd->vport_use_count,
and only raw QPs call mana_ib_cfg_vport()/mana_ib_uncfg_vport(). RSS
QPs in mana_ib_create_qp_rss() read mpc->eqs and latch eq->eq->id into
the HW CQ via cq_spec.attached_eq, but they do not bump
pd->vport_use_count.
So if a user creates a raw QP on a PD (use_count=1, mpc->eqs allocated),
then creates an RSS QP on the same PD whose CQs now reference those EQ
ids, and finally destroys the raw QP first, this path runs with
use_count dropping to 0 and calls mana_destroy_eq(mpc) -> kfree(apc->eqs)
and mana_gd_destroy_queue() for each EQ while the surviving RSS QP's HW
CQs are still attached to those EQs.
Is there an intended rule that enforces destruction ordering between raw
and RSS QPs on the same PD, or should RSS QPs also participate in the
refcount that guards mpc->eqs?
> @@ -55,15 +57,22 @@ int mana_ib_cfg_vport(struct mana_ib_dev *dev, u32 port, struct mana_ib_pd *pd,
> return err;
> }
>
> - mutex_unlock(&pd->vport_mutex);
>
> pd->tx_shortform_allowed = mpc->tx_shortform_allowed;
> pd->tx_vp_offset = mpc->tx_vp_offset;
> + err = mana_create_eq(mpc);
> + if (err) {
> + mana_uncfg_vport(mpc);
> + pd->vport_use_count--;
> + }
>
> - ibdev_dbg(&dev->ib_dev, "vport handle %llx pdid %x doorbell_id %x\n",
> - mpc->port_handle, pd->pdn, doorbell_id);
> + mutex_unlock(&pd->vport_mutex);
>
> - return 0;
> + if (!err)
> + ibdev_dbg(&dev->ib_dev, "vport handle %llx pdid %x doorbell_id %x\n",
> + mpc->port_handle, pd->pdn, doorbell_id);
> +
> + return err;
> }
[Low]
pd->tx_shortform_allowed and pd->tx_vp_offset are written before
mana_create_eq() is attempted. On the mana_create_eq() failure branch,
mana_uncfg_vport(mpc) is called and pd->vport_use_count is decremented
back to 0, but pd->tx_shortform_allowed/pd->tx_vp_offset are left set to
the values derived from a vport whose HW-side configuration has been
reverted. Should these fields only be committed to pd after
mana_create_eq() succeeds?
> diff --git a/drivers/infiniband/hw/mana/qp.c b/drivers/infiniband/hw/mana/qp.c
> index 0fbcf449c134..6167742df9d2 100644
> --- a/drivers/infiniband/hw/mana/qp.c
> +++ b/drivers/infiniband/hw/mana/qp.c
> @@ -171,7 +171,15 @@ static int mana_ib_create_qp_rss(struct ib_qp *ibqp, struct ib_pd *pd,
> cq_spec.gdma_region = cq->queue.gdma_region;
> cq_spec.queue_size = cq->cqe * COMP_ENTRY_SIZE;
> cq_spec.modr_ctx_id = 0;
> - eq = &mpc->ac->eqs[cq->comp_vector];
> + /* EQs are created when a raw QP configures the vport.
> + * A raw QP must be created before creating rwq_ind_tbl.
> + */
> + if (!mpc->eqs) {
> + ret = -EINVAL;
> + i--;
> + goto fail;
> + }
> + eq = &mpc->eqs[cq->comp_vector % mpc->num_queues];
> cq_spec.attached_eq = eq->eq->id;
[Critical]
mana_ib_create_qp_rss() does not acquire pd->vport_mutex and does not
touch pd->vport_use_count. The NULL check above reads mpc->eqs without
synchronization; immediately after it, the same mpc->eqs is dereferenced
and eq->eq->id is latched into cq_spec.attached_eq.
Can the following sequence occur?
thread A (creating RSS QP on pd) thread B (destroying last raw QP on pd)
mana_ib_create_qp_rss()
if (!mpc->eqs) /* non-NULL */
... mana_ib_uncfg_vport()
mutex_lock(&pd->vport_mutex);
if (!--pd->vport_use_count)
mana_destroy_eq(mpc);
kfree(apc->eqs);
eq = &mpc->eqs[... % mpc->num_queues];
cq_spec.attached_eq = eq->eq->id; /* reads freed memory */
Is there something else serializing these paths that this reviewer is
missing?
[Medium]
The off-by-one on the error path looks inconsistent with the other fail
sites in this function. Every other goto fail in this loop leaves i
alone and relies on:
while (i-- > 0) {
ibwq = ind_tbl->ind_tbl[i];
...
mana_ib_remove_cq_cb(mdev, cq);
mana_destroy_wq_obj(mpc, GDMA_RQ, wq->rx_object);
}
Here the new branch does ret = -EINVAL; i--; goto fail, which drops one
extra iteration of cleanup. If this check ever fires at i > 0 (for
example if the lifetime issue above is fixed by making mpc->eqs
observable as NULL mid-loop), the WQ object and installed CQ callback
created in the previous iteration are not torn down. Should this match
the other fail sites and omit the i--?
[Medium]
Before this patch ac->eqs was populated unconditionally in mana_probe()
and existed for the lifetime of the device, so mana_ib_create_qp_rss()
and mana_ib_create_qp_raw() always had EQs available regardless of
whether any netdev was up or any raw QP had been created.
After this patch, mpc->eqs is populated only by mana_alloc_queues()
(netdev open) or by mana_ib_cfg_vport() (which is only invoked from
mana_ib_create_qp_raw()). An RDMA user who creates an RSS QP on a PD
while the netdev is down and without a prior raw QP on the same PD will
now see -EINVAL from this new check, where the same call succeeded
before.
The commit message describes this work as preparatory and does not
mention this user-visible ordering requirement. Should this semantic
change be called out in the commit log, and should the implicit "a raw
QP must be created first" rule be documented somewhere user-facing?
> diff --git a/drivers/net/ethernet/microsoft/mana/mana_en.c b/drivers/net/ethernet/microsoft/mana/mana_en.c
> index b2faa7cf398f..131a997e1083 100644
> --- a/drivers/net/ethernet/microsoft/mana/mana_en.c
> +++ b/drivers/net/ethernet/microsoft/mana/mana_en.c
> @@ -1615,78 +1615,83 @@ void mana_destroy_wq_obj(struct mana_port_context *apc, u32 wq_type,
> }
> EXPORT_SYMBOL_NS(mana_destroy_wq_obj, "NET_MANA");
>
> -static void mana_destroy_eq(struct mana_context *ac)
> +void mana_destroy_eq(struct mana_port_context *apc)
> {
> + struct mana_context *ac = apc->ac;
> struct gdma_context *gc = ac->gdma_dev->gdma_context;
> struct gdma_queue *eq;
> int i;
>
> - if (!ac->eqs)
> + if (!apc->eqs)
> return;
>
> - debugfs_remove_recursive(ac->mana_eqs_debugfs);
> - ac->mana_eqs_debugfs = NULL;
> + debugfs_remove_recursive(apc->mana_eqs_debugfs);
> + apc->mana_eqs_debugfs = NULL;
>
> - for (i = 0; i < gc->max_num_queues; i++) {
> - eq = ac->eqs[i].eq;
> + for (i = 0; i < apc->num_queues; i++) {
> + eq = apc->eqs[i].eq;
> if (!eq)
> continue;
>
> mana_gd_destroy_queue(gc, eq);
> }
[High]
The loop bound is now apc->num_queues rather than the immutable
gc->max_num_queues. mana_create_eq() allocates apc->eqs sized to
apc->num_queues, and mana_destroy_eq() walks the same count.
When RDMA has called mana_ib_cfg_vport() -> mana_create_eq() while the
netdev is down, apc->num_queues can still be mutated underneath it.
mana_detach(false) skips mana_dealloc_queues() when port_st_save is
false, and then mana_set_channels() (ethtool -L) writes
apc->num_queues = new_count while mpc->eqs is already held by the RDMA
side. mana_attach() then also skips mana_alloc_queues().
When the RDMA user later destroys the last raw QP and this function
runs, i iterates 0..new_count-1. If new_count is larger than the count
used at kzalloc_objs() time, apc->eqs is read out of bounds and
potentially passed to mana_gd_destroy_queue(). If new_count is smaller,
the tail EQs (real gdma_queue objects with MSI-X and DMA regions) are
leaked.
Before this patch both sides used gc->max_num_queues so no mismatch was
possible. Should apc->num_queues be snapshotted into apc at
mana_create_eq() time (or should changing num_queues be forbidden while
mpc->eqs is populated by the RDMA path)?
> -static int mana_create_eq(struct mana_context *ac)
> +int mana_create_eq(struct mana_port_context *apc)
> {
> - struct gdma_dev *gd = ac->gdma_dev;
> + struct gdma_dev *gd = apc->ac->gdma_dev;
> struct gdma_context *gc = gd->gdma_context;
> struct gdma_queue_spec spec = {};
> int err;
> int i;
>
> - ac->eqs = kzalloc_objs(struct mana_eq, gc->max_num_queues);
> - if (!ac->eqs)
> + WARN_ON(apc->eqs);
> + apc->eqs = kzalloc_objs(struct mana_eq, apc->num_queues);
> + if (!apc->eqs)
> return -ENOMEM;
[Low]
WARN_ON(apc->eqs) here is advisory only; execution continues into
apc->eqs = kzalloc_objs(...), which unconditionally overwrites any
non-NULL pointer. If the invariant is ever violated by a future caller
or error path, the prior EQ array and its gdma_queue objects are
orphaned (kernel memory and firmware EQ state leaked) with only a WARN
splat as the signal.
Would "if (WARN_ON(apc->eqs)) return -EEXIST;" express the intent more
safely?
--
This is an AI-generated review.
^ permalink raw reply
* Re: [PATCH net-next v8 3/6] net: mana: Introduce GIC context with refcounting for interrupt management
From: Paolo Abeni @ 2026-05-12 11:36 UTC (permalink / raw)
To: Long Li, Konstantin Taranov, Jakub Kicinski, David S . Miller,
Eric Dumazet, Andrew Lunn, Jason Gunthorpe, Leon Romanovsky,
Haiyang Zhang, K . Y . Srinivasan, Wei Liu, Dexuan Cui,
shradhagupta
Cc: Simon Horman, netdev, linux-rdma, linux-hyperv, linux-kernel
In-Reply-To: <20260508221202.15725-4-longli@microsoft.com>
On 5/9/26 12:11 AM, Long Li wrote:
> diff --git a/drivers/net/ethernet/microsoft/mana/gdma_main.c b/drivers/net/ethernet/microsoft/mana/gdma_main.c
> index 4673ff62e6d9..78cb89c46ff3 100644
> --- a/drivers/net/ethernet/microsoft/mana/gdma_main.c
> +++ b/drivers/net/ethernet/microsoft/mana/gdma_main.c
> @@ -1618,6 +1618,164 @@ static irqreturn_t mana_gd_intr(int irq, void *arg)
> return IRQ_HANDLED;
> }
>
> +void mana_gd_put_gic(struct gdma_context *gc, bool use_msi_bitmap, int msi)
> +{
> + struct pci_dev *dev = to_pci_dev(gc->dev);
> + struct msi_map irq_map;
> + struct gdma_irq_context *gic;
> + int irq;
Since a new revision is needed, please fix the reverse christmas tree
above and elsewhere, thanks!
/P
^ permalink raw reply
* Re: [PATCH v4 17/18] mshv: Publish VP to pt_vp_array before installing the file descriptor
From: Anirudh Rayabharam @ 2026-05-12 12:46 UTC (permalink / raw)
To: Stanislav Kinsburskii
Cc: kys, haiyangz, wei.liu, decui, longli, linux-hyperv, linux-kernel
In-Reply-To: <agH17u_h2ljX5sUb@skinsburskii.localdomain>
On Mon, May 11, 2026 at 08:29:50AM -0700, Stanislav Kinsburskii wrote:
> On Mon, May 11, 2026 at 02:26:45PM +0000, Anirudh Rayabharam wrote:
> > On Thu, May 07, 2026 at 03:44:32PM +0000, Stanislav Kinsburskii wrote:
> > > mshv_partition_ioctl_create_vp() called anon_inode_getfd() before
> > > publishing the new VP into partition->pt_vp_array. anon_inode_getfd()
> > > includes fd_install(), so the fd was live in current->files before the
> > > publish ran.
> > >
> > > A concurrent MSHV_RUN_VP ioctl on that fd does not serialise against the
> > > in-progress MSHV_CREATE_VP — it takes vp->vp_mutex, not the partition
> > > mutex. Once the VP starts running and traps, mshv_intercept_isr() can look
> > > up partition->pt_vp_array[vp_index] and observe NULL, silently dropping the
> > > intercept message.
> > >
> > > Split the fd creation: reserve an fd with get_unused_fd_flags(), create the
> > > file with anon_inode_getfile(), publish the VP via smp_store_release(), and
> > > finally call fd_install() as the userspace-visibility commit point.
> > >
> > > Fixes: 621191d709b14 ("Drivers: hv: Introduce mshv_root module to expose /dev/mshv to VMMs")
> > > Signed-off-by: Stanislav Kinsburskii <skinsburskii@linux.microsoft.com>
> > > ---
> > > drivers/hv/mshv_root_main.c | 29 ++++++++++++++++++++++-------
> > > 1 file changed, 22 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/drivers/hv/mshv_root_main.c b/drivers/hv/mshv_root_main.c
> > > index e32f6e0f9f637..1c18d1c1f7947 100644
> > > --- a/drivers/hv/mshv_root_main.c
> > > +++ b/drivers/hv/mshv_root_main.c
> > > @@ -1142,6 +1142,8 @@ mshv_partition_ioctl_create_vp(struct mshv_partition *partition,
> > > struct mshv_vp *vp;
> > > struct page *intercept_msg_page, *register_page, *ghcb_page;
> > > struct hv_stats_page *stats_pages[2];
> > > + struct file *file;
> > > + int fd;
> > > long ret;
> > >
> > > if (copy_from_user(&args, arg, sizeof(args)))
> > > @@ -1214,14 +1216,18 @@ mshv_partition_ioctl_create_vp(struct mshv_partition *partition,
> > > if (ret)
> > > goto put_partition;
> > >
> > > - /*
> > > - * Keep anon_inode_getfd last: it installs fd in the file struct and
> > > - * thus makes the state accessible in user space.
> > > - */
> > > - ret = anon_inode_getfd("mshv_vp", &mshv_vp_fops, vp,
> > > - O_RDWR | O_CLOEXEC);
> >
> > Why not just move this anon_inode_getfd() after the smp_store_release()
> > call?
> >
>
> Because anon_inode_getfd() can still fail at the fd-table step after the
> VP is already visible to lockless ISR readers, and unpublishing it would
> require a grace-period wait under pt_mutex while ISRs may have already
> observed and acted on the doomed VP. The split form keeps every failable
> step before the publish, so failure paths stay simple and the publish
> itself cannot fail.
Reviewed-by: Anirudh Rayabharam (Microsoft) <anirudh@anirudhrb.com>
^ permalink raw reply
* Re: [PATCH 1/3] mm/hmm: Add hmm_range_fault_unlockable() for mmap lock-drop support
From: Stanislav Kinsburskii @ 2026-05-12 16:18 UTC (permalink / raw)
To: David Hildenbrand (Arm)
Cc: kys, Liam.Howlett, akpm, decui, haiyangz, jgg, corbet, leon,
longli, ljs, mhocko, rppt, shuah, skhan, surenb, vbabka, wei.liu,
linux-doc, linux-hyperv, linux-kernel, linux-kselftest, linux-mm
In-Reply-To: <563bb216-c270-4711-adda-b91484af40dc@kernel.org>
On Tue, May 12, 2026 at 10:42:14AM +0200, David Hildenbrand (Arm) wrote:
>
> > + for (; addr < end; addr += PAGE_SIZE) {
> > + vm_fault_t ret;
> > +
> > + ret = handle_mm_fault(vma, addr, fault_flags, NULL);
> > +
> > + if (ret & (VM_FAULT_RETRY | VM_FAULT_COMPLETED)) {
> > + /*
> > + * The mmap lock has been dropped by the fault handler.
> > + * Record the failing address and signal lock-drop to
> > + * the caller.
> > + */
> > + *hmm_vma_walk->locked = 0;
> > + hmm_vma_walk->last = addr;
> > + return -EAGAIN;
>
>
> Okay, so we'll return straight from hmm_vma_fault() to
> hmm_vma_handle_pte()/hmm_vma_walk_pmd() -> walk_page_range() machinery.
>
> Hopefully we don't refer to the MM/VMA on any path there? It would be nicer if
> the hmm_vma_fault() could be called by the caller of walk_page_range(), but
> that's tricky I guess, as hmm_vma_fault() consumes the walk structure and
> requires the vma in there.
>
It looks like a caller can provide a post_vma callback in mm_walk_ops. I
missed that case here. This callback cannot be supported by this change.
I will update the patch.
>
> Note: am I wrong, or is hmm_vma_fault() really always called with
> required_fault=true?
>
No, hmm_pte_need_fault can return false.
> > + }
> > +
> > + if (ret & VM_FAULT_ERROR)
> > return -EFAULT;
> > + }
> > return -EBUSY;
> > }
> >
> > @@ -566,6 +585,17 @@ static int hmm_vma_walk_hugetlb_entry(pte_t *pte, unsigned long hmask,
> > if (required_fault) {
> > int ret;
> >
> > + /*
> > + * Faulting hugetlb pages on the unlockable path is not
> > + * supported. The walk framework holds hugetlb_vma_lock_read
> > + * which must be dropped before handle_mm_fault, but if the
> > + * mmap lock is also dropped (VM_FAULT_RETRY), the vma may
> > + * be freed and the walk framework's unconditional unlock
> > + * becomes a use-after-free.
> > + */
> > + if (hmm_vma_walk->locked)
> > + return -EFAULT;
>
> Just because it's unlockable doesn't mean that you must unlock. Can't this be
> kept working as is, just simulating here as if it would not be unlockable?
>
I’m not sure how to implement this. The walk_page_range code expects the
hugetlb VMA to still be read-locked when we return from
hmm_vma_walk_hugetlb_entry. How can we guarantee that if the VMA might
be gone?
I added a note in the docs. Whoever tackles this will likely need to
either rework `walk_page_range` to handle the case where the VMA is
gone, or use a different approach.
Do you have any other suggestions on how to implement it?
Thanks,
Stanislav
>
> --
> Cheers,
>
> David
^ permalink raw reply
* Re: [PATCH V3 08/11] PCI: hv: VMBus and PCI device IDs for PCI passthru
From: Bjorn Helgaas @ 2026-05-12 17:41 UTC (permalink / raw)
To: Mukesh R
Cc: hpa, robin.murphy, robh, wei.liu, mhklinux, muislam, namjain,
magnuskulke, anbelski, linux-kernel, linux-hyperv, iommu,
linux-pci, linux-arch, kys, haiyangz, decui, longli, tglx, mingo,
bp, dave.hansen, x86, joro, will, lpieralisi, kwilczynski,
bhelgaas, arnd, jacob.pan
In-Reply-To: <20260512020259.1678627-9-mrathor@linux.microsoft.com>
On Mon, May 11, 2026 at 07:02:56PM -0700, Mukesh R wrote:
> On Hyper-V, most hypercalls related to PCI passthru to map/unmap regions,
> interrupts, etc need a device ID as a parameter. This device ID refers
> to that specific device during the lifetime of passthru.
>
> An L1VH VM only contains VMBus based devices. A device ID for a VMBus
> device is slightly different in that it uses the hv_pcibus_device info
> for building it to make sure it matches exactly what the hypervisor
> expects. This VMBus based device ID is needed when attaching devices in
> an L1VH based guest VM. Add a function to build and export it. Before
> building it, a check is done to make sure the device is a valid VMBus
> device.
>
> In remaining cases, PCI device ID is used. So, also make PCI device ID
> build function hv_build_devid_type_pci() public.
The subject line should start with a verb to help say what this patch
does. I guess it's something about adding a function to build the
device IDs expected by the hypervisor? Would be good to include the
function name too, e.g., something like this:
PCI: hv: Add foo() to build hypervisor device IDs
^ permalink raw reply
* [PATCH 6.18 121/270] hv: Select CONFIG_SYSFB only for CONFIG_HYPERV_VMBUS
From: Greg Kroah-Hartman @ 2026-05-12 17:38 UTC (permalink / raw)
To: stable
Cc: Greg Kroah-Hartman, patches, Thomas Zimmermann, Michael Kelley,
Saurabh Sengar, Wei Liu, K. Y. Srinivasan, Haiyang Zhang,
Dexuan Cui, Long Li, linux-hyperv
In-Reply-To: <20260512173938.452574370@linuxfoundation.org>
6.18-stable review patch. If anyone has any objections, please let me know.
------------------
From: Thomas Zimmermann <tzimmermann@suse.de>
commit d33db956c9618e7cb08c2520ce708437914214ec upstream.
Hyperv's sysfb access only exists in the VMBUS support. Therefore
only select CONFIG_SYSFB for CONFIG_HYPERV_VMBUS. Avoids sysfb code
on systems that don't need it.
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Fixes: 96959283a58d ("Drivers: hv: Always select CONFIG_SYSFB for Hyper-V guests")
Cc: Michael Kelley <mhklinux@outlook.com>
Cc: Saurabh Sengar <ssengar@linux.microsoft.com>
Cc: Wei Liu <wei.liu@kernel.org>
Cc: "K. Y. Srinivasan" <kys@microsoft.com>
Cc: Haiyang Zhang <haiyangz@microsoft.com>
Cc: Dexuan Cui <decui@microsoft.com>
Cc: Long Li <longli@microsoft.com>
Cc: linux-hyperv@vger.kernel.org
Cc: <stable@vger.kernel.org> # v6.16+
Reviewed-by: Saurabh Sengar <ssengar@linux.microsoft.com>
Link: https://patch.msgid.link/20260402092305.208728-2-tzimmermann@suse.de
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
drivers/hv/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
--- a/drivers/hv/Kconfig
+++ b/drivers/hv/Kconfig
@@ -9,7 +9,6 @@ config HYPERV
select PARAVIRT
select X86_HV_CALLBACK_VECTOR if X86
select OF_EARLY_FLATTREE if OF
- select SYSFB if EFI && !HYPERV_VTL_MODE
select IRQ_MSI_LIB if X86
help
Select this option to run Linux as a Hyper-V client operating
@@ -61,6 +60,7 @@ config HYPERV_VMBUS
tristate "Microsoft Hyper-V VMBus driver"
depends on HYPERV
default HYPERV
+ select SYSFB if EFI && !HYPERV_VTL_MODE
help
Select this option to enable Hyper-V Vmbus driver.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox