Virtualization - Sep 2022 - [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>

Thanks for the reply.
> This can be simplified with min_not_zero().
Ok, I will do it in the next version.
> 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.
Ok.

I'll send a new version once we agree on the max_secure_erase_seg = 0
scenario.
Do you have an opinion on that?
Do you think that using sg_elems as the number of secure erase/discard
segments when the value in the virtio config is 0 is good enough?