From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:45728) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RaI44-0002sO-DS for qemu-devel@nongnu.org; Mon, 12 Dec 2011 21:25:57 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1RaI43-0001Ye-DO for qemu-devel@nongnu.org; Mon, 12 Dec 2011 21:25:56 -0500 Received: from mail-yx0-f173.google.com ([209.85.213.173]:37166) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RaI43-0001Ya-9c for qemu-devel@nongnu.org; Mon, 12 Dec 2011 21:25:55 -0500 Received: by yenm6 with SMTP id m6so5987500yen.4 for ; Mon, 12 Dec 2011 18:25:54 -0800 (PST) Message-ID: <4EE6B7AF.90007@codemonkey.ws> Date: Mon, 12 Dec 2011 20:25:51 -0600 From: Anthony Liguori 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> <4EE6B3FA.60900@codemonkey.ws> <1323742931.4576.8.camel@concordia> In-Reply-To: <1323742931.4576.8.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: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: michael@ellerman.id.au Cc: Kevin Wolf , Peter Maydell , Stefan Hajnoczi , Jan Kiszka , qemu-devel@nongnu.org, Markus Armbruster , Luiz Capitulino , David Gibson On 12/12/2011 08:22 PM, Michael Ellerman wrote: > On Mon, 2011-12-12 at 20:10 -0600, Anthony Liguori wrote: >> 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. > > But I just read all 197 of them ! ;) > >>>> 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. > > OK I'll talk to him about it. I don't think device_init() works, because > we only want to register the hcalls if an instance of the device is > instantiated. But I guess we'll come up with something. Since the hcalls are well known and CPUState doesn't get touched at all, why not register all of the possible hcalls in the VIO bus and then use a higher level interface to dispatch to devices? I think that conceptionally makes more sense than the devices directly registering hcalls themselves. I know you're dealing with an existing virtual I/O model, but it's strange to have an hcall dispatched directly to a device without going through any type of controller/bus hierarchy. It doesn't fit how hardware works very well. Regards, Anthony Liguori > > cheers >