Re: [PATCH] nvme-pci: prevent SK Hynix PE8000 from using Write Zeroes command

[Gopal Tiwari <gtiwari@redhat.com>]

Hi Chaitanya, 

No feedback on the patch yet. 

Thanks 
Gopal  

----- Original Message -----
From: "Chaitanya Kulkarni" <Chaitanya.Kulkarni@wdc.com>
To: "Christoph Hellwig" <hch@lst.de>
Cc: hch@infradead.org, kbusch@kernel.org, "Gopal Tiwari" <gtiwari@redhat.com>, linux-nvme@lists.infradead.org
Sent: Thursday, February 25, 2021 7:39:33 AM
Subject: Re: [PATCH] nvme-pci: prevent SK Hynix PE8000 from using Write Zeroes command

On 2/24/21 01:13, Christoph Hellwig wrote:
> On Thu, Feb 11, 2021 at 06:11:19PM +0000, Chaitanya Kulkarni wrote:
>> On 2/10/21 11:07 PM, Christoph Hellwig wrote:
>>>>> I'd also love to retest most Write Zeroes quirks with that in place.
>>>> Do you prefer some variant of the following patch (totally untested)? OR
>>>> something else ?
>>> Somwhat.  As said I suspect defaulting to MDTS with a big fat comment
>>> might make most sense unless the new WZSL is set.
>>>
>> Okay, will send out patche(s) soon.
> Did you get to this?  Or did I just miss it?
>
I sent out WIP to Gopal on 02/13 due to lack of drive.

Not sure if makes sense to start the review where patch is
not tested on the real H/w, here it is :-

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index e68a8c4ac5a6..62c928fb0ded 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -1965,6 +1965,7 @@ static void nvme_config_discard(struct gendisk
*disk, struct nvme_ns *ns)
 
 static void nvme_config_write_zeroes(struct gendisk *disk, struct
nvme_ns *ns)
 {
+       bool use_mdts = ns->ctrl->quirks & NVME_QUIRK_USE_MDTS_WRITE_ZEROES;
        u64 max_blocks;
 
        if (!(ns->ctrl->oncs & NVME_CTRL_ONCS_WRITE_ZEROES) ||
@@ -1980,7 +1981,9 @@ static void nvme_config_write_zeroes(struct
gendisk *disk, struct nvme_ns *ns)
         * configured based on the controller's MDTS field in the
         * nvme_init_identify() if available.
         */
-       if (ns->ctrl->max_hw_sectors == UINT_MAX)
+       if (use_mdts && ns->ctrl->max_mdts_hw_sectors)
+               max_blocks = ns->ctrl->max_mdts_hw_sectors + 1;
+       else if (ns->ctrl->max_hw_sectors == UINT_MAX)
                max_blocks = (u64)USHRT_MAX + 1;
        else
                max_blocks = ns->ctrl->max_hw_sectors + 1;
@@ -3136,10 +3139,10 @@ int nvme_init_identify(struct nvme_ctrl *ctrl)
 
        atomic_set(&ctrl->abort_limit, id->acl + 1);
        ctrl->vwc = id->vwc;
+       ctrl->max_mdts_hw_sectors = 0;
+       max_hw_sectors = UINT_MAX;
        if (id->mdts)
                max_hw_sectors = 1 << (id->mdts + page_shift - 9);
-       else
-               max_hw_sectors = UINT_MAX;
        ctrl->max_hw_sectors =
                min_not_zero(ctrl->max_hw_sectors, max_hw_sectors);
 
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index 07b34175c6ce..906b26b3d306 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -150,6 +150,12 @@ enum nvme_quirks {
         * 48 bits.
         */
        NVME_QUIRK_DMA_ADDRESS_BITS_48          = (1 << 16),
+
+       /*
+        * The controller doesn't allow the write zeores command of size
+        * more than reported by the MDTS
+        */
+       NVME_QUIRK_USE_MDTS_WRITE_ZEROES        = (1 << 17)
 };
 
 /*
@@ -274,6 +280,7 @@ struct nvme_ctrl {
 
        u64 cap;
        u32 max_hw_sectors;
+       u32 max_mdts_hw_sectors;
        u32 max_segments;
        u32 max_integrity_segments;
 #ifdef CONFIG_BLK_DEV_ZONED
diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index 207137a0ed8e..f9f36d26828b 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -3268,6 +3268,8 @@ static const struct pci_device_id nvme_id_table[] = {
                .driver_data = NVME_QUIRK_DISABLE_WRITE_ZEROES, },
        { PCI_DEVICE(0x1d97, 0x2263),   /* SPCC */
                .driver_data = NVME_QUIRK_DISABLE_WRITE_ZEROES, },
+       { PCI_DEVICE(0x1c5c, 0x2839),  /* SK Hynix PE8000 U.3 NVMe
storage */
+               .driver_data = NVME_QUIRK_USE_MDTS_WRITE_ZEROES, },
        { PCI_DEVICE(0x2646, 0x2262),   /* KINGSTON SKC2000 NVMe SSD */
                .driver_data = NVME_QUIRK_NO_DEEPEST_PS, },
        { PCI_DEVICE(0x2646, 0x2263),   /* KINGSTON A2000 NVMe SSD  */
diff --git a/drivers/nvme/target/loop.c b/drivers/nvme/target/loop.c
index cb6f86572b24..38fffee6b85e 100644
--- a/drivers/nvme/target/loop.c
+++ b/drivers/nvme/target/loop.c
@@ -688,7 +688,8 @@ static struct nvmf_transport_ops nvme_loop_transport = {
        .name           = "loop",
        .module         = THIS_MODULE,
        .create_ctrl    = nvme_loop_create_ctrl,
-       .allowed_opts   = NVMF_OPT_TRADDR,
+       .allowed_opts   = NVMF_OPT_TRADDR | NVMF_OPT_CTRL_LOSS_TMO,
+
 };
 
 static int __init nvme_loop_init_module(void)


_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme


_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme