Skip to content

Commit c338436

Browse files
hfreudehcahca
hfreude
authored and
hcahca
committed
s390/zcrypt: improve zcrypt retry behavior
This patch reworks and improves the zcrypt retry behavior: - The zcrypt_rescan_req counter has been removed. This counter variable has been increased on some transport errors and was used as a gatekeeper for AP bus rescans. - Rework of the zcrypt_process_rescan() function to not use the above counter variable any more. Instead now always the ap_bus_force_rescan() function is called (as this has been improved with a previous patch). - As the zcrpyt_process_rescan() function is called in all cprb send functions in case of the first attempt to send failed with ENODEV now before the next attempt to send an cprb is started. - Introduce a define ZCRYPT_WAIT_BINDINGS_COMPLETE_MS for the amount of milliseconds to have the zcrypt API wait for AP bindings complete. This amount has been reduced to 30s (was 60s). Some playing around showed that 30s is a really fair limit. The result of the above together with the patches to improve the AP scan bus functions is that after the first loop of cprb send retries when the result is a ENODEV the AP bus scan is always triggered (synchronous). If the AP bus scan detects changes in the configuration, all the send functions now retry when the first attempt was failing with ENODEV in the hope that now a suitable device has appeared. About concurrency: The ap_bus_force_rescan() uses a mutex to ensure only one active AP bus scan is running. Another caller of this function is blocked as long as the scan is running but does not cause yet another scan. Instead the result of the 'other' scan is used. This affects only tasks which run into an initial ENODEV. Tasks with successful delivery of cprbs will never invoke the bus scan and thus never get blocked by the mutex. Signed-off-by: Harald Freudenberger <[email protected]> Reviewed-by: Holger Dengler <[email protected]> Signed-off-by: Heiko Carstens <[email protected]>
1 parent 77c51fc commit c338436

File tree

3 files changed

+58
-80
lines changed

3 files changed

+58
-80
lines changed

‎drivers/s390/crypto/zcrypt_api.c

Lines changed: 47 additions & 77 deletions
Original file line numberDiff line numberDiff line change
@@ -60,10 +60,6 @@ DEFINE_SPINLOCK(zcrypt_list_lock);
6060
LIST_HEAD(zcrypt_card_list);
6161

6262
static atomic_t zcrypt_open_count = ATOMIC_INIT(0);
63-
static atomic_t zcrypt_rescan_count = ATOMIC_INIT(0);
64-
65-
atomic_t zcrypt_rescan_req = ATOMIC_INIT(0);
66-
EXPORT_SYMBOL(zcrypt_rescan_req);
6763

6864
static LIST_HEAD(zcrypt_ops_list);
6965

@@ -72,20 +68,15 @@ debug_info_t *zcrypt_dbf_info;
7268

