From: Tokunori Ikegami <ikegami.t@gmail.com>
To: Caleb Sander <csander@purestorage.com>
Cc: linux-nvme@lists.infradead.org
Subject: Re: [PATCH] nvme: convert little endian NVME definitions to native format
Date: Tue, 17 Sep 2024 01:41:07 +0900 [thread overview]
Message-ID: <1bcdab96-f2cb-4116-9f04-4016b01bad36@gmail.com> (raw)
In-Reply-To: <CADUfDZrqAgp49K2Ak-b230ERY8P66aXA4iAoCyyYr45PAbq+1Q@mail.gmail.com>
For example the definition below is not native byte order I think.
NVME_CTRL_ONCS_TIMESTAMP = 1 << 6,
On 2024/09/17 1:12, Caleb Sander wrote:
> Why is this necessary? All these values obtained from the Identify
> Controller data structure are already stored in the CPU's native byte
> order. So testing them against native byte order constants looks
> correct.
>
> ctrl->oacs = le16_to_cpu(id->oacs);
> ctrl->oncs = le16_to_cpu(id->oncs);
> ctrl->mtfa = le16_to_cpu(id->mtfa);
> ctrl->oaes = le32_to_cpu(id->oaes);
> // ...
> ctrl->ctratt = le32_to_cpu(id->ctratt);
>
> On Mon, Sep 16, 2024 at 8:46 AM Tokunori Ikegami <ikegami.t@gmail.com> wrote:
>> This is to compare the definitions correctly with the converted values.
>>
>> Signed-off-by: Tokunori Ikegami <ikegami.t@gmail.com>
>> ---
>> drivers/nvme/host/core.c | 20 ++++++++++----------
>> drivers/nvme/host/nvme.h | 4 ++--
>> drivers/nvme/host/pci.c | 2 +-
>> 3 files changed, 13 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
>> index ca9959a8fb9e..5e26546e811a 100644
>> --- a/drivers/nvme/host/core.c
>> +++ b/drivers/nvme/host/core.c
>> @@ -1273,7 +1273,7 @@ static unsigned long nvme_keep_alive_work_period(struct nvme_ctrl *ctrl)
>> * command completion can postpone sending a keep alive command
>> * by up to twice the delay between runs.
>> */
>> - if (ctrl->ctratt & NVME_CTRL_ATTR_TBKAS)
>> + if (ctrl->ctratt & le32_to_cpu(NVME_CTRL_ATTR_TBKAS))
>> delay /= 2;
>> return delay;
>> }
>> @@ -1343,7 +1343,7 @@ static void nvme_keep_alive_work(struct work_struct *work)
>>
>> ctrl->ka_last_check_time = jiffies;
>>
>> - if ((ctrl->ctratt & NVME_CTRL_ATTR_TBKAS) && comp_seen) {
>> + if ((ctrl->ctratt & le32_to_cpu(NVME_CTRL_ATTR_TBKAS)) && comp_seen) {
>> dev_dbg(ctrl->device,
>> "reschedule traffic based keep-alive timer\n");
>> ctrl->comp_seen = false;
>> @@ -1699,7 +1699,7 @@ EXPORT_SYMBOL_GPL(nvme_set_queue_count);
>>
>> static void nvme_enable_aen(struct nvme_ctrl *ctrl)
>> {
>> - u32 result, supported_aens = ctrl->oaes & NVME_AEN_SUPPORTED;
>> + u32 result, supported_aens = ctrl->oaes & le32_to_cpu(NVME_AEN_SUPPORTED);
>> int status;
>>
>> if (!supported_aens)
>> @@ -1829,7 +1829,7 @@ static void nvme_config_discard(struct nvme_ns *ns, struct queue_limits *lim)
>> if (ctrl->dmrsl && ctrl->dmrsl <= nvme_sect_to_lba(ns->head, UINT_MAX))
>> lim->max_hw_discard_sectors =
>> nvme_lba_to_sect(ns->head, ctrl->dmrsl);
>> - else if (ctrl->oncs & NVME_CTRL_ONCS_DSM)
>> + else if (ctrl->oncs & le16_to_cpu(NVME_CTRL_ONCS_DSM))
>> lim->max_hw_discard_sectors = UINT_MAX;
>> else
>> lim->max_hw_discard_sectors = 0;
>> @@ -1913,7 +1913,7 @@ static void nvme_configure_metadata(struct nvme_ctrl *ctrl,
>> if (!head->ms || !(ctrl->ops->flags & NVME_F_METADATA_SUPPORTED))
>> return;
>>
>> - if (nvm && (ctrl->ctratt & NVME_CTRL_ATTR_ELBAS)) {
>> + if (nvm && (ctrl->ctratt & le32_to_cpu(NVME_CTRL_ATTR_ELBAS))) {
>> nvme_configure_pi_elbas(head, id, nvm);
>> } else {
>> head->pi_size = sizeof(struct t10_pi_tuple);
>> @@ -2137,7 +2137,7 @@ static int nvme_update_ns_info_block(struct nvme_ns *ns,
>> }
>> lbaf = nvme_lbaf_index(id->flbas);
>>
>> - if (ns->ctrl->ctratt & NVME_CTRL_ATTR_ELBAS) {
>> + if (ns->ctrl->ctratt & le32_to_cpu(NVME_CTRL_ATTR_ELBAS)) {
>> ret = nvme_identify_ns_nvm(ns->ctrl, info->nsid, &nvm);
>> if (ret < 0)
>> goto out;
>> @@ -2341,7 +2341,7 @@ static int nvme_sec_submit(void *data, u16 spsp, u8 secp, void *buffer, size_t l
>>
>> static void nvme_configure_opal(struct nvme_ctrl *ctrl, bool was_suspended)
>> {
>> - if (ctrl->oacs & NVME_CTRL_OACS_SEC_SUPP) {
>> + if (ctrl->oacs & le16_to_cpu(NVME_CTRL_OACS_SEC_SUPP)) {
>> if (!ctrl->opal_dev)
>> ctrl->opal_dev = init_opal_dev(ctrl, &nvme_sec_submit);
>> else if (was_suspended)
>> @@ -2520,7 +2520,7 @@ static int nvme_configure_timestamp(struct nvme_ctrl *ctrl)
>> __le64 ts;
>> int ret;
>>
>> - if (!(ctrl->oncs & NVME_CTRL_ONCS_TIMESTAMP))
>> + if (!(ctrl->oncs & le16_to_cpu(NVME_CTRL_ONCS_TIMESTAMP)))
>> return 0;
>>
>> ts = cpu_to_le64(ktime_to_ms(ktime_get_real()));
>> @@ -2541,7 +2541,7 @@ static int nvme_configure_host_options(struct nvme_ctrl *ctrl)
>> /* Don't bother enabling the feature if retry delay is not reported */
>> if (ctrl->crdt[0])
>> acre = NVME_ENABLE_ACRE;
>> - if (ctrl->ctratt & NVME_CTRL_ATTR_ELBAS)
>> + if (ctrl->ctratt & le32_to_cpu(NVME_CTRL_ATTR_ELBAS))
>> lbafee = NVME_ENABLE_LBAFEE;
>>
>> if (!acre && !lbafee)
>> @@ -3109,7 +3109,7 @@ static int nvme_init_non_mdts_limits(struct nvme_ctrl *ctrl)
>> * controllers max_hw_sectors value, which is based on the MDTS field
>> * and possibly other limiting factors.
>> */
>> - if ((ctrl->oncs & NVME_CTRL_ONCS_WRITE_ZEROES) &&
>> + if ((ctrl->oncs & le16_to_cpu(NVME_CTRL_ONCS_WRITE_ZEROES)) &&
>> !(ctrl->quirks & NVME_QUIRK_DISABLE_WRITE_ZEROES))
>> ctrl->max_zeroes_sectors = ctrl->max_hw_sectors;
>> else
>> diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
>> index 313a4f978a2c..25886b9d2796 100644
>> --- a/drivers/nvme/host/nvme.h
>> +++ b/drivers/nvme/host/nvme.h
>> @@ -860,9 +860,9 @@ static inline bool nvme_is_unique_nsid(struct nvme_ctrl *ctrl,
>> struct nvme_ns_head *head)
>> {
>> return head->shared ||
>> - (ctrl->oacs & NVME_CTRL_OACS_NS_MNGT_SUPP) ||
>> + (ctrl->oacs & le16_to_cpu(NVME_CTRL_OACS_NS_MNGT_SUPP)) ||
>> (ctrl->subsys->cmic & NVME_CTRL_CMIC_ANA) ||
>> - (ctrl->ctratt & NVME_CTRL_CTRATT_NVM_SETS);
>> + (ctrl->ctratt & le32_to_cpu(NVME_CTRL_CTRATT_NVM_SETS));
>> }
>>
>> /*
>> diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
>> index 7990c3f22ecf..f8fa424fa952 100644
>> --- a/drivers/nvme/host/pci.c
>> +++ b/drivers/nvme/host/pci.c
>> @@ -249,7 +249,7 @@ static void nvme_dbbuf_dma_alloc(struct nvme_dev *dev)
>> {
>> unsigned int mem_size = nvme_dbbuf_size(dev);
>>
>> - if (!(dev->ctrl.oacs & NVME_CTRL_OACS_DBBUF_SUPP))
>> + if (!(dev->ctrl.oacs & le16_to_cpu(NVME_CTRL_OACS_DBBUF_SUPP)))
>> return;
>>
>> if (dev->dbbuf_dbs) {
>> --
>> 2.43.0
>>
>>
next prev parent reply other threads:[~2024-09-16 16:41 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-09-16 15:42 [PATCH] nvme: convert little endian NVME definitions to native format Tokunori Ikegami
2024-09-16 16:12 ` Caleb Sander
2024-09-16 16:41 ` Tokunori Ikegami [this message]
2024-09-16 18:12 ` Caleb Sander
2024-09-16 21:24 ` Tokunori Ikegami
2024-09-17 14:31 ` Tokunori Ikegami
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1bcdab96-f2cb-4116-9f04-4016b01bad36@gmail.com \
--to=ikegami.t@gmail.com \
--cc=csander@purestorage.com \
--cc=linux-nvme@lists.infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox