| Message ID | 1384678169-28228-1-git-send-email-acourbot@nvidia.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 17 November 2013 08:49, Alexandre Courbot <acourbot@nvidia.com> wrote: > The ARM tree includes a firmware_ops interface that is designed to > implement support for simple, TrustZone-based firmwares but could > also cover other use-cases. It has been suggested that this > interface might be useful to other architectures (e.g. arm64) and > that it should be moved out of arch/arm. NAK. I'm for code sharing with arm via common locations but this API goes against the ARMv8 firmware standardisation efforts like PSCI, encouraging each platform to define there own non-standard interface. > --- /dev/null > +++ b/include/linux/platform_firmware.h > @@ -0,0 +1,69 @@ > +/* > + * Copyright (C) 2012 Samsung Electronics. > + * Kyungmin Park <kyungmin.park@samsung.com> > + * Tomasz Figa <t.figa@samsung.com> > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 as > + * published by the Free Software Foundation. > + */ > + > +#ifndef _PLATFORM_FIRMWARE_H > +#define _PLATFORM_FIRMWARE_H > + > +#include <linux/bug.h> > + > +/* > + * struct platform_firmware_ops > + * > + * A structure to specify available firmware operations. > + * > + * A filled up structure can be registered with > + * register_platform_firmware_ops(). > + */ > +struct platform_firmware_ops { > + /* > + * Enters CPU idle mode > + */ > + int (*do_idle)(void); Covered by PSCI already. > + /* > + * Sets boot address of specified physical CPU > + */ > + int (*set_cpu_boot_addr)(int cpu, unsigned long boot_addr); Covered either by PSCI or spin-table release method (PSCI if firmware call is required). > + /* > + * Boots specified physical CPU > + */ > + int (*cpu_boot)(int cpu); PSCI. > + /* > + * Initializes L2 cache > + */ > + int (*l2x0_init)(void); No L2x0 (L210, L220, PL310) cache on ARMv8. And here I strongly recommend the hardware people to make proper external caches which can be flushed by standard CPU instructions, not MMIO. Any such caches must be enabled by firmware before Linux starts. The above firmware API is 32-bit ARM only. Form 64-bit ARM, you have the choice of PSCI so far but as I said in a long thread to Nico, I'm open to other standard interfaces if there are good reasons PSCI cannot be used. Note the _standard_ part, I don't want every SoC with their own firmware API for standard things like secondary CPU booting/hotplug/idle.
On 11/18/2013 12:59 AM, Catalin Marinas wrote: > On 17 November 2013 08:49, Alexandre Courbot <acourbot@nvidia.com> wrote: >> The ARM tree includes a firmware_ops interface that is designed to >> implement support for simple, TrustZone-based firmwares but could >> also cover other use-cases. It has been suggested that this >> interface might be useful to other architectures (e.g. arm64) and >> that it should be moved out of arch/arm. > > NAK. I'm for code sharing with arm via common locations but this API > goes against the ARMv8 firmware standardisation efforts like PSCI, > encouraging each platform to define there own non-standard interface. I have to say, I pretty much agree with your NAK. The reason for this patch is that the suggestion to move firmware_ops out of arch/arm is the last (I hope) thing that prevents my Trusted Foundation support series from being merged. Now if we can all agree: * that ARMv8 will only use PSCI * that there is no use-case of this interface outside of arch/arm as of today (and none foreseen in the near future) * that the firmware_ops interface is quite ARMv7-specific anyway, * that should a need to move it (for whatever reason) occur later, it will be easy to do (as this patch hopefully demonstrates). ... then this has indeed no reason to be. And maybe I can finally get Russell's blessing on my series. > >> --- /dev/null >> +++ b/include/linux/platform_firmware.h >> @@ -0,0 +1,69 @@ >> +/* >> + * Copyright (C) 2012 Samsung Electronics. >> + * Kyungmin Park <kyungmin.park@samsung.com> >> + * Tomasz Figa <t.figa@samsung.com> >> + * >> + * This program is free software; you can redistribute it and/or modify >> + * it under the terms of the GNU General Public License version 2 as >> + * published by the Free Software Foundation. >> + */ >> + >> +#ifndef _PLATFORM_FIRMWARE_H >> +#define _PLATFORM_FIRMWARE_H >> + >> +#include <linux/bug.h> >> + >> +/* >> + * struct platform_firmware_ops >> + * >> + * A structure to specify available firmware operations. >> + * >> + * A filled up structure can be registered with >> + * register_platform_firmware_ops(). >> + */ >> +struct platform_firmware_ops { >> + /* >> + * Enters CPU idle mode >> + */ >> + int (*do_idle)(void); > > Covered by PSCI already. > >> + /* >> + * Sets boot address of specified physical CPU >> + */ >> + int (*set_cpu_boot_addr)(int cpu, unsigned long boot_addr); > > Covered either by PSCI or spin-table release method (PSCI if firmware > call is required). > >> + /* >> + * Boots specified physical CPU >> + */ >> + int (*cpu_boot)(int cpu); > > PSCI. > >> + /* >> + * Initializes L2 cache >> + */ >> + int (*l2x0_init)(void); > > No L2x0 (L210, L220, PL310) cache on ARMv8. And here I strongly > recommend the hardware people to make proper external caches which can > be flushed by standard CPU instructions, not MMIO. Any such caches > must be enabled by firmware before Linux starts. > > The above firmware API is 32-bit ARM only. Form 64-bit ARM, you have > the choice of PSCI so far but as I said in a long thread to Nico, I'm > open to other standard interfaces if there are good reasons PSCI > cannot be used. Note the _standard_ part, I don't want every SoC with > their own firmware API for standard things like secondary CPU > booting/hotplug/idle. >
On Mon, Nov 18, 2013 at 03:05:59AM +0000, Alex Courbot wrote: > On 11/18/2013 12:59 AM, Catalin Marinas wrote: > > On 17 November 2013 08:49, Alexandre Courbot <acourbot@nvidia.com> wrote: > >> The ARM tree includes a firmware_ops interface that is designed to > >> implement support for simple, TrustZone-based firmwares but could > >> also cover other use-cases. It has been suggested that this > >> interface might be useful to other architectures (e.g. arm64) and > >> that it should be moved out of arch/arm. > > > > NAK. I'm for code sharing with arm via common locations but this API > > goes against the ARMv8 firmware standardisation efforts like PSCI, > > encouraging each platform to define there own non-standard interface. > > I have to say, I pretty much agree with your NAK. > > The reason for this patch is that the suggestion to move firmware_ops > out of arch/arm is the last (I hope) thing that prevents my Trusted > Foundation support series from being merged. Moving it into drivers shouldn't be a workaround. Nice try ;). > Now if we can all agree: > > * that ARMv8 will only use PSCI Or spin-table (which does not require secure calls). Otherwise, if secure firmware is present, SoCs should use PSCI (as the only firmware standard currently supported in the arm64 kernel). However, things evolve and we may have other needs in the future or PSCI may not be sufficient or we get newer PSCI revisions. This can be extended but my requirement is to decouple booting standard from SoC support (together with the aim of having no SoC-specific code under arch/arm64). I really don't see why SoCs can't agree on one (or very few) standard booting protocol (and legacy argument doesn't work since the ARMv8 firmware needs to be converted to AArch64 anyway). > * that there is no use-case of this interface outside of arch/arm as of > today (and none foreseen in the near future) The firmware_ops are only used under arch/arm so far, I don't see any drivers doing anything with it. Also, l2x0_init is ARMv7 only. On arm64, support for PSCI is handled via cpu_operations in the latest kernel. That's an arm64 abstraction and is extensible (but we want to keep tight control of this, hence no register_cpu_ops function). > * that the firmware_ops interface is quite ARMv7-specific anyway, This was introduced to allow SoC code to enable hooks for SoC-specific firmware calls like cpu_idle, l2x0_init. By standardising the interface and decoupling it from SoC code on arm64, we don't need per-SoC firmware_ops. Of course, trusted foundations interface could be plugged into cpu_ops on arm64 but I will NAK it on the grounds of not using the PSCI API, nor the SMC calling convention (and it's easy to fix when porting to ARMv8). If a supported standard API is used, then there is no need for additional code in the kernel. BTW, is legacy code the reason for not converting the SMC # to PSCI? It's already supported on ARMv7, so you may not have much code left to merge in the kernel ;). > * that should a need to move it (for whatever reason) occur later, it > will be easy to do (as this patch hopefully demonstrates). I agree, it's not hard to unify this but so far I haven't seen a good reason.
On 11/17/2013 08:59 AM, Catalin Marinas wrote: > On 17 November 2013 08:49, Alexandre Courbot <acourbot@nvidia.com> wrote: >> The ARM tree includes a firmware_ops interface that is designed to >> implement support for simple, TrustZone-based firmwares but could >> also cover other use-cases. It has been suggested that this >> interface might be useful to other architectures (e.g. arm64) and >> that it should be moved out of arch/arm. > > NAK. I'm for code sharing with arm via common locations but this API > goes against the ARMv8 firmware standardisation efforts like PSCI, > encouraging each platform to define there own non-standard interface. Surely PSCI is *an* implementation of firmware_ops? Couldn't firmware_ops be relevant to non-ARM architectures too? If so, that would support my previous point; we're presumably not requiring non-ARM architectures to implement PSCI? On a practical note, unless ARM mandates by ARM architecture licensing condition that mechanisms other than PSCI are not allowed, then they're going to exist even if the upstream Linux community doesn't like it. History has certainly shown that.
On 11/18/2013 04:58 AM, Catalin Marinas wrote: ... > Of course, trusted foundations interface could be plugged into cpu_ops > on arm64 but I will NAK it on the grounds of not using the PSCI API, nor > the SMC calling convention (and it's easy to fix when porting to ARMv8). > If a supported standard API is used, then there is no need for > additional code in the kernel. What happens when someone takes an existing working secure-mode SW stack and simply re-uses it on some new ARMv8 SoC. Are you going to force people working on upstream to re-write the secure mode firmware in shipped hardware before allowing upstream kernel support?
On Mon, Nov 18, 2013 at 10:03:37AM -0700, Stephen Warren wrote: > On 11/18/2013 04:58 AM, Catalin Marinas wrote: > ... > > Of course, trusted foundations interface could be plugged into cpu_ops > > on arm64 but I will NAK it on the grounds of not using the PSCI API, nor > > the SMC calling convention (and it's easy to fix when porting to ARMv8). > > If a supported standard API is used, then there is no need for > > additional code in the kernel. > > What happens when someone takes an existing working secure-mode SW stack > and simply re-uses it on some new ARMv8 SoC. Are you going to force > people working on upstream to re-write the secure mode firmware in > shipped hardware before allowing upstream kernel support? I'll tell you what will happen. They'll say "screw mainline, we're doing our own thing". Vendors have been doing that for years, this will be no different and require no additional effort from them.
On Sun, Nov 17, 2013 at 03:59:04PM +0000, Catalin Marinas wrote: > No L2x0 (L210, L220, PL310) cache on ARMv8. And here I strongly > recommend the hardware people to make proper external caches which can > be flushed by standard CPU instructions, not MMIO. Any such caches > must be enabled by firmware before Linux starts. Yea, and we've seen what a trainwreck that causes on 32-bit ARM with the kernel exploding at boot time because of a dirty L2 cache.
On 11/18/2013 10:10 AM, Russell King - ARM Linux wrote: > On Mon, Nov 18, 2013 at 10:03:37AM -0700, Stephen Warren wrote: >> On 11/18/2013 04:58 AM, Catalin Marinas wrote: >> ... >>> Of course, trusted foundations interface could be plugged into cpu_ops >>> on arm64 but I will NAK it on the grounds of not using the PSCI API, nor >>> the SMC calling convention (and it's easy to fix when porting to ARMv8). >>> If a supported standard API is used, then there is no need for >>> additional code in the kernel. >> >> What happens when someone takes an existing working secure-mode SW stack >> and simply re-uses it on some new ARMv8 SoC. Are you going to force >> people working on upstream to re-write the secure mode firmware in >> shipped hardware before allowing upstream kernel support? > > I'll tell you what will happen. They'll say "screw mainline, we're doing > our own thing". Vendors have been doing that for years, this will be no > different and require no additional effort from them. Yes, that's my point. However, what does that mean for those people within the company trying to move the SW stack towards mainline? If the answer from upstream is: "no, you can't support the shipping HW in mainline", rather than a practical approach that recognizes that the HW/SW/firmware really does exist and might be useful to support in mainline, then that just makes the life of people trying to push for mainline that much harder.
On Mon, Nov 18, 2013 at 05:00:32PM +0000, Stephen Warren wrote: > On 11/17/2013 08:59 AM, Catalin Marinas wrote: > > On 17 November 2013 08:49, Alexandre Courbot <acourbot@nvidia.com> wrote: > >> The ARM tree includes a firmware_ops interface that is designed to > >> implement support for simple, TrustZone-based firmwares but could > >> also cover other use-cases. It has been suggested that this > >> interface might be useful to other architectures (e.g. arm64) and > >> that it should be moved out of arch/arm. > > > > NAK. I'm for code sharing with arm via common locations but this API > > goes against the ARMv8 firmware standardisation efforts like PSCI, > > encouraging each platform to define there own non-standard interface. > > Surely PSCI is *an* implementation of firmware_ops? > > Couldn't firmware_ops be relevant to non-ARM architectures too? There are similarities but you don't ask other architectures to implement an l2x0_init function. If we find other things we want to describe in here, does this structure become a pool of function pointers to be shared by other architectures? What's the common functionality that you want to place in this structure? > If so, that would support my previous point; we're presumably not > requiring non-ARM architectures to implement PSCI? So you think non-ARM architectures could make use of the firmware_ops? > On a practical note, unless ARM mandates by ARM architecture licensing > condition that mechanisms other than PSCI are not allowed, then they're > going to exist even if the upstream Linux community doesn't like it. > History has certainly shown that. I'm pretty sure they will exist, as there is tons of kernel code in production devices that never reach mainline. And I already stated a few times, this interface can be extended, I just don't want one per SoC just because some vendors want to use different SMC numbers.
On Mon, Nov 18, 2013 at 05:03:37PM +0000, Stephen Warren wrote: > On 11/18/2013 04:58 AM, Catalin Marinas wrote: > ... > > Of course, trusted foundations interface could be plugged into cpu_ops > > on arm64 but I will NAK it on the grounds of not using the PSCI API, nor > > the SMC calling convention (and it's easy to fix when porting to ARMv8). > > If a supported standard API is used, then there is no need for > > additional code in the kernel. > > What happens when someone takes an existing working secure-mode SW stack > and simply re-uses it on some new ARMv8 SoC. Are you going to force > people working on upstream to re-write the secure mode firmware in > shipped hardware before allowing upstream kernel support? Don't confuse the secure stack with the secure monitor running at EL3. If you want AArch64 support for lower levels (EL2, EL1, EL0), your monitor _must_ be AArch64. You can't run legacy AArch32 code at EL3 and have lower levels in AArch64 mode (architectural constraint). You can still keep the secure services at S-EL1 in AArch32, only that the SMCs are handled by EL3 (and that's another aspect the SMC calling convention spec is trying to address, mixed register-width secure/non-secure OSes).
On 11/18/2013 10:30 AM, Catalin Marinas wrote: > On Mon, Nov 18, 2013 at 05:03:37PM +0000, Stephen Warren wrote: >> On 11/18/2013 04:58 AM, Catalin Marinas wrote: >> ... >>> Of course, trusted foundations interface could be plugged into cpu_ops >>> on arm64 but I will NAK it on the grounds of not using the PSCI API, nor >>> the SMC calling convention (and it's easy to fix when porting to ARMv8). >>> If a supported standard API is used, then there is no need for >>> additional code in the kernel. >> >> What happens when someone takes an existing working secure-mode SW stack >> and simply re-uses it on some new ARMv8 SoC. Are you going to force >> people working on upstream to re-write the secure mode firmware in >> shipped hardware before allowing upstream kernel support? > > Don't confuse the secure stack with the secure monitor running at EL3. > If you want AArch64 support for lower levels (EL2, EL1, EL0), your > monitor _must_ be AArch64. You can't run legacy AArch32 code at EL3 and > have lower levels in AArch64 mode (architectural constraint). I was assuming that vendors would take the existing source code and simply rebuild it to create the AArch64 secure world. As such, the same SMC IDs, same structures, etc. would be used. The only source difference would be to perhaps change some 32-bit registers/struct-fields up to 64-bit. Naively that sounds like the lowest-effort way to get an AArch64 secure world, so I'm purely guessing that that's what vendors will do. > You can > still keep the secure services at S-EL1 in AArch32, only that the SMCs > are handled by EL3 (and that's another aspect the SMC calling convention > spec is trying to address, mixed register-width secure/non-secure OSes). I'm not sure of the implications of that statement. Since you mention SMCs being handled by EL3, I think the quick-and-dirty conversion I mention above is still likely to be used.
Hi Catalin, On 11/18/2013 12:30 PM, Catalin Marinas wrote: [...] > You can't run legacy AArch32 code at EL3 and have lower levels in AArch64 > mode (architectural constraint). What prevents AArch32 code from running at EL3 and then requesting a reset to AArch64 by writing to the Reset Management Register before sliding down to lower exception levels? Thanks, Christopher
On 11/18/2013 08:58 PM, Catalin Marinas wrote: > On Mon, Nov 18, 2013 at 03:05:59AM +0000, Alex Courbot wrote: >> On 11/18/2013 12:59 AM, Catalin Marinas wrote: >>> On 17 November 2013 08:49, Alexandre Courbot <acourbot@nvidia.com> wrote: >>>> The ARM tree includes a firmware_ops interface that is designed to >>>> implement support for simple, TrustZone-based firmwares but could >>>> also cover other use-cases. It has been suggested that this >>>> interface might be useful to other architectures (e.g. arm64) and >>>> that it should be moved out of arch/arm. >>> >>> NAK. I'm for code sharing with arm via common locations but this API >>> goes against the ARMv8 firmware standardisation efforts like PSCI, >>> encouraging each platform to define there own non-standard interface. >> >> I have to say, I pretty much agree with your NAK. >> >> The reason for this patch is that the suggestion to move firmware_ops >> out of arch/arm is the last (I hope) thing that prevents my Trusted >> Foundation support series from being merged. > > Moving it into drivers shouldn't be a workaround. Nice try ;). Hehe. I thought that just sending a patch would settle the issue one way or the other and avoid a huge discussion. Woke up this morning to see how wrong I was. > >> Now if we can all agree: >> >> * that ARMv8 will only use PSCI > > Or spin-table (which does not require secure calls). Otherwise, if > secure firmware is present, SoCs should use PSCI (as the only firmware > standard currently supported in the arm64 kernel). > > However, things evolve and we may have other needs in the future or PSCI > may not be sufficient or we get newer PSCI revisions. This can be > extended but my requirement is to decouple booting standard from SoC > support (together with the aim of having no SoC-specific code under > arch/arm64). I really don't see why SoCs can't agree on one (or very > few) standard booting protocol (and legacy argument doesn't work since > the ARMv8 firmware needs to be converted to AArch64 anyway). > >> * that there is no use-case of this interface outside of arch/arm as of >> today (and none foreseen in the near future) > > The firmware_ops are only used under arch/arm so far, I don't see any > drivers doing anything with it. Also, l2x0_init is ARMv7 only. > > On arm64, support for PSCI is handled via cpu_operations in the latest > kernel. That's an arm64 abstraction and is extensible (but we want to > keep tight control of this, hence no register_cpu_ops function). > >> * that the firmware_ops interface is quite ARMv7-specific anyway, > > This was introduced to allow SoC code to enable hooks for SoC-specific > firmware calls like cpu_idle, l2x0_init. By standardising the interface > and decoupling it from SoC code on arm64, we don't need per-SoC > firmware_ops. > > Of course, trusted foundations interface could be plugged into cpu_ops > on arm64 but I will NAK it on the grounds of not using the PSCI API, nor > the SMC calling convention (and it's easy to fix when porting to ARMv8). > If a supported standard API is used, then there is no need for > additional code in the kernel. > > BTW, is legacy code the reason for not converting the SMC # to PSCI? > It's already supported on ARMv7, so you may not have much code left to > merge in the kernel ;). The problem here is twofold: 1) we are just consumers of the TrustZone secure monitor who receive a binary and do not have any control over its calling conventions. I agree that it would be trivial to make it compatible with PSCI, but it's just not something we can make by ourselves (TF does not even follow the SMC calling convention). If this problem is to be addressed, it should be done by forcing the TrustZone secure monitors providers to follow PSCI. 2) devices have already shipped with this firmware. Are we going to just renounce supporting them, even though the necessary support is lightweight and fits within already existing interfaces? I certainly do hope that for ARMv8 things will be different and more standardized. But that's not something that can be guaranteed unless ARM strongly enforces it to firmware vendors. In case such a non-standard firmware gets used again, I *do* hope that using cpu_ops will be preferred over saying "this device cannot be supported in mainline, ever". The kernel already supports non-standard hardware, BIOS, ACPI through hacks that are *way* more horrible than that. This should certainly not be encouraged, but that's not a valid reason to forbid otherwise perfectly fine devices to run mainline IMHO. > >> * that should a need to move it (for whatever reason) occur later, it >> will be easy to do (as this patch hopefully demonstrates). > > I agree, it's not hard to unify this but so far I haven't seen a good > reason. Same here. arm64 has its own cpu_operations. Other archs have different needs and if we move this to a common place it will just become a messy placeholder for function pointers from which each arch will only use a subset. Not to mention that if we follow the logic completely, we should then implement PCSI on top of cpu_ops and cpu_ops on top of firmware_ops (or the other way around ;)). Alex.
On Mon, Nov 18, 2013 at 07:04:50PM +0000, Christopher Covington wrote: > On 11/18/2013 12:30 PM, Catalin Marinas wrote: > [...] > > You can't run legacy AArch32 code at EL3 and have lower levels in AArch64 > > mode (architectural constraint). > > What prevents AArch32 code from running at EL3 and then requesting a reset to > AArch64 by writing to the Reset Management Register before sliding down to > lower exception levels? You can do this for some initial code but the firmware still needs to switch to AArch64 before dropping to lower exception levels. What this thread is about is run-time calls to firmware for booting secondary CPUs, idle, l2x0. At this point, the code at EL3 must run in AArch64 mode. There is no way you can bounce between AArch32 and AArch64 modes using reset just to handle some SMCs.
On Mon, Nov 18, 2013 at 05:52:36PM +0000, Stephen Warren wrote: > On 11/18/2013 10:30 AM, Catalin Marinas wrote: > > On Mon, Nov 18, 2013 at 05:03:37PM +0000, Stephen Warren wrote: > >> On 11/18/2013 04:58 AM, Catalin Marinas wrote: > >> ... > >>> Of course, trusted foundations interface could be plugged into cpu_ops > >>> on arm64 but I will NAK it on the grounds of not using the PSCI API, nor > >>> the SMC calling convention (and it's easy to fix when porting to ARMv8). > >>> If a supported standard API is used, then there is no need for > >>> additional code in the kernel. > >> > >> What happens when someone takes an existing working secure-mode SW stack > >> and simply re-uses it on some new ARMv8 SoC. Are you going to force > >> people working on upstream to re-write the secure mode firmware in > >> shipped hardware before allowing upstream kernel support? > > > > Don't confuse the secure stack with the secure monitor running at EL3. > > If you want AArch64 support for lower levels (EL2, EL1, EL0), your > > monitor _must_ be AArch64. You can't run legacy AArch32 code at EL3 and > > have lower levels in AArch64 mode (architectural constraint). > > I was assuming that vendors would take the existing source code and > simply rebuild it to create the AArch64 secure world. Some C code can probably be reused but not the EL3 entry/exit code, world switching and AArch64 initialisation. The main differences in ARMv8 EL3 is that it has its own MMU and it can only be entered via SMC and exit via ERET (you can no longer switch from/to secure SVC by writing to CPSR). So apart from a different instruction set, the new exception model requires a rewrite of the secure monitor logic used to handle SMCs, switch worlds, pass arguments between worlds. > As such, the same SMC IDs, same structures, etc. would be used. The > only source difference would be to perhaps change some 32-bit > registers/struct-fields up to 64-bit. Naively that sounds like the > lowest-effort way to get an AArch64 secure world, so I'm purely > guessing that that's what vendors will do. It looks simpler in theory until you hit the new exception model and realise the clear separation between EL3 (previously secure monitor) and secure EL1 (previously secure SVC). I'm not referring to the whole secure stack here, just the things I mentioned above. > > You can > > still keep the secure services at S-EL1 in AArch32, only that the SMCs > > are handled by EL3 (and that's another aspect the SMC calling convention > > spec is trying to address, mixed register-width secure/non-secure OSes). > > I'm not sure of the implications of that statement. Since you mention > SMCs being handled by EL3, I think the quick-and-dirty conversion I > mention above is still likely to be used. What I meant is that a secure OS (providing cryptography, banking etc. services) can run in secure EL1 in AArch32 mode, it does not need to be converted (though it helps from a performance perspective, new instructions). But the world switching (which is pretty tightly coupled with secure SVC on ARMv7) and SMC handling need to be rewritten. And it's usually EL3 where you would place power management firmware on ARMv8 (cache/TLB maintenance, power controller access). This is usually done by the SoC vendor and not the secure OS provider (the latter may do the final link).
On Tue, Nov 19, 2013 at 02:46:55AM +0000, Alex Courbot wrote: > On 11/18/2013 08:58 PM, Catalin Marinas wrote: > > On Mon, Nov 18, 2013 at 03:05:59AM +0000, Alex Courbot wrote: > >> On 11/18/2013 12:59 AM, Catalin Marinas wrote: > >>> On 17 November 2013 08:49, Alexandre Courbot <acourbot@nvidia.com> wrote: > >>>> The ARM tree includes a firmware_ops interface that is designed to > >>>> implement support for simple, TrustZone-based firmwares but could > >>>> also cover other use-cases. It has been suggested that this > >>>> interface might be useful to other architectures (e.g. arm64) and > >>>> that it should be moved out of arch/arm. > >>> > >>> NAK. I'm for code sharing with arm via common locations but this API > >>> goes against the ARMv8 firmware standardisation efforts like PSCI, > >>> encouraging each platform to define there own non-standard interface. > >> > >> I have to say, I pretty much agree with your NAK. > >> > >> The reason for this patch is that the suggestion to move firmware_ops > >> out of arch/arm is the last (I hope) thing that prevents my Trusted > >> Foundation support series from being merged. > > > > Moving it into drivers shouldn't be a workaround. Nice try ;). > > Hehe. I thought that just sending a patch would settle the issue one way > or the other and avoid a huge discussion. Woke up this morning to see > how wrong I was. It's a sensitive topic ;). > > BTW, is legacy code the reason for not converting the SMC # to PSCI? > > It's already supported on ARMv7, so you may not have much code left to > > merge in the kernel ;). > > The problem here is twofold: > > 1) we are just consumers of the TrustZone secure monitor who receive a > binary and do not have any control over its calling conventions. I agree > that it would be trivial to make it compatible with PSCI, but it's just > not something we can make by ourselves (TF does not even follow the SMC > calling convention). If this problem is to be addressed, it should be > done by forcing the TrustZone secure monitors providers to follow PSCI. I agree and such discussions do happen ('forcing' is a bit harder, more like 'strongly recommending'). On my side, I voice this message via the Linux channels, so SoC vendors can also encourage their secure provider in this direction. The benefit is that the Linux changes are minimal afterwards, single image is easier. But as I replied to Stephen, make sure you separate the secure OS (EL1) from the secure firmware (EL3). The latter (or parts of it) are provided by the SoC vendor (e.g. NVidia) and may be eventually linked into a big blob by the secure OS provider. ARM is encouraging separation here and a multi-stage firmware loading approach (and ARM started a public generic firmware project, it's in the early days now). > 2) devices have already shipped with this firmware. Are we going to just > renounce supporting them, even though the necessary support is > lightweight and fits within already existing interfaces? I'm talking only about ARMv8 here. Please see my reply to Stephen for the details of (not) reusing existing firmware. > I certainly do hope that for ARMv8 things will be different and more > standardized. But that's not something that can be guaranteed unless ARM > strongly enforces it to firmware vendors. In case such a non-standard > firmware gets used again, I *do* hope that using cpu_ops will be > preferred over saying "this device cannot be supported in mainline, ever". cpu_ops or firmware_ops is just a name and can be unified (TBD what common functionality it contains). What I don't want to encourage is each SoC registering its own firmware interface. > The kernel already supports non-standard hardware, BIOS, ACPI through > hacks that are *way* more horrible than that. This should certainly not > be encouraged, but that's not a valid reason to forbid otherwise > perfectly fine devices to run mainline IMHO. So you say we should just stop trying to standardise anything because people don't care anyway. Why do we even bother with DT (or ACPI) since board files were fine in the past (with a bit more code)? > >> * that should a need to move it (for whatever reason) occur later, it > >> will be easy to do (as this patch hopefully demonstrates). > > > > I agree, it's not hard to unify this but so far I haven't seen a good > > reason. > > Same here. arm64 has its own cpu_operations. Other archs have different > needs and if we move this to a common place it will just become a messy > placeholder for function pointers from which each arch will only use a > subset. That was my initial point but it turned into a thread against PSCI (again ;)).
On Tue, Nov 19, 2013 at 9:26 PM, Catalin Marinas <catalin.marinas@arm.com> wrote: > On Tue, Nov 19, 2013 at 02:46:55AM +0000, Alex Courbot wrote: >> On 11/18/2013 08:58 PM, Catalin Marinas wrote: >> > On Mon, Nov 18, 2013 at 03:05:59AM +0000, Alex Courbot wrote: >> >> On 11/18/2013 12:59 AM, Catalin Marinas wrote: >> >>> On 17 November 2013 08:49, Alexandre Courbot <acourbot@nvidia.com> wrote: >> >>>> The ARM tree includes a firmware_ops interface that is designed to >> >>>> implement support for simple, TrustZone-based firmwares but could >> >>>> also cover other use-cases. It has been suggested that this >> >>>> interface might be useful to other architectures (e.g. arm64) and >> >>>> that it should be moved out of arch/arm. >> >>> >> >>> NAK. I'm for code sharing with arm via common locations but this API >> >>> goes against the ARMv8 firmware standardisation efforts like PSCI, >> >>> encouraging each platform to define there own non-standard interface. >> >> >> >> I have to say, I pretty much agree with your NAK. >> >> >> >> The reason for this patch is that the suggestion to move firmware_ops >> >> out of arch/arm is the last (I hope) thing that prevents my Trusted >> >> Foundation support series from being merged. >> > >> > Moving it into drivers shouldn't be a workaround. Nice try ;). >> >> Hehe. I thought that just sending a patch would settle the issue one way >> or the other and avoid a huge discussion. Woke up this morning to see >> how wrong I was. > > It's a sensitive topic ;). > >> > BTW, is legacy code the reason for not converting the SMC # to PSCI? >> > It's already supported on ARMv7, so you may not have much code left to >> > merge in the kernel ;). >> >> The problem here is twofold: >> >> 1) we are just consumers of the TrustZone secure monitor who receive a >> binary and do not have any control over its calling conventions. I agree >> that it would be trivial to make it compatible with PSCI, but it's just >> not something we can make by ourselves (TF does not even follow the SMC >> calling convention). If this problem is to be addressed, it should be >> done by forcing the TrustZone secure monitors providers to follow PSCI. > > I agree and such discussions do happen ('forcing' is a bit harder, more > like 'strongly recommending'). On my side, I voice this message via the > Linux channels, so SoC vendors can also encourage their secure provider > in this direction. The benefit is that the Linux changes are minimal > afterwards, single image is easier. > > But as I replied to Stephen, make sure you separate the secure OS (EL1) > from the secure firmware (EL3). The latter (or parts of it) are provided > by the SoC vendor (e.g. NVidia) and may be eventually linked into a big > blob by the secure OS provider. ARM is encouraging separation here and a > multi-stage firmware loading approach (and ARM started a public generic > firmware project, it's in the early days now). Will keep that in mind and check whether that could apply to future devices, thanks. > >> 2) devices have already shipped with this firmware. Are we going to just >> renounce supporting them, even though the necessary support is >> lightweight and fits within already existing interfaces? > > I'm talking only about ARMv8 here. Please see my reply to Stephen for > the details of (not) reusing existing firmware. > >> I certainly do hope that for ARMv8 things will be different and more >> standardized. But that's not something that can be guaranteed unless ARM >> strongly enforces it to firmware vendors. In case such a non-standard >> firmware gets used again, I *do* hope that using cpu_ops will be >> preferred over saying "this device cannot be supported in mainline, ever". > > cpu_ops or firmware_ops is just a name and can be unified (TBD what > common functionality it contains). What I don't want to encourage is > each SoC registering its own firmware interface. Sorry, are you talking about interface as in SMC interface, or as in cpu_operations/firmware_ops? > >> The kernel already supports non-standard hardware, BIOS, ACPI through >> hacks that are *way* more horrible than that. This should certainly not >> be encouraged, but that's not a valid reason to forbid otherwise >> perfectly fine devices to run mainline IMHO. > > So you say we should just stop trying to standardise anything because > people don't care anyway. Why do we even bother with DT (or ACPI) since > board files were fine in the past (with a bit more code)? Oh no, that's not what I am saying at all. Standardization is good. PSCI is good. Of course I would prefer that the secure monitor we use follow established conventions - that'd be less work to support it and less hassle to get my patches merged. I may have misunderstood you, but I felt your mail sounded a bit like "we won't merge support for firmwares that do not follow PSCI". I agree that whenever it is possible to support a firmware through a standard interface, this should be done - no discussion. But right now I have two devices that are good representatives of Tegra 4 and available in stores, which I would like to see supported in mainline to satisfy requests from the community for Tegra development platforms, and also initiate the habit to support future NVIDIA-branded devices in mainline. Their secure monitor unfortunately does not follow PSCI or the SMC convention and needs a custom firmware_ops. Are they unworthy of mainline? And if, by sheer misfortune, the same thing happened on an ARMv8 device (despite the EL1/EL3 separation), what would be the outcome? IMHO, more devices in mainline is beneficial to everybody, and actually *encourages* SoC vendors/firmware providers to follow conventions. Banning devices is what triggers the kind of "screw it" reactions mentioned earlier, and on the contrary once a device is in, you tend to make sure the next ones follow the kernel trends because you know you will need to support them in mainline as well and it will make your life easier. > >> >> * that should a need to move it (for whatever reason) occur later, it >> >> will be easy to do (as this patch hopefully demonstrates). >> > >> > I agree, it's not hard to unify this but so far I haven't seen a good >> > reason. >> >> Same here. arm64 has its own cpu_operations. Other archs have different >> needs and if we move this to a common place it will just become a messy >> placeholder for function pointers from which each arch will only use a >> subset. > > That was my initial point but it turned into a thread against PSCI > (again ;)). Not at all, hurray for PSCI! :) But let the uncool devices get some mainline love too!
On Tue, Nov 19, 2013 at 02:29:39PM +0000, Alexandre Courbot wrote: > On Tue, Nov 19, 2013 at 9:26 PM, Catalin Marinas > <catalin.marinas@arm.com> wrote: > > On Tue, Nov 19, 2013 at 02:46:55AM +0000, Alex Courbot wrote: > >> 2) devices have already shipped with this firmware. Are we going to just > >> renounce supporting them, even though the necessary support is > >> lightweight and fits within already existing interfaces? > > > > I'm talking only about ARMv8 here. Please see my reply to Stephen for > > the details of (not) reusing existing firmware. > > > >> I certainly do hope that for ARMv8 things will be different and more > >> standardized. But that's not something that can be guaranteed unless ARM > >> strongly enforces it to firmware vendors. In case such a non-standard > >> firmware gets used again, I *do* hope that using cpu_ops will be > >> preferred over saying "this device cannot be supported in mainline, ever". > > > > cpu_ops or firmware_ops is just a name and can be unified (TBD what > > common functionality it contains). What I don't want to encourage is > > each SoC registering its own firmware interface. > > Sorry, are you talking about interface as in SMC interface, or as in > cpu_operations/firmware_ops? Both. I don't want to see platforms defining their own SMC interface for no good reason. The cpu_ops/firmware_ops handling in the kernel is just some naming but the key is having standard SMC interfaces for CPU operations. > >> The kernel already supports non-standard hardware, BIOS, ACPI through > >> hacks that are *way* more horrible than that. This should certainly not > >> be encouraged, but that's not a valid reason to forbid otherwise > >> perfectly fine devices to run mainline IMHO. > > > > So you say we should just stop trying to standardise anything because > > people don't care anyway. Why do we even bother with DT (or ACPI) since > > board files were fine in the past (with a bit more code)? > > Oh no, that's not what I am saying at all. Standardization is good. > PSCI is good. Of course I would prefer that the secure monitor we use > follow established conventions - that'd be less work to support it and > less hassle to get my patches merged. > > I may have misunderstood you, but I felt your mail sounded a bit like > "we won't merge support for firmwares that do not follow PSCI". Just to clarify it: I won't merge support for _ARMv8_ firmware that does not follow a standard CPU booting/power protocol supported by Linux. Currently we only support PSCI. If there is a need for another protocol and a good proposal, I'm open for discussions. The above is all related to having no SoC code under arch/arm64 (or board files, whatever you want to call them). > I > agree that whenever it is possible to support a firmware through a > standard interface, this should be done - no discussion. But right now > I have two devices that are good representatives of Tegra 4 and > available in stores, which I would like to see supported in mainline > to satisfy requests from the community for Tegra development > platforms, and also initiate the habit to support future > NVIDIA-branded devices in mainline. Their secure monitor unfortunately > does not follow PSCI or the SMC convention and needs a custom > firmware_ops. Are they unworthy of mainline? Are they ARMv8? Since we didn't have any such rules on ARMv7 and earlier, standard secure interface is nice to have but should not prevent upstreaming. I made this clear already that it is ARMv8 only, please don't try to generalise it. > And if, by sheer misfortune, the same thing happened on an ARMv8 > device (despite the EL1/EL3 separation), what would be the outcome? If some people get it wrong and they have to work around firmware bugs for devices already in the field, we may need to bend the rules (bugs do happen, both in software and hardware). But definitely _not_ when people don't even bother. > IMHO, more devices in mainline is beneficial to everybody, and > actually *encourages* SoC vendors/firmware providers to follow > conventions. Banning devices is what triggers the kind of "screw it" > reactions mentioned earlier, By following some rules and doing things in a standard way (firmware interface in this case), it is more likely that their SoC support requires minimal kernel code and it's easier to upstream and maintain. > and on the contrary once a device is in, > you tend to make sure the next ones follow the kernel trends because > you know you will need to support them in mainline as well and it will > make your life easier. Not really. The next device won't follow the kernel trends but just the same non-standard way of doing things that were accepted in the first place.
On Wed, Nov 20, 2013 at 12:07 AM, Catalin Marinas <catalin.marinas@arm.com> wrote: > On Tue, Nov 19, 2013 at 02:29:39PM +0000, Alexandre Courbot wrote: >> On Tue, Nov 19, 2013 at 9:26 PM, Catalin Marinas >> <catalin.marinas@arm.com> wrote: >> > On Tue, Nov 19, 2013 at 02:46:55AM +0000, Alex Courbot wrote: >> >> 2) devices have already shipped with this firmware. Are we going to just >> >> renounce supporting them, even though the necessary support is >> >> lightweight and fits within already existing interfaces? >> > >> > I'm talking only about ARMv8 here. Please see my reply to Stephen for >> > the details of (not) reusing existing firmware. >> > >> >> I certainly do hope that for ARMv8 things will be different and more >> >> standardized. But that's not something that can be guaranteed unless ARM >> >> strongly enforces it to firmware vendors. In case such a non-standard >> >> firmware gets used again, I *do* hope that using cpu_ops will be >> >> preferred over saying "this device cannot be supported in mainline, ever". >> > >> > cpu_ops or firmware_ops is just a name and can be unified (TBD what >> > common functionality it contains). What I don't want to encourage is >> > each SoC registering its own firmware interface. >> >> Sorry, are you talking about interface as in SMC interface, or as in >> cpu_operations/firmware_ops? > > Both. I don't want to see platforms defining their own SMC interface for > no good reason. The cpu_ops/firmware_ops handling in the kernel is just > some naming but the key is having standard SMC interfaces for CPU > operations. Fair enough. > >> >> The kernel already supports non-standard hardware, BIOS, ACPI through >> >> hacks that are *way* more horrible than that. This should certainly not >> >> be encouraged, but that's not a valid reason to forbid otherwise >> >> perfectly fine devices to run mainline IMHO. >> > >> > So you say we should just stop trying to standardise anything because >> > people don't care anyway. Why do we even bother with DT (or ACPI) since >> > board files were fine in the past (with a bit more code)? >> >> Oh no, that's not what I am saying at all. Standardization is good. >> PSCI is good. Of course I would prefer that the secure monitor we use >> follow established conventions - that'd be less work to support it and >> less hassle to get my patches merged. >> >> I may have misunderstood you, but I felt your mail sounded a bit like >> "we won't merge support for firmwares that do not follow PSCI". > > Just to clarify it: I won't merge support for _ARMv8_ firmware that does > not follow a standard CPU booting/power protocol supported by Linux. > Currently we only support PSCI. If there is a need for another protocol > and a good proposal, I'm open for discussions. > > The above is all related to having no SoC code under arch/arm64 (or > board files, whatever you want to call them). > >> I >> agree that whenever it is possible to support a firmware through a >> standard interface, this should be done - no discussion. But right now >> I have two devices that are good representatives of Tegra 4 and >> available in stores, which I would like to see supported in mainline >> to satisfy requests from the community for Tegra development >> platforms, and also initiate the habit to support future >> NVIDIA-branded devices in mainline. Their secure monitor unfortunately >> does not follow PSCI or the SMC convention and needs a custom >> firmware_ops. Are they unworthy of mainline? > > Are they ARMv8? Since we didn't have any such rules on ARMv7 and > earlier, standard secure interface is nice to have but should not > prevent upstreaming. I made this clear already that it is ARMv8 only, > please don't try to generalise it. Sorry, that was not my intention at all - I just misunderstood what you meant. Thanks for clarifying it. > >> And if, by sheer misfortune, the same thing happened on an ARMv8 >> device (despite the EL1/EL3 separation), what would be the outcome? > > If some people get it wrong and they have to work around firmware bugs > for devices already in the field, we may need to bend the rules (bugs do > happen, both in software and hardware). But definitely _not_ when people > don't even bother. Ok, I guess for ARMv8 there is absolutely no excuse not to follow PSCI anyways. We'll need to be careful about this one. > >> IMHO, more devices in mainline is beneficial to everybody, and >> actually *encourages* SoC vendors/firmware providers to follow >> conventions. Banning devices is what triggers the kind of "screw it" >> reactions mentioned earlier, > > By following some rules and doing things in a standard way (firmware > interface in this case), it is more likely that their SoC support > requires minimal kernel code and it's easier to upstream and maintain. > >> and on the contrary once a device is in, >> you tend to make sure the next ones follow the kernel trends because >> you know you will need to support them in mainline as well and it will >> make your life easier. > > Not really. The next device won't follow the kernel trends but just the > same non-standard way of doing things that were accepted in the first > place. I guess that depends on whether you see the glass as half-empty or half-full. ;) So just in case this was not clear already, this patch is withdrawn, right. :P I hope the ARM maintainers will be ok with Trusted Foundations support using firmware_ops in arch/arm for that's the best we can do. Thanks, Alex.
diff --git a/Documentation/arm/firmware.txt b/Documentation/arm/firmware.txt deleted file mode 100644 index c2e468f..0000000 --- a/Documentation/arm/firmware.txt +++ /dev/null @@ -1,88 +0,0 @@ -Interface for registering and calling firmware-specific operations for ARM. ----- -Written by Tomasz Figa <t.figa@samsung.com> - -Some boards are running with secure firmware running in TrustZone secure -world, which changes the way some things have to be initialized. This makes -a need to provide an interface for such platforms to specify available firmware -operations and call them when needed. - -Firmware operations can be specified using struct firmware_ops - - struct firmware_ops { - /* - * Enters CPU idle mode - */ - int (*do_idle)(void); - /* - * Sets boot address of specified physical CPU - */ - int (*set_cpu_boot_addr)(int cpu, unsigned long boot_addr); - /* - * Boots specified physical CPU - */ - int (*cpu_boot)(int cpu); - /* - * Initializes L2 cache - */ - int (*l2x0_init)(void); - }; - -and then registered with register_firmware_ops function - - void register_firmware_ops(const struct firmware_ops *ops) - -the ops pointer must be non-NULL. - -There is a default, empty set of operations provided, so there is no need to -set anything if platform does not require firmware operations. - -To call a firmware operation, a helper macro is provided - - #define call_firmware_op(op, ...) \ - ((firmware_ops->op) ? firmware_ops->op(__VA_ARGS__) : (-ENOSYS)) - -the macro checks if the operation is provided and calls it or otherwise returns --ENOSYS to signal that given operation is not available (for example, to allow -fallback to legacy operation). - -Example of registering firmware operations: - - /* board file */ - - static int platformX_do_idle(void) - { - /* tell platformX firmware to enter idle */ - return 0; - } - - static int platformX_cpu_boot(int i) - { - /* tell platformX firmware to boot CPU i */ - return 0; - } - - static const struct firmware_ops platformX_firmware_ops = { - .do_idle = exynos_do_idle, - .cpu_boot = exynos_cpu_boot, - /* other operations not available on platformX */ - }; - - /* init_early callback of machine descriptor */ - static void __init board_init_early(void) - { - register_firmware_ops(&platformX_firmware_ops); - } - -Example of using a firmware operation: - - /* some platform code, e.g. SMP initialization */ - - __raw_writel(virt_to_phys(exynos4_secondary_startup), - CPU1_BOOT_REG); - - /* Call Exynos specific smc call */ - if (call_firmware_op(cpu_boot, cpu) == -ENOSYS) - cpu_boot_legacy(...); /* Try legacy way */ - - gic_raise_softirq(cpumask_of(cpu), 1); diff --git a/Documentation/firmware/platform_firmware.txt b/Documentation/firmware/platform_firmware.txt new file mode 100644 index 0000000..db2a3e7 --- /dev/null +++ b/Documentation/firmware/platform_firmware.txt @@ -0,0 +1,89 @@ +Interface for registering and calling firmware-specific operations. +---- +Written by Tomasz Figa <t.figa@samsung.com> + +Some boards are running with secure firmware running in secure world, which +changes the way some things have to be initialized. This makes a need to provide +an interface for such platforms to specify available firmware operations and +call them when needed. + +Firmware operations can be specified using struct platform_firmware_ops + + struct platform_firmware_ops { + /* + * Enters CPU idle mode + */ + int (*do_idle)(void); + /* + * Sets boot address of specified physical CPU + */ + int (*set_cpu_boot_addr)(int cpu, unsigned long boot_addr); + /* + * Boots specified physical CPU + */ + int (*cpu_boot)(int cpu); + /* + * Initializes L2 cache + */ + int (*l2x0_init)(void); + }; + +and then registered with register_platform_firmware_ops function + +void register_platform_firmware_ops(const struct platform_firmware_ops *ops) + +the ops pointer must be non-NULL. + +There is a default, empty set of operations provided, so there is no need to +set anything if platform does not require firmware operations. + +To call a firmware operation, a helper macro is provided + + #define call_platform_firmware_op(op, ...) \ + ((platform_firmware_ops->op) ? + platform_firmware_ops->op(__VA_ARGS__) : (-ENOSYS)) + +the macro checks if the operation is provided and calls it or otherwise returns +-ENOSYS to signal that given operation is not available (for example, to allow +fallback to legacy operation). + +Example of registering firmware operations: + + /* board file */ + + static int platformX_do_idle(void) + { + /* tell platformX firmware to enter idle */ + return 0; + } + + static int platformX_cpu_boot(int i) + { + /* tell platformX firmware to boot CPU i */ + return 0; + } + + static const struct platform_firmware_ops platformX_firmware_ops = { + .do_idle = exynos_do_idle, + .cpu_boot = exynos_cpu_boot, + /* other operations not available on platformX */ + }; + + /* init_early callback of machine descriptor */ + static void __init board_init_early(void) + { + register_platform_firmware_ops(&platformX_firmware_ops); + } + +Example of using a firmware operation: + + /* some platform code, e.g. SMP initialization */ + + __raw_writel(virt_to_phys(exynos4_secondary_startup), + CPU1_BOOT_REG); + + /* Call Exynos specific smc call */ + if (call_platform_firmware_op(cpu_boot, cpu) == -ENOSYS) + cpu_boot_legacy(...); /* Try legacy way */ + + gic_raise_softirq(cpumask_of(cpu), 1); diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index cc68e6d..9026af0 100644 --- a/arch/arm/Kconfig +++ b/arch/arm/Kconfig @@ -218,6 +218,9 @@ config NEED_RET_TO_USER config ARCH_MTD_XIP bool +config ARCH_SUPPORTS_FIRMWARE + bool + config VECTORS_BASE hex default 0xffff0000 if MMU || CPU_HIGH_VECTOR @@ -806,6 +809,8 @@ config ARCH_EXYNOS select ARCH_HAS_HOLES_MEMORYMODEL select ARCH_REQUIRE_GPIOLIB select ARCH_SPARSEMEM_ENABLE + select ARCH_SUPPORTS_FIRMWARE + select EXYNOS_FIRMWARE select ARM_GIC select COMMON_CLK select CPU_V7 @@ -2256,6 +2261,10 @@ source "net/Kconfig" source "drivers/Kconfig" +if ARCH_SUPPORTS_FIRMWARE +source "drivers/firmware/Kconfig" +endif + source "fs/Kconfig" source "arch/arm/Kconfig.debug" diff --git a/arch/arm/common/Makefile b/arch/arm/common/Makefile index 4bdc416..f98a991 100644 --- a/arch/arm/common/Makefile +++ b/arch/arm/common/Makefile @@ -2,8 +2,6 @@ # Makefile for the linux kernel. # -obj-y += firmware.o - obj-$(CONFIG_ICST) += icst.o obj-$(CONFIG_SA1111) += sa1111.o obj-$(CONFIG_DMABOUNCE) += dmabounce.o diff --git a/arch/arm/common/firmware.c b/arch/arm/common/firmware.c deleted file mode 100644 index 27ddccb..0000000 --- a/arch/arm/common/firmware.c +++ /dev/null @@ -1,18 +0,0 @@ -/* - * Copyright (C) 2012 Samsung Electronics. - * Kyungmin Park <kyungmin.park@samsung.com> - * Tomasz Figa <t.figa@samsung.com> - * - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License version 2 as - * published by the Free Software Foundation. - */ - -#include <linux/kernel.h> -#include <linux/suspend.h> - -#include <asm/firmware.h> - -static const struct firmware_ops default_firmware_ops; - -const struct firmware_ops *firmware_ops = &default_firmware_ops; diff --git a/arch/arm/include/asm/firmware.h b/arch/arm/include/asm/firmware.h deleted file mode 100644 index 1563130..0000000 --- a/arch/arm/include/asm/firmware.h +++ /dev/null @@ -1,66 +0,0 @@ -/* - * Copyright (C) 2012 Samsung Electronics. - * Kyungmin Park <kyungmin.park@samsung.com> - * Tomasz Figa <t.figa@samsung.com> - * - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License version 2 as - * published by the Free Software Foundation. - */ - -#ifndef __ASM_ARM_FIRMWARE_H -#define __ASM_ARM_FIRMWARE_H - -#include <linux/bug.h> - -/* - * struct firmware_ops - * - * A structure to specify available firmware operations. - * - * A filled up structure can be registered with register_firmware_ops(). - */ -struct firmware_ops { - /* - * Enters CPU idle mode - */ - int (*do_idle)(void); - /* - * Sets boot address of specified physical CPU - */ - int (*set_cpu_boot_addr)(int cpu, unsigned long boot_addr); - /* - * Boots specified physical CPU - */ - int (*cpu_boot)(int cpu); - /* - * Initializes L2 cache - */ - int (*l2x0_init)(void); -}; - -/* Global pointer for current firmware_ops structure, can't be NULL. */ -extern const struct firmware_ops *firmware_ops; - -/* - * call_firmware_op(op, ...) - * - * Checks if firmware operation is present and calls it, - * otherwise returns -ENOSYS - */ -#define call_firmware_op(op, ...) \ - ((firmware_ops->op) ? firmware_ops->op(__VA_ARGS__) : (-ENOSYS)) - -/* - * register_firmware_ops(ops) - * - * A function to register platform firmware_ops struct. - */ -static inline void register_firmware_ops(const struct firmware_ops *ops) -{ - BUG_ON(!ops); - - firmware_ops = ops; -} - -#endif diff --git a/arch/arm/mach-exynos/Makefile b/arch/arm/mach-exynos/Makefile index 8930b66..7132d03 100644 --- a/arch/arm/mach-exynos/Makefile +++ b/arch/arm/mach-exynos/Makefile @@ -25,7 +25,7 @@ obj-$(CONFIG_SMP) += platsmp.o headsmp.o obj-$(CONFIG_HOTPLUG_CPU) += hotplug.o obj-$(CONFIG_ARCH_EXYNOS) += exynos-smc.o -obj-$(CONFIG_ARCH_EXYNOS) += firmware.o +obj-$(CONFIG_EXYNOS_FIRMWARE) += firmware.o plus_sec := $(call as-instr,.arch_extension sec,+sec) AFLAGS_exynos-smc.o :=-Wa,-march=armv7-a$(plus_sec) diff --git a/arch/arm/mach-exynos/firmware.c b/arch/arm/mach-exynos/firmware.c index 932129e..6dc4938 100644 --- a/arch/arm/mach-exynos/firmware.c +++ b/arch/arm/mach-exynos/firmware.c @@ -13,8 +13,7 @@ #include <linux/init.h> #include <linux/of.h> #include <linux/of_address.h> - -#include <asm/firmware.h> +#include <linux/platform_firmware.h> #include <mach/map.h> @@ -40,7 +39,7 @@ static int exynos_set_cpu_boot_addr(int cpu, unsigned long boot_addr) return 0; } -static const struct firmware_ops exynos_firmware_ops = { +static const struct platform_firmware_ops exynos_firmware_ops = { .do_idle = exynos_do_idle, .set_cpu_boot_addr = exynos_set_cpu_boot_addr, .cpu_boot = exynos_cpu_boot, @@ -64,5 +63,5 @@ void __init exynos_firmware_init(void) pr_info("Running under secure firmware.\n"); - register_firmware_ops(&exynos_firmware_ops); + register_platform_firmware_ops(&exynos_firmware_ops); } diff --git a/arch/arm/mach-exynos/platsmp.c b/arch/arm/mach-exynos/platsmp.c index 58b43e6..132d21c 100644 --- a/arch/arm/mach-exynos/platsmp.c +++ b/arch/arm/mach-exynos/platsmp.c @@ -20,11 +20,11 @@ #include <linux/jiffies.h> #include <linux/smp.h> #include <linux/io.h> +#include <linux/platform_firmware.h> #include <asm/cacheflush.h> #include <asm/smp_plat.h> #include <asm/smp_scu.h> -#include <asm/firmware.h> #include <mach/hardware.h> #include <mach/regs-clock.h> @@ -150,10 +150,11 @@ static int exynos_boot_secondary(unsigned int cpu, struct task_struct *idle) * Try to set boot address using firmware first * and fall back to boot register if it fails. */ - if (call_firmware_op(set_cpu_boot_addr, phys_cpu, boot_addr)) + if (call_platform_firmware_op(set_cpu_boot_addr, phys_cpu, + boot_addr)) __raw_writel(boot_addr, cpu_boot_reg(phys_cpu)); - call_firmware_op(cpu_boot, phys_cpu); + call_platform_firmware_op(cpu_boot, phys_cpu); arch_send_wakeup_ipi_mask(cpumask_of(cpu)); @@ -225,7 +226,8 @@ static void __init exynos_smp_prepare_cpus(unsigned int max_cpus) phys_cpu = cpu_logical_map(i); boot_addr = virt_to_phys(exynos4_secondary_startup); - if (call_firmware_op(set_cpu_boot_addr, phys_cpu, boot_addr)) + if (call_platform_firmware_op(set_cpu_boot_addr, phys_cpu, + boot_addr)) __raw_writel(boot_addr, cpu_boot_reg(phys_cpu)); } } diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig index 0747872..1fb9da6 100644 --- a/drivers/firmware/Kconfig +++ b/drivers/firmware/Kconfig @@ -129,6 +129,14 @@ config ISCSI_IBFT detect iSCSI boot parameters dynamically during system boot, say Y. Otherwise, say N. +config PLATFORM_FIRMWARE + bool + +config EXYNOS_FIRMWARE + bool "Support for Exynos secure firmware" + select PLATFORM_FIRMWARE + depends on ARM && ARCH_EXYNOS + source "drivers/firmware/google/Kconfig" source "drivers/firmware/efi/Kconfig" diff --git a/drivers/firmware/Makefile b/drivers/firmware/Makefile index 299fad6..9cd2231 100644 --- a/drivers/firmware/Makefile +++ b/drivers/firmware/Makefile @@ -11,6 +11,7 @@ obj-$(CONFIG_DMIID) += dmi-id.o obj-$(CONFIG_ISCSI_IBFT_FIND) += iscsi_ibft_find.o obj-$(CONFIG_ISCSI_IBFT) += iscsi_ibft.o obj-$(CONFIG_FIRMWARE_MEMMAP) += memmap.o +obj-$(CONFIG_PLATFORM_FIRMWARE) += platform_firmware.o obj-$(CONFIG_GOOGLE_FIRMWARE) += google/ obj-$(CONFIG_EFI) += efi/ diff --git a/drivers/firmware/platform_firmware.c b/drivers/firmware/platform_firmware.c new file mode 100644 index 0000000..1644df2 --- /dev/null +++ b/drivers/firmware/platform_firmware.c @@ -0,0 +1,17 @@ +/* + * Copyright (C) 2012 Samsung Electronics. + * Kyungmin Park <kyungmin.park@samsung.com> + * Tomasz Figa <t.figa@samsung.com> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ + +#include <linux/kernel.h> +#include <linux/suspend.h> +#include <linux/platform_firmware.h> + +static const struct platform_firmware_ops default_ops; + +const struct platform_firmware_ops *platform_firmware_ops = &default_ops; diff --git a/include/linux/platform_firmware.h b/include/linux/platform_firmware.h new file mode 100644 index 0000000..601e3fd --- /dev/null +++ b/include/linux/platform_firmware.h @@ -0,0 +1,69 @@ +/* + * Copyright (C) 2012 Samsung Electronics. + * Kyungmin Park <kyungmin.park@samsung.com> + * Tomasz Figa <t.figa@samsung.com> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ + +#ifndef _PLATFORM_FIRMWARE_H +#define _PLATFORM_FIRMWARE_H + +#include <linux/bug.h> + +/* + * struct platform_firmware_ops + * + * A structure to specify available firmware operations. + * + * A filled up structure can be registered with + * register_platform_firmware_ops(). + */ +struct platform_firmware_ops { + /* + * Enters CPU idle mode + */ + int (*do_idle)(void); + /* + * Sets boot address of specified physical CPU + */ + int (*set_cpu_boot_addr)(int cpu, unsigned long boot_addr); + /* + * Boots specified physical CPU + */ + int (*cpu_boot)(int cpu); + /* + * Initializes L2 cache + */ + int (*l2x0_init)(void); +}; + +/* Global pointer for current firmware_ops structure, can't be NULL. */ +extern const struct platform_firmware_ops *platform_firmware_ops; + +/* + * call_platform_firmware_op(op, ...) + * + * Checks if firmware operation is present and calls it, + * otherwise returns -ENOSYS + */ +#define call_platform_firmware_op(op, ...) \ + ((platform_firmware_ops->op) ? \ + platform_firmware_ops->op(__VA_ARGS__) : (-ENOSYS)) + +/* + * register_platform_firmware_ops(ops) + * + * A function to register platform firmware_ops struct. + */ +static inline +void register_platform_firmware_ops(const struct platform_firmware_ops *ops) +{ + BUG_ON(!ops); + + platform_firmware_ops = ops; +} + +#endif
The ARM tree includes a firmware_ops interface that is designed to implement support for simple, TrustZone-based firmwares but could also cover other use-cases. It has been suggested that this interface might be useful to other architectures (e.g. arm64) and that it should be moved out of arch/arm. This patch takes care of this by performing the following: * Move the ARM firmware interface into drivers/firmware/ and rename it to platform_firmware, * Update its documentation accordingly, * Update the only user of the API to date (Exynos secure firmware support) to use the new API. The ARM architecture's Kconfig now needs to include the Kconfig of drivers/firmware, which will result in an empty menu for most platforms that don't make use of any firmware. To avoid this, a new invisible ARCH_SUPPORTS_FIRMWARE configuration variable is also defined for ARM, that should explicitly be set by platforms that support firmware so that the firmware menu is included. Signed-off-by: Alexandre Courbot <acourbot@nvidia.com> --- Documentation/arm/firmware.txt | 88 --------------------------- Documentation/firmware/platform_firmware.txt | 89 ++++++++++++++++++++++++++++ arch/arm/Kconfig | 9 +++ arch/arm/common/Makefile | 2 - arch/arm/common/firmware.c | 18 ------ arch/arm/include/asm/firmware.h | 66 --------------------- arch/arm/mach-exynos/Makefile | 2 +- arch/arm/mach-exynos/firmware.c | 7 +-- arch/arm/mach-exynos/platsmp.c | 10 ++-- drivers/firmware/Kconfig | 8 +++ drivers/firmware/Makefile | 1 + drivers/firmware/platform_firmware.c | 17 ++++++ include/linux/platform_firmware.h | 69 +++++++++++++++++++++ 13 files changed, 203 insertions(+), 183 deletions(-) delete mode 100644 Documentation/arm/firmware.txt create mode 100644 Documentation/firmware/platform_firmware.txt delete mode 100644 arch/arm/common/firmware.c delete mode 100644 arch/arm/include/asm/firmware.h create mode 100644 drivers/firmware/platform_firmware.c create mode 100644 include/linux/platform_firmware.h