7369
/*
7470
* Process a rescan of the transport layer.
75-
*
76-
* Returns 1, if the rescan has been processed, otherwise 0.
71+
* Runs a synchronous AP bus rescan.
72+
* Returns true if something has changed (for example the
73+
* bus scan has found and build up new devices) and it is
74+
* worth to do a retry. Otherwise false is returned meaning
75+
* no changes on the AP bus level.
7776
*/
78-
static inline int zcrypt_process_rescan(void)
79-
{
80-
if (atomic_read(&zcrypt_rescan_req)) {
81-
atomic_set(&zcrypt_rescan_req, 0);
82-
atomic_inc(&zcrypt_rescan_count);
83-
ap_bus_force_rescan();
84-
ZCRYPT_DBF_INFO("%s rescan count=%07d\n", __func__,
85-
atomic_inc_return(&zcrypt_rescan_count));
86-
return 1;
87-
}
88-
return 0;
77+
static inline bool zcrypt_process_rescan(void)
78+
{
79+
return ap_bus_force_rescan();
8980
}
9081

9182
void zcrypt_msgtype_register(struct zcrypt_ops *zops)
@@ -1481,16 +1472,13 @@ static int icarsamodexpo_ioctl(struct ap_perms *perms, unsigned long arg)
14811472

14821473
do {
14831474
rc = zcrypt_rsa_modexpo(perms, &tr, &mex);
1484-
if (rc == -EAGAIN)
1485-
tr.again_counter++;
1486-
} while (rc == -EAGAIN && tr.again_counter < TRACK_AGAIN_MAX);
1487-
/* on failure: retry once again after a requested rescan */
1488-
if ((rc == -ENODEV) && (zcrypt_process_rescan()))
1475+
} while (rc == -EAGAIN && ++tr.again_counter < TRACK_AGAIN_MAX);
1476+
1477+
/* on ENODEV failure: retry once again after a requested rescan */
1478+
if (rc == -ENODEV && zcrypt_process_rescan())
14891479
do {
14901480
rc = zcrypt_rsa_modexpo(perms, &tr, &mex);
1491-
if (rc == -EAGAIN)
1492-
tr.again_counter++;
1493-
} while (rc == -EAGAIN && tr.again_counter < TRACK_AGAIN_MAX);
1481+
} while (rc == -EAGAIN && ++tr.again_counter < TRACK_AGAIN_MAX);
14941482
if (rc == -EAGAIN && tr.again_counter >= TRACK_AGAIN_MAX)
14951483
rc = -EIO;
14961484
if (rc) {
@@ -1513,16 +1501,13 @@ static int icarsacrt_ioctl(struct ap_perms *perms, unsigned long arg)
15131501

15141502
do {
15151503
rc = zcrypt_rsa_crt(perms, &tr, &crt);
1516-
if (rc == -EAGAIN)
1517-
tr.again_counter++;
1518-
} while (rc == -EAGAIN && tr.again_counter < TRACK_AGAIN_MAX);
1519-
/* on failure: retry once again after a requested rescan */
1520-
if ((rc == -ENODEV) && (zcrypt_process_rescan()))
1504+
} while (rc == -EAGAIN && ++tr.again_counter < TRACK_AGAIN_MAX);
1505+
1506+
/* on ENODEV failure: retry once again after a requested rescan */
1507+
if (rc == -ENODEV && zcrypt_process_rescan())
15211508
do {
15221509
rc = zcrypt_rsa_crt(perms, &tr, &crt);
1523-
if (rc == -EAGAIN)
1524-
tr.again_counter++;
1525-
} while (rc == -EAGAIN && tr.again_counter < TRACK_AGAIN_MAX);
1510+
} while (rc == -EAGAIN && ++tr.again_counter < TRACK_AGAIN_MAX);
15261511
if (rc == -EAGAIN && tr.again_counter >= TRACK_AGAIN_MAX)
15271512
rc = -EIO;
15281513
if (rc) {
@@ -1545,16 +1530,13 @@ static int zsecsendcprb_ioctl(struct ap_perms *perms, unsigned long arg)
15451530

15461531
do {
15471532
rc = _zcrypt_send_cprb(true, perms, &tr, &xcrb);
1548-
if (rc == -EAGAIN)
1549-
tr.again_counter++;
1550-
} while (rc == -EAGAIN && tr.again_counter < TRACK_AGAIN_MAX);
1551-
/* on failure: retry once again after a requested rescan */
1552-
if ((rc == -ENODEV) && (zcrypt_process_rescan()))
1533+
} while (rc == -EAGAIN && ++tr.again_counter < TRACK_AGAIN_MAX);
1534+
1535+
/* on ENODEV failure: retry once again after a requested rescan */
1536+
if (rc == -ENODEV && zcrypt_process_rescan())
15531537
do {
15541538
rc = _zcrypt_send_cprb(true, perms, &tr, &xcrb);
1555-
if (rc == -EAGAIN)
1556-
tr.again_counter++;
1557-
} while (rc == -EAGAIN && tr.again_counter < TRACK_AGAIN_MAX);
1539+
} while (rc == -EAGAIN && ++tr.again_counter < TRACK_AGAIN_MAX);
15581540
if (rc == -EAGAIN && tr.again_counter >= TRACK_AGAIN_MAX)
15591541
rc = -EIO;
15601542
if (rc)
@@ -1578,16 +1560,13 @@ static int zsendep11cprb_ioctl(struct ap_perms *perms, unsigned long arg)
15781560

