* [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.