Re: [PATCH] SMP initialization: detection and enumeration

[Almudena Garcia]

Thanks yours!! Now I can continue my work ;)

I like your latest SMP commit too. It will be very useful to the next
development stage. :-)

http://git.savannah.gnu.org/cgit/hurd/gnumach.git/commit/?id=a76bc939142f61e615fcc39fc940961e39a26207

Thanks!!

El sáb., 19 sept. 2020 a las 21:58, Samuel Thibault (<
samuel.thiba...@gnu.org>) escribió:

> Hello,
>
> Juan's copyright assignation was complete, so I could push your code,
> thanks!
>
> Samuel
>
>

Re: [PATCH] SMP initialization: detection and enumeration

[Samuel Thibault]

Hello,

Juan's copyright assignation was complete, so I could push your code,
thanks!

Samuel

Re: [PATCH] SMP initialization: detection and enumeration

[Almudena Garcia]

I attach new set of patches, fixing the latest errors

El jue., 13 ago. 2020 a las 0:41, Samuel Thibault ()
escribió:

> Almudena Garcia, le jeu. 13 août 2020 00:35:36 +0200, a ecrit:
> > > Users do need that to understand what's happening.
> > But, if by any reason, the machine has more than cpu but the detection
> fails,
> > could be interesting to advise about the system being running with an
> only cpu.
> > It's not?
>
> Yes. But your current code is printing an error also in other cases.
>
> Samuel
>
From 0f3be00652da041440c9e51e1881eff532671392 Mon Sep 17 00:00:00 2001
From: Almudena Garcia 
Date: Thu, 13 Aug 2020 15:11:33 +0200
Subject: [PATCH 1/3] smp: Add APIC finder and parser

To find the processors, It's necessary to find the APIC (MADT) table
This table is found inside ACPI tables.

This set of functions find the MADT table, and parse it to find the APIC structures
and register it in the kernel.

*acpi_parse_apic.h: ACPI structures and function prototypes.
*acpi_parse_apic.c: ACPI/APIC function definitions.
---
 i386/Makefrag.am  |   2 +
 i386/i386at/acpi_parse_apic.c | 543 ++
 i386/i386at/acpi_parse_apic.h | 164 ++
 3 files changed, 709 insertions(+)
 create mode 100644 i386/i386at/acpi_parse_apic.c
 create mode 100644 i386/i386at/acpi_parse_apic.h

diff --git a/i386/Makefrag.am b/i386/Makefrag.am
index 035fd34d..0f788f30 100644
--- a/i386/Makefrag.am
+++ b/i386/Makefrag.am
@@ -32,6 +32,8 @@ libkernel_a_SOURCES += \
 
 if PLATFORM_at
 libkernel_a_SOURCES += \
+	i386/i386at/acpi_parse_apic.h \
+	i386/i386at/acpi_parse_apic.c \
 	i386/i386at/boothdr.S \
 	i386/i386at/com.c \
 	i386/i386at/com.h \
