[PATCH v1 0/2] Rework cpacf_kma() function

[PATCH v1 0/2] Rework cpacf_kma() function

[Harald Freudenberger]

Rework the CPACF inline function cpacf_kma() and the only caller - the
AES GCM implementation in arch/s390/crypto/aes_s390.c

Changelog:
v1: initial version

Harald Freudenberger (2):
  s390/cpacf: Rework cpacf_kma() to return condition code
  s390/crypto: Handle cpacf_kma() return code

 arch/s390/crypto/aes_s390.c   | 11 +++++++----
 arch/s390/include/asm/cpacf.h | 24 ++++++++++++++++++------
 2 files changed, 25 insertions(+), 10 deletions(-)

--
2.43.0

[PATCH v1 1/2] s390/cpacf: Rework cpacf_kma() to return condition code

[Harald Freudenberger]

The KMA instruction can signal a verification-pattern mismatch (CC 1)
when a protected key is used, or incomplete processing (CC 2) when a
sub-block-size chunk is submitted without the appropriate LAAD/LPC
flag. Both conditions shall be visible to the caller similar like the
other CPACF instructions.

So rework the cpacf_kma() inline function to return the condition code
instead of void. However, CC 3 (partial completion) continues to be
handled internally. Also improve the function to use the CC macros for
better readability. Additionally add an kmsan_unpoison_memory()
statement to reflect the updated memory by hardware.

Signed-off-by: Harald Freudenberger <freude@linux.ibm.com>
---
 arch/s390/include/asm/cpacf.h | 24 ++++++++++++++++++------
 1 file changed, 18 insertions(+), 6 deletions(-)

diff --git a/arch/s390/include/asm/cpacf.h b/arch/s390/include/asm/cpacf.h
index 6174552d856d..00aca3194b67 100644
--- a/arch/s390/include/asm/cpacf.h
+++ b/arch/s390/include/asm/cpacf.h
@@ -715,12 +715,20 @@ static inline void cpacf_pckmo(long func, void *param)
  * @src_len: length of src operand in bytes
  * @aad: address of additional authenticated data memory area
  * @aad_len: length of aad operand in bytes
+ *
+ * Returns the condition code:
+ *  0 - normal completion
+ *  1 - verification-pattern mismatch
+ *  2 - incomplete processing (see POP for details)
+ * Condition code 3 (partial completion) is handled within the asm code
+ * and never returned.
  */
-static inline void cpacf_kma(unsigned long func, void *param, u8 *dest,
-			     const u8 *src, unsigned long src_len,
-			     const u8 *aad, unsigned long aad_len)
+static inline int cpacf_kma(unsigned long func, void *param, u8 *dest,
+			    const u8 *src, unsigned long src_len,
+			    const u8 *aad, unsigned long aad_len)
 {
 	union register_pair d, s, a;
+	int cc;
 
 	d.even = (unsigned long)dest;
 	s.even = (unsigned long)src;
@@ -731,12 +739,16 @@ static inline void cpacf_kma(unsigned long func, void *param, u8 *dest,
 		"	lgr	0,%[fc]\n"
 		"	lgr	1,%[pba]\n"
 		"0:	.insn	rrf,%[opc] << 16,%[dst],%[src],%[aad],0\n"
-		"	brc	1,0b"	/* handle partial completion */
-		: [dst] "+&d" (d.pair), [src] "+&d" (s.pair),
+		"	brc	1,0b\n"	/* handle partial completion */
+		CC_IPM(cc)
+		: CC_OUT(cc, cc), [dst] "+&d" (d.pair), [src] "+&d" (s.pair),
 		  [aad] "+&d" (a.pair)
 		: [fc] "d" (func), [pba] "d" ((unsigned long)param),
 		  [opc] "i" (CPACF_KMA)
-		: "cc", "memory", "0", "1");
+		: CC_CLOBBER_LIST("memory", "0", "1"));
+
+	kmsan_unpoison_memory(dest, src_len - s.odd);
+	return CC_TRANSFORM(cc);
 }
 
 #endif	/* _ASM_S390_CPACF_H */
-- 
2.43.0

[PATCH v1 2/2] s390/crypto: Handle cpacf_kma() return code

[Harald Freudenberger]

Now that cpacf_kma() returns the condition code, check it and bail out
with -EIO on any non-zero result. In practice CC 1 and CC 2 cannot
occur: the GCM function codes are clear-key only, and the scatterwalk
loop ensures block-aligned chunks with LAAD/LPC set on the final call.
However, it is a defensive improvement, not a bug fix.

Signed-off-by: Harald Freudenberger <freude@linux.ibm.com>
---
 arch/s390/crypto/aes_s390.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/arch/s390/crypto/aes_s390.c b/arch/s390/crypto/aes_s390.c
