Patchwork [QEMU,RFC,5/7] pc: write lapic info (apic IDs) to fw_cfg so seabios can use it

login
register
mail settings
Submitter Eduardo Habkost
Date July 10, 2012, 8:22 p.m.
Message ID <1341951743-2285-6-git-send-email-ehabkost@redhat.com>
Download mbox | patch
Permalink /patch/170279/
State New
Headers show

Comments

Eduardo Habkost - July 10, 2012, 8:22 p.m.
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 hw/pc.c |   23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)
Blue Swirl - July 12, 2012, 7:29 p.m.
On Tue, Jul 10, 2012 at 8:22 PM, Eduardo Habkost <ehabkost@redhat.com> wrote:
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
>  hw/pc.c |   23 +++++++++++++++++++++++
>  1 file changed, 23 insertions(+)
>
> diff --git a/hw/pc.c b/hw/pc.c
> index 3b8e469..dc95fb8 100644
> --- a/hw/pc.c
> +++ b/hw/pc.c
> @@ -71,6 +71,7 @@
>  #define FW_CFG_IRQ0_OVERRIDE (FW_CFG_ARCH_LOCAL + 2)
>  #define FW_CFG_E820_TABLE (FW_CFG_ARCH_LOCAL + 3)
>  #define FW_CFG_HPET (FW_CFG_ARCH_LOCAL + 4)
> +#define FW_CFG_LAPIC_INFO (FW_CFG_ARCH_LOCAL + 5)
>
>  #define MSI_ADDR_BASE 0xfee00000
>
> @@ -87,6 +88,18 @@ struct e820_table {
>      struct e820_entry entry[E820_NR_ENTRIES];
>  } QEMU_PACKED __attribute((__aligned__(4)));
>
> +struct lapic_info_entry {
> +    uint8_t apic_id;
> +    uint8_t reserved1;
> +    uint16_t reserved2;
> +    uint32_t reserved3;
> +} QEMU_PACKED;

CODING_STYLE requires CamelCase, also below.

> +
> +struct lapic_info_table {
> +    uint64_t count;
> +    struct lapic_info_entry entries[];
> +} QEMU_PACKED;
> +
>  static struct e820_table e820_table;
>  struct hpet_fw_config hpet_cfg = {.count = UINT8_MAX};
>
> @@ -652,6 +665,16 @@ static void *bochs_bios_init(void)
>      fw_cfg_add_bytes(fw_cfg, FW_CFG_NUMA, (uint8_t *)numa_fw_cfg,
>                       (1 + max_cpus + nb_numa_nodes) * 8);
>
> +    size_t lapic_info_size = sizeof(struct lapic_info_table) + \
> +                             sizeof(struct lapic_info_entry) * max_cpus;
> +    struct lapic_info_table *lapic_table = g_malloc0(lapic_info_size);;

;--

> +    lapic_table->count = max_cpus;
> +    for (i = 0; i < max_cpus; i++) {
> +        lapic_table->entries[i].apic_id = apic_id_for_cpu(i);
> +    }
> +    fw_cfg_add_bytes(fw_cfg, FW_CFG_LAPIC_INFO, (uint8_t *)lapic_table,

You are passing a structure with host endianness. Please convert the
fields with cpu_to_le32().

> +                     lapic_info_size);
> +
>      return fw_cfg;
>  }
>
> --
> 1.7.10.4
>
>
Eduardo Habkost - July 13, 2012, 6:09 p.m.
On Thu, Jul 12, 2012 at 07:29:02PM +0000, Blue Swirl wrote:
[...]
> CODING_STYLE requires CamelCase, also below.
> 
[...]
> > +    struct lapic_info_table *lapic_table = g_malloc0(lapic_info_size);;
> 
> ;--
> 
> > +    lapic_table->count = max_cpus;
> > +    for (i = 0; i < max_cpus; i++) {
> > +        lapic_table->entries[i].apic_id = apic_id_for_cpu(i);
> > +    }
> > +    fw_cfg_add_bytes(fw_cfg, FW_CFG_LAPIC_INFO, (uint8_t *)lapic_table,
> 
> You are passing a structure with host endianness. Please convert the
> fields with cpu_to_le32().

Thanks. I will fix the issues if I send a new version of this patch. But
I believe we'll find a way to make FW_CFG_LAPIC_INFO unnecessary.

Patch

diff --git a/hw/pc.c b/hw/pc.c
index 3b8e469..dc95fb8 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -71,6 +71,7 @@ 
 #define FW_CFG_IRQ0_OVERRIDE (FW_CFG_ARCH_LOCAL + 2)
 #define FW_CFG_E820_TABLE (FW_CFG_ARCH_LOCAL + 3)
 #define FW_CFG_HPET (FW_CFG_ARCH_LOCAL + 4)
+#define FW_CFG_LAPIC_INFO (FW_CFG_ARCH_LOCAL + 5)
 
 #define MSI_ADDR_BASE 0xfee00000
 
@@ -87,6 +88,18 @@  struct e820_table {
     struct e820_entry entry[E820_NR_ENTRIES];
 } QEMU_PACKED __attribute((__aligned__(4)));
 
+struct lapic_info_entry {
+    uint8_t apic_id;
+    uint8_t reserved1;
+    uint16_t reserved2;
+    uint32_t reserved3;
+} QEMU_PACKED;
+
+struct lapic_info_table {
+    uint64_t count;
+    struct lapic_info_entry entries[];
+} QEMU_PACKED;
+
 static struct e820_table e820_table;
 struct hpet_fw_config hpet_cfg = {.count = UINT8_MAX};
 
@@ -652,6 +665,16 @@  static void *bochs_bios_init(void)
     fw_cfg_add_bytes(fw_cfg, FW_CFG_NUMA, (uint8_t *)numa_fw_cfg,
                      (1 + max_cpus + nb_numa_nodes) * 8);
 
+    size_t lapic_info_size = sizeof(struct lapic_info_table) + \
+                             sizeof(struct lapic_info_entry) * max_cpus;
+    struct lapic_info_table *lapic_table = g_malloc0(lapic_info_size);;
+    lapic_table->count = max_cpus;
+    for (i = 0; i < max_cpus; i++) {
+        lapic_table->entries[i].apic_id = apic_id_for_cpu(i);
+    }
+    fw_cfg_add_bytes(fw_cfg, FW_CFG_LAPIC_INFO, (uint8_t *)lapic_table,
+                     lapic_info_size);
+
     return fw_cfg;
 }