Re: [PATCH 1/1] sed-opal: geometry feature reporting command

[Milan Broz <gmazyland@xxxxxxxxx>]

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 */