On Tue, Jun 04, 2024 at 05:04:40PM +0100, Pierre-Clément Tosi wrote: > On Mon, Jun 03, 2024 at 03:45:30PM +0100, Will Deacon wrote: > > On Wed, May 29, 2024 at 01:12:16PM +0100, Pierre-Clément Tosi wrote: > > > diff --git a/arch/arm64/kvm/hyp/nvhe/hyp-init.S b/arch/arm64/kvm/hyp/nvhe/hyp-init.S > > > index d859c4de06b6..b1c8977e2812 100644 > > > --- a/arch/arm64/kvm/hyp/nvhe/hyp-init.S > > > +++ b/arch/arm64/kvm/hyp/nvhe/hyp-init.S > > > @@ -5,6 +5,7 @@ > > > */ > > > > > > #include <linux/arm-smccc.h> > > > +#include <linux/cfi_types.h> > > > #include <linux/linkage.h> > > > > > > #include <asm/alternative.h> > > > @@ -267,8 +268,11 @@ SYM_CODE_END(__kvm_handle_stub_hvc) > > > > > > /* > > > * void __pkvm_init_switch_pgd(phys_addr_t pgd, void *sp, void (*fn)(void)); > > > + * > > > + * SYM_TYPED_FUNC_START() allows C to call this ID-mapped function indirectly > > > + * using a physical pointer without triggering a kCFI failure. > > > */ > > > -SYM_FUNC_START(__pkvm_init_switch_pgd) > > > +SYM_TYPED_FUNC_START(__pkvm_init_switch_pgd) > > > /* Turn the MMU off */ > > > pre_disable_mmu_workaround > > > mrs x9, sctlr_el2 > > > > I still think this last hunk should be merged with the earlier patch > > fixing up the prototype of __pkvm_init_switch_pgd(). > > Unfortunately, this is not possible because > > SYM_TYPED_FUNC_START(__pkvm_init_switch_pgd) > > makes the assembler generate an unresolved symbol for the function type, which > the compiler only generates (from the C declaration) if it compiles with kCFI so > that moving this hunk to an earlier patch results in a linker error: > > ld.lld: error: undefined symbol: __kvm_nvhe___kcfi_typeid___pkvm_init_switch_pgd > > OTOH, moving it to a later patch triggers a kCFI (runtime) panic. > > As a result, this hunk *must* be part of this patch. Argh, thanks for the explanation. I thought CONFIG_CFI_CLANG would save us here, but that's already enabled for the rest of the kernel so now I understand what you mean. > > With that: > > > > Acked-by: Will Deacon <will@xxxxxxxxxx> > > As I haven't followed your suggestion, I'll ignore this. You can keep the Ack. Cheers, Will
