* Re: [RFC PATCH 2/9] KVM: PPC: Book3S 64: Move GUEST_MODE_SKIP test into KVM
From: Nicholas Piggin @ 2021-02-19 7:53 UTC (permalink / raw)
To: Fabiano Rosas, kvm-ppc; +Cc: linuxppc-dev
In-Reply-To: <87tuqhxc01.fsf@linux.ibm.com>
Excerpts from Fabiano Rosas's message of February 13, 2021 6:33 am:
> Nicholas Piggin <npiggin@gmail.com> writes:
>
>> Move the GUEST_MODE_SKIP logic into KVM code. This is quite a KVM
>> internal detail that has no real need to be in common handlers.
>
> LGTM,
>
>>
>> (XXX: Need to confirm CBE handlers etc)
>
> Do these interrupts exist only in Cell? I see that they set HSRRs and
> MSR_HV, but CPU_FTRS_CELL does not contain CPU_HVMODE. So I don't get
> why they use the KVM macros.
Good question, I asked Michael Ellerman and he said there is a bare
metal Cell which predates or otherwise does not define HVMODE.
However it does not support KVM. So I think we can remove it.
> And for the instruction_breakpoint (0x1300) I think it would help if we
> could at least restrict when it is built. But I don't know what
> ISA/processor version it is from.
Yeah we should be documenting these non-architected handlers a little
better IMO.
I think we can get rid of the kvm skip from this one though. I've done
that in a separate patch in the next series.
Thanks,
Nick
^ permalink raw reply
* Re: [RFC PATCH 2/9] KVM: PPC: Book3S 64: Move GUEST_MODE_SKIP test into KVM
From: Nicholas Piggin @ 2021-02-19 7:56 UTC (permalink / raw)
To: Daniel Axtens, kvm-ppc; +Cc: linuxppc-dev
In-Reply-To: <87lfbka92i.fsf@linkitivity.dja.id.au>
Excerpts from Daniel Axtens's message of February 19, 2021 4:03 pm:
> Hi Nick,
>
>> +maybe_skip:
>> + cmpwi r12,0x200
>> + beq 1f
>> + cmpwi r12,0x300
>> + beq 1f
>> + cmpwi r12,0x380
>> + beq 1f
>> +#ifdef CONFIG_KVM_BOOK3S_HV_POSSIBLE
>> + /* XXX: cbe stuff? instruction breakpoint? */
>> + cmpwi r12,0xe02
>> + beq 2f
>> +#endif
>> + b no_skip
>> +1: mfspr r9,SPRN_SRR0
>> + addi r9,r9,4
>> + mtspr SPRN_SRR0,r9
>> + ld r12,HSTATE_SCRATCH0(r13)
>> + ld r9,HSTATE_SCRATCH2(r13)
>> + GET_SCRATCH0(r13)
>> + RFI_TO_KERNEL
>> +#ifdef CONFIG_KVM_BOOK3S_HV_POSSIBLE
>> +2: mfspr r9,SPRN_HSRR0
>> + addi r9,r9,4
>> + mtspr SPRN_HSRR0,r9
>> + ld r12,HSTATE_SCRATCH0(r13)
>> + ld r9,HSTATE_SCRATCH2(r13)
>> + GET_SCRATCH0(r13)
>> + HRFI_TO_KERNEL
>> +#endif
>
> If I understand correctly, label 1 is the kvmppc_skip_interrupt and
> label 2 is the kvmppc_skip_Hinterrupt. Would it be easier to understand
> if we used symbolic labels, or do you think the RFI_TO_KERNEL vs
> HRFI_TO_KERNEL and other changes are sufficient?
Yeah my thinking was it's okay this way because we've got all the
context there, whereas prior to this patch those were branched to from
far away places so the names helped more.
If the discontiguity or nesting was any larger then yes I would say
naming the labels is probably a good idea (e.g., maybe_skip / no_skip).
> Apart from that, I haven't checked the precise copy-paste to make sure
> nothing has changed by accident, but I am able to follow the general
> idea of the patch and am vigorously in favour of anything that
> simplifies our exception/interrupt paths!
Thanks,
Nick
^ permalink raw reply
* Re: [RFC PATCH 1/9] KVM: PPC: Book3S 64: move KVM interrupt entry to a common entry point
From: Nicholas Piggin @ 2021-02-19 8:03 UTC (permalink / raw)
To: Daniel Axtens, kvm-ppc; +Cc: linuxppc-dev
In-Reply-To: <87o8ggab50.fsf@linkitivity.dja.id.au>
Excerpts from Daniel Axtens's message of February 19, 2021 3:18 pm:
> Hi Nick,
>
>> +++ b/arch/powerpc/kvm/book3s_64_entry.S
>> @@ -0,0 +1,34 @@
>> +#include <asm/cache.h>
>> +#include <asm/ppc_asm.h>
>> +#include <asm/kvm_asm.h>
>> +#include <asm/reg.h>
>> +#include <asm/asm-offsets.h>
>> +#include <asm/kvm_book3s_asm.h>
>> +
>> +/*
>> + * We come here from the first-level interrupt handlers.
>> + */
>> +.global kvmppc_interrupt
>> +.balign IFETCH_ALIGN_BYTES
>> +kvmppc_interrupt:
>> + /*
>> + * Register contents:
>
> Clearly r9 contains some data at this point, and I think it's guest r9
> because of what you say later on in
> book3s_hv_rmhandlers.S::kvmppc_interrupt_hv. Is that right?
Yes I hope so.
> Should that
> be documented in this comment as well?
Normally it can be assumed the registers not explicitly enumerated would
be unchanged from the interrupted context, so they would all contain
guest values. I added the R9 contents comment later because I changed
it later.
>
>> + * R12 = (guest CR << 32) | interrupt vector
>> + * R13 = PACA
>> + * guest R12 saved in shadow VCPU SCRATCH0
>> + * guest R13 saved in SPRN_SCRATCH0
>> + */
>> +#ifdef CONFIG_KVM_BOOK3S_HV_POSSIBLE
>> + std r9, HSTATE_SCRATCH2(r13)
>> + lbz r9, HSTATE_IN_GUEST(r13)
>> + cmpwi r9, KVM_GUEST_MODE_HOST_HV
>> + beq kvmppc_bad_host_intr
>> +#ifdef CONFIG_KVM_BOOK3S_PR_POSSIBLE
>> + cmpwi r9, KVM_GUEST_MODE_GUEST
>> + ld r9, HSTATE_SCRATCH2(r13)
>> + beq kvmppc_interrupt_pr
>> +#endif
>> + b kvmppc_interrupt_hv
>> +#else
>> + b kvmppc_interrupt_pr
>> +#endif
>
> Apart from that I had a look and convinced myself that the code will
> behave the same as before. On that basis:
>
> Reviewed-by: Daniel Axtens <dja@axtens.net>
>
> Kind regards,
> Daniel
Thanks,
Nick
>
^ permalink raw reply
* Re: [PATCH v12 13/14] mm/vmalloc: Hugepage vmalloc mappings
From: Ding Tianhong @ 2021-02-19 8:52 UTC (permalink / raw)
To: Nicholas Piggin, Andrew Morton, linux-mm
Cc: linux-arch, linux-kernel, Christoph Hellwig, Jonathan Cameron,
Rick Edgecombe, linuxppc-dev
In-Reply-To: <1613720396.pnvmwaa8om.astroid@bobo.none>
On 2021/2/19 15:45, Nicholas Piggin wrote:
> Excerpts from Ding Tianhong's message of February 19, 2021 1:45 pm:
>> Hi Nicholas:
>>
>> I met some problem for this patch, like this:
>>
>> kva = vmalloc(3*1024k);
>>
>> remap_vmalloc_range(xxx, kva, xxx)
>>
>> It failed because that the check for page_count(page) is null so return, it break the some logic for current modules.
>> because the new huge page is not valid for composed page.
>
> Hey Ding, that's a good catch. How are you testing this stuff, do you
> have a particular driver that does this?
>
yes, The driver would get a memory from the vmalloc in kernel space, and then the physical same memory will mmap to the user space. The drivers could not work when applying this patch.
>> I think some guys really don't get used to the changes for the vmalloc that the small pages was transparency to the hugepage
>> when the size is bigger than the PMD_SIZE.
>
> I think in this case vmalloc could allocate the large page as a compound
> page which would solve this problem I think? (without having actually
> tested it)
>
yes, i think the __GFP_COMP flag could fix this.
>> can we think about give a new static huge page to fix it? just like use a a new vmalloc_huge_xxx function to disginguish the current function,
>> the user could choose to use the transparent hugepage or static hugepage for vmalloc.
>
> Yeah that's a good question, there are a few things in the huge vmalloc
> code that accounts things as small pages and you can't assume large or
> small. If there is benefit from forcing large pages that could certainly
> be added.
>
The vmalloc transparent is good, but not fit every user scenes, some guys like to use the deterministic function
for performance critical area.
Thanks
Ding
> Interestingly, remap_vmalloc_range in theory could map the pages as
> large in userspace as well. That takes more work but if something
> really needs that for performance, it could be done.
>
> Thanks,
> Nick
> .
>
^ permalink raw reply
* Re: [PATCH] of: error: 'const struct kimage' has no member named 'arch'
From: Thiago Jung Bauermann @ 2021-02-19 14:08 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: <3ca0aa87-ca83-8024-4067-c2382a360db9@linux.microsoft.com>
Lakshmi Ramasubramanian <nramas@linux.microsoft.com> writes:
> On 2/18/21 5:13 PM, Thiago Jung Bauermann wrote:
>> 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.
>
> I enabled CONFIG_OF_KEXEC for s390. With my patch applied, CONFIG_OF_KEXEC is
> removed. So I think the robot enabling this config would not be a problem.
>
>> 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.
>>
>
> I think this is a good idea - a new CONFIG_HAVE_KIMAGE_ARCH, which will be
> selected by arm64 and ppc for now. I tried this, and it fixes the build issue.
>
> Although, the name for the new config can be misleading since PARISC, for
> instance, also defines "struct kimage_arch". Perhaps,
> CONFIG_HAVE_ELF_KIMAGE_ARCH since of_kexec_alloc_and_setup_fdt() is
> accessing ELF specific fields in "struct kimage_arch"?
Ah, right. I should have digged into the code before making my
suggestion. CONFIG_HAVE_KIMAGE_ARCH isn't appropriate, indeed.
>
> Rob/Mimi - please let us know which approach you think is better.
Ah! We can actually use the existing CONFIG_HAVE_IMA_KEXEC, no? I don't
know why I didn't think of it before.
--
Thiago Jung Bauermann
IBM Linux Technology Center
^ permalink raw reply
* Re: [PATCH] of: error: 'const struct kimage' has no member named 'arch'
From: Rob Herring @ 2021-02-19 14:16 UTC (permalink / raw)
To: Lakshmi Ramasubramanian
Cc: Sasha Levin, Stephen Rothwell, devicetree, Greg Kroah-Hartman,
linuxppc-dev, linux-kernel@vger.kernel.org, Mimi Zohar,
AKASHI, Takahiro, James Morse, Catalin Marinas, Joe Perches,
linux-integrity, Will Deacon, Thiago Jung Bauermann,
linux-arm-kernel
In-Reply-To: <3ca0aa87-ca83-8024-4067-c2382a360db9@linux.microsoft.com>
On Thu, Feb 18, 2021 at 8:53 PM Lakshmi Ramasubramanian
<nramas@linux.microsoft.com> wrote:
>
> On 2/18/21 5:13 PM, Thiago Jung Bauermann wrote:
> >
> > 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.
>
> I enabled CONFIG_OF_KEXEC for s390. With my patch applied,
> CONFIG_OF_KEXEC is removed. So I think the robot enabling this config
> would not be a problem.
>
> >
> > 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.
> >
>
> I think this is a good idea - a new CONFIG_HAVE_KIMAGE_ARCH, which will
> be selected by arm64 and ppc for now. I tried this, and it fixes the
> build issue.
>
> Although, the name for the new config can be misleading since PARISC,
> for instance, also defines "struct kimage_arch". Perhaps,
> CONFIG_HAVE_ELF_KIMAGE_ARCH since of_kexec_alloc_and_setup_fdt() is
> accessing ELF specific fields in "struct kimage_arch"?
>
> Rob/Mimi - please let us know which approach you think is better.
I'd just move the fields to kimage.
Rob
^ permalink raw reply
* [PATCH] powerpc/kexec_file: Restore FDT size estimation for kdump kernel
From: Thiago Jung Bauermann @ 2021-02-19 14:25 UTC (permalink / raw)
To: linuxppc-dev
Cc: Rob Herring, kexec, linux-kernel, Mimi Zohar,
Lakshmi Ramasubramanian, Thiago Jung Bauermann, Hari Bathini
Commit 2377c92e37fe ("powerpc/kexec_file: fix FDT size estimation for kdump
kernel") fixed how elf64_load() estimates the FDT size needed by the
crashdump kernel.
At the same time, commit 130b2d59cec0 ("powerpc: Use common
of_kexec_alloc_and_setup_fdt()") changed the same code to use the generic
function of_kexec_alloc_and_setup_fdt() to calculate the FDT size. That
change made the code overestimate it a bit by counting twice the space
required for the kernel command line and /chosen properties.
Therefore change kexec_fdt_totalsize_ppc64() to calculate just the extra
space needed by the kdump kernel, and change the function name so that it
better reflects what the function is now doing.
Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
---
arch/powerpc/include/asm/kexec.h | 2 +-
arch/powerpc/kexec/elf_64.c | 2 +-
arch/powerpc/kexec/file_load_64.c | 26 ++++++++------------------
3 files changed, 10 insertions(+), 20 deletions(-)
Applies on top of next-20210219.
diff --git a/arch/powerpc/include/asm/kexec.h b/arch/powerpc/include/asm/kexec.h
index baab158e215c..5a11cc8d2350 100644
--- a/arch/powerpc/include/asm/kexec.h
+++ b/arch/powerpc/include/asm/kexec.h
@@ -128,7 +128,7 @@ int load_crashdump_segments_ppc64(struct kimage *image,
int setup_purgatory_ppc64(struct kimage *image, const void *slave_code,
const void *fdt, unsigned long kernel_load_addr,
unsigned long fdt_load_addr);
-unsigned int kexec_fdt_totalsize_ppc64(struct kimage *image);
+unsigned int kexec_extra_fdt_size_ppc64(struct kimage *image);
int setup_new_fdt_ppc64(const struct kimage *image, void *fdt,
unsigned long initrd_load_addr,
unsigned long initrd_len, const char *cmdline);
diff --git a/arch/powerpc/kexec/elf_64.c b/arch/powerpc/kexec/elf_64.c
index 0492ca6003f3..5a569bb51349 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,
- kexec_fdt_totalsize_ppc64(image));
+ kexec_extra_fdt_size_ppc64(image));
if (!fdt) {
pr_err("Error setting up the new device tree.\n");
ret = -EINVAL;
diff --git a/arch/powerpc/kexec/file_load_64.c b/arch/powerpc/kexec/file_load_64.c
index 3609de30a170..8541ba731908 100644
--- a/arch/powerpc/kexec/file_load_64.c
+++ b/arch/powerpc/kexec/file_load_64.c
@@ -927,37 +927,27 @@ int setup_purgatory_ppc64(struct kimage *image, const void *slave_code,
}
/**
- * kexec_fdt_totalsize_ppc64 - Return the estimated size needed to setup FDT
- * for kexec/kdump kernel.
- * @image: kexec image being loaded.
+ * kexec_extra_fdt_size_ppc63 - Return the estimated size needed to setup FDT
+ * for kexec/kdump kernel.
+ * @image: kexec image being loaded.
*
- * Returns the estimated size needed for kexec/kdump kernel FDT.
+ * Returns the estimated extra size needed for kexec/kdump kernel FDT.
*/
-unsigned int kexec_fdt_totalsize_ppc64(struct kimage *image)
+unsigned int kexec_extra_fdt_size_ppc64(struct kimage *image)
{
- unsigned int fdt_size;
u64 usm_entries;
- /*
- * The below estimate more than accounts for a typical kexec case where
- * the additional space is to accommodate things like kexec cmdline,
- * chosen node with properties for initrd start & end addresses and
- * a property to indicate kexec boot..
- */
- fdt_size = fdt_totalsize(initial_boot_params) + (2 * COMMAND_LINE_SIZE);
if (image->type != KEXEC_TYPE_CRASH)
- return fdt_size;
+ return 0;
/*
- * For kdump kernel, also account for linux,usable-memory and
+ * For kdump kernel, account for linux,usable-memory and
* linux,drconf-usable-memory properties. Get an approximate on the
* number of usable memory entries and use for FDT size estimation.
*/
usm_entries = ((memblock_end_of_DRAM() / drmem_lmb_size()) +
(2 * (resource_size(&crashk_res) / drmem_lmb_size())));
- fdt_size += (unsigned int)(usm_entries * sizeof(u64));
-
- return fdt_size;
+ return (unsigned int)(usm_entries * sizeof(u64));
}
/**
^ permalink raw reply related
* Re: [PATCH] of: error: 'const struct kimage' has no member named 'arch'
From: Mimi Zohar @ 2021-02-19 14:41 UTC (permalink / raw)
To: Thiago Jung Bauermann, Lakshmi Ramasubramanian
Cc: sashal, robh, sfr, gregkh, linuxppc-dev, linux-kernel,
takahiro.akashi, devicetree, james.morse, catalin.marinas, joe,
linux-integrity, will, linux-arm-kernel
In-Reply-To: <87eehcxi88.fsf@manicouagan.localdomain>
On Fri, 2021-02-19 at 11:08 -0300, Thiago Jung Bauermann wrote:
> Lakshmi Ramasubramanian <nramas@linux.microsoft.com> writes:
>
> > On 2/18/21 5:13 PM, Thiago Jung Bauermann wrote:
> >> 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.
> >
> > I enabled CONFIG_OF_KEXEC for s390. With my patch applied, CONFIG_OF_KEXEC is
> > removed. So I think the robot enabling this config would not be a problem.
> >
> >> 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.
> >>
> >
> > I think this is a good idea - a new CONFIG_HAVE_KIMAGE_ARCH, which will be
> > selected by arm64 and ppc for now. I tried this, and it fixes the build issue.
> >
> > Although, the name for the new config can be misleading since PARISC, for
> > instance, also defines "struct kimage_arch". Perhaps,
> > CONFIG_HAVE_ELF_KIMAGE_ARCH since of_kexec_alloc_and_setup_fdt() is
> > accessing ELF specific fields in "struct kimage_arch"?
>
> Ah, right. I should have digged into the code before making my
> suggestion. CONFIG_HAVE_KIMAGE_ARCH isn't appropriate, indeed.
>
> >
> > Rob/Mimi - please let us know which approach you think is better.
>
> Ah! We can actually use the existing CONFIG_HAVE_IMA_KEXEC, no? I don't
> know why I didn't think of it before.
Including kexec.o based on CONFIG_HAVE_IMA_KEXEC is a bisect issue on
ARM64, as Lakshmi pointed out. Defining a new, maybe temporary, flag
would solve the problem.
Mimi
^ permalink raw reply
* Re: [PATCH] powerpc/kexec_file: Restore FDT size estimation for kdump kernel
From: Lakshmi Ramasubramanian @ 2021-02-19 16:00 UTC (permalink / raw)
To: Thiago Jung Bauermann, linuxppc-dev
Cc: Rob Herring, kexec, linux-kernel, Mimi Zohar, Hari Bathini
In-Reply-To: <20210219142552.762608-1-bauerman@linux.ibm.com>
On 2/19/21 6:25 AM, Thiago Jung Bauermann wrote:
One small nit in the function header (please see below), but otherwise
the change looks good.
Reviewed-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com>
> Commit 2377c92e37fe ("powerpc/kexec_file: fix FDT size estimation for kdump
> kernel") fixed how elf64_load() estimates the FDT size needed by the
> crashdump kernel.
>
> At the same time, commit 130b2d59cec0 ("powerpc: Use common
> of_kexec_alloc_and_setup_fdt()") changed the same code to use the generic
> function of_kexec_alloc_and_setup_fdt() to calculate the FDT size. That
> change made the code overestimate it a bit by counting twice the space
> required for the kernel command line and /chosen properties.
>
> Therefore change kexec_fdt_totalsize_ppc64() to calculate just the extra
> space needed by the kdump kernel, and change the function name so that it
> better reflects what the function is now doing.
>
> Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
> ---
> arch/powerpc/include/asm/kexec.h | 2 +-
> arch/powerpc/kexec/elf_64.c | 2 +-
> arch/powerpc/kexec/file_load_64.c | 26 ++++++++------------------
> 3 files changed, 10 insertions(+), 20 deletions(-)
>
> Applies on top of next-20210219.
>
> diff --git a/arch/powerpc/include/asm/kexec.h b/arch/powerpc/include/asm/kexec.h
> index baab158e215c..5a11cc8d2350 100644
> --- a/arch/powerpc/include/asm/kexec.h
> +++ b/arch/powerpc/include/asm/kexec.h
> @@ -128,7 +128,7 @@ int load_crashdump_segments_ppc64(struct kimage *image,
> int setup_purgatory_ppc64(struct kimage *image, const void *slave_code,
> const void *fdt, unsigned long kernel_load_addr,
> unsigned long fdt_load_addr);
> -unsigned int kexec_fdt_totalsize_ppc64(struct kimage *image);
> +unsigned int kexec_extra_fdt_size_ppc64(struct kimage *image);
> int setup_new_fdt_ppc64(const struct kimage *image, void *fdt,
> unsigned long initrd_load_addr,
> unsigned long initrd_len, const char *cmdline);
> diff --git a/arch/powerpc/kexec/elf_64.c b/arch/powerpc/kexec/elf_64.c
> index 0492ca6003f3..5a569bb51349 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,
> - kexec_fdt_totalsize_ppc64(image));
> + kexec_extra_fdt_size_ppc64(image));
> if (!fdt) {
> pr_err("Error setting up the new device tree.\n");
> ret = -EINVAL;
> diff --git a/arch/powerpc/kexec/file_load_64.c b/arch/powerpc/kexec/file_load_64.c
> index 3609de30a170..8541ba731908 100644
> --- a/arch/powerpc/kexec/file_load_64.c
> +++ b/arch/powerpc/kexec/file_load_64.c
> @@ -927,37 +927,27 @@ int setup_purgatory_ppc64(struct kimage *image, const void *slave_code,
> }
>
> /**
> - * kexec_fdt_totalsize_ppc64 - Return the estimated size needed to setup FDT
> - * for kexec/kdump kernel.
> - * @image: kexec image being loaded.
> + * kexec_extra_fdt_size_ppc63 - Return the estimated size needed to setup FDT
Perhaps change to
"Return the estimated additional size needed to setup FDT for
kexec/kdump kernel"?
-lakshmi
> + * for kexec/kdump kernel.
> + * @image: kexec image being loaded.
> *
> - * Returns the estimated size needed for kexec/kdump kernel FDT.
> + * Returns the estimated extra size needed for kexec/kdump kernel FDT.
> */
> -unsigned int kexec_fdt_totalsize_ppc64(struct kimage *image)
> +unsigned int kexec_extra_fdt_size_ppc64(struct kimage *image)
> {
> - unsigned int fdt_size;
> u64 usm_entries;
>
> - /*
> - * The below estimate more than accounts for a typical kexec case where
> - * the additional space is to accommodate things like kexec cmdline,
> - * chosen node with properties for initrd start & end addresses and
> - * a property to indicate kexec boot..
> - */
> - fdt_size = fdt_totalsize(initial_boot_params) + (2 * COMMAND_LINE_SIZE);
> if (image->type != KEXEC_TYPE_CRASH)
> - return fdt_size;
> + return 0;
>
> /*
> - * For kdump kernel, also account for linux,usable-memory and
> + * For kdump kernel, account for linux,usable-memory and
> * linux,drconf-usable-memory properties. Get an approximate on the
> * number of usable memory entries and use for FDT size estimation.
> */
> usm_entries = ((memblock_end_of_DRAM() / drmem_lmb_size()) +
> (2 * (resource_size(&crashk_res) / drmem_lmb_size())));
> - fdt_size += (unsigned int)(usm_entries * sizeof(u64));
> -
> - return fdt_size;
> + return (unsigned int)(usm_entries * sizeof(u64));
> }
>
> /**
>
^ permalink raw reply
* Re: [PATCH] of: error: 'const struct kimage' has no member named 'arch'
From: Lakshmi Ramasubramanian @ 2021-02-19 16:56 UTC (permalink / raw)
To: Rob Herring
Cc: Sasha Levin, Stephen Rothwell, devicetree, Greg Kroah-Hartman,
linuxppc-dev, linux-kernel@vger.kernel.org, Mimi Zohar,
AKASHI, Takahiro, James Morse, Catalin Marinas, Joe Perches,
linux-integrity, Will Deacon, Thiago Jung Bauermann,
linux-arm-kernel
In-Reply-To: <CAL_JsqJiRV5xShOgso0PH2pFhv-yozay58i1uGQC0dJCVxkJPA@mail.gmail.com>
On 2/19/21 6:16 AM, Rob Herring wrote:
> On Thu, Feb 18, 2021 at 8:53 PM Lakshmi Ramasubramanian
> <nramas@linux.microsoft.com> wrote:
>>
>> On 2/18/21 5:13 PM, Thiago Jung Bauermann wrote:
>>>
>>> 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.
>>
>> I enabled CONFIG_OF_KEXEC for s390. With my patch applied,
>> CONFIG_OF_KEXEC is removed. So I think the robot enabling this config
>> would not be a problem.
>>
>>>
>>> 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.
>>>
>>
>> I think this is a good idea - a new CONFIG_HAVE_KIMAGE_ARCH, which will
>> be selected by arm64 and ppc for now. I tried this, and it fixes the
>> build issue.
>>
>> Although, the name for the new config can be misleading since PARISC,
>> for instance, also defines "struct kimage_arch". Perhaps,
>> CONFIG_HAVE_ELF_KIMAGE_ARCH since of_kexec_alloc_and_setup_fdt() is
>> accessing ELF specific fields in "struct kimage_arch"?
>>
>> Rob/Mimi - please let us know which approach you think is better.
>
> I'd just move the fields to kimage.
>
I think Mimi's suggestion to use CONFIG_HAVE_IMA_KEXEC for building
drivers/of/kexec.c would work and also avoid the bisect issue if we do
the following:
- In the patch set for carrying forward the IMA log on kexec, move the
following patch to a later point in the set
"[PATCH v18 04/11] arm64: Use common of_kexec_alloc_and_setup_fdt()"
and merge the above patch with the following patch
"[PATCH v18 11/11] arm64: Enable passing IMA log to next kernel on kexec"
I will try this now, and update.
thanks,
-lakshmi
^ permalink raw reply
* [PATCH] powerpc/mm: Move the linear_mapping_mutex to the ifdef where it is used
From: Sebastian Andrzej Siewior @ 2021-02-19 16:56 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Thomas Gleixner, Paul Mackerras, Sebastian Andrzej Siewior
The mutex linear_mapping_mutex is defined at the of the file while its
only two user are within the CONFIG_MEMORY_HOTPLUG block.
A compile without CONFIG_MEMORY_HOTPLUG set fails on PREEMPT_RT because
its mutex implementation is smart enough to realize that it is unused.
Move the definition of linear_mapping_mutex to ifdef block where it is
used.
Fixes: 1f73ad3e8d755 ("powerpc/mm: print warning in arch_remove_linear_mapping()")
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
arch/powerpc/mm/mem.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c
index afab328d08874..d6c3f0b79f1d1 100644
--- a/arch/powerpc/mm/mem.c
+++ b/arch/powerpc/mm/mem.c
@@ -54,7 +54,6 @@
#include <mm/mmu_decl.h>
-static DEFINE_MUTEX(linear_mapping_mutex);
unsigned long long memory_limit;
bool init_mem_is_free;
@@ -72,6 +71,7 @@ pgprot_t phys_mem_access_prot(struct file *file, unsigned long pfn,
EXPORT_SYMBOL(phys_mem_access_prot);
#ifdef CONFIG_MEMORY_HOTPLUG
+static DEFINE_MUTEX(linear_mapping_mutex);
#ifdef CONFIG_NUMA
int memory_add_physaddr_to_nid(u64 start)
--
2.30.0
^ permalink raw reply related
* Re: [PATCH] of: error: 'const struct kimage' has no member named 'arch'
From: Rob Herring @ 2021-02-19 17:43 UTC (permalink / raw)
To: Lakshmi Ramasubramanian
Cc: Sasha Levin, Stephen Rothwell, devicetree, Greg Kroah-Hartman,
linuxppc-dev, linux-kernel@vger.kernel.org, Mimi Zohar,
AKASHI, Takahiro, James Morse, Catalin Marinas, Joe Perches,
linux-integrity, Will Deacon, Thiago Jung Bauermann,
linux-arm-kernel
In-Reply-To: <98a061d1-05ea-eff2-5c5c-a59f491fe924@linux.microsoft.com>
On Fri, Feb 19, 2021 at 10:57 AM Lakshmi Ramasubramanian
<nramas@linux.microsoft.com> wrote:
>
> On 2/19/21 6:16 AM, Rob Herring wrote:
> > On Thu, Feb 18, 2021 at 8:53 PM Lakshmi Ramasubramanian
> > <nramas@linux.microsoft.com> wrote:
> >>
> >> On 2/18/21 5:13 PM, Thiago Jung Bauermann wrote:
> >>>
> >>> 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.
> >>
> >> I enabled CONFIG_OF_KEXEC for s390. With my patch applied,
> >> CONFIG_OF_KEXEC is removed. So I think the robot enabling this config
> >> would not be a problem.
> >>
> >>>
> >>> 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.
> >>>
> >>
> >> I think this is a good idea - a new CONFIG_HAVE_KIMAGE_ARCH, which will
> >> be selected by arm64 and ppc for now. I tried this, and it fixes the
> >> build issue.
> >>
> >> Although, the name for the new config can be misleading since PARISC,
> >> for instance, also defines "struct kimage_arch". Perhaps,
> >> CONFIG_HAVE_ELF_KIMAGE_ARCH since of_kexec_alloc_and_setup_fdt() is
> >> accessing ELF specific fields in "struct kimage_arch"?
> >>
> >> Rob/Mimi - please let us know which approach you think is better.
> >
> > I'd just move the fields to kimage.
> >
>
> I think Mimi's suggestion to use CONFIG_HAVE_IMA_KEXEC for building
> drivers/of/kexec.c would work and also avoid the bisect issue if we do
> the following:
That seems wrong given only a portion of the file depends on IMA. And
it reduces our compile coverage.
> - In the patch set for carrying forward the IMA log on kexec, move the
> following patch to a later point in the set
>
> "[PATCH v18 04/11] arm64: Use common of_kexec_alloc_and_setup_fdt()"
>
> and merge the above patch with the following patch
> "[PATCH v18 11/11] arm64: Enable passing IMA log to next kernel on kexec"
If we're doing all that, then I'm dropping all this for 5.12.
Rob
^ permalink raw reply
* Re: [PATCH] of: error: 'const struct kimage' has no member named 'arch'
From: Mimi Zohar @ 2021-02-19 18:03 UTC (permalink / raw)
To: Rob Herring, Lakshmi Ramasubramanian
Cc: Sasha Levin, Stephen Rothwell, devicetree, Catalin Marinas,
linuxppc-dev, linux-kernel@vger.kernel.org, AKASHI, Takahiro,
James Morse, Greg Kroah-Hartman, Joe Perches, linux-integrity,
Will Deacon, Thiago Jung Bauermann, linux-arm-kernel
In-Reply-To: <CAL_Jsq+R-zOT581_W0Ar5H58rfPnGiWeetoF_b+BaW7er22bPA@mail.gmail.com>
On Fri, 2021-02-19 at 11:43 -0600, Rob Herring wrote:
> On Fri, Feb 19, 2021 at 10:57 AM Lakshmi Ramasubramanian
> <nramas@linux.microsoft.com> wrote:
> >
> > On 2/19/21 6:16 AM, Rob Herring wrote:
> > > On Thu, Feb 18, 2021 at 8:53 PM Lakshmi Ramasubramanian
> > > <nramas@linux.microsoft.com> wrote:
> > >>
> > >> On 2/18/21 5:13 PM, Thiago Jung Bauermann wrote:
> > >>>
> > >>> 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.
> > >>
> > >> I enabled CONFIG_OF_KEXEC for s390. With my patch applied,
> > >> CONFIG_OF_KEXEC is removed. So I think the robot enabling this config
> > >> would not be a problem.
> > >>
> > >>>
> > >>> 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.
> > >>>
> > >>
> > >> I think this is a good idea - a new CONFIG_HAVE_KIMAGE_ARCH, which will
> > >> be selected by arm64 and ppc for now. I tried this, and it fixes the
> > >> build issue.
> > >>
> > >> Although, the name for the new config can be misleading since PARISC,
> > >> for instance, also defines "struct kimage_arch". Perhaps,
> > >> CONFIG_HAVE_ELF_KIMAGE_ARCH since of_kexec_alloc_and_setup_fdt() is
> > >> accessing ELF specific fields in "struct kimage_arch"?
> > >>
> > >> Rob/Mimi - please let us know which approach you think is better.
> > >
> > > I'd just move the fields to kimage.
> > >
> >
> > I think Mimi's suggestion to use CONFIG_HAVE_IMA_KEXEC for building
> > drivers/of/kexec.c would work and also avoid the bisect issue if we do
> > the following:
>
> That seems wrong given only a portion of the file depends on IMA. And
> it reduces our compile coverage.
I agree with you this is the wrong solution. Lakshmi's patch
introduced a new option to prevent other arch's from including kexec.o,
which is the same functionality as CONFIG_HAVE_IMA_KEXEC. I'm just not
sure what the right solution would be.
Mimi
^ permalink raw reply
* Re: [PATCH] of: error: 'const struct kimage' has no member named 'arch'
From: Thiago Jung Bauermann @ 2021-02-19 18:09 UTC (permalink / raw)
To: Mimi Zohar
Cc: Sasha Levin, Rob Herring, Lakshmi Ramasubramanian,
Greg Kroah-Hartman, linuxppc-dev, linux-kernel@vger.kernel.org,
AKASHI, Takahiro, devicetree, James Morse, Catalin Marinas,
Joe Perches, Stephen Rothwell, linux-integrity, Will Deacon,
linux-arm-kernel
In-Reply-To: <6a197963deb8e44c71384ea9b89d7f3f13c947bf.camel@linux.ibm.com>
Mimi Zohar <zohar@linux.ibm.com> writes:
> On Fri, 2021-02-19 at 11:43 -0600, Rob Herring wrote:
>> On Fri, Feb 19, 2021 at 10:57 AM Lakshmi Ramasubramanian
>> <nramas@linux.microsoft.com> wrote:
>> >
>> > On 2/19/21 6:16 AM, Rob Herring wrote:
>> > > On Thu, Feb 18, 2021 at 8:53 PM Lakshmi Ramasubramanian
>> > > <nramas@linux.microsoft.com> wrote:
>> > >>
>> > >> On 2/18/21 5:13 PM, Thiago Jung Bauermann wrote:
>> > >>>
>> > >>> 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.
>> > >>
>> > >> I enabled CONFIG_OF_KEXEC for s390. With my patch applied,
>> > >> CONFIG_OF_KEXEC is removed. So I think the robot enabling this config
>> > >> would not be a problem.
>> > >>
>> > >>>
>> > >>> 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.
>> > >>>
>> > >>
>> > >> I think this is a good idea - a new CONFIG_HAVE_KIMAGE_ARCH, which will
>> > >> be selected by arm64 and ppc for now. I tried this, and it fixes the
>> > >> build issue.
>> > >>
>> > >> Although, the name for the new config can be misleading since PARISC,
>> > >> for instance, also defines "struct kimage_arch". Perhaps,
>> > >> CONFIG_HAVE_ELF_KIMAGE_ARCH since of_kexec_alloc_and_setup_fdt() is
>> > >> accessing ELF specific fields in "struct kimage_arch"?
>> > >>
>> > >> Rob/Mimi - please let us know which approach you think is better.
>> > >
>> > > I'd just move the fields to kimage.
>> > >
>> >
>> > I think Mimi's suggestion to use CONFIG_HAVE_IMA_KEXEC for building
>> > drivers/of/kexec.c would work and also avoid the bisect issue if we do
>> > the following:
>>
>> That seems wrong given only a portion of the file depends on IMA. And
>> it reduces our compile coverage.
>
> I agree with you this is the wrong solution. Lakshmi's patch
> introduced a new option to prevent other arch's from including kexec.o,
> which is the same functionality as CONFIG_HAVE_IMA_KEXEC. I'm just not
> sure what the right solution would be.
I think Rob's suggestion of just moving the elf_load_addr,
elf_headers_sz fields (and for consistency, elf_headers as well even though it
isn't used in tihs file) from kimage_arch to kimage.
The downside is that these fields will go unused on a number of
architectures, but it's not worth complicating the code just because of
it.
The patch to do that would have to go before "of: Add a common kexec FDT
setup function". That should be enough to preserve bisectability for all arches.
--
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 18:43 UTC (permalink / raw)
To: Thiago Jung Bauermann, Mimi Zohar
Cc: Sasha Levin, Rob Herring, Stephen Rothwell, Greg Kroah-Hartman,
linuxppc-dev, linux-kernel@vger.kernel.org, AKASHI, Takahiro,
devicetree, James Morse, Catalin Marinas, Joe Perches,
linux-integrity, Will Deacon, linux-arm-kernel
In-Reply-To: <87blcgx72l.fsf@manicouagan.localdomain>
On 2/19/21 10:09 AM, Thiago Jung Bauermann wrote:
>
> Mimi Zohar <zohar@linux.ibm.com> writes:
>
>> On Fri, 2021-02-19 at 11:43 -0600, Rob Herring wrote:
>>> On Fri, Feb 19, 2021 at 10:57 AM Lakshmi Ramasubramanian
>>> <nramas@linux.microsoft.com> wrote:
>>>>
>>>> On 2/19/21 6:16 AM, Rob Herring wrote:
>>>>> On Thu, Feb 18, 2021 at 8:53 PM Lakshmi Ramasubramanian
>>>>> <nramas@linux.microsoft.com> wrote:
>>>>>>
>>>>>> On 2/18/21 5:13 PM, Thiago Jung Bauermann wrote:
>>>>>>>
>>>>>>> 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.
>>>>>>
>>>>>> I enabled CONFIG_OF_KEXEC for s390. With my patch applied,
>>>>>> CONFIG_OF_KEXEC is removed. So I think the robot enabling this config
>>>>>> would not be a problem.
>>>>>>
>>>>>>>
>>>>>>> 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.
>>>>>>>
>>>>>>
>>>>>> I think this is a good idea - a new CONFIG_HAVE_KIMAGE_ARCH, which will
>>>>>> be selected by arm64 and ppc for now. I tried this, and it fixes the
>>>>>> build issue.
>>>>>>
>>>>>> Although, the name for the new config can be misleading since PARISC,
>>>>>> for instance, also defines "struct kimage_arch". Perhaps,
>>>>>> CONFIG_HAVE_ELF_KIMAGE_ARCH since of_kexec_alloc_and_setup_fdt() is
>>>>>> accessing ELF specific fields in "struct kimage_arch"?
>>>>>>
>>>>>> Rob/Mimi - please let us know which approach you think is better.
>>>>>
>>>>> I'd just move the fields to kimage.
>>>>>
>>>>
>>>> I think Mimi's suggestion to use CONFIG_HAVE_IMA_KEXEC for building
>>>> drivers/of/kexec.c would work and also avoid the bisect issue if we do
>>>> the following:
>>>
>>> That seems wrong given only a portion of the file depends on IMA. And
>>> it reduces our compile coverage.
>>
>> I agree with you this is the wrong solution. Lakshmi's patch
>> introduced a new option to prevent other arch's from including kexec.o,
>> which is the same functionality as CONFIG_HAVE_IMA_KEXEC. I'm just not
>> sure what the right solution would be.
>
> I think Rob's suggestion of just moving the elf_load_addr,
> elf_headers_sz fields (and for consistency, elf_headers as well even though it
> isn't used in tihs file) from kimage_arch to kimage.
>
> The downside is that these fields will go unused on a number of
> architectures, but it's not worth complicating the code just because of
> it.
>
> The patch to do that would have to go before "of: Add a common kexec FDT
> setup function". That should be enough to preserve bisectability for all arches.
>
Agreed. I'll make this change and update.
-lakshmi
^ permalink raw reply
* Re: [PATCH 2/8] xen-swiotlb: use is_swiotlb_buffer in is_xen_swiotlb_buffer
From: Konrad Rzeszutek Wilk @ 2021-02-19 20:43 UTC (permalink / raw)
To: Christoph Hellwig
Cc: iommu, xen-devel, Claire Chang, linuxppc-dev, Dongli Zhang
In-Reply-To: <20210207160934.2955931-3-hch@lst.de>
On Sun, Feb 07, 2021 at 05:09:28PM +0100, Christoph Hellwig wrote:
> Use the is_swiotlb_buffer to check if a physical address is
> a swiotlb buffer. This works because xen-swiotlb does use the
> same buffer as the main swiotlb code, and xen_io_tlb_{start,end}
> are just the addresses for it that went through phys_to_virt.
>
Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> drivers/xen/swiotlb-xen.c | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c
> index 2b385c1b4a99cb..a4026822a889f7 100644
> --- a/drivers/xen/swiotlb-xen.c
> +++ b/drivers/xen/swiotlb-xen.c
> @@ -111,10 +111,8 @@ static int is_xen_swiotlb_buffer(struct device *dev, dma_addr_t dma_addr)
> * have the same virtual address as another address
> * in our domain. Therefore _only_ check address within our domain.
> */
> - if (pfn_valid(PFN_DOWN(paddr))) {
> - return paddr >= virt_to_phys(xen_io_tlb_start) &&
> - paddr < virt_to_phys(xen_io_tlb_end);
> - }
> + if (pfn_valid(PFN_DOWN(paddr)))
> + return is_swiotlb_buffer(paddr);
> return 0;
> }
>
> --
> 2.29.2
>
^ permalink raw reply
* Re: [PATCH 3/8] xen-swiotlb: use io_tlb_end in xen_swiotlb_dma_supported
From: Konrad Rzeszutek Wilk @ 2021-02-19 21:00 UTC (permalink / raw)
To: Christoph Hellwig
Cc: iommu, xen-devel, Claire Chang, linuxppc-dev, Dongli Zhang
In-Reply-To: <20210207160934.2955931-4-hch@lst.de>
On Sun, Feb 07, 2021 at 05:09:29PM +0100, Christoph Hellwig wrote:
> Use the existing variable that holds the physical address for
> xen_io_tlb_end to simplify xen_swiotlb_dma_supported a bit, and remove
> the otherwise unused xen_io_tlb_end variable and the xen_virt_to_bus
> helper.
>
Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> drivers/xen/swiotlb-xen.c | 10 ++--------
> 1 file changed, 2 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c
> index a4026822a889f7..4298f74a083985 100644
> --- a/drivers/xen/swiotlb-xen.c
> +++ b/drivers/xen/swiotlb-xen.c
> @@ -46,7 +46,7 @@
> * API.
> */
>
> -static char *xen_io_tlb_start, *xen_io_tlb_end;
> +static char *xen_io_tlb_start;
> static unsigned long xen_io_tlb_nslabs;
> /*
> * Quick lookup value of the bus address of the IOTLB.
> @@ -82,11 +82,6 @@ static inline phys_addr_t xen_dma_to_phys(struct device *dev,
> return xen_bus_to_phys(dev, dma_to_phys(dev, dma_addr));
> }
>
> -static inline dma_addr_t xen_virt_to_bus(struct device *dev, void *address)
> -{
> - return xen_phys_to_dma(dev, virt_to_phys(address));
> -}
> -
> static inline int range_straddles_page_boundary(phys_addr_t p, size_t size)
> {
> unsigned long next_bfn, xen_pfn = XEN_PFN_DOWN(p);
> @@ -250,7 +245,6 @@ int __ref xen_swiotlb_init(int verbose, bool early)
> rc = swiotlb_late_init_with_tbl(xen_io_tlb_start, xen_io_tlb_nslabs);
>
> end:
> - xen_io_tlb_end = xen_io_tlb_start + bytes;
> if (!rc)
> swiotlb_set_max_segment(PAGE_SIZE);
>
> @@ -558,7 +552,7 @@ xen_swiotlb_sync_sg_for_device(struct device *dev, struct scatterlist *sgl,
> static int
> xen_swiotlb_dma_supported(struct device *hwdev, u64 mask)
> {
> - return xen_virt_to_bus(hwdev, xen_io_tlb_end - 1) <= mask;
> + return xen_phys_to_dma(hwdev, io_tlb_end - 1) <= mask;
> }
>
> const struct dma_map_ops xen_swiotlb_dma_ops = {
> --
> 2.29.2
>
^ permalink raw reply
* [PATCH 1/9] ASoC: fsl: fsl_asrc: remove useless assignment
From: Pierre-Louis Bossart @ 2021-02-19 23:29 UTC (permalink / raw)
To: alsa-devel
Cc: Liam Girdwood, Timur Tabi, Xiubo Li, tiwai, Shengjiu Wang,
Takashi Iwai, linux-kernel, Pierre-Louis Bossart, Nicolin Chen,
open list:FREESCALE SOC SOUND DRIVERS, broonie, Jaroslav Kysela,
Fabio Estevam
In-Reply-To: <20210219232937.6440-1-pierre-louis.bossart@linux.intel.com>
cppcheck warning:
sound/soc/fsl/fsl_asrc.c:613:8: style: Variable 'i' is assigned a
value that is never used. [unreadVariable]
int i = 0, j = 0;
^
The same issue occurs for the 'j' variable.
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
sound/soc/fsl/fsl_asrc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/soc/fsl/fsl_asrc.c b/sound/soc/fsl/fsl_asrc.c
index c325c984d165..63d236ef5c4d 100644
--- a/sound/soc/fsl/fsl_asrc.c
+++ b/sound/soc/fsl/fsl_asrc.c
@@ -610,7 +610,7 @@ static void fsl_asrc_select_clk(struct fsl_asrc_priv *asrc_priv,
struct asrc_config *config = pair_priv->config;
int rate[2], select_clk[2]; /* Array size 2 means IN and OUT */
int clk_rate, clk_index;
- int i = 0, j = 0;
+ int i, j;
rate[IN] = in_rate;
rate[OUT] = out_rate;
--
2.25.1
^ permalink raw reply related
* [PATCH 2/9] ASoC: fsl: fsl_dma: remove unused variable
From: Pierre-Louis Bossart @ 2021-02-19 23:29 UTC (permalink / raw)
To: alsa-devel
Cc: Liam Girdwood, Timur Tabi, Xiubo Li, tiwai, Shengjiu Wang,
Takashi Iwai, linux-kernel, Pierre-Louis Bossart, Nicolin Chen,
open list:FREESCALE SOC SOUND DRIVERS, broonie, Jaroslav Kysela,
Fabio Estevam
In-Reply-To: <20210219232937.6440-1-pierre-louis.bossart@linux.intel.com>
cppcheck warning:
sound/soc/fsl/fsl_dma.c:411:10: style: Variable 'channel' is assigned
a value that is never used. [unreadVariable]
channel = substream->stream == SNDRV_PCM_STREAM_PLAYBACK ? 0 : 1;
^
Removing this line shows the variable isn't needed any longer so
remove declaration as well.
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
sound/soc/fsl/fsl_dma.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/sound/soc/fsl/fsl_dma.c b/sound/soc/fsl/fsl_dma.c
index e0c39c5f4854..84bd8a5356eb 100644
--- a/sound/soc/fsl/fsl_dma.c
+++ b/sound/soc/fsl/fsl_dma.c
@@ -392,7 +392,6 @@ static int fsl_dma_open(struct snd_soc_component *component,
dma_addr_t ld_buf_phys;
u64 temp_link; /* Pointer to next link descriptor */
u32 mr;
- unsigned int channel;
int ret = 0;
unsigned int i;
@@ -408,8 +407,6 @@ static int fsl_dma_open(struct snd_soc_component *component,
return ret;
}
- channel = substream->stream == SNDRV_PCM_STREAM_PLAYBACK ? 0 : 1;
-
if (dma->assigned) {
dev_err(dev, "dma channel already assigned\n");
return -EBUSY;
--
2.25.1
^ permalink raw reply related
* [PATCH 3/9] ASoC: fsl: fsl_easrc: remove useless assignments
From: Pierre-Louis Bossart @ 2021-02-19 23:29 UTC (permalink / raw)
To: alsa-devel
Cc: Liam Girdwood, Timur Tabi, Xiubo Li, tiwai, Shengjiu Wang,
Takashi Iwai, linux-kernel, Pierre-Louis Bossart, Nicolin Chen,
open list:FREESCALE SOC SOUND DRIVERS, broonie, Jaroslav Kysela,
Fabio Estevam
In-Reply-To: <20210219232937.6440-1-pierre-louis.bossart@linux.intel.com>
cppcheck warnings:
sound/soc/fsl/fsl_easrc.c:751:53: style: Variable 'st2_mem_alloc' is
assigned a value that is never used. [unreadVariable]
int st1_chanxexp, st1_mem_alloc = 0, st2_mem_alloc = 0;
^
sound/soc/fsl/fsl_easrc.c:1331:11: style: Variable 'size' is assigned
a value that is never used. [unreadVariable]
int size = 0;
^
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
sound/soc/fsl/fsl_easrc.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/sound/soc/fsl/fsl_easrc.c b/sound/soc/fsl/fsl_easrc.c
index 636a702f37a6..725a5d3aaa02 100644
--- a/sound/soc/fsl/fsl_easrc.c
+++ b/sound/soc/fsl/fsl_easrc.c
@@ -710,7 +710,7 @@ static int fsl_easrc_max_ch_for_slot(struct fsl_asrc_pair *ctx,
struct fsl_easrc_slot *slot)
{
struct fsl_easrc_ctx_priv *ctx_priv = ctx->private;
- int st1_mem_alloc = 0, st2_mem_alloc = 0;
+ int st1_mem_alloc = 0, st2_mem_alloc;
int pf_mem_alloc = 0;
int max_channels = 8 - slot->num_channel;
int channels = 0;
@@ -748,7 +748,7 @@ static int fsl_easrc_config_one_slot(struct fsl_asrc_pair *ctx,
{
struct fsl_asrc *easrc = ctx->asrc;
struct fsl_easrc_ctx_priv *ctx_priv = ctx->private;
- int st1_chanxexp, st1_mem_alloc = 0, st2_mem_alloc = 0;
+ int st1_chanxexp, st1_mem_alloc = 0, st2_mem_alloc;
unsigned int reg0, reg1, reg2, reg3;
unsigned int addr;
@@ -1328,7 +1328,7 @@ static int fsl_easrc_stop_context(struct fsl_asrc_pair *ctx)
{
struct fsl_asrc *easrc = ctx->asrc;
int val, i;
- int size = 0;
+ int size;
int retry = 200;
regmap_read(easrc->regmap, REG_EASRC_CC(ctx->index), &val);
--
2.25.1
^ permalink raw reply related
* [PATCH 4/9] ASoC: fsl: fsl_esai: clarify expression
From: Pierre-Louis Bossart @ 2021-02-19 23:29 UTC (permalink / raw)
To: alsa-devel
Cc: Liam Girdwood, Timur Tabi, Xiubo Li, tiwai, Shengjiu Wang,
Takashi Iwai, linux-kernel, Pierre-Louis Bossart, Nicolin Chen,
open list:FREESCALE SOC SOUND DRIVERS, broonie, Jaroslav Kysela,
Fabio Estevam
In-Reply-To: <20210219232937.6440-1-pierre-louis.bossart@linux.intel.com>
cppcheck warning:
sound/soc/fsl/fsl_esai.c:307:16: style: Clarify calculation precedence
for '%' and '?'. [clarifyCalculation]
clk_id % 2 ? "extal" : "fsys");
^
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
sound/soc/fsl/fsl_esai.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/soc/fsl/fsl_esai.c b/sound/soc/fsl/fsl_esai.c
index 08056fa0a0fa..41b154417b92 100644
--- a/sound/soc/fsl/fsl_esai.c
+++ b/sound/soc/fsl/fsl_esai.c
@@ -304,7 +304,7 @@ static int fsl_esai_set_dai_sysclk(struct snd_soc_dai *dai, int clk_id,
if (IS_ERR(clksrc)) {
dev_err(dai->dev, "no assigned %s clock\n",
- clk_id % 2 ? "extal" : "fsys");
+ (clk_id % 2) ? "extal" : "fsys");
return PTR_ERR(clksrc);
}
clk_rate = clk_get_rate(clksrc);
--
2.25.1
^ permalink raw reply related
* [PATCH 5/9] ASoC: fsl: fsl_ssi: remove unnecessary tests
From: Pierre-Louis Bossart @ 2021-02-19 23:29 UTC (permalink / raw)
To: alsa-devel
Cc: Liam Girdwood, Timur Tabi, Xiubo Li, tiwai, Shengjiu Wang,
Takashi Iwai, linux-kernel, Pierre-Louis Bossart, Nicolin Chen,
open list:FREESCALE SOC SOUND DRIVERS, broonie, Jaroslav Kysela,
Fabio Estevam
In-Reply-To: <20210219232937.6440-1-pierre-louis.bossart@linux.intel.com>
cppcheck warnings:
sound/soc/fsl/fsl_ssi.c:767:34: style: Condition 'div2' is always
false [knownConditionTrueFalse]
stccr = SSI_SxCCR_PM(pm + 1) | (div2 ? SSI_SxCCR_DIV2 : 0) |
^
sound/soc/fsl/fsl_ssi.c:722:9: note: Assignment 'div2=0', assigned value is 0
div2 = 0;
^
sound/soc/fsl/fsl_ssi.c:767:34: note: Condition 'div2' is always false
stccr = SSI_SxCCR_PM(pm + 1) | (div2 ? SSI_SxCCR_DIV2 : 0) |
^
sound/soc/fsl/fsl_ssi.c:768:4: style: Condition 'psr' is always false
[knownConditionTrueFalse]
(psr ? SSI_SxCCR_PSR : 0);
^
sound/soc/fsl/fsl_ssi.c:721:8: note: Assignment 'psr=0', assigned
value is 0
psr = 0;
^
sound/soc/fsl/fsl_ssi.c:768:4: note: Condition 'psr' is always false
(psr ? SSI_SxCCR_PSR : 0);
^
Upon further analysis, the variables 'div2' and 'psr' are set to zero
and never modified. All the tests can be removed.
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
sound/soc/fsl/fsl_ssi.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c
index 57811743c294..c57d0428c0a3 100644
--- a/sound/soc/fsl/fsl_ssi.c
+++ b/sound/soc/fsl/fsl_ssi.c
@@ -747,7 +747,7 @@ static int fsl_ssi_set_bclk(struct snd_pcm_substream *substream,
sub *= 100000;
do_div(sub, freq);
- if (sub < savesub && !(i == 0 && psr == 0 && div2 == 0)) {
+ if (sub < savesub && !(i == 0)) {
baudrate = tmprate;
savesub = sub;
pm = i;
@@ -764,8 +764,7 @@ static int fsl_ssi_set_bclk(struct snd_pcm_substream *substream,
return -EINVAL;
}
- stccr = SSI_SxCCR_PM(pm + 1) | (div2 ? SSI_SxCCR_DIV2 : 0) |
- (psr ? SSI_SxCCR_PSR : 0);
+ stccr = SSI_SxCCR_PM(pm + 1);
mask = SSI_SxCCR_PM_MASK | SSI_SxCCR_DIV2 | SSI_SxCCR_PSR;
/* STCCR is used for RX in synchronous mode */
--
2.25.1
^ permalink raw reply related
* [PATCH 6/9] ASoC: fsl: imx-hdmi: remove unused structure members
From: Pierre-Louis Bossart @ 2021-02-19 23:29 UTC (permalink / raw)
To: alsa-devel
Cc: Liam Girdwood, Timur Tabi, Xiubo Li, tiwai, Shengjiu Wang,
Sascha Hauer, Takashi Iwai, linux-kernel, Pierre-Louis Bossart,
Nicolin Chen, open list:FREESCALE SOC SOUND DRIVERS, broonie,
NXP Linux Team, Pengutronix Kernel Team, Shawn Guo,
Jaroslav Kysela, Fabio Estevam,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE
In-Reply-To: <20210219232937.6440-1-pierre-louis.bossart@linux.intel.com>
cppcheck warning:
sound/soc/fsl/imx-hdmi.c:21:16: style: struct member
'cpu_priv::sysclk_freq' is never used. [unusedStructMember]
unsigned long sysclk_freq[2];
^
Additional checks show the sysclk_dir member is also not used.
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
sound/soc/fsl/imx-hdmi.c | 4 ----
1 file changed, 4 deletions(-)
diff --git a/sound/soc/fsl/imx-hdmi.c b/sound/soc/fsl/imx-hdmi.c
index dbbb7618351c..1ebcb9a2336b 100644
--- a/sound/soc/fsl/imx-hdmi.c
+++ b/sound/soc/fsl/imx-hdmi.c
@@ -10,16 +10,12 @@
/**
* struct cpu_priv - CPU private data
- * @sysclk_freq: SYSCLK rates for set_sysclk()
- * @sysclk_dir: SYSCLK directions for set_sysclk()
* @sysclk_id: SYSCLK ids for set_sysclk()
* @slot_width: Slot width of each frame
*
* Note: [1] for tx and [0] for rx
*/
struct cpu_priv {
- unsigned long sysclk_freq[2];
- u32 sysclk_dir[2];
u32 sysclk_id[2];
u32 slot_width;
};
--
2.25.1
^ permalink raw reply related
* [PATCH 8/9] ASoC: fsl: mpc8610: remove useless assignment
From: Pierre-Louis Bossart @ 2021-02-19 23:29 UTC (permalink / raw)
To: alsa-devel
Cc: Liam Girdwood, Timur Tabi, Xiubo Li, tiwai, Shengjiu Wang,
Takashi Iwai, linux-kernel, Pierre-Louis Bossart, Nicolin Chen,
open list:FREESCALE SOC SOUND DRIVERS, broonie, Jaroslav Kysela,
Fabio Estevam
In-Reply-To: <20210219232937.6440-1-pierre-louis.bossart@linux.intel.com>
cppcheck warning:
sound/soc/fsl/mpc8610_hpcd.c:333:6: style: Redundant initialization
for 'ret'. The initialized value is overwritten before it is
read. [redundantInitialization]
ret = fsl_asoc_get_dma_channel(np, "fsl,playback-dma",
^
sound/soc/fsl/mpc8610_hpcd.c:193:10: note: ret is initialized
int ret = -ENODEV;
^
sound/soc/fsl/mpc8610_hpcd.c:333:6: note: ret is overwritten
ret = fsl_asoc_get_dma_channel(np, "fsl,playback-dma",
^
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
sound/soc/fsl/mpc8610_hpcd.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/soc/fsl/mpc8610_hpcd.c b/sound/soc/fsl/mpc8610_hpcd.c
index eccc833390d4..58b9ca3c4da0 100644
--- a/sound/soc/fsl/mpc8610_hpcd.c
+++ b/sound/soc/fsl/mpc8610_hpcd.c
@@ -190,7 +190,7 @@ static int mpc8610_hpcd_probe(struct platform_device *pdev)
struct device_node *codec_np = NULL;
struct mpc8610_hpcd_data *machine_data;
struct snd_soc_dai_link_component *comp;
- int ret = -ENODEV;
+ int ret;
const char *sprop;
const u32 *iprop;
--
2.25.1
^ permalink raw reply related
* Re: [PATCH] powerpc/kexec_file: Restore FDT size estimation for kdump kernel
From: Thiago Jung Bauermann @ 2021-02-19 23:42 UTC (permalink / raw)
To: Lakshmi Ramasubramanian
Cc: Rob Herring, kexec, linux-kernel, Mimi Zohar, linuxppc-dev,
Hari Bathini
In-Reply-To: <5a28907e-9231-7a19-62ff-3ed1c0282642@linux.microsoft.com>
Lakshmi Ramasubramanian <nramas@linux.microsoft.com> writes:
> On 2/19/21 6:25 AM, Thiago Jung Bauermann wrote:
>
> One small nit in the function header (please see below), but otherwise the
> change looks good.
>
> Reviewed-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com>
Thanks for your review. I incorporated your suggestion and will send v2
shortly.
>> --- a/arch/powerpc/kexec/file_load_64.c
>> +++ b/arch/powerpc/kexec/file_load_64.c
>> @@ -927,37 +927,27 @@ int setup_purgatory_ppc64(struct kimage *image, const void *slave_code,
>> }
>> /**
>> - * kexec_fdt_totalsize_ppc64 - Return the estimated size needed to setup FDT
>> - * for kexec/kdump kernel.
>> - * @image: kexec image being loaded.
>> + * kexec_extra_fdt_size_ppc63 - Return the estimated size needed to setup FDT
>
> Perhaps change to
>
> "Return the estimated additional size needed to setup FDT for kexec/kdump
> kernel"?
That's better indeed. I also hadn't noticed that I changed ppc64 to
ppc63. Fixed as well.
--
Thiago Jung Bauermann
IBM Linux Technology Center
^ 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