Date: Sun, 03 Jun 2012 22:45:33 +0300 From: Andriy Gapon <avg@FreeBSD.org> To: Mitsuru IWASAKI <iwasaki@jp.FreeBSD.org> Cc: attilio@FreeBSD.org, freebsd-acpi@FreeBSD.org, freebsd-arch@FreeBSD.org Subject: Re: cpu stopping Message-ID: <4FCBBEDD.5000604@FreeBSD.org> In-Reply-To: <20120603.234243.28389486.iwasaki@jp.FreeBSD.org> References: <20120603.002554.119853142.iwasaki@jp.FreeBSD.org> <CAJ-FndAfm4_XqFSwBqXK=cgWkE6YVrtkS5BbcH7zcRd-100xTw@mail.gmail.com> <4FCB0FE5.4050607@FreeBSD.org> <20120603.234243.28389486.iwasaki@jp.FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?4FCBBEDD.5000604>