Re: [PATCH] virtio_blk: add SECURE ERASE command support

Re: [PATCH] virtio_blk: add SECURE ERASE command support

[Stefan Hajnoczi]

[-- Attachment #1.1: Type: text/plain, Size: 514 bytes --]

On Mon, Aug 22, 2022 at 07:20:55PM +0300, Alvaro Karsz wrote:
> @@ -1075,6 +1079,12 @@ 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)) {
> +		virtio_cread(vdev, struct virtio_blk_config,
> +			     max_secure_erase_sectors, &v);
> +		blk_queue_max_secure_erase_sectors(q, v ? v : UINT_MAX);
> +	}

What about max_secure_erase_seg and secure_erase_sector_alignment?

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 183 bytes --]

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH] virtio_blk: add SECURE ERASE command support

[Alvaro Karsz]

> What about max_secure_erase_seg and secure_erase_sector_alignment?

Hi Stefan,
If I understand correctly, the Linux kernel uses the same "max
segments" value for a discard and a secure erase command.

> unsigned int blk_recalc_rq_segments(struct request *rq)
> {
>     unsigned int nr_phys_segs = 0;
>     unsigned int bytes = 0;
>     struct req_iterator iter;
>     struct bio_vec bv;
>
>
>     if (!rq->bio)
>         return 0;
>
>
>     switch (bio_op(rq->bio)) {
>     case REQ_OP_DISCARD:
>     case REQ_OP_SECURE_ERASE:
>         if (queue_max_discard_segments(rq->q) > 1) {
>             struct bio *bio = rq->bio;
>
>
>             for_each_bio(bio)
>                 nr_phys_segs++;
>             return nr_phys_segs;
>         }
>
> ....


> struct bio *__bio_split_to_limits(struct bio *bio, struct queue_limits *lim,
>       unsigned int *nr_segs)
> {
>     struct bio_set *bs = &bio->bi_bdev->bd_disk->bio_split;
>     struct bio *split;
>
>
>     switch (bio_op(bio)) {
>     case REQ_OP_DISCARD:
>     case REQ_OP_SECURE_ERASE:
>         split = bio_split_discard(bio, lim, nr_segs, bs);
>
>         break;
>
> ...

What do you suggest?

BTW, the same happens for the write zeros command implementation.
max_write_zeroes_seg and write_zeroes_may_unmap are ignored in the Linux kernel.
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH] virtio_blk: add SECURE ERASE command support

[Stefan Hajnoczi]

[-- Attachment #1.1: Type: text/plain, Size: 1587 bytes --]

On Wed, Aug 24, 2022 at 09:45:57AM +0300, Alvaro Karsz wrote:
> > What about max_secure_erase_seg and secure_erase_sector_alignment?
> 
> Hi Stefan,
> If I understand correctly, the Linux kernel uses the same "max
> segments" value for a discard and a secure erase command.
> 
> > unsigned int blk_recalc_rq_segments(struct request *rq)
> > {
> >     unsigned int nr_phys_segs = 0;
> >     unsigned int bytes = 0;
> >     struct req_iterator iter;
> >     struct bio_vec bv;
> >
> >
> >     if (!rq->bio)
> >         return 0;
> >
> >
> >     switch (bio_op(rq->bio)) {
> >     case REQ_OP_DISCARD:
> >     case REQ_OP_SECURE_ERASE:
> >         if (queue_max_discard_segments(rq->q) > 1) {
> >             struct bio *bio = rq->bio;
> >
> >
> >             for_each_bio(bio)
> >                 nr_phys_segs++;
> >             return nr_phys_segs;
> >         }
> >
> > ....
> 
> 
> > struct bio *__bio_split_to_limits(struct bio *bio, struct queue_limits *lim,
> >       unsigned int *nr_segs)
> > {
> >     struct bio_set *bs = &bio->bi_bdev->bd_disk->bio_split;
> >     struct bio *split;
> >
> >
> >     switch (bio_op(bio)) {
> >     case REQ_OP_DISCARD:
> >     case REQ_OP_SECURE_ERASE:
> >         split = bio_split_discard(bio, lim, nr_segs, bs);
> >
> >         break;
> >
> > ...
> 
> What do you suggest?
> 
> BTW, the same happens for the write zeros command implementation.
> max_write_zeroes_seg and write_zeroes_may_unmap are ignored in the Linux kernel.

How about calculating the minimum of the limits?

Stefan

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 183 bytes --]

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH] virtio_blk: add SECURE ERASE command support

[Alvaro Karsz]

> How about calculating the minimum of the limits?

Sounds reasonable.
Should I add it to this patch (as v2)?
Or maybe it can be a follow up patch, and it can include the write
zeros command as well.
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH] virtio_blk: add SECURE ERASE command support

[Stefan Hajnoczi]

[-- Attachment #1.1: Type: text/plain, Size: 360 bytes --]

On Wed, Aug 24, 2022 at 10:48:30PM +0300, Alvaro Karsz wrote:
> > How about calculating the minimum of the limits?
> 
> Sounds reasonable.
> Should I add it to this patch (as v2)?
> Or maybe it can be a follow up patch, and it can include the write
> zeros command as well.

If you can include it in a v2 series that would be nice.

Thanks,
Stefan

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 183 bytes --]

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH] virtio_blk: add SECURE ERASE command support

[Alvaro Karsz]

> If you can include it in a v2 series that would be nice.

Sure, I'll create a new version.
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization