Hi Ayan,
> On 7 Apr 2025, at 15:29, Ayan Kumar Halder <ayank...@amd.com> wrote: > > Hi Luca, > > On 04/04/2025 10:06, Luca Fancellu wrote: >> Hi Ayan, >> >>> On 3 Apr 2025, at 18:12, Ayan Kumar Halder <ayan.kumar.hal...@amd.com> >>> wrote: >>> >>> Added a new file prepare_xen_region.inc to hold the common earlyboot MPU >>> regions >>> configurations across arm64 and arm32. >>> >>> prepare_xen_region, fail_insufficient_regions() will be used by both arm32 >>> and >>> arm64. Thus, they have been moved to prepare_xen_region.inc. >>> >>> enable_secondary_cpu_mm() is a stub which is moved to >>> prepare_xen_region.inc as >>> SMP is currently not supported for MPU. >>> >>> *_PRBAR are moved to arm64/sysregs.h. >>> *_PRLAR are moved to prepare_xen_region.inc as they are common between arm32 >>> and arm64. >>> >>> Introduce WRITE_SYSREG_ASM to write to the system registers from the common >>> asm >>> file. >>> >>> Signed-off-by: Ayan Kumar Halder <ayan.kumar.hal...@amd.com> >>> --- >> The split for the common code seems ok to me, but this patch is introducing >> an issue for the arm64 compilation, > Sorry, I moved something at the last moment without testing. :( >> I’ve done an experiment and with these changes I’m able to compile both, but >> feel free to ignore if it’s no what you >> had in mind. > The change looks good. However, ... >> >> diff --git a/xen/arch/arm/include/asm/arm32/sysregs.h >> b/xen/arch/arm/include/asm/arm32/sysregs.h >> index 22871999afb3..a90d1610a155 100644 >> --- a/xen/arch/arm/include/asm/arm32/sysregs.h >> +++ b/xen/arch/arm/include/asm/arm32/sysregs.h >> @@ -20,6 +20,13 @@ >> * uses r0 as a placeholder register. */ >> #define CMD_CP32(name...) "mcr " __stringify(CP32(r0, name)) ";" >> +#define REGION_TEXT_PRBAR 0x18 /* SH=11 AP=10 XN=0 */ >> +#define REGION_RO_PRBAR 0x1D /* SH=11 AP=10 XN=1 */ >> +#define REGION_DATA_PRBAR 0x19 /* SH=11 AP=00 XN=1 */ >> +#define REGION_DEVICE_PRBAR 0x11 /* SH=10 AP=00 XN=1 */ >> + >> +#define WRITE_SYSREG_ASM(v, name) mcr CP32(v, name) >> + >> #ifndef __ASSEMBLY__ >> /* C wrappers */ >> diff --git a/xen/arch/arm/include/asm/cpregs.h >> b/xen/arch/arm/include/asm/cpregs.h >> index 6019a2cbdd89..b909adc102a5 100644 >> --- a/xen/arch/arm/include/asm/cpregs.h >> +++ b/xen/arch/arm/include/asm/cpregs.h >> @@ -1,10 +1,6 @@ >> #ifndef __ASM_ARM_CPREGS_H >> #define __ASM_ARM_CPREGS_H >> -#ifdef CONFIG_MPU >> -#include <asm/mpu/cpregs.h> >> -#endif >> - >> /* >> * AArch32 Co-processor registers. >> * >> @@ -502,6 +498,12 @@ >> #define MVFR0_EL1 MVFR0 >> #define MVFR1_EL1 MVFR1 >> #define MVFR2_EL1 MVFR2 >> +#define HMPUIR p15,4,c0,c0,4 >> +#define HPRSELR p15,4,c6,c2,1 >> +#define PRBAR_EL2 p15,4,c6,c3,0 >> +#define PRLAR_EL2 p15,4,c6,c8,1 >> +#define MPUIR_EL2 HMPUIR >> +#define PRSELR_EL2 HPRSELR > > Considering that there will be lots of arm32 MPU specific registers, do you > want to move them to mpu/cpregs.h ? > > That would be my style preference. I don’t have any strong opinion so it’s fine if you want them in cpregs.h. Cheers, Luca