diff --git a/i386/i386at/acpi_parse_apic.c b/i386/i386at/acpi_parse_apic.c
new file mode 100644
index ..55b6a09b
--- /dev/null
+++ b/i386/i386at/acpi_parse_apic.c
@@ -0,0 +1,543 @@
+/* acpi_parse_apic.h - ACPI-MADT table parser. Source file
+   Copyright (C) 2018 Juan Bosco Garcia
+   Copyright (C) 2019 2020 Almudena Garcia Jurado-Centurion
+   Written by Juan Bosco Garcia and Almudena Garcia Jurado-Centurion
+
+   This file is part of Min_SMP.
+
+   Min_SMP is free software; you can redistribute it and/or modify it
+   under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 2, or (at your option)
+   any later version.
+
+   Min_SMP is distributed in the hope that it will be useful, but
+   WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program; if not, write to the Free Software
+   Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA  02111, USA. */
+
+#include  /* memcmp, memcpy... */
+
+#include  /* uint16_t, uint32_t... */
+
+#include/* machine_slot */
+
+#include /* printf */
+#include 
+#include   /* phystokv */
+#include   /* lapic, ioapic... */
+#include 
+#include 
+
+static struct acpi_apic *apic_madt = NULL;
+
+/*
+ * acpi_print_info: shows by screen the ACPI's rsdp and rsdt virtual address
+ * and the number of entries stored in RSDT table.
+ *
+ * Receives as input the references of RSDP and RSDT tables,
+ * and the number of entries stored in RSDT.
+ */
+void
+acpi_print_info(struct acpi_rsdp *rsdp, struct acpi_rsdt *rsdt, int acpi_rsdt_n)
+{
+
+printf("ACPI:\n");
+printf(" rsdp = %x; rsdp->rsdt_addr = %x\n", rsdp, rsdp->rsdt_addr);
+printf(" rsdt = %x; rsdt->length = %x (n = %x)\n", rsdt, rsdt->header.length,
+   acpi_rsdt_n);
+}
+
+/*
+ * acpi_checksum: calculates the checksum of an ACPI table.
+ * Receives as input the virtual address of the table.
+ *
+ * Returns 0 if success, other value if error.
+ */
+static uint8_t
+acpi_checksum(void *addr, uint32_t length)
+{
+uint8_t *bytes = addr;
+uint8_t checksum = 0;
+unsigned int i;
+
+/* Sum all bytes of addr */
+for (i = 0; i < length; i++)
+checksum += bytes[i];
+
+return checksum;
+}
+
+/*
+ * acpi_check_signature: check if a signature match with the signature of its table.
+ *
+ * Receive as parameter both signatures: table signature, the signature which needs to check,
+ * and real signature, the genuine signature of the table.
+ *
+ * Return 0 if success, other if error.
+ */
+
+static int
+acpi_check_signature(uint8_t table_signature[], uint8_t real_signature[], uint8_t length)
+{
+return memcmp(table_signature, real_signature, length);
+}
+
+
+/*
+ * acpi_check_rsdp:
+ * check if the RDSP "candidate" table is the real RSDP table.
+ *
+ * Compare the table signature with the ACPI signature for this table
+ * and check is the checksum is correct.
+ *
+ * Receives as input the reference of RSDT table.
+ *
+ * Preconditions: RSDP pointer must not be NULL.
+ *
+ * Returns 0 if correct.
+ */
+static int8_t
+acpi_check_rsdp(

Re: [PATCH] SMP initialization: detection and enumeration

[Samuel Thibault]

Almudena Garcia, le jeu. 13 août 2020 00:35:36 +0200, a ecrit:
> > Users do need that to understand what's happening.
> But, if by any reason, the machine has more than cpu but the detection fails,
> could be interesting to advise about the system being running with an only 
> cpu.
> It's not?

Yes. But your current code is printing an error also in other cases.

Samuel

Re: [PATCH] SMP initialization: detection and enumeration

[Almudena Garcia]

> Applied and fixed a bit, thanks!

Thanks. Now I only have to fix the next patches. We are closer now

> But that will only enter if when api_success != 0, thus when we actually
> have a failure. Compare explicitly against ACPI_SUCCESS

Oops, I forget this. I will fix It.

> Then there is no need to expose the struct content in the .h file, keep
> it in the .c file.
Ok, It's a good solution.

> This will have to wait for the assignment paper for Juan.
I advised him that he has to write yours to explain how to do the
assignment.

> When a machine doesn't have apic/acpi at all we don't want to print an
> error.
Ok, then I can simply remove that print.

> Users do need that to understand what's happening.
But, if by any reason, the machine has more than cpu but the detection
fails, could be interesting to advise about the system being running with
an only cpu.
It's not?

Thanks by all. I'm impatient to finish this patch and start the work in
cpus'  enabling and configuration.

El mié., 12 ago. 2020 a las 23:52, Samuel Thibault ()
escribió:

> Almudena Garcia, le mar. 11 août 2020 02:17:35 +0200, a ecrit:
> > > What for?
> > > num_cpus is something that you make returned by a smp_get_numcpus()
> > > function, so it's not actually useful to also expose it in a struct.
> > > What else would go into that structure?
> > By example, the master cpu (BSP in x86)
>
> More precisely?
>
> (saying it like this, it looks like an x86-specific thing that we
> wouldn't actually expose to the rest of the code...)
>
> > > But the function returns an APIC id. Really not much can be done with
> > > that without knowing details of the APIC.
> > It's true. I didn't remember this. I can search the kernel ID of this
> APIC ID,
> > but cpu_number() already will do that.
> > Then, maybe it can be simpler to remove this function.
>
> Yes.
>
> > My idea was to avoid calling directly to the apic function when I will
> > implement cpu_number(). But maybe this is not the best solution for this.
>
> It'll be arch-dependent anyway, so it makes sense to just call it
> directly.
>
> > > What for?
> > > num_cpus is something that you make returned by a smp_get_numcpus()
> > > function, so it's not actually useful to also expose it in a struct.
> > > What else would go into that structure? Won't we actually rather
> > > use functions to return such information rather than imposing that
> > > structure over all archs?
> >
> > I will not use this struct to share the information, simply to ease the
> access
> > to the functions which really return such information.
>
> Then there is no need to expose the struct content in the .h file, keep
> it in the .c file.
>
> > > Re-read what I wrote. I do *not* think we want to print this as an
> > > error like you did. Not having SMP is not an error. Just do not print
> > > anything.
> > Really, the mistake is in the message. I wanted to advise that the
> processor
> > detection has failed in any step (although maybe the cause is not that
> SMP
> > doesn't exist in the machine...)
>
> When a machine doesn't have apic/acpi at all we don't want to print an
> error.
>
> > Then, I will have to change this message to a better explanation. But
> showing a
> > different message for each error code can be very lazy... :(
>
> Users do need that to understand what's happening.
>
> Samuel
>
>

Re: [PATCH] SMP initialization: detection and enumeration OFF TOPIC PRAISE

[Joshua Branson]

Fair enough.  I'll see if I can't try to help you in your project.  Fair
warning, I am a little out of my league here.  Just writing plain
ordinary C programs with pointers to strucks and memory management is
hard for me.

--
Joshua Branson
Sent from Emacs and Gnus

Re: [PATCH] SMP initialization: detection and enumeration

[Samuel Thibault]

Almudena Garcia, le mar. 11 août 2020 02:17:35 +0200, a ecrit:
> > What for?
> > num_cpus is something that you make returned by a smp_get_numcpus()
> > function, so it's not actually useful to also expose it in a struct.
> > What else would go into that structure?
> By example, the master cpu (BSP in x86)

More precisely?

(saying it like this, it looks like an x86-specific thing that we
wouldn't actually expose to the rest of the code...)

> > But the function returns an APIC id. Really not much can be done with
> > that without knowing details of the APIC.
> It's true. I didn't remember this. I can search the kernel ID of this APIC ID,
> but cpu_number() already will do that.
> Then, maybe it can be simpler to remove this function.

Yes.

> My idea was to avoid calling directly to the apic function when I will
> implement cpu_number(). But maybe this is not the best solution for this.

It'll be arch-dependent anyway, so it makes sense to just call it
directly.

> > What for?
> > num_cpus is something that you make returned by a smp_get_numcpus()
> > function, so it's not actually useful to also expose it in a struct.
> > What else would go into that structure? Won't we actually rather
> > use functions to return such information rather than imposing that
> > structure over all archs?
> 
> I will not use this struct to share the information, simply to ease the access
> to the functions which really return such information.

Then there is no need to expose the struct content in the .h file, keep
it in the .c file.

> > Re-read what I wrote. I do *not* think we want to print this as an
> > error like you did. Not having SMP is not an error. Just do not print
> > anything.
> Really, the mistake is in the message. I wanted to advise that the processor
> detection has failed in any step (although maybe the cause is not that SMP
> doesn't exist in the machine...)

When a machine doesn't have apic/acpi at all we don't want to print an
error.

> Then, I will have to change this message to a better explanation. But showing 
> a
> different message for each error code can be very lazy... :(

Users do need that to understand what's happening.

Samuel

Re: [PATCH] SMP initialization: detection and enumeration

[Samuel Thibault]

Almudena Garcia, le mer. 12 août 2020 16:03:06 +0200, a ecrit:
> Subject: [PATCH 1/5] smp: Add pseudoclass to manage APIC operations

Applied, thanks!

> Subject: [PATCH 2/5] vm_kern: Add kmem_alloc_aligned_table

Applied and fixed a bit, thanks!

> Subject: [PATCH 3/5] smp: Add APIC finder and parser

This will have to wait for the assignment paper for Juan.

> @@ -152,6 +152,8 @@ EXTRA_DIST += \
>  
>  if PLATFORM_at
>  libkernel_a_SOURCES += \
> + i386/i386at/acpi_parse_apic.h \
> + i386/i386at/acpi_parse_apic.c \
>   i386/i386/apic.h \
>   i386/i386/apic.c \
>   i386/i386/hardclock.c \

Add these along the other lines adding i386/i386at files, not along the
lines adding i386/i386 files.

> From d7c1f9d043d39500ebb39b6566f55da584832480 Mon Sep 17 00:00:00 2001
> From: Almudena Garcia 
> Date: Wed, 12 Aug 2020 15:53:56 +0200
> Subject: [PATCH 4/5] smp: Add generic smp pseudoclass
> 
> diff --git a/i386/i386/smp.c b/i386/i386/smp.c
> new file mode 100644
> index ..09e4bafa
> --- /dev/null
> +++ b/i386/i386/smp.c

> + * smp_init: initialize the SMP support, starting the cpus searching
> + * and enumeration.
> + */
> +int smp_init(void)
> +{
> +int apic_success;
> +
> +apic_success = acpi_apic_init();
> +if (apic_success) {

But that will only enter if when api_success != 0, thus when we actually
have a failure. Compare explicitly against ACPI_SUCCESS

> +smp_data_init();
> +}
> +
> +return apic_success;
> +}

Samuel

Re: [PATCH] SMP initialization: detection and enumeration

[Almudena Garcia]

I attach a new set of patches, fixing some errors

El mar., 11 ago. 2020 a las 23:44, Almudena Garcia (<
liberamenso10...@gmail.com>) escribió:

> I fixed the problem manually. I expect the patch works fine.
>
> El mar., 11 ago. 2020 a las 23:24, Almudena Garcia (<
> liberamenso10...@gmail.com>) escribió:
>
>> oops!! I forgot add i386/i386/smp.h and i386/i386/smp.c to
>> i386/Makefrag.am
>>
>> I will try to fix the patch to add this
>>
>> El mar., 11 ago. 2020 a las 23:03, Almudena Garcia (<
>> liberamenso10...@gmail.com>) escribió:
>>
>>> I attach a new set of patches. I wait this time will be better than last
>>> time
>>>
>>> El mar., 11 ago. 2020 a las 2:27, Almudena Garcia (<
>>> liberamenso10...@gmail.com>) escribió:
>>>
 > So don't bring them online then? The user asked for 4 CPUs, so bring
 up
 > 3 APs alongside the BSP and that's that.
 It's not a bad idea. Then I will fix my cpu enumeration loop to add
 this new condition.

 El mar., 11 ago. 2020 a las 2:23, Jessica Clarke ()
 escribió:

> On 11 Aug 2020, at 01:17, Almudena Garcia 
> wrote:
> >
> > > That being said, instead of hardcoding the maximum number of CPUs
> to
> > > be 256, you can just let the user choose whatever value is
> preferred.
> > > That's what Linux does.
> >
> > But this could cause coherency problems.
>
> Coherency is a very specific thing in operating systems about the
> properties of the underlying memory subsystem. It's not the word you're
> looking for.
>
> > By example, if the user sets mach_ncpus=4 , and the processor has 8
> cores, It can produce an out-of-index error in the access to the arrays
> which store the info about the cpus.
>
> So don't bring them online then? The user asked for 4 CPUs, so bring up
> 3 APs alongside the BSP and that's that.
>
> Jess
>
>
From c6675aef4adee4e902107b44ba3c01ce89da7e3f Mon Sep 17 00:00:00 2001
From: Almudena Garcia 
Date: Wed, 12 Aug 2020 15:38:24 +0200
Subject: [PATCH 1/5] smp: Add pseudoclass to manage APIC operations

The SMP support requires access, register and configure some APIC structures,
like Local APIC and IOAPIC.
This pseudoclass includes functions to register some APIC structures into the kernel,
and access to these structures to extract some information.

*apic.h: Header file recovered from Mach 4 source code, and updated for xAPIC.
Includes some structs with Local APIC and IOAPIC fields,
and the declaration of the functions.

*apic.c: Source file. Includes the definition of the functions, within some globals
of apic_info structures and the lapic reference.
---
 i386/Makefrag.am |   2 +
 i386/i386/apic.c | 221 +++
 i386/i386/apic.h | 139 +
 3 files changed, 362 insertions(+)
 create mode 100644 i386/i386/apic.c
 create mode 100644 i386/i386/apic.h

diff --git a/i386/Makefrag.am b/i386/Makefrag.am
index b2b4f77f..035fd34d 100644
--- a/i386/Makefrag.am
+++ b/i386/Makefrag.am
@@ -152,6 +152,8 @@ EXTRA_DIST += \
 
 if PLATFORM_at
 libkernel_a_SOURCES += \
+	i386/i386/apic.h \
+	i386/i386/apic.c \
 	i386/i386/hardclock.c \
 	i386/i386/hardclock.h \
 	i386/i386/io_map.c \
diff --git a/i386/i386/apic.c b/i386/i386/apic.c
new file mode 100644
index ..f0b4a153
--- /dev/null
+++ b/i386/i386/apic.c
@@ -0,0 +1,221 @@
+/* apic.c - APIC controller management for Mach.
+   Copyright (C) 2020 Free Software Foundation, Inc.
+   Written by Almudena Garcia Jurado-Centurion
+
+   This file is part of GNU Mach.
+
+   GNU Mach is free software; you can redistribute it and/or modify it
+   under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 2, or (at your option)
+   any later version.
+
+   GNU Mach is distributed in the hope that it will be useful, but
+   WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program; if not, write to the Free Software
+   Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA  02111, USA. */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+
+volatile ApicLocalUnit* lapic = NULL;
+
+ApicInfo apic_data;
+
+/*
+ * apic_data_init: initialize the apic_data structures to preliminary values.
+ * Reserve memory to the lapic list dynamic vector.
+ * Returns 0 if success, -1 if error.
+ */
+int
+apic_data_init(void)
+{
+apic_data.cpu_lapic_list = NULL;
+apic_data.ncpus = 0;
+apic_data.nioapics = 0;
+apic_data.nirqoverride = 0;
+
+/* Reserve the vector memory for the maximum number of processors. */
+apic_data.cpu_lapic_list = (uint16_t*) kalloc(NCPUS*sizeof(uint16_t));
+
+/* If the memory reserve fails, return -1 to advice about the error

Re: [PATCH] SMP initialization: detection and enumeration

[Almudena Garcia]

I fixed the problem manually. I expect the patch works fine.

El mar., 11 ago. 2020 a las 23:24, Almudena Garcia (<
liberamenso10...@gmail.com>) escribió:

> oops!! I forgot add i386/i386/smp.h and i386/i386/smp.c to i386/Makefrag.am
>
> I will try to fix the patch to add this
>
> El mar., 11 ago. 2020 a las 23:03, Almudena Garcia (<
> liberamenso10...@gmail.com>) escribió:
>
>> I attach a new set of patches. I wait this time will be better than last
>> time
>>
>> El mar., 11 ago. 2020 a las 2:27, Almudena Garcia (<
>> liberamenso10...@gmail.com>) escribió:
>>
>>> > So don't bring them online then? The user asked for 4 CPUs, so bring up
>>> > 3 APs alongside the BSP and that's that.
>>> It's not a bad idea. Then I will fix my cpu enumeration loop to add this
>>> new condition.
>>>
>>> El mar., 11 ago. 2020 a las 2:23, Jessica Clarke ()
>>> escribió:
>>>
 On 11 Aug 2020, at 01:17, Almudena Garcia 
 wrote:
 >
 > > That being said, instead of hardcoding the maximum number of CPUs to
 > > be 256, you can just let the user choose whatever value is
 preferred.
 > > That's what Linux does.
 >
 > But this could cause coherency problems.

 Coherency is a very specific thing in operating systems about the
 properties of the underlying memory subsystem. It's not the word you're
 looking for.

 > By example, if the user sets mach_ncpus=4 , and the processor has 8
 cores, It can produce an out-of-index error in the access to the arrays
 which store the info about the cpus.

 So don't bring them online then? The user asked for 4 CPUs, so bring up
 3 APs alongside the BSP and that's that.

 Jess


From 79240dbb2267744655797607919f75f79f30c88c Mon Sep 17 00:00:00 2001
From: Almudena Garcia 
Date: Tue, 11 Aug 2020 22:07:39 +0200
Subject: [PATCH 1/5] smp: Add pseudoclass to manage APIC operations

The SMP support requires access, register and configure some APIC structures,
like Local APIC and IOAPIC.
This pseudoclass includes functions to register some APIC structures into the kernel,
and access to these structures to extract some information.

*apic.h: Header file recovered from Mach 4 source code, and updated for xAPIC.
Includes some structs with Local APIC and IOAPIC fields,
and the declaration of the functions.

*apic.c: Source file. Includes the definition of the functions, within some globals
of apic_info structures and the lapic reference.
---
 i386/Makefrag.am |   4 +-
 i386/i386/apic.c | 223 +++
 i386/i386/apic.h | 139 +
 3 files changed, 365 insertions(+), 1 deletion(-)
 create mode 100644 i386/i386/apic.c
 create mode 100644 i386/i386/apic.h

diff --git a/i386/Makefrag.am b/i386/Makefrag.am
index b2b4f77f..b45378fd 100644
--- a/i386/Makefrag.am
+++ b/i386/Makefrag.am
@@ -160,7 +160,9 @@ libkernel_a_SOURCES += \
 	i386/i386/pic.c \
 	i386/i386/pic.h \
 	i386/i386/pit.c \
-	i386/i386/pit.h
+	i386/i386/pit.h \
+	i386/i386/apic.h \
+	i386/i386/apic.c
 endif
 
 #
diff --git a/i386/i386/apic.c b/i386/i386/apic.c
new file mode 100644
index ..ccba3857
--- /dev/null
+++ b/i386/i386/apic.c
@@ -0,0 +1,223 @@
+/* apic.c - APIC controller management for Mach.
+   Copyright (C) 2020 Free Software Foundation, Inc.
+   Written by Almudena Garcia Jurado-Centurion
+
+   This file is part of GNU Mach.
+
+   GNU Mach is free software; you can redistribute it and/or modify it
+   under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 2, or (at your option)
+   any later version.
+
+   GNU Mach is distributed in the hope that it will be useful, but
+   WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program; if not, write to the Free Software
+   Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA  02111, USA. */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+
+volatile ApicLocalUnit* lapic = NULL;
+
+ApicInfo apic_data;
+
+/*
+ * apic_data_init: initialize the apic_data structures to preliminary values.
+ * Reserve memory to the lapic list dynamic vector.
+ * Returns 0 if success, -1 if error.
+ */
+int
+apic_data_init(void)
+{
+apic_data.cpu_lapic_list = NULL;
+apic_data.ncpus = 0;
+apic_data.nioapics = 0;
+apic_data.nirqoverride = 0;
+
+/* Reserve the vector memory for the maximum number of processors. */
+apic_data.cpu_lapic_list = (uint16_t*) kalloc(NCPUS*sizeof(uint16_t));
+
+/* If the memory reserve fails, return -1 to advice about the error. */
+if (apic_data.cpu_lapic_list == NULL)
+return -1;
+
+return 0;
+}
+
+/*
+ * apic_lapic_init: initialize lapic pointer to the memory common address.
+ * Receives as input a poin

Re: [PATCH] SMP initialization: detection and enumeration

[Almudena Garcia]

oops!! I forgot add i386/i386/smp.h and i386/i386/smp.c to i386/Makefrag.am

I will try to fix the patch to add this

El mar., 11 ago. 2020 a las 23:03, Almudena Garcia (<
liberamenso10...@gmail.com>) escribió:

> I attach a new set of patches. I wait this time will be better than last
> time
>
> El mar., 11 ago. 2020 a las 2:27, Almudena Garcia (<
> liberamenso10...@gmail.com>) escribió:
>
>> > So don't bring them online then? The user asked for 4 CPUs, so bring up
>> > 3 APs alongside the BSP and that's that.
>> It's not a bad idea. Then I will fix my cpu enumeration loop to add this
>> new condition.
>>
>> El mar., 11 ago. 2020 a las 2:23, Jessica Clarke ()
>> escribió:
>>
>>> On 11 Aug 2020, at 01:17, Almudena Garcia 
>>> wrote:
>>> >
>>> > > That being said, instead of hardcoding the maximum number of CPUs to
>>> > > be 256, you can just let the user choose whatever value is preferred.
>>> > > That's what Linux does.
>>> >
>>> > But this could cause coherency problems.
>>>
>>> Coherency is a very specific thing in operating systems about the
>>> properties of the underlying memory subsystem. It's not the word you're
>>> looking for.
>>>
>>> > By example, if the user sets mach_ncpus=4 , and the processor has 8
>>> cores, It can produce an out-of-index error in the access to the arrays
>>> which store the info about the cpus.
>>>
>>> So don't bring them online then? The user asked for 4 CPUs, so bring up
>>> 3 APs alongside the BSP and that's that.
>>>
>>> Jess
>>>
>>>

Re: [PATCH] SMP initialization: detection and enumeration

[Almudena Garcia]

I attach a new set of patches. I wait this time will be better than last
time

El mar., 11 ago. 2020 a las 2:27, Almudena Garcia (<
liberamenso10...@gmail.com>) escribió:

> > So don't bring them online then? The user asked for 4 CPUs, so bring up
> > 3 APs alongside the BSP and that's that.
> It's not a bad idea. Then I will fix my cpu enumeration loop to add this
> new condition.
>
> El mar., 11 ago. 2020 a las 2:23, Jessica Clarke ()
> escribió:
>
>> On 11 Aug 2020, at 01:17, Almudena Garcia 
>> wrote:
>> >
>> > > That being said, instead of hardcoding the maximum number of CPUs to
>> > > be 256, you can just let the user choose whatever value is preferred.
>> > > That's what Linux does.
>> >
>> > But this could cause coherency problems.
>>
>> Coherency is a very specific thing in operating systems about the
>> properties of the underlying memory subsystem. It's not the word you're
>> looking for.
>>
>> > By example, if the user sets mach_ncpus=4 , and the processor has 8
>> cores, It can produce an out-of-index error in the access to the arrays
>> which store the info about the cpus.
>>
>> So don't bring them online then? The user asked for 4 CPUs, so bring up
>> 3 APs alongside the BSP and that's that.
>>
>> Jess
>>
>>
From 79240dbb2267744655797607919f75f79f30c88c Mon Sep 17 00:00:00 2001
From: Almudena Garcia 
Date: Tue, 11 Aug 2020 22:07:39 +0200
Subject: [PATCH 1/5] smp: Add pseudoclass to manage APIC operations

The SMP support requires access, register and configure some APIC structures,
like Local APIC and IOAPIC.
This pseudoclass includes functions to register some APIC structures into the kernel,
and access to these structures to extract some information.

*apic.h: Header file recovered from Mach 4 source code, and updated for xAPIC.
Includes some structs with Local APIC and IOAPIC fields,
and the declaration of the functions.

*apic.c: Source file. Includes the definition of the functions, within some globals
of apic_info structures and the lapic reference.
---
 i386/Makefrag.am |   4 +-
 i386/i386/apic.c | 223 +++
 i386/i386/apic.h | 139 +
 3 files changed, 365 insertions(+), 1 deletion(-)
 create mode 100644 i386/i386/apic.c
 create mode 100644 i386/i386/apic.h

diff --git a/i386/Makefrag.am b/i386/Makefrag.am
index b2b4f77f..b45378fd 100644
--- a/i386/Makefrag.am
+++ b/i386/Makefrag.am
@@ -160,7 +160,9 @@ libkernel_a_SOURCES += \
 	i386/i386/pic.c \
 	i386/i386/pic.h \
 	i386/i386/pit.c \
-	i386/i386/pit.h
+	i386/i386/pit.h \
+	i386/i386/apic.h \
+	i386/i386/apic.c
 endif
 
 #
diff --git a/i386/i386/apic.c b/i386/i386/apic.c
new file mode 100644
index ..ccba3857
--- /dev/null
+++ b/i386/i386/apic.c
@@ -0,0 +1,223 @@
+/* apic.c - APIC controller management for Mach.
+   Copyright (C) 2020 Free Software Foundation, Inc.
+   Written by Almudena Garcia Jurado-Centurion
+
+   This file is part of GNU Mach.
+
+   GNU Mach is free software; you can redistribute it and/or modify it
+   under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 2, or (at your option)
+   any later version.
+
+   GNU Mach is distributed in the hope that it will be useful, but
+   WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program; if not, write to the Free Software
+   Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA  02111, USA. */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+
+volatile ApicLocalUnit* lapic = NULL;
+
+ApicInfo apic_data;
+
+/*
+ * apic_data_init: initialize the apic_data structures to preliminary values.
+ * Reserve memory to the lapic list dynamic vector.
+ * Returns 0 if success, -1 if error.
+ */
+int
+apic_data_init(void)
+{
+apic_data.cpu_lapic_list = NULL;
+apic_data.ncpus = 0;
+apic_data.nioapics = 0;
+apic_data.nirqoverride = 0;
+
+/* Reserve the vector memory for the maximum number of processors. */
+apic_data.cpu_lapic_list = (uint16_t*) kalloc(NCPUS*sizeof(uint16_t));
+
+/* If the memory reserve fails, return -1 to advice about the error. */
+if (apic_data.cpu_lapic_list == NULL)
+return -1;
+
+return 0;
+}
+
+/*
+ * apic_lapic_init: initialize lapic pointer to the memory common address.
+ * Receives as input a pointer to the virtual memory address, previously mapped in a page.
+ */
+void
+apic_lapic_init(ApicLocalUnit* lapic_ptr)
+{
+lapic = lapic_ptr;
+}
+
+/*
+ * apic_add_cpu: add a new lapic/cpu entry to the cpu_lapic list.
+ * Receives as input the lapic's APIC ID.
+ */
+void
+apic_add_cpu(uint16_t apic_id)
+{
+apic_data.cpu_lapic_list[apic_data.ncpus] = apic_id;
+apic_data.ncpus++;
+}
+
+/*
+ * apic_add_ioapic: add a new ioapic entry to the ioapic lis

Re: [PATCH] SMP initialization: detection and enumeration OFF TOPIC PRAISE

[Samuel Thibault]

Almudena Garcia, le mar. 11 août 2020 14:17:17 +0200, a ecrit:
> > You can use b (break) within a git rebase session to stop at some point.
> > That way you can stop before the big change, commit part of it, and use
> > git rebase --continue, and let git discover that part of the big change
> > is already there and drop it from the big change.
> 
> But I have tons of commits. Is there any simple way to only take the latest
> version of each file and generate a set of commits for them?

Simplest might be to just do it by hand: git diff between master and
your branch to get the complete patch, start a new branch from master,
and apply the patch by pieces to make up proper commits, and finish with
a diff between your old branch and your new branch, to check that you
have everything in place.

Samuel

Re: [PATCH] SMP initialization: detection and enumeration OFF TOPIC PRAISE

[Almudena Garcia]

Yes, It is. This is the branch.
https://github.com/AlmuHS/GNUMach_SMP/tree/smp-new

All commits are related with this patch. The development was long

El mar., 11 ago. 2020 a las 14:30, Amos Jeffries ()
escribió:

> On 12/08/20 12:17 am, Almudena Garcia wrote:
> >> You can use b (break) within a git rebase session to stop at some point.
> >> That way you can stop before the big change, commit part of it, and use
> >> git rebase --continue, and let git discover that part of the big change
> >> is already there and drop it from the big change.
> >
> > But I have tons of commits. Is there any simple way to only take the
> > latest version of each file and generate a set of commits for them?
> >
>
> Is your branch public? I can take some time to go over the history and
> make suggestions on cleaning it up if you like.
>
>
> Amos
>

Re: [PATCH] SMP initialization: detection and enumeration OFF TOPIC PRAISE

[Amos Jeffries]

On 12/08/20 12:17 am, Almudena Garcia wrote:
>> You can use b (break) within a git rebase session to stop at some point.
>> That way you can stop before the big change, commit part of it, and use
>> git rebase --continue, and let git discover that part of the big change
>> is already there and drop it from the big change.
> 
> But I have tons of commits. Is there any simple way to only take the
> latest version of each file and generate a set of commits for them?
> 

Is your branch public? I can take some time to go over the history and
make suggestions on cleaning it up if you like.


Amos

Re: [PATCH] SMP initialization: detection and enumeration OFF TOPIC PRAISE

[Amos Jeffries]

On 11/08/20 11:57 pm, Samuel Thibault wrote:
> Almudena Garcia, le mar. 11 août 2020 13:45:19 +0200, a ecrit:
>>> P.S.  You might be interested in using the program git send-email.
>>> Apparently it makes it really easy to send in a patches series.  It's
>>> actually what GNU Guix contributing guidelines recommends.
>> My problem is that, in my internal repository, I do many commits for each
>> change (even for each file). The development is progressive.
>> Then, if I take the commit history to generate my patches, I would generate
>> many spurious patches, making it very difficult to check and apply them.
> 
> Sure, that's a common thing when developping.
> 
>> Even doing a rebase over another branch, the rebase adds the commit history, 
>> so
>> I keep the same problem.
> 
> ? On the contrary, rebase lets you decide what exactly you keep in the
> history and what you squash.
> 

Almundena:

To clarify in case you are not aware. The "rebase -i" operation is what
is being suggested. The -i option enables much more flexibility of
control over the patch sequence and history during rebase.


HTH
Amos

Re: [PATCH] SMP initialization: detection and enumeration OFF TOPIC PRAISE

[Almudena Garcia]

> You can use b (break) within a git rebase session to stop at some point.
> That way you can stop before the big change, commit part of it, and use
> git rebase --continue, and let git discover that part of the big change
> is already there and drop it from the big change.

But I have tons of commits. Is there any simple way to only take the latest
version of each file and generate a set of commits for them?

El mar., 11 ago. 2020 a las 13:57, Samuel Thibault ()
escribió:

> Almudena Garcia, le mar. 11 août 2020 13:45:19 +0200, a ecrit:
> > > P.S.  You might be interested in using the program git send-email.
> > > Apparently it makes it really easy to send in a patches series.  It's
> > > actually what GNU Guix contributing guidelines recommends.
> > My problem is that, in my internal repository, I do many commits for each
> > change (even for each file). The development is progressive.
> > Then, if I take the commit history to generate my patches, I would
> generate
> > many spurious patches, making it very difficult to check and apply them.
>
> Sure, that's a common thing when developping.
>
> > Even doing a rebase over another branch, the rebase adds the commit
> history, so
> > I keep the same problem.
>
> ? On the contrary, rebase lets you decide what exactly you keep in the
> history and what you squash.
>
> > And squash commits can be difficult too, because some commits are doing
> changes
> > in many files.
>
> You can use b (break) within a git rebase session to stop at some point.
> That way you can stop before the big change, commit part of it, and use
> git rebase --continue, and let git discover that part of the big change
> is already there and drop it from the big change.
>
> Samuel
>
>

Re: [PATCH] SMP initialization: detection and enumeration OFF TOPIC PRAISE

[Samuel Thibault]

Almudena Garcia, le mar. 11 août 2020 13:45:19 +0200, a ecrit:
> > P.S.  You might be interested in using the program git send-email.
> > Apparently it makes it really easy to send in a patches series.  It's
> > actually what GNU Guix contributing guidelines recommends.
> My problem is that, in my internal repository, I do many commits for each
> change (even for each file). The development is progressive.
> Then, if I take the commit history to generate my patches, I would generate
> many spurious patches, making it very difficult to check and apply them.

Sure, that's a common thing when developping.

> Even doing a rebase over another branch, the rebase adds the commit history, 
> so
> I keep the same problem.

? On the contrary, rebase lets you decide what exactly you keep in the
history and what you squash.

> And squash commits can be difficult too, because some commits are doing 
> changes
> in many files.

You can use b (break) within a git rebase session to stop at some point.
That way you can stop before the big change, commit part of it, and use
git rebase --continue, and let git discover that part of the big change
is already there and drop it from the big change.

Samuel

Re: [PATCH] SMP initialization: detection and enumeration OFF TOPIC PRAISE

[Almudena Garcia]

Hi! Thanks for your comments!!

> P.S.  You might be interested in using the program git send-email.
> Apparently it makes it really easy to send in a patches series.  It's
> actually what GNU Guix contributing guidelines recommends.
My problem is that, in my internal repository, I do many commits for each
change (even for each file). The development is progressive.
Then, if I take the commit history to generate my patches, I would generate
many spurious patches, making it very difficult to check and apply them.

Even doing a rebase over another branch, the rebase adds the commit
history, so I keep the same problem.
And squash commits can be difficult too, because some commits are doing
changes in many files.

So, at the moment, I only know dirty tricks to generate the patches.

Thanks anyway


El mar., 11 ago. 2020 a las 8:27, Joshua Branson ()
escribió:

>
> Almudena!  Nice work on this! and Holy cow!  Your English is getting
> fantastic!  It inspires me, when I see how much you're learning on this
> SMP project.  Keep up the good work!
>
> P.S.  You might be interested in using the program git send-email.
> Apparently it makes it really easy to send in a patches series.  It's
> actually what GNU Guix contributing guidelines recommends.
>
> https://guix.gnu.org/manual/en/html_node/Submitting-Patches.html
>
> P.P.S.  I feel slightly silly giving tips, when everyone else on this
> email list is several orders of magnitude a better developer than I am.
> In fact, I probably only understood 50% of what this email chain was
> about.  I hope this off topic email is not offensive or annoying to
> anyone.  I just wanted an excuse to encourage and praise Almudena
> publicly!
>
> --
> Joshua Branson
> Sent from Emacs and Gnus
>
>

Re: [PATCH] SMP initialization: detection and enumeration OFF TOPIC PRAISE

[Joshua Branson]

Almudena!  Nice work on this! and Holy cow!  Your English is getting
fantastic!  It inspires me, when I see how much you're learning on this
SMP project.  Keep up the good work!

P.S.  You might be interested in using the program git send-email.
Apparently it makes it really easy to send in a patches series.  It's
actually what GNU Guix contributing guidelines recommends.

https://guix.gnu.org/manual/en/html_node/Submitting-Patches.html

P.P.S.  I feel slightly silly giving tips, when everyone else on this
email list is several orders of magnitude a better developer than I am.
In fact, I probably only understood 50% of what this email chain was
about.  I hope this off topic email is not offensive or annoying to
anyone.  I just wanted an excuse to encourage and praise Almudena
publicly!

--
Joshua Branson
Sent from Emacs and Gnus

Re: [PATCH] SMP initialization: detection and enumeration

[Almudena Garcia]

> So don't bring them online then? The user asked for 4 CPUs, so bring up
> 3 APs alongside the BSP and that's that.
It's not a bad idea. Then I will fix my cpu enumeration loop to add this
new condition.

El mar., 11 ago. 2020 a las 2:23, Jessica Clarke ()
escribió:

> On 11 Aug 2020, at 01:17, Almudena Garcia 
> wrote:
> >
> > > That being said, instead of hardcoding the maximum number of CPUs to
> > > be 256, you can just let the user choose whatever value is preferred.
> > > That's what Linux does.
> >
> > But this could cause coherency problems.
>
> Coherency is a very specific thing in operating systems about the
> properties of the underlying memory subsystem. It's not the word you're
> looking for.
>
> > By example, if the user sets mach_ncpus=4 , and the processor has 8
> cores, It can produce an out-of-index error in the access to the arrays
> which store the info about the cpus.
>
> So don't bring them online then? The user asked for 4 CPUs, so bring up
> 3 APs alongside the BSP and that's that.
>
> Jess
>
>

Re: [PATCH] SMP initialization: detection and enumeration

[Jessica Clarke]

On 11 Aug 2020, at 01:17, Almudena Garcia  wrote:
> 
> > That being said, instead of hardcoding the maximum number of CPUs to
> > be 256, you can just let the user choose whatever value is preferred.
> > That's what Linux does.
> 
> But this could cause coherency problems. 

Coherency is a very specific thing in operating systems about the
properties of the underlying memory subsystem. It's not the word you're
looking for.

> By example, if the user sets mach_ncpus=4 , and the processor has 8 cores, It 
> can produce an out-of-index error in the access to the arrays which store the 
> info about the cpus.

So don't bring them online then? The user asked for 4 CPUs, so bring up
3 APs alongside the BSP and that's that.

Jess

Re: [PATCH] SMP initialization: detection and enumeration

[Almudena Garcia]

> That being said, instead of hardcoding the maximum number of CPUs to
> be 256, you can just let the user choose whatever value is preferred.
> That's what Linux does.

But this could cause coherency problems.
By example, if the user sets mach_ncpus=4 , and the processor has 8 cores,
It can produce an out-of-index error in the access to the arrays which
store the info about the cpus.

> Re-read what I wrote. I was talking about lapic being qualified
> volatile, and apic_get_lapic returning it as non-volatile. Aren't you
> getting a warning there?
Yes, I've just fixed It.

> I can't see how this declaration can't be in kern/smp.h. It's not even
> returning a struct or whatever. What is the actual error message when
> you put it in kern/smp.h?
I've just solved It declaring *smp_info *in kern/smp.c , and adding an *extern
*declaration in its header.

> What for?
> num_cpus is something that you make returned by a smp_get_numcpus()
> function, so it's not actually useful to also expose it in a struct.
> What else would go into that structure?
By example, the master cpu (BSP in x86)

> But the function returns an APIC id. Really not much can be done with
> that without knowing details of the APIC.
It's true. I didn't remember this. I can search the kernel ID of this APIC
ID, but cpu_number() already will do that.
Then, maybe it can be simpler to remove this function.
My idea was to avoid calling directly to the apic function when I will
implement cpu_number(). But maybe this is not the best solution for this.
I have to rethink this.

> What for?
> num_cpus is something that you make returned by a smp_get_numcpus()
> function, so it's not actually useful to also expose it in a struct.
> What else would go into that structure? Won't we actually rather
> use functions to return such information rather than imposing that
> structure over all archs?

I will not use this struct to share the information, simply to ease the
access to the functions which really return such information.

smp_get_numcpus() doesn't return NCPUS value, but returns the real number
of cpus existent in the machine.
I simply use a struct to store this generic information instead access to
apic's ApicInfo struct (which stores this value in x86).

> Re-read what I wrote. I do *not* think we want to print this as an
> error like you did. Not having SMP is not an error. Just do not print
> anything.
Really, the mistake is in the message. I wanted to advise that the
processor detection has failed in any step (although maybe the cause is not
that SMP doesn't exist in the machine...)
Then, I will have to change this message to a better explanation. But
showing a different message for each error code can be very lazy... :(



El mar., 11 ago. 2020 a las 1:39, Samuel Thibault ()
escribió:

> Almudena Garcia, le mar. 11 août 2020 01:23:49 +0200, a ecrit:
> > > ? git diff produces the same as format-patch, in terms of formatting
> > > mistakes... The disadvantage of git diff is that your patches then mix
> > > things altogether, see the additions to Makefrag.am. I just cannot
> apply
> > > the series as it is.
> >
> > Yes, The problem is that I didn't write each file in a single commit.
>
> Then use git rebase to rewrite your patch history?
>
> > > ? The whole point of the mach_ncpus variable is to contain the number
> > > of processors. I don't see why we could want mach_ncpus to be defined
> to
> > > a different value than NCPUS.
> >
> > As I explained in a previous mail, in Mach source code the are many
> structures
> > (arrays) which size is defined by NCPUS.
> > Added to this, all SMP special cases are controlled by preprocessor
> directives
> > like "if NCPUS > 1". For this reason, removing NCPUS can be a very
> difficult
> > task.
>
> I didn't write about removing NCPUS. I wrote about your code defining
> NCPUS to a value that is different from mach_ncpus, while it could just
> define mach_ncpus, and let NCPUS inherit the value from mach_ncpus so
> it's all coherent.
>
> That being said, instead of hardcoding the maximum number of CPUs to
> be 256, you can just let the user choose whatever value is preferred.
> That's what Linux does.
>
> > > Err, it's empty, so just drop the file.
> > I prefer to keep this file, to use that as an arch-independent interface
> for
> > SMP.
>
> But there is nothing there to be compiled. Some compilation toolchains
> would even emit a warning in such a case, or just plainly error out
> because they cannot produce an empty .o file.
>
> > > Does it really need to be a separate function? Will we ever want to
> call
> > > it somewhere else than smp_init?  If not just fold it into smp_init, or
> > > make it static, there is no point in making it extern.
> >
> > I put It in a separate function because It's possible that, in next
> steps, I
> > will need to add more fields to the structure, which will need to be
> > initialized together.
>
> Ok, but since smp_init is almost empty, you can as well initialize the
> data in th

Re: [PATCH] SMP initialization: detection and enumeration

[Samuel Thibault]

Almudena Garcia, le mar. 11 août 2020 01:23:49 +0200, a ecrit:
> > ? git diff produces the same as format-patch, in terms of formatting
> > mistakes... The disadvantage of git diff is that your patches then mix
> > things altogether, see the additions to Makefrag.am. I just cannot apply
> > the series as it is.
> 
> Yes, The problem is that I didn't write each file in a single commit.

Then use git rebase to rewrite your patch history?

> > ? The whole point of the mach_ncpus variable is to contain the number
> > of processors. I don't see why we could want mach_ncpus to be defined to
> > a different value than NCPUS.
> 
> As I explained in a previous mail, in Mach source code the are many structures
> (arrays) which size is defined by NCPUS.
> Added to this, all SMP special cases are controlled by preprocessor directives
> like "if NCPUS > 1". For this reason, removing NCPUS can be a very difficult
> task.

I didn't write about removing NCPUS. I wrote about your code defining
NCPUS to a value that is different from mach_ncpus, while it could just
define mach_ncpus, and let NCPUS inherit the value from mach_ncpus so
it's all coherent.

That being said, instead of hardcoding the maximum number of CPUs to
be 256, you can just let the user choose whatever value is preferred.
That's what Linux does.

> > Err, it's empty, so just drop the file.
> I prefer to keep this file, to use that as an arch-independent interface for
> SMP.

But there is nothing there to be compiled. Some compilation toolchains
would even emit a warning in such a case, or just plainly error out
because they cannot produce an empty .o file.

> > Does it really need to be a separate function? Will we ever want to call
> > it somewhere else than smp_init?  If not just fold it into smp_init, or
> > make it static, there is no point in making it extern.
> 
> I put It in a separate function because It's possible that, in next steps, I
> will need to add more fields to the structure, which will need to be
> initialized together.

Ok, but since smp_init is almost empty, you can as well initialize the
data in there.

> > Is this function really useful?  I mean in the long run we will want a
> > CPU number from 0, which will have to be knowing the apic enumeration,
> > and thus that's probably acpi.c that will define it anyway, and that is
> > the function whose declaration will belong to kernel/smp.h
> 
> The idea about this function is to know the number of cpus (this will be
> necessary to enable the cpus in next steps), without knowing the details about
> APIC.

But the function returns an APIC id. Really not much can be done with
that without knowing details of the APIC.

> > > +volatile ApicLocalUnit* lapic = NULL;
> > [...]
> 
> If, by any reason, we can't find APIC structures, o we need to have a secure
> value for lapic pointer, to take notice that APIC search or parsing has 
> failed.
> And I simply prefered set this value at start. When we find the APIC table, if
> the parser is successful, this pointer will take the real value of the common
> Local APIC memory address.

For global variables the default value is *already* asserted to be NULL.
But anyway I was not talking about that at all.

Re-read what I wrote. I was talking about lapic being qualified
volatile, and apic_get_lapic returning it as non-volatile. Aren't you
getting a warning there?

> > That one belongs to kern/smp.h: non-i386 code will probably want to use
> > it.
> Yes, this was my idea. But It was very difficult to declare smp_info structure
> in kern/apic.c , and then share this structure with i386/i386/smp.c to
> initialize its data.
> I had many compilation problems, so I had to put this function there to ease
> the compilation.

?

I can't see how this declaration can't be in kern/smp.h. It's not even
returning a struct or whatever. What is the actual error message when
you put it in kern/smp.h?

> > ? Is this really useful to expose to the rest of the kernel? Isn't that
> > structure something specific to the i386 SMP implementation?
> 
> Really, the smp_data structure might be generic for all architectures.

What for?
num_cpus is something that you make returned by a smp_get_numcpus()
function, so it's not actually useful to also expose it in a struct.
What else would go into that structure? Won't we actually rather
use functions to return such information rather than imposing that
structure over all archs?

> The only reason because I declared smp_info in i386 files is because I
> didn't get to compile the code declaring it in kern/smp.c.

Which is possibly another sign that it's generally simpler to keep it in
the arch-specific part, and only expose simple functions like
int smp_get_numcpus(void); which pose no declaration problems.

> > ? I don't think we want to print this as an error?
> Is there a function to print messages as errors? If yes, then I replace this
> print with It.

Re-read what I wrote. I do *not* think we want to pri

Re: [PATCH] SMP initialization: detection and enumeration

[Almudena Garcia]

> ? git diff produces the same as format-patch, in terms of formatting
> mistakes... The disadvantage of git diff is that your patches then mix
> things altogether, see the additions to Makefrag.am. I just cannot apply
> the series as it is.

Yes, The problem is that I didn't write each file in a single commit. Then,
I have to split the changes in different patches on my own.
This is the reason because the additions to Makefrag.am are not in a exact
order, because I had to split the changes file to file.

> ? The whole point of the mach_ncpus variable is to contain the number
> of processors. I don't see why we could want mach_ncpus to be defined to
> a different value than NCPUS.

As I explained in a previous mail, in Mach source code the are many
structures (arrays) which size is defined by NCPUS.
Added to this, all SMP special cases are controlled by preprocessor
directives like "if NCPUS > 1". For this reason, removing NCPUS can be a
very difficult task.

So, to avoid a big rewrite of the code, I simply use "mach_ncpus" as a
switch, to enable or disable SMP support.
But, when its value is bigger than 1, I set NCPUS to the maximum number of
cpus, to fix the size of the structures to a safety value.
Then, when SMP is enabled (mach_ncpus > 1), the arrays which store the
information about cpus take 256 (NCPUS value) positions by default.

> Err, it's empty, so just drop the file.
I prefer to keep this file, to use that as an arch-independent interface
for SMP.

> i386 file references go to i386/Makefrag.am, please.
Ok, I forgot this detail.

> Does it really need to be a separate function? Will we ever want to call
> it somewhere else than smp_init?  If not just fold it into smp_init, or
> make it static, there is no point in making it extern.

I put It in a separate function because It's possible that, in next steps,
I will need to add more fields to the structure, which will need to be
initialized together.
But I agree that this function might be st

> Is this function really useful?  I mean in the long run we will want a
> CPU number from 0, which will have to be knowing the apic enumeration,
> and thus that's probably acpi.c that will define it anyway, and that is
> the function whose declaration will belong to kernel/smp.h

The idea about this function is to know the number of cpus (this will be
necessary to enable the cpus in next steps), without knowing the details
about APIC.
In other terms, the SMP functions are a simple interface to manage and get
info about SMP, without needing to know architecture internals (in this
case, APIC).

> > +volatile ApicLocalUnit* lapic = NULL;
> [...]

If, by any reason, we can't find APIC structures, o we need to have a
secure value for lapic pointer, to take notice that APIC search or parsing
has failed.
And I simply prefered set this value at start. When we find the APIC table,
if the parser is successful, this pointer will take the real value of the
common Local APIC memory address.

> That one belongs to kern/smp.h: non-i386 code will probably want to use
> it.
Yes, this was my idea. But It was very difficult to declare *smp_info*
structure in kern/apic.c , and then share this structure with
i386/i386/smp.c to initialize its data.
I had many compilation problems, so I had to put this function there to
ease the compilation.

> ? Is this really useful to expose to the rest of the kernel? Isn't that
> structure something specific to the i386 SMP implementation?

Really, the smp_data structure might be generic for all architectures. The
only reason because I declared *smp_info *in i386 files is because I didn't
get to compile the code declaring it in kern/smp.c.
The entire smp pseudoclass might be a generic smp interface for any
architecture.

> ? I don't think we want to print this as an error?
Is there a function to print messages as errors? If yes, then I replace
this print with It.

Thanks for the review. I will try to fix the mistakes in my code.


El mar., 11 ago. 2020 a las 0:25, Samuel Thibault ()
escribió:

> Hello,
>
> Almudena Garcia, le lun. 10 août 2020 20:56:13 +0200, a ecrit:
> > I attach a new version of my patch, fixing some errors and following the
> latest
> > comments about this.
>
> That looks nice overall!
>
> > This time, I've generated the files manually from "git diff", instead
> using
> > "git format-patch", so the patches could contain little format mistakes.
>
> ? git diff produces the same as format-patch, in terms of formatting
> mistakes... The disadvantage of git diff is that your patches then mix
> things altogether, see the additions to Makefrag.am. I just cannot apply
> the series as it is.
>
> Please try to attach patches in their order, so I don't have to reorder
> while reviewing...
>
> > From: Almudena Garcia 
> > Date: Mon, 10 Aug 2020 19:52:44 +0200
> > Subject: [PATCH 1/6] configfrag.ac: Define NCPUS to 256 if mach_ncpus >
> 1
> >
> > This allows to define the size of cpus structures to the maximum value
> allowed by xAPIC,

Re: [PATCH] SMP initialization: detection and enumeration

[Samuel Thibault]

Hello,

Almudena Garcia, le lun. 10 août 2020 20:56:13 +0200, a ecrit:
> I attach a new version of my patch, fixing some errors and following the 
> latest
> comments about this.

That looks nice overall!

> This time, I've generated the files manually from "git diff", instead using
> "git format-patch", so the patches could contain little format mistakes.

? git diff produces the same as format-patch, in terms of formatting
mistakes... The disadvantage of git diff is that your patches then mix
things altogether, see the additions to Makefrag.am. I just cannot apply
the series as it is.

Please try to attach patches in their order, so I don't have to reorder
while reviewing...

> From: Almudena Garcia 
> Date: Mon, 10 Aug 2020 19:52:44 +0200
> Subject: [PATCH 1/6] configfrag.ac: Define NCPUS to 256 if mach_ncpus > 1
> 
> This allows to define the size of cpus structures to the maximum value 
> allowed by xAPIC, keeping this size independant of mach_ncpus value

? The whole point of the mach_ncpus variable is to contain the number
of processors. I don't see why we could want mach_ncpus to be defined to
a different value than NCPUS.

> ---
> diff --git a/configfrag.ac b/configfrag.ac
> index 91d737ef..05c33a28 100644
> --- a/configfrag.ac
> +++ b/configfrag.ac
> @@ -46,9 +46,11 @@ AC_DEFINE([BOOTSTRAP_SYMBOLS], [0], [BOOTSTRAP_SYMBOLS])
>  # Multiprocessor support is still broken.
>  AH_TEMPLATE([MULTIPROCESSOR], [set things up for a uniprocessor])
>  mach_ncpus=1
> +
>  AC_DEFINE_UNQUOTED([NCPUS], [$mach_ncpus], [number of CPUs])
>  [if [ $mach_ncpus -gt 1 ]; then]
>AC_DEFINE([MULTIPROCESSOR], [1], [set things up for a multiprocessor])
> +  AC_DEFINE_UNQUOTED([NCPUS], [256], [number of CPUs])
>  [fi]
>  
>  # Restartable Atomic Sequences to get a really fast test-n-set.  Can't be


> From: Almudena Garcia 
> Date: Mon, 10 Aug 2020 19:54:02 +0200
> Subject: [PATCH 2/6] smp: Add pseudoclass to manage APIC operations
> 

> diff --git a/Makefrag.am b/Makefrag.am
> index ea612275..03821d03 100644
> --- a/Makefrag.am
> +++ b/Makefrag.am
> @@ -122,6 +122,24 @@ EXTRA_DIST += \
>   ipc/notify.defs
>  
>  
> +#
> +# SMP implementation (APIC, ACPI, etc)
> +#
> +
> + 
> +libkernel_a_SOURCES += \
> + i386/i386/apic.h \
> + i386/i386/apic.c

i386 file references go to i386/Makefrag.am, please.

Also, no need to put them in a dedicated section, just file them in the
list of i386/i386/ files

Also, put them inside the list guarded by "if PLATFORM_at": acpi/apic do
not make sense on the Xen platform. You'll see the pic files referenced
there, so see that yes, apic definitely belongs there.


And similarly for the i386/i386at/acpi* files: file them into i386/Makefrag.am,
within the existing i386/i386at/ list. And same for i386/i386/smp*
files. kern/smp.h however belongs to the root Makefrag.am, since it's
arch-independent.

> +volatile ApicLocalUnit* lapic = NULL;

[...]

> +/* apic_get_lapic: returns a reference to the common memory address for 
> Local APIC. */
> +ApicLocalUnit*
> +apic_get_lapic(void)
> +{
> +return lapic;
> +}

Aren't you getting a warning here about the volatile qualifier?
Yes, apic_get_lapic should be made to return a volatile ApicLocalUnit*.
Always pay attention.

> +typedef struct ApicLocalUnit {
> +/* 0x000 */
> +ApicReg reserved0;
> +/* 0x010 */
> +ApicReg reserved1;

As I already said, please put the comments on the right, that will look
much better.

> +void*
> +kmem_map_aligned_table(
> + phys_addr_t phys_address,
> + vm_size_t   size,
> + int mode)
> +{
> + vm_offset_t virt_addr;
> + kern_return_t ret;
> + uintptr_t into_page = phys_address % PAGE_SIZE;
> + uintptr_t nearest_page = (uintptr_t)trunc_page(phys_address);

Both of these need to be a phys_addr_t: uintptr_t is the size of a
native pointer only. On a 32bit arch, we can have 32bit pointers but
40bit physical addresses for instance.

> From: Almudena Garcia 
> Date: Mon, 10 Aug 2020 19:59:00 +0200
> Subject: [PATCH 5/6] smp: Add generic smp pseudoclass
> 
> diff --git a/i386/i386/smp.c b/i386/i386/smp.c
> new file mode 100644
> index ..9fbc1ca1
> --- /dev/null
> +++ b/i386/i386/smp.c

> +/*
> + * smp_data_init: initialize smp_data structure
> + * Must be called after smp_init(), once all APIC structures
> + * has been initialized
> + */
> +void smp_data_init(void)
> +{
> +smp_info.num_cpus = apic_get_numcpus();
> +}

Does it really need to be a separate function? Will we ever want to call
it somewhere else than smp_init?  If not just fold it into smp_init, or
make it static, there is no point in making it extern.

> +/*
> + * smp_get_current_cpu: return the hardware identifier (APIC ID in x86)
> + * of current CPU
> + */
> +int smp_get_current_cpu(void)
> +{
> +return apic_get_current_cpu();
> +}

Is this function really useful?  I mean in the long run we will want a
CPU number from 0, which will have to be kno

Re: [PATCH] SMP initialization: detection and enumeration

[Almudena Garcia]

I attach a new version of my patch, fixing some errors and following the
latest comments about this.
This time, I've generated the files manually from "git diff", instead using
"git format-patch", so the patches could contain little format mistakes.
Excuse me about this issue.

El jue., 30 jul. 2020 a las 23:46, Almudena Garcia (<
liberamenso10...@gmail.com>) escribió:

> > You don't. Read about GDT and segments register, and segmentation in
> > general. In the GDT you'd only store a pointer to the per-cpu data, and
> > processors will load it.
>
> > > Is there any way to avoid using an array for that?
>
> > Yes, you'd just store the ID in the per-cpu data.
>
> Thanks, I'll take notes about this
>
> El jue., 30 jul. 2020 a las 23:35, Samuel Thibault (<
> samuel.thiba...@gnu.org>) escribió:
>
>> Almudena Garcia, le jeu. 30 juil. 2020 23:30:55 +0200, a ecrit:
>> > > As Richard said, we just want to have different GDTs on the different
>> > > processors, so that we wan use the fs segment register to implement
>> TLS
>> > > in the kernel and have per-cpu data cost essentially the same as
>> global
>> > > data.
>> >
>> > Yes, but how can I store the relation APIC ID - Kernel ID in the GDT?
>>
>> You don't. Read about GDT and segments register, and segmentation in
>> general. In the GDT you'd only store a pointer to the per-cpu data, and
>> processors will load it.
>>
>> > Is there any way to avoid using an array for that?
>>
>> Yes, you'd just store the ID in the per-cpu data.
>>
>> Samuel
>>
>
From: Almudena Garcia 
Date: Mon, 10 Aug 2020 19:59:00 +0200
Subject: [PATCH 5/6] smp: Add generic smp pseudoclass

This pseudoclass generalize the initialization and access of SMP data,
allowing expands it to other architectures. In x86, the functions calls to apic functions.

*kern/smp.c: Source file. Implements a interface to load the SMP functions for the current architecture.
*kern/smp.h: Header file. Add declaration for smp_data structure.
*i386/i386/smp.c: Source file. Implements a set of functions to manage the SMP actions in i386
*i386/i386/smp.h: Header file. Add declarations for SMP functions in i386.
---
diff --git a/Makefrag.am b/Makefrag.am
index ea612275..03821d03 100644
--- a/Makefrag.am
+++ b/Makefrag.am
@@ -122,6 +122,24 @@ EXTRA_DIST += \
 	ipc/notify.defs
 
 
+#
+# SMP implementation (APIC, ACPI, etc)
+#
+
+libkernel_a_SOURCES += \
+	i386/i386at/acpi_parse_apic.h \
+	i386/i386at/acpi_parse_apic.c
+	
+libkernel_a_SOURCES += \
+	i386/i386/apic.h \
+	i386/i386/apic.c
+	
+libkernel_a_SOURCES += \
+	kern/smp.h \
+	kern/smp.c \
+	i386/i386/smp.h \
+	i386/i386/smp.c
+	
 #
 # `kernel' implementation (tasks, threads, trivia, etc.).
 #

diff --git a/i386/i386/smp.c b/i386/i386/smp.c
new file mode 100644
index ..9fbc1ca1
--- /dev/null
+++ b/i386/i386/smp.c
@@ -0,0 +1,70 @@
+/* smp.h - i386 SMP controller for Mach
+   Copyright (C) 2020 Free Software Foundation, Inc.
+   Written by Almudena Garcia Jurado-Centurion
+
+   This file is part of GNU Mach.
+
+   GNU Mach is free software; you can redistribute it and/or modify it
+   under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 2, or (at your option)
+   any later version.
+
+   GNU Mach is distributed in the hope that it will be useful, but
+   WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program; if not, write to the Free Software
+   Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA  02111, USA. */
+
+#include 
+#include 
+#include 
+
+#include 
+
+struct smp_data smp_info;
+
+/*
+ * smp_data_init: initialize smp_data structure
+ * Must be called after smp_init(), once all APIC structures
+ * has been initialized
+ */
+void smp_data_init(void)
+{
+smp_info.num_cpus = apic_get_numcpus();
+}
+
+/*
+ * smp_get_current_cpu: return the hardware identifier (APIC ID in x86)
+ * of current CPU
+ */
+int smp_get_current_cpu(void)
+{
+return apic_get_current_cpu();
+}
+
+/*
+ * smp_init: initialize the SMP support, starting the cpus searching
+ * and enumeration.
+ */
+int smp_init(void)
+{
+int apic_success;
+
+apic_success = acpi_apic_init();
+if (apic_success) {
+smp_data_init();
+}
+
+return apic_success;
+}
+
+/*
+ * smp_get_numcpus: returns the number of cpus existing in the machine
+ */
+int smp_get_numcpus(void)
+{
+return smp_info.num_cpus;
+}
diff --git a/i386/i386/smp.h b/i386/i386/smp.h
new file mode 100644
index ..97684335
--- /dev/null
+++ b/i386/i386/smp.h
@@ -0,0 +1,25 @@
+/* smp.h - i386 SMP controller for Mach. Header file
+   Copyright (C) 2020 Free Software Foundation, Inc.
+   Written by Almudena Garcia Jurado-Centurion
+
+   This file is part of GNU Mach.
+
+   GNU Mach is free software; you can redistribut

Re: [PATCH] SMP initialization: detection and enumeration

[Almudena Garcia]

> You don't. Read about GDT and segments register, and segmentation in
> general. In the GDT you'd only store a pointer to the per-cpu data, and
> processors will load it.

> > Is there any way to avoid using an array for that?

> Yes, you'd just store the ID in the per-cpu data.

Thanks, I'll take notes about this

El jue., 30 jul. 2020 a las 23:35, Samuel Thibault ()
escribió:

> Almudena Garcia, le jeu. 30 juil. 2020 23:30:55 +0200, a ecrit:
> > > As Richard said, we just want to have different GDTs on the different
> > > processors, so that we wan use the fs segment register to implement TLS
> > > in the kernel and have per-cpu data cost essentially the same as global
> > > data.
> >
> > Yes, but how can I store the relation APIC ID - Kernel ID in the GDT?
>
> You don't. Read about GDT and segments register, and segmentation in
> general. In the GDT you'd only store a pointer to the per-cpu data, and
> processors will load it.
>
> > Is there any way to avoid using an array for that?
>
> Yes, you'd just store the ID in the per-cpu data.
>
> Samuel
>

Re: [PATCH] SMP initialization: detection and enumeration

[Samuel Thibault]

Almudena Garcia, le jeu. 30 juil. 2020 23:30:55 +0200, a ecrit:
> > As Richard said, we just want to have different GDTs on the different
> > processors, so that we wan use the fs segment register to implement TLS
> > in the kernel and have per-cpu data cost essentially the same as global
> > data.
> 
> Yes, but how can I store the relation APIC ID - Kernel ID in the GDT?

You don't. Read about GDT and segments register, and segmentation in
general. In the GDT you'd only store a pointer to the per-cpu data, and
processors will load it.

> Is there any way to avoid using an array for that?

Yes, you'd just store the ID in the per-cpu data.

Samuel

Re: [PATCH] SMP initialization: detection and enumeration

[Almudena Garcia]

> As Richard said, we just want to have different GDTs on the different
> processors, so that we wan use the fs segment register to implement TLS
> in the kernel and have per-cpu data cost essentially the same as global
> data.

Yes, but how can I store the relation APIC ID - Kernel ID in the GDT?
Is there any way to avoid using an array for that?

> (I don't mean it has to be implemented so for commiting to mainline,
> just saying how we can optimize after it)

I'm still learning yet, so many concepts are difficult for me.
My idea is to get a basic and functional SMP implementation, and then get
help from more experienced developers to improve it.



El jue., 30 jul. 2020 a las 23:21, Samuel Thibault ()
escribió:

> Samuel Thibault, le jeu. 30 juil. 2020 23:21:02 +0200, a ecrit:
> > As Richard said, we just want to have different GDTs on the different
> > processors, so that we wan use the fs segment register to implement TLS
> > in the kernel and have per-cpu data cost essentially the same as global
> > data.
>
> (I don't mean it has to be implemented so for commiting to mainline,
> just saying how we can optimize after it)
>
> Samuel
>
>

Re: [PATCH] SMP initialization: detection and enumeration

[Almudena Garcia]

> Well, yes, though you probably don't want to be using a fancy hash
> table in such low-level code, so you'd either have an array of 2^16 CPU
> IDs which is a bit absurd.
The maximum number of cpus in xAPIC is 256, not 2^16. My array is indexed
by Kernel ID.
My array is refitted to the exact number of cpus after enumerating them, to
save space.

El jue., 30 jul. 2020 a las 23:17, Jessica Clarke ()
escribió:

> On 30 Jul 2020, at 22:12, Richard Braun  wrote:
> > On Thu, Jul 30, 2020 at 10:09:01PM +0100, Jessica Clarke wrote:
> >>> It's physically memory mapped to the local APIC address space, but
> >>> because of that, it's also not optimal. All systems I know use a scheme
> >>> similar to TLS, i.e. using the fs or gs segment register, to fetch
> >>> a per-CPU structure and from it, per-CPU data. This avoids relying on
> >>> hardware running at a lower frequency than the CPU.
> >>
> >> You need to do that anyway if you want any guarantees over _what_ the
> >> IDs are (normally you want 0 for the BSP, 1 to N-1 for the APs).
> >
> > Not really. You can build a CPU ID table on the BSP, and associate CPU
> IDs
> > with all local APIC IDs discovered, and use that look-up table later
> > to obtain a CPU ID from the local APIC ID of the CPU running the code.
> > It's just too slow compared to using a segment register.
>
> Well, yes, though you probably don't want to be using a fancy hash
> table in such low-level code, so you'd either have an array of 2^16 CPU
> IDs which is a bit absurd (or at least _used_ to be, these days it's
> not that big) or just do some kind of search. I kind of ruled both out
> from the get-go because they're too absurd, especially on other systems
> where the APIC ID equivalent is 32-bit or 64-bit and you might have
> hundreds of cores' IDs to search through to find your own.
>
> Jess
>
>

Re: [PATCH] SMP initialization: detection and enumeration

[Samuel Thibault]

Samuel Thibault, le jeu. 30 juil. 2020 23:21:02 +0200, a ecrit:
> As Richard said, we just want to have different GDTs on the different
> processors, so that we wan use the fs segment register to implement TLS
> in the kernel and have per-cpu data cost essentially the same as global
> data.

(I don't mean it has to be implemented so for commiting to mainline,
just saying how we can optimize after it)

Samuel

Re: [PATCH] SMP initialization: detection and enumeration

[Samuel Thibault]

As Richard said, we just want to have different GDTs on the different
processors, so that we wan use the fs segment register to implement TLS
in the kernel and have per-cpu data cost essentially the same as global
data.

Samuel

Re: [PATCH] SMP initialization: detection and enumeration

[Almudena Garcia]

> You can build a CPU ID table on the BSP, and associate CPU IDs
> with all local APIC IDs discovered, and use that look-up table later
> to obtain a CPU ID from the local APIC ID of the CPU running the code.

It's my idea, of course. I don't know what you refer to exactly with "using
a segment register". How Does It Work?
Maybe, in next versions, I can improve the implementation using that.

El jue., 30 jul. 2020 a las 23:12, Richard Braun ()
escribió:

> On Thu, Jul 30, 2020 at 10:09:01PM +0100, Jessica Clarke wrote:
> > > It's physically memory mapped to the local APIC address space, but
> > > because of that, it's also not optimal. All systems I know use a scheme
> > > similar to TLS, i.e. using the fs or gs segment register, to fetch
> > > a per-CPU structure and from it, per-CPU data. This avoids relying on
> > > hardware running at a lower frequency than the CPU.
> >
> > You need to do that anyway if you want any guarantees over _what_ the
> > IDs are (normally you want 0 for the BSP, 1 to N-1 for the APs).
>
> Not really. You can build a CPU ID table on the BSP, and associate CPU IDs
> with all local APIC IDs discovered, and use that look-up table later
> to obtain a CPU ID from the local APIC ID of the CPU running the code.
> It's just too slow compared to using a segment register.
>
> --
> Richard Braun
>

Re: [PATCH] SMP initialization: detection and enumeration

[Jessica Clarke]

On 30 Jul 2020, at 22:12, Richard Braun  wrote:
> On Thu, Jul 30, 2020 at 10:09:01PM +0100, Jessica Clarke wrote:
>>> It's physically memory mapped to the local APIC address space, but
>>> because of that, it's also not optimal. All systems I know use a scheme
>>> similar to TLS, i.e. using the fs or gs segment register, to fetch
>>> a per-CPU structure and from it, per-CPU data. This avoids relying on
>>> hardware running at a lower frequency than the CPU.
>> 
>> You need to do that anyway if you want any guarantees over _what_ the
>> IDs are (normally you want 0 for the BSP, 1 to N-1 for the APs).
> 
> Not really. You can build a CPU ID table on the BSP, and associate CPU IDs
> with all local APIC IDs discovered, and use that look-up table later
> to obtain a CPU ID from the local APIC ID of the CPU running the code.
> It's just too slow compared to using a segment register.

Well, yes, though you probably don't want to be using a fancy hash
table in such low-level code, so you'd either have an array of 2^16 CPU
IDs which is a bit absurd (or at least _used_ to be, these days it's
not that big) or just do some kind of search. I kind of ruled both out
from the get-go because they're too absurd, especially on other systems
where the APIC ID equivalent is 32-bit or 64-bit and you might have
hundreds of cores' IDs to search through to find your own.

Jess

Re: [PATCH] SMP initialization: detection and enumeration

[Richard Braun]

On Thu, Jul 30, 2020 at 10:09:01PM +0100, Jessica Clarke wrote:
> > It's physically memory mapped to the local APIC address space, but
> > because of that, it's also not optimal. All systems I know use a scheme
> > similar to TLS, i.e. using the fs or gs segment register, to fetch
> > a per-CPU structure and from it, per-CPU data. This avoids relying on
> > hardware running at a lower frequency than the CPU.
> 
> You need to do that anyway if you want any guarantees over _what_ the
> IDs are (normally you want 0 for the BSP, 1 to N-1 for the APs).

Not really. You can build a CPU ID table on the BSP, and associate CPU IDs
with all local APIC IDs discovered, and use that look-up table later
to obtain a CPU ID from the local APIC ID of the CPU running the code.
It's just too slow compared to using a segment register.

-- 
Richard Braun

Re: [PATCH] SMP initialization: detection and enumeration

[Jessica Clarke]

On 30 Jul 2020, at 22:06, Richard Braun  wrote:
> On Thu, Jul 30, 2020 at 10:44:40PM +0200, Samuel Thibault wrote:
 I'm wondering: is it really *that* simple to get the current cpu number,
 just read a memory location?  I'm surprised that this would provide
 different results on different cpus.
>>> 
>>> The APIC ID is stored in the Local APIC of each cpu. This address is common 
>>> for
>>> all Local APIC: accessing this from each cpu, it shows the Local APIC of 
>>> this
>>> cpu.
>>> By example, if you access this address from cpu1, you can see the Local 
>>> APIC of
>>> cpu1.
>> 
>> So it's a special address whose accesses are trapped within the chip and
>> don't actually get out on the memory bus?
> 
> It's physically memory mapped to the local APIC address space, but
> because of that, it's also not optimal. All systems I know use a scheme
> similar to TLS, i.e. using the fs or gs segment register, to fetch
> a per-CPU structure and from it, per-CPU data. This avoids relying on
> hardware running at a lower frequency than the CPU.

You need to do that anyway if you want any guarantees over _what_ the
IDs are (normally you want 0 for the BSP, 1 to N-1 for the APs).

Jess

Re: [PATCH] SMP initialization: detection and enumeration

[Samuel Thibault]

Almudena Garcia, le jeu. 30 juil. 2020 22:59:02 +0200, a ecrit:
> The local APIC's registers are memory-mapped in physical page FEE00xxx (as 
> seen
> in table 8-1 of Intel P4 SPG). This address is the same for each local APIC
> that exists in a configuration, meaning you are only able to directly access
> the registers of the local APIC of the core that your code is currently
> executing on.

Ok, so it indeed seems so.

Samuel

Re: [PATCH] SMP initialization: detection and enumeration

[Richard Braun]

On Thu, Jul 30, 2020 at 10:44:40PM +0200, Samuel Thibault wrote:
> > > I'm wondering: is it really *that* simple to get the current cpu number,
> > > just read a memory location?  I'm surprised that this would provide
> > > different results on different cpus.
> > 
> > The APIC ID is stored in the Local APIC of each cpu. This address is common 
> > for
> > all Local APIC: accessing this from each cpu, it shows the Local APIC of 
> > this
> > cpu.
> > By example, if you access this address from cpu1, you can see the Local 
> > APIC of
> > cpu1.
> 
> So it's a special address whose accesses are trapped within the chip and
> don't actually get out on the memory bus?

It's physically memory mapped to the local APIC address space, but
because of that, it's also not optimal. All systems I know use a scheme
similar to TLS, i.e. using the fs or gs segment register, to fetch
a per-CPU structure and from it, per-CPU data. This avoids relying on
hardware running at a lower frequency than the CPU.

-- 
Richard Braun

Re: [PATCH] SMP initialization: detection and enumeration

[Almudena Garcia]

> That's alright while debugging. But before we commit the source code, we
> need it to be in a way which is the most readable, so people can easily
> work it out.
Ok, then I will modify my code this way.

> ? No. Introducing a variable, remembering that it holds a value, and
> eventually returning it, is much more brain overhead than just directly
> returning the value.
Same

Also, having a few returns early in the function, to eliminate the error
cases and trivial cases, allows to get rid of that from the mind, and
focus on the real stuff for the rest of the function.

> So it's a special address whose accesses are trapped within the chip and
> don't actually get out on the memory bus?
I don't know the technical details at all. In OSDev (
https://wiki.osdev.org/APIC) It explains this:


*The local APIC's registers are memory-mapped in physical page FEE00xxx (as
seen in table 8-1 of Intel P4 SPG). This address is the same for each local
APIC that exists in a configuration, meaning you are only able to directly
access the registers of the local APIC of the core that your code is
currently executing on. *

The physical address is not always the same: each machine can map the Local
APIC in a different address. By this reason, I search It in ACPI tables, in
MADT (APIC) table.


Thanks for your review :) . This weekend I will try to fix the code.

El jue., 30 jul. 2020 a las 22:44, Samuel Thibault ()
escribió:

> Almudena Garcia, le mar. 28 juil. 2020 10:27:55 +0200, a ecrit:
> > > Simply return rather than using a variable.
> > I prefer using a temporary variable to ease debugging in case of error.
> Direct
> > return can be difficult to debug in this case.
>
> That's alright while debugging. But before we commit the source code, we
> need it to be in a way which is the most readable, so people can easily
> work it out.
>
> > > You can just return it.
> > I prefer to avoid multiple return, to ease the reading. A single return
> can be
> > easy to read.
>
> ? No. Introducing a variable, remembering that it holds a value, and
> eventually returning it, is much more brain overhead than just directly
> returning the value.
>
> Also, having a few returns early in the function, to eliminate the error
> cases and trivial cases, allows to get rid of that from the mind, and
> focus on the real stuff for the rest of the function.
>
> > > So you picked it up from somewhere?
> > > Since it's a BSD license it is fine, just making sure that it is the
> > > proper copyright notice.
> >
> > This file existed in older versions of gnumach. Thomas removed It in
> 2007.
> > Simply, I recovered It and updated.
> > [1]
> http://git.savannah.gnu.org/cgit/hurd/gnumach.git/tree/i386/imps/apic.h?id=
> > f20666fc6b0471738829363e20c27f282f65dbf2
>
> Oh, interesting, it dates back so long ago :)
>
> > > I'm wondering: is it really *that* simple to get the current cpu
> number,
> > > just read a memory location?  I'm surprised that this would provide
> > > different results on different cpus.
> >
> > The APIC ID is stored in the Local APIC of each cpu. This address is
> common for
> > all Local APIC: accessing this from each cpu, it shows the Local APIC of
> this
> > cpu.
> > By example, if you access this address from cpu1, you can see the Local
> APIC of
> > cpu1.
>
> So it's a special address whose accesses are trapped within the chip and
> don't actually get out on the memory bus?
>
> Samuel
>

Re: [PATCH] SMP initialization: detection and enumeration

[Samuel Thibault]

Almudena Garcia, le mar. 28 juil. 2020 10:27:55 +0200, a ecrit:
> > Simply return rather than using a variable.
> I prefer using a temporary variable to ease debugging in case of error. Direct
> return can be difficult to debug in this case.

That's alright while debugging. But before we commit the source code, we
need it to be in a way which is the most readable, so people can easily
work it out.

> > You can just return it.
> I prefer to avoid multiple return, to ease the reading. A single return can be
> easy to read.

? No. Introducing a variable, remembering that it holds a value, and
eventually returning it, is much more brain overhead than just directly
returning the value.

Also, having a few returns early in the function, to eliminate the error
cases and trivial cases, allows to get rid of that from the mind, and
focus on the real stuff for the rest of the function.

> > So you picked it up from somewhere?
> > Since it's a BSD license it is fine, just making sure that it is the
> > proper copyright notice.
> 
> This file existed in older versions of gnumach. Thomas removed It in 2007.
> Simply, I recovered It and updated.
> [1]http://git.savannah.gnu.org/cgit/hurd/gnumach.git/tree/i386/imps/apic.h?id=
> f20666fc6b0471738829363e20c27f282f65dbf2

Oh, interesting, it dates back so long ago :)

> > I'm wondering: is it really *that* simple to get the current cpu number,
> > just read a memory location?  I'm surprised that this would provide
> > different results on different cpus.
> 
> The APIC ID is stored in the Local APIC of each cpu. This address is common 
> for
> all Local APIC: accessing this from each cpu, it shows the Local APIC of this
> cpu.
> By example, if you access this address from cpu1, you can see the Local APIC 
> of
> cpu1.

So it's a special address whose accesses are trapped within the chip and
don't actually get out on the memory bus?

Samuel

Re: [PATCH] SMP initialization: detection and enumeration

[Almudena Garcia]

> @@ -170,6 +172,14 @@ void machine_init(void)
>   linux_init();
>  #endif
>
> +#if NCPUS > 1
> + int smp_success = smp_init();
> +
> + if(smp_success != 0) {
> +printf("Error: no SMP found");
> +}
> +#endif /* NCPUS > 1 */
> +

> There's a bogus indent here. Tell your editor to show tab vs spaces to
> avoid introducing incoherent combinations.

Thanks. I'll fix It. I'm using CodeBlocks as editor, and this doesn't show
the tabs properly.

> Rather fold this into the commits that add these files. Otherwise the
> code is not even getting compiled when checking out the other commits.

Ok, I'll add It.

> Juan didn't assign copyright to the FSF. We usually do this, so we don't
> have long-term licensing concerns. Could you send his email address so
> we can send him the paper work? Alternatively the file could be put
> under a BSD license, which poses way less concerns.

Ok. I will tell him to solve this.

> +#ifdef __i386__
> +#include 
> +#include 

> We avoid putting arch-specific code in kern/, and rather put it in
> i386/i386. Note that you can have smp.h in i386/i386, and make other
> files include 

Ok. Then I will fix It. I will add an i386/i386/smp.h to add these includes.

> > +static int acpi_apic_parse_table(struct acpi_apic *apic);
> This doesn't exist?

It's possible. Maybe I renamed this function, and I forget to rename its
declaration (or the inverse). I think I will rename the definition: It's
more coherent.

> @@ -46,9 +46,12 @@ AC_DEFINE([BOOTSTRAP_SYMBOLS], [0],
[BOOTSTRAP_SYMBOLS])
>  # Multiprocessor support is still broken.
>  AH_TEMPLATE([MULTIPROCESSOR], [set things up for a uniprocessor])
>  mach_ncpus=1
> +AC_DEFINE_UNQUOTED([NCPUS], [$mach_ncpus], [number of CPUs])
> +
>  AC_DEFINE_UNQUOTED([NCPUS], [$mach_ncpus], [number of CPUs])

> Err, this seems spurious?

Yes, It's a mistake. You can remove the duplicated line.

> Simply return rather than using a variable.
I prefer using a temporary variable to ease debugging in case of error.
Direct return can be difficult to debug in this case.

> You can just return it.
I prefer to avoid multiple return, to ease the reading. A single return can
be easy to read.

> That's too verbose for production?
I can remove the prints, If necessary.

> So you picked it up from somewhere?
> Since it's a BSD license it is fine, just making sure that it is the
> proper copyright notice.

This file existed in older versions of gnumach. Thomas removed It in 2007.
Simply, I recovered It and updated.
http://git.savannah.gnu.org/cgit/hurd/gnumach.git/tree/i386/imps/apic.h?id=f20666fc6b0471738829363e20c27f282f65dbf2


> Rather put the comment on the right of the corresponding field, that will
look much better.
Ok, I'll fix It.

> Ditto, invert the if to avoid indentation. Also notice the memory leak
> in that case, you'll want to make the kalloc only in the success case.

> +for (i = 0; i < apic_data.ncpus; i++)
> +new_list[i] = old_list[i];
> +
> +apic_data.cpu_lapic_list = new_list;
> +kfree(old_list);

Thanks, I'll solve this.

> I'm wondering: is it really *that* simple to get the current cpu number,
> just read a memory location?  I'm surprised that this would provide
> different results on different cpus.

The APIC ID is stored in the Local APIC of each cpu. This address is common
for all Local APIC: accessing this from each cpu, it shows the Local APIC
of this cpu.
By example, if you access this address from cpu1, you can see the Local
APIC of cpu1.

About the rest of the comments, I will try to fix it soon.

Thanks


El mar., 28 jul. 2020 a las 1:45, Samuel Thibault ()
escribió:

> Hello,
>
> Thanks for your work, we're getting closer!
>
> Almudena Garcia, le lun. 27 juil. 2020 21:15:10 +0200, a ecrit:
> > @@ -170,6 +172,14 @@ void machine_init(void)
> >   linux_init();
> >  #endif
> >
> > +#if NCPUS > 1
> > + int smp_success = smp_init();
> > +
> > + if(smp_success != 0) {
> > +printf("Error: no SMP found");
> > +}
> > +#endif /* NCPUS > 1 */
> > +
>
> There's a bogus indent here. Tell your editor to show tab vs spaces to
> avoid introducing incoherent combinations.
>
> > Add smp.c, smp.h, apic.c, apic.h, acpi_parse_apic.c and
> acpi_parse_apic,h to compilation list
> > ---
> >  Makefrag.am | 12 
> >  1 file changed, 12 insertions(+)
> >
> > diff --git a/Makefrag.am b/Makefrag.am
> > index ea612275..69ac31a1 100644
> > --- a/Makefrag.am
> > +++ b/Makefrag.am
> > @@ -122,6 +122,18 @@ EXTRA_DIST += \
> >   ipc/notify.defs
> >
> >
> > +#
> > +# SMP implementation (APIC, ACPI, etc)
> > +#
> > +
> > +libkernel_a_SOURCES += \
> > + i386/i386at/acpi_parse_apic.h \
> > + i386/i386at/acpi_parse_apic.c \
> > + i386/i386/apic.h \
> > + i386/i386/apic.c \
> > + kern/smp.h \
> > + kern/smp.c
>
> Rather fold this into the commits that add these files. Otherwise the
> code is not even getting compiled when checking out the other commits.
>
> > -

Re: [PATCH] SMP initialization: detection and enumeration

[Almudena Garcia]

> 256 is a common default
> (I know that's what FreeBSD uses for x86) as the APIC IDs are 8-bit,
> and I think this should be 256 rather than 255 too
I agree. It's a mistake.

> Though I do
> question what the point of mach_ncpus is if it isn't being used to
> determine NCPUS.

Currently, mach_ncpus is used to define NCPUS. I keep It, because It can be
useful to enable/disable the SMP support, and this patch is low-invasive,
avoiding to broke more things.

> AC_DEFINE_UNQUOTED([NCPUS], [$mach_ncpus], [number of CPUs])
This define can be removed, of course. I don't know because I keep It
there.

>> +return apic_id;

This line doesn't get the Kernel ID, but the APIC ID.
The APIC ID can be obtained from the common Local APIC address, which
points to the Local APIC of the current CPU (if you access this address
from cpu1, you get the APIC ID of cpu1).

The ACPI tables also stores the APIC ID of each CPU, so I enumerate the
processors in an array using this. The array is indexed by Kernel ID (the
logical ID), and stores the APIC ID for each cpu.



El mar., 28 jul. 2020 a las 2:19, Jessica Clarke ()
escribió:

> On 28 Jul 2020, at 00:44, Samuel Thibault  wrote:
> > Almudena Garcia, le lun. 27 juil. 2020 21:15:10 +0200, a ecrit:
> >> [if [ $mach_ncpus -gt 1 ]; then]
> >>   AC_DEFINE([MULTIPROCESSOR], [1], [set things up for a multiprocessor])
> >> +  AC_DEFINE_UNQUOTED([NCPUS], [255], [number of CPUs])
> >
> > Perhaps useless to define to so many by default?
>
> That's fairly normal, having some static per-CPU data structures with
> an upper limit on the number of CPUs supported. 256 is a common default
> (I know that's what FreeBSD uses for x86) as the APIC IDs are 8-bit,
> and I think this should be 256 rather than 255 too. Though I do
> question what the point of mach_ncpus is if it isn't being used to
> determine NCPUS. Either always do `AC_DEFINE_UNQUOTED([NCPUS], [255],
> [number of CPUs])` (in which case you don't need the extra define) or
> make it mach_smp instead. Also, I personally don't like having two
> "calls" to AC_DEFINE_UNQUOTED; I know it's autoconf and m4 and all that
> so calls aren't always quite what you think, but it still just looks
> weird and confusing.
>
> >> +/*
> >> + * apic_get_current_cpu: returns the apic_id of current cpu.
> >> + */
> >> +uint16_t
> >> +apic_get_current_cpu(void)
> >> +{
> >> +uint16_t apic_id;
> >> +
> >> +if(lapic == NULL)
>
> You have a bunch of statements like this with missing whitespace.
>
> >> +return apic_id;
> >
> > I'm wondering: is it really *that* simple to get the current cpu number,
> > just read a memory location?  I'm surprised that this would provide
> > different results on different cpus.
>
> No, it's not. They can be non-sequential, and there's no guarantee that
> the BSP's is even 0, but there'll be a bunch of simple chips where both
> are true (especially in emulators etc). Normally the current CPU's ID
> would be stored in per-CPU data, and each AP is told (or knows how to
> look up based on its hardware ID) its logical ID when being brought
> online.
>
> Jess
>
>

Re: [PATCH] SMP initialization: detection and enumeration

[Jessica Clarke]

On 28 Jul 2020, at 00:44, Samuel Thibault  wrote:
> Almudena Garcia, le lun. 27 juil. 2020 21:15:10 +0200, a ecrit:
>> [if [ $mach_ncpus -gt 1 ]; then]
>>   AC_DEFINE([MULTIPROCESSOR], [1], [set things up for a multiprocessor])
>> +  AC_DEFINE_UNQUOTED([NCPUS], [255], [number of CPUs])
> 
> Perhaps useless to define to so many by default?

That's fairly normal, having some static per-CPU data structures with
an upper limit on the number of CPUs supported. 256 is a common default
(I know that's what FreeBSD uses for x86) as the APIC IDs are 8-bit,
and I think this should be 256 rather than 255 too. Though I do
question what the point of mach_ncpus is if it isn't being used to
determine NCPUS. Either always do `AC_DEFINE_UNQUOTED([NCPUS], [255],
[number of CPUs])` (in which case you don't need the extra define) or
make it mach_smp instead. Also, I personally don't like having two
"calls" to AC_DEFINE_UNQUOTED; I know it's autoconf and m4 and all that
so calls aren't always quite what you think, but it still just looks
weird and confusing.

>> +/*
>> + * apic_get_current_cpu: returns the apic_id of current cpu.
>> + */
>> +uint16_t
>> +apic_get_current_cpu(void)
>> +{
>> +uint16_t apic_id;
>> +
>> +if(lapic == NULL)

You have a bunch of statements like this with missing whitespace.

>> +return apic_id;
> 
> I'm wondering: is it really *that* simple to get the current cpu number,
> just read a memory location?  I'm surprised that this would provide
> different results on different cpus.

No, it's not. They can be non-sequential, and there's no guarantee that
the BSP's is even 0, but there'll be a bunch of simple chips where both
are true (especially in emulators etc). Normally the current CPU's ID
would be stored in per-CPU data, and each AP is told (or knows how to
look up based on its hardware ID) its logical ID when being brought
online.

Jess

Re: [PATCH] SMP initialization: detection and enumeration

[Samuel Thibault]

Hello,

Thanks for your work, we're getting closer!

Almudena Garcia, le lun. 27 juil. 2020 21:15:10 +0200, a ecrit:
> @@ -170,6 +172,14 @@ void machine_init(void)
>   linux_init();
>  #endif
>  
> +#if NCPUS > 1
> + int smp_success = smp_init();
> +
> + if(smp_success != 0) {
> +printf("Error: no SMP found");
> +}
> +#endif /* NCPUS > 1 */
> +

There's a bogus indent here. Tell your editor to show tab vs spaces to
avoid introducing incoherent combinations.

> Add smp.c, smp.h, apic.c, apic.h, acpi_parse_apic.c and acpi_parse_apic,h to 
> compilation list
> ---
>  Makefrag.am | 12 
>  1 file changed, 12 insertions(+)
> 
> diff --git a/Makefrag.am b/Makefrag.am
> index ea612275..69ac31a1 100644
> --- a/Makefrag.am
> +++ b/Makefrag.am
> @@ -122,6 +122,18 @@ EXTRA_DIST += \
>   ipc/notify.defs
>  
>  
> +#
> +# SMP implementation (APIC, ACPI, etc)
> +#
> +
> +libkernel_a_SOURCES += \
> + i386/i386at/acpi_parse_apic.h \
> + i386/i386at/acpi_parse_apic.c \
> + i386/i386/apic.h \
> + i386/i386/apic.c \
> + kern/smp.h \
> + kern/smp.c

Rather fold this into the commits that add these files. Otherwise the
code is not even getting compiled when checking out the other commits.

> --- /dev/null
> +++ b/kern/smp.c
> @@ -0,0 +1,72 @@

> +#ifdef __i386__
> +#include 
> +#include 

We avoid putting arch-specific code in kern/, and rather put it in
i386/i386. Note that you can have smp.h in i386/i386, and make other
files include 

> diff --git a/i386/i386at/acpi_parse_apic.c b/i386/i386at/acpi_parse_apic.c
> new file mode 100644
> index ..4b579c5d
> --- /dev/null
> +++ b/i386/i386at/acpi_parse_apic.c
> @@ -0,0 +1,567 @@
> +/*
> + * Copyright 2018 Juan Bosco Garcia

Juan didn't assign copyright to the FSF. We usually do this, so we don't
have long-term licensing concerns. Could you send his email address so
we can send him the paper work? Alternatively the file could be put
under a BSD license, which poses way less concerns.

> + * Copyright 2018 2019 2020 Almudena Garcia Jurado-Centurion
> + * This file is part of Min_SMP.
> + * Min_SMP is free software: you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation, either version 2 of the License, or
> + * (at your option) any later version.
> + * Min_SMP is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + * You should have received a copy of the GNU General Public License
> + * along with Min_SMP.  If not, see .
> + */

Please use the same formatting as other GPL-licensed files. You did well
in the smp.[ch] files for instance.

> +#include 
> +#include  //memcmp, memcpy...
> +
> +#include  //lapic, ioapic...
> +#include  //printf
> +#include  //uint16_t, uint32_t...
> +#include  //machine_slot
> +#include  //phystokv
> +#include 
> +#include 

We usually group includes by directory, see how other files do it.

> +#define BAD_CHECKSUM -1
> +#define NO_RSDP -2
> +#define NO_RSDT -2
> +#define BAD_SIGNATURE -3
> +#define NO_APIC -4

Rather use an enum, which you will then have to put in the .h file,
which makes sense anyway. Prepend the names with ACPI_, and add an
ACPI_SUCCESS equal to 0, so you can write return ACPI_SUCCESS; instead
of return 0;

> +struct acpi_apic *apic_madt = NULL;

Make it static?

> +static struct acpi_rsdp* acpi_get_rsdp(void);
> +static int acpi_check_rsdt(struct acpi_rsdt *);
> +static struct acpi_rsdt* acpi_get_rsdt(struct acpi_rsdp *rsdp, int* 
> acpi_rsdt_n);
> +static struct acpi_apic* acpi_get_apic(struct acpi_rsdt *rsdt, int 
> acpi_rsdt_n);
> +static int acpi_apic_setup(struct acpi_apic *apic);
> +static int acpi_apic_add_ioapic(struct acpi_apic_ioapic *ioapic_entry);
> +static int acpi_apic_add_lapic(struct acpi_apic_lapic *lapic_entry);

These forward declarations don't seem actually needed: you can just put
acpi_apic_init at the end of the file.

> +static int acpi_apic_parse_table(struct acpi_apic *apic);

This doesn't exist?

> +static int
> +acpi_checksum(void *addr, uint32_t length)
> +{
> +char *bytes = addr;
> +int checksum = 0;
> +unsigned int i;
> +
> +/* Sum all bytes of addr */
> +for (i = 0; i < length; i++)
> +checksum += bytes[i];
> +
> +return checksum & 0xff;
> +}

Rather use uint8_t everywhere here, even for the returned value. You
won't even need to use & 0xff then.

> +static int
> +acpi_check_rsdp(struct acpi_rsdp *rsdp)
> +{
> +uint32_t checksum;
> +int is_rsdp;
> +
> +int ret_value = 0;
> +
> +/* Check the integrity of RSDP. */
> +if (rsdp == NULL)
> +ret_value = NO_RSDP;

Simply rather "return NO_RSDP;"? That'll even avoid the "else".

> +else {
> +/* Check if rsdp signat

Re: [PATCH] SMP initialization: detection and enumeration

[Almudena Garcia]

New patches collection, fixing the formatting

El lun., 20 jul. 2020 a las 0:35, Almudena Garcia (<
liberamenso10...@gmail.com>) escribió:

> Here are the fixed patches. Check this and advise me if there are any
> errors.
>
> El dom., 19 jul. 2020 a las 23:53, Almudena Garcia (<
> liberamenso10...@gmail.com>) escribió:
>
>> > Note: before doing rebases etc. I'd advise to push your current tree to
>> > a separate branch that you can always restart from, to avoid risking
>> > losing everything. There are ways to restore things, but it's way easier
>> > to just keep a branch on the side where you know your changes are safe.
>>
>> At first, I'm doing manually, without rebase. My commit history is very
>> dirty, and the commit squash is complicated.
>> I have a clone of the "master" branch from upstream gnumach, and I'm
>> creating a new branch derived from It, in which I'm adding manually the
>> latest changes to commit.
>>
>> I'll late a bit
>>
>> El dom., 19 jul. 2020 a las 23:48, Samuel Thibault (<
>> samuel.thiba...@gnu.org>) escribió:
>>
>>> Note: before doing rebases etc. I'd advise to push your current tree to
>>> a separate branch that you can always restart from, to avoid risking
>>> losing everything. There are ways to restore things, but it's way easier
>>> to just keep a branch on the side where you know your changes are safe.
>>>
>>> Samuel
>>>
>>
From adaeaba16c56ae05b5573f263715ae146a3bbf91 Mon Sep 17 00:00:00 2001
From: Almudena Garcia 
Date: Mon, 27 Jul 2020 20:02:59 +0200
Subject: [PATCH 6/7] model_dep.c: Add smp_init call

if NCPUS > 1, call to smp_init to start the search and enumeration of the cpus

*i386/i386/model_dep.c (machine_init): add smp_init() call
---
 i386/i386at/model_dep.c | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/i386/i386at/model_dep.c b/i386/i386at/model_dep.c
index aaeed807..75a902d5 100644
--- a/i386/i386at/model_dep.c
+++ b/i386/i386at/model_dep.c
@@ -72,6 +72,8 @@
 #include 
 #include 
 #include 
+#include 
+
 #ifdef	MACH_XEN
 #include 
 #include 
@@ -170,6 +172,14 @@ void machine_init(void)
 	linux_init();
 #endif
 
+#if NCPUS > 1
+	int smp_success = smp_init();
+
+	if(smp_success != 0) {
+printf("Error: no SMP found");
+}
+#endif /* NCPUS > 1 */
+
 	/*
 	 * Find the devices
 	 */
-- 
2.27.0

From 4e963d00b34dc96f0473dc65726d4b29649efae3 Mon Sep 17 00:00:00 2001
From: Almudena Garcia 
Date: Mon, 27 Jul 2020 20:03:35 +0200
Subject: [PATCH 7/7] Add SMP files to compilation list

Add smp.c, smp.h, apic.c, apic.h, acpi_parse_apic.c and acpi_parse_apic,h to compilation list
---
 Makefrag.am | 12 
 1 file changed, 12 insertions(+)

diff --git a/Makefrag.am b/Makefrag.am
index ea612275..69ac31a1 100644
--- a/Makefrag.am
+++ b/Makefrag.am
@@ -122,6 +122,18 @@ EXTRA_DIST += \
 	ipc/notify.defs
 
 
+#
+# SMP implementation (APIC, ACPI, etc)
+#
+
+libkernel_a_SOURCES += \
+	i386/i386at/acpi_parse_apic.h \
+	i386/i386at/acpi_parse_apic.c \
+	i386/i386/apic.h \
+	i386/i386/apic.c \
+	kern/smp.h \
+	kern/smp.c
+
 #
 # `kernel' implementation (tasks, threads, trivia, etc.).
 #
-- 
2.27.0

From 371664ba6c77c48fd85c349a527d5d22c930b9d9 Mon Sep 17 00:00:00 2001
From: Almudena Garcia 
Date: Mon, 27 Jul 2020 19:59:00 +0200
Subject: [PATCH 5/7] smp: Add generic smp pseudoclass

This pseudoclass generalize the initialization and access of SMP data,
allowing expands it to other architectures. In x86, the functions calls to apic functions.

*kern/smp.c: Source file. Implements some functions to get the number of cpus,
and get current cpu.
*kern/smp.h: Header file. Add declarations of functions and structures.
---
 kern/smp.c | 72 ++
 kern/smp.h | 28 +
 2 files changed, 100 insertions(+)
 create mode 100644 kern/smp.c
 create mode 100644 kern/smp.h

diff --git a/kern/smp.c b/kern/smp.c
new file mode 100644
index ..a6aae2d2
--- /dev/null
+++ b/kern/smp.c
@@ -0,0 +1,72 @@
+/* smp.c - Template for generic SMP controller for Mach.
+   Copyright (C) 2020 Free Software Foundation, Inc.
+   Written by Almudena Garcia Jurado-Centurion
+
+   This file is part of GNU Mach.
+
+   GNU Mach is free software; you can redistribute it and/or modify it
+   under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 2, or (at your option)
+   any later version.
+
+   GNU Mach is distributed in the hope that it will be useful, but
+   WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program; if not, write to the Free Software
+   Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA  02111, USA. */
+
+#include 
+
+struct smp_data smp_info;
+
+#ifdef __i386__
+#include 
+#include 
+
+/*
+ * smp_data_init: initialize sm

Re: [PATCH] SMP initialization: detection and enumeration

[Almudena Garcia]

Here are the fixed patches. Check this and advise me if there are any
errors.

El dom., 19 jul. 2020 a las 23:53, Almudena Garcia (<
liberamenso10...@gmail.com>) escribió:

> > Note: before doing rebases etc. I'd advise to push your current tree to
> > a separate branch that you can always restart from, to avoid risking
> > losing everything. There are ways to restore things, but it's way easier
> > to just keep a branch on the side where you know your changes are safe.
>
> At first, I'm doing manually, without rebase. My commit history is very
> dirty, and the commit squash is complicated.
> I have a clone of the "master" branch from upstream gnumach, and I'm
> creating a new branch derived from It, in which I'm adding manually the
> latest changes to commit.
>
> I'll late a bit
>
> El dom., 19 jul. 2020 a las 23:48, Samuel Thibault (<
> samuel.thiba...@gnu.org>) escribió:
>
>> Note: before doing rebases etc. I'd advise to push your current tree to
>> a separate branch that you can always restart from, to avoid risking
>> losing everything. There are ways to restore things, but it's way easier
>> to just keep a branch on the side where you know your changes are safe.
>>
>> Samuel
>>
>
From 2c65b97a09aec2ad8915606fec38bd5fabe3df1c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Almudena=20Garc=C3=ADa?= 
Date: Mon, 20 Jul 2020 00:32:14 +0200
Subject: [PATCH 7/7] Add SMP files to compilation list

Add smp.c, smp.h, apic.c, apic.h, acpi_parse_apic.c and acpi_parse_apic,h to compilation list
---
 Makefrag.am | 12 
 1 file changed, 12 insertions(+)

diff --git a/Makefrag.am b/Makefrag.am
index ea612275..69ac31a1 100644
--- a/Makefrag.am
+++ b/Makefrag.am
@@ -122,6 +122,18 @@ EXTRA_DIST += \
 	ipc/notify.defs
 
 
+#
+# SMP implementation (APIC, ACPI, etc)
+#
+
+libkernel_a_SOURCES += \
+	i386/i386at/acpi_parse_apic.h \
+	i386/i386at/acpi_parse_apic.c \
+	i386/i386/apic.h \
+	i386/i386/apic.c \
+	kern/smp.h \
+	kern/smp.c
+
 #
 # `kernel' implementation (tasks, threads, trivia, etc.).
 #
-- 
2.27.0

From c5670393c5ce7c6f6a39bd669c4a6f8487510068 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Almudena=20Garc=C3=ADa?= 
Date: Mon, 20 Jul 2020 00:16:11 +0200
Subject: [PATCH 5/7] smp: Add generic smp pseudoclass

This pseudoclass generalize the initialization and access of SMP data,
allowing expands it to other architectures. In x86, the functions calls to apic functions.

*kern/smp.c: Source file. Implements some functions to get the number of cpus,
and get current cpu.
*kern/smp.h: Header file. Add declarations of functions and structures.
---
 kern/smp.c | 68 ++
 kern/smp.h | 28 ++
 2 files changed, 96 insertions(+)
 create mode 100644 kern/smp.c
 create mode 100644 kern/smp.h

diff --git a/kern/smp.c b/kern/smp.c
new file mode 100644
index ..3e0e4323
--- /dev/null
+++ b/kern/smp.c
@@ -0,0 +1,68 @@
+/* smp.c - Template for generic SMP controller for Mach.
+   Copyright (C) 2020 Free Software Foundation, Inc.
+   Written by Almudena Garcia Jurado-Centurion
+
+   This file is part of GNU Mach.
+
+   GNU Mach is free software; you can redistribute it and/or modify it
+   under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 2, or (at your option)
+   any later version.
+
+   GNU Mach is distributed in the hope that it will be useful, but
+   WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program; if not, write to the Free Software
+   Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA  02111, USA. */
+
+#include 
+
+struct smp_data smp_info;
+
+#ifdef __i386__
+#include 
+#include 
+
+/* smp_data_init: initialize smp_data structure
+ * Must be called after smp_init(), once all APIC structures
+ * has been initialized
+ */
+
+void smp_data_init(void)
+{
+smp_info.num_cpus = apic_get_numcpus();
+}
+
+/* smp_get_numcpus: returns the number of cpus existing in the machine */
+
+int smp_get_numcpus(void)
+{
+return smp_info.num_cpus;
+}
+
+/* smp_get_current_cpu: return the hardware identifier (APIC ID in x86) of current CPU */
+
+int smp_get_current_cpu(void)
+{
+return apic_get_current_cpu();
+}
+
+/* smp_init: initialize the SMP support, starting the cpus searching
+ *  and enumeration */
+
+int smp_init(void)
+{
+int apic_success;
+
+apic_success = acpi_apic_init();
+if (apic_success) {
+smp_data_init();
+}
+
+return apic_success;
+}
+
+#endif
diff --git a/kern/smp.h b/kern/smp.h
new file mode 100644
index ..031e5bac
--- /dev/null
+++ b/kern/smp.h
@@ -0,0 +1,28 @@
+/* smp.h - Template for generic SMP controller for Mach. Header file
+   Copyright (C) 2020 Free Software Foundation, Inc.
+   Written by Almudena Garc

Re: [PATCH] SMP initialization: detection and enumeration

[Joshua Branson]

I also find it really helpful to use magit.  Emacs' plugin to help me do
git things.

This video is a short introduction for it:

https://www.youtube.com/watch?v=mtliRYQd0j4

--
Joshua Branson
Sent from Emacs and Gnus

Re: [PATCH] SMP initialization: detection and enumeration

[Almudena Garcia]

> Note: before doing rebases etc. I'd advise to push your current tree to
> a separate branch that you can always restart from, to avoid risking
> losing everything. There are ways to restore things, but it's way easier
> to just keep a branch on the side where you know your changes are safe.

At first, I'm doing manually, without rebase. My commit history is very
dirty, and the commit squash is complicated.
I have a clone of the "master" branch from upstream gnumach, and I'm
creating a new branch derived from It, in which I'm adding manually the
latest changes to commit.

I'll late a bit

El dom., 19 jul. 2020 a las 23:48, Samuel Thibault ()
escribió:

> Note: before doing rebases etc. I'd advise to push your current tree to
> a separate branch that you can always restart from, to avoid risking
> losing everything. There are ways to restore things, but it's way easier
> to just keep a branch on the side where you know your changes are safe.
>
> Samuel
>

Re: [PATCH] SMP initialization: detection and enumeration

[Samuel Thibault]

Note: before doing rebases etc. I'd advise to push your current tree to
a separate branch that you can always restart from, to avoid risking
losing everything. There are ways to restore things, but it's way easier
to just keep a branch on the side where you know your changes are safe.

Samuel

Re: [PATCH] SMP initialization: detection and enumeration

[Ricardo Wurmus]

Almudena Garcia  writes:

> But I've already committed these changes in my repository. How can I
> recommit them?

You can change commits with “git rebase”.  Use “git rebase -i” for
interactive rebasing, which gives you a list of commits to which you can
apply changes.  If you want to change all commits and their contents you
might also just reset to a commit before your changes and then re-stage
changes as needed from your worktree.

> Added to this, I need to generate the patch using the "master" branch
> (which points to gnumach's upstream) to compare with the mine.

You can rebase your changes on top of the master branch of the gnumach
repository.  Add the gnumach repository as a remote with “git remote add
upstream https://…” and then do “git fetch upstream; git rebase
upstream/master” to rebase your commits on top of that remote’s master
branch.

-- 
Ricardo

Re: [PATCH] SMP initialization: detection and enumeration

[Jessica Clarke]

On 19 Jul 2020, at 20:07, Almudena Garcia  wrote:
> 
> But I've already committed these changes in my repository. How can I recommit 
> them? 
> Added to this, I need to generate the patch using the "master" branch (which 
> points to gnumach's upstream) to compare with the mine.
> 
> How can I solve this?

Look up git rebase.

> El dom., 19 jul. 2020 a las 20:51, Almudena Garcia 
> () escribió:
> ok. I had splitted It manually. Now I'll try again this way.
> 
> El dom., 19 jul. 2020 a las 20:49, Jessica Clarke () 
> escribió:
> On 19 Jul 2020, at 19:46, Almudena Garcia  wrote:
> > 
> > > for any patch it’s best to not just show a single large diff but to
> > > split the changes into logically related commits
> > I've just split the patch. I enumerated them following the dependencies 
> > order. 
> > 
> > > The commit
> > > message should describe the changes in a GNU-style ChangeLog format; you
> > > may also add additional descriptions.  
> > 
> > Where can I add this information? In the same patch or in a log file?
> 
> In the commit message. Then use git format-patch to create the patch
> files, not git diff/show. Personally I also find it easier when people
> use git send-email as then the patches are inline rather than as
> attachments, it makes it easier to reply and annotate.
> 
> Jess
> 
> > El dom., 19 jul. 2020 a las 20:23, Almudena Garcia 
> > () escribió:
> > Anyway, my patch is short. Maybe I can split It manually, taking care about 
> > dependencies between blocks. 
> > 
> > El dom., 19 jul. 2020 a las 20:17, Almudena Garcia 
> > () escribió:
> > Thanks for your explanation:
> > 
> > > To commit only some changes and not others you can select lines of
> > > interest with “git add -p” (or similar).  Once all connected changes
> > > have been staged you can commit them.  Do this repeatedly until you have
> > > a series of commits that are all small enough that a reviewer can
> > > understand them (and thus your thinking) at a glance.
> > 
> > I have already a commit list pushed in my GitHub repository. You can see It 
> > here: https://github.com/AlmuHS/GNUMach_SMP/commits/smp-new
> > But, in this case, my code is almost written from scratch, so It's complex 
> > to filter line by line.
> > The code only makes sense in a single piece. Otherway, the code doesn't 
> > compile or does nothing. 
> > 
> > > You can then turn that series of commits into a series of patches with
> > > “git format-patch”.  For example, “git format-patch -10” will generate
> > > 10 patch files from the last 10 commits.
> > 
> > Ok, I'll try this. But there are so many commits. 
> > 
> > 
> > El dom., 19 jul. 2020 a las 19:52, Ricardo Wurmus () 
> > escribió:
> > 
> > Hi,
> > 
> > for any patch it’s best to not just show a single large diff but to
> > split the changes into logically related commits.  You’re probably
> > working with Git, so the unit that we’re working with is a Git commit.
> > 
> > You should group related changes and commit them together.  The commit
> > message should describe the changes in a GNU-style ChangeLog format; you
> > may also add additional descriptions.  Here’s an example:
> > 
> > --8<---cut here---start->8---
> > kern: Frobnicate the jabberwocky.
> > 
> > In order to frobnicate the jabberwocky without confusion we only add the
> > core functionality here.
> > 
> > * kern/smp.c, kern/smp.h: New files.
> > * Makefrag.am (libkernel_a_SOURCES): Add them.
> > --8<---cut here---end--->8---
> > 
> > To commit only some changes and not others you can select lines of
> > interest with “git add -p” (or similar).  Once all connected changes
> > have been staged you can commit them.  Do this repeatedly until you have
> > a series of commits that are all small enough that a reviewer can
> > understand them (and thus your thinking) at a glance.
> > 
> > You can then turn that series of commits into a series of patches with
> > “git format-patch”.  For example, “git format-patch -10” will generate
> > 10 patch files from the last 10 commits.  You can attach these patches
> > to an email, or if you have configured “git send-email” correctly you
> > could send them directly via email to this list.  A reviewer can then
> > comment on each commit individually and apply them one by one if they
> > pass muster.
> > 
> > (This process is similar for most GNU packages.)
> > 
> > Hope this helps!
> > 
> > -- 
> > Ricardo
> 

Re: [PATCH] SMP initialization: detection and enumeration

[Almudena Garcia]

But I've already committed these changes in my repository. How can I
recommit them?
Added to this, I need to generate the patch using the "master" branch
(which points to gnumach's upstream) to compare with the mine.

How can I solve this?

El dom., 19 jul. 2020 a las 20:51, Almudena Garcia (<
liberamenso10...@gmail.com>) escribió:

> ok. I had splitted It manually. Now I'll try again this way.
>
> El dom., 19 jul. 2020 a las 20:49, Jessica Clarke ()
> escribió:
>
>> On 19 Jul 2020, at 19:46, Almudena Garcia 
>> wrote:
>> >
>> > > for any patch it’s best to not just show a single large diff but to
>> > > split the changes into logically related commits
>> > I've just split the patch. I enumerated them following the dependencies
>> order.
>> >
>> > > The commit
>> > > message should describe the changes in a GNU-style ChangeLog format;
>> you
>> > > may also add additional descriptions.
>> >
>> > Where can I add this information? In the same patch or in a log file?
>>
>> In the commit message. Then use git format-patch to create the patch
>> files, not git diff/show. Personally I also find it easier when people
>> use git send-email as then the patches are inline rather than as
>> attachments, it makes it easier to reply and annotate.
>>
>> Jess
>>
>> > El dom., 19 jul. 2020 a las 20:23, Almudena Garcia (<
>> liberamenso10...@gmail.com>) escribió:
>> > Anyway, my patch is short. Maybe I can split It manually, taking care
>> about dependencies between blocks.
>> >
>> > El dom., 19 jul. 2020 a las 20:17, Almudena Garcia (<
>> liberamenso10...@gmail.com>) escribió:
>> > Thanks for your explanation:
>> >
>> > > To commit only some changes and not others you can select lines of
>> > > interest with “git add -p” (or similar).  Once all connected changes
>> > > have been staged you can commit them.  Do this repeatedly until you
>> have
>> > > a series of commits that are all small enough that a reviewer can
>> > > understand them (and thus your thinking) at a glance.
>> >
>> > I have already a commit list pushed in my GitHub repository. You can
>> see It here: https://github.com/AlmuHS/GNUMach_SMP/commits/smp-new
>> > But, in this case, my code is almost written from scratch, so It's
>> complex to filter line by line.
>> > The code only makes sense in a single piece. Otherway, the code doesn't
>> compile or does nothing.
>> >
>> > > You can then turn that series of commits into a series of patches with
>> > > “git format-patch”.  For example, “git format-patch -10” will generate
>> > > 10 patch files from the last 10 commits.
>> >
>> > Ok, I'll try this. But there are so many commits.
>> >
>> >
>> > El dom., 19 jul. 2020 a las 19:52, Ricardo Wurmus ()
>> escribió:
>> >
>> > Hi,
>> >
>> > for any patch it’s best to not just show a single large diff but to
>> > split the changes into logically related commits.  You’re probably
>> > working with Git, so the unit that we’re working with is a Git commit.
>> >
>> > You should group related changes and commit them together.  The commit
>> > message should describe the changes in a GNU-style ChangeLog format; you
>> > may also add additional descriptions.  Here’s an example:
>> >
>> > --8<---cut here---start->8---
>> > kern: Frobnicate the jabberwocky.
>> >
>> > In order to frobnicate the jabberwocky without confusion we only add the
>> > core functionality here.
>> >
>> > * kern/smp.c, kern/smp.h: New files.
>> > * Makefrag.am (libkernel_a_SOURCES): Add them.
>> > --8<---cut here---end--->8---
>> >
>> > To commit only some changes and not others you can select lines of
>> > interest with “git add -p” (or similar).  Once all connected changes
>> > have been staged you can commit them.  Do this repeatedly until you have
>> > a series of commits that are all small enough that a reviewer can
>> > understand them (and thus your thinking) at a glance.
>> >
>> > You can then turn that series of commits into a series of patches with
>> > “git format-patch”.  For example, “git format-patch -10” will generate
>> > 10 patch files from the last 10 commits.  You can attach these patches
>> > to an email, or if you have configured “git send-email” correctly you
>> > could send them directly via email to this list.  A reviewer can then
>> > comment on each commit individually and apply them one by one if they
>> > pass muster.
>> >
>> > (This process is similar for most GNU packages.)
>> >
>> > Hope this helps!
>> >
>> > --
>> > Ricardo
>>
>>

Re: [PATCH] SMP initialization: detection and enumeration

[Almudena Garcia]

ok. I had splitted It manually. Now I'll try again this way.

El dom., 19 jul. 2020 a las 20:49, Jessica Clarke ()
escribió:

> On 19 Jul 2020, at 19:46, Almudena Garcia 
> wrote:
> >
> > > for any patch it’s best to not just show a single large diff but to
> > > split the changes into logically related commits
> > I've just split the patch. I enumerated them following the dependencies
> order.
> >
> > > The commit
> > > message should describe the changes in a GNU-style ChangeLog format;
> you
> > > may also add additional descriptions.
> >
> > Where can I add this information? In the same patch or in a log file?
>
> In the commit message. Then use git format-patch to create the patch
> files, not git diff/show. Personally I also find it easier when people
> use git send-email as then the patches are inline rather than as
> attachments, it makes it easier to reply and annotate.
>
> Jess
>
> > El dom., 19 jul. 2020 a las 20:23, Almudena Garcia (<
> liberamenso10...@gmail.com>) escribió:
> > Anyway, my patch is short. Maybe I can split It manually, taking care
> about dependencies between blocks.
> >
> > El dom., 19 jul. 2020 a las 20:17, Almudena Garcia (<
> liberamenso10...@gmail.com>) escribió:
> > Thanks for your explanation:
> >
> > > To commit only some changes and not others you can select lines of
> > > interest with “git add -p” (or similar).  Once all connected changes
> > > have been staged you can commit them.  Do this repeatedly until you
> have
> > > a series of commits that are all small enough that a reviewer can
> > > understand them (and thus your thinking) at a glance.
> >
> > I have already a commit list pushed in my GitHub repository. You can see
> It here: https://github.com/AlmuHS/GNUMach_SMP/commits/smp-new
> > But, in this case, my code is almost written from scratch, so It's
> complex to filter line by line.
> > The code only makes sense in a single piece. Otherway, the code doesn't
> compile or does nothing.
> >
> > > You can then turn that series of commits into a series of patches with
> > > “git format-patch”.  For example, “git format-patch -10” will generate
> > > 10 patch files from the last 10 commits.
> >
> > Ok, I'll try this. But there are so many commits.
> >
> >
> > El dom., 19 jul. 2020 a las 19:52, Ricardo Wurmus ()
> escribió:
> >
> > Hi,
> >
> > for any patch it’s best to not just show a single large diff but to
> > split the changes into logically related commits.  You’re probably
> > working with Git, so the unit that we’re working with is a Git commit.
> >
> > You should group related changes and commit them together.  The commit
> > message should describe the changes in a GNU-style ChangeLog format; you
> > may also add additional descriptions.  Here’s an example:
> >
> > --8<---cut here---start->8---
> > kern: Frobnicate the jabberwocky.
> >
> > In order to frobnicate the jabberwocky without confusion we only add the
> > core functionality here.
> >
> > * kern/smp.c, kern/smp.h: New files.
> > * Makefrag.am (libkernel_a_SOURCES): Add them.
> > --8<---cut here---end--->8---
> >
> > To commit only some changes and not others you can select lines of
> > interest with “git add -p” (or similar).  Once all connected changes
> > have been staged you can commit them.  Do this repeatedly until you have
> > a series of commits that are all small enough that a reviewer can
> > understand them (and thus your thinking) at a glance.
> >
> > You can then turn that series of commits into a series of patches with
> > “git format-patch”.  For example, “git format-patch -10” will generate
> > 10 patch files from the last 10 commits.  You can attach these patches
> > to an email, or if you have configured “git send-email” correctly you
> > could send them directly via email to this list.  A reviewer can then
> > comment on each commit individually and apply them one by one if they
> > pass muster.
> >
> > (This process is similar for most GNU packages.)
> >
> > Hope this helps!
> >
> > --
> > Ricardo
>
>

Re: [PATCH] SMP initialization: detection and enumeration

[Jessica Clarke]

On 19 Jul 2020, at 19:46, Almudena Garcia  wrote:
> 
> > for any patch it’s best to not just show a single large diff but to
> > split the changes into logically related commits
> I've just split the patch. I enumerated them following the dependencies 
> order. 
> 
> > The commit
> > message should describe the changes in a GNU-style ChangeLog format; you
> > may also add additional descriptions.  
> 
> Where can I add this information? In the same patch or in a log file?

In the commit message. Then use git format-patch to create the patch
files, not git diff/show. Personally I also find it easier when people
use git send-email as then the patches are inline rather than as
attachments, it makes it easier to reply and annotate.

Jess

> El dom., 19 jul. 2020 a las 20:23, Almudena Garcia 
> () escribió:
> Anyway, my patch is short. Maybe I can split It manually, taking care about 
> dependencies between blocks. 
> 
> El dom., 19 jul. 2020 a las 20:17, Almudena Garcia 
> () escribió:
> Thanks for your explanation:
> 
> > To commit only some changes and not others you can select lines of
> > interest with “git add -p” (or similar).  Once all connected changes
> > have been staged you can commit them.  Do this repeatedly until you have
> > a series of commits that are all small enough that a reviewer can
> > understand them (and thus your thinking) at a glance.
> 
> I have already a commit list pushed in my GitHub repository. You can see It 
> here: https://github.com/AlmuHS/GNUMach_SMP/commits/smp-new
> But, in this case, my code is almost written from scratch, so It's complex to 
> filter line by line.
> The code only makes sense in a single piece. Otherway, the code doesn't 
> compile or does nothing. 
> 
> > You can then turn that series of commits into a series of patches with
> > “git format-patch”.  For example, “git format-patch -10” will generate
> > 10 patch files from the last 10 commits.
> 
> Ok, I'll try this. But there are so many commits. 
> 
> 
> El dom., 19 jul. 2020 a las 19:52, Ricardo Wurmus () 
> escribió:
> 
> Hi,
> 
> for any patch it’s best to not just show a single large diff but to
> split the changes into logically related commits.  You’re probably
> working with Git, so the unit that we’re working with is a Git commit.
> 
> You should group related changes and commit them together.  The commit
> message should describe the changes in a GNU-style ChangeLog format; you
> may also add additional descriptions.  Here’s an example:
> 
> --8<---cut here---start->8---
> kern: Frobnicate the jabberwocky.
> 
> In order to frobnicate the jabberwocky without confusion we only add the
> core functionality here.
> 
> * kern/smp.c, kern/smp.h: New files.
> * Makefrag.am (libkernel_a_SOURCES): Add them.
> --8<---cut here---end--->8---
> 
> To commit only some changes and not others you can select lines of
> interest with “git add -p” (or similar).  Once all connected changes
> have been staged you can commit them.  Do this repeatedly until you have
> a series of commits that are all small enough that a reviewer can
> understand them (and thus your thinking) at a glance.
> 
> You can then turn that series of commits into a series of patches with
> “git format-patch”.  For example, “git format-patch -10” will generate
> 10 patch files from the last 10 commits.  You can attach these patches
> to an email, or if you have configured “git send-email” correctly you
> could send them directly via email to this list.  A reviewer can then
> comment on each commit individually and apply them one by one if they
> pass muster.
> 
> (This process is similar for most GNU packages.)
> 
> Hope this helps!
> 
> -- 
> Ricardo

Re: [PATCH] SMP initialization: detection and enumeration

[Almudena Garcia]

> for any patch it’s best to not just show a single large diff but to
> split the changes into logically related commits
I've just split the patch. I enumerated them following the dependencies
order.

> The commit
> message should describe the changes in a GNU-style ChangeLog format; you
> may also add additional descriptions.

Where can I add this information? In the same patch or in a log file?

Thanks


El dom., 19 jul. 2020 a las 20:23, Almudena Garcia (<
liberamenso10...@gmail.com>) escribió:

> Anyway, my patch is short. Maybe I can split It manually, taking care
> about dependencies between blocks.
>
> El dom., 19 jul. 2020 a las 20:17, Almudena Garcia (<
> liberamenso10...@gmail.com>) escribió:
>
>> Thanks for your explanation:
>>
>> > To commit only some changes and not others you can select lines of
>> > interest with “git add -p” (or similar).  Once all connected changes
>> > have been staged you can commit them.  Do this repeatedly until you have
>> > a series of commits that are all small enough that a reviewer can
>> > understand them (and thus your thinking) at a glance.
>>
>> I have already a commit list pushed in my GitHub repository. You can see
>> It here: https://github.com/AlmuHS/GNUMach_SMP/commits/smp-new
>> But, in this case, my code is almost written from scratch, so It's
>> complex to filter line by line.
>> The code only makes sense in a single piece. Otherway, the code doesn't
>> compile or does nothing.
>>
>> > You can then turn that series of commits into a series of patches with
>> > “git format-patch”.  For example, “git format-patch -10” will generate
>> > 10 patch files from the last 10 commits.
>>
>> Ok, I'll try this. But there are so many commits.
>>
>>
>> El dom., 19 jul. 2020 a las 19:52, Ricardo Wurmus ()
>> escribió:
>>
>>>
>>> Hi,
>>>
>>> for any patch it’s best to not just show a single large diff but to
>>> split the changes into logically related commits.  You’re probably
>>> working with Git, so the unit that we’re working with is a Git commit.
>>>
>>> You should group related changes and commit them together.  The commit
>>> message should describe the changes in a GNU-style ChangeLog format; you
>>> may also add additional descriptions.  Here’s an example:
>>>
>>> --8<---cut here---start->8---
>>> kern: Frobnicate the jabberwocky.
>>>
>>> In order to frobnicate the jabberwocky without confusion we only add the
>>> core functionality here.
>>>
>>> * kern/smp.c, kern/smp.h: New files.
>>> * Makefrag.am (libkernel_a_SOURCES): Add them.
>>> --8<---cut here---end--->8---
>>>
>>> To commit only some changes and not others you can select lines of
>>> interest with “git add -p” (or similar).  Once all connected changes
>>> have been staged you can commit them.  Do this repeatedly until you have
>>> a series of commits that are all small enough that a reviewer can
>>> understand them (and thus your thinking) at a glance.
>>>
>>> You can then turn that series of commits into a series of patches with
>>> “git format-patch”.  For example, “git format-patch -10” will generate
>>> 10 patch files from the last 10 commits.  You can attach these patches
>>> to an email, or if you have configured “git send-email” correctly you
>>> could send them directly via email to this list.  A reviewer can then
>>> comment on each commit individually and apply them one by one if they
>>> pass muster.
>>>
>>> (This process is similar for most GNU packages.)
>>>
>>> Hope this helps!
>>>
>>> --
>>> Ricardo
>>>
>>
diff --git a/i386/i386at/acpi_parse_apic.c b/i386/i386at/acpi_parse_apic.c
new file mode 100644
index ..b10aa79c
--- /dev/null
+++ b/i386/i386at/acpi_parse_apic.c
@@ -0,0 +1,624 @@
+/*Copyright 2018 Juan Bosco Garcia
+ *Copyright 2018 2019 2020 Almudena Garcia Jurado-Centurion
+ *This file is part of Min_SMP.
+ *Min_SMP is free software: you can redistribute it and/or modify
+ *it under the terms of the GNU General Public License as published by
+ *the Free Software Foundation, either version 2 of the License, or
+ *(at your option) any later version.
+ *Min_SMP is distributed in the hope that it will be useful,
+ *but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *GNU General Public License for more details.
+ *You should have received a copy of the GNU General Public License
+ *along with Min_SMP.  If not, see .
+ */
+
+#include 
+#include  //memcmp, memcpy...
+
+#include  //lapic, ioapic...
+#include  //printf
+#include  //uint16_t, uint32_t...
+#include  //machine_slot
+#include  //phystokv
+#include 
+#include 
+
+#define BAD_CHECKSUM -1
+#define NO_RSDP -2
+#define NO_RSDT -2
+#define BAD_SIGNATURE -3
+#define NO_APIC -4
+
+struct acpi_apic *apic_madt = NULL;
+
+static struct acpi_rsdp* acpi_get_rsdp(void);
+static int acpi_check_rsdt(struct acpi_rsdt *);
+static struct acpi_rsdt* acpi_get_

Re: [PATCH] SMP initialization: detection and enumeration

[Almudena Garcia]

Anyway, my patch is short. Maybe I can split It manually, taking care about
dependencies between blocks.

El dom., 19 jul. 2020 a las 20:17, Almudena Garcia (<
liberamenso10...@gmail.com>) escribió:

> Thanks for your explanation:
>
> > To commit only some changes and not others you can select lines of
> > interest with “git add -p” (or similar).  Once all connected changes
> > have been staged you can commit them.  Do this repeatedly until you have
> > a series of commits that are all small enough that a reviewer can
> > understand them (and thus your thinking) at a glance.
>
> I have already a commit list pushed in my GitHub repository. You can see
> It here: https://github.com/AlmuHS/GNUMach_SMP/commits/smp-new
> But, in this case, my code is almost written from scratch, so It's complex
> to filter line by line.
> The code only makes sense in a single piece. Otherway, the code doesn't
> compile or does nothing.
>
> > You can then turn that series of commits into a series of patches with
> > “git format-patch”.  For example, “git format-patch -10” will generate
> > 10 patch files from the last 10 commits.
>
> Ok, I'll try this. But there are so many commits.
>
>
> El dom., 19 jul. 2020 a las 19:52, Ricardo Wurmus ()
> escribió:
>
>>
>> Hi,
>>
>> for any patch it’s best to not just show a single large diff but to
>> split the changes into logically related commits.  You’re probably
>> working with Git, so the unit that we’re working with is a Git commit.
>>
>> You should group related changes and commit them together.  The commit
>> message should describe the changes in a GNU-style ChangeLog format; you
>> may also add additional descriptions.  Here’s an example:
>>
>> --8<---cut here---start->8---
>> kern: Frobnicate the jabberwocky.
>>
>> In order to frobnicate the jabberwocky without confusion we only add the
>> core functionality here.
>>
>> * kern/smp.c, kern/smp.h: New files.
>> * Makefrag.am (libkernel_a_SOURCES): Add them.
>> --8<---cut here---end--->8---
>>
>> To commit only some changes and not others you can select lines of
>> interest with “git add -p” (or similar).  Once all connected changes
>> have been staged you can commit them.  Do this repeatedly until you have
>> a series of commits that are all small enough that a reviewer can
>> understand them (and thus your thinking) at a glance.
>>
>> You can then turn that series of commits into a series of patches with
>> “git format-patch”.  For example, “git format-patch -10” will generate
>> 10 patch files from the last 10 commits.  You can attach these patches
>> to an email, or if you have configured “git send-email” correctly you
>> could send them directly via email to this list.  A reviewer can then
>> comment on each commit individually and apply them one by one if they
>> pass muster.
>>
>> (This process is similar for most GNU packages.)
>>
>> Hope this helps!
>>
>> --
>> Ricardo
>>
>

Re: [PATCH] SMP initialization: detection and enumeration

[Almudena Garcia]

Thanks for your explanation:

> To commit only some changes and not others you can select lines of
> interest with “git add -p” (or similar).  Once all connected changes
> have been staged you can commit them.  Do this repeatedly until you have
> a series of commits that are all small enough that a reviewer can
> understand them (and thus your thinking) at a glance.

I have already a commit list pushed in my GitHub repository. You can see It
here: https://github.com/AlmuHS/GNUMach_SMP/commits/smp-new
But, in this case, my code is almost written from scratch, so It's complex
to filter line by line.
The code only makes sense in a single piece. Otherway, the code doesn't
compile or does nothing.

> You can then turn that series of commits into a series of patches with
> “git format-patch”.  For example, “git format-patch -10” will generate
> 10 patch files from the last 10 commits.

Ok, I'll try this. But there are so many commits.


El dom., 19 jul. 2020 a las 19:52, Ricardo Wurmus ()
escribió:

>
> Hi,
>
> for any patch it’s best to not just show a single large diff but to
> split the changes into logically related commits.  You’re probably
> working with Git, so the unit that we’re working with is a Git commit.
>
> You should group related changes and commit them together.  The commit
> message should describe the changes in a GNU-style ChangeLog format; you
> may also add additional descriptions.  Here’s an example:
>
> --8<---cut here---start->8---
> kern: Frobnicate the jabberwocky.
>
> In order to frobnicate the jabberwocky without confusion we only add the
> core functionality here.
>
> * kern/smp.c, kern/smp.h: New files.
> * Makefrag.am (libkernel_a_SOURCES): Add them.
> --8<---cut here---end--->8---
>
> To commit only some changes and not others you can select lines of
> interest with “git add -p” (or similar).  Once all connected changes
> have been staged you can commit them.  Do this repeatedly until you have
> a series of commits that are all small enough that a reviewer can
> understand them (and thus your thinking) at a glance.
>
> You can then turn that series of commits into a series of patches with
> “git format-patch”.  For example, “git format-patch -10” will generate
> 10 patch files from the last 10 commits.  You can attach these patches
> to an email, or if you have configured “git send-email” correctly you
> could send them directly via email to this list.  A reviewer can then
> comment on each commit individually and apply them one by one if they
> pass muster.
>
> (This process is similar for most GNU packages.)
>
> Hope this helps!
>
> --
> Ricardo
>

Re: [PATCH] SMP initialization: detection and enumeration

[Ricardo Wurmus]

Hi,

for any patch it’s best to not just show a single large diff but to
split the changes into logically related commits.  You’re probably
working with Git, so the unit that we’re working with is a Git commit.

You should group related changes and commit them together.  The commit
message should describe the changes in a GNU-style ChangeLog format; you
may also add additional descriptions.  Here’s an example:

--8<---cut here---start->8---
kern: Frobnicate the jabberwocky.

In order to frobnicate the jabberwocky without confusion we only add the
core functionality here.

* kern/smp.c, kern/smp.h: New files.
* Makefrag.am (libkernel_a_SOURCES): Add them.
--8<---cut here---end--->8---

To commit only some changes and not others you can select lines of
interest with “git add -p” (or similar).  Once all connected changes
have been staged you can commit them.  Do this repeatedly until you have
a series of commits that are all small enough that a reviewer can
understand them (and thus your thinking) at a glance.

You can then turn that series of commits into a series of patches with
“git format-patch”.  For example, “git format-patch -10” will generate
10 patch files from the last 10 commits.  You can attach these patches
to an email, or if you have configured “git send-email” correctly you
could send them directly via email to this list.  A reviewer can then
comment on each commit individually and apply them one by one if they
pass muster.

(This process is similar for most GNU packages.)

Hope this helps!

-- 
Ricardo

Re: [PATCH] SMP initialization: detection and enumeration

[Almudena Garcia]

There are many files with different code styles. I'm confused about what is
the correct style.

El dom., 19 jul. 2020 a las 19:10, Jessica Clarke ()
escribió:

> On 19 Jul 2020, at 18:06, Almudena Garcia 
> wrote:
> >
> > I fixed tabulations and comments style. Now better?
>
> Not hugely. Go read files in the same directory and see if they look
> like what you've written.
>
> Jess
>
> > El dom., 19 jul. 2020 a las 18:49, Jessica Clarke ()
> escribió:
> > On 19 Jul 2020, at 17:44, Almudena Garcia 
> wrote:
> > >
> > > Hi all:
> > >
> > > I attach a patch, with the code to find the cpus and enumerate them,
> reading from ACPI tables and MADT (APIC) tables.
> > >
> > > I've tested it over Qemu, but I recommends to test It before
> committing, anyway.
> > >
> > > You can find the rest of the work in my GitHub repository
> > > https://github.com/AlmuHS/GNUMach_SMP/tree/smp-new
> > >
> > > Check this, and advice me about errors or other necessary fixes
> >
> > Before posting patches, please learn to go through and check you've
> > conformed to the code style, i.e. whitespace, comment styles etc. I see
> > a huge number of cases where those are not adhered to in your patch at a
> > glance. Having someone else point out all the issues is a huge waste of
> > time; better to fix them all yourself and then ask someone to review it
> > when they're not going to be constantly having to point out sloppiness.
> >
> > Jess
> >
> > 
>
>

Re: [PATCH] SMP initialization: detection and enumeration

[Almudena Garcia]

Thanks. I'm grateful for your support of my work.
It's a difficult task, and I'm very noob yet. But I'm trying to advance
until my knowledge allows.

The Hurd devs support is very important to learn the necessary to get this
objective.

El dom., 19 jul. 2020 a las 19:01,  escribió:

> Hey Almudena!
>
> I quite admire your determination to work on the Hurd SMP! I would be
> terrified to try to work on something crazy cool like that. But as a friend
> told me recently, "No one cares about your theories and thoughts. People
> care about your actions. There were nights when I was doing a stand up
> comedy routine and failing miserably. BUT I KEPT AT IT! I learned though my
> mistakes. Courageous people act. They act often."
>
> Thanks for being a role model for me to follow!
>
> Joshua
>
> July 19, 2020 12:54 PM, "Almudena Garcia"  >
> wrote:
>
> Ok. I'll check the coding style. I'm trying to follow GNU style, but maybe
> I missed It in some files.
> El dom., 19 jul. 2020 a las 18:49, Jessica Clarke ()
> escribió:
>
> On 19 Jul 2020, at 17:44, Almudena Garcia 
> wrote:
> >
> > Hi all:
> >
> > I attach a patch, with the code to find the cpus and enumerate them,
> reading from ACPI tables and MADT (APIC) tables.
> >
> > I've tested it over Qemu, but I recommends to test It before committing,
> anyway.
> >
> > You can find the rest of the work in my GitHub repository
> > https://github.com/AlmuHS/GNUMach_SMP/tree/smp-new
> >
> > Check this, and advice me about errors or other necessary fixes
>
> Before posting patches, please learn to go through and check you've
> conformed to the code style, i.e. whitespace, comment styles etc. I see
> a huge number of cases where those are not adhered to in your patch at a
> glance. Having someone else point out all the issues is a huge waste of
> time; better to fix them all yourself and then ask someone to review it
> when they're not going to be constantly having to point out sloppiness.
>
> Jess
>
>
>
>

Re: [PATCH] SMP initialization: detection and enumeration

[Jessica Clarke]

On 19 Jul 2020, at 18:06, Almudena Garcia  wrote:
> 
> I fixed tabulations and comments style. Now better? 

Not hugely. Go read files in the same directory and see if they look
like what you've written.

Jess

> El dom., 19 jul. 2020 a las 18:49, Jessica Clarke () 
> escribió:
> On 19 Jul 2020, at 17:44, Almudena Garcia  wrote:
> > 
> > Hi all:
> > 
> > I attach a patch, with the code to find the cpus and enumerate them, 
> > reading from ACPI tables and MADT (APIC) tables.
> > 
> > I've tested it over Qemu, but I recommends to test It before committing, 
> > anyway.
> > 
> > You can find the rest of the work in my GitHub repository
> > https://github.com/AlmuHS/GNUMach_SMP/tree/smp-new
> > 
> > Check this, and advice me about errors or other necessary fixes
> 
> Before posting patches, please learn to go through and check you've
> conformed to the code style, i.e. whitespace, comment styles etc. I see
> a huge number of cases where those are not adhered to in your patch at a
> glance. Having someone else point out all the issues is a huge waste of
> time; better to fix them all yourself and then ask someone to review it
> when they're not going to be constantly having to point out sloppiness.
> 
> Jess
> 
> 

Re: [PATCH] SMP initialization: detection and enumeration

[Almudena Garcia]

I fixed tabulations and comments style. Now better?

El dom., 19 jul. 2020 a las 18:49, Jessica Clarke ()
escribió:

> On 19 Jul 2020, at 17:44, Almudena Garcia 
> wrote:
> >
> > Hi all:
> >
> > I attach a patch, with the code to find the cpus and enumerate them,
> reading from ACPI tables and MADT (APIC) tables.
> >
> > I've tested it over Qemu, but I recommends to test It before committing,
> anyway.
> >
> > You can find the rest of the work in my GitHub repository
> > https://github.com/AlmuHS/GNUMach_SMP/tree/smp-new
> >
> > Check this, and advice me about errors or other necessary fixes
>
> Before posting patches, please learn to go through and check you've
> conformed to the code style, i.e. whitespace, comment styles etc. I see
> a huge number of cases where those are not adhered to in your patch at a
> glance. Having someone else point out all the issues is a huge waste of
> time; better to fix them all yourself and then ask someone to review it
> when they're not going to be constantly having to point out sloppiness.
>
> Jess
>
>
diff --git a/Makefrag.am b/Makefrag.am
index ea612275..69ac31a1 100644
--- a/Makefrag.am
+++ b/Makefrag.am
@@ -122,6 +122,18 @@ EXTRA_DIST += \
 	ipc/notify.defs
 
 
+#
+# SMP implementation (APIC, ACPI, etc)
+#
+
+libkernel_a_SOURCES += \
+	i386/i386at/acpi_parse_apic.h \
+	i386/i386at/acpi_parse_apic.c \
+	i386/i386/apic.h \
+	i386/i386/apic.c \
+	kern/smp.h \
+	kern/smp.c
+
 #
 # `kernel' implementation (tasks, threads, trivia, etc.).
 #
diff --git a/configfrag.ac b/configfrag.ac
index 91d737ef..454d9f23 100644
--- a/configfrag.ac
+++ b/configfrag.ac
@@ -46,9 +46,12 @@ AC_DEFINE([BOOTSTRAP_SYMBOLS], [0], [BOOTSTRAP_SYMBOLS])
 # Multiprocessor support is still broken.
 AH_TEMPLATE([MULTIPROCESSOR], [set things up for a uniprocessor])
 mach_ncpus=1
+AC_DEFINE_UNQUOTED([NCPUS], [$mach_ncpus], [number of CPUs])
+
 AC_DEFINE_UNQUOTED([NCPUS], [$mach_ncpus], [number of CPUs])
 [if [ $mach_ncpus -gt 1 ]; then]
   AC_DEFINE([MULTIPROCESSOR], [1], [set things up for a multiprocessor])
+  AC_DEFINE_UNQUOTED([NCPUS], [255], [number of CPUs])
 [fi]
 
 # Restartable Atomic Sequences to get a really fast test-n-set.  Can't be
diff --git a/i386/i386/apic.c b/i386/i386/apic.c
new file mode 100644
index ..ddc51afb
--- /dev/null
+++ b/i386/i386/apic.c
@@ -0,0 +1,257 @@
+/* apic.c - APIC controller management for Mach.
+   Copyright (C) 2020 Free Software Foundation, Inc.
+   Written by Almudena Garcia Jurado-Centurion
+
+   This file is part of GNU Mach.
+
+   GNU Mach is free software; you can redistribute it and/or modify it
+   under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 2, or (at your option)
+   any later version.
+
+   GNU Mach is distributed in the hope that it will be useful, but
+   WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program; if not, write to the Free Software
+   Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA  02111, USA. */
+
+
+#include 
+#include 
+#include 
+#include  //printf
+
+
+#define MAX_CPUS 256
+
+volatile ApicLocalUnit* lapic = NULL;
+
+struct apic_info apic_data;
+
+/* apic_data_init: initialize the apic_data structures to preliminary values
+ *  Reserve memory to the lapic list dynamic vector
+ *  Returns 0 if success, -1 if error
+ */
+
+int
+apic_data_init(void)
+{
+int success = 0;
+
+apic_data.cpu_lapic_list = NULL;
+apic_data.ncpus = 0;
+apic_data.nioapics = 0;
+apic_data.cpu_lapic_list = (uint16_t*) kalloc(MAX_CPUS*sizeof(uint16_t));
+apic_data.nirqoverride = 0;
+
+if(apic_data.cpu_lapic_list == NULL)
+{
+success = -1;
+}
+
+return success;
+}
+
+/* apic_lapic_init: initialize lapic pointer to the memory common address
+ *Receives as input a pointer to the virtual memory address, previously mapped in a page
+ */
+
+void
+apic_lapic_init(ApicLocalUnit* lapic_ptr)
+{
+lapic = lapic_ptr;
+}
+
+/* apic_add_cpu: add a new lapic/cpu entry to the cpu_lapic list
+ *   Receives as input the lapic's APIC ID
+ */
+
+void
+apic_add_cpu(uint16_t apic_id)
+{
+int numcpus = apic_data.ncpus;
+apic_data.cpu_lapic_list[numcpus] = apic_id;
+apic_data.ncpus++;
+}
+
+/* apic_add_ioapic: add a new ioapic entry to the ioapic list
+ *   Receives as input an ioapic_data structure, filled with the IOAPIC entry's data
+ */
+
+void
+apic_add_ioapic(struct ioapic_data ioapic)
+{
+int nioapic = apic_data.nioapics;
+
+apic_data.ioapic_list[nioapic] = ioapic;
+
+apic_data.nioapics++;
+}
+
+/* apic_add_irq_override: add a new IRQ to the irq_override list
+ *   Receives as input an irq_override_data structure, filled with the IRQ entry's data
+ */
+
+void
+apic_add_irq_override(struct

Re: [PATCH] SMP initialization: detection and enumeration

[jbranso]

Hey Almudena!

I quite admire your determination to work on the Hurd SMP! I would be terrified 
to try to work on something crazy cool like that. But as a friend told me 
recently, "No one cares about your theories and thoughts. People care about 
your actions. There were nights when I was doing a stand up comedy routine and 
failing miserably. BUT I KEPT AT IT! I learned though my mistakes. Courageous 
people act. They act often."

Thanks for being a role model for me to follow!

Joshua

July 19, 2020 12:54 PM, "Almudena Garcia" mailto:liberamenso10...@gmail.com?to=%22Almudena%20Garcia%22%20)>
 wrote:
 Ok. I'll check the coding style. I'm trying to follow GNU style, but maybe I 
missed It in some files. 
 El dom., 19 jul. 2020 a las 18:49, Jessica Clarke (mailto:jrt...@jrtc27.com)>) escribió: On 19 Jul 2020, at 17:44, Almudena 
Garcia mailto:liberamenso10...@gmail.com)> wrote:
>
> Hi all:
>
> I attach a patch, with the code to find the cpus and enumerate them, reading 
> from ACPI tables and MADT (APIC) tables.
>
> I've tested it over Qemu, but I recommends to test It before committing, 
> anyway.
>
> You can find the rest of the work in my GitHub repository
> https://github.com/AlmuHS/GNUMach_SMP/tree/smp-new 
> (https://github.com/AlmuHS/GNUMach_SMP/tree/smp-new)
>
> Check this, and advice me about errors or other necessary fixes

Before posting patches, please learn to go through and check you've
conformed to the code style, i.e. whitespace, comment styles etc. I see
a huge number of cases where those are not adhered to in your patch at a
glance. Having someone else point out all the issues is a huge waste of
time; better to fix them all yourself and then ask someone to review it
when they're not going to be constantly having to point out sloppiness.

Jess

Re: [PATCH] SMP initialization: detection and enumeration

[Almudena Garcia]

Ok. I'll check the coding style. I'm trying to follow GNU style, but maybe
I missed It in some files.

El dom., 19 jul. 2020 a las 18:49, Jessica Clarke ()
escribió:

> On 19 Jul 2020, at 17:44, Almudena Garcia 
> wrote:
> >
> > Hi all:
> >
> > I attach a patch, with the code to find the cpus and enumerate them,
> reading from ACPI tables and MADT (APIC) tables.
> >
> > I've tested it over Qemu, but I recommends to test It before committing,
> anyway.
> >
> > You can find the rest of the work in my GitHub repository
> > https://github.com/AlmuHS/GNUMach_SMP/tree/smp-new
> >
> > Check this, and advice me about errors or other necessary fixes
>
> Before posting patches, please learn to go through and check you've
> conformed to the code style, i.e. whitespace, comment styles etc. I see
> a huge number of cases where those are not adhered to in your patch at a
> glance. Having someone else point out all the issues is a huge waste of
> time; better to fix them all yourself and then ask someone to review it
> when they're not going to be constantly having to point out sloppiness.
>
> Jess
>
>

Re: [PATCH] SMP initialization: detection and enumeration

[Jessica Clarke]

On 19 Jul 2020, at 17:44, Almudena Garcia  wrote:
> 
> Hi all:
> 
> I attach a patch, with the code to find the cpus and enumerate them, reading 
> from ACPI tables and MADT (APIC) tables.
> 
> I've tested it over Qemu, but I recommends to test It before committing, 
> anyway.
> 
> You can find the rest of the work in my GitHub repository
> https://github.com/AlmuHS/GNUMach_SMP/tree/smp-new
> 
> Check this, and advice me about errors or other necessary fixes

Before posting patches, please learn to go through and check you've
conformed to the code style, i.e. whitespace, comment styles etc. I see
a huge number of cases where those are not adhered to in your patch at a
glance. Having someone else point out all the issues is a huge waste of
time; better to fix them all yourself and then ask someone to review it
when they're not going to be constantly having to point out sloppiness.

Jess

[PATCH] SMP initialization: detection and enumeration

[Almudena Garcia]

Hi all:

I attach a patch, with the code to find the cpus and enumerate them,
reading from ACPI tables and MADT (APIC) tables.

I've tested it over Qemu, but I recommends to test It before committing,
anyway.

You can find the rest of the work in my GitHub repository
https://github.com/AlmuHS/GNUMach_SMP/tree/smp-new

Check this, and advice me about errors or other necessary fixes

Thanks
diff --git a/Makefrag.am b/Makefrag.am
index ea612275..69ac31a1 100644
--- a/Makefrag.am
+++ b/Makefrag.am
@@ -122,6 +122,18 @@ EXTRA_DIST += \
 	ipc/notify.defs
 
 
+#
+# SMP implementation (APIC, ACPI, etc)
+#
+
+libkernel_a_SOURCES += \
+	i386/i386at/acpi_parse_apic.h \
+	i386/i386at/acpi_parse_apic.c \
+	i386/i386/apic.h \
+	i386/i386/apic.c \
+	kern/smp.h \
+	kern/smp.c
+
 #
 # `kernel' implementation (tasks, threads, trivia, etc.).
 #
diff --git a/configfrag.ac b/configfrag.ac
index 91d737ef..454d9f23 100644
--- a/configfrag.ac
+++ b/configfrag.ac
@@ -46,9 +46,12 @@ AC_DEFINE([BOOTSTRAP_SYMBOLS], [0], [BOOTSTRAP_SYMBOLS])
 # Multiprocessor support is still broken.
 AH_TEMPLATE([MULTIPROCESSOR], [set things up for a uniprocessor])
 mach_ncpus=1
+AC_DEFINE_UNQUOTED([NCPUS], [$mach_ncpus], [number of CPUs])
+
 AC_DEFINE_UNQUOTED([NCPUS], [$mach_ncpus], [number of CPUs])
 [if [ $mach_ncpus -gt 1 ]; then]
   AC_DEFINE([MULTIPROCESSOR], [1], [set things up for a multiprocessor])
+  AC_DEFINE_UNQUOTED([NCPUS], [255], [number of CPUs])
 [fi]
 
 # Restartable Atomic Sequences to get a really fast test-n-set.  Can't be
diff --git a/i386/i386/apic.c b/i386/i386/apic.c
new file mode 100644
index ..17ccc722
--- /dev/null
+++ b/i386/i386/apic.c
@@ -0,0 +1,257 @@
+/* apic.c - APIC controller management for Mach.
+   Copyright (C) 2020 Free Software Foundation, Inc.
+   Written by Almudena Garcia Jurado-Centurion
+
+   This file is part of GNU Mach.
+
+   GNU Mach is free software; you can redistribute it and/or modify it
+   under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 2, or (at your option)
+   any later version.
+
+   GNU Mach is distributed in the hope that it will be useful, but
+   WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program; if not, write to the Free Software
+   Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA  02111, USA. */
+
+
+#include 
+#include 
+#include 
+#include  //printf
+
+
+#define MAX_CPUS 256
+
+volatile ApicLocalUnit* lapic = NULL;
+
+struct apic_info apic_data;
+
+/* apic_data_init: initialize the apic_data structures to preliminary values
+ *  Reserve memory to the lapic list dynamic vector
+ *  Returns 0 if success, -1 if error
+ */
+
+int
+apic_data_init(void)
+{
+int success = 0;
+
+apic_data.cpu_lapic_list = NULL;
+apic_data.ncpus = 0;
+apic_data.nioapics = 0;
+apic_data.cpu_lapic_list = (uint16_t*) kalloc(MAX_CPUS*sizeof(uint16_t));
+apic_data.nirqoverride = 0;
+
+if(apic_data.cpu_lapic_list == NULL)
+{
+success = -1;
+}
+
+return success;
+}
+
+/* apic_lapic_init: initialize lapic pointer to the memory common address
+ *Receives as input a pointer to the virtual memory address, previously mapped in a page
+ */
+
+void
+apic_lapic_init(ApicLocalUnit* lapic_ptr)
+{
+lapic = lapic_ptr;
+}
+
+/* apic_add_cpu: add a new lapic/cpu entry to the cpu_lapic list
+ *   Receives as input the lapic's APIC ID
+ */
+
+void
+apic_add_cpu(uint16_t apic_id)
+{
+int numcpus = apic_data.ncpus;
+apic_data.cpu_lapic_list[numcpus] = apic_id;
+apic_data.ncpus++;
+}
+
+/* apic_add_ioapic: add a new ioapic entry to the ioapic list
+ *   Receives as input an ioapic_data structure, filled with the IOAPIC entry's data
+ */
+
+void
+apic_add_ioapic(struct ioapic_data ioapic)
+{
+int nioapic = apic_data.nioapics;
+
+apic_data.ioapic_list[nioapic] = ioapic;
+
+apic_data.nioapics++;
+}
+
+/* apic_add_irq_override: add a new IRQ to the irq_override list
+ *   Receives as input an irq_override_data structure, filled with the IRQ entry's data
+ */
+
+void
+apic_add_irq_override(struct irq_override_data irq_over)
+{
+int nirq = apic_data.nirqoverride;
+
+apic_data.irq_override_list[nirq] = irq_over;
+apic_data.nirqoverride++;
+}
+
+/* apic_get_cpu_apic_id: returns the apic_id of a cpu
+ *   Receives as input the kernel ID of a CPU
+ */
+
+uint16_t
+apic_get_cpu_apic_id(int kernel_id)
+{
+uint16_t apic_id;
+
+if(kernel_id < MAX_CPUS)
+{
+apic_id = apic_data.cpu_lapic_list[kernel_id];
+}
+else
+{
+apic_id = -1;
+}
+
+return apic_id;
+}
+
+/* apic_get_lapic: returns a reference to the common memory address for Local APIC */
+
+ApicLocalUnit*
+a