* [PATCH v2 4/5] ibmvfc: store return code of H_FREE_SUB_CRQ during cleanup
From: Tyrel Datwyler @ 2021-02-25 20:48 UTC (permalink / raw)
To: james.bottomley
Cc: Tyrel Datwyler, martin.petersen, linux-scsi, linux-kernel, brking,
linuxppc-dev
In-Reply-To: <20210225204824.14570-1-tyreld@linux.ibm.com>
The H_FREE_SUB_CRQ hypercall can return a retry delay return code that
indicates the call needs to be retried after a specific amount of time
delay. The error path to free a sub-CRQ in case of a failure during
channel registration fails to capture the return code of H_FREE_SUB_CRQ
which will result in the delay loop being skipped in the case of a retry
delay return code.
Store the return code result of the H_FREE_SUB_CRQ call such that the
return code check in the delay loop evaluates a meaningful value. Also,
use the rtas_busy_delay() to check the rc value and delay for the
appropriate amount of time.
Fixes: 9288d35d70b5 ("ibmvfc: map/request irq and register Sub-CRQ interrupt handler")
Signed-off-by: Tyrel Datwyler <tyreld@linux.ibm.com>
---
drivers/scsi/ibmvscsi/ibmvfc.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/scsi/ibmvscsi/ibmvfc.c b/drivers/scsi/ibmvscsi/ibmvfc.c
index ba6fcf9cbc57..4ac2c442e1e2 100644
--- a/drivers/scsi/ibmvscsi/ibmvfc.c
+++ b/drivers/scsi/ibmvscsi/ibmvfc.c
@@ -21,6 +21,7 @@
#include <linux/bsg-lib.h>
#include <asm/firmware.h>
#include <asm/irq.h>
+#include <asm/rtas.h>
#include <asm/vio.h>
#include <scsi/scsi.h>
#include <scsi/scsi_cmnd.h>
@@ -5670,8 +5671,8 @@ static int ibmvfc_register_scsi_channel(struct ibmvfc_host *vhost,
irq_failed:
do {
- plpar_hcall_norets(H_FREE_SUB_CRQ, vdev->unit_address, scrq->cookie);
- } while (rc == H_BUSY || H_IS_LONG_BUSY(rc));
+ rc = plpar_hcall_norets(H_FREE_SUB_CRQ, vdev->unit_address, scrq->cookie);
+ } while (rtas_busy_delay(rc));
reg_failed:
ibmvfc_free_queue(vhost, scrq);
LEAVE;
--
2.27.0
^ permalink raw reply related
* Re: [PATCH v2 5/5] ibmvfc: reinitialize sub-CRQs and perform channel enquiry after LPM
From: Tyrel Datwyler @ 2021-02-25 20:52 UTC (permalink / raw)
To: james.bottomley
Cc: brking, linuxppc-dev, linux-scsi, martin.petersen, linux-kernel
In-Reply-To: <20210225204824.14570-6-tyreld@linux.ibm.com>
On 2/25/21 12:48 PM, Tyrel Datwyler wrote:
> A live partition migration (LPM) results in a CRQ disconnect similar to
> a hard reset. In this LPM case the hypervisor moslty perserves the CRQ
> transport such that it simply needs to be reenabled. However, the
> capabilities may have changed such as fewer channels, or no channels at
> all. Further, its possible that there may be sub-CRQ support, but no
> channel support. The CRQ reenable path currently doesn't take any of
> this into consideration.
>
> For simpilicty release and reinitialize sub-CRQs during reenable, and
> set do_enquiry and using_channels with the appropriate values to trigger
> channel renegotiation.
>
This should of had the same fixes tag as patch 2.
fixes: faacf8c5f1d5 ("ibmvfc: add alloc/dealloc routines for SCSI Sub-CRQ Channels")
> Signed-off-by: Tyrel Datwyler <tyreld@linux.ibm.com>
> ---
> drivers/scsi/ibmvscsi/ibmvfc.c | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
>
> diff --git a/drivers/scsi/ibmvscsi/ibmvfc.c b/drivers/scsi/ibmvscsi/ibmvfc.c
> index 4ac2c442e1e2..9ae6be56e375 100644
> --- a/drivers/scsi/ibmvscsi/ibmvfc.c
> +++ b/drivers/scsi/ibmvscsi/ibmvfc.c
> @@ -903,6 +903,9 @@ static int ibmvfc_reenable_crq_queue(struct ibmvfc_host *vhost)
> {
> int rc = 0;
> struct vio_dev *vdev = to_vio_dev(vhost->dev);
> + unsigned long flags;
> +
> + ibmvfc_release_sub_crqs(vhost);
>
> /* Re-enable the CRQ */
> do {
> @@ -914,6 +917,16 @@ static int ibmvfc_reenable_crq_queue(struct ibmvfc_host *vhost)
> if (rc)
> dev_err(vhost->dev, "Error enabling adapter (rc=%d)\n", rc);
>
> + spin_lock_irqsave(vhost->host->host_lock, flags);
> + spin_lock(vhost->crq.q_lock);
> + vhost->do_enquiry = 1;
> + vhost->using_channels = 0;
> +
> + ibmvfc_init_sub_crqs(vhost);
> +
> + spin_unlock(vhost->crq.q_lock);
> + spin_unlock_irqrestore(vhost->host->host_lock, flags);
> +
> return rc;
> }
>
>
^ permalink raw reply
* Freeing page tables through RCU
From: Matthew Wilcox @ 2021-02-25 20:58 UTC (permalink / raw)
To: linux-mm, linuxppc-dev, linux-s390; +Cc: linux-kernel
In order to walk the page tables without the mmap semaphore, it must
be possible to prevent them from being freed and reused (eg if munmap()
races with viewing /proc/$pid/smaps).
There is various commentary within the mm on how to prevent this. One way
is to disable interrupts, relying on that to block rcu_sched or IPIs.
I don't think the RT people are terribly happy about reading a proc file
disabling interrupts, and it doesn't work for architectures that free
page tables directly instead of batching them into an rcu_sched (because
the IPI may not be sent to this CPU if the task has never run on it).
See "Fast GUP" in mm/gup.c
Ideally, I'd like rcu_read_lock() to delay page table reuse. This is
close to trivial for architectures which use entire pages or multiple
pages for levels of their page tables as we can use the rcu_head embedded
in struct page to queue the page for RCU.
s390 and powerpc are the only two architectures I know of that have
levels of their page table that are smaller than their PAGE_SIZE.
I'd like to discuss options. There may be a complicated scheme that
allows partial pages to be freed via RCU, but I have something simpler
in mind. For powerpc in particular, it can have a PAGE_SIZE of 64kB
and then the MMU wants to see 4kB entries in the PMD. I suggest that
instead of allocating each 4kB entry individually, we allocate a 64kB
page and fill in 16 consecutive PMDs. This could cost a bit more memory
(although if you've asked for a CONFIG_PAGE_SIZE of 64kB, you presumably
don't care too much about it), but it'll make future page faults cheaper
(as the PMDs will already be present, assuming you have good locality
of reference).
I'd like to hear better ideas than this.
^ permalink raw reply
* Re: [PATCH v2 4/5] ibmvfc: store return code of H_FREE_SUB_CRQ during cleanup
From: Brian King @ 2021-02-25 21:05 UTC (permalink / raw)
To: Tyrel Datwyler, james.bottomley
Cc: brking, linuxppc-dev, linux-scsi, martin.petersen, linux-kernel
In-Reply-To: <20210225204824.14570-5-tyreld@linux.ibm.com>
Reviewed-by: Brian King <brking@linux.vnet.ibm.com>
--
Brian King
Power Linux I/O
IBM Linux Technology Center
^ permalink raw reply
* Re: [PATCH v2 5/5] ibmvfc: reinitialize sub-CRQs and perform channel enquiry after LPM
From: Brian King @ 2021-02-25 21:17 UTC (permalink / raw)
To: Tyrel Datwyler, james.bottomley
Cc: brking, linuxppc-dev, linux-scsi, martin.petersen, linux-kernel
In-Reply-To: <20210225204824.14570-6-tyreld@linux.ibm.com>
On 2/25/21 2:48 PM, Tyrel Datwyler wrote:
> A live partition migration (LPM) results in a CRQ disconnect similar to
> a hard reset. In this LPM case the hypervisor moslty perserves the CRQ
> transport such that it simply needs to be reenabled. However, the
> capabilities may have changed such as fewer channels, or no channels at
> all. Further, its possible that there may be sub-CRQ support, but no
> channel support. The CRQ reenable path currently doesn't take any of
> this into consideration.
>
> For simpilicty release and reinitialize sub-CRQs during reenable, and
> set do_enquiry and using_channels with the appropriate values to trigger
> channel renegotiation.
>
> Signed-off-by: Tyrel Datwyler <tyreld@linux.ibm.com>
> ---
> drivers/scsi/ibmvscsi/ibmvfc.c | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
>
> diff --git a/drivers/scsi/ibmvscsi/ibmvfc.c b/drivers/scsi/ibmvscsi/ibmvfc.c
> index 4ac2c442e1e2..9ae6be56e375 100644
> --- a/drivers/scsi/ibmvscsi/ibmvfc.c
> +++ b/drivers/scsi/ibmvscsi/ibmvfc.c
> @@ -903,6 +903,9 @@ static int ibmvfc_reenable_crq_queue(struct ibmvfc_host *vhost)
> {
> int rc = 0;
> struct vio_dev *vdev = to_vio_dev(vhost->dev);
> + unsigned long flags;
> +
> + ibmvfc_release_sub_crqs(vhost);
>
> /* Re-enable the CRQ */
> do {
> @@ -914,6 +917,16 @@ static int ibmvfc_reenable_crq_queue(struct ibmvfc_host *vhost)
> if (rc)
> dev_err(vhost->dev, "Error enabling adapter (rc=%d)\n", rc);
>
> + spin_lock_irqsave(vhost->host->host_lock, flags);
> + spin_lock(vhost->crq.q_lock);
> + vhost->do_enquiry = 1;
> + vhost->using_channels = 0;
> +
> + ibmvfc_init_sub_crqs(vhost);
> +
> + spin_unlock(vhost->crq.q_lock);
> + spin_unlock_irqrestore(vhost->host->host_lock, flags);
ibmvfc_init_sub_crqs can sleep, for multiple reasons, so you can't hold
a lock when you call it. There is a GFP_KERNEL allocation in it, and the
patch before this one adds an msleep in an error path.
Thanks,
Brian
--
Brian King
Power Linux I/O
IBM Linux Technology Center
^ permalink raw reply
* [PATCH v3 2/5] ibmvfc: fix invalid sub-CRQ handles after hard reset
From: Tyrel Datwyler @ 2021-02-25 21:42 UTC (permalink / raw)
To: james.bottomley
Cc: Tyrel Datwyler, martin.petersen, linux-scsi, linux-kernel, brking,
linuxppc-dev
In-Reply-To: <20210225214237.22400-1-tyreld@linux.ibm.com>
A hard reset results in a complete transport disconnect such that the
CRQ connection with the partner VIOS is broken. This has the side effect
of also invalidating the associated sub-CRQs. The current code assumes
that the sub-CRQs are perserved resulting in a protocol violation after
trying to reconnect them with the VIOS. This introduces an infinite loop
such that the VIOS forces a disconnect after each subsequent attempt to
re-register with invalid handles.
Avoid the aforementioned issue by releasing the sub-CRQs prior to CRQ
disconnect, and driving a reinitialization of the sub-CRQs once a new
CRQ is registered with the hypervisor.
fixes: faacf8c5f1d5 ("ibmvfc: add alloc/dealloc routines for SCSI Sub-CRQ Channels")
Signed-off-by: Tyrel Datwyler <tyreld@linux.ibm.com>
Reviewed-by: Brian King <brking@linux.ibm.com>
---
drivers/scsi/ibmvscsi/ibmvfc.c | 21 +++++++++------------
1 file changed, 9 insertions(+), 12 deletions(-)
diff --git a/drivers/scsi/ibmvscsi/ibmvfc.c b/drivers/scsi/ibmvscsi/ibmvfc.c
index 384960036f8b..2cca55f2e464 100644
--- a/drivers/scsi/ibmvscsi/ibmvfc.c
+++ b/drivers/scsi/ibmvscsi/ibmvfc.c
@@ -158,6 +158,9 @@ static void ibmvfc_npiv_logout(struct ibmvfc_host *);
static void ibmvfc_tgt_implicit_logout_and_del(struct ibmvfc_target *);
static void ibmvfc_tgt_move_login(struct ibmvfc_target *);
+static void ibmvfc_release_sub_crqs(struct ibmvfc_host *);
+static void ibmvfc_init_sub_crqs(struct ibmvfc_host *);
+
static const char *unknown_error = "unknown error";
static long h_reg_sub_crq(unsigned long unit_address, unsigned long ioba,
@@ -926,8 +929,8 @@ static int ibmvfc_reset_crq(struct ibmvfc_host *vhost)
unsigned long flags;
struct vio_dev *vdev = to_vio_dev(vhost->dev);
struct ibmvfc_queue *crq = &vhost->crq;
- struct ibmvfc_queue *scrq;
- int i;
+
+ ibmvfc_release_sub_crqs(vhost);
/* Close the CRQ */
do {
@@ -936,6 +939,8 @@ static int ibmvfc_reset_crq(struct ibmvfc_host *vhost)
rc = plpar_hcall_norets(H_FREE_CRQ, vdev->unit_address);
} while (rc == H_BUSY || H_IS_LONG_BUSY(rc));
+ ibmvfc_init_sub_crqs(vhost);
+
spin_lock_irqsave(vhost->host->host_lock, flags);
spin_lock(vhost->crq.q_lock);
vhost->state = IBMVFC_NO_CRQ;
@@ -947,16 +952,6 @@ static int ibmvfc_reset_crq(struct ibmvfc_host *vhost)
memset(crq->msgs.crq, 0, PAGE_SIZE);
crq->cur = 0;
- if (vhost->scsi_scrqs.scrqs) {
- for (i = 0; i < nr_scsi_hw_queues; i++) {
- scrq = &vhost->scsi_scrqs.scrqs[i];
- spin_lock(scrq->q_lock);
- memset(scrq->msgs.scrq, 0, PAGE_SIZE);
- scrq->cur = 0;
- spin_unlock(scrq->q_lock);
- }
- }
-
/* And re-open it again */
rc = plpar_hcall_norets(H_REG_CRQ, vdev->unit_address,
crq->msg_token, PAGE_SIZE);
@@ -966,6 +961,7 @@ static int ibmvfc_reset_crq(struct ibmvfc_host *vhost)
dev_warn(vhost->dev, "Partner adapter not ready\n");
else if (rc != 0)
dev_warn(vhost->dev, "Couldn't register crq (rc=%d)\n", rc);
+
spin_unlock(vhost->crq.q_lock);
spin_unlock_irqrestore(vhost->host->host_lock, flags);
@@ -5692,6 +5688,7 @@ static void ibmvfc_deregister_scsi_channel(struct ibmvfc_host *vhost, int index)
free_irq(scrq->irq, scrq);
irq_dispose_mapping(scrq->irq);
+ scrq->irq = 0;
do {
rc = plpar_hcall_norets(H_FREE_SUB_CRQ, vdev->unit_address,
--
2.27.0
^ permalink raw reply related
* [PATCH v3 1/5] ibmvfc: simplify handling of sub-CRQ initialization
From: Tyrel Datwyler @ 2021-02-25 21:42 UTC (permalink / raw)
To: james.bottomley
Cc: Tyrel Datwyler, martin.petersen, linux-scsi, linux-kernel, brking,
linuxppc-dev
In-Reply-To: <20210225214237.22400-1-tyreld@linux.ibm.com>
If ibmvfc_init_sub_crqs() fails ibmvfc_probe() simply parrots
registration failure reported elsewhere, and futher
vhost->scsi_scrq.scrq == NULL is indication enough to the driver that it
has no sub-CRQs available. The mq_enabled check can also be moved into
ibmvfc_init_sub_crqs() such that each caller doesn't have to gate the
call with a mq_enabled check. Finally, in the case of sub-CRQ setup
failure setting do_enquiry can be turned off to putting the driver into
single queue fallback mode.
The aforementioned changes also simplify the next patch in the series
that fixes a hard reset issue, by tying a sub-CRQ setup failure and
do_enquiry logic into ibmvfc_init_sub_crqs().
Signed-off-by: Tyrel Datwyler <tyreld@linux.ibm.com>
Reviewed-by: Brian King <brking@linux.ibm.com>
---
drivers/scsi/ibmvscsi/ibmvfc.c | 21 ++++++++++-----------
1 file changed, 10 insertions(+), 11 deletions(-)
diff --git a/drivers/scsi/ibmvscsi/ibmvfc.c b/drivers/scsi/ibmvscsi/ibmvfc.c
index 7097028d4cb6..384960036f8b 100644
--- a/drivers/scsi/ibmvscsi/ibmvfc.c
+++ b/drivers/scsi/ibmvscsi/ibmvfc.c
@@ -5705,17 +5705,21 @@ static void ibmvfc_deregister_scsi_channel(struct ibmvfc_host *vhost, int index)
LEAVE;
}
-static int ibmvfc_init_sub_crqs(struct ibmvfc_host *vhost)
+static void ibmvfc_init_sub_crqs(struct ibmvfc_host *vhost)
{
int i, j;
ENTER;
+ if (!vhost->mq_enabled)
+ return;
vhost->scsi_scrqs.scrqs = kcalloc(nr_scsi_hw_queues,
sizeof(*vhost->scsi_scrqs.scrqs),
GFP_KERNEL);
- if (!vhost->scsi_scrqs.scrqs)
- return -1;
+ if (!vhost->scsi_scrqs.scrqs) {
+ vhost->do_enquiry = 0;
+ return;
+ }
for (i = 0; i < nr_scsi_hw_queues; i++) {
if (ibmvfc_register_scsi_channel(vhost, i)) {
@@ -5724,13 +5728,12 @@ static int ibmvfc_init_sub_crqs(struct ibmvfc_host *vhost)
kfree(vhost->scsi_scrqs.scrqs);
vhost->scsi_scrqs.scrqs = NULL;
vhost->scsi_scrqs.active_queues = 0;
- LEAVE;
- return -1;
+ vhost->do_enquiry = 0;
+ break;
}
}
LEAVE;
- return 0;
}
static void ibmvfc_release_sub_crqs(struct ibmvfc_host *vhost)
@@ -5997,11 +6000,7 @@ static int ibmvfc_probe(struct vio_dev *vdev, const struct vio_device_id *id)
goto remove_shost;
}
- if (vhost->mq_enabled) {
- rc = ibmvfc_init_sub_crqs(vhost);
- if (rc)
- dev_warn(dev, "Failed to allocate Sub-CRQs. rc=%d\n", rc);
- }
+ ibmvfc_init_sub_crqs(vhost);
if (shost_to_fc_host(shost)->rqst_q)
blk_queue_max_segments(shost_to_fc_host(shost)->rqst_q, 1);
--
2.27.0
^ permalink raw reply related
* [PATCH v3 0/5] ibmvfc: hard reset fixes
From: Tyrel Datwyler @ 2021-02-25 21:42 UTC (permalink / raw)
To: james.bottomley
Cc: Tyrel Datwyler, martin.petersen, linux-scsi, linux-kernel, brking,
linuxppc-dev
This series contains a minor simplification of ibmvfc_init_sub_crqs() followed
by a couple fixes for sub-CRQ handling which effect hard reset of the
client/host adapter CRQ pair.
changes in v3:
* Patch 1 & 5: moved ibmvfc_init_sub_crqs out of locked patch
changes in v2:
* added Reviewed-by tags for patches 1-3
* Patch 4: use rtas_busy_delay to test rc and delay correct amount of time
* Patch 5: (new) similar fix for LPM case where CRQ pair needs re-enablement
Tyrel Datwyler (5):
powerpc/pseries: extract host bridge from pci_bus prior to bus removal
ibmvfc: simplify handling of sub-CRQ initialization
ibmvfc: fix invalid sub-CRQ handles after hard reset
ibmvfc: treat H_CLOSED as success during sub-CRQ registration
ibmvfc: store return code of H_FREE_SUB_CRQ during cleanup
arch/powerpc/platforms/pseries/pci_dlpar.c | 4 +-
drivers/scsi/ibmvscsi/ibmvfc.c | 49 ++++++++++------------
2 files changed, 26 insertions(+), 27 deletions(-)
--
2.27.0
^ permalink raw reply
* [PATCH v3 3/5] ibmvfc: treat H_CLOSED as success during sub-CRQ registration
From: Tyrel Datwyler @ 2021-02-25 21:42 UTC (permalink / raw)
To: james.bottomley
Cc: Tyrel Datwyler, martin.petersen, linux-scsi, linux-kernel, brking,
linuxppc-dev
In-Reply-To: <20210225214237.22400-1-tyreld@linux.ibm.com>
A non-zero return code for H_REG_SUB_CRQ is currently treated as a
failure resulting in failing sub-CRQ setup. The case of H_CLOSED should
not be treated as a failure. This return code translates to a successful
sub-CRQ registration by the hypervisor, and is meant to communicate back
that there is currently no partner VIOS CRQ connection established as of
yet. This is a common occurrence during a disconnect where the client
adapter can possibly come back up prior to the partner adapter.
For non-zero return code from H_REG_SUB_CRQ treat a H_CLOSED as success
so that sub-CRQs are successfully setup.
Fixes: faacf8c5f1d5 ("ibmvfc: add alloc/dealloc routines for SCSI Sub-CRQ Channels")
Signed-off-by: Tyrel Datwyler <tyreld@linux.ibm.com>
Reviewed-by: Brian King <brking@linux.ibm.com>
---
drivers/scsi/ibmvscsi/ibmvfc.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/scsi/ibmvscsi/ibmvfc.c b/drivers/scsi/ibmvscsi/ibmvfc.c
index 2cca55f2e464..274c5a1fac9c 100644
--- a/drivers/scsi/ibmvscsi/ibmvfc.c
+++ b/drivers/scsi/ibmvscsi/ibmvfc.c
@@ -5636,7 +5636,8 @@ static int ibmvfc_register_scsi_channel(struct ibmvfc_host *vhost,
rc = h_reg_sub_crq(vdev->unit_address, scrq->msg_token, PAGE_SIZE,
&scrq->cookie, &scrq->hw_irq);
- if (rc) {
+ /* H_CLOSED indicates successful register, but no CRQ partner */
+ if (rc && rc != H_CLOSED) {
dev_warn(dev, "Error registering sub-crq: %d\n", rc);
if (rc == H_PARAMETER)
dev_warn_once(dev, "Firmware may not support MQ\n");
--
2.27.0
^ permalink raw reply related
* [PATCH v3 4/5] ibmvfc: store return code of H_FREE_SUB_CRQ during cleanup
From: Tyrel Datwyler @ 2021-02-25 21:42 UTC (permalink / raw)
To: james.bottomley
Cc: Tyrel Datwyler, martin.petersen, linux-scsi, linux-kernel, brking,
linuxppc-dev
In-Reply-To: <20210225214237.22400-1-tyreld@linux.ibm.com>
The H_FREE_SUB_CRQ hypercall can return a retry delay return code that
indicates the call needs to be retried after a specific amount of time
delay. The error path to free a sub-CRQ in case of a failure during
channel registration fails to capture the return code of H_FREE_SUB_CRQ
which will result in the delay loop being skipped in the case of a retry
delay return code.
Store the return code result of the H_FREE_SUB_CRQ call such that the
return code check in the delay loop evaluates a meaningful value. Also,
use the rtas_busy_delay() to check the rc value and delay for the
appropriate amount of time.
Fixes: 9288d35d70b5 ("ibmvfc: map/request irq and register Sub-CRQ interrupt handler")
Signed-off-by: Tyrel Datwyler <tyreld@linux.ibm.com>
Reviewed-by: Brian King <brking@linux.ibm.com>
---
drivers/scsi/ibmvscsi/ibmvfc.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/scsi/ibmvscsi/ibmvfc.c b/drivers/scsi/ibmvscsi/ibmvfc.c
index 274c5a1fac9c..1bb08e5f3674 100644
--- a/drivers/scsi/ibmvscsi/ibmvfc.c
+++ b/drivers/scsi/ibmvscsi/ibmvfc.c
@@ -21,6 +21,7 @@
#include <linux/bsg-lib.h>
#include <asm/firmware.h>
#include <asm/irq.h>
+#include <asm/rtas.h>
#include <asm/vio.h>
#include <scsi/scsi.h>
#include <scsi/scsi_cmnd.h>
@@ -5670,8 +5671,8 @@ static int ibmvfc_register_scsi_channel(struct ibmvfc_host *vhost,
irq_failed:
do {
- plpar_hcall_norets(H_FREE_SUB_CRQ, vdev->unit_address, scrq->cookie);
- } while (rc == H_BUSY || H_IS_LONG_BUSY(rc));
+ rc = plpar_hcall_norets(H_FREE_SUB_CRQ, vdev->unit_address, scrq->cookie);
+ } while (rtas_busy_delay(rc));
reg_failed:
ibmvfc_free_queue(vhost, scrq);
LEAVE;
--
2.27.0
^ permalink raw reply related
* [PATCH v3 5/5] ibmvfc: reinitialize sub-CRQs and perform channel enquiry after LPM
From: Tyrel Datwyler @ 2021-02-25 21:42 UTC (permalink / raw)
To: james.bottomley
Cc: Tyrel Datwyler, martin.petersen, linux-scsi, linux-kernel, brking,
linuxppc-dev
In-Reply-To: <20210225214237.22400-1-tyreld@linux.ibm.com>
A live partition migration (LPM) results in a CRQ disconnect similar to
a hard reset. In this LPM case the hypervisor moslty perserves the CRQ
transport such that it simply needs to be reenabled. However, the
capabilities may have changed such as fewer channels, or no channels at
all. Further, its possible that there may be sub-CRQ support, but no
channel support. The CRQ reenable path currently doesn't take any of
this into consideration.
For simpilicty release and reinitialize sub-CRQs during reenable, and
set do_enquiry and using_channels with the appropriate values to trigger
channel renegotiation.
Signed-off-by: Tyrel Datwyler <tyreld@linux.ibm.com>
---
drivers/scsi/ibmvscsi/ibmvfc.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/drivers/scsi/ibmvscsi/ibmvfc.c b/drivers/scsi/ibmvscsi/ibmvfc.c
index 1bb08e5f3674..6bbc2697ad5a 100644
--- a/drivers/scsi/ibmvscsi/ibmvfc.c
+++ b/drivers/scsi/ibmvscsi/ibmvfc.c
@@ -903,6 +903,9 @@ static int ibmvfc_reenable_crq_queue(struct ibmvfc_host *vhost)
{
int rc = 0;
struct vio_dev *vdev = to_vio_dev(vhost->dev);
+ unsigned long flags;
+
+ ibmvfc_release_sub_crqs(vhost);
/* Re-enable the CRQ */
do {
@@ -914,6 +917,15 @@ static int ibmvfc_reenable_crq_queue(struct ibmvfc_host *vhost)
if (rc)
dev_err(vhost->dev, "Error enabling adapter (rc=%d)\n", rc);
+ ibmvfc_init_sub_crqs(vhost);
+
+ spin_lock_irqsave(vhost->host->host_lock, flags);
+ spin_lock(vhost->crq.q_lock);
+ vhost->do_enquiry = 1;
+ vhost->using_channels = 0;
+ spin_unlock(vhost->crq.q_lock);
+ spin_unlock_irqrestore(vhost->host->host_lock, flags);
+
return rc;
}
--
2.27.0
^ permalink raw reply related
* [PATCH v4 1/5] ibmvfc: simplify handling of sub-CRQ initialization
From: Tyrel Datwyler @ 2021-02-25 21:50 UTC (permalink / raw)
To: james.bottomley
Cc: Tyrel Datwyler, martin.petersen, linux-scsi, linux-kernel, brking,
linuxppc-dev
In-Reply-To: <20210225215057.23020-1-tyreld@linux.ibm.com>
If ibmvfc_init_sub_crqs() fails ibmvfc_probe() simply parrots
registration failure reported elsewhere, and futher
vhost->scsi_scrq.scrq == NULL is indication enough to the driver that it
has no sub-CRQs available. The mq_enabled check can also be moved into
ibmvfc_init_sub_crqs() such that each caller doesn't have to gate the
call with a mq_enabled check. Finally, in the case of sub-CRQ setup
failure setting do_enquiry can be turned off to putting the driver into
single queue fallback mode.
The aforementioned changes also simplify the next patch in the series
that fixes a hard reset issue, by tying a sub-CRQ setup failure and
do_enquiry logic into ibmvfc_init_sub_crqs().
Signed-off-by: Tyrel Datwyler <tyreld@linux.ibm.com>
Reviewed-by: Brian King <brking@linux.ibm.com>
---
drivers/scsi/ibmvscsi/ibmvfc.c | 21 ++++++++++-----------
1 file changed, 10 insertions(+), 11 deletions(-)
diff --git a/drivers/scsi/ibmvscsi/ibmvfc.c b/drivers/scsi/ibmvscsi/ibmvfc.c
index 7097028d4cb6..384960036f8b 100644
--- a/drivers/scsi/ibmvscsi/ibmvfc.c
+++ b/drivers/scsi/ibmvscsi/ibmvfc.c
@@ -5705,17 +5705,21 @@ static void ibmvfc_deregister_scsi_channel(struct ibmvfc_host *vhost, int index)
LEAVE;
}
-static int ibmvfc_init_sub_crqs(struct ibmvfc_host *vhost)
+static void ibmvfc_init_sub_crqs(struct ibmvfc_host *vhost)
{
int i, j;
ENTER;
+ if (!vhost->mq_enabled)
+ return;
vhost->scsi_scrqs.scrqs = kcalloc(nr_scsi_hw_queues,
sizeof(*vhost->scsi_scrqs.scrqs),
GFP_KERNEL);
- if (!vhost->scsi_scrqs.scrqs)
- return -1;
+ if (!vhost->scsi_scrqs.scrqs) {
+ vhost->do_enquiry = 0;
+ return;
+ }
for (i = 0; i < nr_scsi_hw_queues; i++) {
if (ibmvfc_register_scsi_channel(vhost, i)) {
@@ -5724,13 +5728,12 @@ static int ibmvfc_init_sub_crqs(struct ibmvfc_host *vhost)
kfree(vhost->scsi_scrqs.scrqs);
vhost->scsi_scrqs.scrqs = NULL;
vhost->scsi_scrqs.active_queues = 0;
- LEAVE;
- return -1;
+ vhost->do_enquiry = 0;
+ break;
}
}
LEAVE;
- return 0;
}
static void ibmvfc_release_sub_crqs(struct ibmvfc_host *vhost)
@@ -5997,11 +6000,7 @@ static int ibmvfc_probe(struct vio_dev *vdev, const struct vio_device_id *id)
goto remove_shost;
}
- if (vhost->mq_enabled) {
- rc = ibmvfc_init_sub_crqs(vhost);
- if (rc)
- dev_warn(dev, "Failed to allocate Sub-CRQs. rc=%d\n", rc);
- }
+ ibmvfc_init_sub_crqs(vhost);
if (shost_to_fc_host(shost)->rqst_q)
blk_queue_max_segments(shost_to_fc_host(shost)->rqst_q, 1);
--
2.27.0
^ permalink raw reply related
* [PATCH v4 4/5] ibmvfc: store return code of H_FREE_SUB_CRQ during cleanup
From: Tyrel Datwyler @ 2021-02-25 21:50 UTC (permalink / raw)
To: james.bottomley
Cc: Tyrel Datwyler, martin.petersen, linux-scsi, linux-kernel, brking,
linuxppc-dev
In-Reply-To: <20210225215057.23020-1-tyreld@linux.ibm.com>
The H_FREE_SUB_CRQ hypercall can return a retry delay return code that
indicates the call needs to be retried after a specific amount of time
delay. The error path to free a sub-CRQ in case of a failure during
channel registration fails to capture the return code of H_FREE_SUB_CRQ
which will result in the delay loop being skipped in the case of a retry
delay return code.
Store the return code result of the H_FREE_SUB_CRQ call such that the
return code check in the delay loop evaluates a meaningful value. Also,
use the rtas_busy_delay() to check the rc value and delay for the
appropriate amount of time.
Fixes: 9288d35d70b5 ("ibmvfc: map/request irq and register Sub-CRQ interrupt handler")
Signed-off-by: Tyrel Datwyler <tyreld@linux.ibm.com>
Reviewed-by: Brian King <brking@linux.ibm.com>
---
drivers/scsi/ibmvscsi/ibmvfc.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/scsi/ibmvscsi/ibmvfc.c b/drivers/scsi/ibmvscsi/ibmvfc.c
index 1d9f961715ca..ef03fa559433 100644
--- a/drivers/scsi/ibmvscsi/ibmvfc.c
+++ b/drivers/scsi/ibmvscsi/ibmvfc.c
@@ -21,6 +21,7 @@
#include <linux/bsg-lib.h>
#include <asm/firmware.h>
#include <asm/irq.h>
+#include <asm/rtas.h>
#include <asm/vio.h>
#include <scsi/scsi.h>
#include <scsi/scsi_cmnd.h>
@@ -5670,8 +5671,8 @@ static int ibmvfc_register_scsi_channel(struct ibmvfc_host *vhost,
irq_failed:
do {
- plpar_hcall_norets(H_FREE_SUB_CRQ, vdev->unit_address, scrq->cookie);
- } while (rc == H_BUSY || H_IS_LONG_BUSY(rc));
+ rc = plpar_hcall_norets(H_FREE_SUB_CRQ, vdev->unit_address, scrq->cookie);
+ } while (rtas_busy_delay(rc));
reg_failed:
ibmvfc_free_queue(vhost, scrq);
LEAVE;
--
2.27.0
^ permalink raw reply related
* [PATCH v4 3/5] ibmvfc: treat H_CLOSED as success during sub-CRQ registration
From: Tyrel Datwyler @ 2021-02-25 21:50 UTC (permalink / raw)
To: james.bottomley
Cc: Tyrel Datwyler, martin.petersen, linux-scsi, linux-kernel, brking,
linuxppc-dev
In-Reply-To: <20210225215057.23020-1-tyreld@linux.ibm.com>
A non-zero return code for H_REG_SUB_CRQ is currently treated as a
failure resulting in failing sub-CRQ setup. The case of H_CLOSED should
not be treated as a failure. This return code translates to a successful
sub-CRQ registration by the hypervisor, and is meant to communicate back
that there is currently no partner VIOS CRQ connection established as of
yet. This is a common occurrence during a disconnect where the client
adapter can possibly come back up prior to the partner adapter.
For non-zero return code from H_REG_SUB_CRQ treat a H_CLOSED as success
so that sub-CRQs are successfully setup.
Fixes: faacf8c5f1d5 ("ibmvfc: add alloc/dealloc routines for SCSI Sub-CRQ Channels")
Signed-off-by: Tyrel Datwyler <tyreld@linux.ibm.com>
Reviewed-by: Brian King <brking@linux.ibm.com>
---
drivers/scsi/ibmvscsi/ibmvfc.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/scsi/ibmvscsi/ibmvfc.c b/drivers/scsi/ibmvscsi/ibmvfc.c
index d34e1a4f74d9..1d9f961715ca 100644
--- a/drivers/scsi/ibmvscsi/ibmvfc.c
+++ b/drivers/scsi/ibmvscsi/ibmvfc.c
@@ -5636,7 +5636,8 @@ static int ibmvfc_register_scsi_channel(struct ibmvfc_host *vhost,
rc = h_reg_sub_crq(vdev->unit_address, scrq->msg_token, PAGE_SIZE,
&scrq->cookie, &scrq->hw_irq);
- if (rc) {
+ /* H_CLOSED indicates successful register, but no CRQ partner */
+ if (rc && rc != H_CLOSED) {
dev_warn(dev, "Error registering sub-crq: %d\n", rc);
if (rc == H_PARAMETER)
dev_warn_once(dev, "Firmware may not support MQ\n");
--
2.27.0
^ permalink raw reply related
* [PATCH v4 0/5] ibmvfc: hard reset fixes
From: Tyrel Datwyler @ 2021-02-25 21:50 UTC (permalink / raw)
To: james.bottomley
Cc: Tyrel Datwyler, martin.petersen, linux-scsi, linux-kernel, brking,
linuxppc-dev
This series contains a minor simplification of ibmvfc_init_sub_crqs() followed
by a couple fixes for sub-CRQ handling which effect hard reset of the
client/host adapter CRQ pair.
changes in v4:
Patch 2: dropped Reviewed-by tag and moved sub-crq init to after locked region
Patch 5: moved sub-crq init to after locked region
changes in v3:
* Patch 1 & 5: moved ibmvfc_init_sub_crqs out of locked patch
changes in v2:
* added Reviewed-by tags for patches 1-3
* Patch 4: use rtas_busy_delay to test rc and delay correct amount of time
* Patch 5: (new) similar fix for LPM case where CRQ pair needs re-enablement
Tyrel Datwyler (5):
powerpc/pseries: extract host bridge from pci_bus prior to bus removal
ibmvfc: simplify handling of sub-CRQ initialization
ibmvfc: fix invalid sub-CRQ handles after hard reset
ibmvfc: treat H_CLOSED as success during sub-CRQ registration
ibmvfc: store return code of H_FREE_SUB_CRQ during cleanup
arch/powerpc/platforms/pseries/pci_dlpar.c | 4 +-
drivers/scsi/ibmvscsi/ibmvfc.c | 49 ++++++++++------------
2 files changed, 26 insertions(+), 27 deletions(-)
--
2.27.0
^ permalink raw reply
* [PATCH v4 5/5] ibmvfc: reinitialize sub-CRQs and perform channel enquiry after LPM
From: Tyrel Datwyler @ 2021-02-25 21:50 UTC (permalink / raw)
To: james.bottomley
Cc: Tyrel Datwyler, martin.petersen, linux-scsi, linux-kernel, brking,
linuxppc-dev
In-Reply-To: <20210225215057.23020-1-tyreld@linux.ibm.com>
A live partition migration (LPM) results in a CRQ disconnect similar to
a hard reset. In this LPM case the hypervisor moslty perserves the CRQ
transport such that it simply needs to be reenabled. However, the
capabilities may have changed such as fewer channels, or no channels at
all. Further, its possible that there may be sub-CRQ support, but no
channel support. The CRQ reenable path currently doesn't take any of
this into consideration.
For simpilicty release and reinitialize sub-CRQs during reenable, and
set do_enquiry and using_channels with the appropriate values to trigger
channel renegotiation.
fixes: faacf8c5f1d5 ("ibmvfc: add alloc/dealloc routines for SCSI Sub-CRQ Channels")
Signed-off-by: Tyrel Datwyler <tyreld@linux.ibm.com>
---
drivers/scsi/ibmvscsi/ibmvfc.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/drivers/scsi/ibmvscsi/ibmvfc.c b/drivers/scsi/ibmvscsi/ibmvfc.c
index ef03fa559433..1e2ea21713ad 100644
--- a/drivers/scsi/ibmvscsi/ibmvfc.c
+++ b/drivers/scsi/ibmvscsi/ibmvfc.c
@@ -903,6 +903,9 @@ static int ibmvfc_reenable_crq_queue(struct ibmvfc_host *vhost)
{
int rc = 0;
struct vio_dev *vdev = to_vio_dev(vhost->dev);
+ unsigned long flags;
+
+ ibmvfc_release_sub_crqs(vhost);
/* Re-enable the CRQ */
do {
@@ -914,6 +917,15 @@ static int ibmvfc_reenable_crq_queue(struct ibmvfc_host *vhost)
if (rc)
dev_err(vhost->dev, "Error enabling adapter (rc=%d)\n", rc);
+ spin_lock_irqsave(vhost->host->host_lock, flags);
+ spin_lock(vhost->crq.q_lock);
+ vhost->do_enquiry = 1;
+ vhost->using_channels = 0;
+ spin_unlock(vhost->crq.q_lock);
+ spin_unlock_irqrestore(vhost->host->host_lock, flags);
+
+ ibmvfc_init_sub_crqs(vhost);
+
return rc;
}
--
2.27.0
^ permalink raw reply related
* [PATCH v4 2/5] ibmvfc: fix invalid sub-CRQ handles after hard reset
From: Tyrel Datwyler @ 2021-02-25 21:50 UTC (permalink / raw)
To: james.bottomley
Cc: Tyrel Datwyler, martin.petersen, linux-scsi, linux-kernel, brking,
linuxppc-dev
In-Reply-To: <20210225215057.23020-1-tyreld@linux.ibm.com>
A hard reset results in a complete transport disconnect such that the
CRQ connection with the partner VIOS is broken. This has the side effect
of also invalidating the associated sub-CRQs. The current code assumes
that the sub-CRQs are perserved resulting in a protocol violation after
trying to reconnect them with the VIOS. This introduces an infinite loop
such that the VIOS forces a disconnect after each subsequent attempt to
re-register with invalid handles.
Avoid the aforementioned issue by releasing the sub-CRQs prior to CRQ
disconnect, and driving a reinitialization of the sub-CRQs once a new
CRQ is registered with the hypervisor.
fixes: faacf8c5f1d5 ("ibmvfc: add alloc/dealloc routines for SCSI Sub-CRQ Channels")
Signed-off-by: Tyrel Datwyler <tyreld@linux.ibm.com>
---
drivers/scsi/ibmvscsi/ibmvfc.c | 21 +++++++++------------
1 file changed, 9 insertions(+), 12 deletions(-)
diff --git a/drivers/scsi/ibmvscsi/ibmvfc.c b/drivers/scsi/ibmvscsi/ibmvfc.c
index 384960036f8b..d34e1a4f74d9 100644
--- a/drivers/scsi/ibmvscsi/ibmvfc.c
+++ b/drivers/scsi/ibmvscsi/ibmvfc.c
@@ -158,6 +158,9 @@ static void ibmvfc_npiv_logout(struct ibmvfc_host *);
static void ibmvfc_tgt_implicit_logout_and_del(struct ibmvfc_target *);
static void ibmvfc_tgt_move_login(struct ibmvfc_target *);
+static void ibmvfc_release_sub_crqs(struct ibmvfc_host *);
+static void ibmvfc_init_sub_crqs(struct ibmvfc_host *);
+
static const char *unknown_error = "unknown error";
static long h_reg_sub_crq(unsigned long unit_address, unsigned long ioba,
@@ -926,8 +929,8 @@ static int ibmvfc_reset_crq(struct ibmvfc_host *vhost)
unsigned long flags;
struct vio_dev *vdev = to_vio_dev(vhost->dev);
struct ibmvfc_queue *crq = &vhost->crq;
- struct ibmvfc_queue *scrq;
- int i;
+
+ ibmvfc_release_sub_crqs(vhost);
/* Close the CRQ */
do {
@@ -947,16 +950,6 @@ static int ibmvfc_reset_crq(struct ibmvfc_host *vhost)
memset(crq->msgs.crq, 0, PAGE_SIZE);
crq->cur = 0;
- if (vhost->scsi_scrqs.scrqs) {
- for (i = 0; i < nr_scsi_hw_queues; i++) {
- scrq = &vhost->scsi_scrqs.scrqs[i];
- spin_lock(scrq->q_lock);
- memset(scrq->msgs.scrq, 0, PAGE_SIZE);
- scrq->cur = 0;
- spin_unlock(scrq->q_lock);
- }
- }
-
/* And re-open it again */
rc = plpar_hcall_norets(H_REG_CRQ, vdev->unit_address,
crq->msg_token, PAGE_SIZE);
@@ -966,9 +959,12 @@ static int ibmvfc_reset_crq(struct ibmvfc_host *vhost)
dev_warn(vhost->dev, "Partner adapter not ready\n");
else if (rc != 0)
dev_warn(vhost->dev, "Couldn't register crq (rc=%d)\n", rc);
+
spin_unlock(vhost->crq.q_lock);
spin_unlock_irqrestore(vhost->host->host_lock, flags);
+ ibmvfc_init_sub_crqs(vhost);
+
return rc;
}
@@ -5692,6 +5688,7 @@ static void ibmvfc_deregister_scsi_channel(struct ibmvfc_host *vhost, int index)
free_irq(scrq->irq, scrq);
irq_dispose_mapping(scrq->irq);
+ scrq->irq = 0;
do {
rc = plpar_hcall_norets(H_FREE_SUB_CRQ, vdev->unit_address,
--
2.27.0
^ permalink raw reply related
* Re: [PATCH next v3 12/15] printk: introduce a kmsg_dump iterator
From: Kees Cook @ 2021-02-25 21:59 UTC (permalink / raw)
To: John Ogness
Cc: linux-hyperv, Sergey Senozhatsky, Ravi Bangoria, Douglas Anderson,
Paul Mackerras, Miquel Raynal, K. Y. Srinivasan, Thomas Meyer,
Vignesh Raghavendra, Wei Liu, Madhavan Srinivasan,
Stephen Hemminger, Anton Vorontsov, Joel Stanley, Jason Wessel,
Anton Ivanov, Wei Li, Haiyang Zhang, Petr Mladek, Pavel Tatashin,
Alistair Popple, Jeff Dike, Colin Cross, linux-um,
Daniel Thompson, Steven Rostedt, Davidlohr Bueso, Nicholas Piggin,
Oleg Nesterov, Thomas Gleixner, Andy Shevchenko, Jordan Niethe,
Michael Kelley, Christophe Leroy, Tony Luck, linux-kernel,
Sergey Senozhatsky, Richard Weinberger, kgdb-bugreport, linux-mtd,
linuxppc-dev, Mike Rapoport
In-Reply-To: <20210225202438.28985-13-john.ogness@linutronix.de>
On Thu, Feb 25, 2021 at 09:24:35PM +0100, John Ogness wrote:
> Rather than storing the iterator information in the registered
> kmsg_dumper structure, create a separate iterator structure. The
> kmsg_dump_iter structure can reside on the stack of the caller, thus
> allowing lockless use of the kmsg_dump functions.
>
> This change also means that the kmsg_dumper dump() callback no
> longer needs to pass in the kmsg_dumper as an argument. If
> kmsg_dumpers want to access the kernel logs, they can use the new
> iterator.
>
> Update the kmsg_dumper callback prototype. Update code that accesses
> the kernel logs using the kmsg_dumper structure to use the new
> kmsg_dump_iter structure. For kmsg_dumpers, this also means adding a
> call to kmsg_dump_rewind() to initialize the iterator.
>
> All this is in preparation for removal of @logbuf_lock.
>
> Signed-off-by: John Ogness <john.ogness@linutronix.de>
> ---
> arch/powerpc/kernel/nvram_64.c | 14 +++---
> arch/powerpc/platforms/powernv/opal-kmsg.c | 3 +-
> arch/powerpc/xmon/xmon.c | 6 +--
> arch/um/kernel/kmsg_dump.c | 8 +--
> drivers/hv/vmbus_drv.c | 7 +--
> drivers/mtd/mtdoops.c | 8 +--
> fs/pstore/platform.c | 8 +--
Reviewed-by: Kees Cook <keescook@chromium.org> # pstore
-Kees
> include/linux/kmsg_dump.h | 38 ++++++++-------
> kernel/debug/kdb/kdb_main.c | 10 ++--
> kernel/printk/printk.c | 57 ++++++++++------------
> 10 files changed, 81 insertions(+), 78 deletions(-)
>
> diff --git a/arch/powerpc/kernel/nvram_64.c b/arch/powerpc/kernel/nvram_64.c
> index 532f22637783..5a64b24a91c2 100644
> --- a/arch/powerpc/kernel/nvram_64.c
> +++ b/arch/powerpc/kernel/nvram_64.c
> @@ -72,8 +72,7 @@ static const char *nvram_os_partitions[] = {
> NULL
> };
>
> -static void oops_to_nvram(struct kmsg_dumper *dumper,
> - enum kmsg_dump_reason reason);
> +static void oops_to_nvram(enum kmsg_dump_reason reason);
>
> static struct kmsg_dumper nvram_kmsg_dumper = {
> .dump = oops_to_nvram
> @@ -642,11 +641,11 @@ void __init nvram_init_oops_partition(int rtas_partition_exists)
> * that we think will compress sufficiently to fit in the lnx,oops-log
> * partition. If that's too much, go back and capture uncompressed text.
> */
> -static void oops_to_nvram(struct kmsg_dumper *dumper,
> - enum kmsg_dump_reason reason)
> +static void oops_to_nvram(enum kmsg_dump_reason reason)
> {
> struct oops_log_info *oops_hdr = (struct oops_log_info *)oops_buf;
> static unsigned int oops_count = 0;
> + static struct kmsg_dump_iter iter;
> static bool panicking = false;
> static DEFINE_SPINLOCK(lock);
> unsigned long flags;
> @@ -681,13 +680,14 @@ static void oops_to_nvram(struct kmsg_dumper *dumper,
> return;
>
> if (big_oops_buf) {
> - kmsg_dump_get_buffer(dumper, false,
> + kmsg_dump_rewind(&iter);
> + kmsg_dump_get_buffer(&iter, false,
> big_oops_buf, big_oops_buf_sz, &text_len);
> rc = zip_oops(text_len);
> }
> if (rc != 0) {
> - kmsg_dump_rewind(dumper);
> - kmsg_dump_get_buffer(dumper, false,
> + kmsg_dump_rewind(&iter);
> + kmsg_dump_get_buffer(&iter, false,
> oops_data, oops_data_sz, &text_len);
> err_type = ERR_TYPE_KERNEL_PANIC;
> oops_hdr->version = cpu_to_be16(OOPS_HDR_VERSION);
> diff --git a/arch/powerpc/platforms/powernv/opal-kmsg.c b/arch/powerpc/platforms/powernv/opal-kmsg.c
> index 6c3bc4b4da98..a7bd6ac681f4 100644
> --- a/arch/powerpc/platforms/powernv/opal-kmsg.c
> +++ b/arch/powerpc/platforms/powernv/opal-kmsg.c
> @@ -19,8 +19,7 @@
> * may not be completely printed. This function does not actually dump the
> * message, it just ensures that OPAL completely flushes the console buffer.
> */
> -static void kmsg_dump_opal_console_flush(struct kmsg_dumper *dumper,
> - enum kmsg_dump_reason reason)
> +static void kmsg_dump_opal_console_flush(enum kmsg_dump_reason reason)
> {
> /*
> * Outside of a panic context the pollers will continue to run,
> diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
> index 80ed3e1becf9..5978b90a885f 100644
> --- a/arch/powerpc/xmon/xmon.c
> +++ b/arch/powerpc/xmon/xmon.c
> @@ -3001,7 +3001,7 @@ print_address(unsigned long addr)
> static void
> dump_log_buf(void)
> {
> - struct kmsg_dumper dumper;
> + struct kmsg_dump_iter iter;
> unsigned char buf[128];
> size_t len;
>
> @@ -3013,9 +3013,9 @@ dump_log_buf(void)
> catch_memory_errors = 1;
> sync();
>
> - kmsg_dump_rewind_nolock(&dumper);
> + kmsg_dump_rewind_nolock(&iter);
> xmon_start_pagination();
> - while (kmsg_dump_get_line_nolock(&dumper, false, buf, sizeof(buf), &len)) {
> + while (kmsg_dump_get_line_nolock(&iter, false, buf, sizeof(buf), &len)) {
> buf[len] = '\0';
> printf("%s", buf);
> }
> diff --git a/arch/um/kernel/kmsg_dump.c b/arch/um/kernel/kmsg_dump.c
> index 4869e2cc787c..9fbc5e5b1023 100644
> --- a/arch/um/kernel/kmsg_dump.c
> +++ b/arch/um/kernel/kmsg_dump.c
> @@ -7,9 +7,9 @@
> #include <shared/kern.h>
> #include <os.h>
>
> -static void kmsg_dumper_stdout(struct kmsg_dumper *dumper,
> - enum kmsg_dump_reason reason)
> +static void kmsg_dumper_stdout(enum kmsg_dump_reason reason)
> {
> + static struct kmsg_dump_iter iter;
> static DEFINE_SPINLOCK(lock);
> static char line[1024];
> struct console *con;
> @@ -34,8 +34,10 @@ static void kmsg_dumper_stdout(struct kmsg_dumper *dumper,
> if (!spin_trylock(&lock))
> return;
>
> + kmsg_dump_rewind(&iter);
> +
> printf("kmsg_dump:\n");
> - while (kmsg_dump_get_line(dumper, true, line, sizeof(line), &len)) {
> + while (kmsg_dump_get_line(&iter, true, line, sizeof(line), &len)) {
> line[len] = '\0';
> printf("%s", line);
> }
> diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
> index 10dce9f91216..1b858f280e22 100644
> --- a/drivers/hv/vmbus_drv.c
> +++ b/drivers/hv/vmbus_drv.c
> @@ -1388,9 +1388,9 @@ static void vmbus_isr(void)
> * Callback from kmsg_dump. Grab as much as possible from the end of the kmsg
> * buffer and call into Hyper-V to transfer the data.
> */
> -static void hv_kmsg_dump(struct kmsg_dumper *dumper,
> - enum kmsg_dump_reason reason)
> +static void hv_kmsg_dump(enum kmsg_dump_reason reason)
> {
> + struct kmsg_dump_iter iter;
> size_t bytes_written;
> phys_addr_t panic_pa;
>
> @@ -1404,7 +1404,8 @@ static void hv_kmsg_dump(struct kmsg_dumper *dumper,
> * Write dump contents to the page. No need to synchronize; panic should
> * be single-threaded.
> */
> - kmsg_dump_get_buffer(dumper, false, hv_panic_page, HV_HYP_PAGE_SIZE,
> + kmsg_dump_rewind(&iter);
> + kmsg_dump_get_buffer(&iter, false, hv_panic_page, HV_HYP_PAGE_SIZE,
> &bytes_written);
> if (bytes_written)
> hyperv_report_panic_msg(panic_pa, bytes_written);
> diff --git a/drivers/mtd/mtdoops.c b/drivers/mtd/mtdoops.c
> index 8bbfba40a554..d179b726a1c9 100644
> --- a/drivers/mtd/mtdoops.c
> +++ b/drivers/mtd/mtdoops.c
> @@ -272,19 +272,21 @@ static void find_next_position(struct mtdoops_context *cxt)
> mtdoops_inc_counter(cxt);
> }
>
> -static void mtdoops_do_dump(struct kmsg_dumper *dumper,
> - enum kmsg_dump_reason reason)
> +static void mtdoops_do_dump(enum kmsg_dump_reason reason)
> {
> struct mtdoops_context *cxt = container_of(dumper,
> struct mtdoops_context, dump);
> + struct kmsg_dump_iter iter;
>
> /* Only dump oopses if dump_oops is set */
> if (reason == KMSG_DUMP_OOPS && !dump_oops)
> return;
>
> + kmsg_dump_rewind(&iter);
> +
> if (test_and_set_bit(0, &cxt->oops_buf_busy))
> return;
> - kmsg_dump_get_buffer(dumper, true, cxt->oops_buf + MTDOOPS_HEADER_SIZE,
> + kmsg_dump_get_buffer(&iter, true, cxt->oops_buf + MTDOOPS_HEADER_SIZE,
> record_size - MTDOOPS_HEADER_SIZE, NULL);
> clear_bit(0, &cxt->oops_buf_busy);
>
> diff --git a/fs/pstore/platform.c b/fs/pstore/platform.c
> index d963ae7902f9..edfc9504e024 100644
> --- a/fs/pstore/platform.c
> +++ b/fs/pstore/platform.c
> @@ -382,9 +382,9 @@ void pstore_record_init(struct pstore_record *record,
> * callback from kmsg_dump. Save as much as we can (up to kmsg_bytes) from the
> * end of the buffer.
> */
> -static void pstore_dump(struct kmsg_dumper *dumper,
> - enum kmsg_dump_reason reason)
> +static void pstore_dump(enum kmsg_dump_reason reason)
> {
> + struct kmsg_dump_iter iter;
> unsigned long total = 0;
> const char *why;
> unsigned int part = 1;
> @@ -405,6 +405,8 @@ static void pstore_dump(struct kmsg_dumper *dumper,
> }
> }
>
> + kmsg_dump_rewind(&iter);
> +
> oopscount++;
> while (total < kmsg_bytes) {
> char *dst;
> @@ -435,7 +437,7 @@ static void pstore_dump(struct kmsg_dumper *dumper,
> dst_size -= header_size;
>
> /* Write dump contents. */
> - if (!kmsg_dump_get_buffer(dumper, true, dst + header_size,
> + if (!kmsg_dump_get_buffer(&iter, true, dst + header_size,
> dst_size, &dump_size))
> break;
>
> diff --git a/include/linux/kmsg_dump.h b/include/linux/kmsg_dump.h
> index 84eaa2090efa..5d3bf20f9f0a 100644
> --- a/include/linux/kmsg_dump.h
> +++ b/include/linux/kmsg_dump.h
> @@ -29,6 +29,16 @@ enum kmsg_dump_reason {
> KMSG_DUMP_MAX
> };
>
> +/**
> + * struct kmsg_dump_iter - iterator for retrieving kernel messages
> + * @cur_seq: Points to the oldest message to dump
> + * @next_seq: Points after the newest message to dump
> + */
> +struct kmsg_dump_iter {
> + u64 cur_seq;
> + u64 next_seq;
> +};
> +
> /**
> * struct kmsg_dumper - kernel crash message dumper structure
> * @list: Entry in the dumper list (private)
> @@ -36,35 +46,29 @@ enum kmsg_dump_reason {
> * through the record iterator
> * @max_reason: filter for highest reason number that should be dumped
> * @registered: Flag that specifies if this is already registered
> - * @cur_seq: Points to the oldest message to dump
> - * @next_seq: Points after the newest message to dump
> */
> struct kmsg_dumper {
> struct list_head list;
> - void (*dump)(struct kmsg_dumper *dumper, enum kmsg_dump_reason reason);
> + void (*dump)(enum kmsg_dump_reason reason);
> enum kmsg_dump_reason max_reason;
> bool registered;
> -
> - /* private state of the kmsg iterator */
> - u64 cur_seq;
> - u64 next_seq;
> };
>
> #ifdef CONFIG_PRINTK
> void kmsg_dump(enum kmsg_dump_reason reason);
>
> -bool kmsg_dump_get_line_nolock(struct kmsg_dumper *dumper, bool syslog,
> +bool kmsg_dump_get_line_nolock(struct kmsg_dump_iter *iter, bool syslog,
> char *line, size_t size, size_t *len);
>
> -bool kmsg_dump_get_line(struct kmsg_dumper *dumper, bool syslog,
> +bool kmsg_dump_get_line(struct kmsg_dump_iter *iter, bool syslog,
> char *line, size_t size, size_t *len);
>
> -bool kmsg_dump_get_buffer(struct kmsg_dumper *dumper, bool syslog,
> +bool kmsg_dump_get_buffer(struct kmsg_dump_iter *iter, bool syslog,
> char *buf, size_t size, size_t *len_out);
>
> -void kmsg_dump_rewind_nolock(struct kmsg_dumper *dumper);
> +void kmsg_dump_rewind_nolock(struct kmsg_dump_iter *iter);
>
> -void kmsg_dump_rewind(struct kmsg_dumper *dumper);
> +void kmsg_dump_rewind(struct kmsg_dump_iter *iter);
>
> int kmsg_dump_register(struct kmsg_dumper *dumper);
>
> @@ -76,30 +80,30 @@ static inline void kmsg_dump(enum kmsg_dump_reason reason)
> {
> }
>
> -static inline bool kmsg_dump_get_line_nolock(struct kmsg_dumper *dumper,
> +static inline bool kmsg_dump_get_line_nolock(struct kmsg_dump_iter *iter,
> bool syslog, const char *line,
> size_t size, size_t *len)
> {
> return false;
> }
>
> -static inline bool kmsg_dump_get_line(struct kmsg_dumper *dumper, bool syslog,
> +static inline bool kmsg_dump_get_line(struct kmsg_dump_iter *iter, bool syslog,
> const char *line, size_t size, size_t *len)
> {
> return false;
> }
>
> -static inline bool kmsg_dump_get_buffer(struct kmsg_dumper *dumper, bool syslog,
> +static inline bool kmsg_dump_get_buffer(struct kmsg_dump_iter *iter, bool syslog,
> char *buf, size_t size, size_t *len)
> {
> return false;
> }
>
> -static inline void kmsg_dump_rewind_nolock(struct kmsg_dumper *dumper)
> +static inline void kmsg_dump_rewind_nolock(struct kmsg_dump_iter *iter)
> {
> }
>
> -static inline void kmsg_dump_rewind(struct kmsg_dumper *dumper)
> +static inline void kmsg_dump_rewind(struct kmsg_dump_iter *iter)
> {
> }
>
> diff --git a/kernel/debug/kdb/kdb_main.c b/kernel/debug/kdb/kdb_main.c
> index 315169d5e119..8544d7a55a57 100644
> --- a/kernel/debug/kdb/kdb_main.c
> +++ b/kernel/debug/kdb/kdb_main.c
> @@ -2101,7 +2101,7 @@ static int kdb_dmesg(int argc, const char **argv)
> int adjust = 0;
> int n = 0;
> int skip = 0;
> - struct kmsg_dumper dumper;
> + struct kmsg_dump_iter iter;
> size_t len;
> char buf[201];
>
> @@ -2126,8 +2126,8 @@ static int kdb_dmesg(int argc, const char **argv)
> kdb_set(2, setargs);
> }
>
> - kmsg_dump_rewind_nolock(&dumper);
> - while (kmsg_dump_get_line_nolock(&dumper, 1, NULL, 0, NULL))
> + kmsg_dump_rewind_nolock(&iter);
> + while (kmsg_dump_get_line_nolock(&iter, 1, NULL, 0, NULL))
> n++;
>
> if (lines < 0) {
> @@ -2159,8 +2159,8 @@ static int kdb_dmesg(int argc, const char **argv)
> if (skip >= n || skip < 0)
> return 0;
>
> - kmsg_dump_rewind_nolock(&dumper);
> - while (kmsg_dump_get_line_nolock(&dumper, 1, buf, sizeof(buf), &len)) {
> + kmsg_dump_rewind_nolock(&iter);
> + while (kmsg_dump_get_line_nolock(&iter, 1, buf, sizeof(buf), &len)) {
> if (skip) {
> skip--;
> continue;
> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> index 45cb3e9c62c5..e58ccc368348 100644
> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
> @@ -3390,7 +3390,6 @@ EXPORT_SYMBOL_GPL(kmsg_dump_reason_str);
> void kmsg_dump(enum kmsg_dump_reason reason)
> {
> struct kmsg_dumper *dumper;
> - unsigned long flags;
>
> rcu_read_lock();
> list_for_each_entry_rcu(dumper, &dump_list, list) {
> @@ -3407,21 +3406,15 @@ void kmsg_dump(enum kmsg_dump_reason reason)
> if (reason > max_reason)
> continue;
>
> - /* initialize iterator with data about the stored records */
> - logbuf_lock_irqsave(flags);
> - dumper->cur_seq = latched_seq_read_nolock(&clear_seq);
> - dumper->next_seq = prb_next_seq(prb);
> - logbuf_unlock_irqrestore(flags);
> -
> /* invoke dumper which will iterate over records */
> - dumper->dump(dumper, reason);
> + dumper->dump(reason);
> }
> rcu_read_unlock();
> }
>
> /**
> * kmsg_dump_get_line_nolock - retrieve one kmsg log line (unlocked version)
> - * @dumper: registered kmsg dumper
> + * @iter: kmsg dump iterator
> * @syslog: include the "<4>" prefixes
> * @line: buffer to copy the line to
> * @size: maximum size of the buffer
> @@ -3438,7 +3431,7 @@ void kmsg_dump(enum kmsg_dump_reason reason)
> *
> * The function is similar to kmsg_dump_get_line(), but grabs no locks.
> */
> -bool kmsg_dump_get_line_nolock(struct kmsg_dumper *dumper, bool syslog,
> +bool kmsg_dump_get_line_nolock(struct kmsg_dump_iter *iter, bool syslog,
> char *line, size_t size, size_t *len)
> {
> struct printk_info info;
> @@ -3451,11 +3444,11 @@ bool kmsg_dump_get_line_nolock(struct kmsg_dumper *dumper, bool syslog,
>
> /* Read text or count text lines? */
> if (line) {
> - if (!prb_read_valid(prb, dumper->cur_seq, &r))
> + if (!prb_read_valid(prb, iter->cur_seq, &r))
> goto out;
> l = record_print_text(&r, syslog, printk_time);
> } else {
> - if (!prb_read_valid_info(prb, dumper->cur_seq,
> + if (!prb_read_valid_info(prb, iter->cur_seq,
> &info, &line_count)) {
> goto out;
> }
> @@ -3464,7 +3457,7 @@ bool kmsg_dump_get_line_nolock(struct kmsg_dumper *dumper, bool syslog,
>
> }
>
> - dumper->cur_seq = r.info->seq + 1;
> + iter->cur_seq = r.info->seq + 1;
> ret = true;
> out:
> if (len)
> @@ -3474,7 +3467,7 @@ bool kmsg_dump_get_line_nolock(struct kmsg_dumper *dumper, bool syslog,
>
> /**
> * kmsg_dump_get_line - retrieve one kmsg log line
> - * @dumper: registered kmsg dumper
> + * @iter: kmsg dump iterator
> * @syslog: include the "<4>" prefixes
> * @line: buffer to copy the line to
> * @size: maximum size of the buffer
> @@ -3489,14 +3482,14 @@ bool kmsg_dump_get_line_nolock(struct kmsg_dumper *dumper, bool syslog,
> * A return value of FALSE indicates that there are no more records to
> * read.
> */
> -bool kmsg_dump_get_line(struct kmsg_dumper *dumper, bool syslog,
> +bool kmsg_dump_get_line(struct kmsg_dump_iter *iter, bool syslog,
> char *line, size_t size, size_t *len)
> {
> unsigned long flags;
> bool ret;
>
> logbuf_lock_irqsave(flags);
> - ret = kmsg_dump_get_line_nolock(dumper, syslog, line, size, len);
> + ret = kmsg_dump_get_line_nolock(iter, syslog, line, size, len);
> logbuf_unlock_irqrestore(flags);
>
> return ret;
> @@ -3505,7 +3498,7 @@ EXPORT_SYMBOL_GPL(kmsg_dump_get_line);
>
> /**
> * kmsg_dump_get_buffer - copy kmsg log lines
> - * @dumper: registered kmsg dumper
> + * @iter: kmsg dump iterator
> * @syslog: include the "<4>" prefixes
> * @buf: buffer to copy the line to
> * @size: maximum size of the buffer
> @@ -3522,7 +3515,7 @@ EXPORT_SYMBOL_GPL(kmsg_dump_get_line);
> * A return value of FALSE indicates that there are no more records to
> * read.
> */
> -bool kmsg_dump_get_buffer(struct kmsg_dumper *dumper, bool syslog,
> +bool kmsg_dump_get_buffer(struct kmsg_dump_iter *iter, bool syslog,
> char *buf, size_t size, size_t *len_out)
> {
> struct printk_info info;
> @@ -3538,15 +3531,15 @@ bool kmsg_dump_get_buffer(struct kmsg_dumper *dumper, bool syslog,
> goto out;
>
> logbuf_lock_irqsave(flags);
> - if (prb_read_valid_info(prb, dumper->cur_seq, &info, NULL)) {
> - if (info.seq != dumper->cur_seq) {
> + if (prb_read_valid_info(prb, iter->cur_seq, &info, NULL)) {
> + if (info.seq != iter->cur_seq) {
> /* messages are gone, move to first available one */
> - dumper->cur_seq = info.seq;
> + iter->cur_seq = info.seq;
> }
> }
>
> /* last entry */
> - if (dumper->cur_seq >= dumper->next_seq) {
> + if (iter->cur_seq >= iter->next_seq) {
> logbuf_unlock_irqrestore(flags);
> goto out;
> }
> @@ -3557,7 +3550,7 @@ bool kmsg_dump_get_buffer(struct kmsg_dumper *dumper, bool syslog,
> * because this function (by way of record_print_text()) will
> * not write more than size-1 bytes of text into @buf.
> */
> - seq = find_first_fitting_seq(dumper->cur_seq, dumper->next_seq,
> + seq = find_first_fitting_seq(iter->cur_seq, iter->next_seq,
> size - 1, syslog, time);
>
> /*
> @@ -3570,7 +3563,7 @@ bool kmsg_dump_get_buffer(struct kmsg_dumper *dumper, bool syslog,
>
> len = 0;
> prb_for_each_record(seq, prb, seq, &r) {
> - if (r.info->seq >= dumper->next_seq)
> + if (r.info->seq >= iter->next_seq)
> break;
>
> len += record_print_text(&r, syslog, time);
> @@ -3579,7 +3572,7 @@ bool kmsg_dump_get_buffer(struct kmsg_dumper *dumper, bool syslog,
> prb_rec_init_rd(&r, &info, buf + len, size - len);
> }
>
> - dumper->next_seq = next_seq;
> + iter->next_seq = next_seq;
> ret = true;
> logbuf_unlock_irqrestore(flags);
> out:
> @@ -3591,7 +3584,7 @@ EXPORT_SYMBOL_GPL(kmsg_dump_get_buffer);
>
> /**
> * kmsg_dump_rewind_nolock - reset the iterator (unlocked version)
> - * @dumper: registered kmsg dumper
> + * @iter: kmsg dump iterator
> *
> * Reset the dumper's iterator so that kmsg_dump_get_line() and
> * kmsg_dump_get_buffer() can be called again and used multiple
> @@ -3599,26 +3592,26 @@ EXPORT_SYMBOL_GPL(kmsg_dump_get_buffer);
> *
> * The function is similar to kmsg_dump_rewind(), but grabs no locks.
> */
> -void kmsg_dump_rewind_nolock(struct kmsg_dumper *dumper)
> +void kmsg_dump_rewind_nolock(struct kmsg_dump_iter *iter)
> {
> - dumper->cur_seq = latched_seq_read_nolock(&clear_seq);
> - dumper->next_seq = prb_next_seq(prb);
> + iter->cur_seq = latched_seq_read_nolock(&clear_seq);
> + iter->next_seq = prb_next_seq(prb);
> }
>
> /**
> * kmsg_dump_rewind - reset the iterator
> - * @dumper: registered kmsg dumper
> + * @iter: kmsg dump iterator
> *
> * Reset the dumper's iterator so that kmsg_dump_get_line() and
> * kmsg_dump_get_buffer() can be called again and used multiple
> * times within the same dumper.dump() callback.
> */
> -void kmsg_dump_rewind(struct kmsg_dumper *dumper)
> +void kmsg_dump_rewind(struct kmsg_dump_iter *iter)
> {
> unsigned long flags;
>
> logbuf_lock_irqsave(flags);
> - kmsg_dump_rewind_nolock(dumper);
> + kmsg_dump_rewind_nolock(iter);
> logbuf_unlock_irqrestore(flags);
> }
> EXPORT_SYMBOL_GPL(kmsg_dump_rewind);
> --
> 2.20.1
>
--
Kees Cook
^ permalink raw reply
* [PATCH v1] powerpc: low_i2c: change @lock to raw_spinlock_t
From: John Ogness @ 2021-02-25 22:06 UTC (permalink / raw)
To: Paul Mackerras; +Cc: linuxppc-dev, linux-kernel, Qinglang Miao
i2c transfers are occurring with local interrupts disabled:
smp_core99_give_timebase()
local_irq_save();
smp_core99_cypress_tb_freeze()
pmac_i2c_xfer()
kw_i2c_xfer()
spin_lock_irqsave(&host->lock, flags)
This is a problem because with PREEMPT_RT a spinlock_t can sleep,
causing the system to hang. Convert the spinlock_t to the
non-sleeping raw_spinlock_t.
Signed-off-by: John Ogness <john.ogness@linutronix.de>
---
arch/powerpc/platforms/powermac/low_i2c.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/arch/powerpc/platforms/powermac/low_i2c.c b/arch/powerpc/platforms/powermac/low_i2c.c
index f77a59b5c2e1..ba89c95ef290 100644
--- a/arch/powerpc/platforms/powermac/low_i2c.c
+++ b/arch/powerpc/platforms/powermac/low_i2c.c
@@ -116,7 +116,7 @@ struct pmac_i2c_host_kw
int polled;
int result;
struct completion complete;
- spinlock_t lock;
+ raw_spinlock_t lock;
struct timer_list timeout_timer;
};
@@ -346,14 +346,14 @@ static irqreturn_t kw_i2c_irq(int irq, void *dev_id)
struct pmac_i2c_host_kw *host = dev_id;
unsigned long flags;
- spin_lock_irqsave(&host->lock, flags);
+ raw_spin_lock_irqsave(&host->lock, flags);
del_timer(&host->timeout_timer);
kw_i2c_handle_interrupt(host, kw_read_reg(reg_isr));
if (host->state != state_idle) {
host->timeout_timer.expires = jiffies + KW_POLL_TIMEOUT;
add_timer(&host->timeout_timer);
}
- spin_unlock_irqrestore(&host->lock, flags);
+ raw_spin_unlock_irqrestore(&host->lock, flags);
return IRQ_HANDLED;
}
@@ -362,7 +362,7 @@ static void kw_i2c_timeout(struct timer_list *t)
struct pmac_i2c_host_kw *host = from_timer(host, t, timeout_timer);
unsigned long flags;
- spin_lock_irqsave(&host->lock, flags);
+ raw_spin_lock_irqsave(&host->lock, flags);
/*
* If the timer is pending, that means we raced with the
@@ -377,7 +377,7 @@ static void kw_i2c_timeout(struct timer_list *t)
add_timer(&host->timeout_timer);
}
skip:
- spin_unlock_irqrestore(&host->lock, flags);
+ raw_spin_unlock_irqrestore(&host->lock, flags);
}
static int kw_i2c_open(struct pmac_i2c_bus *bus)
@@ -470,9 +470,9 @@ static int kw_i2c_xfer(struct pmac_i2c_bus *bus, u8 addrdir, int subsize,
unsigned long flags;
u8 isr = kw_i2c_wait_interrupt(host);
- spin_lock_irqsave(&host->lock, flags);
+ raw_spin_lock_irqsave(&host->lock, flags);
kw_i2c_handle_interrupt(host, isr);
- spin_unlock_irqrestore(&host->lock, flags);
+ raw_spin_unlock_irqrestore(&host->lock, flags);
}
}
@@ -508,7 +508,7 @@ static struct pmac_i2c_host_kw *__init kw_i2c_host_init(struct device_node *np)
}
mutex_init(&host->mutex);
init_completion(&host->complete);
- spin_lock_init(&host->lock);
+ raw_spin_lock_init(&host->lock);
timer_setup(&host->timeout_timer, kw_i2c_timeout, 0);
psteps = of_get_property(np, "AAPL,address-step", NULL);
--
2.20.1
^ permalink raw reply related
* Re: [PATCH v3 5/5] ibmvfc: reinitialize sub-CRQs and perform channel enquiry after LPM
From: Tyrel Datwyler @ 2021-02-25 22:12 UTC (permalink / raw)
To: james.bottomley
Cc: brking, linuxppc-dev, linux-scsi, martin.petersen, linux-kernel
In-Reply-To: <20210225214237.22400-6-tyreld@linux.ibm.com>
On 2/25/21 1:42 PM, Tyrel Datwyler wrote:
> A live partition migration (LPM) results in a CRQ disconnect similar to
> a hard reset. In this LPM case the hypervisor moslty perserves the CRQ
> transport such that it simply needs to be reenabled. However, the
> capabilities may have changed such as fewer channels, or no channels at
> all. Further, its possible that there may be sub-CRQ support, but no
> channel support. The CRQ reenable path currently doesn't take any of
> this into consideration.
>
> For simpilicty release and reinitialize sub-CRQs during reenable, and
> set do_enquiry and using_channels with the appropriate values to trigger
> channel renegotiation.
>
> Signed-off-by: Tyrel Datwyler <tyreld@linux.ibm.com>
> ---
> drivers/scsi/ibmvscsi/ibmvfc.c | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/drivers/scsi/ibmvscsi/ibmvfc.c b/drivers/scsi/ibmvscsi/ibmvfc.c
> index 1bb08e5f3674..6bbc2697ad5a 100644
> --- a/drivers/scsi/ibmvscsi/ibmvfc.c
> +++ b/drivers/scsi/ibmvscsi/ibmvfc.c
> @@ -903,6 +903,9 @@ static int ibmvfc_reenable_crq_queue(struct ibmvfc_host *vhost)
> {
> int rc = 0;
> struct vio_dev *vdev = to_vio_dev(vhost->dev);
> + unsigned long flags;
> +
> + ibmvfc_release_sub_crqs(vhost);
>
> /* Re-enable the CRQ */
> do {
> @@ -914,6 +917,15 @@ static int ibmvfc_reenable_crq_queue(struct ibmvfc_host *vhost)
> if (rc)
> dev_err(vhost->dev, "Error enabling adapter (rc=%d)\n", rc);
>
> + ibmvfc_init_sub_crqs(vhost);
Realized that if this fails it set the do_enquiry flag to zero which the locked
region below will then flip back to one. Need to move sub-crq init to after
locked region.
-T
> +
> + spin_lock_irqsave(vhost->host->host_lock, flags);
> + spin_lock(vhost->crq.q_lock);
> + vhost->do_enquiry = 1;
> + vhost->using_channels = 0;
> + spin_unlock(vhost->crq.q_lock);
> + spin_unlock_irqrestore(vhost->host->host_lock, flags);
> +
> return rc;
> }
>
>
^ permalink raw reply
* Re: [PATCH v3 2/5] ibmvfc: fix invalid sub-CRQ handles after hard reset
From: Tyrel Datwyler @ 2021-02-25 22:13 UTC (permalink / raw)
To: james.bottomley
Cc: brking, linuxppc-dev, linux-scsi, martin.petersen, linux-kernel
In-Reply-To: <20210225214237.22400-3-tyreld@linux.ibm.com>
On 2/25/21 1:42 PM, Tyrel Datwyler wrote:
> A hard reset results in a complete transport disconnect such that the
> CRQ connection with the partner VIOS is broken. This has the side effect
> of also invalidating the associated sub-CRQs. The current code assumes
> that the sub-CRQs are perserved resulting in a protocol violation after
> trying to reconnect them with the VIOS. This introduces an infinite loop
> such that the VIOS forces a disconnect after each subsequent attempt to
> re-register with invalid handles.
>
> Avoid the aforementioned issue by releasing the sub-CRQs prior to CRQ
> disconnect, and driving a reinitialization of the sub-CRQs once a new
> CRQ is registered with the hypervisor.
>
> fixes: faacf8c5f1d5 ("ibmvfc: add alloc/dealloc routines for SCSI Sub-CRQ Channels")
> Signed-off-by: Tyrel Datwyler <tyreld@linux.ibm.com>
> Reviewed-by: Brian King <brking@linux.ibm.com>
> ---
> drivers/scsi/ibmvscsi/ibmvfc.c | 21 +++++++++------------
> 1 file changed, 9 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/scsi/ibmvscsi/ibmvfc.c b/drivers/scsi/ibmvscsi/ibmvfc.c
> index 384960036f8b..2cca55f2e464 100644
> --- a/drivers/scsi/ibmvscsi/ibmvfc.c
> +++ b/drivers/scsi/ibmvscsi/ibmvfc.c
> @@ -158,6 +158,9 @@ static void ibmvfc_npiv_logout(struct ibmvfc_host *);
> static void ibmvfc_tgt_implicit_logout_and_del(struct ibmvfc_target *);
> static void ibmvfc_tgt_move_login(struct ibmvfc_target *);
>
> +static void ibmvfc_release_sub_crqs(struct ibmvfc_host *);
> +static void ibmvfc_init_sub_crqs(struct ibmvfc_host *);
> +
> static const char *unknown_error = "unknown error";
>
> static long h_reg_sub_crq(unsigned long unit_address, unsigned long ioba,
> @@ -926,8 +929,8 @@ static int ibmvfc_reset_crq(struct ibmvfc_host *vhost)
> unsigned long flags;
> struct vio_dev *vdev = to_vio_dev(vhost->dev);
> struct ibmvfc_queue *crq = &vhost->crq;
> - struct ibmvfc_queue *scrq;
> - int i;
> +
> + ibmvfc_release_sub_crqs(vhost);
>
> /* Close the CRQ */
> do {
> @@ -936,6 +939,8 @@ static int ibmvfc_reset_crq(struct ibmvfc_host *vhost)
> rc = plpar_hcall_norets(H_FREE_CRQ, vdev->unit_address);
> } while (rc == H_BUSY || H_IS_LONG_BUSY(rc));
>
> + ibmvfc_init_sub_crqs(vhost);
This has the same issue as patch 5 in that if fail to set up sub-crqs do_enquiry
will be set to zero, but the locked code region below will then flip it back to
one which we don't want.
-T
> +
> spin_lock_irqsave(vhost->host->host_lock, flags);
> spin_lock(vhost->crq.q_lock);
> vhost->state = IBMVFC_NO_CRQ;
> @@ -947,16 +952,6 @@ static int ibmvfc_reset_crq(struct ibmvfc_host *vhost)
> memset(crq->msgs.crq, 0, PAGE_SIZE);
> crq->cur = 0;
>
> - if (vhost->scsi_scrqs.scrqs) {
> - for (i = 0; i < nr_scsi_hw_queues; i++) {
> - scrq = &vhost->scsi_scrqs.scrqs[i];
> - spin_lock(scrq->q_lock);
> - memset(scrq->msgs.scrq, 0, PAGE_SIZE);
> - scrq->cur = 0;
> - spin_unlock(scrq->q_lock);
> - }
> - }
> -
> /* And re-open it again */
> rc = plpar_hcall_norets(H_REG_CRQ, vdev->unit_address,
> crq->msg_token, PAGE_SIZE);
> @@ -966,6 +961,7 @@ static int ibmvfc_reset_crq(struct ibmvfc_host *vhost)
> dev_warn(vhost->dev, "Partner adapter not ready\n");
> else if (rc != 0)
> dev_warn(vhost->dev, "Couldn't register crq (rc=%d)\n", rc);
> +
> spin_unlock(vhost->crq.q_lock);
> spin_unlock_irqrestore(vhost->host->host_lock, flags);
>
> @@ -5692,6 +5688,7 @@ static void ibmvfc_deregister_scsi_channel(struct ibmvfc_host *vhost, int index)
>
> free_irq(scrq->irq, scrq);
> irq_dispose_mapping(scrq->irq);
> + scrq->irq = 0;
>
> do {
> rc = plpar_hcall_norets(H_FREE_SUB_CRQ, vdev->unit_address,
>
^ permalink raw reply
* [PATCH v2] vio: make remove callback return void
From: Uwe Kleine-König @ 2021-02-25 22:18 UTC (permalink / raw)
To: Michael Ellerman
Cc: Cristobal Forno, Tyrel Datwyler, Paul Mackerras,
Breno Leitão, Peter Huewe, Sukadev Bhattiprolu, Jiri Slaby,
Herbert Xu, linux-scsi, Nayna Jain, Jason Gunthorpe, Michael Cyr,
Jakub Kicinski, Arnd Bergmann, James E.J. Bottomley, Lijun Pan,
Matt Mackall, Steven Royer, Martin K. Petersen,
Greg Kroah-Hartman, linux-kernel, Jarkko Sakkinen, linux-crypto,
netdev, Dany Madden, Paulo Flabiano Smorigo, linux-integrity,
linuxppc-dev, David S. Miller
The driver core ignores the return value of struct bus_type::remove()
because there is only little that can be done. To simplify the quest to
make this function return void, let struct vio_driver::remove() return
void, too. All users already unconditionally return 0, this commit makes
it obvious that returning an error code is a bad idea.
Note there are two nominally different implementations for a vio bus:
one in arch/sparc/kernel/vio.c and the other in
arch/powerpc/platforms/pseries/vio.c. This patch only adapts the powerpc
one.
Before this patch for a device that was bound to a driver without a
remove callback vio_cmo_bus_remove(viodev) wasn't called. As the device
core still considers the device unbound after vio_bus_remove() returns
calling this unconditionally is the consistent behaviour which is
implemented here.
Reviewed-by: Tyrel Datwyler <tyreld@linux.ibm.com>
Acked-by: Lijun Pan <ljp@linux.ibm.com>
Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Uwe Kleine-König <uwe@kleine-koenig.org>
---
Hello,
I dropped the sparc specific files (i.e. all that Michael Ellerman
didn't characterize as powerpc specific and verified that they are
indeed sparc-only).
The commit log is adapted accordingly.
Best regards
Uwe
arch/powerpc/include/asm/vio.h | 2 +-
arch/powerpc/platforms/pseries/vio.c | 7 +++----
drivers/char/hw_random/pseries-rng.c | 3 +--
drivers/char/tpm/tpm_ibmvtpm.c | 4 +---
drivers/crypto/nx/nx-842-pseries.c | 4 +---
drivers/crypto/nx/nx.c | 4 +---
drivers/misc/ibmvmc.c | 4 +---
drivers/net/ethernet/ibm/ibmveth.c | 4 +---
drivers/net/ethernet/ibm/ibmvnic.c | 4 +---
drivers/scsi/ibmvscsi/ibmvfc.c | 3 +--
drivers/scsi/ibmvscsi/ibmvscsi.c | 4 +---
drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c | 4 +---
drivers/tty/hvc/hvcs.c | 3 +--
13 files changed, 15 insertions(+), 35 deletions(-)
diff --git a/arch/powerpc/include/asm/vio.h b/arch/powerpc/include/asm/vio.h
index 0cf52746531b..721c0d6715ac 100644
--- a/arch/powerpc/include/asm/vio.h
+++ b/arch/powerpc/include/asm/vio.h
@@ -113,7 +113,7 @@ struct vio_driver {
const char *name;
const struct vio_device_id *id_table;
int (*probe)(struct vio_dev *dev, const struct vio_device_id *id);
- int (*remove)(struct vio_dev *dev);
+ void (*remove)(struct vio_dev *dev);
/* A driver must have a get_desired_dma() function to
* be loaded in a CMO environment if it uses DMA.
*/
diff --git a/arch/powerpc/platforms/pseries/vio.c b/arch/powerpc/platforms/pseries/vio.c
index b2797cfe4e2b..9cb4fc839fd5 100644
--- a/arch/powerpc/platforms/pseries/vio.c
+++ b/arch/powerpc/platforms/pseries/vio.c
@@ -1261,7 +1261,6 @@ static int vio_bus_remove(struct device *dev)
struct vio_dev *viodev = to_vio_dev(dev);
struct vio_driver *viodrv = to_vio_driver(dev->driver);
struct device *devptr;
- int ret = 1;
/*
* Hold a reference to the device after the remove function is called
@@ -1270,13 +1269,13 @@ static int vio_bus_remove(struct device *dev)
devptr = get_device(dev);
if (viodrv->remove)
- ret = viodrv->remove(viodev);
+ viodrv->remove(viodev);
- if (!ret && firmware_has_feature(FW_FEATURE_CMO))
+ if (firmware_has_feature(FW_FEATURE_CMO))
vio_cmo_bus_remove(viodev);
put_device(devptr);
- return ret;
+ return 0;
}
/**
diff --git a/drivers/char/hw_random/pseries-rng.c b/drivers/char/hw_random/pseries-rng.c
index 8038a8a9fb58..f4949b689bd5 100644
--- a/drivers/char/hw_random/pseries-rng.c
+++ b/drivers/char/hw_random/pseries-rng.c
@@ -54,10 +54,9 @@ static int pseries_rng_probe(struct vio_dev *dev,
return hwrng_register(&pseries_rng);
}
-static int pseries_rng_remove(struct vio_dev *dev)
+static void pseries_rng_remove(struct vio_dev *dev)
{
hwrng_unregister(&pseries_rng);
- return 0;
}
static const struct vio_device_id pseries_rng_driver_ids[] = {
diff --git a/drivers/char/tpm/tpm_ibmvtpm.c b/drivers/char/tpm/tpm_ibmvtpm.c
index 994385bf37c0..903604769de9 100644
--- a/drivers/char/tpm/tpm_ibmvtpm.c
+++ b/drivers/char/tpm/tpm_ibmvtpm.c
@@ -343,7 +343,7 @@ static int ibmvtpm_crq_send_init_complete(struct ibmvtpm_dev *ibmvtpm)
*
* Return: Always 0.
*/
-static int tpm_ibmvtpm_remove(struct vio_dev *vdev)
+static void tpm_ibmvtpm_remove(struct vio_dev *vdev)
{
struct tpm_chip *chip = dev_get_drvdata(&vdev->dev);
struct ibmvtpm_dev *ibmvtpm = dev_get_drvdata(&chip->dev);
@@ -372,8 +372,6 @@ static int tpm_ibmvtpm_remove(struct vio_dev *vdev)
kfree(ibmvtpm);
/* For tpm_ibmvtpm_get_desired_dma */
dev_set_drvdata(&vdev->dev, NULL);
-
- return 0;
}
/**
diff --git a/drivers/crypto/nx/nx-842-pseries.c b/drivers/crypto/nx/nx-842-pseries.c
index 2de5e3672e42..cc8dd3072b8b 100644
--- a/drivers/crypto/nx/nx-842-pseries.c
+++ b/drivers/crypto/nx/nx-842-pseries.c
@@ -1042,7 +1042,7 @@ static int nx842_probe(struct vio_dev *viodev,
return ret;
}
-static int nx842_remove(struct vio_dev *viodev)
+static void nx842_remove(struct vio_dev *viodev)
{
struct nx842_devdata *old_devdata;
unsigned long flags;
@@ -1063,8 +1063,6 @@ static int nx842_remove(struct vio_dev *viodev)
if (old_devdata)
kfree(old_devdata->counters);
kfree(old_devdata);
-
- return 0;
}
static const struct vio_device_id nx842_vio_driver_ids[] = {
diff --git a/drivers/crypto/nx/nx.c b/drivers/crypto/nx/nx.c
index 0d2dc5be7f19..1d0e8a1ba160 100644
--- a/drivers/crypto/nx/nx.c
+++ b/drivers/crypto/nx/nx.c
@@ -783,7 +783,7 @@ static int nx_probe(struct vio_dev *viodev, const struct vio_device_id *id)
return nx_register_algs();
}
-static int nx_remove(struct vio_dev *viodev)
+static void nx_remove(struct vio_dev *viodev)
{
dev_dbg(&viodev->dev, "entering nx_remove for UA 0x%x\n",
viodev->unit_address);
@@ -811,8 +811,6 @@ static int nx_remove(struct vio_dev *viodev)
nx_unregister_skcipher(&nx_ecb_aes_alg, NX_FC_AES,
NX_MODE_AES_ECB);
}
-
- return 0;
}
diff --git a/drivers/misc/ibmvmc.c b/drivers/misc/ibmvmc.c
index 2d778d0f011e..c0fe3295c330 100644
--- a/drivers/misc/ibmvmc.c
+++ b/drivers/misc/ibmvmc.c
@@ -2288,15 +2288,13 @@ static int ibmvmc_probe(struct vio_dev *vdev, const struct vio_device_id *id)
return -EPERM;
}
-static int ibmvmc_remove(struct vio_dev *vdev)
+static void ibmvmc_remove(struct vio_dev *vdev)
{
struct crq_server_adapter *adapter = dev_get_drvdata(&vdev->dev);
dev_info(adapter->dev, "Entering remove for UA 0x%x\n",
vdev->unit_address);
ibmvmc_release_crq_queue(adapter);
-
- return 0;
}
static struct vio_device_id ibmvmc_device_table[] = {
diff --git a/drivers/net/ethernet/ibm/ibmveth.c b/drivers/net/ethernet/ibm/ibmveth.c
index c3ec9ceed833..7fea9ae60f13 100644
--- a/drivers/net/ethernet/ibm/ibmveth.c
+++ b/drivers/net/ethernet/ibm/ibmveth.c
@@ -1758,7 +1758,7 @@ static int ibmveth_probe(struct vio_dev *dev, const struct vio_device_id *id)
return 0;
}
-static int ibmveth_remove(struct vio_dev *dev)
+static void ibmveth_remove(struct vio_dev *dev)
{
struct net_device *netdev = dev_get_drvdata(&dev->dev);
struct ibmveth_adapter *adapter = netdev_priv(netdev);
@@ -1771,8 +1771,6 @@ static int ibmveth_remove(struct vio_dev *dev)
free_netdev(netdev);
dev_set_drvdata(&dev->dev, NULL);
-
- return 0;
}
static struct attribute veth_active_attr;
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
index 118a4bd3f877..eb39318766f6 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -5396,7 +5396,7 @@ static int ibmvnic_probe(struct vio_dev *dev, const struct vio_device_id *id)
return rc;
}
-static int ibmvnic_remove(struct vio_dev *dev)
+static void ibmvnic_remove(struct vio_dev *dev)
{
struct net_device *netdev = dev_get_drvdata(&dev->dev);
struct ibmvnic_adapter *adapter = netdev_priv(netdev);
@@ -5437,8 +5437,6 @@ static int ibmvnic_remove(struct vio_dev *dev)
device_remove_file(&dev->dev, &dev_attr_failover);
free_netdev(netdev);
dev_set_drvdata(&dev->dev, NULL);
-
- return 0;
}
static ssize_t failover_store(struct device *dev, struct device_attribute *attr,
diff --git a/drivers/scsi/ibmvscsi/ibmvfc.c b/drivers/scsi/ibmvscsi/ibmvfc.c
index 755313b766b9..e663085a8944 100644
--- a/drivers/scsi/ibmvscsi/ibmvfc.c
+++ b/drivers/scsi/ibmvscsi/ibmvfc.c
@@ -6038,7 +6038,7 @@ static int ibmvfc_probe(struct vio_dev *vdev, const struct vio_device_id *id)
* Return value:
* 0
**/
-static int ibmvfc_remove(struct vio_dev *vdev)
+static void ibmvfc_remove(struct vio_dev *vdev)
{
struct ibmvfc_host *vhost = dev_get_drvdata(&vdev->dev);
LIST_HEAD(purge);
@@ -6070,7 +6070,6 @@ static int ibmvfc_remove(struct vio_dev *vdev)
spin_unlock(&ibmvfc_driver_lock);
scsi_host_put(vhost->host);
LEAVE;
- return 0;
}
/**
diff --git a/drivers/scsi/ibmvscsi/ibmvscsi.c b/drivers/scsi/ibmvscsi/ibmvscsi.c
index 29fcc44be2d5..77fafb1bc173 100644
--- a/drivers/scsi/ibmvscsi/ibmvscsi.c
+++ b/drivers/scsi/ibmvscsi/ibmvscsi.c
@@ -2335,7 +2335,7 @@ static int ibmvscsi_probe(struct vio_dev *vdev, const struct vio_device_id *id)
return -1;
}
-static int ibmvscsi_remove(struct vio_dev *vdev)
+static void ibmvscsi_remove(struct vio_dev *vdev)
{
struct ibmvscsi_host_data *hostdata = dev_get_drvdata(&vdev->dev);
@@ -2356,8 +2356,6 @@ static int ibmvscsi_remove(struct vio_dev *vdev)
spin_unlock(&ibmvscsi_driver_lock);
scsi_host_put(hostdata->host);
-
- return 0;
}
/**
diff --git a/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c b/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
index cc3908c2d2f9..9abd9e253af6 100644
--- a/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
+++ b/drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c
@@ -3595,7 +3595,7 @@ static int ibmvscsis_probe(struct vio_dev *vdev,
return rc;
}
-static int ibmvscsis_remove(struct vio_dev *vdev)
+static void ibmvscsis_remove(struct vio_dev *vdev)
{
struct scsi_info *vscsi = dev_get_drvdata(&vdev->dev);
@@ -3622,8 +3622,6 @@ static int ibmvscsis_remove(struct vio_dev *vdev)
list_del(&vscsi->list);
spin_unlock_bh(&ibmvscsis_dev_lock);
kfree(vscsi);
-
- return 0;
}
static ssize_t system_id_show(struct device *dev,
diff --git a/drivers/tty/hvc/hvcs.c b/drivers/tty/hvc/hvcs.c
index c90848919644..01fc97e3c5c8 100644
--- a/drivers/tty/hvc/hvcs.c
+++ b/drivers/tty/hvc/hvcs.c
@@ -819,7 +819,7 @@ static int hvcs_probe(
return 0;
}
-static int hvcs_remove(struct vio_dev *dev)
+static void hvcs_remove(struct vio_dev *dev)
{
struct hvcs_struct *hvcsd = dev_get_drvdata(&dev->dev);
unsigned long flags;
@@ -849,7 +849,6 @@ static int hvcs_remove(struct vio_dev *dev)
printk(KERN_INFO "HVCS: vty-server@%X removed from the"
" vio bus.\n", dev->unit_address);
- return 0;
};
static struct vio_driver hvcs_vio_driver = {
base-commit: 2c87f7a38f930ef6f6a7bdd04aeb82ce3971b54b
--
2.30.0
^ permalink raw reply related
* Re: [RFC PATCH 4/8] powerpc/ppc_asm: use plain numbers for registers
From: Daniel Axtens @ 2021-02-26 0:12 UTC (permalink / raw)
To: Segher Boessenkool; +Cc: linuxppc-dev, llvmlinux
In-Reply-To: <20210225152547.GE28121@gate.crashing.org>
Segher Boessenkool <segher@kernel.crashing.org> writes:
> On Thu, Feb 25, 2021 at 02:10:02PM +1100, Daniel Axtens wrote:
>> This is dumb but makes the llvm integrated assembler happy.
>> https://github.com/ClangBuiltLinux/linux/issues/764
>
>> -#define r0 %r0
>
>> +#define r0 0
>
> This is a big step back (compare 9a13a524ba37).
>
> If you use a new enough GAS, you can use the -mregnames option and just
> say "r0" directly (so not define it at all, or define it to itself).
>
> ===
> addi 3,3,3
> addi r3,r3,3
> addi %r3,%r3,3
>
> addi 3,3,3
> addi r3,r3,r3
> addi %r3,%r3,%r3
> ===
>
> $ as t.s -o t.o -mregnames
> t.s: Assembler messages:
> t.s:6: Warning: invalid register expression
> t.s:7: Warning: invalid register expression
>
>
> Many people do not like bare numbers. It is a bit like not wearing
> seatbelts (but so is all assembler code really: you just have to pay
> attention). A better argument is that it is harder to read for people
> not used to assembler code like this.
>
> We used to have "#define r0 0" etc., and that was quite problematic.
> Like that "addi r3,r3,r3" example, but also, people wrote "r0" where
> only a plain 0 is allowed (like in "lwzx r3,0,r3": "r0" would be
> misleading there!)
So an overarching comment on all of these patches is that they're not
intended to be ready to merge, nor are they necessarily what I think is
the best solution. I'm just swinging a big hammer to see how far towards
LLVM_IAS=1 I can get on powerpc, and I accept I'm going to have to come
back and clean things up.
Anyway, noted, I'll push harder on trying to get llvm to accept %rN:
there was a patch that went in after llvm-11 that should help.
Kind regards,
Daniel
>
>
> Segher
^ permalink raw reply
* Re: [RFC PATCH 5/8] poweprc/lib/quad: Provide macros for lq/stq
From: Daniel Axtens @ 2021-02-26 0:13 UTC (permalink / raw)
To: Segher Boessenkool; +Cc: linuxppc-dev, llvmlinux
In-Reply-To: <20210225154444.GF28121@gate.crashing.org>
Segher Boessenkool <segher@kernel.crashing.org> writes:
> On Thu, Feb 25, 2021 at 02:10:03PM +1100, Daniel Axtens wrote:
>> +#define PPC_RAW_LQ(t, a, dq) (0xe0000000 | ___PPC_RT(t) | ___PPC_RA(a) | (((dq) & 0xfff) << 3))
>
> Please keep the operand order the same as for the assembler insns? So
> t,dq,a here.
>
> It should be ((dq) & 0x0fff) << 4) .
>
>> +#define PPC_RAW_STQ(t, a, ds) (0xf8000002 | ___PPC_RT(t) | ___PPC_RA(a) | (((ds) & 0xfff) << 3))
>
> And t,ds,a here. (But it should use "s" instead of "t" preferably, and
> use ___PPC_RS, because it is a source field, not a target).
>
> It should be ((ds) & 0x3fff) << 2) as well.
>
Ah, thank you. I'll fix this up.
Kind regards,
Daniel
>
> Segher
^ permalink raw reply
* Re: [RFC PATCH 7/8] powerpc/purgatory: drop .machine specifier
From: Daniel Axtens @ 2021-02-26 0:17 UTC (permalink / raw)
To: Segher Boessenkool; +Cc: linuxppc-dev, llvmlinux
In-Reply-To: <20210225155836.GG28121@gate.crashing.org>
Segher Boessenkool <segher@kernel.crashing.org> writes:
> On Thu, Feb 25, 2021 at 02:10:05PM +1100, Daniel Axtens wrote:
>> It's ignored by future versions of llvm's integrated assembler (by not -11).
>> I'm not sure what it does for us in gas.
>
> It enables all insns that exist on 620 (the first 64-bit PowerPC CPU).
>
>> --- a/arch/powerpc/purgatory/trampoline_64.S
>> +++ b/arch/powerpc/purgatory/trampoline_64.S
>> @@ -12,7 +12,7 @@
>> #include <asm/asm-compat.h>
>> #include <asm/crashdump-ppc64.h>
>>
>> - .machine ppc64
>> +//upgrade clang, gets ignored .machine ppc64
>
> Why delete it if it is ignored? Why add a cryptic comment?
Sorry, poor form on my part. I think I will give up on having llvm-11
work and target llvm HEAD, which means I can drop this.
>
>
> Segher
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox