[v2,2/6] qdev: reset qdev along with qdev tree

On Thu, Dec 16, 2010 at 08:31:21PM +0100, Stefan Weil wrote:
> The second regression also occurs with MIPS malta.
> Networking no longer works with the default pcnet nic.
>
> This is caused because the reset function for pcnet is no
> longer called during system boot. The result in an invalid
> mac address (all zero) and a non-working nic.
>
> For this second regression I still have no simple solution.
> Of course mips_malta.c should be converted to qdev which
> would fix both problems (but only for malta system emulation).
>
> Maybe other systems / devices don't get their reset functions
> called during system boot, too.

Arg, sorry for regression. How about the following?

On Fri, Dec 17, 2010 at 01:29:42PM +0900, Isaku Yamahata wrote:
> On Thu, Dec 16, 2010 at 08:31:21PM +0100, Stefan Weil wrote:
> > The second regression also occurs with MIPS malta.
> > Networking no longer works with the default pcnet nic.
> >
> > This is caused because the reset function for pcnet is no
> > longer called during system boot. The result in an invalid
> > mac address (all zero) and a non-working nic.
> >
> > For this second regression I still have no simple solution.
> > Of course mips_malta.c should be converted to qdev which
> > would fix both problems (but only for malta system emulation).
> >
> > Maybe other systems / devices don't get their reset functions
> > called during system boot, too.
> 
> Arg, sorry for regression. How about the following?

The issue is, it is assumed that all qbuses, qdeves
are under main_system_bus.
But there are qbuses whose parent is NULL. So it is necessary
to trigger reset for those qbuses.

Ideally those buses should be moved under bus controller
device which is qdev. But it's not done yet.


> 
> diff --git a/hw/qdev.c b/hw/qdev.c
> index 10e28df..774c53b 100644
> --- a/hw/qdev.c
> +++ b/hw/qdev.c
> @@ -753,8 +753,11 @@ void qbus_create_inplace(BusState *bus, BusInfo *info,
>      if (parent) {
>          QLIST_INSERT_HEAD(&parent->child_bus, bus, sibling);
>          parent->num_child_bus++;
> +    } else if (bus != main_system_bus) {
> +        /* TODO: once all device is qdevified,
> +           reset handler for main_system_bus should also be registered here */
> +        qemu_register_reset((void *)qbus_reset_all, bus);
>      }
> -
>  }
>  
>  BusState *qbus_create(BusInfo *info, DeviceState *parent, const char *name)
> diff --git a/vl.c b/vl.c
> index c4d3fc0..2ec3989 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -3088,6 +3088,8 @@ int main(int argc, char **argv, char **envp)
>          exit(1);
>      }
>  
> +    /* TODO: once all device is qdevified, this should be done by
> +       qbus_create_inplace() */
>      qemu_register_reset((void *)qbus_reset_all, sysbus_get_default());
>      qemu_run_machine_init_done_notifiers();
>  
> 
> -- 
> yamahata
>

Am 17.12.2010 05:47, schrieb Isaku Yamahata:
> On Fri, Dec 17, 2010 at 01:29:42PM +0900, Isaku Yamahata wrote:
>> On Thu, Dec 16, 2010 at 08:31:21PM +0100, Stefan Weil wrote:
>>> The second regression also occurs with MIPS malta.
>>> Networking no longer works with the default pcnet nic.
>>>
>>> This is caused because the reset function for pcnet is no
>>> longer called during system boot. The result in an invalid
>>> mac address (all zero) and a non-working nic.
>>>
>>> For this second regression I still have no simple solution.
>>> Of course mips_malta.c should be converted to qdev which
>>> would fix both problems (but only for malta system emulation).
>>>
>>> Maybe other systems / devices don't get their reset functions
>>> called during system boot, too.
>>
>> Arg, sorry for regression. How about the following?
>
> The issue is, it is assumed that all qbuses, qdeves
> are under main_system_bus.
> But there are qbuses whose parent is NULL. So it is necessary
> to trigger reset for those qbuses.
>
> Ideally those buses should be moved under bus controller
> device which is qdev. But it's not done yet.
>
>>
>> diff --git a/hw/qdev.c b/hw/qdev.c
>> index 10e28df..774c53b 100644
>> --- a/hw/qdev.c
>> +++ b/hw/qdev.c
>> @@ -753,8 +753,11 @@ void qbus_create_inplace(BusState *bus, BusInfo 
>> *info,
>> if (parent) {
>> QLIST_INSERT_HEAD(&parent->child_bus, bus, sibling);
>> parent->num_child_bus++;
>> + } else if (bus != main_system_bus) {
>> + /* TODO: once all device is qdevified,
>> + reset handler for main_system_bus should also be registered here */
>> + qemu_register_reset((void *)qbus_reset_all, bus);
>> }
>> -
>> }
>>
>> BusState *qbus_create(BusInfo *info, DeviceState *parent, const char 
>> *name)
>> diff --git a/vl.c b/vl.c
>> index c4d3fc0..2ec3989 100644
>> --- a/vl.c
>> +++ b/vl.c
>> @@ -3088,6 +3088,8 @@ int main(int argc, char **argv, char **envp)
>> exit(1);
>> }
>>
>> + /* TODO: once all device is qdevified, this should be done by
>> + qbus_create_inplace() */
>> qemu_register_reset((void *)qbus_reset_all, sysbus_get_default());
>> qemu_run_machine_init_done_notifiers();
>>
>>
>> -- 
>> yamahata
>

I can confirm that this patch fixes the second regression:
the reset code for pcnet is called during system boot,
and networking with pcnet works fine again.

Could you please provide a signed patch which is ready for commit?

Michael, my patch (which fixes the first regression / crash)
is needed, too. Please add it to your git queue.

Stefan

> Michael, my patch (which fixes the first regression / crash)
> is needed, too. Please add it to your git queue.
> 
> Stefan

Could you pls review/test the pci branch of my tree?
This should have all the necessary bits.