| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Fri, 11 Jul 2014 21:33:11 -0400
From: Konrad Rzeszutek Wilk <konrad.wilk@...cle.com>
To: Daniel Kiper <daniel.kiper@...cle.com>
Cc: x86@...nel.org,
Stefano Stabellini <stefano.stabellini@...citrix.com>,
tglx@...utronix.de, andrew.cooper3@...rix.com,
Boris Ostrovsky <boris.ostrovsky@...cle.com>,
david.vrabel@...rix.com, linux-kernel@...r.kernel.org,
hpa@...or.com, matt.fleming@...el.com, jbeulich@...e.com,
xen-devel@...ts.xenproject.org, mingo@...hat.com, jeremy@...p.org,
ian.campbell@...rix.com
Subject: Re: [PATCH 2/2] arch/x86/xen: Silence compiler warnings
. snip ..
> > > Please loot at arch/x86/xen/enlighten.c:xen_check_mwait() and
> > > arch/x86/xen/enlighten.c:xen_boot_params_init_edd() (probably
> > > there are more stuff like that around). As I can see this is fairly
> > > common solution and probably compiler cope with it quite well.
> > >
> >
> > Those are some examples of some rather bad examples.
>
> What is wrong with them?
#ifdef should not be in C files. It is making the code a bit of a mess.
>
> > The way that is preferred in the Linux code is to have the ifdef in headers.
> >
> > See
> > http://lxr.free-electrons.com/source/arch/x86/include/asm/xen/swiotlb-xen.h
> > Or
> > http://lxr.free-electrons.com/source/arch/x86/include/asm/xen/pci.h
> >
> > You can create a similar file there and for the 32 bit implementation just make an empty static function.
> >
> > The 64 bit implementation has to be somewhere. Can it be in the Xen EFI file which is only compiled on 64 bit platforms?
>
> OK, this (putting declaration/definition in *.h file) makes sens if you
> declare/define functions which must be called from different places.
Right.
> However, xen_efi_init() is called only once in arch/x86/xen/enlighten.c.
And the vga (see arch/x86/xen/vga.c) is also called only once.
> Of course, I could define this function here in similar way like it is done
> in above headers but it take a bit more place. However, if you wish why not.
I was thinking something along this (not compile tested):
>From 436461a33cf93eed2cd96774bfca78fb08930de1 Mon Sep 17 00:00:00 2001
From: Konrad Rzeszutek Wilk <konrad.wilk@...cle.com>
Date: Fri, 11 Jul 2014 22:30:33 -0400
Subject: [PATCH] Like this.
---
arch/x86/xen/Makefile | 1 +
arch/x86/xen/efi.c | 22 ++++++++++++++++++++++
arch/x86/xen/enlighten.c | 23 +----------------------
arch/x86/xen/xen-ops.h | 8 ++++++++
4 files changed, 32 insertions(+), 22 deletions(-)
create mode 100644 arch/x86/xen/efi.c
diff --git a/arch/x86/xen/Makefile b/arch/x86/xen/Makefile
index b187df5..aa045ad 100644
--- a/arch/x86/xen/Makefile
+++ b/arch/x86/xen/Makefile
@@ -22,3 +22,4 @@ obj-$(CONFIG_PARAVIRT_SPINLOCKS)+= spinlock.o
obj-$(CONFIG_XEN_DEBUG_FS) += debugfs.o
obj-$(CONFIG_XEN_DOM0) += apic.o vga.o
obj-$(CONFIG_SWIOTLB_XEN) += pci-swiotlb-xen.o
+obj-$(CONFIG_XEN_EFI) += efi.c
diff --git a/arch/x86/xen/efi.c b/arch/x86/xen/efi.c
new file mode 100644
index 0000000..0b751d9
--- /dev/null
+++ b/arch/x86/xen/efi.c
@@ -0,0 +1,22 @@
+/* Need some headers. */
+
+extern efi_system_table_t __init *xen_efi_probe(void);
+
+void __init xen_efi_init(void)
+{
+ efi_system_table_t *efi_systab_xen;
+
+ efi_systab_xen = xen_efi_probe();
+
+ if (!efi_systab_xen)
+ return;
+
+ strncpy((char *)&boot_params.efi_info.efi_loader_signature, "Xen",
+ sizeof(boot_params.efi_info.efi_loader_signature));
+ boot_params.efi_info.efi_systab = (__u32)__pa(efi_systab_xen);
+ boot_params.efi_info.efi_systab_hi = (__u32)(__pa(efi_systab_xen) >> 32);
+
+ set_bit(EFI_BOOT, &efi.flags);
+ set_bit(EFI_NO_DIRECT, &efi.flags);
+ set_bit(EFI_64BIT, &efi.flags);
+}
diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
index abd8013..2d71db3 100644
--- a/arch/x86/xen/enlighten.c
+++ b/arch/x86/xen/enlighten.c
@@ -152,15 +152,6 @@ struct shared_info *HYPERVISOR_shared_info = &xen_dummy_shared_info;
*/
static int have_vcpu_info_placement = 1;
-#ifdef CONFIG_XEN_EFI
-extern efi_system_table_t __init *xen_efi_probe(void);
-#else
-static efi_system_table_t __init *xen_efi_probe(void)
-{
- return NULL;
-}
-#endif
-
struct tls_descs {
struct desc_struct desc[3];
};
@@ -1581,7 +1572,6 @@ asmlinkage __visible void __init xen_start_kernel(void)
{
struct physdev_set_iopl set_iopl;
int rc;
- efi_system_table_t *efi_systab_xen;
if (!xen_start_info)
return;
@@ -1777,18 +1767,7 @@ asmlinkage __visible void __init xen_start_kernel(void)
xen_setup_runstate_info(0);
- efi_systab_xen = xen_efi_probe();
-
- if (efi_systab_xen) {
- strncpy((char *)&boot_params.efi_info.efi_loader_signature, "Xen",
- sizeof(boot_params.efi_info.efi_loader_signature));
- boot_params.efi_info.efi_systab = (__u32)__pa(efi_systab_xen);
- boot_params.efi_info.efi_systab_hi = (__u32)(__pa(efi_systab_xen) >> 32);
-
- set_bit(EFI_BOOT, &efi.flags);
- set_bit(EFI_NO_DIRECT, &efi.flags);
- set_bit(EFI_64BIT, &efi.flags);
- }
+ xen_efi_init();
/* Start the world */
#ifdef CONFIG_X86_32
diff --git a/arch/x86/xen/xen-ops.h b/arch/x86/xen/xen-ops.h
index 12a884d..0908dec 100644
--- a/arch/x86/xen/xen-ops.h
+++ b/arch/x86/xen/xen-ops.h
@@ -127,4 +127,12 @@ __visible void xen_adjust_exception_frame(void);
extern int xen_panic_handler_init(void);
void xen_pvh_secondary_vcpu_init(int cpu);
+
+#ifdef CONFIG_X86_64
+void __init xen_efi_init(void);
+#else
+static inline void __init xen_efi_init(void)
+{
+}
+#endif
#endif /* XEN_OPS_H */
--
1.7.7.6
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists