From: Alex Williamson <alex@shazbot.org>
To: Ankit Agrawal <ankita@nvidia.com>
Cc: <gregkh@linuxfoundation.org>, <jai.luthra@ideasonboard.com>,
<ilpo.jarvinen@linux.intel.com>, <W_Armin@gmx.de>,
<jgg@nvidia.com>, <vsethi@nvidia.com>, <mhonap@nvidia.com>,
<mochs@nvidia.com>, <clg@redhat.com>,
<linux-kernel@vger.kernel.org>,
alex@shazbot.org
Subject: Re: [PATCH 3/4] platform/nvidia: Handle retired ECC pages and expose via ioctl
Date: Wed, 5 Aug 2026 17:18:42 -0600 [thread overview]
Message-ID: <20260805171842.7dbcbf67@shazbot.org> (raw)
In-Reply-To: <20260702192532.455400-4-ankita@nvidia.com>
On Thu, 2 Jul 2026 19:25:31 +0000
Ankit Agrawal <ankita@nvidia.com> wrote:
> EGM regions may contain pages retired due to ECC/poison errors that
> are known to the system firmware beforehand. SBIOS records the list
> in a dedicated carveout region whose SPA is advertised via the ACPI
> DSD property "nvidia,egm-retired-pages-data-base". The carveout begins
> with a u64 count followed by that many u64 system physical addresses
> each the base SPA of a retired 64K page within the EGM region.
>
> The EGM region is linear, so the SPA - EGM_base calculation can be
> used to determine the offset within the EGM region. Moreover, the region
> is assigned linearly to the VM. So offset within EGM region on the host
> would be same as on the VM. Hence the retired pages offset is calculated
> and stored by the nvgrace-egm. An xarray is used to track the retired
> page offsets. These are exposed to the userspace (Qemu) which may use the
> information to skip the retired pages while forming the memory mapping
> for the VM.
>
> Note that a retired page SPA SBIOS entry is for 64k size, but the
> xarray list tracks offsets in PAGE_SIZE. So SZ_64K/PAGE_SIZE entries
> are added to the xarray to ensure comaptibility with 4k OS.
>
> Patch implements the following:
> Retired page tracking:
> - Map the carveout region with memremap() and validate the count.
> - convert each physical address to an in-EGM-region offset and
> populate a per-EGM-device hashtable (keyed by that offset).
>
> Userspace ioctl:
> - Introduce EGM_RETIRED_PAGES_LIST ioctl and add a new UAPI header
> Userspace (QEMU) calls this ioctl to retrieve the list of retired
> page offsets. QEMU then communicates the list to the VM so the
> guest can take appropriate action.
>
> Update MAINTAINERS to list include/uapi/linux/egm.h
>
> Assisted-by: Claude:claude-opus-4.8
> Signed-off-by: Ankit Agrawal <ankita@nvidia.com>
> ---
> .../userspace-api/ioctl/ioctl-number.rst | 1 +
> MAINTAINERS | 1 +
> drivers/platform/nvidia/egm.c | 211 +++++++++++++++++-
> include/uapi/linux/egm.h | 28 +++
> 4 files changed, 231 insertions(+), 10 deletions(-)
> create mode 100644 include/uapi/linux/egm.h
>
> diff --git a/Documentation/userspace-api/ioctl/ioctl-number.rst b/Documentation/userspace-api/ioctl/ioctl-number.rst
> index 3f0ef1e27eb0..94c9ca4a89ff 100644
> --- a/Documentation/userspace-api/ioctl/ioctl-number.rst
> +++ b/Documentation/userspace-api/ioctl/ioctl-number.rst
> @@ -306,6 +306,7 @@ Code Seq# Include File Comments
> 'v' 20-27 arch/powerpc/include/uapi/asm/vas-api.h VAS API
> 'v' C0-FF linux/meye.h conflict!
> 'w' all CERN SCI driver
> +'x' 00-1F include/uapi/linux/egm.h NVIDIA nvgrace-egm driver
> 'y' 00-1F packet based user level communications
> <mailto:zapman@interlan.net>
> 'z' 00-3F CAN bus card conflict!
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 2edf903a7746..e77c22a9e598 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -19348,6 +19348,7 @@ NVIDIA GRACE EGM PLATFORM DRIVER
> M: Ankit Agrawal <ankita@nvidia.com>
> S: Supported
> F: drivers/platform/nvidia/
> +F: include/uapi/linux/egm.h
>
> NVIDIA VRS RTC DRIVER
> M: Shubhi Garg <shgarg@nvidia.com>
> diff --git a/drivers/platform/nvidia/egm.c b/drivers/platform/nvidia/egm.c
> index 09ab00213de2..df963caedaa8 100644
> --- a/drivers/platform/nvidia/egm.c
> +++ b/drivers/platform/nvidia/egm.c
> @@ -7,10 +7,28 @@
> #include <linux/sched/signal.h>
> #include <linux/sizes.h>
> #include <linux/vfio_pci_core.h>
> +#include <linux/xarray.h>
> +#include <uapi/linux/egm.h>
>
> #define NVGRACE_EGM_DEV_NAME "egm"
> #define MAX_EGM_NODES 4
>
> +/*
> + * Presence marker stored in nvgrace_egm_dev.retired_pages. Only the
> + * page offset within the region carries information and the value
> + * just needs to be a non-NULL xa entry.
> + */
> +#define EGM_RETIRED_MARK xa_mk_value(1)
> +
> +/*
> + * The structure match the format of the retired pages information populated
> + * by the system firmware.
> + */
> +struct fw_egm_retired_pages {
> + u64 num_retired_pages;
> + __u64 retired_page_addr[4096];
> +};
> +
> static dev_t dev;
> static struct class *class;
>
> @@ -26,8 +44,14 @@ struct nvgrace_egm_dev {
> struct mutex open_lock;
> /* protected by open_lock */
> unsigned int open_count;
> + /*
> + * Set of retired page offsets within the
> + * EGM region and keyed by page index.
> + */
> + struct xarray retired_pages;
> phys_addr_t egmphys;
> size_t egmlength;
> + phys_addr_t retiredpagesphys;
> u64 egmpxm;
> struct list_head gpus;
> };
> @@ -44,6 +68,9 @@ struct nvgrace_egm_dev_entry {
> */
> static LIST_HEAD(egm_chardevs);
>
> +static void cleanup_retired_pages(struct nvgrace_egm_dev *egm_dev);
> +static int nvgrace_egm_fetch_retired_pages(struct nvgrace_egm_dev *egm_dev);
> +
> static int nvgrace_egm_open(struct inode *inode, struct file *file)
> {
> struct nvgrace_egm_dev *egm_dev =
> @@ -171,11 +198,68 @@ static int nvgrace_egm_mmap(struct file *file, struct vm_area_struct *vma)
> vma->vm_page_prot);
> }
>
> +static long nvgrace_egm_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
> +{
> + unsigned long minsz = offsetofend(struct egm_retired_pages_list, count);
> + struct egm_retired_pages_list info;
> + void __user *uarg = (void __user *)arg;
> + struct nvgrace_egm_dev *egm_dev = file->private_data;
> +
> + if (copy_from_user(&info, uarg, minsz))
> + return -EFAULT;
> +
> + if (info.argsz < minsz || !egm_dev)
> + return -EINVAL;
!egm_dev cannot happen, it's a dead test.
> +
> + switch (cmd) {
> + case EGM_RETIRED_PAGES_LIST: {
> + unsigned long retired_page_struct_size = sizeof(struct egm_retired_pages_info);
> + struct egm_retired_pages_info tmp;
> + unsigned long page_index;
> + void *entry;
> + int count = 0, fill = 0;
> +
> + xa_for_each(&egm_dev->retired_pages, page_index, entry)
> + count++;
> +
> + if (info.argsz < (minsz + count * retired_page_struct_size)) {
> + info.argsz = minsz + count * retired_page_struct_size;
> + info.count = 0;
> + goto done;
> + }
Similar mechanics to vfio, but where is this uAPI defined other than
deciphering the code. Also, this is a new uAPI, not a backwards
compatible uAPI extension. We can return an appropriate errno to
indicate space errors.
> +
> + xa_for_each(&egm_dev->retired_pages, page_index, entry) {
> + if (fill >= count)
> + break;
> +
> + tmp.offset = (u64)page_index << PAGE_SHIFT;
> + tmp.size = PAGE_SIZE;
> +
> + if (copy_to_user((u8 __user *)uarg + minsz +
> + fill * retired_page_struct_size,
> + &tmp, retired_page_struct_size))
> + return -EFAULT;
> +
> + fill++;
> + }
> +
> + info.count = fill;
> + break;
> + }
> + default:
> + return -EINVAL;
> + }
> +
> +done:
> + return copy_to_user(uarg, &info, minsz) ? -EFAULT : 0;
> +}
> +
> static const struct file_operations file_ops = {
> .owner = THIS_MODULE,
> .open = nvgrace_egm_open,
> .release = nvgrace_egm_release,
> .mmap = nvgrace_egm_mmap,
> + .unlocked_ioctl = nvgrace_egm_ioctl,
> };
>
> static int nvgrace_egm_create_gpu_links(struct nvgrace_egm_dev *egm_dev,
> @@ -236,13 +320,15 @@ static void egm_chardev_release(struct device *dev)
> {
> struct nvgrace_egm_dev *egm_chardev = container_of(dev, struct nvgrace_egm_dev, device);
>
> + cleanup_retired_pages(egm_chardev);
> remove_gpus(egm_chardev);
> mutex_destroy(&egm_chardev->open_lock);
> kfree(egm_chardev);
> }
>
> static struct nvgrace_egm_dev *setup_egm_chardev(u64 egmphys, u64 egmlength,
> - u64 egmpxm)
> + u64 egmpxm,
> + u64 retiredpagesphys)
> {
> struct nvgrace_egm_dev *egm_chardev;
> unsigned int baseminor = MINOR(dev);
> @@ -271,7 +357,9 @@ static struct nvgrace_egm_dev *setup_egm_chardev(u64 egmphys, u64 egmlength,
> egm_chardev->egmphys = egmphys;
> egm_chardev->egmlength = egmlength;
> egm_chardev->egmpxm = egmpxm;
> + egm_chardev->retiredpagesphys = retiredpagesphys;
> mutex_init(&egm_chardev->open_lock);
> + xa_init(&egm_chardev->retired_pages);
> INIT_LIST_HEAD(&egm_chardev->gpus);
>
> egm_chardev->device.devt = MKDEV(MAJOR(dev), egm_chardev->egmpxm);
> @@ -284,6 +372,10 @@ static struct nvgrace_egm_dev *setup_egm_chardev(u64 egmphys, u64 egmlength,
> if (ret)
> goto error_exit;
>
> + ret = nvgrace_egm_fetch_retired_pages(egm_chardev);
> + if (ret)
> + goto error_exit;
> +
> ret = cdev_device_add(&egm_chardev->cdev, &egm_chardev->device);
> if (ret)
> goto error_exit;
> @@ -302,6 +394,85 @@ static void del_egm_chardev(struct nvgrace_egm_dev *egm_chardev)
> put_device(&egm_chardev->device);
> }
>
> +static void cleanup_retired_pages(struct nvgrace_egm_dev *egm_dev)
> +{
> + /* Entries are value marks, so xa_destroy() frees the table itself. */
> + xa_destroy(&egm_dev->retired_pages);
> +}
> +
> +static int nvgrace_egm_fetch_retired_pages(struct nvgrace_egm_dev *egm_dev)
> +{
> + struct fw_egm_retired_pages *egm_retired;
> + u64 count;
> + int index, ret = 0;
> +
> + /* No retired-pages region was advertised; nothing to populate. */
> + if (!egm_dev->retiredpagesphys)
> + return 0;
> +
> + egm_retired = memremap(egm_dev->retiredpagesphys,
> + sizeof(*egm_retired), MEMREMAP_WB);
> + if (!egm_retired)
> + return -ENOMEM;
> +
> + count = egm_retired->num_retired_pages;
> + if (count > ARRAY_SIZE(egm_retired->retired_page_addr)) {
> + memunmap(egm_retired);
> + return -EINVAL;
> + }
> +
> + for (index = 0; index < count && !ret; index++) {
> + phys_addr_t base = egm_retired->retired_page_addr[index];
> + int sub;
> +
> + /*
> + * Entries come from SBIOS. Skip any base outside the EGM
> + * region [egmphys, egmphys + egmlength).
> + */
> + if (base < egm_dev->egmphys ||
> + base >= egm_dev->egmphys + egm_dev->egmlength) {
> + dev_warn_ratelimited(&egm_dev->device,
> + "Ignoring retired page %pa outside EGM region\n",
> + &base);
> + continue;
> + }
Don't we need to take the page itself into account, not just the base?
> +
> + /*
> + * Since the EGM is linearly mapped, the offset in the
> + * carveout is the same offset in the VM system memory.
> + *
> + * Calculate the offset to communicate to the usermode
> + * apps.
> + *
> + * The retired page entry represent a retired region of
> + * size 64K. So on a 4K kernel, each entry spans 0x10
> + * 4K sub-pages; add one entry per sub-page so that any
> + * lookup at 4K granularity hits the right retired range.
> + */
> + for (sub = 0; sub < SZ_64K / PAGE_SIZE; sub++) {
> + unsigned long offset = base +
> + (phys_addr_t)sub * PAGE_SIZE - egm_dev->egmphys;
> + void *old;
> +
> + old = xa_store(&egm_dev->retired_pages,
> + offset >> PAGE_SHIFT, EGM_RETIRED_MARK,
> + GFP_KERNEL);
> + if (xa_is_err(old)) {
> + ret = xa_err(old);
> + break;
> + }
> + }
> + }
> +
> + memunmap(egm_retired);
> +
> + /*
> + * On failure the partially populated table is freed by
> + * egm_chardev_release() via the caller's error path.
> + */
> + return ret;
> +}
> +
> static char *egm_devnode(const struct device *device, umode_t *mode)
> {
> if (mode)
> @@ -347,22 +518,39 @@ static int has_egm_property(struct pci_dev *pdev, u64 *pegmpxm)
> }
>
> static int fetch_egm_property(struct pci_dev *pdev, u64 *pegmphys,
> - u64 *pegmlength)
> + u64 *pegmlength, u64 *pretiredpagesphys)
> {
> int ret;
>
> /*
> - * The memory information is present in the system ACPI tables as DSD
> - * properties nvidia,egm-base-pa and nvidia,egm-size.
> + * The EGM memory information is present in the system ACPI tables
> + * as DSD properties nvidia,egm-base-pa and nvidia,egm-size.
> */
> ret = device_property_read_u64(&pdev->dev, "nvidia,egm-size",
> pegmlength);
> if (ret)
> - return ret;
> + goto error_exit;
>
> ret = device_property_read_u64(&pdev->dev, "nvidia,egm-base-pa",
> pegmphys);
> + if (ret)
> + goto error_exit;
> +
> + /*
> + * SBIOS puts the list of retired pages on a region exposed as
> + * "nvidia,egm-retired-pages-data-base". Older firmware may not expose
> + * it. Treat an absent or zero property as 0 retired pages so the
> + * EGM device is still created (with an empty retired-pages table).
> + */
> + ret = device_property_read_u64(&pdev->dev,
> + "nvidia,egm-retired-pages-data-base",
> + pretiredpagesphys);
> + if (ret) {
> + *pretiredpagesphys = 0;
> + ret = 0;
> + }
>
> +error_exit:
> return ret;
> }
>
> @@ -409,12 +597,13 @@ static int nvgrace_egm_create_pci_egm_devs(void)
> int ret;
>
> for_each_pci_dev(pdev) {
> - u64 egmphys, egmlength, egmpxm;
> + u64 egmphys, egmlength, egmpxm, retiredpagesphys;
>
> if (has_egm_property(pdev, &egmpxm))
> continue;
>
> - ret = fetch_egm_property(pdev, &egmphys, &egmlength);
> + ret = fetch_egm_property(pdev, &egmphys, &egmlength,
> + &retiredpagesphys);
> if (ret)
> continue;
>
> @@ -432,7 +621,8 @@ static int nvgrace_egm_create_pci_egm_devs(void)
> return -ENOMEM;
> }
>
> - egm_dev = setup_egm_chardev(egmphys, egmlength, egmpxm);
> + egm_dev = setup_egm_chardev(egmphys, egmlength, egmpxm,
> + retiredpagesphys);
> if (!egm_dev) {
> kfree(egm_entry);
> pci_dev_put(pdev);
> @@ -478,12 +668,13 @@ static int nvgrace_egm_lowest_pxm(u64 *pmin_pxm)
> bool found = false;
>
> for_each_pci_dev(pdev) {
> - u64 egmphys, egmlength, egmpxm;
> + u64 egmphys, egmlength, egmpxm, retiredpagesphys;
>
> if (has_egm_property(pdev, &egmpxm))
> continue;
>
> - if (fetch_egm_property(pdev, &egmphys, &egmlength))
> + if (fetch_egm_property(pdev, &egmphys, &egmlength,
> + &retiredpagesphys))
> continue;
>
> if (!found || egmpxm < *pmin_pxm) {
> diff --git a/include/uapi/linux/egm.h b/include/uapi/linux/egm.h
> new file mode 100644
> index 000000000000..ee165056c2f6
> --- /dev/null
> +++ b/include/uapi/linux/egm.h
> @@ -0,0 +1,28 @@
> +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
> +/*
> + * Copyright (c) 2026, NVIDIA CORPORATION & AFFILIATES. All rights reserved
> + */
> +
> +#ifndef _UAPI_LINUX_EGM_H
> +#define _UAPI_LINUX_EGM_H
> +
> +#include <linux/types.h>
> +
> +#define EGM_TYPE ('x')
> +
> +struct egm_retired_pages_info {
> + __aligned_u64 offset;
> + __aligned_u64 size;
> +};
> +
> +struct egm_retired_pages_list {
> + __u32 argsz;
> + /* out */
> + __u32 count;
> + /* out */
> + struct egm_retired_pages_info retired_pages[];
> +};
> +
> +#define EGM_RETIRED_PAGES_LIST _IOWR(EGM_TYPE, 0x01, struct egm_retired_pages_list)
> +
> +#endif /* _UAPI_LINUX_EGM_H */
It's generally a good idea to formalize the uAPI in comments here to
define the expected behavior and content rather than necessarily
letting the implementation define the uAPI. Thanks,
Alex
next prev parent reply other threads:[~2026-08-05 23:19 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-07-02 19:25 [PATCH 0/4] Introduce nvgrace-egm driver for Extended GPU Memory Ankit Agrawal
2026-07-02 19:25 ` [PATCH 1/4] platform/nvidia: Introduce nvgrace-egm driver and enumerate EGM regions Ankit Agrawal
2026-07-02 22:22 ` Armin Wolf
2026-07-03 13:59 ` Jason Gunthorpe
2026-08-05 23:18 ` Alex Williamson
2026-08-06 20:39 ` Jason Gunthorpe
2026-07-02 19:25 ` [PATCH 2/4] platform/nvidia: Implement mmap and memory scrubbing for EGM chardev Ankit Agrawal
2026-08-05 23:18 ` Alex Williamson
2026-08-06 20:39 ` Jason Gunthorpe
2026-07-02 19:25 ` [PATCH 3/4] platform/nvidia: Handle retired ECC pages and expose via ioctl Ankit Agrawal
2026-08-05 23:18 ` Alex Williamson [this message]
2026-08-06 20:39 ` Jason Gunthorpe
2026-07-02 19:25 ` [PATCH 4/4] platform/nvidia: Register EGM PFNMAP range with memory_failure Ankit Agrawal
2026-08-03 16:43 ` [PATCH 0/4] Introduce nvgrace-egm driver for Extended GPU Memory Ankit Agrawal
2026-08-04 7:24 ` gregkh
2026-08-04 14:14 ` Jason Gunthorpe
2026-08-04 14:50 ` Ankit Agrawal
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260805171842.7dbcbf67@shazbot.org \
--to=alex@shazbot.org \
--cc=W_Armin@gmx.de \
--cc=ankita@nvidia.com \
--cc=clg@redhat.com \
--cc=gregkh@linuxfoundation.org \
--cc=ilpo.jarvinen@linux.intel.com \
--cc=jai.luthra@ideasonboard.com \
--cc=jgg@nvidia.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mhonap@nvidia.com \
--cc=mochs@nvidia.com \
--cc=vsethi@nvidia.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox