On Thu, 10 Dec 2015 10:07:43 +0800
Shannon Zhao <zhaoshengl...@huawei.com> wrote:
>
>
> On 2015/12/10 7:41, Igor Mammedov wrote:
> > Signed-off-by: Igor Mammedov <imamm...@redhat.com>
> > ---
> > hw/acpi/aml-build.c | 4 ++--
> > hw/arm/virt-acpi-build.c | 2 +-
> > hw/i386/acpi-build.c | 8 +++++---
> > include/hw/acpi/aml-build.h | 2 +-
> > 4 files changed, 9 insertions(+), 7 deletions(-)
> >
> > diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
> > index 4f62512..2ca9207 100644
> > --- a/hw/acpi/aml-build.c
> > +++ b/hw/acpi/aml-build.c
> > @@ -499,9 +499,9 @@ build_opcode_2arg_dst(uint8_t op, Aml *arg1,
> > Aml *arg2, Aml *dst) }
> >
> > /* ACPI 1.0b: 16.2.5.4 Type 2 Opcodes Encoding: DefAnd */
> > -Aml *aml_and(Aml *arg1, Aml *arg2)
> > +Aml *aml_and(Aml *arg1, Aml *arg2, Aml *dst)
> > {
> > - return build_opcode_2arg_dst(0x7B /* AndOp */, arg1, arg2,
> > NULL);
> > + return build_opcode_2arg_dst(0x7B /* AndOp */, arg1, arg2,
> > dst); }
> >
> > /* ACPI 1.0b: 16.2.5.4 Type 2 Opcodes Encoding: DefOr */
> > diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> > index 1cc98f5..698b5f2 100644
> > --- a/hw/arm/virt-acpi-build.c
> > +++ b/hw/arm/virt-acpi-build.c
> > @@ -272,7 +272,7 @@ static void acpi_dsdt_add_pci(Aml *scope, const
> > MemMapEntry *memmap, int irq, aml_create_dword_field(aml_arg(3),
> > aml_int(8), "CDW3")); aml_append(ifctx, aml_store(aml_name("CDW2"),
> > aml_name("SUPP"))); aml_append(ifctx, aml_store(aml_name("CDW3"),
> > aml_name("CTRL")));
> > - aml_append(ifctx, aml_store(aml_and(aml_name("CTRL"),
> > aml_int(0x1D)),
> > + aml_append(ifctx, aml_store(aml_and(aml_name("CTRL"),
> > aml_int(0x1D), NULL),
>
> I'm not sure why you must extend this kind functions now. When I post
> the patch to add aml_and(), you said
> "
> >>>> +Aml *aml_and(Aml *arg1, Aml *arg2, Aml *arg3)
> >> I know that it's possible to Store inside of And(a, b, save_here)
> >> ASL op, but could you instead rewrite it to
> >>
> >> Store(And(a, b), save_here)
> >>
> >> so it wouldn't clatter trivial And(a,b) uses and drop this hunk.
> >>
> > Yes, we can use Store(And(a, b), save_here) but according to the
> > SPEC the And op can have 3 args. We don't support it?
> I don't think that we should do it if it could be implemented
> using 2 already existing API calls to keep it simple and not to
> pollute code with extra ", NULL" argument in most cases.
> "
I'd prefer 'Store(And(a, b), save_here)' but piix4/q35 DSDT currently
utilizes And(arg1,arg2, dst) form and if I do that during conversion
that will break exact match with IASL compiled DSDT and hence
break 'make check' tests, since it produces different series of AML
opcodes.
Hence making hard to prove that conversion doesn't introduce any
regression.
