| Message ID | 20240821122503.2315844-5-ayan.kumar.halder@amd.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | xen: make VMAP support in MMU system only | expand |
On 21.08.2024 14:25, Ayan Kumar Halder wrote: > From: Penny Zheng <penny.zheng@arm.com> > > Introduce CONFIG_VMAP which is selected by the architectures that use > MMU. vm_init() does not do anything if CONFIG_VMAP is not enabled. > > VMAP is widely used in ALTERNATIVE feature 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 ALTERNATIVE now > depends on VMAP. There is a mix of VMAP and HAS_VMAP throughout here. Once that's sorted I think there'll be redundancy in what is being said, which likely will want consolidating then, too. > HARDEN_BRANCH_PREDICTOR is now gated on MMU as speculative > attacks are not possible on non MMU based systems (ie Cortex-R52, R82). > See https://developer.arm.com/Arm%20Security%20Center/Speculative%20Processor%20Vulnerability. I think that increasingly this wants to be a separate change. It's entirely unrelated to what the patch's purpose is (or else the connection isn't clear to me, and also isn't being made in the description). Apart from this the code changes look okay to me now. Jan
Hi Ayan, On 21/08/2024 13:25, Ayan Kumar Halder wrote: > From: Penny Zheng <penny.zheng@arm.com> > > Introduce CONFIG_VMAP which is selected by the architectures that use > MMU. vm_init() does not do anything if CONFIG_VMAP is not enabled. > > VMAP is widely used in ALTERNATIVE feature 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 ALTERNATIVE now > depends on VMAP. > > HARDEN_BRANCH_PREDICTOR is now gated on MMU as speculative > attacks are not possible on non MMU based systems (ie Cortex-R52, R82). > See https://developer.arm.com/Arm%20Security%20Center/Speculative%20Processor%20Vulnerability. I don't have a strong opinion on whether it should be split. But I do agree this deserves a bit more explanation. Also, as I mentioned before, speculative attacks may be possible on non-MMU based systems. In fact some the Cortex-R are in the affected list... The R82 and R52 are not listed, but note: "For information about any unlisted processors please contact Arm". So how about the following explanation: "At the moment, the users of HARDEN_BRANCH_PREDICTOR requires to use the vmap() to update the exceptions vectors. While it might be possible to rework the code, it is believed that speculative attackes would be difficult to exploit on non-MMU because the software is tightly controlled. So for now make HARDEN_PREDICTOR to depend on the !MMU. > > Also took the opportunity to remove "#ifdef VMAP_VIRT_START .. endif" > from vmap.c. Instead vmap.c is compiled when HAS_VMAP is enabled. Thus, > HAS_VMAP is now enabled from x86, ppc and riscv architectures as all of > them use MMU and has VMAP_VIRT_START defined. > > Signed-off-by: Penny Zheng <penny.zheng@arm.com> > Signed-off-by: Wei Chen <wei.chen@arm.com> > Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@amd.com> With the typo pointed out by Jan: Acked-by: Julien Grall <jgrall@amazon.com> Cheers,
On 21/08/2024 22:14, Julien Grall wrote: > Hi Ayan, Hi Julien, > > On 21/08/2024 13:25, Ayan Kumar Halder wrote: >> From: Penny Zheng <penny.zheng@arm.com> >> >> Introduce CONFIG_VMAP which is selected by the architectures that use >> MMU. vm_init() does not do anything if CONFIG_VMAP is not enabled. >> >> VMAP is widely used in ALTERNATIVE feature 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 ALTERNATIVE now >> depends on VMAP. >> >> HARDEN_BRANCH_PREDICTOR is now gated on MMU as speculative >> attacks are not possible on non MMU based systems (ie Cortex-R52, R82). >> See >> https://developer.arm.com/Arm%20Security%20Center/Speculative%20Processor%20Vulnerability. > > I don't have a strong opinion on whether it should be split. But I do > agree this deserves a bit more explanation. > > Also, as I mentioned before, speculative attacks may be possible on > non-MMU based systems. In fact some the Cortex-R are in the affected > list... The R82 and R52 are not listed, but note: > > "For information about any unlisted processors please contact Arm". > > So how about the following explanation: > > "At the moment, the users of HARDEN_BRANCH_PREDICTOR requires to use > the vmap() to update the exceptions vectors. While it might be > possible to rework the code, it is believed that speculative attackes > would be difficult to exploit on non-MMU because the software is > tightly controlled. So for now make HARDEN_PREDICTOR to depend on the > !MMU. This makes sense. However, I think you mean ..... make HARDEN_BRANCH_PREDICTOR to depend on the **MMU**. > >> >> Also took the opportunity to remove "#ifdef VMAP_VIRT_START .. endif" >> from vmap.c. Instead vmap.c is compiled when HAS_VMAP is enabled. Thus, >> HAS_VMAP is now enabled from x86, ppc and riscv architectures as all of >> them use MMU and has VMAP_VIRT_START defined. >> >> Signed-off-by: Penny Zheng <penny.zheng@arm.com> >> Signed-off-by: Wei Chen <wei.chen@arm.com> >> Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@amd.com> > > With the typo pointed out by Jan: Yes, I will use HAS_VMAP. I will update the commit message as below. Let me know if this makes sense. ``` xen: make VMAP support in MMU system only Introduce CONFIG_HAS_VMAP which is selected by the architectures that use MMU. vm_init() does not do anything if CONFIG_HAS_VMAP is not enabled. HAS_VMAP is widely used in ALTERNATIVE feature to remap a range of memory with new memory attributes. Since this is highly dependent on virtual address translation, we choose to fold HAS_VMAP in MMU. And ALTERNATIVE depends on HAS_VMAP. At the moment, the users of HARDEN_BRANCH_PREDICTOR requires to use the vmap() to update the exceptions vectors. While it might be possible to rework the code, it is believed that speculative attackes would be difficult to exploit on non-MMU because the software is tightly controlled. So for now make HARDEN_BRANCH_PREDICTOR to depend on the MMU. Also took the opportunity to remove "#ifdef VMAP_VIRT_START .. endif" from vmap.c. Instead vmap.c is compiled when HAS_VMAP is enabled. Thus, HAS_VMAP is now enabled from x86, ppc and riscv architectures as all of them use MMU and has VMAP_VIRT_START defined. ``` - Ayan > > Acked-by: Julien Grall <jgrall@amazon.com> > > Cheers, >
On 22.08.2024 12:52, Ayan Kumar Halder wrote: > I will update the commit message as below. Let me know if this makes > sense. Certainly better. One more question though: > ``` > xen: make VMAP support in MMU system only > > Introduce CONFIG_HAS_VMAP which is selected by the architectures that > use MMU. vm_init() does not do anything if CONFIG_HAS_VMAP is not > enabled. > > HAS_VMAP is widely used in ALTERNATIVE feature to remap a range of > memory with new memory attributes. Since this is highly dependent on > virtual address translation, we choose to fold HAS_VMAP in MMU. And > ALTERNATIVE depends on HAS_VMAP. What is "fold HAS_VMAP in MMU"? I see no folding anywhere. My only guess is that this means to describe the "select HAS_VMAP" being added to MMU. But then why the word "fold"? Jan > At the moment, the users of HARDEN_BRANCH_PREDICTOR requires to use the > vmap() to update the exceptions vectors. While it might be possible to > rework the code, it is believed that speculative attackes would be > difficult to exploit on non-MMU because the software is tightly > controlled. So for now make HARDEN_BRANCH_PREDICTOR to depend on the > MMU. > > Also took the opportunity to remove "#ifdef VMAP_VIRT_START .. endif" > from vmap.c. Instead vmap.c is compiled when HAS_VMAP is enabled. Thus, > HAS_VMAP is now enabled from x86, ppc and riscv architectures as all of > them use MMU and has VMAP_VIRT_START defined. > > ``` > > - Ayan > >> >> Acked-by: Julien Grall <jgrall@amazon.com> >> >> Cheers, >>
Hi Jan, On 29/08/2024 08:54, Jan Beulich wrote: > On 22.08.2024 12:52, Ayan Kumar Halder wrote: >> I will update the commit message as below. Let me know if this makes >> sense. > Certainly better. One more question though: > >> ``` >> xen: make VMAP support in MMU system only >> >> Introduce CONFIG_HAS_VMAP which is selected by the architectures that >> use MMU. vm_init() does not do anything if CONFIG_HAS_VMAP is not >> enabled. >> >> HAS_VMAP is widely used in ALTERNATIVE feature to remap a range of >> memory with new memory attributes. Since this is highly dependent on >> virtual address translation, we choose to fold HAS_VMAP in MMU. And >> ALTERNATIVE depends on HAS_VMAP. > What is "fold HAS_VMAP in MMU"? I see no folding anywhere. My only guess > is that this means to describe the "select HAS_VMAP" being added to MMU. > But then why the word "fold"? Sorry, I used the incorrect word. Rather I should say "... we choose to make HAS_VMAP selected by MMU. And ALTERNATIVE depends ..." - Ayan
diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig index 21d03d9f44..323c967361 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 @@ -61,6 +61,7 @@ config PADDR_BITS config MMU def_bool y select HAS_PMAP + select HAS_VMAP source "arch/Kconfig" @@ -171,6 +172,7 @@ config ARM_SSBD config HARDEN_BRANCH_PREDICTOR bool "Harden the branch predictor against aliasing attacks" if EXPERT + depends on MMU default y help Speculation attacks against some high-performance processors rely on diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c index cb2c0a16b8..7f686d2cca 100644 --- a/xen/arch/arm/setup.c +++ b/xen/arch/arm/setup.c @@ -447,7 +447,9 @@ 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_ALTERNATIVE apply_alternatives_all(); +#endif enable_errata_workarounds(); enable_cpu_features(); diff --git a/xen/arch/ppc/Kconfig b/xen/arch/ppc/Kconfig index f6a77a8200..6db575a48d 100644 --- a/xen/arch/ppc/Kconfig +++ b/xen/arch/ppc/Kconfig @@ -2,6 +2,7 @@ config PPC def_bool y select FUNCTION_ALIGNMENT_4B select HAS_DEVICE_TREE + select HAS_VMAP config PPC64 def_bool y diff --git a/xen/arch/riscv/Kconfig b/xen/arch/riscv/Kconfig index 259eea8d3b..7a113c7774 100644 --- a/xen/arch/riscv/Kconfig +++ b/xen/arch/riscv/Kconfig @@ -3,6 +3,7 @@ config RISCV select FUNCTION_ALIGNMENT_16B select GENERIC_BUG_FRAME select HAS_DEVICE_TREE + select HAS_VMAP config RISCV_64 def_bool y diff --git a/xen/arch/x86/Kconfig b/xen/arch/x86/Kconfig index 7ef5c8bc48..1784f01aad 100644 --- a/xen/arch/x86/Kconfig +++ b/xen/arch/x86/Kconfig @@ -29,6 +29,7 @@ config X86 select HAS_PIRQ select HAS_SCHED_GRANULARITY select HAS_UBSAN + select HAS_VMAP select HAS_VPCI if HVM select NEEDS_LIBELF diff --git a/xen/common/Kconfig b/xen/common/Kconfig index 565ceda741..90268d9249 100644 --- a/xen/common/Kconfig +++ b/xen/common/Kconfig @@ -83,6 +83,9 @@ config HAS_SCHED_GRANULARITY config HAS_UBSAN bool +config HAS_VMAP + bool + config MEM_ACCESS_ALWAYS_ON bool diff --git a/xen/common/Makefile b/xen/common/Makefile index 7e66802a9e..fc52e0857d 100644 --- a/xen/common/Makefile +++ b/xen/common/Makefile @@ -52,7 +52,7 @@ obj-$(CONFIG_TRACEBUFFER) += trace.o obj-y += version.o obj-y += virtual_region.o obj-y += vm_event.o -obj-y += vmap.o +obj-$(CONFIG_HAS_VMAP) += vmap.o obj-y += vsprintf.o obj-y += wait.o obj-bin-y += warning.init.o diff --git a/xen/common/vmap.c b/xen/common/vmap.c index ced9cbf26f..47225fecc0 100644 --- a/xen/common/vmap.c +++ b/xen/common/vmap.c @@ -1,4 +1,3 @@ -#ifdef VMAP_VIRT_START #include <xen/bitmap.h> #include <xen/sections.h> #include <xen/init.h> @@ -427,4 +426,3 @@ void *_xvrealloc(void *va, size_t size, unsigned int align) return ptr; } -#endif diff --git a/xen/include/xen/vmap.h b/xen/include/xen/vmap.h index fdae37e950..c1dd7ac22f 100644 --- a/xen/include/xen/vmap.h +++ b/xen/include/xen/vmap.h @@ -141,7 +141,9 @@ void *arch_vmap_virt_end(void); /* Initialises the VMAP_DEFAULT virtual range */ static inline void vm_init(void) { +#ifdef CONFIG_HAS_VMAP vm_init_type(VMAP_DEFAULT, (void *)VMAP_VIRT_START, arch_vmap_virt_end()); +#endif } #endif /* __XEN_VMAP_H__ */