All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/1] sed-opal: geometry feature reporting command
@ 2023-04-06 13:19 Ondrej Kozina
  2023-04-06 13:19 ` [PATCH 1/1] " Ondrej Kozina
  2023-04-11  9:09 ` [PATCH v2 0/1] " Ondrej Kozina
  0 siblings, 2 replies; 11+ messages in thread
From: Ondrej Kozina @ 2023-04-06 13:19 UTC (permalink / raw)
  To: linux-block
  Cc: bluca, gmazyland, axboe, hch, brauner, jonathan.derrick, Ondrej Kozina

While further testing various OPAL drives we have discovered that
some drives may impose alignment requirements that can not be
satisfied without reading some aditional parameters reported
by sed-opal iface.

We used to stick to physical block size reported by general block
device in place of LogicalBlockSize as reported by opal 'geometry',
but at least 2 aditional restrictions can not be easily mapped to
anything currently reported by block layer.

Without this patch we can not properly align locking ranges created
via sed-opal iface.

(Also it helps us to explain to userspace what went wrong)

Ondrej Kozina (1):
  sed-opal: geometry feature reporting command

 block/sed-opal.c              | 29 ++++++++++++++++++++++++++++-
 include/linux/sed-opal.h      |  1 +
 include/uapi/linux/sed-opal.h | 13 +++++++++++++
 3 files changed, 42 insertions(+), 1 deletion(-)

-- 
2.31.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 1/1] sed-opal: geometry feature reporting command
  2023-04-06 13:19 [PATCH 0/1] sed-opal: geometry feature reporting command Ondrej Kozina
@ 2023-04-06 13:19 ` Ondrej Kozina
  2023-04-06 20:32   ` Milan Broz
  2023-04-11  9:09 ` [PATCH v2 0/1] " Ondrej Kozina
  1 sibling, 1 reply; 11+ messages in thread
From: Ondrej Kozina @ 2023-04-06 13:19 UTC (permalink / raw)
  To: linux-block
  Cc: bluca, gmazyland, axboe, hch, brauner, jonathan.derrick, Ondrej Kozina

Locking range start and locking range length
attributes may be require to satisfy restrictions
exposed by OPAL2 geometry feature reporting.

Geometry reporting feature is described in TCG OPAL SSC,
section 3.1.1.4 (ALIGN, LogicalBlockSize, AlignmentGranularity
and LowestAlignedLBA).

4.3.5.2.1.1 RangeStart Behavior:

[ StartAlignment = (RangeStart modulo AlignmentGranularity) - LowestAlignedLBA ]

When processing a Set method or CreateRow method on the Locking
table for a non-Global Range row, if:

a) the AlignmentRequired (ALIGN above) column in the LockingInfo
   table is TRUE;
b) RangeStart is non-zero; and
c) StartAlignment is non-zero, then the method SHALL fail and
   return an error status code INVALID_PARAMETER.

4.3.5.2.1.2 RangeLength Behavior:

If RangeStart is zero, then
	[ LengthAlignment = (RangeLength modulo AlignmentGranularity) - LowestAlignedLBA ]

If RangeStart is non-zero, then
	[ LengthAlignment = (RangeLength modulo AlignmentGranularity) ]

When processing a Set method or CreateRow method on the Locking
table for a non-Global Range row, if:

a) the AlignmentRequired (ALIGN above) column in the LockingInfo
   table is TRUE;
b) RangeLength is non-zero; and
c) LengthAlignment is non-zero, then the method SHALL fail and
   return an error status code INVALID_PARAMETER

In userspace we stuck to logical block size reported by general
block device (via sysfs or ioctl), but we can not read
'AlignmentGranularity' or 'LowestAlignedLBA' anywhere else and
we need to get those values from sed-opal interface otherwise
we will not be able to report or avoid locking range setup
INVALID_PARAMETER errors above.

Signed-off-by: Ondrej Kozina <okozina@redhat.com>
---
 block/sed-opal.c              | 29 ++++++++++++++++++++++++++++-
 include/linux/sed-opal.h      |  1 +
 include/uapi/linux/sed-opal.h | 13 +++++++++++++
 3 files changed, 42 insertions(+), 1 deletion(-)

diff --git a/block/sed-opal.c b/block/sed-opal.c
index 3fc4e65db111..137d47a5e674 100644
--- a/block/sed-opal.c
+++ b/block/sed-opal.c
@@ -83,8 +83,10 @@ struct opal_dev {
 	u16 comid;
 	u32 hsn;
 	u32 tsn;
-	u64 align;
+	u64 align; /* alignment granularity */
 	u64 lowest_lba;
+	u32 logical_block_size;
+	u8  align_required; /* ALIGN: 0 or 1 */
 
 	size_t pos;
 	u8 *cmd;
@@ -409,6 +411,8 @@ static void check_geometry(struct opal_dev *dev, const void *data)
 
 	dev->align = be64_to_cpu(geo->alignment_granularity);
 	dev->lowest_lba = be64_to_cpu(geo->lowest_aligned_lba);
+	dev->logical_block_size = be32_to_cpu(geo->logical_block_size);
+	dev->align_required = (geo->reserved01 & 0b10000000) ? 1 : 0;
 }
 
 static int execute_step(struct opal_dev *dev,
@@ -2956,6 +2960,26 @@ static int opal_get_status(struct opal_dev *dev, void __user *data)
 	return 0;
 }
 
+static int opal_get_geometry(struct opal_dev *dev, void __user *data)
+{
+	struct opal_geometry geo = {0};
+
+	if (check_opal_support(dev))
+		return -EINVAL;
+
+	geo.align = dev->align_required;
+	geo.logical_block_size = dev->logical_block_size;
+	geo.alignment_granularity =  dev->align;
+	geo.lowest_aligned_lba = dev->lowest_lba;
+
+	if (copy_to_user(data, &geo, sizeof(geo))) {
+		pr_debug("Error copying geometry data to userspace\n");
+		return -EFAULT;
+	}
+
+	return 0;
+}
+
 int sed_ioctl(struct opal_dev *dev, unsigned int cmd, void __user *arg)
 {
 	void *p;
@@ -3029,6 +3053,9 @@ int sed_ioctl(struct opal_dev *dev, unsigned int cmd, void __user *arg)
 	case IOC_OPAL_GET_LR_STATUS:
 		ret = opal_locking_range_status(dev, p, arg);
 		break;
+	case IOC_OPAL_GET_GEOMETRY:
+		ret = opal_get_geometry(dev, arg);
+		break;
 	default:
 		break;
 	}
diff --git a/include/linux/sed-opal.h b/include/linux/sed-opal.h
index 042c1e2cb0ce..bbae1e52ab4f 100644
--- a/include/linux/sed-opal.h
+++ b/include/linux/sed-opal.h
@@ -46,6 +46,7 @@ static inline bool is_sed_ioctl(unsigned int cmd)
 	case IOC_OPAL_GENERIC_TABLE_RW:
 	case IOC_OPAL_GET_STATUS:
 	case IOC_OPAL_GET_LR_STATUS:
+	case IOC_OPAL_GET_GEOMETRY:
 		return true;
 	}
 	return false;
diff --git a/include/uapi/linux/sed-opal.h b/include/uapi/linux/sed-opal.h
index 3905c8ffedbf..dc2efd345133 100644
--- a/include/uapi/linux/sed-opal.h
+++ b/include/uapi/linux/sed-opal.h
@@ -161,6 +161,18 @@ struct opal_status {
 	__u32 reserved;
 };
 
+/*
+ * Geometry Reporting per TCG Storage OPAL SSC
+ * section 3.1.1.4
+ */
+struct opal_geometry {
+	__u8 align;
+	__u32 logical_block_size;
+	__u64 alignment_granularity;
+	__u64 lowest_aligned_lba;
+	__u8  __align[3];
+};
+
 #define IOC_OPAL_SAVE		    _IOW('p', 220, struct opal_lock_unlock)
 #define IOC_OPAL_LOCK_UNLOCK	    _IOW('p', 221, struct opal_lock_unlock)
 #define IOC_OPAL_TAKE_OWNERSHIP	    _IOW('p', 222, struct opal_key)
@@ -179,5 +191,6 @@ struct opal_status {
 #define IOC_OPAL_GENERIC_TABLE_RW   _IOW('p', 235, struct opal_read_write_table)
 #define IOC_OPAL_GET_STATUS         _IOR('p', 236, struct opal_status)
 #define IOC_OPAL_GET_LR_STATUS      _IOW('p', 237, struct opal_lr_status)
+#define IOC_OPAL_GET_GEOMETRY       _IOR('p', 238, struct opal_geometry)
 
 #endif /* _UAPI_SED_OPAL_H */
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/1] sed-opal: geometry feature reporting command
  2023-04-06 13:19 ` [PATCH 1/1] " Ondrej Kozina
@ 2023-04-06 20:32   ` Milan Broz
  2023-04-11  9:05     ` Ondrej Kozina
  0 siblings, 1 reply; 11+ messages in thread
From: Milan Broz @ 2023-04-06 20:32 UTC (permalink / raw)
  To: Ondrej Kozina, linux-block; +Cc: bluca, axboe, hch, brauner, jonathan.derrick


On 4/6/23 15:19, Ondrej Kozina wrote:
> Locking range start and locking range length
> attributes may be require to satisfy restrictions
> exposed by OPAL2 geometry feature reporting.

...

> diff --git a/block/sed-opal.c b/block/sed-opal.c
> index 3fc4e65db111..137d47a5e674 100644
> --- a/block/sed-opal.c
> +++ b/block/sed-opal.c
> @@ -83,8 +83,10 @@ struct opal_dev {
>   	u16 comid;
>   	u32 hsn;
>   	u32 tsn;
> -	u64 align;
> +	u64 align; /* alignment granularity */
>   	u64 lowest_lba;
> +	u32 logical_block_size;
> +	u8  align_required; /* ALIGN: 0 or 1 */
>   
>   	size_t pos;
>   	u8 *cmd;
> @@ -409,6 +411,8 @@ static void check_geometry(struct opal_dev *dev, const void *data)
>   
>   	dev->align = be64_to_cpu(geo->alignment_granularity);
>   	dev->lowest_lba = be64_to_cpu(geo->lowest_aligned_lba);
> +	dev->logical_block_size = be32_to_cpu(geo->logical_block_size);
> +	dev->align_required = (geo->reserved01 & 0b10000000) ? 1 : 0;

I am not sure if we can use 0b prefix as it is compiler extension (not in C standard, IIRC).

Anyway, align_required returns always 0 for my test drives even when discovery shows ALIGN = 1.
Does it mask the correct bit?

Other geometry attributes work correctly.

Milan


>   }
>   
>   static int execute_step(struct opal_dev *dev,
> @@ -2956,6 +2960,26 @@ static int opal_get_status(struct opal_dev *dev, void __user *data)
>   	return 0;
>   }
>   
> +static int opal_get_geometry(struct opal_dev *dev, void __user *data)
> +{
> +	struct opal_geometry geo = {0};
> +
> +	if (check_opal_support(dev))
> +		return -EINVAL;
> +
> +	geo.align = dev->align_required;
> +	geo.logical_block_size = dev->logical_block_size;
> +	geo.alignment_granularity =  dev->align;
> +	geo.lowest_aligned_lba = dev->lowest_lba;
> +
> +	if (copy_to_user(data, &geo, sizeof(geo))) {
> +		pr_debug("Error copying geometry data to userspace\n");
> +		return -EFAULT;
> +	}
> +
> +	return 0;
> +}
> +
>   int sed_ioctl(struct opal_dev *dev, unsigned int cmd, void __user *arg)
>   {
>   	void *p;
> @@ -3029,6 +3053,9 @@ int sed_ioctl(struct opal_dev *dev, unsigned int cmd, void __user *arg)
>   	case IOC_OPAL_GET_LR_STATUS:
>   		ret = opal_locking_range_status(dev, p, arg);
>   		break;
> +	case IOC_OPAL_GET_GEOMETRY:
> +		ret = opal_get_geometry(dev, arg);
> +		break;
>   	default:
>   		break;
>   	}
> diff --git a/include/linux/sed-opal.h b/include/linux/sed-opal.h
> index 042c1e2cb0ce..bbae1e52ab4f 100644
> --- a/include/linux/sed-opal.h
> +++ b/include/linux/sed-opal.h
> @@ -46,6 +46,7 @@ static inline bool is_sed_ioctl(unsigned int cmd)
>   	case IOC_OPAL_GENERIC_TABLE_RW:
>   	case IOC_OPAL_GET_STATUS:
>   	case IOC_OPAL_GET_LR_STATUS:
> +	case IOC_OPAL_GET_GEOMETRY:
>   		return true;
>   	}
>   	return false;
> diff --git a/include/uapi/linux/sed-opal.h b/include/uapi/linux/sed-opal.h
> index 3905c8ffedbf..dc2efd345133 100644
> --- a/include/uapi/linux/sed-opal.h
> +++ b/include/uapi/linux/sed-opal.h
> @@ -161,6 +161,18 @@ struct opal_status {
>   	__u32 reserved;
>   };
>   
> +/*
> + * Geometry Reporting per TCG Storage OPAL SSC
> + * section 3.1.1.4
> + */
> +struct opal_geometry {
> +	__u8 align;
> +	__u32 logical_block_size;
> +	__u64 alignment_granularity;
> +	__u64 lowest_aligned_lba;
> +	__u8  __align[3];
> +};
> +
>   #define IOC_OPAL_SAVE		    _IOW('p', 220, struct opal_lock_unlock)
>   #define IOC_OPAL_LOCK_UNLOCK	    _IOW('p', 221, struct opal_lock_unlock)
>   #define IOC_OPAL_TAKE_OWNERSHIP	    _IOW('p', 222, struct opal_key)
> @@ -179,5 +191,6 @@ struct opal_status {
>   #define IOC_OPAL_GENERIC_TABLE_RW   _IOW('p', 235, struct opal_read_write_table)
>   #define IOC_OPAL_GET_STATUS         _IOR('p', 236, struct opal_status)
>   #define IOC_OPAL_GET_LR_STATUS      _IOW('p', 237, struct opal_lr_status)
> +#define IOC_OPAL_GET_GEOMETRY       _IOR('p', 238, struct opal_geometry)
>   
>   #endif /* _UAPI_SED_OPAL_H */

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/1] sed-opal: geometry feature reporting command
  2023-04-06 20:32   ` Milan Broz
@ 2023-04-11  9:05     ` Ondrej Kozina
  0 siblings, 0 replies; 11+ messages in thread