15791561
do {
15801562
rc = _zcrypt_send_ep11_cprb(true, perms, &tr, &xcrb);
1581-
if (rc == -EAGAIN)
1582-
tr.again_counter++;
1583-
} while (rc == -EAGAIN && tr.again_counter < TRACK_AGAIN_MAX);
1584-
/* on failure: retry once again after a requested rescan */
1585-
if ((rc == -ENODEV) && (zcrypt_process_rescan()))
1563+
} while (rc == -EAGAIN && ++tr.again_counter < TRACK_AGAIN_MAX);
1564+
1565+
/* on ENODEV failure: retry once again after a requested rescan */
1566+
if (rc == -ENODEV && zcrypt_process_rescan())
15861567
do {
15871568
rc = _zcrypt_send_ep11_cprb(true, perms, &tr, &xcrb);
1588-
if (rc == -EAGAIN)
1589-
tr.again_counter++;
1590-
} while (rc == -EAGAIN && tr.again_counter < TRACK_AGAIN_MAX);
1569+
} while (rc == -EAGAIN && ++tr.again_counter < TRACK_AGAIN_MAX);
15911570
if (rc == -EAGAIN && tr.again_counter >= TRACK_AGAIN_MAX)
15921571
rc = -EIO;
15931572
if (rc)
@@ -1758,16 +1737,13 @@ static long trans_modexpo32(struct ap_perms *perms, struct file *filp,
17581737
mex64.n_modulus = compat_ptr(mex32.n_modulus);
17591738
do {
17601739
rc = zcrypt_rsa_modexpo(perms, &tr, &mex64);
1761-
if (rc == -EAGAIN)
1762-
tr.again_counter++;
1763-
} while (rc == -EAGAIN && tr.again_counter < TRACK_AGAIN_MAX);
1764-
/* on failure: retry once again after a requested rescan */
1765-
if ((rc == -ENODEV) && (zcrypt_process_rescan()))
1740+
} while (rc == -EAGAIN && ++tr.again_counter < TRACK_AGAIN_MAX);
1741+
1742+
/* on ENODEV failure: retry once again after a requested rescan */
1743+
if (rc == -ENODEV && zcrypt_process_rescan())
17661744
do {
17671745
rc = zcrypt_rsa_modexpo(perms, &tr, &mex64);
1768-
if (rc == -EAGAIN)
1769-
tr.again_counter++;
1770-
} while (rc == -EAGAIN && tr.again_counter < TRACK_AGAIN_MAX);
1746+
} while (rc == -EAGAIN && ++tr.again_counter < TRACK_AGAIN_MAX);
17711747
if (rc == -EAGAIN && tr.again_counter >= TRACK_AGAIN_MAX)
17721748
rc = -EIO;
17731749
if (rc)
@@ -1811,16 +1787,13 @@ static long trans_modexpo_crt32(struct ap_perms *perms, struct file *filp,
18111787
crt64.u_mult_inv = compat_ptr(crt32.u_mult_inv);
18121788
do {
18131789
rc = zcrypt_rsa_crt(perms, &tr, &crt64);
1814-
if (rc == -EAGAIN)
1815-
tr.again_counter++;
1816-
} while (rc == -EAGAIN && tr.again_counter < TRACK_AGAIN_MAX);
1817-
/* on failure: retry once again after a requested rescan */
1818-
if ((rc == -ENODEV) && (zcrypt_process_rescan()))
1790+
} while (rc == -EAGAIN && ++tr.again_counter < TRACK_AGAIN_MAX);
1791+
1792+
/* on ENODEV failure: retry once again after a requested rescan */
1793+
if (rc == -ENODEV && zcrypt_process_rescan())
18191794
do {
18201795
rc = zcrypt_rsa_crt(perms, &tr, &crt64);
1821-
if (rc == -EAGAIN)
1822-
tr.again_counter++;
1823-
} while (rc == -EAGAIN && tr.again_counter < TRACK_AGAIN_MAX);
1796+
} while (rc == -EAGAIN && ++tr.again_counter < TRACK_AGAIN_MAX);
18241797
if (rc == -EAGAIN && tr.again_counter >= TRACK_AGAIN_MAX)
18251798
rc = -EIO;
18261799
if (rc)
@@ -1883,16 +1856,13 @@ static long trans_xcrb32(struct ap_perms *perms, struct file *filp,
18831856
xcrb64.status = xcrb32.status;
18841857
do {
18851858
rc = _zcrypt_send_cprb(true, perms, &tr, &xcrb64);
1886-
if (rc == -EAGAIN)
1887-
tr.again_counter++;
1888-
} while (rc == -EAGAIN && tr.again_counter < TRACK_AGAIN_MAX);
1889-
/* on failure: retry once again after a requested rescan */
1890-
if ((rc == -ENODEV) && (zcrypt_process_rescan()))
1859+
} while (rc == -EAGAIN && ++tr.again_counter < TRACK_AGAIN_MAX);
1860+
1861+
/* on ENODEV failure: retry once again after a requested rescan */
1862+
if (rc == -ENODEV && zcrypt_process_rescan())
18911863
do {
18921864
rc = _zcrypt_send_cprb(true, perms, &tr, &xcrb64);
1893-
if (rc == -EAGAIN)
1894-
tr.again_counter++;
1895-
} while (rc == -EAGAIN && tr.again_counter < TRACK_AGAIN_MAX);
1865+
} while (rc == -EAGAIN && ++tr.again_counter < TRACK_AGAIN_MAX);
18961866
if (rc == -EAGAIN && tr.again_counter >= TRACK_AGAIN_MAX)
18971867
rc = -EIO;
18981868
xcrb32.reply_control_blk_length = xcrb64.reply_control_blk_length;
@@ -1964,8 +1934,8 @@ static int zcrypt_rng_data_read(struct hwrng *rng, u32 *data)
19641934
*/
19651935
if (zcrypt_rng_buffer_index == 0) {
19661936
rc = zcrypt_rng((char *)zcrypt_rng_buffer);
1967-
/* on failure: retry once again after a requested rescan */
1968-
if ((rc == -ENODEV) && (zcrypt_process_rescan()))
1937+
/* on ENODEV failure: retry once again after an AP bus rescan */
1938+
if (rc == -ENODEV && zcrypt_process_rescan())
19691939
rc = zcrypt_rng((char *)zcrypt_rng_buffer);
19701940
if (rc < 0)
19711941
return -EIO;
@@ -2027,7 +1997,7 @@ void zcrypt_rng_device_remove(void)
20271997
* an asynchronous job. This function waits until these initial jobs
20281998
* are done and so the zcrypt api should be ready to serve crypto
20291999
* requests - if there are resources available. The function uses an
2030-
* internal timeout of 60s. The very first caller will either wait for
2000+
* internal timeout of 30s. The very first caller will either wait for
20312001
* ap bus bindings complete or the timeout happens. This state will be
20322002
* remembered for further callers which will only be blocked until a
20332003
* decision is made (timeout or bindings complete).
@@ -2047,7 +2017,7 @@ int zcrypt_wait_api_operational(void)
20472017
case 0:
20482018
/* initial state, invoke wait for the ap bus complete */
20492019
rc = ap_wait_apqn_bindings_complete(
2050-
msecs_to_jiffies(60 * 1000));
2020+
msecs_to_jiffies(ZCRYPT_WAIT_BINDINGS_COMPLETE_MS));
20512021
switch (rc) {
20522022
case 0:
20532023
/* ap bus bindings are complete */

‎drivers/s390/crypto/zcrypt_api.h

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,15 @@
3838
*/
3939
#define ZCRYPT_RNG_BUFFER_SIZE 4096
4040

41+
/**
42+
* The zcrypt_wait_api_operational() function waits this
43+
* amount in milliseconds for ap_wait_aqpn_bindings_complete().
44+
* Also on a cprb send failure with ENODEV the send functions
45+
* trigger an ap bus rescan and wait this time in milliseconds
46+
* for ap_wait_aqpn_bindings_complete() before resending.
47+
*/
48+
#define ZCRYPT_WAIT_BINDINGS_COMPLETE_MS 30000
49+
4150
/*
4251
* Identifier for Crypto Request Performance Index
4352
*/

‎drivers/s390/crypto/zcrypt_error.h

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -119,10 +119,9 @@ static inline int convert_error(struct zcrypt_queue *zq,
119119
case REP82_ERROR_MESSAGE_TYPE: /* 0x20 */
120120
case REP82_ERROR_TRANSPORT_FAIL: /* 0x90 */
121121
/*
122-
* Msg to wrong type or card/infrastructure failure.
123-
* Trigger rescan of the ap bus, trigger retry request.
122+
* Msg to wrong type or card/infrastructure failure. Return
123+
* EAGAIN, the upper layer may do a retry on the request.
124124
*/
125-
atomic_set(&zcrypt_rescan_req, 1);
126125
/* For type 86 response show the apfs value (failure reason) */
127126
if (ehdr->reply_code == REP82_ERROR_TRANSPORT_FAIL &&
128127
ehdr->type == TYPE86_RSP_CODE) {

0 commit comments

Comments
 (0)