| Message ID | 20240802121443.1531693-3-ayan.kumar.halder@amd.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | xen: arm: Split MMU code in preparation for MPU work (part 2) | expand |
On 02.08.2024 14:14, Ayan Kumar Halder wrote:> --- a/xen/include/xen/vmap.h > +++ b/xen/include/xen/vmap.h > @@ -138,10 +138,16 @@ static inline void iounmap(void __iomem *va) > /* Pointer to 1 octet past the end of the VMAP_DEFAULT virtual area */ > void *arch_vmap_virt_end(void); > > +#ifdef CONFIG_MMU > /* Initialises the VMAP_DEFAULT virtual range */ > static inline void vm_init(void) > { > vm_init_type(VMAP_DEFAULT, (void *)VMAP_VIRT_START, arch_vmap_virt_end()); > } > +#else > +static inline void vm_init(void) > +{ > +} > +#endif Imo in such a case the #ifdef wants to be put inside the existing function, avoiding the need for a 2nd instance. Jan
Hi, On 02/08/2024 13:14, Ayan Kumar Halder wrote: > From: Penny Zheng <penny.zheng@arm.com> > > VMAP is widely used in ALTERNATIVE feature, CPUERRATA feature, etc to > remap a range of memory with new memory attributes. Since this is > highly dependent on virtual address translation, we choose to fold VMAP > in MMU system. > > In this patch, we introduce a new Kconfig CONFIG_HAS_VMAP, and make it > only support in MMU system on ARM architecture. And we make features > like ALTERNATIVE, CPUERRATA, etc, now depend on VMAP. While I agree that alternative should depend on VMAP (for now), I feel this is incorrect for CPUERRATA. Very likely, you will need to deal with them on the MPU. Before making any suggestion, would you be able to clarify how you envision to deal with errata? Will they be selected at built time or boot time? Cheers,
On 02/08/2024 14:27, Julien Grall wrote: > Hi, Hi Julien, > > On 02/08/2024 13:14, Ayan Kumar Halder wrote: >> From: Penny Zheng <penny.zheng@arm.com> >> >> VMAP is widely used in ALTERNATIVE feature, CPUERRATA feature, etc to >> remap a range of memory with new memory attributes. Since this is >> highly dependent on virtual address translation, we choose to fold VMAP >> in MMU system. >> >> In this patch, we introduce a new Kconfig CONFIG_HAS_VMAP, and make it >> only support in MMU system on ARM architecture. And we make features >> like ALTERNATIVE, CPUERRATA, etc, now depend on VMAP. > > While I agree that alternative should depend on VMAP (for now), I feel > this is incorrect for CPUERRATA. Very likely, you will need to deal > with them on the MPU. > > Before making any suggestion, would you be able to clarify how you > envision to deal with errata? Will they be selected at built time or > boot time? TBH, I hadn't thought that through. I am thinking about selecting them at built time (like it is been done for Arm64 cpus). However given that there are lesser number of MPU cpus (only R52 and R82) compared to MMU ones, could there be a simpler approach. I am open to any suggestions you have. Also, can we disable the CPUERRATA on MPU until we add support for the first errata ? - Ayan > > Cheers, >
Hi Ayan, On 05/08/2024 11:44, Ayan Kumar Halder wrote: > > On 02/08/2024 14:27, Julien Grall wrote: >> Hi, > Hi Julien, >> >> On 02/08/2024 13:14, Ayan Kumar Halder wrote: >>> From: Penny Zheng <penny.zheng@arm.com> >>> >>> VMAP is widely used in ALTERNATIVE feature, CPUERRATA feature, etc to >>> remap a range of memory with new memory attributes. Since this is >>> highly dependent on virtual address translation, we choose to fold VMAP >>> in MMU system. >>> >>> In this patch, we introduce a new Kconfig CONFIG_HAS_VMAP, and make it >>> only support in MMU system on ARM architecture. And we make features >>> like ALTERNATIVE, CPUERRATA, etc, now depend on VMAP. >> >> While I agree that alternative should depend on VMAP (for now), I feel >> this is incorrect for CPUERRATA. Very likely, you will need to deal >> with them on the MPU. >> >> Before making any suggestion, would you be able to clarify how you >> envision to deal with errata? Will they be selected at built time or >> boot time? > > TBH, I hadn't thought that through. I am thinking about selecting them > at built time (like it is been done for Arm64 cpus). To clarify, on arm64, the decision to enable the bulk of mitigation is done at runtime. The Kconfig is just to remove it from the binary if you you are not targeting such HW. With that in mind... > > However given that there are lesser number of MPU cpus (only R52 and > R82) compared to MMU ones, could there be a simpler approach. ... do you mean always an errata enabled in the Kconfig would be always mitigate (IOW no runtime selection)? > > I am open to any suggestions you have. > > Also, can we disable the CPUERRATA on MPU until we add support for the > first errata ? So I have looked at the code and it is rather unclear why you actually need to disable CPU Errata. Can you clarify what would happen if cpuerrata is always built *and* CONFIG_ARM64_HARDEN_BRANCH_PREDICATOR is gated by HAS_VMAP? I am assuming we will not need the branch predictor hardening on Cortex-R (at least this is the implication from [1]). Now regarding alternative, at least for arm64 this is used outside of the errata. Not critical right now, but at some point you will want to re-enable alternatives. Cheers, [1] https://developer.arm.com/Arm%20Security%20Center/Speculative%20Processor%20Vulnerability
diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig index 21d03d9f44..c8d417298c 100644 --- a/xen/arch/arm/Kconfig +++ b/xen/arch/arm/Kconfig @@ -12,7 +12,7 @@ config ARM_64 config ARM def_bool y select FUNCTION_ALIGNMENT_4B - select HAS_ALTERNATIVE + select HAS_ALTERNATIVE if HAS_VMAP select HAS_DEVICE_TREE select HAS_PASSTHROUGH select HAS_UBSAN @@ -58,9 +58,13 @@ config PADDR_BITS default 40 if ARM_PA_BITS_40 default 48 if ARM_64 +config HAS_VMAP + def_bool y + config MMU def_bool y select HAS_PMAP + select HAS_VMAP source "arch/Kconfig" diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile index 45dc29ea53..6882814d38 100644 --- a/xen/arch/arm/Makefile +++ b/xen/arch/arm/Makefile @@ -11,7 +11,7 @@ obj-$(CONFIG_HAS_VPCI) += vpci.o obj-$(CONFIG_HAS_ALTERNATIVE) += alternative.o obj-y += bootfdt.init.o -obj-y += cpuerrata.o +obj-$(CONFIG_HAS_VMAP) += cpuerrata.o obj-y += cpufeature.o obj-y += decode.o obj-y += device.o diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c index 0c2fdaceaf..9d34ac7f64 100644 --- a/xen/arch/arm/setup.c +++ b/xen/arch/arm/setup.c @@ -804,11 +804,13 @@ void asmlinkage __init start_xen(unsigned long boot_phys_offset, nr_cpu_ids = smp_get_max_cpus(); printk(XENLOG_INFO "SMP: Allowing %u CPUs\n", nr_cpu_ids); +#ifdef CONFIG_HAS_VMAP /* * Some errata relies on SMCCC version which is detected by psci_init() * (called from smp_init_cpus()). */ check_local_cpu_errata(); +#endif check_local_cpu_features(); @@ -879,8 +881,10 @@ void asmlinkage __init start_xen(unsigned long boot_phys_offset, * It needs to be called after do_initcalls to be able to use * stop_machine (tasklets initialized via an initcall). */ +#ifdef CONFIG_HAS_VMAP apply_alternatives_all(); enable_errata_workarounds(); +#endif enable_cpu_features(); /* Create initial domain 0. */ diff --git a/xen/arch/arm/smpboot.c b/xen/arch/arm/smpboot.c index 04e363088d..999afc028e 100644 --- a/xen/arch/arm/smpboot.c +++ b/xen/arch/arm/smpboot.c @@ -406,7 +406,9 @@ void asmlinkage start_secondary(void) local_abort_enable(); +#ifdef CONFIG_HAS_VMAP check_local_cpu_errata(); +#endif check_local_cpu_features(); printk(XENLOG_DEBUG "CPU %u booted.\n", smp_processor_id()); diff --git a/xen/include/xen/vmap.h b/xen/include/xen/vmap.h index fdae37e950..84797acfc0 100644 --- a/xen/include/xen/vmap.h +++ b/xen/include/xen/vmap.h @@ -138,10 +138,16 @@ static inline void iounmap(void __iomem *va) /* Pointer to 1 octet past the end of the VMAP_DEFAULT virtual area */ void *arch_vmap_virt_end(void); +#ifdef CONFIG_MMU /* Initialises the VMAP_DEFAULT virtual range */ static inline void vm_init(void) { vm_init_type(VMAP_DEFAULT, (void *)VMAP_VIRT_START, arch_vmap_virt_end()); } +#else +static inline void vm_init(void) +{ +} +#endif #endif /* __XEN_VMAP_H__ */