Re: [Qemu-devel] [PATCH v4 for-2.3 01/25] acpi: fix aml_equal term implementation

[Igor Mammedov]

On Mon, 9 Mar 2015 12:04:51 +0100
"Michael S. Tsirkin" <address@hidden> wrote:

> On Mon, Mar 09, 2015 at 11:28:22AM +0100, Igor Mammedov wrote:
> > On Sun,  8 Mar 2015 13:16:03 +0200
> > Marcel Apfelbaum <address@hidden> wrote:
> > 
> > > The DefLEqual op does not have a target operand. Remove it.
> > > 
> > > Signed-off-by: Marcel Apfelbaum <address@hidden>
> > Reviewed-by: Igor Mammedov <address@hidden>
> > 
> > > ---
> > >  hw/acpi/aml-build.c | 1 -
> > >  1 file changed, 1 deletion(-)
> > > 
> > > diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
> > > index 876cada..0d14561 100644
> > > --- a/hw/acpi/aml-build.c
> > > +++ b/hw/acpi/aml-build.c
> > > @@ -542,7 +542,6 @@ Aml *aml_equal(Aml *arg1, Aml *arg2)
> > >      Aml *var = aml_opcode(0x93 /* LequalOp */);
> > >      aml_append(var, arg1);
> > >      aml_append(var, arg2);
> > > -    build_append_int(var->buf, 0x00); /* NullNameOp */
> > It's just happens to work in case CPU and PCI hotplug because
> > it LEqual was the only predicate in if block and NullNameOp
> > was considered as part of inner code-block, like:
> > if (LEqual(a, b)) {
> >    NullName; // nop
> >    ...
> > }
> 
> So - maybe aml_if should get 3rd parameter - the command?
it's not only one command it's block of AML code inside of 'if' scope.
Adding 3rd argument would mean inventing another not defined by spec element
like aml_block() where you could put AML items that are in block,
I'd like to keep non spec items to minimum and not add them unless we have to.

Then for consistence purposes we would add this 'aml_block' argument
to other block constructs like 'device, scope, package, ...'
So I think current way of defining context and then putting items in it
is pretty clean way as opposed to doing it backwards, first defining
elements somewhere and then passing that somewhere as argument to
a AML block construct.

> 
> > >      return var;
> > >  }
> > >