index 0be6fa779d2c..ec88ce88f3ef 100644
--- a/arch/s390/crypto/aes_s390.c
+++ b/arch/s390/crypto/aes_s390.c
@@ -905,10 +905,13 @@ static int gcm_aes_crypt(struct aead_request *req, unsigned int flags)
 		if (aad_bytes > 0)
 			memcpy(gw_out.ptr, gw_in.ptr, aad_bytes);
 
-		cpacf_kma(ctx->fc | flags, &param,
-			  gw_out.ptr + aad_bytes,
-			  gw_in.ptr + aad_bytes, pc_bytes,
-			  gw_in.ptr, aad_bytes);
+		if (cpacf_kma(ctx->fc | flags, &param,
+			      gw_out.ptr + aad_bytes,
+			      gw_in.ptr + aad_bytes, pc_bytes,
+			      gw_in.ptr, aad_bytes)) {
+			ret = -EIO;
+			goto out;
+		}
 
 		n = aad_bytes + pc_bytes;
 		if (gcm_in_walk_done(&gw_in, n) != n) {
-- 
2.43.0

Re: [PATCH v1 2/2] s390/crypto: Handle cpacf_kma() return code

[sashiko-bot]

> Now that cpacf_kma() returns the condition code, check it and bail out
> with -EIO on any non-zero result. In practice CC 1 and CC 2 cannot
> occur: the GCM function codes are clear-key only, and the scatterwalk
> loop ensures block-aligned chunks with LAAD/LPC set on the final call.
> However, it is a defensive improvement, not a bug fix.
> 
> Signed-off-by: Harald Freudenberger <freude@linux.ibm.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261005141417.65822-1-freude@linux.ibm.com?part=2

Re: [PATCH v1 1/2] s390/cpacf: Rework cpacf_kma() to return condition code

[sashiko-bot]

> The KMA instruction can signal a verification-pattern mismatch (CC 1)
> when a protected key is used, or incomplete processing (CC 2) when a
> sub-block-size chunk is submitted without the appropriate LAAD/LPC
> flag. Both conditions shall be visible to the caller similar like the
> other CPACF instructions.
> 
> So rework the cpacf_kma() inline function to return the condition code
> instead of void. However, CC 3 (partial completion) continues to be
> handled internally. Also improve the function to use the CC macros for
> better readability. Additionally add an kmsan_unpoison_memory()
> statement to reflect the updated memory by hardware.
> 
> Signed-off-by: Harald Freudenberger <freude@linux.ibm.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261005141417.65822-1-freude@linux.ibm.com?part=1

Re: [PATCH v1 1/2] s390/cpacf: Rework cpacf_kma() to return condition code

[Holger Dengler]

On 10/5/26 16:14, Harald Freudenberger wrote:
> The KMA instruction can signal a verification-pattern mismatch (CC 1)
> when a protected key is used, or incomplete processing (CC 2) when a
> sub-block-size chunk is submitted without the appropriate LAAD/LPC
> flag. Both conditions shall be visible to the caller similar like the
> other CPACF instructions.
> 
> So rework the cpacf_kma() inline function to return the condition code
> instead of void. However, CC 3 (partial completion) continues to be
> handled internally. Also improve the function to use the CC macros for
> better readability. Additionally add an kmsan_unpoison_memory()
> statement to reflect the updated memory by hardware.
> 
> Signed-off-by: Harald Freudenberger <freude@linux.ibm.com>

Reviewed-by: Holger Dengler <dengler@linux.ibm.com>

-- 
Mit freundlichen Grüßen / Kind regards
Holger Dengler

Re: [PATCH v1 2/2] s390/crypto: Handle cpacf_kma() return code

[Holger Dengler]

On 10/5/26 16:14, Harald Freudenberger wrote:
> Now that cpacf_kma() returns the condition code, check it and bail out
> with -EIO on any non-zero result. In practice CC 1 and CC 2 cannot
> occur: the GCM function codes are clear-key only, and the scatterwalk
> loop ensures block-aligned chunks with LAAD/LPC set on the final call.
> However, it is a defensive improvement, not a bug fix.
> 
> Signed-off-by: Harald Freudenberger <freude@linux.ibm.com>

Reviewed-by: Holger Dengler <dengler@linux.ibm.com>

-- 
Mit freundlichen Grüßen / Kind regards
Holger Dengler

Re: [PATCH v1 1/2] s390/cpacf: Rework cpacf_kma() to return condition code

[Heiko Carstens]

On Mon, Oct 05, 2026 at 04:14:15PM +0200, Harald Freudenberger wrote:
...
> better readability. Additionally add an kmsan_unpoison_memory()
> statement to reflect the updated memory by hardware.
...
> @@ -731,12 +739,16 @@ static inline void cpacf_kma(unsigned long func, void *param, u8 *dest,
>  		"	lgr	0,%[fc]\n"
>  		"	lgr	1,%[pba]\n"
>  		"0:	.insn	rrf,%[opc] << 16,%[dst],%[src],%[aad],0\n"
> -		"	brc	1,0b"	/* handle partial completion */
> -		: [dst] "+&d" (d.pair), [src] "+&d" (s.pair),
> +		"	brc	1,0b\n"	/* handle partial completion */
> +		CC_IPM(cc)
> +		: CC_OUT(cc, cc), [dst] "+&d" (d.pair), [src] "+&d" (s.pair),
>  		  [aad] "+&d" (a.pair)
>  		: [fc] "d" (func), [pba] "d" ((unsigned long)param),
>  		  [opc] "i" (CPACF_KMA)
> -		: "cc", "memory", "0", "1");
> +		: CC_CLOBBER_LIST("memory", "0", "1"));
> +
> +	kmsan_unpoison_memory(dest, src_len - s.odd);

