[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 0/4] hw/nvme: add support for TP4084
From: |
Klaus Jensen |
Subject: |
Re: [PATCH 0/4] hw/nvme: add support for TP4084 |
Date: |
Thu, 16 Jun 2022 12:42:49 +0200 |
On Jun 8 03:28, Niklas Cassel via wrote:
> Hello there,
>
> considering that Linux v5.19-rc1 is out which includes support for
> NVMe TP4084:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/nvme/host/core.c?id=354201c53e61e493017b15327294b0c8ab522d69
>
> I thought that it might be nice to have QEMU support for the same.
>
> TP4084 adds a new mode, CC.CRIME, that can be used to mark a namespace
> as ready independently from the controller.
>
> When CC.CRIME is 0 (default), things behave as before, all namespaces
> are ready when CSTS.RDY gets set to 1.
>
> Add a new "ready_delay" namespace device parameter, in order to emulate
> different ready latencies for namespaces when CC.CRIME is 1.
>
> The patch series also adds a "crwmt" controller parameter, in order to
> be able to expose the worst case timeout that the host should wait for
> all namespaces to become ready.
>
>
> Example qemu cmd line for the new options:
>
> # delay in s (20s)
> NS1_DELAY_S=20
> # convert to units of 500ms
> NS1_DELAY=$((NS1_DELAY_S*2))
>
> # delay in s (60s)
> NS2_DELAY_S=60
> # convert to units of 500ms
> NS2_DELAY=$((NS2_DELAY_S*2))
>
> # timeout in s (120s)
> CRWMT_S=120
> # convert to units of 500ms
> CRWMT=$((CRWMT_S*2))
>
> -device nvme,serial=deadbeef,crwmt=$CRWMT \
> -drive file=$NS1_DATA,id=nvm-1,format=raw,if=none \
> -device nvme-ns,drive=nvm-1,ready_delay=$NS1_DELAY \
> -drive file=$NS2_DATA,id=nvm-2,format=raw,if=none \
> -device nvme-ns,drive=nvm-2,ready_delay=$NS2_DELAY \
>
>
> Niklas Cassel (4):
> hw/nvme: claim NVMe 2.0 compliance
> hw/nvme: store a pointer to the NvmeSubsystem in the NvmeNamespace
> hw/nvme: add support for ratified TP4084
> hw/nvme: add new never_ready parameter to test the DNR bit
>
> hw/nvme/ctrl.c | 151 +++++++++++++++++++++++++++++++++++++++++--
> hw/nvme/ns.c | 17 +++++
> hw/nvme/nvme.h | 9 +++
> hw/nvme/trace-events | 1 +
> include/block/nvme.h | 60 ++++++++++++++++-
> 5 files changed, 233 insertions(+), 5 deletions(-)
>
> --
> 2.36.1
>
>
Hi Niklas,
I've been going back and forth on my position on this.
I'm not straight up against it, but this only seems useful as a one-off
patch to test the kernel support for this. Considering the limitations
you state and the limited use case, I fear this is a little bloaty to
carry upstream.
But I totally acknowledge that this is a horrible complicated behavior
to implement on the driver side, so I guess we might all benefit from
this.
Keith, do you have an opinion on this?
signature.asc
Description: PGP signature