Alvaro Karsz
2022-Aug-29 08:23 UTC
[PATCH v2] virtio_blk: add SECURE ERASE command support
Support for the VIRTIO_BLK_F_SECURE_ERASE VirtIO feature. A device that offers this feature can receive VIRTIO_BLK_T_SECURE_ERASE commands. A device which supports this feature has the following fields in the virtio config: - max_secure_erase_sectors - max_secure_erase_seg - secure_erase_sector_alignment max_secure_erase_sectors and secure_erase_sector_alignment are expressed in 512-byte units. Every secure erase command has the following fields: - sectors: The starting offset in 512-byte units. - num_sectors: The number of sectors. Signed-off-by: Alvaro Karsz <alvaro.karsz at solid-run.com> --- v2: - Set queue max discard segments as the minimum between max_secure_erase_seg and max_discard_seg. - Set queue discard granularity as the minimum between secure_erase_sector_alignment and discard_sector_alignment. --- drivers/block/virtio_blk.c | 82 +++++++++++++++++++++++++-------- include/uapi/linux/virtio_blk.h | 19 ++++++++ 2 files changed, 83 insertions(+), 18 deletions(-) diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c index 30255fcaf18..8a2f00cdf52 100644 --- a/drivers/block/virtio_blk.c +++ b/drivers/block/virtio_blk.c @@ -130,7 +130,7 @@ static int virtblk_add_req(struct virtqueue *vq, struct virtblk_req *vbr) return virtqueue_add_sgs(vq, sgs, num_out, num_in, vbr, GFP_ATOMIC); } -static int virtblk_setup_discard_write_zeroes(struct request *req, bool unmap) +static int virtblk_setup_discard_write_zeroes_erase(struct request *req, bool unmap) { unsigned short segments = blk_rq_nr_discard_segments(req); unsigned short n = 0; @@ -240,6 +240,9 @@ static blk_status_t virtblk_setup_cmd(struct virtio_device *vdev, type = VIRTIO_BLK_T_WRITE_ZEROES; unmap = !(req->cmd_flags & REQ_NOUNMAP); break; + case REQ_OP_SECURE_ERASE: + type = VIRTIO_BLK_T_SECURE_ERASE; + break; case REQ_OP_DRV_IN: type = VIRTIO_BLK_T_GET_ID; break; @@ -251,8 +254,9 @@ static blk_status_t virtblk_setup_cmd(struct virtio_device *vdev, vbr->out_hdr.type = cpu_to_virtio32(vdev, type); vbr->out_hdr.ioprio = cpu_to_virtio32(vdev, req_get_ioprio(req)); - if (type == VIRTIO_BLK_T_DISCARD || type == VIRTIO_BLK_T_WRITE_ZEROES) { - if (virtblk_setup_discard_write_zeroes(req, unmap)) + if (type == VIRTIO_BLK_T_DISCARD || type == VIRTIO_BLK_T_WRITE_ZEROES || + type == VIRTIO_BLK_T_SECURE_ERASE) { + if (virtblk_setup_discard_write_zeroes_erase(req, unmap)) return BLK_STS_RESOURCE; } @@ -889,6 +893,8 @@ static int virtblk_probe(struct virtio_device *vdev) int err, index; u32 v, blk_size, max_size, sg_elems, opt_io_size; + u32 max_discard_segs = 0; + u32 discard_granularity = 0; u16 min_io_size; u8 physical_block_exp, alignment_offset; unsigned int queue_depth; @@ -1046,27 +1052,14 @@ static int virtblk_probe(struct virtio_device *vdev) if (virtio_has_feature(vdev, VIRTIO_BLK_F_DISCARD)) { virtio_cread(vdev, struct virtio_blk_config, - discard_sector_alignment, &v); - if (v) - q->limits.discard_granularity = v << SECTOR_SHIFT; - else - q->limits.discard_granularity = blk_size; + discard_sector_alignment, &discard_granularity); virtio_cread(vdev, struct virtio_blk_config, max_discard_sectors, &v); blk_queue_max_discard_sectors(q, v ? v : UINT_MAX); virtio_cread(vdev, struct virtio_blk_config, max_discard_seg, - &v); - - /* - * max_discard_seg == 0 is out of spec but we always - * handled it. - */ - if (!v) - v = sg_elems; - blk_queue_max_discard_segments(q, - min(v, MAX_DISCARD_SEGMENTS)); + &max_discard_segs); } if (virtio_has_feature(vdev, VIRTIO_BLK_F_WRITE_ZEROES)) { @@ -1075,6 +1068,57 @@ static int virtblk_probe(struct virtio_device *vdev) blk_queue_max_write_zeroes_sectors(q, v ? v : UINT_MAX); } + if (virtio_has_feature(vdev, VIRTIO_BLK_F_SECURE_ERASE)) { + /* The discard alignment value should be the minimum between + * secure_erase_sector_alignment and discard_sector_alignment + * (if VIRTIO_BLK_F_DISCARD is negotiated). + */ + virtio_cread(vdev, struct virtio_blk_config, + secure_erase_sector_alignment, &v); + if (v) { + if (discard_granularity) + discard_granularity = min(discard_granularity, v); + else + discard_granularity = v; + } + + virtio_cread(vdev, struct virtio_blk_config, + max_secure_erase_sectors, &v); + blk_queue_max_secure_erase_sectors(q, v ? v : UINT_MAX); + + /* The max discard segments value should be the minimum between + * max_secure_erase_seg and max_discard_seg (if VIRTIO_BLK_F_DISCARD + * is negotiated). + */ + virtio_cread(vdev, struct virtio_blk_config, + max_secure_erase_seg, &v); + if (v) { + if (max_discard_segs) + max_discard_segs = min(max_discard_segs, v); + else + max_discard_segs = v; + } + } + + if (virtio_has_feature(vdev, VIRTIO_BLK_F_DISCARD) || + virtio_has_feature(vdev, VIRTIO_BLK_F_SECURE_ERASE)) { + + /* max_discard_seg == 0 or max_secure_erase_seg == 0 + * are out of spec but we always handled it. + */ + if (!max_discard_segs) + max_discard_segs = sg_elems; + + blk_queue_max_discard_segments(q, + min(max_discard_segs, MAX_DISCARD_SEGMENTS)); + + /* If alignemnt is 0, use block size alignment */ + if (discard_granularity) + q->limits.discard_granularity = discard_granularity << SECTOR_SHIFT; + else + q->limits.discard_granularity = blk_size; + } + virtblk_update_capacity(vblk, false); virtio_device_ready(vdev); @@ -1170,6 +1214,7 @@ static unsigned int features_legacy[] = { VIRTIO_BLK_F_RO, VIRTIO_BLK_F_BLK_SIZE, VIRTIO_BLK_F_FLUSH, VIRTIO_BLK_F_TOPOLOGY, VIRTIO_BLK_F_CONFIG_WCE, VIRTIO_BLK_F_MQ, VIRTIO_BLK_F_DISCARD, VIRTIO_BLK_F_WRITE_ZEROES, + VIRTIO_BLK_F_SECURE_ERASE, } ; static unsigned int features[] = { @@ -1177,6 +1222,7 @@ static unsigned int features[] = { VIRTIO_BLK_F_RO, VIRTIO_BLK_F_BLK_SIZE, VIRTIO_BLK_F_FLUSH, VIRTIO_BLK_F_TOPOLOGY, VIRTIO_BLK_F_CONFIG_WCE, VIRTIO_BLK_F_MQ, VIRTIO_BLK_F_DISCARD, VIRTIO_BLK_F_WRITE_ZEROES, + VIRTIO_BLK_F_SECURE_ERASE, }; static struct virtio_driver virtio_blk = { diff --git a/include/uapi/linux/virtio_blk.h b/include/uapi/linux/virtio_blk.h index d888f013d9f..58e70b24b50 100644 --- a/include/uapi/linux/virtio_blk.h +++ b/include/uapi/linux/virtio_blk.h @@ -40,6 +40,7 @@ #define VIRTIO_BLK_F_MQ 12 /* support more than one vq */ #define VIRTIO_BLK_F_DISCARD 13 /* DISCARD is supported */ #define VIRTIO_BLK_F_WRITE_ZEROES 14 /* WRITE ZEROES is supported */ +#define VIRTIO_BLK_F_SECURE_ERASE 16 /* Secure Erase is supported */ /* Legacy feature bits */ #ifndef VIRTIO_BLK_NO_LEGACY @@ -121,6 +122,21 @@ struct virtio_blk_config { __u8 write_zeroes_may_unmap; __u8 unused1[3]; + + /* the next 3 entries are guarded by VIRTIO_BLK_F_SECURE_ERASE */ + /* + * The maximum secure erase sectors (in 512-byte sectors) for + * one segment. + */ + __virtio32 max_secure_erase_sectors; + /* + * The maximum number of secure erase segments in a + * secure erase command. + */ + __virtio32 max_secure_erase_seg; + /* Secure erase commands must be aligned to this number of sectors. */ + __virtio32 secure_erase_sector_alignment; + } __attribute__((packed)); /* @@ -155,6 +171,9 @@ struct virtio_blk_config { /* Write zeroes command */ #define VIRTIO_BLK_T_WRITE_ZEROES 13 +/* Secure erase command */ +#define VIRTIO_BLK_T_SECURE_ERASE 14 + #ifndef VIRTIO_BLK_NO_LEGACY /* Barrier before this op. */ #define VIRTIO_BLK_T_BARRIER 0x80000000 -- 2.32.0
Alvaro Karsz
2022-Sep-01 07:44 UTC
[PATCH v2] virtio_blk: add SECURE ERASE command support
Is that what you meant Stefan?
Michael S. Tsirkin
2022-Sep-18 13:30 UTC
[PATCH v2] virtio_blk: add SECURE ERASE command support
On Mon, Aug 29, 2022 at 11:23:13AM +0300, Alvaro Karsz wrote:> Support for the VIRTIO_BLK_F_SECURE_ERASE VirtIO feature. > > A device that offers this feature can receive VIRTIO_BLK_T_SECURE_ERASE > commands. > > A device which supports this feature has the following fields in the > virtio config: > > - max_secure_erase_sectors > - max_secure_erase_seg > - secure_erase_sector_alignment > > max_secure_erase_sectors and secure_erase_sector_alignment are expressed > in 512-byte units. > > Every secure erase command has the following fields: > > - sectors: The starting offset in 512-byte units. > - num_sectors: The number of sectors. > > Signed-off-by: Alvaro Karsz <alvaro.karsz at solid-run.com>Review from storage maintainers before I queue this? Thanks!> --- > v2: > - Set queue max discard segments as the minimum between > max_secure_erase_seg and max_discard_seg. > - Set queue discard granularity as the minimum between > secure_erase_sector_alignment and discard_sector_alignment. > --- > drivers/block/virtio_blk.c | 82 +++++++++++++++++++++++++-------- > include/uapi/linux/virtio_blk.h | 19 ++++++++ > 2 files changed, 83 insertions(+), 18 deletions(-) > > diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c > index 30255fcaf18..8a2f00cdf52 100644 > --- a/drivers/block/virtio_blk.c > +++ b/drivers/block/virtio_blk.c > @@ -130,7 +130,7 @@ static int virtblk_add_req(struct virtqueue *vq, struct virtblk_req *vbr) > return virtqueue_add_sgs(vq, sgs, num_out, num_in, vbr, GFP_ATOMIC); > } > > -static int virtblk_setup_discard_write_zeroes(struct request *req, bool unmap) > +static int virtblk_setup_discard_write_zeroes_erase(struct request *req, bool unmap) > { > unsigned short segments = blk_rq_nr_discard_segments(req); > unsigned short n = 0; > @@ -240,6 +240,9 @@ static blk_status_t virtblk_setup_cmd(struct virtio_device *vdev, > type = VIRTIO_BLK_T_WRITE_ZEROES; > unmap = !(req->cmd_flags & REQ_NOUNMAP); > break; > + case REQ_OP_SECURE_ERASE: > + type = VIRTIO_BLK_T_SECURE_ERASE; > + break; > case REQ_OP_DRV_IN: > type = VIRTIO_BLK_T_GET_ID; > break; > @@ -251,8 +254,9 @@ static blk_status_t virtblk_setup_cmd(struct virtio_device *vdev, > vbr->out_hdr.type = cpu_to_virtio32(vdev, type); > vbr->out_hdr.ioprio = cpu_to_virtio32(vdev, req_get_ioprio(req)); > > - if (type == VIRTIO_BLK_T_DISCARD || type == VIRTIO_BLK_T_WRITE_ZEROES) { > - if (virtblk_setup_discard_write_zeroes(req, unmap)) > + if (type == VIRTIO_BLK_T_DISCARD || type == VIRTIO_BLK_T_WRITE_ZEROES || > + type == VIRTIO_BLK_T_SECURE_ERASE) { > + if (virtblk_setup_discard_write_zeroes_erase(req, unmap)) > return BLK_STS_RESOURCE; > } > > @@ -889,6 +893,8 @@ static int virtblk_probe(struct virtio_device *vdev) > int err, index; > > u32 v, blk_size, max_size, sg_elems, opt_io_size; > + u32 max_discard_segs = 0; > + u32 discard_granularity = 0; > u16 min_io_size; > u8 physical_block_exp, alignment_offset; > unsigned int queue_depth; > @@ -1046,27 +1052,14 @@ static int virtblk_probe(struct virtio_device *vdev) > > if (virtio_has_feature(vdev, VIRTIO_BLK_F_DISCARD)) { > virtio_cread(vdev, struct virtio_blk_config, > - discard_sector_alignment, &v); > - if (v) > - q->limits.discard_granularity = v << SECTOR_SHIFT; > - else > - q->limits.discard_granularity = blk_size; > + discard_sector_alignment, &discard_granularity); > > virtio_cread(vdev, struct virtio_blk_config, > max_discard_sectors, &v); > blk_queue_max_discard_sectors(q, v ? v : UINT_MAX); > > virtio_cread(vdev, struct virtio_blk_config, max_discard_seg, > - &v); > - > - /* > - * max_discard_seg == 0 is out of spec but we always > - * handled it. > - */ > - if (!v) > - v = sg_elems; > - blk_queue_max_discard_segments(q, > - min(v, MAX_DISCARD_SEGMENTS)); > + &max_discard_segs); > } > > if (virtio_has_feature(vdev, VIRTIO_BLK_F_WRITE_ZEROES)) { > @@ -1075,6 +1068,57 @@ static int virtblk_probe(struct virtio_device *vdev) > blk_queue_max_write_zeroes_sectors(q, v ? v : UINT_MAX); > } > > + if (virtio_has_feature(vdev, VIRTIO_BLK_F_SECURE_ERASE)) { > + /* The discard alignment value should be the minimum between > + * secure_erase_sector_alignment and discard_sector_alignment > + * (if VIRTIO_BLK_F_DISCARD is negotiated).why minimum?> + */ > + virtio_cread(vdev, struct virtio_blk_config, > + secure_erase_sector_alignment, &v); > + if (v) { > + if (discard_granularity) > + discard_granularity = min(discard_granularity, v); > + else > + discard_granularity = v; > + } > + > + virtio_cread(vdev, struct virtio_blk_config, > + max_secure_erase_sectors, &v); > + blk_queue_max_secure_erase_sectors(q, v ? v : UINT_MAX); > + > + /* The max discard segments value should be the minimum between > + * max_secure_erase_seg and max_discard_seg (if VIRTIO_BLK_F_DISCARD > + * is negotiated).why is that?> + */ > + virtio_cread(vdev, struct virtio_blk_config, > + max_secure_erase_seg, &v); > + if (v) { > + if (max_discard_segs) > + max_discard_segs = min(max_discard_segs, v); > + else > + max_discard_segs = v;is this logic repeating code from below?> + } > + } > + > + if (virtio_has_feature(vdev, VIRTIO_BLK_F_DISCARD) || > + virtio_has_feature(vdev, VIRTIO_BLK_F_SECURE_ERASE)) { > + > + /* max_discard_seg == 0 or max_secure_erase_seg == 0 > + * are out of spec but we always handled it.Always? What's going on here? which versions handled max_secure_erase_seg == 0?> + */ > + if (!max_discard_segs) > + max_discard_segs = sg_elems; > + > + blk_queue_max_discard_segments(q, > + min(max_discard_segs, MAX_DISCARD_SEGMENTS)); > + > + /* If alignemnt is 0, use block size alignment */ > + if (discard_granularity) > + q->limits.discard_granularity = discard_granularity << SECTOR_SHIFT; > + else > + q->limits.discard_granularity = blk_size; > + } > + > virtblk_update_capacity(vblk, false); > virtio_device_ready(vdev); > > @@ -1170,6 +1214,7 @@ static unsigned int features_legacy[] = { > VIRTIO_BLK_F_RO, VIRTIO_BLK_F_BLK_SIZE, > VIRTIO_BLK_F_FLUSH, VIRTIO_BLK_F_TOPOLOGY, VIRTIO_BLK_F_CONFIG_WCE, > VIRTIO_BLK_F_MQ, VIRTIO_BLK_F_DISCARD, VIRTIO_BLK_F_WRITE_ZEROES, > + VIRTIO_BLK_F_SECURE_ERASE, > } > ; > static unsigned int features[] = { > @@ -1177,6 +1222,7 @@ static unsigned int features[] = { > VIRTIO_BLK_F_RO, VIRTIO_BLK_F_BLK_SIZE, > VIRTIO_BLK_F_FLUSH, VIRTIO_BLK_F_TOPOLOGY, VIRTIO_BLK_F_CONFIG_WCE, > VIRTIO_BLK_F_MQ, VIRTIO_BLK_F_DISCARD, VIRTIO_BLK_F_WRITE_ZEROES, > + VIRTIO_BLK_F_SECURE_ERASE, > }; > > static struct virtio_driver virtio_blk = { > diff --git a/include/uapi/linux/virtio_blk.h b/include/uapi/linux/virtio_blk.h > index d888f013d9f..58e70b24b50 100644 > --- a/include/uapi/linux/virtio_blk.h > +++ b/include/uapi/linux/virtio_blk.h > @@ -40,6 +40,7 @@ > #define VIRTIO_BLK_F_MQ 12 /* support more than one vq */ > #define VIRTIO_BLK_F_DISCARD 13 /* DISCARD is supported */ > #define VIRTIO_BLK_F_WRITE_ZEROES 14 /* WRITE ZEROES is supported */ > +#define VIRTIO_BLK_F_SECURE_ERASE 16 /* Secure Erase is supported */ > > /* Legacy feature bits */ > #ifndef VIRTIO_BLK_NO_LEGACY > @@ -121,6 +122,21 @@ struct virtio_blk_config { > __u8 write_zeroes_may_unmap; > > __u8 unused1[3]; > + > + /* the next 3 entries are guarded by VIRTIO_BLK_F_SECURE_ERASE */ > + /* > + * The maximum secure erase sectors (in 512-byte sectors) for > + * one segment. > + */ > + __virtio32 max_secure_erase_sectors; > + /* > + * The maximum number of secure erase segments in a > + * secure erase command. > + */ > + __virtio32 max_secure_erase_seg; > + /* Secure erase commands must be aligned to this number of sectors. */ > + __virtio32 secure_erase_sector_alignment; > + > } __attribute__((packed)); > > /* > @@ -155,6 +171,9 @@ struct virtio_blk_config { > /* Write zeroes command */ > #define VIRTIO_BLK_T_WRITE_ZEROES 13 > > +/* Secure erase command */ > +#define VIRTIO_BLK_T_SECURE_ERASE 14 > + > #ifndef VIRTIO_BLK_NO_LEGACY > /* Barrier before this op. */ > #define VIRTIO_BLK_T_BARRIER 0x80000000 > -- > 2.32.0
Stefan Hajnoczi
2022-Sep-19 17:33 UTC
[PATCH v2] virtio_blk: add SECURE ERASE command support
On Mon, Aug 29, 2022 at 11:23:13AM +0300, Alvaro Karsz wrote:> @@ -1075,6 +1068,57 @@ static int virtblk_probe(struct virtio_device *vdev) > blk_queue_max_write_zeroes_sectors(q, v ? v : UINT_MAX); > } > > + if (virtio_has_feature(vdev, VIRTIO_BLK_F_SECURE_ERASE)) { > + /* The discard alignment value should be the minimum between > + * secure_erase_sector_alignment and discard_sector_alignment > + * (if VIRTIO_BLK_F_DISCARD is negotiated). > + */ > + virtio_cread(vdev, struct virtio_blk_config, > + secure_erase_sector_alignment, &v); > + if (v) { > + if (discard_granularity) > + discard_granularity = min(discard_granularity, v); > + else > + discard_granularity = v;This can be simplified with min_not_zero().> + } > + > + virtio_cread(vdev, struct virtio_blk_config, > + max_secure_erase_sectors, &v); > + blk_queue_max_secure_erase_sectors(q, v ? v : UINT_MAX); > + > + /* The max discard segments value should be the minimum between > + * max_secure_erase_seg and max_discard_seg (if VIRTIO_BLK_F_DISCARD > + * is negotiated). > + */ > + virtio_cread(vdev, struct virtio_blk_config, > + max_secure_erase_seg, &v); > + if (v) { > + if (max_discard_segs) > + max_discard_segs = min(max_discard_segs, v); > + else > + max_discard_segs = v;Same here.> + } > + } > + > + if (virtio_has_feature(vdev, VIRTIO_BLK_F_DISCARD) || > + virtio_has_feature(vdev, VIRTIO_BLK_F_SECURE_ERASE)) {It's worth including a comment here that the discard and secure erase limits are combined because the Linux block layer only has one limit value. If the block layer supported independent limit values we wouldn't need to do this. -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 488 bytes Desc: not available URL: <http://lists.linuxfoundation.org/pipermail/virtualization/attachments/20220919/1b42e96f/attachment-0001.sig>