Adaptation secure erase forwarding for 4.1x kernels

PATCH New Denis Semakin 2 participants 13 messages Mar 13, 2018 – Mar 27, 2018 Device mapper

Share & tools

Denis Semakin

author

2018-03-13 09:23 UTC #0 lore raw

PATCH 1/1 Adaptation secure erase forwarding for 4.1x kernels

Hello.
Here is fixed patch for modern 4.1x kernels.
The idea is to forward secure erase request within device mapper layer to block device driver which can support secure erase.
Could you please review?
Thanks.

drivers/md/dm-table.c +30 -0

index 7eb3e2a..d955a57 100644
@@ -1846,6 +1846,33 @@ static bool dm_table_supports_discards(struct dm_table *t)
        return true;
 }
 
+static int device_not_secerase_capable(struct dm_target *ti,
+                                          struct dm_dev *dev, sector_t start,
+                                          sector_t len, void *data)
+{
+       struct request_queue *q = bdev_get_queue(dev->bdev);
+
+       return q && !blk_queue_secure_erase(q);
+}
+
+static bool dm_targets_support_secure_erase(struct dm_table *t)
+{
+       unsigned int i = 0;
+       struct dm_target *ti;
+
+       while (i < dm_table_get_num_targets(t)) {
+               ti = dm_table_get_target(t, i++);
+
+               if (!ti->type->iterate_devices ||
+                   ti->type->iterate_devices(ti, device_not_secerase_capable,
+                                             NULL))
+               return false;
+       }
+
+       return true;
+
+}
+
 void dm_table_set_restrictions(struct dm_table *t, struct request_queue *q,
                               struct queue_limits *limits)
 {

@@ -1867,6 +1894,9 @@ void dm_table_set_restrictions(struct dm_table *t, struct request_queue *q,
        } else
                queue_flag_set_unlocked(QUEUE_FLAG_DISCARD, q);
 
+       if (dm_targets_support_secure_erase(t))
+               queue_flag_set_unlocked(QUEUE_FLAG_SECERASE, q);
+
        if (dm_table_supports_flush(t, (1UL << QUEUE_FLAG_WC))) {
                wc = true;
                if (dm_table_supports_flush(t, (1UL << QUEUE_FLAG_FUA)))

Signature · 3 lines

Best regards,

Denis Semakin

Mike Snitzer

2018-03-22 15:10 UTC #1 lore raw

PATCH 1/1 dm table: add support for secure erase forwarding [was: Re: Adaptation secure erase forwarding for 4.1x kernels]

Quoted 5 lines · Denis Semakin

Hello.
Here is fixed patch for modern 4.1x kernels.
The idea is to forward secure erase request within device mapper layer to block device driver which can support secure erase.
Could you please review?

There were various issues with your patch that I cleaned up, please see the following.

But I'm left skeptical that this is enough. Don't targets need to explicitly handle these REQ_OP_SECURE_ERASE requests? Similar to how REQ_OP_DISCARD is handled?

I'd feel safer about having targets opt-in with setting (a new) ti->num_secure_erase_bios.

Which DM target(s) have you been wanting to pass REQ_OP_SECURE_ERASE bios?

Mike

From: Denis Semakin <[email protected]>
Date: Tue, 13 Mar 2018 13:23:45 +0400
Subject: [PATCH] dm table: add support for secure erase forwarding

Set QUEUE_FLAG_SECERASE in DM device's queue_flags if a DM table's data devices support secure erase.

Signed-off-by Denis Semakin <[email protected]>

Signed-off-by Mike Snitzer <[email protected]>

---
 drivers/md/dm-table.c |   28 ++++++++++++++++++++++++++++
 1 files changed, 28 insertions(+), 0 deletions(-)

drivers/md/dm-table.c +28 -0

index 7eb3e2a..d857369 100644
@@ -1846,6 +1846,31 @@ static bool dm_table_supports_discards(struct dm_table *t)
 	return true;
 }
 
+static int device_not_secure_erase_capable(struct dm_target *ti,
+					   struct dm_dev *dev, sector_t start,
+					   sector_t len, void *data)
+{
+       struct request_queue *q = bdev_get_queue(dev->bdev);
+
+       return q && !blk_queue_secure_erase(q);
+}
+
+static bool dm_table_supports_secure_erase(struct dm_table *t)
+{
+       struct dm_target *ti;
+       unsigned int i;
+
+       for (i = 0; i < dm_table_get_num_targets(t); i++) {
+               ti = dm_table_get_target(t, i);
+
+               if (!ti->type->iterate_devices ||
+                   ti->type->iterate_devices(ti, device_not_secure_erase_capable, NULL))
+		       return false;
+       }
+
+       return true;
+}
+
 void dm_table_set_restrictions(struct dm_table *t, struct request_queue *q,
 			       struct queue_limits *limits)
 {

@@ -1867,6 +1892,9 @@ void dm_table_set_restrictions(struct dm_table *t, struct request_queue *q,
 	} else
 		queue_flag_set_unlocked(QUEUE_FLAG_DISCARD, q);
 
+	if (dm_table_supports_secure_erase(t))
+		queue_flag_set_unlocked(QUEUE_FLAG_SECERASE, q);
+
 	if (dm_table_supports_flush(t, (1UL << QUEUE_FLAG_WC))) {
 		wc = true;
 		if (dm_table_supports_flush(t, (1UL << QUEUE_FLAG_FUA)))

Signature · 1 line

1.7.4.4

Denis Semakin

author

2018-03-23 08:14 UTC #2 lore raw

Re: [PATCH] dm table: add support for secure erase forwarding [was: Re: Adaptation secure erase forwarding for 4.1x kernels]

Hi.
Soon or later everybody start to think about security.
One of the most frequently requirement is 100% reliable data deletion from any device in case of compromising or loss or theft.

For this, drive and memory cards manufacturers provide ERASE and TRIM features which can notice (inform) controller of the device to erase sectors (write down only zeros or one or random data). Features can be triggered by calling ioctl() requests or a mount options (like ext4 does). But this works only with whole device -- /dev/sdX, /dev/mmcblk0pX...
But what if for some security reasons we need to secure delete a single file. A file-system layer provide this functionality... one may call __blkdev_issue_discard() with BLKDEV_DISCARD_SECURE flag.
But...
All this works well if there is no virtual layer (like device-mapper) between file-system and block-layer, because if device driver supports this feature it can set up related flag in request_queue flags.

I have ext4 lvm partitions on my test instance and a drive which can
secure erase sectors.
Without lvm it works, with lvm it doesn't.
That's the purpose if this patch - to provide the opportunity to secure erase given sectors (through device-mapper layer, forward request) that were assigned for regular file.

Quoted 3 lines

But I'm left skeptical that this is enough. Don't targets need to explicitly handle these REQ_OP_SECURE_ERASE requests? Similar to how REQ_OP_DISCARD is handled?

I think yes, REQ_OP_DISCARD will not secure erase the data and it can be possible to get it from device.

Quoted 2 lines

I'd feel safer about having targets opt-in with setting (a new) ti->num_secure_erase_bios.

Well... May it make sense but I didn't see any reasons to add it in patch.

Quoted 2 lines

Which DM target(s) have you been wanting to pass REQ_OP_SECURE_ERASE bios?

I think first of all a linear target of course should have this. For others I'm not sure, I need to investigate.

Hopefully, I answered to all your question.

Denis

----- Исходное сообщение -----
От: "snitzer" <[email protected]>
Кому: "Denis Semakin" <[email protected]>
Копия: "dm-devel" <[email protected]>
Отправленные: Четверг, 22 Март 2018 г 18:10:46
Тема: [PATCH] dm table: add support for secure erase forwarding [was: Re: Adaptation secure erase forwarding for 4.1x kernels]

Quoted 5 lines · Denis Semakin

Hello.
Here is fixed patch for modern 4.1x kernels.
The idea is to forward secure erase request within device mapper layer to block device driver which can support secure erase.
Could you please review?

There were various issues with your patch that I cleaned up, please see the following.

But I'm left skeptical that this is enough. Don't targets need to explicitly handle these REQ_OP_SECURE_ERASE requests? Similar to how REQ_OP_DISCARD is handled?

I'd feel safer about having targets opt-in with setting (a new) ti->num_secure_erase_bios.

Which DM target(s) have you been wanting to pass REQ_OP_SECURE_ERASE bios?

Mike

From: Denis Semakin <[email protected]>
Date: Tue, 13 Mar 2018 13:23:45 +0400
Subject: [PATCH] dm table: add support for secure erase forwarding

Set QUEUE_FLAG_SECERASE in DM device's queue_flags if a DM table's data devices support secure erase.

Signed-off-by Denis Semakin <[email protected]>

Signed-off-by Mike Snitzer <[email protected]>

---
 drivers/md/dm-table.c |   28 ++++++++++++++++++++++++++++
 1 files changed, 28 insertions(+), 0 deletions(-)

drivers/md/dm-table.c +28 -0

index 7eb3e2a..d857369 100644
@@ -1846,6 +1846,31 @@ static bool dm_table_supports_discards(struct dm_table *t)
 	return true;
 }
 
+static int device_not_secure_erase_capable(struct dm_target *ti,
+					   struct dm_dev *dev, sector_t start,
+					   sector_t len, void *data)
+{
+       struct request_queue *q = bdev_get_queue(dev->bdev);
+
+       return q && !blk_queue_secure_erase(q);
+}
+
+static bool dm_table_supports_secure_erase(struct dm_table *t)
+{
+       struct dm_target *ti;
+       unsigned int i;
+
+       for (i = 0; i < dm_table_get_num_targets(t); i++) {
+               ti = dm_table_get_target(t, i);
+
+               if (!ti->type->iterate_devices ||
+                   ti->type->iterate_devices(ti, device_not_secure_erase_capable, NULL))
+		       return false;
+       }
+
+       return true;
+}
+
 void dm_table_set_restrictions(struct dm_table *t, struct request_queue *q,
 			       struct queue_limits *limits)
 {

@@ -1867,6 +1892,9 @@ void dm_table_set_restrictions(struct dm_table *t, struct request_queue *q,
 	} else
 		queue_flag_set_unlocked(QUEUE_FLAG_DISCARD, q);
 
+	if (dm_table_supports_secure_erase(t))
+		queue_flag_set_unlocked(QUEUE_FLAG_SECERASE, q);
+
 	if (dm_table_supports_flush(t, (1UL << QUEUE_FLAG_WC))) {
 		wc = true;
 		if (dm_table_supports_flush(t, (1UL << QUEUE_FLAG_FUA)))

Signature · 8 lines

1.7.4.4
-- 


--
dm-devel mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/dm-devel

Mike Snitzer

2018-03-23 15:36 UTC #3 lore raw

Re: dm table: add support for secure erase forwarding [was: Re: Adaptation secure erase forwarding for 4.1x kernels]

Quoted 18 lines · Denis Semakin

Hi.
Soon or later everybody start to think about security.
One of the most frequently requirement is 100% reliable data deletion from any device in case of compromising or loss or theft.
For this, drive and memory cards manufacturers provide ERASE and TRIM features which can notice (inform) controller of the device to erase sectors (write down only zeros or one or random data). Features can be triggered by calling ioctl() requests or a mount options (like ext4 does). But this works only with whole device -- /dev/sdX, /dev/mmcblk0pX...
But what if for some security reasons we need to secure delete a single file. A file-system layer provide this functionality... one may call __blkdev_issue_discard() with BLKDEV_DISCARD_SECURE flag.
But...
All this works well if there is no virtual layer (like device-mapper) between file-system and block-layer, because if device driver supports this feature it can set up related flag in request_queue flags.

Quoted 5 lines

I have ext4 lvm partitions on my test instance and a drive which can
secure erase sectors.
Without lvm it works, with lvm it doesn't.
That's the purpose if this patch - to provide the opportunity to secure erase given sectors (through device-mapper layer, forward request) that were assigned for regular file.

I was aware of all that. I understand that DM currently isn't setting the QUEUE_FLAG_SECERASE, etc.

Quoted 6 lines

Quoted 3 lines
But I'm left skeptical that this is enough. Don't targets need to explicitly handle these REQ_OP_SECURE_ERASE requests? Similar to how REQ_OP_DISCARD is handled?
I think yes, REQ_OP_DISCARD will not secure erase the data and it can be possible to get it from device.

Think you misunderstood me.

I meant that: DM must take special care to properly handle REQ_OP_DISCARD. Particularly more complex targets like thinp, etc.

But even more basic targets (e.g. dm-stripe.c) have code to optimize handling of REQ_OP_DISCARD. We'd likely want REQ_OP_SECURE_ERASE to be using dm-stripe.c's REQ_OP_DISCARD optimization too.

Quoted 4 lines

Quoted 2 lines
I'd feel safer about having targets opt-in with setting (a new) ti->num_secure_erase_bios.
Well... May it make sense but I didn't see any reasons to add it in patch.

Thinking further, it is absolutely needed, otherwise dm-stripe.c won't be able to use the optimization I mentioned above. In addition, other targets shouldn't be getting REQ_OP_SECURE_ERASE unless there is a real need. ti->num_secure_erase_bios will allow for a much safer and controlled rollout of this support.

Quoted 4 lines

Quoted 2 lines
Which DM target(s) have you been wanting to pass REQ_OP_SECURE_ERASE bios?
I think first of all a linear target of course should have this. For others I'm not sure, I need to investigate.

I think "linear" and "striped" would be the ideal first targets to add this support to.

I can work through adding the other changes at some point soon.

Mike

Denis Semakin

author

2018-03-23 13:37 UTC #4 lore raw

Re: [PATCH] dm table: add support for secure erase forwarding [was: Re: Adaptation secure erase forwarding for 4.1x kernels]

In reply to Mike Snitzer

Additional.

Quoted 2 lines

Which DM target(s) have you been wanting to pass REQ_OP_SECURE_ERASE bios?

I've tested only with linear targets, so I suppose it's needed to check what kind of target(s) we have.

Quoted 5 lines · Denis Semakin

Hello.
Here is fixed patch for modern 4.1x kernels.
The idea is to forward secure erase request within device mapper layer to block device driver which can support secure erase.
Could you please review?

There were various issues with your patch that I cleaned up, please see the following.

But I'm left skeptical that this is enough. Don't targets need to explicitly handle these REQ_OP_SECURE_ERASE requests? Similar to how REQ_OP_DISCARD is handled?

I'd feel safer about having targets opt-in with setting (a new) ti->num_secure_erase_bios.

Which DM target(s) have you been wanting to pass REQ_OP_SECURE_ERASE bios?

Mike

From: Denis Semakin <[email protected]>
Date: Tue, 13 Mar 2018 13:23:45 +0400
Subject: [PATCH] dm table: add support for secure erase forwarding

Set QUEUE_FLAG_SECERASE in DM device's queue_flags if a DM table's data devices support secure erase.

Signed-off-by Denis Semakin <[email protected]>

Signed-off-by Mike Snitzer <[email protected]>

---
 drivers/md/dm-table.c |   28 ++++++++++++++++++++++++++++
 1 files changed, 28 insertions(+), 0 deletions(-)

drivers/md/dm-table.c +28 -0

index 7eb3e2a..d857369 100644
@@ -1846,6 +1846,31 @@ static bool dm_table_supports_discards(struct dm_table *t)
 	return true;
 }
 
+static int device_not_secure_erase_capable(struct dm_target *ti,
+					   struct dm_dev *dev, sector_t start,
+					   sector_t len, void *data)
+{
+       struct request_queue *q = bdev_get_queue(dev->bdev);
+
+       return q && !blk_queue_secure_erase(q);
+}
+
+static bool dm_table_supports_secure_erase(struct dm_table *t)
+{
+       struct dm_target *ti;
+       unsigned int i;
+
+       for (i = 0; i < dm_table_get_num_targets(t); i++) {
+               ti = dm_table_get_target(t, i);
+
+               if (!ti->type->iterate_devices ||
+                   ti->type->iterate_devices(ti, device_not_secure_erase_capable, NULL))
+		       return false;
+       }
+
+       return true;
+}
+
 void dm_table_set_restrictions(struct dm_table *t, struct request_queue *q,
 			       struct queue_limits *limits)
 {

@@ -1867,6 +1892,9 @@ void dm_table_set_restrictions(struct dm_table *t, struct request_queue *q,
 	} else
 		queue_flag_set_unlocked(QUEUE_FLAG_DISCARD, q);
 
+	if (dm_table_supports_secure_erase(t))
+		queue_flag_set_unlocked(QUEUE_FLAG_SECERASE, q);
+
 	if (dm_table_supports_flush(t, (1UL << QUEUE_FLAG_WC))) {
 		wc = true;
 		if (dm_table_supports_flush(t, (1UL << QUEUE_FLAG_FUA)))

Signature · 12 lines

1.7.4.4
-- 
Best regards,

Denis Semakin
Software Developer
Open Mobile Platform

--
dm-devel mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/dm-devel

Denis Semakin

author

2018-03-23 14:47 UTC #5 lore raw

Re: [PATCH] dm table: add support for secure erase forwarding [was: Re: Adaptation secure erase forwarding for 4.1x kernels]

In reply to Mike Snitzer

Quoted 2 lines

I'd feel safer about having targets opt-in with setting (a new) ti->num_secure_erase_bios.

May be add a new field "bool secure_erase_supported:1" in dm_target structure instead? And set up it "true" in constructor for linear targets.

Quoted 5 lines · Denis Semakin

Hello.
Here is fixed patch for modern 4.1x kernels.
The idea is to forward secure erase request within device mapper layer to block device driver which can support secure erase.
Could you please review?

There were various issues with your patch that I cleaned up, please see the following.

But I'm left skeptical that this is enough. Don't targets need to explicitly handle these REQ_OP_SECURE_ERASE requests? Similar to how REQ_OP_DISCARD is handled?

I'd feel safer about having targets opt-in with setting (a new) ti->num_secure_erase_bios.

Which DM target(s) have you been wanting to pass REQ_OP_SECURE_ERASE bios?

Mike

From: Denis Semakin <[email protected]>
Date: Tue, 13 Mar 2018 13:23:45 +0400
Subject: [PATCH] dm table: add support for secure erase forwarding

Set QUEUE_FLAG_SECERASE in DM device's queue_flags if a DM table's data devices support secure erase.

Signed-off-by Denis Semakin <[email protected]>

Signed-off-by Mike Snitzer <[email protected]>

---
 drivers/md/dm-table.c |   28 ++++++++++++++++++++++++++++
 1 files changed, 28 insertions(+), 0 deletions(-)

drivers/md/dm-table.c +28 -0

index 7eb3e2a..d857369 100644
@@ -1846,6 +1846,31 @@ static bool dm_table_supports_discards(struct dm_table *t)
 	return true;
 }
 
+static int device_not_secure_erase_capable(struct dm_target *ti,
+					   struct dm_dev *dev, sector_t start,
+					   sector_t len, void *data)
+{
+       struct request_queue *q = bdev_get_queue(dev->bdev);
+
+       return q && !blk_queue_secure_erase(q);
+}
+
+static bool dm_table_supports_secure_erase(struct dm_table *t)
+{
+       struct dm_target *ti;
+       unsigned int i;
+
+       for (i = 0; i < dm_table_get_num_targets(t); i++) {
+               ti = dm_table_get_target(t, i);
+
+               if (!ti->type->iterate_devices ||
+                   ti->type->iterate_devices(ti, device_not_secure_erase_capable, NULL))
+		       return false;
+       }
+
+       return true;
+}
+
 void dm_table_set_restrictions(struct dm_table *t, struct request_queue *q,
 			       struct queue_limits *limits)
 {

@@ -1867,6 +1892,9 @@ void dm_table_set_restrictions(struct dm_table *t, struct request_queue *q,
 	} else
 		queue_flag_set_unlocked(QUEUE_FLAG_DISCARD, q);
 
+	if (dm_table_supports_secure_erase(t))
+		queue_flag_set_unlocked(QUEUE_FLAG_SECERASE, q);
+
 	if (dm_table_supports_flush(t, (1UL << QUEUE_FLAG_WC))) {
 		wc = true;
 		if (dm_table_supports_flush(t, (1UL << QUEUE_FLAG_FUA)))

Signature · 12 lines

1.7.4.4
-- 
Best regards,

Denis Semakin
Software Developer
Open Mobile Platform

--
dm-devel mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/dm-devel

Mike Snitzer

2018-03-23 15:38 UTC #6 lore raw

Re: dm table: add support for secure erase forwarding [was: Re: Adaptation secure erase forwarding for 4.1x kernels]

Quoted 4 lines · Denis Semakin

Quoted 2 lines
I'd feel safer about having targets opt-in with setting (a new) ti->num_secure_erase_bios.
May be add a new field "bool secure_erase_supported:1" in dm_target structure instead? And set up it "true" in constructor for linear targets.

No, that type of change is for a DM target to support a feature even if the underlying device doesn't.

As my previous email elaborated, ti->num_secure_erase_bios is the right way forward.

Mike

Denis Semakin

author

2018-03-26 07:45 UTC #7 lore raw

Re: dm table: add support for secure erase forwarding [was: Re: Adaptation secure erase forwarding for 4.1x kernels]

Hi, sorry for my misunderstanding.

Quoted 5 lines

Thinking further, it is absolutely needed, otherwise dm-stripe.c won't be able to use the optimization I mentioned above. In addition, other targets shouldn't be getting REQ_OP_SECURE_ERASE unless there is a real need. ti->num_secure_erase_bios will allow for a much safer and controlled rollout of this support.

Ok. I got it.

Quoted 2 lines

As my previous email elaborated, ti->num_secure_erase_bios is the right way forward.

Then a patch for this may look like:

From 2b9dfd5af813023cdaded4d91b4c25cec8a1783d Mon Sep 17 00:00:00 2001
From: Denis Semakin <[email protected]>
Date: Mon, 26 Mar 2018 10:44:14 +0300
Subject: [PATCH] Add num_secure_erase_bios field

Signed-off-by Denis Semakin <[email protected]>

---
 drivers/md/dm-linear.c        | 1 +
 drivers/md/dm-table.c         | 3 +++
 include/linux/device-mapper.h | 2 ++
 3 files changed, 6 insertions(+)

drivers/md/dm-linear.c +1 -0

index d5f8eff..ff751b0 100644
@@ -59,6 +59,7 @@ static int linear_ctr(struct dm_target *ti, unsigned int argc, char **argv)
 
        ti->num_flush_bios = 1;
        ti->num_discard_bios = 1;
+       ti->num_secure_erase_bios = 1;
        ti->num_write_same_bios = 1;
        ti->num_write_zeroes_bios = 1;
        ti->private = lc;

drivers/md/dm-table.c +3 -0

index d857369..0acffdb 100644
@@ -1863,6 +1863,9 @@ static bool dm_table_supports_secure_erase(struct dm_table *t)
        for (i = 0; i < dm_table_get_num_targets(t); i++) {
                ti = dm_table_get_target(t, i);
 
+              if (!ti->num_secure_erase_bios)
+                      return false;
+
                if (!ti->type->iterate_devices ||
                    ti->type->iterate_devices(ti, device_not_secure_erase_capable, NULL))
                       return false;

include/linux/device-mapper.h +2 -0

index 1e2426c..233ac8f 100644
@@ -267,6 +267,8 @@ struct dm_target {
         */
        unsigned num_discard_bios;
 
+       unsigned num_secure_erase_bios;
+
        /*
         * The number of WRITE SAME bios that will be submitted to the target.
         * The bio number can be accessed with dm_bio_get_target_bio_nr.

Signature · 5 lines

1.9.1

What do you think?

Denis.

Denis Semakin

author

2018-03-26 09:58 UTC #8 lore raw

Re: dm table: add support for secure erase forwarding [was: Re: Adaptation secure erase forwarding for 4.1x kernels]

Hi. Here is a full patch for adding num_secure_erase_bios field in struct dm_targer for linear DMs and QUEUE_FLAG_SECERASE forwarding.
Could you please review.

Thanks.

drivers/md/dm-linear.c +1 -0

index d5f8eff..ff751b0 100644
@@ -59,6 +59,7 @@ static int linear_ctr(struct dm_target *ti, unsigned int argc, char **argv)
 
        ti->num_flush_bios = 1;
        ti->num_discard_bios = 1;
+       ti->num_secure_erase_bios = 1;
        ti->num_write_same_bios = 1;
        ti->num_write_zeroes_bios = 1;
        ti->private = lc;

drivers/md/dm-table.c +33 -0

index 7eb3e2a..13a2c23 100644
@@ -1846,6 +1846,36 @@ static bool dm_table_supports_discards(struct dm_table *t)
        return true;
 }
 
+static int device_not_secerase_capable(struct dm_target *ti,
+                                          struct dm_dev *dev, sector_t start,
+                                          sector_t len, void *data)
+{
+       struct request_queue *q = bdev_get_queue(dev->bdev);
+
+       return q && !blk_queue_secure_erase(q);
+}
+
+static bool dm_targets_support_secure_erase(struct dm_table *t)
+{
+       unsigned int i = 0;
+       struct dm_target *ti;
+
+       while (i < dm_table_get_num_targets(t)) {
+               ti = dm_table_get_target(t, i++);
+
+               if (!ti->num_secure_erase_bios)
+                       return false;
+
+               if (!ti->type->iterate_devices ||
+                   ti->type->iterate_devices(ti, device_not_secerase_capable,
+                                             NULL))
+               return false;
+       }
+
+       return true;
+
+}
+
 void dm_table_set_restrictions(struct dm_table *t, struct request_queue *q,
                               struct queue_limits *limits)
 {

@@ -1867,6 +1897,9 @@ void dm_table_set_restrictions(struct dm_table *t, struct request_queue *q,
        } else
                queue_flag_set_unlocked(QUEUE_FLAG_DISCARD, q);
 
+       if (dm_targets_support_secure_erase(t))
+               queue_flag_set_unlocked(QUEUE_FLAG_SECERASE, q);
+
        if (dm_table_supports_flush(t, (1UL << QUEUE_FLAG_WC))) {
                wc = true;
                if (dm_table_supports_flush(t, (1UL << QUEUE_FLAG_FUA)))

include/linux/device-mapper.h +2 -0

index 1e2426c..233ac8f 100644
@@ -267,6 +267,8 @@ struct dm_target {
         */
        unsigned num_discard_bios;
 
+       unsigned num_secure_erase_bios;
+
        /*
         * The number of WRITE SAME bios that will be submitted to the target.
         * The bio number can be accessed with dm_bio_get_target_bio_nr.

Denis Semakin

author

2018-03-26 14:12 UTC #9 lore raw

Re: dm table: add support for secure erase forwarding [was: Re: Adaptation secure erase forwarding for 4.1x kernels]

In reply to Mike Snitzer

Also, will it be usefully to add a "get...()" function for num_secure_erase_bios? Let's say get_num_secure_erase_bios(struct dm_target &ti); As it's done for num_discard_bios...

----- Исходное сообщение -----
От: "snitzer" <[email protected]>
Кому: "Denis Semakin"
Копия: "dm-devel" <[email protected]>
Отправленные: Пятница, 23 Март 2018 г 18:38:44
Тема: Re: dm table: add support for secure erase forwarding [was: Re: Adaptation secure erase forwarding for 4.1x kernels]

Quoted 4 lines · Denis Semakin

Quoted 2 lines
I'd feel safer about having targets opt-in with setting (a new) ti->num_secure_erase_bios.
May be add a new field "bool secure_erase_supported:1" in dm_target structure instead? And set up it "true" in constructor for linear targets.

No, that type of change is for a DM target to support a feature even if the underlying device doesn't.

As my previous email elaborated, ti->num_secure_erase_bios is the right way forward.

Mike

Signature · 10 lines

Best regards,

Denis Semakin
Software Developer
Open Mobile Platform

--
dm-devel mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/dm-devel

Mike Snitzer

2018-03-26 16:11 UTC #10 lore raw

Re: dm table: add support for secure erase forwarding [was: Re: Adaptation secure erase forwarding for 4.1x kernels]

Quoted 3 lines · Denis Semakin

Also, will it be usefully to add a "get...()" function for num_secure_erase_bios? Let's say get_num_secure_erase_bios(struct dm_target &ti); As it's done for num_discard_bios...

Yes, it is required. I'll share a link to the final patch shortly, I've still attributed the work to you.

BTW, please look into your text editor's settings: seems your patches are using spaces rather than tabs (and you aren't even using 8 spaces?)

Mike

Denis Semakin

author

2018-03-27 08:54 UTC #11 lore raw

Re: dm table: add support for secure erase forwarding [was: Re: Adaptation secure erase forwarding for 4.1x kernels]

Hi.

Quoted 6 lines

Quoted 3 lines
Also, will it be usefully to add a "get...()" function for num_secure_erase_bios? Let's say get_num_secure_erase_bios(struct dm_target &ti); As it's done for num_discard_bios...
Yes, it is required. I'll share a link to the final patch shortly, I've still attributed the work to you.

Thanks.
I've already seen your final patch in device-mapper git repository and I have nothing to add from my side.

Quoted 3 lines

BTW, please look into your text editor's settings: seems your patches are using spaces rather than tabs (and you aren't even using 8 spaces?)

It's not blame of editor (I prefer to use git), it's email client re-formats a message. Next time I'll do it via git send-mail.

Quoted 1 line

Mike

Signature · 3 lines

Best regards,

Denis

Denis Semakin

author

2018-03-27 09:03 UTC #12 lore raw

Re: dm table: add support for secure erase forwarding [was: Re: Adaptation secure erase forwarding for 4.1x kernels]

Quoted 7 lines

Quoted 4 lines
BTW, please look into your text editor's settings: seems your patches are using spaces rather than tabs (and you aren't even using 8 spaces?)
It's not blame of editor (I prefer to use git), it's email client re-formats a message.

I prefer to use vim of course

Quoted 7 lines

Next time I'll do it via git send-mail.
Quoted 1 line
Mike
Signature · 3 lines
Best regards,

Denis

Signature · 3 lines

Best regards,

Denis