On Mon, Mar 18, 2013 at 02:03:42PM +0100, Laszlo Ersek wrote:
On 03/18/13 12:00, Michael S. Tsirkin wrote:
Since commit f7e4dd6c18ccfbaf6cd2f5eaaed2b77cabc8a406 QEMU loads its own copy of DSDT, so let's not build in PIIX. This makes building in the DSDT an option, default to on (built-in). For the builds bundled with qemu (where we _know_ qemu is new enougth that it actually works) we can flip the switch to 'n' (via roms/config.seabios). This way we save some space in the rom file and make it easy to avoid shipping dead code in qemu. At some point we might be able to switch it off by default and then maybe remove altogether.
With CONFIG_ACPI_DSDT = y Total size: 127348 Fixed: 58892 Free: 3724 (used 97.2% of 128KiB rom) With CONFIG_ACPI_DSDT = n Total size: 122844 Fixed: 58884 Free: 8228 (used 93.7% of 128KiB rom)
Signed-off-by: Michael S. Tsirkin mst@redhat.com
Changes from v2: - fixed and extended the commit log addressing comments by Gerd, Laszlo, Paul. No functional changes.
Did you ignore my v2 comment below on purpose, or just miss it?
I missed it, sorry.
diff --git a/src/acpi.c b/src/acpi.c index 119d1c1..69dd3aa 100644 --- a/src/acpi.c +++ b/src/acpi.c @@ -202,7 +202,11 @@ struct srat_memory_affinity u32 reserved3[2]; } PACKED;
+#ifdef CONFIG_ACPI_DSDT #include "acpi-dsdt.hex" +#else +static u8 AmlCode[1]; +#endif
On 03/18/13 11:41, Laszlo Ersek wrote:
The macro CONFIG_ACPI_DSDT is always defined (you use it just below in a controlling expression); its value is what varies.
In other words, "#ifdef CONFIG_ACPI_DSDT" always evaluates to true, independently of how the new config option is set. The space is probably saved only because gcc sees (with -fwhole-program) that there are no accesses to AmlCode[]. Ultimately the effect matches your high-level intent, but the above #ifdef code doesn't do what you expect it to do.
I think you should write
#if CONFIG_ACPI_DSDT == 1CONFIG_XXXX macros are not used in preprocessing directives very frequently, but see
$ git grep CONFIG_ | grep '# *if' [...] src/pirtable.c:#if CONFIG_PIRTABLE src/pmm.c:#if CONFIG_PMM src/pnpbios.c:#if CONFIG_PNPBIOS src/romlayout.S:#if CONFIG_DISABLE_A20 src/romlayout.S:#if CONFIG_PCIBIOS src/romlayout.S:#if CONFIG_ENTRY_EXTRASTACK [...] vgasrc/vgaentry.S:#if CONFIG_VGA_PCI == 1"#if" and "#ifdef" are different.
Laszlo
Right. Thanks for the correction.