xen: fix setting vcpu id for APs

[royger]

Summary

So that it's set before calling xen_setup_cpus (which happens at
SI_SUB_SMP-1).

This requires moving the setup of the vcpu id for the APs before
calling into init_secondary_tail, because that part of the AP
initialization happens at SI_SUB_SMP.

Reported by: cperciva
Sponsored by: Citrix Systems R&D

Test Plan

Can you test this on EC2? Thanks!

[cperciva]

Hmm, I agree that it looks like this *should* fix the problem, but I'm still seeing the instance hang waiting for TLB shootdown IPIs to complete. Maybe there's something else going on here.

[royger]

• Remove xenpv_set_ids.
• Make sure acpi_id is set when starting APs.

[cperciva]

Still doesn't work. We have a chicken-and-egg problem between launching CPUs and recording their IDs, it seems...

sys/x86/acpica/madt.c

762 ↗

(On Diff #47688)

Alas, this doesn't work either. madt_set_ids uses CPU_FOREACH, which iterates through the all_cpus mask, which isn't set until after native_start_all_aps finishes launching an AP.

sys/x86/xen/pvcpu_enum.c

262 ↗

(On Diff #47688)

Is it correct to nuke this completely? I assumed that xen_hvm_cpu_init would only set vcpu_id when running in HVM and so we would still need xenpv_set_ids for PV?

[cperciva]

I wonder, can we make this work by changing the lines

PCPU_SET(vcpu_id, (regs[0] & XEN_HVM_CPUID_VCPU_ID_PRESENT) ?
       regs[1] : PCPU_GET(acpi_id));

in xen_hvm_cpu_init to

PCPU_SET(vcpu_id, (regs[0] & XEN_HVM_CPUID_VCPU_ID_PRESENT) ?
       regs[1] : -1);

and then resurrecting xen_set_vcpu_id but having it only set vcpu_id if it is currently set to -1?

That way if we have XEN_HVM_CPUID_VCPU_ID_PRESENT we'll get vcpu_id from there (and it doesn't matter if that runs before acpi_id is set) and if we don't then we'll fill in the right value slightly later (after acpi_id is set, but we don't need to be running on the AP in question at that point).

[cperciva]

Hmm, I guess filling in vcpu_id after xen_hvm_cpu_init returns won't work, since we use that value in the "Set the vCPU info." code at the end of that function.

Can the vCPU info be set in a CPU_FOREACH loop in a later SYSINIT instead?

[royger]

In D17013#363424, @cperciva wrote:

I wonder, can we make this work by changing the lines

PCPU_SET(vcpu_id, (regs[0] & XEN_HVM_CPUID_VCPU_ID_PRESENT) ?
       regs[1] : PCPU_GET(acpi_id));

in xen_hvm_cpu_init to

PCPU_SET(vcpu_id, (regs[0] & XEN_HVM_CPUID_VCPU_ID_PRESENT) ?
       regs[1] : -1);

and then resurrecting xen_set_vcpu_id but having it only set vcpu_id if it is currently set to -1?

That way if we have XEN_HVM_CPUID_VCPU_ID_PRESENT we'll get vcpu_id from there (and it doesn't matter if that runs before acpi_id is set) and if we don't then we'll fill in the right value slightly later (after acpi_id is set, but we don't need to be running on the AP in question at that point).

xen_hvm_cpu_init is also called on resume from suspension, so we would have to add a guard in order to avoid setting vcpu_id to -1 on resume.

[royger]

sys/x86/xen/pvcpu_enum.c

262 ↗

(On Diff #47688)

No, xen_hvm_cpu_init is called by all guest types, and PV guests (without ACPI) will get the vcpu_id from the cpuid instruction. All the Xen versions that can run FreeBSD as PV will expose the vcpu_id through the cpuid leaf.

[royger]

• Bind PV IPI event channels once vcpu_id is set.

[cperciva]

Awesome, this fixes the boot on all the EC2 instances I've tried. Please commit!