Re: [PATCH v4 2/3] mac_via: fix incorrect creation of mos6522 device in mac_via

2020-04-02 Thread Markus Armbruster
Paolo Bonzini writes: > On 19/03/20 08:01, Markus Armbruster wrote: >> Paolo Bonzini writes: >> >>> On 18/03/20 16:06, Markus Armbruster wrote: > - object initialization should cause no side effects outside the subtree > of the object object_initialize_child() and its users

Re: [PATCH v4 2/3] mac_via: fix incorrect creation of mos6522 device in mac_via

2020-03-19 Thread Paolo Bonzini
On 19/03/20 08:01, Markus Armbruster wrote: > Paolo Bonzini writes: > >> On 18/03/20 16:06, Markus Armbruster wrote: - object initialization should cause no side effects outside the subtree of the object >>> >>> object_initialize_child() and its users like sysbus_init_child_obj() >>>

Re: [PATCH v4 2/3] mac_via: fix incorrect creation of mos6522 device in mac_via

2020-03-19 Thread Markus Armbruster
Paolo Bonzini writes: > On 18/03/20 16:06, Markus Armbruster wrote: >>> - object initialization should cause no side effects outside the subtree >>> of the object >> >> object_initialize_child() and its users like sysbus_init_child_obj() >> violate this rule: they add a child property to the

Re: [PATCH v4 2/3] mac_via: fix incorrect creation of mos6522 device in mac_via

2020-03-18 Thread Paolo Bonzini
On 18/03/20 16:06, Markus Armbruster wrote: >> - object initialization should cause no side effects outside the subtree >> of the object > > object_initialize_child() and its users like sysbus_init_child_obj() > violate this rule: they add a child property to the subtree's parent. > Correct? At

Re: [PATCH v4 2/3] mac_via: fix incorrect creation of mos6522 device in mac_via

2020-03-18 Thread Markus Armbruster
Paolo Bonzini writes: > On 18/03/20 14:02, Markus Armbruster wrote: >> Object instantiation must be completely reverted by finalization. >> >> device-introspect-test guards against a particularly egregious violation >> of this rule, namely output of "info qtree" after initialization + >>

Re: [PATCH v4 2/3] mac_via: fix incorrect creation of mos6522 device in mac_via

