diff mbox

[RFC,7/7] fw_cfg DMA for x86

Message ID 1437494626-3773-8-git-send-email-markmb@redhat.com
State New
Headers show

Commit Message

Marc Marí July 21, 2015, 4:03 p.m. UTC 
Enable fw_cfg for x86 machines. Create new machine to avoid
incompatibilites.

Signed-off-by: Marc Marí <markmb@redhat.com>
---
 hw/i386/pc.c         | 21 ++++++++++++++++++---
 hw/i386/pc_piix.c    | 25 +++++++++++++++++++++++--
 hw/i386/pc_q35.c     | 26 ++++++++++++++++++++++++--
 include/hw/i386/pc.h |  1 +
 4 files changed, 66 insertions(+), 7 deletions(-)

Comments

Peter Maydell July 21, 2015, 5:14 p.m. UTC | #1 
On 21 July 2015 at 17:03, Marc Marí <markmb@redhat.com> wrote:
> Enable fw_cfg for x86 machines. Create new machine to avoid
> incompatibilites.

> @@ -1391,7 +1399,14 @@ FWCfgState *pc_memory_init(MachineState *machine,
>                                          option_rom_mr,
>                                          1);
>
> -    fw_cfg = bochs_bios_init();
> +    if (guest_info->fw_cfg_dma) {
> +        as = g_malloc(sizeof(*as));
> +        address_space_init(as, ram_below_4g, "pc.as");
> +        fw_cfg = bochs_bios_init(as, BIOS_CFG_DMA_ADDR);
> +    } else {
> +        fw_cfg = bochs_bios_init(NULL, 0);
> +    }
> +

As with ARM: what's the rationale for choosing this particular
AddressSpace as the target for fw_cfg DMA? (I'm not saying it's
wrong, necessarily -- I was just surprised we didn't just use
the system address space.)

thanks
-- PMM
Marc Marí July 22, 2015, 9:06 a.m. UTC | #2 
On Tue, 21 Jul 2015 18:14:40 +0100
Peter Maydell <peter.maydell@linaro.org> wrote:

> On 21 July 2015 at 17:03, Marc Marí <markmb@redhat.com> wrote:
> > Enable fw_cfg for x86 machines. Create new machine to avoid
> > incompatibilites.
> 
> > @@ -1391,7 +1399,14 @@ FWCfgState *pc_memory_init(MachineState
> > *machine, option_rom_mr,
> >                                          1);
> >
> > -    fw_cfg = bochs_bios_init();
> > +    if (guest_info->fw_cfg_dma) {
> > +        as = g_malloc(sizeof(*as));
> > +        address_space_init(as, ram_below_4g, "pc.as");
> > +        fw_cfg = bochs_bios_init(as, BIOS_CFG_DMA_ADDR);
> > +    } else {
> > +        fw_cfg = bochs_bios_init(NULL, 0);
> > +    }
> > +
> 
> As with ARM: what's the rationale for choosing this particular
> AddressSpace as the target for fw_cfg DMA? (I'm not saying it's
> wrong, necessarily -- I was just surprised we didn't just use
> the system address space.)
> 
> thanks
> -- PMM

I took something that seemed logical for me, as the BIOS will be
working in 32 bits. But, as with ARM, I'm not really sure which
address space should I use for this purpose. I'd appreciate some help
in the issue.

Thanks
Marc
diff mbox

Patch

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 7661ea9..5b202fa 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -87,6 +87,7 @@  void pc_set_legacy_acpi_data_size(void)
 }
 
 #define BIOS_CFG_IOPORT 0x510
+#define BIOS_CFG_DMA_ADDR 0xfef00000
 #define FW_CFG_ACPI_TABLES (FW_CFG_ARCH_LOCAL + 0)
 #define FW_CFG_SMBIOS_ENTRIES (FW_CFG_ARCH_LOCAL + 1)
 #define FW_CFG_IRQ0_OVERRIDE (FW_CFG_ARCH_LOCAL + 2)
