[PATCH] hpsa: dont meddle with hw which isn't ours (cciss)

[PATCH] hpsa: dont meddle with hw which isn't ours (cciss)

[Tomas Henzl]

The hpsa driver touches the hardware before checking the pci-id table.
This way, especially in kdump, it may confuse the proper driver (cciss).

Signed-off-by: Tomas Henzl <thenzl@redhat.com>
---
 drivers/scsi/hpsa.c | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
index bddb33d30f..c33a077636 100644
--- a/drivers/scsi/hpsa.c
+++ b/drivers/scsi/hpsa.c
@@ -6737,7 +6737,7 @@ static int controller_reset_failed(struct CfgTable __iomem *cfgtable)
 /* This does a hard reset of the controller using PCI power management
  * states or the using the doorbell register.
  */
-static int hpsa_kdump_hard_reset_controller(struct pci_dev *pdev)
+static int hpsa_kdump_hard_reset_controller(struct pci_dev *pdev, u32 board_id)
 {
 	u64 cfg_offset;
 	u32 cfg_base_addr;
@@ -6748,7 +6748,6 @@ static int hpsa_kdump_hard_reset_controller(struct pci_dev *pdev)
 	int rc;
 	struct CfgTable __iomem *cfgtable;
 	u32 use_doorbell;
-	u32 board_id;
 	u16 command_register;
 
 	/* For controllers as old as the P600, this is very nearly
@@ -6764,11 +6763,6 @@ static int hpsa_kdump_hard_reset_controller(struct pci_dev *pdev)
 	 * using the doorbell register.
 	 */
 
-	rc = hpsa_lookup_board_id(pdev, &board_id);
-	if (rc < 0) {
-		dev_warn(&pdev->dev, "Board ID not found\n");
-		return rc;
-	}
 	if (!ctlr_is_resettable(board_id)) {
 		dev_warn(&pdev->dev, "Controller not resettable\n");
 		return -ENODEV;
@@ -7413,7 +7407,7 @@ static void hpsa_hba_inquiry(struct ctlr_info *h)
 	}
 }
 
