mbox series

[0/4] hisi_sas: DIF/DIX support

Message ID 1542725944-244183-1-git-send-email-john.garry@huawei.com
Headers show
Series hisi_sas: DIF/DIX support | expand

Message

John Garry Nov. 20, 2018, 2:59 p.m. UTC 
This patchset introduces support to the driver for DIF/DIX (or PI -
protection information).

We will only support PI in v3 hw at the moment, even though previous hw
versions also support.

The series is broken down as follows:
- Add a generic helper for checking Scsi command normal operation
- Some tidy-up to accept PI support
- Add components for PI support for main and v3 driver

John Garry (1):
  scsi: Add scsi_prot_op_normal()

Xiang Chen (3):
  scsi: hisi_sas: Relocate some code to reduce complexity
  scsi: hisi_sas: Make sg_tablesize consistent value
  scsi: hisi_sas: Add support for DIF/DIX feature for v3 hw

 drivers/scsi/hisi_sas/hisi_sas.h       |  18 +++
 drivers/scsi/hisi_sas/hisi_sas_main.c  | 242 ++++++++++++++++++++++++---------
 drivers/scsi/hisi_sas/hisi_sas_v1_hw.c |   2 +-
 drivers/scsi/hisi_sas/hisi_sas_v2_hw.c |   2 +-
 drivers/scsi/hisi_sas/hisi_sas_v3_hw.c | 192 +++++++++++++++++++++++++-
 include/scsi/scsi_cmnd.h               |   5 +
 6 files changed, 385 insertions(+), 76 deletions(-)

-- 
1.9.1

Comments

Steffen Maier Nov. 21, 2018, 11:02 a.m. UTC | #1 
On 11/20/2018 03:59 PM, John Garry wrote:
> From: Xiang Chen <chenxiang66@hisilicon.com>

> 

> Sht->sg_tablesize is set in the driver, and it will be assigned to

> shost->sg_tablesize in SCSI mid-layer. So it is not necessary to

> assign shost->sg_table one more time in the driver.

> 

> In addition to the change, change each scsi_host_template.sg_tablesize

> to HISI_SAS_SGE_PAGE_CNT instead of SG_ALL.


Might be completely irrelevant, so just as information: I once had 
problems due to changing (reducing) SHT.sg_tablesize because block queue 
limits of BSG devices of Scsi_Host and fc_host (not sure if you have an 
equivalent bsg device for your transport(s)) inherit from SHT, but don't 
update (automatically) on later updates of shost->sg_tablesize, which in 
turn affect scsi_devices allocated after the shost update.
Cf. 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/s390/scsi?id=5fea4291deacd80188b996d2f555fc6a1940e5d4
("[SCSI] zfcp: block queue limits with data router")
if you need more details.

> Signed-off-by: Xiang Chen <chenxiang66@hisilicon.com>

> Signed-off-by: John Garry <john.garry@huawei.com>

> ---

>   drivers/scsi/hisi_sas/hisi_sas_main.c  | 1 -

>   drivers/scsi/hisi_sas/hisi_sas_v1_hw.c | 2 +-

>   drivers/scsi/hisi_sas/hisi_sas_v2_hw.c | 2 +-

>   drivers/scsi/hisi_sas/hisi_sas_v3_hw.c | 3 +--

>   4 files changed, 3 insertions(+), 5 deletions(-)

> 

> diff --git a/drivers/scsi/hisi_sas/hisi_sas_main.c b/drivers/scsi/hisi_sas/hisi_sas_main.c

> index d13a662..cbda48e 100644

> --- a/drivers/scsi/hisi_sas/hisi_sas_main.c

> +++ b/drivers/scsi/hisi_sas/hisi_sas_main.c

