From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stephen Warren Date: Wed, 9 Nov 2016 08:50:10 -0700 Subject: [U-Boot] [PATCH v3 1/3] ARM: bcm283x: Implement EFI RTS reset_system In-Reply-To: References: <1478079380-176304-1-git-send-email-agraf@suse.de> <1478079380-176304-2-git-send-email-agraf@suse.de> <32465357-6ce4-5876-523c-90b471626ea9@wwwdotorg.org> <4aa518ba-555e-5caf-7e53-030510a04ea4@suse.de> <638879b7-d206-69b2-2e32-53ec00c9d2cd@wwwdotorg.org> Message-ID: List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On 11/09/2016 04:43 AM, Alexander Graf wrote: > > > On 07/11/2016 22:26, Stephen Warren wrote: >> On 11/06/2016 03:24 AM, Alexander Graf wrote: >>> >>> >>> On 05/11/2016 23:01, Stephen Warren wrote: >>>> On 11/02/2016 03:36 AM, Alexander Graf wrote: >>>>> The rpi has a pretty simple way of resetting the whole system. All it >>>>> takes >>>>> is to poke a few registers at a well defined location in MMIO space. >>>>> >>>>> This patch adds support for the EFI loader implementation to allow an >>>>> OS to >>>>> reset and power off the system when we're outside of boot time. >>>> >>>> (As an aside, I'm not sure why someone wanting EFI wouldn't just use a >>>> complete EFI implementation such as TianoCore.) >>>> >>>>> diff --git a/arch/arm/mach-bcm283x/reset.c >>>>> b/arch/arm/mach-bcm283x/reset.c >>>> >>>>> +__efi_runtime_data struct bcm2835_wdog_regs *wdog_regs = >>>>> + (struct bcm2835_wdog_regs *)BCM2835_WDOG_PHYSADDR; >>>>> + >>>>> +void __efi_runtime reset_cpu(ulong addr) >>>>> { >>>>> - struct bcm2835_wdog_regs *regs = >>>>> - (struct bcm2835_wdog_regs *)BCM2835_WDOG_PHYSADDR; >>>> >>>> I'm not sure why that change is required. The value of the variable is >>>> the same in both cases? >>> >>> Take a look a few lines down in the patch: >>> >>>> +void efi_reset_system_init(void) >>>> +{ >>>> + efi_add_runtime_mmio(&wdog_regs, sizeof(*wdog_regs)); >>>> +} >>> >>> What this does is register a *pointer* as run time service pointer. What >>> does that mean? >>> >>> When we enter RTS, Linux can map any region in the EFI memory map into a >>> different place in its own virtual memory map. So any pointers we use >>> inside RTS have to be relocated to the new locations. >>> >>> For normal relocations, we move the relocations from linker time to run >>> time, so that we can relocate ourselves when Linux does the switch-over >>> to a new address space. >>> >>> However, for MMIO that's trickier. That's where the >>> efi_add_runtime_mmio() function comes into play. It takes care of adding >>> the page around the references address to the EFI memory map as RTS MMIO >>> and relocates the pointer when Linux switches us into the new address >>> space. >>> >>> Does that explain why we need to move from an inline address to an >>> address stored in a memory location? >> >> So EFI RTS runs in the same exception level as the rich OS, and not in >> EL3? I would have expected EFI to run in EL3 with a completely separate >> MMU configuration. If that's not the case, then this part of the patch >> does make sense. > > Right, it runs in EL2/EL1 with a virtual memory layout that is provided > by the OS. > >> >>>> Perhaps it's trying to ensure that if this gets compiled into an ldr >>>> instruction, the referenced data value is in a linker section that's >>>> still around when EFI runs? If so fine, but how is that ensured for all >>>> the other constants that this code uses, and if that happens >>>> automatically due to the __efi_runtime marker above, why doesn't it >>>> work >>>> for this one constant? >>>> >>>> Does U-Boot have a halt/poweroff/shutdown shell command? If so, it >>>> might >>>> be nice to enable it as part of this series, since the code to perform >>>> that operation is now present. >>> >>> That's what I originally wanted, yes :). Unfortunately due to the >>> relocation explained above, it's basically impossible for any reset >>> function that calls into MMIO space. >>> >>> However, we do have it now for PSCI. If you have a PSCI enabled system, >>> we don't need to call into MMIO space and thus make the common reset >>> function available as RTS. >> >> Can't the same U-Boot function be called both (a) during U-Boot runtime, >> where wdog_regs are pre-initialized to match U-Boot's MMU configuration, >> and (b) once the OS has booted, where wdog_regs has been modified >> according to the new memory map? > > That's exactly what this patch does, no? I assume not, since you said just a few lines above that doing so was impossible, hence why it doesn't implement any halt/poweroff/shutdown shell commands.