Re: [PATCH 3/4] platform/nvidia: Handle retired ECC pages and expose via ioctl

[Alex Williamson]

On Thu, 2 Jul 2026 19:25:31 +0000
Ankit Agrawal <ankita@xxxxxxxxxx> 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@xxxxxxxxxx>
> ---
>  .../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@xxxxxxxxxxxx>
>  '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@xxxxxxxxxx>
>  S:	Supported
>  F:	drivers/platform/nvidia/
> +F:	include/uapi/linux/egm.h
>  
>  NVIDIA VRS RTC DRIVER
>  M:	Shubhi Garg <shgarg@xxxxxxxxxx>
> 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