All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] nvme: mask CSE effects for security receive
@ 2023-01-20 17:17 Keith Busch
  2023-01-20 19:39 ` Jens Axboe
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Keith Busch @ 2023-01-20 17:17 UTC (permalink / raw)
  To: linux-nvme, hch; +Cc: sagi, Keith Busch, Jens Axboe

From: Keith Busch <kbusch@kernel.org>

The nvme driver will freeze the IO queues in response to an admin
command with CSE bits set. These bits notify the host that the command
that's about to be executed needs to be done exclusively, hence the
freeze.

The Security Receive command is often reported by multiple vendors with
CSE bits set. The reason for this is that the result depends on the
previous Security Send. This has nothing to do with IO queues, though,
so the driver is taking an overly cautious response to seeing this
passthrough command, while unable to fufill the intended admin queue
action.

Rather than freeze IO during this harmless command, mask off the
effects. This freezing is observed to cause IO latency spikes when host
software periodically validates the security state of the drives.

Cc: Jens Axboe <axboe@kernel.dk>
Signed-off-by: Keith Busch <kbusch@kernel.org>
---
 drivers/nvme/host/core.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index d7d2c2b342ba4..1b10b27deb999 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -3101,6 +3101,23 @@ static void nvme_init_known_nvm_effects(struct nvme_ctrl *ctrl)
 	log->acs[nvme_admin_sanitize_nvm] |= cpu_to_le32(NVME_CMD_EFFECTS_LBCC |
 						NVME_CMD_EFFECTS_CSE_MASK);
 
+	/*
+	 * The spec says the result of a security receive command depends on
+	 * the previous security send command. As such, many vendors logs this
+	 * command as one to submitted only when no other commands to the same
+	 * namespace are outstanding. The intention is to tell the host to
+	 * prevent mixing security send and receive.
+	 *
+	 * This driver can only enforce such exclusive access against IO
+	 * queues, though. We are not readily able to enforce such a rule for
+	 * two commands to the admin queue, which is the only queue that
+	 * matters for this command.
+	 *
+	 * Rather than blindly freezing the IO queues for this effect that
+	 * doesn't even apply to IO, mask it off.
+	 */
+	log->acs[nvme_admin_security_recv] &= ~NVME_CMD_EFFECTS_CSE_MASK;
+
 	log->iocs[nvme_cmd_write] |= cpu_to_le32(NVME_CMD_EFFECTS_LBCC);
 	log->iocs[nvme_cmd_write_zeroes] |= cpu_to_le32(NVME_CMD_EFFECTS_LBCC);
 	log->iocs[nvme_cmd_write_uncor] |= cpu_to_le32(NVME_CMD_EFFECTS_LBCC);
-- 
2.30.2



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] nvme: mask CSE effects for security receive
  2023-01-20 17:17 [PATCH] nvme: mask CSE effects for security receive Keith Busch
@ 2023-01-20 19:39 ` Jens Axboe
  2023-01-23  9:03 ` Sagi Grimberg
  2023-01-23 16:41 ` Christoph Hellwig
  2 siblings, 0 replies; 6+ messages in thread
From: Jens Axboe @ 2023-01-20 19:39 UTC (permalink / raw)
  To: Keith Busch, linux-nvme, hch; +Cc: sagi, Keith Busch

On 1/20/23 10:17 AM, Keith Busch wrote:
> From: Keith Busch <kbusch@kernel.org>
> 
> The nvme driver will freeze the IO queues in response to an admin
> command with CSE bits set. These bits notify the host that the command
> that's about to be executed needs to be done exclusively, hence the
> freeze.
> 
> The Security Receive command is often reported by multiple vendors with
> CSE bits set. The reason for this is that the result depends on the
> previous Security Send. This has nothing to do with IO queues, though,
> so the driver is taking an overly cautious response to seeing this
> passthrough command, while unable to fufill the intended admin queue
> action.
> 
> Rather than freeze IO during this harmless command, mask off the
> effects. This freezing is observed to cause IO latency spikes when host
> software periodically validates the security state of the drives.

Reviewed-by: Jens Axboe <axboe@kernel.dk>

-- 
Jens Axboe




^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] nvme: mask CSE effects for security receive
  2023-01-20 17:17 [PATCH] nvme: mask CSE effects for security receive Keith Busch
  2023-01-20 19:39 ` Jens Axboe
@ 2023-01-23  9:03 ` Sagi Grimberg
  2023-01-23 16:41 ` Christoph Hellwig
  2 siblings, 0 replies; 6+ messages in thread
From: Sagi Grimberg @ 2023-01-23  9:03 UTC (permalink / raw)
  To: Keith Busch, linux-nvme, hch; +Cc: Keith Busch, Jens Axboe

Reviewed-by: Sagi Grimberg <sagi@grimberg.me>


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] nvme: mask CSE effects for security receive
  2023-01-20 17:17 [PATCH] nvme: mask CSE effects for security receive Keith Busch
  2023-01-20 19:39 ` Jens Axboe
  2023-01-23  9:03 ` Sagi Grimberg
@ 2023-01-23 16:41 ` Christoph Hellwig
  2023-01-23 16:43   ` Christoph Hellwig
  2023-01-23 16:47   ` Keith Busch
  2 siblings, 2 replies; 6+ messages in thread
From: Christoph Hellwig @ 2023-01-23 16:41 UTC (permalink / raw)
  To: Keith Busch; +Cc: linux-nvme, hch, sagi, Keith Busch, Jens Axboe

As-is this only applies to nvme-6.3, is that intended?


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] nvme: mask CSE effects for security receive
  2023-01-23 16:41 ` Christoph Hellwig
@ 2023-01-23 16:43   ` Christoph Hellwig
  2023-01-23 16:47   ` Keith Busch
  1 sibling, 0 replies; 6+ messages in thread
From: Christoph Hellwig @ 2023-01-23 16:43 UTC (permalink / raw)
  To: Keith Busch; +Cc: linux-nvme, hch, sagi, Keith Busch, Jens Axboe

On Mon, Jan 23, 2023 at 05:41:18PM +0100, Christoph Hellwig wrote:
> As-is this only applies to nvme-6.3, is that intended?

In fact it doesn't apply to nvme-6.3 either, I guess we need your
CSE initialization patch first.


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] nvme: mask CSE effects for security receive
  2023-01-23 16:41 ` Christoph Hellwig
  2023-01-23 16:43   ` Christoph Hellwig
@ 2023-01-23 16:47   ` Keith Busch
  1 sibling, 0 replies; 6+ messages in thread
From: Keith Busch @ 2023-01-23 16:47 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: Keith Busch, linux-nvme, sagi, Jens Axboe

On Mon, Jan 23, 2023 at 05:41:18PM +0100, Christoph Hellwig wrote:
> As-is this only applies to nvme-6.3, is that intended?

Yeah, it also has a dependency on my other pending patch that always
allocates the log page, which isn't in 6.3 yet (I have to send a new
version of that, anyway).

I can backport if desired for 6.2, but I don't think this is urgent
enough to get in right away.


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2023-01-23 16:52 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-20 17:17 [PATCH] nvme: mask CSE effects for security receive Keith Busch
2023-01-20 19:39 ` Jens Axboe
2023-01-23  9:03 ` Sagi Grimberg
2023-01-23 16:41 ` Christoph Hellwig
2023-01-23 16:43   ` Christoph Hellwig
2023-01-23 16:47   ` Keith Busch

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.