[PATCH 06/24] armv7m: Bury unwanted "ARM,bitband-memory" devices

Markus Armbruster posted 24 patches 5 years, 6 months ago
Maintainers: Sagar Karandikar <sagark@eecs.berkeley.edu>, Fabien Chouteau <chouteau@adacore.com>, Peter Maydell <peter.maydell@linaro.org>, Alistair Francis <Alistair.Francis@wdc.com>, Bastian Koppelmann <kbastian@mail.uni-paderborn.de>, Alistair Francis <alistair@alistair23.me>, Paolo Bonzini <pbonzini@redhat.com>, "Edgar E. Iglesias" <edgar.iglesias@gmail.com>, Andrzej Zaborowski <balrogg@gmail.com>, KONRAD Frederic <frederic.konrad@adacore.com>, "Daniel P. Berrangé" <berrange@redhat.com>, Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>, Artyom Tarasenko <atar4qemu@gmail.com>, Eduardo Habkost <ehabkost@redhat.com>, Laurent Vivier <laurent@vivier.eu>, David Gibson <david@gibson.dropbear.id.au>, Palmer Dabbelt <palmer@dabbelt.com>
There is a newer version of this series
[PATCH 06/24] armv7m: Bury unwanted "ARM,bitband-memory" devices
Posted by Markus Armbruster 5 years, 6 months ago
These devices are optional, and enabled by property "enable-bitband".
armv7m_instance_init() creates them unconditionally, because the
property has not been set then.  armv7m_realize() realizes them only
when the property is true.  Works, although it leaves unrealized
devices hanging around in the QOM composition tree.  Affects machines
microbit, mps2-an505, mps2-an521, musca-a, and musca-b1.

Bury the unwanted devices by making armv7m_realize() unparent them.
Visible in "info qom-tree"; here's the change for microbit:

     /machine (microbit-machine)
       /microbit.twi (microbit.i2c)
         /microbit.twi[0] (qemu:memory-region)
       /nrf51 (nrf51-soc)
         /armv6m (armv7m)
           /armv7m-container[0] (qemu:memory-region)
    -      /bitband[0] (ARM,bitband-memory)
    -        /bitband[0] (qemu:memory-region)
    -      /bitband[1] (ARM,bitband-memory)
    -        /bitband[0] (qemu:memory-region)
           /cpu (cortex-m0-arm-cpu)

Cc: Peter Maydell <peter.maydell@linaro.org>
Cc: qemu-arm@nongnu.org
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/arm/armv7m.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/hw/arm/armv7m.c b/hw/arm/armv7m.c
index 7da57f56d3..f930619f53 100644
--- a/hw/arm/armv7m.c
+++ b/hw/arm/armv7m.c
@@ -245,8 +245,8 @@ static void armv7m_realize(DeviceState *dev, Error **errp)
     memory_region_add_subregion(&s->container, 0xe000e000,
                                 sysbus_mmio_get_region(sbd, 0));
 
