[PATCH] of: error: 'const struct kimage' has no member named 'arch'

[PATCH] of: error: 'const struct kimage' has no member named 'arch'

[Lakshmi Ramasubramanian]

of_kexec_alloc_and_setup_fdt() defined in drivers/of/kexec.c builds
a new device tree object that includes architecture specific data
for kexec system call.  This should be defined only if the architecture
being built defines kexec architecture structure "struct kimage_arch".

Define a new boolean config OF_KEXEC that is enabled if
CONFIG_KEXEC_FILE and CONFIG_OF_FLATTREE are enabled, and
the architecture is arm64 or powerpc64.  Build drivers/of/kexec.c
if CONFIG_OF_KEXEC is enabled.

Signed-off-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com>
Fixes: 33488dc4d61f ("of: Add a common kexec FDT setup function")
Reported-by: kernel test robot <lkp@intel.com>
---
 drivers/of/Kconfig  | 6 ++++++
 drivers/of/Makefile | 7 +------
 2 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig
index 18450437d5d5..f2e8fa54862a 100644
--- a/drivers/of/Kconfig
+++ b/drivers/of/Kconfig
@@ -100,4 +100,10 @@ config OF_DMA_DEFAULT_COHERENT
 	# arches should select this if DMA is coherent by default for OF devices
 	bool
 
+config OF_KEXEC
+	bool
+	depends on KEXEC_FILE
+	depends on OF_FLATTREE
+	default y if ARM64 || PPC64
+
 endif # OF
diff --git a/drivers/of/Makefile b/drivers/of/Makefile
index c13b982084a3..287579dd1695 100644
--- a/drivers/of/Makefile
+++ b/drivers/of/Makefile
@@ -13,11 +13,6 @@ obj-$(CONFIG_OF_RESERVED_MEM) += of_reserved_mem.o
 obj-$(CONFIG_OF_RESOLVE)  += resolver.o
 obj-$(CONFIG_OF_OVERLAY) += overlay.o
 obj-$(CONFIG_OF_NUMA) += of_numa.o
-
-ifdef CONFIG_KEXEC_FILE
-ifdef CONFIG_OF_FLATTREE
-obj-y	+= kexec.o
-endif
-endif
+obj-$(CONFIG_OF_KEXEC) += kexec.o
 
 obj-$(CONFIG_OF_UNITTEST) += unittest-data/
-- 
2.30.0

Re: [PATCH] of: error: 'const struct kimage' has no member named 'arch'

[Mimi Zohar]

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

Re: [PATCH] of: error: 'const struct kimage' has no member named 'arch'

[Lakshmi Ramasubramanian]

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

Re: [PATCH] of: error: 'const struct kimage' has no member named 'arch'

[Thiago Jung Bauermann]

Lakshmi Ramasubramanian <nramas@linux.microsoft.com> writes:

