From mboxrd@z Thu Jan 1 00:00:00 1970 From: Christopher Covington Subject: Re: [PATCH v2] clocksource: arch_timer: Allow the device tree to specify the physical timer Date: Tue, 16 Sep 2014 07:22:50 -0400 Message-ID: <54181D8A.5040300@codeaurora.org> References: <5412DC48.7030708@codeaurora.org> <5412E3BB.9030800@arm.com> <54134291.3040700@codeaurora.org> <20140915111055.GD1577@arm.com> <54174CFF.7050504@codeaurora.org> <54175EFE.6010509@codeaurora.org> <54176D62.9010600@codeaurora.org> <20140916104242.GC27273@arm.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20140916104242.GC27273-5wv7dgnIgG8@public.gmane.org> Sender: devicetree-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org To: Catalin Marinas Cc: Sonny Rao , Stephen Boyd , Marc Zyngier , Doug Anderson , Will Deacon , "olof-nZhT3qVonbNeoWH0uzbU5w@public.gmane.org" , Mark Rutland , Sudeep Holla , Lorenzo Pieralisi , Thomas Gleixner , Daniel Lezcano , Nathan Lynch , "linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org" , "robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org" , Pawel Moll , "ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg@public.gmane.org" , "galak-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org" , "devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" , "linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" , =?ISO-8859-1?Q?Heiko_St=F List-Id: devicetree@vger.kernel.org On 09/16/2014 06:42 AM, Catalin Marinas wrote: > On Mon, Sep 15, 2014 at 11:51:14PM +0100, Christopher Covington wrote: >> Hi Sonny, >> >> On 09/15/2014 06:04 PM, Sonny Rao wrote: >>> On Mon, Sep 15, 2014 at 2:52 PM, Sonny Rao wrote: >>>> On Mon, Sep 15, 2014 at 2:49 PM, Stephen Boyd wrote: >>>>> On 09/15/14 14:47, Sonny Rao wrote: >>>>>> On Mon, Sep 15, 2014 at 1:33 PM, Stephen Boyd wrote: >>>>>>> On 09/15/14 04:10, Catalin Marinas wrote: >>>>>>>> On Fri, Sep 12, 2014 at 07:59:29PM +0100, Stephen Boyd wrote: >>>>>>>>> On 09/12/14 05:14, Marc Zyngier wrote: >>>>>>>>>> We surely can handle the UNDEF and do something there. We just can't do >>>>>>>>>> it the way Doug described it above. >>>>>>>>> I suggested doing that for something else a while ago and Will and Dave >>>>>>>>> we're not thrilled[1]. The suggestion back then was to use DT to >>>>>>>>> indicate what mode the kernel is running in. >>>>>>>>> >>>>>>>>> [1] >>>>>>>>> http://lists.infradead.org/pipermail/linux-arm-kernel/2012-June/105321.html >>>>>>>> I think the context was slightly different. As I re-read the thread, it >>>>>>>> seems that the discussion was around whether to use some SMC interface >>>>>>>> or not based on whether the kernel is running secure or non-secure. The >>>>>>>> argument made by Will was to actually specify the type of the firmware >>>>>>>> SMC interface in the DT and use it in the kernel (and probably assume >>>>>>>> the kernel is running in secure mode if no smc interface is specified in >>>>>>>> the DT; you could have both though, running in secure mode and also >>>>>>>> having firmware). >>>>>>>> >>>>>>>> In this arch timer case, we need to work around a firmware bug (or >>>>>>>> feature as 32-bit ARM kernels never required CNTVOFF initialisation by >>>>>>>> firmware, no matter how small such firmware is). We don't expect a >>>>>>>> specific SMC call to initialise CNTVOFF, so we can't describe it in the >>>>>>>> DT. >>>>>>> Agreed, we can't described SMC calls that don't exist. From my >>>>>>> perspective it's just another part of the cpu boot sequence that needs >>>>>>> to be handled in the kernel, so describing the requirement via the >>>>>>> cpu-boot method seems appropriate. It seems like we're making it harder >>>>>>> than it should be by handling the undef when we could have slightly >>>>>>> different SMP boot code (and suspend/resume code) depending on the boot >>>>>>> method property. >>>>>> >>>>>> +heiko >>>>>> >>>>>> So, for the case of rk3288, based on this discussion what I'm going to >>>>>> propose is to add code to rockchip.c which looks for a particular SMP >>>>>> enable method -- say something like "rockchip,rk3288-smp-secure-svc" >>>>>> which will then assume we have been booted in secure SVC mode and do >>>>>> the CNTVOFF fixup. I believe, it will need to do this on the boot CPU >>>>>> as well, so I think it will need to scan the DT fairly early on the >>>>>> boot CPU and also perform the function there. >>>>>> >>>>>> I'll look into implementing this and post code. Comments and >>>>>> suggestions appreciated, thanks. >>>>> >>>>> What goes wrong if we read the cntvoff from the boot CPU during >>>>> smp_prepare_cpus() phase and use that to set the cntvoff on the other >>>>> CPUs? That avoids needing to do anything very early by making the value >>>>> the same. It does mean that cntvoff is some random out of reset value >>>>> for CPU0, but at least it's consistent. >>>> >>>> I think we cannot read the value if we're not in hyp mode. >>> >>> Well, thinking about it a little more, I think you still have a good point. >>> >>> We don't need to do this early on, as long as we haven't started using >>> the arch timers yet. If we are still able to do this at the point >>> where we're executing the code in arch/arm/mach-rockchip/platsmp.c >>> that finds the enable method then we can just handle it there. >> >> I've been playing around with the probe-based approach and while I need to do >> a lot more testing, it seems to be working for the first tens of instructions. >> I hope to be able to share a draft of that soon. Basically, I just read the >> current NSACR value and write it back (although maybe in the long term we >> would want to make sure a few of those bits are set or cleared). If that >> succeeds, we know we're in secure SVC and can proceed to set up MON and HYP. > > But when it doesn't succeed, you get an undefined instruction fault > (since NSACR is only writable in secure mode). Yes. I see it as a conditional branch to VBAR+4 with a mode switch side effect. Christopher -- Employee of Qualcomm Innovation Center, Inc. Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by the Linux Foundation. -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html