Skip to content

Commit

Permalink
s390/ap: fix state machine hang after failure to enable irq
Browse files Browse the repository at this point in the history
[ Upstream commit cabebb6 ]

If for any reason the interrupt enable for an ap queue fails the
state machine run for the queue returned wrong return codes to the
caller. So the caller assumed interrupt support for this queue in
enabled and thus did not re-establish the high resolution timer used
for polling. In the end this let to a hang for the user space process
waiting "forever" for the reply.

This patch reworks these return codes to return correct indications
for the caller to re-establish the timer when a queue runs without
interrupt support.

Please note that this is fixing a wrong behavior after a first
failure (enable interrupt support for the queue) failed. However,
looks like this occasionally happens on KVM systems.

Signed-off-by: Harald Freudenberger <freude@linux.ibm.com>
Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
  • Loading branch information
hfreude authored and gregkh committed Sep 15, 2021
1 parent 86f9980 commit a758b1d
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 34 deletions.
25 changes: 8 additions & 17 deletions drivers/s390/crypto/ap_bus.c
Expand Up @@ -114,22 +114,13 @@ static struct bus_type ap_bus_type;
/* Adapter interrupt definitions */
static void ap_interrupt_handler(struct airq_struct *airq, bool floating);

static int ap_airq_flag;
static bool ap_irq_flag;

static struct airq_struct ap_airq = {
.handler = ap_interrupt_handler,
.isc = AP_ISC,
};

/**
* ap_using_interrupts() - Returns non-zero if interrupt support is
* available.
*/
static inline int ap_using_interrupts(void)
{
return ap_airq_flag;
}

