| Message ID | 20200913221436.22844-3-dmitry.fomichev@wdc.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | hw/block/nvme: Support Namespace Types and Zoned Namespace Command Set | expand |
On Sep 14 07:14, Dmitry Fomichev wrote: > Calculate the data shift value to report based on the set value of > logical_block_size device property. > > In the process, use a local variable to calculate the LBA format > index instead of the hardcoded value 0. This makes the code more > readable and it will make it easier to add support for multiple LBA > formats in the future. > > Signed-off-by: Dmitry Fomichev <dmitry.fomichev@wdc.com> > --- > hw/block/nvme.c | 4 +++- > hw/block/nvme.h | 11 +++++++++++ > 2 files changed, 14 insertions(+), 1 deletion(-) > > diff --git a/hw/block/nvme.c b/hw/block/nvme.c > index 3a90d80694..1cfc136042 100644 > --- a/hw/block/nvme.c > +++ b/hw/block/nvme.c > @@ -2203,6 +2203,7 @@ static void nvme_init_namespace(NvmeCtrl *n, NvmeNamespace *ns, Error **errp) > { > int64_t bs_size; > NvmeIdNs *id_ns = &ns->id_ns; > + int lba_index; > > bs_size = blk_getlength(n->conf.blk); > if (bs_size < 0) { > @@ -2212,7 +2213,8 @@ static void nvme_init_namespace(NvmeCtrl *n, NvmeNamespace *ns, Error **errp) > > n->ns_size = bs_size; > > - id_ns->lbaf[0].ds = BDRV_SECTOR_BITS; > + lba_index = NVME_ID_NS_FLBAS_INDEX(ns->id_ns.flbas); > + id_ns->lbaf[lba_index].ds = nvme_ilog2(n->conf.logical_block_size); Instead of defining a new function, we can directly use clz32(). 31 - clz32(n->conf.logical_block_size)
> -----Original Message----- > From: Klaus Jensen <its@irrelevant.dk> > Sent: Tuesday, September 15, 2020 3:34 AM > To: Dmitry Fomichev <Dmitry.Fomichev@wdc.com> > Cc: Keith Busch <kbusch@kernel.org>; Klaus Jensen > <k.jensen@samsung.com>; Kevin Wolf <kwolf@redhat.com>; Philippe > Mathieu-Daudé <philmd@redhat.com>; Maxim Levitsky > <mlevitsk@redhat.com>; Fam Zheng <fam@euphon.net>; Niklas Cassel > <Niklas.Cassel@wdc.com>; Damien Le Moal <Damien.LeMoal@wdc.com>; > qemu-block@nongnu.org; qemu-devel@nongnu.org; Alistair Francis > <Alistair.Francis@wdc.com>; Matias Bjorling <Matias.Bjorling@wdc.com> > Subject: Re: [PATCH v3 02/15] hw/block/nvme: Report actual LBA data shift in > LBAF > > On Sep 14 07:14, Dmitry Fomichev wrote: > > Calculate the data shift value to report based on the set value of > > logical_block_size device property. > > > > In the process, use a local variable to calculate the LBA format > > index instead of the hardcoded value 0. This makes the code more > > readable and it will make it easier to add support for multiple LBA > > formats in the future. > > > > Signed-off-by: Dmitry Fomichev <dmitry.fomichev@wdc.com> > > --- > > hw/block/nvme.c | 4 +++- > > hw/block/nvme.h | 11 +++++++++++ > > 2 files changed, 14 insertions(+), 1 deletion(-) > > > > diff --git a/hw/block/nvme.c b/hw/block/nvme.c > > index 3a90d80694..1cfc136042 100644 > > --- a/hw/block/nvme.c > > +++ b/hw/block/nvme.c > > @@ -2203,6 +2203,7 @@ static void nvme_init_namespace(NvmeCtrl *n, > NvmeNamespace *ns, Error **errp) > > { > > int64_t bs_size; > > NvmeIdNs *id_ns = &ns->id_ns; > > + int lba_index; > > > > bs_size = blk_getlength(n->conf.blk); > > if (bs_size < 0) { > > @@ -2212,7 +2213,8 @@ static void nvme_init_namespace(NvmeCtrl *n, > NvmeNamespace *ns, Error **errp) > > > > n->ns_size = bs_size; > > > > - id_ns->lbaf[0].ds = BDRV_SECTOR_BITS; > > + lba_index = NVME_ID_NS_FLBAS_INDEX(ns->id_ns.flbas); > > + id_ns->lbaf[lba_index].ds = nvme_ilog2(n->conf.logical_block_size); > > Instead of defining a new function, we can directly use clz32(). > > 31 - clz32(n->conf.logical_block_size) Ok nice! I looked up what QEMU uses for binary log, but couldn't find it quickly so I decided to define a function for that :) Will switch to clzXX in the patch set - I believe there are three occurrences where ilog2 is used in the added code.
diff --git a/hw/block/nvme.c b/hw/block/nvme.c index 3a90d80694..1cfc136042 100644 --- a/hw/block/nvme.c +++ b/hw/block/nvme.c @@ -2203,6 +2203,7 @@ static void nvme_init_namespace(NvmeCtrl *n, NvmeNamespace *ns, Error **errp) { int64_t bs_size; NvmeIdNs *id_ns = &ns->id_ns; + int lba_index; bs_size = blk_getlength(n->conf.blk); if (bs_size < 0) { @@ -2212,7 +2213,8 @@ static void nvme_init_namespace(NvmeCtrl *n, NvmeNamespace *ns, Error **errp) n->ns_size = bs_size; - id_ns->lbaf[0].ds = BDRV_SECTOR_BITS; + lba_index = NVME_ID_NS_FLBAS_INDEX(ns->id_ns.flbas); + id_ns->lbaf[lba_index].ds = nvme_ilog2(n->conf.logical_block_size); id_ns->nsze = cpu_to_le64(nvme_ns_nlbas(n, ns)); /* no thin provisioning */ diff --git a/hw/block/nvme.h b/hw/block/nvme.h index 52ba794f2e..190c974b6c 100644 --- a/hw/block/nvme.h +++ b/hw/block/nvme.h @@ -137,4 +137,15 @@ static inline uint64_t nvme_ns_nlbas(NvmeCtrl *n, NvmeNamespace *ns) return n->ns_size >> nvme_ns_lbads(ns); } +static inline int nvme_ilog2(uint64_t i) +{ + int log = -1; + + while (i) { + i >>= 1; + log++; + } + return log; +} + #endif /* HW_NVME_H */
Calculate the data shift value to report based on the set value of logical_block_size device property. In the process, use a local variable to calculate the LBA format index instead of the hardcoded value 0. This makes the code more readable and it will make it easier to add support for multiple LBA formats in the future. Signed-off-by: Dmitry Fomichev <dmitry.fomichev@wdc.com> --- hw/block/nvme.c | 4 +++- hw/block/nvme.h | 11 +++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-)