[PATCH] nvme: Support user mode processing of ZNS Zone Changed AEN

[PATCH] nvme: Support user mode processing of ZNS Zone Changed AEN

[clay.mayers]

From: Clay Mayers <clay.mayers@kioxia.com>

The ZNS Zone-Descriptor-Changes AEN is of type notify and uses CQE.DW1
to convey the namespace of the log page to read. Unhandled notify AENs
now generate an NVME_AEN uevent and include the value of CQE.DW1 as a
new property when non-zero.

All unhandled notify events are include to avoid command set specific
code in the aen handling.  This support is planned to be used by both
zone based applications and another unreleased device with an alternate
command set.

Signed-off-by: Clay Mayers <clay.mayers@kioxia.com>
---
 drivers/nvme/host/core.c | 19 +++++++++++--------
 drivers/nvme/host/nvme.h |  1 +
 2 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 66446f1e06cf..fa865b8da1d3 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -4615,17 +4615,20 @@ static void nvme_change_uevent(struct nvme_ctrl *ctrl, char *envdata)
 
 static void nvme_aen_uevent(struct nvme_ctrl *ctrl)
 {
-	char *envp[2] = { NULL, NULL };
+	char *envp[3] = { NULL, NULL, NULL };
 	u32 aen_result = ctrl->aen_result;
+	u32 aen_data = ctrl->aen_data;
 
 	ctrl->aen_result = 0;
+	ctrl->aen_data = 0;
 	if (!aen_result)
 		return;
 
 	envp[0] = kasprintf(GFP_KERNEL, "NVME_AEN=%#08x", aen_result);
-	if (!envp[0])
-		return;
-	kobject_uevent_env(&ctrl->device->kobj, KOBJ_CHANGE, envp);
+	envp[1] = kasprintf(GFP_KERNEL, "NVME_AEN_DATA=%#08x", aen_data);
+	if (envp[0] && envp[1])
+		kobject_uevent_env(&ctrl->device->kobj, KOBJ_CHANGE, envp);
+	kfree(envp[1]);
 	kfree(envp[0]);
 }
 
@@ -4748,11 +4751,9 @@ static bool nvme_handle_aen_notice(struct nvme_ctrl *ctrl, u32 result)
 		queue_work(nvme_wq, &ctrl->ana_work);
 		break;
 #endif
-	case NVME_AER_NOTICE_DISC_CHANGED:
+	default:
 		ctrl->aen_result = result;
 		break;
-	default:
-		dev_warn(ctrl->device, "async event result %08x\n", result);
 	}
 	return requeue;
 }
@@ -4799,8 +4800,10 @@ void nvme_complete_async_event(struct nvme_ctrl *ctrl, __le16 status,
 		break;
 	}
 
