Re: [PATCH v4 3/9] hw/i386/pc_q35: Reuse machine parameter

[Michael S. Tsirkin]

On Wed, Feb 22, 2023 at 06:52:02PM +0100, Bernhard Beschow wrote:
> Am 22. Februar 2023 11:03:38 UTC schrieb "Philippe Mathieu-Daudé"
> <philmd@linaro.org>:
> >On 13/2/23 17:19, Bernhard Beschow wrote:
> >> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> >> Reviewed-by: Thomas Huth <thuth@redhat.com>
> >> ---
> >>   hw/i386/pc_q35.c | 2 +-
> >>   1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> >> index 66cd718b70..dee2b38474 100644
> >> --- a/hw/i386/pc_q35.c
> >> +++ b/hw/i386/pc_q35.c
> >> @@ -218,7 +218,7 @@ static void pc_q35_init(MachineState *machine)
> >>       pc_memory_init(pcms, get_system_memory(), rom_memory, &ram_memory,
> >>                      pci_hole64_size);
> >>   -    object_property_add_child(qdev_get_machine(), "q35", 
> >> OBJECT(q35_host));
> >> +    object_property_add_child(OBJECT(machine), "q35", OBJECT(q35_host));
> >>       object_property_set_link(OBJECT(q35_host), MCH_HOST_PROP_RAM_MEM,
> >>                                OBJECT(ram_memory), NULL);
> >>       object_property_set_link(OBJECT(q35_host), MCH_HOST_PROP_PCI_MEM,
> >
> >Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> >
> >Long term we should duplicate/extract Q35MachineState from
> >PCMachineState and add a Q35PCIHost field, then use object_initialize_child; 
> >removing this object_property_add_child()
> >call.
> 
> The Q35 and PC machines duplicate a lot of code indeed. So I was
> thinking more along the lines of consolidating with pc_piix ;)

The reason is that we are trying to limit changes to pc_piix and
focus development on Q35.

> The
> idea would be to get a peek preview into a configuration-driven future
> where the PCI host bridges (Q35 or I440FX) are dynamically
> instantiated based on configuration data. They would also be
> configured through their QOM interfaces only.
> 
> I've submitted a series where the Q35 host bridge gets a QOM cleanup
> [1] and I've got a series locally resolving i440fx_init(). Both series
> combined bring these two device models close together regarding their
> QOM interface. I've not submitted the i440fx series yet since it is
> blocked by this series.
> 
> One further step for pc_q35 and pc_piix consolidation would be to
> factor ICH9 PCI devices (not functions!) into self-contained models,
> like is underway with PIIX(3). I've started with ICH9 cleanup already
> [2] and I'm waiting for the PIIX consolidation to land in order to be
> able to make more progress here.
> 
> Note that pc_q35 and pc_piix consolidation is just an idea for now
> which could also turn out to be a bad one. If the two machines just
> ended up sharing more code that could IMO be considered a success as
> well.
> 
> Best regards,
> Bernhard
> 
> [1] 20230214131441.101760-1-shentey@gmail.com/">https://patchew.org/QEMU/20230214131441.101760-1-shentey@gmail.com/
> [2] 20230213173033.98762-1-shentey@gmail.com/">https://patchew.org/QEMU/20230213173033.98762-1-shentey@gmail.com/