Re: [PATCH 0/9] hw: Use QOM alias properties and few QOM/QDev cleanups

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

On 6/2/23 22:54, Mark Cave-Ayland wrote:
> On 06/02/2023 15:27, Philippe Mathieu-Daudé wrote:
> 
>> On 6/2/23 00:29, Mark Cave-Ayland wrote:
>>> On 03/02/2023 11:36, Philippe Mathieu-Daudé wrote:
>>>
>>>> These patches are extracted from a QOM/QDev refactor series,
>>>> so they are preliminary cleanups noticed while working on it:
>>>>
>>>> - Use correct type when calling qdev_prop_set_xxx()
>>>> - Unify some qdev properties in MIPS models
>>>> - Replace intermediate properties by link properties
>>>> - Remove DEFINE_PROP_DMAADDR() macro which is used one time
>>>> - Use qdev_realize_and_unref() instead of open-coding it


>>> I must admit to being slightly nervous about using QOM alias 
>>> properties in this way, simply because you start creating implicit 
>>> dependencies between QOM objects. How would this work when trying to 
>>> build machines from configuration files and/or the monitor? Or are 
>>> the changes restricted to container devices i.e. those which consist 
>>> of in-built child devices?
>>
>> The latter. All parents forward a property to a contained child.
>>
>> The parent forwarding property is replaced by a link into the child,
>> so accessing the parent property transparently access the child one.
>>
>> The dependencies are already explicit. We can not create a parent
>> without its children (the children creation is implicit when we
>> create the parent object).
>>
>> I thought this was the canonical QOM alias properties use. What is
>> the normal use then?
> 
> The problem I've found with this approach in the past is that it fails 
> when you have more than one child device of the same type.
> 
> For example imagine the scenario where there is a QEMU device that 
> contains 2 child UARTs and each UART has a property to disable hardware 
> handshaking: if you add a property alias to the container device, it can 
> only map to a single child UART. Furthermore if you then try to alias 
> the UART IRQs onto the container device using qdev_pass_gpios(), then 
> that also fails with 2 UARTs because the gpios from each UART have the 
> same property name.

I noticed some qdev gpio namespace issues. Thanks for pointing that
qdev_pass_gpios() restriction.

> You could then think about solving that problem by using 
> object_property_add_alias() directly to specify a different property 
> name for each UART's mapped property on the container device, but then 
> you end up accessing the child UART properties with different names, but 
> only when using that particular parent container device(!).
> 
> For this reason I've tended to avoid aliases and setup child objects 
> from the container like this:
> 
>     static void container_init(Object *obj)
>     {
>         object_initialize_child(obj, "uart0", &s->uart0, TYPE_UART);
>         object_initialize_child(obj, "uart1", &s->uart1, TYPE_UART);
>     }

Hmm OK, this is the approach used in IMX:

@@ -120,8 +120,12 @@ static void fsl_imx6ul_init(Object *obj)
       * Ethernet
       */
      for (i = 0; i < FSL_IMX6UL_NUM_ETHS; i++) {
+        g_autofree gchar *propname = g_strdup_printf("fec%d-phy-num", i 
+ 1);
          snprintf(name, NAME_SIZE, "eth%d", i);
          object_initialize_child(obj, name, &s->eth[i], TYPE_IMX_ENET);
+        qdev_prop_set_uint32(DEVICE(&s->eth[i]), "phy-num", i);
+        object_property_add_alias(obj, propname,
+                                  OBJECT(&s->eth[i]), "phy-num");
      }

But then this is how it is used today:

  static Property fsl_imx6ul_properties[] = {
-    DEFINE_PROP_UINT32("fec1-phy-num", FslIMX6ULState, phy_num[0], 0),
-    DEFINE_PROP_UINT32("fec2-phy-num", FslIMX6ULState, phy_num[1], 1),
      DEFINE_PROP_END_OF_LIST(),
  };

What do you mean by "you end up accessing the child UART properties with
different names, but only when using that particular parent container
device(!)."? I tend to see QOM modelling as matching hardware design,
so if a container is used, there is a similar relationship / hierarchy
in the hardware, then accessing the children via a particular parent
container path sounds the correct way. QOM indexed child must have the
same meaning in the hardware layout.

> And then when configuring the board it is possible to obtain the UART 
> references like this:
> 
>     uart0 = UART(object_resolve_path_component(OBJECT(container), 
> "uart0"));
>     irq0 = qdev_connect_gpio_out(DEVICE(uart0), 0, ... );
> 
>     uart1 = UART(object_resolve_path_component(OBJECT(container), 
> "uart1"));
>     irq1 = qdev_connect_gpio_out(DEVICE(uart1), 0, ... );
> 
> This allows all UART configuration to be done in the same way regardless 
> of the parent container device and number of child devices, and without 
> having to think about using different property names depending upon the 
> container device.

OK I think this is the same explanation as what I just wrote earlier.

> One place where it could conceivably be useful is where you have a chip 
> modelled as a device and you want to expose the memory regions and IRQs 
> to an interface such as ISA, but often even that doesn't work (think PCI 
> IRQs for example).

IRQ wiring is an unsolved problem in my TODO, in particular when a bus
is involved...

> The only valid use cases I can think of are the /rtc property (which is 
> an alias to the RTC device, regardless of where it exists in the QOM 
> tree) and perhaps in future adding similar array aliases to the root of 
> the machine that can point to things like block devices, network 
> devices, chardevs and audio devices (i.e. anything that has a 
> corresponding QEMU backend).

Hmm I see, but this seems a very restrictive use of QOM link property
concept IMHO. For me a QOM link allows to share pointer to any QOM
object (with the QOM type checked). Am I abusing the concept?

BTW DEFINE_PROP_xxx() macros are a QDev concept. In particular
DEFINE_PROP_LINK(). The 'realize' step is also a QDev concept...

Markus suggested I watch Paolo's QOM talk to use the standard
terminology from the expert. I suppose this is "QOM exegesis and
apocalypse" from 2014.

Thanks for the brainstorming and clarifications!

Phil.