Re: [PATCHv4 03/13] nvmet: implement supported log pages

["Matias Bjørling" <m@bjorling.me>]

On 07-11-2024 20:38, Keith Busch wrote:
> From: Keith Busch <kbusch@kernel.org>
> 
> This log is required for nvme 2.1.
> 
> Signed-off-by: Keith Busch <kbusch@kernel.org>
> ---
>   drivers/nvme/target/admin-cmd.c | 27 +++++++++++++++++++++++++++
>   include/linux/nvme.h            |  9 +++++++++
>   2 files changed, 36 insertions(+)
> 
> diff --git a/drivers/nvme/target/admin-cmd.c b/drivers/nvme/target/admin-cmd.c
> index a13242e791c0f..712b962fe77ac 100644
> --- a/drivers/nvme/target/admin-cmd.c
> +++ b/drivers/nvme/target/admin-cmd.c
> @@ -71,6 +71,31 @@ static void nvmet_execute_get_log_page_error(struct nvmet_req *req)
>   	nvmet_req_complete(req, 0);
>   }
>   
> +static void nvmet_execute_get_supported_log_pages(struct nvmet_req *req)
> +{
> +	struct nvme_supported_log *logs;
> +	u16 status;
> +
> +	logs = kzalloc(sizeof(*logs), GFP_KERNEL);
> +	if (!logs) {
> +		status = NVME_SC_INTERNAL;
> +		goto out;
> +	}
> +
> +	logs->lids[NVME_LOG_SUPPORTED] = cpu_to_le32(NVME_LIDS_LSUPP);
> +	logs->lids[NVME_LOG_ERROR] = cpu_to_le32(NVME_LIDS_LSUPP);
> +	logs->lids[NVME_LOG_SMART] = cpu_to_le32(NVME_LIDS_LSUPP);
> +	logs->lids[NVME_LOG_FW_SLOT] = cpu_to_le32(NVME_LIDS_LSUPP);
> +	logs->lids[NVME_LOG_CHANGED_NS] = cpu_to_le32(NVME_LIDS_LSUPP);
> +	logs->lids[NVME_LOG_CMD_EFFECTS] = cpu_to_le32(NVME_LIDS_LSUPP);
> +	logs->lids[NVME_LOG_ANA] = cpu_to_le32(NVME_LIDS_LSUPP);
> +
> +	status = nvmet_copy_to_sgl(req, 0, logs, sizeof(*logs));
> +	kfree(logs);
> +out:
> +	nvmet_req_complete(req, status);
> +}
> +
>   static u16 nvmet_get_smart_log_nsid(struct nvmet_req *req,
>   		struct nvme_smart_log *slog)
>   {
> @@ -323,6 +348,8 @@ static void nvmet_execute_get_log_page(struct nvmet_req *req)
>   		return;
>   
>   	switch (req->cmd->get_log_page.lid) {
> +	case NVME_LOG_SUPPORTED:
> +		return nvmet_execute_get_supported_log_pages(req);
>   	case NVME_LOG_ERROR:
>   		return nvmet_execute_get_log_page_error(req);
>   	case NVME_LOG_SMART:
> diff --git a/include/linux/nvme.h b/include/linux/nvme.h
> index 0f263c7e63192..692d606edeed3 100644
> --- a/include/linux/nvme.h
> +++ b/include/linux/nvme.h
> @@ -1245,6 +1245,7 @@ enum {
>   	NVME_FEAT_WRITE_PROTECT	= 0x84,
>   	NVME_FEAT_VENDOR_START	= 0xC0,
>   	NVME_FEAT_VENDOR_END	= 0xFF,
> +	NVME_LOG_SUPPORTED	= 0x00,
>   	NVME_LOG_ERROR		= 0x01,
>   	NVME_LOG_SMART		= 0x02,
>   	NVME_LOG_FW_SLOT	= 0x03,
> @@ -1262,6 +1263,14 @@ enum {
>   	NVME_FWACT_ACTV		= (2 << 3),
>   };
>   
> +struct nvme_supported_log {
> +	__le32	lids[256];
> +};
> +
> +enum {
> +	NVME_LIDS_LSUPP	= 1 << 0,
> +};
> +
>   /* NVMe Namespace Write Protect State */
>   enum {
>   	NVME_NS_NO_WRITE_PROTECT = 0,

Looks good. The patch description could be updated to say it's required 
for NVMe 2.0 and later devices.

Reviewed-by: Matias Bjørling <matias.bjorling@wdc.com>