[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 2/2] hw/nvme: move device-scoped functions
|
From: |
Minwoo Im |
|
Subject: |
Re: [PATCH 2/2] hw/nvme: move device-scoped functions |
|
Date: |
Tue, 23 Feb 2021 19:54:34 +0900 |
|
User-agent: |
Mutt/1.11.4 (2019-03-13) |
On 21-02-09 12:08:26, Klaus Jensen wrote:
> From: Klaus Jensen <k.jensen@samsung.com>
>
> Move a bunch of functions that are internal to a device out of the
> shared header.
>
> Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
> ---
> hw/nvme/nvme.h | 110 +------------------------------------------------
> hw/nvme/ctrl.c | 90 +++++++++++++++++++++++++++++++++++++++-
> hw/nvme/ns.c | 7 +++-
> 3 files changed, 97 insertions(+), 110 deletions(-)
>
> diff --git a/hw/nvme/nvme.h b/hw/nvme/nvme.h
> index 452a64499b1b..929c6c553ca2 100644
> --- a/hw/nvme/nvme.h
> +++ b/hw/nvme/nvme.h
> @@ -96,36 +96,13 @@ static inline uint32_t nvme_nsid(NvmeNamespace *ns)
> return -1;
> }
>
> -static inline bool nvme_ns_shared(NvmeNamespace *ns)
> -{
> - return !!ns->subsys;
> -}
Re-raising this up.
Something like this helper is related to the data structure usages
itself like, if ns->subsys is not NULL, it "can" mean that this
namespace is shared among controllers. This helper represents that the
'subsys' member itself is indicating some meaning, not only just for the
subsystem itself.
That's why I've been hesitating to simply ack to this patch ;)
But, I am not strongly against to this so please make a decision with
Keith and go ahead!
Thanks!
> static inline uint8_t nvme_ns_lbads(NvmeNamespace *ns)
> {
> - return nvme_ns_lbaf(ns)->ds;
> -}
> + NvmeLBAF lbaf = ns->id_ns.lbaf[NVME_ID_NS_FLBAS_INDEX(ns->id_ns.flbas)];
>
> -/* calculate the number of LBAs that the namespace can accomodate */
> -static inline uint64_t nvme_ns_nlbas(NvmeNamespace *ns)
> -{
> - return ns->size >> nvme_ns_lbads(ns);
> + return lbaf.ds;
> }
>
> -/* convert an LBA to the equivalent in bytes */
> -static inline size_t nvme_l2b(NvmeNamespace *ns, uint64_t lba)
> -{
> - return lba << nvme_ns_lbads(ns);
> -}
> -
> -typedef struct NvmeCtrl NvmeCtrl;
> -
> static inline NvmeZoneState nvme_get_zone_state(NvmeZone *zone)
> {
> return zone->d.zs >> 4;
> @@ -136,31 +113,6 @@ static inline void nvme_set_zone_state(NvmeZone *zone,
> NvmeZoneState state)
> zone->d.zs = state << 4;
> }
>
> -static inline uint64_t nvme_zone_rd_boundary(NvmeNamespace *ns, NvmeZone
> *zone)
> -{
> - return zone->d.zslba + ns->zone_size;
> -}
> -
> -static inline uint64_t nvme_zone_wr_boundary(NvmeZone *zone)
> -{
> - return zone->d.zslba + zone->d.zcap;
> -}
> -
> -static inline bool nvme_wp_is_valid(NvmeZone *zone)
> -{
> - uint8_t st = nvme_get_zone_state(zone);
> -
> - return st != NVME_ZONE_STATE_FULL &&
> - st != NVME_ZONE_STATE_READ_ONLY &&
> - st != NVME_ZONE_STATE_OFFLINE;
> -}
> -
> -static inline uint8_t *nvme_get_zd_extension(NvmeNamespace *ns,
> - uint32_t zone_idx)
> -{
> - return &ns->zd_extensions[zone_idx * ns->params.zd_extension_size];
> -}
> -
> static inline void nvme_aor_inc_open(NvmeNamespace *ns)
> {
> assert(ns->nr_open_zones >= 0);
> @@ -203,7 +155,6 @@ void nvme_ns_drain(NvmeNamespace *ns);
> void nvme_ns_shutdown(NvmeNamespace *ns);
> void nvme_ns_cleanup(NvmeNamespace *ns);
>
> -
> typedef struct NvmeParams {
> char *serial;
> uint32_t num_queues; /* deprecated since 5.1 */
> @@ -237,40 +188,6 @@ typedef struct NvmeRequest {
> QTAILQ_ENTRY(NvmeRequest)entry;
> } NvmeRequest;
>
> -static inline const char *nvme_adm_opc_str(uint8_t opc)
> -{
> - switch (opc) {
> - case NVME_ADM_CMD_DELETE_SQ: return "NVME_ADM_CMD_DELETE_SQ";
> - case NVME_ADM_CMD_CREATE_SQ: return "NVME_ADM_CMD_CREATE_SQ";
> - case NVME_ADM_CMD_GET_LOG_PAGE: return "NVME_ADM_CMD_GET_LOG_PAGE";
> - case NVME_ADM_CMD_DELETE_CQ: return "NVME_ADM_CMD_DELETE_CQ";
> - case NVME_ADM_CMD_CREATE_CQ: return "NVME_ADM_CMD_CREATE_CQ";
> - case NVME_ADM_CMD_IDENTIFY: return "NVME_ADM_CMD_IDENTIFY";
> - case NVME_ADM_CMD_ABORT: return "NVME_ADM_CMD_ABORT";
> - case NVME_ADM_CMD_SET_FEATURES: return "NVME_ADM_CMD_SET_FEATURES";
> - case NVME_ADM_CMD_GET_FEATURES: return "NVME_ADM_CMD_GET_FEATURES";
> - case NVME_ADM_CMD_ASYNC_EV_REQ: return "NVME_ADM_CMD_ASYNC_EV_REQ";
> - default: return "NVME_ADM_CMD_UNKNOWN";
> - }
> -}
> -
> -static inline const char *nvme_io_opc_str(uint8_t opc)
> -{
> - switch (opc) {
> - case NVME_CMD_FLUSH: return "NVME_NVM_CMD_FLUSH";
> - case NVME_CMD_WRITE: return "NVME_NVM_CMD_WRITE";
> - case NVME_CMD_READ: return "NVME_NVM_CMD_READ";
> - case NVME_CMD_COMPARE: return "NVME_NVM_CMD_COMPARE";
> - case NVME_CMD_WRITE_ZEROES: return "NVME_NVM_CMD_WRITE_ZEROES";
> - case NVME_CMD_DSM: return "NVME_NVM_CMD_DSM";
> - case NVME_CMD_COPY: return "NVME_NVM_CMD_COPY";
> - case NVME_CMD_ZONE_MGMT_SEND: return "NVME_ZONED_CMD_MGMT_SEND";
> - case NVME_CMD_ZONE_MGMT_RECV: return "NVME_ZONED_CMD_MGMT_RECV";
> - case NVME_CMD_ZONE_APPEND: return "NVME_ZONED_CMD_ZONE_APPEND";
> - default: return "NVME_NVM_CMD_UNKNOWN";
> - }
> -}
> -
> typedef struct NvmeSQueue {
> struct NvmeCtrl *ctrl;
> uint16_t sqid;
> @@ -379,29 +296,6 @@ typedef struct NvmeCtrl {
> NvmeFeatureVal features;
> } NvmeCtrl;
>
> -static inline NvmeNamespace *nvme_ns(NvmeCtrl *n, uint32_t nsid)
> -{
> - if (!nsid || nsid > n->num_namespaces) {
> - return NULL;
> - }
> -
> - return n->namespaces[nsid - 1];
> -}
> -
> -static inline NvmeCQueue *nvme_cq(NvmeRequest *req)
> -{
> - NvmeSQueue *sq = req->sq;
> - NvmeCtrl *n = sq->ctrl;
> -
> - return n->cq[sq->cqid];
> -}
> -
> -static inline NvmeCtrl *nvme_ctrl(NvmeRequest *req)
> -{
> - NvmeSQueue *sq = req->sq;
> - return sq->ctrl;
> -}
> -
> int nvme_register_namespace(NvmeCtrl *n, NvmeNamespace *ns, Error **errp);
>
> #endif /* HW_NVME_H */
> diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
> index 262c20c1cba7..c245339be9da 100644
> --- a/hw/nvme/ctrl.c
> +++ b/hw/nvme/ctrl.c
> @@ -204,6 +204,40 @@ static const uint32_t nvme_cse_iocs_zoned[256] = {
>
> static void nvme_process_sq(void *opaque);
>
> +static inline const char *nvme_adm_opc_str(uint8_t opc)
> +{
> + switch (opc) {
> + case NVME_ADM_CMD_DELETE_SQ: return "NVME_ADM_CMD_DELETE_SQ";
> + case NVME_ADM_CMD_CREATE_SQ: return "NVME_ADM_CMD_CREATE_SQ";
> + case NVME_ADM_CMD_GET_LOG_PAGE: return "NVME_ADM_CMD_GET_LOG_PAGE";
> + case NVME_ADM_CMD_DELETE_CQ: return "NVME_ADM_CMD_DELETE_CQ";
> + case NVME_ADM_CMD_CREATE_CQ: return "NVME_ADM_CMD_CREATE_CQ";
> + case NVME_ADM_CMD_IDENTIFY: return "NVME_ADM_CMD_IDENTIFY";
> + case NVME_ADM_CMD_ABORT: return "NVME_ADM_CMD_ABORT";
> + case NVME_ADM_CMD_SET_FEATURES: return "NVME_ADM_CMD_SET_FEATURES";
> + case NVME_ADM_CMD_GET_FEATURES: return "NVME_ADM_CMD_GET_FEATURES";
> + case NVME_ADM_CMD_ASYNC_EV_REQ: return "NVME_ADM_CMD_ASYNC_EV_REQ";
> + default: return "NVME_ADM_CMD_UNKNOWN";
> + }
> +}
> +
> +static inline const char *nvme_io_opc_str(uint8_t opc)
> +{
> + switch (opc) {
> + case NVME_CMD_FLUSH: return "NVME_NVM_CMD_FLUSH";
> + case NVME_CMD_WRITE: return "NVME_NVM_CMD_WRITE";
> + case NVME_CMD_READ: return "NVME_NVM_CMD_READ";
> + case NVME_CMD_COMPARE: return "NVME_NVM_CMD_COMPARE";
> + case NVME_CMD_WRITE_ZEROES: return "NVME_NVM_CMD_WRITE_ZEROES";
> + case NVME_CMD_DSM: return "NVME_NVM_CMD_DSM";
> + case NVME_CMD_COPY: return "NVME_NVM_CMD_COPY";
> + case NVME_CMD_ZONE_MGMT_SEND: return "NVME_ZONED_CMD_MGMT_SEND";
> + case NVME_CMD_ZONE_MGMT_RECV: return "NVME_ZONED_CMD_MGMT_RECV";
> + case NVME_CMD_ZONE_APPEND: return "NVME_ZONED_CMD_ZONE_APPEND";
> + default: return "NVME_NVM_CMD_UNKNOWN";
> + }
> +}
> +
> static uint16_t nvme_cid(NvmeRequest *req)
> {
> if (!req) {
> @@ -213,11 +247,65 @@ static uint16_t nvme_cid(NvmeRequest *req)
> return le16_to_cpu(req->cqe.cid);
> }
>
> +static inline NvmeCQueue *nvme_cq(NvmeRequest *req)
> +{
> + NvmeSQueue *sq = req->sq;
> + NvmeCtrl *n = sq->ctrl;
> +
> + return n->cq[sq->cqid];
> +}
> +
> +static inline NvmeCtrl *nvme_ctrl(NvmeRequest *req)
> +{
> + NvmeSQueue *sq = req->sq;
> + return sq->ctrl;
> +}
> +
> static uint16_t nvme_sqid(NvmeRequest *req)
> {
> return le16_to_cpu(req->sq->sqid);
> }
>
> +static inline NvmeNamespace *nvme_ns(NvmeCtrl *n, uint32_t nsid)
> +{
> + if (!nsid || nsid > n->num_namespaces) {
> + return NULL;
> + }
> +
> + return n->namespaces[nsid - 1];
> +}
> +
> +/* convert an LBA to the equivalent in bytes */
> +static inline size_t nvme_l2b(NvmeNamespace *ns, uint64_t lba)
> +{
> + return lba << nvme_ns_lbads(ns);
> +}
> +
> +static inline uint64_t nvme_zone_rd_boundary(NvmeNamespace *ns, NvmeZone
> *zone)
> +{
> + return zone->d.zslba + ns->zone_size;
> +}
> +
> +static inline uint64_t nvme_zone_wr_boundary(NvmeZone *zone)
> +{
> + return zone->d.zslba + zone->d.zcap;
> +}
> +
> +static inline bool nvme_wp_is_valid(NvmeZone *zone)
> +{
> + uint8_t st = nvme_get_zone_state(zone);
> +
> + return st != NVME_ZONE_STATE_FULL &&
> + st != NVME_ZONE_STATE_READ_ONLY &&
> + st != NVME_ZONE_STATE_OFFLINE;
> +}
> +
> +static inline uint8_t *nvme_get_zd_extension(NvmeNamespace *ns,
> + uint32_t zone_idx)
> +{
> + return &ns->zd_extensions[zone_idx * ns->params.zd_extension_size];
> +}
> +
> static void nvme_assign_zone_state(NvmeNamespace *ns, NvmeZone *zone,
> NvmeZoneState state)
> {
> @@ -2487,7 +2575,7 @@ static uint16_t nvme_zone_mgmt_recv(NvmeCtrl *n,
> NvmeRequest *req)
> uint32_t zone_idx, zra, zrasf, partial;
> uint64_t max_zones, nr_zones = 0;
> uint16_t status;
> - uint64_t slba, capacity = nvme_ns_nlbas(ns);
> + uint64_t slba, capacity = le64_to_cpu(ns->id_ns.nsze);
> NvmeZoneDescr *z;
> NvmeZone *zone;
> NvmeZoneReportHeader *header;
> diff --git a/hw/nvme/ns.c b/hw/nvme/ns.c
> index a7d55d71d9de..d0810523172c 100644
> --- a/hw/nvme/ns.c
> +++ b/hw/nvme/ns.c
> @@ -31,6 +31,11 @@
>
> #define MIN_DISCARD_GRANULARITY (4 * KiB)
>
> +static inline bool nvme_ns_shared(NvmeNamespace *ns)
> +{
> + return !!ns->subsys;
> +}
> +
> static int nvme_ns_init(NvmeNamespace *ns, Error **errp)
> {
> BlockDriverInfo bdi;
> @@ -42,7 +47,7 @@ static int nvme_ns_init(NvmeNamespace *ns, Error **errp)
>
> id_ns->lbaf[lba_index].ds = 31 - clz32(ns->blkconf.logical_block_size);
>
> - id_ns->nsze = cpu_to_le64(nvme_ns_nlbas(ns));
> + id_ns->nsze = cpu_to_le64(ns->size >> nvme_ns_lbads(ns));
>
> ns->csi = NVME_CSI_NVM;
>
> --
> 2.30.0
>
>