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

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(-)

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.

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
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] https://patchew.org/QEMU/20230214131441.101760-1-shentey@gmail.com/
[2] https://patchew.org/QEMU/20230213173033.98762-1-shentey@gmail.com/

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] https://patchew.org/QEMU/20230214131441.101760-1-shentey@gmail.com/
> [2] https://patchew.org/QEMU/20230213173033.98762-1-shentey@gmail.com/