[PATCH 1/2] target/s390x: Make PRNO TRNG interruptible

Ilya Leoshkevich posted 2 patches 2 months ago
Maintainers: Richard Henderson <richard.henderson@linaro.org>, Ilya Leoshkevich <iii@linux.ibm.com>, David Hildenbrand <david@kernel.org>, Cornelia Huck <cohuck@redhat.com>, Eric Farman <farman@linux.ibm.com>, Matthew Rosato <mjrosato@linux.ibm.com>
There is a newer version of this series
[PATCH 1/2] target/s390x: Make PRNO TRNG interruptible
Posted by Ilya Leoshkevich 2 months ago
fill_buf_random() writes the entire guest-requested amount of random
bytes in one go. Since the length is a full 64-bit value, a guest can
request several gigabytes and keep the vCPU spinning inside the helper,
without a chance to react to interrupts.

Do the same thing as cpacf_sha512(): process only a limited amount of
data per instruction execution and report partial completion with
condition code 3. This way the guest reissues the instruction, and
pending interrupts can be handled in between.

Reported-by: Christian Borntraeger <borntraeger@linux.ibm.com>
Fixes: 3dbc5fdacb5a ("target/s390x: support PRNO_TRNG instruction")
Cc: qemu-stable@nongnu.org
Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
 target/s390x/tcg/crypto_helper.c | 25 +++++++++++++++++++------
 1 file changed, 19 insertions(+), 6 deletions(-)

diff --git a/target/s390x/tcg/crypto_helper.c b/target/s390x/tcg/crypto_helper.c
index 8fe0a222198..4902c4e93ba 100644
--- a/target/s390x/tcg/crypto_helper.c
+++ b/target/s390x/tcg/crypto_helper.c
@@ -242,12 +242,13 @@ static int cpacf_sha512(CPUS390XState *env, const int mmu_idx, uintptr_t ra,
     return !len ? 0 : 3;
 }
 
