On Fri, Jan 21, 2022 at 3:23 PM Sean Christopherson <seanjc@xxxxxxxxxx> wrote:
>
> Force use of xAPIC to retrieve the APIC ID during TSS setup to fix an
> issue where an AP can switch apic_ops to point at x2apic_ops before
> setup_tss() completes, leading to a #GP and triple fault due to trying
> to read an x2APIC MSR without x2APIC being enabled.
>
> A future patch will make apic_ops a per-cpu pointer, but that's not of
> any help for 32-bit, which uses the APIC ID to determine the GS selector,
> i.e. 32-bit KUT has a chicken-and-egg problem. All setup_tss() callers
> ensure the local APIC is in xAPIC mode, so just force use of xAPIC in
> this case.
>
> Fixes: 7e33895 ("x86: Move 32-bit GDT and TSS to desc.c")
> Fixes: dbd3800 ("x86: Move 64-bit GDT and TSS to desc.c")
FYI there was recently a report [1] to the KVM mailing list that Fixes
tags should use at least 12 characters for hashes. So I guess this
should be:
Fixes: 7e33895d3232 ("x86: Move 32-bit GDT and TSS to desc.c")
Fixes: dbd380049042 ("x86: Move 64-bit GDT and TSS to desc.c")
[1] https://lore.kernel.org/kvm/20220121081432.5b671602@xxxxxxxxxxxxxxxx/
> Signed-off-by: Sean Christopherson <seanjc@xxxxxxxxxx>
> ---
> lib/x86/apic.c | 5 +++++
> lib/x86/apic.h | 2 ++
> lib/x86/setup.c | 4 ++--
> 3 files changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/lib/x86/apic.c b/lib/x86/apic.c
> index da8f3013..b404d580 100644
> --- a/lib/x86/apic.c
> +++ b/lib/x86/apic.c
> @@ -48,6 +48,11 @@ static uint32_t xapic_id(void)
> return xapic_read(APIC_ID) >> 24;
> }
>
> +uint32_t pre_boot_apic_id(void)
> +{
> + return xapic_id();
> +}
> +
> static const struct apic_ops xapic_ops = {
> .reg_read = xapic_read,
> .reg_write = xapic_write,
> diff --git a/lib/x86/apic.h b/lib/x86/apic.h
> index c4821716..7844324b 100644
> --- a/lib/x86/apic.h
> +++ b/lib/x86/apic.h
> @@ -53,6 +53,8 @@ bool apic_read_bit(unsigned reg, int n);
> void apic_write(unsigned reg, uint32_t val);
> void apic_icr_write(uint32_t val, uint32_t dest);
> uint32_t apic_id(void);
> +uint32_t pre_boot_apic_id(void);
> +
>
> int enable_x2apic(void);
> void disable_apic(void);
> diff --git a/lib/x86/setup.c b/lib/x86/setup.c
> index bbd34682..64217add 100644
> --- a/lib/x86/setup.c
> +++ b/lib/x86/setup.c
> @@ -109,7 +109,7 @@ unsigned long setup_tss(u8 *stacktop)
> u32 id;
> tss64_t *tss_entry;
>
> - id = apic_id();
> + id = pre_boot_apic_id();
>
> /* Runtime address of current TSS */
> tss_entry = &tss[id];
> @@ -129,7 +129,7 @@ unsigned long setup_tss(u8 *stacktop)
> u32 id;
> tss32_t *tss_entry;
>
> - id = apic_id();
> + id = pre_boot_apic_id();
>
> /* Runtime address of current TSS */
> tss_entry = &tss[id];
> --
> 2.35.0.rc0.227.g00780c9af4-goog
>
