Re: [PATCH v1 13/26] s390: Introduce read/write ARM sysreg instructions

[Janosch Frank <frankja@linux.ibm.com>]

On 5/29/26 17:55, Steffen Eiden wrote:
> Introduce Extract Arm System Register and Store Arm System Register to
> enable s390 hosts to read and write system registers for arm64 guests.
> The new instructions use the new RIE_H instruction format. Add assembler
> macros to create instructions in RIE_H format manually. Add Support for
> disassembling the new instructions.
> 
> Co-developed-by: Andreas Grapentin <gra@linux.ibm.com>
> Signed-off-by: Andreas Grapentin <gra@linux.ibm.com>
> Signed-off-by: Steffen Eiden <seiden@linux.ibm.com>
> ---
>   arch/s390/include/asm/sae-asm.h | 48 +++++++++++++++++++++++++++
>   arch/s390/include/asm/sae.h     | 58 +++++++++++++++++++++++++++++++++
>   arch/s390/kernel/dis.c          |  1 +
>   arch/s390/tools/opcodes.txt     |  2 ++
>   4 files changed, 109 insertions(+)
>   create mode 100644 arch/s390/include/asm/sae-asm.h
> 
> diff --git a/arch/s390/include/asm/sae-asm.h b/arch/s390/include/asm/sae-asm.h
> new file mode 100644
> index 000000000000..d81ed89eb4ed
> --- /dev/null
> +++ b/arch/s390/include/asm/sae-asm.h
> @@ -0,0 +1,48 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef __ASM_S390_SAE_ASM_H
> +#define __ASM_S390_SAE_ASM_H
> +
> +#ifdef __ASSEMBLER__
> +
> +.macro	GPR_NUM	opd gr
> +	\opd = 255
> +	.irp rs,0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15
> +	.ifc \gr,%r\rs
> +		\opd = \rs
> +	.endif
> +	.endr
> +	.if \opd == 255
> +		\opd = \gr
> +	.endif
> +.endm
> +
> +/*
> + * RIE_H - RIE-h instruction format
> + *
> + * RIE-h format: <insn> R1, R3, I2, M4
> + *    +--------+----+----+----+-----------------+----+--------+
> + *    | OpCode | R1 |////| R3 |        I2       | M4 | Opcode |
> + *    +--------+----+----+----+-----------------+----+--------+
> + *    0        8    12   16   20                36   40      47
> + */
> +.macro RIE_H	opc, gr1, gr3, imm2, m4
> +	GPR_NUM	r1, \gr1
> +	GPR_NUM	r3, \gr3
> +	.byte	(\opc & 0xff00) >> 8
> +	.byte	r1 << 4
> +	.byte	(r3 << 4) | ((\imm2 & 0xf000) >> 12)
> +	.byte	((\imm2 & 0x0ff0) >> 4)
> +	.byte	((\imm2 & 0x000f) << 4) | (\m4 & 0xf)
> +	.byte	\opc & 0xff
> +.endm
> +
> +.macro SASR r1, r3, i2, m4
> +	RIE_H 0xed99, \r1, \r3, \i2, \m4,
> +.endm
> +
> +.macro EASR r1, r3, i2, m4
> +	RIE_H 0xed9b, \r1, \r3, \i2, \m4,
> +.endm
> +
> +#endif /* __ASSEMBLER__ */
> +#endif /* __ASM_S390_SAE_ASM_H */
> diff --git a/arch/s390/include/asm/sae.h b/arch/s390/include/asm/sae.h
> index fe010a1a7729..1d9a16b91b23 100644
> --- a/arch/s390/include/asm/sae.h
> +++ b/arch/s390/include/asm/sae.h
> @@ -4,6 +4,7 @@
>   
>   #include "linux/linkage.h"
>   #include <linux/types.h>
> +#include <asm/sae-asm.h>
>   
>   /* defined in arch/s390/kernel/entry.S */
>   asmlinkage int __sae64a(phys_addr_t sae_block_phys);
> @@ -12,6 +13,12 @@ asmlinkage int __sae64a(phys_addr_t sae_block_phys);
>   #include <linux/io.h>
>   #include <asm/kvm_host_arm64_types.h>
>   
> +asm(".include \"asm/sae-asm.h\"\n");
> +
> +#define _SAE_ASR_REG_SHIFT	5
> +#define SASR_FLAG_INITIALIZED  0x8
> +#define EASR_FLAG_SA           0x8

s/SA/SAVE_AREA/ or SAVEAREA

I think both might fit.

> +
>   /**
>    * __sae64a() - Start Arm Execution
>    */
> @@ -20,6 +27,57 @@ static inline void sae64a(struct kvm_sae_block *sae_block)
>   	__sae64a(virt_to_phys(sae_block));
>   }
>   
> +/**
> + * sasr() - Set Arm System Register
> + * @arm_reg: ARM system register identifier; compile-time constant
> + * @val: Value to set
> + * @save_area: Pointer to SAE save area
> + * @flags: Operation flags; compile-time constant
> + *
> + * Sets an ARM system register value.
> + */
> +static __always_inline void sasr(unsigned int arm_reg, u64 val,
> +				 struct kvm_sae_save_area *save_area,
> +				 u64 flags)

m4 is 4 bits in length, any reason why we use a u64 here?
Same for easr.

> +{
> +	struct kvm_sae_save_area *sdo = (void *)save_area->sdo;
> +	u16 reg = arm_reg >> _SAE_ASR_REG_SHIFT;
> +
> +	asm volatile (
> +		"	SASR	%[r1],%[r3],%[i2],%[m4]\n"
> +		: "+m" (*save_area), "+m" (*sdo)
> +		: [r1] "d" (val),
> +		  [r3] "a" (save_area), [i2] "K" (reg), [m4] "I" (flags)
> +	);
> +}