-    if (s->enable_bitband) {
-        for (i = 0; i < ARRAY_SIZE(s->bitband); i++) {
+    for (i = 0; i < ARRAY_SIZE(s->bitband); i++) {
+        if (s->enable_bitband) {
             Object *obj = OBJECT(&s->bitband[i]);
             SysBusDevice *sbd = SYS_BUS_DEVICE(&s->bitband[i]);
 
@@ -265,6 +265,8 @@ static void armv7m_realize(DeviceState *dev, Error **errp)
 
             memory_region_add_subregion(&s->container, bitband_output_addr[i],
                                         sysbus_mmio_get_region(sbd, 0));
+        } else {
+            object_unparent(OBJECT(&s->bitband[i]));
         }
     }
 }
-- 
2.21.1


Re: [PATCH 06/24] armv7m: Bury unwanted "ARM,bitband-memory" devices
Posted by Peter Maydell 5 years, 5 months ago
On Mon, 18 May 2020 at 06:04, Markus Armbruster <armbru@redhat.com> wrote:
>
> These devices are optional, and enabled by property "enable-bitband".
> armv7m_instance_init() creates them unconditionally, because the
> property has not been set then.  armv7m_realize() realizes them only
> when the property is true.  Works, although it leaves unrealized
> devices hanging around in the QOM composition tree.  Affects machines
> microbit, mps2-an505, mps2-an521, musca-a, and musca-b1.
>
> Bury the unwanted devices by making armv7m_realize() unparent them.
> Visible in "info qom-tree"; here's the change for microbit:
>
>      /machine (microbit-machine)
>        /microbit.twi (microbit.i2c)
>          /microbit.twi[0] (qemu:memory-region)
>        /nrf51 (nrf51-soc)
>          /armv6m (armv7m)
>            /armv7m-container[0] (qemu:memory-region)
>     -      /bitband[0] (ARM,bitband-memory)
>     -        /bitband[0] (qemu:memory-region)
>     -      /bitband[1] (ARM,bitband-memory)
>     -        /bitband[0] (qemu:memory-region)
>            /cpu (cortex-m0-arm-cpu)

What does "bury" mean here? To me it implies "they still
exist but we've stuck them in a hole somewhere and covered
them up", but the qom-tree delta suggests we've actually
really deleted them?

thanks
-- PMM

Re: [PATCH 06/24] armv7m: Bury unwanted "ARM,bitband-memory" devices
Posted by Markus Armbruster 5 years, 5 months ago
Peter Maydell <peter.maydell@linaro.org> writes:

> On Mon, 18 May 2020 at 06:04, Markus Armbruster <armbru@redhat.com> wrote:
>>
>> These devices are optional, and enabled by property "enable-bitband".
>> armv7m_instance_init() creates them unconditionally, because the
>> property has not been set then.  armv7m_realize() realizes them only
>> when the property is true.  Works, although it leaves unrealized
>> devices hanging around in the QOM composition tree.  Affects machines
>> microbit, mps2-an505, mps2-an521, musca-a, and musca-b1.
>>
>> Bury the unwanted devices by making armv7m_realize() unparent them.
>> Visible in "info qom-tree"; here's the change for microbit:
>>
>>      /machine (microbit-machine)
>>        /microbit.twi (microbit.i2c)
>>          /microbit.twi[0] (qemu:memory-region)
>>        /nrf51 (nrf51-soc)
>>          /armv6m (armv7m)
>>            /armv7m-container[0] (qemu:memory-region)
>>     -      /bitband[0] (ARM,bitband-memory)
>>     -        /bitband[0] (qemu:memory-region)
>>     -      /bitband[1] (ARM,bitband-memory)
>>     -        /bitband[0] (qemu:memory-region)
>>            /cpu (cortex-m0-arm-cpu)
>
> What does "bury" mean here? To me it implies "they still
> exist but we've stuck them in a hole somewhere and covered
> them up", but the qom-tree delta suggests we've actually
> really deleted them?

We really delete them now.

"They've been lying dead in the streets; give them a decent burial".

Would you like me to s/Bury/Delete/?


Re: [PATCH 06/24] armv7m: Bury unwanted "ARM,bitband-memory" devices
Posted by Paolo Bonzini 5 years, 5 months ago
On 25/05/20 07:50, Markus Armbruster wrote:
> Peter Maydell <peter.maydell@linaro.org> writes:
> 
>> On Mon, 18 May 2020 at 06:04, Markus Armbruster <armbru@redhat.com> wrote:
>>> These devices are optional, and enabled by property "enable-bitband".
>>> armv7m_instance_init() creates them unconditionally, because the
>>> property has not been set then.  armv7m_realize() realizes them only
>>> when the property is true.  Works, although it leaves unrealized
>>> devices hanging around in the QOM composition tree.  Affects machines
>>> microbit, mps2-an505, mps2-an521, musca-a, and musca-b1.
>>>
>>> Bury the unwanted devices by making armv7m_realize() unparent them.
>>> Visible in "info qom-tree"; here's the change for microbit:
>>
>> What does "bury" mean here? To me it implies "they still
>> exist but we've stuck them in a hole somewhere and covered
>> them up", but the qom-tree delta suggests we've actually
>> really deleted them?
> 
> We really delete them now.
> 
> "They've been lying dead in the streets; give them a decent burial".
> 
> Would you like me to s/Bury/Delete/?

"Bury unwanted" -> "Dispose of unused"?


Paolo


Re: [PATCH 06/24] armv7m: Bury unwanted "ARM,bitband-memory" devices
Posted by Peter Maydell 5 years, 5 months ago
On Mon, 25 May 2020 at 13:32, Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> On 25/05/20 07:50, Markus Armbruster wrote:
> > Peter Maydell <peter.maydell@linaro.org> writes:
> >
> >> On Mon, 18 May 2020 at 06:04, Markus Armbruster <armbru@redhat.com> wrote:
> >>> These devices are optional, and enabled by property "enable-bitband".
> >>> armv7m_instance_init() creates them unconditionally, because the
> >>> property has not been set then.  armv7m_realize() realizes them only
> >>> when the property is true.  Works, although it leaves unrealized
> >>> devices hanging around in the QOM composition tree.  Affects machines
> >>> microbit, mps2-an505, mps2-an521, musca-a, and musca-b1.
> >>>
> >>> Bury the unwanted devices by making armv7m_realize() unparent them.
> >>> Visible in "info qom-tree"; here's the change for microbit:
> >>
> >> What does "bury" mean here? To me it implies "they still
> >> exist but we've stuck them in a hole somewhere and covered
> >> them up", but the qom-tree delta suggests we've actually
> >> really deleted them?
> >
> > We really delete them now.
> >
> > "They've been lying dead in the streets; give them a decent burial".
> >
> > Would you like me to s/Bury/Delete/?
>
> "Bury unwanted" -> "Dispose of unused"?

Yeah, delete or dispose of would be clearer I think.

thanks
-- PMM

Re: [PATCH 06/24] armv7m: Bury unwanted "ARM,bitband-memory" devices
Posted by Markus Armbruster 5 years, 5 months ago
Peter Maydell <peter.maydell@linaro.org> writes:

> On Mon, 25 May 2020 at 13:32, Paolo Bonzini <pbonzini@redhat.com> wrote:
>>
>> On 25/05/20 07:50, Markus Armbruster wrote:
>> > Peter Maydell <peter.maydell@linaro.org> writes:
>> >
>> >> On Mon, 18 May 2020 at 06:04, Markus Armbruster <armbru@redhat.com> wrote:
>> >>> These devices are optional, and enabled by property "enable-bitband".
>> >>> armv7m_instance_init() creates them unconditionally, because the
>> >>> property has not been set then.  armv7m_realize() realizes them only
>> >>> when the property is true.  Works, although it leaves unrealized
>> >>> devices hanging around in the QOM composition tree.  Affects machines
>> >>> microbit, mps2-an505, mps2-an521, musca-a, and musca-b1.
>> >>>
>> >>> Bury the unwanted devices by making armv7m_realize() unparent them.
>> >>> Visible in "info qom-tree"; here's the change for microbit:
>> >>
>> >> What does "bury" mean here? To me it implies "they still
>> >> exist but we've stuck them in a hole somewhere and covered
>> >> them up", but the qom-tree delta suggests we've actually
>> >> really deleted them?
>> >
>> > We really delete them now.
>> >
>> > "They've been lying dead in the streets; give them a decent burial".
>> >
>> > Would you like me to s/Bury/Delete/?
>>
>> "Bury unwanted" -> "Dispose of unused"?
>
> Yeah, delete or dispose of would be clearer I think.

Okay, the subjects are short enough to accomodate a change to 'Delete
unused "..." devices'.

Thanks!