From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:39641) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZvlkX-0002DU-6q for qemu-devel@nongnu.org; Mon, 09 Nov 2015 07:40:42 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZvlkT-0008JF-Rm for qemu-devel@nongnu.org; Mon, 09 Nov 2015 07:40:41 -0500 Received: from mx1.redhat.com ([209.132.183.28]:37152) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZvlkT-0008J3-Jw for qemu-devel@nongnu.org; Mon, 09 Nov 2015 07:40:37 -0500 Date: Mon, 9 Nov 2015 13:40:32 +0100 From: Igor Mammedov Message-ID: <20151109134032.45e9a6fc@nial.brq.redhat.com> In-Reply-To: <20151109113551.GC3324@redhat.com> References: <1446184587-142784-1-git-send-email-guangrong.xiao@linux.intel.com> <1446184587-142784-6-git-send-email-guangrong.xiao@linux.intel.com> <20151109113551.GC3324@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v6 05/33] acpi: add aml_object_type List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Michael S. Tsirkin" Cc: Xiao Guangrong , ehabkost@redhat.com, kvm@vger.kernel.org, gleb@kernel.org, mtosatti@redhat.com, qemu-devel@nongnu.org, stefanha@redhat.com, pbonzini@redhat.com, dan.j.williams@intel.com, rth@twiddle.net On Mon, 9 Nov 2015 13:35:51 +0200 "Michael S. Tsirkin" wrote: > On Fri, Oct 30, 2015 at 01:55:59PM +0800, Xiao Guangrong wrote: > > Implement ObjectType which is used by NVDIMM _DSM method in > > later patch > > > > Signed-off-by: Xiao Guangrong > > I had to go dig in the _DSM patch to see how it's used. > And sure enough, callers have to know AML to make > sense of it. So pls don't split out tiny patches like this. > include the callee with the caller. I'd prefer AML API patches as separate ones, as it makes easier to review vs ACPI spec and also easier to reuse in another series. And once they are ok/reviewed we can merge them ahead of users, so next series respins won't have to post the same patches over and over and we won't have to review them again every respin and others could use already merged API instead of duplicating work that's been already done. > > > --- > > hw/acpi/aml-build.c | 8 ++++++++ > > include/hw/acpi/aml-build.h | 1 + > > 2 files changed, 9 insertions(+) > > > > diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c > > index efc06ab..9f792ab 100644 > > --- a/hw/acpi/aml-build.c > > +++ b/hw/acpi/aml-build.c > > @@ -1178,6 +1178,14 @@ Aml *aml_concatenate(Aml *source1, Aml *source2, Aml *target) > > return var; > > } > > > > +/* ACPI 1.0b: 16.2.5.4 Type 2 Opcodes Encoding: DefObjectType */ > > +Aml *aml_object_type(Aml *object) > > +{ > > + Aml *var = aml_opcode(0x8E /* ObjectTypeOp */); > > + aml_append(var, object); > > + return var; > > +} > > + > > It would be better to have a higher level API > that can be used without knowning AML. > For example: > > aml_object_type_is_package() Higher level API is fine but it's better be done on top of AML one, i.e. add it in addition to aml_object_type() > > > > > void > > build_header(GArray *linker, GArray *table_data, > > AcpiTableHeader *h, const char *sig, int len, uint8_t rev) > > diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h > > index 325782d..5b8a118 100644 > > --- a/include/hw/acpi/aml-build.h > > +++ b/include/hw/acpi/aml-build.h > > @@ -278,6 +278,7 @@ Aml *aml_derefof(Aml *arg); > > Aml *aml_sizeof(Aml *arg); > > Aml *aml_create_field(Aml *srcbuf, Aml *index, Aml *len, const char *name); > > Aml *aml_concatenate(Aml *source1, Aml *source2, Aml *target); > > +Aml *aml_object_type(Aml *object); > > > > void > > build_header(GArray *linker, GArray *table_data, > > -- > > 1.8.3.1 > -- > To unsubscribe from this list: send the line "unsubscribe kvm" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html