From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:35957) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aihHT-0005qu-Sl for qemu-devel@nongnu.org; Wed, 23 Mar 2016 07:48:56 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aihHS-0008G8-Rv for qemu-devel@nongnu.org; Wed, 23 Mar 2016 07:48:55 -0400 Date: Wed, 23 Mar 2016 19:48:30 +0800 From: Peter Xu Message-ID: <20160323114830.GR28183@pxdev.xzpeter.org> References: <1458271654-23706-1-git-send-email-peterx@redhat.com> <1458271654-23706-2-git-send-email-peterx@redhat.com> <87zitqjrhj.fsf@blackfin.pond.sub.org> <87vb4eicdu.fsf@blackfin.pond.sub.org> <20160323025819.GD28183@pxdev.xzpeter.org> <87shzhbktm.fsf@blackfin.pond.sub.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <87shzhbktm.fsf@blackfin.pond.sub.org> Subject: Re: [Qemu-devel] [PATCH v5 1/5] arm: qmp: add GICCapability struct List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: wei@redhat.com, peter.maydell@linaro.org, drjones@redhat.com, qemu-devel@nongnu.org, mdroth@linux.vnet.ibm.com, abologna@redhat.com, qemu-arm@nongnu.org On Wed, Mar 23, 2016 at 10:33:09AM +0100, Markus Armbruster wrote: > Peter Xu writes: > > > On Tue, Mar 22, 2016 at 07:41:17PM +0100, Markus Armbruster wrote: > >> Markus Armbruster writes: > >> >> +## > >> >> +# @GICCapability: > >> >> +# > >> >> +# This struct describes capability for a specific GIC version. These > >> >> +# bits are not only decided by QEMU/KVM software version, but also > >> >> +# decided by the hardware that the program is running upon. > >> >> +# > >> >> +# @version: version of GIC to be described. > >> >> +# > >> >> +# @emulated: whether current QEMU/hardware supports emulated GIC > >> >> +# device in user space. > >> >> +# > >> >> +# @kernel: whether current QEMU/hardware supports hardware > >> >> +# accelerated GIC device in kernel. > >> >> +# > >> >> +# Since: 2.6 > >> >> +## > >> >> +{ 'struct': 'GICCapability', > >> >> + 'data': { 'version': 'int', > >> >> + 'emulated': 'bool', > >> >> + 'kernel': 'bool' } } [*] Marking here... > So GICCapability essentially tells its users whether certain > configurations have a chance to work. > > I think what's missing in your patch is the connection from > GICCapability to the actual configuration options. As is, you just have > to know what options the presence of each possible GICCapability value > unlocks. It needs to be documented instead. What I understand is that, above [*] should have explained what does each entry mean. E.g., as mentioned in the qapi-schema, there are explainations about "version", "emulated" and "kernel" key words. If we go deeper into e.g., "emulated" key word, we will got: "whether current QEMU/hardware supports emulated GIC device in user space." So this boolean will tell just as it is explained. Maybe I failed to get the point of your review comment... :( Would you please give an example on how should I better express this relationship? (btw, I have updated the commit message in v6 for current patch, to tell more about why we need this, and why we decided to add this ad hoc command. I'd be glad if we can continue the discussion based on that one. Thanks!) -- peterx