Re: [SeaBIOS] [PATCH 2/5] Introduce framework for detecting what platform SeaBIOS is running on.

[Fred .]

I see functions named coreboot_preinit() with underscore.
Then I see functions named runningOnCoreboot() with camelcase.

This is inconsistent naming.

Shouldn't you be sticking to some coding convention or naming guideline?

On Fri, Feb 8, 2013 at 1:56 PM, Fred .  wrote:
> Maybe would be nice with a:
> int get_platform();
> 0 = QEMU
> 1 = Xen
> 2 = Coreboot
> 3 = CSM
>
> Then you have one get_platform() function, instead of four separate
> startedOnQEMU(), startedOnCoreboot, startedOnCSM, etc functions.
>
> On Fri, Feb 8, 2013 at 6:07 AM, Kevin O'Connor  wrote:
> > Introduce startedOnQEMU()/startedOnCoreboot()/etc. calls to enable
> > code to determine what platform invoked the initial SeaBIOS startup.
> > Also introduce runningOnQEMU()/etc. calls for cases where SeaBIOS can
> > detect it is running on a platform even though it wasn't directly
> > launched by that platform (eg, Xen may have started SeaBIOS, but Xen
> > may be running under qemu).
> > Signed-off-by: Kevin O'Connor
> >
> > src/coreboot.c |  3 ++
> >  src/csm.c      |  2 ++
> >  src/paravirt.c | 10 +++++--
> >  src/paravirt.h | 93
> > ++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
> >  src/xen.c      |  1 +
> >  5 files changed, 104 insertions(+), 5 deletions(-)
> >
> > diff --git a/src/coreboot.c b/src/coreboot.c
> > index 57c9737..40a7e72 100644
> > --- a/src/coreboot.c
> > +++ b/src/coreboot.c
> > @@ -12,6 +12,7 @@
> >  #include "boot.h" // boot_add_cbfs
> >  #include "disk.h" // MAXDESCSIZE
> >  #include "config.h" // CONFIG_*
> > +#include "paravirt.h" // PlatformStartedOn
> >
> > /****************************************************************
> > @@ -145,6 +146,8 @@ coreboot_preinit(void)
> >      if (!cbm)
> >          goto fail;
> > PlatformStartedOn = PlatformRunningOn = PF_COREBOOT;
> > u64 maxram = 0, maxram_over4G = 0;
> >    int i, count = MEM_RANGE_COUNT(cbm);
> >    for (i=0; i<count; i++) {
> >
> > diff --git a/src/csm.c b/src/csm.c
> > index 169b608..c8069d2 100644
> > --- a/src/csm.c
> > +++ b/src/csm.c
> > @@ -17,6 +17,7 @@
> >  #include "boot.h"
> >  #include "smbios.h"
> >  #include "pic.h"
> > +#include "paravirt.h" // PlatformStartedOn
> >
> > struct rsdp_descriptor VAR32FLATVISIBLE __aligned(16) csm_rsdp;
> >
> > @@ -74,6 +75,7 @@ handle_csm_0000(struct bregs *regs)
> >      dprintf(3, "LoPmmMemory     %08x\n", csm_init_table->LowPmmMemory);
> >      dprintf(3, "LoPmmMemorySize %08x\n",
> > csm_init_table->LowPmmMemorySizeInBytes);
> > PlatformStartedOn = PlatformRunningOn = PF_CSM;
> > csm_malloc_preinit(csm_init_table->LowPmmMemory,
> >                    csm_init_table->LowPmmMemorySizeInBytes,
> >                    csm_init_table->HiPmmMemory,
> >
> > diff --git a/src/paravirt.c b/src/paravirt.c
> > index ebab256..35b7c11 100644
> > --- a/src/paravirt.c
> > +++ b/src/paravirt.c
> > @@ -19,6 +19,8 @@
> >  #include "mptable.h" // mptable_setup
> >  #include "pci.h" // create_pirtable
> >
> > +int PlatformStartedOn, PlatformRunningOn;
> >
> > int qemu_cfg_present;
> >
> > void
> > @@ -27,6 +29,7 @@ qemu_preinit(void)
> >      if (!CONFIG_QEMU)
> >          return;
> >
> > PlatformStartedOn = PlatformRunningOn = PF_QEMU;
> > qemu_cfg_preinit();
> >
> > // On emulators, get memory size from nvram.
> >
> > @@ -108,12 +111,13 @@ qemu_cfg_read_entry(void *buf, int e, int len)
> >
> > void qemu_cfg_preinit(void)
> >  {
> > if (!CONFIG_QEMU)
> >    return;
> > PlatformRunningOn |= PF_QEMU;
> > char *sig = "QEMU";
> >    int i;
> >
> > if (CONFIG_COREBOOT)
> >    return;
> >
> > qemu_cfg_present = 1;
> >
> > qemu_cfg_select(QEMU_CFG_SIGNATURE);
> >
> > diff --git a/src/paravirt.h b/src/paravirt.h
> > index 2448993..3b00697 100644
> > --- a/src/paravirt.h
> > +++ b/src/paravirt.h
> > @@ -1,8 +1,97 @@
> >  #ifndef __PV_H
> >  #define __PV_H
> >
> > -#include "config.h" // CONFIG_COREBOOT
> > -#include "util.h"
> > +#include "config.h" // CONFIG_*
> > +#include "util.h" // memcpy
> >
> > +/****************************************************************
> > Current platform detection
> > ****************************************************************/
> >
> > +#define PF_QEMU     (1<<0)
> > +#define PF_COREBOOT (1<<1)
> > +#define PF_XEN      (1<<2)
> > +#define PF_CSM      (1<<3)
> >
> > +extern int PlatformStartedOn, PlatformRunningOn;
> >
> > +static inline int startedOnQEMU(void)
> > +{
> > if (!CONFIG_QEMU)
> >    return 0;
> > if (!CONFIG_COREBOOT && !CONFIG_XEN && !CONFIG_CSM)
> >    return 1;
> > return PlatformStartedOn == PF_QEMU;
> >
> > +}
> >
> > +static inline int startedOnCoreboot(void)
> > +{
> > if (!CONFIG_COREBOOT)
> >    return 0;
> > if (!CONFIG_QEMU && !CONFIG_XEN && !CONFIG_CSM)
> >    return 1;
> > return PlatformStartedOn == PF_COREBOOT;
> >
> > +}
> >
> > +static inline int startedOnXen(void)
> > +{
> > if (!CONFIG_XEN)
> >    return 0;
> > if (!CONFIG_QEMU && !CONFIG_COREBOOT && !CONFIG_CSM)
> >    return 1;
> > return PlatformStartedOn == PF_XEN;
> >
> > +}
> >
> > +static inline int startedOnCSM(void)
> > +{
> > if (!CONFIG_CSM)
> >    return 0;
> > if (!CONFIG_QEMU && !CONFIG_COREBOOT && !CONFIG_XEN)
> >    return 1;
> > return PlatformStartedOn == PF_CSM;
> >
> > +}
> >
> > +static inline int runningOnQEMU(void)
> > +{
> > if (!CONFIG_QEMU)
> >    return 0;
> > if (!CONFIG_COREBOOT && !CONFIG_XEN && !CONFIG_CSM)
> >    return 1;
> > return PlatformRunningOn & PF_QEMU;
> >
> > +}
> >
> > +static inline int runningOnCoreboot(void)
> > +{
> > if (!CONFIG_COREBOOT)
> >    return 0;
> > if (!CONFIG_QEMU && !CONFIG_XEN && !CONFIG_CSM)
> >    return 1;
> > return PlatformRunningOn & PF_COREBOOT;
> >
> > +}
> >
> > +static inline int runningOnXen(void)
> > +{
> > if (!CONFIG_XEN)
> >    return 0;
> > if (!CONFIG_QEMU && !CONFIG_COREBOOT && !CONFIG_CSM)
> >    return 1;
> > return PlatformRunningOn & PF_XEN;
> >
> > +}
> >
> > +static inline int runningOnCSM(void)
> > +{
> > if (!CONFIG_CSM)
> >    return 0;
> > if (!CONFIG_QEMU && !CONFIG_COREBOOT && !CONFIG_XEN)
> >    return 1;
> > return PlatformRunningOn & PF_CSM;
> >
> > +}
> >
> > +/****************************************************************
> > KVM/QEMU firmware
> > ****************************************************************/
> >
> > /* This CPUID returns the signature 'KVMKVMKVM' in ebx, ecx, and edx.  It
> > should be used to determine that a VM is running under KVM.
> >
> > diff --git a/src/xen.c b/src/xen.c
> > index a506b42..5122a3c 100644
> > --- a/src/xen.c
> > +++ b/src/xen.c
> > @@ -81,6 +81,7 @@ void xen_preinit(void)
> >          return;
> >      }
> >
> > PlatformStartedOn = PlatformRunningOn = PF_XEN;
> > qemu_cfg_preinit();
> >
> > u64 maxram = 0, maxram_over4G = 0;
> >
> > SeaBIOS mailing list
> > [e-mail]
> > http://www.seabios.org/mailman/listinfo/seabios