From: Ondrej Kozina @ 2023-04-11  9:05 UTC (permalink / raw)
  To: Milan Broz, linux-block; +Cc: bluca, axboe, hch, brauner, jonathan.derrick

On 06. 04. 23 22:32, Milan Broz wrote:
> 
> On 4/6/23 15:19, Ondrej Kozina wrote:
>> Locking range start and locking range length
>> attributes may be require to satisfy restrictions
>> exposed by OPAL2 geometry feature reporting.
> 
> ...

>> +	dev->align_required = (geo->reserved01 & 0b10000000) ? 1 : 0;
> 
> I am not sure if we can use 0b prefix as it is compiler extension (not in C standard, IIRC).
> 
> Anyway, align_required returns always 0 for my test drives even when discovery shows ALIGN = 1.
> Does it mask the correct bit?

Yikes, sending a fix right now and my apologies for the rushed patch.

O.

> 
> Other geometry attributes work correctly.
> 
> Milan
> 
> 


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v2 0/1] sed-opal: geometry feature reporting command
  2023-04-06 13:19 [PATCH 0/1] sed-opal: geometry feature reporting command Ondrej Kozina
  2023-04-06 13:19 ` [PATCH 1/1] " Ondrej Kozina
@ 2023-04-11  9:09 ` Ondrej Kozina
  2023-04-11  9:09   ` [PATCH v2 1/1] " Ondrej Kozina
                     ` (2 more replies)
  1 sibling, 3 replies; 11+ messages in thread
From: Ondrej Kozina @ 2023-04-11  9:09 UTC (permalink / raw)
  To: linux-block
  Cc: bluca, gmazyland, axboe, hch, brauner, jonathan.derrick, Ondrej Kozina

While further testing various OPAL drives we have discovered that
some drives may impose alignment requirements that can not be
satisfied without reading some aditional parameters reported
by sed-opal iface.

We used to stick to physical block size reported by general block
device in place of LogicalBlockSize as reported by opal 'geometry',
but at least 2 aditional restrictions can not be easily mapped to
anything currently reported by block layer.

Without this patch we can not properly align locking ranges created
via sed-opal iface.

(Also it helps us to explain to userspace what went wrong)

Changes since previous version:

v2:
  - Mask proper bit for ALIGN value

Ondrej Kozina (1):
  sed-opal: geometry feature reporting command

 block/sed-opal.c              | 29 ++++++++++++++++++++++++++++-
 include/linux/sed-opal.h      |  1 +
 include/uapi/linux/sed-opal.h | 13 +++++++++++++
 3 files changed, 42 insertions(+), 1 deletion(-)

-- 
2.31.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v2 1/1] sed-opal: geometry feature reporting command
  2023-04-11  9:09 ` [PATCH v2 0/1] " Ondrej Kozina
@ 2023-04-11  9:09   ` Ondrej Kozina
  2023-04-11 12:31     ` Christoph Hellwig
                       ` (2 more replies)
  2023-04-19  7:33   ` [PATCH v2 0/1] " Ondrej Kozina
  2023-04-19 20:07   ` Jens Axboe
  2 siblings, 3 replies; 11+ messages in thread
From: Ondrej Kozina @ 2023-04-11  9:09 UTC (permalink / raw)
  To: linux-block
  Cc: bluca, gmazyland, axboe, hch, brauner, jonathan.derrick, Ondrej Kozina

Locking range start and locking range length
attributes may be require to satisfy restrictions
exposed by OPAL2 geometry feature reporting.

Geometry reporting feature is described in TCG OPAL SSC,
section 3.1.1.4 (ALIGN, LogicalBlockSize, AlignmentGranularity
and LowestAlignedLBA).

4.3.5.2.1.1 RangeStart Behavior:

[ StartAlignment = (RangeStart modulo AlignmentGranularity) - LowestAlignedLBA ]

When processing a Set method or CreateRow method on the Locking
table for a non-Global Range row, if:

a) the AlignmentRequired (ALIGN above) column in the LockingInfo
   table is TRUE;
b) RangeStart is non-zero; and
c) StartAlignment is non-zero, then the method SHALL fail and
   return an error status code INVALID_PARAMETER.

4.3.5.2.1.2 RangeLength Behavior:

If RangeStart is zero, then
	[ LengthAlignment = (RangeLength modulo AlignmentGranularity) - LowestAlignedLBA ]

If RangeStart is non-zero, then
	[ LengthAlignment = (RangeLength modulo AlignmentGranularity) ]

When processing a Set method or CreateRow method on the Locking
table for a non-Global Range row, if:

a) the AlignmentRequired (ALIGN above) column in the LockingInfo
   table is TRUE;
b) RangeLength is non-zero; and
c) LengthAlignment is non-zero, then the method SHALL fail and
   return an error status code INVALID_PARAMETER

In userspace we stuck to logical block size reported by general
block device (via sysfs or ioctl), but we can not read
'AlignmentGranularity' or 'LowestAlignedLBA' anywhere else and
we need to get those values from sed-opal interface otherwise
we will not be able to report or avoid locking range setup
INVALID_PARAMETER errors above.

Signed-off-by: Ondrej Kozina <okozina@redhat.com>
---
 block/sed-opal.c              | 29 ++++++++++++++++++++++++++++-
 include/linux/sed-opal.h      |  1 +
 include/uapi/linux/sed-opal.h | 13 +++++++++++++
 3 files changed, 42 insertions(+), 1 deletion(-)

diff --git a/block/sed-opal.c b/block/sed-opal.c
index 3fc4e65db111..c18339446ef3 100644
--- a/block/sed-opal.c
+++ b/block/sed-opal.c
@@ -83,8 +83,10 @@ struct opal_dev {
 	u16 comid;
 	u32 hsn;
 	u32 tsn;
-	u64 align;
+	u64 align; /* alignment granularity */
 	u64 lowest_lba;
+	u32 logical_block_size;
+	u8  align_required; /* ALIGN: 0 or 1 */
 
 	size_t pos;
 	u8 *cmd;
@@ -409,6 +411,8 @@ static void check_geometry(struct opal_dev *dev, const void *data)
 
 	dev->align = be64_to_cpu(geo->alignment_granularity);
 	dev->lowest_lba = be64_to_cpu(geo->lowest_aligned_lba);
+	dev->logical_block_size = be32_to_cpu(geo->logical_block_size);
+	dev->align_required = geo->reserved01 & 1;
 }
 
 static int execute_step(struct opal_dev *dev,
@@ -2956,6 +2960,26 @@ static int opal_get_status(struct opal_dev *dev, void __user *data)
 	return 0;
 }
 
