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

[Igor Mammedov]

On Thu, 9 Apr 2015 14:09:23 +0800
Shannon Zhao <address@hidden> wrote:

> On 2015/4/8 22:57, Alex Bennée wrote:
> > 
> > Shannon Zhao <address@hidden> writes:
> > 
> >> From: Shannon Zhao <address@hidden>
> >>
> >> 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 <address@hidden>
> >> Signed-off-by: Shannon Zhao <address@hidden>
> >> ---
> >>  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
}

...

> But the reserve bits are different in ACPI 5.1.
> 
> Bit[7:5] Reserved (must be 0)
> Bit[4] Wake Capability, _WKC
> 
> >> +    build_append_byte(var->buf, 0x01); /* Interrupt table length = 1 */
> >> +    build_append_byte(var->buf, irq & 0xff); /* Interrupt Number 
> >> bits[7:0] */
> >> +    build_append_byte(var->buf, (irq >> 8) & 0xff); /* Interrupt Number 
> >> bits[15:8] */
> >> +    build_append_byte(var->buf, (irq >> 16) & 0xff); /* Interrupt Number 
> >> bits[23:16] */
> >> +    build_append_byte(var->buf, (irq >> 24) & 0xff); /* Interrupt
> >> Number bits[31:24] */
> > 
> > Again extractNN bitops?
> > 
> >> +    return var;
> >> +}
> >> +
> >>  /* ACPI 1.0b: 6.4.2.5 I/O Port Descriptor */
> >>  Aml *aml_io(AmlIODecode dec, uint16_t min_base, uint16_t max_base,
> >>              uint8_t aln, uint8_t len)
> >> diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h
> >> index baa0652..315c729 100644
> >> --- a/include/hw/acpi/aml-build.h
> >> +++ b/include/hw/acpi/aml-build.h
> >> @@ -163,6 +163,7 @@ Aml *aml_call2(const char *method, Aml *arg1, Aml 
> >> *arg2);
> >>  Aml *aml_call3(const char *method, Aml *arg1, Aml *arg2, Aml *arg3);
> >>  Aml *aml_call4(const char *method, Aml *arg1, Aml *arg2, Aml *arg3, Aml 
> >> *arg4);
> >>  Aml *aml_memory32_fixed(uint64_t addr, uint64_t size, uint8_t rw_flag);
> >> +Aml *aml_interrupt(uint8_t irq_flags, int irq);
> >>  Aml *aml_io(AmlIODecode dec, uint16_t min_base, uint16_t max_base,
> >>              uint8_t aln, uint8_t len);
> >>  Aml *aml_operation_region(const char *name, AmlRegionSpace rs,
> > 
>