Re: [PATCH 6/8] pm80xx: do not examine registers for iButton feature if ATTO adapter
On 10/30/2015 03:53 PM, Benjamin Rood wrote: > ATTO adapters do not support this feature. If the firmware fails to be > ready, it should not check the examined registers in order to examine > the state of the feature in order to prevent undefined behavior. > > Signed-off-by: Benjamin Rood > --- > drivers/scsi/pm8001/pm80xx_hwi.c | 2 ++ > 1 file changed, 2 insertions(+) > Reviewed-by: Hannes Reinecke Cheers, Hannes -- Dr. Hannes Reinecke zSeries & Storage h...@suse.de +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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 5/8] pm80xx: set PHY profiles for ATTO 12Gb SAS controllers
On 10/30/2015 03:53 PM, Benjamin Rood wrote: > PHY profiles are not saved in NVRAM on ATTO 12Gb SAS controllers. > Therefore, in order for the controller to function in a wide range of > configurations, the PHY profiles must be statically set. This patch > provides the necessary functionality to do so. > > Signed-off-by: Benjamin Rood > --- > drivers/scsi/pm8001/pm8001_init.c | 130 > ++ > drivers/scsi/pm8001/pm8001_sas.h | 2 + > drivers/scsi/pm8001/pm80xx_hwi.c | 32 ++ > 3 files changed, 164 insertions(+) > Hmm. Can't say I like compiling in magic constants, but I guess there's no easy way out here. So: Reviewed-by: Hannes Reinecke Cheers, Hannes -- Dr. Hannes Reinecke zSeries & Storage h...@suse.de +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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/8] pm80xx: add support for ATTO devices during SAS address initiailization
On 10/30/2015 03:53 PM, Benjamin Rood wrote: > ATTO SAS controllers retrieve the SAS address from the NVRAM in a location > different from non-ATTO PMC Sierra SAS controllers. This patch makes the > necessary adjustments in order to retrieve the SAS address on these types > of adapters. > > Signed-off-by: Benjamin Rood > --- > drivers/scsi/pm8001/pm8001_init.c | 10 ++ > 1 file changed, 10 insertions(+) > > diff --git a/drivers/scsi/pm8001/pm8001_init.c > b/drivers/scsi/pm8001/pm8001_init.c > index feaf504..fdbfab6 100644 > --- a/drivers/scsi/pm8001/pm8001_init.c > +++ b/drivers/scsi/pm8001/pm8001_init.c > @@ -636,6 +636,11 @@ static void pm8001_init_sas_add(struct pm8001_hba_info > *pm8001_ha) > payload.minor_function = 0; > payload.length = 128; > } > + } else if ((pm8001_ha->chip_id == chip_8070 || > + pm8001_ha->chip_id == chip_8072) && > + pm8001_ha->pdev->subsystem_vendor == PCI_VENDOR_ID_ATTO) { > + payload.minor_function = 4; > + payload.length = 4096; > } else { > payload.minor_function = 1; > payload.length = 4096; > @@ -662,6 +667,11 @@ static void pm8001_init_sas_add(struct pm8001_hba_info > *pm8001_ha) > else if (deviceid == 0x0042) > pm8001_ha->sas_addr[j] = > payload.func_specific[0x010 + i]; > + } else if ((pm8001_ha->chip_id == chip_8070 || > + pm8001_ha->chip_id == chip_8072) && > + pm8001_ha->pdev->subsystem_vendor == PCI_VENDOR_ID_ATTO) { > + pm8001_ha->sas_addr[j] = > + payload.func_specific[0x010 + i]; > } else > pm8001_ha->sas_addr[j] = > payload.func_specific[0x804 + i]; > The indentation is a bit skewed here. Other than that: Reviewed-by: Hannes Reinecke Cheers, Hannes -- Dr. Hannes Reinecke zSeries & Storage h...@suse.de +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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/8] pm80xx: add ATTO PCI IDs to pm8001_pci_table
On 10/30/2015 03:53 PM, Benjamin Rood wrote: > These PCI IDs allow the pm8001 driver to load against ATTO 12Gb SAS > controllers that use PMC Sierra 8070 and PMC Sierra 8072 SAS chips. > > Signed-off-by: Benjamin Rood > --- > drivers/scsi/pm8001/pm8001_init.c | 14 ++ > 1 file changed, 14 insertions(+) > Reviewed-by: Hannes Reinecke Cheers, Hannes -- Dr. Hannes Reinecke zSeries & Storage h...@suse.de +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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/8] pm80xx: configure PHY settings based on subsystem vendor ID
On 10/30/2015 03:53 PM, Benjamin Rood wrote: > Previuosly, all PMC Sierra 80xx controllers are assumed to be a > motherboard controller, except if the subsystem vendor ID was equal to > PCI_VENDOR_ID_ADAPTEC. The driver then attempts to load PHY settings > from NVRAM. While this may be correct behavior for most controllers, it > does not work with Adaptec and ATTO controllers since they do not store > PHY settings in NVRAM and choose to use either custom PHY settings or chip > defaults. Loading random values from NVRAM may cause the controllers to > malfunction in this edge case. > > Signed-off-by: Benjamin Rood > --- > drivers/scsi/pm8001/pm8001_init.c | 26 -- > 1 file changed, 20 insertions(+), 6 deletions(-) > Reviewed-by: Hannes Reinecke Cheers, Hannes -- Dr. Hannes Reinecke zSeries & Storage h...@suse.de +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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/8] pm80xx: add support for PMC Sierra 8070 and PMC Sierra 8072 SAS controllers
On 10/30/2015 03:53 PM, Benjamin Rood wrote: > These SAS controllers support speeds up to 12Gb. > > Signed-off-by: Benjamin Rood > --- > drivers/scsi/pm8001/pm8001_defs.h | 2 ++ > drivers/scsi/pm8001/pm8001_init.c | 4 +++- > drivers/scsi/pm8001/pm8001_sas.h | 4 +++- > 3 files changed, 8 insertions(+), 2 deletions(-) > Reviewed-by: Hannes Reinecke Cheers, Hannes -- Dr. Hannes Reinecke zSeries & Storage h...@suse.de +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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RESEND PATCH v3] scsi: stex: Remove use of struct timeval
On 10/30/2015 01:45 PM, Arnd Bergmann wrote: > On Friday 30 October 2015 12:58:33 Hannes Reinecke wrote: >>> @@ -1076,7 +1069,7 @@ static int stex_ss_handshake(struct st_hba *hba) >>> h->req_cnt = cpu_to_le16(hba->rq_count+1); >>> h->status_sz = cpu_to_le16(sizeof(struct status_msg)); >>> h->status_cnt = cpu_to_le16(hba->sts_count+1); >>> - stex_gettime(&h->hosttime); >>> + h->hosttime = cpu_to_le64(ktime_get_real_seconds()); >>> h->partner_type = HMU_PARTNER_TYPE; >>> h->extra_offset = h->extra_size = 0; >>> scratch_size = (hba->sts_count+1)*sizeof(u32); >>> >> Just remove 'hosttime' altogether. It serves no purpose whatsoever. >> >> > > Are you sure? It is defined in a struct that is shared with the HBA > as part of hba->dma_mem, so unless you have access to the firmware, > it's hard to guess what it might be used for inside of the firmware. > Ah, you're right. Okay, so we'll need to convert it. Cheers, Hannes -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: IBM request to allow unprivledged ioctls [Was: Revert "dm mpath: fix stalls when handling invalid ioctls"]
On 10/31/2015 11:47 PM, Mike Snitzer wrote: > On Sat, Oct 31 2015 at 3:07pm -0400, > Paolo Bonzini wrote: > >> >> >> On 31/10/2015 19:13, Mike Snitzer wrote: But that's wrong, I think. It's a false positive in scsi_verify_blk_ioctl(). If the ioctl is valid when bdev becomes non-NULL (and it will be if ti->len becomes equal to i_size_read(bdev->bd_inode) >> SECTOR_SHIFT), you should not return -ENOIOCTLCMD aka ENOTTY, because userspace doesn't think the ioctls can go away and come back. So Hannes's patch broke the userspace ABI. :( >>> >>> Huh? All that Hannes' patch did was add early verification of the ioctl >>> if there are no paths, since: there is no point queueing an ioctl that >>> is invalid. >>> >>> [snip discussion of Christoph's patches] >>> >>> The point is scsi_verify_blk_ioctl() is saying the ioctl isn't valid. >>> It has nothing to do with the existance of a bdev or not; but everything >>> to do with the unprivledged user's request to issue an ioctl. >> >> ... but the call is skipped (and all ioctls are valid) if ti->len == >> i_size_read(bdev->bd_inode) >> SECTOR_SHIFT. Therefore, until you have >> the bdev you don't know which ioctls are valid, and you must assume all >> of them are. You can't do anything unsafe anyway until you have the >> bdev. This is the reasoning prior to Hannes's change. > > Yes, with your commit ec8013be ("dm: do not forward ioctls from logical > volumes to the underlying device") you added protections to disallow > issuing ioctls to a partition that could impact the rest of the device. > > Given that I can see why you're seizing on the "ti->len != > i_size_read(bdev->bd_inode) >> SECTOR_SHIFT" negative checks that gate > the call to scsi_verify_blk_ioctl(). > >> Afterwards, you end up calling scsi_verify_blk_ioctl() when bdev == >> NULL. If the future bdev satisfies the above condition on ti->len, this >> means that ioctl(SG_IO) switches from ENOTTY to available. Userspace is >> clearly not expecting that. > > For Hannes, and in my head, it didn't matter if a future bdev satisfies > the length condition. I don't think Hannes was trying to guard against > dangerous partition ioctls being issues by udev... but now I _do_ > question what exactly _is_ the point of his commit 21a2807bc3f. Which > invalid ioctls, from udev, did Hannes' change actually disallow? > The reasoning is thus: With the original code we would need to wait for path activation before we would be able to figure out if the ioctl is valid. However, the callback to verify the ioctl is sd_ioctl(), which as a first step calls scsi_verify_ioctl(). So my reasoning was that we can as well call scsi_verify_ioctl() early, and allow it to filter out known invalid ioctls. Then we wouldn't need to wait for path activation and return immediately. Incidentally, in the 'r == -ENOTCONN' case, we're waiting for path activation, but then just bail out with -ENOTCONN. As we're not resetting -ENOTCONN, where's the point in activate the paths here? Shouldn't we retry to figure out if -ENOTCONN is still true? Cheers, Hannes -- Dr. Hannes Reinecke zSeries & Storage h...@suse.de +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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4.3-rc7] be2iscsi : Fix bogus WARN_ON length check
On 10/30/2015 02:59 PM, Johannes Thumshirn wrote: > Hi Tim, > tim.gard...@canonical.com writes: > >> From: Tim Gardner >> >> drivers/scsi/be2iscsi/be_main.c: In function 'be_sgl_create_contiguous': >> drivers/scsi/be2iscsi/be_main.c:3187:18: warning: logical not is only >> applied to the left hand side of comparison [-Wlogical-not-parentheses] >> WARN_ON(!length > 0); >> >> gcc version 5.2.1 > > This patch (or similar) was already posted on Oct 1 by Joel Stanley. > See http://comments.gmane.org/gmane.linux.scsi/105462 > > Thanks, > Johannes > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ > Mechanical application of prarens makes that expression more complicated then it needs to be. It is, after all, an unsigned integer. rtg -- Tim Gardner tim.gard...@canonical.com -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 01/26] scsi/atari_scsi: Dont select CONFIG_NVRAM
On powerpc, setting CONFIG_NVRAM=n builds a kernel with no NVRAM support. Setting CONFIG_NVRAM=m enables the /dev/nvram misc device module without enabling NVRAM support in drivers. Setting CONFIG_NVRAM=y enables the misc device (built-in) and also enables NVRAM support in drivers. m68k shares the valkyriefb driver with powerpc, and since that driver uses NVRAM, it is affected by CONFIG_ATARI_SCSI, because of the use of "select NVRAM". Adopt the powerpc convention on m68k to avoid surprises. Signed-off-by: Finn Thain Tested-by: Christian T. Steigies --- This patch temporarily disables CONFIG_NVRAM on Atari, to prevent build failures when bisecting the rest of this patch series. It gets enabled again with the introduction of CONFIG_HAVE_ARCH_NVRAM_OPS, once the nvram_* global functions have been moved to an ops struct. The removal of "select NVRAM" may mean that some kernel configs (such as Debian/m68k) may need tweaking. --- drivers/char/Kconfig |5 + drivers/scsi/Kconfig |6 +++--- drivers/scsi/atari_scsi.c |8 3 files changed, 8 insertions(+), 11 deletions(-) Index: linux/drivers/char/Kconfig === --- linux.orig/drivers/char/Kconfig 2015-11-01 21:41:24.0 +1100 +++ linux/drivers/char/Kconfig 2015-11-01 21:41:24.0 +1100 @@ -247,7 +247,7 @@ source "drivers/char/hw_random/Kconfig" config NVRAM tristate "/dev/nvram support" - depends on ATARI || X86 || (ARM && RTC_DRV_CMOS) || GENERIC_NVRAM + depends on X86 || (ARM && RTC_DRV_CMOS) || GENERIC_NVRAM ---help--- If you say Y here and create a character special file /dev/nvram with major number 10 and minor number 144 using mknod ("man mknod"), @@ -265,9 +265,6 @@ config NVRAM should NEVER idly tamper with it. See Ralf Brown's interrupt list for a guide to the use of CMOS bytes by your BIOS. - On Atari machines, /dev/nvram is always configured and does not need - to be selected. - To compile this driver as a module, choose M here: the module will be called nvram. Index: linux/drivers/scsi/Kconfig === --- linux.orig/drivers/scsi/Kconfig 2015-11-01 21:41:24.0 +1100 +++ linux/drivers/scsi/Kconfig 2015-11-01 21:41:24.0 +1100 @@ -1610,14 +1610,14 @@ config ATARI_SCSI tristate "Atari native SCSI support" depends on ATARI && SCSI select SCSI_SPI_ATTRS - select NVRAM ---help--- If you have an Atari with built-in NCR5380 SCSI controller (TT, Falcon, ...) say Y to get it supported. Of course also, if you have a compatible SCSI controller (e.g. for Medusa). - To compile this driver as a module, choose M here: the - module will be called atari_scsi. + To compile this driver as a module, choose M here: the module will + be called atari_scsi. If you also enable NVRAM support, the SCSI + host's ID is taken from the setting in TT RTC NVRAM. This driver supports both styles of NCR integration into the system: the TT style (separate DMA), and the Falcon style (via Index: linux/drivers/scsi/atari_scsi.c === --- linux.orig/drivers/scsi/atari_scsi.c2015-11-01 21:41:24.0 +1100 +++ linux/drivers/scsi/atari_scsi.c 2015-11-01 21:41:24.0 +1100 @@ -875,9 +875,10 @@ static int __init atari_scsi_probe(struc if (ATARIHW_PRESENT(TT_SCSI) && setup_sg_tablesize >= 0) atari_scsi_template.sg_tablesize = setup_sg_tablesize; - if (setup_hostid >= 0) { + if (setup_hostid >= 0) atari_scsi_template.this_id = setup_hostid & 7; - } else { +#ifdef CONFIG_NVRAM + else /* Test if a host id is set in the NVRam */ if (ATARIHW_PRESENT(TT_CLK) && nvram_check_checksum()) { unsigned char b = nvram_read_byte(14); @@ -888,8 +889,7 @@ static int __init atari_scsi_probe(struc if (b & 0x80) atari_scsi_template.this_id = b & 7; } - } - +#endif #ifdef REAL_DMA /* If running on a Falcon and if there's TT-Ram (i.e., more than one -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 03/26] m68k/atari: Replace nvram_{read,write}_byte with arch_nvram_ops
By implementing an arch_nvram_ops struct, any platform can re-use the drivers/char/nvram module without needing any arch-specific code in that module. Atari does so here. Atari has one user of nvram_check_checksum() whereas the other platforms (i.e. x86 and ARM platforms) have none at all. Replace this validate-checksum-and-read-byte sequence with the equivalent rtc_nvram_ops.read() call and remove the now unused functions. Signed-off-by: Finn Thain Tested-by: Christian T. Steigies Acked-by: Geert Uytterhoeven --- The advantage of the new ops struct over the old global nvram_* functions is that the misc device module can be shared by different platforms without requiring every platform to implement every nvram_* function. E.g. only RTC "CMOS" NVRAMs have a checksum for the entire NVRAM and only PowerPC platforms have a "sync" ioctl. --- arch/m68k/atari/nvram.c | 89 -- drivers/scsi/atari_scsi.c |8 ++-- include/linux/nvram.h |9 3 files changed, 70 insertions(+), 36 deletions(-) Index: linux/arch/m68k/atari/nvram.c === --- linux.orig/arch/m68k/atari/nvram.c 2015-11-01 21:41:25.0 +1100 +++ linux/arch/m68k/atari/nvram.c 2015-11-01 21:41:27.0 +1100 @@ -38,33 +38,12 @@ unsigned char __nvram_read_byte(int i) return CMOS_READ(NVRAM_FIRST_BYTE + i); } -unsigned char nvram_read_byte(int i) -{ - unsigned long flags; - unsigned char c; - - spin_lock_irqsave(&rtc_lock, flags); - c = __nvram_read_byte(i); - spin_unlock_irqrestore(&rtc_lock, flags); - return c; -} -EXPORT_SYMBOL(nvram_read_byte); - /* This races nicely with trying to read with checksum checking */ void __nvram_write_byte(unsigned char c, int i) { CMOS_WRITE(c, NVRAM_FIRST_BYTE + i); } -void nvram_write_byte(unsigned char c, int i) -{ - unsigned long flags; - - spin_lock_irqsave(&rtc_lock, flags); - __nvram_write_byte(c, i); - spin_unlock_irqrestore(&rtc_lock, flags); -} - /* On Ataris, the checksum is over all bytes except the checksum bytes * themselves; these are at the very end. */ @@ -83,18 +62,6 @@ int __nvram_check_checksum(void) (__nvram_read_byte(ATARI_CKS_LOC + 1) == (sum & 0xff)); } -int nvram_check_checksum(void) -{ - unsigned long flags; - int rv; - - spin_lock_irqsave(&rtc_lock, flags); - rv = __nvram_check_checksum(); - spin_unlock_irqrestore(&rtc_lock, flags); - return rv; -} -EXPORT_SYMBOL(nvram_check_checksum); - static void __nvram_set_checksum(void) { int i; @@ -106,6 +73,62 @@ static void __nvram_set_checksum(void) __nvram_write_byte(sum, ATARI_CKS_LOC + 1); } +static ssize_t nvram_read(char *buf, size_t count, loff_t *ppos) +{ + char *p = buf; + loff_t i; + + spin_lock_irq(&rtc_lock); + + if (!__nvram_check_checksum()) { + spin_unlock_irq(&rtc_lock); + return -EIO; + } + + for (i = *ppos; count > 0 && i < NVRAM_BYTES; --count, ++i, ++p) + *p = __nvram_read_byte(i); + + spin_unlock_irq(&rtc_lock); + + *ppos = i; + return p - buf; +} + +static ssize_t nvram_write(char *buf, size_t count, loff_t *ppos) +{ + char *p = buf; + loff_t i; + + spin_lock_irq(&rtc_lock); + + if (!__nvram_check_checksum()) { + spin_unlock_irq(&rtc_lock); + return -EIO; + } + + for (i = *ppos; count > 0 && i < NVRAM_BYTES; --count, ++i, ++p) + __nvram_write_byte(*p, i); + + __nvram_set_checksum(); + + spin_unlock_irq(&rtc_lock); + + *ppos = i; + return p - buf; +} + +static ssize_t nvram_get_size(void) +{ + return NVRAM_BYTES; +} + +const struct nvram_ops arch_nvram_ops = { + .read = nvram_read, + .write = nvram_write, + .get_size = nvram_get_size, +}; +EXPORT_SYMBOL(arch_nvram_ops); + #ifdef CONFIG_PROC_FS static struct { unsigned char val; Index: linux/drivers/scsi/atari_scsi.c === --- linux.orig/drivers/scsi/atari_scsi.c2015-11-01 21:41:24.0 +1100 +++ linux/drivers/scsi/atari_scsi.c 2015-11-01 21:41:27.0 +1100 @@ -880,13 +880,15 @@ static int __init atari_scsi_probe(struc #ifdef CONFIG_NVRAM else /* Test if a host id is set in the NVRam */ - if (ATARIHW_PRESENT(TT_CLK) && nvram_check_checksum()) { - unsigned char b = nvram_read_byte(14); + if (ATARIHW_PRESENT(TT_CLK)) { + unsigned char b; + loff_t offset = 14; + ssize_t count = arch_nvram_ops.read(&b, 1, &offset); /* Arbitration enabled? (for TOS) * If yes