+static int opal_get_geometry(struct opal_dev *dev, void __user *data)
+{
+	struct opal_geometry geo = {0};
+
+	if (check_opal_support(dev))
+		return -EINVAL;
+
+	geo.align = dev->align_required;
+	geo.logical_block_size = dev->logical_block_size;
+	geo.alignment_granularity =  dev->align;
+	geo.lowest_aligned_lba = dev->lowest_lba;
+
+	if (copy_to_user(data, &geo, sizeof(geo))) {
+		pr_debug("Error copying geometry data to userspace\n");
+		return -EFAULT;
+	}
+
+	return 0;
+}
+
 int sed_ioctl(struct opal_dev *dev, unsigned int cmd, void __user *arg)
 {
 	void *p;
@@ -3029,6 +3053,9 @@ int sed_ioctl(struct opal_dev *dev, unsigned int cmd, void __user *arg)
 	case IOC_OPAL_GET_LR_STATUS:
 		ret = opal_locking_range_status(dev, p, arg);
 		break;
+	case IOC_OPAL_GET_GEOMETRY:
+		ret = opal_get_geometry(dev, arg);
+		break;
 	default:
 		break;
 	}
diff --git a/include/linux/sed-opal.h b/include/linux/sed-opal.h
index 042c1e2cb0ce..bbae1e52ab4f 100644
--- a/include/linux/sed-opal.h
+++ b/include/linux/sed-opal.h
@@ -46,6 +46,7 @@ static inline bool is_sed_ioctl(unsigned int cmd)
 	case IOC_OPAL_GENERIC_TABLE_RW:
 	case IOC_OPAL_GET_STATUS:
 	case IOC_OPAL_GET_LR_STATUS:
+	case IOC_OPAL_GET_GEOMETRY:
 		return true;
 	}
 	return false;
diff --git a/include/uapi/linux/sed-opal.h b/include/uapi/linux/sed-opal.h
index 3905c8ffedbf..dc2efd345133 100644
--- a/include/uapi/linux/sed-opal.h
+++ b/include/uapi/linux/sed-opal.h
@@ -161,6 +161,18 @@ struct opal_status {
 	__u32 reserved;
 };
 
+/*
+ * Geometry Reporting per TCG Storage OPAL SSC
+ * section 3.1.1.4
+ */
+struct opal_geometry {
+	__u8 align;
+	__u32 logical_block_size;
+	__u64 alignment_granularity;
+	__u64 lowest_aligned_lba;
+	__u8  __align[3];
+};
+
 #define IOC_OPAL_SAVE		    _IOW('p', 220, struct opal_lock_unlock)
 #define IOC_OPAL_LOCK_UNLOCK	    _IOW('p', 221, struct opal_lock_unlock)
 #define IOC_OPAL_TAKE_OWNERSHIP	    _IOW('p', 222, struct opal_key)
@@ -179,5 +191,6 @@ struct opal_status {
 #define IOC_OPAL_GENERIC_TABLE_RW   _IOW('p', 235, struct opal_read_write_table)
 #define IOC_OPAL_GET_STATUS         _IOR('p', 236, struct opal_status)
 #define IOC_OPAL_GET_LR_STATUS      _IOW('p', 237, struct opal_lr_status)
+#define IOC_OPAL_GET_GEOMETRY       _IOR('p', 238, struct opal_geometry)
 
 #endif /* _UAPI_SED_OPAL_H */
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 1/1] sed-opal: geometry feature reporting command
  2023-04-11  9:09   ` [PATCH v2 1/1] " Ondrej Kozina
@ 2023-04-11 12:31     ` Christoph Hellwig
  2023-04-11 13:28     ` Christian Brauner
  2023-04-11 14:46     ` Milan Broz
  2 siblings, 0 replies; 11+ messages in thread
