From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([140.186.70.92]:54084)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <anthony@codemonkey.ws>) id 1RaHol-0006jk-6J
	for qemu-devel@nongnu.org; Mon, 12 Dec 2011 21:10:08 -0500
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <anthony@codemonkey.ws>) id 1RaHoj-0000Q8-TY
	for qemu-devel@nongnu.org; Mon, 12 Dec 2011 21:10:07 -0500
Received: from mail-yw0-f45.google.com ([209.85.213.45]:63708)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <anthony@codemonkey.ws>) id 1RaHoj-0000PX-R4
	for qemu-devel@nongnu.org; Mon, 12 Dec 2011 21:10:05 -0500
Received: by yhgg71 with SMTP id g71so33088yhg.4
	for <qemu-devel@nongnu.org>; Mon, 12 Dec 2011 18:10:05 -0800 (PST)
Message-ID: <4EE6B3FA.60900@codemonkey.ws>
Date: Mon, 12 Dec 2011 20:10:02 -0600
From: Anthony Liguori <anthony@codemonkey.ws>
MIME-Version: 1.0
References: <1323721273-32404-1-git-send-email-aliguori@us.ibm.com>	<1323721273-32404-65-git-send-email-aliguori@us.ibm.com>
	<1323741866.4576.4.camel@concordia>
In-Reply-To: <1323741866.4576.4.camel@concordia>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [PATCH v3 064/197] killall VIOsPAPRDeviceInfo
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: michael@ellerman.id.au
Cc: Kevin Wolf <kwolf@redhat.com>, Peter Maydell <peter.maydell@linaro.org>, Anthony Liguori <aliguori@us.ibm.com>, Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>, Jan Kiszka <jan.kiszka@siemens.com>, qemu-devel@nongnu.org, Markus Armbruster <armbru@redhat.com>, Luiz Capitulino <lcapitulino@redhat.com>, David Gibson <david@gibson.dropbear.id.au>

On 12/12/2011 08:04 PM, Michael Ellerman wrote:
> On Mon, 2011-12-12 at 14:19 -0600, Anthony Liguori wrote:
>> This was doing something evil building a dt tree so we broke the device.
>
>> @@ -711,8 +711,12 @@ VIOsPAPRBus *spapr_vio_bus_init(void)
>>       spapr_rtas_register("ibm,set-tce-bypass", rtas_set_tce_bypass);
>>       spapr_rtas_register("quiesce", rtas_quiesce);
>>
>> +#if 0
>> +    /* Evil and broken */
>
> By which you mean: works fine, broken by your patch?

These patches were never supposed to go out.  Ignore this series entirely.

>
>> +
>>       for (qinfo = device_info_list; qinfo; qinfo = qinfo->next) {
>>           VIOsPAPRDeviceInfo *info = (VIOsPAPRDeviceInfo *)qinfo;
>> +        VIOsPAPRDeviceClass *pc = VIO_SPAPR_DEVICE_GET_CLASS(dev);
>>
>>           if (qinfo->bus_info !=&spapr_vio_bus_info) {
>>               continue;
>> @@ -722,6 +726,7 @@ VIOsPAPRBus *spapr_vio_bus_init(void)
>>               info->hcalls(bus);
>>           }
>>       }
>> +#endif
>
> It's registering hcalls for each class of device we find on the spapr
> vio bus. I don't understand why that is evil, but what do you suggest we
> do instead?

I talked to David about this, the hcalls can just be registered as part the 
device_init entry points.

If you must initialize them via a per-device callback, then you should walk it 
from the bus's children, not by walking the entire device model.  That was the 
bit that I was referring to as evil.   It's a layering violation.

Regards,

Anthony Liguori


> cheers
>
>