[v6,05/33] acpi: add aml_object_type
diff mbox

Message ID 1446184587-142784-6-git-send-email-guangrong.xiao@linux.intel.com
State New
Headers show

Commit Message

Xiao Guangrong Oct. 30, 2015, 5:55 a.m. UTC
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(+)

Comments

Michael S. Tsirkin Nov. 9, 2015, 11:35 a.m. UTC | #1
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
Igor Mammedov Nov. 9, 2015, 12:40 p.m. UTC | #2
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

Patch
diff mbox

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,