From d03df0c7e25298e2ed8646c59f2afc8f264fd5ae Mon Sep 17 00:00:00 2001 From: Vijay Dhanraj Date: Thu, 21 May 2020 13:23:14 -0700 Subject: [PATCH] HV: Fix MP Init sequence hang by adding a delay As per the BWG a delay should be provided between the INIT IPI and Startup IPI. Without the delay observe hangs on certain platforms during MP Init sequence. So Setting a delay of 10us between assert INIT IPI and Startup IPI. Also, as per SDM section 10.7 the the de-assert INIT IPI is only used for Pentium and P6 processors. This is not applicable for Pentium4 and Xeon processors so removing this sequence. Tracked-On: #4835 Signed-off-by: Vijay Dhanraj Acked-by: Eddie Dong --- hypervisor/arch/x86/cpu.c | 2 +- hypervisor/arch/x86/lapic.c | 34 +++++++++-------------------- hypervisor/include/arch/x86/lapic.h | 6 +---- 3 files changed, 12 insertions(+), 30 deletions(-) diff --git a/hypervisor/arch/x86/cpu.c b/hypervisor/arch/x86/cpu.c index d053465aa..55214e4be 100644 --- a/hypervisor/arch/x86/cpu.c +++ b/hypervisor/arch/x86/cpu.c @@ -317,7 +317,7 @@ static void start_pcpu(uint16_t pcpu_id) write_trampoline_stack_sym(pcpu_id); clac(); - send_startup_ipi(INTR_CPU_STARTUP_USE_DEST, pcpu_id, startup_paddr); + send_startup_ipi(pcpu_id, startup_paddr); /* Wait until the pcpu with pcpu_id is running and set the active bitmap or * configured time-out has expired diff --git a/hypervisor/arch/x86/lapic.c b/hypervisor/arch/x86/lapic.c index b7999a7d8..8f48186f7 100644 --- a/hypervisor/arch/x86/lapic.c +++ b/hypervisor/arch/x86/lapic.c @@ -167,50 +167,36 @@ uint32_t get_cur_lapic_id(void) return lapic_id; } -/** - * @pre cpu_startup_shorthand < INTR_CPU_STARTUP_UNKNOWN - */ void -send_startup_ipi(enum intr_cpu_startup_shorthand cpu_startup_shorthand, - uint16_t dest_pcpu_id, uint64_t cpu_startup_start_address) +send_startup_ipi(uint16_t dest_pcpu_id, uint64_t cpu_startup_start_address) { union apic_icr icr; - uint8_t shorthand; struct cpuinfo_x86 *cpu_info = get_pcpu_info(); icr.value = 0U; - icr.bits.destination_mode = INTR_LAPIC_ICR_PHYSICAL; - - if (cpu_startup_shorthand == INTR_CPU_STARTUP_USE_DEST) { - shorthand = INTR_LAPIC_ICR_USE_DEST_ARRAY; - icr.value_32.hi_32 = per_cpu(lapic_id, dest_pcpu_id); - } else { /* Use destination shorthand */ - shorthand = INTR_LAPIC_ICR_ALL_EX_SELF; - icr.value_32.hi_32 = 0U; - } + icr.value_32.hi_32 = per_cpu(lapic_id, dest_pcpu_id); /* Assert INIT IPI */ - icr.bits.shorthand = shorthand; + icr.bits.destination_mode = INTR_LAPIC_ICR_PHYSICAL; + icr.bits.shorthand = INTR_LAPIC_ICR_USE_DEST_ARRAY; icr.bits.delivery_mode = INTR_LAPIC_ICR_INIT; - icr.bits.level = INTR_LAPIC_ICR_ASSERT; - icr.bits.trigger_mode = INTR_LAPIC_ICR_LEVEL; msr_write(MSR_IA32_EXT_APIC_ICR, icr.value); /* Give 10ms for INIT sequence to complete for old processors. - * Modern processors (family == 6) don't need to wait here. + * BWG states that a delay cannot be avoided between the INIT IPI + * and first Startup IPI, so on Modern processors (family == 6) + * setting a delay value of 10us. */ if (cpu_info->family != 6U) { /* delay 10ms */ udelay(10000U); + } else { + udelay(10U); /* 10us is enough for Modern processors */ } - /* De-assert INIT IPI */ - icr.bits.level = INTR_LAPIC_ICR_DEASSERT; - msr_write(MSR_IA32_EXT_APIC_ICR, icr.value); - /* Send Start IPI with page number of secondary reset code */ icr.value_32.lo_32 = 0U; - icr.bits.shorthand = shorthand; + icr.bits.shorthand = INTR_LAPIC_ICR_USE_DEST_ARRAY; icr.bits.delivery_mode = INTR_LAPIC_ICR_STARTUP; icr.bits.vector = (uint8_t)(cpu_startup_start_address >> 12U); msr_write(MSR_IA32_EXT_APIC_ICR, icr.value); diff --git a/hypervisor/include/arch/x86/lapic.h b/hypervisor/include/arch/x86/lapic.h index ca721d892..5d4ed845a 100644 --- a/hypervisor/include/arch/x86/lapic.h +++ b/hypervisor/include/arch/x86/lapic.h @@ -131,15 +131,11 @@ void send_lapic_eoi(void); * * Send an Startup IPI to a specific cpu, to notify the cpu to start booting. * - * @param[in] cpu_startup_shorthand The startup_shorthand * @param[in] dest_pcpu_id The id of destination physical cpu * @param[in] cpu_startup_start_address The address for the dest pCPU to start running * - * @pre cpu_startup_shorthand < INTR_CPU_STARTUP_UNKNOWN */ -void send_startup_ipi(enum intr_cpu_startup_shorthand cpu_startup_shorthand, - uint16_t dest_pcpu_id, - uint64_t cpu_startup_start_address); +void send_startup_ipi(uint16_t dest_pcpu_id, uint64_t cpu_startup_start_address); /** * @brief Send an IPI to multiple pCPUs