* Re: [PATCH RFC 2/2] PCI: hv: Add vPCI device reset support
From: sashiko-bot @ 2026-07-24 23:25 UTC (permalink / raw)
To: wei.liu; +Cc: linux-hyperv, linux-pci
In-Reply-To: <20260724230844.3259741-3-wei.liu@kernel.org>
> From: Wei Liu <wei.liu@kernel.org>
>
> Hyper-V vPCI protocol version 1.5 adds a RESET_DEVICE request for projected
> PCI functions. Negotiate the new protocol version and issue the request
> through the vPCI VMBus channel from the PCI controller reset callback.
>
> Use the existing VMBus response path. Return -ENOTTY when the host reports
> STATUS_NOT_SUPPORTED so PCI core may try another reset method.
>
> Signed-off-by: Wei Liu <wei.liu@kernel.org>
Sashiko has reviewed this patch and found no issues. It looks great!
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260724230844.3259741-1-wei.liu@kernel.org?part=2
^ permalink raw reply
* Re: [PATCH RFC 1/2] PCI: Add controller reset method
From: sashiko-bot @ 2026-07-24 23:16 UTC (permalink / raw)
To: wei.liu; +Cc: linux-hyperv, linux-pci
In-Reply-To: <20260724230844.3259741-2-wei.liu@kernel.org>
> From: Wei Liu <wei.liu@kernel.org>
>
> Some PCI controllers provide a function reset mechanism that is not
> advertised in PCI configuration space. Allow them to expose it through an
> optional pci_ops callback.
>
> Add the controller reset method to the standard reset_method interface.
> Prefer FLR and AF FLR by default, and use the controller operation before
> PM and bus reset fallbacks.
>
> Signed-off-by: Wei Liu <wei.liu@kernel.org>
Sashiko has reviewed this patch and found no issues. It looks great!
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260724230844.3259741-1-wei.liu@kernel.org?part=1
^ permalink raw reply
* [PATCH RFC 2/2] PCI: hv: Add vPCI device reset support
From: wei.liu @ 2026-07-24 23:08 UTC (permalink / raw)
To: Linux on Hyper-V List, linux-pci
Cc: Wei Liu, K. Y. Srinivasan, Haiyang Zhang, Dexuan Cui, Long Li,
Jonathan Corbet, Shuah Khan, Lorenzo Pieralisi,
Krzysztof Wilczyński, Manivannan Sadhasivam, Rob Herring,
Bjorn Helgaas, open list:DOCUMENTATION, open list
In-Reply-To: <20260724230844.3259741-1-wei.liu@kernel.org>
From: Wei Liu <wei.liu@kernel.org>
Hyper-V vPCI protocol version 1.5 adds a RESET_DEVICE request for projected
PCI functions. Negotiate the new protocol version and issue the request
through the vPCI VMBus channel from the PCI controller reset callback.
Use the existing VMBus response path. Return -ENOTTY when the host reports
STATUS_NOT_SUPPORTED so PCI core may try another reset method.
Signed-off-by: Wei Liu <wei.liu@kernel.org>
---
Documentation/virt/hyperv/vpci.rst | 2 +-
drivers/pci/controller/pci-hyperv.c | 61 +++++++++++++++++++++++++++++
2 files changed, 62 insertions(+), 1 deletion(-)
diff --git a/Documentation/virt/hyperv/vpci.rst b/Documentation/virt/hyperv/vpci.rst
index b65b2126ede3..6bfee7225c14 100644
--- a/Documentation/virt/hyperv/vpci.rst
+++ b/Documentation/virt/hyperv/vpci.rst
@@ -65,7 +65,7 @@ exchange messages with the vPCI VSP for the purpose of setting
up and configuring the vPCI device in Linux. Once the device
is fully configured in Linux as a PCI device, the VMBus
channel is used only if Linux changes the vCPU to be interrupted
-in the guest, or if the vPCI device is removed from
+in the guest, or if the vPCI device is reset or removed from
the VM while the VM is running. The ongoing operation of the
device happens directly between the Linux device driver for
the device and the hardware, with VMBus and the VMBus channel
diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci-hyperv.c
index cfc8fa403dad..d3c0fd5e1d8e 100644
--- a/drivers/pci/controller/pci-hyperv.c
+++ b/drivers/pci/controller/pci-hyperv.c
@@ -68,6 +68,7 @@ enum pci_protocol_version_t {
PCI_PROTOCOL_VERSION_1_2 = PCI_MAKE_VERSION(1, 2), /* RS1 */
PCI_PROTOCOL_VERSION_1_3 = PCI_MAKE_VERSION(1, 3), /* Vibranium */
PCI_PROTOCOL_VERSION_1_4 = PCI_MAKE_VERSION(1, 4), /* WS2022 */
+ PCI_PROTOCOL_VERSION_1_5 = PCI_MAKE_VERSION(1, 5), /* GE, device reset */
};
#define CPU_AFFINITY_ALL -1ULL
@@ -77,6 +78,7 @@ enum pci_protocol_version_t {
* first.
*/
static enum pci_protocol_version_t pci_protocol_versions[] = {
+ PCI_PROTOCOL_VERSION_1_5,
PCI_PROTOCOL_VERSION_1_4,
PCI_PROTOCOL_VERSION_1_3,
PCI_PROTOCOL_VERSION_1_2,
@@ -90,6 +92,7 @@ static enum pci_protocol_version_t pci_protocol_versions[] = {
#define MAX_SUPPORTED_MSI_MESSAGES 0x400
#define STATUS_REVISION_MISMATCH 0xC0000059
+#define STATUS_NOT_SUPPORTED 0xC00000BB
/* space for 32bit serial number as string */
#define SLOT_NAME_SIZE 11
@@ -136,6 +139,7 @@ enum pci_message_type {
PCI_BUS_RELATIONS2 = PCI_MESSAGE_BASE + 0x19,
PCI_RESOURCES_ASSIGNED3 = PCI_MESSAGE_BASE + 0x1A,
PCI_CREATE_INTERRUPT_MESSAGE3 = PCI_MESSAGE_BASE + 0x1B,
+ PCI_RESET_DEVICE = PCI_MESSAGE_BASE + 0x1C,
PCI_MESSAGE_MAXIMUM
};
@@ -1397,10 +1401,66 @@ static int hv_pcifront_write_config(struct pci_bus *bus, unsigned int devfn,
return PCIBIOS_SUCCESSFUL;
}
+static int hv_pcifront_reset(struct pci_dev *pdev, bool probe)
+{
+ struct hv_pcibus_device *hbus =
+ container_of(pdev->bus->sysdata, struct hv_pcibus_device, sysdata);
+ struct pci_child_message reset = {};
+ struct hv_pci_compl comp_pkt;
+ struct pci_packet pkt = {
+ .completion_func = hv_pci_generic_compl,
+ .compl_ctxt = &comp_pkt,
+ };
+ enum hv_pcibus_state state;
+ int ret;
+
+ /* Device reset was added in vPCI protocol version 1.5. */
+ if (hbus->protocol_version < PCI_PROTOCOL_VERSION_1_5)
+ return -ENOTTY;
+
+ /* Hyper-V exposes projected functions directly on the root bus. */
+ if (!pci_is_root_bus(pdev->bus))
+ return -ENOTTY;
+
+ if (probe)
+ return 0;
+
+ /* Do not take state_lock: eject holds it while removing/locking pdev. */
+ state = READ_ONCE(hbus->state);
+ if (state != hv_pcibus_probed && state != hv_pcibus_installed)
+ return -ENODEV;
+
+ init_completion(&comp_pkt.host_event);
+ reset.message_type.type = PCI_RESET_DEVICE;
+ reset.wslot.slot = devfn_to_wslot(pdev->devfn);
+
+ ret = vmbus_sendpacket(hbus->hdev->channel, &reset, sizeof(reset),
+ (unsigned long)&pkt, VM_PKT_DATA_INBAND,
+ VMBUS_DATA_PACKET_FLAG_COMPLETION_REQUESTED);
+ if (ret)
+ return ret;
+
+ ret = wait_for_response(hbus->hdev, &comp_pkt.host_event);
+ if (ret)
+ return ret;
+
+ if (comp_pkt.completion_status == STATUS_NOT_SUPPORTED)
+ return -ENOTTY;
+
+ if (comp_pkt.completion_status) {
+ pci_err(pdev, "Hyper-V device reset failed: %#x\n",
+ comp_pkt.completion_status);
+ return -EIO;
+ }
+
+ return 0;
+}
+
/* PCIe operations */
static struct pci_ops hv_pcifront_ops = {
.read = hv_pcifront_read_config,
.write = hv_pcifront_write_config,
+ .reset = hv_pcifront_reset,
};
/*
@@ -1996,6 +2056,7 @@ static void hv_compose_msi_msg(struct irq_data *data, struct msi_msg *msg)
break;
case PCI_PROTOCOL_VERSION_1_4:
+ case PCI_PROTOCOL_VERSION_1_5:
size = hv_compose_msi_req_v3(&ctxt.int_pkts.v3,
cpu,
hpdev->desc.win_slot.slot,
--
2.53.0
^ permalink raw reply related
* [PATCH RFC 1/2] PCI: Add controller reset method
From: wei.liu @ 2026-07-24 23:08 UTC (permalink / raw)
To: Linux on Hyper-V List, linux-pci; +Cc: Wei Liu, Bjorn Helgaas, open list
In-Reply-To: <20260724230844.3259741-1-wei.liu@kernel.org>
From: Wei Liu <wei.liu@kernel.org>
Some PCI controllers provide a function reset mechanism that is not
advertised in PCI configuration space. Allow them to expose it through an
optional pci_ops callback.
Add the controller reset method to the standard reset_method interface.
Prefer FLR and AF FLR by default, and use the controller operation before
PM and bus reset fallbacks.
Signed-off-by: Wei Liu <wei.liu@kernel.org>
---
drivers/pci/pci.c | 22 ++++++++++++++++++++++
include/linux/pci.h | 3 ++-
2 files changed, 24 insertions(+), 1 deletion(-)
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 77b17b13ee61..4e55da202cad 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -4962,6 +4962,27 @@ static int pci_reset_bus_function(struct pci_dev *dev, bool probe)
return rc;
}
+static int pci_controller_reset(struct pci_dev *dev, bool probe)
+{
+ int rc;
+
+ if (!dev->bus->ops->reset)
+ return -ENOTTY;
+
+ if (probe)
+ return dev->bus->ops->reset(dev, probe);
+
+ rc = pci_dev_reset_iommu_prepare(dev);
+ if (rc) {
+ pci_err(dev, "failed to stop IOMMU for a PCI reset: %d\n", rc);
+ return rc;
+ }
+
+ rc = dev->bus->ops->reset(dev, probe);
+ pci_dev_reset_iommu_done(dev);
+ return rc;
+}
+
static int cxl_reset_bus_function(struct pci_dev *dev, bool probe)
{
struct pci_dev *bridge;
@@ -5094,6 +5115,7 @@ const struct pci_reset_fn_method pci_reset_fn_methods[] = {
{ pci_dev_acpi_reset, .name = "acpi" },
{ pcie_reset_flr, .name = "flr" },
{ pci_af_flr, .name = "af_flr" },
+ { pci_controller_reset, .name = "controller" },
{ pci_pm_reset, .name = "pm" },
{ pci_reset_bus_function, .name = "bus" },
{ cxl_reset_bus_function, .name = "cxl_bus" },
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 64b308b6e61c..d7759ee70670 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -52,7 +52,7 @@
PCI_STATUS_PARITY)
/* Number of reset methods used in pci_reset_fn_methods array in pci.c */
-#define PCI_NUM_RESET_METHODS 8
+#define PCI_NUM_RESET_METHODS 9
#define PCI_RESET_PROBE true
#define PCI_RESET_DO_RESET false
@@ -875,6 +875,7 @@ struct pci_ops {
void __iomem *(*map_bus)(struct pci_bus *bus, unsigned int devfn, int where);
int (*read)(struct pci_bus *bus, unsigned int devfn, int where, int size, u32 *val);
int (*write)(struct pci_bus *bus, unsigned int devfn, int where, int size, u32 val);
+ int (*reset)(struct pci_dev *dev, bool probe);
};
/*
--
2.53.0
^ permalink raw reply related
* [PATCH RFC 0/2] Support Hyper-V vPCI controller reset method
From: wei.liu @ 2026-07-24 23:08 UTC (permalink / raw)
To: Linux on Hyper-V List, linux-pci; +Cc: Wei Liu
From: Wei Liu <wei.liu@kernel.org>
Hi,
On Azure, there are devices that don't support function level or PM
reset. They can only be reset by sending a VMBus message to the host.
This series is RFC because I modelled the reset method at the controller
level. If there is a better way to do it, I'm happy to change the code.
Thanks,
Wei
Wei Liu (2):
PCI: Add controller reset method
PCI: hv: Add vPCI device reset support
Documentation/virt/hyperv/vpci.rst | 2 +-
drivers/pci/controller/pci-hyperv.c | 61 +++++++++++++++++++++++++++++
drivers/pci/pci.c | 22 +++++++++++
include/linux/pci.h | 3 +-
4 files changed, 86 insertions(+), 2 deletions(-)
--
2.53.0
^ permalink raw reply
* Re: [PATCH V4 1/9] mshv: Provide a way to get partition ID if running in a VMM process
From: Jacob Pan @ 2026-07-24 22:08 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: <95a1bed4-dd5a-654b-9e7a-7d5a453e3966@linux.microsoft.com>
Hi Mukesh,
On Fri, 24 Jul 2026 12:14:08 -0700
Mukesh R <mrathor@linux.microsoft.com> wrote:
> On 7/23/26 15:21, Jacob Pan wrote:
> > Hi Mukesh,
>
> Hey Jacob, pl see inline..
>
> > On Fri, 17 Jul 2026 19:19:41 -0700
> > Mukesh R <mrathor@linux.microsoft.com> wrote:
>
> ... snip...
>
> >> static int
> >> add_partition(struct mshv_partition *partition)
> >> {
> >> @@ -2073,6 +2094,7 @@ mshv_ioctl_create_partition(void __user
> >> *user_arg, struct device *module_dev) goto cleanup_irq_srcu;
> >>
> >> partition->pt_id = pt_id;
> >> + partition->pt_vmm_tgid = current->tgid;
> > I wonder how robust this mechanism is to identify target partition
> > via tgid.
> > 1) what prevents a VMM process create more than one partition? in
> > that case each partition would have the same tgid.
>
> Currently, none of the VMMs we support do that, and doesn't look like
> there is much of a demand for it.
>
My point is that from kernel UAPI pov, we cannot count on user behavior
nor current VMM's implementation.
> > 2) IIUC, the lifetime of the partition is tied to FD, which is
> > different than the lifetime of a PID. The partition FD can be
> > inherited or passed to another process. The VMM tg can exit while
> > the FDs can be alive. Then the tgid can be reused by another
> > unrelated process, right?
>
> yeah, AI keeps telling me that, but not super accurate imo.
>
> we are using tgid and not pid. tgid is process group id, and that will
> stay around as long as there is at least one process in it. if we used
> pid, then that would be the case.
>
I understand it is tgid but don't think tgid makes difference in terms
of FD lifetime.
e.g. process A creates a partition, then passes the partition fd to
process B over a Unix socket. Process A can then exit while process B
still holds a reference to the partition file. At that point the tgid
that was stored in pt_vmm_tgid can be reused by an unrelated process C,
while the partition's file object remains alive.
> > Would it be more robust to based this on the partition FD instead of
> > tgid?
>
> it might be, but problem with that is we need pt-id in other cases
> where that is not available: for example in
> hv_iommu_domain_alloc_paging and in irq remapping paths for direct
> attached devices. if we can sort that out somehow, then we can do
> that. but i suspect, it would take some time to figure that out, so i
> hope we can make that a future enhancement.
>
> For now, i've been thinking of just putting a check and returning
> ENOTSUPP if a vmm tries to create another partition.
>
IMHO, that only solves the uniqueness issue of partition ID but not the
lifetime issue.
maybe EEXIST instead of ENOTSUPP?
> Thanks,
> -Mukesh
>
> >> ret = add_partition(partition);
> >> if (ret)
> >> diff --git a/include/asm-generic/mshyperv.h
> >> b/include/asm-generic/mshyperv.h index bf601d67cecb..e8cbc4e3f7ad
> >> 100644 --- a/include/asm-generic/mshyperv.h
> >> +++ b/include/asm-generic/mshyperv.h
> >> @@ -350,6 +350,7 @@ int hv_call_add_logical_proc(int node, u32
> >> lp_index, u32 acpi_id); int
> >> hv_call_notify_all_processors_started(void); bool hv_lp_exists(u32
> >> lp_index); int hv_call_create_vp(int node, u64 partition_id, u32
> >> vp_index, u32 flags); +u64 mshv_current_partid(void);
> >>
> >> #else /* CONFIG_MSHV_ROOT */
> >> static inline bool hv_root_partition(void) { return false; }
> >> @@ -380,6 +381,10 @@ static inline int hv_call_create_vp(int node,
> >> u64 partition_id, u32 vp_index, u3 {
> >> return -EOPNOTSUPP;
> >> }
> >> +static inline u64 mshv_current_partid(void)
> >> +{
> >> + return HV_PARTITION_ID_INVALID;
> >> +}
> >> #endif /* CONFIG_MSHV_ROOT */
> >>
> >> static inline int hv_deposit_memory(u64 partition_id, u64
> >> status)
^ permalink raw reply
* Re: [PATCH V4 8/9] mshv: Populate mmio mappings for PCI passthru
From: Mukesh R @ 2026-07-24 21:41 UTC (permalink / raw)
To: sashiko-reviews; +Cc: linux-pci, linux-hyperv
In-Reply-To: <20260718023321.925F11F000E9@smtp.kernel.org>
On 7/17/26 19:33, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 8 potential issue(s) to consider:
>
> New issues:
> - [Critical] hv_map_mmio_pages extrapolates the physical base address for an entire MMIO region from a single page fault, allowing unauthorized arbitrary physical memory access.
> - [High] Flawed alignment logic allows mapping unaligned addresses as 2M large pages and fails to iterate the physical frame variable.
> - [High] Unconditional use of FAULT_FLAG_WRITE in mshv_chk_get_mmio_start_pfn breaks read-only MMIO mappings.
Above 2 addressed in next version.
Thanks,
-Mukesh
... snip ...
^ permalink raw reply
* Re: [PATCH V4 4/9] mshv: Add ioctl support for MSHV-VFIO bridge device
From: Mukesh R @ 2026-07-24 21:27 UTC (permalink / raw)
To: Jacob Pan
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
In-Reply-To: <20260724102029.00001c54@linux.microsoft.com>
On 7/24/26 10:20, Jacob Pan wrote:
> Hi Mukesh,
>
> On Fri, 17 Jul 2026 19:19:44 -0700
> Mukesh R <mrathor@linux.microsoft.com> wrote:
>
>> + hlist_add_head(&mshv_dev->device_ptnode,
>> &partition->pt_devices); +
>> + mshv_partition_get(partition);
>> + rc = anon_inode_getfd(vfio_ops->device_name,
>> &mshv_device_fops,
>> + mshv_dev, O_RDWR | O_CLOEXEC);
>> + if (rc < 0)
>> + goto undo_out;
>> +
>> + devargk.fd = rc;
>> + if (copy_to_user(uarg, &devargk, sizeof(devargk)))
>> + return -EFAULT; /* cleanup in
>> mshv_device_fop_release() */ +
> In failure, user never gets the fd, so it never close it. We are leaking
> fd until process exit, right?
>
> Maybe we should do anon_inode_getfile(...) and do fd_install() only if
> copy_to_user succeeds.
>
> Thanks,
>
> Jacob
Hey,
right, but efault should result in sigsegv and immediate exit... unless
some rogue vmm is trapping it and doing something malicous in which case
all it can do is fill up only its own file descriptor table.
given that the above is same as kvm_ioctl_create_device(), i think it
is ok to leave as is. if you think both need changing, lmk.
Thanks,
-Mukesh
^ permalink raw reply
* Re: [PATCH v11 1/8] mm/hmm: move page fault handling out of walk callbacks
From: Stanislav Kinsburskii @ 2026-07-24 19:48 UTC (permalink / raw)
To: David Hildenbrand (Arm)
Cc: Jason Gunthorpe, Leon Romanovsky, Andrew Morton, Lorenzo Stoakes,
Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
Suren Baghdasaryan, Michal Hocko, Jonathan Corbet, Shuah Khan,
Shuah Khan, K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui,
Long Li, Lyude Paul, Danilo Krummrich, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter,
Min Ma, Lizhi Hou, Oded Gabbay, linux-mm, linux-kernel, linux-doc,
linux-kselftest, linux-hyperv, dri-devel, nouveau, linux-rdma
In-Reply-To: <7655eaad-dcbf-4275-93f6-1f92ffedfa76@kernel.org>
On Fri, Jul 24, 2026 at 09:00:24PM +0200, David Hildenbrand (Arm) wrote:
> On 7/23/26 19:36, Stanislav Kinsburskii wrote:
> > hmm_range_fault() currently triggers page faults from inside the page-table
> > walk callbacks: hmm_vma_walk_pmd(), hmm_vma_walk_pud(),
> > hmm_vma_walk_hugetlb_entry() and the pte-level helper all call
> > hmm_vma_fault(), which in turn calls handle_mm_fault() while the walker
> > still holds nested locks. The pte spinlock is dropped explicitly by each
> > caller, and the hugetlb path manually drops and retakes
> > hugetlb_vma_lock_read around the fault to dodge a deadlock against the walk
> > framework's unconditional unlock.
> >
> > This layering does not extend cleanly to fault handlers that may release
> > mmap_lock (VM_FAULT_RETRY, VM_FAULT_COMPLETED). If the lock is dropped
> > while walk_page_range() is mid-traversal, the VMA can be freed before the
> > walk framework's matching hugetlb_vma_unlock_read(), turning that unlock
> > into a use-after-free.
> >
> > Split the responsibilities the way get_user_pages() does. Walk callbacks
> > become inspect-only: when they detect a range that needs to be faulted in,
> > they record it in struct hmm_vma_walk and return a private sentinel
> > (HMM_FAULT_PENDING). The outer loop in hmm_range_fault() then drops out of
> > walk_page_range(), invokes a new helper hmm_do_fault() that calls
> > handle_mm_fault() with only mmap_lock held, and restarts the walk so the
> > now-present entries are collected into hmm_pfns.
> >
> > No functional change for existing callers. As a side effect the hugetlb
> > callback no longer needs the hugetlb_vma_{un}lock_read dance, and every
> > fault-path exit from the callbacks now releases the pte spinlock on a
> > single, common path. This refactor is also a precursor for adding an
> > unlockable variant of hmm_range_fault() in a follow-up patch.
> >
> > Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
> > Signed-off-by: Stanislav Kinsburskii <skinsburskii@gmail.com>
> > ---
>
> Any reason my RB got dropped?
>
> https://lore.kernel.org/all/0b9be5b3-93aa-407f-b83d-409bec4b55e1@kernel.org/
>
No reason, just an omission on my side.
Andrew, could you add David's RB to this patch, please?
Thanks,
Stanislav
> --
> Cheers,
>
> David
^ permalink raw reply
* RE: [RFC PATCH 2/2] hv_netvsc: back GPADL buffers with kmalloc + decrypt + vmap
From: Michael Kelley @ 2026-07-24 19:36 UTC (permalink / raw)
To: Kameron Carr, decui@microsoft.com, haiyangz@microsoft.com,
kys@microsoft.com, longli@microsoft.com, wei.liu@kernel.org,
Michael Kelley
Cc: andrew+netdev@lunn.ch, davem@davemloft.net, edumazet@google.com,
kuba@kernel.org, pabeni@redhat.com, linux-hyperv@vger.kernel.org,
linux-kernel@vger.kernel.org, netdev@vger.kernel.org
In-Reply-To: <20260721195633.1438361-3-kameroncarr@linux.microsoft.com>
From: Kameron Carr <kameroncarr@linux.microsoft.com> Sent: Tuesday, July 21, 2026 12:57 PM
>
> Allocate the buffer as a list of physically-contiguous chunks using
> alloc_pages_node() starting at MAX_PAGE_ORDER and falling back to smaller
> orders. Each chunk is transitioned to host-visible via
> set_memory_decrypted() on its direct-map address, then all chunks are
> stitched together via vmap() into a single virtually-contiguous address.
The commit message should describe the "why" and not just the "what".
So mention the high-level motivations for this approach:
1) Avoids the need for set_memory_decrypted()/encrypted() to work
on vmalloc'ed memory, since (so far) that requirement is specific to
the Hyper-V netvsc driver.
2) Minimizes scattering of decrypted 4 KiB pages through the kernel
direct map, with the resulting impact of shattering large page mappings.
3) Meets the requirement to allocate the large netvsc buffers if a
synthetic NIC is added after the kernel has been running a while and
memory is fragmented to the point that high order allocations fail.
>
> Use vmbus_establish_gpadl_caller_decrypted() so the vmbus layer doesn't
> try to decrypt the virtual address. At teardown, vunmap() the range and
> re-encrypt and free each chunk individually; any chunk that fails
> re-encryption is leaked to prevent accidentally freeing decrypted memory.
>
> Because vunmap() and set_memory_encrypted() must run in process context,
> replace the rcu_head/call_rcu() pair used to defer free_netvsc_device()
> with rcu_work/queue_rcu_work().
This special RCU handling points out a problem with the current
upstream code. I don't know the exact motivation, but presumably
the freeing of the large buffers is done after the RCU grace period
because the buffers might still be accessed by code in the networking
stack. In current code, tearing down the GPADL is done before doing
the RCU free. But tearing down the GPADL re-encrypts the large
buffer. Re-encryption is a multi-step process, and if the networking
stack should access the buffer at certain points in that process,
unhanded faults will occur. It's a small timing window on a
relatively infrequent operation, but it's a risk nonetheless.
This patch fixes the race condition.
>
> Signed-off-by: Kameron Carr <kameroncarr@linux.microsoft.com>
> ---
> drivers/net/hyperv/hyperv_net.h | 18 ++-
> drivers/net/hyperv/netvsc.c | 271 ++++++++++++++++++++++++++++----
> drivers/net/hyperv/netvsc_drv.c | 6 +
> 3 files changed, 261 insertions(+), 34 deletions(-)
>
> diff --git a/drivers/net/hyperv/hyperv_net.h b/drivers/net/hyperv/hyperv_net.h
> index 7397c693f984a..b0c4cb0f7a4ce 100644
> --- a/drivers/net/hyperv/hyperv_net.h
> +++ b/drivers/net/hyperv/hyperv_net.h
> @@ -220,6 +220,8 @@ struct net_device_context;
>
> extern u32 netvsc_ring_bytes;
>
> +int netvsc_workqueue_init(void);
> +void netvsc_workqueue_destroy(void);
> struct netvsc_device *netvsc_device_add(struct hv_device *device,
> const struct netvsc_device_info *info);
> int netvsc_alloc_recv_comp_ring(struct netvsc_device *net_device, u32 q_idx);
> @@ -1147,6 +1149,16 @@ struct netvsc_channel {
> struct netvsc_stats_rx rx_stats;
> };
>
> +/* A physically-contiguous chunk of netvsc buffer.
> + *
> + * The chunk list is preserved so each chunk can be individually freed at
> + * teardown.
> + */
> +struct netvsc_buf_chunk {
> + struct page *page;
> + unsigned int order;
> +};
> +
> /* Per netvsc device */
> struct netvsc_device {
> u32 nvsp_version;
> @@ -1158,6 +1170,8 @@ struct netvsc_device {
> /* Receive buffer allocated by us but manages by NetVSP */
> void *recv_buf;
> u32 recv_buf_size; /* allocated bytes */
> + struct netvsc_buf_chunk *recv_buf_chunks;
> + u32 recv_buf_chunk_cnt;
> struct vmbus_gpadl recv_buf_gpadl_handle;
> u32 recv_section_cnt;
> u32 recv_section_size;
> @@ -1166,6 +1180,8 @@ struct netvsc_device {
> /* Send buffer allocated by us */
> void *send_buf;
> u32 send_buf_size;
> + struct netvsc_buf_chunk *send_buf_chunks;
> + u32 send_buf_chunk_cnt;
> struct vmbus_gpadl send_buf_gpadl_handle;
> u32 send_section_cnt;
> u32 send_section_size;
> @@ -1193,7 +1209,7 @@ struct netvsc_device {
>
> struct netvsc_channel chan_table[VRSS_CHANNEL_MAX];
>
> - struct rcu_head rcu;
> + struct rcu_work rwork;
> };
>
> /* NdisInitialize message */
> diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c
> index 59e95341f9b1e..2505d10e22010 100644
> --- a/drivers/net/hyperv/netvsc.c
> +++ b/drivers/net/hyperv/netvsc.c
> @@ -14,6 +14,8 @@
> #include <linux/mm.h>
> #include <linux/delay.h>
> #include <linux/io.h>
> +#include <linux/log2.h>
> +#include <linux/set_memory.h>
> #include <linux/slab.h>
> #include <linux/netdevice.h>
> #include <linux/if_ether.h>
> @@ -28,6 +30,8 @@
> #include "hyperv_net.h"
> #include "netvsc_trace.h"
>
> +static struct workqueue_struct *netvsc_wq;
> +
> /*
> * Switch the data path from the synthetic interface to the VF
> * interface.
> @@ -125,6 +129,194 @@ static void netvsc_subchan_work(struct work_struct *w)
> rtnl_unlock();
> }
>
> +/*
> + * netvsc_free_buf_pages - release a netvsc send/receive buffer.
> + *
> + * @addr: buffer address, or NULL if none was allocated (e.g. cleanup from a
> + * failed allocation)
> + * @chunks: chunks array from netvsc_alloc_buf_pages(), or NULL
> + * @chunk_cnt: number of entries in @chunks
> + *
> + * When @chunks is NULL the buffer is a plain vzalloc() allocation.
> + *
> + * Otherwise tear down the vmap, and for each chunk re-encrypt and free
> + * the underlying pages. Any chunk that cannot be re-encrypted is leaked.
> + */
> +static void netvsc_free_buf_pages(void *addr,
> + struct netvsc_buf_chunk *chunks,
> + u32 chunk_cnt)
> +{
> + u32 i;
> +
> + if (!chunks) {
> + vfree(addr);
> + return;
> + }
> +
> + vunmap(addr);
> +
> + for (i = 0; i < chunk_cnt; i++) {
> + unsigned long vaddr =
> + (unsigned long)page_address(chunks[i].page);
> + unsigned int order = chunks[i].order;
> +
> + if (set_memory_encrypted(vaddr, 1U << order))
> + continue;
> + __free_pages(chunks[i].page, order);
> + }
> +
> + kvfree(chunks);
> +}
> +
> +/*
> + * netvsc_alloc_buf_pages - allocate a virtually-contiguous, host-visible
> + * buffer for the netvsc send/receive area.
> + *
> + * @node: NUMA node hint (NUMA_NO_NODE for any node)
> + * @size: requested buffer size in bytes (rounded up to PAGE_SIZE)
A slight ambiguity about rounding up. Is the text here saying that the
requested size must already be a multiple of PAGE_SIZE, or that this
function will round up the requested size? I think it's the latter, so
maybe "(will be rounded up to PAGE_SIZE)" is more explicit and clear.
> + * @chunks_out: on success, set to the array of underlying chunks
> + * @chunk_cnt_out: on success, set to the number of chunks
> + *
> + * Allocates the buffer as a series of physically-contiguous chunks,
> + * starting at MAX_PAGE_ORDER and falling back to smaller orders on
> + * allocation failure. Each chunk is transitioned to host-visible via
> + * set_memory_decrypted() on its direct-map address, then all chunks are
> + * combined into a virtually-contiguous range via vmap().
> + *
> + * Return: the vmap()ed virtual address, or NULL on failure.
> + */
> +static void *netvsc_alloc_buf_pages(int node, u32 size,
> + struct netvsc_buf_chunk **chunks_out,
> + u32 *chunk_cnt_out)
> +{
> + u32 nr_pages = PFN_UP(size);
> + struct netvsc_buf_chunk *chunks = NULL;
> + struct page **pages = NULL;
> + unsigned int order;
> + u32 chunk_cnt = 0;
> + u32 page_idx = 0;
> + u32 remaining = nr_pages;
> + void *addr;
> + u32 i;
> + int ret;
> +
> + *chunks_out = NULL;
> + *chunk_cnt_out = 0;
> +
> + if (!nr_pages)
> + return NULL;
> +
> + /* Worst case: every chunk is a single page. */
> + chunks = kvmalloc_array(nr_pages, sizeof(*chunks),
> + GFP_KERNEL | __GFP_ZERO);
> + if (!chunks)
> + goto err;
> +
> + pages = kvmalloc_array(nr_pages, sizeof(*pages), GFP_KERNEL);
> + if (!pages)
> + goto err;
> +
> + /*
> + * @order monotonically decreases across iterations
> + *
> + * Use __GFP_NORETRY | __GFP_NOWARN to avoid OOM-killing, but try
> + * harder at order 0 since that is the final fallback.
> + */
> + order = min_t(unsigned int, MAX_PAGE_ORDER, ilog2(nr_pages));
Prefer using min() instead of min_t(). For normal integer values,
min() should work correctly.
> + while (remaining) {
> + struct page *page;
> + gfp_t gfp;
> +
> + order = min_t(unsigned int, order, ilog2(remaining));
Same here.
> +
> + for (;;) {
> + gfp = GFP_KERNEL | __GFP_ZERO;
> + if (order)
> + gfp |= __GFP_NORETRY | __GFP_NOWARN;
> + page = alloc_pages_node(node, gfp, order);
> + if (page)
> + break;
> + if (!order)
> + goto err;
> + order--;
> + }
> +
> + ret = set_memory_decrypted((unsigned long)page_address(page),
> + 1U << order);
> + if (ret) {
> + /*
> + * set_memory_decrypted() failed; the page state is
> + * unknown so it must be leaked rather than freed.
> + */
> + goto err;
> + }
> +
> + chunks[chunk_cnt].page = page;
> + chunks[chunk_cnt].order = order;
Is it necessary to explicitly save "order" in the chunks array? For
contiguously allocated memory like this, where "page" is a
struct page *, I think netvsc_free_buf_pages() can do
folio_order(page_folio(page))
to get the order.
> + chunk_cnt++;
> +
> + for (i = 0; i < (1U << order); i++)
> + pages[page_idx++] = page + i;
> +
> + remaining -= 1U << order;
> + }
The memory was zero'ed before decryption. But after decryption, it
may no longer be zero, depending on the underlying hardware. TDX
zero's the memory after decryption. But SEV-SNP does not. I'm unsure
about CCA.
It's not clear to me that these buffers really *need* to be zero'ed
for netvsc's purposes, but historically they have been, so you would
need to zero the full buffer again. It's probably best to do it a few lines
below as a single operation using the vmap() address.
> +
> + addr = vmap(pages, nr_pages, VM_MAP, pgprot_decrypted(PAGE_KERNEL));
> + if (!addr)
> + goto err;
> +
> + kvfree(pages);
> + *chunks_out = chunks;
> + *chunk_cnt_out = chunk_cnt;
> + return addr;
> +
> +err:
> + kvfree(pages);
> + netvsc_free_buf_pages(NULL, chunks, chunk_cnt);
> + return NULL;
> +}
> +
> +static void __free_netvsc_device(struct netvsc_device *nvdev)
> +{
> + int i;
> +
> + kfree(nvdev->extension);
> +
> + netvsc_free_buf_pages(nvdev->recv_buf, nvdev->recv_buf_chunks,
> + nvdev->recv_buf_chunk_cnt);
> + netvsc_free_buf_pages(nvdev->send_buf, nvdev->send_buf_chunks,
> + nvdev->send_buf_chunk_cnt);
> + bitmap_free(nvdev->send_section_map);
> +
> + for (i = 0; i < VRSS_CHANNEL_MAX; i++) {
> + xdp_rxq_info_unreg(&nvdev->chan_table[i].xdp_rxq);
> + kfree(nvdev->chan_table[i].recv_buf);
> + vfree(nvdev->chan_table[i].mrc.slots);
> + }
> +
> + kfree(nvdev);
> +}
> +
> +static void free_netvsc_device(struct work_struct *w)
> +{
> + struct rcu_work *rwork = to_rcu_work(w);
> +
> + __free_netvsc_device(container_of(rwork, struct netvsc_device, rwork));
> +}
> +
> +int netvsc_workqueue_init(void)
> +{
> + netvsc_wq = alloc_workqueue("hv_netvsc", WQ_UNBOUND, 0);
> +
> + return netvsc_wq ? 0 : -ENOMEM;
> +}
> +
> +void netvsc_workqueue_destroy(void)
> +{
> + rcu_barrier();
> + destroy_workqueue(netvsc_wq);
> +}
> +
> static struct netvsc_device *alloc_net_device(void)
> {
> struct netvsc_device *net_device;
> @@ -143,36 +335,18 @@ static struct netvsc_device *alloc_net_device(void)
> init_completion(&net_device->channel_init_wait);
> init_waitqueue_head(&net_device->subchan_open);
> INIT_WORK(&net_device->subchan_work, netvsc_subchan_work);
> + INIT_RCU_WORK(&net_device->rwork, free_netvsc_device);
>
> return net_device;
> }
>
> -static void free_netvsc_device(struct rcu_head *head)
> -{
> - struct netvsc_device *nvdev
> - = container_of(head, struct netvsc_device, rcu);
> - int i;
> -
> - kfree(nvdev->extension);
> -
> - if (!nvdev->recv_buf_gpadl_handle.decrypted)
> - vfree(nvdev->recv_buf);
> - if (!nvdev->send_buf_gpadl_handle.decrypted)
> - vfree(nvdev->send_buf);
> - bitmap_free(nvdev->send_section_map);
> -
> - for (i = 0; i < VRSS_CHANNEL_MAX; i++) {
> - xdp_rxq_info_unreg(&nvdev->chan_table[i].xdp_rxq);
> - kfree(nvdev->chan_table[i].recv_buf);
> - vfree(nvdev->chan_table[i].mrc.slots);
> - }
> -
> - kfree(nvdev);
> -}
> -
> static void free_netvsc_device_rcu(struct netvsc_device *nvdev)
> {
> - call_rcu(&nvdev->rcu, free_netvsc_device);
> + /*
> + * Defer the actual free to process context: vunmap() and
> + * set_memory_encrypted() cannot run from RCU softirq context.
> + */
> + queue_rcu_work(netvsc_wq, &nvdev->rwork);
> }
>
> static void netvsc_revoke_recv_buf(struct hv_device *device,
> @@ -351,7 +525,20 @@ static int netvsc_init_buf(struct hv_device *device,
> buf_size = min_t(unsigned int, buf_size,
> NETVSC_RECEIVE_BUFFER_SIZE_LEGACY);
>
> - net_device->recv_buf = vzalloc(buf_size);
> + if (device->channel->co_external_memory) {
> + /* Confidential VM Bus leaves buffer encrypted */
> + net_device->recv_buf_chunks = NULL;
> + net_device->recv_buf_chunk_cnt = 0;
> + net_device->recv_buf = vzalloc(buf_size);
> + } else {
> + /* Otherwise, allocate decrypted buffer */
> + net_device->recv_buf =
> + netvsc_alloc_buf_pages(cpu_to_node(device->channel->target_cpu),
> + buf_size,
> + &net_device->recv_buf_chunks,
> + &net_device->recv_buf_chunk_cnt);
> + }
> +
I have several thoughts about this code, and the similar code below
for the send buffer.
Deciding whether to allocate decrypted or not in the netvsc driver
seems out-of-place. That decision should be in the VMBus code. I'd
recommend renaming and moving netvsc_free_buf_pages() and
netvsc_alloc_buf_pages() to drivers/hv/channel.c where the GPADL
code is. Then make the decision about decrypting in the new
vmbus_alloc_buf_pages() function and fallback to vzalloc() when
not decrypting. That way vmbus_alloc_buf_pages() will more
closely parallel vmbus_free_buf_pages(), which already has
code for the vfree() case. The recv_buf_chunks and
recv_buf_chunk_cnt fields stay in struct netvsc_device, and are
passed into vmbus_alloc/free_buf_pages().
In the new vmbus_alloc_buf_pages(), the decision to decrypt or not
is more complicated than you have here. A "normal" VM (i.e., not
Confidential) has co_external_memory set to false, so as coded here,
a normal VM takes the decrypted path instead of doing just vzalloc().
That actually works because set_memory_decrypted() and
pgprot_decrypted() are no-op's in a normal VM. Still, I think it's
probably better to do just vzalloc() in a normal VM and let vzalloc()
handle populating the pages, since there's no scattering problem
to ameliorate.
That's my comments for now. Next week, I may look at some
areas more closely, but don't necessarily wait for those
additional comments before spinning a new version.
Michael
> if (!net_device->recv_buf) {
> netdev_err(ndev,
> "unable to allocate receive buffer of size %u\n",
> @@ -367,9 +554,10 @@ static int netvsc_init_buf(struct hv_device *device,
> * channel. Note: This call uses the vmbus connection rather
> * than the channel to establish the gpadl handle.
> */
> - ret = vmbus_establish_gpadl(device->channel, net_device->recv_buf,
> - buf_size,
> - &net_device->recv_buf_gpadl_handle);
> + ret = vmbus_establish_gpadl_caller_decrypted(device->channel,
> + net_device->recv_buf,
> + buf_size,
> + &net_device->recv_buf_gpadl_handle);
> if (ret != 0) {
> netdev_err(ndev,
> "unable to establish receive buffer's gpadl\n");
> @@ -457,7 +645,19 @@ static int netvsc_init_buf(struct hv_device *device,
> buf_size = device_info->send_sections * device_info->send_section_size;
> buf_size = round_up(buf_size, PAGE_SIZE);
>
> - net_device->send_buf = vzalloc(buf_size);
> + if (device->channel->co_external_memory) {
> + /* Confidential VM Bus leaves buffer encrypted */
> + net_device->send_buf_chunks = NULL;
> + net_device->send_buf_chunk_cnt = 0;
> + net_device->send_buf = vzalloc(buf_size);
> + } else {
> + /* Otherwise, allocate decrypted buffer */
> + net_device->send_buf =
> + netvsc_alloc_buf_pages(cpu_to_node(device->channel->target_cpu),
> + buf_size,
> + &net_device->send_buf_chunks,
> + &net_device->send_buf_chunk_cnt);
> + }
> if (!net_device->send_buf) {
> netdev_err(ndev, "unable to allocate send buffer of size %u\n",
> buf_size);
> @@ -470,9 +670,10 @@ static int netvsc_init_buf(struct hv_device *device,
> * channel. Note: This call uses the vmbus connection rather
> * than the channel to establish the gpadl handle.
> */
> - ret = vmbus_establish_gpadl(device->channel, net_device->send_buf,
> - buf_size,
> - &net_device->send_buf_gpadl_handle);
> + ret = vmbus_establish_gpadl_caller_decrypted(device->channel,
> + net_device->send_buf,
> + buf_size,
> + &net_device-
> >send_buf_gpadl_handle);
> if (ret != 0) {
> netdev_err(ndev,
> "unable to establish send buffer's gpadl\n");
> @@ -1863,7 +2064,11 @@ struct netvsc_device *netvsc_device_add(struct hv_device *device,
> netif_napi_del(&net_device->chan_table[0].napi);
>
> cleanup2:
> - free_netvsc_device(&net_device->rcu);
> + /*
> + * net_device was never published, so we don't need to wait for an
> + * RCU grace period -- call the free routine synchronously.
> + */
> + __free_netvsc_device(net_device);
>
> return ERR_PTR(ret);
> }
> diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
> index ee5ab5ceb2be2..1d43c73fd73f1 100644
> --- a/drivers/net/hyperv/netvsc_drv.c
> +++ b/drivers/net/hyperv/netvsc_drv.c
> @@ -2867,12 +2867,17 @@ static void __exit netvsc_drv_exit(void)
> {
> unregister_netdevice_notifier(&netvsc_netdev_notifier);
> vmbus_driver_unregister(&netvsc_drv);
> + netvsc_workqueue_destroy();
> }
>
> static int __init netvsc_drv_init(void)
> {
> int ret;
>
> + ret = netvsc_workqueue_init();
> + if (ret)
> + return ret;
> +
> if (ring_size < RING_SIZE_MIN) {
> ring_size = RING_SIZE_MIN;
> pr_info("Increased ring_size to %u (min allowed)\n",
> @@ -2890,6 +2895,7 @@ static int __init netvsc_drv_init(void)
>
> err_vmbus_reg:
> unregister_netdevice_notifier(&netvsc_netdev_notifier);
> + netvsc_workqueue_destroy();
> return ret;
> }
>
> --
> 2.45.4
>
^ permalink raw reply
* Re: [PATCH V4 1/9] mshv: Provide a way to get partition ID if running in a VMM process
From: Mukesh R @ 2026-07-24 19:14 UTC (permalink / raw)
To: Jacob Pan
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
In-Reply-To: <20260723152132.000033d0@linux.microsoft.com>
On 7/23/26 15:21, Jacob Pan wrote:
> Hi Mukesh,
Hey Jacob, pl see inline..
> On Fri, 17 Jul 2026 19:19:41 -0700
> Mukesh R <mrathor@linux.microsoft.com> wrote:
... snip...
>> static int
>> add_partition(struct mshv_partition *partition)
>> {
>> @@ -2073,6 +2094,7 @@ mshv_ioctl_create_partition(void __user
>> *user_arg, struct device *module_dev) goto cleanup_irq_srcu;
>>
>> partition->pt_id = pt_id;
>> + partition->pt_vmm_tgid = current->tgid;
> I wonder how robust this mechanism is to identify target partition via
> tgid.
> 1) what prevents a VMM process create more than one partition? in that
> case each partition would have the same tgid.
Currently, none of the VMMs we support do that, and doesn't look like
there is much of a demand for it.
> 2) IIUC, the lifetime of the partition is tied to FD, which is
> different than the lifetime of a PID. The partition FD can be
> inherited or passed to another process. The VMM tg can exit while
> the FDs can be alive. Then the tgid can be reused by another unrelated
> process, right?
yeah, AI keeps telling me that, but not super accurate imo.
we are using tgid and not pid. tgid is process group id, and that will
stay around as long as there is at least one process in it. if we used
pid, then that would be the case.
> Would it be more robust to based this on the partition FD instead of
> tgid?
it might be, but problem with that is we need pt-id in other cases
where that is not available: for example in hv_iommu_domain_alloc_paging
and in irq remapping paths for direct attached devices. if we can sort
that out somehow, then we can do that. but i suspect, it would take
some time to figure that out, so i hope we can make that a future
enhancement.
For now, i've been thinking of just putting a check and returning
ENOTSUPP if a vmm tries to create another partition.
Thanks,
-Mukesh
>> ret = add_partition(partition);
>> if (ret)
>> diff --git a/include/asm-generic/mshyperv.h
>> b/include/asm-generic/mshyperv.h index bf601d67cecb..e8cbc4e3f7ad
>> 100644 --- a/include/asm-generic/mshyperv.h
>> +++ b/include/asm-generic/mshyperv.h
>> @@ -350,6 +350,7 @@ int hv_call_add_logical_proc(int node, u32
>> lp_index, u32 acpi_id); int
>> hv_call_notify_all_processors_started(void); bool hv_lp_exists(u32
>> lp_index); int hv_call_create_vp(int node, u64 partition_id, u32
>> vp_index, u32 flags); +u64 mshv_current_partid(void);
>>
>> #else /* CONFIG_MSHV_ROOT */
>> static inline bool hv_root_partition(void) { return false; }
>> @@ -380,6 +381,10 @@ static inline int hv_call_create_vp(int node,
>> u64 partition_id, u32 vp_index, u3 {
>> return -EOPNOTSUPP;
>> }
>> +static inline u64 mshv_current_partid(void)
>> +{
>> + return HV_PARTITION_ID_INVALID;
>> +}
>> #endif /* CONFIG_MSHV_ROOT */
>>
>> static inline int hv_deposit_memory(u64 partition_id, u64 status)
^ permalink raw reply
* Re: [PATCH v11 1/8] mm/hmm: move page fault handling out of walk callbacks
From: David Hildenbrand (Arm) @ 2026-07-24 19:00 UTC (permalink / raw)
To: Stanislav Kinsburskii, Jason Gunthorpe, Leon Romanovsky,
Andrew Morton, Lorenzo Stoakes, Liam R. Howlett, Vlastimil Babka,
Mike Rapoport, Suren Baghdasaryan, Michal Hocko, Jonathan Corbet,
Shuah Khan, Shuah Khan, K. Y. Srinivasan, Haiyang Zhang, Wei Liu,
Dexuan Cui, Long Li, Lyude Paul, Danilo Krummrich,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Min Ma, Lizhi Hou, Oded Gabbay
Cc: linux-mm, linux-kernel, linux-doc, linux-kselftest, linux-hyperv,
dri-devel, nouveau, linux-rdma
In-Reply-To: <20260723-hmm-v10-v11-1-c55b003a4b61@gmail.com>
On 7/23/26 19:36, Stanislav Kinsburskii wrote:
> hmm_range_fault() currently triggers page faults from inside the page-table
> walk callbacks: hmm_vma_walk_pmd(), hmm_vma_walk_pud(),
> hmm_vma_walk_hugetlb_entry() and the pte-level helper all call
> hmm_vma_fault(), which in turn calls handle_mm_fault() while the walker
> still holds nested locks. The pte spinlock is dropped explicitly by each
> caller, and the hugetlb path manually drops and retakes
> hugetlb_vma_lock_read around the fault to dodge a deadlock against the walk
> framework's unconditional unlock.
>
> This layering does not extend cleanly to fault handlers that may release
> mmap_lock (VM_FAULT_RETRY, VM_FAULT_COMPLETED). If the lock is dropped
> while walk_page_range() is mid-traversal, the VMA can be freed before the
> walk framework's matching hugetlb_vma_unlock_read(), turning that unlock
> into a use-after-free.
>
> Split the responsibilities the way get_user_pages() does. Walk callbacks
> become inspect-only: when they detect a range that needs to be faulted in,
> they record it in struct hmm_vma_walk and return a private sentinel
> (HMM_FAULT_PENDING). The outer loop in hmm_range_fault() then drops out of
> walk_page_range(), invokes a new helper hmm_do_fault() that calls
> handle_mm_fault() with only mmap_lock held, and restarts the walk so the
> now-present entries are collected into hmm_pfns.
>
> No functional change for existing callers. As a side effect the hugetlb
> callback no longer needs the hugetlb_vma_{un}lock_read dance, and every
> fault-path exit from the callbacks now releases the pte spinlock on a
> single, common path. This refactor is also a precursor for adding an
> unlockable variant of hmm_range_fault() in a follow-up patch.
>
> Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
> Signed-off-by: Stanislav Kinsburskii <skinsburskii@gmail.com>
> ---
Any reason my RB got dropped?
https://lore.kernel.org/all/0b9be5b3-93aa-407f-b83d-409bec4b55e1@kernel.org/
--
Cheers,
David
^ permalink raw reply
* Re: [PATCH V4 4/9] mshv: Add ioctl support for MSHV-VFIO bridge device
From: Jacob Pan @ 2026-07-24 17:20 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: <20260718021949.926306-5-mrathor@linux.microsoft.com>
Hi Mukesh,
On Fri, 17 Jul 2026 19:19:44 -0700
Mukesh R <mrathor@linux.microsoft.com> wrote:
> + hlist_add_head(&mshv_dev->device_ptnode,
> &partition->pt_devices); +
> + mshv_partition_get(partition);
> + rc = anon_inode_getfd(vfio_ops->device_name,
> &mshv_device_fops,
> + mshv_dev, O_RDWR | O_CLOEXEC);
> + if (rc < 0)
> + goto undo_out;
> +
> + devargk.fd = rc;
> + if (copy_to_user(uarg, &devargk, sizeof(devargk)))
> + return -EFAULT; /* cleanup in
> mshv_device_fop_release() */ +
In failure, user never gets the fd, so it never close it. We are leaking
fd until process exit, right?
Maybe we should do anon_inode_getfile(...) and do fd_install() only if
copy_to_user succeeds.
Thanks,
Jacob
^ permalink raw reply
* Re: [PATCH v11 0/8] mm/hmm: Add mmap lock-drop support for userfaultfd-backed mappings
From: Stanislav Kinsburskii @ 2026-07-23 22:22 UTC (permalink / raw)
To: Andrew Morton
Cc: Jason Gunthorpe, Leon Romanovsky, David Hildenbrand,
Lorenzo Stoakes, Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
Suren Baghdasaryan, Michal Hocko, Jonathan Corbet, Shuah Khan,
Shuah Khan, K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui,
Long Li, Lyude Paul, Danilo Krummrich, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter,
Min Ma, Lizhi Hou, Oded Gabbay, linux-mm, linux-kernel, linux-doc,
linux-kselftest, linux-hyperv, dri-devel, nouveau, linux-rdma
In-Reply-To: <20260723142242.5d3f87208d47819d873ae458@linux-foundation.org>
On Thu, Jul 23, 2026 at 02:22:42PM -0700, Andrew Morton wrote:
> On Thu, 23 Jul 2026 10:36:32 -0700 Stanislav Kinsburskii <skinsburskii@gmail.com> wrote:
>
> > This series extends the HMM framework to support userfaultfd-backed memory
> > by allowing the mmap read lock to be dropped during hmm_range_fault().
> >
>
> Thanks, I've updated mm.git to this version.
>
> AI review suggests there may be some problems. Sorry, I don't recall if
> these were considered in previous versions of the patchset:
>
> https://sashiko.dev/#/patchset/20260723-hmm-v10-v11-0-c55b003a4b61@gmail.com
>
These look like new findings, perhaps from a newer model or review prompt.
I do not think they apply here.
The unlockable path sets FAULT_FLAG_ALLOW_RETRY only when HMM provides a
non-NULL lock state pointer. The legacy hmm_range_fault() path passes NULL
and therefore does not opt in to lock-dropping faults. Fault handlers are not
supposed to return VM_FAULT_RETRY/VM_FAULT_COMPLETED with the mmap lock
dropped unless FAULT_FLAG_ALLOW_RETRY allows that, so the NULL locked state is
not expected to be dereferenced on the legacy path.
For the FAULT_FLAG_TRIED concern, HMM is not trying to emulate GUP's exact
single-address retry loop. If a fault drops mmap_lock, HMM has to restart the
range walk because the VMA/page table state may have changed. Resetting the
timeout on that path is intentional: a lock-dropping fault made progress, and
the timeout is meant to bound mmu-notifier retry churn rather than the time
spent servicing faults.
Also, the newly converted HMM users are not using this path for ordinary
file-backed page-cache population, and process-context callers can still be
interrupted by fatal signals. All the existent kernel thread callers are
capped by a timeout.
So I do not think this requires a code change.
Thanks,
Stanislav
> >
> > Changes in v11:
> > - Reject unstable address spaces in hmm_range_fault_unlocked_timeout()
> > after taking mmap_lock and before walking page tables.
> > - Compute the remaining HMM timeout budget before the time_after_eq()
> > check in drm_gpusvm_get_pages() to make sure it can't result in zero
> > and lead to infinite HMM range faulting loop.
>
> Here's how v11 altered mm.git:
>
>
> drivers/gpu/drm/drm_gpusvm.c | 4 ++--
> mm/hmm.c | 6 ++++++
> 2 files changed, 8 insertions(+), 2 deletions(-)
>
> --- a/drivers/gpu/drm/drm_gpusvm.c~b
> +++ a/drivers/gpu/drm/drm_gpusvm.c
> @@ -1422,11 +1422,11 @@ int drm_gpusvm_get_pages(struct drm_gpus
> struct dma_iova_state *state = &svm_pages->state;
>
> retry:
> + remaining = timeout - jiffies;
> +
> if (time_after_eq(jiffies, timeout))
> return -EBUSY;
>
> - remaining = timeout - jiffies;
> -
> hmm_range.notifier_seq = mmu_interval_read_begin(notifier);
> if (drm_gpusvm_pages_valid_unlocked(gpusvm, svm_pages))
> goto set_seqno;
> --- a/mm/hmm.c~b
> +++ a/mm/hmm.c
> @@ -17,6 +17,7 @@
> #include <linux/slab.h>
> #include <linux/sched.h>
> #include <linux/mmzone.h>
> +#include <linux/oom.h>
> #include <linux/pagemap.h>
> #include <linux/leafops.h>
> #include <linux/hugetlb.h>
> @@ -806,6 +807,11 @@ int hmm_range_fault_unlocked_timeout(str
> if (ret)
> return ret;
>
> + if (check_stable_address_space(mm)) {
> + mmap_read_unlock(mm);
> + return -EFAULT;
> + }
> +
> if (timeout && time_after(jiffies, deadline)) {
> mmap_read_unlock(mm);
> return -EBUSY;
> _
>
^ permalink raw reply
* Re: [PATCH V4 1/9] mshv: Provide a way to get partition ID if running in a VMM process
From: Jacob Pan @ 2026-07-23 22:21 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: <20260718021949.926306-2-mrathor@linux.microsoft.com>
Hi Mukesh,
On Fri, 17 Jul 2026 19:19:41 -0700
Mukesh R <mrathor@linux.microsoft.com> wrote:
> Many PCI passthru related hypercalls require partition ID of the
> target guest. Guests are actually managed by MSHV driver and the
> partition ID is only maintained there. Add a field in the partition
> struct in MSHV driver to save the tgid of the VMM process creating
> the partition, and add a function there to retrieve partition ID if
> the current process is a VMM process.
>
> Signed-off-by: Mukesh R <mrathor@linux.microsoft.com>
> Reviewed-by: Anirudh Rayabharam (Microsoft) <anirudh@anirudhrb.com>
> ---
> drivers/hv/mshv_root.h | 1 +
> drivers/hv/mshv_root_main.c | 22 ++++++++++++++++++++++
> include/asm-generic/mshyperv.h | 5 +++++
> 3 files changed, 28 insertions(+)
>
> diff --git a/drivers/hv/mshv_root.h b/drivers/hv/mshv_root.h
> index 1f086dcb7aa1..a85c24dcc701 100644
> --- a/drivers/hv/mshv_root.h
> +++ b/drivers/hv/mshv_root.h
> @@ -138,6 +138,7 @@ struct mshv_partition {
>
> struct mshv_girq_routing_table __rcu *pt_girq_tbl;
> u64 isolation_type;
> + pid_t pt_vmm_tgid;
> bool import_completed;
> bool pt_initialized;
> #if IS_ENABLED(CONFIG_DEBUG_FS)
> diff --git a/drivers/hv/mshv_root_main.c b/drivers/hv/mshv_root_main.c
> index bd1359eb58dd..02c107458be9 100644
> --- a/drivers/hv/mshv_root_main.c
> +++ b/drivers/hv/mshv_root_main.c
> @@ -1908,6 +1908,27 @@ mshv_partition_release(struct inode *inode,
> struct file *filp) return 0;
> }
>
> +/* Given a process tgid, return partition id if it is a VMM process
> */ +u64 mshv_current_partid(void)
> +{
> + struct mshv_partition *pt;
> + int i;
> + u64 ret_ptid = HV_PARTITION_ID_INVALID;
> +
> + rcu_read_lock();
> +
> + hash_for_each_rcu(mshv_root.pt_htable, i, pt, pt_hnode) {
> + if (pt->pt_vmm_tgid == current->tgid) {
> + ret_ptid = pt->pt_id;
> + break;
> + }
> + }
> +
> + rcu_read_unlock();
> + return ret_ptid;
> +}
> +EXPORT_SYMBOL_GPL(mshv_current_partid);
> +
> static int
> add_partition(struct mshv_partition *partition)
> {
> @@ -2073,6 +2094,7 @@ mshv_ioctl_create_partition(void __user
> *user_arg, struct device *module_dev) goto cleanup_irq_srcu;
>
> partition->pt_id = pt_id;
> + partition->pt_vmm_tgid = current->tgid;
I wonder how robust this mechanism is to identify target partition via
tgid.
1) what prevents a VMM process create more than one partition? in that
case each partition would have the same tgid.
2) IIUC, the lifetime of the partition is tied to FD, which is
different than the lifetime of a PID. The partition FD can be
inherited or passed to another process. The VMM tg can exit while
the FDs can be alive. Then the tgid can be reused by another unrelated
process, right?
Would it be more robust to based this on the partition FD instead of
tgid?
> ret = add_partition(partition);
> if (ret)
> diff --git a/include/asm-generic/mshyperv.h
> b/include/asm-generic/mshyperv.h index bf601d67cecb..e8cbc4e3f7ad
> 100644 --- a/include/asm-generic/mshyperv.h
> +++ b/include/asm-generic/mshyperv.h
> @@ -350,6 +350,7 @@ int hv_call_add_logical_proc(int node, u32
> lp_index, u32 acpi_id); int
> hv_call_notify_all_processors_started(void); bool hv_lp_exists(u32
> lp_index); int hv_call_create_vp(int node, u64 partition_id, u32
> vp_index, u32 flags); +u64 mshv_current_partid(void);
>
> #else /* CONFIG_MSHV_ROOT */
> static inline bool hv_root_partition(void) { return false; }
> @@ -380,6 +381,10 @@ static inline int hv_call_create_vp(int node,
> u64 partition_id, u32 vp_index, u3 {
> return -EOPNOTSUPP;
> }
> +static inline u64 mshv_current_partid(void)
> +{
> + return HV_PARTITION_ID_INVALID;
> +}
> #endif /* CONFIG_MSHV_ROOT */
>
> static inline int hv_deposit_memory(u64 partition_id, u64 status)
^ permalink raw reply
* Re: [PATCH v11 0/8] mm/hmm: Add mmap lock-drop support for userfaultfd-backed mappings
From: Andrew Morton @ 2026-07-23 21:22 UTC (permalink / raw)
To: Stanislav Kinsburskii
Cc: Jason Gunthorpe, Leon Romanovsky, David Hildenbrand,
Lorenzo Stoakes, Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
Suren Baghdasaryan, Michal Hocko, Jonathan Corbet, Shuah Khan,
Shuah Khan, K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui,
Long Li, Lyude Paul, Danilo Krummrich, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter,
Min Ma, Lizhi Hou, Oded Gabbay, linux-mm, linux-kernel, linux-doc,
linux-kselftest, linux-hyperv, dri-devel, nouveau, linux-rdma
In-Reply-To: <20260723-hmm-v10-v11-0-c55b003a4b61@gmail.com>
On Thu, 23 Jul 2026 10:36:32 -0700 Stanislav Kinsburskii <skinsburskii@gmail.com> wrote:
> This series extends the HMM framework to support userfaultfd-backed memory
> by allowing the mmap read lock to be dropped during hmm_range_fault().
>
Thanks, I've updated mm.git to this version.
AI review suggests there may be some problems. Sorry, I don't recall if
these were considered in previous versions of the patchset:
https://sashiko.dev/#/patchset/20260723-hmm-v10-v11-0-c55b003a4b61@gmail.com
>
> Changes in v11:
> - Reject unstable address spaces in hmm_range_fault_unlocked_timeout()
> after taking mmap_lock and before walking page tables.
> - Compute the remaining HMM timeout budget before the time_after_eq()
> check in drm_gpusvm_get_pages() to make sure it can't result in zero
> and lead to infinite HMM range faulting loop.
Here's how v11 altered mm.git:
drivers/gpu/drm/drm_gpusvm.c | 4 ++--
mm/hmm.c | 6 ++++++
2 files changed, 8 insertions(+), 2 deletions(-)
--- a/drivers/gpu/drm/drm_gpusvm.c~b
+++ a/drivers/gpu/drm/drm_gpusvm.c
@@ -1422,11 +1422,11 @@ int drm_gpusvm_get_pages(struct drm_gpus
struct dma_iova_state *state = &svm_pages->state;
retry:
+ remaining = timeout - jiffies;
+
if (time_after_eq(jiffies, timeout))
return -EBUSY;
- remaining = timeout - jiffies;
-
hmm_range.notifier_seq = mmu_interval_read_begin(notifier);
if (drm_gpusvm_pages_valid_unlocked(gpusvm, svm_pages))
goto set_seqno;
--- a/mm/hmm.c~b
+++ a/mm/hmm.c
@@ -17,6 +17,7 @@
#include <linux/slab.h>
#include <linux/sched.h>
#include <linux/mmzone.h>
+#include <linux/oom.h>
#include <linux/pagemap.h>
#include <linux/leafops.h>
#include <linux/hugetlb.h>
@@ -806,6 +807,11 @@ int hmm_range_fault_unlocked_timeout(str
if (ret)
return ret;
+ if (check_stable_address_space(mm)) {
+ mmap_read_unlock(mm);
+ return -EFAULT;
+ }
+
if (timeout && time_after(jiffies, deadline)) {
mmap_read_unlock(mm);
return -EBUSY;
_
^ permalink raw reply
* RE: [RFC PATCH 1/2] Drivers: hv: vmbus: add vmbus_establish_gpadl_caller_decrypted()
From: Kameron Carr @ 2026-07-23 21:14 UTC (permalink / raw)
To: 'Michael Kelley', decui, haiyangz, kys, longli, wei.liu
Cc: andrew+netdev, davem, edumazet, kuba, pabeni, linux-hyperv,
linux-kernel, netdev
In-Reply-To: <BN7PR02MB414815ABD36AD06CA3319306D4C02@BN7PR02MB4148.namprd02.prod.outlook.com>
On Thursday, July 23, 2026 10:53 AM, Michael Kelley wrote:
> From: Kameron Carr <kameroncarr@linux.microsoft.com> Sent: Tuesday, July
21, 2026 12:57 PM
> >
> > Refactor vmbus_establish_gpadl() to separate the encryption lifecycle
> > from the rest of the GPADL establishment logic.
> >
> > Add a new vmbus_establish_gpadl_caller_decrypted() for callers that want
> > to decrypt their own buffers.
> >
> > No functional change for existing callers.
>
> I have a suggestion for a different way to accomplish this. I haven't
coded
> it, but I think it will result in less code churn and be simpler overall.
>
> 1) In enum hv_gpadl_type, add value HV_GPADL_BUFFER_DECRYPTED
>
> 2) Update hv_gpadl_size() and hv_gpadl_hvpfn() to treat the new enum
> value just like HV_GPADL_BUFFER
>
> 3) In __vmbus_establish_gpadl(), change this code
>
> gpadl->decrypted = !((channel->co_external_memory && type ==
HV_GPADL_BUFFER) ||
> (channel->co_ring_buffer && type == HV_GPADL_RING));
>
> to
>
> gpadl->decrypted = !((channel->co_external_memory && type ==
HV_GPADL_BUFFER) ||
> (channel->co_ring_buffer && type == HV_GPADL_RING) ||
> (type == HV_GPADL_BUFFER_DECRYPTED));
>
> 4) Add the function vmbus_establish_gpadl_caller_decrypted() like you
> have in this patch. It just calls __vmbus_establish_gpadl(), passing
> HV_GPADL_BUFFER_DECRYPTED.
Ack. Thank you for the feedback. I will use this approach in v2.
Regards,
Kameron
> I think that's all that is needed, though I haven't gone through
> everything rigorously and might be missing something.
>
> FWIW, the meaning of the "decrypted" field in struct vmbus_gpadl is
> a bit abused, but it was already that way. In current code, it initially
> means "decryption needed". Then after the gpadl is created, it
> means "was decrypted" so that gpadl teardown will re-encrypt.
> But my suggested approach doesn't really make the abuse any worse.
[...]
^ permalink raw reply
* Re: [PATCH v11 8/8] drm/gpusvm: Use hmm_range_fault_unlocked_timeout() for range faults
From: sashiko-bot @ 2026-07-23 17:54 UTC (permalink / raw)
To: Stanislav Kinsburskii; +Cc: dri-devel, linux-hyperv
In-Reply-To: <20260723-hmm-v10-v11-8-c55b003a4b61@gmail.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Missing `mmput()` on the `pfns` allocation failure path causes an `mm_struct` refcount leak.
--
--- Patch [8]: [PATCH v11 8/8] drm/gpusvm: Use hmm_range_fault_unlocked_timeout() for range faults ---
commit 83e2ac7a16c88b9dee7c9dc0f7c00f0dcd69b81d
Author: Stanislav Kinsburskii <skinsburskii@gmail.com>
drm/gpusvm: Use hmm_range_fault_unlocked_timeout() for range faults
This commit replaces open-coded HMM fault loops that hold the mmap read lock
with the hmm_range_fault_unlocked_timeout() helper. It delegates the lock
acquisition and retry logic to the helper while maintaining the existing
timeout budgets.
> diff --git a/drivers/gpu/drm/drm_gpusvm.c b/drivers/gpu/drm/drm_gpusvm.c
> index 958cb605aedd5..e912e994beb21 100644
> --- a/drivers/gpu/drm/drm_gpusvm.c
> +++ b/drivers/gpu/drm/drm_gpusvm.c
[ ... ]
> @@ -1736,24 +1708,12 @@ int drm_gpusvm_range_evict(struct drm_gpusvm *gpusvm,
> return -ENOMEM;
[Severity: High]
This is a pre-existing issue, but does this allocation failure path leak the
mm_struct reference?
In drm_gpusvm_range_evict() in drivers/gpu/drm/drm_gpusvm.c, earlier in this
function mmget_not_zero(mm) successfully acquires a reference to the mm. If
the subsequent kvmalloc_array() call fails, the function immediately returns
-ENOMEM.
Should there be an mmput(mm) before returning to prevent leaking the
mm_struct reference?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260723-hmm-v10-v11-0-c55b003a4b61@gmail.com?part=8
^ permalink raw reply
* Re: [PATCH v11 7/8] accel/amdxdna: Use hmm_range_fault_unlocked_timeout() for range population
From: sashiko-bot @ 2026-07-23 17:53 UTC (permalink / raw)
To: Stanislav Kinsburskii; +Cc: linux-hyperv, dri-devel
In-Reply-To: <20260723-hmm-v10-v11-7-c55b003a4b61@gmail.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Dereferencing `mapp->vma` without holding `mmap_lock` or a VMA lock leads to potential Use-After-Free.
--
--- Patch [7]: [PATCH v11 7/8] accel/amdxdna: Use hmm_range_fault_unlocked_timeout() for range population ---
commit 49a458a983bff0bc836f15feb7b3aa4763d7eb91
Author: Stanislav Kinsburskii <skinsburskii@gmail.com>
accel/amdxdna: Use hmm_range_fault_unlocked_timeout() for range population
This patch updates aie2_populate_range() to use the new
hmm_range_fault_unlocked_timeout() helper. The helper manages the mmap lock
and retry sequences internally. The driver now scopes the HMM retry timeout to
the helper while maintaining its outer invalidation loop.
> diff --git a/drivers/accel/amdxdna/aie2_ctx.c b/drivers/accel/amdxdna/aie2_ctx.c
> index 54486960cbf5e..b5b4ca263002d 100644
> --- a/drivers/accel/amdxdna/aie2_ctx.c
> +++ b/drivers/accel/amdxdna/aie2_ctx.c
[ ... ]
> @@ -1061,24 +1061,9 @@ static int aie2_populate_range(struct amdxdna_gem_obj *abo)
[Severity: High]
This isn't a bug introduced by this patch, but I noticed a potential
use-after-free with the VMA just before this block in aie2_populate_range():
XDNA_DBG(xdna, "populate memory range %lx %lx",
mapp->vma->vm_start, mapp->vma->vm_end);
mm = mapp->notifier.mm;
if (!mmget_not_zero(mm)) {
Is it safe to dereference mapp->vma here?
Since aie2_populate_range() only holds the xdna->notifier_lock at this point,
it doesn't appear to be tied to the VMA's lifetime. If a concurrent munmap
from another thread triggers an invalidation and frees the VMA entirely, could
the logging statement access a freed VMA structure when reading vm_start and
vm_end?
> return -EFAULT;
> }
>
> - mapp->range.notifier_seq = mmu_interval_read_begin(&mapp->notifier);
> - mmap_read_lock(mm);
> - ret = hmm_range_fault(&mapp->range);
> - mmap_read_unlock(mm);
> - if (ret) {
> - if (time_after(jiffies, timeout)) {
> - ret = -ETIME;
> - goto put_mm;
> - }
> -
> - if (ret == -EBUSY) {
> - amdxdna_umap_put(mapp);
> - mmput(mm);
> - goto again;
> - }
> -
> + ret = hmm_range_fault_unlocked_timeout(&mapp->range, timeout);
> + if (ret)
> goto put_mm;
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260723-hmm-v10-v11-0-c55b003a4b61@gmail.com?part=7
^ permalink raw reply
* RE: [RFC PATCH 1/2] Drivers: hv: vmbus: add vmbus_establish_gpadl_caller_decrypted()
From: Michael Kelley @ 2026-07-23 17:52 UTC (permalink / raw)
To: Kameron Carr, decui@microsoft.com, haiyangz@microsoft.com,
kys@microsoft.com, longli@microsoft.com, wei.liu@kernel.org,
Michael Kelley
Cc: andrew+netdev@lunn.ch, davem@davemloft.net, edumazet@google.com,
kuba@kernel.org, pabeni@redhat.com, linux-hyperv@vger.kernel.org,
linux-kernel@vger.kernel.org, netdev@vger.kernel.org
In-Reply-To: <20260721195633.1438361-2-kameroncarr@linux.microsoft.com>
From: Kameron Carr <kameroncarr@linux.microsoft.com> Sent: Tuesday, July 21, 2026 12:57 PM
>
> Refactor vmbus_establish_gpadl() to separate the encryption lifecycle
> from the rest of the GPADL establishment logic.
>
> Add a new vmbus_establish_gpadl_caller_decrypted() for callers that want
> to decrypt their own buffers.
>
> No functional change for existing callers.
I have a suggestion for a different way to accomplish this. I haven't coded
it, but I think it will result in less code churn and be simpler overall.
1) In enum hv_gpadl_type, add value HV_GPADL_BUFFER_DECRYPTED
2) Update hv_gpadl_size() and hv_gpadl_hvpfn() to treat the new enum
value just like HV_GPADL_BUFFER
3) In __vmbus_establish_gpadl(), change this code
gpadl->decrypted = !((channel->co_external_memory && type == HV_GPADL_BUFFER) ||
(channel->co_ring_buffer && type == HV_GPADL_RING));
to
gpadl->decrypted = !((channel->co_external_memory && type == HV_GPADL_BUFFER) ||
(channel->co_ring_buffer && type == HV_GPADL_RING) ||
(type == HV_GPADL_BUFFER_DECRYPTED));
4) Add the function vmbus_establish_gpadl_caller_decrypted() like you
have in this patch. It just calls __vmbus_establish_gpadl(), passing
HV_GPADL_BUFFER_DECRYPTED.
I think that's all that is needed, though I haven't gone through
everything rigorously and might be missing something.
FWIW, the meaning of the "decrypted" field in struct vmbus_gpadl is
a bit abused, but it was already that way. In current code, it initially
means "decryption needed". Then after the gpadl is created, it
means "was decrypted" so that gpadl teardown will re-encrypt.
But my suggested approach doesn't really make the abuse any worse.
>
> Signed-off-by: Kameron Carr <kameroncarr@linux.microsoft.com>
> ---
> drivers/hv/channel.c | 123 +++++++++++++++++++++++++++--------------
> include/linux/hyperv.h | 11 ++++
> 2 files changed, 93 insertions(+), 41 deletions(-)
>
> diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c
> index 6821f225248b1..0166367a4df37 100644
> --- a/drivers/hv/channel.c
> +++ b/drivers/hv/channel.c
> @@ -426,7 +426,13 @@ static void vmbus_free_channel_msginfo(struct vmbus_channel_msginfo *msginfo)
> }
>
> /*
> - * __vmbus_establish_gpadl - Establish a GPADL for a buffer or ringbuffer
> + * __vmbus_establish_gpadl - Establish a GPADL for a buffer or ringbuffer.
> + *
> + * This function only handles the GPADL handshake with the host. The caller
> + * is responsible for ensuring that @kbuffer is in the encryption state the
> + * host expects (host-visible / "decrypted" for confidential VMs); this
> + * function never calls set_memory_decrypted() or set_memory_encrypted().
> + * It also leaves gpadl->decrypted untouched, so the caller must set it.
> *
> * @channel: a channel
> * @type: the type of the corresponding GPADL, only meaningful for the guest.
> @@ -434,7 +440,7 @@ static void vmbus_free_channel_msginfo(struct vmbus_channel_msginfo *msginfo)
> * @size: page-size multiple
> * @send_offset: the offset (in bytes) where the send ring buffer starts,
> * should be 0 for BUFFER type gpadl
> - * @gpadl_handle: some funky thing
> + * @gpadl: out parameter receiving the established GPADL handle/buffer/size
> */
> static int __vmbus_establish_gpadl(struct vmbus_channel *channel,
> enum hv_gpadl_type type, void *kbuffer,
> @@ -454,30 +460,8 @@ static int __vmbus_establish_gpadl(struct vmbus_channel *channel,
> (atomic_inc_return(&vmbus_connection.next_gpadl_handle) - 1);
>
> ret = create_gpadl_header(type, kbuffer, size, send_offset, &msginfo);
> - if (ret) {
> - gpadl->decrypted = false;
> + if (ret)
> return ret;
> - }
> -
> - gpadl->decrypted = !((channel->co_external_memory && type == HV_GPADL_BUFFER) ||
> - (channel->co_ring_buffer && type == HV_GPADL_RING));
> - if (gpadl->decrypted) {
> - /*
> - * The "decrypted" flag being true assumes that set_memory_decrypted() succeeds.
> - * But if it fails, the encryption state of the memory is unknown. In that case,
> - * leave "decrypted" as true to ensure the memory is leaked instead of going back
> - * on the free list.
> - */
> - ret = set_memory_decrypted((unsigned long)kbuffer,
> - PFN_UP(size));
> - if (ret) {
> - dev_warn(&channel->device_obj->device,
> - "Failed to set host visibility for new GPADL %d.\n",
> - ret);
> - vmbus_free_channel_msginfo(msginfo);
> - return ret;
> - }
> - }
>
> init_completion(&msginfo->waitevent);
> msginfo->waiting_channel = channel;
> @@ -553,19 +537,47 @@ static int __vmbus_establish_gpadl(struct vmbus_channel *channel,
> spin_unlock_irqrestore(&vmbus_connection.channelmsg_lock, flags);
>
> vmbus_free_channel_msginfo(msginfo);
> + return ret;
> +}
>
> - if (ret) {
> - /*
> - * If set_memory_encrypted() fails, the decrypted flag is
> - * left as true so the memory is leaked instead of being
> - * put back on the free list.
> - */
> - if (gpadl->decrypted) {
> - if (!set_memory_encrypted((unsigned long)kbuffer, PFN_UP(size)))
> - gpadl->decrypted = false;
> +/*
> + * vmbus_establish_gpadl_and_decrypt - Manage the encryption lifecycle of
> + * @kbuffer around a __vmbus_establish_gpadl() call.
> + *
> + * Decrypts @kbuffer (making it host-visible) unless the channel was created
> + * with confidential memory for @type, then establishes the GPADL. On
> + * establish failure the buffer is re-encrypted so the caller can free it.
> + *
> + * Sets gpadl->decrypted; see the comment on struct vmbus_gpadl::decrypted.
> + */
> +static int vmbus_establish_gpadl_and_decrypt(struct vmbus_channel *channel,
> + enum hv_gpadl_type type,
> + void *kbuffer, u32 size,
> + u32 send_offset,
> + struct vmbus_gpadl *gpadl)
> +{
> + bool decrypt = !((channel->co_external_memory && type == HV_GPADL_BUFFER) ||
> + (channel->co_ring_buffer && type == HV_GPADL_RING));
> + int ret;
> +
> + if (decrypt) {
> + ret = set_memory_decrypted((unsigned long)kbuffer, PFN_UP(size));
> + if (ret) {
> + dev_warn(&channel->device_obj->device,
> + "Failed to set host visibility for new GPADL %d.\n",
> + ret);
> + /* Encryption state unknown; signal caller to leak. */
> + gpadl->decrypted = true;
> + return ret;
> }
> }
>
> + gpadl->decrypted = decrypt;
> + ret = __vmbus_establish_gpadl(channel, type, kbuffer, size, send_offset,
> + gpadl);
> + if (ret && decrypt &&
> + !set_memory_encrypted((unsigned long)kbuffer, PFN_UP(size)))
> + gpadl->decrypted = false;
> return ret;
> }
>
> @@ -580,11 +592,40 @@ static int __vmbus_establish_gpadl(struct vmbus_channel *channel,
> int vmbus_establish_gpadl(struct vmbus_channel *channel, void *kbuffer,
> u32 size, struct vmbus_gpadl *gpadl)
> {
> - return __vmbus_establish_gpadl(channel, HV_GPADL_BUFFER, kbuffer, size,
> - 0U, gpadl);
> + return vmbus_establish_gpadl_and_decrypt(channel, HV_GPADL_BUFFER,
> + kbuffer, size, 0U, gpadl);
> }
> EXPORT_SYMBOL_GPL(vmbus_establish_gpadl);
>
> +/*
> + * vmbus_establish_gpadl_caller_decrypted - Establish a GPADL for a buffer
> + * whose encryption state is managed by the caller.
> + *
> + * @channel: a channel
> + * @kbuffer: a buffer that the caller has already transitioned to host-visible
> + * (decrypted) via set_memory_decrypted() or an equivalent mechanism.
> + * @size: page-size multiple
> + * @gpadl: out parameter receiving the established GPADL
> + *
> + * Unlike vmbus_establish_gpadl(), this function does not call
> + * set_memory_decrypted() on @kbuffer, and the matching vmbus_teardown_gpadl()
> + * call will not call set_memory_encrypted() on it. The caller is responsible
> + * for the full encryption lifecycle of @kbuffer; on return gpadl->decrypted
> + * is set to false to record that vmbus does not own the encryption state.
> + */
> +int vmbus_establish_gpadl_caller_decrypted(struct vmbus_channel *channel,
> + void *kbuffer, u32 size,
> + struct vmbus_gpadl *gpadl)
> +{
> + int ret = __vmbus_establish_gpadl(channel, HV_GPADL_BUFFER, kbuffer,
> + size, 0U, gpadl);
> +
> + /* Caller owns @kbuffer's encryption; teardown must not touch it. */
> + gpadl->decrypted = false;
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(vmbus_establish_gpadl_caller_decrypted);
> +
> /**
> * request_arr_init - Allocates memory for the requestor array. Each slot
> * keeps track of the next available slot in the array. Initially, each
> @@ -685,11 +726,11 @@ static int __vmbus_open(struct vmbus_channel *newchannel,
> /* Establish the gpadl for the ring buffer */
> newchannel->ringbuffer_gpadlhandle.gpadl_handle = 0;
>
> - err = __vmbus_establish_gpadl(newchannel, HV_GPADL_RING,
> - page_address(newchannel->ringbuffer_page),
> - (send_pages + recv_pages) << PAGE_SHIFT,
> - newchannel->ringbuffer_send_offset << PAGE_SHIFT,
> - &newchannel->ringbuffer_gpadlhandle);
> + err = vmbus_establish_gpadl_and_decrypt(newchannel, HV_GPADL_RING,
> + page_address(newchannel->ringbuffer_page),
> + (send_pages + recv_pages) << PAGE_SHIFT,
> + newchannel->ringbuffer_send_offset << PAGE_SHIFT,
> + &newchannel->ringbuffer_gpadlhandle);
> if (err)
> goto error_clean_ring;
>
> diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
> index 964f1be8150c5..9c6b1b7794b48 100644
> --- a/include/linux/hyperv.h
> +++ b/include/linux/hyperv.h
> @@ -781,6 +781,12 @@ struct vmbus_gpadl {
> u32 gpadl_handle;
> u32 size;
> void *buffer;
> + /*
> + * Only used if the vmbus layer owns the encryption lifecycle of
s/VMBus/vmbus/
> + * @buffer.
> + * Indicates @buffer must be re-encrypted at teardown or that it
> + * must be leaked because re-encryption failed.
Overall, this comment would be a bit different with my suggested approach.
> + */
> bool decrypted;
> };
>
> @@ -1205,6 +1211,11 @@ extern int vmbus_establish_gpadl(struct vmbus_channel *channel,
> u32 size,
> struct vmbus_gpadl *gpadl);
>
> +extern int vmbus_establish_gpadl_caller_decrypted(struct vmbus_channel *channel,
> + void *kbuffer,
> + u32 size,
> + struct vmbus_gpadl *gpadl);
> +
> extern int vmbus_teardown_gpadl(struct vmbus_channel *channel,
> struct vmbus_gpadl *gpadl);
>
> --
> 2.45.4
>
^ permalink raw reply
* Re: [PATCH v11 2/8] mm/hmm: add hmm_range_fault_unlocked_timeout() for mmap lock-drop support
From: sashiko-bot @ 2026-07-23 17:50 UTC (permalink / raw)
To: Stanislav Kinsburskii; +Cc: dri-devel, linux-hyperv
In-Reply-To: <20260723-hmm-v10-v11-2-c55b003a4b61@gmail.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] Unconditional dereference of hmm_vma_walk->locked in hmm_do_fault() on VM_FAULT_RETRY causes a kernel panic.
- [High] hmm_range_fault_unlocked_timeout() causes an infinite 100% CPU busy loop on faults that drop the mmap lock (like disk I/O) by unconditionally passing FAULT_FLAG_ALLOW_RETRY without tracking retries, and continuously resetting the timeout.
--
--- Patch [2]: [PATCH v11 2/8] mm/hmm: add hmm_range_fault_unlocked_timeout() for mmap lock-drop support ---
commit 05dcbcb5ba6adcdc92b16e648fa82986518acbaa
Author: Stanislav Kinsburskii <skinsburskii@gmail.com>
mm/hmm: add hmm_range_fault_unlocked_timeout() for mmap lock-drop support
This commit introduces hmm_range_fault_unlocked_timeout() to allow HMM
fault callers to handle mapping faults that drop the mmap_read_lock, such
as userfaultfd regions. It restructures the internal fault loop to track
the lock state and gracefully retry when the lock is dropped.
> --- a/mm/hmm.c
> +++ b/mm/hmm.c
[ ... ]
> @@ -641,6 +651,9 @@ static int hmm_do_fault(struct mm_struct *mm,
> unsigned int fault_flags = FAULT_FLAG_REMOTE;
> struct vm_area_struct *vma;
>
> if (hmm_vma_walk->locked)
> fault_flags |= FAULT_FLAG_ALLOW_RETRY | FAULT_FLAG_KILLABLE;
[Severity: High]
Does this missing FAULT_FLAG_TRIED tracking cause a livelock on file-backed
faults?
When filemap_fault() sees FAULT_FLAG_ALLOW_RETRY without FAULT_FLAG_TRIED,
it initiates background I/O, drops the lock, and returns VM_FAULT_RETRY
immediately. Because FAULT_FLAG_TRIED is never set on subsequent retries,
this will continually drop the lock and retry until the I/O completes.
[ ... ]
> for (; addr < end; addr += PAGE_SIZE) {
> vm_fault_t ret;
>
> ret = handle_mm_fault(vma, addr, fault_flags, NULL);
>
> if (ret & (VM_FAULT_COMPLETED | VM_FAULT_RETRY)) {
> *hmm_vma_walk->locked = false;
[Severity: Critical]
If hmm_range_fault() is called, locked is NULL. Can this dereference a NULL
pointer if a fault handler returns VM_FAULT_RETRY?
Even without FAULT_FLAG_ALLOW_RETRY, some fault handlers (like
exfat_page_mkwrite or BPF arena_vm_fault) return VM_FAULT_RETRY
unconditionally when internal locks fail. If this happens during a legacy
hmm_range_fault() call, *hmm_vma_walk->locked will cause a panic.
[ ... ]
> static int hmm_range_fault_locked(struct hmm_range *range, bool *locked)
> {
> struct hmm_vma_walk hmm_vma_walk = {
> .range = range,
> .locked = locked,
> .last = range->start,
> };
[Severity: High]
When a lock is dropped, the outer function reacquires it and calls
hmm_range_fault_locked() again. Because hmm_vma_walk.last is re-initialized
to range->start on every call, forward progress is lost and the walk
restarts from the beginning of the range.
[ ... ]
> +int hmm_range_fault_unlocked_timeout(struct hmm_range *range,
> + unsigned long timeout)
> +{
[ ... ]
> + do {
> + /*
> + * If the previous fault dropped mmap_lock, then the fault
> + * handler made progress. Restart the retry timeout in that
> + * case, but keep the existing deadline for ordinary -EBUSY
> + * retries.
> + */
> + if (timeout && !locked)
> + deadline = jiffies + timeout;
[Severity: High]
Coupled with the missing FAULT_FLAG_TRIED logic above, does this continually
reset the timeout?
Since filemap_fault() drops the lock and returns VM_FAULT_RETRY instantly
while waiting for I/O, locked will be false on every iteration. The timeout
deadline will be pushed forward infinitely, creating a 100% CPU busy loop
that entirely bypasses the caller's timeout policy until the I/O finishes.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260723-hmm-v10-v11-0-c55b003a4b61@gmail.com?part=2
^ permalink raw reply
* [PATCH v11 8/8] drm/gpusvm: Use hmm_range_fault_unlocked_timeout() for range faults
From: Stanislav Kinsburskii @ 2026-07-23 17:36 UTC (permalink / raw)
To: Jason Gunthorpe, Leon Romanovsky, Andrew Morton,
David Hildenbrand, Lorenzo Stoakes, Liam R. Howlett,
Vlastimil Babka, Mike Rapoport, Suren Baghdasaryan, Michal Hocko,
Jonathan Corbet, Shuah Khan, Shuah Khan, K. Y. Srinivasan,
Haiyang Zhang, Wei Liu, Dexuan Cui, Long Li, Lyude Paul,
Danilo Krummrich, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Min Ma, Lizhi Hou,
Oded Gabbay, skinsburskii
Cc: linux-mm, linux-kernel, linux-doc, linux-kselftest, linux-hyperv,
dri-devel, nouveau, linux-rdma, Jason Gunthorpe
In-Reply-To: <20260723-hmm-v10-v11-0-c55b003a4b61@gmail.com>
Several GPU SVM paths take mmap_read_lock() only to call hmm_range_fault()
and open-code mmu interval sequence setup before each HMM walk. They also
retry -EBUSY until HMM_RANGE_DEFAULT_TIMEOUT expires.
Use hmm_range_fault_unlocked_timeout() for those faults. The HMM helper now
owns mmap_lock acquisition and refreshes range->notifier_seq for its
internal retries, while GPU SVM keeps its existing driver-lock validation
with mmu_interval_read_retry() after a successful fault.
drm_gpusvm_scan_mm() and drm_gpusvm_range_evict() pass
HMM_RANGE_DEFAULT_TIMEOUT as the helper retry budget for each HMM fault
attempt. drm_gpusvm_get_pages() keeps its existing absolute outer deadline
because it can be reached from GPU page-fault workers, where fatal signals
from the faulting process cannot stop an endless invalidation retry loop.
It passes the remaining time from that deadline to HMM for each fault
attempt.
Leave drm_gpusvm_check_pages() on hmm_range_fault() because that path is
called with the mmap lock already held by its caller.
Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
Signed-off-by: Stanislav Kinsburskii <skinsburskii@gmail.com>
---
drivers/gpu/drm/drm_gpusvm.c | 60 ++++++++------------------------------------
1 file changed, 10 insertions(+), 50 deletions(-)
diff --git a/drivers/gpu/drm/drm_gpusvm.c b/drivers/gpu/drm/drm_gpusvm.c
index 958cb605aedd..e912e994beb2 100644
--- a/drivers/gpu/drm/drm_gpusvm.c
+++ b/drivers/gpu/drm/drm_gpusvm.c
@@ -773,8 +773,7 @@ enum drm_gpusvm_scan_result drm_gpusvm_scan_mm(struct drm_gpusvm_range *range,
.end = end,
.dev_private_owner = dev_private_owner,
};
- unsigned long timeout =
- jiffies + msecs_to_jiffies(HMM_RANGE_DEFAULT_TIMEOUT);
+ unsigned long timeout = msecs_to_jiffies(HMM_RANGE_DEFAULT_TIMEOUT);
enum drm_gpusvm_scan_result state = DRM_GPUSVM_SCAN_UNPOPULATED, new_state;
unsigned long *pfns;
unsigned long npages = npages_in_range(start, end);
@@ -788,22 +787,7 @@ enum drm_gpusvm_scan_result drm_gpusvm_scan_mm(struct drm_gpusvm_range *range,
hmm_range.hmm_pfns = pfns;
retry:
- hmm_range.notifier_seq = mmu_interval_read_begin(notifier);
- mmap_read_lock(range->gpusvm->mm);
-
- while (true) {
- err = hmm_range_fault(&hmm_range);
- if (err == -EBUSY) {
- if (time_after(jiffies, timeout))
- break;
-
- hmm_range.notifier_seq =
- mmu_interval_read_begin(notifier);
- continue;
- }
- break;
- }
- mmap_read_unlock(range->gpusvm->mm);
+ err = hmm_range_fault_unlocked_timeout(&hmm_range, timeout);
if (err)
goto err_free;
@@ -1408,6 +1392,7 @@ int drm_gpusvm_get_pages(struct drm_gpusvm *gpusvm,
void *zdd;
unsigned long timeout =
jiffies + msecs_to_jiffies(HMM_RANGE_DEFAULT_TIMEOUT);
+ unsigned long remaining;
unsigned long i, j;
unsigned long npages = npages_in_range(pages_start, pages_end);
unsigned long num_dma_mapped;
@@ -1422,7 +1407,9 @@ int drm_gpusvm_get_pages(struct drm_gpusvm *gpusvm,
struct dma_iova_state *state = &svm_pages->state;
retry:
- if (time_after(jiffies, timeout))
+ remaining = timeout - jiffies;
+
+ if (time_after_eq(jiffies, timeout))
return -EBUSY;
hmm_range.notifier_seq = mmu_interval_read_begin(notifier);
@@ -1439,21 +1426,7 @@ int drm_gpusvm_get_pages(struct drm_gpusvm *gpusvm,
}
hmm_range.hmm_pfns = pfns;
- while (true) {
- mmap_read_lock(mm);
- err = hmm_range_fault(&hmm_range);
- mmap_read_unlock(mm);
-
- if (err == -EBUSY) {
- if (time_after(jiffies, timeout))
- break;
-
- hmm_range.notifier_seq =
- mmu_interval_read_begin(notifier);
- continue;
- }
- break;
- }
+ err = hmm_range_fault_unlocked_timeout(&hmm_range, remaining);
mmput(mm);
if (err)
goto err_free;
@@ -1720,8 +1693,7 @@ int drm_gpusvm_range_evict(struct drm_gpusvm *gpusvm,
.end = drm_gpusvm_range_end(range),
.dev_private_owner = NULL,
};
- unsigned long timeout =
- jiffies + msecs_to_jiffies(HMM_RANGE_DEFAULT_TIMEOUT);
+ unsigned long timeout = msecs_to_jiffies(HMM_RANGE_DEFAULT_TIMEOUT);
unsigned long *pfns;
unsigned long npages = npages_in_range(drm_gpusvm_range_start(range),
drm_gpusvm_range_end(range));
@@ -1736,24 +1708,12 @@ int drm_gpusvm_range_evict(struct drm_gpusvm *gpusvm,
return -ENOMEM;
hmm_range.hmm_pfns = pfns;
- while (!time_after(jiffies, timeout)) {
- hmm_range.notifier_seq = mmu_interval_read_begin(notifier);
- if (time_after(jiffies, timeout)) {
- err = -ETIME;
- break;
- }
-
- mmap_read_lock(mm);
- err = hmm_range_fault(&hmm_range);
- mmap_read_unlock(mm);
- if (err != -EBUSY)
- break;
- }
+ err = hmm_range_fault_unlocked_timeout(&hmm_range, timeout);
kvfree(pfns);
mmput(mm);
- return err;
+ return err == -EBUSY ? -ETIME : err;
}
EXPORT_SYMBOL_GPL(drm_gpusvm_range_evict);
--
2.43.0
^ permalink raw reply related
* [PATCH v11 7/8] accel/amdxdna: Use hmm_range_fault_unlocked_timeout() for range population
From: Stanislav Kinsburskii @ 2026-07-23 17:36 UTC (permalink / raw)
To: Jason Gunthorpe, Leon Romanovsky, Andrew Morton,
David Hildenbrand, Lorenzo Stoakes, Liam R. Howlett,
Vlastimil Babka, Mike Rapoport, Suren Baghdasaryan, Michal Hocko,
Jonathan Corbet, Shuah Khan, Shuah Khan, K. Y. Srinivasan,
Haiyang Zhang, Wei Liu, Dexuan Cui, Long Li, Lyude Paul,
Danilo Krummrich, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Min Ma, Lizhi Hou,
Oded Gabbay, skinsburskii
Cc: linux-mm, linux-kernel, linux-doc, linux-kselftest, linux-hyperv,
dri-devel, nouveau, linux-rdma, Jason Gunthorpe
In-Reply-To: <20260723-hmm-v10-v11-0-c55b003a4b61@gmail.com>
aie2_populate_range() takes mmap_read_lock() only around hmm_range_fault().
It also open-codes the mmu interval sequence setup before each HMM walk and
retries -EBUSY until HMM_RANGE_DEFAULT_TIMEOUT expires.
Use hmm_range_fault_unlocked_timeout() instead. The HMM helper now owns
the mmap lock and refreshes mapp->range.notifier_seq for its internal
retries, so the driver only needs to call the helper and then validate the
sequence before marking the mapping populated.
Pass HMM_RANGE_DEFAULT_TIMEOUT as the helper retry budget for each HMM
population attempt. This scopes the timeout to repeated HMM notifier
retries while preserving the existing outer loop that moves between invalid
mappings and restarts when the interval is invalidated before the driver
updates its mapping state.
Keep returning -ETIME when the HMM retry budget expires, matching the
driver's existing timeout error convention.
Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
Signed-off-by: Stanislav Kinsburskii <skinsburskii@gmail.com>
---
drivers/accel/amdxdna/aie2_ctx.c | 23 ++++-------------------
1 file changed, 4 insertions(+), 19 deletions(-)
diff --git a/drivers/accel/amdxdna/aie2_ctx.c b/drivers/accel/amdxdna/aie2_ctx.c
index 54486960cbf5..b5b4ca263002 100644
--- a/drivers/accel/amdxdna/aie2_ctx.c
+++ b/drivers/accel/amdxdna/aie2_ctx.c
@@ -1034,7 +1034,7 @@ static int aie2_populate_range(struct amdxdna_gem_obj *abo)
bool found;
int ret;
- timeout = jiffies + msecs_to_jiffies(HMM_RANGE_DEFAULT_TIMEOUT);
+ timeout = msecs_to_jiffies(HMM_RANGE_DEFAULT_TIMEOUT);
again:
found = false;
down_write(&xdna->notifier_lock);
@@ -1061,24 +1061,9 @@ static int aie2_populate_range(struct amdxdna_gem_obj *abo)
return -EFAULT;
}
- mapp->range.notifier_seq = mmu_interval_read_begin(&mapp->notifier);
- mmap_read_lock(mm);
- ret = hmm_range_fault(&mapp->range);
- mmap_read_unlock(mm);
- if (ret) {
- if (time_after(jiffies, timeout)) {
- ret = -ETIME;
- goto put_mm;
- }
-
- if (ret == -EBUSY) {
- amdxdna_umap_put(mapp);
- mmput(mm);
- goto again;
- }
-
+ ret = hmm_range_fault_unlocked_timeout(&mapp->range, timeout);
+ if (ret)
goto put_mm;
- }
down_write(&xdna->notifier_lock);
if (mmu_interval_read_retry(&mapp->notifier, mapp->range.notifier_seq)) {
@@ -1096,7 +1081,7 @@ static int aie2_populate_range(struct amdxdna_gem_obj *abo)
put_mm:
amdxdna_umap_put(mapp);
mmput(mm);
- return ret;
+ return ret == -EBUSY ? -ETIME : ret;
}
int aie2_cmd_submit(struct amdxdna_hwctx *hwctx, struct amdxdna_sched_job *job, u64 *seq)
--
2.43.0
^ permalink raw reply related
* [PATCH v11 6/8] RDMA/umem: Use hmm_range_fault_unlocked_timeout() for ODP faults
From: Stanislav Kinsburskii @ 2026-07-23 17:36 UTC (permalink / raw)
To: Jason Gunthorpe, Leon Romanovsky, Andrew Morton,
David Hildenbrand, Lorenzo Stoakes, Liam R. Howlett,
Vlastimil Babka, Mike Rapoport, Suren Baghdasaryan, Michal Hocko,
Jonathan Corbet, Shuah Khan, Shuah Khan, K. Y. Srinivasan,
Haiyang Zhang, Wei Liu, Dexuan Cui, Long Li, Lyude Paul,
Danilo Krummrich, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Min Ma, Lizhi Hou,
Oded Gabbay, skinsburskii
Cc: linux-mm, linux-kernel, linux-doc, linux-kselftest, linux-hyperv,
dri-devel, nouveau, linux-rdma, Jason Gunthorpe
In-Reply-To: <20260723-hmm-v10-v11-0-c55b003a4b61@gmail.com>
ib_umem_odp_map_dma_and_lock() takes mmap_read_lock() only around
hmm_range_fault(), then retries -EBUSY until HMM_RANGE_DEFAULT_TIMEOUT
expires.
Use hmm_range_fault_unlocked_timeout() instead. The HMM helper now owns
the mmap lock and refreshes range->notifier_seq for its internal retries.
ODP keeps using HMM_RANGE_DEFAULT_TIMEOUT for each HMM fault attempt,
while interval invalidation retries continue to be handled by the existing
outer loop.
ODP still validates the interval notifier sequence while holding umem_mutex
before DMA mapping pages.
Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
Signed-off-by: Stanislav Kinsburskii <skinsburskii@gmail.com>
---
drivers/infiniband/core/umem_odp.c | 18 +++++-------------
1 file changed, 5 insertions(+), 13 deletions(-)
diff --git a/drivers/infiniband/core/umem_odp.c b/drivers/infiniband/core/umem_odp.c
index 404fa1cc3254..9cc21cd762d9 100644
--- a/drivers/infiniband/core/umem_odp.c
+++ b/drivers/infiniband/core/umem_odp.c
@@ -329,7 +329,7 @@ int ib_umem_odp_map_dma_and_lock(struct ib_umem_odp *umem_odp, u64 user_virt,
struct mm_struct *owning_mm = umem_odp->umem.owning_mm;
int pfn_index, dma_index, ret = 0, start_idx;
unsigned int page_shift, hmm_order, pfn_start_idx;
- unsigned long num_pfns, current_seq;
+ unsigned long num_pfns;
struct hmm_range range = {};
unsigned long timeout;
@@ -363,26 +363,18 @@ int ib_umem_odp_map_dma_and_lock(struct ib_umem_odp *umem_odp, u64 user_virt,
}
range.hmm_pfns = &(umem_odp->map.pfn_list[pfn_start_idx]);
- timeout = jiffies + msecs_to_jiffies(HMM_RANGE_DEFAULT_TIMEOUT);
+ timeout = msecs_to_jiffies(HMM_RANGE_DEFAULT_TIMEOUT);
retry:
- current_seq = range.notifier_seq =
- mmu_interval_read_begin(&umem_odp->notifier);
-
- mmap_read_lock(owning_mm);
- ret = hmm_range_fault(&range);
- mmap_read_unlock(owning_mm);
- if (unlikely(ret)) {
- if (ret == -EBUSY && !time_after(jiffies, timeout))
- goto retry;
+ ret = hmm_range_fault_unlocked_timeout(&range, timeout);
+ if (unlikely(ret))
goto out_put_mm;
- }
start_idx = (range.start - ib_umem_start(umem_odp)) >> page_shift;
dma_index = start_idx;
mutex_lock(&umem_odp->umem_mutex);
- if (mmu_interval_read_retry(&umem_odp->notifier, current_seq)) {
+ if (mmu_interval_read_retry(&umem_odp->notifier, range.notifier_seq)) {
mutex_unlock(&umem_odp->umem_mutex);
goto retry;
}
--
2.43.0
^ permalink raw reply related
* [PATCH v11 4/8] mshv: Use hmm_range_fault_unlocked_timeout() for region faults
From: Stanislav Kinsburskii @ 2026-07-23 17:36 UTC (permalink / raw)
To: Jason Gunthorpe, Leon Romanovsky, Andrew Morton,
David Hildenbrand, Lorenzo Stoakes, Liam R. Howlett,
Vlastimil Babka, Mike Rapoport, Suren Baghdasaryan, Michal Hocko,
Jonathan Corbet, Shuah Khan, Shuah Khan, K. Y. Srinivasan,
Haiyang Zhang, Wei Liu, Dexuan Cui, Long Li, Lyude Paul,
Danilo Krummrich, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Min Ma, Lizhi Hou,
Oded Gabbay, skinsburskii
Cc: linux-mm, linux-kernel, linux-doc, linux-kselftest, linux-hyperv,
dri-devel, nouveau, linux-rdma, Jason Gunthorpe
In-Reply-To: <20260723-hmm-v10-v11-0-c55b003a4b61@gmail.com>
MSHV currently faults movable memory regions by taking mmap_read_lock()
around hmm_range_fault(). That prevents the fault path from handling VMAs
whose fault handlers need to drop mmap_lock, such as userfaultfd-backed
mappings.
Use hmm_range_fault_unlocked_timeout() instead. Passing a timeout of 0
preserves MSHV's existing unbounded retry behavior while letting the HMM
helper own mmap_lock acquisition and refresh range->notifier_seq internally
before walking the range. After the fault succeeds, MSHV still takes
mreg_mutex and checks mmu_interval_read_retry() before installing the pages
into the region, so the existing invalidation synchronization is preserved.
Fold the small fault-and-lock helper into mshv_region_range_fault(), since
the remaining retry path is just the standard "fault, take the driver lock,
check the interval notifier sequence" pattern.
Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
Signed-off-by: Stanislav Kinsburskii <skinsburskii@gmail.com>
---
drivers/hv/mshv_regions.c | 54 +++++++++--------------------------------------
1 file changed, 10 insertions(+), 44 deletions(-)
diff --git a/drivers/hv/mshv_regions.c b/drivers/hv/mshv_regions.c
index 6d65e5b42152..dddaade31b5d 100644
--- a/drivers/hv/mshv_regions.c
+++ b/drivers/hv/mshv_regions.c
@@ -381,46 +381,6 @@ int mshv_region_get(struct mshv_mem_region *region)
return kref_get_unless_zero(®ion->mreg_refcount);
}
-/**
- * mshv_region_hmm_fault_and_lock - Handle HMM faults and lock the memory region
- * @region: Pointer to the memory region structure
- * @range: Pointer to the HMM range structure
- *
- * This function performs the following steps:
- * 1. Reads the notifier sequence for the HMM range.
- * 2. Acquires a read lock on the memory map.
- * 3. Handles HMM faults for the specified range.
- * 4. Releases the read lock on the memory map.
- * 5. If successful, locks the memory region mutex.
- * 6. Verifies if the notifier sequence has changed during the operation.
- * If it has, releases the mutex and returns -EBUSY to match with
- * hmm_range_fault() return code for repeating.
- *
- * Return: 0 on success, a negative error code otherwise.
- */
-static int mshv_region_hmm_fault_and_lock(struct mshv_mem_region *region,
- struct hmm_range *range)
-{
- int ret;
-
- range->notifier_seq = mmu_interval_read_begin(range->notifier);
- mmap_read_lock(region->mreg_mni.mm);
- ret = hmm_range_fault(range);
- mmap_read_unlock(region->mreg_mni.mm);
- if (ret)
- return ret;
-
- mutex_lock(®ion->mreg_mutex);
-
- if (mmu_interval_read_retry(range->notifier, range->notifier_seq)) {
- mutex_unlock(®ion->mreg_mutex);
- cond_resched();
- return -EBUSY;
- }
-
- return 0;
-}
-
/**
* mshv_region_range_fault - Handle memory range faults for a given region.
* @region: Pointer to the memory region structure.
@@ -452,13 +412,19 @@ static int mshv_region_range_fault(struct mshv_mem_region *region,
range.start = region->start_uaddr + page_offset * HV_HYP_PAGE_SIZE;
range.end = range.start + page_count * HV_HYP_PAGE_SIZE;
- do {
- ret = mshv_region_hmm_fault_and_lock(region, &range);
- } while (ret == -EBUSY);
-
+again:
+ ret = hmm_range_fault_unlocked_timeout(&range, 0);
if (ret)
goto out;
+ mutex_lock(®ion->mreg_mutex);
+
+ if (mmu_interval_read_retry(range.notifier, range.notifier_seq)) {
+ mutex_unlock(®ion->mreg_mutex);
+ cond_resched();
+ goto again;
+ }
+
for (i = 0; i < page_count; i++)
region->mreg_pages[page_offset + i] = hmm_pfn_to_page(pfns[i]);
--
2.43.0
^ permalink raw reply related
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