From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from fhigh-a5-smtp.messagingengine.com (fhigh-a5-smtp.messagingengine.com [103.168.172.156]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 1E2953D9667 for ; Wed, 5 Aug 2026 23:19:23 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=103.168.172.156 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1785971969; cv=none; b=XPSEbvJuFvboq4uiVIco/VHhtEN2QOw3qrNoim2HMJ883nyk1pJIn7KBhtwnfqYbigRfCf3jJc2NiP/cTkNpWAckem3xcU/AovPngt/Sg23SwYin9GAObEVJYtFOT04/+qts769HDb8UxxJzfl7FJaP0tiJghd6mUwZnfl0Fmyk= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1785971969; c=relaxed/simple; bh=GiBhTs6oH8MYsQM2J1iDPmyh9gij2atf7aZin+KqdPU=; h=Date:From:To:Cc:Subject:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=GBj6mii8wDHZuXPJIO9nr+4/mzZplcwy4Y/9/W4Onvpbinheqz5daQK2kjYDbMvdpFSihAp1w0lIZlFCvdd+QgCdgnwFsGxmTtDLZQ8L58YqgvExputL6bemzn0yKcN9ae1iALZ2lN1sgZmjtx9bh5iCFiuP16u+s3V9wtrTNkU= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=shazbot.org; spf=pass smtp.mailfrom=shazbot.org; dkim=pass (2048-bit key) header.d=shazbot.org header.i=@shazbot.org header.b=y8mJt6nb; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b=O7sCGzP/; arc=none smtp.client-ip=103.168.172.156 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=shazbot.org Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=shazbot.org Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=shazbot.org header.i=@shazbot.org header.b="y8mJt6nb"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="O7sCGzP/" Received: from phl-compute-02.internal (phl-compute-02.internal [10.202.2.42]) by mailfhigh.phl.internal (Postfix) with ESMTP id C14AA1400089; Wed, 5 Aug 2026 19:19:22 -0400 (EDT) Received: from phl-frontend-03 ([10.202.2.162]) by phl-compute-02.internal (MEProxy); Wed, 05 Aug 2026 19:19:22 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=shazbot.org; h= cc:cc:content-transfer-encoding:content-type:content-type:date :date:from:from:in-reply-to:in-reply-to:message-id:mime-version :references:reply-to:subject:subject:to:to; s=fm2; t=1785971961; x=1786058361; bh=H4MsU9uRTXA0rywdnBcydPi+YkHwaLB3CrEaysdJ634=; b= y8mJt6nb0uUnCixKunpjqjJtOM2dVyCl3ycTqtN76jk1DVjxVr/qE6YDMLDpRhEk ihLj/31Bwrra/7XQn5lf+LWsVqkMQjiGbvRRjnDuNrTWJ9B/SEEL6yYXhDIjGrd5 R7DtXG3/XpYceT0LGww0/v9G4L7U5VkxorXEA+bBLGBMxDLU3ESEW+g+yVXKqwF1 rwxWRJx0oXFImVMMd2S9XwOi2dVpWoUxb+WWp6qVMOy68OaaqbSTnFaeCzWgAuaD QvM90vahmYlgeOTcPzKFfhcQYTKu2W+nXxY2iBPImOdyC4kkri75htMFTe9gM6W/ UaE6KXpniF9J4xmHIPdutw== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-transfer-encoding :content-type:content-type:date:date:feedback-id:feedback-id :from:from:in-reply-to:in-reply-to:message-id:mime-version :references:reply-to:subject:subject:to:to:x-me-proxy :x-me-sender:x-me-sender:x-sasl-enc; s=fm3; t=1785971961; x= 1786058361; bh=H4MsU9uRTXA0rywdnBcydPi+YkHwaLB3CrEaysdJ634=; b=O 7sCGzP/7KRrmMsckddDPsuwcEYNaRJ8oKf48sD9PMHbzFkc0OX3UFcSrMHqAc3aQ svwLSNDHAFRnCMT6hP9qbGClTLuRfwnPnNQAJwowH9H3TbdlvRAYoZzrbQtes/O5 WocVyis1tql/fl8WqvcRoKiVMKdHxmgQ2wWgwlmEvPstv6+UTgZgHCWp/KpmPOUt OzpORExpjj8OkLBVtlww/MpWPU5yrFGe8bnApfHXVoeWMGYcQHMM4ySh/pr9zc4g KmHlGqQhR+hhTqspYpUVbrlOlCFpaOMadjsERK+ZznnG2M+QkTd7ndLx0h8wdaQn FRoLANsG1s+vnu1l/wc8w== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: dmFkZTEaQauUmjYtyLSMvDfzfctlxuzaIQuKAWjrMZ3+JypKXMPdPgy/+6evspw68XnDap TWBzGRTLYJDhrmUG7MLnAtKClN8cvEcV/BuPh7/b5vb3Zu5cuXBPICQQ64ldNAAzp9ZlKZ Cjcg8pRQJCrRkKzchsCq4wiugprX3NsRcDom3iDxDwskHTh+1OadHEPdhWg42sw5x81TsC R2CMzQEln1Sh8ZPnoyQxpubord2uqiEFB0CuCBMy0mT5iboZjFgOXH/V34D6CbznXmM0gW vFBSpQ/7RgdxPh+V3b6oL2NofKkuGU4QA2zLCDzPVjifrfWKYnXm7jmmO1zpIFJUZap5Id Rbw4ti6wx5ErJyMhHm1GBe4+h2TdJOjADtwgGakWrpTwBJJ+wr/TdPzjqcUYUynjb7RJdD /WIf5xHI26vB8braw+u2z2ZYpsxmatx+L2DEIAaLGl20/rLNTPd3Zo60vbOQJaiBjYJD8F W7S1eLcqxwIiiLghHwAstMX5GryifgTavsEZPuNH90gr0sRUsHA5RfPgdU3aIOh2Ljt3qY OeLSBbqPCFDVDat3/XFY8Gx1dFfHe2GKg3GTnM2t6uegFGmlgJynodEAhuNH2Dl/t8fkwA RH4b8Oshs2KN8a5zD1vur8OuOprh5RJHiq93MhQukv3ClVJ6PxR34oNnWKbg X-ME-Proxy: Feedback-ID: i03f14258:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Wed, 5 Aug 2026 19:19:19 -0400 (EDT) Date: Wed, 5 Aug 2026 17:18:49 -0600 From: Alex Williamson To: Ankit Agrawal Cc: , , , , , , , , , , alex@shazbot.org Subject: Re: [PATCH 2/4] platform/nvidia: Implement mmap and memory scrubbing for EGM chardev Message-ID: <20260805171849.0f00d33e@shazbot.org> In-Reply-To: <20260702192532.455400-3-ankita@nvidia.com> References: <20260702192532.455400-1-ankita@nvidia.com> <20260702192532.455400-3-ankita@nvidia.com> X-Mailer: Claws Mail 4.4.0 (GTK 3.24.52; x86_64-pc-linux-gnu) Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit On Thu, 2 Jul 2026 19:25:30 +0000 Ankit Agrawal wrote: > Setup the file operations (open, close and mmap) for the EGM char device. > > Memory scrubbing: > - The EGM region is invisible to the host Linux kernel and is not > managed by the page allocator. So the driver is responsible for > zeroing it before handing it to a VM. > - Clear the entire EGM region on the first open() call using memremap() > and memset() in 1 GiB chunks with cond_resched() between iterations > to avoid RCU stalls on large regions. > - The region is handed to a single VM at a time and must be scrubbed > before each handover. Serialise openers with a mutex guarding open_count > and refuse a second opener with -EBUSY while the region is still held. > > mmap: > - Implement nvgrace_egm_mmap() using remap_pfn_range(). The EGM region > is not part of the host EFI map and has no struct pages making > remap_pfn_range() the correct interface for mapping it directly into > QEMU's virtual address space. > - Validate that the requested range fits within the EGM region. Reject a > vm_pgoff that already lies outside the region before it is shifted into > a byte count so that the page offset cannot overflow and slip past the > range check. > > Suggested-by: Aniket Agashe > Suggested-by: Vikram Sethi > Assisted-by: Claude:claude-opus-4.8 > Signed-off-by: Ankit Agrawal > --- > drivers/platform/nvidia/egm.c | 125 ++++++++++++++++++++++++++++++++-- > 1 file changed, 121 insertions(+), 4 deletions(-) > > diff --git a/drivers/platform/nvidia/egm.c b/drivers/platform/nvidia/egm.c > index a340c4fcbc7a..09ab00213de2 100644 > --- a/drivers/platform/nvidia/egm.c > +++ b/drivers/platform/nvidia/egm.c > @@ -3,6 +3,9 @@ > * Copyright (c) 2026, NVIDIA CORPORATION & AFFILIATES. All rights reserved > */ > > +#include > +#include > +#include > #include > > #define NVGRACE_EGM_DEV_NAME "egm" > @@ -19,6 +22,10 @@ struct gpu_node { > struct nvgrace_egm_dev { > struct device device; > struct cdev cdev; > + /* serialises the first-open scrub */ > + struct mutex open_lock; > + /* protected by open_lock */ > + unsigned int open_count; > phys_addr_t egmphys; > size_t egmlength; > u64 egmpxm; > @@ -39,21 +46,129 @@ static LIST_HEAD(egm_chardevs); > > static int nvgrace_egm_open(struct inode *inode, struct file *file) > { > - return 0; > + struct nvgrace_egm_dev *egm_dev = > + container_of(inode->i_cdev, struct nvgrace_egm_dev, cdev); > + phys_addr_t phys; > + size_t remaining, chunk_size; > + void *chunk_addr; > + int ret; Nit, reverse christmas tree ordering. > + > + file->private_data = egm_dev; > + > + /* > + * The EGM region is a physical carveout handed to a single VM at a > + * time and must be scrubbed before each handover. Take open_lock > + * killably around the scrub which can run for long time, so that > + * the waiters stay killable. > + */ > + if (mutex_lock_killable(&egm_dev->open_lock)) > + return -EINTR; > + > + /* > + * Refuse a second opener while the region is still held. release() > + * runs only when the last reference to the struct file drops but an > + * mmap keeps that reference (and hence open_count) alive past > + * close for the lifetime of the VMA. So open_count returns to zero > + * only when every mapping is gone. A concurrent opener cannot be > + * handed the region: it cannot be re-scrubbed without destroying the > + * current consumer's live data. > + */ > + if (egm_dev->open_count) { > + ret = -EBUSY; > + goto unlock; > + } Do we need to hold the lock for the extent of the scrub? Currently a 2nd open is blocked, only to fail once the scrub completes successfully. Can't we increment open_count here, let the 2nd open fail fast, scrub w/o lock, re-acquire lock and decrement open_count if scrub fails. Ideally though, we probably want to kick off a scrub thread on init/close, where open does a killable wait for completion (completion signaled by the workqueue thread), and exit does a cancel_work_sync() to force the workqueue flush. Scrubbing is then asynchronous and open can be instant if the background thread has already completed. Just watch out for the ordering of exposing the chardev vs setting the completion state. (Note that I'm including scrub on exit, not just open, it seems like a coverage gap if the memory isn't also scrubbed before being released to another driver, just like vfio-pci resets devices on close.) > + > + /* > + * nvgrace-egm module is responsible to manage the EGM memory as > + * the host kernel has no knowledge of it. Clear the region before > + * handing over to userspace. > + * > + * The EGM region can be very large (hundreds of GiB). So Map and zero > + * one chunk at a time rather than mapping the whole region at once. > + */ > + phys = egm_dev->egmphys; > + remaining = egm_dev->egmlength; > + > + while (remaining > 0) { > + /* > + * The scrub holds the lock for a long time; let it be > + * SIGKILL'd. > + */ > + if (fatal_signal_pending(current)) { > + ret = -EINTR; > + goto unlock; > + } > + > + chunk_size = min(remaining, SZ_1G); > + > + chunk_addr = memremap(phys, chunk_size, MEMREMAP_WB); > + if (!chunk_addr) { > + ret = -ENOMEM; > + goto unlock; > + } > + > + memset(chunk_addr, 0, chunk_size); > + memunmap(chunk_addr); > + cond_resched(); > + > + phys += chunk_size; > + remaining -= chunk_size; > + } > + > + /* > + * Mark the device open only after the scrub completes so that a > + * concurrent opener cannot observe a non-zero count and proceed > + * before the region has been cleared. > + */ > + egm_dev->open_count = 1; > + ret = 0; > + > +unlock: > + mutex_unlock(&egm_dev->open_lock); With the above, we don't need the killable mutex acquire and this can just be a scoped guard mutex for open_count. > + return ret; > } > > static int nvgrace_egm_release(struct inode *inode, struct file *file) > { > + struct nvgrace_egm_dev *egm_dev = > + container_of(inode->i_cdev, struct nvgrace_egm_dev, cdev); > + > + guard(mutex)(&egm_dev->open_lock); > + > + if (!--egm_dev->open_count) > + file->private_data = NULL; open_count is only ever 0/1, there's no increment in this thread, so the decrement and test is out of place here. > + > return 0; > } > > static int nvgrace_egm_mmap(struct file *file, struct vm_area_struct *vma) > { > + struct nvgrace_egm_dev *egm_dev = file->private_data; > + u64 req_len, pgoff, end; > + unsigned long start_pfn, num_pages; > + > + pgoff = vma->vm_pgoff; > + num_pages = egm_dev->egmlength >> PAGE_SHIFT; > + > + /* Reject a page offset that already lies outside the EGM region. */ > + if (pgoff >= num_pages) > + return -EINVAL; > + > + if (check_sub_overflow(vma->vm_end, vma->vm_start, &req_len) || > + check_add_overflow(PHYS_PFN(egm_dev->egmphys), pgoff, &start_pfn) || > + check_add_overflow(PFN_PHYS(pgoff), req_len, &end)) > + return -EOVERFLOW; > + > + if (end > egm_dev->egmlength) > + return -EINVAL; Shouldn't we validate MAP_SHARED as well? Is this also a gap in current nvgrace-gpu? > + > /* > - * Mapping the EGM region into userspace is implemented by a later > - * patch. Until then refuse the mmap. > + * EGM memory is invisible to the host kernel and is not managed > + * by it. Map the usermode VMA to the EGM region. > */ > - return -EOPNOTSUPP; > + return remap_pfn_range(vma, vma->vm_start, > + start_pfn, req_len, > + vma->vm_page_prot); > } Is huge_fault an option for an iteration after this gets merged to make use of pud/pmd mappings given the size of this range? Thanks, Alex > > static const struct file_operations file_ops = { > @@ -122,6 +237,7 @@ static void egm_chardev_release(struct device *dev) > struct nvgrace_egm_dev *egm_chardev = container_of(dev, struct nvgrace_egm_dev, device); > > remove_gpus(egm_chardev); > + mutex_destroy(&egm_chardev->open_lock); > kfree(egm_chardev); > } > > @@ -155,6 +271,7 @@ static struct nvgrace_egm_dev *setup_egm_chardev(u64 egmphys, u64 egmlength, > egm_chardev->egmphys = egmphys; > egm_chardev->egmlength = egmlength; > egm_chardev->egmpxm = egmpxm; > + mutex_init(&egm_chardev->open_lock); > INIT_LIST_HEAD(&egm_chardev->gpus); > > egm_chardev->device.devt = MKDEV(MAJOR(dev), egm_chardev->egmpxm);