* Re: [PATCH 5/6] powerpc/mm/64s/hash: Add real-mode change_memory_range() for hash LPAR
From: Daniel Axtens @ 2021-02-19 2:43 UTC (permalink / raw)
To: Michael Ellerman, linuxppc-dev; +Cc: aneesh.kumar
In-Reply-To: <20210211135130.3474832-5-mpe@ellerman.id.au>
Michael Ellerman <mpe@ellerman.id.au> writes:
> When we enabled STRICT_KERNEL_RWX we received some reports of boot
> failures when using the Hash MMU and running under phyp. The crashes
> are intermittent, and often exhibit as a completely unresponsive
> system, or possibly an oops.
>
> One example, which was caught in xmon:
>
> [ 14.068327][ T1] devtmpfs: mounted
> [ 14.069302][ T1] Freeing unused kernel memory: 5568K
> [ 14.142060][ T347] BUG: Unable to handle kernel instruction fetch
> [ 14.142063][ T1] Run /sbin/init as init process
> [ 14.142074][ T347] Faulting instruction address: 0xc000000000004400
> cpu 0x2: Vector: 400 (Instruction Access) at [c00000000c7475e0]
> pc: c000000000004400: exc_virt_0x4400_instruction_access+0x0/0x80
> lr: c0000000001862d4: update_rq_clock+0x44/0x110
> sp: c00000000c747880
> msr: 8000000040001031
> current = 0xc00000000c60d380
> paca = 0xc00000001ec9de80 irqmask: 0x03 irq_happened: 0x01
> pid = 347, comm = kworker/2:1
> ...
> enter ? for help
> [c00000000c747880] c0000000001862d4 update_rq_clock+0x44/0x110 (unreliable)
> [c00000000c7478f0] c000000000198794 update_blocked_averages+0xb4/0x6d0
> [c00000000c7479f0] c000000000198e40 update_nohz_stats+0x90/0xd0
> [c00000000c747a20] c0000000001a13b4 _nohz_idle_balance+0x164/0x390
> [c00000000c747b10] c0000000001a1af8 newidle_balance+0x478/0x610
> [c00000000c747be0] c0000000001a1d48 pick_next_task_fair+0x58/0x480
> [c00000000c747c40] c000000000eaab5c __schedule+0x12c/0x950
> [c00000000c747cd0] c000000000eab3e8 schedule+0x68/0x120
> [c00000000c747d00] c00000000016b730 worker_thread+0x130/0x640
> [c00000000c747da0] c000000000174d50 kthread+0x1a0/0x1b0
> [c00000000c747e10] c00000000000e0f0 ret_from_kernel_thread+0x5c/0x6c
>
> This shows that CPU 2, which was idle, woke up and then appears to
> randomly take an instruction fault on a completely valid area of
> kernel text.
>
> The cause turns out to be the call to hash__mark_rodata_ro(), late in
> boot. Due to the way we layout text and rodata, that function actually
> changes the permissions for all of text and rodata to read-only plus
> execute.
>
> To do the permission change we use a hypervisor call, H_PROTECT. On
> phyp that appears to be implemented by briefly removing the mapping of
> the kernel text, before putting it back with the updated permissions.
> If any other CPU is executing during that window, it will see spurious
> faults on the kernel text and/or data, leading to crashes.
Jordan asked why we saw this on phyp but not under KVM? We had a look at
book3s_hv_rm_mmu.c but the code is a bit too obtuse for me to reason
about!
Nick suggests that the KVM hypervisor is invalidating the HPTE, but
because we run guests in VPM mode, the hypervisor would catch the page
fault and not reflect it down to the guest. It looks like Linux-as-a-HV
will take HPTE_V_HVLOCK, and then because it's running in VPM mode, the
hypervisor will catch the fault and not pass it to the guest. But if
phyp runs with VPM mode off, the guest will see the fault before the
hypervisor. (we think this is what's going on anyway.)
We spent a while pondering if phyp is doing something buggy or not...
Looking at the PAPR definition of H_PROTECT, that claims the hypervisor
will do the 'architected “Modifying a Page Table Entry General Case”
sequence'. s 5.10.1.2 of Book IIIS of the ISAv3 defines that, and the
non-atomic hardware sequence does indeed modify the PTE by going through
the invalid state. So it looks like if phyp is running without VPM mode
it's technically not buggy.
Hopefully I'll get to have a look at the rest of the patch shortly!
Kind regards,
Daniel
> To fix it we use stop machine to collect all other CPUs, and then have
> them drop into real mode (MMU off), while we change the mapping. That
> way they are unaffected by the mapping temporarily disappearing.
>
> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
> ---
> arch/powerpc/mm/book3s64/hash_pgtable.c | 105 +++++++++++++++++++++++-
> 1 file changed, 104 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/mm/book3s64/hash_pgtable.c b/arch/powerpc/mm/book3s64/hash_pgtable.c
> index 3663d3cdffac..01de985df2c4 100644
> --- a/arch/powerpc/mm/book3s64/hash_pgtable.c
> +++ b/arch/powerpc/mm/book3s64/hash_pgtable.c
> @@ -8,6 +8,7 @@
> #include <linux/sched.h>
> #include <linux/mm_types.h>
> #include <linux/mm.h>
> +#include <linux/stop_machine.h>
>
> #include <asm/sections.h>
> #include <asm/mmu.h>
> @@ -400,6 +401,19 @@ EXPORT_SYMBOL_GPL(hash__has_transparent_hugepage);
> #endif /* CONFIG_TRANSPARENT_HUGEPAGE */
>
> #ifdef CONFIG_STRICT_KERNEL_RWX
> +
> +struct change_memory_parms {
> + unsigned long start, end, newpp;
> + unsigned int step, nr_cpus, master_cpu;
> + atomic_t cpu_counter;
> +};
> +
> +// We'd rather this was on the stack but it has to be in the RMO
> +static struct change_memory_parms chmem_parms;
> +
> +// And therefore we need a lock to protect it from concurrent use
> +static DEFINE_MUTEX(chmem_lock);
> +
> static void change_memory_range(unsigned long start, unsigned long end,
> unsigned int step, unsigned long newpp)
> {
> @@ -414,6 +428,73 @@ static void change_memory_range(unsigned long start, unsigned long end,
> mmu_kernel_ssize);
> }
>
> +static int notrace chmem_secondary_loop(struct change_memory_parms *parms)
> +{
> + unsigned long msr, tmp, flags;
> + int *p;
> +
> + p = &parms->cpu_counter.counter;
> +
> + local_irq_save(flags);
> + __hard_EE_RI_disable();
> +
> + asm volatile (
> + // Switch to real mode and leave interrupts off
> + "mfmsr %[msr] ;"
> + "li %[tmp], %[MSR_IR_DR] ;"
> + "andc %[tmp], %[msr], %[tmp] ;"
> + "mtmsrd %[tmp] ;"
> +
> + // Tell the master we are in real mode
> + "1: "
> + "lwarx %[tmp], 0, %[p] ;"
> + "addic %[tmp], %[tmp], -1 ;"
> + "stwcx. %[tmp], 0, %[p] ;"
> + "bne- 1b ;"
> +
> + // Spin until the counter goes to zero
> + "2: ;"
> + "lwz %[tmp], 0(%[p]) ;"
> + "cmpwi %[tmp], 0 ;"
> + "bne- 2b ;"
> +
> + // Switch back to virtual mode
> + "mtmsrd %[msr] ;"
> +
> + : // outputs
> + [msr] "=&r" (msr), [tmp] "=&b" (tmp), "+m" (*p)
> + : // inputs
> + [p] "b" (p), [MSR_IR_DR] "i" (MSR_IR | MSR_DR)
> + : // clobbers
> + "cc", "xer"
> + );
> +
> + local_irq_restore(flags);
> +
> + return 0;
> +}
> +
> +static int change_memory_range_fn(void *data)
> +{
> + struct change_memory_parms *parms = data;
> +
> + if (parms->master_cpu != smp_processor_id())
> + return chmem_secondary_loop(parms);
> +
> + // Wait for all but one CPU (this one) to call-in
> + while (atomic_read(&parms->cpu_counter) > 1)
> + barrier();
> +
> + change_memory_range(parms->start, parms->end, parms->step, parms->newpp);
> +
> + mb();
> +
> + // Signal the other CPUs that we're done
> + atomic_dec(&parms->cpu_counter);
> +
> + return 0;
> +}
> +
> static bool hash__change_memory_range(unsigned long start, unsigned long end,
> unsigned long newpp)
> {
> @@ -428,7 +509,29 @@ static bool hash__change_memory_range(unsigned long start, unsigned long end,
> if (start >= end)
> return false;
>
> - change_memory_range(start, end, step, newpp);
> + if (firmware_has_feature(FW_FEATURE_LPAR)) {
> + mutex_lock(&chmem_lock);
> +
> + chmem_parms.start = start;
> + chmem_parms.end = end;
> + chmem_parms.step = step;
> + chmem_parms.newpp = newpp;
> + chmem_parms.master_cpu = smp_processor_id();
> +
> + cpus_read_lock();
> +
> + atomic_set(&chmem_parms.cpu_counter, num_online_cpus());
> +
> + // Ensure state is consistent before we call the other CPUs
> + mb();
> +
> + stop_machine_cpuslocked(change_memory_range_fn, &chmem_parms,
> + cpu_online_mask);
> +
> + cpus_read_unlock();
> + mutex_unlock(&chmem_lock);
> + } else
> + change_memory_range(start, end, step, newpp);
>
> return true;
> }
> --
> 2.25.1
^ permalink raw reply
* Re: [PATCH 4/6] powerpc/mm/64s/hash: Factor out change_memory_range()
From: Daniel Axtens @ 2021-02-19 2:08 UTC (permalink / raw)
To: Michael Ellerman, linuxppc-dev; +Cc: aneesh.kumar
In-Reply-To: <20210211135130.3474832-4-mpe@ellerman.id.au>
Michael Ellerman <mpe@ellerman.id.au> writes:
> Pull the loop calling hpte_updateboltedpp() out of
> hash__change_memory_range() into a helper function. We need it to be a
> separate function for the next patch.
>
> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
> ---
> arch/powerpc/mm/book3s64/hash_pgtable.c | 23 +++++++++++++++--------
> 1 file changed, 15 insertions(+), 8 deletions(-)
>
> diff --git a/arch/powerpc/mm/book3s64/hash_pgtable.c b/arch/powerpc/mm/book3s64/hash_pgtable.c
> index 03819c259f0a..3663d3cdffac 100644
> --- a/arch/powerpc/mm/book3s64/hash_pgtable.c
> +++ b/arch/powerpc/mm/book3s64/hash_pgtable.c
> @@ -400,10 +400,23 @@ EXPORT_SYMBOL_GPL(hash__has_transparent_hugepage);
> #endif /* CONFIG_TRANSPARENT_HUGEPAGE */
>
> #ifdef CONFIG_STRICT_KERNEL_RWX
> +static void change_memory_range(unsigned long start, unsigned long end,
> + unsigned int step, unsigned long newpp)
Looking at the call paths, this gets called only in bare metal, not
virtualised: should the name reflect that?
> +{
> + unsigned long idx;
> +
> + pr_debug("Changing page protection on range 0x%lx-0x%lx, to 0x%lx, step 0x%x\n",
> + start, end, newpp, step);
> +
> + for (idx = start; idx < end; idx += step)
> + /* Not sure if we can do much with the return value */
Hmm, I realise this comment isn't changed, but it did make me wonder
what the return value!
It turns out that the function doesn't actually return anything.
Tracking back the history of hpte_updateboltedpp, it looks like it has
not had a return value since the start of git history:
^1da177e4c3f4 include/asm-ppc64/machdep.h void (*hpte_updateboltedpp)(unsigned long newpp,
3c726f8dee6f5 include/asm-powerpc/machdep.h unsigned long ea,
1189be6508d45 include/asm-powerpc/machdep.h int psize, int ssize);
The comment comes from commit cd65d6971334 ("powerpc/mm/hash: Implement
mark_rodata_ro() for hash") where Balbir added the comment, but again I
can't figure out what sort of return value there would be to ignore.
Should we drop the comment? (or return something from hpte_updateboltedpp)
> + mmu_hash_ops.hpte_updateboltedpp(newpp, idx, mmu_linear_psize,
> + mmu_kernel_ssize);
> +}
> +
> static bool hash__change_memory_range(unsigned long start, unsigned long end,
> unsigned long newpp)
> {
> - unsigned long idx;
> unsigned int step, shift;
>
> shift = mmu_psize_defs[mmu_linear_psize].shift;
> @@ -415,13 +428,7 @@ static bool hash__change_memory_range(unsigned long start, unsigned long end,
> if (start >= end)
> return false;
>
> - pr_debug("Changing page protection on range 0x%lx-0x%lx, to 0x%lx, step 0x%x\n",
> - start, end, newpp, step);
> -
> - for (idx = start; idx < end; idx += step)
> - /* Not sure if we can do much with the return value */
> - mmu_hash_ops.hpte_updateboltedpp(newpp, idx, mmu_linear_psize,
> - mmu_kernel_ssize);
> + change_memory_range(start, end, step, newpp);
Looking at how change_memory_range is called, step is derived by:
shift = mmu_psize_defs[mmu_linear_psize].shift;
step = 1 << shift;
We probably therefore don't really need to pass step in to
change_memory_range. Having said that, I'm not sure it would really be that
much tidier to compute step in change_memory_range, especially since we
also need step for the other branch in hash__change_memory_range.
Beyond that it all looks reasonable to me!
I also checked that the loop operations made sense, I think they do - we
cover from start inclusive to end exclusive and the alignment is done
before we call into change_memory_range.
Regards,
Daniel
> return true;
> }
> --
> 2.25.1
^ permalink raw reply
* Re: [PATCH] of: error: 'const struct kimage' has no member named 'arch'
From: Thiago Jung Bauermann @ 2021-02-19 1:13 UTC (permalink / raw)
To: Lakshmi Ramasubramanian
Cc: sashal, robh, sfr, gregkh, linuxppc-dev, linux-kernel, Mimi Zohar,
takahiro.akashi, devicetree, james.morse, catalin.marinas, joe,
linux-integrity, will, linux-arm-kernel
In-Reply-To: <8b8c0b70-c7ab-33f3-b66c-9ea03388497b@linux.microsoft.com>
Lakshmi Ramasubramanian <nramas@linux.microsoft.com> writes:
> On 2/18/21 4:07 PM, Mimi Zohar wrote:
>
> Hi Mimi,
>
>> On Thu, 2021-02-18 at 14:33 -0800, Lakshmi Ramasubramanian wrote:
>>> of_kexec_alloc_and_setup_fdt() defined in drivers/of/kexec.c builds
>>> a new device tree object that includes architecture specific data
>>> for kexec system call. This should be defined only if the architecture
>>> being built defines kexec architecture structure "struct kimage_arch".
>>>
>>> Define a new boolean config OF_KEXEC that is enabled if
>>> CONFIG_KEXEC_FILE and CONFIG_OF_FLATTREE are enabled, and
>>> the architecture is arm64 or powerpc64. Build drivers/of/kexec.c
>>> if CONFIG_OF_KEXEC is enabled.
>>>
>>> Signed-off-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com>
>>> Fixes: 33488dc4d61f ("of: Add a common kexec FDT setup function")
>>> Reported-by: kernel test robot <lkp@intel.com>
>>> ---
>>> drivers/of/Kconfig | 6 ++++++
>>> drivers/of/Makefile | 7 +------
>>> 2 files changed, 7 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig
>>> index 18450437d5d5..f2e8fa54862a 100644
>>> --- a/drivers/of/Kconfig
>>> +++ b/drivers/of/Kconfig
>>> @@ -100,4 +100,10 @@ config OF_DMA_DEFAULT_COHERENT
>>> # arches should select this if DMA is coherent by default for OF devices
>>> bool
>>> +config OF_KEXEC
>>> + bool
>>> + depends on KEXEC_FILE
>>> + depends on OF_FLATTREE
>>> + default y if ARM64 || PPC64
>>> +
>>> endif # OF
>>> diff --git a/drivers/of/Makefile b/drivers/of/Makefile
>>> index c13b982084a3..287579dd1695 100644
>>> --- a/drivers/of/Makefile
>>> +++ b/drivers/of/Makefile
>>> @@ -13,11 +13,6 @@ obj-$(CONFIG_OF_RESERVED_MEM) += of_reserved_mem.o
>>> obj-$(CONFIG_OF_RESOLVE) += resolver.o
>>> obj-$(CONFIG_OF_OVERLAY) += overlay.o
>>> obj-$(CONFIG_OF_NUMA) += of_numa.o
>>> -
>>> -ifdef CONFIG_KEXEC_FILE
>>> -ifdef CONFIG_OF_FLATTREE
>>> -obj-y += kexec.o
>>> -endif
>>> -endif
>>> +obj-$(CONFIG_OF_KEXEC) += kexec.o
>>> obj-$(CONFIG_OF_UNITTEST) += unittest-data/
>> Is it possible to reuse CONFIG_HAVE_IMA_KEXEC here?
>>
>
> For ppc64 CONFIG_HAVE_IMA_KEXEC is selected when CONFIG_KEXEC_FILE is enabled.
> So I don't see a problem in reusing CONFIG_HAVE_IMA_KEXEC for ppc.
>
> But for arm64, CONFIG_HAVE_IMA_KEXEC is enabled in the final patch in the patch
> set (the one for carrying forward IMA log across kexec for arm64). arm64 calls
> of_kexec_alloc_and_setup_fdt() prior to enabling CONFIG_HAVE_IMA_KEXEC and hence
> breaks the build for arm64.
One problem is that I believe that this patch won't placate the robot,
because IIUC it generates config files at random and this change still
allows hppa and s390 to enable CONFIG_OF_KEXEC.
Perhaps a new CONFIG_HAVE_KIMAGE_ARCH option? Not having that option
would still allow building kexec.o, but would be used inside kexec.c to
avoid accessing kimage.arch members.
--
Thiago Jung Bauermann
IBM Linux Technology Center
^ permalink raw reply
* Re: [PATCH] of: error: 'const struct kimage' has no member named 'arch'
From: Lakshmi Ramasubramanian @ 2021-02-19 0:57 UTC (permalink / raw)
To: Mimi Zohar, bauerman, robh, takahiro.akashi, gregkh, will, joe,
catalin.marinas, mpe, sfr
Cc: sashal, devicetree, linux-kernel, james.morse, linux-integrity,
linuxppc-dev, linux-arm-kernel
In-Reply-To: <c6490f6a126a2f10e3e3445b51ea552a26f896a9.camel@linux.ibm.com>
On 2/18/21 4:07 PM, Mimi Zohar wrote:
Hi Mimi,
> On Thu, 2021-02-18 at 14:33 -0800, Lakshmi Ramasubramanian wrote:
>> of_kexec_alloc_and_setup_fdt() defined in drivers/of/kexec.c builds
>> a new device tree object that includes architecture specific data
>> for kexec system call. This should be defined only if the architecture
>> being built defines kexec architecture structure "struct kimage_arch".
>>
>> Define a new boolean config OF_KEXEC that is enabled if
>> CONFIG_KEXEC_FILE and CONFIG_OF_FLATTREE are enabled, and
>> the architecture is arm64 or powerpc64. Build drivers/of/kexec.c
>> if CONFIG_OF_KEXEC is enabled.
>>
>> Signed-off-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com>
>> Fixes: 33488dc4d61f ("of: Add a common kexec FDT setup function")
>> Reported-by: kernel test robot <lkp@intel.com>
>> ---
>> drivers/of/Kconfig | 6 ++++++
>> drivers/of/Makefile | 7 +------
>> 2 files changed, 7 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig
>> index 18450437d5d5..f2e8fa54862a 100644
>> --- a/drivers/of/Kconfig
>> +++ b/drivers/of/Kconfig
>> @@ -100,4 +100,10 @@ config OF_DMA_DEFAULT_COHERENT
>> # arches should select this if DMA is coherent by default for OF devices
>> bool
>>
>> +config OF_KEXEC
>> + bool
>> + depends on KEXEC_FILE
>> + depends on OF_FLATTREE
>> + default y if ARM64 || PPC64
>> +
>> endif # OF
>> diff --git a/drivers/of/Makefile b/drivers/of/Makefile
>> index c13b982084a3..287579dd1695 100644
>> --- a/drivers/of/Makefile
>> +++ b/drivers/of/Makefile
>> @@ -13,11 +13,6 @@ obj-$(CONFIG_OF_RESERVED_MEM) += of_reserved_mem.o
>> obj-$(CONFIG_OF_RESOLVE) += resolver.o
>> obj-$(CONFIG_OF_OVERLAY) += overlay.o
>> obj-$(CONFIG_OF_NUMA) += of_numa.o
>> -
>> -ifdef CONFIG_KEXEC_FILE
>> -ifdef CONFIG_OF_FLATTREE
>> -obj-y += kexec.o
>> -endif
>> -endif
>> +obj-$(CONFIG_OF_KEXEC) += kexec.o
>>
>> obj-$(CONFIG_OF_UNITTEST) += unittest-data/
>
> Is it possible to reuse CONFIG_HAVE_IMA_KEXEC here?
>
For ppc64 CONFIG_HAVE_IMA_KEXEC is selected when CONFIG_KEXEC_FILE is
enabled. So I don't see a problem in reusing CONFIG_HAVE_IMA_KEXEC for ppc.
But for arm64, CONFIG_HAVE_IMA_KEXEC is enabled in the final patch in
the patch set (the one for carrying forward IMA log across kexec for
arm64). arm64 calls of_kexec_alloc_and_setup_fdt() prior to enabling
CONFIG_HAVE_IMA_KEXEC and hence breaks the build for arm64.
thanks,
-lakshmi
^ permalink raw reply
* Re: [PATCH] of: error: 'const struct kimage' has no member named 'arch'
From: Mimi Zohar @ 2021-02-19 0:07 UTC (permalink / raw)
To: Lakshmi Ramasubramanian, bauerman, robh, takahiro.akashi, gregkh,
will, joe, catalin.marinas, mpe, sfr
Cc: sashal, devicetree, linux-kernel, james.morse, linux-integrity,
linuxppc-dev, linux-arm-kernel
In-Reply-To: <20210218223305.2044-1-nramas@linux.microsoft.com>
On Thu, 2021-02-18 at 14:33 -0800, Lakshmi Ramasubramanian wrote:
> of_kexec_alloc_and_setup_fdt() defined in drivers/of/kexec.c builds
> a new device tree object that includes architecture specific data
> for kexec system call. This should be defined only if the architecture
> being built defines kexec architecture structure "struct kimage_arch".
>
> Define a new boolean config OF_KEXEC that is enabled if
> CONFIG_KEXEC_FILE and CONFIG_OF_FLATTREE are enabled, and
> the architecture is arm64 or powerpc64. Build drivers/of/kexec.c
> if CONFIG_OF_KEXEC is enabled.
>
> Signed-off-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com>
> Fixes: 33488dc4d61f ("of: Add a common kexec FDT setup function")
> Reported-by: kernel test robot <lkp@intel.com>
> ---
> drivers/of/Kconfig | 6 ++++++
> drivers/of/Makefile | 7 +------
> 2 files changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig
> index 18450437d5d5..f2e8fa54862a 100644
> --- a/drivers/of/Kconfig
> +++ b/drivers/of/Kconfig
> @@ -100,4 +100,10 @@ config OF_DMA_DEFAULT_COHERENT
> # arches should select this if DMA is coherent by default for OF devices
> bool
>
> +config OF_KEXEC
> + bool
> + depends on KEXEC_FILE
> + depends on OF_FLATTREE
> + default y if ARM64 || PPC64
> +
> endif # OF
> diff --git a/drivers/of/Makefile b/drivers/of/Makefile
> index c13b982084a3..287579dd1695 100644
> --- a/drivers/of/Makefile
> +++ b/drivers/of/Makefile
> @@ -13,11 +13,6 @@ obj-$(CONFIG_OF_RESERVED_MEM) += of_reserved_mem.o
> obj-$(CONFIG_OF_RESOLVE) += resolver.o
> obj-$(CONFIG_OF_OVERLAY) += overlay.o
> obj-$(CONFIG_OF_NUMA) += of_numa.o
> -
> -ifdef CONFIG_KEXEC_FILE
> -ifdef CONFIG_OF_FLATTREE
> -obj-y += kexec.o
> -endif
> -endif
> +obj-$(CONFIG_OF_KEXEC) += kexec.o
>
> obj-$(CONFIG_OF_UNITTEST) += unittest-data/
Is it possible to reuse CONFIG_HAVE_IMA_KEXEC here?
Mimi
^ permalink raw reply
* Re: linux-next: manual merge of the devicetree tree with the powerpc tree
From: Michael Ellerman @ 2021-02-18 23:28 UTC (permalink / raw)
To: Rob Herring, Stephen Rothwell
Cc: Lakshmi Ramasubramanian, Linux Next Mailing List, PowerPC,
Linux Kernel Mailing List, Hari Bathini
In-Reply-To: <CAL_JsqJ9Ske4hkWn3uo8-nef29MQ1DkNdtE=gxbqj8CKrtQorg@mail.gmail.com>
Rob Herring <robherring2@gmail.com> writes:
> On Thu, Feb 18, 2021 at 5:34 AM Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>> On Thu, 18 Feb 2021 21:44:37 +1100 Michael Ellerman <mpe@ellerman.id.au> wrote:
>> >
>> > I think it just needs this?
>> >
>> > diff --git a/arch/powerpc/kexec/elf_64.c b/arch/powerpc/kexec/elf_64.c
>> > index 87e34611f93d..0492ca6003f3 100644
>> > --- a/arch/powerpc/kexec/elf_64.c
>> > +++ b/arch/powerpc/kexec/elf_64.c
>> > @@ -104,7 +104,7 @@ static void *elf64_load(struct kimage *image, char *kernel_buf,
>> >
>> > fdt = of_kexec_alloc_and_setup_fdt(image, initrd_load_addr,
>> > initrd_len, cmdline,
>> > - fdt_totalsize(initial_boot_params));
>> > + kexec_fdt_totalsize_ppc64(image));
>> > if (!fdt) {
>> > pr_err("Error setting up the new device tree.\n");
>> > ret = -EINVAL;
>> >
>>
>> I thought about that, but the last argument to
>> of_kexec_alloc_and_setup_fdt() is extra_fdt_size and the allocation
>> done is for this:
>>
>> fdt_size = fdt_totalsize(initial_boot_params) +
>> (cmdline ? strlen(cmdline) : 0) +
>> FDT_EXTRA_SPACE +
>> extra_fdt_size;
>>
>> and kexec_fdt_totalsize_ppc64() also includes
>> fdt_totalsize(initial_boot_params) so I was not sure. Maybe
>> kexec_fdt_totalsize_ppc64() needs modification as well?
>
> You're both right. Michael's fix is sufficient for the merge. The only
> risk with a larger size is failing to allocate it, but we're talking
> only 10s of KB. Historically until the commit causing the conflict,
> PPC was just used 2x fdt_totalsize(initial_boot_params). You could
> drop 'fdt_size = fdt_totalsize(initial_boot_params) + (2 *
> COMMAND_LINE_SIZE);' from kexec_fdt_totalsize_ppc64() as well, but
> then the function name is misleading.
>
> Lakshmi can send a follow-up patch to fine tune the size and rename
> kexec_fdt_totalsize_ppc64.
Sounds good.
cheers
^ permalink raw reply
* Re: [PATCH 2/6] powerpc/pseries: Add key to flags in pSeries_lpar_hpte_updateboltedpp()
From: Michael Ellerman @ 2021-02-18 23:25 UTC (permalink / raw)
To: Daniel Axtens, linuxppc-dev; +Cc: aneesh.kumar
In-Reply-To: <87tuqca7vi.fsf@linkitivity.dja.id.au>
Daniel Axtens <dja@axtens.net> writes:
> Michael Ellerman <mpe@ellerman.id.au> writes:
>
>> The flags argument to plpar_pte_protect() (aka. H_PROTECT), includes
>> the key in bits 9-13, but currently we always set those bits to zero.
>>
>> In the past that hasn't been a problem because we always used key 0
>> for the kernel, and updateboltedpp() is only used for kernel mappings.
>>
>> However since commit d94b827e89dc ("powerpc/book3s64/kuap: Use Key 3
>> for kernel mapping with hash translation") we are now inadvertently
>> changing the key (to zero) when we call plpar_pte_protect().
>>
>> That hasn't broken anything because updateboltedpp() is only used for
>> STRICT_KERNEL_RWX, which is currently disabled on 64s due to other
>> bugs.
>>
>> But we want to fix that, so first we need to pass the key correctly to
>> plpar_pte_protect(). In the `newpp` value the low 3 bits of the key
>> are already in the correct spot, but the high 2 bits of the key need
>> to be shifted down.
>>
>> Fixes: d94b827e89dc ("powerpc/book3s64/kuap: Use Key 3 for kernel mapping with hash translation")
>> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
>> ---
>> arch/powerpc/platforms/pseries/lpar.c | 4 +++-
>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/powerpc/platforms/pseries/lpar.c b/arch/powerpc/platforms/pseries/lpar.c
>> index 764170fdb0f7..8bbbddff7226 100644
>> --- a/arch/powerpc/platforms/pseries/lpar.c
>> +++ b/arch/powerpc/platforms/pseries/lpar.c
>> @@ -976,11 +976,13 @@ static void pSeries_lpar_hpte_updateboltedpp(unsigned long newpp,
>> slot = pSeries_lpar_hpte_find(vpn, psize, ssize);
>> BUG_ON(slot == -1);
>>
>> - flags = newpp & 7;
>> + flags = newpp & (HPTE_R_PP | HPTE_R_N);
>> if (mmu_has_feature(MMU_FTR_KERNEL_RO))
>> /* Move pp0 into bit 8 (IBM 55) */
>> flags |= (newpp & HPTE_R_PP0) >> 55;
>>
>> + flags |= ((newpp & HPTE_R_KEY_HI) >> 48) | (newpp & HPTE_R_KEY_LO);
>> +
>
> I'm really confused about how these bits are getting packed into the
> flags parameter. It seems to match how they are unpacked in
> kvmppc_h_pr_protect, but I cannot figure out why they are packed in that
> order, and the LoPAR doesn't seem especially illuminating on this topic
> - although I may have missed the relevant section.
Yeah I agree it's not very clearly specified.
The hcall we're using here is H_PROTECT, which is specified in section
14.5.4.1.6 of LoPAPR v1.1.
It takes a `flags` parameter, and the description for flags says:
* flags: AVPN, pp0, pp1, pp2, key0-key4, n, and for the CMO
option: CMO Option flags as defined in Table 189‚
If you then go to the start of the parent section, 14.5.4.1, on page
405, it says:
Register Linkage (For hcall() tokens 0x04 - 0x18)
* On Call
* R3 function call token
* R4 flags (see Table 178‚ “Page Frame Table Access flags field definition‚” on page 401)
Then you have to go to section 14.5.3, and on page 394 there is a list
of hcalls and their tokens (table 176), and there you can see that
H_PROTECT == 0x18.
Finally you can look at table 178, on page 401, where it specifies the
layout of the bits for the key:
Bit Function
------------------
50-54 | key0-key4
Those are big-endian bit numbers, converting to normal bit numbers you
get bits 9-13, or 0x3e00.
If you look at the kernel source we have:
#define HPTE_R_KEY_HI ASM_CONST(0x3000000000000000)
#define HPTE_R_KEY_LO ASM_CONST(0x0000000000000e00)
So the LO bits are already in the right place, and the HI bits just need
to be shifted down by 48.
Hope that makes it clearer :)
cheers
^ permalink raw reply
* [PATCH] of: error: 'const struct kimage' has no member named 'arch'
From: Lakshmi Ramasubramanian @ 2021-02-18 22:33 UTC (permalink / raw)
To: zohar, bauerman, robh, takahiro.akashi, gregkh, will, joe,
catalin.marinas, mpe, sfr
Cc: sashal, devicetree, linux-kernel, james.morse, linux-integrity,
linuxppc-dev, linux-arm-kernel
of_kexec_alloc_and_setup_fdt() defined in drivers/of/kexec.c builds
a new device tree object that includes architecture specific data
for kexec system call. This should be defined only if the architecture
being built defines kexec architecture structure "struct kimage_arch".
Define a new boolean config OF_KEXEC that is enabled if
CONFIG_KEXEC_FILE and CONFIG_OF_FLATTREE are enabled, and
the architecture is arm64 or powerpc64. Build drivers/of/kexec.c
if CONFIG_OF_KEXEC is enabled.
Signed-off-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com>
Fixes: 33488dc4d61f ("of: Add a common kexec FDT setup function")
Reported-by: kernel test robot <lkp@intel.com>
---
drivers/of/Kconfig | 6 ++++++
drivers/of/Makefile | 7 +------
2 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig
index 18450437d5d5..f2e8fa54862a 100644
--- a/drivers/of/Kconfig
+++ b/drivers/of/Kconfig
@@ -100,4 +100,10 @@ config OF_DMA_DEFAULT_COHERENT
# arches should select this if DMA is coherent by default for OF devices
bool
+config OF_KEXEC
+ bool
+ depends on KEXEC_FILE
+ depends on OF_FLATTREE
+ default y if ARM64 || PPC64
+
endif # OF
diff --git a/drivers/of/Makefile b/drivers/of/Makefile
index c13b982084a3..287579dd1695 100644
--- a/drivers/of/Makefile
+++ b/drivers/of/Makefile
@@ -13,11 +13,6 @@ obj-$(CONFIG_OF_RESERVED_MEM) += of_reserved_mem.o
obj-$(CONFIG_OF_RESOLVE) += resolver.o
obj-$(CONFIG_OF_OVERLAY) += overlay.o
obj-$(CONFIG_OF_NUMA) += of_numa.o
-
-ifdef CONFIG_KEXEC_FILE
-ifdef CONFIG_OF_FLATTREE
-obj-y += kexec.o
-endif
-endif
+obj-$(CONFIG_OF_KEXEC) += kexec.o
obj-$(CONFIG_OF_UNITTEST) += unittest-data/
--
2.30.0
^ permalink raw reply related
* Re: linux-next: manual merge of the devicetree tree with the powerpc tree
From: Stephen Rothwell @ 2021-02-18 20:31 UTC (permalink / raw)
To: Rob Herring
Cc: Linux Kernel Mailing List, Lakshmi Ramasubramanian,
Linux Next Mailing List, PowerPC, Hari Bathini
In-Reply-To: <CAL_JsqJ9Ske4hkWn3uo8-nef29MQ1DkNdtE=gxbqj8CKrtQorg@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 2287 bytes --]
Hi all,
On Thu, 18 Feb 2021 07:52:52 -0600 Rob Herring <robherring2@gmail.com> wrote:
>
> On Thu, Feb 18, 2021 at 5:34 AM Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> >
> > On Thu, 18 Feb 2021 21:44:37 +1100 Michael Ellerman <mpe@ellerman.id.au> wrote:
> > >
> > > I think it just needs this?
> > >
> > > diff --git a/arch/powerpc/kexec/elf_64.c b/arch/powerpc/kexec/elf_64.c
> > > index 87e34611f93d..0492ca6003f3 100644
> > > --- a/arch/powerpc/kexec/elf_64.c
> > > +++ b/arch/powerpc/kexec/elf_64.c
> > > @@ -104,7 +104,7 @@ static void *elf64_load(struct kimage *image, char *kernel_buf,
> > >
> > > fdt = of_kexec_alloc_and_setup_fdt(image, initrd_load_addr,
> > > initrd_len, cmdline,
> > > - fdt_totalsize(initial_boot_params));
> > > + kexec_fdt_totalsize_ppc64(image));
> > > if (!fdt) {
> > > pr_err("Error setting up the new device tree.\n");
> > > ret = -EINVAL;
> > >
> >
> > I thought about that, but the last argument to
> > of_kexec_alloc_and_setup_fdt() is extra_fdt_size and the allocation
> > done is for this:
> >
> > fdt_size = fdt_totalsize(initial_boot_params) +
> > (cmdline ? strlen(cmdline) : 0) +
> > FDT_EXTRA_SPACE +
> > extra_fdt_size;
> >
> > and kexec_fdt_totalsize_ppc64() also includes
> > fdt_totalsize(initial_boot_params) so I was not sure. Maybe
> > kexec_fdt_totalsize_ppc64() needs modification as well?
>
> You're both right. Michael's fix is sufficient for the merge. The only
> risk with a larger size is failing to allocate it, but we're talking
> only 10s of KB. Historically until the commit causing the conflict,
> PPC was just used 2x fdt_totalsize(initial_boot_params). You could
> drop 'fdt_size = fdt_totalsize(initial_boot_params) + (2 *
> COMMAND_LINE_SIZE);' from kexec_fdt_totalsize_ppc64() as well, but
> then the function name is misleading.
>
> Lakshmi can send a follow-up patch to fine tune the size and rename
> kexec_fdt_totalsize_ppc64.
OK, I have mode Michael's suggested change to my resolution from today.
--
Cheers,
Stephen Rothwell
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 484 bytes --]
^ permalink raw reply
* Re: [PATCH] powerpc/4xx: Fix build errors from mfdcr()
From: Feng Tang @ 2021-02-18 13:04 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev
In-Reply-To: <20210218123058.748882-1-mpe@ellerman.id.au>
On Thu, Feb 18, 2021 at 11:30:58PM +1100, Michael Ellerman wrote:
> lkp reported a build error in fsp2.o:
>
> CC arch/powerpc/platforms/44x/fsp2.o
> {standard input}:577: Error: unsupported relocation against base
>
> Which comes from:
>
> pr_err("GESR0: 0x%08x\n", mfdcr(base + PLB4OPB_GESR0));
>
> Where our mfdcr() macro is stringifying "base + PLB4OPB_GESR0", and
> passing that to the assembler, which obviously doesn't work.
>
> The mfdcr() macro already checks that the argument is constant using
> __builtin_constant_p(), and if not calls the out-of-line version of
> mfdcr(). But in this case GCC is smart enough to notice that "base +
> PLB4OPB_GESR0" will be constant, even though it's not something we can
> immediately stringify into a register number.
>
> Segher pointed out that passing the register number to the inline asm
> as a constant would be better, and in fact it fixes the build error,
> presumably because it gives GCC a chance to resolve the value.
>
> While we're at it, change mtdcr() similarly.
>
> Reported-by: kernel test robot <lkp@intel.com>
> Suggested-by: Segher Boessenkool <segher@kernel.crashing.org>
> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
Acked-by: Feng Tang <feng.tang@intel.com>
Thanks!
> ---
> arch/powerpc/include/asm/dcr-native.h | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/dcr-native.h b/arch/powerpc/include/asm/dcr-native.h
> index 7141ccea8c94..a92059964579 100644
> --- a/arch/powerpc/include/asm/dcr-native.h
> +++ b/arch/powerpc/include/asm/dcr-native.h
> @@ -53,8 +53,8 @@ static inline void mtdcrx(unsigned int reg, unsigned int val)
> #define mfdcr(rn) \
> ({unsigned int rval; \
> if (__builtin_constant_p(rn) && rn < 1024) \
> - asm volatile("mfdcr %0," __stringify(rn) \
> - : "=r" (rval)); \
> + asm volatile("mfdcr %0, %1" : "=r" (rval) \
> + : "n" (rn)); \
> else if (likely(cpu_has_feature(CPU_FTR_INDEXED_DCR))) \
> rval = mfdcrx(rn); \
> else \
> @@ -64,8 +64,8 @@ static inline void mtdcrx(unsigned int reg, unsigned int val)
> #define mtdcr(rn, v) \
> do { \
> if (__builtin_constant_p(rn) && rn < 1024) \
> - asm volatile("mtdcr " __stringify(rn) ",%0" \
> - : : "r" (v)); \
> + asm volatile("mtdcr %0, %1" \
> + : : "n" (rn), "r" (v)); \
> else if (likely(cpu_has_feature(CPU_FTR_INDEXED_DCR))) \
> mtdcrx(rn, v); \
> else \
> --
> 2.25.1
^ permalink raw reply
* Re: linux-next: manual merge of the devicetree tree with the powerpc tree
From: Rob Herring @ 2021-02-18 13:52 UTC (permalink / raw)
To: Stephen Rothwell
Cc: Linux Kernel Mailing List, Lakshmi Ramasubramanian,
Linux Next Mailing List, PowerPC, Hari Bathini
In-Reply-To: <20210218223427.77109d83@canb.auug.org.au>
On Thu, Feb 18, 2021 at 5:34 AM Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>
> Hi Michael,
>
> On Thu, 18 Feb 2021 21:44:37 +1100 Michael Ellerman <mpe@ellerman.id.au> wrote:
> >
> > I think it just needs this?
> >
> > diff --git a/arch/powerpc/kexec/elf_64.c b/arch/powerpc/kexec/elf_64.c
> > index 87e34611f93d..0492ca6003f3 100644
> > --- a/arch/powerpc/kexec/elf_64.c
> > +++ b/arch/powerpc/kexec/elf_64.c
> > @@ -104,7 +104,7 @@ static void *elf64_load(struct kimage *image, char *kernel_buf,
> >
> > fdt = of_kexec_alloc_and_setup_fdt(image, initrd_load_addr,
> > initrd_len, cmdline,
> > - fdt_totalsize(initial_boot_params));
> > + kexec_fdt_totalsize_ppc64(image));
> > if (!fdt) {
> > pr_err("Error setting up the new device tree.\n");
> > ret = -EINVAL;
> >
>
> I thought about that, but the last argument to
> of_kexec_alloc_and_setup_fdt() is extra_fdt_size and the allocation
> done is for this:
>
> fdt_size = fdt_totalsize(initial_boot_params) +
> (cmdline ? strlen(cmdline) : 0) +
> FDT_EXTRA_SPACE +
> extra_fdt_size;
>
> and kexec_fdt_totalsize_ppc64() also includes
> fdt_totalsize(initial_boot_params) so I was not sure. Maybe
> kexec_fdt_totalsize_ppc64() needs modification as well?
You're both right. Michael's fix is sufficient for the merge. The only
risk with a larger size is failing to allocate it, but we're talking
only 10s of KB. Historically until the commit causing the conflict,
PPC was just used 2x fdt_totalsize(initial_boot_params). You could
drop 'fdt_size = fdt_totalsize(initial_boot_params) + (2 *
COMMAND_LINE_SIZE);' from kexec_fdt_totalsize_ppc64() as well, but
then the function name is misleading.
Lakshmi can send a follow-up patch to fine tune the size and rename
kexec_fdt_totalsize_ppc64.
Rob
^ permalink raw reply
* Re: [PATCH kernel] powerpc/iommu: Annotate nested lock for lockdep
From: Frederic Barrat @ 2021-02-18 12:59 UTC (permalink / raw)
To: Alexey Kardashevskiy, linuxppc-dev; +Cc: kvm-ppc
In-Reply-To: <20210216032000.21642-1-aik@ozlabs.ru>
On 16/02/2021 04:20, Alexey Kardashevskiy wrote:
> The IOMMU table is divided into pools for concurrent mappings and each
> pool has a separate spinlock. When taking the ownership of an IOMMU group
> to pass through a device to a VM, we lock these spinlocks which triggers
> a false negative warning in lockdep (below).
>
> This fixes it by annotating the large pool's spinlock as a nest lock.
>
> ===
> WARNING: possible recursive locking detected
> 5.11.0-le_syzkaller_a+fstn1 #100 Not tainted
> --------------------------------------------
> qemu-system-ppc/4129 is trying to acquire lock:
> c0000000119bddb0 (&(p->lock)/1){....}-{2:2}, at: iommu_take_ownership+0xac/0x1e0
>
> but task is already holding lock:
> c0000000119bdd30 (&(p->lock)/1){....}-{2:2}, at: iommu_take_ownership+0xac/0x1e0
>
> other info that might help us debug this:
> Possible unsafe locking scenario:
>
> CPU0
> ----
> lock(&(p->lock)/1);
> lock(&(p->lock)/1);
> ===
>
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> ---
> arch/powerpc/kernel/iommu.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/kernel/iommu.c b/arch/powerpc/kernel/iommu.c
> index 557a09dd5b2f..2ee642a6731a 100644
> --- a/arch/powerpc/kernel/iommu.c
> +++ b/arch/powerpc/kernel/iommu.c
> @@ -1089,7 +1089,7 @@ int iommu_take_ownership(struct iommu_table *tbl)
>
> spin_lock_irqsave(&tbl->large_pool.lock, flags);
> for (i = 0; i < tbl->nr_pools; i++)
> - spin_lock(&tbl->pools[i].lock);
> + spin_lock_nest_lock(&tbl->pools[i].lock, &tbl->large_pool.lock);
We have the same pattern and therefore should have the same problem in
iommu_release_ownership().
But as I understand, we're hacking our way around lockdep here, since
conceptually, those locks are independent. I was wondering why it seems
to fix it by worrying only about the large pool lock. That loop can take
many locks (up to 4 with current config). However, if the dma window is
less than 1GB, we would only have one, so it would make sense for
lockdep to stop complaining. Is it what happened? In which case, this
patch doesn't really fix it. Or I'm missing something :-)
Fred
> iommu_table_release_pages(tbl);
>
>
^ permalink raw reply
* [PATCH] powerpc/4xx: Fix build errors from mfdcr()
From: Michael Ellerman @ 2021-02-18 12:30 UTC (permalink / raw)
To: linuxppc-dev; +Cc: feng.tang
lkp reported a build error in fsp2.o:
CC arch/powerpc/platforms/44x/fsp2.o
{standard input}:577: Error: unsupported relocation against base
Which comes from:
pr_err("GESR0: 0x%08x\n", mfdcr(base + PLB4OPB_GESR0));
Where our mfdcr() macro is stringifying "base + PLB4OPB_GESR0", and
passing that to the assembler, which obviously doesn't work.
The mfdcr() macro already checks that the argument is constant using
__builtin_constant_p(), and if not calls the out-of-line version of
mfdcr(). But in this case GCC is smart enough to notice that "base +
PLB4OPB_GESR0" will be constant, even though it's not something we can
immediately stringify into a register number.
Segher pointed out that passing the register number to the inline asm
as a constant would be better, and in fact it fixes the build error,
presumably because it gives GCC a chance to resolve the value.
While we're at it, change mtdcr() similarly.
Reported-by: kernel test robot <lkp@intel.com>
Suggested-by: Segher Boessenkool <segher@kernel.crashing.org>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
arch/powerpc/include/asm/dcr-native.h | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/include/asm/dcr-native.h b/arch/powerpc/include/asm/dcr-native.h
index 7141ccea8c94..a92059964579 100644
--- a/arch/powerpc/include/asm/dcr-native.h
+++ b/arch/powerpc/include/asm/dcr-native.h
@@ -53,8 +53,8 @@ static inline void mtdcrx(unsigned int reg, unsigned int val)
#define mfdcr(rn) \
({unsigned int rval; \
if (__builtin_constant_p(rn) && rn < 1024) \
- asm volatile("mfdcr %0," __stringify(rn) \
- : "=r" (rval)); \
+ asm volatile("mfdcr %0, %1" : "=r" (rval) \
+ : "n" (rn)); \
else if (likely(cpu_has_feature(CPU_FTR_INDEXED_DCR))) \
rval = mfdcrx(rn); \
else \
@@ -64,8 +64,8 @@ static inline void mtdcrx(unsigned int reg, unsigned int val)
#define mtdcr(rn, v) \
do { \
if (__builtin_constant_p(rn) && rn < 1024) \
- asm volatile("mtdcr " __stringify(rn) ",%0" \
- : : "r" (v)); \
+ asm volatile("mtdcr %0, %1" \
+ : : "n" (rn), "r" (v)); \
else if (likely(cpu_has_feature(CPU_FTR_INDEXED_DCR))) \
mtdcrx(rn, v); \
else \
--
2.25.1
^ permalink raw reply related
* Re: linux-next: manual merge of the devicetree tree with the powerpc tree
From: Stephen Rothwell @ 2021-02-18 11:34 UTC (permalink / raw)
To: Michael Ellerman
Cc: Rob Herring, Linux Kernel Mailing List, Lakshmi Ramasubramanian,
Linux Next Mailing List, Rob Herring, PowerPC, Hari Bathini
In-Reply-To: <874ki9vene.fsf@mpe.ellerman.id.au>
[-- Attachment #1: Type: text/plain, Size: 1229 bytes --]
Hi Michael,
On Thu, 18 Feb 2021 21:44:37 +1100 Michael Ellerman <mpe@ellerman.id.au> wrote:
>
> I think it just needs this?
>
> diff --git a/arch/powerpc/kexec/elf_64.c b/arch/powerpc/kexec/elf_64.c
> index 87e34611f93d..0492ca6003f3 100644
> --- a/arch/powerpc/kexec/elf_64.c
> +++ b/arch/powerpc/kexec/elf_64.c
> @@ -104,7 +104,7 @@ static void *elf64_load(struct kimage *image, char *kernel_buf,
>
> fdt = of_kexec_alloc_and_setup_fdt(image, initrd_load_addr,
> initrd_len, cmdline,
> - fdt_totalsize(initial_boot_params));
> + kexec_fdt_totalsize_ppc64(image));
> if (!fdt) {
> pr_err("Error setting up the new device tree.\n");
> ret = -EINVAL;
>
I thought about that, but the last argument to
of_kexec_alloc_and_setup_fdt() is extra_fdt_size and the allocation
done is for this:
fdt_size = fdt_totalsize(initial_boot_params) +
(cmdline ? strlen(cmdline) : 0) +
FDT_EXTRA_SPACE +
extra_fdt_size;
and kexec_fdt_totalsize_ppc64() also includes
fdt_totalsize(initial_boot_params) so I was not sure. Maybe
kexec_fdt_totalsize_ppc64() needs modification as well?
--
Cheers,
Stephen Rothwell
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH v5 05/10] powerpc/signal64: Remove TM ifdefery in middle of if/else block
From: Michael Ellerman @ 2021-02-18 10:47 UTC (permalink / raw)
To: Christopher M. Riedl, Daniel Axtens, linuxppc-dev
In-Reply-To: <C9C1XKNVRQC4.LHKIIQAIC3L7@oc8246131445.ibm.com>
"Christopher M. Riedl" <cmr@codefail.de> writes:
> On Thu Feb 11, 2021 at 11:21 PM CST, Daniel Axtens wrote:
...
>>
>> My only concern here was whether it was valid to access
>> if (__get_user(msr, &uc->uc_mcontext.gp_regs[PT_MSR]))
>> if CONFIG_PPC_TRANSACTIONAL_MEM was not defined, but I didn't think of
>> any obvious reason why it wouldn't be...
>
> Hmm, we don't really need it for the non-TM case and it is another extra
> uaccess. I will take your suggestion to remove the need for the other
> ifdefs but might keep this one. Keeping it also makes it very clear this
> call is only here for TM and possible to remove in a potentially TM-less
> future :)
Yep I agree.
cheers
^ permalink raw reply
* Re: linux-next: manual merge of the devicetree tree with the powerpc tree
From: Michael Ellerman @ 2021-02-18 10:44 UTC (permalink / raw)
To: Stephen Rothwell, Rob Herring, PowerPC
Cc: Lakshmi Ramasubramanian, Linux Next Mailing List,
Linux Kernel Mailing List, Hari Bathini, Rob Herring
In-Reply-To: <20210218144815.5673ae6f@canb.auug.org.au>
Stephen Rothwell <sfr@canb.auug.org.au> writes:
> Hi all,
>
> Today's linux-next merge of the devicetree tree got a conflict in:
>
> arch/powerpc/kexec/elf_64.c
>
> between commit:
>
> 2377c92e37fe ("powerpc/kexec_file: fix FDT size estimation for kdump kernel")
>
> from the powerpc tree and commit:
>
> 130b2d59cec0 ("powerpc: Use common of_kexec_alloc_and_setup_fdt()")
>
> from the devicetree tree.
>
> I can't easily see how to resolve these, so for now I have just used
> the latter' changes to this file.
I think it just needs this?
diff --git a/arch/powerpc/kexec/elf_64.c b/arch/powerpc/kexec/elf_64.c
index 87e34611f93d..0492ca6003f3 100644
--- a/arch/powerpc/kexec/elf_64.c
+++ b/arch/powerpc/kexec/elf_64.c
@@ -104,7 +104,7 @@ static void *elf64_load(struct kimage *image, char *kernel_buf,
fdt = of_kexec_alloc_and_setup_fdt(image, initrd_load_addr,
initrd_len, cmdline,
- fdt_totalsize(initial_boot_params));
+ kexec_fdt_totalsize_ppc64(image));
if (!fdt) {
pr_err("Error setting up the new device tree.\n");
ret = -EINVAL;
cheers
^ permalink raw reply related
* Re: [PATCH v2 2/7] ASoC: fsl_rpmsg: Add CPU DAI driver for audio base on rpmsg
From: Shengjiu Wang @ 2021-02-18 7:57 UTC (permalink / raw)
To: Mark Brown
Cc: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
alsa-devel, Timur Tabi, Xiubo Li, Fabio Estevam, Shengjiu Wang,
Takashi Iwai, Liam Girdwood, linux-kernel, Nicolin Chen,
Rob Herring, linuxppc-dev
In-Reply-To: <20210210153808.GB4748@sirena.org.uk>
On Wed, Feb 10, 2021 at 11:39 PM Mark Brown <broonie@kernel.org> wrote:
>
> On Wed, Feb 10, 2021 at 02:35:29PM +0800, Shengjiu Wang wrote:
> > On Wed, Feb 10, 2021 at 6:30 AM Mark Brown <broonie@kernel.org> wrote:
>
> > > Like I say I'd actually recommend moving this control to DAPM.
>
> > I may understand your point, you suggest to use the .set_bias_level
> > interface. But in my case I need to enable the clock in earlier stage
> > and keep the clock on when system go to suspend.
>
> The device can be kept alive over system suspend if that's needed, or
> possibly it sounds like runtime PM is a better fit? There's callbacks
> in the core to keep the device runtime PM enabled while it's open which
> is probably about the time range you're looking for.
Before enabling the clock, I need to reparent the clock according to
the sample rate, Maybe the hw_params is the right place to do
these things.
Can I add a flag:
"rpmsg->mclk_streams & BIT(substream->stream)"
for avoiding multiple calls of hw_params function before enabling
clock?
^ permalink raw reply
* Re: [PATCH v2 7/7] ASoC: dt-bindings: imx-rpmsg: Add binding doc for rpmsg machine driver
From: Shengjiu Wang @ 2021-02-18 7:23 UTC (permalink / raw)
To: Rob Herring
Cc: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
alsa-devel, Timur Tabi, Xiubo Li, Fabio Estevam, Shengjiu Wang,
Takashi Iwai, Liam Girdwood, linux-kernel, Nicolin Chen,
Mark Brown, linuxppc-dev
In-Reply-To: <20210210221704.GA2894134@robh.at.kernel.org>
On Thu, Feb 11, 2021 at 6:18 AM Rob Herring <robh@kernel.org> wrote:
>
> On Sun, Feb 07, 2021 at 06:23:55PM +0800, Shengjiu Wang wrote:
> > Imx-rpmsg is a new added machine driver for supporting audio on Cortex-M
> > core. The Cortex-M core will control the audio interface, DMA and audio
> > codec, setup the pipeline, the audio driver on Cortex-A core side is just
> > to communitcate with M core, it is a virtual sound card and don't touch
> > the hardware.
>
> I don't understand why there are 2 nodes for this other than you happen
> to want to split this into 2 Linux drivers. It's 1 h/w thing.
This one is for the sound card machine driver. Another one is
for the sound card cpu dai driver. so there are 2 nodes.
>
> >
> > Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
> > ---
> > .../bindings/sound/imx-audio-rpmsg.yaml | 48 +++++++++++++++++++
> > 1 file changed, 48 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/sound/imx-audio-rpmsg.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/sound/imx-audio-rpmsg.yaml b/Documentation/devicetree/bindings/sound/imx-audio-rpmsg.yaml
> > new file mode 100644
> > index 000000000000..b941aeb80678
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/sound/imx-audio-rpmsg.yaml
> > @@ -0,0 +1,48 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/sound/imx-audio-rpmsg.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: NXP i.MX audio complex with rpmsg
> > +
> > +maintainers:
> > + - Shengjiu Wang <shengjiu.wang@nxp.com>
> > +
> > +properties:
> > + compatible:
> > + enum:
> > + - fsl,imx-audio-rpmsg
> > +
> > + model:
> > + $ref: /schemas/types.yaml#/definitions/string
> > + description: User specified audio sound card name
> > +
> > + audio-cpu:
> > + description: The phandle of an CPU DAI controller
> > +
> > + rpmsg-out:
> > + description: |
> > + This is a boolean property. If present, the transmitting function
> > + will be enabled,
> > +
> > + rpmsg-in:
> > + description: |
> > + This is a boolean property. If present, the receiving function
> > + will be enabled.
> > +
> > +required:
> > + - compatible
> > + - model
> > + - audio-cpu
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > + - |
> > + sound-rpmsg {
> > + compatible = "fsl,imx-audio-rpmsg";
> > + model = "ak4497-audio";
> > + audio-cpu = <&rpmsg_audio>;
> > + rpmsg-out;
> > + };
> > --
> > 2.27.0
> >
^ permalink raw reply
* Re: [PATCH v2 3/7] ASoC: dt-bindings: fsl_rpmsg: Add binding doc for rpmsg cpu dai driver
From: Shengjiu Wang @ 2021-02-18 7:21 UTC (permalink / raw)
To: Rob Herring
Cc: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
alsa-devel, Timur Tabi, Xiubo Li, Fabio Estevam, Shengjiu Wang,
Takashi Iwai, Liam Girdwood, linux-kernel, Nicolin Chen,
Mark Brown, linuxppc-dev
In-Reply-To: <20210210221252.GA2885308@robh.at.kernel.org>
On Thu, Feb 11, 2021 at 6:13 AM Rob Herring <robh@kernel.org> wrote:
>
> On Sun, Feb 07, 2021 at 06:23:51PM +0800, Shengjiu Wang wrote:
> > fsl_rpmsg cpu dai driver is driver for rpmsg audio, which is mainly used
> > for getting the user's configuration from device tree and configure the
> > clocks which is used by Cortex-M core. So in this document define the
> > needed property.
> >
> > Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
> > ---
> > .../devicetree/bindings/sound/fsl,rpmsg.yaml | 80 +++++++++++++++++++
> > 1 file changed, 80 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/sound/fsl,rpmsg.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/sound/fsl,rpmsg.yaml b/Documentation/devicetree/bindings/sound/fsl,rpmsg.yaml
> > new file mode 100644
> > index 000000000000..2d3ce10d42fc
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/sound/fsl,rpmsg.yaml
> > @@ -0,0 +1,80 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/sound/fsl,rpmsg.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: NXP Audio RPMSG CPU DAI Controller
> > +
> > +maintainers:
> > + - Shengjiu Wang <shengjiu.wang@nxp.com>
> > +
> > +properties:
> > + compatible:
> > + enum:
> > + - fsl,imx7ulp-rpmsg
> > + - fsl,imx8mn-rpmsg
> > + - fsl,imx8mm-rpmsg
> > + - fsl,imx8mp-rpmsg
>
> rpmsg is a protocol. What's the h/w block?
On Linux side this driver is a virtual driver, it is running
on Arm Cortex-A core. The h/w block is controlled by
another core (cortex-M core). so this driver actually
doesn't touch any hardware, it just does configuration
for rpmsg channel.
>
> > +
> > + clocks:
> > + items:
> > + - description: Peripheral clock for register access
> > + - description: Master clock
> > + - description: DMA clock for DMA register access
> > + - description: Parent clock for multiple of 8kHz sample rates
> > + - description: Parent clock for multiple of 11kHz sample rates
> > + minItems: 5
> > +
> > + clock-names:
> > + items:
> > + - const: ipg
> > + - const: mclk
> > + - const: dma
> > + - const: pll8k
> > + - const: pll11k
> > + minItems: 5
> > +
> > + power-domains:
> > + maxItems: 1
> > +
> > + fsl,audioindex:
> > + $ref: /schemas/types.yaml#/definitions/uint32
> > + description: instance index for rpmsg image
> > +
> > + fsl,version:
> > + $ref: /schemas/types.yaml#/definitions/uint32
> > + description: rpmsg image version index
>
> What are these 2 used for?
fsl,audioindex: As we may support multiple instance, for example
two sound card with one rpmsg channel, this is the instance index.
fsl,version: There are maybe different image running on M core, this
is the image version, different image has different function.
>
> > +
> > + fsl,buffer-size:
> > + $ref: /schemas/types.yaml#/definitions/uint32
> > + description: pre allocate dma buffer size
> > +
> > + fsl,enable-lpa:
> > + $ref: /schemas/types.yaml#/definitions/flag
> > + description: enable low power audio path.
> > +
> > + fsl,codec-type:
> > + $ref: /schemas/types.yaml#/definitions/uint32
> > + description: Sometimes the codec is registered by
> > + driver not the device tree, this items
> > + can be used to distinguish codecs
>
> 0-2^32 are valid values?
I should add range for it.
>
> > +
> > +required:
> > + - compatible
> > + - fsl,audioindex
> > + - fsl,version
> > + - fsl,buffer-size
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > + - |
> > + rpmsg_audio: rpmsg_audio {
> > + compatible = "fsl,imx8mn-rpmsg";
> > + fsl,audioindex = <0> ;
> > + fsl,version = <2>;
> > + fsl,buffer-size = <0x6000000>;
> > + fsl,enable-lpa;
> > + status = "okay";
>
> Don't show status in examples.
ok, will remove it.
>
> > + };
> > --
> > 2.27.0
> >
^ permalink raw reply
* Re: [PATCH 4/4] ASoC: fsl: drop unneeded snd_soc_dai_set_drvdata
From: Nicolin Chen @ 2021-02-18 6:30 UTC (permalink / raw)
To: Julia Lawall
Cc: alsa-devel, linuxppc-dev, Timur Tabi, Xiubo Li, Shengjiu Wang,
Takashi Iwai, kernel-janitors, Liam Girdwood, Jaroslav Kysela,
Mark Brown, Fabio Estevam, linux-kernel
In-Reply-To: <20210213101907.1318496-5-Julia.Lawall@inria.fr>
On Sat, Feb 13, 2021 at 11:19:07AM +0100, Julia Lawall wrote:
> snd_soc_dai_set_drvdata is not needed when the set data comes from
> snd_soc_dai_get_drvdata or dev_get_drvdata. The problem was fixed
> usingthe following semantic patch: (http://coccinelle.lip6.fr/)
>
> // <smpl>
> @@
> expression x,y,e;
> @@
> x = dev_get_drvdata(y->dev)
> ... when != x = e
> - snd_soc_dai_set_drvdata(y,x);
>
> @@
> expression x,y,e;
> @@
> x = snd_soc_dai_get_drvdata(y)
> ... when != x = e
> - snd_soc_dai_set_drvdata(y,x);
> // </smpl>
>
> Signed-off-by: Julia Lawall <Julia.Lawall@inria.fr>
Acked-by: Nicolin Chen <nicoleotsuka@gmail.com>
^ permalink raw reply
* Re: [PATCH] arm64: defconfig: enable modern virtio pci device
From: Jason Wang @ 2021-02-18 6:04 UTC (permalink / raw)
To: Arnd Bergmann, Anders Roxell
Cc: Chris Zankel, Thomas Bogendoerfer, Michael S. Tsirkin,
Arnd Bergmann, linuxppc-dev, Catalin Marinas, linux-xtensa,
Paul Walmsley, virtualization, linux-kernel@vger.kernel.org,
Russell King - ARM Linux, Max Filippov, SoC Team, Albert Ou,
Palmer Dabbelt, linux-riscv, open list:BROADCOM NVRAM DRIVER,
Will Deacon, Linux ARM
In-Reply-To: <CAK8P3a2ysNApoG2FDsLdNoWA7nPXvzLMzkjXWdCig9jaSWwuKw@mail.gmail.com>
On 2021/2/11 下午7:52, Arnd Bergmann wrote:
> On Wed, Feb 10, 2021 at 8:05 PM Anders Roxell <anders.roxell@linaro.org> wrote:
>> Since patch ("virtio-pci: introduce modern device module") got added it
>> is not possible to boot a defconfig kernel in qemu with a virtio pci
>> device. Add CONFIG_VIRTIO_PCI_MODERN=y fragment makes the kernel able
>> to boot.
>>
>> Signed-off-by: Anders Roxell <anders.roxell@linaro.org>
>> ---
>> arch/arm/configs/multi_v7_defconfig | 1 +
>> arch/arm64/configs/defconfig | 1 +
> Acked-by: Arnd Bergmann <arnd@arndb.de>
>
> Michael, can you pick this up in the vhost tree that introduces the regression?
>
> Arnd
>
Hi:
Based on the discussion previously, the plan is to select
VIRTIO_PCI_MODERN, and document the module that select it must depend on
PCI.
I will post a patch soon.
Thanks
^ permalink raw reply
* linux-next: manual merge of the devicetree tree with the powerpc tree
From: Stephen Rothwell @ 2021-02-18 3:48 UTC (permalink / raw)
To: Rob Herring, Michael Ellerman, PowerPC
Cc: Lakshmi Ramasubramanian, Linux Next Mailing List,
Linux Kernel Mailing List, Hari Bathini, Rob Herring
[-- Attachment #1: Type: text/plain, Size: 858 bytes --]
Hi all,
Today's linux-next merge of the devicetree tree got a conflict in:
arch/powerpc/kexec/elf_64.c
between commit:
2377c92e37fe ("powerpc/kexec_file: fix FDT size estimation for kdump kernel")
from the powerpc tree and commit:
130b2d59cec0 ("powerpc: Use common of_kexec_alloc_and_setup_fdt()")
from the devicetree tree.
I can't easily see how to resolve these, so for now I have just used
the latter' changes to this file.
I fixed it up and can carry the fix as necessary. This is now fixed as
far as linux-next is concerned, but any non trivial conflicts should be
mentioned to your upstream maintainer when your tree is submitted for
merging. You may also want to consider cooperating with the maintainer
of the conflicting tree to minimise any particularly complex conflicts.
--
Cheers,
Stephen Rothwell
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH v18 00/10] Carry forward IMA measurement log on kexec on ARM64
From: Rob Herring @ 2021-02-18 1:25 UTC (permalink / raw)
To: Lakshmi Ramasubramanian
Cc: mark.rutland, tao.li, zohar, paulus, vincenzo.frascino,
frowand.list, sashal, masahiroy, jmorris, takahiro.akashi,
linux-arm-kernel, catalin.marinas, serge, devicetree,
pasha.tatashin, will, prsriva, hsinyi, allison, christophe.leroy,
mbrugger, balajib, dmitry.kasatkin, linux-kernel, james.morse,
gregkh, joe, linux-integrity, linuxppc-dev, bauerman
In-Reply-To: <20210213161049.6190-1-nramas@linux.microsoft.com>
On Sat, Feb 13, 2021 at 08:10:38AM -0800, Lakshmi Ramasubramanian wrote:
> On kexec file load Integrity Measurement Architecture (IMA) subsystem
> may verify the IMA signature of the kernel and initramfs, and measure
> it. The command line parameters passed to the kernel in the kexec call
> may also be measured by IMA. A remote attestation service can verify
> a TPM quote based on the TPM event log, the IMA measurement list, and
> the TPM PCR data. This can be achieved only if the IMA measurement log
> is carried over from the current kernel to the next kernel across
> the kexec call.
>
> powerpc already supports carrying forward the IMA measurement log on
> kexec. This patch set adds support for carrying forward the IMA
> measurement log on kexec on ARM64.
>
> This patch set moves the platform independent code defined for powerpc
> such that it can be reused for other platforms as well. A chosen node
> "linux,ima-kexec-buffer" is added to the DTB for ARM64 to hold
> the address and the size of the memory reserved to carry
> the IMA measurement log.
>
> This patch set has been tested for ARM64 platform using QEMU.
> I would like help from the community for testing this change on powerpc.
> Thanks.
>
> This patch set is based on
> commit f31e3386a4e9 ("ima: Free IMA measurement buffer after kexec syscall")
> in https://git.kernel.org/pub/scm/linux/kernel/git/zohar/linux-integrity.git
> "ima-kexec-fixes" branch.
>
> Changelog:
>
> v18
> - Added a parameter to of_kexec_alloc_and_setup_fdt() for the caller
> to specify additional space needed for the FDT buffer
> - Renamed arm64 and powerpc ELF buffer address field in
> "struct kimage_arch" to elf_load_addr to match x86_64 name.
> - Removed of_ima_add_kexec_buffer() and instead directly set
> ima_buffer_addr and ima_buffer_size in ima_add_kexec_buffer()
> - Moved FDT_EXTRA_SPACE definition from include/linux/of.h to
> drivers/of/kexec.c
>
> v17
> - Renamed of_kexec_setup_new_fdt() to of_kexec_alloc_and_setup_fdt(),
> and moved memory allocation for the new FDT to this function.
>
> v16
> - Defined functions to allocate and free buffer for FDT for powerpc
> and arm64.
> - Moved ima_buffer_addr and ima_buffer_size fields from
> "struct kimage_arch" in powerpc to "struct kimage"
> v15
> - Included Rob's patches in the patch set, and rebased
> the changes to "next-integrity" branch.
> - Allocate memory for DTB, on arm64, using kmalloc() instead of
> vmalloc() to keep it consistent with powerpc implementation.
> - Call of_kexec_setup_new_fdt() from setup_new_fdt_ppc64() and
> remove setup_new_fdt() in the same patch to keep it bisect safe.
>
> v14
> - Select CONFIG_HAVE_IMA_KEXEC for CONFIG_KEXEC_FILE, for powerpc
> and arm64, if CONFIG_IMA is enabled.
> - Use IS_ENABLED() macro instead of "#ifdef" in remove_ima_buffer(),
> ima_get_kexec_buffer(), and ima_free_kexec_buffer().
> - Call of_kexec_setup_new_fdt() from setup_new_fdt_ppc64() and
> remove setup_new_fdt() in "arch/powerpc/kexec/file_load.c".
>
> v13
> - Moved the arch independent functions to drivers/of/kexec.c
> and then refactored the code.
> - Moved arch_ima_add_kexec_buffer() to
> security/integrity/ima/ima_kexec.c
>
> v12
> - Use fdt_appendprop_addrrange() in setup_ima_buffer()
> to setup the IMA measurement list property in
> the device tree.
> - Moved architecture independent functions from
> "arch/powerpc/kexec/ima.c" to "drivers/of/kexec."
> - Deleted "arch/powerpc/kexec/ima.c" and
> "arch/powerpc/include/asm/ima.h".
>
> v11
> - Rebased the changes on the kexec code refactoring done by
> Rob Herring in his "dt/kexec" branch
> - Removed "extern" keyword in function declarations
> - Removed unnecessary header files included in C files
> - Updated patch descriptions per Thiago's comments
>
> v10
> - Moved delete_fdt_mem_rsv(), remove_ima_buffer(),
> get_ima_kexec_buffer, and get_root_addr_size_cells()
> to drivers/of/kexec.c
> - Moved arch_ima_add_kexec_buffer() to
> security/integrity/ima/ima_kexec.c
> - Conditionally define IMA buffer fields in struct kimage_arch
>
> v9
> - Moved delete_fdt_mem_rsv() to drivers/of/kexec_fdt.c
> - Defined a new function get_ima_kexec_buffer() in
> drivers/of/ima_kexec.c to replace do_get_kexec_buffer()
> - Changed remove_ima_kexec_buffer() to the original function name
> remove_ima_buffer()
> - Moved remove_ima_buffer() to drivers/of/ima_kexec.c
> - Moved ima_get_kexec_buffer() and ima_free_kexec_buffer()
> to security/integrity/ima/ima_kexec.c
>
> v8:
> - Moved remove_ima_kexec_buffer(), do_get_kexec_buffer(), and
> delete_fdt_mem_rsv() to drivers/of/fdt.c
> - Moved ima_dump_measurement_list() and ima_add_kexec_buffer()
> back to security/integrity/ima/ima_kexec.c
>
> v7:
> - Renamed remove_ima_buffer() to remove_ima_kexec_buffer() and moved
> this function definition to kernel.
> - Moved delete_fdt_mem_rsv() definition to kernel
> - Moved ima_dump_measurement_list() and ima_add_kexec_buffer() to
> a new file namely ima_kexec_fdt.c in IMA
>
> v6:
> - Remove any existing FDT_PROP_IMA_KEXEC_BUFFER property in the device
> tree and also its corresponding memory reservation in the currently
> running kernel.
> - Moved the function remove_ima_buffer() defined for powerpc to IMA
> and renamed the function to ima_remove_kexec_buffer(). Also, moved
> delete_fdt_mem_rsv() from powerpc to IMA.
>
> v5:
> - Merged get_addr_size_cells() and do_get_kexec_buffer() into a single
> function when moving the arch independent code from powerpc to IMA
> - Reverted the change to use FDT functions in powerpc code and added
> back the original code in get_addr_size_cells() and
> do_get_kexec_buffer() for powerpc.
> - Added fdt_add_mem_rsv() for ARM64 to reserve the memory for
> the IMA log buffer during kexec.
> - Fixed the warning reported by kernel test bot for ARM64
> arch_ima_add_kexec_buffer() - moved this function to a new file
> namely arch/arm64/kernel/ima_kexec.c
>
> v4:
> - Submitting the patch series on behalf of the original author
> Prakhar Srivastava <prsriva@linux.microsoft.com>
> - Moved FDT_PROP_IMA_KEXEC_BUFFER ("linux,ima-kexec-buffer") to
> libfdt.h so that it can be shared by multiple platforms.
>
> v3:
> Breakup patches further into separate patches.
> - Refactoring non architecture specific code out of powerpc
> - Update powerpc related code to use fdt functions
> - Update IMA buffer read related code to use of functions
> - Add support to store the memory information of the IMA
> measurement logs to be carried forward.
> - Update the property strings to align with documented nodes
> https://github.com/devicetree-org/dt-schema/pull/46
>
> v2:
> Break patches into separate patches.
> - Powerpc related Refactoring
> - Updating the docuemntation for chosen node
> - Updating arm64 to support IMA buffer pass
>
> v1:
> Refactoring carrying over IMA measuremnet logs over Kexec. This patch
> moves the non-architecture specific code out of powerpc and adds to
> security/ima.(Suggested by Thiago)
> Add Documentation regarding the ima-kexec-buffer node in the chosen
> node documentation
>
> v0:
> Add a layer of abstraction to use the memory reserved by device tree
> for ima buffer pass.
> Add support for ima buffer pass using reserved memory for arm64 kexec.
> Update the arch sepcific code path in kexec file load to store the
> ima buffer in the reserved memory. The same reserved memory is read
> on kexec or cold boot.
>
> Lakshmi Ramasubramanian (7):
> arm64: Rename kexec elf_headers_mem to elf_load_addr
> powerpc: Move ima buffer fields to struct kimage
> powerpc: Enable passing IMA log to next kernel on kexec
> powerpc: Move arch independent ima kexec functions to
> drivers/of/kexec.c
> kexec: Use fdt_appendprop_addrrange() to add ima buffer to FDT
> powerpc: Delete unused function delete_fdt_mem_rsv()
> arm64: Enable passing IMA log to next kernel on kexec
>
> Rob Herring (4):
> powerpc: Rename kexec elfcorehdr_addr to elf_load_addr
> of: Add a common kexec FDT setup function
> arm64: Use common of_kexec_alloc_and_setup_fdt()
> powerpc: Use common of_kexec_alloc_and_setup_fdt()
I've applied the series. The merge conflict is not too bad, but will
need a follow on patch to change the powerpc FDT size calculation
(dropping the current FDT's size out).
Thanks for persisting!
Rob
^ permalink raw reply
* Re: [PATCH 1/4] add generic builtin command line
From: Andrew Morton @ 2021-02-17 21:16 UTC (permalink / raw)
To: Daniel Gimpelevich
Cc: Christophe Leroy, Maksym Kokhan, linux-kernel, Rob Herring,
Paul Mackerras, xe-linux-external, Daniel Walker, linuxppc-dev,
Daniel Walker
In-Reply-To: <1613417521.3853.5.camel@chimera>
On Mon, 15 Feb 2021 11:32:01 -0800 Daniel Gimpelevich <daniel@gimpelevich.san-francisco.ca.us> wrote:
> On Thu, 2019-03-21 at 15:15 -0700, Andrew Morton wrote:
> > On Thu, 21 Mar 2019 08:13:08 -0700 Daniel Walker <danielwa@cisco.com> wrote:
> > > On Wed, Mar 20, 2019 at 08:14:33PM -0700, Andrew Morton wrote:
> > > > The patches (or some version of them) are already in linux-next,
> > > > which messes me up. I'll disable them for now.
> > >
> > > Those are from my tree, but I remove them when you picked up the series. The
> > > next linux-next should not have them.
> >
> > Yup, thanks, all looks good now.
>
> This patchset is currently neither in mainline nor in -next. May I ask
> what happened to it? Thanks.
Seems that I didn't bring them back after the confict with the powerpc
tree resolved itself.
Please resend everything for -rc1 and let's await the reviewer
feedback,
^ permalink raw reply
* Re: [PATCH v5 10/10] powerpc/signal64: Use __get_user() to copy sigset_t
From: Christopher M. Riedl @ 2021-02-17 19:53 UTC (permalink / raw)
To: Daniel Axtens, linuxppc-dev
In-Reply-To: <87v9axc78z.fsf@dja-thinkpad.axtens.net>
On Fri Feb 12, 2021 at 3:21 PM CST, Daniel Axtens wrote:
> "Christopher M. Riedl" <cmr@codefail.de> writes:
>
> > Usually sigset_t is exactly 8B which is a "trivial" size and does not
> > warrant using __copy_from_user(). Use __get_user() directly in
> > anticipation of future work to remove the trivial size optimizations
> > from __copy_from_user(). Calling __get_user() also results in a small
> > boost to signal handling throughput here.
>
> Modulo the comments from Christophe, this looks good to me. It seems to
> do what it says on the tin.
>
> Reviewed-by: Daniel Axtens <dja@axtens.net>
>
> Do you know if this patch is responsible for the slightly increase in
> radix performance you observed in your cover letter? The rest of the
> series shouldn't really make things faster than the no-KUAP case...
No, this patch just results in a really small improvement overall.
I looked over this again and I think the reason for the speedup is that
my implementation of unsafe_copy_from_user() in this series calls
__copy_tofrom_user() directly avoiding a barrier_nospec(). This speeds
up all the unsafe_get_from_user() calls in this version.
Skipping the barrier_nospec() like that is incorrect, but Christophe
recently sent a patch which moves barrier_nospec() into the uaccess
allowance helpers. It looks like mpe has already accepted that patch:
https://lists.ozlabs.org/pipermail/linuxppc-dev/2021-February/223959.html
That means that my implementation of unsafe_copy_from_user() is _now_
correct _and_ faster. We do not need to call barrier_nospec() since the
precondition for the 'unsafe' routine is that we called one of the
helpers to open up a uaccess window earlier.
>
> Kind regards,
> Daniel
>
> >
> > Signed-off-by: Christopher M. Riedl <cmr@codefail.de>
> > ---
> > arch/powerpc/kernel/signal_64.c | 14 ++++++++++++--
> > 1 file changed, 12 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/powerpc/kernel/signal_64.c b/arch/powerpc/kernel/signal_64.c
> > index 817b64e1e409..42fdc4a7ff72 100644
> > --- a/arch/powerpc/kernel/signal_64.c
> > +++ b/arch/powerpc/kernel/signal_64.c
> > @@ -97,6 +97,14 @@ static void prepare_setup_sigcontext(struct task_struct *tsk, int ctx_has_vsx_re
> > #endif /* CONFIG_VSX */
> > }
> >
> > +static inline int get_user_sigset(sigset_t *dst, const sigset_t *src)
> > +{
> > + if (sizeof(sigset_t) <= 8)
> > + return __get_user(dst->sig[0], &src->sig[0]);
> > + else
> > + return __copy_from_user(dst, src, sizeof(sigset_t));
> > +}
> > +
> > /*
> > * Set up the sigcontext for the signal frame.
> > */
> > @@ -701,8 +709,9 @@ SYSCALL_DEFINE3(swapcontext, struct ucontext __user *, old_ctx,
> > * We kill the task with a SIGSEGV in this situation.
> > */
> >
> > - if (__copy_from_user(&set, &new_ctx->uc_sigmask, sizeof(set)))
> > + if (get_user_sigset(&set, &new_ctx->uc_sigmask))
> > do_exit(SIGSEGV);
> > +
> > set_current_blocked(&set);
> >
> > if (!user_read_access_begin(new_ctx, ctx_size))
> > @@ -740,8 +749,9 @@ SYSCALL_DEFINE0(rt_sigreturn)
> > if (!access_ok(uc, sizeof(*uc)))
> > goto badframe;
> >
> > - if (__copy_from_user(&set, &uc->uc_sigmask, sizeof(set)))
> > + if (get_user_sigset(&set, &uc->uc_sigmask))
> > goto badframe;
> > +
> > set_current_blocked(&set);
> >
> > #ifdef CONFIG_PPC_TRANSACTIONAL_MEM
> > --
> > 2.26.1
^ 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