[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH V3 7/8] hw/block/nvme: support changed namespace asyncrohous
From: |
Klaus Jensen |
Subject: |
Re: [PATCH V3 7/8] hw/block/nvme: support changed namespace asyncrohous event |
Date: |
Mon, 1 Mar 2021 06:56:02 +0100 |
On Mar 1 01:10, Minwoo Im wrote:
> If namespace inventory is changed due to some reasons (e.g., namespace
> attachment/detachment), controller can send out event notifier to the
> host to manage namespaces.
>
> This patch sends out the AEN to the host after either attach or detach
> namespaces from controllers. To support clear of the event from the
> controller, this patch also implemented Get Log Page command for Changed
> Namespace List log type. To return namespace id list through the
> command, when namespace inventory is updated, id is added to the
> per-controller list (changed_ns_list).
>
> To indicate the support of this async event, this patch set
> OAES(Optional Asynchronous Events Supported) in Identify Controller data
> structure.
>
> Signed-off-by: Minwoo Im <minwoo.im.dev@gmail.com>
> ---
> hw/block/nvme.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
> hw/block/nvme.h | 7 +++++++
> include/block/nvme.h | 7 +++++++
> 3 files changed, 58 insertions(+)
>
> diff --git a/hw/block/nvme.c b/hw/block/nvme.c
> index 68c2e63d9412..fc06f806e58e 100644
> --- a/hw/block/nvme.c
> +++ b/hw/block/nvme.c
> @@ -2980,6 +2980,32 @@ static uint16_t nvme_error_info(NvmeCtrl *n, uint8_t
> rae, uint32_t buf_len,
> DMA_DIRECTION_FROM_DEVICE, req);
> }
>
> +static uint16_t nvme_changed_nslist(NvmeCtrl *n, uint8_t rae, uint32_t
> buf_len,
> + uint64_t off, NvmeRequest *req)
> +{
> + uint32_t nslist[1024];
> + uint32_t trans_len;
> + NvmeChangedNs *ns, *next;
> + int i = 0;
> +
> + memset(nslist, 0x0, sizeof(nslist));
> + trans_len = MIN(sizeof(nslist) - off, buf_len);
> +
> + QTAILQ_FOREACH_SAFE(ns, &n->changed_ns_list, entry, next) {
> + nslist[i++] = ns->nsid;
> +
> + QTAILQ_REMOVE(&n->changed_ns_list, ns, entry);
> + g_free(ns);
> + }
> +
> + if (!rae) {
> + nvme_clear_events(n, NVME_AER_TYPE_NOTICE);
> + }
> +
> + return nvme_dma(n, ((uint8_t *)nslist) + off, trans_len,
> + DMA_DIRECTION_FROM_DEVICE, req);
> +}
> +
> static uint16_t nvme_cmd_effects(NvmeCtrl *n, uint8_t csi, uint32_t buf_len,
> uint64_t off, NvmeRequest *req)
> {
> @@ -3064,6 +3090,8 @@ static uint16_t nvme_get_log(NvmeCtrl *n, NvmeRequest
> *req)
> return nvme_smart_info(n, rae, len, off, req);
> case NVME_LOG_FW_SLOT_INFO:
> return nvme_fw_log_info(n, len, off, req);
> + case NVME_LOG_CHANGED_NSLIST:
> + return nvme_changed_nslist(n, rae, len, off, req);
> case NVME_LOG_CMD_EFFECTS:
> return nvme_cmd_effects(n, csi, len, off, req);
> default:
> @@ -3882,6 +3910,7 @@ static uint16_t nvme_ns_attachment(NvmeCtrl *n,
> NvmeRequest *req)
> uint16_t *ids = &list[1];
> uint16_t ret;
> int i;
> + NvmeChangedNs *changed_nsid;
>
> trace_pci_nvme_ns_attachment(nvme_cid(req), dw10 & 0xf);
>
> @@ -3920,6 +3949,18 @@ static uint16_t nvme_ns_attachment(NvmeCtrl *n,
> NvmeRequest *req)
>
> nvme_ns_detach(ctrl, ns);
> }
> +
> + /*
> + * Add namespace id to the changed namespace id list for event
> clearing
> + * via Get Log Page command.
> + */
> + changed_nsid = g_new(NvmeChangedNs, 1);
> + changed_nsid->nsid = nsid;
> + QTAILQ_INSERT_TAIL(&ctrl->changed_ns_list, changed_nsid, entry);
> +
> + nvme_enqueue_event(ctrl, NVME_AER_TYPE_NOTICE,
> + NVME_AER_INFO_NOTICE_NS_ATTR_CHANGED,
> + NVME_LOG_CHANGED_NSLIST);
> }
If one just keeps attaching/detaching we end up with more than 1024
entries here and go out of bounds in nvme_changed_nslist.
How about having the QTAILQ_ENTRY directly on the NvmeNamespace struct
and use QTAILQ_IN_USE to check if the namespace is already in the list?
>
> return NVME_SUCCESS;
> @@ -4714,6 +4755,7 @@ static void nvme_init_state(NvmeCtrl *n)
> n->features.temp_thresh_hi = NVME_TEMPERATURE_WARNING;
> n->starttime_ms = qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL);
> n->aer_reqs = g_new0(NvmeRequest *, n->params.aerl + 1);
> + QTAILQ_INIT(&n->changed_ns_list);
> }
>
> static int nvme_attach_namespace(NvmeCtrl *n, NvmeNamespace *ns, Error
> **errp)
> @@ -4910,6 +4952,8 @@ static void nvme_init_ctrl(NvmeCtrl *n, PCIDevice
> *pci_dev)
>
> id->cntlid = cpu_to_le16(n->cntlid);
>
> + id->oaes = cpu_to_le32(NVME_OAES_NS_ATTR);
> +
> id->rab = 6;
>
> if (n->params.use_intel_id) {
> diff --git a/hw/block/nvme.h b/hw/block/nvme.h
> index 74a00ab21a55..d5eaea003ea5 100644
> --- a/hw/block/nvme.h
> +++ b/hw/block/nvme.h
> @@ -132,6 +132,11 @@ typedef struct NvmeFeatureVal {
> uint32_t async_config;
> } NvmeFeatureVal;
>
> +typedef struct NvmeChangedNs {
> + uint32_t nsid;
> + QTAILQ_ENTRY(NvmeChangedNs) entry;
> +} NvmeChangedNs;
> +
> typedef struct NvmeCtrl {
> PCIDevice parent_obj;
> MemoryRegion bar0;
> @@ -177,6 +182,8 @@ typedef struct NvmeCtrl {
> QTAILQ_HEAD(, NvmeAsyncEvent) aer_queue;
> int aer_queued;
>
> + QTAILQ_HEAD(, NvmeChangedNs) changed_ns_list; /* Changed NS list log */
> +
> NvmeSubsystem *subsys;
>
> NvmeNamespace namespace;
> diff --git a/include/block/nvme.h b/include/block/nvme.h
> index 339784d9c23a..eb0b31e949c2 100644
> --- a/include/block/nvme.h
> +++ b/include/block/nvme.h
> @@ -760,6 +760,7 @@ typedef struct QEMU_PACKED NvmeCopySourceRange {
> enum NvmeAsyncEventRequest {
> NVME_AER_TYPE_ERROR = 0,
> NVME_AER_TYPE_SMART = 1,
> + NVME_AER_TYPE_NOTICE = 2,
> NVME_AER_TYPE_IO_SPECIFIC = 6,
> NVME_AER_TYPE_VENDOR_SPECIFIC = 7,
> NVME_AER_INFO_ERR_INVALID_DB_REGISTER = 0,
> @@ -771,6 +772,7 @@ enum NvmeAsyncEventRequest {
> NVME_AER_INFO_SMART_RELIABILITY = 0,
> NVME_AER_INFO_SMART_TEMP_THRESH = 1,
> NVME_AER_INFO_SMART_SPARE_THRESH = 2,
> + NVME_AER_INFO_NOTICE_NS_ATTR_CHANGED = 0,
> };
>
> typedef struct QEMU_PACKED NvmeAerResult {
> @@ -940,6 +942,7 @@ enum NvmeLogIdentifier {
> NVME_LOG_ERROR_INFO = 0x01,
> NVME_LOG_SMART_INFO = 0x02,
> NVME_LOG_FW_SLOT_INFO = 0x03,
> + NVME_LOG_CHANGED_NSLIST = 0x04,
> NVME_LOG_CMD_EFFECTS = 0x05,
> };
>
> @@ -1046,6 +1049,10 @@ typedef struct NvmeIdCtrlZoned {
> uint8_t rsvd1[4095];
> } NvmeIdCtrlZoned;
>
> +enum NvmeIdCtrlOaes {
> + NVME_OAES_NS_ATTR = 1 << 8,
> +};
> +
> enum NvmeIdCtrlOacs {
> NVME_OACS_SECURITY = 1 << 0,
> NVME_OACS_FORMAT = 1 << 1,
> --
> 2.25.1
>
--
One of us - No more doubt, silence or taboo about mental illness.
signature.asc
Description: PGP signature
- Re: [PATCH V3 7/8] hw/block/nvme: support changed namespace asyncrohous event,
Klaus Jensen <=