-	if (requeue)
+	if (requeue) {
+		ctrl->aen_data = le64_to_cpu(res->u64) >> 32;
 		queue_work(nvme_wq, &ctrl->async_event_work);
+	}
 }
 EXPORT_SYMBOL_GPL(nvme_complete_async_event);
 
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index 1bdf714dcd9e..a1bab447dc65 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -299,6 +299,7 @@ struct nvme_ctrl {
 	u16 cctemp;
 	u32 oaes;
 	u32 aen_result;
+	u32 aen_data;
 	u32 ctratt;
 	unsigned int shutdown_timeout;
 	unsigned int kato;
-- 
2.27.0

Re: [PATCH] nvme: Support user mode processing of ZNS Zone Changed AEN

[Sagi Grimberg]

> The ZNS Zone-Descriptor-Changes AEN is of type notify and uses CQE.DW1
> to convey the namespace of the log page to read. Unhandled notify AENs
> now generate an NVME_AEN uevent and include the value of CQE.DW1 as a
> new property when non-zero.

But you always send it...

> 
> All unhandled notify events are include to avoid command set specific
> code in the aen handling.  This support is planned to be used by both
> zone based applications and another unreleased device with an alternate
> command set.

This needs to be split into two patches.

> 
> Signed-off-by: Clay Mayers <clay.mayers@kioxia.com>
> ---
>   drivers/nvme/host/core.c | 19 +++++++++++--------
>   drivers/nvme/host/nvme.h |  1 +
>   2 files changed, 12 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> index 66446f1e06cf..fa865b8da1d3 100644
> --- a/drivers/nvme/host/core.c
> +++ b/drivers/nvme/host/core.c
> @@ -4615,17 +4615,20 @@ static void nvme_change_uevent(struct nvme_ctrl *ctrl, char *envdata)
>   
>   static void nvme_aen_uevent(struct nvme_ctrl *ctrl)
>   {
> -	char *envp[2] = { NULL, NULL };
> +	char *envp[3] = { NULL, NULL, NULL };
>   	u32 aen_result = ctrl->aen_result;
> +	u32 aen_data = ctrl->aen_data;
>   
>   	ctrl->aen_result = 0;
> +	ctrl->aen_data = 0;
>   	if (!aen_result)
>   		return;
>   
>   	envp[0] = kasprintf(GFP_KERNEL, "NVME_AEN=%#08x", aen_result);
> -	if (!envp[0])
> -		return;
> -	kobject_uevent_env(&ctrl->device->kobj, KOBJ_CHANGE, envp);
> +	envp[1] = kasprintf(GFP_KERNEL, "NVME_AEN_DATA=%#08x", aen_data);
> +	if (envp[0] && envp[1])
> +		kobject_uevent_env(&ctrl->device->kobj, KOBJ_CHANGE, envp);
> +	kfree(envp[1]);
>   	kfree(envp[0]);
>   }
>   
> @@ -4748,11 +4751,9 @@ static bool nvme_handle_aen_notice(struct nvme_ctrl *ctrl, u32 result)
>   		queue_work(nvme_wq, &ctrl->ana_work);
>   		break;
>   #endif
> -	case NVME_AER_NOTICE_DISC_CHANGED:
> +	default:
>   		ctrl->aen_result = result;
>   		break;
> -	default:
> -		dev_warn(ctrl->device, "async event result %08x\n", result);

I'd keep the log... and also don't know if we want to pass any unknown
possible spam to userspace... not sure that being blindly forward
compatible is the right choice here.

>   	}
>   	return requeue;
>   }
> @@ -4799,8 +4800,10 @@ void nvme_complete_async_event(struct nvme_ctrl *ctrl, __le16 status,
>   		break;
>   	}
>   
> -	if (requeue)
> +	if (requeue) {
> +		ctrl->aen_data = le64_to_cpu(res->u64) >> 32;

Please use a helper for that.
aen_data is not a great name... maybe use a union for that for the
specific aen subtype?

Maybe it'd be better that zoned namespace aen is handled explicitly
and passes a AEN_NSID env veriable to the uevent.

>   		queue_work(nvme_wq, &ctrl->async_event_work);
> +	}
>   }
>   EXPORT_SYMBOL_GPL(nvme_complete_async_event);
>   
> diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
> index 1bdf714dcd9e..a1bab447dc65 100644
> --- a/drivers/nvme/host/nvme.h
> +++ b/drivers/nvme/host/nvme.h
> @@ -299,6 +299,7 @@ struct nvme_ctrl {
>   	u16 cctemp;
>   	u32 oaes;
>   	u32 aen_result;
> +	u32 aen_data;
>   	u32 ctratt;
>   	unsigned int shutdown_timeout;
>   	unsigned int kato;

RE: [PATCH] nvme: Support user mode processing of ZNS Zone Changed AEN

[Clay Mayers]

> From: Sagi Grimberg <sagi@grimberg.me>
> Sent: Thursday, September 29, 2022 1:19 AM
> 
> > The ZNS Zone-Descriptor-Changes AEN is of type notify and uses CQE.DW1
> > to convey the namespace of the log page to read. Unhandled notify AENs
> > now generate an NVME_AEN uevent and include the value of CQE.DW1 as a
> > new property when non-zero.
> 
> But you always send it...

Thanks for the quick review and catching my mistake.

> 
> >
> > All unhandled notify events are include to avoid command set specific
> > code in the aen handling.  This support is planned to be used by both
> > zone based applications and another unreleased device with an alternate
> > command set.
> 
> This needs to be split into two patches.
> 

It will be split in two for V2