From owner-freebsd-arch@FreeBSD.ORG Sun Jun 3 19:45:41 2012 Return-Path: Delivered-To: freebsd-arch@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 24093106564A; Sun, 3 Jun 2012 19:45:41 +0000 (UTC) (envelope-from avg@FreeBSD.org) Received: from citadel.icyb.net.ua (citadel.icyb.net.ua [212.40.38.140]) by mx1.freebsd.org (Postfix) with ESMTP id DBFA78FC1F; Sun, 3 Jun 2012 19:45:39 +0000 (UTC) Received: from porto.starpoint.kiev.ua (porto-e.starpoint.kiev.ua [212.40.38.100]) by citadel.icyb.net.ua (8.8.8p3/ICyb-2.3exp) with ESMTP id WAA01145; Sun, 03 Jun 2012 22:45:34 +0300 (EEST) (envelope-from avg@FreeBSD.org) Received: from localhost ([127.0.0.1]) by porto.starpoint.kiev.ua with esmtp (Exim 4.34 (FreeBSD)) id 1SbGk1-000MpY-T6; Sun, 03 Jun 2012 22:45:33 +0300 Message-ID: <4FCBBEDD.5000604@FreeBSD.org> Date: Sun, 03 Jun 2012 22:45:33 +0300 From: Andriy Gapon User-Agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:12.0) Gecko/20120503 Thunderbird/12.0.1 MIME-Version: 1.0 To: Mitsuru IWASAKI References: <20120603.002554.119853142.iwasaki@jp.FreeBSD.org> <4FCB0FE5.4050607@FreeBSD.org> <20120603.234243.28389486.iwasaki@jp.FreeBSD.org> In-Reply-To: <20120603.234243.28389486.iwasaki@jp.FreeBSD.org> X-Enigmail-Version: 1.5pre Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Cc: attilio@FreeBSD.org, freebsd-acpi@FreeBSD.org, freebsd-arch@FreeBSD.org Subject: Re: cpu stopping X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 03 Jun 2012 19:45:41 -0000 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