Re: [PATCH v1 01/20] [SCSI] mpt3sas: Added Combined Reply Queue feature to extend up-to 96 MSIX vector support
Sreekanth == Sreekanth Reddy sreekanth.re...@avagotech.com writes: Sreekanth I will remove these extra brackets. but SAS3 HBA's less than Sreekanth C0 revision (which doesn't support this Combined Reply Queue Sreekanth feature) will support up to 16 MSI-X vectors. Ah, I missed that. Fair enough. -- Martin K. Petersen Oracle Linux Engineering -- To unsubscribe from this list: send the line unsubscribe linux-scsi in
Re: [PATCH v1 01/20] [SCSI] mpt3sas: Added Combined Reply Queue feature to extend up-to 96 MSIX vector support
On Fri, Jun 19, 2015 at 8:27 PM, Martin K. Petersen
martin.peter...@oracle.com wrote:
Sreekanth == Sreekanth Reddy sreekanth.re...@avagotech.com writes:
Sreekanth,
It's fine that you outline the 96 / 12 = 8 layout in the patch
description. But that relationship is not made clear when reading the
code. Please add a comment describing why things are set up this way.
Accepted. I will add the comments where ever required.
@@ -1009,8 +1009,15 @@ _base_interrupt(int irq, void *bus_id)
}
wmb();
- writel(reply_q-reply_post_host_index | (msix_index
- MPI2_RPHI_MSIX_INDEX_SHIFT), ioc-chip-ReplyPostHostIndex);
+ if (ioc-msix96_vector) {
+ writel(reply_q-reply_post_host_index | ((msix_index 7)
+ MPI2_RPHI_MSIX_INDEX_SHIFT),
+ ioc-replyPostRegisterIndex[msix_index/8]);
+ } else {
+ writel(reply_q-reply_post_host_index | (msix_index
+ MPI2_RPHI_MSIX_INDEX_SHIFT),
+ ioc-chip-ReplyPostHostIndex);
+ }
Too many brackets. Why don't you do:
I will remove these extra brackets. but SAS3 HBA's less than C0
revision (which doesn't support this Combined Reply Queue feature)
will support up to 16 MSI-X vectors. so can't update the MSIxIndex
field with just from 0 to 7 by taking the modular of 8 on msix_index
value. so can't do below this.
index = reply_q-reply_post_host_index |
((msix_index 7) MPI_RPHI_MSIX_INDEX_SHIFT);
if (ioc-msix96_vector)
writel(index, ioc-replyPostRegisterIndex[msix_index / 8]);
else
writel(index, ioc-chip-ReplyPostHostIndex);
+ if (ioc-msix96_vector ioc-reply_queue_count 8) {
+ /* If this is an 96 vector supported device,
+ set up ReplyPostIndex addresses */
Bad comment formatting.
Accepted, will update this.
+ ioc-replyPostRegisterIndex = kcalloc(12,
+ sizeof(resource_size_t *), GFP_KERNEL);
[...]
+ for (i = 0; i 12; i++) {
Make 12 a constant or at the very least a variable with a comment.
Agreed. Will do this.
+ ioc-replyPostRegisterIndex[i] = (resource_size_t *)
+ ((u8 *)ioc-chip-Doorbell +
+ MPI25_SUP_REPLY_POST_HOST_INDEX_OFFSET +
+ (i * 0x10));
0x10 - Another magic constant.
Accepted, Will update.
@@ -4522,8 +4554,16 @@ _base_make_ioc_operational(struct MPT3SAS_ADAPTER
*ioc, int sleep_flag)
/* initialize reply post host index */
list_for_each_entry(reply_q, ioc-reply_queue_list, list) {
- writel(reply_q-msix_index MPI2_RPHI_MSIX_INDEX_SHIFT,
- ioc-chip-ReplyPostHostIndex);
+ if (ioc-msix96_vector) {
+ writel((reply_q-msix_index 7)
+MPI2_RPHI_MSIX_INDEX_SHIFT,
+ioc-replyPostRegisterIndex[reply_q-msix_index/8]);
+ } else {
+ writel(reply_q-msix_index
+ MPI2_RPHI_MSIX_INDEX_SHIFT,
+ ioc-chip-ReplyPostHostIndex);
+ }
+
Too many brackets.
I will remove these brackets and also will post the next version of
this patch by considering all above comments.
--
Martin K. Petersen Oracle Linux Engineering
--
Regards,
Sreekanth
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
[PATCH v1 01/20] [SCSI] mpt3sas: Added Combined Reply Queue feature to extend up-to 96 MSIX vector support
In this patch, increased the number of MSIX vector support for SAS3 C0 HBAs to
up-to 96.
Following are changes that are done in this patch
1. Support this feature only for SAS3 C0 cards and also only when reply post
free queue count is greater than 8.
2. Instead of using single ReplyPostHostIndex system interface, here 12
ReplyPostHostIndex system interfaces are used. reply post free queues numbered
from 0 to 7 use the first ReplyPostHostIndex system interface to update its
corresponding ReplyPostHostIndex values, reply post free queues numbered from 8
to 15 will use the second ReplyPostHostIndex system interface and so on. These
12 ReplyPostHostIndex system interfaces address are saved in the array
replyPostRegisterIndex[].
3. Update the ReplyPostHostIndex value of corresponding reply post free queue
in the (its msix_index/8)th entry of replyPostRegisterIndex[] array after
processing the reply post descriptor.
Changes in v1:
Updated the description of module parameter max_msix_vectors
Signed-off-by: Sreekanth Reddy sreekanth.re...@avagotech.com
---
drivers/scsi/mpt3sas/mpt3sas_base.c | 72 -
drivers/scsi/mpt3sas/mpt3sas_base.h | 7 +++-
2 files changed, 70 insertions(+), 9 deletions(-)
diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c
b/drivers/scsi/mpt3sas/mpt3sas_base.c
index 14a781b..7d0ec5c 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
@@ -83,10 +83,10 @@ static int msix_disable = -1;
module_param(msix_disable, int, 0);
MODULE_PARM_DESC(msix_disable, disable msix routed interrupts (default=0));
-static int max_msix_vectors = 8;
+static int max_msix_vectors = -1;
module_param(max_msix_vectors, int, 0);
MODULE_PARM_DESC(max_msix_vectors,
-max msix vectors - (default=8));
+max msix vectors);
static int mpt3sas_fwfault_debug;
MODULE_PARM_DESC(mpt3sas_fwfault_debug,
@@ -1009,8 +1009,15 @@ _base_interrupt(int irq, void *bus_id)
}
wmb();
- writel(reply_q-reply_post_host_index | (msix_index
- MPI2_RPHI_MSIX_INDEX_SHIFT), ioc-chip-ReplyPostHostIndex);
+ if (ioc-msix96_vector) {
+ writel(reply_q-reply_post_host_index | ((msix_index 7)
+ MPI2_RPHI_MSIX_INDEX_SHIFT),
+ ioc-replyPostRegisterIndex[msix_index/8]);
+ } else {
+ writel(reply_q-reply_post_host_index | (msix_index
+ MPI2_RPHI_MSIX_INDEX_SHIFT),
+ ioc-chip-ReplyPostHostIndex);
+ }
atomic_dec(reply_q-busy);
return IRQ_HANDLED;
}
@@ -1560,8 +1567,6 @@ _base_check_enable_msix(struct MPT3SAS_ADAPTER *ioc)
pci_read_config_word(ioc-pdev, base + 2, message_control);
ioc-msix_vector_count = (message_control 0x3FF) + 1;
- if (ioc-msix_vector_count 8)
- ioc-msix_vector_count = 8;
dinitprintk(ioc, pr_info(MPT3SAS_FMT
msix is supported, vector_count(%d)\n,
ioc-name, ioc-msix_vector_count));
@@ -1880,6 +1885,31 @@ mpt3sas_base_map_resources(struct MPT3SAS_ADAPTER *ioc)
if (r)
goto out_fail;
+ /* Use the Combined reply queue feature only for SAS3 C0 HBAs and
+* also only when reply queue count is greater than 8
+*/
+ if (ioc-msix96_vector ioc-reply_queue_count 8) {
+ /* If this is an 96 vector supported device,
+ set up ReplyPostIndex addresses */
+ ioc-replyPostRegisterIndex = kcalloc(12,
+ sizeof(resource_size_t *), GFP_KERNEL);
+ if (!ioc-replyPostRegisterIndex) {
+ dfailprintk(ioc, printk(MPT3SAS_FMT
+ allocation for reply Post Register Index failed!!!\n,
+ ioc-name));
+ r = -ENOMEM;
+ goto out_fail;
+ }
+
+ for (i = 0; i 12; i++) {
+ ioc-replyPostRegisterIndex[i] = (resource_size_t *)
+ ((u8 *)ioc-chip-Doorbell +
+ MPI25_SUP_REPLY_POST_HOST_INDEX_OFFSET +
+ (i * 0x10));
+ }
+ } else
+ ioc-msix96_vector = 0;
+
list_for_each_entry(reply_q, ioc-reply_queue_list, list)
pr_info(MPT3SAS_FMT %s: IRQ %d\n,
reply_q-name, ((ioc-msix_enable) ? PCI-MSI-X enabled :
@@ -1901,6 +1931,8 @@ mpt3sas_base_map_resources(struct MPT3SAS_ADAPTER *ioc)
pci_release_selected_regions(ioc-pdev, ioc-bars);
pci_disable_pcie_error_reporting(pdev);
pci_disable_device(pdev);
+ if (ioc-msix96_vector)
+ kfree(ioc-replyPostRegisterIndex);
return r;
}
@@ -4522,8 +4554,16 @@ _base_make_ioc_operational(struct MPT3SAS_ADAPTER *ioc,
int
Re: [PATCH v1 01/20] [SCSI] mpt3sas: Added Combined Reply Queue feature to extend up-to 96 MSIX vector support
On Fri, Jun 19, 2015 at 04:25:46PM +0530, Sreekanth Reddy wrote:
In this patch, increased the number of MSIX vector support for SAS3 C0 HBAs
to up-to 96.
Following are changes that are done in this patch
1. Support this feature only for SAS3 C0 cards and also only when reply post
free queue count is greater than 8.
2. Instead of using single ReplyPostHostIndex system interface, here 12
ReplyPostHostIndex system interfaces are used. reply post free queues
numbered from 0 to 7 use the first ReplyPostHostIndex system interface to
update its corresponding ReplyPostHostIndex values, reply post free queues
numbered from 8 to 15 will use the second ReplyPostHostIndex system interface
and so on. These 12 ReplyPostHostIndex system interfaces address are saved in
the array replyPostRegisterIndex[].
3. Update the ReplyPostHostIndex value of corresponding reply post free queue
in the (its msix_index/8)th entry of replyPostRegisterIndex[] array after
processing the reply post descriptor.
Changes in v1:
Updated the description of module parameter max_msix_vectors
Signed-off-by: Sreekanth Reddy sreekanth.re...@avagotech.com
---
drivers/scsi/mpt3sas/mpt3sas_base.c | 72
-
drivers/scsi/mpt3sas/mpt3sas_base.h | 7 +++-
2 files changed, 70 insertions(+), 9 deletions(-)
diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c
b/drivers/scsi/mpt3sas/mpt3sas_base.c
index 14a781b..7d0ec5c 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
@@ -83,10 +83,10 @@ static int msix_disable = -1;
module_param(msix_disable, int, 0);
MODULE_PARM_DESC(msix_disable, disable msix routed interrupts
(default=0));
-static int max_msix_vectors = 8;
+static int max_msix_vectors = -1;
module_param(max_msix_vectors, int, 0);
MODULE_PARM_DESC(max_msix_vectors,
- max msix vectors - (default=8));
+ max msix vectors);
static int mpt3sas_fwfault_debug;
MODULE_PARM_DESC(mpt3sas_fwfault_debug,
@@ -1009,8 +1009,15 @@ _base_interrupt(int irq, void *bus_id)
}
wmb();
- writel(reply_q-reply_post_host_index | (msix_index
- MPI2_RPHI_MSIX_INDEX_SHIFT), ioc-chip-ReplyPostHostIndex);
+ if (ioc-msix96_vector) {
+ writel(reply_q-reply_post_host_index | ((msix_index 7)
+ MPI2_RPHI_MSIX_INDEX_SHIFT),
+ ioc-replyPostRegisterIndex[msix_index/8]);
+ } else {
+ writel(reply_q-reply_post_host_index | (msix_index
+ MPI2_RPHI_MSIX_INDEX_SHIFT),
+ ioc-chip-ReplyPostHostIndex);
+ }
atomic_dec(reply_q-busy);
return IRQ_HANDLED;
}
@@ -1560,8 +1567,6 @@ _base_check_enable_msix(struct MPT3SAS_ADAPTER *ioc)
pci_read_config_word(ioc-pdev, base + 2, message_control);
ioc-msix_vector_count = (message_control 0x3FF) + 1;
- if (ioc-msix_vector_count 8)
- ioc-msix_vector_count = 8;
dinitprintk(ioc, pr_info(MPT3SAS_FMT
msix is supported, vector_count(%d)\n,
ioc-name, ioc-msix_vector_count));
@@ -1880,6 +1885,31 @@ mpt3sas_base_map_resources(struct MPT3SAS_ADAPTER *ioc)
if (r)
goto out_fail;
+ /* Use the Combined reply queue feature only for SAS3 C0 HBAs and
+ * also only when reply queue count is greater than 8
+ */
+ if (ioc-msix96_vector ioc-reply_queue_count 8) {
+ /* If this is an 96 vector supported device,
+ set up ReplyPostIndex addresses */
+ ioc-replyPostRegisterIndex = kcalloc(12,
+ sizeof(resource_size_t *), GFP_KERNEL);
+ if (!ioc-replyPostRegisterIndex) {
+ dfailprintk(ioc, printk(MPT3SAS_FMT
+ allocation for reply Post Register Index failed!!!\n,
+ioc-name));
+ r = -ENOMEM;
+ goto out_fail;
+ }
+
+ for (i = 0; i 12; i++) {
+ ioc-replyPostRegisterIndex[i] = (resource_size_t *)
+ ((u8 *)ioc-chip-Doorbell +
+ MPI25_SUP_REPLY_POST_HOST_INDEX_OFFSET +
+ (i * 0x10));
+ }
+ } else
+ ioc-msix96_vector = 0;
+
list_for_each_entry(reply_q, ioc-reply_queue_list, list)
pr_info(MPT3SAS_FMT %s: IRQ %d\n,
reply_q-name, ((ioc-msix_enable) ? PCI-MSI-X enabled :
@@ -1901,6 +1931,8 @@ mpt3sas_base_map_resources(struct MPT3SAS_ADAPTER *ioc)
pci_release_selected_regions(ioc-pdev, ioc-bars);
pci_disable_pcie_error_reporting(pdev);
pci_disable_device(pdev);
+ if (ioc-msix96_vector)
+ kfree(ioc-replyPostRegisterIndex);
return r;
}
@@ -4522,8 +4554,16 @@
Re: [PATCH v1 01/20] [SCSI] mpt3sas: Added Combined Reply Queue feature to extend up-to 96 MSIX vector support
Sreekanth == Sreekanth Reddy sreekanth.re...@avagotech.com writes:
Sreekanth,
It's fine that you outline the 96 / 12 = 8 layout in the patch
description. But that relationship is not made clear when reading the
code. Please add a comment describing why things are set up this way.
@@ -1009,8 +1009,15 @@ _base_interrupt(int irq, void *bus_id)
}
wmb();
- writel(reply_q-reply_post_host_index | (msix_index
- MPI2_RPHI_MSIX_INDEX_SHIFT), ioc-chip-ReplyPostHostIndex);
+ if (ioc-msix96_vector) {
+ writel(reply_q-reply_post_host_index | ((msix_index 7)
+ MPI2_RPHI_MSIX_INDEX_SHIFT),
+ ioc-replyPostRegisterIndex[msix_index/8]);
+ } else {
+ writel(reply_q-reply_post_host_index | (msix_index
+ MPI2_RPHI_MSIX_INDEX_SHIFT),
+ ioc-chip-ReplyPostHostIndex);
+ }
Too many brackets. Why don't you do:
index = reply_q-reply_post_host_index |
((msix_index 7) MPI_RPHI_MSIX_INDEX_SHIFT);
if (ioc-msix96_vector)
writel(index, ioc-replyPostRegisterIndex[msix_index / 8]);
else
writel(index, ioc-chip-ReplyPostHostIndex);
+ if (ioc-msix96_vector ioc-reply_queue_count 8) {
+ /* If this is an 96 vector supported device,
+ set up ReplyPostIndex addresses */
Bad comment formatting.
+ ioc-replyPostRegisterIndex = kcalloc(12,
+ sizeof(resource_size_t *), GFP_KERNEL);
[...]
+ for (i = 0; i 12; i++) {
Make 12 a constant or at the very least a variable with a comment.
+ ioc-replyPostRegisterIndex[i] = (resource_size_t *)
+ ((u8 *)ioc-chip-Doorbell +
+ MPI25_SUP_REPLY_POST_HOST_INDEX_OFFSET +
+ (i * 0x10));
0x10 - Another magic constant.
@@ -4522,8 +4554,16 @@ _base_make_ioc_operational(struct MPT3SAS_ADAPTER
*ioc, int sleep_flag)
/* initialize reply post host index */
list_for_each_entry(reply_q, ioc-reply_queue_list, list) {
- writel(reply_q-msix_index MPI2_RPHI_MSIX_INDEX_SHIFT,
- ioc-chip-ReplyPostHostIndex);
+ if (ioc-msix96_vector) {
+ writel((reply_q-msix_index 7)
+MPI2_RPHI_MSIX_INDEX_SHIFT,
+ioc-replyPostRegisterIndex[reply_q-msix_index/8]);
+ } else {
+ writel(reply_q-msix_index
+ MPI2_RPHI_MSIX_INDEX_SHIFT,
+ ioc-chip-ReplyPostHostIndex);
+ }
+
Too many brackets.
--
Martin K. Petersen Oracle Linux Engineering
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
