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

[Jason Gunthorpe]

> 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.

This can probably be two patches, it will make it easier to see the uapi

> +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;

This is missing struct zero content checking for compatability.


> +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[];
> +};

I think you are better to follow the ioctl design from iommufd..

struct egm_retired_pages_list {
       __u32 size;
       __u32 flags;
       __u32 out_count;
       __aligned_u64 retired_pages; // Pointer to struct egm_retired_pages_list[]
}

We have hit in vfio again and again that these trailing structs become a
problem for extension..

You can cut and paste the simplified version from fwctl_fops_ioctl()

-- 
Jason