Re: [SeaBIOS] [PATCHv2 0/6] Improved multi-platform support

[Ian Campbell]

On Mon, 2013-02-11 at 10:54 +0000, David Woodhouse wrote:
> On Sat, 2013-02-09 at 14:08 -0500, Kevin O'Connor wrote:
> > This patch series is less ambitious than the previous - SeaBIOS can't
> > be compiled for multiple platforms (eg, QEMU, CSM, coreboot) at the
> > same time.
>
> Out of interest, why not include Xen in that list? Do we gain any real
> benefit from building a multi-platform binary that can be used both
> natively under qemu *and* from Xen?

I suppose it could be argued that from the PoV of SeaBIOS a Xen guest
(which uses qemu for device models) is not all that different from
qemu/kvm.

I don't know if distros prefer to have fewer images or not, once you
have 2 I guess having N is not such a big deal for something the size
and build time of SeaBIOS.

I don't have any objections to either approach from the Xen PoV.
> Thoroughly untested patch on top of your series at
> git://github.com/KevinOConnor/seabios.git test-20130209
>
> How would I go about testing this myself?

You would need to install and configure Xen. It's not as complex as it
once was but it isn't a 5 minute hack either. I'm happy to give details
if you like though.
> And should CONFIG_XEN select
> CONFIG_QEMU_HARDWARE, as CONFIG_QEMU does?

Does QEMU_HARDWARE mean hardware "defined" (so to speak) by QEMU (e.g.
virtio type stuff) or does it include regular hardware emulated by QEMU
(e.g. real IDE disks etc). Xen uses the latter but not the former. The
use of the symbol in both the CONFIG_VIRTIO_FOO and scsi_drive_setup()
suggests it covers both?
> There a no users of runningOnXen() now, so perhaps that can be removed too?

