Re: [Qemu-devel] [RFC PATCH 09/11] hw/acpi/acpi-build-utils: Add acpi_fixed_memory32() and acpi_extended_irq()

[Igor Mammedov]

On Sun, 25 Jan 2015 10:39:21 +0200
"Michael S. Tsirkin" <address@hidden> wrote:

> On Sat, Jan 24, 2015 at 05:21:18PM +0800, Shannon Zhao wrote:
> > Add acpi_fixed_memory32() for describing device mmio region in resource 
> > template.
> > Add acpi_extended_irq() 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>
> > ---
> >  hw/acpi/acpi-build-utils.c         |   42 
> > ++++++++++++++++++++++++++++++++++++
> >  include/hw/acpi/acpi-build-utils.h |    2 +
> >  2 files changed, 44 insertions(+), 0 deletions(-)
> > 
> > diff --git a/hw/acpi/acpi-build-utils.c b/hw/acpi/acpi-build-utils.c
> > index 59873e3..211c4d3 100644
> > --- a/hw/acpi/acpi-build-utils.c
> > +++ b/hw/acpi/acpi-build-utils.c
> > @@ -493,6 +493,48 @@ AcpiAml acpi_call4(const char *method, AcpiAml arg1, 
> > AcpiAml arg2,
> >  }
> >  
> >  /*
> > + * ACPI 5.1: 19.5.80 Memory32Fixed (Memory Resource Descriptor Macro)
> 
> Pls document the first spec which includes a feature, not the last one.
> 
> > + *           6.4.2 Small Resource Data Type
> 
> So add API for small resource data type?
Probably not worth it, I'll take a closer look at it since
I add quite a bit of resources in initial series, so
helper to compose header might save some LOCs.

But it shouldn't be externally available API since it's
lowlevel concept basically reflecting header format
with variable amount of bytes after it depending on type.

> 
> > + */
> > +AcpiAml acpi_fixed_memory32(uint64_t addr, uint64_t size, uint8_t rw_flag)
> 
> so name it memory32_fixed?
> 
> > +{
> > +    AcpiAml var = aml_allocate_internal(0, NON_BLOCK);
> > +    build_append_byte(var.buf, 0x86); /* Extended irq descriptor */
> > +    build_append_byte(var.buf, 9);
> > +    build_append_byte(var.buf, 0);
> > +    build_append_byte(var.buf, rw_flag);
> > +    build_append_byte(var.buf, addr & 0xff);
> > +    build_append_byte(var.buf, (addr >> 8) & 0xff);
> > +    build_append_byte(var.buf, (addr >> 16) & 0xff);
> > +    build_append_byte(var.buf, (addr >> 24) & 0xff);
> > +
> > +    build_append_byte(var.buf, size & 0xff);
> > +    build_append_byte(var.buf, (size >> 8) & 0xff);
> > +    build_append_byte(var.buf, (size >> 16) & 0xff);
> > +    build_append_byte(var.buf, (size >> 24) & 0xff);
> > +    return var;
> > +}
> > +
> > +/*
> > + * ACPI 5.1: 19.5.61 Interrupt (Interrupt Resource Descriptor Macro)
> > + *           6.4.2 Small Resource Data Type
> > + */
> > +AcpiAml acpi_extended_irq(uint8_t irq_flags, int irq)
> 
> So acpi_interrupt?
> 
> > +{
> > +    AcpiAml var = aml_allocate_internal(0, NON_BLOCK);
> > +    build_append_byte(var.buf, 0x89); /* Extended irq descriptor */
> > +    build_append_byte(var.buf, 6);
> > +    build_append_byte(var.buf, 0);
> > +    build_append_byte(var.buf, irq_flags);
> > +    build_append_byte(var.buf, 0x01);
> > +    build_append_byte(var.buf, irq & 0xff);
> > +    build_append_byte(var.buf, (irq >> 8) & 0xff);
> > +    build_append_byte(var.buf, (irq >> 16) & 0xff);
> > +    build_append_byte(var.buf, (irq >> 24) & 0xff);
> > +    return var;
> 
> 
> Pls add comments to document opcode constants.
> 
> > +}
> > +
> > +/*
> >   * ACPI 5.0: 19.5.62 IO (IO Resource Descriptor Macro)
> >   *           6.4.2 Small Resource Data Type
> >  */
> > diff --git a/include/hw/acpi/acpi-build-utils.h 
> > b/include/hw/acpi/acpi-build-utils.h
> > index d39b5b1..bfed546 100644
> > --- a/include/hw/acpi/acpi-build-utils.h
> > +++ b/include/hw/acpi/acpi-build-utils.h
> > @@ -115,6 +115,8 @@ AcpiAml acpi_call3(const char *method, AcpiAml arg1, 
> > AcpiAml arg2,
> >                     AcpiAml arg3);
> >  AcpiAml acpi_call4(const char *method, AcpiAml arg1, AcpiAml arg2,
> >                     AcpiAml arg3, AcpiAml arg4);
> > +AcpiAml acpi_fixed_memory32(uint64_t addr, uint64_t size, uint8_t rw_flag);
> > +AcpiAml acpi_extended_irq(uint8_t irq_flags, int irq);
> >  AcpiAml acpi_io(acpiIODecode dec, uint16_t min_base, uint16_t max_base,
> >                  uint8_t aln, uint8_t len);
> >  AcpiAml acpi_iqr_no_flags(uint8_t irq);
> > -- 
> > 1.7.1
> >