> @@ -2410,7 +2410,6 @@ int hisi_sas_probe(struct platform_device *pdev,

>   	shost->max_lun = ~0;

>   	shost->max_channel = 1;

>   	shost->max_cmd_len = 16;

> -	shost->sg_tablesize = min_t(u16, SG_ALL, HISI_SAS_SGE_PAGE_CNT);

>   	if (hisi_hba->hw->slot_index_alloc) {

>   		shost->can_queue = hisi_hba->hw->max_command_entries;

>   		shost->cmd_per_lun = hisi_hba->hw->max_command_entries;

> diff --git a/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c b/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c

> index d24342b..2d035cc 100644

> --- a/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c

> +++ b/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c

> @@ -1815,7 +1815,7 @@ static int hisi_sas_v1_init(struct hisi_hba *hisi_hba)

>   	.change_queue_depth	= sas_change_queue_depth,

>   	.bios_param		= sas_bios_param,

>   	.this_id		= -1,

> -	.sg_tablesize		= SG_ALL,

> +	.sg_tablesize		= HISI_SAS_SGE_PAGE_CNT,

>   	.max_sectors		= SCSI_DEFAULT_MAX_SECTORS,

>   	.use_clustering		= ENABLE_CLUSTERING,

>   	.eh_device_reset_handler = sas_eh_device_reset_handler,

> diff --git a/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c b/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c

> index e78a97e..79e58a7 100644

> --- a/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c

> +++ b/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c

> @@ -3570,7 +3570,7 @@ struct device_attribute *host_attrs_v2_hw[] = {

>   	.change_queue_depth	= sas_change_queue_depth,

>   	.bios_param		= sas_bios_param,

>   	.this_id		= -1,

> -	.sg_tablesize		= SG_ALL,

> +	.sg_tablesize		= HISI_SAS_SGE_PAGE_CNT,

>   	.max_sectors		= SCSI_DEFAULT_MAX_SECTORS,

>   	.use_clustering		= ENABLE_CLUSTERING,

>   	.eh_device_reset_handler = sas_eh_device_reset_handler,

> diff --git a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c

> index 7e2b020..8a08078 100644

> --- a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c

> +++ b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c

> @@ -2224,7 +2224,7 @@ struct device_attribute *host_attrs_v3_hw[] = {

>   	.change_queue_depth	= sas_change_queue_depth,

>   	.bios_param		= sas_bios_param,

>   	.this_id		= -1,

> -	.sg_tablesize		= SG_ALL,

> +	.sg_tablesize		= HISI_SAS_SGE_PAGE_CNT,

>   	.max_sectors		= SCSI_DEFAULT_MAX_SECTORS,

>   	.use_clustering		= ENABLE_CLUSTERING,

>   	.eh_device_reset_handler = sas_eh_device_reset_handler,

> @@ -2366,7 +2366,6 @@ struct device_attribute *host_attrs_v3_hw[] = {

>   	shost->max_lun = ~0;

>   	shost->max_channel = 1;

>   	shost->max_cmd_len = 16;

> -	shost->sg_tablesize = min_t(u16, SG_ALL, HISI_SAS_SGE_PAGE_CNT);

>   	shost->can_queue = hisi_hba->hw->max_command_entries -

>   		HISI_SAS_RESERVED_IPTT_CNT;

>   	shost->cmd_per_lun = hisi_hba->hw->max_command_entries -

> 



-- 
Mit freundlichen Gruessen / Kind regards
Steffen Maier

Linux on IBM Z Development

IBM Deutschland Research & Development GmbH
Vorsitzende des Aufsichtsrats: Martina Koederitz
Geschaeftsfuehrung: Dirk Wittkopp
Sitz der Gesellschaft: Boeblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
Steffen Maier Nov. 21, 2018, 11:08 a.m. UTC | #2 
On 11/21/2018 12:02 PM, Steffen Maier wrote:
> On 11/20/2018 03:59 PM, John Garry wrote:

>> From: Xiang Chen <chenxiang66@hisilicon.com>

>>

>> Sht->sg_tablesize is set in the driver, and it will be assigned to

>> shost->sg_tablesize in SCSI mid-layer. So it is not necessary to

>> assign shost->sg_table one more time in the driver.

>>

>> In addition to the change, change each scsi_host_template.sg_tablesize

>> to HISI_SAS_SGE_PAGE_CNT instead of SG_ALL.

> 

> Might be completely irrelevant, so just as information: I once had 

> problems due to changing (reducing) SHT.sg_tablesize because block queue 

> limits of BSG devices of Scsi_Host and fc_host (not sure if you have an 

> equivalent bsg device for your transport(s)) inherit from SHT, but don't 

> update (automatically) on later updates of shost->sg_tablesize, which in 

> turn affect scsi_devices allocated after the shost update.

> Cf. 

> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/s390/scsi?id=5fea4291deacd80188b996d2f555fc6a1940e5d4 


Figured, your new constant seems to have the same value so no problem.

#define SG_CHUNK_SIZE	128
#define SG_ALL	SG_CHUNK_SIZE
#define HISI_SAS_SGE_PAGE_CNT SG_CHUNK_SIZE

> ("[SCSI] zfcp: block queue limits with data router")

> if you need more details.

> 

>> Signed-off-by: Xiang Chen <chenxiang66@hisilicon.com>

>> Signed-off-by: John Garry <john.garry@huawei.com>

>> ---

>>   drivers/scsi/hisi_sas/hisi_sas_main.c  | 1 -

>>   drivers/scsi/hisi_sas/hisi_sas_v1_hw.c | 2 +-

>>   drivers/scsi/hisi_sas/hisi_sas_v2_hw.c | 2 +-

>>   drivers/scsi/hisi_sas/hisi_sas_v3_hw.c | 3 +--

>>   4 files changed, 3 insertions(+), 5 deletions(-)

>>

>> diff --git a/drivers/scsi/hisi_sas/hisi_sas_main.c 

>> b/drivers/scsi/hisi_sas/hisi_sas_main.c

>> index d13a662..cbda48e 100644

>> --- a/drivers/scsi/hisi_sas/hisi_sas_main.c

>> +++ b/drivers/scsi/hisi_sas/hisi_sas_main.c

>> @@ -2410,7 +2410,6 @@ int hisi_sas_probe(struct platform_device *pdev,

>>       shost->max_lun = ~0;

>>       shost->max_channel = 1;

>>       shost->max_cmd_len = 16;

>> -    shost->sg_tablesize = min_t(u16, SG_ALL, HISI_SAS_SGE_PAGE_CNT);

>>       if (hisi_hba->hw->slot_index_alloc) {

>>           shost->can_queue = hisi_hba->hw->max_command_entries;

>>           shost->cmd_per_lun = hisi_hba->hw->max_command_entries;

>> diff --git a/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c 

>> b/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c

>> index d24342b..2d035cc 100644

>> --- a/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c

>> +++ b/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c

>> @@ -1815,7 +1815,7 @@ static int hisi_sas_v1_init(struct hisi_hba 

>> *hisi_hba)

>>       .change_queue_depth    = sas_change_queue_depth,

>>       .bios_param        = sas_bios_param,

>>       .this_id        = -1,

>> -    .sg_tablesize        = SG_ALL,

>> +    .sg_tablesize        = HISI_SAS_SGE_PAGE_CNT,

>>       .max_sectors        = SCSI_DEFAULT_MAX_SECTORS,

>>       .use_clustering        = ENABLE_CLUSTERING,

>>       .eh_device_reset_handler = sas_eh_device_reset_handler,

>> diff --git a/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c 

>> b/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c

>> index e78a97e..79e58a7 100644

>> --- a/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c

>> +++ b/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c

>> @@ -3570,7 +3570,7 @@ struct device_attribute *host_attrs_v2_hw[] = {

>>       .change_queue_depth    = sas_change_queue_depth,

>>       .bios_param        = sas_bios_param,

>>       .this_id        = -1,

>> -    .sg_tablesize        = SG_ALL,

>> +    .sg_tablesize        = HISI_SAS_SGE_PAGE_CNT,

>>       .max_sectors        = SCSI_DEFAULT_MAX_SECTORS,

>>       .use_clustering        = ENABLE_CLUSTERING,

>>       .eh_device_reset_handler = sas_eh_device_reset_handler,

>> diff --git a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c 

>> b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c

>> index 7e2b020..8a08078 100644

>> --- a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c

>> +++ b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c

>> @@ -2224,7 +2224,7 @@ struct device_attribute *host_attrs_v3_hw[] = {

>>       .change_queue_depth    = sas_change_queue_depth,

>>       .bios_param        = sas_bios_param,

>>       .this_id        = -1,

>> -    .sg_tablesize        = SG_ALL,

>> +    .sg_tablesize        = HISI_SAS_SGE_PAGE_CNT,

>>       .max_sectors        = SCSI_DEFAULT_MAX_SECTORS,

>>       .use_clustering        = ENABLE_CLUSTERING,

>>       .eh_device_reset_handler = sas_eh_device_reset_handler,

>> @@ -2366,7 +2366,6 @@ struct device_attribute *host_attrs_v3_hw[] = {

>>       shost->max_lun = ~0;

>>       shost->max_channel = 1;

>>       shost->max_cmd_len = 16;

>> -    shost->sg_tablesize = min_t(u16, SG_ALL, HISI_SAS_SGE_PAGE_CNT);

>>       shost->can_queue = hisi_hba->hw->max_command_entries -

>>           HISI_SAS_RESERVED_IPTT_CNT;

>>       shost->cmd_per_lun = hisi_hba->hw->max_command_entries -

>>

> 

> 



-- 
Mit freundlichen Gruessen / Kind regards
Steffen Maier

Linux on IBM Z Development

IBM Deutschland Research & Development GmbH
Vorsitzende des Aufsichtsrats: Martina Koederitz
Geschaeftsfuehrung: Dirk Wittkopp
Sitz der Gesellschaft: Boeblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
John Garry Nov. 21, 2018, 11:45 a.m. UTC | #3 
On 21/11/2018 11:08, Steffen Maier wrote:
> On 11/21/2018 12:02 PM, Steffen Maier wrote:

>> On 11/20/2018 03:59 PM, John Garry wrote:

>>> From: Xiang Chen <chenxiang66@hisilicon.com>

>>>

>>> Sht->sg_tablesize is set in the driver, and it will be assigned to

>>> shost->sg_tablesize in SCSI mid-layer. So it is not necessary to

>>> assign shost->sg_table one more time in the driver.

>>>

>>> In addition to the change, change each scsi_host_template.sg_tablesize

>>> to HISI_SAS_SGE_PAGE_CNT instead of SG_ALL.

>>

>> Might be completely irrelevant, so just as information: I once had

>> problems due to changing (reducing) SHT.sg_tablesize because block

>> queue limits of BSG devices of Scsi_Host and fc_host (not sure if you

>> have an equivalent bsg device for your transport(s)) inherit from SHT,

>> but don't update (automatically) on later updates of

>> shost->sg_tablesize, which in turn affect scsi_devices allocated after

>> the shost update.

>> Cf.

>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/s390/scsi?id=5fea4291deacd80188b996d2f555fc6a1940e5d4

>

>

> Figured, your new constant seems to have the same value so no problem.

>


Right, so previously we were doing similar to what you describe - 
setting the value in the SHT and then setting shost->sg_tablesize after 
the host is allocated.

However the values were the same, so in this patch we're just removing 
setting shost->sg_tablesize (again).

Thanks,
John

> #define SG_CHUNK_SIZE    128

> #define SG_ALL    SG_CHUNK_SIZE

> #define HISI_SAS_SGE_PAGE_CNT SG_CHUNK_SIZE

>

>> ("[SCSI] zfcp: block queue limits with data router")

>> if you need more details.

>>

>>> Signed-off-by: Xiang Chen <chenxiang66@hisilicon.com>

>>> Signed-off-by: John Garry <john.garry@huawei.com>

>>> ---

>>>   drivers/scsi/hisi_sas/hisi_sas_main.c  | 1 -

>>>   drivers/scsi/hisi_sas/hisi_sas_v1_hw.c | 2 +-

>>>   drivers/scsi/hisi_sas/hisi_sas_v2_hw.c | 2 +-

>>>   drivers/scsi/hisi_sas/hisi_sas_v3_hw.c | 3 +--

>>>   4 files changed, 3 insertions(+), 5 deletions(-)

>>>

>>> diff --git a/drivers/scsi/hisi_sas/hisi_sas_main.c

>>> b/drivers/scsi/hisi_sas/hisi_sas_main.c

>>> index d13a662..cbda48e 100644

>>> --- a/drivers/scsi/hisi_sas/hisi_sas_main.c

>>> +++ b/drivers/scsi/hisi_sas/hisi_sas_main.c

>>> @@ -2410,7 +2410,6 @@ int hisi_sas_probe(struct platform_device *pdev,

>>>       shost->max_lun = ~0;

>>>       shost->max_channel = 1;

>>>       shost->max_cmd_len = 16;

>>> -    shost->sg_tablesize = min_t(u16, SG_ALL, HISI_SAS_SGE_PAGE_CNT);

>>>       if (hisi_hba->hw->slot_index_alloc) {

>>>           shost->can_queue = hisi_hba->hw->max_command_entries;

>>>           shost->cmd_per_lun = hisi_hba->hw->max_command_entries;

>>> diff --git a/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c

>>> b/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c

>>> index d24342b..2d035cc 100644

>>> --- a/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c

>>> +++ b/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c

>>> @@ -1815,7 +1815,7 @@ static int hisi_sas_v1_init(struct hisi_hba

>>> *hisi_hba)

>>>       .change_queue_depth    = sas_change_queue_depth,

>>>       .bios_param        = sas_bios_param,

>>>       .this_id        = -1,

>>> -    .sg_tablesize        = SG_ALL,

>>> +    .sg_tablesize        = HISI_SAS_SGE_PAGE_CNT,

>>>       .max_sectors        = SCSI_DEFAULT_MAX_SECTORS,

>>>       .use_clustering        = ENABLE_CLUSTERING,

>>>       .eh_device_reset_handler = sas_eh_device_reset_handler,

>>> diff --git a/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c

>>> b/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c

>>> index e78a97e..79e58a7 100644

>>> --- a/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c

>>> +++ b/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c

>>> @@ -3570,7 +3570,7 @@ struct device_attribute *host_attrs_v2_hw[] = {

>>>       .change_queue_depth    = sas_change_queue_depth,

>>>       .bios_param        = sas_bios_param,

>>>       .this_id        = -1,

>>> -    .sg_tablesize        = SG_ALL,

>>> +    .sg_tablesize        = HISI_SAS_SGE_PAGE_CNT,

>>>       .max_sectors        = SCSI_DEFAULT_MAX_SECTORS,

>>>       .use_clustering        = ENABLE_CLUSTERING,

>>>       .eh_device_reset_handler = sas_eh_device_reset_handler,

>>> diff --git a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c

>>> b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c

>>> index 7e2b020..8a08078 100644

>>> --- a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c

>>> +++ b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c

>>> @@ -2224,7 +2224,7 @@ struct device_attribute *host_attrs_v3_hw[] = {

>>>       .change_queue_depth    = sas_change_queue_depth,

>>>       .bios_param        = sas_bios_param,

>>>       .this_id        = -1,

>>> -    .sg_tablesize        = SG_ALL,

>>> +    .sg_tablesize        = HISI_SAS_SGE_PAGE_CNT,

>>>       .max_sectors        = SCSI_DEFAULT_MAX_SECTORS,

>>>       .use_clustering        = ENABLE_CLUSTERING,

>>>       .eh_device_reset_handler = sas_eh_device_reset_handler,

>>> @@ -2366,7 +2366,6 @@ struct device_attribute *host_attrs_v3_hw[] = {

>>>       shost->max_lun = ~0;

>>>       shost->max_channel = 1;

>>>       shost->max_cmd_len = 16;

>>> -    shost->sg_tablesize = min_t(u16, SG_ALL, HISI_SAS_SGE_PAGE_CNT);

>>>       shost->can_queue = hisi_hba->hw->max_command_entries -

>>>           HISI_SAS_RESERVED_IPTT_CNT;

>>>       shost->cmd_per_lun = hisi_hba->hw->max_command_entries -

>>>

>>

>>

>

>