diff mbox

[v4,04/20] hw/acpi/aml-build: Add aml_memory32_fixed() term

Message ID 1428055432-12120-5-git-send-email-zhaoshenglong@huawei.com
State New
Headers show

Commit Message

Shannon Zhao April 3, 2015, 10:03 a.m. UTC
From: Shannon Zhao <shannon.zhao@linaro.org>

Add aml_memory32_fixed() for describing device mmio region 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         | 22 ++++++++++++++++++++++
 include/hw/acpi/aml-build.h |  1 +
 2 files changed, 23 insertions(+)

Comments

Alex Bennée April 8, 2015, 2:54 p.m. UTC | #1
Shannon Zhao <zhaoshenglong@huawei.com> writes:

> From: Shannon Zhao <shannon.zhao@linaro.org>
>
> Add aml_memory32_fixed() for describing device mmio region 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         | 22 ++++++++++++++++++++++
>  include/hw/acpi/aml-build.h |  1 +
>  2 files changed, 23 insertions(+)
>
> diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
> index 8d01959..fefe7c7 100644
> --- a/hw/acpi/aml-build.c
> +++ b/hw/acpi/aml-build.c
> @@ -505,6 +505,28 @@ Aml *aml_call4(const char *method, Aml *arg1, Aml *arg2, Aml *arg3, Aml *arg4)
>      return var;
>  }
>  
> +/*
> + * ACPI 1.0: 6.4.3.4 Memory32Fixed (Memory Resource Descriptor Macro)
> + */
> +Aml *aml_memory32_fixed(uint64_t addr, uint64_t size, uint8_t rw_flag)
> +{
> +    Aml *var = aml_alloc();

This is more aimed at the ACPI maintainers but I wonder if there should
be an aml_alloc_sized that pre-allocates the GArray? Otherwise we spend
a lot of time realloc'ing while building these entries up. Or even a
varidac build_append_bytes?

> +    build_append_byte(var->buf, 0x86); /* Memory32Fixed Resource Descriptor */
> +    build_append_byte(var->buf, 9); /* Length, bits[7:0] value = 9 */
> +    build_append_byte(var->buf, 0); /* Length, bits[15:8] value = 0 */
> +    build_append_byte(var->buf, rw_flag); /* Write status, 1 rw 0 ro */
> +    build_append_byte(var->buf, addr & 0xff); /* Range base address bits[7:0] */
> +    build_append_byte(var->buf, (addr >> 8) & 0xff); /* Range base address bits[15:8] */
> +    build_append_byte(var->buf, (addr >> 16) & 0xff); /* Range base address bits[23:16] */
> +    build_append_byte(var->buf, (addr >> 24) & 0xff); /* Range base
> address bits[31:24] */

I'm should point out we have handy utility functions for bit fiddling:

    build_append_byte(var->buf, extract64(addr, 8, 8)); /* Range base address bits[15:8] */

> +
> +    build_append_byte(var->buf, size & 0xff); /* Range length bits[7:0] */
> +    build_append_byte(var->buf, (size >> 8) & 0xff); /* Range length bits[15:8] */
> +    build_append_byte(var->buf, (size >> 16) & 0xff); /* Range length bits[23:16] */
> +    build_append_byte(var->buf, (size >> 24) & 0xff); /* Range length
> bits[31:24] */

Hmm we seem to have two 64 bit inputs which we only use 32 bits worth
of. Maybe the prototype should be fixed to avoid accidents of accidentally
passing in 64 bit values.


> +    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 1705001..baa0652 100644
> --- a/include/hw/acpi/aml-build.h
> +++ b/include/hw/acpi/aml-build.h
> @@ -162,6 +162,7 @@ Aml *aml_call1(const char *method, Aml *arg1);
>  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_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,
Alex Bennée April 9, 2015, 9:27 a.m. UTC | #2
Michael S. Tsirkin <mst@redhat.com> writes:

> On Wed, Apr 08, 2015 at 03:54:45PM +0100, Alex Bennée wrote:
>> > diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
>> > index 8d01959..fefe7c7 100644
>> > --- a/hw/acpi/aml-build.c
>> > +++ b/hw/acpi/aml-build.c
>> > @@ -505,6 +505,28 @@ Aml *aml_call4(const char *method, Aml *arg1, Aml *arg2, Aml *arg3, Aml *arg4)
>> >      return var;
>> >  }
>> >  
>> > +/*
>> > + * ACPI 1.0: 6.4.3.4 Memory32Fixed (Memory Resource Descriptor Macro)
>> > + */
>> > +Aml *aml_memory32_fixed(uint64_t addr, uint64_t size, uint8_t rw_flag)
>> > +{
>> > +    Aml *var = aml_alloc();
>> 
>> This is more aimed at the ACPI maintainers but I wonder if there should
>> be an aml_alloc_sized that pre-allocates the GArray? Otherwise we spend
>> a lot of time realloc'ing while building these entries up. Or even a
>> varidac build_append_bytes?
>
> Can you show measureable VM boot speedup from this?
> If not, it's not worth bothering with.

Fair enough, this is a start-up thing and I guess you could probably
only measure a difference in a highly targeted pathaological test case.
diff mbox

Patch

diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
index 8d01959..fefe7c7 100644
--- a/hw/acpi/aml-build.c
+++ b/hw/acpi/aml-build.c
@@ -505,6 +505,28 @@  Aml *aml_call4(const char *method, Aml *arg1, Aml *arg2, Aml *arg3, Aml *arg4)
     return var;
 }
 
+/*
+ * ACPI 1.0: 6.4.3.4 Memory32Fixed (Memory Resource Descriptor Macro)
+ */
+Aml *aml_memory32_fixed(uint64_t addr, uint64_t size, uint8_t rw_flag)
+{
+    Aml *var = aml_alloc();
+    build_append_byte(var->buf, 0x86); /* Memory32Fixed Resource Descriptor */
+    build_append_byte(var->buf, 9); /* Length, bits[7:0] value = 9 */
+    build_append_byte(var->buf, 0); /* Length, bits[15:8] value = 0 */
+    build_append_byte(var->buf, rw_flag); /* Write status, 1 rw 0 ro */
+    build_append_byte(var->buf, addr & 0xff); /* Range base address bits[7:0] */
+    build_append_byte(var->buf, (addr >> 8) & 0xff); /* Range base address bits[15:8] */
+    build_append_byte(var->buf, (addr >> 16) & 0xff); /* Range base address bits[23:16] */
+    build_append_byte(var->buf, (addr >> 24) & 0xff); /* Range base address bits[31:24] */
+
+    build_append_byte(var->buf, size & 0xff); /* Range length bits[7:0] */
+    build_append_byte(var->buf, (size >> 8) & 0xff); /* Range length bits[15:8] */
+    build_append_byte(var->buf, (size >> 16) & 0xff); /* Range length bits[23:16] */
+    build_append_byte(var->buf, (size >> 24) & 0xff); /* Range length bits[31:24] */
+    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 1705001..baa0652 100644
--- a/include/hw/acpi/aml-build.h
+++ b/include/hw/acpi/aml-build.h
@@ -162,6 +162,7 @@  Aml *aml_call1(const char *method, Aml *arg1);
 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_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,