Re: [PATCH 1/5] x86/pvh: Call C code via the kernel virtual mapping

[Jason Andryuk]

On 2024-09-26 06:55, Ard Biesheuvel wrote:
> On Thu, 26 Sept 2024 at 12:41, Ard Biesheuvel <ardb+git@xxxxxxxxxx> wrote:
> > From: Ard Biesheuvel <ardb@xxxxxxxxxx>
> >
> > 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@xxxxxxx>
> > Reviewed-by: Jason Andryuk <jason.andryuk@xxxxxxx>
> > Signed-off-by: Ard Biesheuvel <ardb@xxxxxxxxxx>
> > ---
> >   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