Re: [Qemu-devel] [PATCH v2 2/2] hw/i386: Fix linker error when ISAPC is disabled

[Philippe Mathieu-Daudé]

Cc'ing Markus, Peter.

On 7/5/19 4:35 PM, Julio Montes wrote:
> v2: include config-devices.h to use CONFIG_IDE_ISA
> 
> ---
> In pc_init1(), ISA IDE is initialized without checking if ISAPC or IDE_ISA
> configs are enabled. This results in a link error when
> CONFIG_ISAPC is set to 'n' in the file default-configs/i386-softmmu.mak:
> 
> hw/i386/pc_piix.o: In function `pc_init1':
> hw/i386/pc_piix.c:261: undefined reference to `isa_ide_init'
> hw/i386/pc_piix.c:261: undefined reference to `isa_ide_init'
> 
> Place ide_isa code under #ifdef CONFIG_IDE_ISA to fix linker errors
> 
> Signed-off-by: Julio Montes <address@hidden>
> ---
>  hw/i386/pc_piix.c    | 11 ++++++++---
>  include/qemu/osdep.h |  1 +
>  2 files changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index f29de58636..c7d4645a3f 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -61,9 +61,11 @@
> 
>  #define MAX_IDE_BUS 2
> 
> +#ifdef CONFIG_IDE_ISA
>  static const int ide_iobase[MAX_IDE_BUS] = { 0x1f0, 0x170 };
>  static const int ide_iobase2[MAX_IDE_BUS] = { 0x3f6, 0x376 };
>  static const int ide_irq[MAX_IDE_BUS] = { 14, 15 };
> +#endif
> 
>  /* PC hardware initialisation */
>  static void pc_init1(MachineState *machine,
> @@ -254,7 +256,10 @@ static void pc_init1(MachineState *machine,
>          }
>          idebus[0] = qdev_get_child_bus(&dev->qdev, "ide.0");
>          idebus[1] = qdev_get_child_bus(&dev->qdev, "ide.1");
> -    } else {
> +        pc_cmos_init(pcms, idebus[0], idebus[1], rtc_state);
> +    }
> +#ifdef CONFIG_IDE_ISA
> +else {
>          for(i = 0; i < MAX_IDE_BUS; i++) {
>              ISADevice *dev;
>              char busname[] = "ide.0";
> @@ -268,9 +273,9 @@ static void pc_init1(MachineState *machine,
>              busname[4] = '0' + i;
>              idebus[i] = qdev_get_child_bus(DEVICE(dev), busname);
>          }
> +        pc_cmos_init(pcms, idebus[0], idebus[1], rtc_state);
>      }
> -
> -    pc_cmos_init(pcms, idebus[0], idebus[1], rtc_state);
> +#endif
> 
>      if (pcmc->pci_enabled && machine_usb(machine)) {
>          pci_create_simple(pci_bus, piix3_devfn + 2, "piix3-usb-uhci");
> diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
> index af2b91f0b8..f1c682e52c 100644
> --- a/include/qemu/osdep.h
> +++ b/include/qemu/osdep.h
> @@ -29,6 +29,7 @@
> 
>  #include "config-host.h"
>  #ifdef NEED_CPU_H
> +#include "config-devices.h"

So now a Kconfig change will update this header, and all our codebase
depends of "osdep.h", so we'll rebuild the whole project...

However so far only one (which happens to be arch-specific) file
requires this header: pc_piix.c.

If we move this header inclusion in "hw/hw.h", this already reduce the
impact (of invalidating ccache, and so on).

Another (not good enough) idea is to only include it where we need
kconfig definitions to be accessible, this single file?

>  #include "config-target.h"
>  #else
>  #include "exec/poison.h"
> --
> 2.17.2
>