@@ -718,7 +719,7 @@  static unsigned int pc_apic_id_limit(unsigned int max_cpus)
     return x86_cpu_apic_id_from_index(max_cpus - 1) + 1;
 }
 
-static FWCfgState *bochs_bios_init(void)
+static FWCfgState *bochs_bios_init(AddressSpace *as, hwaddr fw_cfg_addr)
 {
     FWCfgState *fw_cfg;
     uint8_t *smbios_tables, *smbios_anchor;
@@ -727,7 +728,13 @@  static FWCfgState *bochs_bios_init(void)
     int i, j;
     unsigned int apic_id_limit = pc_apic_id_limit(max_cpus);
 
-    fw_cfg = fw_cfg_init_io(BIOS_CFG_IOPORT);
+    if (as && fw_cfg_addr) {
+        fw_cfg = fw_cfg_init_mem_wide(fw_cfg_addr + 8, fw_cfg_addr, 8,
+                                                    fw_cfg_addr + 10, as);
+    } else {
+        fw_cfg = fw_cfg_init_io(BIOS_CFG_IOPORT);
+    }
+
     /* FW_CFG_MAX_CPUS is a bit confusing/problematic on x86:
      *
      * SeaBIOS needs FW_CFG_MAX_CPUS for CPU hotplug, but the CPU hotplug
@@ -1302,6 +1309,7 @@  FWCfgState *pc_memory_init(MachineState *machine,
     MemoryRegion *ram_below_4g, *ram_above_4g;
     FWCfgState *fw_cfg;
     PCMachineState *pcms = PC_MACHINE(machine);
+    AddressSpace *as;
 
     assert(machine->ram_size == below_4g_mem_size + above_4g_mem_size);
 
@@ -1391,7 +1399,14 @@  FWCfgState *pc_memory_init(MachineState *machine,
                                         option_rom_mr,
                                         1);
 
-    fw_cfg = bochs_bios_init();
+    if (guest_info->fw_cfg_dma) {
+        as = g_malloc(sizeof(*as));
+        address_space_init(as, ram_below_4g, "pc.as");
+        fw_cfg = bochs_bios_init(as, BIOS_CFG_DMA_ADDR);
+    } else {
+        fw_cfg = bochs_bios_init(NULL, 0);
+    }
+
     rom_set_fw(fw_cfg);
 
     if (guest_info->has_reserved_memory && pcms->hotplug_memory.base) {
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 8167b12..bb94c87 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -74,6 +74,7 @@  static bool smbios_uuid_encoded = true;
 static bool gigabyte_align = true;
 static bool has_reserved_memory = true;
 static bool kvmclock_enabled = true;
+static bool fw_cfg_dma = true;
 
 /* PC hardware initialisation */
 static void pc_init1(MachineState *machine)
@@ -168,6 +169,7 @@  static void pc_init1(MachineState *machine)
     guest_info->isapc_ram_fw = !pci_enabled;
     guest_info->has_reserved_memory = has_reserved_memory;
     guest_info->rsdp_in_ram = rsdp_in_ram;
+    guest_info->fw_cfg_dma = fw_cfg_dma;
 
     if (smbios_defaults) {
         MachineClass *mc = MACHINE_GET_CLASS(machine);
@@ -304,9 +306,16 @@  static void pc_init1(MachineState *machine)
     }
 }
 
+static void pc_compat_2_4(MachineState *machine)
+{
+    fw_cfg_dma = false;
+}
+
 static void pc_compat_2_3(MachineState *machine)
 {
     PCMachineState *pcms = PC_MACHINE(machine);
+
+    pc_compat_2_4(machine);
     savevm_skip_section_footers();
     if (kvm_enabled()) {
         pcms->smm = ON_OFF_AUTO_OFF;
@@ -477,7 +486,7 @@  static void pc_i440fx_machine_options(MachineClass *m)
     m->hot_add_cpu = pc_hot_add_cpu;
 }
 
-static void pc_i440fx_2_4_machine_options(MachineClass *m)
+static void pc_i440fx_2_5_machine_options(MachineClass *m)
 {
     pc_i440fx_machine_options(m);
     m->default_machine_opts = "firmware=bios-256k.bin";
@@ -486,7 +495,19 @@  static void pc_i440fx_2_4_machine_options(MachineClass *m)
     m->is_default = 1;
 }
 
-DEFINE_I440FX_MACHINE(v2_4, "pc-i440fx-2.4", NULL,
+DEFINE_I440FX_MACHINE(v2_5, "pc-i440fx-2.5", NULL,
+                      pc_i440fx_2_5_machine_options)
+
+static void pc_i440fx_2_4_machine_options(MachineClass *m)
+{
+    pc_i440fx_machine_options(m);
+    m->default_machine_opts = "firmware=bios-256k.bin";
+    m->default_display = "std";
+    m->alias = NULL;
+    m->is_default = 0;
+}
+
+DEFINE_I440FX_MACHINE(v2_4, "pc-i440fx-2.4", pc_compat_2_4,
                       pc_i440fx_2_4_machine_options)
 
 
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 974aead..7230da6 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -61,6 +61,7 @@  static bool smbios_uuid_encoded = true;
  */
 static bool gigabyte_align = true;
 static bool has_reserved_memory = true;
+static bool fw_cfg_dma = true;
 
 /* PC hardware initialisation */
 static void pc_q35_init(MachineState *machine)
@@ -156,6 +157,7 @@  static void pc_q35_init(MachineState *machine)
     guest_info->has_acpi_build = has_acpi_build;
     guest_info->has_reserved_memory = has_reserved_memory;
     guest_info->rsdp_in_ram = rsdp_in_ram;
+    guest_info->fw_cfg_dma = fw_cfg_dma;
 
     /* Migration was not supported in 2.0 for Q35, so do not bother
      * with this hack (see hw/i386/acpi-build.c).
@@ -287,9 +289,16 @@  static void pc_q35_init(MachineState *machine)
     }
 }
 
+static void pc_compat_2_4(MachineState *machine)
+{
+    fw_cfg_dma = false;
+}
+
 static void pc_compat_2_3(MachineState *machine)
 {
     PCMachineState *pcms = PC_MACHINE(machine);
+
+    pc_compat_2_4(machine);
     savevm_skip_section_footers();
     if (kvm_enabled()) {
         pcms->smm = ON_OFF_AUTO_OFF;
@@ -392,7 +401,7 @@  static void pc_q35_machine_options(MachineClass *m)
     m->units_per_default_bus = 1;
 }
 
-static void pc_q35_2_4_machine_options(MachineClass *m)
+static void pc_q35_2_5_machine_options(MachineClass *m)
 {
     pc_q35_machine_options(m);
     m->default_machine_opts = "firmware=bios-256k.bin";
@@ -402,7 +411,20 @@  static void pc_q35_2_4_machine_options(MachineClass *m)
     m->alias = "q35";
 }
 
-DEFINE_Q35_MACHINE(v2_4, "pc-q35-2.4", NULL,
+DEFINE_Q35_MACHINE(v2_5, "pc-q35-2.5", NULL,
+                   pc_q35_2_5_machine_options);
+
+static void pc_q35_2_4_machine_options(MachineClass *m)
+{
+    pc_q35_machine_options(m);
+    m->default_machine_opts = "firmware=bios-256k.bin";
+    m->default_display = "std";
+    m->no_floppy = 1;
+    m->no_tco = 0;
+    m->alias = NULL;
+}
+
+DEFINE_Q35_MACHINE(v2_4, "pc-q35-2.4", pc_compat_2_4,
                    pc_q35_2_4_machine_options);
 
 
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 15e3352..6c7eb65 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -103,6 +103,7 @@  struct PcGuestInfo {
     bool has_acpi_build;
     bool has_reserved_memory;
     bool rsdp_in_ram;
+    bool fw_cfg_dma;
 };
 
 /* parallel.c */