Might as well?
> By losing some of the runtime tests and making them purely config-based,
> the image size for Xen drops from 139KiB to 116KiB. And that's with
> MTRR, SMM and all the bios tables turned off in the config.
>
> commit eff724e682ce557cd9d4a09d7892cb28950bb886
> Author: David Woodhouse 
> Date:   Mon Feb 11 10:47:47 2013 +0000
> Make Xen one of the top-level build target choices
>
> Signed-off-by: David Woodhouse <[e-mail]>
> diff --git a/src/Kconfig b/src/Kconfig
> index 6dbea79..f5dab76 100644
> --- a/src/Kconfig
> +++ b/src/Kconfig
> @@ -14,10 +14,10 @@ choice
>              Configure as a coreboot payload.
>  config QEMU
>    bool "Build for QEMU/Xen/KVM/Bochs"
>    bool "Build for QEMU/KVM/Bochs"
>    select QEMU_HARDWARE
>    help
>        Configure for an emulated machine (QEMU, Xen, KVM, or Bochs).
>        Configure for an emulated machine (QEMU, KVM, or Bochs).
>
> config CSM
>       bool "Build as Compatibilty Support Module for EFI BIOS"
>
> @@ -25,6 +25,11 @@ choice
>             Configure to be used by EFI firmware as Compatibility Support
>             module (CSM) to provide legacy BIOS services.
> config XEN
>    bool "Build for Xen HVM"
>    help
>        Configure to be used by xen hvmloader, for a HVM guest.
>
> endchoice
>  config QEMU_HARDWARE
>
> @@ -34,13 +39,6 @@ endchoice
>              Support virtual hardware when the code detects it is
>              running on an emulator.
> config XEN
>    depends on QEMU
>    bool "Support Xen HVM"
>    default y
>    help
>        Configure to be used by xen hvmloader, for a HVM guest.
> config THREADS
>        bool "Parallelize hardware init"
>        default y
>
> diff --git a/src/mtrr.c b/src/mtrr.c
> index 0575b14..56f85f9 100644
> --- a/src/mtrr.c
> +++ b/src/mtrr.c
> @@ -6,7 +6,6 @@
>
> #include "util.h" // dprintf
>  #include "config.h" // CONFIG_*
> -#include "paravirt.h" // runningOnXen
>  #include "pci.h" // pcimem_start
>
> #define MSR_MTRRcap                    0x000000fe
> @@ -34,7 +33,7 @@
>
> void mtrr_setup(void)
>  {
> if (!CONFIG_MTRR_INIT || runningOnXen())
>
> if (!CONFIG_MTRR_INIT)
>     return;
>
> u32 eax, ebx, ecx, edx, cpuid_features;
>
> diff --git a/src/pciinit.c b/src/pciinit.c
> index 1d34653..0781b96 100644
> --- a/src/pciinit.c
> +++ b/src/pciinit.c
> @@ -11,7 +11,6 @@
>  #include "pci_regs.h" // PCI_COMMAND
>  #include "ioport.h" // PORT_ATA1_CMD_BASE
>  #include "config.h" // CONFIG_*
> -#include "paravirt.h" // runningOnXen
>  #include "memmap.h" // add_e820
>  #include "dev-q35.h"
>
> @@ -734,7 +733,7 @@ static void pci_bios_map_devices(struct pci_bus *busses)
>  void
>  pci_setup(void)
>  {
> if (!CONFIG_QEMU || runningOnXen()) {
> if (!CONFIG_QEMU) {
>     // PCI setup already done by coreboot or Xen - just do probe.
>     pci_probe_devices();
>     return;
>
> diff --git a/src/post.c b/src/post.c
> index 2c5e34e..c76144a 100644
> --- a/src/post.c
> +++ b/src/post.c
> @@ -179,7 +179,7 @@ platform_hardware_setup(void)
>      // Setup external BIOS interface tables
>      if (CONFIG_COREBOOT)
>          coreboot_biostable_setup();
> else if (runningOnXen())
> else if (CONFIG_XEN)
>     xen_biostable_setup();
> else
>     qemu_biostable_setup();
>
> @@ -319,7 +319,7 @@ dopost(void)
>      qemu_cfg_preinit();
>      if (CONFIG_COREBOOT)
>          coreboot_preinit();
> else if (runningOnXen())
> else if (CONFIG_XEN)
>     xen_ramsize_preinit();
> else
>     qemu_ramsize_preinit();
>
> diff --git a/src/shadow.c b/src/shadow.c
> index c9e8165..026d966 100644
> --- a/src/shadow.c
> +++ b/src/shadow.c
> @@ -10,7 +10,6 @@
>  #include "config.h" // CONFIG_*
>  #include "pci_ids.h" // PCI_VENDOR_ID_INTEL
>  #include "pci_regs.h" // PCI_VENDOR_ID
> -#include "paravirt.h" // runningOnXen
>  #include "dev-q35.h" // PCI_VENDOR_ID_INTEL
>
> // On the emulators, the bios at 0xf0000 is also at 0xffff0000
> @@ -119,7 +118,7 @@ static const struct pci_device_id dram_controller_make_readonly_tbl[] = {
>  void
>  make_bios_writable(void)
>  {
> if (!CONFIG_QEMU || runningOnXen())
>
> if (!CONFIG_QEMU)
>     return;
>
> dprintf(3, "enabling shadow ram\n");
>
> @@ -148,7 +147,7 @@ make_bios_writable(void)
>  void
>  make_bios_readonly(void)
>  {
> if (!CONFIG_QEMU || runningOnXen())
>
> if (!CONFIG_QEMU)
>     return;
>
> dprintf(3, "locking shadow ram\n");
>
> @@ -161,7 +160,7 @@ make_bios_readonly(void)
>  void
>  qemu_prep_reset(void)
>  {
> if (!CONFIG_QEMU || runningOnXen())
> if (!CONFIG_QEMU)
>     return;
> // QEMU doesn't map 0xc0000-0xfffff back to the original rom on a
> // reset, so do that manually before invoking a hard reset.
>
> diff --git a/src/smm.c b/src/smm.c
> index 4128296..2083804 100644
> --- a/src/smm.c
> +++ b/src/smm.c
> @@ -10,7 +10,6 @@
>  #include "config.h" // CONFIG_*
>  #include "ioport.h" // outb
>  #include "pci_ids.h" // PCI_VENDOR_ID_INTEL
> -#include "paravirt.h" // runningOnXen
>  #include "dev-q35.h"
>
> ASM32FLAT(
> @@ -184,8 +183,8 @@ static const struct pci_device_id smm_init_tbl[] = {
>  void
>  smm_setup(void)
>  {
> if (!CONFIG_USE_SMM || runningOnXen())
> return;
>
> if (!CONFIG_USE_SMM)
>    return;
>
> dprintf(3, "init smm\n");
>    pci_find_init_device(smm_init_tbl, NULL);
>
> diff --git a/src/xen.c b/src/xen.c
> index 32edcd1..e8ca9e0 100644
> --- a/src/xen.c
> +++ b/src/xen.c
> @@ -76,10 +76,9 @@ void xen_preinit(void)
>              break;
>          }
>      }
> if (!xen_cpuid_base) {
>    dprintf(1, "No Xen hypervisor found.\n");
>    return;
> }
> if (!xen_cpuid_base)
>    panic("No Xen hypervisor found.\n");
> PlatformRunningOn = PF_QEMU|PF_XEN;
>
> }
>
> @@ -95,7 +94,7 @@ void xen_hypercall_setup(void)
>      xen_extraversion_t extraversion;
>      unsigned long i;
> if (!runningOnXen())
>
> if (!CONFIG_XEN)
>     return;
>
> cpuid(xen_cpuid_base + 2, &eax, &ebx, &ecx, &edx);