cpu stopping

[Andriy Gapon]

on 03/06/2012 17:42 Mitsuru IWASAKI said the following:
> Hi, thanks for comments.
> 
>> As the first thing I must admit that I haven't looked at the patch :-)
> 
> Never mind :) What I'm trying to do in the patches is just to unify
> amd64/i386 independent part (acpi_wakeup.c) for the code maintenance,
> so please let's commit it first, then start re-design the
> cpususpend_handler().

In no way I am trying to delay your work :)
Just shared my view on the design of cpu stopping code.

>> But really I don't see why we need to differentiate between stopped and
>> suspended state as both of them ultimately mean exactly the same thing - CPUs
>> are spinning on some condition (and they are in a well-defined place and state).
> 
> Yes, amd64/i386 cpususpend_handler() is very similar to cpustop_handler()
> actually, some resume related procedures are added for suspend.
> 
>> My view of how this should work is:
>> - there can be only one master CPU that controls all other (slave) CPUs
>> - the master sets entry and exit hooks
> 
> Entry hook for suspending might be
> ----
>                 ctx_fpusave(suspfpusave[cpu]);
>                 wbinvd();
>                 CPU_SET_ATOMIC(cpu, &stopped_cpus);
> ----
> 
> and for stopping is
> ----
>         /* Indicate that we are stopped */
>         CPU_SET_ATOMIC(cpu, &stopped_cpus);
> ----
> 
> Correct?

Yes.  The only nit is that CPU_SET_ATOMIC(cpu, &stopped_cpus) could be part of
the wait loop prologue.  No need to duplicate it in each hook.

> I think stopping hook can be replaced with suspending hook.

Perhaps... But let's not go into this topic just yet.

> Exit hook for suspending is
> ----
>                 pmap_init_pat();
>                 load_cr3(susppcbs[cpu]->pcb_cr3);
>                 initializecpu();
>                 PCPU_SET(switchtime, 0);
>                 PCPU_SET(switchticks, ticks);
> [snip]
>         /* Resume MCA and local APIC */
>         mca_resume();
>         lapic_setup(0);
> ----
> 
> For stopping should be
> ----
>         if (cpu == 0 && cpustop_restartfunc != NULL) {
>                 cpustop_restartfunc();
>                 cpustop_restartfunc = NULL;
>         }
> ----
> 
>> - the master signals slaves to enter the stop state
>> - the slaves execute the enter hook and start spinning on the release condition
>> - the master does whatever it wants to do in this special system state
>> - the master signals the slaves to resume
>> - the slave exit the spin loop and execute the exit hook
> 
> I think it would be possible.  However I personally think that
> priority of x86/x86/mp_machdep.c is higher and more effective than
> merging cpususpend/stop_handler().

I do not disagree.

-- 
Andriy Gapon