-static void fill_buf_random(CPUS390XState *env, const int mmu_idx, uintptr_t ra,
-                            uint64_t *buf_reg, uint64_t *len_reg)
+static int fill_buf_random(CPUS390XState *env, const int mmu_idx, uintptr_t ra,
+                           uint64_t *buf_reg, uint64_t *len_reg)
 {
+    enum { MAX_BYTES_PER_RUN = 8192 }; /* Arbitrary: keep interactivity. */
     const MemOpIdx oi = make_memop_idx(MO_8, mmu_idx);
     uint8_t tmp[256];
-    uint64_t len = *len_reg;
+    uint64_t len = *len_reg, processed = 0;
     int buf_reg_len = 64;
 
     if (!(env->psw.mask & PSW_MASK_64)) {
@@ -258,6 +259,10 @@ static void fill_buf_random(CPUS390XState *env, const int mmu_idx, uintptr_t ra,
     while (len) {
         size_t block = MIN(len, sizeof(tmp));
 
+        if (processed >= MAX_BYTES_PER_RUN) {
+            break;
+        }
+
         qemu_guest_getrandom_nofail(tmp, block);
         for (size_t i = 0; i < block; ++i) {
             cpu_stb_mmu(env, wrap_address(env, *buf_reg), tmp[i], oi, ra);
@@ -265,7 +270,10 @@ static void fill_buf_random(CPUS390XState *env, const int mmu_idx, uintptr_t ra,
             --*len_reg;
         }
         len -= block;
+        processed += block;
     }
+
+    return !len ? 0 : 3;
 }
 
 uint32_t HELPER(msa)(CPUS390XState *env, uint32_t r1, uint32_t r2, uint32_t r3,
@@ -278,6 +286,7 @@ uint32_t HELPER(msa)(CPUS390XState *env, uint32_t r1, uint32_t r2, uint32_t r3,
     uint8_t subfunc[16] = { 0 };
     uint64_t param_addr;
     MemOpIdx oi;
+    int cc;
 
     switch (type) {
     case S390_FEAT_TYPE_KMAC:
@@ -308,9 +317,13 @@ uint32_t HELPER(msa)(CPUS390XState *env, uint32_t r1, uint32_t r2, uint32_t r3,
         return cpacf_sha512(env, mmu_idx, ra, env->regs[1], &env->regs[r2],
                             &env->regs[r2 + 1], type);
     case 114: /* CPACF_PRNO_TRNG */
-        fill_buf_random(env, mmu_idx, ra, &env->regs[r1], &env->regs[r1 + 1]);
-        fill_buf_random(env, mmu_idx, ra, &env->regs[r2], &env->regs[r2 + 1]);
-        break;
+        cc = fill_buf_random(env, mmu_idx, ra,
+                             &env->regs[r1], &env->regs[r1 + 1]);
+        if (cc == 0) {
+            cc = fill_buf_random(env, mmu_idx, ra,
+                                 &env->regs[r2], &env->regs[r2 + 1]);
+        }
+        return cc;
     default:
         /* we don't implement any other subfunction yet */
         g_assert_not_reached();
-- 
2.55.0
Re: [PATCH 1/2] target/s390x: Make PRNO TRNG interruptible
Posted by Richard Henderson 2 months ago
On 7/14/26 04:34, Ilya Leoshkevich wrote:
> fill_buf_random() writes the entire guest-requested amount of random
> bytes in one go. Since the length is a full 64-bit value, a guest can
> request several gigabytes and keep the vCPU spinning inside the helper,
> without a chance to react to interrupts.
> 
> Do the same thing as cpacf_sha512(): process only a limited amount of
> data per instruction execution and report partial completion with
> condition code 3. This way the guest reissues the instruction, and
> pending interrupts can be handled in between.
> 
> Reported-by: Christian Borntraeger <borntraeger@linux.ibm.com>
> Fixes: 3dbc5fdacb5a ("target/s390x: support PRNO_TRNG instruction")
> Cc: qemu-stable@nongnu.org
> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> ---
>   target/s390x/tcg/crypto_helper.c | 25 +++++++++++++++++++------
>   1 file changed, 19 insertions(+), 6 deletions(-)
> 
> diff --git a/target/s390x/tcg/crypto_helper.c b/target/s390x/tcg/crypto_helper.c
> index 8fe0a222198..4902c4e93ba 100644
> --- a/target/s390x/tcg/crypto_helper.c
> +++ b/target/s390x/tcg/crypto_helper.c
> @@ -242,12 +242,13 @@ static int cpacf_sha512(CPUS390XState *env, const int mmu_idx, uintptr_t ra,
>       return !len ? 0 : 3;
>   }
>   
> -static void fill_buf_random(CPUS390XState *env, const int mmu_idx, uintptr_t ra,
> -                            uint64_t *buf_reg, uint64_t *len_reg)
> +static int fill_buf_random(CPUS390XState *env, const int mmu_idx, uintptr_t ra,
> +                           uint64_t *buf_reg, uint64_t *len_reg)
>   {
> +    enum { MAX_BYTES_PER_RUN = 8192 }; /* Arbitrary: keep interactivity. */
>       const MemOpIdx oi = make_memop_idx(MO_8, mmu_idx);
>       uint8_t tmp[256];
> -    uint64_t len = *len_reg;
> +    uint64_t len = *len_reg, processed = 0;
>       int buf_reg_len = 64;
>   
>       if (!(env->psw.mask & PSW_MASK_64)) {
> @@ -258,6 +259,10 @@ static void fill_buf_random(CPUS390XState *env, const int mmu_idx, uintptr_t ra,
>       while (len) {
>           size_t block = MIN(len, sizeof(tmp));
>   
> +        if (processed >= MAX_BYTES_PER_RUN) {
> +            break;
> +        }

Alternately, stop with cpu_loop_exit_requested() at the bottom of the loop.

Either way,
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~