Re: [PATCH 1/6] hw/arm: Inline sysbus_create_simple(PL110 / PL111)

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

On 19/2/24 12:27, BALATON Zoltan wrote:
> On Mon, 19 Feb 2024, Philippe Mathieu-Daudé wrote:
>> On 16/2/24 20:54, Philippe Mathieu-Daudé wrote:
>>> On 16/2/24 18:14, BALATON Zoltan wrote:
>>>> On Fri, 16 Feb 2024, Philippe Mathieu-Daudé wrote:
>>>>> We want to set another qdev property (a link) for the pl110
>>>>> and pl111 devices, we can not use sysbus_create_simple() which
>>>>> only passes sysbus base address and IRQs as arguments. Inline
>>>>> it so we can set the link property in the next commit.
>>>>>
>>>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>>>> ---
>>>>> hw/arm/realview.c    |  5 ++++-
>>>>> hw/arm/versatilepb.c |  6 +++++-
>>>>> hw/arm/vexpress.c    | 10 ++++++++--
>>>>> 3 files changed, 17 insertions(+), 4 deletions(-)
>>>>>
>>>>> diff --git a/hw/arm/realview.c b/hw/arm/realview.c
>>>>> index 9058f5b414..77300e92e5 100644
>>>>> --- a/hw/arm/realview.c
>>>>> +++ b/hw/arm/realview.c
>>>>> @@ -238,7 +238,10 @@ static void realview_init(MachineState *machine,
>>>>>     sysbus_create_simple("pl061", 0x10014000, pic[7]);
>>>>>     gpio2 = sysbus_create_simple("pl061", 0x10015000, pic[8]);
>>>>>
>>>>> -    sysbus_create_simple("pl111", 0x10020000, pic[23]);
>>>>> +    dev = qdev_new("pl111");
>>>>> +    sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
>>>>> +    sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, 0x10020000);
>>>>> +    sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, pic[23]);
>>>>
>>>> Not directly related to this patch but this blows up 1 line into 4 
>>>> just to allow setting a property. Maybe just to keep some simplicity 
>>>> we'd rather need either a sysbus_realize_simple function that takes 
>>>> a sysbus device instead of the name and does not create the device 
>>>> itself or some way to pass properties to sysbus create simple (but 
>>>> the latter may not be easy to do in a generic way so not sure about 
>>>> that). What do you think?
>>>
>>> Unfortunately sysbus doesn't scale in heterogeneous setup.
>>
>> Regarding the HW modelling API complexity you are pointing at, we'd
>> like to move from the current imperative programming paradigm to a
>> declarative one, likely DSL driven. Meanwhile it is being investigated
>> (as part of "Dynamic Machine"), I'm trying to get the HW APIs right
> 
> I'm aware of that activity but we're currently still using board code to 
> construct machines and probably will continue to do so for a while. Also 
> because likely not all current machines will be converted to new 
> declarative way so having a convenient API for that is still useful.
> 
> (As for the language to describe the devices of a machine and their 
> connections declaratively the device tree does just that but dts is not 
> a very user friendly descrtiption language so I haven't brought that up 
> as a possibility. But you may still could get some clues by looking at 
> the problems it had to solve to at least get a requirements for the 
> machine description language.)
> 
>> for heterogeneous emulation. Current price to pay is a verbose
>> imperative QDev API, hoping we'll get later a trivial declarative one
>> (like this single sysbus_create_simple call), where we shouldn't worry
>> about the order of low level calls, whether to use link or not, etc.
> 
> Having a detailed low level API does not prevent a more convenient for 
> current use higher level API on top so keeping that around for current 
> machines would allow you to chnage the low level API without having to 
> change all the board codes because you's only need to update the simple 
> high level API.

So what is your suggestion here, add a new complex helper to keep
a one-line style?

DeviceState *sysbus_create_simple_dma_link(const char *typename,
                                            hwaddr baseaddr,
                                            const char *linkname,
                                            Object *linkobj,
                                            qemu_irq irq);

I wonder why this is that important since you never modified
any of the files changed by this series:

$ git shortlog -es hw/arm/realview.c hw/arm/versatilepb.c 
hw/arm/vexpress.c hw/display/pl110.c hw/arm/exynos4210.c 
hw/display/exynos4210_fimd.c hw/i386/kvmvapic.c
     66  Peter Maydell <peter.maydell@linaro.org>
     34  Markus Armbruster <armbru@redhat.com>
     29  Philippe Mathieu-Daudé <philmd@linaro.org>
     28  Paolo Bonzini <pbonzini@redhat.com>
     17  Andreas Färber <afaerber@suse.de>
     13  Eduardo Habkost <ehabkost@redhat.com>
      8  Greg Bellows <greg.bellows@linaro.org>
      7  Krzysztof Kozlowski <krzk@kernel.org>
      6  Gerd Hoffmann <kraxel@redhat.com>
      5  Richard Henderson <richard.henderson@linaro.org>
      5  Jan Kiszka <jan.kiszka@siemens.com>
      5  Igor Mammedov <imammedo@redhat.com>
      4  Xiaoqiang Zhao <zxq_yx_007@163.com>
      4  Thomas Huth <thuth@redhat.com>
      4  Anthony Liguori <anthony@codemonkey.ws>
      3  Stefan Weil <sw@weilnetz.de>
      3  Pavel Dovgaluk <Pavel.Dovgaluk@ispras.ru>
      3  Guenter Roeck <linux@roeck-us.net>
      3  Daniel P. Berrangé <berrange@redhat.com>
      3  Alistair Francis <alistair.francis@xilinx.com>
      2  Roy Franz <roy.franz@linaro.org>
      2  Pavel Dovgaluk <pavel.dovgaluk@ispras.ru>
      2  Marcel Apfelbaum <marcel.a@redhat.com>
      2  Linus Walleij <linus.walleij@linaro.org>
      2  Like Xu <like.xu@linux.intel.com>
      2  Juan Quintela <quintela@trasno.org>
      2  Igor Mitsyanko <i.mitsyanko@samsung.com>
      2  Hu Tao <hutao@cn.fujitsu.com>
      2  David Woodhouse <dwmw@amazon.co.uk>
      1  Zongyuan Li <zongyuan.li@smartx.com>
      1  Wen, Jianxian <Jianxian.Wen@verisilicon.com>
      1  Vincent Palatin <vpalatin@chromium.org>
      1  Tao Xu <tao3.xu@intel.com>
      1  Sergey Fedorov <serge.fdrv@gmail.com>
      1  Prasad J Pandit <ppandit@redhat.com>
      1  Prasad J Pandit <pjp@fedoraproject.org>
      1  Pranith Kumar <bobby.prani@gmail.com>
      1  Peter Crosthwaite <peter.crosthwaite@xilinx.com>
      1  Nikita Belov <zodiac@ispras.ru>
      1  Martin Kletzander <mkletzan@redhat.com>
      1  Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
      1  Marcelo Tosatti <mtosatti@redhat.com>
      1  Marcel Apfelbaum <marcel@redhat.com>
      1  Marc-André Lureau <marcandre.lureau@redhat.com>
      1  Laurent Vivier <lvivier@redhat.com>
      1  Laszlo Ersek <lersek@redhat.com>
      1  Kevin Wolf <kwolf@redhat.com>
      1  Jean-Christophe Dubois <jcd@tribudubois.net>
      1  Igor Mitsyanko <i.mitsyanko@gmail.com>
      1  Grant Likely <grant.likely@linaro.org>
      1  Gonglei (Arei) <arei.gonglei@huawei.com>
      1  Frederic Konrad <konrad.frederic@yahoo.fr>
      1  Fabian Aggeler <aggelerf@ethz.ch>
      1  Eric Auger <eric.auger@linaro.org>
      1  Emilio G. Cota <cota@braap.org>
      1  Dirk Müller <dirk@dmllr.de>
      1  David Gibson <david@gibson.dropbear.id.au>
      1  Chen Qun <kuhn.chenqun@huawei.com>
      1  Chen Fan <chen.fan.fnst@cn.fujitsu.com>
      1  Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
      1  Anthony Liguori <aliguori@amazon.com>
      1  Alexander Graf <agraf@csgraf.de>
      1  Alex Chen <alex.chen@huawei.com>
      1  Alex Bennée <alex.bennee@linaro.org>