Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Wed, 6 Jul 2022 09:53:08 +0800
From:   Coiby Xu <coxu@redhat.com>
To:     Baoquan He <bhe@redhat.com>,
        "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com>
Cc:     Andrew Morton <akpm@linux-foundation.org>,
        Eric Biederman <ebiederm@xmission.com>,
        linux-kernel@vger.kernel.org, kexec@lists.infradead.org
Subject: Re: [PATCH 1/2] kexec_file: Drop weak attribute from functions
Message-ID: <20220706015308.heynqgictt47r3c5@Rk>
References: <cover.1656659357.git.naveen.n.rao@linux.vnet.ibm.com>
 <2cd7ca1fe4d6bb6ca38e3283c717878388ed6788.1656659357.git.naveen.n.rao@linux.vnet.ibm.com>
 <YsJoGKribMH77HBQ@MiWiFi-R3L-srv>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii; format=flowed
Content-Disposition: inline
In-Reply-To: <YsJoGKribMH77HBQ@MiWiFi-R3L-srv>
Precedence: bulk

Hi Baoquan and Naveen,

On Mon, Jul 04, 2022 at 12:10:00PM +0800, Baoquan He wrote:
>On 07/01/22 at 01:04pm, Naveen N. Rao wrote:
>> Drop __weak attribute from functions in kexec_file.c:
>> - arch_kexec_kernel_image_probe()
>> - arch_kimage_file_post_load_cleanup()
>> - arch_kexec_kernel_image_load()
>> - arch_kexec_locate_mem_hole()
>> - arch_kexec_kernel_verify_sig()
>>
>> arch_kexec_kernel_image_load() calls into kexec_image_load_default(), so
>> drop the static attribute for the latter.
>>
>> arch_kexec_kernel_verify_sig() is not overridden by any architecture, so
>> drop the __weak attribute.
>
>The dropping of arch_kexec_kernel_verify_sig() conflicts with patch 1 of
>anotherpatchset, and the other patches in the patchset depends on the
>patch 1.
>
>[PATCH v9 0/4] unify the keyrings of arm64 and s390 with x86 to verify kexec'ed kernel signature
>
>Hi, Naveen, Coiby,
>
>Please negotiate how to solve the conflict.

Thanks Baoquan for letting me know of this conflict. Naveen, how about
resolving the conflict based on which patch is merged first? If your
patch set is going to be merged early, I will resolve the conflict in my
patch set, and vice versa.

