| Message ID | 20190415191120.9472-1-nadav.amit@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [kvm-unit-tests] x86: Fix INIT during boot | expand |
On Mon, Apr 15, 2019 at 12:11:20PM -0700, nadav.amit@gmail.com wrote: > From: Nadav Amit <nadav.amit@gmail.com> > > INIT is a level event and the trigger mode should be marked as such. In > addition, the SDM says that INIT deassertion should specify the "all > including-self" shorthand. > > Signed-off-by: Nadav Amit <nadav.amit@gmail.com> > --- > x86/cstart64.S | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/x86/cstart64.S b/x86/cstart64.S > index 21db10f..57c3552 100644 > --- a/x86/cstart64.S > +++ b/x86/cstart64.S > @@ -284,8 +284,8 @@ smp_init: > mov $(sipi_end - sipi_entry), %rcx > rep/movsb > mov $APIC_DEFAULT_PHYS_BASE, %eax > - movl $(APIC_DEST_ALLBUT | APIC_DEST_PHYSICAL | APIC_DM_INIT | APIC_INT_ASSERT), APIC_ICR(%rax) > - movl $(APIC_DEST_ALLBUT | APIC_DEST_PHYSICAL | APIC_DM_INIT), APIC_ICR(%rax) > + movl $(APIC_DEST_ALLBUT | APIC_DEST_PHYSICAL | APIC_DM_INIT | APIC_INT_ASSERT | APIC_INT_LEVELTRIG), APIC_ICR(%rax) > + movl $(APIC_DEST_ALLINC | APIC_DEST_PHYSICAL | APIC_DM_INIT | APIC_INT_LEVELTRIG), APIC_ICR(%rax) Level vs. Edge is ignored for everything except the de-assert IPI, i.e. it's just as wrong to send the INIT as level triggered. Selects the trigger mode when using the INIT level de-assert delivery mode: edge (0) or level (1). It is ignored for all other delivery modes. (This flag has no meaning in Pentium 4 and Intel Xeon processors, and will always be issued as a 0.) And KVM simply ignores the de-assert IPI (when it's correctly formed). In other words, we can simply remove the malformed de-assert IPI. > movl $(APIC_DEST_ALLBUT | APIC_DEST_PHYSICAL | APIC_DM_STARTUP), APIC_ICR(%rax) > call fwcfg_get_nb_cpus > 1: pause > -- > 2.17.1 >
> On Apr 15, 2019, at 1:17 PM, Sean Christopherson <sean.j.christopherson@intel.com> wrote: > > On Mon, Apr 15, 2019 at 12:11:20PM -0700, nadav.amit@gmail.com wrote: >> From: Nadav Amit <nadav.amit@gmail.com> >> >> INIT is a level event and the trigger mode should be marked as such. In >> addition, the SDM says that INIT deassertion should specify the "all >> including-self" shorthand. >> >> Signed-off-by: Nadav Amit <nadav.amit@gmail.com> >> --- >> x86/cstart64.S | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/x86/cstart64.S b/x86/cstart64.S >> index 21db10f..57c3552 100644 >> --- a/x86/cstart64.S >> +++ b/x86/cstart64.S >> @@ -284,8 +284,8 @@ smp_init: >> mov $(sipi_end - sipi_entry), %rcx >> rep/movsb >> mov $APIC_DEFAULT_PHYS_BASE, %eax >> - movl $(APIC_DEST_ALLBUT | APIC_DEST_PHYSICAL | APIC_DM_INIT | APIC_INT_ASSERT), APIC_ICR(%rax) >> - movl $(APIC_DEST_ALLBUT | APIC_DEST_PHYSICAL | APIC_DM_INIT), APIC_ICR(%rax) >> + movl $(APIC_DEST_ALLBUT | APIC_DEST_PHYSICAL | APIC_DM_INIT | APIC_INT_ASSERT | APIC_INT_LEVELTRIG), APIC_ICR(%rax) >> + movl $(APIC_DEST_ALLINC | APIC_DEST_PHYSICAL | APIC_DM_INIT | APIC_INT_LEVELTRIG), APIC_ICR(%rax) > > Level vs. Edge is ignored for everything except the de-assert IPI, i.e. it's > just as wrong to send the INIT as level triggered. > > Selects the trigger mode when using the INIT level de-assert delivery mode: > edge (0) or level (1). It is ignored for all other delivery modes. (This flag > has no meaning in Pentium 4 and Intel Xeon processors, and will always be > issued as a 0.) > > And KVM simply ignores the de-assert IPI (when it's correctly formed). In > other words, we can simply remove the malformed de-assert IPI. Thanks for the correction. I’ll send a v2.
diff --git a/x86/cstart64.S b/x86/cstart64.S index 21db10f..57c3552 100644 --- a/x86/cstart64.S +++ b/x86/cstart64.S @@ -284,8 +284,8 @@ smp_init: mov $(sipi_end - sipi_entry), %rcx rep/movsb mov $APIC_DEFAULT_PHYS_BASE, %eax - movl $(APIC_DEST_ALLBUT | APIC_DEST_PHYSICAL | APIC_DM_INIT | APIC_INT_ASSERT), APIC_ICR(%rax) - movl $(APIC_DEST_ALLBUT | APIC_DEST_PHYSICAL | APIC_DM_INIT), APIC_ICR(%rax) + movl $(APIC_DEST_ALLBUT | APIC_DEST_PHYSICAL | APIC_DM_INIT | APIC_INT_ASSERT | APIC_INT_LEVELTRIG), APIC_ICR(%rax) + movl $(APIC_DEST_ALLINC | APIC_DEST_PHYSICAL | APIC_DM_INIT | APIC_INT_LEVELTRIG), APIC_ICR(%rax) movl $(APIC_DEST_ALLBUT | APIC_DEST_PHYSICAL | APIC_DM_STARTUP), APIC_ICR(%rax) call fwcfg_get_nb_cpus 1: pause