Hi Sonny, On 09/15/2014 06:04 PM, Sonny Rao wrote: > On Mon, Sep 15, 2014 at 2:52 PM, Sonny Rao <sonnyrao@xxxxxxxxxxxx> wrote: >> On Mon, Sep 15, 2014 at 2:49 PM, Stephen Boyd <sboyd@xxxxxxxxxxxxxx> wrote: >>> On 09/15/14 14:47, Sonny Rao wrote: >>>> On Mon, Sep 15, 2014 at 1:33 PM, Stephen Boyd <sboyd@xxxxxxxxxxxxxx> 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. 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@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