From: Christoph Hellwig @ 2023-04-11 12:31 UTC (permalink / raw)
  To: Ondrej Kozina
  Cc: linux-block, bluca, gmazyland, axboe, hch, brauner, jonathan.derrick

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 1/1] sed-opal: geometry feature reporting command
  2023-04-11  9:09   ` [PATCH v2 1/1] " Ondrej Kozina
  2023-04-11 12:31     ` Christoph Hellwig
@ 2023-04-11 13:28     ` Christian Brauner
  2023-04-11 14:46     ` Milan Broz
  2 siblings, 0 replies; 11+ messages in thread
From: Christian Brauner @ 2023-04-11 13:28 UTC (permalink / raw)
  To: Ondrej Kozina; +Cc: linux-block, bluca, gmazyland, axboe, hch, jonathan.derrick

On Tue, Apr 11, 2023 at 11:09:31AM +0200, Ondrej Kozina wrote:
> Locking range start and locking range length
> attributes may be require to satisfy restrictions
> exposed by OPAL2 geometry feature reporting.
> 
> Geometry reporting feature is described in TCG OPAL SSC,
> section 3.1.1.4 (ALIGN, LogicalBlockSize, AlignmentGranularity
> and LowestAlignedLBA).
> 
> 4.3.5.2.1.1 RangeStart Behavior:
> 
> [ StartAlignment = (RangeStart modulo AlignmentGranularity) - LowestAlignedLBA ]
> 
> When processing a Set method or CreateRow method on the Locking
> table for a non-Global Range row, if:
> 
> a) the AlignmentRequired (ALIGN above) column in the LockingInfo
>    table is TRUE;
> b) RangeStart is non-zero; and
> c) StartAlignment is non-zero, then the method SHALL fail and
>    return an error status code INVALID_PARAMETER.
> 
> 4.3.5.2.1.2 RangeLength Behavior:
> 
> If RangeStart is zero, then
> 	[ LengthAlignment = (RangeLength modulo AlignmentGranularity) - LowestAlignedLBA ]
> 
> If RangeStart is non-zero, then
> 	[ LengthAlignment = (RangeLength modulo AlignmentGranularity) ]
> 
> When processing a Set method or CreateRow method on the Locking
> table for a non-Global Range row, if:
> 
> a) the AlignmentRequired (ALIGN above) column in the LockingInfo
>    table is TRUE;
> b) RangeLength is non-zero; and
> c) LengthAlignment is non-zero, then the method SHALL fail and
>    return an error status code INVALID_PARAMETER
> 
> In userspace we stuck to logical block size reported by general
> block device (via sysfs or ioctl), but we can not read
> 'AlignmentGranularity' or 'LowestAlignedLBA' anywhere else and
> we need to get those values from sed-opal interface otherwise
> we will not be able to report or avoid locking range setup
> INVALID_PARAMETER errors above.
> 
> Signed-off-by: Ondrej Kozina <okozina@redhat.com>
> ---
>  block/sed-opal.c              | 29 ++++++++++++++++++++++++++++-
>  include/linux/sed-opal.h      |  1 +
>  include/uapi/linux/sed-opal.h | 13 +++++++++++++
>  3 files changed, 42 insertions(+), 1 deletion(-)
> 
> diff --git a/block/sed-opal.c b/block/sed-opal.c
> index 3fc4e65db111..c18339446ef3 100644
> --- a/block/sed-opal.c
> +++ b/block/sed-opal.c
> @@ -83,8 +83,10 @@ struct opal_dev {
>  	u16 comid;
>  	u32 hsn;
>  	u32 tsn;
> -	u64 align;
> +	u64 align; /* alignment granularity */
>  	u64 lowest_lba;
> +	u32 logical_block_size;
> +	u8  align_required; /* ALIGN: 0 or 1 */
>  
>  	size_t pos;
>  	u8 *cmd;
> @@ -409,6 +411,8 @@ static void check_geometry(struct opal_dev *dev, const void *data)
>  
>  	dev->align = be64_to_cpu(geo->alignment_granularity);
>  	dev->lowest_lba = be64_to_cpu(geo->lowest_aligned_lba);
> +	dev->logical_block_size = be32_to_cpu(geo->logical_block_size);
> +	dev->align_required = geo->reserved01 & 1;

This is really ugly. Both the naming of the variable and testing against
an unnamed constant. I'd prefer if that could be fixed but otherwise,

Reviewed-by: Christian Brauner <brauner@kernel.org>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 1/1] sed-opal: geometry feature reporting command
  2023-04-11  9:09   ` [PATCH v2 1/1] " Ondrej Kozina
  2023-04-11 12:31     ` Christoph Hellwig
  2023-04-11 13:28     ` Christian Brauner
@ 2023-04-11 14:46     ` Milan Broz
  2 siblings, 0 replies; 11+ messages in thread
