Re: [Qemu-devel] [PATCH 07/74] acpi: aml: add helper for Opcode Arg2 Arg2 [Dst] AML pattern

[Shannon Zhao]

On 2015/12/10 7:41, Igor Mammedov wrote:
> Currently AML API doesn't compose terms in form of
> following pattern:
> 
>    Opcode Arg2 Arg2 [Dst]
> 
> but ASL used in piix4/q35 DSDT ACPI tables uses that
> form, so for clean conversion of it, AML API should
> be able to handle an optional 'Dst' argumet used there.
> 
> Since above pattern is used by arithmetic/bit ops,
> introduce helper that they could reuse.
> It reduces code duplication in existing 5 aml_foo()
> functions and also will prevent more duplication
> when exiting functions are extended to support
> optional 'Dst' argument.
> 
> Signed-off-by: Igor Mammedov <address@hidden>
> ---
>  hw/acpi/aml-build.c | 61 
> ++++++++++++++++++++++++++++-------------------------
>  1 file changed, 32 insertions(+), 29 deletions(-)
> 
> diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
> index a6e4c54..22015d2 100644
> --- a/hw/acpi/aml-build.c
> +++ b/hw/acpi/aml-build.c
> @@ -436,44 +436,55 @@ Aml *aml_store(Aml *val, Aml *target)
>      return var;
>  }
>  
> -/* ACPI 1.0b: 16.2.5.4 Type 2 Opcodes Encoding: DefAnd */
> -Aml *aml_and(Aml *arg1, Aml *arg2)
> +/**
> + * build_opcode_2arg_dst:
> + * @op: 1-byte opcode
> + * @arg1: 1st operand
> + * @arg2: 2nd operand
> + * @dst: optional target to store to, set to NULL if it's not required
> + *
> + * An internal helper to compose AML terms that have
> + *   "Op Operand Operand Target"
> + * pattern.
> + *
> + * Returns: The newly allocated and composed according to patter Aml object.
> + */
> +static Aml *
> +build_opcode_2arg_dst(uint8_t op, Aml *arg1, Aml *arg2, Aml *dst)
>  {
> -    Aml *var = aml_opcode(0x7B /* AndOp */);
> +    Aml *var = aml_opcode(op);
>      aml_append(var, arg1);
>      aml_append(var, arg2);
> -    build_append_byte(var->buf, 0x00 /* NullNameOp */);
> +    if (dst) {
> +        aml_append(var, dst);
> +    } else {
> +        build_append_byte(var->buf, 0x00 /* NullNameOp */);
> +    }
>      return var;
>  }
> 
This looks good. Maybe you could add a helper like build_opcode_2arg(),
then aml_lor() and aml_lgreater() could use this.

-- 
Shannon