On 6/1/22 02:50, Tyler Erickson wrote:
> From: Tyler Erickson <tyler.j.erickson@xxxxxxxxxxx>
>
> The concurrent positioning ranges log is not a fixed size and may depend
> on how many ranges are supported by the device. This patch uses the size
> reported in the GPL directory to determine the number of pages supported
> by the device before attempting to read this log page.
>
> Also fixing the page length in the SCSI translation for the concurrent
> positioning ranges VPD page.
>
> This resolves this error from the dmesg output:
> ata6.00: Read log 0x47 page 0x00 failed, Emask 0x1
>
> Signed-off-by: Tyler Erickson <tyler.j.erickson@xxxxxxxxxxx>
> Reviewed-by: Muhammad Ahmad <muhammad.ahmad@xxxxxxxxxxx>
> Tested-by: Michael English <michael.english@xxxxxxxxxxx>
This needs a Fixes tag and cc stable.
>
> diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
> index ca64837641be..3d57fa84e2be 100644
> --- a/drivers/ata/libata-core.c
> +++ b/drivers/ata/libata-core.c
> @@ -2003,16 +2003,16 @@ unsigned int ata_read_log_page(struct ata_device *dev, u8 log,
> return err_mask;
> }
>
> -static bool ata_log_supported(struct ata_device *dev, u8 log)
> +static int ata_log_supported(struct ata_device *dev, u8 log)
> {
> struct ata_port *ap = dev->link->ap;
>
> if (dev->horkage & ATA_HORKAGE_NO_LOG_DIR)
> - return false;
> + return 0;
>
> if (ata_read_log_page(dev, ATA_LOG_DIRECTORY, 0, ap->sector_buf, 1))
> - return false;
> - return get_unaligned_le16(&ap->sector_buf[log * 2]) ? true : false;
> + return 0;
> + return get_unaligned_le16(&ap->sector_buf[log * 2]);
> }
>
> static bool ata_identify_page_supported(struct ata_device *dev, u8 page)
> @@ -2448,15 +2448,20 @@ static void ata_dev_config_cpr(struct ata_device *dev)
> struct ata_cpr_log *cpr_log = NULL;
> u8 *desc, *buf = NULL;
>
> - if (ata_id_major_version(dev->id) < 11 ||
> - !ata_log_supported(dev, ATA_LOG_CONCURRENT_POSITIONING_RANGES))
> + if (ata_id_major_version(dev->id) < 11)
> + goto out;> +
> + buf_len = ata_log_supported(dev, ATA_LOG_CONCURRENT_POSITIONING_RANGES);
> + if (buf_len == 0)
> goto out;
>
> /*
> * Read the concurrent positioning ranges log (0x47). We can have at
> - * most 255 32B range descriptors plus a 64B header.
> + * most 255 32B range descriptors plus a 64B header. This log varies in
> + * size, so use the size reported in the GPL directory. Reading beyond
> + * the supported length will result in an error.
> */
> - buf_len = (64 + 255 * 32 + 511) & ~511;
> + buf_len <<= 9;
> buf = kzalloc(buf_len, GFP_KERNEL);
> if (!buf)
> goto out;
> diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
> index 06c9d90238d9..0ea9c3115529 100644
> --- a/drivers/ata/libata-scsi.c
> +++ b/drivers/ata/libata-scsi.c
> @@ -2101,7 +2101,7 @@ static unsigned int ata_scsiop_inq_b9(struct ata_scsi_args *args, u8 *rbuf)
>
> /* SCSI Concurrent Positioning Ranges VPD page: SBC-5 rev 1 or later */
> rbuf[1] = 0xb9;
> - put_unaligned_be16(64 + (int)cpr_log->nr_cpr * 32 - 4, &rbuf[3]);
> + put_unaligned_be16(64 + (int)cpr_log->nr_cpr * 32 - 4, &rbuf[2]);
>
> for (i = 0; i < cpr_log->nr_cpr; i++, desc += 32) {
> desc[0] = cpr_log->cpr[i].num;
This scsi hunk needs to be a different patch with a fixes tag and cc stable.
--
Damien Le Moal
Western Digital Research
