nvmet: add ns write protect feature
Subsystems (2)
v3 cover nvmet: add ns write protect feature
Hi,
This patch series implements the support for NVMeOF target ns write protect get/set-features command. In this implementation we support Write Protect and No Write Protect state for the target ns which can be toggled/read by set/get-features commands.
Regards,
Chaitanya
Changes since V2:-
1. Fix the offset numbering.
2. Use short-cut in nvmet_ns_wp_cmd_allow() for admin-cmds.
3. Get rid of conditional operators in nvmet_write_protect_flush_sync().
4. Use the original status from the file/bdev flush call.
5. Use simplified set and unset write-protect switch with subsus->mutex_lock synchronization.
6. Add helper to handle get-features for write-protect.
7. Don't initialize ns->readonly in nvmet_ns_alloc().
8. Use ! instead of false for nvmet_ns_wp_cmd_allow().
9. Get rid of blk_status_t in nvmet_bdev_flush().
10. Adjust the patch for latest ANA related code changes.
Changes since V1:-
1. Incorporate Christoph's comments.
2. Use feature not changeable error code in case of invalid state transition.
3. Split the patche into 3 patches.
4. Set the gendisk associated with the nvme namespace into readonly mode if identify namespace nsattr attribute indicates namesapce is write protected.
Chaitanya Kulkarni (3):
nvme: add support for ns write protect definitions
nvme-core: set gendisk read only based on nsattr
nvmet: add ns write protect support
drivers/nvme/host/core.c | 6 ++ drivers/nvme/target/admin-cmd.c | 97 +++++++++++++++++++++++++++++++ drivers/nvme/target/core.c | 5 +- drivers/nvme/target/io-cmd-bdev.c | 9 +++ drivers/nvme/target/io-cmd-file.c | 15 ++++- drivers/nvme/target/nvmet.h | 6 ++ include/linux/nvme.h | 17 +++++- 7 files changed, 149 insertions(+), 6 deletions(-)
Signature · 1 line
2.17.0
PATCH v3 1/3 nvme: add support for ns write protect definitions
Add various definitions from NVMe 1.3 TP 4005.
--- include/linux/nvme.h | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-)
include/linux/nvme.h +15 -2
@@ -259,7 +259,7 @@ struct nvme_id_ctrl {__le16 awun;__le16 awupf;__u8 nvscc; -__u8 rsvd531; +__u8 nwpc;__le16 acwu;__u8 rsvd534[2 ];__le32 sgls;
@@ -320,7 +320,9 @@ struct nvme_id_ns {__u8 nvmcap[16 ];__u8 rsvd64[28 ];__le32 anagrpid; -__u8 rsvd96[8 ]; +__u8 rsvd96[3 ]; +__u8 nsattr; +__u8 rsvd100[4 ];__u8 nguid[16 ];__u8 eui64[8 ];struct nvme_lbaf lbaf[16 ];
@@ -794,6 +796,7 @@ enum {NVME_FEAT_HOST_ID =0x81 ,NVME_FEAT_RESV_MASK =0x82 ,NVME_FEAT_RESV_PERSIST =0x83 , +NVME_FEAT_WRITE_PROTECT =0x84 ,NVME_LOG_ERROR =0x01 ,NVME_LOG_SMART =0x02 , NVME_LOG_FW_SLOT = 0x03,
@@ -807,6 +810,14 @@ enum { NVME_FWACT_ACTV = (2 << 3), }; +/* NVMe Namespace Write Protect State */ +enum { +NVME_NS_NO_WRITE_PROTECT =0 , +NVME_NS_WRITE_PROTECT , +NVME_NS_WRITE_PROTECT_POWER_CYCLE , +NVME_NS_WRITE_PROTECT_PERMANENT , +}; +#define NVME_MAX_CHANGED_NAMESPACES 1024struct nvme_identify {
@@ -1153,6 +1164,8 @@ enum {NVME_SC_SGL_INVALID_OFFSET =0x16 ,NVME_SC_SGL_INVALID_SUBTYPE =0x17 , +NVME_SC_NS_WRITE_PROTECTED =0x20 , +NVME_SC_LBA_RANGE =0x80 ,NVME_SC_CAP_EXCEEDED =0x81 , NVME_SC_NS_NOT_READY = 0x82,
Signature · 1 line
2.17.0
PATCH v3 1/3 nvme: add support for ns write protect definitions
Quoted 3 lines · Chaitanya Kulkarni
Add various definitions from NVMe 1.3 TP 4005.
Signed-off-by Chaitanya Kulkarni <chaitanya.kulkarni at wdc.com>
Looks good,
PATCH v3 2/3 nvme-core: set gendisk read only based on nsattr
In reply to Chaitanya KulkarniNVMe 1.3 TP 4005 introduces new filed (NSATTR). This field indicates whether given namespace is write protected or not. This patch sets the gendisk associated with the namespace to read only based on the identify namespace nsattr field structure to set the.
--- drivers/nvme/host/core.c | 6 ++++++ 1 file changed, 6 insertions(+)
drivers/nvme/host/core.c +6 -0
@@ -1452,6 +1452,12 @@ static void nvme_update_disk_info(struct gendisk *disk,set_capacity (disk, capacity);nvme_config_discard (ns); + +if (id->nsattr & (1 <<0 )) +set_disk_ro (disk, true); +else +set_disk_ro (disk, false); +blk_mq_unfreeze_queue (disk->queue); }
Signature · 1 line
2.17.0
PATCH v3 3/3 nvmet: add ns write protect support
In reply to Chaitanya KulkarniThis patch implements the Namespace Write Protect feature described in "NVMe TP 4005a Namespace Write Protect". In this version, we implement No Write Protect and Write Protect states for target ns which can be toggled by set-features commands from the host side.
For write-protect state transition, we need to flush the ns specified as a part of command so we also add helpers for carrying out synchronous flush operations.
--- drivers/nvme/target/admin-cmd.c | 97 +++++++++++++++++++++++++++++++ drivers/nvme/target/core.c | 5 +- drivers/nvme/target/io-cmd-bdev.c | 9 +++ drivers/nvme/target/io-cmd-file.c | 15 ++++- drivers/nvme/target/nvmet.h | 6 ++ 5 files changed, 128 insertions(+), 4 deletions(-)
drivers/nvme/target/admin-cmd.c +97 -0
@@ -32,6 +32,26 @@ static inline void nvmet_clear_aen(struct nvmet_req *req, u32 aen_bit)clear_bit (aen_bit, &req->sq->ctrl->aen_masked); } +bool nvmet_ns_wp_cmd_allow (struct nvmet_req *req) +{ +bool ret = false; + +if (likely (req->sq->qid !=0 )) { +switch (req->cmd->common.opcode) { +case nvme_cmd_read: +case nvme_cmd_flush: +/* fall thru */ + ret = true; + } +return ret; + } +if (le32_to_cpu (req->cmd->common.nsid) ==0 || +le32_to_cpu (req->cmd->common.nsid) ==NVME_NSID_ALL ) + ret = true; + +return ret; +} +u32 nvmet_get_log_page_len (struct nvme_command *cmd) {u32 len =le16_to_cpu (cmd->get_log_page.numdu);
@@ -372,6 +392,8 @@ static void nvmet_execute_identify_ctrl(struct nvmet_req *req) id->psd[0 ].entry_lat =cpu_to_le32 (0x10 ); id->psd[0 ].exit_lat =cpu_to_le32 (0x4 ); + id->nwpc =1 <<0 ;/* write protect and no write protect */ + status =nvmet_copy_to_sgl (req,0 , id,sizeof (*id));kfree (id);
@@ -433,6 +455,8 @@ static void nvmet_execute_identify_ns(struct nvmet_req *req) id->lbaf[0 ].ds = ns->blksize_shift; +if (ns->readonly) + id->nsattr |= (1 <<0 );nvmet_put_namespace (ns); done: status =nvmet_copy_to_sgl (req,0 , id,sizeof (*id));
@@ -545,6 +569,52 @@ static void nvmet_execute_abort(struct nvmet_req *req)nvmet_req_complete (req,0 ); } +static u16 nvmet_write_protect_flush_sync (struct nvmet_req *req) +{ +u16 status; + +if (req->ns->file) + status =nvmet_file_flush (req); +else + status =nvmet_bdev_flush (req); + +if (status) +pr_err ("write protect flush failed nsid: %u\n" , req->ns->nsid); +return status; +} + +static u16 nvmet_set_feat_write_protect (struct nvmet_req *req) +{ +u32 write_protect =le32_to_cpu (req->cmd->common.cdw10[1 ]); +struct nvmet_subsys *subsys = req->sq->ctrl->subsys; +u16 status =NVME_SC_FEATURE_NOT_CHANGEABLE ; + + req->ns =nvmet_find_namespace (req->sq->ctrl, req->cmd->rw.nsid); +if (unlikely (!req->ns)) +return status; + +mutex_lock (&subsys->lock); +switch (write_protect) { +case NVME_NS_WRITE_PROTECT : + req->ns->readonly = true; + status =nvmet_write_protect_flush_sync (req); +if (status) + req->ns->readonly = false; +break ; +case NVME_NS_NO_WRITE_PROTECT : + req->ns->readonly = false; + status =NVME_SC_SUCCESS ; +break ; +default : +break ; + } + +if (!status) +nvmet_ns_changed (subsys, req->ns->nsid); +mutex_unlock (&subsys->lock); +return status; +} +static void nvmet_execute_set_features (struct nvmet_req *req) {struct nvmet_subsys *subsys = req->sq->ctrl->subsys;
@@ -575,6 +645,9 @@ static void nvmet_execute_set_features(struct nvmet_req *req)case NVME_FEAT_HOST_ID : status =NVME_SC_CMD_SEQ_ERROR |NVME_SC_DNR ;break ; +case NVME_FEAT_WRITE_PROTECT : + status =nvmet_set_feat_write_protect (req); +break ;default : status =NVME_SC_INVALID_FIELD |NVME_SC_DNR ;break ;
@@ -583,6 +656,27 @@ static void nvmet_execute_set_features(struct nvmet_req *req)nvmet_req_complete (req, status); } +static u16 nvmet_get_feat_write_protect (struct nvmet_req *req) +{ +struct nvmet_subsys *subsys = req->sq->ctrl->subsys; +u32 nsid =le32_to_cpu (req->cmd->common.nsid); +u32 result; + + req->ns =nvmet_find_namespace (req->sq->ctrl, nsid); +if (!req->ns) +return NVME_SC_INVALID_NS |NVME_SC_DNR ; + +mutex_lock (&subsys->lock); +if (req->ns->readonly == true) + result =NVME_NS_WRITE_PROTECT ; +else + result =NVME_NS_NO_WRITE_PROTECT ; +nvmet_set_result (req, result); +mutex_unlock (&subsys->lock); + +return NVME_SC_SUCCESS ; +} +static void nvmet_execute_get_features (struct nvmet_req *req) {struct nvmet_subsys *subsys = req->sq->ctrl->subsys;
@@ -634,6 +728,9 @@ static void nvmet_execute_get_features(struct nvmet_req *req) status =nvmet_copy_to_sgl (req,0 , &req->sq->ctrl->hostid,sizeof (req->sq->ctrl->hostid));break ; +case NVME_FEAT_WRITE_PROTECT : + status =nvmet_get_feat_write_protect (req); +break ;default : status =NVME_SC_INVALID_FIELD |NVME_SC_DNR ;break ;
drivers/nvme/target/core.c +4 -1
@@ -180,7 +180,7 @@ static void nvmet_add_to_changed_ns_log(struct nvmet_ctrl *ctrl, __le32 nsid)mutex_unlock (&ctrl->lock); } -static void nvmet_ns_changed (struct nvmet_subsys *subsys,u32 nsid) +void nvmet_ns_changed (struct nvmet_subsys *subsys,u32 nsid) {struct nvmet_ctrl *ctrl;
@@ -625,6 +625,9 @@ static u16 nvmet_parse_io_cmd(struct nvmet_req *req)if (unlikely (ret))return ret; +if (unlikely (req->ns->readonly && !nvmet_ns_wp_cmd_allow (req))) +return NVME_SC_NS_WRITE_PROTECTED ; +if (req->ns->file)return nvmet_file_parse_io_cmd (req);else
drivers/nvme/target/io-cmd-bdev.c +9 -0
@@ -124,6 +124,15 @@ static void nvmet_bdev_execute_flush(struct nvmet_req *req)submit_bio (bio); } +u16 nvmet_bdev_flush (struct nvmet_req *req) +{ +int ret; + + ret =blkdev_issue_flush (req->ns->bdev,GFP_KERNEL ,NULL ); + +return ret ?NVME_SC_INTERNAL |NVME_SC_DNR :0 ; +} +static u16 nvmet_bdev_discard_range (struct nvmet_ns *ns,struct nvme_dsm_range *range,struct bio **bio) {
drivers/nvme/target/io-cmd-file.c +12 -3
@@ -211,14 +211,23 @@ static void nvmet_file_execute_rw_buffered_io(struct nvmet_req *req)queue_work (buffered_io_wq, &req->f.work); } -static void nvmet_file_flush_work (struct work_struct *w) +u16 nvmet_file_flush (struct nvmet_req *req) { - struct nvmet_req *req = container_of(w, struct nvmet_req, f.work);int ret; ret =vfs_fsync (req->ns->file,1 ); -nvmet_req_complete (req, ret <0 ?NVME_SC_INTERNAL |NVME_SC_DNR :0 ); +return ret <0 ?NVME_SC_INTERNAL |NVME_SC_DNR :0 ; +} + +static void nvmet_file_flush_work (struct work_struct *w) +{ + struct nvmet_req *req = container_of(w, struct nvmet_req, f.work); +u16 status; + + status =nvmet_file_flush (req); + +nvmet_req_complete (req, status); }static void nvmet_file_execute_flush (struct nvmet_req *req)
drivers/nvme/target/nvmet.h +6 -0
@@ -58,6 +58,7 @@ struct nvmet_ns {struct percpu_ref ref;struct block_device *bdev;struct file *file; +bool readonly;u32 nsid;u32 blksize_shift;loff_t size;
@@ -429,6 +430,11 @@ int nvmet_bdev_ns_enable(struct nvmet_ns *ns);int nvmet_file_ns_enable (struct nvmet_ns *ns);void nvmet_bdev_ns_disable (struct nvmet_ns *ns);void nvmet_file_ns_disable (struct nvmet_ns *ns); +u16 nvmet_bdev_flush (struct nvmet_req *req); +u16 nvmet_file_flush (struct nvmet_req *req); +int nvmet_ns_is_wp (struct nvmet_ns *ns); +bool nvmet_ns_wp_cmd_allow (struct nvmet_req *req); +void nvmet_ns_changed (struct nvmet_subsys *subsys,u32 nsid);static inline u32 nvmet_rw_len (struct nvmet_req *req) {
Signature · 1 line
2.17.0
PATCH v3 3/3 nvmet: add ns write protect support
Quoted 16 lines
drivers/nvme/target/admin-cmd.c +8
-0@@ -32,6 +32,26 @@ static inline void nvmet_clear_aen(struct nvmet_req *req, u32 aen_bit)clear_bit (aen_bit, &req->sq->ctrl->aen_masked); } +bool nvmet_ns_wp_cmd_allow (struct nvmet_req *req) +{ +bool ret = false; + +if (likely (req->sq->qid !=0 )) { +switch (req->cmd->common.opcode) { +case nvme_cmd_read: +case nvme_cmd_flush:
drivers/nvme/target/admin-cmd.c static inline void nvmet_clear_aen(struct nvmet_req *req, u32 aen_bit) · line 52
Quoted 10 lines
drivers/nvme/target/admin-cmd.c +10
-0+/* fall thru */ + ret = true; + } +return ret; + } +if (le32_to_cpu (req->cmd->common.nsid) ==0 || +le32_to_cpu (req->cmd->common.nsid) ==NVME_NSID_ALL ) + ret = true; + +return ret;
Currently this function is only called from the I/O command path, so the admin part is dead code. I think just split this into two helpers in core.c and admin.c respectively:
if (unlikely(req->ns->readonly && (req->cmd->common.opcode != nvme_cmd_read && req->cmd->common.opcode != nvme_cmd_flush))
functions, one in admin-cmd.c for the admin command:
static inline bool nvmet_io_cmd_check_access(struct nvmet_req *req)
{
if (likely(!req->ns->readonly))
return true;
switch (req->cmd->common.opcode) {
case nvme_cmd_read:
case nvme_cmd_flush:
return true;
default:
return false;
}
}static inline bool nvmet_admin_cmd_check_access(struct nvmet_req *req)
{
if (likely(!req->ns->readonly))
return true;
if (req->cmd->common.nsid == 0 ||
req->cmd->common.nsid == cpu_to_le32(NVME_NSID_ALL))
return true;
return false;
}this also prepares for a later addition of persistent reservations.
drivers/nvme/target/admin-cmd.c static inline void nvmet_clear_aen(struct nvmet_req *req, u32 aen_bit)
Quoted 8 lines
drivers/nvme/target/admin-cmd.c +8
-0+u16 nvmet_bdev_flush (struct nvmet_req *req) +{ +int ret; + + ret =blkdev_issue_flush (req->ns->bdev,GFP_KERNEL ,NULL ); + +return ret ?NVME_SC_INTERNAL |NVME_SC_DNR :0 ; +}
This could be further simplified down to:
if (blkdev_issue_flush(req->ns->bdev, GFP_KERNEL, NULL)) return NVME_SC_INTERNAL | NVME_SC_DNR; return 0;
drivers/nvme/target/admin-cmd.c
Quoted 8 lines
drivers/nvme/target/admin-cmd.c +8
-0+static void nvmet_file_flush_work (struct work_struct *w) +{ + struct nvmet_req *req = container_of(w, struct nvmet_req, f.work); +u16 status; + + status =nvmet_file_flush (req); + +nvmet_req_complete (req, status);
nvmet_req_complete(req, nvmet_file_flush(req));