Since you added explicit instrumentation here it would sense so add
kasan/kcsan instrumentation by adding an instrument_write() call.

Also, given that this is in-kernel crypto, shouldn't this series go
via the crypto tree?

Re: [PATCH v1 1/2] s390/cpacf: Rework cpacf_kma() to return condition code

[Harald Freudenberger]

On 2026-10-06 10:30, Heiko Carstens wrote:
> On Mon, Oct 05, 2026 at 04:14:15PM +0200, Harald Freudenberger wrote:
> ...
>> better readability. Additionally add an kmsan_unpoison_memory()
>> statement to reflect the updated memory by hardware.
> ...
>> @@ -731,12 +739,16 @@ static inline void cpacf_kma(unsigned long func, 
>> void *param, u8 *dest,
>>  		"	lgr	0,%[fc]\n"
>>  		"	lgr	1,%[pba]\n"
>>  		"0:	.insn	rrf,%[opc] << 16,%[dst],%[src],%[aad],0\n"
>> -		"	brc	1,0b"	/* handle partial completion */
>> -		: [dst] "+&d" (d.pair), [src] "+&d" (s.pair),
>> +		"	brc	1,0b\n"	/* handle partial completion */
>> +		CC_IPM(cc)
>> +		: CC_OUT(cc, cc), [dst] "+&d" (d.pair), [src] "+&d" (s.pair),
>>  		  [aad] "+&d" (a.pair)
>>  		: [fc] "d" (func), [pba] "d" ((unsigned long)param),
>>  		  [opc] "i" (CPACF_KMA)
>> -		: "cc", "memory", "0", "1");
>> +		: CC_CLOBBER_LIST("memory", "0", "1"));
>> +
>> +	kmsan_unpoison_memory(dest, src_len - s.odd);
> 
> Since you added explicit instrumentation here it would sense so add
> kasan/kcsan instrumentation by adding an instrument_write() call.

I have no idea what you mean here...

> 
> Also, given that this is in-kernel crypto, shouldn't this series go
> via the crypto tree?

Well, this is to be negotiated. I first wanted to collect an RB 
statement.
For cpacf.h I see this in the s390 realm but the 2nd patch is clearly
in-kernel crypto. So I would have asked Herbert to have this processed
via s390 subsystem as the important changes are in cpacf.h. However,
let's first solve your "instrument_write()" call.
I was just following the other kmsan_unpoison_memory() statements
which recently were added by Ilya. I do not see any instrument_write()
in the whole cpacf.h.

Re: [PATCH v1 1/2] s390/cpacf: Rework cpacf_kma() to return condition code

[Heiko Carstens]

On Tue, Oct 06, 2026 at 03:29:28PM +0200, Harald Freudenberger wrote:
> On 2026-10-06 10:30, Heiko Carstens wrote:
> > On Mon, Oct 05, 2026 at 04:14:15PM +0200, Harald Freudenberger wrote:
> > ...
> > > better readability. Additionally add an kmsan_unpoison_memory()
> > > statement to reflect the updated memory by hardware.
> > ...
> > > @@ -731,12 +739,16 @@ static inline void cpacf_kma(unsigned long
> > > func, void *param, u8 *dest,
> > >  		"	lgr	0,%[fc]\n"
> > >  		"	lgr	1,%[pba]\n"
> > >  		"0:	.insn	rrf,%[opc] << 16,%[dst],%[src],%[aad],0\n"
> > > -		"	brc	1,0b"	/* handle partial completion */
> > > -		: [dst] "+&d" (d.pair), [src] "+&d" (s.pair),
> > > +		"	brc	1,0b\n"	/* handle partial completion */
> > > +		CC_IPM(cc)
> > > +		: CC_OUT(cc, cc), [dst] "+&d" (d.pair), [src] "+&d" (s.pair),
> > >  		  [aad] "+&d" (a.pair)
> > >  		: [fc] "d" (func), [pba] "d" ((unsigned long)param),
> > >  		  [opc] "i" (CPACF_KMA)
> > > -		: "cc", "memory", "0", "1");
> > > +		: CC_CLOBBER_LIST("memory", "0", "1"));
> > > +
> > > +	kmsan_unpoison_memory(dest, src_len - s.odd);
> > 
> > Since you added explicit instrumentation here it would sense so add
> > kasan/kcsan instrumentation by adding an instrument_write() call.