2020-03-18 Thread Peter Maydell
On Wed, 18 Mar 2020 at 13:22, Paolo Bonzini wrote: > Here are some random thoughts about it: > > - object initialization should cause no side effects outside the subtree > of the object > > - setting properties can cause side effects outside the subtree of the > object (e.g. marking an object as

Re: [PATCH v4 2/3] mac_via: fix incorrect creation of mos6522 device in mac_via

2020-03-18 Thread Paolo Bonzini
On 18/03/20 14:02, Markus Armbruster wrote: > Object instantiation must be completely reverted by finalization. > > device-introspect-test guards against a particularly egregious violation > of this rule, namely output of "info qtree" after initialization + > finalization differing from output

Re: [PATCH v4 2/3] mac_via: fix incorrect creation of mos6522 device in mac_via

2020-03-18 Thread Markus Armbruster
Paolo Bonzini writes: > On 16/03/20 07:03, Markus Armbruster wrote: >> Paolo Bonzini writes: >> >>> On 15/03/20 15:56, Markus Armbruster wrote: > > The question is why they are not, i.e. where does the above reasoning > break. I don't know. But let's for the sake of the

Re: [PATCH v4 2/3] mac_via: fix incorrect creation of mos6522 device in mac_via

2020-03-16 Thread Paolo Bonzini
On 16/03/20 07:03, Markus Armbruster wrote: > Paolo Bonzini writes: > >> On 15/03/20 15:56, Markus Armbruster wrote: The question is why they are not, i.e. where does the above reasoning break. >>> I don't know. But let's for the sake of the argument assume this >>> actually

Re: [PATCH v4 2/3] mac_via: fix incorrect creation of mos6522 device in mac_via

2020-03-16 Thread Markus Armbruster
Paolo Bonzini writes: > On 15/03/20 15:56, Markus Armbruster wrote: >>> >>> The question is why they are not, i.e. where does the above reasoning break. >> I don't know. But let's for the sake of the argument assume this >> actually worked. Asking for help in the monitor then *still* has side

Re: [PATCH v4 2/3] mac_via: fix incorrect creation of mos6522 device in mac_via

2020-03-15 Thread Paolo Bonzini
On 15/03/20 15:56, Markus Armbruster wrote: >> >> The question is why they are not, i.e. where does the above reasoning break. > I don't know. But let's for the sake of the argument assume this > actually worked. Asking for help in the monitor then *still* has side > effects visible in the time

Re: [PATCH v4 2/3] mac_via: fix incorrect creation of mos6522 device in mac_via

2020-03-15 Thread Markus Armbruster
Mark Cave-Ayland writes: > On 10/03/2020 09:07, Markus Armbruster wrote: > >> Widespread QOM usage anti-pattern ahead; cc: QOM maintainers. >> >> Peter Maydell writes: >> >>> On Mon, 9 Mar 2020 at 10:02, Pan Nengyuan wrote: On 3/9/2020 5:21 PM, Peter Maydell wrote: > Could you

Re: [PATCH v4 2/3] mac_via: fix incorrect creation of mos6522 device in mac_via

2020-03-15 Thread Markus Armbruster
Paolo Bonzini writes: > On 14/03/20 14:19, Mark Cave-Ayland wrote: >>> Observe that mac_via_init() has obvious side effects. In particular, it >>> creates two devices that are then visible in "info qtree", and that's >>> caught by device-introspect-test. >>> >>> I believe these things need to

Re: [PATCH v4 2/3] mac_via: fix incorrect creation of mos6522 device in mac_via

2020-03-14 Thread Paolo Bonzini
On 14/03/20 14:19, Mark Cave-Ayland wrote: >> Observe that mac_via_init() has obvious side effects. In particular, it >> creates two devices that are then visible in "info qtree", and that's >> caught by device-introspect-test. >> >> I believe these things need to be done in .realize(). That is

Re: [PATCH v4 2/3] mac_via: fix incorrect creation of mos6522 device in mac_via

2020-03-14 Thread Mark Cave-Ayland
On 10/03/2020 09:07, Markus Armbruster wrote: > Widespread QOM usage anti-pattern ahead; cc: QOM maintainers. > > Peter Maydell writes: > >> On Mon, 9 Mar 2020 at 10:02, Pan Nengyuan wrote: >>> On 3/9/2020 5:21 PM, Peter Maydell wrote: Could you explain more? My thought is that we should

Re: [PATCH v4 2/3] mac_via: fix incorrect creation of mos6522 device in mac_via

2020-03-10 Thread BALATON Zoltan
On Tue, 10 Mar 2020, Markus Armbruster wrote: Root cause of this issue: nobody knows how to use QOM correctly (first order approximation). In particular, people are perenially confused on how to split work between .instance_init() and .realize(). We really, really, really need to provide some

Re: [PATCH v4 2/3] mac_via: fix incorrect creation of mos6522 device in mac_via

2020-03-10 Thread Peter Maydell
On Tue, 10 Mar 2020 at 09:08, Markus Armbruster wrote: > We have >200 calls of sysbus_init_child_obj() in some 40 files. I'm > arbitrarily picking hw/arm/allwinner-a10.c for a closer look. > > It calls it from device allwinner-a10's .instance_init() method > aw_a10_init(). Side effect, clearly

Re: [PATCH v4 2/3] mac_via: fix incorrect creation of mos6522 device in mac_via

2020-03-10 Thread Markus Armbruster
Widespread QOM usage anti-pattern ahead; cc: QOM maintainers. Peter Maydell writes: > On Mon, 9 Mar 2020 at 10:02, Pan Nengyuan wrote: >> On 3/9/2020 5:21 PM, Peter Maydell wrote: >> > Could you explain more? My thought is that we should be using >> > sysbus_init_child_obj() and we should be

Re: [PATCH v4 2/3] mac_via: fix incorrect creation of mos6522 device in mac_via

2020-03-09 Thread Pan Nengyuan
On 3/10/2020 12:16 AM, Mark Cave-Ayland wrote: > On 09/03/2020 14:14, Markus Armbruster wrote: > >> Pan Nengyuan writes: >> >>> On 3/9/2020 8:34 PM, Markus Armbruster wrote: Peter Maydell writes: > On Mon, 9 Mar 2020 at 10:02, Pan Nengyuan wrote: >> On 3/9/2020 5:21 PM,

Re: [PATCH v4 2/3] mac_via: fix incorrect creation of mos6522 device in mac_via

2020-03-09 Thread Mark Cave-Ayland
On 09/03/2020 14:14, Markus Armbruster wrote: > Pan Nengyuan writes: > >> On 3/9/2020 8:34 PM, Markus Armbruster wrote: >>> Peter Maydell writes: >>> On Mon, 9 Mar 2020 at 10:02, Pan Nengyuan wrote: > On 3/9/2020 5:21 PM, Peter Maydell wrote: >> Could you explain more? My thought

Re: [PATCH v4 2/3] mac_via: fix incorrect creation of mos6522 device in mac_via

2020-03-09 Thread Markus Armbruster
Pan Nengyuan writes: > On 3/9/2020 8:34 PM, Markus Armbruster wrote: >> Peter Maydell writes: >> >>> On Mon, 9 Mar 2020 at 10:02, Pan Nengyuan wrote: On 3/9/2020 5:21 PM, Peter Maydell wrote: > Could you explain more? My thought is that we should be using >

Re: [PATCH v4 2/3] mac_via: fix incorrect creation of mos6522 device in mac_via

2020-03-09 Thread Pan Nengyuan
On 3/9/2020 8:34 PM, Markus Armbruster wrote: > Peter Maydell writes: > >> On Mon, 9 Mar 2020 at 10:02, Pan Nengyuan wrote: >>> On 3/9/2020 5:21 PM, Peter Maydell wrote: Could you explain more? My thought is that we should be using sysbus_init_child_obj() and we should be doing it

Re: [PATCH v4 2/3] mac_via: fix incorrect creation of mos6522 device in mac_via

2020-03-09 Thread Markus Armbruster
Peter Maydell writes: > On Mon, 9 Mar 2020 at 10:02, Pan Nengyuan wrote: >> On 3/9/2020 5:21 PM, Peter Maydell wrote: >> > Could you explain more? My thought is that we should be using >> > sysbus_init_child_obj() and we should be doing it in the init method. >> > Why does that break the tests

Re: [PATCH v4 2/3] mac_via: fix incorrect creation of mos6522 device in mac_via

2020-03-09 Thread Peter Maydell
On Mon, 9 Mar 2020 at 10:02, Pan Nengyuan wrote: > On 3/9/2020 5:21 PM, Peter Maydell wrote: > > Could you explain more? My thought is that we should be using > > sysbus_init_child_obj() and we should be doing it in the init method. > > Why does that break the tests ? It's the same thing various

Re: [PATCH v4 2/3] mac_via: fix incorrect creation of mos6522 device in mac_via

2020-03-09 Thread Pan Nengyuan
On 3/9/2020 5:21 PM, Peter Maydell wrote: > On Mon, 9 Mar 2020 at 00:56, Pan Nengyuan wrote: >> >> >> >> On 3/8/2020 9:29 PM, Peter Maydell wrote: >>> On Thu, 5 Mar 2020 at 06:39, Pan Nengyuan wrote: -/* Init VIAs 1 and 2 */ -sysbus_init_child_obj(OBJECT(dev), "via1",

Re: [PATCH v4 2/3] mac_via: fix incorrect creation of mos6522 device in mac_via

2020-03-09 Thread Peter Maydell
On Mon, 9 Mar 2020 at 00:56, Pan Nengyuan wrote: > > > > On 3/8/2020 9:29 PM, Peter Maydell wrote: > > On Thu, 5 Mar 2020 at 06:39, Pan Nengyuan wrote: > >> -/* Init VIAs 1 and 2 */ > >> -sysbus_init_child_obj(OBJECT(dev), "via1", >mos6522_via1, > >> -

Re: [PATCH v4 2/3] mac_via: fix incorrect creation of mos6522 device in mac_via

2020-03-08 Thread Pan Nengyuan
On 3/8/2020 9:29 PM, Peter Maydell wrote: > On Thu, 5 Mar 2020 at 06:39, Pan Nengyuan wrote: >> >> This patch fix a bug in mac_via where it failed to actually realize devices >> it was using. >> And move the init codes which inits the mos6522 objects and properties on >> them from realize()

Re: [PATCH v4 2/3] mac_via: fix incorrect creation of mos6522 device in mac_via

2020-03-08 Thread Peter Maydell
On Thu, 5 Mar 2020 at 06:39, Pan Nengyuan wrote: > > This patch fix a bug in mac_via where it failed to actually realize devices > it was using. > And move the init codes which inits the mos6522 objects and properties on > them from realize() > into init(). However, we keep qdev_set_parent_bus

Re: [PATCH v4 2/3] mac_via: fix incorrect creation of mos6522 device in mac_via

2020-03-04 Thread Pan Nengyuan
On 3/5/2020 2:54 PM, Pan Nengyuan wrote: > This patch fix a bug in mac_via where it failed to actually realize devices > it was using. > And move the init codes which inits the mos6522 objects and properties on > them from realize() > into init(). However, we keep qdev_set_parent_bus in

Re: [PATCH v4 2/3] mac_via: fix incorrect creation of mos6522 device in mac_via

2020-03-04 Thread Pan Nengyuan
On 3/5/2020 2:54 PM, Pan Nengyuan wrote: > This patch fix a bug in mac_via where it failed to actually realize devices > it was using. > And move the init codes which inits the mos6522 objects and properties on > them from realize() > into init(). However, we keep qdev_set_parent_bus in

[PATCH v4 2/3] mac_via: fix incorrect creation of mos6522 device in mac_via

2020-03-04 Thread Pan Nengyuan
This patch fix a bug in mac_via where it failed to actually realize devices it was using. And move the init codes which inits the mos6522 objects and properties on them from realize() into init(). However, we keep qdev_set_parent_bus in realize(), otherwise it will cause device-introspect-test