diff mbox series

[v2,2/2] hw/i386: Fix linker error when ISAPC is disabled

Message ID 20190705143554.10295-2-julio.montes@intel.com (mailing list archive)
State New, archived
Headers show
Series [v2,1/2] Makefile: generate header file with the list of devices enabled | expand

Commit Message

Montes, Julio July 5, 2019, 2:35 p.m. UTC 
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 <julio.montes@intel.com>
---
 hw/i386/pc_piix.c    | 11 ++++++++---
 include/qemu/osdep.h |  1 +
 2 files changed, 9 insertions(+), 3 deletions(-)

--
2.17.2

Comments

Paolo Bonzini July 5, 2019, 4:34 p.m. UTC | #1 
On 05/07/19 16:35, 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 <julio.montes@intel.com>
> ---
>  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"
>  #include "config-target.h"
>  #else
>  #include "exec/poison.h"
> --
> 2.17.2
> 

Queued, thanks.

Paolo
Philippe Mathieu-Daudé July 5, 2019, 7:40 p.m. UTC | #2 
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 <julio.montes@intel.com>
> ---
>  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
>
diff mbox series

Patch

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"
 #include "config-target.h"
 #else
 #include "exec/poison.h"