Re: [PATCH 4/5] smartpqi: fix critical ARM issue reading PQI index registers
On Mon, 2018-06-18 at 13:23 -0500, Don Brace wrote:
> From: Kevin Barnett
>
> - use the readl() kernel function to read all index
> registers. For ARM systems, this function includes a
> read memory barrier that eliminates ci/pi corruption.
>
> Reviewed-by: Scott Benesh
> Reviewed-by: Scott Teel
> Signed-off-by: Kevin Barnett
> Signed-off-by: Don Brace
> ---
> drivers/scsi/smartpqi/smartpqi.h | 10 ---
> drivers/scsi/smartpqi/smartpqi_init.c | 45 ++-
> --
> 2 files changed, 30 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/scsi/smartpqi/smartpqi.h
> b/drivers/scsi/smartpqi/smartpqi.h
> index a8e7c4d48061..e97bf2670315 100644
> --- a/drivers/scsi/smartpqi/smartpqi.h
> +++ b/drivers/scsi/smartpqi/smartpqi.h
> @@ -583,8 +583,8 @@ struct pqi_admin_queues_aligned {
> struct pqi_admin_queues {
> void*iq_element_array;
> void*oq_element_array;
> - volatile pqi_index_t *iq_ci;
> - volatile pqi_index_t *oq_pi;
> + pqi_index_t *iq_ci;
> + pqi_index_t __iomem *oq_pi;
> dma_addr_t iq_element_array_bus_addr;
> dma_addr_t oq_element_array_bus_addr;
> dma_addr_t iq_ci_bus_addr;
> @@ -608,8 +608,8 @@ struct pqi_queue_group {
> dma_addr_t oq_element_array_bus_addr;
> __le32 __iomem *iq_pi[2];
> pqi_index_t iq_pi_copy[2];
> - volatile pqi_index_t *iq_ci[2];
> - volatile pqi_index_t *oq_pi;
> + pqi_index_t __iomem *iq_ci[2];
> + pqi_index_t __iomem *oq_pi;
> dma_addr_t iq_ci_bus_addr[2];
> dma_addr_t oq_pi_bus_addr;
> __le32 __iomem *oq_ci;
> @@ -622,7 +622,7 @@ struct pqi_event_queue {
> u16 oq_id;
> u16 int_msg_num;
> void*oq_element_array;
> - volatile pqi_index_t *oq_pi;
> + pqi_index_t __iomem *oq_pi;
> dma_addr_t oq_element_array_bus_addr;
> dma_addr_t oq_pi_bus_addr;
> __le32 __iomem *oq_ci;
> diff --git a/drivers/scsi/smartpqi/smartpqi_init.c
> b/drivers/scsi/smartpqi/smartpqi_init.c
> index 8b70b879735e..b4a685ed9ed1 100644
> --- a/drivers/scsi/smartpqi/smartpqi_init.c
> +++ b/drivers/scsi/smartpqi/smartpqi_init.c
> @@ -2703,7 +2703,7 @@ static unsigned int pqi_process_io_intr(struct
> pqi_ctrl_info *ctrl_info,
> oq_ci = queue_group->oq_ci_copy;
>
> while (1) {
> - oq_pi = *queue_group->oq_pi;
> + oq_pi = readl(queue_group->oq_pi);
> if (oq_pi == oq_ci)
> break;
>
> @@ -2794,7 +2794,7 @@ static void pqi_send_event_ack(struct
> pqi_ctrl_info *ctrl_info,
> spin_lock_irqsave(&queue_group-
> >submit_lock[RAID_PATH], flags);
>
> iq_pi = queue_group->iq_pi_copy[RAID_PATH];
> - iq_ci = *queue_group->iq_ci[RAID_PATH];
> + iq_ci = readl(queue_group->iq_ci[RAID_PATH]);
>
> if (pqi_num_elements_free(iq_pi, iq_ci,
> ctrl_info->num_elements_per_iq))
> @@ -2953,7 +2953,7 @@ static unsigned int
> pqi_process_event_intr(struct pqi_ctrl_info *ctrl_info)
> oq_ci = event_queue->oq_ci_copy;
>
> while (1) {
> - oq_pi = *event_queue->oq_pi;
> + oq_pi = readl(event_queue->oq_pi);
> if (oq_pi == oq_ci)
> break;
>
> @@ -3177,7 +3177,7 @@ static int pqi_alloc_operational_queues(struct
> pqi_ctrl_info *ctrl_info)
> size_t element_array_length_per_iq;
> size_t element_array_length_per_oq;
> void *element_array;
> - void *next_queue_index;
> + void __iomem *next_queue_index;
> void *aligned_pointer;
> unsigned int num_inbound_queues;
> unsigned int num_outbound_queues;
> @@ -3273,7 +3273,7 @@ static int pqi_alloc_operational_queues(struct
> pqi_ctrl_info *ctrl_info)
> element_array += PQI_NUM_EVENT_QUEUE_ELEMENTS *
> PQI_EVENT_OQ_ELEMENT_LENGTH;
>
> - next_queue_index = PTR_ALIGN(element_array,
> + next_queue_index = (void __iomem *)PTR_ALIGN(element_array,
> PQI_OPERATIONAL_INDEX_ALIGNMENT);
>
> for (i = 0; i < ctrl_info->num_queue_groups; i++) {
> @@ -3281,21 +3281,24 @@ static int
> pqi_alloc_operational_queues(struct pqi_ctrl_info *ctrl_info)
> queue_group->iq_ci[RAID_PATH] = next_queue_index;
> queue_group->iq_ci_bus_addr[RAID_PATH] =
> ctrl_info->queue_memory_base_dma_handle +
> - (next_queue_index - ctrl_info-
> >queue_memory_base);
> + (next_queue_index -
> + (void __iomem *)ctrl_info-
> >queue_memory_base);
> next_queue_index += sizeof(pqi_index_t);
> next_queue_index = PTR_ALIGN(next_queue_index,
> PQI_OPERATIONAL_INDEX_ALIGNMENT);
> queue_group->iq_ci[AIO_PATH] = next_queue_index;
> queue_gro
[PATCH 4/5] smartpqi: fix critical ARM issue reading PQI index registers
From: Kevin Barnett
- use the readl() kernel function to read all index
registers. For ARM systems, this function includes a
read memory barrier that eliminates ci/pi corruption.
Reviewed-by: Scott Benesh
Reviewed-by: Scott Teel
Signed-off-by: Kevin Barnett
Signed-off-by: Don Brace
---
drivers/scsi/smartpqi/smartpqi.h | 10 ---
drivers/scsi/smartpqi/smartpqi_init.c | 45 ++---
2 files changed, 30 insertions(+), 25 deletions(-)
diff --git a/drivers/scsi/smartpqi/smartpqi.h b/drivers/scsi/smartpqi/smartpqi.h
index a8e7c4d48061..e97bf2670315 100644
--- a/drivers/scsi/smartpqi/smartpqi.h
+++ b/drivers/scsi/smartpqi/smartpqi.h
@@ -583,8 +583,8 @@ struct pqi_admin_queues_aligned {
struct pqi_admin_queues {
void*iq_element_array;
void*oq_element_array;
- volatile pqi_index_t *iq_ci;
- volatile pqi_index_t *oq_pi;
+ pqi_index_t *iq_ci;
+ pqi_index_t __iomem *oq_pi;
dma_addr_t iq_element_array_bus_addr;
dma_addr_t oq_element_array_bus_addr;
dma_addr_t iq_ci_bus_addr;
@@ -608,8 +608,8 @@ struct pqi_queue_group {
dma_addr_t oq_element_array_bus_addr;
__le32 __iomem *iq_pi[2];
pqi_index_t iq_pi_copy[2];
- volatile pqi_index_t *iq_ci[2];
- volatile pqi_index_t *oq_pi;
+ pqi_index_t __iomem *iq_ci[2];
+ pqi_index_t __iomem *oq_pi;
dma_addr_t iq_ci_bus_addr[2];
dma_addr_t oq_pi_bus_addr;
__le32 __iomem *oq_ci;
@@ -622,7 +622,7 @@ struct pqi_event_queue {
u16 oq_id;
u16 int_msg_num;
void*oq_element_array;
- volatile pqi_index_t *oq_pi;
+ pqi_index_t __iomem *oq_pi;
dma_addr_t oq_element_array_bus_addr;
dma_addr_t oq_pi_bus_addr;
__le32 __iomem *oq_ci;
diff --git a/drivers/scsi/smartpqi/smartpqi_init.c
b/drivers/scsi/smartpqi/smartpqi_init.c
index 8b70b879735e..b4a685ed9ed1 100644
--- a/drivers/scsi/smartpqi/smartpqi_init.c
+++ b/drivers/scsi/smartpqi/smartpqi_init.c
@@ -2703,7 +2703,7 @@ static unsigned int pqi_process_io_intr(struct
pqi_ctrl_info *ctrl_info,
oq_ci = queue_group->oq_ci_copy;
while (1) {
- oq_pi = *queue_group->oq_pi;
+ oq_pi = readl(queue_group->oq_pi);
if (oq_pi == oq_ci)
break;
@@ -2794,7 +2794,7 @@ static void pqi_send_event_ack(struct pqi_ctrl_info
*ctrl_info,
spin_lock_irqsave(&queue_group->submit_lock[RAID_PATH], flags);
iq_pi = queue_group->iq_pi_copy[RAID_PATH];
- iq_ci = *queue_group->iq_ci[RAID_PATH];
+ iq_ci = readl(queue_group->iq_ci[RAID_PATH]);
if (pqi_num_elements_free(iq_pi, iq_ci,
ctrl_info->num_elements_per_iq))
@@ -2953,7 +2953,7 @@ static unsigned int pqi_process_event_intr(struct
pqi_ctrl_info *ctrl_info)
oq_ci = event_queue->oq_ci_copy;
while (1) {
- oq_pi = *event_queue->oq_pi;
+ oq_pi = readl(event_queue->oq_pi);
if (oq_pi == oq_ci)
break;
@@ -3177,7 +3177,7 @@ static int pqi_alloc_operational_queues(struct
pqi_ctrl_info *ctrl_info)
size_t element_array_length_per_iq;
size_t element_array_length_per_oq;
void *element_array;
- void *next_queue_index;
+ void __iomem *next_queue_index;
void *aligned_pointer;
unsigned int num_inbound_queues;
unsigned int num_outbound_queues;
@@ -3273,7 +3273,7 @@ static int pqi_alloc_operational_queues(struct
pqi_ctrl_info *ctrl_info)
element_array += PQI_NUM_EVENT_QUEUE_ELEMENTS *
PQI_EVENT_OQ_ELEMENT_LENGTH;
- next_queue_index = PTR_ALIGN(element_array,
+ next_queue_index = (void __iomem *)PTR_ALIGN(element_array,
PQI_OPERATIONAL_INDEX_ALIGNMENT);
for (i = 0; i < ctrl_info->num_queue_groups; i++) {
@@ -3281,21 +3281,24 @@ static int pqi_alloc_operational_queues(struct
pqi_ctrl_info *ctrl_info)
queue_group->iq_ci[RAID_PATH] = next_queue_index;
queue_group->iq_ci_bus_addr[RAID_PATH] =
ctrl_info->queue_memory_base_dma_handle +
- (next_queue_index - ctrl_info->queue_memory_base);
+ (next_queue_index -
+ (void __iomem *)ctrl_info->queue_memory_base);
next_queue_index += sizeof(pqi_index_t);
next_queue_index = PTR_ALIGN(next_queue_index,
PQI_OPERATIONAL_INDEX_ALIGNMENT);
queue_group->iq_ci[AIO_PATH] = next_queue_index;
queue_group->iq_ci_bus_addr[AIO_PATH] =
ctrl_info->queue_memory_base_dma_handle +
- (next_queue_index - ctrl_info->que
