From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:48395) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VJh8o-0002Y7-8U for qemu-devel@nongnu.org; Wed, 11 Sep 2013 05:55:24 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VJh8i-0000Fc-84 for qemu-devel@nongnu.org; Wed, 11 Sep 2013 05:55:18 -0400 Received: from mx1.redhat.com ([209.132.183.28]:60526) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VJh8h-0000FY-W5 for qemu-devel@nongnu.org; Wed, 11 Sep 2013 05:55:12 -0400 Date: Wed, 11 Sep 2013 12:57:17 +0300 From: "Michael S. Tsirkin" Message-ID: <20130911095717.GA21006@redhat.com> References: <1374681580-17439-1-git-send-email-mst@redhat.com> <51F1495F.2010502@suse.de> <20130725161917.GA7079@redhat.com> <51F2693B.5070000@suse.de> <51F45651.9010600@suse.de> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline In-Reply-To: <51F45651.9010600@suse.de> Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v3 00/14] qemu: generate acpi tables for the guest List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Andreas =?iso-8859-1?Q?F=E4rber?= Cc: qemu-devel@nongnu.org, Anthony Liguori On Sun, Jul 28, 2013 at 01:22:57AM +0200, Andreas F=E4rber wrote: > Am 26.07.2013 14:19, schrieb Andreas F=E4rber: > > Am 25.07.2013 18:19, schrieb Michael S. Tsirkin: > >> On Thu, Jul 25, 2013 at 05:50:55PM +0200, Andreas F=E4rber wrote: > >>> Am 24.07.2013 18:01, schrieb Michael S. Tsirkin: > >>>> This code can also be found here: > >>>> git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git acpi > >>>> > >>>> Please review, and consider for 1.6. > >>> > >>> Quite frankly, this is still not looking the way I imagined it base= d on > >>> the KVM call discussion and Anthony's comments that I remember: > >>> > >>> I believe Anthony asked to extract the information from the QOM tre= e, > >>> originally from the SeaBIOS side, then later agreeing to do it on t= he > >>> QEMU side. > >>> > >>> However here I am still seeing *functions* added in device code to = check > >>> device existence and to extract individual fields. I was assuming (= and > >>> clearly prefer) such code to live in a central place, be it acpi-bu= ild.c > >>> or something else, and to use QOM *API*s to obtain information when > >>> needed rather than building up lots of new structs duplicating that > >>> data. That would at the same time be a test case for how useful the= QOM > >>> tree is > >>> > >>> I'm not sure if there was a misunderstanding or whether the PC QOM = model > >>> still sucks^W is incomplete? Anthony and Ping Fang(?) had both post= ed > >>> patches to improve the composition tree once. If there's properties > >>> missing that you need to access for ACPI, we should simply add them. > >>> For i440fx we have /machine/i440fx. > >>> For q35 I encountered an mch child on q35-pcihost, but what's trivi= ally > >>> missing apparently is to add q35-pcihost as a child to /machine, e.= g. > >>> /machine/q35. > >>> Then you'll end up doing > >>> Object *obj =3D object_resolve_path_component(qdev_get_machine(), "= q35/mch"); > >>> object_property_get_int(obj, "foo", &err); > >>> object_property_get_string(obj, "bar", &err); > >>> and so on. No need to do the TYPE_... based search for everything. > >>> > >>> User-added -devices will show up in /machine/peripheral or > >>> /machine/peripheral-anon depending on whether id=3D is used, so the= re a > >>> type-based search probably makes sense. And there is nothing wrong = with > >>> moving the TYPE_* constants to a device header where not yet the ca= se, > >>> to allow that from generic code. > >>> > >>> Similarly, please don't open-code OBJECT_CHECK()s, do a trivial pat= ch > >>> with a macro that we can then reuse elsewhere. I'd be happy to revi= ew > >>> such QOM patches and help fast-track them into master. > >>> > >>> Will take a closer look at the implementation later. > >> > >> This is not my understanding of previous comments on list > >> or on KVM call. > >> > >> Basically it sounds like you want to make my work depend on completi= on > >> of QOM conversion. > >> I think we explicitly agreed full QOM convertion is not a blocker. > >=20 > > Not sure what you mean with "completion of QOM conversion" or "full Q= OM > > conversion". What I am saying is that instead of spending time adding > > functions to devices that fulfill your own ACPI needs only, that time > > were better spent adding QOM properties where not yet existent. > >=20 > > Because then what you can access for ACPI can also be accessed by > > libvirt and other management tools as well as qtest - I consider it a > > test case. QMP does not offer an instance/path search by type. >=20 > To clarify for everyone what we're talking about here, I'm attaching > /machine composition tree dumps for pc,accel=3Dkvm and q35,accel=3Dkvm = plus > the rudimentary script I used to generate it. >=20 > It shows for instance the mentioned /machine/i440fx and lack of > /machine/q35. It also shows that there would be a /machine/fw_cfg. >=20 > Paths starting with /machine/unassigned shouldn't be hardcoded anywhere > (that's the nobody-added-it-as-a-child<> bucket), except maybe for > /machine/unassigned/sysbus. But whenever there's a link from a named > device to a /machine/unassigned/device[n] that may of course be used > dynamically, e.g. /machine/icc-bridge/icc to discover CPUs and APICs. >=20 > HTH, > Andreas So q35 is all "unassigned". I assume it's fine to use APIs for that, then? --=20 MST