[PATCH kexec-tools] x86: re-order includes to avoid duplicate struct e820entry

Dave Young dyoung at redhat.com
Thu Jul 4 19:28:51 PDT 2019


Hi Kairui,
On 07/03/19 at 05:08pm, Kairui Song wrote:
> On Wed, Jul 3, 2019 at 4:34 PM Dave Young <dyoung at redhat.com> wrote:
> >
> > On 07/03/19 at 10:04am, Simon Horman wrote:
> > > xenctrl.h defines struct e820entry as:
> > >
> > >       if defined(__i386__) || defined(__x86_64__)
> > >       ...
> > >       #define E820_RAM        1
> > >       ...
> > >       struct e820entry {
> > >               uint64_t addr;
> > >               uint64_t size;
> > >               uint32_t type;
> > >       } __attribute__((packed));
> > >       ...
> > >       #endif
> > >
> > >  $ dpkg-query -S /usr/include/xenctrl.h
> > >  libxen-dev:amd64: /usr/include/xenctrl.h
> > >  $  dpkg-query -W libxen-dev:amd64
> > >  libxen-dev:amd64     4.8.5+shim4.10.2+xsa282-1+deb9u11
> > >
> > > ./include/x86/x86-linux.h defines struct e820entry as:
> > >
> > >       #ifndef E820_RAM
> > >       struct e820entry {
> > >               uint64_t addr;  /* start of memory segment */
> > >               uint64_t size;  /* size of memory segment */
> > >               uint32_t type;  /* type of memory segment */
> > >               #define E820_RAM    1
> > >               ...
> > >       } __attribute__((packed));
> > >       #endif
> > >
> > > Since cedeee0a3007 ("x86: Introduce helpers for getting RSDP address")
> > > ./kexec/arch/i386/kexec-x86-common.c includes
> > >
> > >       +#include "x86-linux-setup.h"
> > >        #include "../../kexec-xen.h"
> >
> > Not sure if those get rsdp code can go to x86-linux-setup.c then no need
> > the including.
> >
> > Let's see if Kairui has some thoughts.
> >
> 
> Yes, move the helper to x86-linux-setup.c should fix it too. So
> following patch should also be able to fix it:

It would be better to send a formal patch, seems the patch format is not
correct. no sob, and no ident.

> 
> ---
>  kexec/arch/i386/kexec-x86-common.c | 45 ------------------------------
>  kexec/arch/i386/kexec-x86.h        |  1 -
>  kexec/arch/i386/x86-linux-setup.c  | 43 ++++++++++++++++++++++++++++
>  kexec/arch/i386/x86-linux-setup.h  |  2 +-
>  4 files changed, 44 insertions(+), 47 deletions(-)
> 
> diff --git a/kexec/arch/i386/kexec-x86-common.c
> b/kexec/arch/i386/kexec-x86-common.c
> index 5c55ec8..63a2823 100644
> --- a/kexec/arch/i386/kexec-x86-common.c
> +++ b/kexec/arch/i386/kexec-x86-common.c
> @@ -39,7 +39,6 @@
>  #include "../../firmware_memmap.h"
>  #include "../../crashdump.h"
>  #include "kexec-x86.h"
> -#include "x86-linux-setup.h"
>  #include "../../kexec-xen.h"
> 
>  /* Used below but not present in (older?) xenctrl.h */
> @@ -392,47 +391,3 @@ int get_memory_ranges(struct memory_range
> **range, int *ranges,
> 
>   return ret;
>  }
> -
> -static uint64_t bootparam_get_acpi_rsdp(void) {
> - uint64_t acpi_rsdp = 0;
> - off_t offset = offsetof(struct x86_linux_param_header, acpi_rsdp_addr);
> -
> - if (get_bootparam(&acpi_rsdp, offset, sizeof(acpi_rsdp)))
> - return 0;
> -
> - return acpi_rsdp;
> -}
> -
> -static uint64_t efi_get_acpi_rsdp(void) {
> - FILE *fp;
> - char line[MAX_LINE], *s;
> - uint64_t acpi_rsdp = 0;
> -
> - fp = fopen("/sys/firmware/efi/systab", "r");
> - if (!fp)
> - return acpi_rsdp;
> -
> - while(fgets(line, sizeof(line), fp) != 0) {
> - /* ACPI20= always goes before ACPI= */
> - if ((strstr(line, "ACPI20=")) || (strstr(line, "ACPI="))) {
> - s = strchr(line, '=') + 1;
> - sscanf(s, "0x%lx", &acpi_rsdp);
> - break;
> - }
> - }
> - fclose(fp);
> -
> - return acpi_rsdp;
> -}
> -
> -uint64_t get_acpi_rsdp(void)
> -{
> - uint64_t acpi_rsdp = 0;
> -
> - acpi_rsdp = bootparam_get_acpi_rsdp();
> -
> - if (!acpi_rsdp)
> - acpi_rsdp = efi_get_acpi_rsdp();
> -
> - return acpi_rsdp;
> -}
> diff --git a/kexec/arch/i386/kexec-x86.h b/kexec/arch/i386/kexec-x86.h
> index 1b58c3b..c2bcd37 100644
> --- a/kexec/arch/i386/kexec-x86.h
> +++ b/kexec/arch/i386/kexec-x86.h
> @@ -86,5 +86,4 @@ int nbi_load(int argc, char **argv, const char *buf,
> off_t len,
>  void nbi_usage(void);
> 
>  extern unsigned xen_e820_to_kexec_type(uint32_t type);
> -extern uint64_t get_acpi_rsdp(void);
>  #endif /* KEXEC_X86_H */
> diff --git a/kexec/arch/i386/x86-linux-setup.c
> b/kexec/arch/i386/x86-linux-setup.c
> index 057ee14..588b1f4 100644
> --- a/kexec/arch/i386/x86-linux-setup.c
> +++ b/kexec/arch/i386/x86-linux-setup.c
> @@ -846,6 +846,49 @@ out:
>   return;
>  }
> 
> +static uint64_t bootparam_get_acpi_rsdp(void) {
> + uint64_t acpi_rsdp = 0;
> + off_t offset = offsetof(struct x86_linux_param_header, acpi_rsdp_addr);
> +
> + if (get_bootparam(&acpi_rsdp, offset, sizeof(acpi_rsdp)))
> + return 0;
> +
> + return acpi_rsdp;
> +}
> +
> +static uint64_t efi_get_acpi_rsdp(void) {
> + FILE *fp;
> + char line[MAX_LINE], *s;
> + uint64_t acpi_rsdp = 0;
> +
> + fp = fopen("/sys/firmware/efi/systab", "r");
> + if (!fp)
> + return acpi_rsdp;
> +
> + while(fgets(line, sizeof(line), fp) != 0) {
> + /* ACPI20= always goes before ACPI= */
> + if ((strstr(line, "ACPI20=")) || (strstr(line, "ACPI="))) {
> + s = strchr(line, '=') + 1;
> + sscanf(s, "0x%lx", &acpi_rsdp);
> + break;
> + }
> + }
> + fclose(fp);
> +
> + return acpi_rsdp;
> +}
> +
> +uint64_t get_acpi_rsdp(void)
> +{
> + uint64_t acpi_rsdp = 0;
> +
> + acpi_rsdp = bootparam_get_acpi_rsdp();
> +
> + if (!acpi_rsdp)
> + acpi_rsdp = efi_get_acpi_rsdp();
> +
> + return acpi_rsdp;
> +}
>  void setup_linux_system_parameters(struct kexec_info *info,
>      struct x86_linux_param_header *real_mode)
>  {
> diff --git a/kexec/arch/i386/x86-linux-setup.h
> b/kexec/arch/i386/x86-linux-setup.h
> index 0c651e5..1e81805 100644
> --- a/kexec/arch/i386/x86-linux-setup.h
> +++ b/kexec/arch/i386/x86-linux-setup.h
> @@ -22,7 +22,7 @@ static inline void setup_linux_bootloader_parameters(
>  void setup_linux_system_parameters(struct kexec_info *info,
>   struct x86_linux_param_header *real_mode);
>  int get_bootparam(void *buf, off_t offset, size_t size);
> -
> +uint64_t get_acpi_rsdp(void);
> 
>  #define SETUP_BASE    0x90000
>  #define KERN32_BASE  0x100000 /* 1MB */
> -- 
> 2.21.0
> 
> Best Regards,
> Kairui Song

Thanks
Dave



More information about the kexec mailing list