On Wed, Dec 30, 2020 at 5:47 AM Viswas G <Viswas.G@xxxxxxxxxxxxxxxxx> wrote:
>
> From: akshatzen <akshatzen@xxxxxxxxxx>
>
> pm8001 driver initializes main configuration, general status, inbound
> queue and outbound queue table address based on value read from
> MSGU_SCRATCH_PAD_0 register.
>
> We should validate these addresses before dereferencing them.
>
> This change adds two validations:
> 1. Check if main configuration table offset lies within the pcibar
> mapped
> 2. Check if first dword of main configuration table reads "PMCS"
>
> There are two calls to init_pci_device_addresses() done during
> pm8001_pci_probe() in this sequence:
> 1. First inside chip_soft_rst, where if init_pci_device_addresses fails
> we will go ahead assuming MPI state is not ready and reset the device as
> long as bootloader is okay. This gives chance to second call of
> init_pci_device_addresses to set up the addresses after reset.
>
> 2. The second call is via pm80xx_chip_init, after soft reset is done and
> firmware is checked to be ready. Once that is done we are safe to go
> ahead and initialize default table values and use them.
>
> Tested:
>
> 1. Enabled debugging logs and observed no issues during initialization,
> with a controller with No issues.
>
> pm80xx0:: pm8001_setup_msix 1034: pci_alloc_irq_vectors request ret:64
> no of intr 64
> pm80xx0:: init_pci_device_addresses 917: Scratchpad 0 Offset: 0x2000
> value 0x40002000
> pm80xx0:: init_pci_device_addresses 925: Scratchpad 0 PCI BAR: 0
> pm80xx0:: init_pci_device_addresses 952: VALID main config signature
> 0x53434d50
> pm80xx0:: init_pci_device_addresses 975: GST OFFSET 0xc4
> pm80xx0:: init_pci_device_addresses 978: INBND OFFSET 0x20000128
> pm80xx0:: init_pci_device_addresses 981: OBND OFFSET 0x24000928
> pm80xx0:: init_pci_device_addresses 984: IVT OFFSET 0x8001408
> pm80xx0:: init_pci_device_addresses 987: PSPA OFFSET 0x8001608
> pm80xx0:: init_pci_device_addresses 991: addr - main cfg (ptrval)
> general status (ptrval)
> pm80xx0:: init_pci_device_addresses 995: addr - inbnd (ptrval) obnd
> (ptrval)
> pm80xx0:: init_pci_device_addresses 999: addr - pspa (ptrval) ivt
> (ptrval)
> pm80xx0:: pm80xx_chip_soft_rst 1446: reset register before write : 0x0
> pm80xx0:: pm80xx_chip_soft_rst 1478: reset register after write 0x40
> pm80xx0:: pm80xx_chip_soft_rst 1544: SPCv soft reset Complete
> pm80xx0:: init_pci_device_addresses 917: Scratchpad 0 Offset: 0x2000
> value 0x40002000
> pm80xx0:: init_pci_device_addresses 925: Scratchpad 0 PCI BAR: 0
> pm80xx0:: init_pci_device_addresses 952: VALID main config signature
> 0x53434d50
> pm80xx0:: init_pci_device_addresses 975: GST OFFSET 0xc4
> pm80xx0:: init_pci_device_addresses 978: INBND OFFSET 0x20000128
> pm80xx0:: init_pci_device_addresses 981: OBND OFFSET 0x24000928
> pm80xx0:: init_pci_device_addresses 984: IVT OFFSET 0x8001408
> pm80xx0:: init_pci_device_addresses 987: PSPA OFFSET 0x8001608
> pm80xx0:: init_pci_device_addresses 991: addr - main cfg (ptrval)
> general status (ptrval)
> pm80xx0:: init_pci_device_addresses 995: addr - inbnd (ptrval) obnd
> (ptrval)
> pm80xx0:: init_pci_device_addresses 999: addr - pspa (ptrval) ivt
> (ptrval)
> pm80xx0:: pm80xx_chip_init 1329: MPI initialize successful!
>
> 2. Tested controller with firmware known to have initialization issue
> and observed no crashes with this fix
>
> pm80xx 0000:01:00.0: pm80xx: driver version 0.1.38
> pm80xx 0000:01:00.0: Removing from 1:1 domain
> pm80xx 0000:01:00.0: Requesting non-1:1 mappings
> pm80xx0:: init_pci_device_addresses 948: BAD main config signature 0x0
> pm80xx0:: mpi_uninit_check 1365: Failed to init pci addresses
> pm80xx0:: pm80xx_chip_soft_rst 1435: MPI state is not ready
> scratch:0:8:62a01000:0
> pm80xx0:: pm80xx_chip_soft_rst 1518: Firmware is not ready!
> pm80xx0:: pm80xx_chip_soft_rst 1532: iButton Feature is not Available!!!
> pm80xx0:: pm80xx_chip_init 1301: Firmware is not ready!
> pm80xx0:: pm8001_pci_probe 1215: chip_init failed [ret: -16]
> pm80xx: probe of 0000:01:00.0 failed with error -16
> pm80xx 0000:07:00.0: pm80xx: driver version 0.1.38
> pm80xx 0000:07:00.0: Removing from 1:1 domain
> pm80xx 0000:07:00.0: Requesting non-1:1 mappings
> scsi host6: pm80xx
> pm80xx1:: pm8001_setup_sgpio 5568: failed sgpio_req timeout
> pm80xx1:: mpi_phy_start_resp 3447: phy start resp status:0x0, phyid:0x0
> pm80xx 0000:08:00.0: pm80xx: driver version 0.1.38
> pm80xx 0000:08:00.0: Removing from 1:1 domain
> pm80xx 0000:08:00.0: Requesting non-1:1 mappings
>
> 3. Without this fix we observe crash on the same controller.
>
> pm80xx 0000:01:00.0: pm80xx: driver version 0.1.38
> pm80xx 0000:01:00.0: Removing from 1:1 domain
> pm80xx 0000:01:00.0: Requesting non-1:1 mappings
> [<ffffffffc0451b3b>] pm80xx_chip_soft_rst+0x6b/0x4c0 [pm80xx]
> [<ffffffffc043a933>] pm8001_pci_probe+0xa43/0x1630 [pm80xx]
> RIP: 0010:pm80xx_chip_soft_rst+0x71/0x4c0 [pm80xx]
> [<ffffffffc0451b3b>] ? pm80xx_chip_soft_rst+0x6b/0x4c0 [pm80xx]
> [<ffffffffc043a933>] pm8001_pci_probe+0xa43/0x1630 [pm80xx]
> pm80xx0:: mpi_uninit_check 1339: TIMEOUT:IBDB value/=2
> pm80xx0:: pm80xx_chip_soft_rst 1387: MPI state is not ready
> scratch:0:8:62a01000:0
> pm80xx0:: pm80xx_chip_soft_rst 1470: Firmware is not ready!
> pm80xx0:: pm80xx_chip_soft_rst 1484: iButton Feature is not Available!!!
> pm80xx0:: pm80xx_chip_init 1266: Firmware is not ready!
> pm80xx0:: pm8001_pci_probe 1207: chip_init failed [ret: -16]
> pm80xx: probe of 0000:01:00.0 failed with error -16
>
> Signed-off-by: akshatzen <akshatzen@xxxxxxxxxx>
> Signed-off-by: Viswas G <Viswas.G@xxxxxxxxxxxxx>
> Signed-off-by: Ruksar Devadi <Ruksar.devadi@xxxxxxxxxxxxx>
> Signed-off-by: Radha Ramachandran <radha@xxxxxxxxxx>
Acked-by: Jack Wang <jinpu.wang@xxxxxxxxxxxxxxx>
Thx
> ---
> drivers/scsi/pm8001/pm8001_init.c | 11 +++++---
> drivers/scsi/pm8001/pm80xx_hwi.c | 53 ++++++++++++++++++++++++++++++++++++---
> 2 files changed, 58 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/scsi/pm8001/pm8001_init.c b/drivers/scsi/pm8001/pm8001_init.c
> index ee2de177d0d0..d21078ca7fb3 100644
> --- a/drivers/scsi/pm8001/pm8001_init.c
> +++ b/drivers/scsi/pm8001/pm8001_init.c
> @@ -466,9 +466,12 @@ static int pm8001_ioremap(struct pm8001_hba_info *pm8001_ha)
> pm8001_ha->io_mem[logicalBar].memvirtaddr =
> ioremap(pm8001_ha->io_mem[logicalBar].membase,
> pm8001_ha->io_mem[logicalBar].memsize);
> - pm8001_dbg(pm8001_ha, INIT,
> - "PCI: bar %d, logicalBar %d\n",
> + if (!pm8001_ha->io_mem[logicalBar].memvirtaddr) {
> + pm8001_dbg(pm8001_ha, INIT,
> + "Failed to ioremap bar %d, logicalBar %d",
> bar, logicalBar);
> + return -ENOMEM;
> + }
> pm8001_dbg(pm8001_ha, INIT,
> "base addr %llx virt_addr=%llx len=%d\n",
> (u64)pm8001_ha->io_mem[logicalBar].membase,
> @@ -540,9 +543,11 @@ static struct pm8001_hba_info *pm8001_pci_alloc(struct pci_dev *pdev,
> tasklet_init(&pm8001_ha->tasklet[j], pm8001_tasklet,
> (unsigned long)&(pm8001_ha->irq_vector[j]));
> #endif
> - pm8001_ioremap(pm8001_ha);
> + if (pm8001_ioremap(pm8001_ha))
> + goto failed_pci_alloc;
> if (!pm8001_alloc(pm8001_ha, ent))
> return pm8001_ha;
> +failed_pci_alloc:
> pm8001_free(pm8001_ha);
> return NULL;
> }
> diff --git a/drivers/scsi/pm8001/pm80xx_hwi.c b/drivers/scsi/pm8001/pm80xx_hwi.c
> index 86a3d483749c..7d0eada11d3c 100644
> --- a/drivers/scsi/pm8001/pm80xx_hwi.c
> +++ b/drivers/scsi/pm8001/pm80xx_hwi.c
> @@ -1115,7 +1115,7 @@ static int check_fw_ready(struct pm8001_hba_info *pm8001_ha)
> return ret;
> }
>
> -static void init_pci_device_addresses(struct pm8001_hba_info *pm8001_ha)
> +static int init_pci_device_addresses(struct pm8001_hba_info *pm8001_ha)
> {
> void __iomem *base_addr;
> u32 value;
> @@ -1124,15 +1124,48 @@ static void init_pci_device_addresses(struct pm8001_hba_info *pm8001_ha)
> u32 pcilogic;
>
> value = pm8001_cr32(pm8001_ha, 0, MSGU_SCRATCH_PAD_0);
> +
> + /**
> + * lower 26 bits of SCRATCHPAD0 register describes offset within the
> + * PCIe BAR where the MPI configuration table is present
> + */
> offset = value & 0x03FFFFFF; /* scratch pad 0 TBL address */
>
> pm8001_dbg(pm8001_ha, DEV, "Scratchpad 0 Offset: 0x%x value 0x%x\n",
> offset, value);
> + /**
> + * Upper 6 bits describe the offset within PCI config space where BAR
> + * is located.
> + */
> pcilogic = (value & 0xFC000000) >> 26;
> pcibar = get_pci_bar_index(pcilogic);
> pm8001_dbg(pm8001_ha, INIT, "Scratchpad 0 PCI BAR: %d\n", pcibar);
> +
> + /**
> + * Make sure the offset falls inside the ioremapped PCI BAR
> + */
> + if (offset > pm8001_ha->io_mem[pcibar].memsize) {
> + pm8001_dbg(pm8001_ha, FAIL,
> + "Main cfg tbl offset outside %u > %u\n",
> + offset, pm8001_ha->io_mem[pcibar].memsize);
> + return -EBUSY;
> + }
> pm8001_ha->main_cfg_tbl_addr = base_addr =
> pm8001_ha->io_mem[pcibar].memvirtaddr + offset;
> +
> + /**
> + * Validate main configuration table address: first DWord should read
> + * "PMCS"
> + */
> + value = pm8001_mr32(pm8001_ha->main_cfg_tbl_addr, 0);
> + if (memcmp(&value, "PMCS", 4) != 0) {
> + pm8001_dbg(pm8001_ha, FAIL,
> + "BAD main config signature 0x%x\n",
> + value);
> + return -EBUSY;
> + }
> + pm8001_dbg(pm8001_ha, INIT,
> + "VALID main config signature 0x%x\n", value);
> pm8001_ha->general_stat_tbl_addr =
> base_addr + (pm8001_cr32(pm8001_ha, pcibar, offset + 0x18) &
> 0xFFFFFF);
> @@ -1171,6 +1204,7 @@ static void init_pci_device_addresses(struct pm8001_hba_info *pm8001_ha)
> pm8001_dbg(pm8001_ha, INIT, "addr - pspa %p ivt %p\n",
> pm8001_ha->pspa_q_tbl_addr,
> pm8001_ha->ivt_tbl_addr);
> + return 0;
> }
>
> /**
> @@ -1438,7 +1472,12 @@ static int pm80xx_chip_init(struct pm8001_hba_info *pm8001_ha)
> pm8001_ha->controller_fatal_error = false;
>
> /* Initialize pci space address eg: mpi offset */
> - init_pci_device_addresses(pm8001_ha);
> + ret = init_pci_device_addresses(pm8001_ha);
> + if (ret) {
> + pm8001_dbg(pm8001_ha, FAIL,
> + "Failed to init pci addresses");
> + return ret;
> + }
> init_default_table_values(pm8001_ha);
> read_main_config_table(pm8001_ha);
> read_general_status_table(pm8001_ha);
> @@ -1482,7 +1521,15 @@ static int mpi_uninit_check(struct pm8001_hba_info *pm8001_ha)
> u32 max_wait_count;
> u32 value;
> u32 gst_len_mpistate;
> - init_pci_device_addresses(pm8001_ha);
> + int ret;
> +
> + ret = init_pci_device_addresses(pm8001_ha);
> + if (ret) {
> + pm8001_dbg(pm8001_ha, FAIL,
> + "Failed to init pci addresses");
> + return ret;
> + }
> +
> /* Write bit1=1 to Inbound DoorBell Register to tell the SPC FW the
> table is stop */
> pm8001_cw32(pm8001_ha, 0, MSGU_IBDB_SET, SPCv_MSGU_CFG_TABLE_RESET);
> --
> 2.16.3
>
