Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753503AbbERPDN (ORCPT ); Mon, 18 May 2015 11:03:13 -0400 Received: from mail-pa0-f47.google.com ([209.85.220.47]:33213 "EHLO mail-pa0-f47.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753286AbbERPDD (ORCPT ); Mon, 18 May 2015 11:03:03 -0400 Date: Mon, 18 May 2015 09:03:02 -0600 From: Lina Iyer To: Ohad Ben-Cohen Cc: "Anna, Suman" , Bjorn Andersson , Andy Gross , "linux-arm-msm@vger.kernel.org" , "linux-kernel@vger.kernel.org" , Kumar Gala , Jeffrey Hugo Subject: Re: [PATCH RFC] hwspinlock: Don't take software spinlock before hwspinlock Message-ID: <20150518150302.GA23206@linaro.org> References: <1430500026-47990-1-git-send-email-lina.iyer@linaro.org> <20150511144608.GB16124@linaro.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii; format=flowed Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4381 Lines: 84 On Sat, May 16 2015 at 03:03 -0600, Ohad Ben-Cohen wrote: >On Mon, May 11, 2015 at 5:46 PM, Lina Iyer wrote: >> On Sat, May 09 2015 at 03:25 -0600, Ohad Ben-Cohen wrote: >>> On Fri, May 1, 2015 at 8:07 PM, Lina Iyer wrote: >>> Let's discuss whether we really want to expose this functionality >>> under the same hwspinlock API or not. >>> >>> In this new mode, unlike previously, users will now be able to sleep >>> after taking the lock, and others trying to take the lock might poll >>> the hardware for a long period of time without the ability to sleep >>> while waiting for the lock. It almost sounds like you were looking for >>> some hwmutex functionality. >> >> I agree, that it opens up a possiblity that user may sleep after holding >> a hw spinlock. But really, why should it prevents us from using it as a >> hw mutex, if the need is legitimate? > >If we want hw mutex functionality, let's discuss how to expose it. >Exposing it using the existing hw spinlock API might not be ideal, as >users might get confused. > >Additionally, there are hardware IP locking blocks out there which >encourage users to sleep while waiting for a lock, by providing >interrupt functionality to wake them up when the lock is freed. So if >we choose to add a hw mutex API it might be used by others in the >future too (though this reason alone is not why we would choose to add >it now of course). > Okay, the API seems to want to dictate what kind of flags be specified for __try_lock(), FLAG_NONE, in my mind, seems to fall into the same classification. But sure, we can discuss a different form of achieving the same thing. Do you have any ideas? >API discussions aside, what do you want to happen in your scenario >while the lock is taken? are you OK with other users spinning on the >lock waiting for it to be released? IIUC that might mean processors >spinning for a non-negligible period of time? > The lock in question is used differently than traditional locks across processors. This lock helps synchronizes context transition from non-secure to secure on the same processor. The usecase, goes like this. In cpuidle, any core can be the last core to power down. The last man also holds the responsibility of shutting down shared resources like caches etc. The way the power down of a core works is, there are some high level decisions made in Linux and these decisions (like to flush and invalidate caches) etc gets transferred over to the the secure layer. The secure layer executes the ARM WFI that powers down the cpu, but uses these decisions passed into to determine if the cache needs to be invalidated upon wakeup etc. There is a possible race condition between what Linux thinks is the last core, vs what secure layer thinks is the last core. Lets say, two cores c0, c1 are going down. c1 is the second last core to go down from Linux as such, will not carry information about shared resources when making the SCM call. c1 made the SCM call, but is stuck handling some FIQs. In the meanwhile c0, goes idle and since its the last core in Linux, figures out the state of the shared resources. c0 calls into SCM, and ends up powering down earlier than c1. Per secure layer, the last core to go down is c1 and the votes of the shared resources are considered from that core. Things like cache invalidation without flush may happen as a result of this inconsistency of last man view point. The way we have solved it, Linux acquires a hw spinlock for each core, when calling into SCM and the secure monitor releases the spinlock. At any given time, only one core can switch the context from Linux to secure for power down operations. This guarantees the last man is synchronized between both Linux and secure. Another core may be spinning waiting for hw mutex, but they all happen serialized. This mutex is held in an irq disable context in cpuidle. There may be another processor spining to wait on hw mutex, but there isnt much to do otherwise, because the only operation at this time while holding the lock is to call into SCM and that would unlock the mutex. Thanks, Lina -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/