That should have been "...it would make sense to..."

> I have no idea what you mean here...

See include/linux/instrumented.h - you can the kernel about
memory accesses also kasan and kcsan. Your patch only adds
that for kmsan. See arch/s390/include/asm/fpu-insn.h for many
examples. I'm aware that such annotations are currently missing
for many of our inline asms. But since you are at it.

Up to you if you want to add it or not. Just a suggestion.

> > Also, given that this is in-kernel crypto, shouldn't this series go
> > via the crypto tree?
> 
> Well, this is to be negotiated. I first wanted to collect an RB statement.

It would be helpful if you would add information like that in the
cover-letter. This saves both of us a bit of time.

Re: [PATCH v1 1/2] s390/cpacf: Rework cpacf_kma() to return condition code

[Harald Freudenberger]

On 2026-10-06 15:39, Heiko Carstens wrote:
> On Tue, Oct 06, 2026 at 03:29:28PM +0200, Harald Freudenberger wrote:
>> On 2026-10-06 10:30, Heiko Carstens wrote:
>> > On Mon, Oct 05, 2026 at 04:14:15PM +0200, Harald Freudenberger wrote:
>> > ...
>> > > better readability. Additionally add an kmsan_unpoison_memory()
>> > > statement to reflect the updated memory by hardware.
>> > ...
>> > > @@ -731,12 +739,16 @@ static inline void cpacf_kma(unsigned long
>> > > func, void *param, u8 *dest,
>> > >  		"	lgr	0,%[fc]\n"
>> > >  		"	lgr	1,%[pba]\n"
>> > >  		"0:	.insn	rrf,%[opc] << 16,%[dst],%[src],%[aad],0\n"
>> > > -		"	brc	1,0b"	/* handle partial completion */
>> > > -		: [dst] "+&d" (d.pair), [src] "+&d" (s.pair),
>> > > +		"	brc	1,0b\n"	/* handle partial completion */
>> > > +		CC_IPM(cc)
>> > > +		: CC_OUT(cc, cc), [dst] "+&d" (d.pair), [src] "+&d" (s.pair),
>> > >  		  [aad] "+&d" (a.pair)
>> > >  		: [fc] "d" (func), [pba] "d" ((unsigned long)param),
>> > >  		  [opc] "i" (CPACF_KMA)
>> > > -		: "cc", "memory", "0", "1");
>> > > +		: CC_CLOBBER_LIST("memory", "0", "1"));
>> > > +
>> > > +	kmsan_unpoison_memory(dest, src_len - s.odd);
>> >
>> > Since you added explicit instrumentation here it would sense so add
>> > kasan/kcsan instrumentation by adding an instrument_write() call.
> 
> That should have been "...it would make sense to..."
> 
>> I have no idea what you mean here...
> 
> See include/linux/instrumented.h - you can the kernel about
> memory accesses also kasan and kcsan. Your patch only adds
> that for kmsan. See arch/s390/include/asm/fpu-insn.h for many
> examples. I'm aware that such annotations are currently missing
> for many of our inline asms. But since you are at it.
> 
> Up to you if you want to add it or not. Just a suggestion.
> 

Ok thanks. But I would prefer to have this seperate from this kma 
rework.
It would make sense to have that notations then for all the inline 
functions
in cpacf.h.

>> > Also, given that this is in-kernel crypto, shouldn't this series go
>> > via the crypto tree?
>> 
>> Well, this is to be negotiated. I first wanted to collect an RB 
>> statement.
> 
> It would be helpful if you would add information like that in the
> cover-letter. This saves both of us a bit of time.

Ok - will do. I avoided to have direct address people in the cover 
letter.
But when you are ok with that, let's run it that way next time.

Re: [PATCH v1 1/2] s390/cpacf: Rework cpacf_kma() to return condition code

[Heiko Carstens]

On Tue, Oct 06, 2026 at 04:16:42PM +0200, Harald Freudenberger wrote:
> On 2026-10-06 15:39, Heiko Carstens wrote:
> > It would be helpful if you would add information like that in the
> > cover-letter. This saves both of us a bit of time.
> 
> Ok - will do. I avoided to have direct address people in the cover letter.
> But when you are ok with that, let's run it that way next time.

Sure. Just write what you expect from whom, unless it is completely
obvious. Usually at least I apply patches if I'm on to/cc, and it is
without question code should go upstream via s390.