From: Milan Broz @ 2023-04-11 14:46 UTC (permalink / raw)
  To: Ondrej Kozina, linux-block; +Cc: bluca, axboe, hch, brauner, jonathan.derrick

On 4/11/23 11:09, Ondrej Kozina wrote:
> Locking range start and locking range length
> attributes may be require to satisfy restrictions
> exposed by OPAL2 geometry feature reporting.

...
> Signed-off-by: Ondrej Kozina <okozina@redhat.com>

v2 works on all of my SATA/NVMe OPAL drives, so

Tested-by: Milan Broz <gmazyland@gmail.com>

m.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 0/1] sed-opal: geometry feature reporting command
  2023-04-11  9:09 ` [PATCH v2 0/1] " Ondrej Kozina
  2023-04-11  9:09   ` [PATCH v2 1/1] " Ondrej Kozina
@ 2023-04-19  7:33   ` Ondrej Kozina
  2023-04-19 20:07   ` Jens Axboe
  2 siblings, 0 replies; 11+ messages in thread
From: Ondrej Kozina @ 2023-04-19  7:33 UTC (permalink / raw)
  To: axboe; +Cc: bluca, gmazyland, hch, brauner, jonathan.derrick, linux-block

On 11. 04. 23 11:09, Ondrej Kozina wrote:
>    sed-opal: geometry feature reporting command
> 

Hi Jens,

could we get this in 6.4? It's the last kernel bit we'd like to have to 
move further with userspace code discussed here:
https://gitlab.com/cryptsetup/cryptsetup/-/merge_requests/461

Kind regards
Ondrej


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 0/1] sed-opal: geometry feature reporting command
  2023-04-11  9:09 ` [PATCH v2 0/1] " Ondrej Kozina
  2023-04-11  9:09   ` [PATCH v2 1/1] " Ondrej Kozina
  2023-04-19  7:33   ` [PATCH v2 0/1] " Ondrej Kozina
@ 2023-04-19 20:07   ` Jens Axboe
  2 siblings, 0 replies; 11+ messages in thread
From: Jens Axboe @ 2023-04-19 20:07 UTC (permalink / raw)
  To: linux-block, Ondrej Kozina
  Cc: bluca, gmazyland, hch, brauner, jonathan.derrick


On Tue, 11 Apr 2023 11:09:30 +0200, Ondrej Kozina wrote:
> While further testing various OPAL drives we have discovered that
> some drives may impose alignment requirements that can not be
> satisfied without reading some aditional parameters reported
> by sed-opal iface.
> 
> We used to stick to physical block size reported by general block
> device in place of LogicalBlockSize as reported by opal 'geometry',
> but at least 2 aditional restrictions can not be easily mapped to
> anything currently reported by block layer.
> 
> [...]

Applied, thanks!

[1/1] sed-opal: geometry feature reporting command
      commit: 9e05a2599a37295eb2dc5c03441daa6741abed4b

Best regards,
-- 
Jens Axboe




^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2023-04-19 20:07 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-04-06 13:19 [PATCH 0/1] sed-opal: geometry feature reporting command Ondrej Kozina
2023-04-06 13:19 ` [PATCH 1/1] " Ondrej Kozina
2023-04-06 20:32   ` Milan Broz
2023-04-11  9:05     ` Ondrej Kozina
2023-04-11  9:09 ` [PATCH v2 0/1] " Ondrej Kozina
2023-04-11  9:09   ` [PATCH v2 1/1] " Ondrej Kozina
2023-04-11 12:31     ` Christoph Hellwig
2023-04-11 13:28     ` Christian Brauner
2023-04-11 14:46     ` Milan Broz
2023-04-19  7:33   ` [PATCH v2 0/1] " Ondrej Kozina
2023-04-19 20:07   ` Jens Axboe

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.