>
>Thanks
>Baoquan
>
>>
>> Suggested-by: Eric Biederman <ebiederm@xmission.com>
>> Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
>> ---
>>  arch/arm64/include/asm/kexec.h   |  4 ++-
>>  arch/powerpc/include/asm/kexec.h |  9 +++++++
>>  arch/s390/include/asm/kexec.h    |  3 +++
>>  arch/x86/include/asm/kexec.h     |  6 +++++
>>  include/linux/kexec.h            | 44 +++++++++++++++++++++++++++-----
>>  kernel/kexec_file.c              | 35 ++-----------------------
>>  6 files changed, 61 insertions(+), 40 deletions(-)
>>
>> diff --git a/arch/arm64/include/asm/kexec.h b/arch/arm64/include/asm/kexec.h
>> index 9839bfc163d714..78d272b26ebd11 100644
>> --- a/arch/arm64/include/asm/kexec.h
>> +++ b/arch/arm64/include/asm/kexec.h
>> @@ -115,7 +115,9 @@ extern const struct kexec_file_ops kexec_image_ops;
>>
>>  struct kimage;
>>
>> -extern int arch_kimage_file_post_load_cleanup(struct kimage *image);
>> +int arch_kimage_file_post_load_cleanup(struct kimage *image);
>> +#define arch_kimage_file_post_load_cleanup arch_kimage_file_post_load_cleanup
>> +
>>  extern int load_other_segments(struct kimage *image,
>>  		unsigned long kernel_load_addr, unsigned long kernel_size,
>>  		char *initrd, unsigned long initrd_len,
>> diff --git a/arch/powerpc/include/asm/kexec.h b/arch/powerpc/include/asm/kexec.h
>> index 2aefe14e144229..1e5e9b6ec78d9d 100644
>> --- a/arch/powerpc/include/asm/kexec.h
>> +++ b/arch/powerpc/include/asm/kexec.h
>> @@ -120,6 +120,15 @@ int setup_purgatory(struct kimage *image, const void *slave_code,
>>  #ifdef CONFIG_PPC64
>>  struct kexec_buf;
>>
>> +int arch_kexec_kernel_image_probe(struct kimage *image, void *buf, unsigned long buf_len);
>> +#define arch_kexec_kernel_image_probe arch_kexec_kernel_image_probe
>> +
>> +int arch_kimage_file_post_load_cleanup(struct kimage *image);
>> +#define arch_kimage_file_post_load_cleanup arch_kimage_file_post_load_cleanup
>> +
>> +int arch_kexec_locate_mem_hole(struct kexec_buf *kbuf);
>> +#define arch_kexec_locate_mem_hole arch_kexec_locate_mem_hole
>> +
>>  int load_crashdump_segments_ppc64(struct kimage *image,
>>  				  struct kexec_buf *kbuf);
>>  int setup_purgatory_ppc64(struct kimage *image, const void *slave_code,
>> diff --git a/arch/s390/include/asm/kexec.h b/arch/s390/include/asm/kexec.h
>> index 649ecdcc873453..8886aadc11a3a6 100644
>> --- a/arch/s390/include/asm/kexec.h
>> +++ b/arch/s390/include/asm/kexec.h
>> @@ -92,5 +92,8 @@ int arch_kexec_apply_relocations_add(struct purgatory_info *pi,
>>  				     const Elf_Shdr *relsec,
>>  				     const Elf_Shdr *symtab);
>>  #define arch_kexec_apply_relocations_add arch_kexec_apply_relocations_add
>> +
>> +int arch_kimage_file_post_load_cleanup(struct kimage *image);
>> +#define arch_kimage_file_post_load_cleanup arch_kimage_file_post_load_cleanup
>>  #endif
>>  #endif /*_S390_KEXEC_H */
>> diff --git a/arch/x86/include/asm/kexec.h b/arch/x86/include/asm/kexec.h
>> index 6ad8d946cd3ebf..5ec359c1b50cb3 100644
>> --- a/arch/x86/include/asm/kexec.h
>> +++ b/arch/x86/include/asm/kexec.h
>> @@ -193,6 +193,12 @@ int arch_kexec_apply_relocations_add(struct purgatory_info *pi,
>>  				     const Elf_Shdr *relsec,
>>  				     const Elf_Shdr *symtab);
>>  #define arch_kexec_apply_relocations_add arch_kexec_apply_relocations_add
>> +
>> +void *arch_kexec_kernel_image_load(struct kimage *image);
>> +#define arch_kexec_kernel_image_load arch_kexec_kernel_image_load
>> +
>> +int arch_kimage_file_post_load_cleanup(struct kimage *image);
>> +#define arch_kimage_file_post_load_cleanup arch_kimage_file_post_load_cleanup
>>  #endif
>>  #endif
>>
>> diff --git a/include/linux/kexec.h b/include/linux/kexec.h
>> index ce6536f1d26997..5e0bc3f9eac3e9 100644
>> --- a/include/linux/kexec.h
>> +++ b/include/linux/kexec.h
>> @@ -188,21 +188,53 @@ int kexec_purgatory_get_set_symbol(struct kimage *image, const char *name,
>>  				   void *buf, unsigned int size,
>>  				   bool get_value);
>>  void *kexec_purgatory_get_symbol_addr(struct kimage *image, const char *name);
>> +void *kexec_image_load_default(struct kimage *image);
>> +
>> +#ifndef arch_kexec_kernel_image_probe
>> +static inline int
>> +arch_kexec_kernel_image_probe(struct kimage *image, void *buf, unsigned long buf_len)
>> +{
>> +	return kexec_image_probe_default(image, buf, buf_len);
>> +}
>> +#endif
>> +
>> +#ifndef arch_kimage_file_post_load_cleanup
>> +static inline int arch_kimage_file_post_load_cleanup(struct kimage *image)
>> +{
>> +	return kexec_image_post_load_cleanup_default(image);
>> +}
>> +#endif
>> +
>> +#ifndef arch_kexec_kernel_image_load
>> +static inline void *arch_kexec_kernel_image_load(struct kimage *image)
>> +{
>> +	return kexec_image_load_default(image);
>> +}
>> +#endif
>>
>> -/* Architectures may override the below functions */
>> -int arch_kexec_kernel_image_probe(struct kimage *image, void *buf,
>> -				  unsigned long buf_len);
>> -void *arch_kexec_kernel_image_load(struct kimage *image);
>> -int arch_kimage_file_post_load_cleanup(struct kimage *image);
>>  #ifdef CONFIG_KEXEC_SIG
>>  int arch_kexec_kernel_verify_sig(struct kimage *image, void *buf,
>>  				 unsigned long buf_len);
>>  #endif
>> -int arch_kexec_locate_mem_hole(struct kexec_buf *kbuf);
>>
>>  extern int kexec_add_buffer(struct kexec_buf *kbuf);
>>  int kexec_locate_mem_hole(struct kexec_buf *kbuf);
>>
>> +#ifndef arch_kexec_locate_mem_hole
>> +/**
>> + * arch_kexec_locate_mem_hole - Find free memory to place the segments.
>> + * @kbuf:                       Parameters for the memory search.
>> + *
>> + * On success, kbuf->mem will have the start address of the memory region found.
>> + *
>> + * Return: 0 on success, negative errno on error.
>> + */
>> +static inline int arch_kexec_locate_mem_hole(struct kexec_buf *kbuf)
>> +{
>> +	return kexec_locate_mem_hole(kbuf);
>> +}
>> +#endif
>> +
>>  /* Alignment required for elf header segment */
>>  #define ELF_CORE_HEADER_ALIGN   4096
>>
>> diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
>> index 9b2839775c837b..66e4ce29fc6961 100644
>> --- a/kernel/kexec_file.c
>> +++ b/kernel/kexec_file.c
>> @@ -56,14 +56,7 @@ int kexec_image_probe_default(struct kimage *image, void *buf,
>>  	return ret;
>>  }
>>
>> -/* Architectures can provide this probe function */
>> -int __weak arch_kexec_kernel_image_probe(struct kimage *image, void *buf,
>> -					 unsigned long buf_len)
>> -{
>> -	return kexec_image_probe_default(image, buf, buf_len);
>> -}
>> -
>> -static void *kexec_image_load_default(struct kimage *image)
>> +void *kexec_image_load_default(struct kimage *image)
>>  {
>>  	if (!image->fops || !image->fops->load)
>>  		return ERR_PTR(-ENOEXEC);
>> @@ -74,11 +67,6 @@ static void *kexec_image_load_default(struct kimage *image)
>>  				 image->cmdline_buf_len);
>>  }
>>
>> -void * __weak arch_kexec_kernel_image_load(struct kimage *image)
>> -{
>> -	return kexec_image_load_default(image);
>> -}
>> -
>>  int kexec_image_post_load_cleanup_default(struct kimage *image)
>>  {
>>  	if (!image->fops || !image->fops->cleanup)
>> @@ -87,11 +75,6 @@ int kexec_image_post_load_cleanup_default(struct kimage *image)
>>  	return image->fops->cleanup(image->image_loader_data);
>>  }
>>
>> -int __weak arch_kimage_file_post_load_cleanup(struct kimage *image)
>> -{
>> -	return kexec_image_post_load_cleanup_default(image);
>> -}
>> -
>>  #ifdef CONFIG_KEXEC_SIG
>>  static int kexec_image_verify_sig_default(struct kimage *image, void *buf,
>>  					  unsigned long buf_len)
>> @@ -104,8 +87,7 @@ static int kexec_image_verify_sig_default(struct kimage *image, void *buf,
>>  	return image->fops->verify_sig(buf, buf_len);
>>  }
>>
>> -int __weak arch_kexec_kernel_verify_sig(struct kimage *image, void *buf,
>> -					unsigned long buf_len)
>> +int arch_kexec_kernel_verify_sig(struct kimage *image, void *buf, unsigned long buf_len)
>>  {
>>  	return kexec_image_verify_sig_default(image, buf, buf_len);
>>  }
>> @@ -616,19 +598,6 @@ int kexec_locate_mem_hole(struct kexec_buf *kbuf)
>>  	return ret == 1 ? 0 : -EADDRNOTAVAIL;
>>  }
>>
>> -/**
>> - * arch_kexec_locate_mem_hole - Find free memory to place the segments.
>> - * @kbuf:                       Parameters for the memory search.
>> - *
>> - * On success, kbuf->mem will have the start address of the memory region found.
>> - *
>> - * Return: 0 on success, negative errno on error.
>> - */
>> -int __weak arch_kexec_locate_mem_hole(struct kexec_buf *kbuf)
>> -{
>> -	return kexec_locate_mem_hole(kbuf);
>> -}
>> -
>>  /**
>>   * kexec_add_buffer - place a buffer in a kexec segment
>>   * @kbuf:	Buffer contents and memory parameters.
>> --
>> 2.36.1
>>
>

-- 
Best regards,
Coiby