Re: [PATCH v3 1/4] hw/acpi/aml-build.c: add aml_irq() representing the 3-byte IRQ descriptor

[Igor Mammedov <imammedo@redhat.com>]

On Fri,  8 May 2026 11:17:40 +0100
Mark Cave-Ayland <mark.caveayland@nutanix.com> wrote:

> The existing aml_interrupt() uses the Extended Interrupt Descriptor to store
> the interrupt information, however newer Windows will only parse the
> standard IRQ Descriptor when enumerating ISA serial ports.
> 
> Signed-off-by: Mark Cave-Ayland <mark.caveayland@nutanix.com>

with nit below fixed:

Reviewed-by: Igor Mammedov <imammedo@redhat.com>

> ---
>  include/hw/acpi/aml-build.h |  2 ++
>  hw/acpi/aml-build-stub.c    |  6 ++++++
>  hw/acpi/aml-build.c         | 25 +++++++++++++++++++++++++
>  3 files changed, 33 insertions(+)
> 
> diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h
> index e70e0643b1..eaff025d26 100644
> --- a/include/hw/acpi/aml-build.h
> +++ b/include/hw/acpi/aml-build.h
> @@ -343,6 +343,8 @@ Aml *aml_io(AmlIODecode dec, uint16_t min_base, uint16_t max_base,
>  Aml *aml_operation_region(const char *name, AmlRegionSpace rs,
>                            Aml *offset, uint32_t len);
>  Aml *aml_irq_no_flags(uint8_t irq);
> +Aml *aml_irq(uint8_t irq, AmlLevelAndEdge level_and_edge,
> +             AmlActiveHighAndLow high_and_low, AmlShared shared);
>  Aml *aml_named_field(const char *name, unsigned length);
>  Aml *aml_reserved_field(unsigned length);
>  Aml *aml_local(int num);
> diff --git a/hw/acpi/aml-build-stub.c b/hw/acpi/aml-build-stub.c
> index 89a8fec4af..3180c7c962 100644
> --- a/hw/acpi/aml-build-stub.c
> +++ b/hw/acpi/aml-build-stub.c
> @@ -67,6 +67,12 @@ Aml *aml_irq_no_flags(uint8_t irq)
>      return NULL;
>  }
>  
> +Aml *aml_irq(uint8_t irq, AmlLevelAndEdge level_and_edge,
> +             AmlActiveHighAndLow high_and_low, AmlShared shared)
> +{
> +    return NULL;
> +}
> +
>  Aml *aml_interrupt(AmlConsumerAndProducer con_and_pro,
>                     AmlLevelAndEdge level_and_edge,
>                     AmlActiveHighAndLow high_and_low, AmlShared shared,
> diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
> index 7edc8aed42..3aaf96c2a7 100644
> --- a/hw/acpi/aml-build.c
> +++ b/hw/acpi/aml-build.c
> @@ -1061,6 +1061,31 @@ Aml *aml_irq_no_flags(uint8_t irq)
>      return var;
>  }
>  
> +/*
> + * ACPI 1.0b: 6.4.2.1.1 ASL Macro for IRQ Descriptor
> + *
> + * More verbose description at:
> + * ACPI 5.0: 19.5.63 IRQ (Interrupt Resource Descriptor Macro)
> + *           6.4.2.1 IRQ Descriptor
> + */
> +Aml *aml_irq(uint8_t irq, AmlLevelAndEdge level_and_edge,
> +             AmlActiveHighAndLow high_and_low, AmlShared shared)
> +{
> +    uint16_t irq_mask;
> +    Aml *var = aml_alloc();
> +    uint8_t irq_flags = level_and_edge | (high_and_low << 3) |
> +                        (shared << 4);

I'd add here an assert for invalid level_and_edge/high_and_low

> +
> +    assert(irq < 16);
> +    build_append_byte(var->buf, 0x23); /* IRQ descriptor 3 byte form */
> +
> +    irq_mask = 1U << irq;
> +    build_append_byte(var->buf, irq_mask & 0xFF); /* IRQ mask bits[7:0] */
> +    build_append_byte(var->buf, irq_mask >> 8); /* IRQ mask bits[15:8] */
> +    build_append_byte(var->buf, irq_flags); /* IRQ flags */
> +    return var;
> +}
> +
>  /* ACPI 1.0b: 16.2.5.4 Type 2 Opcodes Encoding: DefLNot */
>  Aml *aml_lnot(Aml *arg)
>  {