| Message ID | 1449704528-289297-24-git-send-email-imammedo@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 2015/12/10 7:41, Igor Mammedov wrote: > Signed-off-by: Igor Mammedov <imammedo@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. " Thanks,
On Thu, 10 Dec 2015 10:07:43 +0800 Shannon Zhao <zhaoshenglong@huawei.com> wrote: > > > On 2015/12/10 7:41, Igor Mammedov wrote: > > Signed-off-by: Igor Mammedov <imammedo@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.
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), aml_name("CTRL"))); ifctx1 = aml_if(aml_lnot(aml_equal(aml_arg(1), aml_int(0x1)))); diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c index fd8ccfc..bbd37e9 100644 --- a/hw/i386/acpi-build.c +++ b/hw/i386/acpi-build.c @@ -469,7 +469,7 @@ static void build_append_pcihp_notify_entry(Aml *method, int slot) Aml *if_ctx; int32_t devfn = PCI_DEVFN(slot, 0); - if_ctx = aml_if(aml_and(aml_arg(0), aml_int(0x1U << slot))); + if_ctx = aml_if(aml_and(aml_arg(0), aml_int(0x1U << slot), NULL)); aml_append(if_ctx, aml_notify(aml_name("S%.02X", devfn), aml_arg(1))); aml_append(method, if_ctx); } @@ -669,7 +669,8 @@ static Aml *build_prt(void) aml_store(aml_shiftright(pin, aml_int(2), NULL), slot)); /* lnk_idx = (slot + pin) & 3 */ aml_append(while_ctx, - aml_store(aml_and(aml_add(pin, slot, NULL), aml_int(3)), lnk_idx)); + aml_store(aml_and(aml_add(pin, slot, NULL), aml_int(3), NULL), + lnk_idx)); /* route[2] = "LNK[D|A|B|C]", selection based on pin % 3 */ aml_append(while_ctx, initialize_route(route, "LNKD", lnk_idx, 0)); @@ -684,7 +685,8 @@ static Aml *build_prt(void) aml_index(route, aml_int(0)))); /* route[1] = pin & 3 */ aml_append(while_ctx, - aml_store(aml_and(pin, aml_int(3)), aml_index(route, aml_int(1)))); + aml_store(aml_and(pin, aml_int(3), NULL), + aml_index(route, aml_int(1)))); /* res[pin] = route */ aml_append(while_ctx, aml_store(route, aml_index(res, pin))); /* pin++ */ diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h index 3be727e..ca365c9 100644 --- a/include/hw/acpi/aml-build.h +++ b/include/hw/acpi/aml-build.h @@ -231,7 +231,7 @@ Aml *aml_to_integer(Aml *arg); Aml *aml_to_hexstring(Aml *src, Aml *dst); Aml *aml_to_buffer(Aml *src, Aml *dst); Aml *aml_store(Aml *val, Aml *target); -Aml *aml_and(Aml *arg1, Aml *arg2); +Aml *aml_and(Aml *arg1, Aml *arg2, Aml *dst); Aml *aml_or(Aml *arg1, Aml *arg2, Aml *dst); Aml *aml_lor(Aml *arg1, Aml *arg2); Aml *aml_shiftleft(Aml *arg1, Aml *count);
Signed-off-by: Igor Mammedov <imammedo@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(-)