Re: [PATCH] ARM: move firmware_ops to drivers/firmware
On Mon, Nov 18, 2013 at 07:04:50PM +, 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. -- Catalin -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ARM: move firmware_ops to drivers/firmware
On Mon, Nov 18, 2013 at 05:52:36PM +, Stephen Warren wrote: On 11/18/2013 10:30 AM, Catalin Marinas wrote: On Mon, Nov 18, 2013 at 05:03:37PM +, 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). -- Catalin -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ARM: move firmware_ops to drivers/firmware
On Tue, Nov 19, 2013 at 02:46:55AM +, Alex Courbot wrote:
On 11/18/2013 08:58 PM, Catalin Marinas wrote:
On Mon, Nov 18, 2013 at 03:05:59AM +, Alex Courbot wrote:
On 11/18/2013 12:59 AM, Catalin Marinas wrote:
On 17 November 2013 08:49, Alexandre Courbot acour...@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 ;)).
--
Catalin
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ARM: move firmware_ops to drivers/firmware
On Tue, Nov 19, 2013 at 9:26 PM, Catalin Marinas
catalin.mari...@arm.com wrote:
On Tue, Nov 19, 2013 at 02:46:55AM +, Alex Courbot wrote:
On 11/18/2013 08:58 PM, Catalin Marinas wrote:
On Mon, Nov 18, 2013 at 03:05:59AM +, Alex Courbot wrote:
On 11/18/2013 12:59 AM, Catalin Marinas wrote:
On 17 November 2013 08:49, Alexandre Courbot acour...@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
Re: [PATCH] ARM: move firmware_ops to drivers/firmware
On Tue, Nov 19, 2013 at 02:29:39PM +, Alexandre Courbot wrote: On Tue, Nov 19, 2013 at 9:26 PM, Catalin Marinas catalin.mari...@arm.com wrote: On Tue, Nov 19, 2013 at 02:46:55AM +, 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. -- Catalin -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ARM: move firmware_ops to drivers/firmware
On Wed, Nov 20, 2013 at 12:07 AM, Catalin Marinas catalin.mari...@arm.com wrote: On Tue, Nov 19, 2013 at 02:29:39PM +, Alexandre Courbot wrote: On Tue, Nov 19, 2013 at 9:26 PM, Catalin Marinas catalin.mari...@arm.com wrote: On Tue, Nov 19, 2013 at 02:46:55AM +, 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
Re: [PATCH] ARM: move firmware_ops to drivers/firmware
On Mon, Nov 18, 2013 at 03:05:59AM +, Alex Courbot wrote: On 11/18/2013 12:59 AM, Catalin Marinas wrote: On 17 November 2013 08:49, Alexandre Courbot acour...@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. -- Catalin -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ARM: move firmware_ops to drivers/firmware
On 11/17/2013 08:59 AM, Catalin Marinas wrote: On 17 November 2013 08:49, Alexandre Courbot acour...@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. -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ARM: move firmware_ops to drivers/firmware
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? -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ARM: move firmware_ops to drivers/firmware
On Sun, Nov 17, 2013 at 03:59:04PM +, 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. -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ARM: move firmware_ops to drivers/firmware
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. -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ARM: move firmware_ops to drivers/firmware
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. -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ARM: move firmware_ops to drivers/firmware
On Mon, Nov 18, 2013 at 05:03:37PM +, 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). -- Catalin -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ARM: move firmware_ops to drivers/firmware
On Mon, Nov 18, 2013 at 05:00:32PM +, Stephen Warren wrote: On 11/17/2013 08:59 AM, Catalin Marinas wrote: On 17 November 2013 08:49, Alexandre Courbot acour...@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. -- Catalin -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ARM: move firmware_ops to drivers/firmware
On 11/18/2013 10:30 AM, Catalin Marinas wrote: On Mon, Nov 18, 2013 at 05:03:37PM +, 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. -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ARM: move firmware_ops to drivers/firmware
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 -- 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 linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ARM: move firmware_ops to drivers/firmware
On 11/18/2013 08:58 PM, Catalin Marinas wrote: On Mon, Nov 18, 2013 at 03:05:59AM +, Alex Courbot wrote: On 11/18/2013 12:59 AM, Catalin Marinas wrote: On 17 November 2013 08:49, Alexandre Courbot acour...@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
[PATCH] ARM: move firmware_ops to drivers/firmware
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 acour...@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
diff --git a/Documentation/arm/firmware.txt b/Documentation/arm/firmware.txt
deleted file mode 100644
index c2e468f..000
--- 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.f...@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);
- }
-
Re: [PATCH] ARM: move firmware_ops to drivers/firmware
On 17 November 2013 08:49, Alexandre Courbot acour...@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.p...@samsung.com
+ * Tomasz Figa t.f...@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.
--
Catalin
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ARM: move firmware_ops to drivers/firmware
On 11/18/2013 12:59 AM, Catalin Marinas wrote:
On 17 November 2013 08:49, Alexandre Courbot acour...@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.p...@samsung.com
+ * Tomasz Figa t.f...@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.
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
