On 09/20/2012 09:53 PM, Martin K. Petersen wrote:
>>>>>> "Hannes" == Hannes Reinecke <hare@xxxxxxx> writes:
>
> Hannes> I recently got my hands on some weird drives, insisting on
> Hannes> having been formatted with protection type 7:
>
> Lovely :|
>
>
> Hannes> I've attached a tentative patch, which allows the system to
> Hannes> boot. However, I'm not completely happy with that, as the
> Hannes> capacity is _still_ updated after revalidation:
>
> How about this?
>
>
> sd: Ensure we correctly disable devices with unknown protection type
>
> We set the capacity to zero when we discovered a device formatted with
> an unknown DIF protection type. However, the read_capacity code would
> override the capacity and cause the device to be enabled regardless.
>
> Make sd_read_protection_type() return an error if the protection type is
> unknown. Also prevent duplicate printk lines when the device is being
> revalidated.
>
> Reported-by: Hannes Reinecke <hare@xxxxxxx>
> Signed-off-by: Martin K. Petersen <martin.petersen@xxxxxxxxxx>
>
> diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
> index 1cf2d5d..1c54564 100644
> --- a/drivers/scsi/sd.c
> +++ b/drivers/scsi/sd.c
> @@ -1820,34 +1820,42 @@ sd_spinup_disk(struct scsi_disk *sdkp)
> /*
> * Determine whether disk supports Data Integrity Field.
> */
> -static void sd_read_protection_type(struct scsi_disk *sdkp, unsigned char *buffer)
> +static int sd_read_protection_type(struct scsi_disk *sdkp, unsigned char *buffer)
> {
> struct scsi_device *sdp = sdkp->device;
> u8 type;
> + int ret = 0;
>
> if (scsi_device_protection(sdp) == 0 || (buffer[12] & 1) == 0)
> - return;
> + return ret;
>
> type = ((buffer[12] >> 1) & 7) + 1; /* P_TYPE 0 = Type 1 */
>
> - if (type == sdkp->protection_type || !sdkp->first_scan)
> - return;
> + if (type > SD_DIF_TYPE3_PROTECTION)
> + ret = -ENODEV;
> + else if (scsi_host_dif_capable(sdp->host, type))
> + ret = 1;
> +
> + if (sdkp->first_scan || type != sdkp->protection_type)
> + switch (ret) {
> + case -ENODEV:
> + sd_printk(KERN_ERR, sdkp, "formatted with unsupported" \
> + " protection type %u. Disabling disk!\n",
> + type);
> + break;
> + case 1:
> + sd_printk(KERN_NOTICE, sdkp,
> + "Enabling DIF Type %u protection\n", type);
> + break;
> + case 0:
> + sd_printk(KERN_NOTICE, sdkp,
> + "Disabling DIF Type %u protection\n", type);
> + break;
> + }
>
> sdkp->protection_type = type;
>
> - if (type > SD_DIF_TYPE3_PROTECTION) {
> - sd_printk(KERN_ERR, sdkp, "formatted with unsupported " \
> - "protection type %u. Disabling disk!\n", type);
> - sdkp->capacity = 0;
> - return;
> - }
> -
> - if (scsi_host_dif_capable(sdp->host, type))
> - sd_printk(KERN_NOTICE, sdkp,
> - "Enabling DIF Type %u protection\n", type);
> - else
> - sd_printk(KERN_NOTICE, sdkp,
> - "Disabling DIF Type %u protection\n", type);
> + return ret;
> }
>
> static void read_capacity_error(struct scsi_disk *sdkp, struct scsi_device *sdp,
> @@ -1943,7 +1951,10 @@ static int read_capacity_16(struct scsi_disk *sdkp, struct scsi_device *sdp,
> sector_size = get_unaligned_be32(&buffer[8]);
> lba = get_unaligned_be64(&buffer[0]);
>
> - sd_read_protection_type(sdkp, buffer);
> + if (sd_read_protection_type(sdkp, buffer) < 0) {
> + sdkp->capacity = 0;
> + return -ENODEV;
> + }
>
> if ((sizeof(sdkp->capacity) == 4) && (lba >= 0xffffffffULL)) {
> sd_printk(KERN_ERR, sdkp, "Too big for this kernel. Use a "
Hehe. It helps to actually test patches.
Now I get:
sd 6:0:0:0: [sdb] formatted with unsupported protection type 7.
Disabling disk!
and silence after that.
However, I still get:
sd 6:0:0:0: [sdb] Write Protect is off
sd 6:0:0:0: [sdb] Mode Sense: cf 00 10 08
sd 6:0:0:0: [sdb] Write cache: disabled, read cache: enabled,
supports DPO and FUA
sd 6:0:0:0: [sdb] Enabling DIX T10-DIF-TYPE1-CRC protection
sd 6:0:0:0: [sdb] Attached SCSI disk
which is a bit odd, given that the disk is disabled.
Plus that we never actually claimed to support type1.
So maybe we should add this patch, too?
diff --git a/drivers/scsi/sd_dif.c b/drivers/scsi/sd_dif.c
index 0cb39ff..e0500bf 100644
--- a/drivers/scsi/sd_dif.c
+++ b/drivers/scsi/sd_dif.c
@@ -320,7 +320,7 @@ void sd_dif_config_host(struct scsi_disk *sdkp)
dif = 0; dix = 1;
}
- if (!dix)
+ if (!dix || type > SD_DIF_TYPE3_PROTECTION)
return;
/* Enable DMA of protection information */
But then, main issue is resolved, as the system continues to boot.
So my Acked-by: still stands.
Cheers,
Hannes
--
Dr. Hannes Reinecke zSeries & Storage
hare@xxxxxxx +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
