From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stephen Warren Date: Mon, 7 Nov 2016 20:26:32 -0700 Subject: [U-Boot] [PATCH v3 1/3] ARM: bcm283x: Implement EFI RTS reset_system In-Reply-To: <4aa518ba-555e-5caf-7e53-030510a04ea4@suse.de> 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> Message-ID: <638879b7-d206-69b2-2e32-53ec00c9d2cd@wwwdotorg.org> 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/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. >> 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? If not, one could implement a reset/powerdown/... function that takes the MMIO virtual address as a pointer, and then separate trivial wrappers that pass in either the static/U-Boot MMIO address, or the value of the EFI runtime variable that points at the MMIO mapping.