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 (<samuel.thibault@gnu.org>) 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