* [PATCH 4/5] arm64: dts: exynos5433: Add bus dt node using VDD_INT for Exynos5433
From: Chanwoo Choi @ 2016-12-07 11:17 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161206192104.GB12683@kozik-lap>
On 2016? 12? 07? 04:21, Krzysztof Kozlowski wrote:
> On Fri, Dec 02, 2016 at 04:18:06PM +0900, Chanwoo Choi wrote:
>> This patch adds the bus nodes using VDD_INT for Exynos5433 SoC.
>> Exynos5433 has the following AMBA AXI buses to translate data
>> between DRAM and sub-blocks.
>>
>> Following list specify the detailed correlation between sub-block and clock:
>> - CLK_ACLK_G2D_{400|266} : Bus clock for G2D
>> - CLK_ACLK_MSCL_400 : Bus clock for MSCL (Mobile Scaler)
>> - CLK_ACLK_GSCL_333 : Bus clock for GSCL (General Scaler)
>> - CLK_SCLK_JPEG_MSCL : Bus clock for JPEG
>> - CLK_ACLK_MFC_400 : Bus clock for MFC (Multi Format Codec)
>> - CLK_ACLK_HEVC_400 : Bus clock for HEVC (High Effective Video Codec)
>> - CLK_ACLK_BUS0_400 : NoC(Network On Chip)'s bus clock for PERIC/PERIS/FSYS/MSCL
>> - CLK_ACLK_BUS1_400 : NoC's bus clock for MFC/HEVC/G3D
>> - CLK_ACLK_BUS2_400 : NoC's bus clock for GSCL/DISP/G2D/CAM0/CAM1/ISP
>>
>> Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
>> ---
>> arch/arm64/boot/dts/exynos/exynos5433-bus.dtsi | 208 +++++++++++++++++++++++++
>> arch/arm64/boot/dts/exynos/exynos5433.dtsi | 1 +
>> 2 files changed, 209 insertions(+)
>> create mode 100644 arch/arm64/boot/dts/exynos/exynos5433-bus.dtsi
>>
>> diff --git a/arch/arm64/boot/dts/exynos/exynos5433-bus.dtsi b/arch/arm64/boot/dts/exynos/exynos5433-bus.dtsi
>> new file mode 100644
>> index 000000000000..b1e1d9c622e1
>> --- /dev/null
>> +++ b/arch/arm64/boot/dts/exynos/exynos5433-bus.dtsi
>> @@ -0,0 +1,208 @@
>> +/*
>> + * Samsung's Exynos5433 SoC Memory interface and AMBA bus device tree source
>> + *
>> + * Copyright (c) 2016 Samsung Electronics Co., Ltd.
>> + * Chanwoo Choi <cw00.choi@samsung.com>
>> + *
>> + * Samsung's Exynos5433 SoC Memory interface and AMBA buses are listed
>> + * as device tree nodes are listed in this file.
>
> This duplicates the introduction line and does not make sense.
I'll remove it.
>
>> + *
>> + * 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.
>> + */
>> +
>> +/ {
>
> Shouldn't these be under soc node? It looks like property of SoC itself.
OK. Move to them under SoC.
- "/ {" -> "&soc {"
>
>> + /* INT (Internal) block using VDD_INT */
>> + bus_g2d_400: bus_g2d_400 {
>
> In node name, the dash '-' is preferred. The name should describe
> general class of device so probably this should be just "bus"... but I
> don't see a way how to do it reasonable anyway.
I'll change them as following with 'busX'.
The each dt node has the unique number('X')
because each dt node does not have the base address
and then need to identify oneself.
bus_g2d_400: bus0 {
bus_g2d_266: bus1 {
bus_gscl: bus2 {
bus_hevc: bus3 {
bus_jpeg: bus4 {
bus_mfc: bus5 {
bus_mscl: bus6 {
bus_noc0: bus7 {
bus_noc1: bus8 {
bus_noc2: bus9 {
>
>> + compatible = "samsung,exynos-bus";
>> + clocks = <&cmu_top CLK_ACLK_G2D_400>;
>> + clock-names = "bus";
>> + operating-points-v2 = <&bus_g2d_400_opp_table>;
>> + status ="disable";
>
> Hm?
I'll fix it. disable -> disabled
>
>
>> + };
>> +
>> + bus_mscl: bus_mscl {
>> + compatible = "samsung,exynos-bus";
>> + clocks = <&cmu_top CLK_ACLK_MSCL_400>;
>> + clock-names = "bus";
>> + operating-points-v2 = <&bus_g2d_400_opp_table>;
>> + status ="disable";
>> + };
>> +
>> + bus_jpeg: bus_jpeg {
>> + compatible = "samsung,exynos-bus";
>> + clocks = <&cmu_top CLK_SCLK_JPEG_MSCL>;
>> + clock-names = "bus";
>> + operating-points-v2 = <&bus_g2d_400_opp_table>;
>> + status ="disable";
>> + };
>> +
>> + bus_mfc: bus_mfc {
>> + compatible = "samsung,exynos-bus";
>> + clocks = <&cmu_top CLK_ACLK_MFC_400>;
>> +
>> + clock-names = "bus";
>> + operating-points-v2 = <&bus_g2d_400_opp_table>;
>> + status ="disable";
>> + };
>> +
>> + bus_g2d_266: bus_g2d_266 {
>> + compatible = "samsung,exynos-bus";
>> + clocks = <&cmu_top CLK_ACLK_G2D_266>;
>> + clock-names = "bus";
>> + operating-points-v2 = <&bus_g2d_266_opp_table>;
>> + status ="disable";
>> + };
>> +
>> + bus_gscl: bus_gscl {
>> + compatible = "samsung,exynos-bus";
>> + clocks = <&cmu_top CLK_ACLK_GSCL_333>;
>> + clock-names = "bus";
>> + operating-points-v2 = <&bus_gscl_opp_table>;
>> + status ="disable";
>> + };
>> +
>> + bus_hevc: bus_hevc {
>> + compatible = "samsung,exynos-bus";
>> + clocks = <&cmu_top CLK_ACLK_HEVC_400>;
>> + clock-names = "bus";
>> + operating-points-v2 = <&bus_hevc_opp_table>;
>> + status ="disable";
>> + };
>> +
>> + bus_bus0: bus_bus0 {
>
> bus, bus, bus, bus, jackpot! Let's try to find better name and label for
> these. :)
I'll change the name with 'noc' prefix because this bus is used
for NoC (Network On Chip)'s bus clock as commit msg.
- old : bus_bus0
- new : bus_noc0
Best Regards,
Chanwoo Choi
^ permalink raw reply
* [RFC PATCH 0/2] arm64: memory-hotplug: Add Memory Hotplug support
From: Xishi Qiu @ 2016-12-07 11:21 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <0f24c35b-10d5-d857-356d-01a21be48c2c@broadcom.com>
On 2016/12/7 16:43, Scott Branden wrote:
> Hi Xishi,
>
> I followed you suggestions and found pfn_valid is always true. Answers to your questions inline.
>
> I could keep debugging this but hope Marcin sends out some code - I'm quite willing to test and help clean up the patchset.
>
> On 16-12-01 07:11 PM, Xishi Qiu wrote:
>> On 2016/12/2 10:38, Scott Branden wrote:
>>
>>> Hi Xishi,
>>>
>>> Thanks for the reply - please see comments below.
>>>
>>> On 16-12-01 05:49 PM, Xishi Qiu wrote:
>>>> On 2016/12/2 8:19, Scott Branden wrote:
>>>>
>>>>> This patchset is sent for comment to add memory hotplug support for ARM64
>>>>> based platforms. It follows hotplug code added for other architectures
>>>>> in the linux kernel.
>>>>>
>>>>> I tried testing the memory hotplug feature following documentation from
>>>>> Documentation/memory-hotplug.txt. I don't think it is working as expected
>>>>> - see below:
>>>>>
>>>>> To add memory to the system I did the following:
>>>>> echo 0x400000000 > /sys/devices/system/memory/probe
>>>>>
>>>>> The memory is displayed as system ram:
>>>>> cat /proc/iomem:
>>>>> 74000000-77ffffff : System RAM
>>>>> 74080000-748dffff : Kernel code
>>>>> 74950000-749d2fff : Kernel data
>>>>> 400000000-43fffffff : System RAM
>>>>>
>>>>> But does not seem to be added to the kernel memory.
>>>>> /proc/meminfo did not change.
>>>>>
>>>>> What else needs to be done so the memory is added to the kernel memory
>>>>> pool for normal allocation?
>>>>>
>>>>
>>>> Hi Scott,
>>>>
>>>> Do you mean it still don't support hod-add after apply this patchset?
>>>
>>> After applying the patch it appears to partially support hot-add. Please let me know if you think it is working as expected?
>>>
>>> The memory probe functions in that the memory is registered with the system and shows up in /proc/iomem. But, the memory is not available in /proc/meminfo. Do you think something else needs to be adjusted for ARM64 to hotadd the memory
>>>
>>> I just found another clue:
>>> under /sys/devices/system/memory I only see one memory entry (before or after I try to hotadd additional memory).
>>>
>>> /sys/devices/system/memory # ls
>>> auto_online_blocks memory0 uevent
>>> block_size_bytes probe
>>>
>>> In arch/arm64/include/asm/sparsemem.h if I change SECTION_SIZE_BITS from 30 to 28 and recompile I get the following:
>>> /sys/devices/system/memory # ls
>>> auto_online_blocks memory7 uevent
>>> block_size_bytes probe
>>>
>>>
>>> In arch/arm64/include/asm/sparsemem.h if I change SECTION_SIZE_BITS from 30 to 27 and recompile I get the following:
>>> /sys/devices/system/memory # ls
>>> auto_online_blocks memory14 uevent
>>> block_size_bytes probe
>>>
>>> If looks to me like something is not working properly in the ARM64 implementation. I should expect to see multiple memoryX entries under /sys/devices/system/memory?
>>>
>>
>> Hi Scott,
>>
>> 1. Do you enable the following configs?
>> CONFIG_SPARSEMEM
>> MEMORY_HOTPLUG
>> CONFIG_MEMORY_HOTPLUG_DEFAULT_ONLINE
> Yes, these configs are enabled
>>
>> 2. I find you missed create mapping in arch_add_memory(), and x86 has it.
> Could you please explain this further? The patch I submitted hass arch_add_memory identical to the ia64 implementation.
Hi Scott,
I think we should create page table first for the new hotadd memory.
e.g. create_mapping_late(start, __phys_to_virt(start), size, PAGE_KERNEL);
I don't know why ia64 don't have this step.
CC Tony
>>
>> 3. We will add memblock first, so pfn_valid() maybe always return true(in the
>> following function), and this will lead __add_section() failed. Please check
>> it.
> You are correct - pfn_valid always returns true. The function is in arch/arm64/mm/init.c and different than the one you indicated below:
>
> #ifdef CONFIG_HAVE_ARCH_PFN_VALID
> int pfn_valid(unsigned long pfn)
> {
> return memblock_is_map_memory(pfn << PAGE_SHIFT);
> }
> EXPORT_SYMBOL(pfn_valid);
> #endif
>
>>
>> int pfn_valid(unsigned long pfn)
>> {
>> return (pfn & PFN_MASK) == pfn && memblock_is_memory(pfn << PAGE_SHIFT);
>> }
>>
>> add_memory
>> add_memory_resource
>> memblock_add_node
>> arch_add_memory
>> __add_pages
>> __add_section
>> pfn_valid
>>
>> Thanks,
>> Xishi Qiu
>>
>>>
>>>
>>>>
>>>> Thanks,
>>>> Xishi Qiu
>>>>
>>>>> Scott Branden (2):
>>>>> arm64: memory-hotplug: Add MEMORY_HOTPLUG, MEMORY_HOTREMOVE,
>>>>> MEMORY_PROBE
>>>>> arm64: defconfig: enable MEMORY_HOTPLUG config options
>>>>>
>>>>> arch/arm64/Kconfig | 10 ++++++++++
>>>>> arch/arm64/configs/defconfig | 3 +++
>>>>> arch/arm64/mm/init.c | 42 ++++++++++++++++++++++++++++++++++++++++++
>>>>> 3 files changed, 55 insertions(+)
>>>>>
>>>>
>>>>
>>>>
>>>
>>> .
>>>
>>
>>
>>
>
> .
>
^ permalink raw reply
* [PATCH 0/7] arm: Add livepatch support
From: Abel Vesa @ 2016-12-07 11:39 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <584767FF.6030807@huawei.com>
On Wed, Dec 07, 2016 at 09:38:07AM +0800, zhouchengming wrote:
> On 2016/12/7 1:06, Abel Vesa wrote:
> >This is just an idea I've been trying out for a while now.
> >
> >Just in case somebody wants to play with it, this applies to linux-arm/for-next.
> >
> >Also please note that this was only tested in qemu, but I will do some testing
> >on some real hardware in the following days.
> >
> >FWICT, on this arch the compiler always generates a function prologue somewhere
> >between these lines:
> >
> >e1a0c00d mov ip, sp
> >e92ddff0 push {r4-r9, sl, fp, ip, lr, pc}
> >e24cb004 sub fp, ip, #4
> >e24dd064 sub sp, sp, #100 ; 0x64<--- local variables
> >e52de004 push {lr} ; (str lr, [sp, #-4]!)
> >ebf9c2c9 bl 80110364<__gnu_mcount_nc>
> >....
> >
> >Every function that follows this pattern (the number of registers pushed and the
> >sp subtraction for the local variables being the only acceptable exception) can
> >be patched with this mechanism. IIRC, only the inline functions and notrace
> >functions do not follow this pattern.
> >
> >Considering that the function is livepatchable, when the time comes to call
> >ftrace_call, the ftrace_regs_caller is called instead.
> >
> >Because this arch didn't have a ftrace with regs implementation, the
> >ftrace_regs_caller was added.
> >
> >This new function adds the regs saving/restoring part, plus the part necessary
> >for the livepatch mechanism to work. After the regs are saved and the r3 is set
> >to contain the sp's value, we're keeping the old pc into r10 in order to be
> >checked later against the new pc.
> >
> >Next, the r1 and r0 are set for the ftrace_func, then, the ftrace_stub is called
> >and the klp_ftrace_handler overwrites the old pc with the new one.
> >
> >Here comes the tricky part. We're checking if the pc is still the old one, if it
> >is we jump the whole livepatching and go ahead with restoring the saved regs.
> >
> >If the pc is modified, it means we're livepatching current function and we need
> >to pop all regs from r1 through r12, jump over the next two regs saved on stack
> >(we're not interested in those since we're trying to get the same regs context
> >as it was at the point the function-to-be-patched was called) and put the new pc
> >into r11.
> >
> >Since r12 contains the sp from when the function just got branched to, we need
> >to set the sp back to that.
> >
> >Then we need to put the new pc on stack so that when we're popping r11 through
> >pc, we will actually jump to the first instruction from the new function.
> >
> >We don't need to worry about the returning phase since the epilogue of the new
> >function will take care of that and from there on everything goes back to
> >normal.
> >
> >The whole advantage of this over adding compiler support is that we're not
> >introducing nops at the beginning of the function. As a matter of fact, we're
> >not changing anything between an image with livepatch and an image without it
> >(except the ftrace_regs_call addition and the livepatch necessary code).
> >
> >As for the implementation of the ftrace_regs_caller, I still think there might
> >be some unsafe stack handling since I'm getting some build warnings. Those are
> >due to pushing/popping of a list of regs in which the sp resides. I'll try to
> >get around those in a next iteration (if necessary), but first I would like to
> >hear some opinions about this work and if it's worth going forward.
> >
>
> Hi, so your idea is that when the pc is modified, we undo the work of the prologue
> of the old function, and then jump to the first instruction of the new function.
> But I doubt if we can really undo the work of the prologue correctly ? I don't know
> about arm, but gcc on arm64 may do some tricky things in prologue. So is there any
> chance we may restore a wrong context for the new function ?
>
> Thanks.
>
I forgot to mention that this is actually taking advantage of how this arch deals
with function calling. This mechanism might not be appliable to any other arch
AFAIK. On arm 32bit, as long as the mcount prologue looks like in the shown
example, the function is livepatchable. I will come back with a new version of
this patch today (latest tomorrow) with comments as Russel and Steven mentioned.
I hope that will clarify why this is working on arm 32bit.
> >Everything else should be pretty straightforward, so I'll skip explaining that.
> >
> >Abel Vesa (7):
> > arm: Add livepatch arch specific code
> > arm: ftrace: Add call modify mechanism
> > arm: module: Add apply_relocate_add
> > arm: Add ftrace with regs support
> > arm: ftrace: Add ARCH_SUPPORTS_FTRACE_OPS for ftrace with regs
> > arm: Add livepatch to build if CONFIG_LIVEPATCH
> > arm: Add livepatch necessary arch selects into Kconfig
> >
> > MAINTAINERS | 3 +++
> > arch/arm/Kconfig | 4 ++++
> > arch/arm/include/asm/ftrace.h | 4 ++++
> > arch/arm/include/asm/livepatch.h | 46 +++++++++++++++++++++++++++++++++++++
> > arch/arm/kernel/Makefile | 1 +
> > arch/arm/kernel/entry-ftrace.S | 49 ++++++++++++++++++++++++++++++++++++++++
> > arch/arm/kernel/ftrace.c | 21 +++++++++++++++++
> > arch/arm/kernel/livepatch.c | 43 +++++++++++++++++++++++++++++++++++
> > arch/arm/kernel/module.c | 9 ++++++++
> > 9 files changed, 180 insertions(+)
> > create mode 100644 arch/arm/include/asm/livepatch.h
> > create mode 100644 arch/arm/kernel/livepatch.c
> >
>
>
^ permalink raw reply
* [RFC PATCH 00/23] arm: defconfigs: use kconfig fragments
From: Bartlomiej Zolnierkiewicz @ 2016-12-07 11:41 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAOesGMi6gqNLUZw_bQZE2foBts0c70z57bHcMVg1hVZ_E4easg@mail.gmail.com>
Hi,
On Tuesday, December 06, 2016 11:03:34 AM Olof Johansson wrote:
> On Tue, Dec 6, 2016 at 4:38 AM, Bartlomiej Zolnierkiewicz
> <b.zolnierkie@samsung.com> wrote:
> > Hi,
> >
> > This RFC patchset starts convertion of ARM defconfigs to use kconfig
> > fragments and dynamically generate defconfigs. The goals of this
> > work are to:
>
> You don't provide any motivation as to why this is better. As far as I
Benefits are:
- no code duplication (this initial patchset alone removes ~1700 lines
from defconfigs without any change in functionality)
- prevention of "multi" defconfigs (i.e. multi_v7_defconfig) going out
of sync with "SoC-family" ones (i.e. exynos_defconfig) - there will
be just one place to update when changing things
- possibility to add support for more optimized defconfigs (i.e. board
specific ones) in the future without duplicating the code
- making it easier to define arch specific parts of defconfigs in
the future if we decide on doing it (i.e. we may want to enable
things like CONFIG_SYSVIPC for all defconfigs)
> am concerned it'll just be a mess.
>
> So:
>
> Nack. So much nack. I really don't want to see a proliferation of
> config fragments like this.
>
> I had a feeling it was a bad idea to pick up that one line config
> fragment before, since it opened the door for this kind of mess. :(
Like I said in the cover-letter I'm not satisfied with the current
patches and they have much room for improvement.
However I see that you don't like the idea itself... :(
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics
^ permalink raw reply
* [PATCH] dt: bindings: zx: Add header for PM domains specifiers
From: Shawn Guo @ 2016-12-07 11:48 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1480983711-29955-1-git-send-email-baoyou.xie@linaro.org>
On Tue, Dec 06, 2016 at 08:21:51AM +0800, Baoyou Xie wrote:
> This patch adds header with values used for ZTE 2967
> SoC's power domain driver.
>
> Signed-off-by: Baoyou Xie <baoyou.xie@linaro.org>
> ---
> include/dt-bindings/arm/zte_pm_domains.h | 23 +++++++++++++++++++++++
Considering that we are adding power domain drivers into
drivers/soc/zte, it might be better to put the header into folder
include/dt-bindings/soc/.
> 1 file changed, 23 insertions(+)
> create mode 100644 include/dt-bindings/arm/zte_pm_domains.h
>
> diff --git a/include/dt-bindings/arm/zte_pm_domains.h b/include/dt-bindings/arm/zte_pm_domains.h
> new file mode 100644
> index 0000000..1485e8d
> --- /dev/null
> +++ b/include/dt-bindings/arm/zte_pm_domains.h
> @@ -0,0 +1,23 @@
> +/*
> + * Copyright (C) 2015 Linaro Ltd.
> + *
> + * Author: Baoyou Xie <baoyou.xie@linaro.org>
> + * License terms: GNU General Public License (GPL) version 2
> + */
> +#ifndef _DT_BINDINGS_ARM_ZTE_PM_DOMAINS_H
> +#define _DT_BINDINGS_ARM_ZTE_PM_DOMAINS_H
> +
> +#define DM_ZX296718_SAPPU 0
> +#define DM_ZX296718_VDE 1 /*g1v6*/
> +#define DM_ZX296718_VCE 2 /*h1v6*/
> +#define DM_ZX296718_HDE 3 /*g2v2*/
The single line comment should be /* blabla */. Note there is space
after and before *.
Shawn
> +#define DM_ZX296718_VIU 4
> +#define DM_ZX296718_USB20 5
> +#define DM_ZX296718_USB21 6
> +#define DM_ZX296718_USB30 7
> +#define DM_ZX296718_HSIC 8
> +#define DM_ZX296718_GMAC 9
> +#define DM_ZX296718_TS 10
> +#define DM_ZX296718_VOU 11
> +
> +#endif /* _DT_BINDINGS_ARM_ZTE_PM_DOMAINS_H */
> --
> 2.7.4
>
^ permalink raw reply
* [PATCH 4/7] arm: Add ftrace with regs support
From: Robin Murphy @ 2016-12-07 11:58 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1481043967-15602-5-git-send-email-abelvesa@linux.com>
Hi Abel,
On 06/12/16 17:06, Abel Vesa wrote:
> This adds __ftrace_regs_caller which, unlike __ftrace_caller,
> adds register saving/restoring and livepatch handling if
> the pc register gets modified by klp_ftrace_handler.
>
> Signed-off-by: Abel Vesa <abelvesa@linux.com>
> ---
> arch/arm/kernel/entry-ftrace.S | 49 ++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 49 insertions(+)
>
> diff --git a/arch/arm/kernel/entry-ftrace.S b/arch/arm/kernel/entry-ftrace.S
> index c73c403..b6ada5c 100644
> --- a/arch/arm/kernel/entry-ftrace.S
> +++ b/arch/arm/kernel/entry-ftrace.S
> @@ -92,6 +92,46 @@
> 2: mcount_exit
> .endm
>
> +#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
> +
> +.macro __ftrace_regs_caller suffix
> +
> + stmdb sp!, {r0-r15}
As the kbuild robot reported, you can't do this. For ARM, it's unknown
what the value stored for r13 will be, and for a Thumb-2 kernel the
whole instruction is flat out unpredictable (i.e. it might just undef or
anything).
> + mov r3, sp
> +
> + ldr r10, [sp, #60]
> +
> + mcount_get_lr r1 @ lr of instrumented func
> + mcount_adjust_addr r0, lr @ instrumented function
> +
> + .globl ftrace_regs_call\suffix
> +ftrace_regs_call\suffix:
> + bl ftrace_stub
> +
> +#ifdef CONFIG_FUNCTION_GRAPH_TRACER
> + .globl ftrace_regs_graph_call\suffix
> +ftrace_regs_graph_call\suffix:
> + mov r0, r0
> +#endif
> +#ifdef CONFIG_LIVEPATCH
> + ldr r0, [sp, #60]
> + cmp r0, r10
> + beq ftrace_regs_caller_end
> + ldmia sp!, {r0-r12}
> + add sp, sp, #8
> + ldmia sp!, {r11}
> + sub sp, r12, #16
> + str r11, [sp, #12]
> + ldmia sp!, {r11, r12, lr, pc}
> +#endif
> +ftrace_regs_caller_end\suffix:
> + ldmia sp!, {r0-r14}
Again, the value of SP at this point is now unknown (regardless of
whether what the value on memory was correct or not). Not good.
Either use non-writeback addressing modes, or avoid saving/restoring SP
at all (AFAICS it isn't necessary, since if the SP was changed in
between, you then wouldn't know where to restore the saved registers
from anyway).
Robin.
> + add sp, sp, #4
> + mov pc, lr
> +.endm
> +
> +#endif
> +
> .macro __ftrace_caller suffix
> mcount_enter
>
> @@ -212,6 +252,15 @@ UNWIND(.fnstart)
> __ftrace_caller
> UNWIND(.fnend)
> ENDPROC(ftrace_caller)
> +
> +#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
> +ENTRY(ftrace_regs_caller)
> +UNWIND(.fnstart)
> + __ftrace_regs_caller
> +UNWIND(.fnend)
> +ENDPROC(ftrace_regs_caller)
> +#endif
> +
> #endif
>
> #ifdef CONFIG_FUNCTION_GRAPH_TRACER
>
^ permalink raw reply
* [PATCH v4 6/7] IIO: add STM32 timer trigger driver
From: Daniel Thompson @ 2016-12-07 12:01 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CA+M3ks4VbV6z6t7aORKGmduZfFF+Ls7Sq8umWRFyo3sn8WZt7g@mail.gmail.com>
On 07/12/16 11:00, Benjamin Gaignard wrote:
> 2016-12-07 11:50 GMT+01:00 Lee Jones <lee.jones@linaro.org>:
>> On Tue, 06 Dec 2016, Benjamin Gaignard wrote:
>>
>>> [snip]
>>>>> +
>>>>> +static const char * const triggers0[] = {
>>>>> + TIM1_TRGO, TIM1_CH1, TIM1_CH2, TIM1_CH3, TIM1_CH4, NULL,
>>>>> +};
>>>>> +
>>>>> +static const char * const triggers1[] = {
>>>>> + TIM2_TRGO, TIM2_CH1, TIM2_CH2, TIM2_CH3, TIM2_CH4, NULL,
>>>>> +};
>>>>> +
>>>>> +static const char * const triggers2[] = {
>>>>> + TIM3_TRGO, TIM3_CH1, TIM3_CH2, TIM3_CH3, TIM3_CH4, NULL,
>>>>> +};
>>>>> +
>>>>> +static const char * const triggers3[] = {
>>>>> + TIM4_TRGO, TIM4_CH1, TIM4_CH2, TIM4_CH3, TIM4_CH4, NULL,
>>>>> +};
>>>>> +
>>>>> +static const char * const triggers4[] = {
>>>>> + TIM5_TRGO, TIM5_CH1, TIM5_CH2, TIM5_CH3, TIM5_CH4, NULL,
>>>>> +};
>>>>> +
>>>>> +static const char * const triggers5[] = {
>>>>> + TIM6_TRGO, NULL,
>>>>> +};
>>>>> +
>>>>> +static const char * const triggers6[] = {
>>>>> + TIM7_TRGO, NULL,
>>>>> +};
>>>>> +
>>>>> +static const char * const triggers7[] = {
>>>>> + TIM8_TRGO, TIM8_CH1, TIM8_CH2, TIM8_CH3, TIM8_CH4, NULL,
>>>>> +};
>>>>> +
>>>>> +static const char * const triggers8[] = {
>>>>> + TIM9_TRGO, TIM9_CH1, TIM9_CH2, NULL,
>>>>> +};
>>>>> +
>>>>> +static const char * const triggers9[] = {
>>>>> + TIM12_TRGO, TIM12_CH1, TIM12_CH2, NULL,
>>>>> +};
>>>>> +
>>>>> +static const void *triggers_table[] = {
>>>>> + triggers0,
>>>>> + triggers1,
>>>>> + triggers2,
>>>>> + triggers3,
>>>>> + triggers4,
>>>>> + triggers5,
>>>>> + triggers6,
>>>>> + triggers7,
>>>>> + triggers8,
>>>>> + triggers9,
>>>>> +};
>>>>
>>>> What about:
>>>>
>>>> static const char * const triggers[][] = {
>>>> { TIM1_TRGO, TIM1_CH1, TIM1_CH2, TIM1_CH3, TIM1_CH4, NULL },
>>>> { TIM2_TRGO, TIM2_CH1, TIM2_CH2, TIM2_CH3, TIM2_CH4, NULL },
>>>> { TIM3_TRGO, TIM3_CH1, TIM3_CH2, TIM3_CH3, TIM3_CH4, NULL },
>>>> { TIM4_TRGO, TIM4_CH1, TIM4_CH2, TIM4_CH3, TIM4_CH4, NULL },
>>>> { TIM5_TRGO, TIM5_CH1, TIM5_CH2, TIM5_CH3, TIM5_CH4, NULL },
>>>> { TIM6_TRGO, NULL },
>>>> { TIM7_TRGO, NULL },
>>>> { TIM8_TRGO, TIM8_CH1, TIM8_CH2, TIM8_CH3, TIM8_CH4, NULL },
>>>> { TIM9_TRGO, TIM9_CH1, TIM9_CH2, NULL },
>>>> { TIM12_TRGO, TIM12_CH1, TIM12_CH2, NULL }
>>>> };
>>>
>>> I can't because the second dimension of the array isn't fix.
>>> I could have between 2 and 6 elements per row... to create a dual dimension
>>> array I would have to add NULL entries like that:
>>>
>>> #define MAX_TRIGGERS 6
>>>
>>> static const void *triggers_table[][MAX_TRIGGERS] = {
>>> { TIM1_TRGO, TIM1_CH1, TIM1_CH2, TIM1_CH3, TIM1_CH4, NULL,},
>>> { TIM2_TRGO, TIM2_CH1, TIM2_CH2, TIM2_CH3, TIM2_CH4, NULL,},
>>> { TIM3_TRGO, TIM3_CH1, TIM3_CH2, TIM3_CH3, TIM3_CH4, NULL,},
>>> { TIM4_TRGO, TIM4_CH1, TIM4_CH2, TIM4_CH3, TIM4_CH4, NULL,},
>>> { TIM5_TRGO, TIM5_CH1, TIM5_CH2, TIM5_CH3, TIM5_CH4, NULL,},
>>> { TIM6_TRGO, NULL, NULL, NULL, NULL, NULL,},
>>> { TIM7_TRGO, NULL, NULL, NULL, NULL, NULL,},
>>> { TIM8_TRGO, TIM8_CH1, TIM8_CH2, TIM8_CH3, TIM8_CH4, NULL,},
>>> { TIM9_TRGO, TIM9_CH1, TIM9_CH2, NULL, NULL, NULL,},
>>> { TIM12_TRGO, TIM12_CH1, TIM12_CH2, NULL, NULL, NULL,},
>>> };
>>
>> It was just an idea, not a tested implementation.
>>
>> I don't understand why you have to pad with NULLs, but either way, it
>> looks much better than before and saves lots of lines of code.
>
> I have tested it this morning and it works fine so I will include it in v5.
> I use NULL as limit when iterate in the table and for table padding too.
If the initializer is shorter than the array then the array will be
implicitly zero/NULL padded. I don't think there is any need to type out
all the NULLs (not even at -Wall).
Daniel.
^ permalink raw reply
* [PATCH 15/16] drivers/fsi: Add documentation for GPIO bindings
From: Mark Rutland @ 2016-12-07 12:02 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1481069677-53660-16-git-send-email-christopher.lee.bostic@gmail.com>
On Tue, Dec 06, 2016 at 06:14:36PM -0600, Chris Bostic wrote:
> From: Chris Bostic <cbostic@us.ibm.com>
>
> Add fsi master gpio device tree binding documentation
Please see Documentation/devicetree/bindings/submitting-patches.txt.
Specifically:
* Please put binding documents earlier in the series than code
implementing the binding.
* Please document _all_ compatible strings used in the
series.
Please also write the binding documents in terms of the hardware, rather
then the driver (e.g. introduce what the hardware is in the document,
don't mention the driver). The bindings are there to describe the former
to the latter, and the latter may change arbitrarily.
> Signed-off-by: Chris Bostic <cbostic@us.ibm.com>
> ---
> .../devicetree/bindings/fsi/fsi-master-gpio.txt | 21 +++++++++++++++++++++
> 1 file changed, 21 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/fsi/fsi-master-gpio.txt
>
> diff --git a/Documentation/devicetree/bindings/fsi/fsi-master-gpio.txt b/Documentation/devicetree/bindings/fsi/fsi-master-gpio.txt
AFAICT, this is the first use of this directory. We should have a
general FSI binding document in there, covering what FSI is, the
"ibm,fsi-master" binding, etc.
> new file mode 100644
> index 0000000..ff3a62e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/fsi/fsi-master-gpio.txt
> @@ -0,0 +1,21 @@
> +Device-tree bindings for gpio-based FSI master driver
There's very little information here, so I'm not sure what to make of
this. Can you please elaborate on the above to make it clear what this
means?
IIUC, this is an FSI controller/master that we only communicate with via
GPIOs, right?
Or is this a *virtual* master? i.e. the GPIOs themselves form the master
and are directly connected to slaves?
> +-----------------------------------------------------
> +
> +Required properties:
> + - compatible = "ibm,fsi-master-gpio";
> + - clk-gpios;
> + - data-gpios;
Please give a description of what each of these are used for, how many
are required, and what order elements must come in.
> +Optional properties:
> + - enable-gpios;
> + - trans-gpios;
> + - mux-gpios;
Likewise.
> +
> +fsi-master {
> + compatible = "ibm,fsi-master", "ibm,fsi-master-gpio";
This is backwards. The most specific string must come first.
> + clk-gpios = <&gpio 0 &gpio 6>;
> + data-gpios = <&gpio 1 &gpio 7>;
> + enable-gpios = <&gpio 2 &gpio 8>; /* Enable FSI data in/out */
> + trans-gpios = <&gpio 3 &gpio 9>; /* Volts translator direction */
> + mux-gpios = <&gpio 4> &gpio 10>; /* Multiplexer for FSI pins */
If this were described above, we don't need the comment here.
I note that in the patch, the mux-gpios property has an unmatched '>'
and won't compile.
As a general nit, please bracket elements of a list individually, e.g.
trans-gpios = <&gpio 3>, <&gpio 9>;
mux-gpios = <&gpio 4>, <&gpio 10>;
Thanks,
Mark.
^ permalink raw reply
* [PATCH v2] crypto: sun4i-ss: support the Security System PRNG
From: Herbert Xu @ 2016-12-07 12:09 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161205125738.GA13525@Red>
On Mon, Dec 05, 2016 at 01:57:38PM +0100, Corentin Labbe wrote:
>
> So how to expose PRNG to user space ? or more generally how to "use" a PRNG ?
We do have the algif_rng interface.
> I found hisi-rng, crypto4xx_ and exynos-rng which seems to be PRNG used as hwrng.
Thanks for checking. Patches to remove these are welcome.
Cheers,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* [PATCH 05/16] drivers/fsi: Add fake master driver
From: Mark Rutland @ 2016-12-07 12:09 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1481069677-53660-6-git-send-email-christopher.lee.bostic@gmail.com>
On Tue, Dec 06, 2016 at 06:14:26PM -0600, Chris Bostic wrote:
> From: Jeremy Kerr <jk@ozlabs.org>
>
> For debugging, add a fake master driver, that only supports reads,
> returning a fixed set of data.
> +config FSI_MASTER_FAKE
> + tristate "Fake FSI master"
> + depends on FSI
> + ---help---
> + This option enables a fake FSI master driver for debugging.
> +endif
> +static const struct of_device_id fsi_master_fake_match[] = {
> + { .compatible = "ibm,fsi-master-fake" },
> + { },
> +};
NAK.
DT should be treated as an ABI, and should describe the HW explicitly.
This makes no sense. This is also missing a binding document.
Have your module take a module parameter allowing you to bind it to
arbitrary devices, or do something like what PCI does where you can
bind/unbind arbitrary drivers to devices using sysfs.
> +
> +static struct platform_driver fsi_master_fake_driver = {
> + .driver = {
> + .name = "fsi-master-fake",
> + .of_match_table = fsi_master_fake_match,
> + },
> + .probe = fsi_master_fake_probe,
> +};
> +
> +static int __init fsi_master_fake_init(void)
> +{
> + struct device_node *np;
> +
> + platform_driver_register(&fsi_master_fake_driver);
> +
> + for_each_compatible_node(np, NULL, "ibm,fsi-master-fake")
> + of_platform_device_create(np, NULL, NULL);
As a general note, please use for_each_matching_node in situations like
this. That way you can reuse your existing of_device_id table, and not
reproduce the string.
That said, this is not necessary. The platform driver has an
of_match_table, so presumes the parent bus registers children, and hence
they should already have platform devices.
Thanks,
Mark.
^ permalink raw reply
* [PATCH v3 0/6] crypto: ARM/arm64 CRC-T10DIF/CRC32/CRC32C roundup
From: Herbert Xu @ 2016-12-07 12:09 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1480963348-24203-1-git-send-email-ard.biesheuvel@linaro.org>
On Mon, Dec 05, 2016 at 06:42:22PM +0000, Ard Biesheuvel wrote:
> This v3 combines the CRC-T10DIF and CRC32 implementations for both ARM and
> arm64 that I sent out a couple of weeks ago, and adds support to the latter
> for CRC32C.
All applied. Thanks.
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* [PATCH 4/7] arm: Add ftrace with regs support
From: Abel Vesa @ 2016-12-07 12:10 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <cf63d647-b5ce-a41b-31ee-6d14da60f541@arm.com>
On Wed, Dec 07, 2016 at 11:58:24AM +0000, Robin Murphy wrote:
> Hi Abel,
>
> On 06/12/16 17:06, Abel Vesa wrote:
> > This adds __ftrace_regs_caller which, unlike __ftrace_caller,
> > adds register saving/restoring and livepatch handling if
> > the pc register gets modified by klp_ftrace_handler.
> >
> > Signed-off-by: Abel Vesa <abelvesa@linux.com>
> > ---
> > arch/arm/kernel/entry-ftrace.S | 49 ++++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 49 insertions(+)
> >
> > diff --git a/arch/arm/kernel/entry-ftrace.S b/arch/arm/kernel/entry-ftrace.S
> > index c73c403..b6ada5c 100644
> > --- a/arch/arm/kernel/entry-ftrace.S
> > +++ b/arch/arm/kernel/entry-ftrace.S
> > @@ -92,6 +92,46 @@
> > 2: mcount_exit
> > .endm
> >
> > +#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
> > +
> > +.macro __ftrace_regs_caller suffix
> > +
> > + stmdb sp!, {r0-r15}
>
> As the kbuild robot reported, you can't do this. For ARM, it's unknown
> what the value stored for r13 will be, and for a Thumb-2 kernel the
> whole instruction is flat out unpredictable (i.e. it might just undef or
> anything).
>
> > + mov r3, sp
> > +
> > + ldr r10, [sp, #60]
> > +
> > + mcount_get_lr r1 @ lr of instrumented func
> > + mcount_adjust_addr r0, lr @ instrumented function
> > +
> > + .globl ftrace_regs_call\suffix
> > +ftrace_regs_call\suffix:
> > + bl ftrace_stub
> > +
> > +#ifdef CONFIG_FUNCTION_GRAPH_TRACER
> > + .globl ftrace_regs_graph_call\suffix
> > +ftrace_regs_graph_call\suffix:
> > + mov r0, r0
> > +#endif
> > +#ifdef CONFIG_LIVEPATCH
> > + ldr r0, [sp, #60]
> > + cmp r0, r10
> > + beq ftrace_regs_caller_end
> > + ldmia sp!, {r0-r12}
> > + add sp, sp, #8
> > + ldmia sp!, {r11}
> > + sub sp, r12, #16
> > + str r11, [sp, #12]
> > + ldmia sp!, {r11, r12, lr, pc}
> > +#endif
> > +ftrace_regs_caller_end\suffix:
> > + ldmia sp!, {r0-r14}
>
> Again, the value of SP at this point is now unknown (regardless of
> whether what the value on memory was correct or not). Not good.
>
> Either use non-writeback addressing modes, or avoid saving/restoring SP
> at all (AFAICS it isn't necessary, since if the SP was changed in
> between, you then wouldn't know where to restore the saved registers
> from anyway).
>
> Robin.
>
Yep, as I said in the cover letter, I'll try to fix that in the next
iteration of this patch. You're right, sp doesn't need to be pushed or
popped. I'll send a another patch series which will include a fix for
that tomorrow, latest.
Thanks.
> > + add sp, sp, #4
> > + mov pc, lr
> > +.endm
> > +
> > +#endif
> > +
> > .macro __ftrace_caller suffix
> > mcount_enter
> >
> > @@ -212,6 +252,15 @@ UNWIND(.fnstart)
> > __ftrace_caller
> > UNWIND(.fnend)
> > ENDPROC(ftrace_caller)
> > +
> > +#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
> > +ENTRY(ftrace_regs_caller)
> > +UNWIND(.fnstart)
> > + __ftrace_regs_caller
> > +UNWIND(.fnend)
> > +ENDPROC(ftrace_regs_caller)
> > +#endif
> > +
> > #endif
> >
> > #ifdef CONFIG_FUNCTION_GRAPH_TRACER
> >
>
^ permalink raw reply
* [PATCH] arm/xen: Use alloc_percpu rather than __alloc_percpu
From: Julien Grall @ 2016-12-07 12:24 UTC (permalink / raw)
To: linux-arm-kernel
The function xen_guest_init is using __alloc_percpu with an alignment
which are not power of two.
However, the percpu allocator never supported alignments which are not power
of two and has always behaved incorectly in thise case.
Commit 3ca45a4 "percpu: ensure requested alignment is power of two"
introduced a check which trigger a warning [1] when booting linux-next
on Xen. But in fine this bug was always present.
This can be fixed by replacing the call to __alloc_percpu with
alloc_percpu. The latter will use an alignment which are a power of two.
[1]
[ 0.023921] illegal size (48) or align (48) for percpu allocation
[ 0.024167] ------------[ cut here ]------------
[ 0.024344] WARNING: CPU: 0 PID: 1 at linux/mm/percpu.c:892 pcpu_alloc+0x88/0x6c0
[ 0.024584] Modules linked in:
[ 0.024708]
[ 0.024804] CPU: 0 PID: 1 Comm: swapper/0 Not tainted
4.9.0-rc7-next-20161128 #473
[ 0.025012] Hardware name: Foundation-v8A (DT)
[ 0.025162] task: ffff80003d870000 task.stack: ffff80003d844000
[ 0.025351] PC is at pcpu_alloc+0x88/0x6c0
[ 0.025490] LR is at pcpu_alloc+0x88/0x6c0
[ 0.025624] pc : [<ffff00000818e678>] lr : [<ffff00000818e678>]
pstate: 60000045
[ 0.025830] sp : ffff80003d847cd0
[ 0.025946] x29: ffff80003d847cd0 x28: 0000000000000000
[ 0.026147] x27: 0000000000000000 x26: 0000000000000000
[ 0.026348] x25: 0000000000000000 x24: 0000000000000000
[ 0.026549] x23: 0000000000000000 x22: 00000000024000c0
[ 0.026752] x21: ffff000008e97000 x20: 0000000000000000
[ 0.026953] x19: 0000000000000030 x18: 0000000000000010
[ 0.027155] x17: 0000000000000a3f x16: 00000000deadbeef
[ 0.027357] x15: 0000000000000006 x14: ffff000088f79c3f
[ 0.027573] x13: ffff000008f79c4d x12: 0000000000000041
[ 0.027782] x11: 0000000000000006 x10: 0000000000000042
[ 0.027995] x9 : ffff80003d847a40 x8 : 6f697461636f6c6c
[ 0.028208] x7 : 6120757063726570 x6 : ffff000008f79c84
[ 0.028419] x5 : 0000000000000005 x4 : 0000000000000000
[ 0.028628] x3 : 0000000000000000 x2 : 000000000000017f
[ 0.028840] x1 : ffff80003d870000 x0 : 0000000000000035
[ 0.029056]
[ 0.029152] ---[ end trace 0000000000000000 ]---
[ 0.029297] Call trace:
[ 0.029403] Exception stack(0xffff80003d847b00 to
0xffff80003d847c30)
[ 0.029621] 7b00: 0000000000000030 0001000000000000
ffff80003d847cd0 ffff00000818e678
[ 0.029901] 7b20: 0000000000000002 0000000000000004
ffff000008f7c060 0000000000000035
[ 0.030153] 7b40: ffff000008f79000 ffff000008c4cd88
ffff80003d847bf0 ffff000008101778
[ 0.030402] 7b60: 0000000000000030 0000000000000000
ffff000008e97000 00000000024000c0
[ 0.030647] 7b80: 0000000000000000 0000000000000000
0000000000000000 0000000000000000
[ 0.030895] 7ba0: 0000000000000035 ffff80003d870000
000000000000017f 0000000000000000
[ 0.031144] 7bc0: 0000000000000000 0000000000000005
ffff000008f79c84 6120757063726570
[ 0.031394] 7be0: 6f697461636f6c6c ffff80003d847a40
0000000000000042 0000000000000006
[ 0.031643] 7c00: 0000000000000041 ffff000008f79c4d
ffff000088f79c3f 0000000000000006
[ 0.031877] 7c20: 00000000deadbeef 0000000000000a3f
[ 0.032051] [<ffff00000818e678>] pcpu_alloc+0x88/0x6c0
[ 0.032229] [<ffff00000818ece8>] __alloc_percpu+0x18/0x20
[ 0.032409] [<ffff000008d9606c>] xen_guest_init+0x174/0x2f4
[ 0.032591] [<ffff0000080830f8>] do_one_initcall+0x38/0x130
[ 0.032783] [<ffff000008d90c34>] kernel_init_freeable+0xe0/0x248
[ 0.032995] [<ffff00000899a890>] kernel_init+0x10/0x100
[ 0.033172] [<ffff000008082ec0>] ret_from_fork+0x10/0x50
Reported-by: Wei Chen <wei.chen@arm.com>
Link: https://lkml.org/lkml/2016/11/28/669
Signed-off-by: Julien Grall <julien.grall@arm.com>
Cc: stable at vger.kernel.org
---
I have requested backport to stable because the percpu allocator may
misbehaves when the alignment is not a power of two.
---
arch/arm/xen/enlighten.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
index f193414..4986dc0 100644
--- a/arch/arm/xen/enlighten.c
+++ b/arch/arm/xen/enlighten.c
@@ -372,8 +372,7 @@ static int __init xen_guest_init(void)
* for secondary CPUs as they are brought up.
* For uniformity we use VCPUOP_register_vcpu_info even on cpu0.
*/
- xen_vcpu_info = __alloc_percpu(sizeof(struct vcpu_info),
- sizeof(struct vcpu_info));
+ xen_vcpu_info = alloc_percpu(struct vcpu_info);
if (xen_vcpu_info == NULL)
return -ENOMEM;
--
1.9.1
^ permalink raw reply related
* [Question] New mmap64 syscall?
From: Yury Norov @ 2016-12-07 12:39 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <0F280FED-870A-42B5-ABC4-1976ACA32462@theobroma-systems.com>
Hi Philipp,
On Wed, Dec 07, 2016 at 12:07:24PM +0100, Dr.Philipp Tomsich wrote:
> [Resend, as my mail-client had insisted on using the wrong MIME type?]
>
> > On 07 Dec 2016, at 11:34, Yury Norov <ynorov@caviumnetworks.com> wrote:
> >
> >> If there is a use case for larger than 16TB offsets, we should add
> >> the call on all architectures, probably using your approach 3. I don't
> >> think that we should treat it as anything special for arm64 though.
> >
> > From this point of view, 16+TB offset is a matter of 16+TB storage,
> > and it's more than real. The other consideration to add it is that
> > we have 64-bit support for offsets in syscalls like sys_llseek().
> > So mmap64() will simply extend this support.
>
> I believe the question is rather if the 16TB offset is a real use-case for ILP32.
This is not for ilp32, but for all 32-bit architectures - both native
and compat. And because the scope is so generic, I think it's the
strong reason for us to support true 64-bit offset in mmap().
> This seems to bring the discussion full-circle, as this would indicate that 64bit is the
> preferred bit-width for all sizes, offsets, etc. throughout all filesystem-related calls
> (i.e. stat, seek, etc.).
AARCH64/ILP32 (and all new arches) exposes ino_t, off_t, blkcnt_t,
fsblkcnt_t, fsfilcnt_t and rlim_t as 64-bit types. (Size_t should
be 32-bit of course, because it's the same lengths as pointer.)
It allows to make syscalls that pass it support 64-bit values, refer
Documentation/arm64/ilp32.txt for details. Stat and seek are both
supporting 64-bit types. From this point of view, mmap() is the (only?)
exception in current ILP32 ABI.
> But if that is the case, then we should have gone with 64bit arguments in a single
> register for our ILP32 definition on AArch64.
There are 2 unrelated matters - the size of types, and the size of
register. Most of 32-bit architectures has hardware limitation on
register size (consider aarch32). And it doesn't mean that they are
forced to stuck with 32-bit off_t etc. This is still opened question
how to pass 64-bit parameters in aarch64/ilp32 because there we have
the choice (the reason why it's RFC). If you have new ideas - welcome
to that discussion. This topic also covers architectures that has to
pass 64-bit parameters in a pair.
> In other words: Why not keep ILP32 simple an ask users that need a 16TB+ offset
> to use LP64? It seems much more consistent with the other choices takes so far.
If user can switch to lp64, he doesn't need ilp32 at all, right? :)
Also, I don't understand how true 64-bit offset in mmap64() would
complicate this port.
Yury
^ permalink raw reply
* [PATCH v3 1/3] soc: zte: pm_domains: Prepare for supporting ARMv8 2967 family
From: Shawn Guo @ 2016-12-07 12:41 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1481091204-6559-1-git-send-email-baoyou.xie@linaro.org>
On Wed, Dec 07, 2016 at 02:13:22PM +0800, Baoyou Xie wrote:
> The ARMv8 2967 family (296718, 296716 etc) uses different value
> for controlling the power domain on/off registers, Choose the
> value depending on the compatible.
>
> Multiple domains are prepared for the family, this patch prepares
> the common functions.
>
> Signed-off-by: Baoyou Xie <baoyou.xie@linaro.org>
> ---
> drivers/soc/Kconfig | 1 +
> drivers/soc/Makefile | 1 +
> drivers/soc/zte/Kconfig | 13 ++++
> drivers/soc/zte/Makefile | 4 ++
> drivers/soc/zte/pm_domains.c | 150 +++++++++++++++++++++++++++++++++++++++++++
> drivers/soc/zte/pm_domains.h | 48 ++++++++++++++
> 6 files changed, 217 insertions(+)
> create mode 100644 drivers/soc/zte/Kconfig
> create mode 100644 drivers/soc/zte/Makefile
> create mode 100644 drivers/soc/zte/pm_domains.c
> create mode 100644 drivers/soc/zte/pm_domains.h
>
> diff --git a/drivers/soc/Kconfig b/drivers/soc/Kconfig
> index f31bceb..f09023f 100644
> --- a/drivers/soc/Kconfig
> +++ b/drivers/soc/Kconfig
> @@ -11,5 +11,6 @@ source "drivers/soc/tegra/Kconfig"
> source "drivers/soc/ti/Kconfig"
> source "drivers/soc/ux500/Kconfig"
> source "drivers/soc/versatile/Kconfig"
> +source "drivers/soc/zte/Kconfig"
>
> endmenu
> diff --git a/drivers/soc/Makefile b/drivers/soc/Makefile
> index 50c23d0..05eae52 100644
> --- a/drivers/soc/Makefile
> +++ b/drivers/soc/Makefile
> @@ -16,3 +16,4 @@ obj-$(CONFIG_ARCH_TEGRA) += tegra/
> obj-$(CONFIG_SOC_TI) += ti/
> obj-$(CONFIG_ARCH_U8500) += ux500/
> obj-$(CONFIG_PLAT_VERSATILE) += versatile/
> +obj-$(CONFIG_ARCH_ZX) += zte/
> diff --git a/drivers/soc/zte/Kconfig b/drivers/soc/zte/Kconfig
> new file mode 100644
> index 0000000..4953c3fa
> --- /dev/null
> +++ b/drivers/soc/zte/Kconfig
> @@ -0,0 +1,13 @@
> +#
> +# zx SoC drivers
> +#
> +menuconfig SOC_ZX
Can we use SOC_ZTE to reflect the folder name 'zte'? If we take
drivers/soc/samsung/Kconfig as example, SAMSUNG is like ZTE, while
EXYNOS is like ZX, I guess.
> + bool "zx SoC driver support"
> +
> +if SOC_ZX
> +
> +config ZX_PM_DOMAINS
> + bool "zx PM domains"
> + depends on PM_GENERIC_DOMAINS
> +
> +endif
> diff --git a/drivers/soc/zte/Makefile b/drivers/soc/zte/Makefile
> new file mode 100644
> index 0000000..97ac8ea
> --- /dev/null
> +++ b/drivers/soc/zte/Makefile
> @@ -0,0 +1,4 @@
> +#
> +# zx SOC drivers
> +#
> +obj-$(CONFIG_ZX_PM_DOMAINS) += pm_domains.o
> diff --git a/drivers/soc/zte/pm_domains.c b/drivers/soc/zte/pm_domains.c
> new file mode 100644
> index 0000000..e4d1235
> --- /dev/null
> +++ b/drivers/soc/zte/pm_domains.c
> @@ -0,0 +1,150 @@
> +/*
> + * Copyright (C) 2015 ZTE Ltd.
Do we at least need year 2016?
> + *
> + * Author: Baoyou Xie <baoyou.xie@linaro.org>
> + * License terms: GNU General Public License (GPL) version 2
I see drivers/soc/zte/pm_domains.h use a different licence format. Can
we make them unified?
> + */
> +#include <linux/delay.h>
> +#include <linux/err.h>
> +#include <linux/io.h>
> +#include <linux/of.h>
> +#include "pm_domains.h"
> +
> +#define PCU_DM_CLKEN(zpd) ((zpd)->reg_offset[REG_CLKEN])
> +#define PCU_DM_ISOEN(zpd) ((zpd)->reg_offset[REG_ISOEN])
> +#define PCU_DM_RSTEN(zpd) ((zpd)->reg_offset[REG_RSTEN])
> +#define PCU_DM_PWREN(zpd) ((zpd)->reg_offset[REG_PWREN])
> +#define PCU_DM_PWRDN(zpd) ((zpd)->reg_offset[REG_PWRDN])
> +#define PCU_DM_ACK_SYNC(zpd) ((zpd)->reg_offset[REG_ACK_SYNC])
> +
> +static void __iomem *pcubase;
> +
> +int zx_normal_power_on(struct generic_pm_domain *domain)
What does 'normal' in the function name mean?
> +{
> + struct zx_pm_domain *zpd = (struct zx_pm_domain *)domain;
> + unsigned long loop = 1000;
> + u32 val;
> +
> + if (zpd->polarity == PWREN) {
> + val = readl_relaxed(pcubase + PCU_DM_PWREN(zpd));
> + val |= BIT(zpd->bit);
> + writel_relaxed(val, pcubase + PCU_DM_PWREN(zpd));
> + } else {
> + val = readl_relaxed(pcubase + PCU_DM_PWRDN(zpd));
> + val &= ~BIT(zpd->bit);
> + writel_relaxed(val, pcubase + PCU_DM_PWRDN(zpd));
> + }
> +
> + do {
> + udelay(1);
> + val = readl_relaxed(pcubase + PCU_DM_ACK_SYNC(zpd))
> + & BIT(zpd->bit);
> + } while (--loop && !val);
> +
> + if (!loop) {
> + pr_err("Error: %s %s fail\n", __func__, domain->name);
> + return -EIO;
> + }
> +
> + val = readl_relaxed(pcubase + PCU_DM_RSTEN(zpd));
> + val &= ~BIT(zpd->bit);
> + writel_relaxed(val | BIT(zpd->bit), pcubase + PCU_DM_RSTEN(zpd));
For a single bit setting, it can be as simple as the following, right?
val = readl_relaxed(pcubase + PCU_DM_RSTEN(zpd));
val |= BIT(zpd->bit);
writel_relaxed(val, pcubase + PCU_DM_RSTEN(zpd));
> + udelay(5);
> +
> + val = readl_relaxed(pcubase + PCU_DM_ISOEN(zpd));
> + val &= ~BIT(zpd->bit);
> + writel_relaxed(val, pcubase + PCU_DM_ISOEN(zpd));
> + udelay(5);
> +
> + val = readl_relaxed(pcubase + PCU_DM_CLKEN(zpd));
> + val &= ~BIT(zpd->bit);
> + writel_relaxed(val | BIT(zpd->bit), pcubase + PCU_DM_CLKEN(zpd));
Ditto
> + udelay(5);
> +
> + pr_info("normal poweron %s\n", domain->name);
Do we really want to see a message on every single power domain on/off
function call? A pr_debug is better?
> +
> + return 0;
> +}
> +
> +int zx_normal_power_off(struct generic_pm_domain *domain)
> +{
> + struct zx_pm_domain *zpd = (struct zx_pm_domain *)domain;
> + unsigned long loop = 1000;
> + u32 val;
> +
> + val = readl_relaxed(pcubase + PCU_DM_CLKEN(zpd));
> + val &= ~BIT(zpd->bit);
> + writel_relaxed(val, pcubase + PCU_DM_CLKEN(zpd));
> + udelay(5);
> +
> + val = readl_relaxed(pcubase + PCU_DM_ISOEN(zpd));
> + val &= ~BIT(zpd->bit);
> + writel_relaxed(val | BIT(zpd->bit), pcubase + PCU_DM_ISOEN(zpd));
Same here.
> + udelay(5);
> +
> + val = readl_relaxed(pcubase + PCU_DM_RSTEN(zpd));
> + val &= ~BIT(zpd->bit);
> + writel_relaxed(val, pcubase + PCU_DM_RSTEN(zpd));
> + udelay(5);
> +
> + if (zpd->polarity == PWREN) {
> + val = readl_relaxed(pcubase + PCU_DM_PWREN(zpd));
> + val &= ~BIT(zpd->bit);
> + writel_relaxed(val, pcubase + PCU_DM_PWREN(zpd));
> + } else {
> + val = readl_relaxed(pcubase + PCU_DM_PWRDN(zpd));
> + val |= BIT(zpd->bit);
> + writel_relaxed(val, pcubase + PCU_DM_PWRDN(zpd));
> + }
> +
> + do {
> + udelay(1);
> + val = readl_relaxed(pcubase + PCU_DM_ACK_SYNC(zpd))
> + & BIT(zpd->bit);
> + } while (--loop && val);
> +
> + if (!loop) {
> + pr_err("Error: %s %s fail\n", __func__, domain->name);
> + return -EIO;
> + }
> +
> + pr_info("normal poweroff %s\n", domain->name);
pr_debug
> +
> + return 0;
> +}
> +
> +int
> +zx_pd_probe(struct platform_device *pdev,
I do not see the need to break above two lines.
> + struct generic_pm_domain **zx_pm_domains,
> + int domain_num)
As long as the line doesn't exceed column 80, we do not need to break
the line into two.
> +{
> + struct genpd_onecell_data *genpd_data;
> + struct resource *res;
> + int i;
> +
> + genpd_data = devm_kzalloc(&pdev->dev, sizeof(*genpd_data), GFP_KERNEL);
> + if (!genpd_data)
> + return -ENOMEM;
> +
> + genpd_data->domains = zx_pm_domains;
> + genpd_data->num_domains = domain_num;
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + if (!res) {
> + dev_err(&pdev->dev, "no memory resource defined\n");
> + return -ENODEV;
> + }
The error check on 'res' is not really necessary, as
devm_ioremap_resource() will do that for you.
> +
> + pcubase = devm_ioremap_resource(&pdev->dev, res);
> + if (IS_ERR(pcubase)) {
> + dev_err(&pdev->dev, "ioremap fail.\n");
> + return -EIO;
PTR_ERR(pcubase) should be returned as error code here.
> + }
> +
> + for (i = 0; i < domain_num; ++i)
> + pm_genpd_init(zx_pm_domains[i], NULL, false);
> +
> + of_genpd_add_provider_onecell(pdev->dev.of_node, genpd_data);
> + dev_info(&pdev->dev, "powerdomain init ok\n");
> + return 0;
> +}
> diff --git a/drivers/soc/zte/pm_domains.h b/drivers/soc/zte/pm_domains.h
> new file mode 100644
> index 0000000..d3a52fd
> --- /dev/null
> +++ b/drivers/soc/zte/pm_domains.h
> @@ -0,0 +1,48 @@
> +/*
> + * Copyright (c) 2015 ZTE Co., Ltd.
> + * http://www.zte.com.cn
> + *
> + * Header for ZTE's Power Domain Driver support
> + *
> + * 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 __ZTE_PM_DOMAIN_H
> +#define __ZTE_PM_DOMAIN_H
> +
> +#include <linux/platform_device.h>
> +#include <linux/pm_domain.h>
> +
> +enum {
> + REG_CLKEN,
> + REG_ISOEN,
> + REG_RSTEN,
> + REG_PWREN,
> + REG_PWRDN,
> + REG_ACK_SYNC,
> +
> + /* The size of the array - must be last */
> + REG_ARRAY_SIZE,
> +};
> +
> +enum zx_power_polarity {
> + PWREN,
> + PWRDN,
> +};
> +
> +struct zx_pm_domain {
> + struct generic_pm_domain dm;
You can chose to have more tabs between 'generic_pm_domain' and 'dm'
for indention, but there should always be one space between 'struct' and
'generic_pm_domain'.
> + const u16 bit;
> + const enum zx_power_polarity polarity;
> + const u16 *reg_offset;
> +};
> +
> +extern int zx_normal_power_on(struct generic_pm_domain *domain);
> +extern int zx_normal_power_off(struct generic_pm_domain *domain);
> +extern int
> +zx_pd_probe(struct platform_device *pdev,
> + struct generic_pm_domain **zx_pm_domains,
> + int domain_num);
We do not need to break it into so many lines.
> +#endif /* __ZTE_PM_DOMAIN_H */
> --
> 2.7.4
>
^ permalink raw reply
* [PATCH v3 1/5] spi: Add basic support for Armada 3700 SPI Controller
From: Romain Perier @ 2016-12-07 12:42 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161205120540.mvihgafelszitzhq@sirena.org.uk>
Hello,
Le 05/12/2016 ? 13:05, Mark Brown a ?crit :
> On Thu, Dec 01, 2016 at 11:27:15AM +0100, Romain Perier wrote:
>
>> +config SPI_ARMADA_3700
>> + tristate "Marvell Armada 3700 SPI Controller"
>> + depends on ARCH_MVEBU && OF
>
> Why does this not have a COMPILE_TEST dependency?
Because that's a mistake, I will fix it.
>
>> + /* Reset SPI unit */
>> + val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
>> + val |= A3700_SPI_SRST;
>> + spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
>> +
>> + for (i = 0; i < A3700_SPI_TIMEOUT; i++)
>> + udelay(1);
>
> Why not just do a single udelay()?
Mhhhh... good point.
>
>> +static irqreturn_t a3700_spi_interrupt(int irq, void *dev_id)
>> +{
>> + struct spi_master *master = dev_id;
>> + struct a3700_spi *a3700_spi;
>> + u32 cause;
>> +
>> + a3700_spi = spi_master_get_devdata(master);
>> +
>> + /* Get interrupt causes */
>> + cause = spireg_read(a3700_spi, A3700_SPI_INT_STAT_REG);
>> +
>> + /* mask and acknowledge the SPI interrupts */
>> + spireg_write(a3700_spi, A3700_SPI_INT_MASK_REG, 0);
>> + spireg_write(a3700_spi, A3700_SPI_INT_STAT_REG, cause);
>> +
>> + /* Wake up the transfer */
>> + if (a3700_spi->wait_mask & cause)
>> + complete(&a3700_spi->done);
>> +
>> + return IRQ_HANDLED;
>> +}
>
> This reports that we handled an interrupt even if we did not in fact
> handle an interrupt. It's also a bit dodgy that it doesn't check what
> the interrupt was but that's less serious.
I don't understand, what do you expect ? That I return something !=
IRQ_HANDLED when the cause of the interrupt is not present in wait_mask ?
>
>> + master->bus_num = (pdev->id != -1) ? pdev->id : 0;
>
> At most this should be just setting the bus number to pdev->id like
> other drivers do.
ack
>
>> + ret = clk_prepare_enable(spi->clk);
>> + if (ret) {
>> + dev_err(dev, "could not prepare clk: %d\n", ret);
>> + goto error;
>> + }
>
> I'd expect the hardware prepare/unprepare to be managing the enable and
> disable of the clock in order to save a little power.
Ok, if that's better for power management, why not then.
>
>> + dev_info(dev, "Marvell Armada 3700 SPI Controller at 0x%08lx, irq %d\n",
>> + (unsigned long)res->start, spi->irq);
>
> This is just adding noise to the boot, remove it. It's not telling us
> anything we read from the hardware or anything.
ack
Thanks,
Romain
^ permalink raw reply
* [PATCH v2] crypto: sun4i-ss: support the Security System PRNG
From: Corentin Labbe @ 2016-12-07 12:51 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161207120900.GC20680@gondor.apana.org.au>
On Wed, Dec 07, 2016 at 08:09:00PM +0800, Herbert Xu wrote:
> On Mon, Dec 05, 2016 at 01:57:38PM +0100, Corentin Labbe wrote:
> >
> > So how to expose PRNG to user space ? or more generally how to "use" a PRNG ?
>
> We do have the algif_rng interface.
>
So I must expose it as a crypto_rng ?
Could you explain why PRNG must not be used as hw_random ?
Regards
Corentin Labbe
^ permalink raw reply
* [PATCH] dmaengine: pl330: do not generate unaligned access
From: Vladimir Murzin @ 2016-12-07 13:17 UTC (permalink / raw)
To: linux-arm-kernel
When PL330 is used with !MMU the following fault is seen:
Unhandled fault: alignment exception (0x801) at 0x8f26a002
Internal error: : 801 [#1] ARM
Modules linked in:
CPU: 0 PID: 640 Comm: dma0chan0-copy0 Not tainted 4.8.0-6a82063-clean+ #1600
Hardware name: ARM-Versatile Express
task: 8f1baa80 task.stack: 8e6fe000
PC is at _setup_req+0x4c/0x350
LR is at 0x8f2cbc00
pc : [<801ea538>] lr : [<8f2cbc00>] psr: 60000093
sp : 8e6ffdc0 ip : 00000000 fp : 00000000
r10: 00000000 r9 : 8f2cba10 r8 : 8f2cbc00
r7 : 80000013 r6 : 8f21a050 r5 : 8f21a000 r4 : 8f2ac800
r3 : 8e6ffe18 r2 : 00944251 r1 : ffffffbc r0 : 8f26a000
Flags: nZCv IRQs off FIQs on Mode SVC_32 ISA ARM Segment none
Control: 00c5387c
Process dma0chan0-copy0 (pid: 640, stack limit = 0x8e6fe210)
Stack: (0x8e6ffdc0 to 0x8e700000)
fdc0: 00000001 60000093 00000000 8f2cba10 8f26a000 00000004 8f0ae000 8f2cbc00
fde0: 8f0ae000 8f2ac800 8f21a000 8f21a050 80000013 8f2cbc00 8f2cba10 00000000
fe00: 60000093 801ebca0 8e6ffe18 000013ff 40000093 00000000 00944251 8f2ac800
fe20: a0000013 8f2b1320 00001986 00000000 00000001 000013ff 8f1e4f00 8f2cba10
fe40: 8e6fff6c 801e9044 00000003 00000000 fef98c80 002faf07 8e6ffe7c 00000000
fe60: 00000002 00000000 00001986 8f1f158d 8f1e4f00 80568de4 00000002 00000000
fe80: 00001986 8f1f53ff 40000001 80580500 8f1f158d 8001e00c 00000000 cfdfdfdf
fea0: fdae2a25 00000001 00000004 8e6fe000 00000008 00000010 00000000 00000005
fec0: 8f2b1330 8f2b1334 8e6ffe80 8e6ffe8c 00001986 00000000 8f21a014 00000001
fee0: 8e6ffe60 8e6ffe78 00000002 00000000 000013ff 00000001 80568de4 8f1e8018
ff00: 0000158d 8055ec30 00000001 803f6b00 00001986 8f2cba10 fdae2a25 00000001
ff20: 8f1baca8 8e6fff24 8e6fff24 00000000 8e6fff24 ac6f3037 00000000 00000000
ff40: 00000000 8e6fe000 8f1e4f40 00000000 8f1e4f40 8f1e4f00 801e84ec 00000000
ff60: 00000000 00000000 00000000 80031714 dfdfdfcf 00000000 dfdfdfcf 8f1e4f00
ff80: 00000000 8e6fff84 8e6fff84 00000000 8e6fff90 8e6fff90 8e6fffac 8f1e4f40
ffa0: 80031640 00000000 00000000 8000f548 00000000 00000000 00000000 00000000
ffc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
ffe0: 00000000 00000000 00000000 00000000 00000013 00000000 dfdfdfcf cfdfdfdf
[<801ea538>] (_setup_req) from [<801ebca0>] (pl330_tasklet+0x41c/0x490)
[<801ebca0>] (pl330_tasklet) from [<801e9044>] (dmatest_func+0xb58/0x149c)
[<801e9044>] (dmatest_func) from [<80031714>] (kthread+0xd4/0xec)
[<80031714>] (kthread) from [<8000f548>] (ret_from_fork+0x14/0x2c)
Code: e3a03001 e3e01043 e5c03001 e59d3048 (e5802002)
This happens because _emit_{ADDH,MOV,GO) accessing to unaligned data
while writing to buffer. Fix it with writing to buffer byte by byte.
Cc: Robin Murphy <robin.murphy@arm.com>
Cc: Vinod Koul <vinod.koul@intel.com>
Signed-off-by: Vladimir Murzin <vladimir.murzin@arm.com>
---
drivers/dma/pl330.c | 15 ++++++++++-----
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c
index 030fe05..e4c1ba7 100644
--- a/drivers/dma/pl330.c
+++ b/drivers/dma/pl330.c
@@ -570,7 +570,8 @@ static inline u32 _emit_ADDH(unsigned dry_run, u8 buf[],
buf[0] = CMD_DMAADDH;
buf[0] |= (da << 1);
- *((__le16 *)&buf[1]) = cpu_to_le16(val);
+ buf[1] = val;
+ buf[2] = val >> 8;
PL330_DBGCMD_DUMP(SZ_DMAADDH, "\tDMAADDH %s %u\n",
da == 1 ? "DA" : "SA", val);
@@ -724,7 +725,10 @@ static inline u32 _emit_MOV(unsigned dry_run, u8 buf[],
buf[0] = CMD_DMAMOV;
buf[1] = dst;
- *((__le32 *)&buf[2]) = cpu_to_le32(val);
+ buf[2] = val;
+ buf[3] = val >> 8;
+ buf[4] = val >> 16;
+ buf[5] = val >> 24;
PL330_DBGCMD_DUMP(SZ_DMAMOV, "\tDMAMOV %s 0x%x\n",
dst == SAR ? "SAR" : (dst == DAR ? "DAR" : "CCR"), val);
@@ -899,10 +903,11 @@ static inline u32 _emit_GO(unsigned dry_run, u8 buf[],
buf[0] = CMD_DMAGO;
buf[0] |= (ns << 1);
-
buf[1] = chan & 0x7;
-
- *((__le32 *)&buf[2]) = cpu_to_le32(addr);
+ buf[2] = addr;
+ buf[3] = addr >> 8;
+ buf[4] = addr >> 16;
+ buf[5] = addr >> 24;
return SZ_DMAGO;
}
--
1.7.9.5
^ permalink raw reply related
* [PATCH 0/8] efi: Pass secure boot mode to kernel [ver #5]
From: David Howells @ 2016-12-07 13:18 UTC (permalink / raw)
To: linux-arm-kernel
Here's a set of patches that can determine the secure boot state of the
UEFI BIOS and pass that along to the main kernel image. This involves
generalising ARM's efi_get_secureboot() function and making it mixed-mode
safe.
Changes:
Ver 5:
- Fix i386 compilation error (rsi should've been changed to esi).
- Fix arm64 compilation error ('sys_table_arg' is a hidden macro parameter).
Ver 4:
- Use an enum to tell the kernel whether secure boot mode is enabled,
disabled, couldn't be determined or wasn't even tried due to not being
in EFI mode.
- Support the UEFI-2.6 DeployedMode flag.
- Don't clear boot_params->secure_boot in x86 sanitize_boot_params().
- Preclear the boot_params->secure_boot on x86 head_*.S entry if we may
not go through efi_main().
The patches can be found here also:
http://git.kernel.org/cgit/linux/kernel/git/dhowells/linux-fs.git/log/?h=efi-secure-boot
at tag:
efi-secure-boot-20161207-2
Note that the patches are not terminal on the branch.
David
---
Ard Biesheuvel (1):
efi: use typed function pointers for runtime services table
David Howells (5):
x86/efi: Allow invocation of arbitrary runtime services
arm/efi: Allow invocation of arbitrary runtime services
efi: Add SHIM and image security database GUID definitions
efi: Get the secure boot status
efi: Handle secure boot from UEFI-2.6
Josh Boyer (2):
efi: Disable secure boot if shim is in insecure mode
efi: Add EFI_SECURE_BOOT bit
Documentation/x86/zero-page.txt | 2 +
arch/arm/include/asm/efi.h | 1
arch/arm64/include/asm/efi.h | 1
arch/x86/boot/compressed/eboot.c | 3 +
arch/x86/boot/compressed/head_32.S | 7 +-
arch/x86/boot/compressed/head_64.S | 9 +--
arch/x86/include/asm/bootparam_utils.h | 5 +
arch/x86/include/asm/efi.h | 5 +
arch/x86/include/uapi/asm/bootparam.h | 3 +
arch/x86/kernel/asm-offsets.c | 1
arch/x86/kernel/setup.c | 15 ++++
drivers/firmware/efi/libstub/Makefile | 2 -
drivers/firmware/efi/libstub/arm-stub.c | 58 +---------------
drivers/firmware/efi/libstub/secureboot.c | 102 +++++++++++++++++++++++++++++
include/linux/efi.h | 52 ++++++++++-----
15 files changed, 182 insertions(+), 84 deletions(-)
create mode 100644 drivers/firmware/efi/libstub/secureboot.c
^ permalink raw reply
* [PATCH 1/8] efi: use typed function pointers for runtime services table [ver #5]
From: David Howells @ 2016-12-07 13:18 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <148111668193.23390.6340512985876251017.stgit@warthog.procyon.org.uk>
From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Instead of using void pointers, and casting them to correctly typed
function pointers upon use, declare the runtime services pointers
as function pointers using their respective prototypes, for which
typedefs are already available.
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Signed-off-by: David Howells <dhowells@redhat.com>
---
include/linux/efi.h | 36 ++++++++++++++++++------------------
1 file changed, 18 insertions(+), 18 deletions(-)
diff --git a/include/linux/efi.h b/include/linux/efi.h
index a07a476178cd..93a82de167eb 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -508,24 +508,6 @@ typedef struct {
u64 query_variable_info;
} efi_runtime_services_64_t;
-typedef struct {
- efi_table_hdr_t hdr;
- void *get_time;
- void *set_time;
- void *get_wakeup_time;
- void *set_wakeup_time;
- void *set_virtual_address_map;
- void *convert_pointer;
- void *get_variable;
- void *get_next_variable;
- void *set_variable;
- void *get_next_high_mono_count;
- void *reset_system;
- void *update_capsule;
- void *query_capsule_caps;
- void *query_variable_info;
-} efi_runtime_services_t;
-
typedef efi_status_t efi_get_time_t (efi_time_t *tm, efi_time_cap_t *tc);
typedef efi_status_t efi_set_time_t (efi_time_t *tm);
typedef efi_status_t efi_get_wakeup_time_t (efi_bool_t *enabled, efi_bool_t *pending,
@@ -560,6 +542,24 @@ typedef efi_status_t efi_query_variable_store_t(u32 attributes,
unsigned long size,
bool nonblocking);
+typedef struct {
+ efi_table_hdr_t hdr;
+ efi_get_time_t *get_time;
+ efi_set_time_t *set_time;
+ efi_get_wakeup_time_t *get_wakeup_time;
+ efi_set_wakeup_time_t *set_wakeup_time;
+ efi_set_virtual_address_map_t *set_virtual_address_map;
+ void *convert_pointer;
+ efi_get_variable_t *get_variable;
+ efi_get_next_variable_t *get_next_variable;
+ efi_set_variable_t *set_variable;
+ efi_get_next_high_mono_count_t *get_next_high_mono_count;
+ efi_reset_system_t *reset_system;
+ efi_update_capsule_t *update_capsule;
+ efi_query_capsule_caps_t *query_capsule_caps;
+ efi_query_variable_info_t *query_variable_info;
+} efi_runtime_services_t;
+
void efi_native_runtime_setup(void);
/*
^ permalink raw reply related
* [PATCH 2/8] x86/efi: Allow invocation of arbitrary runtime services [ver #5]
From: David Howells @ 2016-12-07 13:18 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <148111668193.23390.6340512985876251017.stgit@warthog.procyon.org.uk>
Provide the ability to perform mixed-mode runtime service calls for x86 in
the same way that commit 0a637ee61247bd4bed9b2a07568ef7a1cfc76187
("x86/efi: Allow invocation of arbitrary boot services") provides the
ability to invoke arbitrary boot services.
Suggested-by: Lukas Wunner <lukas@wunner.de>
Signed-off-by: David Howells <dhowells@redhat.com>
---
arch/x86/boot/compressed/eboot.c | 1 +
arch/x86/boot/compressed/head_32.S | 6 +++---
arch/x86/boot/compressed/head_64.S | 8 ++++----
arch/x86/include/asm/efi.h | 5 +++++
4 files changed, 13 insertions(+), 7 deletions(-)
diff --git a/arch/x86/boot/compressed/eboot.c b/arch/x86/boot/compressed/eboot.c
index ff01c8fc76f7..c8c32ebcdfdb 100644
--- a/arch/x86/boot/compressed/eboot.c
+++ b/arch/x86/boot/compressed/eboot.c
@@ -32,6 +32,7 @@ static void setup_boot_services##bits(struct efi_config *c) \
\
table = (typeof(table))sys_table; \
\
+ c->runtime_services = table->runtime; \
c->boot_services = table->boottime; \
c->text_output = table->con_out; \
}
diff --git a/arch/x86/boot/compressed/head_32.S b/arch/x86/boot/compressed/head_32.S
index fd0b6a272dd5..d85b9625e836 100644
--- a/arch/x86/boot/compressed/head_32.S
+++ b/arch/x86/boot/compressed/head_32.S
@@ -82,7 +82,7 @@ ENTRY(efi_pe_entry)
/* Relocate efi_config->call() */
leal efi32_config(%esi), %eax
- add %esi, 32(%eax)
+ add %esi, 40(%eax)
pushl %eax
call make_boot_params
@@ -108,7 +108,7 @@ ENTRY(efi32_stub_entry)
/* Relocate efi_config->call() */
leal efi32_config(%esi), %eax
- add %esi, 32(%eax)
+ add %esi, 40(%eax)
pushl %eax
2:
call efi_main
@@ -264,7 +264,7 @@ relocated:
#ifdef CONFIG_EFI_STUB
.data
efi32_config:
- .fill 4,8,0
+ .fill 5,8,0
.long efi_call_phys
.long 0
.byte 0
diff --git a/arch/x86/boot/compressed/head_64.S b/arch/x86/boot/compressed/head_64.S
index efdfba21a5b2..beab8322f72a 100644
--- a/arch/x86/boot/compressed/head_64.S
+++ b/arch/x86/boot/compressed/head_64.S
@@ -265,7 +265,7 @@ ENTRY(efi_pe_entry)
/*
* Relocate efi_config->call().
*/
- addq %rbp, efi64_config+32(%rip)
+ addq %rbp, efi64_config+40(%rip)
movq %rax, %rdi
call make_boot_params
@@ -285,7 +285,7 @@ handover_entry:
* Relocate efi_config->call().
*/
movq efi_config(%rip), %rax
- addq %rbp, 32(%rax)
+ addq %rbp, 40(%rax)
2:
movq efi_config(%rip), %rdi
call efi_main
@@ -457,14 +457,14 @@ efi_config:
#ifdef CONFIG_EFI_MIXED
.global efi32_config
efi32_config:
- .fill 4,8,0
+ .fill 5,8,0
.quad efi64_thunk
.byte 0
#endif
.global efi64_config
efi64_config:
- .fill 4,8,0
+ .fill 5,8,0
.quad efi_call
.byte 1
#endif /* CONFIG_EFI_STUB */
diff --git a/arch/x86/include/asm/efi.h b/arch/x86/include/asm/efi.h
index e99675b9c861..2f77bcefe6b4 100644
--- a/arch/x86/include/asm/efi.h
+++ b/arch/x86/include/asm/efi.h
@@ -191,6 +191,7 @@ static inline efi_status_t efi_thunk_set_virtual_address_map(
struct efi_config {
u64 image_handle;
u64 table;
+ u64 runtime_services;
u64 boot_services;
u64 text_output;
efi_status_t (*call)(unsigned long, ...);
@@ -226,6 +227,10 @@ static inline bool efi_is_64bit(void)
#define __efi_call_early(f, ...) \
__efi_early()->call((unsigned long)f, __VA_ARGS__);
+#define efi_call_runtime(f, ...) \
+ __efi_early()->call(efi_table_attr(efi_runtime_services, f, \
+ __efi_early()->runtime_services), __VA_ARGS__)
+
extern bool efi_reboot_required(void);
#else
^ permalink raw reply related
* [PATCH 3/8] arm/efi: Allow invocation of arbitrary runtime services [ver #5]
From: David Howells @ 2016-12-07 13:18 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <148111668193.23390.6340512985876251017.stgit@warthog.procyon.org.uk>
efi_call_runtime() is provided for x86 to be able abstract mixed mode
support. Provide this for ARM also so that common code work in mixed mode
also.
Suggested-by: Lukas Wunner <lukas@wunner.de>
Signed-off-by: David Howells <dhowells@redhat.com>
---
arch/arm/include/asm/efi.h | 1 +
arch/arm64/include/asm/efi.h | 1 +
2 files changed, 2 insertions(+)
diff --git a/arch/arm/include/asm/efi.h b/arch/arm/include/asm/efi.h
index 0b06f5341b45..e4e6a9d6a825 100644
--- a/arch/arm/include/asm/efi.h
+++ b/arch/arm/include/asm/efi.h
@@ -55,6 +55,7 @@ void efi_virtmap_unload(void);
#define efi_call_early(f, ...) sys_table_arg->boottime->f(__VA_ARGS__)
#define __efi_call_early(f, ...) f(__VA_ARGS__)
+#define efi_call_runtime(f, ...) sys_table_arg->runtime->f(__VA_ARGS__)
#define efi_is_64bit() (false)
#define efi_call_proto(protocol, f, instance, ...) \
diff --git a/arch/arm64/include/asm/efi.h b/arch/arm64/include/asm/efi.h
index 771b3f0bc757..d74ae223d89f 100644
--- a/arch/arm64/include/asm/efi.h
+++ b/arch/arm64/include/asm/efi.h
@@ -49,6 +49,7 @@ int efi_set_mapping_permissions(struct mm_struct *mm, efi_memory_desc_t *md);
#define efi_call_early(f, ...) sys_table_arg->boottime->f(__VA_ARGS__)
#define __efi_call_early(f, ...) f(__VA_ARGS__)
+#define efi_call_runtime(f, ...) sys_table_arg->runtime->f(__VA_ARGS__)
#define efi_is_64bit() (true)
#define efi_call_proto(protocol, f, instance, ...) \
^ permalink raw reply related
* [PATCH 4/8] efi: Add SHIM and image security database GUID definitions [ver #5]
From: David Howells @ 2016-12-07 13:18 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <148111668193.23390.6340512985876251017.stgit@warthog.procyon.org.uk>
Add the definitions for shim and image security database, both of which
are used widely in various Linux distros.
Signed-off-by: Josh Boyer <jwboyer@fedoraproject.org>
Signed-off-by: David Howells <dhowells@redhat.com>
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
include/linux/efi.h | 3 +++
1 file changed, 3 insertions(+)
diff --git a/include/linux/efi.h b/include/linux/efi.h
index 93a82de167eb..c7904556d7a8 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -610,6 +610,9 @@ void efi_native_runtime_setup(void);
#define EFI_CONSOLE_OUT_DEVICE_GUID EFI_GUID(0xd3b36f2c, 0xd551, 0x11d4, 0x9a, 0x46, 0x00, 0x90, 0x27, 0x3f, 0xc1, 0x4d)
#define APPLE_PROPERTIES_PROTOCOL_GUID EFI_GUID(0x91bd12fe, 0xf6c3, 0x44fb, 0xa5, 0xb7, 0x51, 0x22, 0xab, 0x30, 0x3a, 0xe0)
+#define EFI_IMAGE_SECURITY_DATABASE_GUID EFI_GUID(0xd719b2cb, 0x3d3a, 0x4596, 0xa3, 0xbc, 0xda, 0xd0, 0x0e, 0x67, 0x65, 0x6f)
+#define EFI_SHIM_LOCK_GUID EFI_GUID(0x605dab50, 0xe046, 0x4300, 0xab, 0xb6, 0x3d, 0xd8, 0x10, 0xdd, 0x8b, 0x23)
+
/*
* This GUID is used to pass to the kernel proper the struct screen_info
* structure that was populated by the stub based on the GOP protocol instance
^ permalink raw reply related
* [PATCH 5/8] efi: Get the secure boot status [ver #5]
From: David Howells @ 2016-12-07 13:18 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <148111668193.23390.6340512985876251017.stgit@warthog.procyon.org.uk>
Get the firmware's secure-boot status in the kernel boot wrapper and stash
it somewhere that the main kernel image can find.
The efi_get_secureboot() function is extracted from the arm stub and (a)
generalised so that it can be called from x86 and (b) made to use
efi_call_runtime() so that it can be run in mixed-mode.
Suggested-by: Lukas Wunner <lukas@wunner.de>
Signed-off-by: David Howells <dhowells@redhat.com>
---
Documentation/x86/zero-page.txt | 2 +
arch/x86/boot/compressed/eboot.c | 2 +
arch/x86/boot/compressed/head_32.S | 1
arch/x86/boot/compressed/head_64.S | 1
arch/x86/include/asm/bootparam_utils.h | 5 +-
arch/x86/include/uapi/asm/bootparam.h | 3 +
arch/x86/kernel/asm-offsets.c | 1
drivers/firmware/efi/libstub/Makefile | 2 -
drivers/firmware/efi/libstub/arm-stub.c | 58 +------------------------
drivers/firmware/efi/libstub/secureboot.c | 66 +++++++++++++++++++++++++++++
include/linux/efi.h | 8 ++++
11 files changed, 90 insertions(+), 59 deletions(-)
create mode 100644 drivers/firmware/efi/libstub/secureboot.c
diff --git a/Documentation/x86/zero-page.txt b/Documentation/x86/zero-page.txt
index 95a4d34af3fd..b8527c6b7646 100644
--- a/Documentation/x86/zero-page.txt
+++ b/Documentation/x86/zero-page.txt
@@ -31,6 +31,8 @@ Offset Proto Name Meaning
1E9/001 ALL eddbuf_entries Number of entries in eddbuf (below)
1EA/001 ALL edd_mbr_sig_buf_entries Number of entries in edd_mbr_sig_buffer
(below)
+1EB/001 ALL kbd_status Numlock is enabled
+1EC/001 ALL secure_boot Secure boot is enabled in the firmware
1EF/001 ALL sentinel Used to detect broken bootloaders
290/040 ALL edd_mbr_sig_buffer EDD MBR signatures
2D0/A00 ALL e820_map E820 memory map table
diff --git a/arch/x86/boot/compressed/eboot.c b/arch/x86/boot/compressed/eboot.c
index c8c32ebcdfdb..5b151c262ac2 100644
--- a/arch/x86/boot/compressed/eboot.c
+++ b/arch/x86/boot/compressed/eboot.c
@@ -1158,6 +1158,8 @@ struct boot_params *efi_main(struct efi_config *c,
else
setup_boot_services32(efi_early);
+ boot_params->secure_boot = efi_get_secureboot(sys_table);
+
setup_graphics(boot_params);
setup_efi_pci(boot_params);
diff --git a/arch/x86/boot/compressed/head_32.S b/arch/x86/boot/compressed/head_32.S
index d85b9625e836..c635f7e32f5c 100644
--- a/arch/x86/boot/compressed/head_32.S
+++ b/arch/x86/boot/compressed/head_32.S
@@ -61,6 +61,7 @@
__HEAD
ENTRY(startup_32)
+ movb $0, BP_secure_boot(%esi)
#ifdef CONFIG_EFI_STUB
jmp preferred_addr
diff --git a/arch/x86/boot/compressed/head_64.S b/arch/x86/boot/compressed/head_64.S
index beab8322f72a..ccd2c7461b7f 100644
--- a/arch/x86/boot/compressed/head_64.S
+++ b/arch/x86/boot/compressed/head_64.S
@@ -244,6 +244,7 @@ ENTRY(startup_64)
* that maps our entire kernel(text+data+bss+brk), zero page
* and command line.
*/
+ movb $0, BP_secure_boot(%rsi)
#ifdef CONFIG_EFI_STUB
/*
* The entry point for the PE/COFF executable is efi_pe_entry, so
diff --git a/arch/x86/include/asm/bootparam_utils.h b/arch/x86/include/asm/bootparam_utils.h
index 4a8cb8d7cbd5..7e16d53ff6a3 100644
--- a/arch/x86/include/asm/bootparam_utils.h
+++ b/arch/x86/include/asm/bootparam_utils.h
@@ -38,9 +38,10 @@ static void sanitize_boot_params(struct boot_params *boot_params)
memset(&boot_params->ext_ramdisk_image, 0,
(char *)&boot_params->efi_info -
(char *)&boot_params->ext_ramdisk_image);
- memset(&boot_params->kbd_status, 0,
+ boot_params->kbd_status = 0;
+ memset(&boot_params->_pad5, 0,
(char *)&boot_params->hdr -
- (char *)&boot_params->kbd_status);
+ (char *)&boot_params->_pad5);
memset(&boot_params->_pad7[0], 0,
(char *)&boot_params->edd_mbr_sig_buffer[0] -
(char *)&boot_params->_pad7[0]);
diff --git a/arch/x86/include/uapi/asm/bootparam.h b/arch/x86/include/uapi/asm/bootparam.h
index b10bf319ed20..5138dacf8bb8 100644
--- a/arch/x86/include/uapi/asm/bootparam.h
+++ b/arch/x86/include/uapi/asm/bootparam.h
@@ -135,7 +135,8 @@ struct boot_params {
__u8 eddbuf_entries; /* 0x1e9 */
__u8 edd_mbr_sig_buf_entries; /* 0x1ea */
__u8 kbd_status; /* 0x1eb */
- __u8 _pad5[3]; /* 0x1ec */
+ __u8 secure_boot; /* 0x1ec */
+ __u8 _pad5[2]; /* 0x1ed */
/*
* The sentinel is set to a nonzero value (0xff) in header.S.
*
diff --git a/arch/x86/kernel/asm-offsets.c b/arch/x86/kernel/asm-offsets.c
index c62e015b126c..de827d6ac8c2 100644
--- a/arch/x86/kernel/asm-offsets.c
+++ b/arch/x86/kernel/asm-offsets.c
@@ -81,6 +81,7 @@ void common(void) {
BLANK();
OFFSET(BP_scratch, boot_params, scratch);
+ OFFSET(BP_secure_boot, boot_params, secure_boot);
OFFSET(BP_loadflags, boot_params, hdr.loadflags);
OFFSET(BP_hardware_subarch, boot_params, hdr.hardware_subarch);
OFFSET(BP_version, boot_params, hdr.version);
diff --git a/drivers/firmware/efi/libstub/Makefile b/drivers/firmware/efi/libstub/Makefile
index 6621b13c370f..9af966863612 100644
--- a/drivers/firmware/efi/libstub/Makefile
+++ b/drivers/firmware/efi/libstub/Makefile
@@ -28,7 +28,7 @@ OBJECT_FILES_NON_STANDARD := y
# Prevents link failures: __sanitizer_cov_trace_pc() is not linked in.
KCOV_INSTRUMENT := n
-lib-y := efi-stub-helper.o gop.o
+lib-y := efi-stub-helper.o gop.o secureboot.o
# include the stub's generic dependencies from lib/ when building for ARM/arm64
arm-deps := fdt_rw.c fdt_ro.c fdt_wip.c fdt.c fdt_empty_tree.c fdt_sw.c sort.c
diff --git a/drivers/firmware/efi/libstub/arm-stub.c b/drivers/firmware/efi/libstub/arm-stub.c
index b4f7d78f9e8b..06d503419071 100644
--- a/drivers/firmware/efi/libstub/arm-stub.c
+++ b/drivers/firmware/efi/libstub/arm-stub.c
@@ -20,52 +20,6 @@
bool __nokaslr;
-static int efi_get_secureboot(efi_system_table_t *sys_table_arg)
-{
- static efi_char16_t const sb_var_name[] = {
- 'S', 'e', 'c', 'u', 'r', 'e', 'B', 'o', 'o', 't', 0 };
- static efi_char16_t const sm_var_name[] = {
- 'S', 'e', 't', 'u', 'p', 'M', 'o', 'd', 'e', 0 };
-
- efi_guid_t var_guid = EFI_GLOBAL_VARIABLE_GUID;
- efi_get_variable_t *f_getvar = sys_table_arg->runtime->get_variable;
- u8 val;
- unsigned long size = sizeof(val);
- efi_status_t status;
-
- status = f_getvar((efi_char16_t *)sb_var_name, (efi_guid_t *)&var_guid,
- NULL, &size, &val);
-
- if (status != EFI_SUCCESS)
- goto out_efi_err;
-
- if (val == 0)
- return 0;
-
- status = f_getvar((efi_char16_t *)sm_var_name, (efi_guid_t *)&var_guid,
- NULL, &size, &val);
-
- if (status != EFI_SUCCESS)
- goto out_efi_err;
-
- if (val == 1)
- return 0;
-
- return 1;
-
-out_efi_err:
- switch (status) {
- case EFI_NOT_FOUND:
- return 0;
- case EFI_DEVICE_ERROR:
- return -EIO;
- case EFI_SECURITY_VIOLATION:
- return -EACCES;
- default:
- return -EINVAL;
- }
-}
-
efi_status_t efi_open_volume(efi_system_table_t *sys_table_arg,
void *__image, void **__fh)
{
@@ -226,7 +180,7 @@ unsigned long efi_entry(void *handle, efi_system_table_t *sys_table,
efi_guid_t loaded_image_proto = LOADED_IMAGE_PROTOCOL_GUID;
unsigned long reserve_addr = 0;
unsigned long reserve_size = 0;
- int secure_boot = 0;
+ enum efi_secureboot_mode secure_boot = efi_secureboot_mode_unknown;
struct screen_info *si;
/* Check if we were booted by the EFI firmware */
@@ -296,19 +250,13 @@ unsigned long efi_entry(void *handle, efi_system_table_t *sys_table,
pr_efi_err(sys_table, "Failed to parse EFI cmdline options\n");
secure_boot = efi_get_secureboot(sys_table);
- if (secure_boot > 0)
- pr_efi(sys_table, "UEFI Secure Boot is enabled.\n");
-
- if (secure_boot < 0) {
- pr_efi_err(sys_table,
- "could not determine UEFI Secure Boot status.\n");
- }
/*
* Unauthenticated device tree data is a security hazard, so
* ignore 'dtb=' unless UEFI Secure Boot is disabled.
*/
- if (secure_boot != 0 && strstr(cmdline_ptr, "dtb=")) {
+ if (secure_boot != efi_secureboot_mode_disabled &&
+ strstr(cmdline_ptr, "dtb=")) {
pr_efi(sys_table, "Ignoring DTB from command line.\n");
} else {
status = handle_cmdline_files(sys_table, image, cmdline_ptr,
diff --git a/drivers/firmware/efi/libstub/secureboot.c b/drivers/firmware/efi/libstub/secureboot.c
new file mode 100644
index 000000000000..70e2a36577d4
--- /dev/null
+++ b/drivers/firmware/efi/libstub/secureboot.c
@@ -0,0 +1,66 @@
+/*
+ * Secure boot handling.
+ *
+ * Copyright (C) 2013,2014 Linaro Limited
+ * Roy Franz <roy.franz at linaro.org
+ * Copyright (C) 2013 Red Hat, Inc.
+ * Mark Salter <msalter@redhat.com>
+ *
+ * This file is part of the Linux kernel, and is made available under the
+ * terms of the GNU General Public License version 2.
+ *
+ */
+
+#include <linux/efi.h>
+#include <asm/efi.h>
+
+/* BIOS variables */
+static const efi_guid_t efi_variable_guid = EFI_GLOBAL_VARIABLE_GUID;
+static const efi_char16_t const efi_SecureBoot_name[] = {
+ 'S', 'e', 'c', 'u', 'r', 'e', 'B', 'o', 'o', 't', 0
+};
+static const efi_char16_t const efi_SetupMode_name[] = {
+ 'S', 'e', 't', 'u', 'p', 'M', 'o', 'd', 'e', 0
+};
+
+#define get_efi_var(name, vendor, ...) \
+ efi_call_runtime(get_variable, \
+ (efi_char16_t *)(name), (efi_guid_t *)(vendor), \
+ __VA_ARGS__);
+
+/*
+ * Determine whether we're in secure boot mode. We return:
+ */
+enum efi_secureboot_mode efi_get_secureboot(efi_system_table_t *sys_table_arg)
+{
+ u8 secboot, setupmode;
+ unsigned long size;
+ efi_status_t status;
+
+ size = sizeof(secboot);
+ status = get_efi_var(efi_SecureBoot_name, &efi_variable_guid,
+ NULL, &size, &secboot);
+ if (status != EFI_SUCCESS)
+ goto out_efi_err;
+
+ size = sizeof(setupmode);
+ status = get_efi_var(efi_SetupMode_name, &efi_variable_guid,
+ NULL, &size, &setupmode);
+ if (status != EFI_SUCCESS)
+ goto out_efi_err;
+
+ if (secboot == 0 || setupmode == 1)
+ goto secure_boot_disabled;
+
+ pr_efi(sys_table_arg, "UEFI Secure Boot is enabled.\n");
+ return efi_secureboot_mode_enabled;
+
+secure_boot_disabled:
+ return efi_secureboot_mode_disabled;
+
+out_efi_err:
+ pr_efi_err(sys_table_arg, "Could not determine UEFI Secure Boot status.\n");
+ if (status == EFI_NOT_FOUND)
+ goto secure_boot_disabled;
+ return efi_secureboot_mode_unknown;
+}
diff --git a/include/linux/efi.h b/include/linux/efi.h
index c7904556d7a8..92e23f03045e 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -1477,6 +1477,14 @@ efi_status_t efi_setup_gop(efi_system_table_t *sys_table_arg,
bool efi_runtime_disabled(void);
extern void efi_call_virt_check_flags(unsigned long flags, const char *call);
+enum efi_secureboot_mode {
+ efi_secureboot_mode_unset,
+ efi_secureboot_mode_unknown,
+ efi_secureboot_mode_disabled,
+ efi_secureboot_mode_enabled,
+};
+enum efi_secureboot_mode efi_get_secureboot(efi_system_table_t *sys_table);
+
/*
* Arch code can implement the following three template macros, avoiding
* reptition for the void/non-void return cases of {__,}efi_call_virt():
^ permalink raw reply related
* [PATCH 6/8] efi: Disable secure boot if shim is in insecure mode [ver #5]
From: David Howells @ 2016-12-07 13:18 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <148111668193.23390.6340512985876251017.stgit@warthog.procyon.org.uk>
From: Josh Boyer <jwboyer@fedoraproject.org>
A user can manually tell the shim boot loader to disable validation of
images it loads. When a user does this, it creates a UEFI variable called
MokSBState that does not have the runtime attribute set. Given that the
user explicitly disabled validation, we can honor that and not enable
secure boot mode if that variable is set.
Signed-off-by: Josh Boyer <jwboyer@fedoraproject.org>
Signed-off-by: David Howells <dhowells@redhat.com>
---
drivers/firmware/efi/libstub/secureboot.c | 24 +++++++++++++++++++++++-
1 file changed, 23 insertions(+), 1 deletion(-)
diff --git a/drivers/firmware/efi/libstub/secureboot.c b/drivers/firmware/efi/libstub/secureboot.c
index 70e2a36577d4..ba6ef717c66f 100644
--- a/drivers/firmware/efi/libstub/secureboot.c
+++ b/drivers/firmware/efi/libstub/secureboot.c
@@ -23,6 +23,12 @@ static const efi_char16_t const efi_SetupMode_name[] = {
'S', 'e', 't', 'u', 'p', 'M', 'o', 'd', 'e', 0
};
+/* SHIM variables */
+static const efi_guid_t shim_guid = EFI_SHIM_LOCK_GUID;
+static efi_char16_t const shim_MokSBState_name[] = {
+ 'M', 'o', 'k', 'S', 'B', 'S', 't', 'a', 't', 'e', 0
+};
+
#define get_efi_var(name, vendor, ...) \
efi_call_runtime(get_variable, \
(efi_char16_t *)(name), (efi_guid_t *)(vendor), \
@@ -33,7 +39,8 @@ static const efi_char16_t const efi_SetupMode_name[] = {
*/
enum efi_secureboot_mode efi_get_secureboot(efi_system_table_t *sys_table_arg)
{
- u8 secboot, setupmode;
+ u32 attr;
+ u8 secboot, setupmode, moksbstate;
unsigned long size;
efi_status_t status;
@@ -52,6 +59,21 @@ enum efi_secureboot_mode efi_get_secureboot(efi_system_table_t *sys_table_arg)
if (secboot == 0 || setupmode == 1)
goto secure_boot_disabled;
+ /* See if a user has put shim into insecure mode. If so, and if the
+ * variable doesn't have the runtime attribute set, we might as well
+ * honor that.
+ */
+ size = sizeof(moksbstate);
+ status = get_efi_var(shim_MokSBState_name, &shim_guid,
+ &attr, &size, &moksbstate);
+
+ /* If it fails, we don't care why. Default to secure */
+ if (status != EFI_SUCCESS)
+ goto secure_boot_enabled;
+ if (!(attr & EFI_VARIABLE_RUNTIME_ACCESS) && moksbstate == 1)
+ goto secure_boot_disabled;
+
+secure_boot_enabled:
pr_efi(sys_table_arg, "UEFI Secure Boot is enabled.\n");
return efi_secureboot_mode_enabled;
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox