[kvm-unit-tests PATCH] x86: Fix INIT during boot

[kvm-unit-tests PATCH] x86: Fix INIT during boot

[nadav.amit]

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)
 	movl $(APIC_DEST_ALLBUT | APIC_DEST_PHYSICAL | APIC_DM_STARTUP), APIC_ICR(%rax)
 	call fwcfg_get_nb_cpus
 1:	pause
-- 
2.17.1

Re: [kvm-unit-tests PATCH] x86: Fix INIT during boot

[Sean Christopherson]

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
> 

Re: [kvm-unit-tests PATCH] x86: Fix INIT during boot

[Nadav Amit]

> 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.