All of lore.kernel.org
 help / color / mirror / Atom feed
From: Keith Busch <kbusch@kernel.org>
To: Chao Shi <coshi036@gmail.com>
Cc: linux-nvme@lists.infradead.org, Christoph Hellwig <hch@lst.de>,
	Sagi Grimberg <sagi@grimberg.me>, Jens Axboe <axboe@kernel.dk>,
	Tatsuya Sasaki <tatsuya6.sasaki@kioxia.com>,
	Maurizio Lombardi <mlombard@arkamax.eu>,
	linux-kernel@vger.kernel.org, Sungwoo Kim <iam@sung-woo.kim>,
	Dave Tian <daveti@purdue.edu>, Weidong Zhu <weizhu@fiu.edu>
Subject: Re: [PATCH v4] nvme: reject keep-alive passthrough on non-fabrics
Date: Fri, 22 May 2026 13:32:38 -0600	[thread overview]
Message-ID: <ahCvVhKxRLRPWscH@kbusch-mbp> (raw)
In-Reply-To: <20260522162639.395802-1-coshi036@gmail.com>

On Fri, May 22, 2026 at 12:26:39PM -0400, Chao Shi wrote:
> +/*
> + * Some Set Features commands change controller behaviour that the driver is
> + * not prepared to handle on every transport.  Reject such commands from
> + * userspace passthrough rather than letting them put the controller into a
> + * state the driver cannot deal with.  The list can be extended as other
> + * problematic features are identified.
> + */
> +static bool nvme_passthru_cmd_allowed(struct nvme_ctrl *ctrl,
> +				      struct nvme_ns *ns,
> +				      struct nvme_command *c)
> +{
> +	/*
> +	 * This only filters admin commands (ns == NULL).  I/O commands share
> +	 * the opcode space with admin commands - Dataset Management is 0x09,
> +	 * the same value as Set Features - so they must not be inspected here.
> +	 */
> +	if (ns || c->common.opcode != nvme_admin_set_features)
> +		return true;
> +
> +	switch (le32_to_cpu(c->common.cdw10) & 0xff) {
> +	case NVME_FEAT_KATO:
> +		/*
> +		 * Keep Alive is optional on PCIe (NVMe 2.0a 5.27.1.12) and the
> +		 * driver only arms keep-alive for fabrics.  Enabling it on
> +		 * other transports starts a keep-alive command the driver is
> +		 * not set up for and harms idle power states, so reject it.
> +		 */
> +		return ctrl->ops->flags & NVME_F_FABRICS;
> +	default:
> +		return true;
> +	}
> +}

This doesn't need to be its own function. You can add these checks to
the existing nvme_cmd_allowed():

---
@@ -50,6 +53,18 @@ static bool nvme_cmd_allowed(struct nvme_ns *ns, struct nvme_command *c,
                        case NVME_ID_CNS_CTRL:
                                return true;
                        }
+		} else if (c->common.opcode == nvme_admin_set_features) {
+			switch (cpu_to_le32(c->features.fid) & 0xff) {
+			case NVME_FEAT_KATO:
+				if (ctrl->ops->flags & NVME_F_FABRICS)
+					break;
+				fallthrough;
+			case NVME_FEAT_HOST_BEHAVIOR:
+			case NVME_FEAT_HOST_MEM_BUF:
+			case NVME_FEAT_NUM_QUEUES:
+			case NVME_FEAT_AUTO_PST:
+				return false;
+			}
                }
                goto admin;
        }
--


  reply	other threads:[~2026-05-22 19:32 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-22 16:26 [PATCH v4] nvme: reject keep-alive passthrough on non-fabrics Chao Shi
2026-05-22 19:32 ` Keith Busch [this message]
2026-05-22 22:56   ` Chao S
2026-05-27 14:21   ` Christoph Hellwig

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=ahCvVhKxRLRPWscH@kbusch-mbp \
    --to=kbusch@kernel.org \
    --cc=axboe@kernel.dk \
    --cc=coshi036@gmail.com \
    --cc=daveti@purdue.edu \
    --cc=hch@lst.de \
    --cc=iam@sung-woo.kim \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-nvme@lists.infradead.org \
    --cc=mlombard@arkamax.eu \
    --cc=sagi@grimberg.me \
    --cc=tatsuya6.sasaki@kioxia.com \
    --cc=weizhu@fiu.edu \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.