[prev in list] [next in list] [prev in thread] [next in thread] 

List:       hurd-bug
Subject:    Re: [PATCH v2 gnumach] apic: Use cpuid to read the apic id for speed OFF TOPIC PRAISE
From:       Joshua Branson <jbranso () dismail ! de>
Date:       2023-08-23 15:22:52
Message-ID: 87o7ixye9f.fsf_-_ () dismail ! de
[Download RAW message or body]

Samuel Thibault <samuel.thibault@gnu.org> writes:

> Applied, thanks!

Congrats Damien!  I don't massively understand this patch, but from what
I have read on the irc logs this improves some performance benchmark by
an order of magnitude.  That's pretty stellar!

>
> Damien Zammit, le mer. 16 août 2023 01:44:50 +0000, a ecrit:
>> ---
>>  i386/i386/apic.c       | 11 +++++------
>>  i386/i386/cpu_number.h | 20 +++++++++++++++++++-
>>  i386/i386/mp_desc.c    |  3 +--
>>  x86_64/locore.S        |  6 +++---
>>  4 files changed, 28 insertions(+), 12 deletions(-)
>> 
>> diff --git a/i386/i386/apic.c b/i386/i386/apic.c
>> index 2bb8e3f1..3a51f506 100644
>> --- a/i386/i386/apic.c
>> +++ b/i386/i386/apic.c
>> @@ -185,7 +185,11 @@ apic_get_num_ioapics(void)
>>  int
>>  apic_get_current_cpu(void)
>>  {
>> -    return (lapic->apic_id.r >> 24) & 0xff;
>> +    unsigned int eax, ebx, ecx, edx;
>> +    eax = 1;
>> +    ecx = 0;
>> +    cpuid(eax, ebx, ecx, edx);
>> +    return (ebx >> 24);
>>  }
>>  
>>  
>> @@ -295,11 +299,6 @@ lapic_enable(void)
>>      cpu_intr_save(&flags);
>>  
>>      apic_id = apic_get_current_cpu();
>> -    if (apic_id < 0)
>> -      {
>> -        printf("apic_get_current_cpu() failed, assuming BSP\n");
>> -        apic_id = 0;
>> -      }
>>  
>>      dummy = lapic->dest_format.r;
>>      lapic->dest_format.r = 0xffffffff;		/* flat model */
>> diff --git a/i386/i386/cpu_number.h b/i386/i386/cpu_number.h
>> index c00896e8..df086370 100644
>> --- a/i386/i386/cpu_number.h
>> +++ b/i386/i386/cpu_number.h
>> @@ -39,12 +39,30 @@
>>  #define	CX(addr, reg)	addr(,reg,8)
>>  #endif
>>  
>> -#define	CPU_NUMBER(reg)	\
>> +#define	CPU_NUMBER_NO_STACK(reg)	\
>>  	movl	%cs:lapic, reg		;\
>>  	movl	%cs:APIC_ID(reg), reg	;\
>>  	shrl	$24, reg		;\
>>  	movl	%cs:CX(cpu_id_lut, reg), reg	;\
>>  
>> +/* Never call CPU_NUMBER(%esi) */
>> +#define CPU_NUMBER(reg)		\
>> +	pushl	%esi		;\
>> +	pushl	%eax		;\
>> +	pushl	%ebx		;\
>> +	pushl	%ecx		;\
>> +	pushl	%edx		;\
>> +	movl	$1, %eax	;\
>> +	cpuid			;\
>> +	shrl	$24, %ebx	;\
>> +	movl	%cs:CX(cpu_id_lut, %ebx), %esi	;\
>> +	popl	%edx		;\
>> +	popl	%ecx		;\
>> +	popl	%ebx		;\
>> +	popl	%eax		;\
>> +	movl	%esi, reg	;\
>> +	popl	%esi		;\
>> +
>>  #ifndef __ASSEMBLER__
>>  #include "kern/cpu_number.h"
>>  int cpu_number(void);
>> diff --git a/i386/i386/mp_desc.c b/i386/i386/mp_desc.c
>> index 88fbb50a..f1a1f989 100644
>> --- a/i386/i386/mp_desc.c
>> +++ b/i386/i386/mp_desc.c
>> @@ -275,8 +275,7 @@ cpu_setup(int cpu)
>>  void
>>  cpu_ap_main()
>>  {
>> -    unsigned apic_id = (((ApicLocalUnit*)phystokv(lapic_addr))->apic_id.r >> 24) & 0xff;
>> -    int cpu = apic_get_cpu_kernel_id(apic_id);
>> +    int cpu = cpu_number();
>>  
>>      do {
>>  	cpu_pause();
>> diff --git a/x86_64/locore.S b/x86_64/locore.S
>> index a330d56b..c75feb23 100644
>> --- a/x86_64/locore.S
>> +++ b/x86_64/locore.S
>> @@ -1171,7 +1171,7 @@ syscall_entry_2:
>>  	movq	%rdx,R_CS(%rsp)		/* fix cs */
>>  	movq	%rbx,R_EFLAGS(%rsp)	/* fix eflags */
>>  
>> -	CPU_NUMBER(%edx)
>> +	CPU_NUMBER_NO_STACK(%edx)
>>  	TIME_TRAP_SENTRY
>>  
>>  	movq	CX(EXT(kernel_stack),%rdx),%rbx
>> @@ -1371,7 +1371,7 @@ ENTRY(syscall64)
>>  	 * save only the callee-preserved status according to the C ABI,
>>  	 * plus RIP and EFLAGS for sysret
>>  	 */
>> -	CPU_NUMBER(%r11)
>> +	CPU_NUMBER_NO_STACK(%r11)
>>  	movq	CX(EXT(active_threads),%r11),%r11 /* point to current thread */
>>  	movq	TH_PCB(%r11),%r11		/* point to pcb */
>>  	addq	$ PCB_ISS,%r11			/* point to saved state */
>> @@ -1405,7 +1405,7 @@ ENTRY(syscall64)
>>  	mov	%r10,%rcx		/* fix arg3 location according to C ABI */
>>  
>>  	/* switch to kernel stack, then we can enable interrupts */
>> -	CPU_NUMBER(%r11)
>> +	CPU_NUMBER_NO_STACK(%r11)
>>  	movq	CX(EXT(kernel_stack),%r11),%rsp
>>  	sti
>>  
>> -- 
>> 2.40.1
>> 
>> 
>> 

-- 

Joshua Branson
Sent from the Hurd

[prev in list] [next in list] [prev in thread] [next in thread]