diff mbox

[v4,06/20] hw/arm/virt-acpi-build: Generation of DSDT table for virt devices

Message ID 1428055432-12120-7-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>

DSDT consists of the usual common table header plus a definition
block in AML encoding which describes all devices in the platform.

After initializing DSDT with header information the namespace is
created which is followed by the device encodings. The devices are
described using the Resource Template for the 32-Bit Fixed Memory
Range and the Extended Interrupt Descriptors.

Signed-off-by: Shannon Zhao <zhaoshenglong@huawei.com>
Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
---
 hw/arm/virt-acpi-build.c | 137 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 137 insertions(+)

Comments

Alex Bennée April 9, 2015, 9:51 a.m. UTC | #1
Shannon Zhao <zhaoshenglong@huawei.com> writes:

> From: Shannon Zhao <shannon.zhao@linaro.org>
>
> DSDT consists of the usual common table header plus a definition
> block in AML encoding which describes all devices in the platform.
>
> After initializing DSDT with header information the namespace is
> created which is followed by the device encodings. The devices are
> described using the Resource Template for the 32-Bit Fixed Memory
> Range and the Extended Interrupt Descriptors.
>
> Signed-off-by: Shannon Zhao <zhaoshenglong@huawei.com>
> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
> ---
>  hw/arm/virt-acpi-build.c | 137 +++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 137 insertions(+)
>
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index 388838a..516c1d0 100644
> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -57,6 +57,139 @@
>  #define ACPI_BUILD_DPRINTF(fmt, ...)
>  #endif
>  
> +static void acpi_dsdt_add_cpus(Aml *scope, int max_cpus)
> +{
> +    Aml *dev, *crs;
> +    int i;
> +    char name[5];

name, dev and crs could be declared inside the for() loop.

> +    for (i = 0; i < max_cpus; i++) {
> +        snprintf(name, 5, "CPU%u", i);

What happens here if you have 10 or 100 CPUs? 

> +        dev = aml_device("%s", name);

Also aml_device seems to take a format string so why not simply:

     dev = aml_device("CPU%u", i)

> +        aml_append(dev, aml_name_decl("_HID", aml_string("ACPI0007")));
> +        aml_append(dev, aml_name_decl("_UID", aml_int(i)));
> +        crs = aml_resource_template();
> +        aml_append(dev, aml_name_decl("_CRS", crs));
> +        aml_append(scope, dev);
> +    }
> +}
> +
> +static void acpi_dsdt_add_uart(Aml *scope, const hwaddr *uart_addr,
> +                                           const int *uart_irq)
> +{
> +    Aml *dev, *crs;
> +
> +    dev = aml_device("COM0");
> +    aml_append(dev, aml_name_decl("_HID", aml_string("ARMH0011")));
> +    aml_append(dev, aml_name_decl("_UID", aml_int(0)));
> +
> +    crs = aml_resource_template();
> +    aml_append(crs,
> +               aml_memory32_fixed(uart_addr[0], uart_addr[1], 0x01));
> +    aml_append(crs,
> +               aml_interrupt(0x01, *uart_irq + 32));
> +    aml_append(dev, aml_name_decl("_CRS", crs));
> +    aml_append(scope, dev);
> +}
> +
> +static void acpi_dsdt_add_rtc(Aml *scope, const hwaddr *rtc_addr,
> +                                          const int *rtc_irq)
> +{
> +    Aml *dev, *crs;
> +
> +    dev = aml_device("RTC0");
> +    aml_append(dev, aml_name_decl("_HID", aml_string("LNRO0013")));
> +    aml_append(dev, aml_name_decl("_UID", aml_int(0)));
> +
> +    crs = aml_resource_template();
> +    aml_append(crs,
> +               aml_memory32_fixed(rtc_addr[0], rtc_addr[1], 0x01));
> +    aml_append(crs,
> +               aml_interrupt(0x01, *rtc_irq + 32));
> +    aml_append(dev, aml_name_decl("_CRS", crs));
> +    aml_append(scope, dev);
> +}
> +
> +static void acpi_dsdt_add_flash(Aml *scope, const hwaddr *flash_addr)
> +{
> +    Aml *dev, *crs;
> +    hwaddr base = flash_addr[0];
> +    hwaddr size = flash_addr[1];
> +
> +    dev = aml_device("FLS0");
> +    aml_append(dev, aml_name_decl("_HID", aml_string("LNRO0015")));
> +    aml_append(dev, aml_name_decl("_UID", aml_int(0)));
> +
> +    crs = aml_resource_template();
> +    aml_append(crs,
> +               aml_memory32_fixed(base, size, 0x01));
> +    aml_append(dev, aml_name_decl("_CRS", crs));
> +    aml_append(scope, dev);
> +
> +    dev = aml_device("FLS1");
> +    aml_append(dev, aml_name_decl("_HID", aml_string("LNRO0015")));
> +    aml_append(dev, aml_name_decl("_UID", aml_int(1)));
> +    crs = aml_resource_template();
> +    aml_append(crs,
> +               aml_memory32_fixed(base + size, size, 0x01));
> +    aml_append(dev, aml_name_decl("_CRS", crs));
> +    aml_append(scope, dev);
> +}
> +
> +static void acpi_dsdt_add_virtio(Aml *scope, const hwaddr *mmio_addrs,
> +                                             const int *mmio_irq, int num)
> +{
> +    Aml *dev, *crs;
> +    hwaddr base = mmio_addrs[0];
> +    hwaddr size = mmio_addrs[1];

What ensures all these hw addresses are in 32 bit space on 64 bit platforms?

> +    int irq = *mmio_irq + 32;
> +    int i;
> +    char name[5];
> +
> +    for (i = 0; i < num; i++) {
> +        snprintf(name, 5, "VR%02u", i);
> +        dev = aml_device("%s", name);

Again why not call aml_device directly?

> +        aml_append(dev, aml_name_decl("_HID", aml_string("LNRO0005")));
> +        aml_append(dev, aml_name_decl("_UID", aml_int(i)));
> +
> +        crs = aml_resource_template();
> +        aml_append(crs,
> +                   aml_memory32_fixed(base, size, 0x01));
> +        aml_append(crs,
> +                   aml_interrupt(0x01, irq + i));
> +        aml_append(dev, aml_name_decl("_CRS", crs));
> +        aml_append(scope, dev);
> +        base += size;
> +    }
> +}
> +
> +/* DSDT */
> +static void
> +build_dsdt(GArray *table_data, GArray *linker, VirtGuestInfo *guest_info)
> +{
> +    Aml *scope, *dsdt;
> +    acpi_dsdt_info *info = guest_info->dsdt_info;
> +
> +    dsdt = init_aml_allocator();
> +    /* Reserve space for header */
> +    acpi_data_push(dsdt->buf, sizeof(AcpiTableHeader));
> +
> +    scope = aml_scope("\\_SB");
> +    acpi_dsdt_add_cpus(scope, guest_info->max_cpus);
> +    acpi_dsdt_add_uart(scope, info->uart_addr, info->uart_irq);
> +    acpi_dsdt_add_rtc(scope, info->rtc_addr, info->rtc_irq);
> +    acpi_dsdt_add_flash(scope, info->flash_addr);
> +    acpi_dsdt_add_virtio(scope, info->virtio_mmio_addr,
> +             info->virtio_mmio_irq, info->virtio_mmio_num);
> +
> +    aml_append(dsdt, scope);
> +    /* copy AML table into ACPI tables blob and patch header there */
> +    g_array_append_vals(table_data, dsdt->buf->data, dsdt->buf->len);
> +    build_header(linker, table_data,
> +        (void *)(table_data->data + table_data->len - dsdt->buf->len),
> +        "DSDT", dsdt->buf->len, 1);
> +    free_aml_allocator();
> +}
> +
>  typedef
>  struct AcpiBuildState {
>      /* Copy of table in RAM (for patching). */
> @@ -72,6 +205,7 @@ static
>  void virt_acpi_build(VirtGuestInfo *guest_info, AcpiBuildTables *tables)
>  {
>      GArray *table_offsets;
> +    GArray *tables_blob = tables->table_data;
>  
>      table_offsets = g_array_new(false, true /* clear */,
>                                          sizeof(uint32_t));
> @@ -89,6 +223,9 @@ void virt_acpi_build(VirtGuestInfo *guest_info, AcpiBuildTables *tables)
>       * DSDT
>       */
>  
> +    /* DSDT is pointed to by FADT */
> +    build_dsdt(tables_blob, tables->linker, guest_info);
> +
>      /* Cleanup memory that's no longer used. */
>      g_array_free(table_offsets, true);
>  }
Igor Mammedov April 9, 2015, 1:03 p.m. UTC | #2
On Thu, 09 Apr 2015 10:51:33 +0100
Alex Bennée <alex.bennee@linaro.org> wrote:

> 
> Shannon Zhao <zhaoshenglong@huawei.com> writes:
> 
> > From: Shannon Zhao <shannon.zhao@linaro.org>
> >
> > DSDT consists of the usual common table header plus a definition
> > block in AML encoding which describes all devices in the platform.
> >
> > After initializing DSDT with header information the namespace is
> > created which is followed by the device encodings. The devices are
> > described using the Resource Template for the 32-Bit Fixed Memory
> > Range and the Extended Interrupt Descriptors.
> >
> > Signed-off-by: Shannon Zhao <zhaoshenglong@huawei.com>
> > Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
> > ---
> >  hw/arm/virt-acpi-build.c | 137 +++++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 137 insertions(+)
> >
> > diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> > index 388838a..516c1d0 100644
> > --- a/hw/arm/virt-acpi-build.c
> > +++ b/hw/arm/virt-acpi-build.c
> > @@ -57,6 +57,139 @@
> >  #define ACPI_BUILD_DPRINTF(fmt, ...)
I wonder if all this DPRINTF crap should be replaced by tracepoints.
Oo at least it shouldn't be used in new code.

> >  #endif
> >  
> > +static void acpi_dsdt_add_cpus(Aml *scope, int max_cpus)
> > +{
> > +    Aml *dev, *crs;
> > +    int i;
> > +    char name[5];
> 
> name, dev and crs could be declared inside the for() loop.
> 
> > +    for (i = 0; i < max_cpus; i++) {
> > +        snprintf(name, 5, "CPU%u", i);
> 
> What happens here if you have 10 or 100 CPUs? 
> 
> > +        dev = aml_device("%s", name);
> 
> Also aml_device seems to take a format string so why not simply:
> 
>      dev = aml_device("CPU%u", i)
On top of that, ACPI name is limited to 4 characters so
CPU%u will allow to specify only 10 cpus, to support more
shrink CPU part and do something like:

Scope(CPUS) {
	Device(C000) {}
        ...
	Device(CFFF) {}
}

that would give us upto max 4096 CPU,
I have on TODO list to convert bitmap based x86 cpu hotplug to
memory hotplug based interface so that we could easily
switch to more CPUs when KVM starts support it.
And for ARM there is no point to have/maintain 2 interfaces
as we would have to do on x86.

> 
> > +        aml_append(dev, aml_name_decl("_HID", aml_string("ACPI0007")));
> > +        aml_append(dev, aml_name_decl("_UID", aml_int(i)));
> > +        crs = aml_resource_template();
> > +        aml_append(dev, aml_name_decl("_CRS", crs));
> > +        aml_append(scope, dev);
> > +    }
> > +}
> > +
> > +static void acpi_dsdt_add_uart(Aml *scope, const hwaddr *uart_addr,
> > +                                           const int *uart_irq)
> > +{
> > +    Aml *dev, *crs;
> > +
> > +    dev = aml_device("COM0");
> > +    aml_append(dev, aml_name_decl("_HID", aml_string("ARMH0011")));
> > +    aml_append(dev, aml_name_decl("_UID", aml_int(0)));
> > +
> > +    crs = aml_resource_template();
> > +    aml_append(crs,
> > +               aml_memory32_fixed(uart_addr[0], uart_addr[1], 0x01));
uart_addr[0], uart_addr[1] doesn't tell anything about what they are.
and doing array of it when it's not dynamic sized is confusing not to
mention passing addresses and sizes in it.
The same goes for the rest of foo_addr[] used in this patch/series,
if it's _addr then it shouldn't contain sizes.

how about:
 acpi_dsdt_add_uart(scope, uint32_t uart_addr, uint32_t uart_size, ...)

 
> > +    aml_append(crs,
> > +               aml_interrupt(0x01, *uart_irq + 32));
> > +    aml_append(dev, aml_name_decl("_CRS", crs));
> > +    aml_append(scope, dev);
> > +}
> > +
> > +static void acpi_dsdt_add_rtc(Aml *scope, const hwaddr *rtc_addr,
> > +                                          const int *rtc_irq)
> > +{
> > +    Aml *dev, *crs;
> > +
> > +    dev = aml_device("RTC0");
> > +    aml_append(dev, aml_name_decl("_HID", aml_string("LNRO0013")));
> > +    aml_append(dev, aml_name_decl("_UID", aml_int(0)));
> > +
> > +    crs = aml_resource_template();
> > +    aml_append(crs,
> > +               aml_memory32_fixed(rtc_addr[0], rtc_addr[1], 0x01));
> > +    aml_append(crs,
> > +               aml_interrupt(0x01, *rtc_irq + 32));
> > +    aml_append(dev, aml_name_decl("_CRS", crs));
> > +    aml_append(scope, dev);
> > +}
> > +
> > +static void acpi_dsdt_add_flash(Aml *scope, const hwaddr *flash_addr)
> > +{
> > +    Aml *dev, *crs;
> > +    hwaddr base = flash_addr[0];
> > +    hwaddr size = flash_addr[1];
> > +
> > +    dev = aml_device("FLS0");
> > +    aml_append(dev, aml_name_decl("_HID", aml_string("LNRO0015")));
> > +    aml_append(dev, aml_name_decl("_UID", aml_int(0)));
> > +
> > +    crs = aml_resource_template();
> > +    aml_append(crs,
> > +               aml_memory32_fixed(base, size, 0x01));
join it with line above it if it doesn't exceed 80 chr
same goes for other places.

> > +    aml_append(dev, aml_name_decl("_CRS", crs));
> > +    aml_append(scope, dev);
> > +
> > +    dev = aml_device("FLS1");
> > +    aml_append(dev, aml_name_decl("_HID", aml_string("LNRO0015")));
> > +    aml_append(dev, aml_name_decl("_UID", aml_int(1)));
> > +    crs = aml_resource_template();
> > +    aml_append(crs,
> > +               aml_memory32_fixed(base + size, size, 0x01));
> > +    aml_append(dev, aml_name_decl("_CRS", crs));
> > +    aml_append(scope, dev);
> > +}
> > +
> > +static void acpi_dsdt_add_virtio(Aml *scope, const hwaddr *mmio_addrs,
> > +                                             const int *mmio_irq, int num)
> > +{
> > +    Aml *dev, *crs;
> > +    hwaddr base = mmio_addrs[0];
> > +    hwaddr size = mmio_addrs[1];
> 
> What ensures all these hw addresses are in 32 bit space on 64 bit platforms?
> 
> > +    int irq = *mmio_irq + 32;
> > +    int i;
> > +    char name[5];
> > +
> > +    for (i = 0; i < num; i++) {
> > +        snprintf(name, 5, "VR%02u", i);
> > +        dev = aml_device("%s", name);
> 
> Again why not call aml_device directly?
> 
> > +        aml_append(dev, aml_name_decl("_HID", aml_string("LNRO0005")));
> > +        aml_append(dev, aml_name_decl("_UID", aml_int(i)));
> > +
> > +        crs = aml_resource_template();
> > +        aml_append(crs,
> > +                   aml_memory32_fixed(base, size, 0x01));
> > +        aml_append(crs,
> > +                   aml_interrupt(0x01, irq + i));
> > +        aml_append(dev, aml_name_decl("_CRS", crs));
> > +        aml_append(scope, dev);
> > +        base += size;
> > +    }
> > +}
> > +
> > +/* DSDT */
> > +static void
> > +build_dsdt(GArray *table_data, GArray *linker, VirtGuestInfo *guest_info)
> > +{
> > +    Aml *scope, *dsdt;
> > +    acpi_dsdt_info *info = guest_info->dsdt_info;
> > +
> > +    dsdt = init_aml_allocator();
> > +    /* Reserve space for header */
> > +    acpi_data_push(dsdt->buf, sizeof(AcpiTableHeader));
> > +
> > +    scope = aml_scope("\\_SB");
> > +    acpi_dsdt_add_cpus(scope, guest_info->max_cpus);
> > +    acpi_dsdt_add_uart(scope, info->uart_addr, info->uart_irq);
> > +    acpi_dsdt_add_rtc(scope, info->rtc_addr, info->rtc_irq);
> > +    acpi_dsdt_add_flash(scope, info->flash_addr);
> > +    acpi_dsdt_add_virtio(scope, info->virtio_mmio_addr,
> > +             info->virtio_mmio_irq, info->virtio_mmio_num);
> > +
and drop this newline

> > +    aml_append(dsdt, scope);
a newline here pls

> > +    /* copy AML table into ACPI tables blob and patch header there */
> > +    g_array_append_vals(table_data, dsdt->buf->data, dsdt->buf->len);
> > +    build_header(linker, table_data,
> > +        (void *)(table_data->data + table_data->len - dsdt->buf->len),
> > +        "DSDT", dsdt->buf->len, 1);
> > +    free_aml_allocator();
> > +}
> > +
> >  typedef
> >  struct AcpiBuildState {
> >      /* Copy of table in RAM (for patching). */
> > @@ -72,6 +205,7 @@ static
> >  void virt_acpi_build(VirtGuestInfo *guest_info, AcpiBuildTables *tables)
> >  {
> >      GArray *table_offsets;
> > +    GArray *tables_blob = tables->table_data;
> >  
> >      table_offsets = g_array_new(false, true /* clear */,
> >                                          sizeof(uint32_t));
> > @@ -89,6 +223,9 @@ void virt_acpi_build(VirtGuestInfo *guest_info, AcpiBuildTables *tables)
> >       * DSDT
> >       */
> >  
> > +    /* DSDT is pointed to by FADT */
> > +    build_dsdt(tables_blob, tables->linker, guest_info);
> > +
> >      /* Cleanup memory that's no longer used. */
> >      g_array_free(table_offsets, true);
> >  }
>
Shannon Zhao April 10, 2015, 5:57 a.m. UTC | #3
On 2015/4/9 17:51, Alex Bennée wrote:
> 
> Shannon Zhao <zhaoshenglong@huawei.com> writes:
> 
>> From: Shannon Zhao <shannon.zhao@linaro.org>
>>
>> DSDT consists of the usual common table header plus a definition
>> block in AML encoding which describes all devices in the platform.
>>
>> After initializing DSDT with header information the namespace is
>> created which is followed by the device encodings. The devices are
>> described using the Resource Template for the 32-Bit Fixed Memory
>> Range and the Extended Interrupt Descriptors.
>>
>> Signed-off-by: Shannon Zhao <zhaoshenglong@huawei.com>
>> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
>> ---
>>  hw/arm/virt-acpi-build.c | 137 +++++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 137 insertions(+)
>>
>> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
>> index 388838a..516c1d0 100644
>> --- a/hw/arm/virt-acpi-build.c
>> +++ b/hw/arm/virt-acpi-build.c
>> @@ -57,6 +57,139 @@
>>  #define ACPI_BUILD_DPRINTF(fmt, ...)
>>  #endif
>>  
>> +static void acpi_dsdt_add_cpus(Aml *scope, int max_cpus)
>> +{
>> +    Aml *dev, *crs;
>> +    int i;
>> +    char name[5];
> 
> name, dev and crs could be declared inside the for() loop.
> 
>> +    for (i = 0; i < max_cpus; i++) {
>> +        snprintf(name, 5, "CPU%u", i);
> 
> What happens here if you have 10 or 100 CPUs? 
> 
>> +        dev = aml_device("%s", name);
> 
> Also aml_device seems to take a format string so why not simply:
> 
>      dev = aml_device("CPU%u", i)
> 

Ok, it's better.

>> +        aml_append(dev, aml_name_decl("_HID", aml_string("ACPI0007")));
>> +        aml_append(dev, aml_name_decl("_UID", aml_int(i)));
>> +        crs = aml_resource_template();
>> +        aml_append(dev, aml_name_decl("_CRS", crs));
>> +        aml_append(scope, dev);
>> +    }
>> +}
>> +
>> +static void acpi_dsdt_add_uart(Aml *scope, const hwaddr *uart_addr,
>> +                                           const int *uart_irq)
>> +{
>> +    Aml *dev, *crs;
>> +
>> +    dev = aml_device("COM0");
>> +    aml_append(dev, aml_name_decl("_HID", aml_string("ARMH0011")));
>> +    aml_append(dev, aml_name_decl("_UID", aml_int(0)));
>> +
>> +    crs = aml_resource_template();
>> +    aml_append(crs,
>> +               aml_memory32_fixed(uart_addr[0], uart_addr[1], 0x01));
>> +    aml_append(crs,
>> +               aml_interrupt(0x01, *uart_irq + 32));
>> +    aml_append(dev, aml_name_decl("_CRS", crs));
>> +    aml_append(scope, dev);
>> +}
>> +
>> +static void acpi_dsdt_add_rtc(Aml *scope, const hwaddr *rtc_addr,
>> +                                          const int *rtc_irq)
>> +{
>> +    Aml *dev, *crs;
>> +
>> +    dev = aml_device("RTC0");
>> +    aml_append(dev, aml_name_decl("_HID", aml_string("LNRO0013")));
>> +    aml_append(dev, aml_name_decl("_UID", aml_int(0)));
>> +
>> +    crs = aml_resource_template();
>> +    aml_append(crs,
>> +               aml_memory32_fixed(rtc_addr[0], rtc_addr[1], 0x01));
>> +    aml_append(crs,
>> +               aml_interrupt(0x01, *rtc_irq + 32));
>> +    aml_append(dev, aml_name_decl("_CRS", crs));
>> +    aml_append(scope, dev);
>> +}
>> +
>> +static void acpi_dsdt_add_flash(Aml *scope, const hwaddr *flash_addr)
>> +{
>> +    Aml *dev, *crs;
>> +    hwaddr base = flash_addr[0];
>> +    hwaddr size = flash_addr[1];
>> +
>> +    dev = aml_device("FLS0");
>> +    aml_append(dev, aml_name_decl("_HID", aml_string("LNRO0015")));
>> +    aml_append(dev, aml_name_decl("_UID", aml_int(0)));
>> +
>> +    crs = aml_resource_template();
>> +    aml_append(crs,
>> +               aml_memory32_fixed(base, size, 0x01));
>> +    aml_append(dev, aml_name_decl("_CRS", crs));
>> +    aml_append(scope, dev);
>> +
>> +    dev = aml_device("FLS1");
>> +    aml_append(dev, aml_name_decl("_HID", aml_string("LNRO0015")));
>> +    aml_append(dev, aml_name_decl("_UID", aml_int(1)));
>> +    crs = aml_resource_template();
>> +    aml_append(crs,
>> +               aml_memory32_fixed(base + size, size, 0x01));
>> +    aml_append(dev, aml_name_decl("_CRS", crs));
>> +    aml_append(scope, dev);
>> +}
>> +
>> +static void acpi_dsdt_add_virtio(Aml *scope, const hwaddr *mmio_addrs,
>> +                                             const int *mmio_irq, int num)
>> +{
>> +    Aml *dev, *crs;
>> +    hwaddr base = mmio_addrs[0];
>> +    hwaddr size = mmio_addrs[1];
> 
> What ensures all these hw addresses are in 32 bit space on 64 bit platforms?
> 

As we're generating the ACPI tables for machine virt, from a15memmap[] we know the hw addresses are below 1GB.

>> +    int irq = *mmio_irq + 32;
>> +    int i;
>> +    char name[5];
>> +
>> +    for (i = 0; i < num; i++) {
>> +        snprintf(name, 5, "VR%02u", i);
>> +        dev = aml_device("%s", name);
> 
> Again why not call aml_device directly?
> 
>> +        aml_append(dev, aml_name_decl("_HID", aml_string("LNRO0005")));
>> +        aml_append(dev, aml_name_decl("_UID", aml_int(i)));
>> +
>> +        crs = aml_resource_template();
>> +        aml_append(crs,
>> +                   aml_memory32_fixed(base, size, 0x01));
>> +        aml_append(crs,
>> +                   aml_interrupt(0x01, irq + i));
>> +        aml_append(dev, aml_name_decl("_CRS", crs));
>> +        aml_append(scope, dev);
>> +        base += size;
>> +    }
>> +}
>> +
>> +/* DSDT */
>> +static void
>> +build_dsdt(GArray *table_data, GArray *linker, VirtGuestInfo *guest_info)
>> +{
>> +    Aml *scope, *dsdt;
>> +    acpi_dsdt_info *info = guest_info->dsdt_info;
>> +
>> +    dsdt = init_aml_allocator();
>> +    /* Reserve space for header */
>> +    acpi_data_push(dsdt->buf, sizeof(AcpiTableHeader));
>> +
>> +    scope = aml_scope("\\_SB");
>> +    acpi_dsdt_add_cpus(scope, guest_info->max_cpus);
>> +    acpi_dsdt_add_uart(scope, info->uart_addr, info->uart_irq);
>> +    acpi_dsdt_add_rtc(scope, info->rtc_addr, info->rtc_irq);
>> +    acpi_dsdt_add_flash(scope, info->flash_addr);
>> +    acpi_dsdt_add_virtio(scope, info->virtio_mmio_addr,
>> +             info->virtio_mmio_irq, info->virtio_mmio_num);
>> +
>> +    aml_append(dsdt, scope);
>> +    /* copy AML table into ACPI tables blob and patch header there */
>> +    g_array_append_vals(table_data, dsdt->buf->data, dsdt->buf->len);
>> +    build_header(linker, table_data,
>> +        (void *)(table_data->data + table_data->len - dsdt->buf->len),
>> +        "DSDT", dsdt->buf->len, 1);
>> +    free_aml_allocator();
>> +}
>> +
>>  typedef
>>  struct AcpiBuildState {
>>      /* Copy of table in RAM (for patching). */
>> @@ -72,6 +205,7 @@ static
>>  void virt_acpi_build(VirtGuestInfo *guest_info, AcpiBuildTables *tables)
>>  {
>>      GArray *table_offsets;
>> +    GArray *tables_blob = tables->table_data;
>>  
>>      table_offsets = g_array_new(false, true /* clear */,
>>                                          sizeof(uint32_t));
>> @@ -89,6 +223,9 @@ void virt_acpi_build(VirtGuestInfo *guest_info, AcpiBuildTables *tables)
>>       * DSDT
>>       */
>>  
>> +    /* DSDT is pointed to by FADT */
>> +    build_dsdt(tables_blob, tables->linker, guest_info);
>> +
>>      /* Cleanup memory that's no longer used. */
>>      g_array_free(table_offsets, true);
>>  }
>
Shannon Zhao April 10, 2015, 5:59 a.m. UTC | #4
On 2015/4/9 21:03, Igor Mammedov wrote:
> On Thu, 09 Apr 2015 10:51:33 +0100
> Alex Bennée <alex.bennee@linaro.org> wrote:
> 
>> > 
>> > Shannon Zhao <zhaoshenglong@huawei.com> writes:
>> > 
>>> > > From: Shannon Zhao <shannon.zhao@linaro.org>
>>> > >
>>> > > DSDT consists of the usual common table header plus a definition
>>> > > block in AML encoding which describes all devices in the platform.
>>> > >
>>> > > After initializing DSDT with header information the namespace is
>>> > > created which is followed by the device encodings. The devices are
>>> > > described using the Resource Template for the 32-Bit Fixed Memory
>>> > > Range and the Extended Interrupt Descriptors.
>>> > >
>>> > > Signed-off-by: Shannon Zhao <zhaoshenglong@huawei.com>
>>> > > Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
>>> > > ---
>>> > >  hw/arm/virt-acpi-build.c | 137 +++++++++++++++++++++++++++++++++++++++++++++++
>>> > >  1 file changed, 137 insertions(+)
>>> > >
>>> > > diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
>>> > > index 388838a..516c1d0 100644
>>> > > --- a/hw/arm/virt-acpi-build.c
>>> > > +++ b/hw/arm/virt-acpi-build.c
>>> > > @@ -57,6 +57,139 @@
>>> > >  #define ACPI_BUILD_DPRINTF(fmt, ...)
> I wonder if all this DPRINTF crap should be replaced by tracepoints.
> Oo at least it shouldn't be used in new code.
> 

Anything wrong with using DPRINTF?
Igor Mammedov April 10, 2015, 7:35 a.m. UTC | #5
On Fri, 10 Apr 2015 13:59:25 +0800
Shannon Zhao <zhaoshenglong@huawei.com> wrote:

> On 2015/4/9 21:03, Igor Mammedov wrote:
> > On Thu, 09 Apr 2015 10:51:33 +0100
> > Alex Bennée <alex.bennee@linaro.org> wrote:
> > 
> >> > 
> >> > Shannon Zhao <zhaoshenglong@huawei.com> writes:
> >> > 
> >>> > > From: Shannon Zhao <shannon.zhao@linaro.org>
> >>> > >
> >>> > > DSDT consists of the usual common table header plus a
> >>> > > definition block in AML encoding which describes all devices
> >>> > > in the platform.
> >>> > >
> >>> > > After initializing DSDT with header information the namespace
> >>> > > is created which is followed by the device encodings. The
> >>> > > devices are described using the Resource Template for the
> >>> > > 32-Bit Fixed Memory Range and the Extended Interrupt
> >>> > > Descriptors.
> >>> > >
> >>> > > Signed-off-by: Shannon Zhao <zhaoshenglong@huawei.com>
> >>> > > Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
> >>> > > ---
> >>> > >  hw/arm/virt-acpi-build.c | 137
> >>> > > +++++++++++++++++++++++++++++++++++++++++++++++ 1 file
> >>> > > changed, 137 insertions(+)
> >>> > >
> >>> > > diff --git a/hw/arm/virt-acpi-build.c
> >>> > > b/hw/arm/virt-acpi-build.c index 388838a..516c1d0 100644
> >>> > > --- a/hw/arm/virt-acpi-build.c
> >>> > > +++ b/hw/arm/virt-acpi-build.c
> >>> > > @@ -57,6 +57,139 @@
> >>> > >  #define ACPI_BUILD_DPRINTF(fmt, ...)
> > I wonder if all this DPRINTF crap should be replaced by tracepoints.
> > Oo at least it shouldn't be used in new code.
> > 
> 
> Anything wrong with using DPRINTF?
tracepoints could be enabled without rebuilding qemu
diff mbox

Patch

diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index 388838a..516c1d0 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -57,6 +57,139 @@ 
 #define ACPI_BUILD_DPRINTF(fmt, ...)
 #endif
 
+static void acpi_dsdt_add_cpus(Aml *scope, int max_cpus)
+{
+    Aml *dev, *crs;
+    int i;
+    char name[5];
+    for (i = 0; i < max_cpus; i++) {
+        snprintf(name, 5, "CPU%u", i);
+        dev = aml_device("%s", name);
+        aml_append(dev, aml_name_decl("_HID", aml_string("ACPI0007")));
+        aml_append(dev, aml_name_decl("_UID", aml_int(i)));
+        crs = aml_resource_template();
+        aml_append(dev, aml_name_decl("_CRS", crs));
+        aml_append(scope, dev);
+    }
+}
+
+static void acpi_dsdt_add_uart(Aml *scope, const hwaddr *uart_addr,
+                                           const int *uart_irq)
+{
+    Aml *dev, *crs;
+
+    dev = aml_device("COM0");
+    aml_append(dev, aml_name_decl("_HID", aml_string("ARMH0011")));
+    aml_append(dev, aml_name_decl("_UID", aml_int(0)));
+
+    crs = aml_resource_template();
+    aml_append(crs,
+               aml_memory32_fixed(uart_addr[0], uart_addr[1], 0x01));
+    aml_append(crs,
+               aml_interrupt(0x01, *uart_irq + 32));
+    aml_append(dev, aml_name_decl("_CRS", crs));
+    aml_append(scope, dev);
+}
+
+static void acpi_dsdt_add_rtc(Aml *scope, const hwaddr *rtc_addr,
+                                          const int *rtc_irq)
+{
+    Aml *dev, *crs;
+
+    dev = aml_device("RTC0");
+    aml_append(dev, aml_name_decl("_HID", aml_string("LNRO0013")));
+    aml_append(dev, aml_name_decl("_UID", aml_int(0)));
+
+    crs = aml_resource_template();
+    aml_append(crs,
+               aml_memory32_fixed(rtc_addr[0], rtc_addr[1], 0x01));
+    aml_append(crs,
+               aml_interrupt(0x01, *rtc_irq + 32));
+    aml_append(dev, aml_name_decl("_CRS", crs));
+    aml_append(scope, dev);
+}
+
+static void acpi_dsdt_add_flash(Aml *scope, const hwaddr *flash_addr)
+{
+    Aml *dev, *crs;
+    hwaddr base = flash_addr[0];
+    hwaddr size = flash_addr[1];
+
+    dev = aml_device("FLS0");
+    aml_append(dev, aml_name_decl("_HID", aml_string("LNRO0015")));
+    aml_append(dev, aml_name_decl("_UID", aml_int(0)));
+
+    crs = aml_resource_template();
+    aml_append(crs,
+               aml_memory32_fixed(base, size, 0x01));
+    aml_append(dev, aml_name_decl("_CRS", crs));
+    aml_append(scope, dev);
+
+    dev = aml_device("FLS1");
+    aml_append(dev, aml_name_decl("_HID", aml_string("LNRO0015")));
+    aml_append(dev, aml_name_decl("_UID", aml_int(1)));
+    crs = aml_resource_template();
+    aml_append(crs,
+               aml_memory32_fixed(base + size, size, 0x01));
+    aml_append(dev, aml_name_decl("_CRS", crs));
+    aml_append(scope, dev);
+}
+
+static void acpi_dsdt_add_virtio(Aml *scope, const hwaddr *mmio_addrs,
+                                             const int *mmio_irq, int num)
+{
+    Aml *dev, *crs;
+    hwaddr base = mmio_addrs[0];
+    hwaddr size = mmio_addrs[1];
+    int irq = *mmio_irq + 32;
+    int i;
+    char name[5];
+
+    for (i = 0; i < num; i++) {
+        snprintf(name, 5, "VR%02u", i);
+        dev = aml_device("%s", name);
+        aml_append(dev, aml_name_decl("_HID", aml_string("LNRO0005")));
+        aml_append(dev, aml_name_decl("_UID", aml_int(i)));
+
+        crs = aml_resource_template();
+        aml_append(crs,
+                   aml_memory32_fixed(base, size, 0x01));
+        aml_append(crs,
+                   aml_interrupt(0x01, irq + i));
+        aml_append(dev, aml_name_decl("_CRS", crs));
+        aml_append(scope, dev);
+        base += size;
+    }
+}
+
+/* DSDT */
+static void
+build_dsdt(GArray *table_data, GArray *linker, VirtGuestInfo *guest_info)
+{
+    Aml *scope, *dsdt;
+    acpi_dsdt_info *info = guest_info->dsdt_info;
+
+    dsdt = init_aml_allocator();
+    /* Reserve space for header */
+    acpi_data_push(dsdt->buf, sizeof(AcpiTableHeader));
+
+    scope = aml_scope("\\_SB");
+    acpi_dsdt_add_cpus(scope, guest_info->max_cpus);
+    acpi_dsdt_add_uart(scope, info->uart_addr, info->uart_irq);
+    acpi_dsdt_add_rtc(scope, info->rtc_addr, info->rtc_irq);
+    acpi_dsdt_add_flash(scope, info->flash_addr);
+    acpi_dsdt_add_virtio(scope, info->virtio_mmio_addr,
+             info->virtio_mmio_irq, info->virtio_mmio_num);
+
+    aml_append(dsdt, scope);
+    /* copy AML table into ACPI tables blob and patch header there */
+    g_array_append_vals(table_data, dsdt->buf->data, dsdt->buf->len);
+    build_header(linker, table_data,
+        (void *)(table_data->data + table_data->len - dsdt->buf->len),
+        "DSDT", dsdt->buf->len, 1);
+    free_aml_allocator();
+}
+
 typedef
 struct AcpiBuildState {
     /* Copy of table in RAM (for patching). */
@@ -72,6 +205,7 @@  static
 void virt_acpi_build(VirtGuestInfo *guest_info, AcpiBuildTables *tables)
 {
     GArray *table_offsets;
+    GArray *tables_blob = tables->table_data;
 
     table_offsets = g_array_new(false, true /* clear */,
                                         sizeof(uint32_t));
@@ -89,6 +223,9 @@  void virt_acpi_build(VirtGuestInfo *guest_info, AcpiBuildTables *tables)
      * DSDT
      */
 
+    /* DSDT is pointed to by FADT */
+    build_dsdt(tables_blob, tables->linker, guest_info);
+
     /* Cleanup memory that's no longer used. */
     g_array_free(table_offsets, true);
 }