| Message ID | 1446184587-142784-6-git-send-email-guangrong.xiao@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 <guangrong.xiao@linux.intel.com> 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. > --- > 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() > 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
On Mon, 9 Nov 2015 13:35:51 +0200 "Michael S. Tsirkin" <mst@redhat.com> 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 <guangrong.xiao@linux.intel.com> > > 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 -- 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
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; +} + 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,
Implement ObjectType which is used by NVDIMM _DSM method in later patch Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com> --- hw/acpi/aml-build.c | 8 ++++++++ include/hw/acpi/aml-build.h | 1 + 2 files changed, 9 insertions(+)