Re: [PULL 00/20] hw/i386/pc: Split PIIX3 southbridge from i440FX northbridge

["Philippe Mathieu-Daudé" <philmd@redhat.com>]

Hi Paolo,

On 10/30/19 10:44 AM, Philippe Mathieu-Daudé wrote:
> Hi Peter, Paolo,
> 
> On 10/30/19 7:15 AM, Peter Maydell wrote:
>> On Mon, 28 Oct 2019 at 17:48, Philippe Mathieu-Daudé 
>> <philmd@redhat.com> wrote:
>>>
>>> ----------------------------------------------------------------
>>> The i440FX northbridge is only used by the PC machine, while the
>>> PIIX southbridge is also used by the Malta MIPS machine.
>>>
>>> Split the PIIX3 southbridge from i440FX northbridge.
>>>
>>> ----------------------------------------------------------------
>>
>> I get a link failure on my 'do a make clean and then make' build :
>>    LINK    i386-softmmu/qemu-system-i386
>> hw/i386/pc_piix.o: In function `pc_init1':
>> /home/petmay01/linaro/qemu-for-merges/hw/i386/pc_piix.c:197: undefined
>> reference to `i440fx_init'
>> /home/petmay01/linaro/qemu-for-merges/hw/i386/pc_piix.c:206: undefined
>> reference to `piix3_create'
>> collect2: error: ld returned 1 exit status
> 
> This is odd, default-configs/i386-softmmu.mak selects CONFIG_I440FX, 
> I440FX selects PCI_I440FX, and the Makefile.objs has:
> common-obj-$(CONFIG_PCI_I440FX) += i440fx.o
> 
> The change is in patch "hw/pci-host: Rename incorrectly named 'piix' as 
> 'i440fx'" which is a simple rename:
> 
> -- >8 --
> diff --git a/hw/i386/Kconfig b/hw/i386/Kconfig
> index d420b35548..5a494342ea 100644
> --- a/hw/i386/Kconfig
> +++ b/hw/i386/Kconfig
> @@ -60,7 +60,7 @@ config I440FX
>       select PC_PCI
>       select PC_ACPI
>       select ACPI_SMBUS
> -    select PCI_PIIX
> +    select PCI_I440FX
>       select PIIX3
>       select IDE_PIIX
>       select DIMM
> diff --git a/hw/pci-host/Kconfig b/hw/pci-host/Kconfig
> index 397043b289..b0aa8351c4 100644
> --- a/hw/pci-host/Kconfig
> +++ b/hw/pci-host/Kconfig
> @@ -28,7 +28,7 @@ config PCI_SABRE
>       select PCI
>       bool
> 
> -config PCI_PIIX
> +config PCI_I440FX
>       bool
>       select PCI
>       select PAM
> diff --git a/hw/pci-host/Makefile.objs b/hw/pci-host/Makefile.objs
> index a9cd3e022d..efd752b766 100644
> --- a/hw/pci-host/Makefile.objs
> +++ b/hw/pci-host/Makefile.objs
> @@ -13,7 +13,7 @@ common-obj-$(CONFIG_VERSATILE_PCI) += versatile.o
> 
>   common-obj-$(CONFIG_PCI_SABRE) += sabre.o
>   common-obj-$(CONFIG_FULONG) += bonito.o
> -common-obj-$(CONFIG_PCI_PIIX) += piix.o
> +common-obj-$(CONFIG_PCI_I440FX) += i440fx.o
>   common-obj-$(CONFIG_PCI_EXPRESS_Q35) += q35.o
>   common-obj-$(CONFIG_PCI_EXPRESS_GENERIC_BRIDGE) += gpex.o
>   common-obj-$(CONFIG_PCI_EXPRESS_XILINX) += xilinx-pcie.o
> diff --git a/hw/pci-host/piix.c b/hw/pci-host/i440fx.c
> similarity index 100%
> rename from hw/pci-host/piix.c
> rename to hw/pci-host/i440fx.c
> ---
> 
> I could reproduce and hw/pci-host/ doesn't contains neither piix.o nor 
> i440fx.o.
> 
> $ fgrep -ri i440fx i386-softmmu/
> i386-softmmu/config-devices.h-timestamp:#define CONFIG_I440FX 1
> i386-softmmu/config-devices.mak.old:CONFIG_I440FX=y
> Binary file i386-softmmu/hw/i386/pc_piix.o matches
> i386-softmmu/hw/i386/pc_piix.d: 
> /home/phil/source/qemu/include/hw/pci-host/i440fx.h \
> i386-softmmu/hw/i386/pc_piix.d:/home/phil/source/qemu/include/hw/pci-host/i440fx.h: 
> 
> Binary file i386-softmmu/hw/i386/acpi-build.o matches
> Binary file i386-softmmu/hw/i386/pc.o matches
> i386-softmmu/config-devices.mak:CONFIG_I440FX=y
> i386-softmmu/config-devices.h:#define CONFIG_I440FX 1
> 
> So CONFIG_I440FX is here, but no CONFIG_PCI_I440FX...
> 
> $ fgrep -r PIIX i386-softmmu/
> i386-softmmu/config-devices.h-timestamp:#define CONFIG_IDE_PIIX 1
> i386-softmmu/config-devices.h-timestamp:#define CONFIG_PCI_PIIX 1
> i386-softmmu/config-devices.mak.old:CONFIG_IDE_PIIX=y
> i386-softmmu/config-devices.mak.old:CONFIG_PCI_PIIX=y
> Binary file i386-softmmu/hw/i386/pc_piix.o matches
> Binary file i386-softmmu/hw/i386/acpi-build.o matches
> Binary file i386-softmmu/hw/i386/pc.o matches
> i386-softmmu/config-devices.mak:CONFIG_IDE_PIIX=y
> i386-softmmu/config-devices.mak:CONFIG_PCI_PIIX=y
> i386-softmmu/config-devices.h:#define CONFIG_IDE_PIIX 1
> i386-softmmu/config-devices.h:#define CONFIG_PCI_PIIX 1
> 
> And the old CONFIG_PCI_PIIX is still there :(
> 
> Paolo, is some kconfig dependency missing?

docs/devel/build-system.txt is misleading:

   - $TARGET-NAME/config-devices.mak

   TARGET-NAME is again the name of a system or userspace emulator. The
   config-devices.mak file is automatically generated by make using the
   scripts/make_device_config.sh program, feeding it the
   default-configs/$TARGET-NAME file as input.

But this script has been removed in e0e312f3525:

   build: switch to Kconfig

   The make_device_config.sh script is replaced by minikconf, which
   is modified to support the same command line as its predecessor.

Anyway, back to my problem. We modified hw/i386/Kconfig and 
hw/pci-host/Kconfig. i386-softmmu/config-devices.mak hasn't been rebuilt.

Here is the make rule matching i386-softmmu/config-devices.mak:

$(SUBDIR_DEVICES_MAK): %/config-devices.mak: default-configs/%.mak 
$(MINIKCONF_INPUTS) $(BUILD_DIR)/config-host.mak
	$(call quiet-command, $(MINIKCONF) $(MINIKCONF_ARGS) > $@.tmp, "GEN", 
"$@.tmp")

With:

MINIKCONF_INPUTS = $(SRC_PATH)/Kconfig.host $(SRC_PATH)/hw/Kconfig

So hw/{i386,pci-host}/Kconfig aren't matched and doesn't trigger the 
regeneration of i386-softmmu/config-devices.mak.

I'm not sure how to properly fix this.

Also, is the backslash useful at the end of this line in Makefile?

MINIKCONF = $(PYTHON) $(SRC_PATH)/scripts/minikconf.py \


Regards,

Phil.