* [PATCHv3 16/19] arm64: use {COMPAT, }SYSCALL_DEFINE0 for sigreturn
From: Catalin Marinas @ 2018-06-19 15:48 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180618120310.39527-17-mark.rutland@arm.com>
On Mon, Jun 18, 2018 at 01:03:07PM +0100, Mark Rutland wrote:
> We don't currently annotate our various sigreturn functions as syscalls,
> as we need to do to use pt_regs syscall wrappers.
>
> Let's mark them as real syscalls.
>
> For compat_sys_sigreturn and compat_sys_rt_sigreturn, this changes the
> return type from int to long, matching the prototypes in sys32.c.
>
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> Reviewed-by: Dominik Brodowski <linux@dominikbrodowski.net>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will.deacon@arm.com>
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
^ permalink raw reply
* [PATCHv3 15/19] arm64: remove in-kernel call to sys_personality()
From: Catalin Marinas @ 2018-06-19 15:46 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180618120310.39527-16-mark.rutland@arm.com>
On Mon, Jun 18, 2018 at 01:03:06PM +0100, Mark Rutland wrote:
> With pt_regs syscall wrappers, the calling convention for
> sys_personality() will change. Use ksys_personality(), which is
> functionally equivalent.
>
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will.deacon@arm.com>
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
^ permalink raw reply
* [PATCHv3 13/19] kernel: add ksys_personality()
From: Catalin Marinas @ 2018-06-19 15:45 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180618120310.39527-14-mark.rutland@arm.com>
On Mon, Jun 18, 2018 at 01:03:04PM +0100, Mark Rutland wrote:
> diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
> index 73810808cdf2..14312d334345 100644
> --- a/include/linux/syscalls.h
> +++ b/include/linux/syscalls.h
> @@ -80,6 +80,7 @@ union bpf_attr;
> #include <linux/unistd.h>
> #include <linux/quota.h>
> #include <linux/key.h>
> +#include <linux/personality.h>
> #include <trace/syscall.h>
>
> #ifdef CONFIG_ARCH_HAS_SYSCALL_WRAPPER
> @@ -1277,4 +1278,14 @@ static inline long ksys_truncate(const char __user *pathname, loff_t length)
> return do_sys_truncate(pathname, length);
> }
>
> +static inline unsigned int ksys_personality(unsigned int personality)
> +{
> + unsigned int old = current->personality;
> +
> + if (personality != 0xffffffff)
> + set_personality(personality);
> +
> + return old;
> +}
> +
> #endif
It looks like this is identical to sys_personality() in
kernel/exec_domain.c. Should you call ksys_personality() from there
directly (a few lines saved)?
--
Catalin
^ permalink raw reply
* [PATCH] arm64: mm: mark tramp_pg_dir read-only
From: Ard Biesheuvel @ 2018-06-19 15:40 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180619153754.GO13984@arm.com>
On 19 June 2018 at 17:37, Will Deacon <will.deacon@arm.com> wrote:
> On Tue, Jun 19, 2018 at 05:29:03PM +0200, Ard Biesheuvel wrote:
>> On 19 June 2018 at 17:28, Will Deacon <will.deacon@arm.com> wrote:
>> > On Tue, Jun 19, 2018 at 05:23:41PM +0200, Ard Biesheuvel wrote:
>> >> On 19 June 2018 at 17:20, Will Deacon <will.deacon@arm.com> wrote:
>> >> > Hi Ard,
>> >> >
>> >> > Sorry, I forgot to reply to this.
>> >> >
>> >> > On Wed, May 30, 2018 at 11:53:20AM +0200, Ard Biesheuvel wrote:
>> >> >> On 30 May 2018 at 11:14, Will Deacon <will.deacon@arm.com> wrote:
>> >> >> > On Wed, May 30, 2018 at 12:48:06PM +0800, YaoJun wrote:
>> >> >> >> To protect against KSMA(Kernel Space Mirroring Attack), make
>> >> >> >> tramp_pg_dir read-only. The principle of KSMA is to insert a
>> >> >> >> carefully constructed PGD entry into the translation table.
>> >> >> >> The type of this entry is block, which maps the kernel text
>> >> >> >> and its access permissions bits are 01. The user process can
>> >> >> >> then modify kernel text directly through this mapping. In this
>> >> >> >> way, an arbitrary write can be converted to multiple arbitrary
>> >> >> >> writes.
>> >> >> >>
>> >> >> >> Signed-off-by: YaoJun <yaojun8558363@gmail.com>
>> >> >> >> ---
>> >> >> >> arch/arm64/mm/mmu.c | 4 ++++
>> >> >> >> 1 file changed, 4 insertions(+)
>> >> >> >>
>> >> >> >> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
>> >> >> >> index 2dbb2c9f1ec1..ac4b22c7e435 100644
>> >> >> >> --- a/arch/arm64/mm/mmu.c
>> >> >> >> +++ b/arch/arm64/mm/mmu.c
>> >> >> >> @@ -551,6 +551,10 @@ static int __init map_entry_trampoline(void)
>> >> >> >> __create_pgd_mapping(tramp_pg_dir, pa_start, TRAMP_VALIAS, PAGE_SIZE,
>> >> >> >> prot, pgd_pgtable_alloc, 0);
>> >> >> >>
>> >> >> >> + update_mapping_prot(__pa_symbol(tramp_pg_dir),
>> >> >> >> + (unsigned long)tramp_pg_dir,
>> >> >> >> + PGD_SIZE, PAGE_KERNEL_RO);
>> >> >> >
>> >> >> > Hmm, I like the idea but is there a risk that the page table has been mapped
>> >> >> > as part of a block entry, which we can't safely split at this point (i.e.
>> >> >> > we'll run into one of the BUG_ONs in the mapping code)?
>> >> >> >
>> >> >>
>> >> >> We'd need to create a separate segment for it initially so the mapping
>> >> >> is already at the right granularity.
>> >> >
>> >> > Why do you think that's the case? I can't see anything that guarantees this
>> >> > for the page table itself.
>> >> >
>> >>
>> >> We'd need to pass NO_BLOCK_MAPPINGS to map_kernel_segment(),
>> >> obviously, but that shouldn't hurt since that segment is relatively
>> >> tiny anyway.
>> >
>> > Ah right, with NO_BLOCK_MAPPINGS | NO_CONT_MAPPINGS, I agree that we're good.
>> > Ideally, we'd move {idmap,swapper,tramp}_pg_dir into .rodata...
>> >
>>
>> idmap and tramp yes, but swapper needs to be modifiable at runtime, no?
>
> Right, but couldn't we swizzle the permissions in e.g. set_pmd? We could
> even predicate that on a sanity check of the prot.
>
Swizzle the permissions of the entire .rodata segment? That sounds
doable, but there is a whole class of data that belongs in this
category, and I think PaX/grsecurity had an API for that (but I don't
think anyone is upstreaming that yet). So let's not reinvent that
wheel for swapper_pg_dir only.
^ permalink raw reply
* [PATCHv3 00/19] arm64: invoke syscalls with pt_regs
From: Mark Rutland @ 2018-06-19 15:38 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180618120310.39527-1-mark.rutland@arm.com>
On Mon, Jun 18, 2018 at 01:02:51PM +0100, Mark Rutland wrote:
> This series reworks arm64's syscall handling to minimize the propagation
> of user-controlled register values into speculated code paths. As with
> x86 [1], a wrapper is generated for each syscall, which extracts the
> argument from a struct pt_regs. During kernel entry from userspace,
> registers are zeroed.
>
> The arm64 kernel code directly invokes some syscalls which the x86 code
> doesn't, so I've added ksys_* wrappers for these, following the x86
> example. The rest of the series is arm64-specific.
>
> I've pushed the series out to my arm64/syscall-regs branch [2] on
> kernel.org.
>
> Since v1 [3]:
> * Rebase atop of arm64 for-next/core (for SSBD conflicts)
> * Move ksys_personality logic into <linux/syscalls.h>
> * Move kcompat_sys_* wrappers to <linux/compat.h>
> * Fix scno bounds check to use unsigned comparison
> * Fix sve_user_reset() call in el0_svc_handler()
> * Add BUILD_BUG() to the !CONFIG_ARM64_SVE stubs
> * Accumulate acked-by / reviewed-by tags
>
> Since v2 [4]:
> * Rebase to v4.18-rc1
> * Take upper 32 RES0 bits of SCTLR_ELx into account
> * Accumulate acks
>
> Thanks,
> Mark.
>
> [1] https://lkml.kernel.org/r/20180330093720.6780-1-linux at dominikbrodowski.net
> [2] git://git.kernel.org/pub/scm/linux/kernel/git/mark/linux.git
> [3] https://lkml.kernel.org/r/20180514094640.27569-1-mark.rutland at arm.com
> [4] https://lkml.kernel.org/r/20180601112441.37810-1-mark.rutland at arm.com
>
> Mark Rutland (19):
> arm64: consistently use unsigned long for thread flags
> arm64: move SCTLR_EL{1,2} assertions to <asm/sysreg.h>
> arm64: introduce sysreg_clear_set()
> arm64: kill config_sctlr_el1()
> arm64: kill change_cpacr()
> arm64: move sve_user_{enable,disable} to <asm/fpsimd.h>
> arm64: remove sigreturn wrappers
> arm64: convert raw syscall invocation to C
> arm64: convert syscall trace logic to C
> arm64: convert native/compat syscall entry to C
> arm64: don't reload GPRs after apply_ssbd
> arm64: zero GPRs upon entry from EL0
I realised that along with these two, we also no longer need to reload
the GPRs for entry tracking, so I've added the following to my local
copy of the series.
Mark.
---->8----
>From b47791fe74fce8bb48696ab0cdda48a9bae0e397 Mon Sep 17 00:00:00 2001
From: Mark Rutland <mark.rutland@arm.com>
Date: Tue, 19 Jun 2018 12:49:54 +0100
Subject: [PATCH] arm64: don't restore GPRs when context tracking
Now that syscalls are invoked with pt_regs, we no longer need to ensure
that the argument regsiters are live in the entry assembly, and it's
fine to not restore them after context_tracking_user_exit() has
corrupted them.
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
---
arch/arm64/kernel/entry.S | 12 +-----------
1 file changed, 1 insertion(+), 11 deletions(-)
diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
index 39440c2ee66d..6b7789d72064 100644
--- a/arch/arm64/kernel/entry.S
+++ b/arch/arm64/kernel/entry.S
@@ -41,19 +41,9 @@
* Context tracking subsystem. Used to instrument transitions
* between user and kernel mode.
*/
- .macro ct_user_exit, syscall = 0
+ .macro ct_user_exit
#ifdef CONFIG_CONTEXT_TRACKING
bl context_tracking_user_exit
- .if \syscall == 1
- /*
- * Save/restore needed during syscalls. Restore syscall arguments from
- * the values already saved on stack during kernel_entry.
- */
- ldp x0, x1, [sp]
- ldp x2, x3, [sp, #S_X2]
- ldp x4, x5, [sp, #S_X4]
- ldp x6, x7, [sp, #S_X6]
- .endif
#endif
.endm
--
2.11.0
^ permalink raw reply related
* [PATCH] arm64: mm: mark tramp_pg_dir read-only
From: Will Deacon @ 2018-06-19 15:37 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAKv+Gu81zgvcLd-N_MMENykxcm6ENvvfCuWOoMGsrfUf9Usy5g@mail.gmail.com>
On Tue, Jun 19, 2018 at 05:29:03PM +0200, Ard Biesheuvel wrote:
> On 19 June 2018 at 17:28, Will Deacon <will.deacon@arm.com> wrote:
> > On Tue, Jun 19, 2018 at 05:23:41PM +0200, Ard Biesheuvel wrote:
> >> On 19 June 2018 at 17:20, Will Deacon <will.deacon@arm.com> wrote:
> >> > Hi Ard,
> >> >
> >> > Sorry, I forgot to reply to this.
> >> >
> >> > On Wed, May 30, 2018 at 11:53:20AM +0200, Ard Biesheuvel wrote:
> >> >> On 30 May 2018 at 11:14, Will Deacon <will.deacon@arm.com> wrote:
> >> >> > On Wed, May 30, 2018 at 12:48:06PM +0800, YaoJun wrote:
> >> >> >> To protect against KSMA(Kernel Space Mirroring Attack), make
> >> >> >> tramp_pg_dir read-only. The principle of KSMA is to insert a
> >> >> >> carefully constructed PGD entry into the translation table.
> >> >> >> The type of this entry is block, which maps the kernel text
> >> >> >> and its access permissions bits are 01. The user process can
> >> >> >> then modify kernel text directly through this mapping. In this
> >> >> >> way, an arbitrary write can be converted to multiple arbitrary
> >> >> >> writes.
> >> >> >>
> >> >> >> Signed-off-by: YaoJun <yaojun8558363@gmail.com>
> >> >> >> ---
> >> >> >> arch/arm64/mm/mmu.c | 4 ++++
> >> >> >> 1 file changed, 4 insertions(+)
> >> >> >>
> >> >> >> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
> >> >> >> index 2dbb2c9f1ec1..ac4b22c7e435 100644
> >> >> >> --- a/arch/arm64/mm/mmu.c
> >> >> >> +++ b/arch/arm64/mm/mmu.c
> >> >> >> @@ -551,6 +551,10 @@ static int __init map_entry_trampoline(void)
> >> >> >> __create_pgd_mapping(tramp_pg_dir, pa_start, TRAMP_VALIAS, PAGE_SIZE,
> >> >> >> prot, pgd_pgtable_alloc, 0);
> >> >> >>
> >> >> >> + update_mapping_prot(__pa_symbol(tramp_pg_dir),
> >> >> >> + (unsigned long)tramp_pg_dir,
> >> >> >> + PGD_SIZE, PAGE_KERNEL_RO);
> >> >> >
> >> >> > Hmm, I like the idea but is there a risk that the page table has been mapped
> >> >> > as part of a block entry, which we can't safely split at this point (i.e.
> >> >> > we'll run into one of the BUG_ONs in the mapping code)?
> >> >> >
> >> >>
> >> >> We'd need to create a separate segment for it initially so the mapping
> >> >> is already at the right granularity.
> >> >
> >> > Why do you think that's the case? I can't see anything that guarantees this
> >> > for the page table itself.
> >> >
> >>
> >> We'd need to pass NO_BLOCK_MAPPINGS to map_kernel_segment(),
> >> obviously, but that shouldn't hurt since that segment is relatively
> >> tiny anyway.
> >
> > Ah right, with NO_BLOCK_MAPPINGS | NO_CONT_MAPPINGS, I agree that we're good.
> > Ideally, we'd move {idmap,swapper,tramp}_pg_dir into .rodata...
> >
>
> idmap and tramp yes, but swapper needs to be modifiable at runtime, no?
Right, but couldn't we swizzle the permissions in e.g. set_pmd? We could
even predicate that on a sanity check of the prot.
Will
^ permalink raw reply
* [PATCH 1/2] arm64: avoid alloc memory on offline node
From: Punit Agrawal @ 2018-06-19 15:35 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180619151425.GH13685@dhcp22.suse.cz>
Michal Hocko <mhocko@kernel.org> writes:
> On Tue 19-06-18 15:54:26, Punit Agrawal wrote:
> [...]
>> In terms of $SUBJECT, I wonder if it's worth taking the original patch
>> as a temporary fix (it'll also be easier to backport) while we work on
>> fixing these other issues and enabling memoryless nodes.
>
> Well, x86 already does that but copying this antipatern is not really
> nice. So it is good as a quick fix but it would be definitely much
> better to have a robust fix. Who knows how many other places might hit
> this. You certainly do not want to add a hack like this all over...
Completely agree! I was only suggesting it as a temporary measure,
especially as it looked like a proper fix might be invasive.
Another fix might be to change the node specific allocation to node
agnostic allocations. It isn't clear why the allocation is being
requested from a specific node. I think Lorenzo suggested this in one of
the threads.
I've started putting together a set fixing the issues identified in this
thread. It should give a better idea on the best course of action.
^ permalink raw reply
* [Dev] [PATCH 0/5] RFC: Mezzanine handling for 96boards
From: Ard Biesheuvel @ 2018-06-19 15:30 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAMuUZTHOuzQm3y-yF6cad9y1UgsK+6R2w7vSzz7MkkYCdYd3hA@mail.gmail.com>
On 19 June 2018 at 17:26, Yang Zhang <yang.zhang@96boards.org> wrote:
> Yes.
>
> https://github.com/96boards/documentation/blob/master/mezzanine/files/mezzanine-design-guidelines.pdf
>
> Under Configuration data section
>
OK, fair enough. Do any such mezzanines exist yet? Should we offer
more guidance on how exactly this discovery should be implemented?
> On 19 June 2018 at 16:25, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>>
>> On 19 June 2018 at 17:14, Yang Zhang <yang.zhang@96boards.org> wrote:
>> >
>> >
>> > On 18 June 2018 at 14:22, Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> > wrote:
>> >>
>> >> On 18 June 2018 at 14:21, Arnd Bergmann <arnd@arndb.de> wrote:
>> >> > On Mon, Jun 18, 2018 at 9:45 AM, Linus Walleij
>> >> > <linus.walleij@linaro.org> wrote:
>> >> >> This is a proposal for how to handle the non-discoverable
>> >> >> 96boards plug-in expansion boards called "mezzanines" in the
>> >> >> Linux kernel. It is a working RFC series meant for discussion
>> >> >> at the moment.
>> >> >>
>> >> >> The RFC was done on the brand new Ultra96 board from Xilinx
>> >> >> with a Secure96 mezzanine expansion board. The main part
>> >> >> is in patch 4, the rest is enabling and examples.
>> >> >>
>> >> >> The code can be obtained from here:
>> >> >>
>> >> >>
>> >> >> https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-integrator.git/log/?h=ultra96
>> >> >>
>> >> >> You can for example probably augment the DTS file for any
>> >> >> upstream-supported 96board and get the Secure96 going with
>> >> >> it with minor efforts.
>> >> >
>> >> > Hi Linus,
>> >> >
>> >> > Thanks for your work on solving this long-standing problem. I've just
>> >> > read through your patches briefly and have a few thoughts:
>> >> >
>> >> > - I really like the idea of having C code deal with the mezzanine
>> >> > connector itself, acting as an intermediate to tie a number of
>> >> > boards to a number of add-on cards, this seems much simpler than
>> >> > trying to do everything with overlays or one of the other more
>> >> > generic mechanisms.
>> >> >
>> >> > - I don't like the idea of having the bus driver contain a list of
>> >> > possible
>> >> > add-ons, this seems to go against our usual driver model. What
>> >> > I think we want instead is to make the connector itself a proper
>> >> > bus_type, to allow drivers to register against it as loadable
>> >> > modules,
>> >> > and devices (maybe limited to one device) being created as probed
>> >> > from DT or some other method as you describe.
>> >> >
>> >> > - You export symbols in the mezzanine_* namespace, which I think
>> >> > is a bit too generic and should perhaps contain something related
>> >> > to 96boards in its name to make it less ambiguous. I suspect we
>> >> > would add a number of further connectors for hats, capes, lures
>> >> > etc,
>> >> > which could all be described as mezzanines. One open question
>> >> > is how we structure the commonality between the various
>> >> > connectors, but we can defer that until we have more than one
>> >> > or two of them.
>> >> >
>> >>
>> >> Hello all,
>> >>
>> >> We should also consider firmware use of the mezzanines. For instance,
>> >> the Secure96 has a RNG which UEFI may want to use so the early boot
>> >> code can access is for KASLR. It also has a TPM, which should not be
>> >> reset/reinitialized/etc by the OS if we want to make meaningful use of
>> >> it.
>> >>
>> >> Also, given that we can (and do) already describe topologies involving
>> >> mezzanines by ignoring the connector altogether (which is not entirely
>> >> unreasonable given the fact that we [as Linaro/96boards] dropped the
>> >> ball on this one and did not mandate discoverability for mezzanines).
>> >
>> >
>> > The design guideline has been reviewed by many inside/outside linaro
>> > through
>> > the mezzanine at lists.96boards.org and 96b-spec-sig
>> > <96b-spec-sig@96boards.org> published as recommended/strongly
>> > recommended
>> > item from day one. 'dropping the ball' is a strong conclusion.
>> >
>>
>> Apologies for using a term that you seem to take issue with. Are you
>> saying the spec currently recommends it?
>
>
^ permalink raw reply
* [PATCH v10 13/14] cpufreq: Add module to register cpufreq on Krait CPUs
From: Craig Tatlor @ 2018-06-19 15:29 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1529415925-28915-14-git-send-email-sricharan@codeaurora.org>
The pvs refuse check is incorrect... With downstream it says it isn't
blown and that it is 11, which also happens on upstream if I import it's
efuse reading code from an older revision, or comment out the check.
Also, I'm still getting my issue about clocks above 2,147,483,647hz
however I think this may be related to the division in the hfpll driver
so I'll have a debug around there.
On Tue, Jun 19, 2018 at 07:15:24PM +0530, Sricharan R wrote:
> From: Stephen Boyd <sboyd@codeaurora.org>
>
> Register a cpufreq-generic device whenever we detect that a
> "qcom,krait" compatible CPU is present in DT.
>
> Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
> [Sricharan: updated to use dev_pm_opp_set_prop_name and
> nvmem apis]
> Signed-off-by: Sricharan R <sricharan@codeaurora.org>
> [Thierry Escande: update to add support for opp-supported-hw]
> Signed-off-by: Thierry Escande <thierry.escande@linaro.org>
> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
> Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
> [v10] updated to add support for opp-supported-hw given by
> Thierry Escande <thierry.escande@linaro.org>
>
> drivers/cpufreq/Kconfig.arm | 10 ++
> drivers/cpufreq/Makefile | 1 +
> drivers/cpufreq/cpufreq-dt-platdev.c | 5 +
> drivers/cpufreq/qcom-cpufreq.c | 201 +++++++++++++++++++++++++++++++++++
> 4 files changed, 217 insertions(+)
> create mode 100644 drivers/cpufreq/qcom-cpufreq.c
>
> diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm
> index 7f56fe5..87e5d8d 100644
> --- a/drivers/cpufreq/Kconfig.arm
> +++ b/drivers/cpufreq/Kconfig.arm
> @@ -134,6 +134,16 @@ config ARM_OMAP2PLUS_CPUFREQ
> depends on ARCH_OMAP2PLUS
> default ARCH_OMAP2PLUS
>
> +config ARM_QCOM_CPUFREQ
> + bool "CPUfreq driver for the QCOM SoCs with KRAIT processors"
> + depends on ARCH_QCOM
> + select PM_OPP
> + help
> + This enables the CPUFreq driver for Qualcomm SoCs with
> + KRAIT processors.
> +
> + If in doubt, say N.
> +
> config ARM_S3C_CPUFREQ
> bool
> help
> diff --git a/drivers/cpufreq/Makefile b/drivers/cpufreq/Makefile
> index 8d24ade..c591e1e 100644
> --- a/drivers/cpufreq/Makefile
> +++ b/drivers/cpufreq/Makefile
> @@ -65,6 +65,7 @@ obj-$(CONFIG_MACH_MVEBU_V7) += mvebu-cpufreq.o
> obj-$(CONFIG_ARM_OMAP2PLUS_CPUFREQ) += omap-cpufreq.o
> obj-$(CONFIG_ARM_PXA2xx_CPUFREQ) += pxa2xx-cpufreq.o
> obj-$(CONFIG_PXA3xx) += pxa3xx-cpufreq.o
> +obj-$(CONFIG_ARM_QCOM_CPUFREQ) += qcom-cpufreq.o
> obj-$(CONFIG_ARM_S3C2410_CPUFREQ) += s3c2410-cpufreq.o
> obj-$(CONFIG_ARM_S3C2412_CPUFREQ) += s3c2412-cpufreq.o
> obj-$(CONFIG_ARM_S3C2416_CPUFREQ) += s3c2416-cpufreq.o
> diff --git a/drivers/cpufreq/cpufreq-dt-platdev.c b/drivers/cpufreq/cpufreq-dt-platdev.c
> index 3b585e4..e2e9a99 100644
> --- a/drivers/cpufreq/cpufreq-dt-platdev.c
> +++ b/drivers/cpufreq/cpufreq-dt-platdev.c
> @@ -127,6 +127,11 @@
> { .compatible = "ti,am43", },
> { .compatible = "ti,dra7", },
>
> + { .compatible = "qcom,ipq8064", },
> + { .compatible = "qcom,apq8064", },
> + { .compatible = "qcom,msm8974", },
> + { .compatible = "qcom,msm8960", },
> +
> { }
> };
>
> diff --git a/drivers/cpufreq/qcom-cpufreq.c b/drivers/cpufreq/qcom-cpufreq.c
> new file mode 100644
> index 0000000..1d4ab54
> --- /dev/null
> +++ b/drivers/cpufreq/qcom-cpufreq.c
> @@ -0,0 +1,201 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// Copyright (c) 2018, The Linux Foundation. All rights reserved.
> +
> +#include <linux/cpu.h>
> +#include <linux/module.h>
> +#include <linux/nvmem-consumer.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_opp.h>
> +#include <linux/slab.h>
> +
> +static void __init get_krait_bin_format_a(int *speed, int *pvs, int *pvs_ver,
> + struct nvmem_cell *pvs_nvmem, u8 *buf)
> +{
> + u32 pte_efuse;
> +
> + pte_efuse = *((u32 *)buf);
> +
> + *speed = pte_efuse & 0xf;
> + if (*speed == 0xf)
> + *speed = (pte_efuse >> 4) & 0xf;
> +
> + if (*speed == 0xf) {
> + *speed = 0;
> + pr_warn("Speed bin: Defaulting to %d\n", *speed);
> + } else {
> + pr_info("Speed bin: %d\n", *speed);
> + }
> +
> + *pvs = (pte_efuse >> 10) & 0x7;
> + if (*pvs == 0x7)
> + *pvs = (pte_efuse >> 13) & 0x7;
> +
> + if (*pvs == 0x7) {
> + *pvs = 0;
> + pr_warn("PVS bin: Defaulting to %d\n", *pvs);
> + } else {
> + pr_info("PVS bin: %d\n", *pvs);
> + }
> +
> + kfree(buf);
> +}
> +
> +static void __init get_krait_bin_format_b(int *speed, int *pvs, int *pvs_ver,
> + struct nvmem_cell *pvs_nvmem, u8 *buf)
> +{
> + u32 pte_efuse, redundant_sel;
> +
> + pte_efuse = *((u32 *)buf);
> + redundant_sel = (pte_efuse >> 24) & 0x7;
> + *speed = pte_efuse & 0x7;
> +
> + /* 4 bits of PVS are in efuse register bits 31, 8-6. */
> + *pvs = ((pte_efuse >> 28) & 0x8) | ((pte_efuse >> 6) & 0x7);
> + *pvs_ver = (pte_efuse >> 4) & 0x3;
> +
> + switch (redundant_sel) {
> + case 1:
> + *speed = (pte_efuse >> 27) & 0xf;
> + break;
> + case 2:
> + *pvs = (pte_efuse >> 27) & 0xf;
> + break;
> + }
> +
> + /* Check SPEED_BIN_BLOW_STATUS */
> + if (pte_efuse & BIT(3)) {
> + pr_info("Speed bin: %d\n", *speed);
> + } else {
> + pr_warn("Speed bin not set. Defaulting to 0!\n");
> + *speed = 0;
> + }
> +
> + /* Check PVS_BLOW_STATUS */
> + pte_efuse = *(((u32 *)buf) + 4);
> + if (pte_efuse) {
> + pr_info("PVS bin: %d\n", *pvs);
> + } else {
> + pr_warn("PVS bin not set. Defaulting to 0!\n");
> + *pvs = 0;
> + }
> +
> + pr_info("PVS version: %d\n", *pvs_ver);
> + kfree(buf);
> +}
> +
> +static int __init qcom_cpufreq_populate_opps(struct nvmem_cell *pvs_nvmem,
> + struct opp_table **tbl1,
> + struct opp_table **tbl2)
> +{
> + int speed = 0, pvs = 0, pvs_ver = 0, cpu, ret;
> + struct device *cpu_dev;
> + u8 *buf;
> + size_t len;
> + char pvs_name[] = "speedXX-pvsXX-vXX";
> + u32 hw_version;
> +
> + buf = nvmem_cell_read(pvs_nvmem, &len);
> + if (len == 4)
> + get_krait_bin_format_a(&speed, &pvs, &pvs_ver, pvs_nvmem, buf);
> + else if (len == 8)
> + get_krait_bin_format_b(&speed, &pvs, &pvs_ver, pvs_nvmem, buf);
> + else
> + pr_warn("Unable to read nvmem data. Defaulting to 0!\n");
> +
> + snprintf(pvs_name, sizeof(pvs_name), "speed%d-pvs%d-v%d",
> + speed, pvs, pvs_ver);
> +
> + hw_version = (1 << speed);
> +
> + for (cpu = 0; cpu < num_possible_cpus(); cpu++) {
> + cpu_dev = get_cpu_device(cpu);
> + if (!cpu_dev)
> + return -ENODEV;
> +
> + tbl1[cpu] = dev_pm_opp_set_prop_name(cpu_dev, pvs_name);
> + if (IS_ERR(tbl1[cpu])) {
> + ret = PTR_ERR(tbl1[cpu]);
> + tbl1[cpu] = 0;
> + pr_warn("failed to add OPP name %s\n", pvs_name);
> + return ret;
> + }
> +
> + tbl2[cpu] = dev_pm_opp_set_supported_hw(cpu_dev, &hw_version,
> + 1);
> + if (IS_ERR(tbl2[cpu])) {
> + ret = PTR_ERR(tbl2[cpu]);
> + tbl2[cpu] = 0;
> + pr_warn("failed to set supported hw version\n");
> + return ret;
> + }
> + }
> +
> + return 0;
> +}
> +
> +static int __init qcom_cpufreq_driver_init(void)
> +{
> + struct platform_device *pdev;
> + struct device *cpu_dev;
> + struct device_node *np;
> + struct nvmem_cell *pvs_nvmem;
> + struct opp_table *tbl1[NR_CPUS] = { NULL }, *tbl2[NR_CPUS] = { NULL };
> + int ret, cpu = 0;
> +
> + cpu_dev = get_cpu_device(0);
> + if (!cpu_dev)
> + return -ENODEV;
> +
> + np = dev_pm_opp_of_get_opp_desc_node(cpu_dev);
> + if (!np)
> + return -ENOENT;
> +
> + if (!of_device_is_compatible(np, "operating-points-v2-krait-cpu")) {
> + ret = -ENOENT;
> + goto free_np;
> + }
> +
> + pvs_nvmem = of_nvmem_cell_get(np, NULL);
> + if (IS_ERR(pvs_nvmem)) {
> + dev_err(cpu_dev, "Could not get nvmem cell\n");
> + ret = PTR_ERR(pvs_nvmem);
> + goto free_np;
> + }
> +
> + ret = qcom_cpufreq_populate_opps(pvs_nvmem, tbl1, tbl2);
> + if (ret)
> + goto free_opp_name;
> +
> + pdev = platform_device_register_simple("cpufreq-dt", -1, NULL, 0);
> + if (IS_ERR(pdev)) {
> + ret = PTR_ERR(pdev);
> + goto free_opp_name;
> + }
> +
> + of_node_put(np);
> +
> + return 0;
> +
> +free_opp_name:
> + while (tbl1[cpu]) {
> + dev_pm_opp_put_prop_name(tbl1[cpu]);
> + cpu++;
> + }
> +
> + cpu = 0;
> + while (tbl2[cpu]) {
> + dev_pm_opp_put_supported_hw(tbl2[cpu]);
> + cpu++;
> + }
> +
> +free_np:
> + of_node_put(np);
> +
> + return ret;
> +}
> +late_initcall(qcom_cpufreq_driver_init);
> +
> +MODULE_DESCRIPTION("Qualcomm CPUfreq driver");
> +MODULE_AUTHOR("Stephen Boyd <sboyd@codeaurora.org>");
> +MODULE_LICENSE("GPL v2");
> --
> 1.9.1
>
^ permalink raw reply
* [PATCH] clk: meson-gxbb: set fclk_div2 as CLK_IS_CRITICAL
From: Jerome Brunet @ 2018-06-19 15:29 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1528892771.2292.2.camel@baylibre.com>
On Wed, 2018-06-13 at 14:26 +0200, Jerome Brunet wrote:
> On Wed, 2018-06-13 at 14:20 +0200, Neil Armstrong wrote:
> > On Amlogic Meson GXBB & GXL platforms, the SCPI Cortex-M4 Co-Processor
> > seems to be dependent on the FCLK_DIV2 to be operationnal.
> >
> > The issue occured since v4.17-rc1 by freezing the kernel boot when
> > the 'schedutil' cpufreq governor was selected as default :
> >
> > [ 12.071837] scpi_protocol scpi: SCP Protocol 0.0 Firmware 0.0.0 version
> > domain-0 init dvfs: 4
> > [ 12.087757] hctosys: unable to open rtc device (rtc0)
> > [ 12.087907] cfg80211: Loading compiled-in X.509 certificates for regulatory database
> > [ 12.102241] cfg80211: Loaded X.509 cert 'sforshee: 00b28ddf47aef9cea7'
> >
> > But when disabling the MMC driver, the boot finished but cpufreq failed to
> > change the CPU frequency :
> >
> > [ 12.153045] cpufreq: __target_index: Failed to change cpu frequency: -5
> >
> > A bisect between v4.16 and v4.16-rc1 gave the 05f814402d61 commit to be
> > the first bad commit.
> > This commit added support for the missing clock gates before the fixed PLL
> > fixed dividers (FCLK_DIVx) and the clock framework basically disabled
> > all the unused fixed dividers, thus disabled a critical clock path for
> > the SCPI Co-Processor.
> >
> > This patch simply sets the FCLK_DIV2 gate as critical to ensure
> > nobody can disable it.
> >
> > Fixes: 05f814402d61 ("clk: meson: add fdiv clock gates")
> > Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
>
> Good catch !
> We'll probably have to check the axg family as well
SCPI is not enabled on AXG at this time. We'll deal with it when the time comes.
patch applied.
Thx
^ permalink raw reply
* [PATCH] arm64: mm: mark tramp_pg_dir read-only
From: Ard Biesheuvel @ 2018-06-19 15:29 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180619152805.GN13984@arm.com>
On 19 June 2018 at 17:28, Will Deacon <will.deacon@arm.com> wrote:
> On Tue, Jun 19, 2018 at 05:23:41PM +0200, Ard Biesheuvel wrote:
>> On 19 June 2018 at 17:20, Will Deacon <will.deacon@arm.com> wrote:
>> > Hi Ard,
>> >
>> > Sorry, I forgot to reply to this.
>> >
>> > On Wed, May 30, 2018 at 11:53:20AM +0200, Ard Biesheuvel wrote:
>> >> On 30 May 2018 at 11:14, Will Deacon <will.deacon@arm.com> wrote:
>> >> > On Wed, May 30, 2018 at 12:48:06PM +0800, YaoJun wrote:
>> >> >> To protect against KSMA(Kernel Space Mirroring Attack), make
>> >> >> tramp_pg_dir read-only. The principle of KSMA is to insert a
>> >> >> carefully constructed PGD entry into the translation table.
>> >> >> The type of this entry is block, which maps the kernel text
>> >> >> and its access permissions bits are 01. The user process can
>> >> >> then modify kernel text directly through this mapping. In this
>> >> >> way, an arbitrary write can be converted to multiple arbitrary
>> >> >> writes.
>> >> >>
>> >> >> Signed-off-by: YaoJun <yaojun8558363@gmail.com>
>> >> >> ---
>> >> >> arch/arm64/mm/mmu.c | 4 ++++
>> >> >> 1 file changed, 4 insertions(+)
>> >> >>
>> >> >> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
>> >> >> index 2dbb2c9f1ec1..ac4b22c7e435 100644
>> >> >> --- a/arch/arm64/mm/mmu.c
>> >> >> +++ b/arch/arm64/mm/mmu.c
>> >> >> @@ -551,6 +551,10 @@ static int __init map_entry_trampoline(void)
>> >> >> __create_pgd_mapping(tramp_pg_dir, pa_start, TRAMP_VALIAS, PAGE_SIZE,
>> >> >> prot, pgd_pgtable_alloc, 0);
>> >> >>
>> >> >> + update_mapping_prot(__pa_symbol(tramp_pg_dir),
>> >> >> + (unsigned long)tramp_pg_dir,
>> >> >> + PGD_SIZE, PAGE_KERNEL_RO);
>> >> >
>> >> > Hmm, I like the idea but is there a risk that the page table has been mapped
>> >> > as part of a block entry, which we can't safely split at this point (i.e.
>> >> > we'll run into one of the BUG_ONs in the mapping code)?
>> >> >
>> >>
>> >> We'd need to create a separate segment for it initially so the mapping
>> >> is already at the right granularity.
>> >
>> > Why do you think that's the case? I can't see anything that guarantees this
>> > for the page table itself.
>> >
>>
>> We'd need to pass NO_BLOCK_MAPPINGS to map_kernel_segment(),
>> obviously, but that shouldn't hurt since that segment is relatively
>> tiny anyway.
>
> Ah right, with NO_BLOCK_MAPPINGS | NO_CONT_MAPPINGS, I agree that we're good.
> Ideally, we'd move {idmap,swapper,tramp}_pg_dir into .rodata...
>
idmap and tramp yes, but swapper needs to be modifiable at runtime, no?
^ permalink raw reply
* [PATCH] arm64: mm: mark tramp_pg_dir read-only
From: Will Deacon @ 2018-06-19 15:28 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAKv+Gu-Mad64U63jjuKGjhg=h2wdoEKm2vywzATBYr-t_UY_bQ@mail.gmail.com>
On Tue, Jun 19, 2018 at 05:23:41PM +0200, Ard Biesheuvel wrote:
> On 19 June 2018 at 17:20, Will Deacon <will.deacon@arm.com> wrote:
> > Hi Ard,
> >
> > Sorry, I forgot to reply to this.
> >
> > On Wed, May 30, 2018 at 11:53:20AM +0200, Ard Biesheuvel wrote:
> >> On 30 May 2018 at 11:14, Will Deacon <will.deacon@arm.com> wrote:
> >> > On Wed, May 30, 2018 at 12:48:06PM +0800, YaoJun wrote:
> >> >> To protect against KSMA(Kernel Space Mirroring Attack), make
> >> >> tramp_pg_dir read-only. The principle of KSMA is to insert a
> >> >> carefully constructed PGD entry into the translation table.
> >> >> The type of this entry is block, which maps the kernel text
> >> >> and its access permissions bits are 01. The user process can
> >> >> then modify kernel text directly through this mapping. In this
> >> >> way, an arbitrary write can be converted to multiple arbitrary
> >> >> writes.
> >> >>
> >> >> Signed-off-by: YaoJun <yaojun8558363@gmail.com>
> >> >> ---
> >> >> arch/arm64/mm/mmu.c | 4 ++++
> >> >> 1 file changed, 4 insertions(+)
> >> >>
> >> >> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
> >> >> index 2dbb2c9f1ec1..ac4b22c7e435 100644
> >> >> --- a/arch/arm64/mm/mmu.c
> >> >> +++ b/arch/arm64/mm/mmu.c
> >> >> @@ -551,6 +551,10 @@ static int __init map_entry_trampoline(void)
> >> >> __create_pgd_mapping(tramp_pg_dir, pa_start, TRAMP_VALIAS, PAGE_SIZE,
> >> >> prot, pgd_pgtable_alloc, 0);
> >> >>
> >> >> + update_mapping_prot(__pa_symbol(tramp_pg_dir),
> >> >> + (unsigned long)tramp_pg_dir,
> >> >> + PGD_SIZE, PAGE_KERNEL_RO);
> >> >
> >> > Hmm, I like the idea but is there a risk that the page table has been mapped
> >> > as part of a block entry, which we can't safely split at this point (i.e.
> >> > we'll run into one of the BUG_ONs in the mapping code)?
> >> >
> >>
> >> We'd need to create a separate segment for it initially so the mapping
> >> is already at the right granularity.
> >
> > Why do you think that's the case? I can't see anything that guarantees this
> > for the page table itself.
> >
>
> We'd need to pass NO_BLOCK_MAPPINGS to map_kernel_segment(),
> obviously, but that shouldn't hurt since that segment is relatively
> tiny anyway.
Ah right, with NO_BLOCK_MAPPINGS | NO_CONT_MAPPINGS, I agree that we're good.
Ideally, we'd move {idmap,swapper,tramp}_pg_dir into .rodata...
Will
^ permalink raw reply
* [Dev] [PATCH 0/5] RFC: Mezzanine handling for 96boards
From: Ard Biesheuvel @ 2018-06-19 15:25 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAMuUZTG0q4f2T=+HjEijUCLb7KNXok9RJHOXOKNs2iap5p7m5Q@mail.gmail.com>
On 19 June 2018 at 17:14, Yang Zhang <yang.zhang@96boards.org> wrote:
>
>
> On 18 June 2018 at 14:22, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>>
>> On 18 June 2018 at 14:21, Arnd Bergmann <arnd@arndb.de> wrote:
>> > On Mon, Jun 18, 2018 at 9:45 AM, Linus Walleij
>> > <linus.walleij@linaro.org> wrote:
>> >> This is a proposal for how to handle the non-discoverable
>> >> 96boards plug-in expansion boards called "mezzanines" in the
>> >> Linux kernel. It is a working RFC series meant for discussion
>> >> at the moment.
>> >>
>> >> The RFC was done on the brand new Ultra96 board from Xilinx
>> >> with a Secure96 mezzanine expansion board. The main part
>> >> is in patch 4, the rest is enabling and examples.
>> >>
>> >> The code can be obtained from here:
>> >>
>> >> https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-integrator.git/log/?h=ultra96
>> >>
>> >> You can for example probably augment the DTS file for any
>> >> upstream-supported 96board and get the Secure96 going with
>> >> it with minor efforts.
>> >
>> > Hi Linus,
>> >
>> > Thanks for your work on solving this long-standing problem. I've just
>> > read through your patches briefly and have a few thoughts:
>> >
>> > - I really like the idea of having C code deal with the mezzanine
>> > connector itself, acting as an intermediate to tie a number of
>> > boards to a number of add-on cards, this seems much simpler than
>> > trying to do everything with overlays or one of the other more
>> > generic mechanisms.
>> >
>> > - I don't like the idea of having the bus driver contain a list of
>> > possible
>> > add-ons, this seems to go against our usual driver model. What
>> > I think we want instead is to make the connector itself a proper
>> > bus_type, to allow drivers to register against it as loadable modules,
>> > and devices (maybe limited to one device) being created as probed
>> > from DT or some other method as you describe.
>> >
>> > - You export symbols in the mezzanine_* namespace, which I think
>> > is a bit too generic and should perhaps contain something related
>> > to 96boards in its name to make it less ambiguous. I suspect we
>> > would add a number of further connectors for hats, capes, lures etc,
>> > which could all be described as mezzanines. One open question
>> > is how we structure the commonality between the various
>> > connectors, but we can defer that until we have more than one
>> > or two of them.
>> >
>>
>> Hello all,
>>
>> We should also consider firmware use of the mezzanines. For instance,
>> the Secure96 has a RNG which UEFI may want to use so the early boot
>> code can access is for KASLR. It also has a TPM, which should not be
>> reset/reinitialized/etc by the OS if we want to make meaningful use of
>> it.
>>
>> Also, given that we can (and do) already describe topologies involving
>> mezzanines by ignoring the connector altogether (which is not entirely
>> unreasonable given the fact that we [as Linaro/96boards] dropped the
>> ball on this one and did not mandate discoverability for mezzanines).
>
>
> The design guideline has been reviewed by many inside/outside linaro through
> the mezzanine at lists.96boards.org and 96b-spec-sig
> <96b-spec-sig@96boards.org> published as recommended/strongly recommended
> item from day one. 'dropping the ball' is a strong conclusion.
>
Apologies for using a term that you seem to take issue with. Are you
saying the spec currently recommends it?
^ permalink raw reply
* [PATCHv3 12/19] arm64: zero GPRs upon entry from EL0
From: Catalin Marinas @ 2018-06-19 15:24 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180618120310.39527-13-mark.rutland@arm.com>
On Mon, Jun 18, 2018 at 01:03:03PM +0100, Mark Rutland wrote:
> We can zero GPRs x0 - x29 upon entry from EL0 to make it harder for
> userspace to control values consumed by speculative gadgets.
>
> We don't blat x30, since this is stashed much later, and we'll blat it
> before invoking C code.
>
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will.deacon@arm.com>
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
^ permalink raw reply
* [PATCH] arm64: mm: mark tramp_pg_dir read-only
From: Ard Biesheuvel @ 2018-06-19 15:23 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180619152035.GM13984@arm.com>
On 19 June 2018 at 17:20, Will Deacon <will.deacon@arm.com> wrote:
> Hi Ard,
>
> Sorry, I forgot to reply to this.
>
> On Wed, May 30, 2018 at 11:53:20AM +0200, Ard Biesheuvel wrote:
>> On 30 May 2018 at 11:14, Will Deacon <will.deacon@arm.com> wrote:
>> > On Wed, May 30, 2018 at 12:48:06PM +0800, YaoJun wrote:
>> >> To protect against KSMA(Kernel Space Mirroring Attack), make
>> >> tramp_pg_dir read-only. The principle of KSMA is to insert a
>> >> carefully constructed PGD entry into the translation table.
>> >> The type of this entry is block, which maps the kernel text
>> >> and its access permissions bits are 01. The user process can
>> >> then modify kernel text directly through this mapping. In this
>> >> way, an arbitrary write can be converted to multiple arbitrary
>> >> writes.
>> >>
>> >> Signed-off-by: YaoJun <yaojun8558363@gmail.com>
>> >> ---
>> >> arch/arm64/mm/mmu.c | 4 ++++
>> >> 1 file changed, 4 insertions(+)
>> >>
>> >> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
>> >> index 2dbb2c9f1ec1..ac4b22c7e435 100644
>> >> --- a/arch/arm64/mm/mmu.c
>> >> +++ b/arch/arm64/mm/mmu.c
>> >> @@ -551,6 +551,10 @@ static int __init map_entry_trampoline(void)
>> >> __create_pgd_mapping(tramp_pg_dir, pa_start, TRAMP_VALIAS, PAGE_SIZE,
>> >> prot, pgd_pgtable_alloc, 0);
>> >>
>> >> + update_mapping_prot(__pa_symbol(tramp_pg_dir),
>> >> + (unsigned long)tramp_pg_dir,
>> >> + PGD_SIZE, PAGE_KERNEL_RO);
>> >
>> > Hmm, I like the idea but is there a risk that the page table has been mapped
>> > as part of a block entry, which we can't safely split at this point (i.e.
>> > we'll run into one of the BUG_ONs in the mapping code)?
>> >
>>
>> We'd need to create a separate segment for it initially so the mapping
>> is already at the right granularity.
>
> Why do you think that's the case? I can't see anything that guarantees this
> for the page table itself.
>
We'd need to pass NO_BLOCK_MAPPINGS to map_kernel_segment(),
obviously, but that shouldn't hurt since that segment is relatively
tiny anyway.
^ permalink raw reply
* [PATCHv3 11/19] arm64: don't reload GPRs after apply_ssbd
From: Catalin Marinas @ 2018-06-19 15:23 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180618120310.39527-12-mark.rutland@arm.com>
On Mon, Jun 18, 2018 at 01:03:02PM +0100, Mark Rutland wrote:
> Now that all of the syscall logic works on the saved pt_regs, apply_ssbd
> can safely corrupt x0-x3 in the entry paths, and we no longer need to
> restore them. So let's remove the logic doing so.
>
> With that logic gone, we can fold the branch target into the macro, so
> that callers need not deal with this. GAS provides \@, which provides a
> unique value per macro invocation, which we can use to create a unique
> label.
>
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> Acked-by: Marc Zyngier <marc.zyngier@arm.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will.deacon@arm.com>
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
^ permalink raw reply
* [PATCH v2 1/4] arm64: export memblock_reserve()d regions via /proc/iomem
From: Dave Kleikamp @ 2018-06-19 15:22 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <ca9e5d9d-e090-f9e0-bc10-3297c9e3b7e1@arm.com>
On 06/19/2018 10:00 AM, James Morse wrote:
> Hi Dave,
>
> On 19/06/18 14:37, Dave Kleikamp wrote:
>> On 06/19/2018 01:44 AM, AKASHI Takahiro wrote:
>>> +static int __init reserve_memblock_reserved_regions(void)
>>> +{
>>> + phys_addr_t start, end, roundup_end = 0;
>>> + struct resource *mem, *res;
>>> + u64 i;
>>> +
>>> + for_each_reserved_mem_region(i, &start, &end) {
>>> + if (end <= roundup_end)
>>> + continue; /* done already */
>>> +
>>> + start = __pfn_to_phys(PFN_DOWN(start));
>>> + end = __pfn_to_phys(PFN_UP(end)) - 1;
>>> + roundup_end = end;
>>> +
>>> + res = kzalloc(sizeof(*res), GFP_ATOMIC);
>>> + if (WARN_ON(!res))
>>> + return -ENOMEM;
>>> + res->start = start;
>>> + res->end = end;
>>> + res->name = "reserved";
>>> + res->flags = IORESOURCE_MEM;
>>> +
>>> + mem = request_resource_conflict(&iomem_resource, res);
>>> + /*
>>> + * We expected memblock_reserve() regions to conflict with
>>> + * memory created by request_standard_resources().
>>> + */
>>> + if (WARN_ON_ONCE(!mem))
>>> + continue;
>>> + kfree(res);
>>
>> Why is kfree() after the conditional continue? This is a memory leak.
>
> request_resource_conflict() inserts res into the iomem_resource tree, or returns
> the conflict if there is already something at this address.
>
> We expect something at this address: a 'System RAM' section added by
> request_resource(). This code wants the conflicting 'System RAM' entry so that
> reserve_region_with_split() can fill in the gaps below it with 'reserved'. See
> the commit-message for an example.
>
> If there was no conflict, it means the memory map doesn't look like we expect,
> we can't pass NULL to reserve_region_with_split(), and we just inserted the
> 'reserved' at the top level. Freeing res at this point would be a use-after-free
> hanging from /proc/iomem.
> This code generates a WARN_ON_ONCE() and leaves the 'reserved' description where
> it is.
Okay. I get it.
> Trying to cleanup here is pointless, we can remove the resource entry and free
> it ... but we still want to report it as reserved, which is what just happened,
> just not quite how we we wanted it.
>
> If you ever see this warning, it means some RAM stopped being nomap between
> request_resources() and reserve_memblock_reserved_regions(). I can't find any
> case where that ever happens.
>
>
> If all that makes sense: how can I improve the comment above the WARN_ON_ONCE()
> to make it clearer?
I guess something like you described above.
/*
* We expect a 'System RAM' section at this address. In the unexpected
* case where one is not found, request_resource_conflict() will insert
* res into the iomem_resource tree.
*/
Do you think this is clearer?
>
>
> Thanks,
>
> James
>
>
>>> +
>>> + reserve_region_with_split(mem, start, end, "reserved");
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +arch_initcall(reserve_memblock_reserved_regions);
>>> +
>>> u64 __cpu_logical_map[NR_CPUS] = { [0 ... NR_CPUS-1] = INVALID_HWID };
>>>
>>> void __init setup_arch(char **cmdline_p)
>>>
>
^ permalink raw reply
* [PATCH] arm64: mm: mark tramp_pg_dir read-only
From: Will Deacon @ 2018-06-19 15:20 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAKv+Gu8Zc7eQbG-Ww51GXUCEmhe2J03vbq5vVdyaKra-s13KrA@mail.gmail.com>
Hi Ard,
Sorry, I forgot to reply to this.
On Wed, May 30, 2018 at 11:53:20AM +0200, Ard Biesheuvel wrote:
> On 30 May 2018 at 11:14, Will Deacon <will.deacon@arm.com> wrote:
> > On Wed, May 30, 2018 at 12:48:06PM +0800, YaoJun wrote:
> >> To protect against KSMA(Kernel Space Mirroring Attack), make
> >> tramp_pg_dir read-only. The principle of KSMA is to insert a
> >> carefully constructed PGD entry into the translation table.
> >> The type of this entry is block, which maps the kernel text
> >> and its access permissions bits are 01. The user process can
> >> then modify kernel text directly through this mapping. In this
> >> way, an arbitrary write can be converted to multiple arbitrary
> >> writes.
> >>
> >> Signed-off-by: YaoJun <yaojun8558363@gmail.com>
> >> ---
> >> arch/arm64/mm/mmu.c | 4 ++++
> >> 1 file changed, 4 insertions(+)
> >>
> >> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
> >> index 2dbb2c9f1ec1..ac4b22c7e435 100644
> >> --- a/arch/arm64/mm/mmu.c
> >> +++ b/arch/arm64/mm/mmu.c
> >> @@ -551,6 +551,10 @@ static int __init map_entry_trampoline(void)
> >> __create_pgd_mapping(tramp_pg_dir, pa_start, TRAMP_VALIAS, PAGE_SIZE,
> >> prot, pgd_pgtable_alloc, 0);
> >>
> >> + update_mapping_prot(__pa_symbol(tramp_pg_dir),
> >> + (unsigned long)tramp_pg_dir,
> >> + PGD_SIZE, PAGE_KERNEL_RO);
> >
> > Hmm, I like the idea but is there a risk that the page table has been mapped
> > as part of a block entry, which we can't safely split at this point (i.e.
> > we'll run into one of the BUG_ONs in the mapping code)?
> >
>
> We'd need to create a separate segment for it initially so the mapping
> is already at the right granularity.
Why do you think that's the case? I can't see anything that guarantees this
for the page table itself.
Will
^ permalink raw reply
* [PATCHv3 10/19] arm64: convert native/compat syscall entry to C
From: Catalin Marinas @ 2018-06-19 15:20 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180618120310.39527-11-mark.rutland@arm.com>
On Mon, Jun 18, 2018 at 01:03:01PM +0100, Mark Rutland wrote:
> Now that the syscall invocation logic is in C, we can migrate the rest
> of the syscall entry logic over, so that the entry assembly needn't look
> at the register values at all.
>
> The SVE reset across syscall logic now unconditionally clears TIF_SVE,
> but sve_user_disable() will only write back to CPACR_EL1 when SVE is
> actually enabled.
>
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will.deacon@arm.com>
Other than Dave's comments, it looks fine to me.
Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
^ permalink raw reply
* [PATCH v3 0/4] allwinner: a64: add SRAM controller / system control
From: Chen-Yu Tsai @ 2018-06-19 15:20 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180618073425.ce2dkipuvrtbqela@flea>
On Mon, Jun 18, 2018 at 3:34 PM, Maxime Ripard
<maxime.ripard@bootlin.com> wrote:
> On Mon, Jun 18, 2018 at 10:11:20AM +0800, Chen-Yu Tsai wrote:
>> On Fri, Jun 15, 2018 at 5:14 PM, Maxime Ripard
>> <maxime.ripard@bootlin.com> wrote:
>> > On Thu, Jun 14, 2018 at 11:35:44PM +0800, Chen-Yu Tsai wrote:
>> >> Hi,
>> >>
>> >> This series is the remaining A64 syscon changes from the R40 DWMAC
>> >> series. The series aligns how the A64 system control exports a regmap
>> >> for the sun8i DWMAC driver to access with what we've done for the R40.
>> >>
>> >> Originally the A64 used the generic syscon for this bit of hardware.
>> >> But this block also contains mapping bits for the onboard SRAM, used
>> >> by various peripherals, and other vendor specific bits we may use in
>> >> the future. It is by no means generic. And we already have a device
>> >> tree binding and driver for the SRAM part.
>> >>
>> >> The first patch make the SRAM control device export a regmap, exposing
>> >> a single EMAC control register, for the DWMAC driver to consume.
>> >>
>> >> The second and third patches rename the A64 compatible string to read
>> >> "system control", which is what the block is named in the user manual.
>> >>
>> >> The last patch fixes up the device node, and also adds the lone mappable
>> >> SRAM block, which is needed by the Display Engine.
>> >
>> > For the serie:
>> > Acked-by: Maxime Ripard <maxime.ripard@bootlin.com>
>>
>> Cool. I'll apply them once you've rebased and pushed out any patches you
>> queued up during the merge window.
>
> All should be pushed now, feel free to apply them :)
Applied. Thanks.
ChenYu
^ permalink raw reply
* [RFC PATCH 1/2] spi: Add QuadSPI driver for Atmel SAMA5D2
From: Mark Brown @ 2018-06-19 15:15 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180618162124.21749-2-bugalski.piotr@gmail.com>
On Mon, Jun 18, 2018 at 06:21:23PM +0200, Piotr Bugalski wrote:
> +static int atmel_qspi_adjust_op_size(struct spi_mem *mem, struct spi_mem_op *op)
> +{
> + return 0;
> +}
If this can be empty should we adjust the callers to allow it to just be
omitted?
> +static int atmel_qspi_remove(struct platform_device *pdev)
> +{
> + struct spi_controller *ctrl = platform_get_drvdata(pdev);
> + struct atmel_qspi *aq = spi_controller_get_devdata(ctrl);
> +
> + qspi_writel(aq, QSPI_CR, QSPI_CR_QSPIDIS);
> + clk_disable_unprepare(aq->clk);
> +
> + spi_unregister_controller(ctrl);
> +
> + return 0;
> +}
You should unregister the controller before disabling the hardware,
otherwise something could come in and try to start an operation on the
controller (or already be running one) while the hardware is disabled
which might blow up.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180619/0ebd21f0/attachment.sig>
^ permalink raw reply
* [PATCHv3 09/19] arm64: convert syscall trace logic to C
From: Mark Rutland @ 2018-06-19 15:14 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180619143245.pjkcutm5hupfunla@localhost>
On Tue, Jun 19, 2018 at 03:32:45PM +0100, Catalin Marinas wrote:
> On Mon, Jun 18, 2018 at 01:03:00PM +0100, Mark Rutland wrote:
> > +asmlinkage void el0_svc_common(struct pt_regs *regs, int scno, int sc_nr,
> > + syscall_fn_t syscall_table[])
> > +{
> > + unsigned long flags = current_thread_info()->flags;
> > +
> > + regs->orig_x0 = regs->regs[0];
> > + regs->syscallno = scno;
> > +
> > + local_daif_restore(DAIF_PROCCTX);
> > + user_exit();
> > +
> > + if (has_syscall_work(flags)) {
> > + /* set default errno for user-issued syscall(-1) */
> > + if (scno == NO_SYSCALL)
> > + regs->regs[0] = -ENOSYS;
> > + scno = syscall_trace_enter(regs);
> > + if (scno == NO_SYSCALL)
> > + goto trace_exit;
> > + }
> > +
> > + invoke_syscall(regs, scno, sc_nr, syscall_table);
> > +
> > + /*
> > + * The tracing status may have changed under our feet, so we have to
> > + * check again. However, if we were tracing entry, then we always trace
> > + * exit regardless, as the old entry assembly did.
> > + */
> > + if (!has_syscall_work(flags)) {
> > + local_daif_mask();
> > + flags = current_thread_info()->flags;
> > + if (!has_syscall_work(flags))
> > + return;
> > + local_daif_restore(DAIF_PROCCTX);
> > + }
>
> IIUC the above 'if' block replaces ret_fast_syscall in entry.S with the
> work_pending handled via ret_to_user (we used to check _TIF_WORK_MASK in
> two places).
Yes. This replaces the _TIF_SYSCALL_WORK check from ret_fast_syscall,
leaving the _TIF_WORK_MASK check to the usual ret_to_user path.
> Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
Thanks!
Mark.
^ permalink raw reply
* [PATCH 1/2] arm64: avoid alloc memory on offline node
From: Michal Hocko @ 2018-06-19 15:14 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <87wouu3jz1.fsf@e105922-lin.cambridge.arm.com>
On Tue 19-06-18 15:54:26, Punit Agrawal wrote:
[...]
> In terms of $SUBJECT, I wonder if it's worth taking the original patch
> as a temporary fix (it'll also be easier to backport) while we work on
> fixing these other issues and enabling memoryless nodes.
Well, x86 already does that but copying this antipatern is not really
nice. So it is good as a quick fix but it would be definitely much
better to have a robust fix. Who knows how many other places might hit
this. You certainly do not want to add a hack like this all over...
--
Michal Hocko
SUSE Labs
^ permalink raw reply
* [RFC PATCH] ARM: Use logical or instead of addition for badr address calculation
From: Russell King - ARM Linux @ 2018-06-19 15:12 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAKv+Gu-T8Fx3Nry7pUdOoOeRU-Ns89w3oo4itmu1q4FzYF5Rag@mail.gmail.com>
On Tue, Jun 19, 2018 at 03:35:07PM +0200, Ard Biesheuvel wrote:
> On 19 June 2018 at 15:29, Guenter Roeck <linux@roeck-us.net> wrote:
> > Hi Ard,
> >
> >
> > On 06/19/2018 12:48 AM, Ard Biesheuvel wrote:
> >>
> >> On 19 June 2018 at 07:07, Guenter Roeck <linux@roeck-us.net> wrote:
> >>>
> >>> Modern assemblers may take the ISA into account when resolving local
> >>> symbols. This can result in bad address calculations when using badr
> >>> in the wrong location since the offset + 1 may be added twice, resulting
> >>> in an even address target for THUMB instructions. This in turn results
> >>> in an exception at (destination address + 2).
> >>>
> >>> Unhandled exception: IPSR = 00000006 LR = fffffff1
> >>> CPU: 0 PID: 1 Comm: init Not tainted 4.18.0-rc1-00026-gf773e5bdf0c9 #15
> >>> Hardware name: MPS2 (Device Tree Support)
> >>> PC is at ret_fast_syscall+0x2/0x58
> >>> LR is at tty_ioctl+0x2a5/0x528
> >>> pc : [<21009002>] lr : [<210d1535>] psr: 4000000b
> >>> sp : 21825fa8 ip : 0000001c fp : 21a95892
> >>> r10: 00000000 r9 : 21824000 r8 : 210091c0
> >>> r7 : 00000036 r6 : 21ae1ee0 r5 : 00000000 r4 : 21ae1f3c
> >>> r3 : 00000000 r2 : 3d9adc25 r1 : 00000000 r0 : 00000000
> >>> xPSR: 4000000b
> >>> CPU: 0 PID: 1 Comm: init Not tainted 4.18.0-rc1-00026-gf773e5bdf0c9 #15
> >>> Hardware name: MPS2 (Device Tree Support)
> >>> [<2100bd8d>] (unwind_backtrace) from [<2100b13b>] (show_stack+0xb/0xc)
> >>> [<2100b13b>] (show_stack) from [<2100b87b>] (__invalid_entry+0x4b/0x4c)
> >>>
> >>> Fix the problem by using a logical or instead of an addition. This is
> >>> less efficient but guaranteed to work.
> >>>
> >>> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> >>> ---
> >>> RFC: I don't really like this, but my ARM assembler knowledge is quite
> >>> limited. Just dropping the "+ 1" from badr doesn't work because some
> >>> other code needs it (the image hangs completely if I try that).
> >>> Ultimately I don't even know if the invoke_syscall macro should just
> >>> have used adr instead of badr (but then how did this ever work ?).
> >>>
> >>> Seen with various toolchains based on gcc 7.x and binutils 2.30 when
> >>> building and testing MPS2 images.
> >>>
> >>
> >> Hello Guenter,
> >>
> >> This issue has been discussed before. It appears the binutils people
> >> suddenly started caring about the thumb annotation of local function
> >> symbols, resulting in behavior that is not backwards compatible.
> >>
> >> https://marc.info/?l=linux-arm-kernel&m=151143776213636&w=2
> >> https://sourceware.org/bugzilla/show_bug.cgi?id=21458
> >>
> >> Can you try the fix below please?
> >>
> >>> arch/arm/include/asm/assembler.h | 3 ++-
> >>> 1 file changed, 2 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/arch/arm/include/asm/assembler.h
> >>> b/arch/arm/include/asm/assembler.h
> >>> index 0cd4dccbae78..24c87ff2060f 100644
> >>> --- a/arch/arm/include/asm/assembler.h
> >>> +++ b/arch/arm/include/asm/assembler.h
> >>> @@ -195,7 +195,8 @@
> >>> .irp c,,eq,ne,cs,cc,mi,pl,vs,vc,hi,ls,ge,lt,gt,le,hs,lo
> >>> .macro badr\c, rd, sym
> >>> #ifdef CONFIG_THUMB2_KERNEL
> >>> - adr\c \rd, \sym + 1
> >>> + adr\c \rd, \sym
> >>> + orr \rd, #1
> >>> #else
> >>> adr\c \rd, \sym
> >>> #endif
> >>> --
> >>> 2.7.4
> >>
> >>
> >> ----------8<------------
> >> From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> >> Date: Tue, 16 Jan 2018 12:12:45 +0000
> >> Subject: [PATCH] ARM: assembler: prevent ADR from setting the Thumb bit
> >> twice
> >>
> >> To work around recent issues where ADR references to Thumb function
> >> symbols may or may not have the Thumb bit set already when they are
> >> resolved by GAS, reference the symbol indirectly via a local symbol
> >> typed as 'object', stripping the ARM/Thumb annotation.
> >>
> >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> >>
> >> diff --git a/arch/arm/include/asm/assembler.h
> >> b/arch/arm/include/asm/assembler.h
> >> index 6ae42ad29518..dd2ff94ad90b 100644
> >> --- a/arch/arm/include/asm/assembler.h
> >> +++ b/arch/arm/include/asm/assembler.h
> >> @@ -195,13 +195,19 @@
> >> .irp c,,eq,ne,cs,cc,mi,pl,vs,vc,hi,ls,ge,lt,gt,le,hs,lo
> >> .macro badr\c, rd, sym
> >> #ifdef CONFIG_THUMB2_KERNEL
> >> - adr\c \rd, \sym + 1
> >> + __badr \c, \rd, \sym
> >> #else
> >> adr\c \rd, \sym
> >> #endif
> >> .endm
> >> .endr
> >>
> >> + /* this needs to be a separate macro or \@ does not work correctly
> >> */
> >> + .macro __badr, c, rd, sym
> >> + .eqv .Lsym\@, \sym
> >> + adr\c \rd, .Lsym\@ + 1
> >
> >
> > Wild shot, but the following works for me.
> >
> > .eqv .Lsym\@, \sym + 1
> > adr\c \rd, .Lsym\@
> >
> > Does it make sense ?
> >
>
> Interesting. Do you mean this works with your 2.30 binutils that
> triggers the original issue?
>
> If so, then yes, it makes sense, although it still seems fragile,
> since we are relying on \sym being resolved without the Thumb bit in
> the context of the .eqv pseudo op. But if this works across all
> implementations we care about, I would be fine with it.
>
> Russell?
Apart from my thoughts about the toolchain changing behaviour after
many years...
It seems that the .eqv solution works on my assembler for the files
I've tested it with, which is great. However...
I'm left wondering how long it will be before the toolchain people
decide to propagate the thumb bit through the .eqv - is the behaviour
there explicitly documented that it won't, or is the above relying on
observed behaviour with a particular assembler version. It could
also be that there's versions out there that do this already.
I think it's really up to _toolchain people_ to tell us how to solve
the problem _they've_ created, rather than us floundering around in
the dark using undocumented behaviour that could well change in the
future.
So, I'm going to continue sitting on the fence on this, and basically
take the attitude that it's better that people don't use the new
binutils until binutils people can provide us with an officially
sanctioned solution that's going to work for both older and future
assemblers.
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up
^ permalink raw reply
* [PATCH v2 1/4] arm64: export memblock_reserve()d regions via /proc/iomem
From: James Morse @ 2018-06-19 15:00 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <72307608-90e0-4842-edc1-d3b284782940@oracle.com>
Hi Dave,
On 19/06/18 14:37, Dave Kleikamp wrote:
> On 06/19/2018 01:44 AM, AKASHI Takahiro wrote:
>> +static int __init reserve_memblock_reserved_regions(void)
>> +{
>> + phys_addr_t start, end, roundup_end = 0;
>> + struct resource *mem, *res;
>> + u64 i;
>> +
>> + for_each_reserved_mem_region(i, &start, &end) {
>> + if (end <= roundup_end)
>> + continue; /* done already */
>> +
>> + start = __pfn_to_phys(PFN_DOWN(start));
>> + end = __pfn_to_phys(PFN_UP(end)) - 1;
>> + roundup_end = end;
>> +
>> + res = kzalloc(sizeof(*res), GFP_ATOMIC);
>> + if (WARN_ON(!res))
>> + return -ENOMEM;
>> + res->start = start;
>> + res->end = end;
>> + res->name = "reserved";
>> + res->flags = IORESOURCE_MEM;
>> +
>> + mem = request_resource_conflict(&iomem_resource, res);
>> + /*
>> + * We expected memblock_reserve() regions to conflict with
>> + * memory created by request_standard_resources().
>> + */
>> + if (WARN_ON_ONCE(!mem))
>> + continue;
>> + kfree(res);
>
> Why is kfree() after the conditional continue? This is a memory leak.
request_resource_conflict() inserts res into the iomem_resource tree, or returns
the conflict if there is already something at this address.
We expect something at this address: a 'System RAM' section added by
request_resource(). This code wants the conflicting 'System RAM' entry so that
reserve_region_with_split() can fill in the gaps below it with 'reserved'. See
the commit-message for an example.
If there was no conflict, it means the memory map doesn't look like we expect,
we can't pass NULL to reserve_region_with_split(), and we just inserted the
'reserved' at the top level. Freeing res at this point would be a use-after-free
hanging from /proc/iomem.
This code generates a WARN_ON_ONCE() and leaves the 'reserved' description where
it is.
Trying to cleanup here is pointless, we can remove the resource entry and free
it ... but we still want to report it as reserved, which is what just happened,
just not quite how we we wanted it.
If you ever see this warning, it means some RAM stopped being nomap between
request_resources() and reserve_memblock_reserved_regions(). I can't find any
case where that ever happens.
If all that makes sense: how can I improve the comment above the WARN_ON_ONCE()
to make it clearer?
Thanks,
James
>> +
>> + reserve_region_with_split(mem, start, end, "reserved");
>> + }
>> +
>> + return 0;
>> +}
>> +arch_initcall(reserve_memblock_reserved_regions);
>> +
>> u64 __cpu_logical_map[NR_CPUS] = { [0 ... NR_CPUS-1] = INVALID_HWID };
>>
>> void __init setup_arch(char **cmdline_p)
>>
^ permalink raw reply
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