Re: [PATCH v4 07/27] arm64: Prepare sharing arm64 headers with s390

[Marc Zyngier]

On Fri, 10 Jul 2026 10:54:26 +0100,
Steffen Eiden <seiden@xxxxxxxxxxxxx> wrote:
> 
> On Mon, Jul 06, 2026 at 10:22:41PM +0100, Marc Zyngier wrote:
> > On Mon, 06 Jul 2026 09:52:07 +0100,
> > Steffen Eiden <seiden@xxxxxxxxxxxxx> wrote:
> > > 
> > > Prepare the sharing of arm64 headers with s390 by marking the shared
> > > regions or add a comment that the whole file is shared.
> > > The regions are marked with:
> > > 
> > >  #define ARM64_S390_COMMON
> > > 
> > >  /* insert shared definitions here */
> > > 
> > >  #endif /* ARM64_S390_COMMON */
> > > 
> > > The preprocessor symbol ARM64_S390_COMMON is always defined for arm64.
> > > Reduce the include scope where possible and beneficial for s390.
> > > No functional change.
> > > 
> > > Signed-off-by: Steffen Eiden <seiden@xxxxxxxxxxxxx>
> > > ---
> > >  arch/arm64/Makefile                  |  2 ++
> > >  arch/arm64/include/asm/brk-imm.h     |  1 +
> > >  arch/arm64/include/asm/esr.h         |  5 +++--
> > >  arch/arm64/include/asm/kvm_arm.h     |  6 ++++--
> > >  arch/arm64/include/asm/kvm_emulate.h | 19 ++++++++++++++++++
> > >  arch/arm64/include/asm/kvm_host.h    | 28 +++++++++++++++++++++++++++
> > >  arch/arm64/include/asm/kvm_mmu.h     |  6 ++++++
> > >  arch/arm64/include/asm/ptrace.h      |  9 +++++++++
> > >  arch/arm64/include/asm/sysreg.h      | 29 +++++++++++++++++++++++++---
> > >  9 files changed, 98 insertions(+), 7 deletions(-)
> > > 
> 
> ...
> 
> > >  		vcpu_set_flag((v), e);					\
> > >  	} while (0)
> > >  
> > > +#endif /* ARM64_S390_COMMON */
> > > +
> > 
> > It'd be worth looking at an initial patch reorganising this file to
> > minimise the number of such markers. This is equally valid for other
> > files in this patch.
> > 
> > [...]
> > 
> 
> So it is fine if I reorganize files a bit to reduce the number of
> markes?

Within reason, yes.

> I feared it would not. I will reorganize the files a bit then if this
> reduces the number of sections. Also if some thiings are required in a
> later series I might alreaddy share the stuff if possible if that
> reduces the number of regions.

We'll have to evaluate the amount of churn this generates, but overall
it would makes sense to keep related things together, and apply the
markers sensibly.

[...]

> > > +#ifdef ARM64_S390_COMMON
> > > +
> > >  /*
> > >   * Instructions for modifying PSTATE fields.
> > >   * As per Arm ARM for v8-A, Section "C.5.1.3 op0 == 0b00, architectural hints,
> > > @@ -91,8 +99,6 @@
> > >   */
> > >  #define pstate_field(op1, op2)		((op1) << Op1_shift | (op2) << Op2_shift)
> > >  #define PSTATE_Imm_shift		CRm_shift
> > > -#define ENCODE_PSTATE(x, r)		(0xd500401f | PSTATE_ ## r | ((!!x) << PSTATE_Imm_shift))
> > > -#define SET_PSTATE(x, r)		__emit_inst(ENCODE_PSTATE(x, r))
> > >  
> > >  #define PSTATE_PAN			pstate_field(0, 4)
> > >  #define PSTATE_UAO			pstate_field(0, 3)
> > > @@ -100,6 +106,11 @@
> > >  #define PSTATE_DIT			pstate_field(3, 2)
> > >  #define PSTATE_TCO			pstate_field(3, 4)
> > >  
> > > +#endif /* ARM64_S390_COMMON */
> > > +
> > > +#define ENCODE_PSTATE(x, r)		(0xd500401f | PSTATE_ ## r | ((!!x) << PSTATE_Imm_shift))
> > > +#define SET_PSTATE(x, r)		__emit_inst(ENCODE_PSTATE(x, r))
> > > +
> > >  #define SET_PSTATE_PAN(x)		SET_PSTATE((x), PAN)
> > >  #define SET_PSTATE_UAO(x)		SET_PSTATE((x), UAO)
> > >  #define SET_PSTATE_SSBS(x)		SET_PSTATE((x), SSBS)
> > > @@ -123,6 +134,8 @@
> > >  #define GSB_SYS_BARRIER_INSN		__SYS_BARRIER_INSN(1, 0, 12, 0, 0, 31)
> > >  #define GSB_ACK_BARRIER_INSN		__SYS_BARRIER_INSN(1, 0, 12, 0, 1, 31)
> > 
> > What is the reason to avoid dragging these definitions? They seem
> > extremely similar to the rest.
> > 
> 
> They emit arm64 instructions, dont they?

They emit instructions if you make use of them. And so do all the
PSTATE macros. Why the different treatment?


Thanks,

	M.

-- 
Jazz isn't dead. It just smells funny.