* Re: [PATCH v3 05/17] arm64, hibernate: check pgd table allocation
From: James Morse @ 2019-09-06 15:17 UTC (permalink / raw)
To: Pavel Tatashin
Cc: sashal, mark.rutland, vladimir.murzin, corbet, marc.zyngier,
catalin.marinas, bhsharma, kexec, linux-kernel, jmorris, linux-mm,
ebiederm, matthias.bgg, will, linux-arm-kernel
In-Reply-To: <20190821183204.23576-6-pasha.tatashin@soleen.com>
Hi Pavel,
On 21/08/2019 19:31, Pavel Tatashin wrote:
> There is a bug in create_safe_exec_page(), when page table is allocated
> it is not checked that table is allocated successfully:
>
> But it is dereferenced in: pgd_none(READ_ONCE(*pgdp)).
If there is a bug, it shouldn't be fixed part way through a series. This makes it
difficult to backport the fix.
Please split this out as an independent patch with a 'Fixes:' tag for the commit that
introduced the bug.
> Another issue,
So this patch does two things? That is rarely a good idea. Again, this makes it difficult
to backport the fix.
> is that phys_to_ttbr() uses an offset in page table instead
> of pgd directly.
If you were going to reuse this, that would be a bug. But because the only page that is
being mapped, is mapped to PAGE_SIZE, all the top bits will be 0. The offset calls are
boiler-plate. It doesn't look intentional, but its harmless.
Please separate out the potential NULL-dereference bits so there is a clean stand-alone
fix that can be sent to the stable trees.
Thanks,
James
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v3 06/17] arm64, hibernate: add trans_pgd public functions
From: James Morse @ 2019-09-06 15:18 UTC (permalink / raw)
To: Pavel Tatashin
Cc: sashal, mark.rutland, vladimir.murzin, corbet, marc.zyngier,
catalin.marinas, bhsharma, kexec, linux-kernel, jmorris, linux-mm,
ebiederm, matthias.bgg, will, linux-arm-kernel
In-Reply-To: <20190821183204.23576-7-pasha.tatashin@soleen.com>
Hi Pavel,
On 21/08/2019 19:31, Pavel Tatashin wrote:
> trans_pgd_create_copy() and trans_pgd_map_page() are going to be
> the basis for public interface of new subsystem that handles page
Please don't call this a subsystem. 'sound' and 'mm' are subsystems, this is just some
shared code.
> tables for cases which are between kernels: kexec, and hibernate.
Even though you've baked the get_safe_page() calls into trans_pgd_map_page()?
> diff --git a/arch/arm64/kernel/hibernate.c b/arch/arm64/kernel/hibernate.c
> index 750ecc7f2cbe..2e29d620b56c 100644
> --- a/arch/arm64/kernel/hibernate.c
> +++ b/arch/arm64/kernel/hibernate.c
> @@ -182,39 +182,15 @@ int arch_hibernation_header_restore(void *addr)
> +int trans_pgd_map_page(pgd_t *trans_pgd, void *page,
> + unsigned long dst_addr,
> + pgprot_t pgprot)
If this thing is going to be exposed, its name should reflect that its creating a set of
page tables, to map a single page.
A function called 'map_page' with this prototype should 'obviously' map @page at @dst_addr
in @trans_pgd using the provided @pgprot... but it doesn't.
This is what 'create' was doing in the old name, if that wasn't obvious, its because
naming things is hard!
| trans_create_single_page_mapping()?
(might be too verbose)
I think this bites you in patch 8, where you 'generalise' this.
> {
> - void *page = (void *)get_safe_page(GFP_ATOMIC);
> - pgd_t *trans_pgd;
> pgd_t *pgdp;
> pud_t *pudp;
> pmd_t *pmdp;
> pte_t *ptep;
>
> - if (!page)
> - return -ENOMEM;
> -
> - memcpy(page, src_start, length);
> - __flush_icache_range((unsigned long)page, (unsigned long)page + length);
> -
> - trans_pgd = (void *)get_safe_page(GFP_ATOMIC);
> - if (!trans_pgd)
> - return -ENOMEM;
> -
> pgdp = pgd_offset_raw(trans_pgd, dst_addr);
> if (pgd_none(READ_ONCE(*pgdp))) {
> pudp = (void *)get_safe_page(GFP_ATOMIC);
> @@ -242,6 +218,44 @@ static int create_safe_exec_page(void *src_start, size_t length,
> ptep = pte_offset_kernel(pmdp, dst_addr);
> set_pte(ptep, pfn_pte(virt_to_pfn(page), PAGE_KERNEL_EXEC));
>
> + return 0;
> +}
> +
> +/*
> + * Copies length bytes, starting at src_start into an new page,
> + * perform cache maintentance, then maps it at the specified address low
Could you fix the spelling of maintenance as git thinks you've moved it?
> + * address as executable.
> + *
> + * This is used by hibernate to copy the code it needs to execute when
> + * overwriting the kernel text. This function generates a new set of page
> + * tables, which it loads into ttbr0.
> + *
> + * Length is provided as we probably only want 4K of data, even on a 64K
> + * page system.
> + */
> +static int create_safe_exec_page(void *src_start, size_t length,
> + unsigned long dst_addr,
> + phys_addr_t *phys_dst_addr)
> +{
Thanks,
James
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH] arm: fix page faults in do_alignment
From: Russell King - ARM Linux admin @ 2019-09-06 15:17 UTC (permalink / raw)
To: Eric W. Biederman
Cc: kstewart, gustavo, gregkh, linux-kernel, Jing Xiangfeng, linux-mm,
sakari.ailus, bhelgaas, tglx, linux-arm-kernel
In-Reply-To: <87mufmioqv.fsf@x220.int.ebiederm.org>
On Mon, Sep 02, 2019 at 12:36:56PM -0500, Eric W. Biederman wrote:
> Russell King - ARM Linux admin <linux@armlinux.org.uk> writes:
>
> > On Fri, Aug 30, 2019 at 04:02:48PM -0500, Eric W. Biederman wrote:
> >> Russell King - ARM Linux admin <linux@armlinux.org.uk> writes:
> >>
> >> > On Fri, Aug 30, 2019 at 02:45:36PM -0500, Eric W. Biederman wrote:
> >> >> Russell King - ARM Linux admin <linux@armlinux.org.uk> writes:
> >> >>
> >> >> > On Fri, Aug 30, 2019 at 09:31:17PM +0800, Jing Xiangfeng wrote:
> >> >> >> The function do_alignment can handle misaligned address for user and
> >> >> >> kernel space. If it is a userspace access, do_alignment may fail on
> >> >> >> a low-memory situation, because page faults are disabled in
> >> >> >> probe_kernel_address.
> >> >> >>
> >> >> >> Fix this by using __copy_from_user stead of probe_kernel_address.
> >> >> >>
> >> >> >> Fixes: b255188 ("ARM: fix scheduling while atomic warning in alignment handling code")
> >> >> >> Signed-off-by: Jing Xiangfeng <jingxiangfeng@huawei.com>
> >> >> >
> >> >> > NAK.
> >> >> >
> >> >> > The "scheduling while atomic warning in alignment handling code" is
> >> >> > caused by fixing up the page fault while trying to handle the
> >> >> > mis-alignment fault generated from an instruction in atomic context.
> >> >> >
> >> >> > Your patch re-introduces that bug.
> >> >>
> >> >> And the patch that fixed scheduling while atomic apparently introduced a
> >> >> regression. Admittedly a regression that took 6 years to track down but
> >> >> still.
> >> >
> >> > Right, and given the number of years, we are trading one regression for
> >> > a different regression. If we revert to the original code where we
> >> > fix up, we will end up with people complaining about a "new" regression
> >> > caused by reverting the previous fix. Follow this policy and we just
> >> > end up constantly reverting the previous revert.
> >> >
> >> > The window is very small - the page in question will have had to have
> >> > instructions read from it immediately prior to the handler being entered,
> >> > and would have had to be made "old" before subsequently being unmapped.
> >>
> >> > Rather than excessively complicating the code and making it even more
> >> > inefficient (as in your patch), we could instead retry executing the
> >> > instruction when we discover that the page is unavailable, which should
> >> > cause the page to be paged back in.
> >>
> >> My patch does not introduce any inefficiencies. It onlys moves the
> >> check for user_mode up a bit. My patch did duplicate the code.
> >>
> >> > If the page really is unavailable, the prefetch abort should cause a
> >> > SEGV to be raised, otherwise the re-execution should replace the page.
> >> >
> >> > The danger to that approach is we page it back in, and it gets paged
> >> > back out before we're able to read the instruction indefinitely.
> >>
> >> I would think either a little code duplication or a function that looks
> >> at user_mode(regs) and picks the appropriate kind of copy to do would be
> >> the best way to go. Because what needs to happen in the two cases for
> >> reading the instruction are almost completely different.
> >
> > That is what I mean. I'd prefer to avoid that with the large chunk of
> > code. How about instead adding a local replacement for
> > probe_kernel_address() that just sorts out the reading, rather than
> > duplicating all the code to deal with thumb fixup.
>
> So something like this should be fine?
>
> Jing Xiangfeng can you test this please? I think this fixes your issue
> but I don't currently have an arm development box where I could test this.
Sorry, only just got around to this again. What I came up with is this:
8<===
From: Russell King <rmk+kernel@armlinux.org.uk>
Subject: [PATCH] ARM: mm: fix alignment
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
arch/arm/mm/alignment.c | 44 ++++++++++++++++++++++++++++++++++++--------
1 file changed, 36 insertions(+), 8 deletions(-)
diff --git a/arch/arm/mm/alignment.c b/arch/arm/mm/alignment.c
index 6067fa4de22b..529f54d94709 100644
--- a/arch/arm/mm/alignment.c
+++ b/arch/arm/mm/alignment.c
@@ -765,6 +765,36 @@ do_alignment_t32_to_handler(unsigned long *pinstr, struct pt_regs *regs,
return NULL;
}
+static int alignment_get_arm(struct pt_regs *regs, u32 *ip, unsigned long *inst)
+{
+ u32 instr = 0;
+ int fault;
+
+ if (user_mode(regs))
+ fault = get_user(instr, ip);
+ else
+ fault = probe_kernel_address(ip, instr);
+
+ *inst = __mem_to_opcode_arm(instr);
+
+ return fault;
+}
+
+static int alignment_get_thumb(struct pt_regs *regs, u16 *ip, u16 *inst)
+{
+ u16 instr = 0;
+ int fault;
+
+ if (user_mode(regs))
+ fault = get_user(instr, ip);
+ else
+ fault = probe_kernel_address(ip, instr);
+
+ *inst = __mem_to_opcode_thumb16(instr);
+
+ return fault;
+}
+
static int
do_alignment(unsigned long addr, unsigned int fsr, struct pt_regs *regs)
{
@@ -772,10 +802,10 @@ do_alignment(unsigned long addr, unsigned int fsr, struct pt_regs *regs)
unsigned long instr = 0, instrptr;
int (*handler)(unsigned long addr, unsigned long instr, struct pt_regs *regs);
unsigned int type;
- unsigned int fault;
u16 tinstr = 0;
int isize = 4;
int thumb2_32b = 0;
+ int fault;
if (interrupts_enabled(regs))
local_irq_enable();
@@ -784,15 +814,14 @@ do_alignment(unsigned long addr, unsigned int fsr, struct pt_regs *regs)
if (thumb_mode(regs)) {
u16 *ptr = (u16 *)(instrptr & ~1);
- fault = probe_kernel_address(ptr, tinstr);
- tinstr = __mem_to_opcode_thumb16(tinstr);
+
+ fault = alignment_get_thumb(regs, ptr, &tinstr);
if (!fault) {
if (cpu_architecture() >= CPU_ARCH_ARMv7 &&
IS_T32(tinstr)) {
/* Thumb-2 32-bit */
- u16 tinst2 = 0;
- fault = probe_kernel_address(ptr + 1, tinst2);
- tinst2 = __mem_to_opcode_thumb16(tinst2);
+ u16 tinst2;
+ fault = alignment_get_thumb(regs, ptr + 1, &tinst2);
instr = __opcode_thumb32_compose(tinstr, tinst2);
thumb2_32b = 1;
} else {
@@ -801,8 +830,7 @@ do_alignment(unsigned long addr, unsigned int fsr, struct pt_regs *regs)
}
}
} else {
- fault = probe_kernel_address((void *)instrptr, instr);
- instr = __mem_to_opcode_arm(instr);
+ fault = alignment_get_arm(regs, (void *)instrptr, &instr);
}
if (fault) {
--
2.7.4
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Re: [PATCH v3 07/17] arm64, hibernate: move page handling function to new trans_pgd.c
From: James Morse @ 2019-09-06 15:18 UTC (permalink / raw)
To: Pavel Tatashin
Cc: sashal, mark.rutland, vladimir.murzin, corbet, marc.zyngier,
catalin.marinas, bhsharma, kexec, linux-kernel, jmorris, linux-mm,
ebiederm, matthias.bgg, will, linux-arm-kernel
In-Reply-To: <20190821183204.23576-8-pasha.tatashin@soleen.com>
Hi Pavel,
On 21/08/2019 19:31, Pavel Tatashin wrote:
> Now, that we abstracted the required functions move them to a new home.
> Later, we will generalize these function in order to be useful outside
> of hibernation.
> diff --git a/arch/arm64/mm/trans_pgd.c b/arch/arm64/mm/trans_pgd.c
> new file mode 100644
> index 000000000000..00b62d8640c2
> --- /dev/null
> +++ b/arch/arm64/mm/trans_pgd.c
> @@ -0,0 +1,211 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +/*
> + * Copyright (c) 2019, Microsoft Corporation.
> + * Pavel Tatashin <patatash@linux.microsoft.com>
Hmmm, while line-count isn't a useful metric: this file contains 41% of the code that was
in hibernate.c, but has stripped the substantial copyright-pedigree that the hibernate
code had built up over the years.
(counting lines identified by 'cloc' as code, not comments or blank)
If you are copying or moving a non trivial quantity of code, you need to preserve the
copyright. Something like 'Derived from the arm64 hibernate support which has:'....
> + */
> +
> +/*
> + * Transitional tables are used during system transferring from one world to
> + * another: such as during hibernate restore, and kexec reboots. During these
> + * phases one cannot rely on page table not being overwritten.
I think you need to mention that hibernate and kexec are rewriting memory, and may
overwrite the live page tables, therefore ...
> + *
> + */
> +
> +#include <asm/trans_pgd.h>
> +#include <asm/pgalloc.h>
> +#include <asm/pgtable.h>
> +#include <linux/suspend.h>
#include <linux/bug.h>
#include <linux/mm.h>
#include <linux/mmzone.h>
> +static void _copy_pte(pte_t *dst_ptep, pte_t *src_ptep, unsigned long addr)
> +{
> + pte_t pte = READ_ONCE(*src_ptep);
> +
> + if (pte_valid(pte)) {
> + /*
> + * Resume will overwrite areas that may be marked
> + * read only (code, rodata). Clear the RDONLY bit from
> + * the temporary mappings we use during restore.
> + */
> + set_pte(dst_ptep, pte_mkwrite(pte));
> + } else if (debug_pagealloc_enabled() && !pte_none(pte)) {
> + /*
> + * debug_pagealloc will removed the PTE_VALID bit if
> + * the page isn't in use by the resume kernel. It may have
> + * been in use by the original kernel, in which case we need
> + * to put it back in our copy to do the restore.
> + *
> + * Before marking this entry valid, check the pfn should
> + * be mapped.
> + */
> + BUG_ON(!pfn_valid(pte_pfn(pte)));
Thanks,
James
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v3 08/17] arm64, trans_pgd: make trans_pgd_map_page generic
From: James Morse @ 2019-09-06 15:20 UTC (permalink / raw)
To: Pavel Tatashin
Cc: sashal, mark.rutland, vladimir.murzin, corbet, marc.zyngier,
catalin.marinas, bhsharma, kexec, linux-kernel, jmorris, linux-mm,
ebiederm, matthias.bgg, will, linux-arm-kernel
In-Reply-To: <20190821183204.23576-9-pasha.tatashin@soleen.com>
Hi Pavel,
On 21/08/2019 19:31, Pavel Tatashin wrote:
> Currently, trans_pgd_map_page has assumptions that are relevant to
> hibernate. But, to make it generic we must allow it to use any allocator
Sounds familiar: you removed this in patch 2.
> and also, can't assume that entries do not exist in the page table
> already.
This thing creates a set of page tables to map one page: the relocation code.
This is mapped in TTBR0_EL1.
It can assume existing entries do not exist, because it creates the single-entry levels as
it goes. Kexec also needs to map precisely one page for relocation. You don't need to
generalise this.
'trans_pgd_create_copy()' is what creates a copy the linear map. This is mapped in TTBR1_EL1.
There is no reason for kexec to behave differently here.
> Also, we can't use init_mm here.
Why not? arm64's pgd_populate() doesn't use the mm. It's only there to make it obvious
this is an EL1 mapping we are creating. We use the kernel-asid with the new mapping.
The __ version is a lot less readable. Please don't use the page tables as an array: this
is what the offset helpers are for.
> Also, add "flags" for trans_pgd_info, they are going to be used
> in copy functions once they are generalized.
You don't need to 'generalize' this to support hypothetical users.
There are only two: hibernate and kexec, both of which are very specialised. Making these
things top-level marionette strings will tangle the logic.
The copy_p?d() functions should decide if they should manipulate _this_ entry based on
_this_ entry and the kernel configuration. This is only really done in _copy_pte(), which
is where it should stay.
> diff --git a/arch/arm64/include/asm/trans_pgd.h b/arch/arm64/include/asm/trans_pgd.h
> index c7b5402b7d87..e3d022b1b526 100644
> --- a/arch/arm64/include/asm/trans_pgd.h
> +++ b/arch/arm64/include/asm/trans_pgd.h
> @@ -11,10 +11,45 @@
> #include <linux/bits.h>
> #include <asm/pgtable-types.h>
>
> +/*
> + * trans_alloc_page
> + * - Allocator that should return exactly one uninitilaized page, if this
> + * allocator fails, trans_pgd returns -ENOMEM error.
> + *
> + * trans_alloc_arg
> + * - Passed to trans_alloc_page as an argument
This is very familiar.
> + * trans_flags
> + * - bitmap with flags that control how page table is filled.
> + * TRANS_MKWRITE: during page table copy make PTE, PME, and PUD page
> + * writeable by removing RDONLY flag from PTE.
Why would you ever keep the read-only flags in a set of page tables that exist to let you
overwrite memory?
> + * TRANS_MKVALID: during page table copy, if PTE present, but not valid,
> + * make it valid.
Please keep this logic together with the !pte_none(pte) and debug_pagealloc_enabled()
check, where it is today.
Making an entry valid without those checks should never be necessary.
> + * TRANS_CHECKPFN: During page table copy, for every PTE entry check that
> + * PFN that this PTE points to is valid. Otherwise return
> + * -ENXIO
Hibernate does this when inventing a new mapping. This is how we check the kernel
should be able to read/write this page. If !pfn_valid(), the page should not be mapped.
Why do you need to turn this off?
It us only necessary at the leaf level, and only if debug-pagealloc is in use. Please keep
all these bits together, as its much harder to understand why this entry needs inventing
when its split up like this.
> diff --git a/arch/arm64/kernel/hibernate.c b/arch/arm64/kernel/hibernate.c
> index 6ee81bbaa37f..17426dc8cb54 100644
> --- a/arch/arm64/kernel/hibernate.c
> +++ b/arch/arm64/kernel/hibernate.c
> @@ -179,6 +179,12 @@ int arch_hibernation_header_restore(void *addr)
> }
> EXPORT_SYMBOL(arch_hibernation_header_restore);
>
> +static void *
> +hibernate_page_alloc(void *arg)
> +{
> + return (void *)get_safe_page((gfp_t)(unsigned long)arg);
> +}
> +
> /*
> * Copies length bytes, starting at src_start into an new page,
> * perform cache maintentance, then maps it at the specified address low
> @@ -195,6 +201,11 @@ static int create_safe_exec_page(void *src_start, size_t length,
> unsigned long dst_addr,
> phys_addr_t *phys_dst_addr)
> {
> + struct trans_pgd_info trans_info = {
> + .trans_alloc_page = hibernate_page_alloc,
> + .trans_alloc_arg = (void *)GFP_ATOMIC,
> + .trans_flags = 0,
> + };
> void *page = (void *)get_safe_page(GFP_ATOMIC);
> pgd_t *trans_pgd;
> int rc;
> @@ -209,7 +220,7 @@ static int create_safe_exec_page(void *src_start, size_t length,
> if (!trans_pgd)
> return -ENOMEM;
>
> - rc = trans_pgd_map_page(trans_pgd, page, dst_addr,
> + rc = trans_pgd_map_page(&trans_info, trans_pgd, page, dst_addr,
> PAGE_KERNEL_EXEC);
> if (rc)
> return rc;
> diff --git a/arch/arm64/mm/trans_pgd.c b/arch/arm64/mm/trans_pgd.c
> index 00b62d8640c2..dbabccd78cc4 100644
> --- a/arch/arm64/mm/trans_pgd.c
> +++ b/arch/arm64/mm/trans_pgd.c
> @@ -17,6 +17,16 @@
> #include <asm/pgtable.h>
> #include <linux/suspend.h>
>
> +static void *trans_alloc(struct trans_pgd_info *info)
> +{
> + void *page = info->trans_alloc_page(info->trans_alloc_arg);
> +
> + if (page)
> + clear_page(page);
The hibernate allocator already does this. As your reason for doing this is to make this
faster, it seems odd we do this twice.
If zeroed pages are necessary, the allocator should do it. (It already needs to be a
use-case specific allocator)
> +
> + return page;
> +}
> +
> static void _copy_pte(pte_t *dst_ptep, pte_t *src_ptep, unsigned long addr)
> {
> pte_t pte = READ_ONCE(*src_ptep);
> @@ -172,40 +182,64 @@ int trans_pgd_create_copy(pgd_t **dst_pgdp, unsigned long start,
> return rc;
> }
>
> -int trans_pgd_map_page(pgd_t *trans_pgd, void *page, unsigned long dst_addr,
> - pgprot_t pgprot)
> +int trans_pgd_map_page(struct trans_pgd_info *info, pgd_t *trans_pgd,
> + void *page, unsigned long dst_addr, pgprot_t pgprot)
> {
> - pgd_t *pgdp;
> - pud_t *pudp;
> - pmd_t *pmdp;
> - pte_t *ptep;
> -
> - pgdp = pgd_offset_raw(trans_pgd, dst_addr);
> - if (pgd_none(READ_ONCE(*pgdp))) {
> - pudp = (void *)get_safe_page(GFP_ATOMIC);
> - if (!pudp)
> + int pgd_idx = pgd_index(dst_addr);
> + int pud_idx = pud_index(dst_addr);
> + int pmd_idx = pmd_index(dst_addr);
> + int pte_idx = pte_index(dst_addr);
Yuck.
> + pgd_t *pgdp = trans_pgd;
> + pgd_t pgd = READ_ONCE(pgdp[pgd_idx]);
> + pud_t *pudp, pud;
> + pmd_t *pmdp, pmd;
> + pte_t *ptep, pte;
> +
> + if (pgd_none(pgd)) {
> + pud_t *t = trans_alloc(info);
> +
> + if (!t)
> return -ENOMEM;
> - pgd_populate(&init_mm, pgdp, pudp);
> +
> + __pgd_populate(&pgdp[pgd_idx], __pa(t), PUD_TYPE_TABLE);
> + pgd = READ_ONCE(pgdp[pgd_idx]);
Please keep the pgd_populate() call. If there is some reason we can't pass init_mm, we can
pass NULL, or a fake mm pointer instead.
Going behind the page table helpers back to play with the table directly is a maintenance
headache.
> }
>
> - pudp = pud_offset(pgdp, dst_addr);
> - if (pud_none(READ_ONCE(*pudp))) {
> - pmdp = (void *)get_safe_page(GFP_ATOMIC);
> - if (!pmdp)
> + pudp = __va(pgd_page_paddr(pgd));
> + pud = READ_ONCE(pudp[pud_idx]);
> + if (pud_sect(pud)) {
> + return -ENXIO;
> + } else if (pud_none(pud) || pud_sect(pud)) {
> + pmd_t *t = trans_alloc(info);
> +
> + if (!t)
> return -ENOMEM;
Choke on block mappings? This should never happen because this function should only create
the tables necessary to map one page. Not a block mapping in sight.
(see my comments on patch 6)
Thanks,
James
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v3 09/17] arm64, trans_pgd: add trans_pgd_create_empty
From: James Morse @ 2019-09-06 15:20 UTC (permalink / raw)
To: Pavel Tatashin
Cc: sashal, mark.rutland, vladimir.murzin, corbet, marc.zyngier,
catalin.marinas, bhsharma, kexec, linux-kernel, jmorris, linux-mm,
ebiederm, matthias.bgg, will, linux-arm-kernel
In-Reply-To: <20190821183204.23576-10-pasha.tatashin@soleen.com>
Hi Pavel,
On 21/08/2019 19:31, Pavel Tatashin wrote:
> This functions returns a zeroed trans_pgd using the allocator that is
> specified in the info argument.
>
> trans_pgds should be created by using this function.
This function takes the allocator you give it, and calls it once.
Given both users need one pgd, and have to provide the allocator, it seems strange that
they aren't trusted to call it.
I don't think this patch is necessary.
Let the caller pass in the pgd_t to the helpers.
Thanks,
James
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v3 10/17] arm64, trans_pgd: adjust trans_pgd_create_copy interface
From: James Morse @ 2019-09-06 15:20 UTC (permalink / raw)
To: Pavel Tatashin
Cc: sashal, mark.rutland, vladimir.murzin, corbet, marc.zyngier,
catalin.marinas, bhsharma, kexec, linux-kernel, jmorris, linux-mm,
ebiederm, matthias.bgg, will, linux-arm-kernel
In-Reply-To: <20190821183204.23576-11-pasha.tatashin@soleen.com>
Hi Pavel,
On 21/08/2019 19:31, Pavel Tatashin wrote:
> Make trans_pgd_create_copy inline with the other functions in
> trans_pgd: use the trans_pgd_info argument, and also use the
> trans_pgd_create_empty.
>
> Note, that the functions that are called by trans_pgd_create_copy are
> not yet adjusted to be compliant with trans_pgd: they do not yet use
> the provided allocator, do not check for generic errors, and do not yet
> use the flags in info argument.
> diff --git a/arch/arm64/include/asm/trans_pgd.h b/arch/arm64/include/asm/trans_pgd.h
> index 26e5a63676b5..f4a5f255d4a7 100644
> --- a/arch/arm64/include/asm/trans_pgd.h
> +++ b/arch/arm64/include/asm/trans_pgd.h
> @@ -43,7 +43,12 @@ struct trans_pgd_info {
> /* Create and empty trans_pgd page table */
> int trans_pgd_create_empty(struct trans_pgd_info *info, pgd_t **trans_pgd);
>
> -int trans_pgd_create_copy(pgd_t **dst_pgdp, unsigned long start,
> +/*
> + * Create trans_pgd and copy entries from from_table to trans_pgd in range
> + * [start, end)
> + */
> +int trans_pgd_create_copy(struct trans_pgd_info *info, pgd_t **trans_pgd,
> + pgd_t *from_table, unsigned long start,
> unsigned long end);
This creates a copy of the linear-map. Why does it need to be told from_table?
> diff --git a/arch/arm64/kernel/hibernate.c b/arch/arm64/kernel/hibernate.c
> index 8c2641a9bb09..8bb602e91065 100644
> --- a/arch/arm64/kernel/hibernate.c
> +++ b/arch/arm64/kernel/hibernate.c
> @@ -323,15 +323,42 @@ int swsusp_arch_resume(void)
> phys_addr_t phys_hibernate_exit;
> void __noreturn (*hibernate_exit)(phys_addr_t, phys_addr_t, void *,
> void *, phys_addr_t, phys_addr_t);
> + struct trans_pgd_info trans_info = {
> + .trans_alloc_page = hibernate_page_alloc,
> + .trans_alloc_arg = (void *)GFP_ATOMIC,
> + /*
> + * Resume will overwrite areas that may be marked read only
> + * (code, rodata). Clear the RDONLY bit from the temporary
> + * mappings we use during restore.
> + */
> + .trans_flags = TRANS_MKWRITE,
> + };
> + /*
> + * debug_pagealloc will removed the PTE_VALID bit if the page isn't in
> + * use by the resume kernel. It may have been in use by the original
> + * kernel, in which case we need to put it back in our copy to do the
> + * restore.
> + *
> + * Before marking this entry valid, check the pfn should be mapped.
> + */
> + if (debug_pagealloc_enabled())
> + trans_info.trans_flags |= (TRANS_MKVALID | TRANS_CHECKPFN);
The debug_pagealloc_enabled() check should be with the code that generates a different
entry. Whether the different entry is correct needs to be considered with
debug_pagealloc_enabled() in mind. You are making this tricky logic less clear.
There is no way the existing code invents an entry for a !pfn_valid() page. With your
'checkpfn' flag, this thing can. You don't need to generalise this for hypothetical users.
If kexec needs to create mappings for bogus pages, I'd like to know why.
> /*
> * Restoring the memory image will overwrite the ttbr1 page tables.
> * Create a second copy of just the linear map, and use this when
> * restoring.
> */
> - rc = trans_pgd_create_copy(&tmp_pg_dir, PAGE_OFFSET, 0);
> - if (rc)
> + rc = trans_pgd_create_copy(&trans_info, &tmp_pg_dir, init_mm.pgd,
> + PAGE_OFFSET, 0);
> + if (rc) {
> + if (rc == -ENOMEM)
> + pr_err("Failed to allocate memory for temporary page tables.\n");
> + else if (rc == -ENXIO)
> + pr_err("Tried to set PTE for PFN that does not exist\n");
> goto out;
> + }
If you think the distinction for this error message is useful, it would be clearer to
change it in the current hibernate code before you move it. (_copy_pte() to return an
error, instead of silently failing). Done here, this is unrelated noise.
I doubt this is specific to kexec.
Thanks,
James
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v3 11/17] arm64, trans_pgd: add PUD_SECT_RDONLY
From: James Morse @ 2019-09-06 15:21 UTC (permalink / raw)
To: Pavel Tatashin
Cc: sashal, mark.rutland, vladimir.murzin, corbet, marc.zyngier,
catalin.marinas, bhsharma, kexec, linux-kernel, jmorris, linux-mm,
ebiederm, matthias.bgg, will, linux-arm-kernel
In-Reply-To: <20190821183204.23576-12-pasha.tatashin@soleen.com>
Hi Pavel,
On 21/08/2019 19:31, Pavel Tatashin wrote:
> Thre is PMD_SECT_RDONLY that is used in pud_* function which is confusing.
Nit: There
I bet it was equally confusing before before you moved it! Could you do this earlier in
the series with the rest of the cleanup?
With that,
Acked-by: James Morse <james.morse@arm.com>
Thanks,
James
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v3 12/17] arm64, trans_pgd: complete generalization of trans_pgds
From: James Morse @ 2019-09-06 15:23 UTC (permalink / raw)
To: Pavel Tatashin
Cc: sashal, mark.rutland, vladimir.murzin, corbet, marc.zyngier,
catalin.marinas, bhsharma, kexec, linux-kernel, jmorris, linux-mm,
ebiederm, matthias.bgg, will, linux-arm-kernel
In-Reply-To: <20190821183204.23576-13-pasha.tatashin@soleen.com>
Hi Pavel,
On 21/08/2019 19:31, Pavel Tatashin wrote:
> Make the last private functions in page table copy path generlized for use
> outside of hibernate.
>
> Switch to use the provided allocator, flags, and source page table. Also,
> unify all copy function implementations to reduce the possibility of bugs.
By changing it? No one has reported any problems. We're more likely to break it making
unnecessary changes.
Why is this necessary?
> All page table levels are implemented symmetrically.
> diff --git a/arch/arm64/mm/trans_pgd.c b/arch/arm64/mm/trans_pgd.c
> index efd42509d069..ccd9900f8edb 100644
> --- a/arch/arm64/mm/trans_pgd.c
> +++ b/arch/arm64/mm/trans_pgd.c
> @@ -27,139 +27,157 @@ static void *trans_alloc(struct trans_pgd_info *info)
> -static void _copy_pte(pte_t *dst_ptep, pte_t *src_ptep, unsigned long addr)
> +static int copy_pte(struct trans_pgd_info *info, pte_t *dst_ptep,
> + pte_t *src_ptep, unsigned long start, unsigned long end)
> {
> - pte_t pte = READ_ONCE(*src_ptep);
> -
> - if (pte_valid(pte)) {
> - /*
> - * Resume will overwrite areas that may be marked
> - * read only (code, rodata). Clear the RDONLY bit from
> - * the temporary mappings we use during restore.
> - */
> - set_pte(dst_ptep, pte_mkwrite(pte));
> - } else if (debug_pagealloc_enabled() && !pte_none(pte)) {
> - /*
> - * debug_pagealloc will removed the PTE_VALID bit if
> - * the page isn't in use by the resume kernel. It may have
> - * been in use by the original kernel, in which case we need
> - * to put it back in our copy to do the restore.
> - *
> - * Before marking this entry valid, check the pfn should
> - * be mapped.
> - */
> - BUG_ON(!pfn_valid(pte_pfn(pte)));
> -
> - set_pte(dst_ptep, pte_mkpresent(pte_mkwrite(pte)));
> - }
> -}
> -static int copy_pte(pmd_t *dst_pmdp, pmd_t *src_pmdp, unsigned long start,
> - unsigned long end)
> -{
> - pte_t *src_ptep;
> - pte_t *dst_ptep;
> unsigned long addr = start;
> + int i = pte_index(addr);
>
> - dst_ptep = (pte_t *)get_safe_page(GFP_ATOMIC);
> - if (!dst_ptep)
> - return -ENOMEM;
> - pmd_populate_kernel(&init_mm, dst_pmdp, dst_ptep);
> - dst_ptep = pte_offset_kernel(dst_pmdp, start);
> -
> - src_ptep = pte_offset_kernel(src_pmdp, start);
> do {
> - _copy_pte(dst_ptep, src_ptep, addr);
> - } while (dst_ptep++, src_ptep++, addr += PAGE_SIZE, addr != end);
> + pte_t src_pte = READ_ONCE(src_ptep[i]);
> +
> + if (pte_none(src_pte))
> + continue;
> + if (info->trans_flags & TRANS_MKWRITE)
> + src_pte = pte_mkwrite(src_pte);
This should be unconditional. The purpose of this thing is to create a set of page tables
you can use to overwrite all of memory. Why would you want to keep the RDONLY flag for
normal memory?
> + if (info->trans_flags & TRANS_MKVALID)
> + src_pte = pte_mkpresent(src_pte);
> + if (info->trans_flags & TRANS_CHECKPFN) {
> + if (!pfn_valid(pte_pfn(src_pte)))
> + return -ENXIO;
> + }
This lets you skip the pfn_valid() check if you want to create bogus mappings. This should
not be conditional.
This removes the BUG_ON(), which is there to make sure we stop if we find page-table
corruption.
Please keep the shape of _copy_pte() as it is. Putting a different mapping in the copied
tables is risky, the code that does it should all be in one place, along with the
justification of why its doing this. Anything else is harder to debug when it goes wrong.
> + set_pte(&dst_ptep[i], src_pte);
> + } while (addr += PAGE_SIZE, i++, addr != end && i < PTRS_PER_PTE);
Incrementing pte/pud/pmg/pgd pointers is a common pattern in the kernel's page table
walkers. Why do we need to change this to index it like an array?
This needs to look like walk_page_range() as the eventual aim is to remove it, and use the
core-code page table walker.
(at the time it was merged the core-code page table walker removed block mappings it
didn't like, which didn't go well.)
This is a backwards step as it makes any attempt to remove this arch-specific walker harder.
>
> return 0;
> }
Thanks,
James
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH] rtc: meson: mark PM functions as __maybe_unused
From: Arnd Bergmann @ 2019-09-06 15:24 UTC (permalink / raw)
To: Alessandro Zummo, Alexandre Belloni, Kevin Hilman
Cc: linux-rtc, Arnd Bergmann, Neil Armstrong, linux-kernel,
linux-amlogic, linux-arm-kernel
The meson_vrtc_set_wakeup_time() function is only used by
the PM functions and causes a warning when they are disabled:
drivers/rtc/rtc-meson-vrtc.c:32:13: error: unused function 'meson_vrtc_set_wakeup_time' [-Werror,-Wunused-function]
Remove the #ifdef around the callers and add a __maybe_unused
annotation as a more reliable way to avoid these warnings.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/rtc/rtc-meson-vrtc.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/rtc/rtc-meson-vrtc.c b/drivers/rtc/rtc-meson-vrtc.c
index 4621a4715179..89e5ba0dae69 100644
--- a/drivers/rtc/rtc-meson-vrtc.c
+++ b/drivers/rtc/rtc-meson-vrtc.c
@@ -91,8 +91,7 @@ static int meson_vrtc_probe(struct platform_device *pdev)
return 0;
}
-#ifdef CONFIG_PM_SLEEP
-static int meson_vrtc_suspend(struct device *dev)
+static int __maybe_unused meson_vrtc_suspend(struct device *dev)
{
struct meson_vrtc_data *vrtc = dev_get_drvdata(dev);
@@ -121,7 +120,7 @@ static int meson_vrtc_suspend(struct device *dev)
return 0;
}
-static int meson_vrtc_resume(struct device *dev)
+static int __maybe_unused meson_vrtc_resume(struct device *dev)
{
struct meson_vrtc_data *vrtc = dev_get_drvdata(dev);
@@ -131,7 +130,7 @@ static int meson_vrtc_resume(struct device *dev)
meson_vrtc_set_wakeup_time(vrtc, 0);
return 0;
}
-#endif
+
static SIMPLE_DEV_PM_OPS(meson_vrtc_pm_ops,
meson_vrtc_suspend, meson_vrtc_resume);
--
2.20.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Re: [PATCH v3 01/17] kexec: quiet down kexec reboot
From: Pavel Tatashin @ 2019-09-06 15:35 UTC (permalink / raw)
To: James Morse
Cc: Sasha Levin, Mark Rutland, Vladimir Murzin, Jonathan Corbet,
Marc Zyngier, Catalin Marinas, Bhupesh Sharma, kexec mailing list,
LKML, James Morris, linux-mm, Eric W. Biederman, Matthias Brugger,
will, Linux ARM
In-Reply-To: <0f83b70e-2f8f-aa05-84d8-41290679003b@arm.com>
On Fri, Sep 6, 2019 at 11:17 AM James Morse <james.morse@arm.com> wrote:
>
> Hi Pavel,
>
> On 21/08/2019 19:31, Pavel Tatashin wrote:
> > Here is a regular kexec command sequence and output:
> > =====
> > $ kexec --reuse-cmdline -i --load Image
> > $ kexec -e
> > [ 161.342002] kexec_core: Starting new kernel
> >
> > Welcome to Buildroot
> > buildroot login:
> > =====
> >
> > Even when "quiet" kernel parameter is specified, "kexec_core: Starting
> > new kernel" is printed.
> >
> > This message has KERN_EMERG level, but there is no emergency, it is a
> > normal kexec operation, so quiet it down to appropriate KERN_NOTICE.
>
> As this doesn't have a dependency with the rest of the series, you may want to post it
> independently so it can be picked up independently.
Hi James,
I have posted it previously, but it has not been picked up. So, I
decided to include it together with this series. Is this alright with
you, otherwise I can remove it from this series.
Thank you,
Pasha
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCHv4 01/10] dt-bindings: omap: add new binding for PRM instances
From: Tony Lindgren @ 2019-09-06 15:36 UTC (permalink / raw)
To: Rob Herring
Cc: devicetree, Tero Kristo, Philipp Zabel, Santosh Shilimkar,
linux-omap,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE
In-Reply-To: <CAL_JsqLHTsEz6RJSi3rZ9AKyTBc00abyAxqwG8B9zAqL6cnv+w@mail.gmail.com>
* Rob Herring <robh+dt@kernel.org> [190906 12:57]:
> On Fri, Sep 6, 2019 at 11:36 AM Tero Kristo <t-kristo@ti.com> wrote:
> >
> > Add new binding for OMAP PRM (Power and Reset Manager) instances. Each
> > of these will act as a power domain controller and potentially as a reset
> > provider.
> >
> > Signed-off-by: Tero Kristo <t-kristo@ti.com>
> > ---
> > v4:
> > - renamed nodes as power-controller
> > - added documentation about hierarchy
> >
> > .../devicetree/bindings/arm/omap/prm-inst.txt | 31 +++++++++++++++++++
> > 1 file changed, 31 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/arm/omap/prm-inst.txt
>
> Reviewed-by: Rob Herring <robh@kernel.org>
Looks good to me too:
Reviewed-by: Tony Lindgren <tony@atomide.com>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH] ARM: xen: unexport HYPERVISOR_platform_op function
From: Arnd Bergmann @ 2019-09-06 15:39 UTC (permalink / raw)
To: Stefano Stabellini
Cc: Arnd Bergmann, Emil Velikov, Russell King, Denis Efremov,
linux-kernel, Masahiro Yamada, xen-devel, linux-arm-kernel
HYPERVISOR_platform_op() is an inline function and should not
be exported. Since commit 15bfc2348d54 ("modpost: check for
static EXPORT_SYMBOL* functions"), this causes a warning:
WARNING: "HYPERVISOR_platform_op" [vmlinux] is a static EXPORT_SYMBOL_GPL
Remove the extraneous export.
Fixes: 15bfc2348d54 ("modpost: check for static EXPORT_SYMBOL* functions")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
arch/arm/xen/enlighten.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
index 1e57692552d9..845c528acf24 100644
--- a/arch/arm/xen/enlighten.c
+++ b/arch/arm/xen/enlighten.c
@@ -437,7 +437,6 @@ EXPORT_SYMBOL_GPL(HYPERVISOR_memory_op);
EXPORT_SYMBOL_GPL(HYPERVISOR_physdev_op);
EXPORT_SYMBOL_GPL(HYPERVISOR_vcpu_op);
EXPORT_SYMBOL_GPL(HYPERVISOR_tmem_op);
-EXPORT_SYMBOL_GPL(HYPERVISOR_platform_op);
EXPORT_SYMBOL_GPL(HYPERVISOR_multicall);
EXPORT_SYMBOL_GPL(HYPERVISOR_vm_assist);
EXPORT_SYMBOL_GPL(HYPERVISOR_dm_op);
--
2.20.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Re: [PATCH v3 02/17] arm64, hibernate: use get_safe_page directly
From: Pavel Tatashin @ 2019-09-06 15:39 UTC (permalink / raw)
To: James Morse
Cc: Sasha Levin, Mark Rutland, Vladimir Murzin, Jonathan Corbet,
Marc Zyngier, Catalin Marinas, Bhupesh Sharma, kexec mailing list,
LKML, James Morris, linux-mm, Eric W. Biederman, Matthias Brugger,
will, Linux ARM
In-Reply-To: <dc6506a0-9b66-f633-8319-9c8a9dc93d4f@arm.com>
On Fri, Sep 6, 2019 at 11:17 AM James Morse <james.morse@arm.com> wrote:
>
> Hi Pavel,
>
> Nit: The pattern for the subject prefix should be "arm64: hibernate:"..
> Its usually possible to spot the pattern from "git log --oneline $file".
Sure, I will change here and in other places to "arm64: hibernate:"
>
> On 21/08/2019 19:31, Pavel Tatashin wrote:
> > create_safe_exec_page is a local function that uses the
> > get_safe_page() to allocate page table and pages and one pages
> > that is getting mapped.
>
> I can't parse this.
>
> create_safe_exec_page() uses hibernate's allocator to create a set of page table to map a
> single page that will contain the relocation code.
Thanks I will rephrase it with your suggestion.
>
>
> > Remove the allocator related arguments, and use get_safe_page
> > directly, as it is done in other local functions in this
> > file.
> ... because kexec can't use this as it doesn't have a working allocator.
> Removing this function pointer makes it easier to refactor the code later.
Thanks, I will add it to the description.
>
> (this thing is only a function pointer so kexec could use it too ... It looks like you're
> creating extra work. Patch 7 moves these new calls out to a new file... presumably so
> another patch can remove them again)
>
> As stand-alone cleanup the patch looks fine, but you probably don't need to do this.
Without this clean-up moving to common code becomes messier. So, I
would like to keep this change.
Thank you,
Pasha
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v3 04/17] arm64, hibernate: rename dst to page in create_safe_exec_page
From: Pavel Tatashin @ 2019-09-06 15:41 UTC (permalink / raw)
To: James Morse
Cc: Sasha Levin, Mark Rutland, Vladimir Murzin, Jonathan Corbet,
Marc Zyngier, Catalin Marinas, Bhupesh Sharma, kexec mailing list,
LKML, James Morris, linux-mm, Eric W. Biederman, Matthias Brugger,
will, Linux ARM
In-Reply-To: <2e826560-4005-fa16-8bbb-fc0e25763dcc@arm.com>
On Fri, Sep 6, 2019 at 11:17 AM James Morse <james.morse@arm.com> wrote:
>
> Hi Pavel,
>
> On 21/08/2019 19:31, Pavel Tatashin wrote:
> > create_safe_exec_page() allocates a safe page and maps it at a
> > specific location, also this function returns the physical address
> > of newly allocated page.
> >
> > The destination VA, and PA are specified in arguments: dst_addr,
> > phys_dst_addr
> >
> > However, within the function it uses "dst" which has unsigned long
> > type, but is actually a pointers in the current virtual space. This
> > is confusing to read.
>
> The type? There are plenty of places in the kernel that an unsigned-long is actually a
> pointer. This isn't unusual.
>
>
> > Rename dst to more appropriate page (page that is created), and also
> > change its time to "void *"
>
> If you think its clearer,
> Reviewed-by: James Morse <james.morse@arm.com>
Thank you for your review.
Pasha
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v3 03/17] arm64, hibernate: remove gotos in create_safe_exec_page
From: Pavel Tatashin @ 2019-09-06 15:41 UTC (permalink / raw)
To: James Morse
Cc: Sasha Levin, Mark Rutland, Vladimir Murzin, Jonathan Corbet,
Marc Zyngier, Catalin Marinas, Bhupesh Sharma, kexec mailing list,
LKML, James Morris, linux-mm, Eric W. Biederman, Matthias Brugger,
will, Linux ARM
In-Reply-To: <99aba737-a959-e352-74d8-a2aff3ae5a88@arm.com>
On Fri, Sep 6, 2019 at 11:17 AM James Morse <james.morse@arm.com> wrote:
>
> Hi Pavel,
>
> On 21/08/2019 19:31, Pavel Tatashin wrote:
> > Usually, gotos are used to handle cleanup after exception, but
> > in case of create_safe_exec_page there are no clean-ups. So,
> > simply return the errors directly.
>
> Reviewed-by: James Morse <james.morse@arm.com>
Thank you.
Pasha
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v3 05/17] arm64, hibernate: check pgd table allocation
From: Pavel Tatashin @ 2019-09-06 15:44 UTC (permalink / raw)
To: James Morse
Cc: Sasha Levin, Mark Rutland, Vladimir Murzin, Jonathan Corbet,
Marc Zyngier, Catalin Marinas, Bhupesh Sharma, kexec mailing list,
LKML, James Morris, linux-mm, Eric W. Biederman, Matthias Brugger,
will, Linux ARM
In-Reply-To: <ddd81093-89fc-5146-0b33-ad3bd9a1c10c@arm.com>
On Fri, Sep 6, 2019 at 11:17 AM James Morse <james.morse@arm.com> wrote:
>
> Hi Pavel,
>
> On 21/08/2019 19:31, Pavel Tatashin wrote:
> > There is a bug in create_safe_exec_page(), when page table is allocated
> > it is not checked that table is allocated successfully:
> >
> > But it is dereferenced in: pgd_none(READ_ONCE(*pgdp)).
>
> If there is a bug, it shouldn't be fixed part way through a series. This makes it
> difficult to backport the fix.
>
> Please split this out as an independent patch with a 'Fixes:' tag for the commit that
> introduced the bug.
>
>
> > Another issue,
>
> So this patch does two things? That is rarely a good idea. Again, this makes it difficult
> to backport the fix.
>
>
> > is that phys_to_ttbr() uses an offset in page table instead
> > of pgd directly.
>
> If you were going to reuse this, that would be a bug. But because the only page that is
> being mapped, is mapped to PAGE_SIZE, all the top bits will be 0. The offset calls are
> boiler-plate. It doesn't look intentional, but its harmless.
Yes, it is harmless otherwise the code would not work. But it is
confusing to read, and looks broken. I will separate this change as
two patches as you suggested.
Thank you,
Pasha
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH] ARM: don't export unused return_address()
From: Arnd Bergmann @ 2019-09-06 15:46 UTC (permalink / raw)
To: Russell King
Cc: Thomas Gleixner, Enrico Weigelt, linux-arm-kernel, Arnd Bergmann,
linux-kernel
Without the frame pointer enabled, return_address() is an inline
function and does not need to be exported, as shown by this warning:
WARNING: "return_address" [vmlinux] is a static EXPORT_SYMBOL_GPL
Move the EXPORT_SYMBOL_GPL() into the #ifdef as well.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
arch/arm/kernel/return_address.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/arch/arm/kernel/return_address.c b/arch/arm/kernel/return_address.c
index b0d2f1fe891d..fb0fc1910102 100644
--- a/arch/arm/kernel/return_address.c
+++ b/arch/arm/kernel/return_address.c
@@ -53,6 +53,7 @@ void *return_address(unsigned int level)
return NULL;
}
+EXPORT_SYMBOL_GPL(return_address);
+
#endif /* if defined(CONFIG_FRAME_POINTER) && !defined(CONFIG_ARM_UNWIND) */
-EXPORT_SYMBOL_GPL(return_address);
--
2.20.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Re: [RFC 5/9] dt-bindings: arm: samsung: Convert Exynos PMU bindings to json-schema
From: Rob Herring @ 2019-09-06 15:48 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Mark Rutland, Alessandro Zummo, Alexandre Belloni,
Lars-Peter Clausen, Arnd Bergmann, devicetree,
open list:IIO SUBSYSTEM AND DRIVERS, Marek Szyprowski,
linux-kernel@vger.kernel.org, Tomasz Figa, linux-samsung-soc,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
Peter Meerwald-Stadler, Hartmut Knaack, Olof Johansson,
open list:REAL TIME CLOCK (RTC) SUBSYSTEM, notify,
Jonathan Cameron, Paweł Chmiel
In-Reply-To: <CAJKOXPfnvu=c5f6AcOSiQ_9E-C2fMf9qbEpy1Tr3QvH8LgAtpQ@mail.gmail.com>
On Tue, Sep 3, 2019 at 12:03 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On Tue, 3 Sep 2019 at 10:25, Rob Herring <robh+dt@kernel.org> wrote:
> >
> > On Tue, Sep 3, 2019 at 8:58 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
> > >
> > > On Mon, 26 Aug 2019 at 13:54, Rob Herring <robh+dt@kernel.org> wrote:
> > > >
> > > > On Fri, Aug 23, 2019 at 9:54 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
> > > > >
> > > > > Convert Samsung Exynos Power Management Unit (PMU) bindings to DT schema
> > > > > format using json-schema.
> > > > >
> > > > > Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
> > > > > ---
> > > > > .../devicetree/bindings/arm/samsung/pmu.txt | 72 --------------
> > > > > .../devicetree/bindings/arm/samsung/pmu.yaml | 93 +++++++++++++++++++
> > > > > 2 files changed, 93 insertions(+), 72 deletions(-)
> > > > > delete mode 100644 Documentation/devicetree/bindings/arm/samsung/pmu.txt
> > > > > create mode 100644 Documentation/devicetree/bindings/arm/samsung/pmu.yaml
> > > >
> > > >
> > > > > diff --git a/Documentation/devicetree/bindings/arm/samsung/pmu.yaml b/Documentation/devicetree/bindings/arm/samsung/pmu.yaml
> > > > > new file mode 100644
> > > > > index 000000000000..818c6f3488ef
> > > > > --- /dev/null
> > > > > +++ b/Documentation/devicetree/bindings/arm/samsung/pmu.yaml
> > > > > @@ -0,0 +1,93 @@
> > > > > +# SPDX-License-Identifier: GPL-2.0
> > > > > +%YAML 1.2
> > > > > +---
> > > > > +$id: http://devicetree.org/schemas/arm/samsung/pmu.yaml#
> > > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > > +
> > > > > +title: Samsung Exynos SoC series Power Management Unit (PMU)
> > > > > +
> > > > > +maintainers:
> > > > > + - Krzysztof Kozlowski <krzk@kernel.org>
> > > > > +
> > > > > +properties:
> > > > > + compatible:
> > > > > + items:
> > > > > + - enum:
> > > > > + - samsung,exynos3250-pmu
> > > > > + - samsung,exynos4210-pmu
> > > > > + - samsung,exynos4412-pmu
> > > > > + - samsung,exynos5250-pmu
> > > > > + - samsung,exynos5260-pmu
> > > > > + - samsung,exynos5410-pmu
> > > > > + - samsung,exynos5420-pmu
> > > > > + - samsung,exynos5433-pmu
> > > > > + - samsung,exynos7-pmu
> > > > > + - const: syscon
> > > > > +
> > > > > + reg:
> > > > > + maxItems: 1
> > > > > +
> > > > > + '#clock-cells':
> > > > > + const: 1
> > > > > +
> > > > > + clock-names:
> > > > > + description:
> > > > > + list of clock names for particular CLKOUT mux inputs
> > > > > + # TODO: what is the maximum number of elements (mux inputs)?
> > > > > + minItems: 1
> > > > > + maxItems: 32
> > > > > + items:
> > > > > + - enum:
> > > >
> > > > This isn't correct as you are only defining possible names for the
> > > > first item. Drop the '-' (making items a schema instead of a list) and
> > > > then it applies to all. However, doing that will cause a meta-schema
> > > > error which I need to fix to allow. Or if there's a small set of
> > > > possibilities of number of inputs, you can list them under a 'oneOf'
> > > > list.
> > >
> > > Mhmm, I cannot test it or I have an error in the schema. if I
> > > understand correctly, this would be:
> > >
> > > clock-names:
> > > description:
> > > List of clock names for particular CLKOUT mux inputs
> > > minItems: 1
> > > maxItems: 16
> > > items:
> > > clkout0
> > > clkout1
> > > clkout2
> > > clkout3
> > > clkout4
> > > clkout5
> > > clkout6
> > > clkout7
> > > clkout8
> > > clkout9
> > > clkout10
> > > clkout11
> > > clkout12
> > > clkout13
> > > clkout14
> > > clkout15
> > > clkout16
> > >
> > > Now it produces the error "ignoring, error in schema 'items'" but
> > > maybe it is expected with current meta-schema?
> >
> > 'make dt_binding_check' will give more detailed errors.
> >
> > Are the inputs always contiguous 0-N? If so, you want:
> >
> > items:
> > - const: clkout0
> > - const: clkout1
> > - const: clkout2
> > ...
> >
> > If you want to express any number and order of strings is valid, then you need:
> >
> > items:
> > enum:
> > - clkout0
> > - clkout1
> > - clkout2
> >
> > Doing that is discouraged for bindings though. Currently, it will
> > generate an error from the meta-schema, but we could change that.
>
> It's the second case. The inputs are not contiguous. Examples:
>
> system-controller {
> compatible = "samsung,exynos3250-pmu", "syscon";
> clock-names = "clkout8";
> clocks = <&cmu CLK_FIN_PLL>;
> }
>
> system-controller {
> compatible = "samsung,exynos4412-pmu", "syscon";
> clock-names = "clkout0", "clkout1", "clkout2", "clkout3",
> "clkout4", "clkout8", "clkout9";
> clocks = <&clock CLK_OUT_DMC>, <&clock CLK_OUT_TOP>,
> <&clock CLK_OUT_LEFTBUS>, <&clock CLK_OUT_RIGHTBUS>,
> <&clock CLK_OUT_CPU>, <&clock CLK_XXTI>, <&clock CLK_XUSBXTI>;
> }
>
> The bindings never required any specific ordering. Also the driver
> just go through all indices and parses them.
>
> Your second syntax fails:
> Documentation/devicetree/bindings/arm/samsung/pmu.yaml:
> properties:clock-names:items: {'enum': ['clkout0', 'clkout1',
> 'clkout2', 'clkout3', 'clkout4', 'clkout5', 'clkout6', 'clkout7',
> 'clkout8', 'clkout9', 'clkout10', 'clkout11', 'clkout12', 'clkout13',
> 'clkout14', 'clkout15', 'clkout16']} is not of type 'array'
Update dt-schema and try again. It should be allowed now. You'll also
need to define minItems and maxItems though.
Rob
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [Xen-devel] [PATCH] ARM: xen: unexport HYPERVISOR_platform_op function
From: Andrew Cooper @ 2019-09-06 15:55 UTC (permalink / raw)
To: Arnd Bergmann, Stefano Stabellini
Cc: Emil Velikov, Russell King, Denis Efremov, linux-kernel,
Masahiro Yamada, xen-devel, linux-arm-kernel
In-Reply-To: <20190906153948.2160342-1-arnd@arndb.de>
On 06/09/2019 16:39, Arnd Bergmann wrote:
> HYPERVISOR_platform_op() is an inline function and should not
> be exported. Since commit 15bfc2348d54 ("modpost: check for
> static EXPORT_SYMBOL* functions"), this causes a warning:
>
> WARNING: "HYPERVISOR_platform_op" [vmlinux] is a static EXPORT_SYMBOL_GPL
>
> Remove the extraneous export.
>
> Fixes: 15bfc2348d54 ("modpost: check for static EXPORT_SYMBOL* functions")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Something is wonky. That symbol is (/ really ought to be) in the
hypercall page and most definitely not inline.
Which tree is that changeset from? I can't find the SHA.
I hate to open a separate can of worms, but why are they tagged GPL?
The Xen hypercall ABI, like the Linux syscall ABI, are specifically not
GPL. Xen has as something very similar to (and probably derived from)
the Linux-syscall-note exception.
~Andrew
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v3 1/2] ASoC: dt-bindings: Convert Allwinner A10 codec to a schema
From: Rob Herring @ 2019-09-06 15:58 UTC (permalink / raw)
To: Maxime Ripard
Cc: Mark Rutland, devicetree, Linux-ALSA, Liam Girdwood, Chen-Yu Tsai,
Mark Brown, Frank Rowand,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE
In-Reply-To: <20190906151221.3148-1-mripard@kernel.org>
On Fri, Sep 6, 2019 at 4:12 PM Maxime Ripard <mripard@kernel.org> wrote:
>
> From: Maxime Ripard <maxime.ripard@bootlin.com>
>
> The Allwinner SoCs have an embedded audio codec that is supported in Linux,
> with a matching Device Tree binding.
>
> Now that we have the DT validation in place, let's convert the device tree
> bindings for that controller over to a YAML schemas.
>
> Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
>
> ---
>
> Changes from v2:
> - Change the audio-routing values to an enum, and enforce boundaries on
> the size
> - Add restrictions to the possible values of audio-routing
>
> Changes from v1:
> - Fix subject prefix
> ---
> .../sound/allwinner,sun4i-a10-codec.yaml | 262 ++++++++++++++++++
> .../devicetree/bindings/sound/sun4i-codec.txt | 94 -------
> 2 files changed, 262 insertions(+), 94 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/sound/allwinner,sun4i-a10-codec.yaml
> delete mode 100644 Documentation/devicetree/bindings/sound/sun4i-codec.txt
>
> diff --git a/Documentation/devicetree/bindings/sound/allwinner,sun4i-a10-codec.yaml b/Documentation/devicetree/bindings/sound/allwinner,sun4i-a10-codec.yaml
> new file mode 100644
> index 000000000000..faa75b91c072
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/sound/allwinner,sun4i-a10-codec.yaml
> @@ -0,0 +1,262 @@
> +# SPDX-License-Identifier: GPL-2.0
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/sound/allwinner,sun4i-a10-codec.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Allwinner A10 Codec Device Tree Bindings
> +
> +maintainers:
> + - Chen-Yu Tsai <wens@csie.org>
> + - Maxime Ripard <maxime.ripard@bootlin.com>
> +
> +properties:
> + "#sound-dai-cells":
> + const: 0
> +
> + compatible:
> + enum:
> + - allwinner,sun4i-a10-codec
> + - allwinner,sun6i-a31-codec
> + - allwinner,sun7i-a20-codec
> + - allwinner,sun8i-a23-codec
> + - allwinner,sun8i-h3-codec
> + - allwinner,sun8i-v3s-codec
> +
> + reg:
> + maxItems: 1
> +
> + interrupts:
> + maxItems: 1
> +
> + clocks:
> + items:
> + - description: Bus Clock
> + - description: Module Clock
> +
> + clock-names:
> + items:
> + - const: apb
> + - const: codec
> +
> + dmas:
> + items:
> + - description: RX DMA Channel
> + - description: TX DMA Channel
> +
> + dma-names:
> + items:
> + - const: rx
> + - const: tx
> +
> + resets:
> + maxItems: 1
> +
> + allwinner,audio-routing:
> + description: |-
> + A list of the connections between audio components. Each entry
> + is a pair of strings, the first being the connection's sink, the
> + second being the connection's source.
> + allOf:
> + - $ref: /schemas/types.yaml#definitions/non-unique-string-array
> + - minItems: 2
> + maxItems: 18
> + enum:
> + # Audio Pins on the SoC
> + - HP
> + - HPCOM
> + - LINEIN
> + - LINEOUT
> + - MIC1
> + - MIC2
> + - MIC3
> +
> + # Microphone Biases from the SoC
> + - HBIAS
> + - MBIAS
> +
> + # Board Connectors
> + - Headphone
> + - Headset Mic
> + - Line In
> + - Line Out
> + - Mic
> + - Speaker
> +
> + allwinner,codec-analog-controls:
> + $ref: /schemas/types.yaml#/definitions/phandle
> + description: Phandle to the codec analog controls in the PRCM
> +
> + allwinner,pa-gpios:
> + description: GPIO to enable the external amplifier
> +
> +required:
> + - "#sound-dai-cells"
> + - compatible
> + - reg
> + - interrupts
> + - clocks
> + - clock-names
> + - dmas
> + - dma-names
> +
> +allOf:
> + - if:
> + properties:
> + compatible:
> + enum:
> + - allwinner,sun6i-a31-codec
> + - allwinner,sun8i-a23-codec
> + - allwinner,sun8i-h3-codec
> + - allwinner,sun8i-v3s-codec
> +
> + then:
> + if:
> + properties:
> + compatible:
> + const: allwinner,sun6i-a31-codec
> +
> + then:
> + required:
> + - resets
> + - allwinner,audio-routing
> +
> + else:
> + required:
> + - resets
> + - allwinner,audio-routing
> + - allwinner,codec-analog-controls
> +
> + - if:
> + properties:
> + compatible:
> + enum:
> + - allwinner,sun6i-a31-codec
> +
> + then:
> + properties:
> + allwinner,audio-routing:
> + enum:
> + - HP
> + - HPCOM
> + - LINEIN
> + - LINEOUT
> + - MIC1
> + - MIC2
> + - MIC3
> + - HBIAS
> + - MBIAS
> + - Headphone
> + - Headset Mic
> + - Line In
> + - Line Out
> + - Mic
> + - Speaker
This looks like the same list as the default...
> +
> + - if:
> + properties:
> + compatible:
> + enum:
> + - allwinner,sun8i-a23-codec
> +
> + then:
> + properties:
> + allwinner,audio-routing:
> + enum:
> + - HP
> + - HPCOM
> + - LINEIN
> + - MIC1
> + - MIC2
> + - HBIAS
> + - MBIAS
> + - Headphone
> + - Headset Mic
> + - Line In
> + - Line Out
> + - Mic
> + - Speaker
> +
> + - if:
> + properties:
> + compatible:
> + enum:
> + - allwinner,sun8i-h3-codec
> +
> + then:
> + properties:
> + allwinner,audio-routing:
> + enum:
> + - HP
> + - HPCOM
> + - LINEIN
> + - LINEOUT
> + - MIC1
> + - MIC2
> + - HBIAS
> + - MBIAS
> + - Headphone
> + - Headset Mic
> + - Line In
> + - Line Out
> + - Mic
> + - Speaker
> +
> + - if:
> + properties:
> + compatible:
> + enum:
> + - allwinner,sun8i-v3s-codec
> +
> + then:
> + properties:
> + allwinner,audio-routing:
> + enum:
> + - HP
> + - HPCOM
> + - MIC1
> + - HBIAS
> + - Headphone
> + - Headset Mic
> + - Line In
> + - Line Out
> + - Mic
> + - Speaker
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + codec@1c22c00 {
> + #sound-dai-cells = <0>;
> + compatible = "allwinner,sun7i-a20-codec";
> + reg = <0x01c22c00 0x40>;
> + interrupts = <0 30 4>;
> + clocks = <&apb0_gates 0>, <&codec_clk>;
> + clock-names = "apb", "codec";
> + dmas = <&dma 0 19>, <&dma 0 19>;
> + dma-names = "rx", "tx";
> + };
> +
> + - |
> + codec@1c22c00 {
> + #sound-dai-cells = <0>;
> + compatible = "allwinner,sun6i-a31-codec";
> + reg = <0x01c22c00 0x98>;
> + interrupts = <0 29 4>;
> + clocks = <&ccu 61>, <&ccu 135>;
> + clock-names = "apb", "codec";
> + resets = <&ccu 42>;
> + dmas = <&dma 15>, <&dma 15>;
> + dma-names = "rx", "tx";
> + allwinner,audio-routing =
> + "Headphone", "HP",
> + "Speaker", "LINEOUT",
> + "LINEIN", "Line In",
> + "MIC1", "MBIAS",
> + "MIC1", "Mic",
> + "MIC2", "HBIAS",
> + "MIC2", "Headset Mic";
> + };
> +
> +...
> diff --git a/Documentation/devicetree/bindings/sound/sun4i-codec.txt b/Documentation/devicetree/bindings/sound/sun4i-codec.txt
> deleted file mode 100644
> index 66579bbd3294..000000000000
> --- a/Documentation/devicetree/bindings/sound/sun4i-codec.txt
> +++ /dev/null
> @@ -1,94 +0,0 @@
> -* Allwinner A10 Codec
> -
> -Required properties:
> -- compatible: must be one of the following compatibles:
> - - "allwinner,sun4i-a10-codec"
> - - "allwinner,sun6i-a31-codec"
> - - "allwinner,sun7i-a20-codec"
> - - "allwinner,sun8i-a23-codec"
> - - "allwinner,sun8i-h3-codec"
> - - "allwinner,sun8i-v3s-codec"
> -- reg: must contain the registers location and length
> -- interrupts: must contain the codec interrupt
> -- dmas: DMA channels for tx and rx dma. See the DMA client binding,
> - Documentation/devicetree/bindings/dma/dma.txt
> -- dma-names: should include "tx" and "rx".
> -- clocks: a list of phandle + clock-specifer pairs, one for each entry
> - in clock-names.
> -- clock-names: should contain the following:
> - - "apb": the parent APB clock for this controller
> - - "codec": the parent module clock
> -
> -Optional properties:
> -- allwinner,pa-gpios: gpio to enable external amplifier
> -
> -Required properties for the following compatibles:
> - - "allwinner,sun6i-a31-codec"
> - - "allwinner,sun8i-a23-codec"
> - - "allwinner,sun8i-h3-codec"
> - - "allwinner,sun8i-v3s-codec"
> -- resets: phandle to the reset control for this device
> -- allwinner,audio-routing: A list of the connections between audio components.
> - Each entry is a pair of strings, the first being the
> - connection's sink, the second being the connection's
> - source. Valid names include:
> -
> - Audio pins on the SoC:
> - "HP"
> - "HPCOM"
> - "LINEIN" (not on sun8i-v3s)
> - "LINEOUT" (not on sun8i-a23 or sun8i-v3s)
> - "MIC1"
> - "MIC2" (not on sun8i-v3s)
> - "MIC3" (sun6i-a31 only)
> -
> - Microphone biases from the SoC:
> - "HBIAS"
> - "MBIAS" (not on sun8i-v3s)
> -
> - Board connectors:
> - "Headphone"
> - "Headset Mic"
> - "Line In"
> - "Line Out"
> - "Mic"
> - "Speaker"
> -
> -Required properties for the following compatibles:
> - - "allwinner,sun8i-a23-codec"
> - - "allwinner,sun8i-h3-codec"
> - - "allwinner,sun8i-v3s-codec"
> -- allwinner,codec-analog-controls: A phandle to the codec analog controls
> - block in the PRCM.
> -
> -Example:
> -codec: codec@1c22c00 {
> - #sound-dai-cells = <0>;
> - compatible = "allwinner,sun7i-a20-codec";
> - reg = <0x01c22c00 0x40>;
> - interrupts = <0 30 4>;
> - clocks = <&apb0_gates 0>, <&codec_clk>;
> - clock-names = "apb", "codec";
> - dmas = <&dma 0 19>, <&dma 0 19>;
> - dma-names = "rx", "tx";
> -};
> -
> -codec: codec@1c22c00 {
> - #sound-dai-cells = <0>;
> - compatible = "allwinner,sun6i-a31-codec";
> - reg = <0x01c22c00 0x98>;
> - interrupts = <GIC_SPI 29 IRQ_TYPE_LEVEL_HIGH>;
> - clocks = <&ccu CLK_APB1_CODEC>, <&ccu CLK_CODEC>;
> - clock-names = "apb", "codec";
> - resets = <&ccu RST_APB1_CODEC>;
> - dmas = <&dma 15>, <&dma 15>;
> - dma-names = "rx", "tx";
> - allwinner,audio-routing =
> - "Headphone", "HP",
> - "Speaker", "LINEOUT",
> - "LINEIN", "Line In",
> - "MIC1", "MBIAS",
> - "MIC1", "Mic",
> - "MIC2", "HBIAS",
> - "MIC2", "Headset Mic";
> -};
> --
> 2.21.0
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [Xen-devel] [PATCH] ARM: xen: unexport HYPERVISOR_platform_op function
From: Jan Beulich @ 2019-09-06 15:59 UTC (permalink / raw)
To: Andrew Cooper
Cc: Stefano Stabellini, Arnd Bergmann, Emil Velikov, Russell King,
Denis Efremov, linux-kernel, Masahiro Yamada, xen-devel,
linux-arm-kernel
In-Reply-To: <7abad95e-ea47-c068-d91c-ba503f2530b9@citrix.com>
On 06.09.2019 17:55, Andrew Cooper wrote:
> On 06/09/2019 16:39, Arnd Bergmann wrote:
>> HYPERVISOR_platform_op() is an inline function and should not
>> be exported. Since commit 15bfc2348d54 ("modpost: check for
>> static EXPORT_SYMBOL* functions"), this causes a warning:
>>
>> WARNING: "HYPERVISOR_platform_op" [vmlinux] is a static EXPORT_SYMBOL_GPL
>>
>> Remove the extraneous export.
>>
>> Fixes: 15bfc2348d54 ("modpost: check for static EXPORT_SYMBOL* functions")
>> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>
> Something is wonky. That symbol is (/ really ought to be) in the
> hypercall page and most definitely not inline.
It's HYPERVISOR_platform_op_raw that's in the hypercall page afaics.
Jan
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v3 06/17] arm64, hibernate: add trans_pgd public functions
From: Pavel Tatashin @ 2019-09-06 16:00 UTC (permalink / raw)
To: James Morse
Cc: Sasha Levin, Mark Rutland, Vladimir Murzin, Jonathan Corbet,
Marc Zyngier, Catalin Marinas, Bhupesh Sharma, kexec mailing list,
LKML, James Morris, linux-mm, Eric W. Biederman, Matthias Brugger,
will, Linux ARM
In-Reply-To: <bcc3f71f-97d2-dff4-c55a-4798c6e2bede@arm.com>
On Fri, Sep 6, 2019 at 11:18 AM James Morse <james.morse@arm.com> wrote:
>
> Hi Pavel,
>
> On 21/08/2019 19:31, Pavel Tatashin wrote:
> > trans_pgd_create_copy() and trans_pgd_map_page() are going to be
> > the basis for public interface of new subsystem that handles page
>
> Please don't call this a subsystem. 'sound' and 'mm' are subsystems, this is just some
> shared code.
Sounds good: just could not find a better term to call trans_pgd_*. I
won't fix log commits.
>
> > tables for cases which are between kernels: kexec, and hibernate.
>
> Even though you've baked the get_safe_page() calls into trans_pgd_map_page()?
It is getting removed later. Just for a cleaner migration to new
place, get_safe_page() is included for now.
>
>
> > diff --git a/arch/arm64/kernel/hibernate.c b/arch/arm64/kernel/hibernate.c
> > index 750ecc7f2cbe..2e29d620b56c 100644
> > --- a/arch/arm64/kernel/hibernate.c
> > +++ b/arch/arm64/kernel/hibernate.c
> > @@ -182,39 +182,15 @@ int arch_hibernation_header_restore(void *addr)
>
> > +int trans_pgd_map_page(pgd_t *trans_pgd, void *page,
> > + unsigned long dst_addr,
> > + pgprot_t pgprot)
>
> If this thing is going to be exposed, its name should reflect that its creating a set of
> page tables, to map a single page.
>
> A function called 'map_page' with this prototype should 'obviously' map @page at @dst_addr
> in @trans_pgd using the provided @pgprot... but it doesn't.
Answered below...
>
> This is what 'create' was doing in the old name, if that wasn't obvious, its because
> naming things is hard!
> | trans_create_single_page_mapping()?
>
> (might be too verbose)
>
> I think this bites you in patch 8, where you 'generalise' this.
The new naming makes more sense to me. The old code had function named:
create_safe_exec_page()
It was doing four things: 1. creating the actual page via provided
allocator, 2. copying content from the provided page to new page, 3
creating a new page table. 4 mapping it to a new page table at
specified destination address
After, I generalize this the function the prototype looks like this:
int trans_pgd_map_page(struct trans_pgd_info *info, pgd_t *trans_pgd,
void *page, unsigned long
dst_addr, pgprot_t pgprot)
The function only does the "4" from the old code: map the specified
page at dst_addr. The trans_pgd is already created. Of course, and
mapping function will have to allocate missing tables in the page
tables when necessary.
> > + return 0;
> > +}
> > +
> > +/*
> > + * Copies length bytes, starting at src_start into an new page,
> > + * perform cache maintentance, then maps it at the specified address low
>
> Could you fix the spelling of maintenance as git thinks you've moved it?
I will.
Thank you,
Pasha
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [Xen-devel] [PATCH] ARM: xen: unexport HYPERVISOR_platform_op function
From: Arnd Bergmann @ 2019-09-06 16:00 UTC (permalink / raw)
To: Andrew Cooper
Cc: Stefano Stabellini, Emil Velikov, Russell King, Denis Efremov,
linux-kernel@vger.kernel.org, Masahiro Yamada, xen-devel,
Linux ARM
In-Reply-To: <7abad95e-ea47-c068-d91c-ba503f2530b9@citrix.com>
On Fri, Sep 6, 2019 at 5:55 PM Andrew Cooper <andrew.cooper3@citrix.com> wrote:
>
> On 06/09/2019 16:39, Arnd Bergmann wrote:
> > HYPERVISOR_platform_op() is an inline function and should not
> > be exported. Since commit 15bfc2348d54 ("modpost: check for
> > static EXPORT_SYMBOL* functions"), this causes a warning:
> >
> > WARNING: "HYPERVISOR_platform_op" [vmlinux] is a static EXPORT_SYMBOL_GPL
> >
> > Remove the extraneous export.
> >
> > Fixes: 15bfc2348d54 ("modpost: check for static EXPORT_SYMBOL* functions")
> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>
> Something is wonky. That symbol is (/ really ought to be) in the
> hypercall page and most definitely not inline.
>
> Which tree is that changeset from? I can't find the SHA.
This is from linux-next, I think from the kbuild tree.
Arnd
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH 1/3] ARM: omap2plus_defconfig: Fix missing video
From: Tony Lindgren @ 2019-09-06 16:17 UTC (permalink / raw)
To: Adam Ford
Cc: Mark Rutland, devicetree, Russell King, linux-kernel, Rob Herring,
Benoît Cousson, linux-omap, linux-arm-kernel
In-Reply-To: <20190828183351.822-1-aford173@gmail.com>
* Adam Ford <aford173@gmail.com> [190828 11:34]:
> When the panel-dpi driver was removed, the simple-panels driver
> was never enabled, so anyone who used the panel-dpi driver lost
> video, and those who used it inconjunction with simple-panels
> would have to manually enable CONFIG_DRM_PANEL_SIMPLE.
>
> This patch makes CONFIG_DRM_PANEL_SIMPLE a module in the same
> way the deprecated panel-dpi was.
>
> Fixes: 8bf4b1621178 ("drm/omap: Remove panel-dpi driver")
Applying all three into fixes. Not going to send out a pull
request for the fixes until next week so it may not land
until after the merge window starts. But looks like these
have been broken since 2018, so not super urgent.
Regards,
Tony
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ 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