| Message ID | 20210218223305.2044-1-nramas@linux.microsoft.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | of: error: 'const struct kimage' has no member named 'arch' | expand |
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
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
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.
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. thanks, -lakshmi
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.
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
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
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
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
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
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.
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
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/
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(-)