Re: [PATCH 01/10] hw/i386: Store pointers to IDE buses in PCMachineState

[Philippe Mathieu-Daudé]

Hi Peter,

On 20/2/24 17:06, Peter Maydell wrote:
> Add the two IDE bus BusState pointers to the set we keep in PCMachineState.
> This allows us to avoid passing them to pc_cmos_init(), and also will
> allow a refactoring of how we call pc_cmos_init_late().
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   include/hw/i386/pc.h |  4 +++-
>   hw/i386/pc.c         |  5 ++---
>   hw/i386/pc_piix.c    | 16 +++++++---------
>   hw/i386/pc_q35.c     |  9 ++++-----
>   4 files changed, 16 insertions(+), 18 deletions(-)


> @@ -300,13 +299,13 @@ static void pc_q35_init(MachineState *machine)
>                                                            ICH9_SATA1_FUNC),
>                                                  "ich9-ahci");
>           ich9 = ICH9_AHCI(pdev);
> -        idebus[0] = qdev_get_child_bus(DEVICE(pdev), "ide.0");
> -        idebus[1] = qdev_get_child_bus(DEVICE(pdev), "ide.1");
> +        pcms->idebus[0] = qdev_get_child_bus(DEVICE(pdev), "ide.0");
> +        pcms->idebus[1] = qdev_get_child_bus(DEVICE(pdev), "ide.1");
>           g_assert(MAX_SATA_PORTS == ich9->ahci.ports);
>           ide_drive_get(hd, ich9->ahci.ports);
>           ahci_ide_create_devs(&ich9->ahci, hd);
>       } else {
> -        idebus[0] = idebus[1] = NULL;
> +        pcms->idebus[0] = pcms->idebus[1] = NULL;

Since PCMachineState is zero-initialized, this part is now
pointless.

Since my ICH9 series clashes with your patch, I'm inclined to
queue it in my hw-misc tree, with the following squashed:

-- >8 --

-    } else {
-        pcms->idebus[0] = pcms->idebus[1] = NULL;

---

>       }
>   
>       if (machine_usb(machine)) {
> @@ -327,7 +326,7 @@ static void pc_q35_init(MachineState *machine)
>           smbus_eeprom_init(pcms->smbus, 8, NULL, 0);
>       }
>   
> -    pc_cmos_init(pcms, idebus[0], idebus[1], rtc_state);
> +    pc_cmos_init(pcms, rtc_state);
>   
>       /* the rest devices to which pci devfn is automatically assigned */
>       pc_vga_init(isa_bus, host_bus);