Re: [Qemu-devel] [PATCH v4 05/20] hw/acpi/aml-build: Add aml_interrupt() term

[Shannon Zhao <zhaoshenglong@huawei.com>]

On 2015/4/9 17:54, Igor Mammedov wrote:
> On Thu, 9 Apr 2015 14:09:23 +0800
> Shannon Zhao <zhaoshenglong@huawei.com> wrote:
> 
>> > On 2015/4/8 22:57, Alex Bennée wrote:
>>> > > 
>>> > > Shannon Zhao <zhaoshenglong@huawei.com> writes:
>>> > > 
>>>> > >> From: Shannon Zhao <shannon.zhao@linaro.org>
>>>> > >>
>>>> > >> Add aml_interrupt() for describing device interrupt in resource template.
>>>> > >> These can be used to generating DSDT table for ACPI on ARM.
>>>> > >>
>>>> > >> Signed-off-by: Shannon Zhao <zhaoshenglong@huawei.com>
>>>> > >> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
>>>> > >> ---
>>>> > >>  hw/acpi/aml-build.c         | 18 ++++++++++++++++++
>>>> > >>  include/hw/acpi/aml-build.h |  1 +
>>>> > >>  2 files changed, 19 insertions(+)
>>>> > >>
>>>> > >> diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
>>>> > >> index fefe7c7..bd1713c 100644
>>>> > >> --- a/hw/acpi/aml-build.c
>>>> > >> +++ b/hw/acpi/aml-build.c
>>>> > >> @@ -527,6 +527,24 @@ Aml *aml_memory32_fixed(uint64_t addr, uint64_t size, uint8_t rw_flag)
>>>> > >>      return var;
>>>> > >>  }
>>>> > >>  
>>>> > >> +/*
>>>> > >> + * ACPI 1.0: 6.4.3.6 Interrupt (Interrupt Resource Descriptor Macro)
>>>> > >> + */
>>>> > >> +Aml *aml_interrupt(uint8_t irq_flags, int irq)
>>>> > >> +{
>>>> > >> +    Aml *var = aml_alloc();
>>>> > >> +    build_append_byte(var->buf, 0x89); /* Extended irq descriptor */
>>>> > >> +    build_append_byte(var->buf, 6); /* Length, bits[7:0] minimum value = 6 */
>>>> > >> +    build_append_byte(var->buf, 0); /* Length, bits[15:8] minimum value = 0 */
>>>> > >> +    build_append_byte(var->buf, irq_flags); /* Interrupt Vector
>>>> > >> Information. */
>>> > > 
>>> > > As the spec says [7:4] is RES0 we might want to assert this is the case.
>>> > > 
>> > 
>> > Yes, we should check although the probability is very small.
> It's revision specific and we don't have infrastructure to check/validate
> per revision differences.
> 
> I'd  split irq_flags from bitmask to a several args, a enum for each implemented
> bit to avoid user setting reserved bits.
> 
> /* ACPI X.X: ... */
> AmlWakeCap {
>     aml_not_wake_capable = 0,
>     aml_wake_capable = 1
> }
> 
> ...
> 

Thanks, will try this way.

-- 
Thanks,
Shannon