/**
* ap_airq_ptr() - Get the address of the adapter interrupt indicator
*
Expand All @@ -139,7 +130,7 @@ static inline int ap_using_interrupts(void)
*/
void *ap_airq_ptr(void)
{
if (ap_using_interrupts())
if (ap_irq_flag)
return ap_airq.lsi_ptr;
return NULL;
}
Expand Down Expand Up @@ -369,7 +360,7 @@ void ap_wait(enum ap_sm_wait wait)
switch (wait) {
case AP_SM_WAIT_AGAIN:
case AP_SM_WAIT_INTERRUPT:
if (ap_using_interrupts())
if (ap_irq_flag)
break;
if (ap_poll_kthread) {
wake_up(&ap_poll_wait);
Expand Down Expand Up @@ -444,7 +435,7 @@ static void ap_tasklet_fn(unsigned long dummy)
* be received. Doing it in the beginning of the tasklet is therefor
* important that no requests on any AP get lost.
*/
if (ap_using_interrupts())
if (ap_irq_flag)
xchg(ap_airq.lsi_ptr, 0);

spin_lock_bh(&ap_queues_lock);
Expand Down Expand Up @@ -514,7 +505,7 @@ static int ap_poll_thread_start(void)
{
int rc;

if (ap_using_interrupts() || ap_poll_kthread)
if (ap_irq_flag || ap_poll_kthread)
return 0;
mutex_lock(&ap_poll_thread_mutex);
ap_poll_kthread = kthread_run(ap_poll_thread, NULL, "appoll");
Expand Down Expand Up @@ -1014,7 +1005,7 @@ static BUS_ATTR_RO(ap_adapter_mask);
static ssize_t ap_interrupts_show(struct bus_type *bus, char *buf)
{
return scnprintf(buf, PAGE_SIZE, "%d\n",
ap_using_interrupts() ? 1 : 0);
ap_irq_flag ? 1 : 0);
}

static BUS_ATTR_RO(ap_interrupts);
Expand Down Expand Up @@ -1687,7 +1678,7 @@ static int __init ap_module_init(void)
/* enable interrupts if available */
if (ap_interrupts_available()) {
rc = register_adapter_interrupt(&ap_airq);
ap_airq_flag = (rc == 0);
ap_irq_flag = (rc == 0);
}

/* Create /sys/bus/ap. */
Expand Down Expand Up @@ -1737,7 +1728,7 @@ static int __init ap_module_init(void)
bus_remove_file(&ap_bus_type, ap_bus_attrs[i]);
bus_unregister(&ap_bus_type);
out:
if (ap_using_interrupts())
if (ap_irq_flag)
unregister_adapter_interrupt(&ap_airq);
kfree(ap_qci_info);
return rc;
Expand Down
10 changes: 2 additions & 8 deletions drivers/s390/crypto/ap_bus.h
Expand Up @@ -77,12 +77,6 @@ static inline int ap_test_bit(unsigned int *ptr, unsigned int nr)
#define AP_FUNC_EP11 5
#define AP_FUNC_APXA 6

/*
* AP interrupt states
*/
#define AP_INTR_DISABLED 0 /* AP interrupt disabled */
#define AP_INTR_ENABLED 1 /* AP interrupt enabled */

/*
* AP queue state machine states
*/
Expand All @@ -109,7 +103,7 @@ enum ap_sm_event {
* AP queue state wait behaviour
*/
enum ap_sm_wait {
AP_SM_WAIT_AGAIN, /* retry immediately */
AP_SM_WAIT_AGAIN = 0, /* retry immediately */
AP_SM_WAIT_TIMEOUT, /* wait for timeout */
AP_SM_WAIT_INTERRUPT, /* wait for thin interrupt (if available) */
AP_SM_WAIT_NONE, /* no wait */
Expand Down Expand Up @@ -182,7 +176,7 @@ struct ap_queue {
enum ap_dev_state dev_state; /* queue device state */
bool config; /* configured state */
ap_qid_t qid; /* AP queue id. */
int interrupt; /* indicate if interrupts are enabled */
bool interrupt; /* indicate if interrupts are enabled */
int queue_count; /* # messages currently on AP queue. */
int pendingq_count; /* # requests on pendingq list. */
int requestq_count; /* # requests on requestq list. */
Expand Down
20 changes: 11 additions & 9 deletions drivers/s390/crypto/ap_queue.c
Expand Up @@ -19,15 +19,15 @@
static void __ap_flush_queue(struct ap_queue *aq);

/**
* ap_queue_enable_interruption(): Enable interruption on an AP queue.
* ap_queue_enable_irq(): Enable interrupt support on this AP queue.
* @qid: The AP queue number
* @ind: the notification indicator byte
*
* Enables interruption on AP queue via ap_aqic(). Based on the return
* value it waits a while and tests the AP queue if interrupts
* have been switched on using ap_test_queue().
*/
static int ap_queue_enable_interruption(struct ap_queue *aq, void *ind)
static int ap_queue_enable_irq(struct ap_queue *aq, void *ind)
{
struct ap_queue_status status;
struct ap_qirq_ctrl qirqctrl = { 0 };
Expand Down Expand Up @@ -198,7 +198,8 @@ static enum ap_sm_wait ap_sm_read(struct ap_queue *aq)
return AP_SM_WAIT_NONE;
case AP_RESPONSE_NO_PENDING_REPLY:
if (aq->queue_count > 0)
return AP_SM_WAIT_INTERRUPT;
return aq->interrupt ?
AP_SM_WAIT_INTERRUPT : AP_SM_WAIT_TIMEOUT;
aq->sm_state = AP_SM_STATE_IDLE;
return AP_SM_WAIT_NONE;
default:
Expand Down Expand Up @@ -252,7 +253,8 @@ static enum ap_sm_wait ap_sm_write(struct ap_queue *aq)
fallthrough;
case AP_RESPONSE_Q_FULL:
aq->sm_state = AP_SM_STATE_QUEUE_FULL;
return AP_SM_WAIT_INTERRUPT;
return aq->interrupt ?
AP_SM_WAIT_INTERRUPT : AP_SM_WAIT_TIMEOUT;
case AP_RESPONSE_RESET_IN_PROGRESS:
aq->sm_state = AP_SM_STATE_RESET_WAIT;
return AP_SM_WAIT_TIMEOUT;
Expand Down Expand Up @@ -302,7 +304,7 @@ static enum ap_sm_wait ap_sm_reset(struct ap_queue *aq)
case AP_RESPONSE_NORMAL:
case AP_RESPONSE_RESET_IN_PROGRESS:
aq->sm_state = AP_SM_STATE_RESET_WAIT;
aq->interrupt = AP_INTR_DISABLED;
aq->interrupt = false;
return AP_SM_WAIT_TIMEOUT;
default:
aq->dev_state = AP_DEV_STATE_ERROR;
Expand Down Expand Up @@ -335,7 +337,7 @@ static enum ap_sm_wait ap_sm_reset_wait(struct ap_queue *aq)
switch (status.response_code) {
case AP_RESPONSE_NORMAL:
lsi_ptr = ap_airq_ptr();
if (lsi_ptr && ap_queue_enable_interruption(aq, lsi_ptr) == 0)
if (lsi_ptr && ap_queue_enable_irq(aq, lsi_ptr) == 0)
aq->sm_state = AP_SM_STATE_SETIRQ_WAIT;
else
aq->sm_state = (aq->queue_count > 0) ?
Expand Down Expand Up @@ -376,7 +378,7 @@ static enum ap_sm_wait ap_sm_setirq_wait(struct ap_queue *aq)

if (status.irq_enabled == 1) {
/* Irqs are now enabled */
aq->interrupt = AP_INTR_ENABLED;
aq->interrupt = true;
aq->sm_state = (aq->queue_count > 0) ?
AP_SM_STATE_WORKING : AP_SM_STATE_IDLE;
}
Expand Down Expand Up @@ -566,7 +568,7 @@ static ssize_t interrupt_show(struct device *dev,
spin_lock_bh(&aq->lock);
if (aq->sm_state == AP_SM_STATE_SETIRQ_WAIT)
rc = scnprintf(buf, PAGE_SIZE, "Enable Interrupt pending.\n");
else if (aq->interrupt == AP_INTR_ENABLED)
else if (aq->interrupt)
rc = scnprintf(buf, PAGE_SIZE, "Interrupts enabled.\n");
else
rc = scnprintf(buf, PAGE_SIZE, "Interrupts disabled.\n");
Expand Down Expand Up @@ -747,7 +749,7 @@ struct ap_queue *ap_queue_create(ap_qid_t qid, int device_type)
aq->ap_dev.device.type = &ap_queue_type;
aq->ap_dev.device_type = device_type;
aq->qid = qid;
aq->interrupt = AP_INTR_DISABLED;
aq->interrupt = false;
spin_lock_init(&aq->lock);
INIT_LIST_HEAD(&aq->pendingq);
INIT_LIST_HEAD(&aq->requestq);
Expand Down

0 comments on commit a758b1d

Please sign in to comment.