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

Kairui Song kasong at redhat.com
Wed Jul 3 02:08:14 PDT 2019


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:

---
 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



More information about the kexec mailing list