> On 2/18/21 4:07 PM, Mimi Zohar wrote:
>
> Hi Mimi,
>
>> On Thu, 2021-02-18 at 14:33 -0800, Lakshmi Ramasubramanian wrote:
>>> of_kexec_alloc_and_setup_fdt() defined in drivers/of/kexec.c builds
>>> a new device tree object that includes architecture specific data
>>> for kexec system call.  This should be defined only if the architecture
>>> being built defines kexec architecture structure "struct kimage_arch".
>>>
>>> Define a new boolean config OF_KEXEC that is enabled if
>>> CONFIG_KEXEC_FILE and CONFIG_OF_FLATTREE are enabled, and
>>> the architecture is arm64 or powerpc64.  Build drivers/of/kexec.c
>>> if CONFIG_OF_KEXEC is enabled.
>>>
>>> Signed-off-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com>
>>> Fixes: 33488dc4d61f ("of: Add a common kexec FDT setup function")
>>> Reported-by: kernel test robot <lkp@intel.com>
>>> ---
>>>   drivers/of/Kconfig  | 6 ++++++
>>>   drivers/of/Makefile | 7 +------
>>>   2 files changed, 7 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig
>>> index 18450437d5d5..f2e8fa54862a 100644
>>> --- a/drivers/of/Kconfig
>>> +++ b/drivers/of/Kconfig
>>> @@ -100,4 +100,10 @@ config OF_DMA_DEFAULT_COHERENT
>>>   	# arches should select this if DMA is coherent by default for OF devices
>>>   	bool
>>>   +config OF_KEXEC
>>> +	bool
>>> +	depends on KEXEC_FILE
>>> +	depends on OF_FLATTREE
>>> +	default y if ARM64 || PPC64
>>> +
>>>   endif # OF
>>> diff --git a/drivers/of/Makefile b/drivers/of/Makefile
>>> index c13b982084a3..287579dd1695 100644
>>> --- a/drivers/of/Makefile
>>> +++ b/drivers/of/Makefile
>>> @@ -13,11 +13,6 @@ obj-$(CONFIG_OF_RESERVED_MEM) += of_reserved_mem.o
>>>   obj-$(CONFIG_OF_RESOLVE)  += resolver.o
>>>   obj-$(CONFIG_OF_OVERLAY) += overlay.o
>>>   obj-$(CONFIG_OF_NUMA) += of_numa.o
>>> -
>>> -ifdef CONFIG_KEXEC_FILE
>>> -ifdef CONFIG_OF_FLATTREE
>>> -obj-y	+= kexec.o
>>> -endif
>>> -endif
>>> +obj-$(CONFIG_OF_KEXEC) += kexec.o
>>>     obj-$(CONFIG_OF_UNITTEST) += unittest-data/
>> Is it possible to reuse CONFIG_HAVE_IMA_KEXEC here?
>> 
>
> For ppc64 CONFIG_HAVE_IMA_KEXEC is selected when CONFIG_KEXEC_FILE is enabled.
> So I don't see a problem in reusing CONFIG_HAVE_IMA_KEXEC for ppc.
>
> But for arm64, CONFIG_HAVE_IMA_KEXEC is enabled in the final patch in the patch
> set (the one for carrying forward IMA log across kexec for arm64). arm64 calls
> of_kexec_alloc_and_setup_fdt() prior to enabling CONFIG_HAVE_IMA_KEXEC and hence
> breaks the build for arm64.

One problem is that I believe that this patch won't placate the robot,
because IIUC it generates config files at random and this change still
allows hppa and s390 to enable CONFIG_OF_KEXEC.

Perhaps a new CONFIG_HAVE_KIMAGE_ARCH option? Not having that option
would still allow building kexec.o, but would be used inside kexec.c to
avoid accessing kimage.arch members.

-- 
Thiago Jung Bauermann
IBM Linux Technology Center

Re: [PATCH] of: error: 'const struct kimage' has no member named 'arch'

[Lakshmi Ramasubramanian]

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

Re: [PATCH] of: error: 'const struct kimage' has no member named 'arch'

[Thiago Jung Bauermann]

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

Re: [PATCH] of: error: 'const struct kimage' has no member named 'arch'

[Mimi Zohar]

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

Re: [PATCH] of: error: 'const struct kimage' has no member named 'arch'

[Rob Herring]

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

Re: [PATCH] of: error: 'const struct kimage' has no member named 'arch'

[Lakshmi Ramasubramanian]

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

Re: [PATCH] of: error: 'const struct kimage' has no member named 'arch'

[Rob Herring]

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

Re: [PATCH] of: error: 'const struct kimage' has no member named 'arch'

[Mimi Zohar]

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

Re: [PATCH] of: error: 'const struct kimage' has no member named 'arch'

[Thiago Jung Bauermann]

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

Re: [PATCH] of: error: 'const struct kimage' has no member named 'arch'

[Lakshmi Ramasubramanian]

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