Re: [PATCH u-boot 5/8] scsi: ufs: refuse erase/UNMAP on NO_PURGE devices

[Neil Armstrong <neil.armstrong@linaro.org>]

Hi,

On 10/4/26 21:22, Petr Hodina via B4 Relay wrote:
> From: Petr Hodina <petr.hodina@protonmail.com>
> 
> Enforce the UFS_DEVICE_QUIRK_NO_PURGE quirk on U-Boot's only path that
> can reach a UFS device with a destructive UNMAP: the block-device erase
> op (scsi_erase() -> SCSI_UNMAP). On Sony Yoshino/Tama a PURGE/UNMAP
> erases the bootloader and permanently bricks the device.
> 
> Signed-off-by: Petr Hodina <petr.hodina@protonmail.com>
> ---
>   drivers/scsi/scsi.c      |  5 +++++
>   drivers/ufs/ufs-uclass.c | 12 ++++++++++++
>   include/scsi.h           |  5 +++++
>   3 files changed, 22 insertions(+)
> 
> diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
> index 50e7d749921..ba7834ea035 100644
> --- a/drivers/scsi/scsi.c
> +++ b/drivers/scsi/scsi.c
> @@ -300,6 +300,11 @@ static ulong scsi_erase(struct udevice *dev, lbaint_t blknr, lbaint_t blkcnt)
>   	lbaint_t start, blks, max_blks, blocks;
>   	struct scsi_cmd *pccb = (struct scsi_cmd *)&tempccb;
>   
> +	if (uc_plat->no_erase) {
> +		printf("%s: erase not supported on this device\n", __func__);
> +		return 0;
> +	}
> +
>   	/* Setup device */
>   	pccb->target = block_dev->target;
>   	pccb->lun = block_dev->lun;
> diff --git a/drivers/ufs/ufs-uclass.c b/drivers/ufs/ufs-uclass.c
> index 977ffebc21f..7dfba3ac1ac 100644
> --- a/drivers/ufs/ufs-uclass.c
> +++ b/drivers/ufs/ufs-uclass.c
> @@ -2366,6 +2366,18 @@ int ufshcd_probe(struct udevice *ufs_dev, struct ufs_hba_ops *hba_ops)
>   	if (err)
>   		return err;
>   
> +	/*
> +	 * Some devices must never receive a PURGE / UNMAP (secure erase): on
> +	 * the affected platforms it erases the bootloader and permanently
> +	 * bricks the device. Disable the SCSI erase op for the whole UFS
> +	 * device in that case.
> +	 */
> +	if (hba->dev_quirks & UFS_DEVICE_QUIRK_NO_PURGE) {
> +		scsi_plat->no_erase = true;
> +		dev_info(hba->dev,
> +			 "NO_PURGE quirk active: SCSI erase/UNMAP disabled\n");
> +	}
> +
>   	return 0;
>   }
>   
> diff --git a/include/scsi.h b/include/scsi.h
> index 2520a8b8fe6..fc296285dd5 100644
> --- a/include/scsi.h
> +++ b/include/scsi.h
> @@ -275,6 +275,11 @@ struct scsi_plat {
>   	unsigned long max_lun;
>   	unsigned long max_id;
>   	unsigned long max_bytes_per_req;
> +	/*
> +	 * Set by the controller driver to forbid the erase (UNMAP) operation,
> +	 * e.g. for UFS devices whose PURGE/UNMAP would erase the bootloader.
> +	 */
> +	bool no_erase;

To avoid proliferation of variables here, could you add a quirks variable instead and
move the no_erase into a SCSI quirk define.

Thanks,
Neil

>   };
>   
>   /* Operations for SCSI */
>