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

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

On 21/2/24 14:07, Philippe Mathieu-Daudé wrote:
> 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;
> 
> ---

Sorry sent too fast. Squashing

-- >8 --
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 8df88a6ccd..77d1b03fdf 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -311,6 +311,4 @@ static void pc_init1(MachineState *machine,
          i8257_dma_init(OBJECT(machine), isa_bus, 0);
          pcms->hpet_enabled = false;
-        pcms->idebus[0] = NULL;
-        pcms->idebus[1] = NULL;
      }

diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 71402c36eb..0e9bd27a6e 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -305,6 +305,4 @@ static void pc_q35_init(MachineState *machine)
          ide_drive_get(hd, ich9->ahci.ports);
          ahci_ide_create_devs(&ich9->ahci, hd);
-    } else {
-        pcms->idebus[0] = pcms->idebus[1] = NULL;
      }

---

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>