| Message ID | 20240926104113.80146-8-ardb+git@google.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | x86/xen: Drop absolute references from startup code | expand |
On Thu, 26 Sept 2024 at 12:41, Ard Biesheuvel <ardb+git@google.com> wrote: > > From: Ard Biesheuvel <ardb@kernel.org> > > Calling C code via a different mapping than it was linked at is > problematic, because the compiler assumes that RIP-relative and absolute > symbol references are interchangeable. GCC in particular may use > RIP-relative per-CPU variable references even when not using -fpic. > > So call xen_prepare_pvh() via its kernel virtual mapping on x86_64, so > that those RIP-relative references produce the correct values. This > matches the pre-existing behavior for i386, which also invokes > xen_prepare_pvh() via the kernel virtual mapping before invoking > startup_32 with paging disabled again. > > Fixes: 7243b93345f7 ("xen/pvh: Bootstrap PVH guest") > Tested-by: Jason Andryuk <jason.andryuk@amd.com> > Reviewed-by: Jason Andryuk <jason.andryuk@amd.com> > Signed-off-by: Ard Biesheuvel <ardb@kernel.org> > --- > arch/x86/platform/pvh/head.S | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/arch/x86/platform/pvh/head.S b/arch/x86/platform/pvh/head.S > index 64fca49cd88f..98ddd552885a 100644 > --- a/arch/x86/platform/pvh/head.S > +++ b/arch/x86/platform/pvh/head.S > @@ -172,7 +172,13 @@ SYM_CODE_START_LOCAL(pvh_start_xen) > movq %rbp, %rbx > subq $_pa(pvh_start_xen), %rbx > movq %rbx, phys_base(%rip) > - call xen_prepare_pvh > + > + /* Call xen_prepare_pvh() via the kernel virtual mapping */ > + leaq xen_prepare_pvh(%rip), %rax Just realized that we probably need + subq phys_base(%rip), %rax here (or grab phys_base from %rbx, but the above is more obvious and less likely to get broken in the future) > + addq $__START_KERNEL_map, %rax > + ANNOTATE_RETPOLINE_SAFE > + call *%rax > + > /* > * Clear phys_base. __startup_64 will *add* to its value, > * so reset to 0. > -- > 2.46.0.792.g87dc391469-goog >
On 2024-09-26 06:55, Ard Biesheuvel wrote: > On Thu, 26 Sept 2024 at 12:41, Ard Biesheuvel <ardb+git@google.com> wrote: >> >> From: Ard Biesheuvel <ardb@kernel.org> >> >> Calling C code via a different mapping than it was linked at is >> problematic, because the compiler assumes that RIP-relative and absolute >> symbol references are interchangeable. GCC in particular may use >> RIP-relative per-CPU variable references even when not using -fpic. >> >> So call xen_prepare_pvh() via its kernel virtual mapping on x86_64, so >> that those RIP-relative references produce the correct values. This >> matches the pre-existing behavior for i386, which also invokes >> xen_prepare_pvh() via the kernel virtual mapping before invoking >> startup_32 with paging disabled again. >> >> Fixes: 7243b93345f7 ("xen/pvh: Bootstrap PVH guest") >> Tested-by: Jason Andryuk <jason.andryuk@amd.com> >> Reviewed-by: Jason Andryuk <jason.andryuk@amd.com> >> Signed-off-by: Ard Biesheuvel <ardb@kernel.org> >> --- >> arch/x86/platform/pvh/head.S | 8 +++++++- >> 1 file changed, 7 insertions(+), 1 deletion(-) >> >> diff --git a/arch/x86/platform/pvh/head.S b/arch/x86/platform/pvh/head.S >> index 64fca49cd88f..98ddd552885a 100644 >> --- a/arch/x86/platform/pvh/head.S >> +++ b/arch/x86/platform/pvh/head.S >> @@ -172,7 +172,13 @@ SYM_CODE_START_LOCAL(pvh_start_xen) >> movq %rbp, %rbx >> subq $_pa(pvh_start_xen), %rbx >> movq %rbx, phys_base(%rip) >> - call xen_prepare_pvh >> + >> + /* Call xen_prepare_pvh() via the kernel virtual mapping */ >> + leaq xen_prepare_pvh(%rip), %rax > > Just realized that we probably need > > + subq phys_base(%rip), %rax Yes, this is necessary when phys_base is non-0. I intended to test a non-0 case yesterday, but it turns out I didn't. Re-testing, I have confirmed this subq is necessary. Thanks, Jason
diff --git a/arch/x86/platform/pvh/head.S b/arch/x86/platform/pvh/head.S index 64fca49cd88f..98ddd552885a 100644 --- a/arch/x86/platform/pvh/head.S +++ b/arch/x86/platform/pvh/head.S @@ -172,7 +172,13 @@ SYM_CODE_START_LOCAL(pvh_start_xen) movq %rbp, %rbx subq $_pa(pvh_start_xen), %rbx movq %rbx, phys_base(%rip) - call xen_prepare_pvh + + /* Call xen_prepare_pvh() via the kernel virtual mapping */ + leaq xen_prepare_pvh(%rip), %rax + addq $__START_KERNEL_map, %rax + ANNOTATE_RETPOLINE_SAFE + call *%rax + /* * Clear phys_base. __startup_64 will *add* to its value, * so reset to 0.