Re: [PATCH v3] [SCSI] mpt2sas, mpt3sas: Abort initialization if no memory I/O resources detected
On 07/15/2015 06:49 AM, Sreekanth Reddy wrote: Driver crashes if the BIOS do not set up at least one memory I/O resource. This failure can happen if the device is too slow to respond during POST and is missed by the BIOS, but Linux then detects the device later in the boot process. Changes in v3: Rearranged the code to remove the code redundancy Signed-off-by: Sreekanth Reddy sreekanth.re...@avagotech.com --- Reviewed-by: Hannes Reinecke h...@suse.com Cheers, Hannes -- Dr. Hannes ReineckezSeries Storage h...@suse.de +49 911 74053 688 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton HRB 21284 (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 v3] [SCSI] mpt2sas, mpt3sas: Abort initialization if no memory I/O resources detected
On Tue, Jul 14, 2015 at 10:36:58PM -0700, Yinghai Lu wrote: On Tue, Jul 14, 2015 at 9:49 PM, Sreekanth Reddy sreekanth.re...@avagotech.com wrote: Driver crashes if the BIOS do not set up at least one memory I/O resource. This failure can happen if the device is too slow to respond during POST and is missed by the BIOS, but Linux then detects the device later in the boot process. But pci subsystem should assign resources to those unassigned BAR. Do you mean even kernel can not assign resource to them? or it takes so long for mpt FW to get ready? This is not an issue from mpt FW. I have just kept the same description provide by Timothy in his initial patch. But I observe that their may be chance of getting unable to handle kernel NULL pointer dereference kernel panic if no Memory Resource available in the PCI subsystem. So agreed to the Timothy proposal of aborting the driver initialization if it doesn't detect any Memory resource instead of whole system get into panic state. Thanks Yinghai -- 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 v3] [SCSI] mpt2sas, mpt3sas: Abort initialization if no memory I/O resources detected
On 07/15/2015 01:28 PM, Yinghai Lu wrote: On Wed, Jul 15, 2015 at 6:52 AM, Timothy Pearson tpear...@raptorengineeringinc.com wrote: I have just kept the same description provide by Timothy in his initial patch. But I observe that their may be chance of getting unable to handle kernel NULL pointer dereference kernel panic if no Memory Resource available in the PCI subsystem. So agreed to the Timothy proposal of aborting the driver initialization if it doesn't detect any Memory resource instead of whole system get into panic state. On some systems Linux is unable / unwilling to assign a BAR if the BIOS does not assign one at startup. I didn't look into the Linux allocator side of things in much detail, but it is quite possible that Linux is unaware the device only has partial resources assigned. Would be great if you can post boot log so we can figure about why those BARs are not assigned. Yinghai Unfortunately the systems exhibiting the issue were upgraded to a later BIOS that does not have this problem; it might be possible to set up a test system in the future but that probably won't happen for some time. -- Timothy Pearson Raptor Engineering +1 (415) 727-8645 http://www.raptorengineeringinc.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
Re: [PATCH v3] [SCSI] mpt2sas, mpt3sas: Abort initialization if no memory I/O resources detected
On Wed, Jul 15, 2015 at 6:52 AM, Timothy Pearson tpear...@raptorengineeringinc.com wrote: I have just kept the same description provide by Timothy in his initial patch. But I observe that their may be chance of getting unable to handle kernel NULL pointer dereference kernel panic if no Memory Resource available in the PCI subsystem. So agreed to the Timothy proposal of aborting the driver initialization if it doesn't detect any Memory resource instead of whole system get into panic state. On some systems Linux is unable / unwilling to assign a BAR if the BIOS does not assign one at startup. I didn't look into the Linux allocator side of things in much detail, but it is quite possible that Linux is unaware the device only has partial resources assigned. Would be great if you can post boot log so we can figure about why those BARs are not assigned. Yinghai -- 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 v3] [SCSI] mpt2sas, mpt3sas: Abort initialization if no memory I/O resources detected
On Wed, Jul 15, 2015 at 10:19:56AM +0530, Sreekanth Reddy wrote:
Driver crashes if the BIOS do not set up at least one
memory I/O resource. This failure can happen if the device is too
slow to respond during POST and is missed by the BIOS, but Linux
then detects the device later in the boot process.
Changes in v3:
Rearranged the code to remove the code redundancy
Signed-off-by: Sreekanth Reddy sreekanth.re...@avagotech.com
---
drivers/scsi/mpt2sas/mpt2sas_base.c | 16 +---
drivers/scsi/mpt3sas/mpt3sas_base.c | 16 +---
2 files changed, 18 insertions(+), 14 deletions(-)
diff --git a/drivers/scsi/mpt2sas/mpt2sas_base.c
b/drivers/scsi/mpt2sas/mpt2sas_base.c
index 11248de..6dec7cf 100644
--- a/drivers/scsi/mpt2sas/mpt2sas_base.c
+++ b/drivers/scsi/mpt2sas/mpt2sas_base.c
@@ -1557,7 +1557,8 @@ mpt2sas_base_map_resources(struct MPT2SAS_ADAPTER *ioc)
goto out_fail;
}
- for (i = 0, memap_sz = 0, pio_sz = 0 ; i DEVICE_COUNT_RESOURCE; i++) {
+ for (i = 0, memap_sz = 0, pio_sz = 0; (i DEVICE_COUNT_RESOURCE)
+ (!memap_sz || !pio_sz); i++) {
if (pci_resource_flags(pdev, i) IORESOURCE_IO) {
if (pio_sz)
continue;
@@ -1572,16 +1573,17 @@ mpt2sas_base_map_resources(struct MPT2SAS_ADAPTER
*ioc)
chip_phys = (u64)ioc-chip_phys;
memap_sz = pci_resource_len(pdev, i);
ioc-chip = ioremap(ioc-chip_phys, memap_sz);
- if (ioc-chip == NULL) {
- printk(MPT2SAS_ERR_FMT unable to map
- adapter memory!\n, ioc-name);
- r = -EINVAL;
- goto out_fail;
- }
}
}
}
+ if (ioc-chip == NULL) {
+ printk(MPT2SAS_ERR_FMT unable to map adapter memory!
+or resource not found\n, ioc-name);
+ r = -EINVAL;
+ goto out_fail;
+ }
- Before this patch, the driver probe succeeds even if it can't find an
MMIO resource. That means we would eventually dereference ioc-chip,
which is zero. This patch definitely fixes that bug.
- I raise my eyebrows a little at the let's use the first MMIO resource
we find idea. A driver should know exactly which BAR it needs, so it
seems sloppy to search this way. And it's possible that BIOS or the
PCI core set up *one* MMIO BAR, but not the one the driver needs. Then
the search will find the wrong BAR and the driver won't work.
- These drivers both do:
ioc-chip_phys = pci_resource_start(pdev, i);
ioc-chip = ioremap(ioc-chip_phys, memap_sz);
Saving ioc-chip_phys is pointless because it's only used to guard
iounmap(ioc-chip), and the driver probe should fail if it can't find
an MMIO resource, so the places that do the iounmap can assume
ioc-chip is always valid.
- These drivers also search for an I/O resource, but they don't actually
use it, except to print it out. I guess that's harmless, but it seems
sloppy and will print junk on machines where there's no I/O space
available. They do use pci_enable_device_mem(), so at least the device
should work even if no I/O space is available.
- It'd be nice if they used dev_printk() and %pR instead of their ad hoc
formatting.
_base_mask_interrupts(ioc);
r = _base_get_ioc_facts(ioc, CAN_SLEEP);
diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c
b/drivers/scsi/mpt3sas/mpt3sas_base.c
index 14a781b..43f87e9 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
@@ -1843,7 +1843,8 @@ mpt3sas_base_map_resources(struct MPT3SAS_ADAPTER *ioc)
goto out_fail;
}
- for (i = 0, memap_sz = 0, pio_sz = 0 ; i DEVICE_COUNT_RESOURCE; i++) {
+ for (i = 0, memap_sz = 0, pio_sz = 0; (i DEVICE_COUNT_RESOURCE)
+ (!memap_sz || !pio_sz); i++) {
if (pci_resource_flags(pdev, i) IORESOURCE_IO) {
if (pio_sz)
continue;
@@ -1856,15 +1857,16 @@ mpt3sas_base_map_resources(struct MPT3SAS_ADAPTER
*ioc)
chip_phys = (u64)ioc-chip_phys;
memap_sz = pci_resource_len(pdev, i);
ioc-chip = ioremap(ioc-chip_phys, memap_sz);
- if (ioc-chip == NULL) {
- pr_err(MPT3SAS_FMT unable to map adapter
memory!\n,
- ioc-name);
- r = -EINVAL;
- goto out_fail;
- }
}
}
+ if (ioc-chip == NULL) {
+ pr_err(MPT3SAS_FMT unable
Re: [PATCH v3] [SCSI] mpt2sas, mpt3sas: Abort initialization if no memory I/O resources detected
On 07/15/2015 01:24 AM, Sreekanth Reddy wrote: On Tue, Jul 14, 2015 at 10:36:58PM -0700, Yinghai Lu wrote: On Tue, Jul 14, 2015 at 9:49 PM, Sreekanth Reddy sreekanth.re...@avagotech.com wrote: Driver crashes if the BIOS do not set up at least one memory I/O resource. This failure can happen if the device is too slow to respond during POST and is missed by the BIOS, but Linux then detects the device later in the boot process. But pci subsystem should assign resources to those unassigned BAR. Do you mean even kernel can not assign resource to them? or it takes so long for mpt FW to get ready? This is not an issue from mpt FW. I have just kept the same description provide by Timothy in his initial patch. But I observe that their may be chance of getting unable to handle kernel NULL pointer dereference kernel panic if no Memory Resource available in the PCI subsystem. So agreed to the Timothy proposal of aborting the driver initialization if it doesn't detect any Memory resource instead of whole system get into panic state. Thanks Yinghai On some systems Linux is unable / unwilling to assign a BAR if the BIOS does not assign one at startup. I didn't look into the Linux allocator side of things in much detail, but it is quite possible that Linux is unaware the device only has partial resources assigned. -- Timothy Pearson Raptor Engineering +1 (415) 727-8645 http://www.raptorengineeringinc.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
Re: [PATCH v3] [SCSI] mpt2sas, mpt3sas: Abort initialization if no memory I/O resources detected
On Wed, Jul 15, 2015 at 08:52:13AM -0500, Timothy Pearson wrote: On 07/15/2015 01:24 AM, Sreekanth Reddy wrote: On Tue, Jul 14, 2015 at 10:36:58PM -0700, Yinghai Lu wrote: On Tue, Jul 14, 2015 at 9:49 PM, Sreekanth Reddy sreekanth.re...@avagotech.com wrote: Driver crashes if the BIOS do not set up at least one memory I/O resource. This failure can happen if the device is too slow to respond during POST and is missed by the BIOS, but Linux then detects the device later in the boot process. But pci subsystem should assign resources to those unassigned BAR. Do you mean even kernel can not assign resource to them? or it takes so long for mpt FW to get ready? This is not an issue from mpt FW. I have just kept the same description provide by Timothy in his initial patch. But I observe that their may be chance of getting unable to handle kernel NULL pointer dereference kernel panic if no Memory Resource available in the PCI subsystem. So agreed to the Timothy proposal of aborting the driver initialization if it doesn't detect any Memory resource instead of whole system get into panic state. On some systems Linux is unable / unwilling to assign a BAR if the BIOS does not assign one at startup. I didn't look into the Linux allocator side of things in much detail, but it is quite possible that Linux is unaware the device only has partial resources assigned. There might be a Linux PCI core bug if we don't assign a BAR, although resource assignment can always fail, even in the absence of bugs. But there is definitely a driver bug if it uses ioc-chip when it hasn't been initialized. That's what this patch seems to fix. Bjorn -- 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 v3] [SCSI] mpt2sas, mpt3sas: Abort initialization if no memory I/O resources detected
Driver crashes if the BIOS do not set up at least one
memory I/O resource. This failure can happen if the device is too
slow to respond during POST and is missed by the BIOS, but Linux
then detects the device later in the boot process.
Changes in v3:
Rearranged the code to remove the code redundancy
Signed-off-by: Sreekanth Reddy sreekanth.re...@avagotech.com
---
drivers/scsi/mpt2sas/mpt2sas_base.c | 16 +---
drivers/scsi/mpt3sas/mpt3sas_base.c | 16 +---
2 files changed, 18 insertions(+), 14 deletions(-)
diff --git a/drivers/scsi/mpt2sas/mpt2sas_base.c
b/drivers/scsi/mpt2sas/mpt2sas_base.c
index 11248de..6dec7cf 100644
--- a/drivers/scsi/mpt2sas/mpt2sas_base.c
+++ b/drivers/scsi/mpt2sas/mpt2sas_base.c
@@ -1557,7 +1557,8 @@ mpt2sas_base_map_resources(struct MPT2SAS_ADAPTER *ioc)
goto out_fail;
}
- for (i = 0, memap_sz = 0, pio_sz = 0 ; i DEVICE_COUNT_RESOURCE; i++) {
+ for (i = 0, memap_sz = 0, pio_sz = 0; (i DEVICE_COUNT_RESOURCE)
+(!memap_sz || !pio_sz); i++) {
if (pci_resource_flags(pdev, i) IORESOURCE_IO) {
if (pio_sz)
continue;
@@ -1572,16 +1573,17 @@ mpt2sas_base_map_resources(struct MPT2SAS_ADAPTER *ioc)
chip_phys = (u64)ioc-chip_phys;
memap_sz = pci_resource_len(pdev, i);
ioc-chip = ioremap(ioc-chip_phys, memap_sz);
- if (ioc-chip == NULL) {
- printk(MPT2SAS_ERR_FMT unable to map
- adapter memory!\n, ioc-name);
- r = -EINVAL;
- goto out_fail;
- }
}
}
}
+ if (ioc-chip == NULL) {
+ printk(MPT2SAS_ERR_FMT unable to map adapter memory!
+ or resource not found\n, ioc-name);
+ r = -EINVAL;
+ goto out_fail;
+ }
+
_base_mask_interrupts(ioc);
r = _base_get_ioc_facts(ioc, CAN_SLEEP);
diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c
b/drivers/scsi/mpt3sas/mpt3sas_base.c
index 14a781b..43f87e9 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
@@ -1843,7 +1843,8 @@ mpt3sas_base_map_resources(struct MPT3SAS_ADAPTER *ioc)
goto out_fail;
}
- for (i = 0, memap_sz = 0, pio_sz = 0 ; i DEVICE_COUNT_RESOURCE; i++) {
+ for (i = 0, memap_sz = 0, pio_sz = 0; (i DEVICE_COUNT_RESOURCE)
+(!memap_sz || !pio_sz); i++) {
if (pci_resource_flags(pdev, i) IORESOURCE_IO) {
if (pio_sz)
continue;
@@ -1856,15 +1857,16 @@ mpt3sas_base_map_resources(struct MPT3SAS_ADAPTER *ioc)
chip_phys = (u64)ioc-chip_phys;
memap_sz = pci_resource_len(pdev, i);
ioc-chip = ioremap(ioc-chip_phys, memap_sz);
- if (ioc-chip == NULL) {
- pr_err(MPT3SAS_FMT unable to map adapter
memory!\n,
- ioc-name);
- r = -EINVAL;
- goto out_fail;
- }
}
}
+ if (ioc-chip == NULL) {
+ pr_err(MPT3SAS_FMT unable to map adapter memory!
+or resource not found\n, ioc-name);
+ r = -EINVAL;
+ goto out_fail;
+ }
+
_base_mask_interrupts(ioc);
r = _base_get_ioc_facts(ioc, CAN_SLEEP);
--
2.0.2
--
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 v3] [SCSI] mpt2sas, mpt3sas: Abort initialization if no memory I/O resources detected
On Tue, Jul 14, 2015 at 9:49 PM, Sreekanth Reddy sreekanth.re...@avagotech.com wrote: Driver crashes if the BIOS do not set up at least one memory I/O resource. This failure can happen if the device is too slow to respond during POST and is missed by the BIOS, but Linux then detects the device later in the boot process. But pci subsystem should assign resources to those unassigned BAR. Do you mean even kernel can not assign resource to them? or it takes so long for mpt FW to get ready? Thanks Yinghai -- 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