-static int hpsa_init_reset_devices(struct pci_dev *pdev)
+static int hpsa_init_reset_devices(struct pci_dev *pdev, u32 board_id)
 {
 	int rc, i;
 	void __iomem *vaddr;
@@ -7449,7 +7443,7 @@ static int hpsa_init_reset_devices(struct pci_dev *pdev)
 	iounmap(vaddr);
 
 	/* Reset the controller with a PCI power-cycle or via doorbell */
-	rc = hpsa_kdump_hard_reset_controller(pdev);
+	rc = hpsa_kdump_hard_reset_controller(pdev, board_id);
 
 	/* -ENOTSUPP here means we cannot reset the controller
 	 * but it's already (and still) up and running in
@@ -7927,11 +7921,18 @@ static int hpsa_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 	struct ctlr_info *h;
 	int try_soft_reset = 0;
 	unsigned long flags;
+	u32 board_id;
 
 	if (number_of_controllers == 0)
 		printk(KERN_INFO DRIVER_NAME "\n");
 
-	rc = hpsa_init_reset_devices(pdev);
+	rc = hpsa_lookup_board_id(pdev, &board_id);
+	if (rc < 0) {
+		dev_warn(&pdev->dev, "Board ID not found\n");
+		return rc;
+	}
+
+	rc = hpsa_init_reset_devices(pdev, board_id);
 	if (rc) {
 		if (rc != -ENOTSUPP)
 			return rc;
-- 
1.9.3

RE: [PATCH] hpsa: dont meddle with hw which isn't ours (cciss)

[Don Brace]

So, in the context of a kdump, you would like the hpsa driver to not manage kdump and allow the cciss driver to manage the kdump operation?

I.E. Is this for a controller managed by cciss in the normal kernel and the hpsa reference driver is managing kdump?

> -----Original Message-----
> From: Tomas Henzl [mailto:thenzl@redhat.com]
> Sent: Thursday, April 02, 2015 8:26 AM
> To: linux-scsi@vger.kernel.org
> Cc: Don Brace; Scott Teel; Justin Lindley; Kevin Barnett
> Subject: [PATCH] hpsa: dont meddle with hw which isn't ours (cciss)
> 
> The hpsa driver touches the hardware before checking the pci-id table.
> This way, especially in kdump, it may confuse the proper driver (cciss).
> 
> Signed-off-by: Tomas Henzl <thenzl@redhat.com>
> ---
>  drivers/scsi/hpsa.c | 21 +++++++++++----------
>  1 file changed, 11 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
> index bddb33d30f..c33a077636 100644
> --- a/drivers/scsi/hpsa.c
> +++ b/drivers/scsi/hpsa.c
> @@ -6737,7 +6737,7 @@ static int controller_reset_failed(struct CfgTable
> __iomem *cfgtable)
>  /* This does a hard reset of the controller using PCI power management
>   * states or the using the doorbell register.
>   */
> -static int hpsa_kdump_hard_reset_controller(struct pci_dev *pdev)
> +static int hpsa_kdump_hard_reset_controller(struct pci_dev *pdev, u32
> board_id)
>  {
>  	u64 cfg_offset;
>  	u32 cfg_base_addr;
> @@ -6748,7 +6748,6 @@ static int hpsa_kdump_hard_reset_controller(struct
> pci_dev *pdev)
>  	int rc;
>  	struct CfgTable __iomem *cfgtable;
>  	u32 use_doorbell;
> -	u32 board_id;
>  	u16 command_register;
> 
>  	/* For controllers as old as the P600, this is very nearly
> @@ -6764,11 +6763,6 @@ static int hpsa_kdump_hard_reset_controller(struct
> pci_dev *pdev)
>  	 * using the doorbell register.
>  	 */
> 
> -	rc = hpsa_lookup_board_id(pdev, &board_id);
> -	if (rc < 0) {
> -		dev_warn(&pdev->dev, "Board ID not found\n");
> -		return rc;
> -	}
>  	if (!ctlr_is_resettable(board_id)) {
>  		dev_warn(&pdev->dev, "Controller not resettable\n");
>  		return -ENODEV;
> @@ -7413,7 +7407,7 @@ static void hpsa_hba_inquiry(struct ctlr_info *h)
>  	}
>  }
> 
> -static int hpsa_init_reset_devices(struct pci_dev *pdev)
> +static int hpsa_init_reset_devices(struct pci_dev *pdev, u32 board_id)
>  {
>  	int rc, i;
>  	void __iomem *vaddr;
> @@ -7449,7 +7443,7 @@ static int hpsa_init_reset_devices(struct pci_dev
> *pdev)
>  	iounmap(vaddr);
> 
>  	/* Reset the controller with a PCI power-cycle or via doorbell */
> -	rc = hpsa_kdump_hard_reset_controller(pdev);
> +	rc = hpsa_kdump_hard_reset_controller(pdev, board_id);
> 
>  	/* -ENOTSUPP here means we cannot reset the controller
>  	 * but it's already (and still) up and running in
> @@ -7927,11 +7921,18 @@ static int hpsa_init_one(struct pci_dev *pdev, const
> struct pci_device_id *ent)
>  	struct ctlr_info *h;
>  	int try_soft_reset = 0;
>  	unsigned long flags;
> +	u32 board_id;
> 
>  	if (number_of_controllers == 0)
>  		printk(KERN_INFO DRIVER_NAME "\n");
> 
> -	rc = hpsa_init_reset_devices(pdev);
> +	rc = hpsa_lookup_board_id(pdev, &board_id);
> +	if (rc < 0) {
> +		dev_warn(&pdev->dev, "Board ID not found\n");
> +		return rc;
> +	}
> +
> +	rc = hpsa_init_reset_devices(pdev, board_id);
>  	if (rc) {
>  		if (rc != -ENOTSUPP)
>  			return rc;
> --
> 1.9.3

Re: [PATCH] hpsa: dont meddle with hw which isn't ours (cciss)

[Tomas Henzl]

On 05/18/2015 11:53 PM, Don Brace wrote:
> So, in the context of a kdump, you would like the hpsa driver to not manage kdump and allow the cciss driver to manage the kdump operation?
> 
> I.E. Is this for a controller managed by cciss in the normal kernel and the hpsa reference driver is managing kdump?
I'm not changing this - so what was managed by certain driver
in normal mode remains so in kdump.

This fixes a situation when there is cciss hw only.
When a kernel starts the hpsa driver is invoked
because of the wildcards in the pci-id table and tries
to talk to the hw even if the hw was _not_ meant for
hpsa. That (in kdump) makes the cciss driver later fail.
The cciss driver should be actually immune against this,
but it isn't.
This patch moves the pci-id check closer to the hpsa driver
start before it communicates with the hw.

> 
>> -----Original Message-----
>> From: Tomas Henzl [mailto:thenzl@redhat.com]
>> Sent: Thursday, April 02, 2015 8:26 AM
>> To: linux-scsi@vger.kernel.org
>> Cc: Don Brace; Scott Teel; Justin Lindley; Kevin Barnett
>> Subject: [PATCH] hpsa: dont meddle with hw which isn't ours (cciss)
>>
>> The hpsa driver touches the hardware before checking the pci-id table.
>> This way, especially in kdump, it may confuse the proper driver (cciss).
>>
>> Signed-off-by: Tomas Henzl <thenzl@redhat.com>
>> ---
>>  drivers/scsi/hpsa.c | 21 +++++++++++----------
>>  1 file changed, 11 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
>> index bddb33d30f..c33a077636 100644
>> --- a/drivers/scsi/hpsa.c
>> +++ b/drivers/scsi/hpsa.c
>> @@ -6737,7 +6737,7 @@ static int controller_reset_failed(struct CfgTable
>> __iomem *cfgtable)
>>  /* This does a hard reset of the controller using PCI power management
>>   * states or the using the doorbell register.
>>   */
>> -static int hpsa_kdump_hard_reset_controller(struct pci_dev *pdev)
>> +static int hpsa_kdump_hard_reset_controller(struct pci_dev *pdev, u32
>> board_id)
>>  {
>>  	u64 cfg_offset;
>>  	u32 cfg_base_addr;
>> @@ -6748,7 +6748,6 @@ static int hpsa_kdump_hard_reset_controller(struct
>> pci_dev *pdev)
>>  	int rc;
>>  	struct CfgTable __iomem *cfgtable;
>>  	u32 use_doorbell;
>> -	u32 board_id;
>>  	u16 command_register;
>>
>>  	/* For controllers as old as the P600, this is very nearly
>> @@ -6764,11 +6763,6 @@ static int hpsa_kdump_hard_reset_controller(struct
>> pci_dev *pdev)
>>  	 * using the doorbell register.
>>  	 */
>>
>> -	rc = hpsa_lookup_board_id(pdev, &board_id);
>> -	if (rc < 0) {
>> -		dev_warn(&pdev->dev, "Board ID not found\n");
>> -		return rc;
>> -	}
>>  	if (!ctlr_is_resettable(board_id)) {
>>  		dev_warn(&pdev->dev, "Controller not resettable\n");
>>  		return -ENODEV;
>> @@ -7413,7 +7407,7 @@ static void hpsa_hba_inquiry(struct ctlr_info *h)
>>  	}
>>  }
>>
>> -static int hpsa_init_reset_devices(struct pci_dev *pdev)
>> +static int hpsa_init_reset_devices(struct pci_dev *pdev, u32 board_id)
>>  {
>>  	int rc, i;
>>  	void __iomem *vaddr;
>> @@ -7449,7 +7443,7 @@ static int hpsa_init_reset_devices(struct pci_dev
>> *pdev)
>>  	iounmap(vaddr);
>>
>>  	/* Reset the controller with a PCI power-cycle or via doorbell */
>> -	rc = hpsa_kdump_hard_reset_controller(pdev);
>> +	rc = hpsa_kdump_hard_reset_controller(pdev, board_id);
>>
>>  	/* -ENOTSUPP here means we cannot reset the controller
>>  	 * but it's already (and still) up and running in
>> @@ -7927,11 +7921,18 @@ static int hpsa_init_one(struct pci_dev *pdev, const
>> struct pci_device_id *ent)
>>  	struct ctlr_info *h;
>>  	int try_soft_reset = 0;
>>  	unsigned long flags;
>> +	u32 board_id;
>>
>>  	if (number_of_controllers == 0)
>>  		printk(KERN_INFO DRIVER_NAME "\n");
>>
>> -	rc = hpsa_init_reset_devices(pdev);
>> +	rc = hpsa_lookup_board_id(pdev, &board_id);
>> +	if (rc < 0) {
>> +		dev_warn(&pdev->dev, "Board ID not found\n");
>> +		return rc;
>> +	}
>> +
>> +	rc = hpsa_init_reset_devices(pdev, board_id);
>>  	if (rc) {
>>  		if (rc != -ENOTSUPP)
>>  			return rc;
>> --
>> 1.9.3
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

RE: [PATCH] hpsa: dont meddle with hw which isn't ours (cciss)

[Don Brace]

> -----Original Message-----
> From: Tomas Henzl [mailto:thenzl@redhat.com]
> Sent: Thursday, April 02, 2015 8:26 AM
> To: linux-scsi@vger.kernel.org
> Cc: Don Brace; Scott Teel; Justin Lindley; Kevin Barnett
> Subject: [PATCH] hpsa: dont meddle with hw which isn't ours (cciss)
> 
> The hpsa driver touches the hardware before checking the pci-id table.
> This way, especially in kdump, it may confuse the proper driver (cciss).
> 
> Signed-off-by: Tomas Henzl <thenzl@redhat.com>
> ---
>  drivers/scsi/hpsa.c | 21 +++++++++++----------
>  1 file changed, 11 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
> index bddb33d30f..c33a077636 100644
> --- a/drivers/scsi/hpsa.c
> +++ b/drivers/scsi/hpsa.c
> @@ -6737,7 +6737,7 @@ static int controller_reset_failed(struct CfgTable
> __iomem *cfgtable)
>  /* This does a hard reset of the controller using PCI power management
>   * states or the using the doorbell register.
>   */
> -static int hpsa_kdump_hard_reset_controller(struct pci_dev *pdev)
> +static int hpsa_kdump_hard_reset_controller(struct pci_dev *pdev, u32
> board_id)
>  {
>  	u64 cfg_offset;
>  	u32 cfg_base_addr;
> @@ -6748,7 +6748,6 @@ static int hpsa_kdump_hard_reset_controller(struct
> pci_dev *pdev)
>  	int rc;
>  	struct CfgTable __iomem *cfgtable;
>  	u32 use_doorbell;
> -	u32 board_id;
>  	u16 command_register;
> 
>  	/* For controllers as old as the P600, this is very nearly
> @@ -6764,11 +6763,6 @@ static int hpsa_kdump_hard_reset_controller(struct
> pci_dev *pdev)
>  	 * using the doorbell register.
>  	 */
> 
> -	rc = hpsa_lookup_board_id(pdev, &board_id);
> -	if (rc < 0) {
> -		dev_warn(&pdev->dev, "Board ID not found\n");
> -		return rc;
> -	}
>  	if (!ctlr_is_resettable(board_id)) {
>  		dev_warn(&pdev->dev, "Controller not resettable\n");
>  		return -ENODEV;
> @@ -7413,7 +7407,7 @@ static void hpsa_hba_inquiry(struct ctlr_info *h)
>  	}
>  }
> 
> -static int hpsa_init_reset_devices(struct pci_dev *pdev)
> +static int hpsa_init_reset_devices(struct pci_dev *pdev, u32 board_id)
>  {
>  	int rc, i;
>  	void __iomem *vaddr;
> @@ -7449,7 +7443,7 @@ static int hpsa_init_reset_devices(struct pci_dev
> *pdev)
>  	iounmap(vaddr);
> 
>  	/* Reset the controller with a PCI power-cycle or via doorbell */
> -	rc = hpsa_kdump_hard_reset_controller(pdev);
> +	rc = hpsa_kdump_hard_reset_controller(pdev, board_id);
> 
>  	/* -ENOTSUPP here means we cannot reset the controller
>  	 * but it's already (and still) up and running in
> @@ -7927,11 +7921,18 @@ static int hpsa_init_one(struct pci_dev *pdev, const
> struct pci_device_id *ent)
>  	struct ctlr_info *h;
>  	int try_soft_reset = 0;
>  	unsigned long flags;
> +	u32 board_id;
> 
>  	if (number_of_controllers == 0)
>  		printk(KERN_INFO DRIVER_NAME "\n");
> 
> -	rc = hpsa_init_reset_devices(pdev);
> +	rc = hpsa_lookup_board_id(pdev, &board_id);
> +	if (rc < 0) {
> +		dev_warn(&pdev->dev, "Board ID not found\n");
> +		return rc;
> +	}
> +
> +	rc = hpsa_init_reset_devices(pdev, board_id);
>  	if (rc) {
>  		if (rc != -ENOTSUPP)
>  			return rc;
> --
> 1.9.3

Signed-off-by: Don Brace <don.brace@pmcs.com>