diff mbox

[v5,19/20] hw/arm/virt-acpi-build: Add PCIe controller in ACPI DSDT table

Message ID 1429104309-3844-20-git-send-email-zhaoshenglong@huawei.com
State New
Headers show

Commit Message

Shannon Zhao April 15, 2015, 1:25 p.m. UTC
From: Shannon Zhao <shannon.zhao@linaro.org>

Add PCIe controller in ACPI DSDT table, so the guest can detect
the PCIe.

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

Comments

Igor Mammedov April 28, 2015, 8:42 a.m. UTC | #1
On Wed, 15 Apr 2015 21:25:08 +0800
Shannon Zhao <zhaoshenglong@huawei.com> wrote:

> From: Shannon Zhao <shannon.zhao@linaro.org>
> 
> Add PCIe controller in ACPI DSDT table, so the guest can detect
> the PCIe.
> 
> Signed-off-by: Shannon Zhao <zhaoshenglong@huawei.com>
> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
> ---
>  hw/arm/virt-acpi-build.c | 152 +++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 152 insertions(+)
> 
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index 85e8242..ceec405 100644
> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -49,6 +49,8 @@
>  #include "qapi/qmp/qint.h"
>  #include "qom/qom-qobject.h"
>  #include "exec/ram_addr.h"
> +#include "hw/pci/pcie_host.h"
> +#include "hw/pci/pci.h"
>  
>  typedef struct VirtAcpiCpuInfo {
>      DECLARE_BITMAP(found_cpus, VIRT_ACPI_CPU_ID_LIMIT);
> @@ -160,6 +162,154 @@ static void acpi_dsdt_add_virtio(Aml *scope, const MemMap *virtio_mmio_memmap,
>      }
>  }
>  
> +static void acpi_dsdt_add_pci(Aml *scope, AcpiPcieInfo *info)
> +{
> +    Aml *method, *crs, *ifctx, *UUID, *ifctx1, *elsectx, *buf;
> +    int i, bus_no;
> +    int irq = *info->pcie_irq + 32;
> +
> +    Aml *dev = aml_device("%s", "PCI0");
> +    aml_append(dev, aml_name_decl("_HID", aml_string("PNP0A08")));
> +    aml_append(dev, aml_name_decl("_CID", aml_string("PNP0A03")));
> +    aml_append(dev, aml_name_decl("_SEG", aml_int(0)));
> +    aml_append(dev, aml_name_decl("_BBN", aml_int(0)));
> +    aml_append(dev, aml_name_decl("_ADR", aml_int(0)));
> +    aml_append(dev, aml_name_decl("_UID", aml_string("PCI0")));
> +    aml_append(dev, aml_name_decl("_STR", aml_string("PCIe 0 Device")));
> +
> +    /* Declare the PCI Routing Table. */
> +    Aml *rt_pkg = aml_package(info->nr_pcie_buses * PCI_NUM_PINS);
> +    for (bus_no = 0; bus_no < info->nr_pcie_buses; bus_no++) {
> +        for (i = 0; i < PCI_NUM_PINS; i++) {
> +            int gsi = (i + bus_no) % PCI_NUM_PINS;
> +            Aml *pkg = aml_package(4);
> +            aml_append(pkg, aml_int((bus_no << 16) | 0xFFFF));
> +            aml_append(pkg, aml_int(i));
> +            aml_append(pkg, aml_name("GSI%d", gsi));
> +            aml_append(pkg, aml_int(0));
> +            aml_append(rt_pkg, pkg);
> +        }
> +    }
> +    aml_append(dev, aml_name_decl("_PRT", rt_pkg));
> +
> +    /* Create GSI link device */
> +    for (i = 0; i < PCI_NUM_PINS; i++) {
> +        Aml *dev_gsi = aml_device("GSI%d", i);
> +        aml_append(dev_gsi, aml_name_decl("_HID", aml_string("PNP0C0F")));
> +        aml_append(dev_gsi, aml_name_decl("_UID", aml_int(0)));
> +        crs = aml_resource_template();
> +        aml_append(crs,
> +                   aml_interrupt(aml_consumer, aml_level, aml_active_high,
> +                   aml_exclusive, aml_not_wake_capable, irq + i));
> +        aml_append(dev_gsi, aml_name_decl("_PRS", crs));
> +        crs = aml_resource_template();
> +        aml_append(crs,
> +                   aml_interrupt(aml_consumer, aml_level, aml_active_high,
> +                   aml_exclusive, aml_not_wake_capable, irq + i));
> +        aml_append(dev_gsi, aml_name_decl("_CRS", crs));
> +        method = aml_method("_SRS", 1);
> +        aml_append(dev_gsi, method);
> +        aml_append(dev, dev_gsi);
> +    }
> +
> +    method = aml_method("_CBA", 0);
> +    aml_append(method, aml_return(aml_int(info->pcie_ecam.addr)));
> +    aml_append(dev, method);
> +
> +    method = aml_method("_CRS", 0);
> +    Aml *rbuf = aml_resource_template();
> +    aml_append(rbuf,
> +        aml_word_bus_number(aml_min_fixed, aml_max_fixed, aml_pos_decode,
> +                            0x0000, 0x0000, info->nr_pcie_buses - 1,
> +                            0x0000, info->nr_pcie_buses));
> +    aml_append(rbuf,
> +        aml_dword_memory(aml_pos_decode, aml_min_fixed, aml_max_fixed,
> +                         aml_non_cacheable, aml_ReadWrite,
> +                         0x0000, info->pcie_mmio.addr,
> +                         info->pcie_mmio.addr + info->pcie_mmio.size - 1,
> +                         0x0000, info->pcie_mmio.size));
> +    aml_append(rbuf,
> +        aml_dword_io(aml_min_fixed, aml_max_fixed,
> +                     aml_pos_decode, aml_entire_range,
> +                     0x0000, 0x0000, info->pcie_ioport.size - 1,
> +                     info->pcie_ioport.addr, info->pcie_ioport.size));
> +
> +    aml_append(method, aml_name_decl("RBUF", rbuf));
> +    aml_append(method, aml_return(rbuf));
> +    aml_append(dev, method);
> +
> +    /* Declare an _OSC (OS Control Handoff) method */
> +    aml_append(dev, aml_name_decl("SUPP", aml_int(0)));
> +    aml_append(dev, aml_name_decl("CTRL", aml_int(0)));
> +    method = aml_method("_OSC", 4);
> +    aml_append(method,
> +        aml_create_dword_field(aml_arg(3), aml_int(0), "CDW1"));
> +
> +    /* PCI Firmware Specification 3.0
> +     * 4.5.1. _OSC Interface for PCI Host Bridge Devices
> +     * The _OSC interface for a PCI/PCI-X/PCI Express hierarchy is
> +     * identified by the Universal Unique IDentifier (UUID)
> +     * 33db4d5b-1ff7-401c-9657-7441c03dd766
> +     */
> +    UUID = aml_touuid("33DB4D5B-1FF7-401C-9657-7441C03DD766");
> +    ifctx = aml_if(aml_equal(aml_arg(0), UUID));
> +    aml_append(ifctx,
> +        aml_create_dword_field(aml_arg(3), aml_int(4), "CDW2"));
> +    aml_append(ifctx,
> +        aml_create_dword_field(aml_arg(3), aml_int(8), "CDW3"));
> +    aml_append(ifctx, aml_store(aml_name("CDW2"), aml_name("SUPP")));
> +    aml_append(ifctx, aml_store(aml_name("CDW3"), aml_name("CTRL")));
> +    aml_append(ifctx, aml_store(aml_and(aml_name("CTRL"), aml_int(0x1D)),
> +                                aml_name("CTRL")));
> +
> +    ifctx1 = aml_if(aml_not(aml_equal(aml_arg(1), aml_int(0x1))));
> +    aml_append(ifctx1, aml_store(aml_or(aml_name("CDW1"), aml_int(0x08)),
> +                                 aml_name("CDW1")));
> +    aml_append(ifctx, ifctx1);
> +
> +    ifctx1 = aml_if(aml_not(aml_equal(aml_name("CDW3"), aml_name("CTRL"))));
> +    aml_append(ifctx1, aml_store(aml_or(aml_name("CDW1"), aml_int(0x10)),
> +                                 aml_name("CDW1")));
> +    aml_append(ifctx, ifctx1);
> +
> +    aml_append(ifctx, aml_store(aml_name("CTRL"), aml_name("CDW3")));
> +    aml_append(ifctx, aml_return(aml_arg(3)));
> +    aml_append(method, ifctx);
> +
> +    elsectx = aml_else();
> +    aml_append(elsectx, aml_store(aml_or(aml_name("CDW1"), aml_int(4)),
> +                                  aml_name("CDW1")));
> +    aml_append(elsectx, aml_return(aml_arg(3)));
> +    aml_append(method, elsectx);
> +    aml_append(dev, method);
> +
> +    method = aml_method("_DSM", 4);
> +
> +    /* PCI Firmware Specification 3.0
> +     * 4.6.1. _DSM for PCI Express Slot Information
> +     * The UUID in _DSM in this context is
> +     * {E5C937D0-3553-4d7a-9117-EA4D19C3434D}
> +     */
> +    UUID = aml_touuid("E5C937D0-3553-4d7a-9117-EA4D19C3434D");
> +    ifctx = aml_if(aml_equal(aml_arg(0), UUID));
> +    ifctx1 = aml_if(aml_equal(aml_arg(2), aml_int(0)));
> +    buf = aml_buffer();
> +    build_append_int_noprefix(buf->buf, 0x01, 1);
pls, do not modify internal buffer of Aml objects outside API,
and do not export build_append_int_noprefix().

Looking at spec _DSM returns bitfield buffer.
for purposes you are using here i.e. set bit 1 to 1 or 0
it's sufficient to use aml_int(1) or aml_int(0) as they are
folded in OneOp /0x1/ and ZeroOp /0x0/ opcodes.

However if you need to manipulate specific bitfields checkout 
CreateBitField /ACPI5.0: 19.5.18 CreateBitField (Create 1-Bit Buffer Field)/
Probably that's what you are looking for.

Also aml_buffer() needs to be modified to handle BufferSize
see /ACPI5.0: 19.5.10 Buffer (Declare Buffer Object)/
so that space for bitfields could be reserved
/* as workaround/hack one can add several aml_int(0) into it to
reserve needed amount of bytes */


> +    aml_append(ifctx1, aml_return(buf));
> +    aml_append(ifctx, ifctx1);
> +    aml_append(method, ifctx);
> +
> +    buf = aml_buffer();
> +    build_append_int_noprefix(buf->buf, 0x00, 1);
> +    aml_append(method, aml_return(buf));
> +    aml_append(dev, method);
> +
> +    Aml *dev_rp0 = aml_device("%s", "RP0");
> +    aml_append(dev_rp0, aml_name_decl("_ADR", aml_int(0)));
> +    aml_append(dev, dev_rp0);
> +    aml_append(scope, dev);
> +}
> +
>  /* RSDP */
>  static GArray *
>  build_rsdp(GArray *rsdp_table, GArray *linker, unsigned rsdt)
> @@ -318,6 +468,8 @@ build_dsdt(GArray *table_data, GArray *linker, VirtGuestInfo *guest_info)
>      acpi_dsdt_add_flash(scope, info->flash_memmap);
>      acpi_dsdt_add_virtio(scope, info->virtio_mmio_memmap,
>               info->virtio_mmio_irq, info->virtio_mmio_num);
> +    acpi_dsdt_add_pci(scope, guest_info->pcie_info);
> +
>      aml_append(dsdt, scope);
>  
>      /* copy AML table into ACPI tables blob and patch header there */
Michael S. Tsirkin April 28, 2015, 8:47 a.m. UTC | #2
On Tue, Apr 28, 2015 at 10:42:25AM +0200, Igor Mammedov wrote:
> On Wed, 15 Apr 2015 21:25:08 +0800
> Shannon Zhao <zhaoshenglong@huawei.com> wrote:
> 
> > From: Shannon Zhao <shannon.zhao@linaro.org>
> > 
> > Add PCIe controller in ACPI DSDT table, so the guest can detect
> > the PCIe.
> > 
> > Signed-off-by: Shannon Zhao <zhaoshenglong@huawei.com>
> > Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
> > ---
> >  hw/arm/virt-acpi-build.c | 152 +++++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 152 insertions(+)
> > 
> > diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> > index 85e8242..ceec405 100644
> > --- a/hw/arm/virt-acpi-build.c
> > +++ b/hw/arm/virt-acpi-build.c
> > @@ -49,6 +49,8 @@
> >  #include "qapi/qmp/qint.h"
> >  #include "qom/qom-qobject.h"
> >  #include "exec/ram_addr.h"
> > +#include "hw/pci/pcie_host.h"
> > +#include "hw/pci/pci.h"
> >  
> >  typedef struct VirtAcpiCpuInfo {
> >      DECLARE_BITMAP(found_cpus, VIRT_ACPI_CPU_ID_LIMIT);
> > @@ -160,6 +162,154 @@ static void acpi_dsdt_add_virtio(Aml *scope, const MemMap *virtio_mmio_memmap,
> >      }
> >  }
> >  
> > +static void acpi_dsdt_add_pci(Aml *scope, AcpiPcieInfo *info)
> > +{
> > +    Aml *method, *crs, *ifctx, *UUID, *ifctx1, *elsectx, *buf;
> > +    int i, bus_no;
> > +    int irq = *info->pcie_irq + 32;
> > +
> > +    Aml *dev = aml_device("%s", "PCI0");
> > +    aml_append(dev, aml_name_decl("_HID", aml_string("PNP0A08")));
> > +    aml_append(dev, aml_name_decl("_CID", aml_string("PNP0A03")));
> > +    aml_append(dev, aml_name_decl("_SEG", aml_int(0)));
> > +    aml_append(dev, aml_name_decl("_BBN", aml_int(0)));
> > +    aml_append(dev, aml_name_decl("_ADR", aml_int(0)));
> > +    aml_append(dev, aml_name_decl("_UID", aml_string("PCI0")));
> > +    aml_append(dev, aml_name_decl("_STR", aml_string("PCIe 0 Device")));
> > +
> > +    /* Declare the PCI Routing Table. */
> > +    Aml *rt_pkg = aml_package(info->nr_pcie_buses * PCI_NUM_PINS);
> > +    for (bus_no = 0; bus_no < info->nr_pcie_buses; bus_no++) {
> > +        for (i = 0; i < PCI_NUM_PINS; i++) {
> > +            int gsi = (i + bus_no) % PCI_NUM_PINS;
> > +            Aml *pkg = aml_package(4);
> > +            aml_append(pkg, aml_int((bus_no << 16) | 0xFFFF));
> > +            aml_append(pkg, aml_int(i));
> > +            aml_append(pkg, aml_name("GSI%d", gsi));
> > +            aml_append(pkg, aml_int(0));
> > +            aml_append(rt_pkg, pkg);
> > +        }
> > +    }
> > +    aml_append(dev, aml_name_decl("_PRT", rt_pkg));
> > +
> > +    /* Create GSI link device */
> > +    for (i = 0; i < PCI_NUM_PINS; i++) {
> > +        Aml *dev_gsi = aml_device("GSI%d", i);
> > +        aml_append(dev_gsi, aml_name_decl("_HID", aml_string("PNP0C0F")));
> > +        aml_append(dev_gsi, aml_name_decl("_UID", aml_int(0)));
> > +        crs = aml_resource_template();
> > +        aml_append(crs,
> > +                   aml_interrupt(aml_consumer, aml_level, aml_active_high,
> > +                   aml_exclusive, aml_not_wake_capable, irq + i));
> > +        aml_append(dev_gsi, aml_name_decl("_PRS", crs));
> > +        crs = aml_resource_template();
> > +        aml_append(crs,
> > +                   aml_interrupt(aml_consumer, aml_level, aml_active_high,
> > +                   aml_exclusive, aml_not_wake_capable, irq + i));
> > +        aml_append(dev_gsi, aml_name_decl("_CRS", crs));
> > +        method = aml_method("_SRS", 1);
> > +        aml_append(dev_gsi, method);
> > +        aml_append(dev, dev_gsi);
> > +    }
> > +
> > +    method = aml_method("_CBA", 0);
> > +    aml_append(method, aml_return(aml_int(info->pcie_ecam.addr)));
> > +    aml_append(dev, method);
> > +
> > +    method = aml_method("_CRS", 0);
> > +    Aml *rbuf = aml_resource_template();
> > +    aml_append(rbuf,
> > +        aml_word_bus_number(aml_min_fixed, aml_max_fixed, aml_pos_decode,
> > +                            0x0000, 0x0000, info->nr_pcie_buses - 1,
> > +                            0x0000, info->nr_pcie_buses));
> > +    aml_append(rbuf,
> > +        aml_dword_memory(aml_pos_decode, aml_min_fixed, aml_max_fixed,
> > +                         aml_non_cacheable, aml_ReadWrite,
> > +                         0x0000, info->pcie_mmio.addr,
> > +                         info->pcie_mmio.addr + info->pcie_mmio.size - 1,
> > +                         0x0000, info->pcie_mmio.size));
> > +    aml_append(rbuf,
> > +        aml_dword_io(aml_min_fixed, aml_max_fixed,
> > +                     aml_pos_decode, aml_entire_range,
> > +                     0x0000, 0x0000, info->pcie_ioport.size - 1,
> > +                     info->pcie_ioport.addr, info->pcie_ioport.size));
> > +
> > +    aml_append(method, aml_name_decl("RBUF", rbuf));
> > +    aml_append(method, aml_return(rbuf));
> > +    aml_append(dev, method);
> > +
> > +    /* Declare an _OSC (OS Control Handoff) method */
> > +    aml_append(dev, aml_name_decl("SUPP", aml_int(0)));
> > +    aml_append(dev, aml_name_decl("CTRL", aml_int(0)));
> > +    method = aml_method("_OSC", 4);
> > +    aml_append(method,
> > +        aml_create_dword_field(aml_arg(3), aml_int(0), "CDW1"));
> > +
> > +    /* PCI Firmware Specification 3.0
> > +     * 4.5.1. _OSC Interface for PCI Host Bridge Devices
> > +     * The _OSC interface for a PCI/PCI-X/PCI Express hierarchy is
> > +     * identified by the Universal Unique IDentifier (UUID)
> > +     * 33db4d5b-1ff7-401c-9657-7441c03dd766
> > +     */
> > +    UUID = aml_touuid("33DB4D5B-1FF7-401C-9657-7441C03DD766");
> > +    ifctx = aml_if(aml_equal(aml_arg(0), UUID));
> > +    aml_append(ifctx,
> > +        aml_create_dword_field(aml_arg(3), aml_int(4), "CDW2"));
> > +    aml_append(ifctx,
> > +        aml_create_dword_field(aml_arg(3), aml_int(8), "CDW3"));
> > +    aml_append(ifctx, aml_store(aml_name("CDW2"), aml_name("SUPP")));
> > +    aml_append(ifctx, aml_store(aml_name("CDW3"), aml_name("CTRL")));
> > +    aml_append(ifctx, aml_store(aml_and(aml_name("CTRL"), aml_int(0x1D)),
> > +                                aml_name("CTRL")));
> > +
> > +    ifctx1 = aml_if(aml_not(aml_equal(aml_arg(1), aml_int(0x1))));
> > +    aml_append(ifctx1, aml_store(aml_or(aml_name("CDW1"), aml_int(0x08)),
> > +                                 aml_name("CDW1")));
> > +    aml_append(ifctx, ifctx1);
> > +
> > +    ifctx1 = aml_if(aml_not(aml_equal(aml_name("CDW3"), aml_name("CTRL"))));
> > +    aml_append(ifctx1, aml_store(aml_or(aml_name("CDW1"), aml_int(0x10)),
> > +                                 aml_name("CDW1")));
> > +    aml_append(ifctx, ifctx1);
> > +
> > +    aml_append(ifctx, aml_store(aml_name("CTRL"), aml_name("CDW3")));
> > +    aml_append(ifctx, aml_return(aml_arg(3)));
> > +    aml_append(method, ifctx);
> > +
> > +    elsectx = aml_else();
> > +    aml_append(elsectx, aml_store(aml_or(aml_name("CDW1"), aml_int(4)),
> > +                                  aml_name("CDW1")));
> > +    aml_append(elsectx, aml_return(aml_arg(3)));
> > +    aml_append(method, elsectx);
> > +    aml_append(dev, method);
> > +
> > +    method = aml_method("_DSM", 4);
> > +
> > +    /* PCI Firmware Specification 3.0
> > +     * 4.6.1. _DSM for PCI Express Slot Information
> > +     * The UUID in _DSM in this context is
> > +     * {E5C937D0-3553-4d7a-9117-EA4D19C3434D}
> > +     */
> > +    UUID = aml_touuid("E5C937D0-3553-4d7a-9117-EA4D19C3434D");
> > +    ifctx = aml_if(aml_equal(aml_arg(0), UUID));
> > +    ifctx1 = aml_if(aml_equal(aml_arg(2), aml_int(0)));
> > +    buf = aml_buffer();
> > +    build_append_int_noprefix(buf->buf, 0x01, 1);
> pls, do not modify internal buffer of Aml objects outside API,
> and do not export build_append_int_noprefix().
> 
> Looking at spec _DSM returns bitfield buffer.
> for purposes you are using here i.e. set bit 1 to 1 or 0
> it's sufficient to use aml_int(1) or aml_int(0) as they are
> folded in OneOp /0x1/ and ZeroOp /0x0/ opcodes.
> 
> However if you need to manipulate specific bitfields checkout 
> CreateBitField /ACPI5.0: 19.5.18 CreateBitField (Create 1-Bit Buffer Field)/
> Probably that's what you are looking for.
> 
> Also aml_buffer() needs to be modified to handle BufferSize
> see /ACPI5.0: 19.5.10 Buffer (Declare Buffer Object)/
> so that space for bitfields could be reserved
> /* as workaround/hack one can add several aml_int(0) into it to
> reserve needed amount of bytes */

I dislike abusing aml_int like this, makes code hard to follow.
Best just do it properly.
Internally you can call append_byte.

> 
> > +    aml_append(ifctx1, aml_return(buf));
> > +    aml_append(ifctx, ifctx1);
> > +    aml_append(method, ifctx);
> > +
> > +    buf = aml_buffer();
> > +    build_append_int_noprefix(buf->buf, 0x00, 1);
> > +    aml_append(method, aml_return(buf));
> > +    aml_append(dev, method);
> > +
> > +    Aml *dev_rp0 = aml_device("%s", "RP0");
> > +    aml_append(dev_rp0, aml_name_decl("_ADR", aml_int(0)));
> > +    aml_append(dev, dev_rp0);
> > +    aml_append(scope, dev);
> > +}
> > +
> >  /* RSDP */
> >  static GArray *
> >  build_rsdp(GArray *rsdp_table, GArray *linker, unsigned rsdt)
> > @@ -318,6 +468,8 @@ build_dsdt(GArray *table_data, GArray *linker, VirtGuestInfo *guest_info)
> >      acpi_dsdt_add_flash(scope, info->flash_memmap);
> >      acpi_dsdt_add_virtio(scope, info->virtio_mmio_memmap,
> >               info->virtio_mmio_irq, info->virtio_mmio_num);
> > +    acpi_dsdt_add_pci(scope, guest_info->pcie_info);
> > +
> >      aml_append(dsdt, scope);
> >  
> >      /* copy AML table into ACPI tables blob and patch header there */
Shannon Zhao April 28, 2015, 9:06 a.m. UTC | #3
On 2015/4/28 16:47, Michael S. Tsirkin wrote:
> On Tue, Apr 28, 2015 at 10:42:25AM +0200, Igor Mammedov wrote:
>> On Wed, 15 Apr 2015 21:25:08 +0800
>> Shannon Zhao <zhaoshenglong@huawei.com> wrote:
>>
>>> From: Shannon Zhao <shannon.zhao@linaro.org>
>>>
>>> Add PCIe controller in ACPI DSDT table, so the guest can detect
>>> the PCIe.
>>>
>>> Signed-off-by: Shannon Zhao <zhaoshenglong@huawei.com>
>>> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
>>> ---
>>>  hw/arm/virt-acpi-build.c | 152 +++++++++++++++++++++++++++++++++++++++++++++++
>>>  1 file changed, 152 insertions(+)
>>>
>>> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
>>> index 85e8242..ceec405 100644
>>> --- a/hw/arm/virt-acpi-build.c
>>> +++ b/hw/arm/virt-acpi-build.c
>>> @@ -49,6 +49,8 @@
>>>  #include "qapi/qmp/qint.h"
>>>  #include "qom/qom-qobject.h"
>>>  #include "exec/ram_addr.h"
>>> +#include "hw/pci/pcie_host.h"
>>> +#include "hw/pci/pci.h"
>>>  
>>>  typedef struct VirtAcpiCpuInfo {
>>>      DECLARE_BITMAP(found_cpus, VIRT_ACPI_CPU_ID_LIMIT);
>>> @@ -160,6 +162,154 @@ static void acpi_dsdt_add_virtio(Aml *scope, const MemMap *virtio_mmio_memmap,
>>>      }
>>>  }
>>>  
>>> +static void acpi_dsdt_add_pci(Aml *scope, AcpiPcieInfo *info)
>>> +{
>>> +    Aml *method, *crs, *ifctx, *UUID, *ifctx1, *elsectx, *buf;
>>> +    int i, bus_no;
>>> +    int irq = *info->pcie_irq + 32;
>>> +
>>> +    Aml *dev = aml_device("%s", "PCI0");
>>> +    aml_append(dev, aml_name_decl("_HID", aml_string("PNP0A08")));
>>> +    aml_append(dev, aml_name_decl("_CID", aml_string("PNP0A03")));
>>> +    aml_append(dev, aml_name_decl("_SEG", aml_int(0)));
>>> +    aml_append(dev, aml_name_decl("_BBN", aml_int(0)));
>>> +    aml_append(dev, aml_name_decl("_ADR", aml_int(0)));
>>> +    aml_append(dev, aml_name_decl("_UID", aml_string("PCI0")));
>>> +    aml_append(dev, aml_name_decl("_STR", aml_string("PCIe 0 Device")));
>>> +
>>> +    /* Declare the PCI Routing Table. */
>>> +    Aml *rt_pkg = aml_package(info->nr_pcie_buses * PCI_NUM_PINS);
>>> +    for (bus_no = 0; bus_no < info->nr_pcie_buses; bus_no++) {
>>> +        for (i = 0; i < PCI_NUM_PINS; i++) {
>>> +            int gsi = (i + bus_no) % PCI_NUM_PINS;
>>> +            Aml *pkg = aml_package(4);
>>> +            aml_append(pkg, aml_int((bus_no << 16) | 0xFFFF));
>>> +            aml_append(pkg, aml_int(i));
>>> +            aml_append(pkg, aml_name("GSI%d", gsi));
>>> +            aml_append(pkg, aml_int(0));
>>> +            aml_append(rt_pkg, pkg);
>>> +        }
>>> +    }
>>> +    aml_append(dev, aml_name_decl("_PRT", rt_pkg));
>>> +
>>> +    /* Create GSI link device */
>>> +    for (i = 0; i < PCI_NUM_PINS; i++) {
>>> +        Aml *dev_gsi = aml_device("GSI%d", i);
>>> +        aml_append(dev_gsi, aml_name_decl("_HID", aml_string("PNP0C0F")));
>>> +        aml_append(dev_gsi, aml_name_decl("_UID", aml_int(0)));
>>> +        crs = aml_resource_template();
>>> +        aml_append(crs,
>>> +                   aml_interrupt(aml_consumer, aml_level, aml_active_high,
>>> +                   aml_exclusive, aml_not_wake_capable, irq + i));
>>> +        aml_append(dev_gsi, aml_name_decl("_PRS", crs));
>>> +        crs = aml_resource_template();
>>> +        aml_append(crs,
>>> +                   aml_interrupt(aml_consumer, aml_level, aml_active_high,
>>> +                   aml_exclusive, aml_not_wake_capable, irq + i));
>>> +        aml_append(dev_gsi, aml_name_decl("_CRS", crs));
>>> +        method = aml_method("_SRS", 1);
>>> +        aml_append(dev_gsi, method);
>>> +        aml_append(dev, dev_gsi);
>>> +    }
>>> +
>>> +    method = aml_method("_CBA", 0);
>>> +    aml_append(method, aml_return(aml_int(info->pcie_ecam.addr)));
>>> +    aml_append(dev, method);
>>> +
>>> +    method = aml_method("_CRS", 0);
>>> +    Aml *rbuf = aml_resource_template();
>>> +    aml_append(rbuf,
>>> +        aml_word_bus_number(aml_min_fixed, aml_max_fixed, aml_pos_decode,
>>> +                            0x0000, 0x0000, info->nr_pcie_buses - 1,
>>> +                            0x0000, info->nr_pcie_buses));
>>> +    aml_append(rbuf,
>>> +        aml_dword_memory(aml_pos_decode, aml_min_fixed, aml_max_fixed,
>>> +                         aml_non_cacheable, aml_ReadWrite,
>>> +                         0x0000, info->pcie_mmio.addr,
>>> +                         info->pcie_mmio.addr + info->pcie_mmio.size - 1,
>>> +                         0x0000, info->pcie_mmio.size));
>>> +    aml_append(rbuf,
>>> +        aml_dword_io(aml_min_fixed, aml_max_fixed,
>>> +                     aml_pos_decode, aml_entire_range,
>>> +                     0x0000, 0x0000, info->pcie_ioport.size - 1,
>>> +                     info->pcie_ioport.addr, info->pcie_ioport.size));
>>> +
>>> +    aml_append(method, aml_name_decl("RBUF", rbuf));
>>> +    aml_append(method, aml_return(rbuf));
>>> +    aml_append(dev, method);
>>> +
>>> +    /* Declare an _OSC (OS Control Handoff) method */
>>> +    aml_append(dev, aml_name_decl("SUPP", aml_int(0)));
>>> +    aml_append(dev, aml_name_decl("CTRL", aml_int(0)));
>>> +    method = aml_method("_OSC", 4);
>>> +    aml_append(method,
>>> +        aml_create_dword_field(aml_arg(3), aml_int(0), "CDW1"));
>>> +
>>> +    /* PCI Firmware Specification 3.0
>>> +     * 4.5.1. _OSC Interface for PCI Host Bridge Devices
>>> +     * The _OSC interface for a PCI/PCI-X/PCI Express hierarchy is
>>> +     * identified by the Universal Unique IDentifier (UUID)
>>> +     * 33db4d5b-1ff7-401c-9657-7441c03dd766
>>> +     */
>>> +    UUID = aml_touuid("33DB4D5B-1FF7-401C-9657-7441C03DD766");
>>> +    ifctx = aml_if(aml_equal(aml_arg(0), UUID));
>>> +    aml_append(ifctx,
>>> +        aml_create_dword_field(aml_arg(3), aml_int(4), "CDW2"));
>>> +    aml_append(ifctx,
>>> +        aml_create_dword_field(aml_arg(3), aml_int(8), "CDW3"));
>>> +    aml_append(ifctx, aml_store(aml_name("CDW2"), aml_name("SUPP")));
>>> +    aml_append(ifctx, aml_store(aml_name("CDW3"), aml_name("CTRL")));
>>> +    aml_append(ifctx, aml_store(aml_and(aml_name("CTRL"), aml_int(0x1D)),
>>> +                                aml_name("CTRL")));
>>> +
>>> +    ifctx1 = aml_if(aml_not(aml_equal(aml_arg(1), aml_int(0x1))));
>>> +    aml_append(ifctx1, aml_store(aml_or(aml_name("CDW1"), aml_int(0x08)),
>>> +                                 aml_name("CDW1")));
>>> +    aml_append(ifctx, ifctx1);
>>> +
>>> +    ifctx1 = aml_if(aml_not(aml_equal(aml_name("CDW3"), aml_name("CTRL"))));
>>> +    aml_append(ifctx1, aml_store(aml_or(aml_name("CDW1"), aml_int(0x10)),
>>> +                                 aml_name("CDW1")));
>>> +    aml_append(ifctx, ifctx1);
>>> +
>>> +    aml_append(ifctx, aml_store(aml_name("CTRL"), aml_name("CDW3")));
>>> +    aml_append(ifctx, aml_return(aml_arg(3)));
>>> +    aml_append(method, ifctx);
>>> +
>>> +    elsectx = aml_else();
>>> +    aml_append(elsectx, aml_store(aml_or(aml_name("CDW1"), aml_int(4)),
>>> +                                  aml_name("CDW1")));
>>> +    aml_append(elsectx, aml_return(aml_arg(3)));
>>> +    aml_append(method, elsectx);
>>> +    aml_append(dev, method);
>>> +
>>> +    method = aml_method("_DSM", 4);
>>> +
>>> +    /* PCI Firmware Specification 3.0
>>> +     * 4.6.1. _DSM for PCI Express Slot Information
>>> +     * The UUID in _DSM in this context is
>>> +     * {E5C937D0-3553-4d7a-9117-EA4D19C3434D}
>>> +     */
>>> +    UUID = aml_touuid("E5C937D0-3553-4d7a-9117-EA4D19C3434D");
>>> +    ifctx = aml_if(aml_equal(aml_arg(0), UUID));
>>> +    ifctx1 = aml_if(aml_equal(aml_arg(2), aml_int(0)));
>>> +    buf = aml_buffer();
>>> +    build_append_int_noprefix(buf->buf, 0x01, 1);
>> pls, do not modify internal buffer of Aml objects outside API,
>> and do not export build_append_int_noprefix().
>>
>> Looking at spec _DSM returns bitfield buffer.
>> for purposes you are using here i.e. set bit 1 to 1 or 0
>> it's sufficient to use aml_int(1) or aml_int(0) as they are
>> folded in OneOp /0x1/ and ZeroOp /0x0/ opcodes.
>>
>> However if you need to manipulate specific bitfields checkout 
>> CreateBitField /ACPI5.0: 19.5.18 CreateBitField (Create 1-Bit Buffer Field)/
>> Probably that's what you are looking for.
>>
>> Also aml_buffer() needs to be modified to handle BufferSize
>> see /ACPI5.0: 19.5.10 Buffer (Declare Buffer Object)/
>> so that space for bitfields could be reserved
>> /* as workaround/hack one can add several aml_int(0) into it to
>> reserve needed amount of bytes */
> 
> I dislike abusing aml_int like this, makes code hard to follow.
> Best just do it properly.
> Internally you can call append_byte.
> 

I agree we could use append_byte to reserve BufferSize of bytes.
But we still need a function to initialize the buffer. A new function
wraps build_append_int_noprefix()?

>>
>>> +    aml_append(ifctx1, aml_return(buf));
>>> +    aml_append(ifctx, ifctx1);
>>> +    aml_append(method, ifctx);
>>> +
>>> +    buf = aml_buffer();
>>> +    build_append_int_noprefix(buf->buf, 0x00, 1);
>>> +    aml_append(method, aml_return(buf));
>>> +    aml_append(dev, method);
>>> +
>>> +    Aml *dev_rp0 = aml_device("%s", "RP0");
>>> +    aml_append(dev_rp0, aml_name_decl("_ADR", aml_int(0)));
>>> +    aml_append(dev, dev_rp0);
>>> +    aml_append(scope, dev);
>>> +}
>>> +
>>>  /* RSDP */
>>>  static GArray *
>>>  build_rsdp(GArray *rsdp_table, GArray *linker, unsigned rsdt)
>>> @@ -318,6 +468,8 @@ build_dsdt(GArray *table_data, GArray *linker, VirtGuestInfo *guest_info)
>>>      acpi_dsdt_add_flash(scope, info->flash_memmap);
>>>      acpi_dsdt_add_virtio(scope, info->virtio_mmio_memmap,
>>>               info->virtio_mmio_irq, info->virtio_mmio_num);
>>> +    acpi_dsdt_add_pci(scope, guest_info->pcie_info);
>>> +
>>>      aml_append(dsdt, scope);
>>>  
>>>      /* copy AML table into ACPI tables blob and patch header there */
> 
> .
>
Igor Mammedov April 28, 2015, 9:54 a.m. UTC | #4
On Tue, 28 Apr 2015 17:06:00 +0800
Shannon Zhao <zhaoshenglong@huawei.com> wrote:

> On 2015/4/28 16:47, Michael S. Tsirkin wrote:
> > On Tue, Apr 28, 2015 at 10:42:25AM +0200, Igor Mammedov wrote:
> >> On Wed, 15 Apr 2015 21:25:08 +0800
> >> Shannon Zhao <zhaoshenglong@huawei.com> wrote:
> >>
> >>> From: Shannon Zhao <shannon.zhao@linaro.org>
> >>>
> >>> Add PCIe controller in ACPI DSDT table, so the guest can detect
> >>> the PCIe.
> >>>
> >>> Signed-off-by: Shannon Zhao <zhaoshenglong@huawei.com>
> >>> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
> >>> ---
> >>>  hw/arm/virt-acpi-build.c | 152 +++++++++++++++++++++++++++++++++++++++++++++++
> >>>  1 file changed, 152 insertions(+)
> >>>
> >>> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> >>> index 85e8242..ceec405 100644
> >>> --- a/hw/arm/virt-acpi-build.c
> >>> +++ b/hw/arm/virt-acpi-build.c
> >>> @@ -49,6 +49,8 @@
> >>>  #include "qapi/qmp/qint.h"
> >>>  #include "qom/qom-qobject.h"
> >>>  #include "exec/ram_addr.h"
> >>> +#include "hw/pci/pcie_host.h"
> >>> +#include "hw/pci/pci.h"
> >>>  
> >>>  typedef struct VirtAcpiCpuInfo {
> >>>      DECLARE_BITMAP(found_cpus, VIRT_ACPI_CPU_ID_LIMIT);
> >>> @@ -160,6 +162,154 @@ static void acpi_dsdt_add_virtio(Aml *scope, const MemMap *virtio_mmio_memmap,
> >>>      }
> >>>  }
> >>>  
> >>> +static void acpi_dsdt_add_pci(Aml *scope, AcpiPcieInfo *info)
> >>> +{
> >>> +    Aml *method, *crs, *ifctx, *UUID, *ifctx1, *elsectx, *buf;
> >>> +    int i, bus_no;
> >>> +    int irq = *info->pcie_irq + 32;
> >>> +
> >>> +    Aml *dev = aml_device("%s", "PCI0");
> >>> +    aml_append(dev, aml_name_decl("_HID", aml_string("PNP0A08")));
> >>> +    aml_append(dev, aml_name_decl("_CID", aml_string("PNP0A03")));
> >>> +    aml_append(dev, aml_name_decl("_SEG", aml_int(0)));
> >>> +    aml_append(dev, aml_name_decl("_BBN", aml_int(0)));
> >>> +    aml_append(dev, aml_name_decl("_ADR", aml_int(0)));
> >>> +    aml_append(dev, aml_name_decl("_UID", aml_string("PCI0")));
> >>> +    aml_append(dev, aml_name_decl("_STR", aml_string("PCIe 0 Device")));
> >>> +
> >>> +    /* Declare the PCI Routing Table. */
> >>> +    Aml *rt_pkg = aml_package(info->nr_pcie_buses * PCI_NUM_PINS);
> >>> +    for (bus_no = 0; bus_no < info->nr_pcie_buses; bus_no++) {
> >>> +        for (i = 0; i < PCI_NUM_PINS; i++) {
> >>> +            int gsi = (i + bus_no) % PCI_NUM_PINS;
> >>> +            Aml *pkg = aml_package(4);
> >>> +            aml_append(pkg, aml_int((bus_no << 16) | 0xFFFF));
> >>> +            aml_append(pkg, aml_int(i));
> >>> +            aml_append(pkg, aml_name("GSI%d", gsi));
> >>> +            aml_append(pkg, aml_int(0));
> >>> +            aml_append(rt_pkg, pkg);
> >>> +        }
> >>> +    }
> >>> +    aml_append(dev, aml_name_decl("_PRT", rt_pkg));
> >>> +
> >>> +    /* Create GSI link device */
> >>> +    for (i = 0; i < PCI_NUM_PINS; i++) {
> >>> +        Aml *dev_gsi = aml_device("GSI%d", i);
> >>> +        aml_append(dev_gsi, aml_name_decl("_HID", aml_string("PNP0C0F")));
> >>> +        aml_append(dev_gsi, aml_name_decl("_UID", aml_int(0)));
> >>> +        crs = aml_resource_template();
> >>> +        aml_append(crs,
> >>> +                   aml_interrupt(aml_consumer, aml_level, aml_active_high,
> >>> +                   aml_exclusive, aml_not_wake_capable, irq + i));
> >>> +        aml_append(dev_gsi, aml_name_decl("_PRS", crs));
> >>> +        crs = aml_resource_template();
> >>> +        aml_append(crs,
> >>> +                   aml_interrupt(aml_consumer, aml_level, aml_active_high,
> >>> +                   aml_exclusive, aml_not_wake_capable, irq + i));
> >>> +        aml_append(dev_gsi, aml_name_decl("_CRS", crs));
> >>> +        method = aml_method("_SRS", 1);
> >>> +        aml_append(dev_gsi, method);
> >>> +        aml_append(dev, dev_gsi);
> >>> +    }
> >>> +
> >>> +    method = aml_method("_CBA", 0);
> >>> +    aml_append(method, aml_return(aml_int(info->pcie_ecam.addr)));
> >>> +    aml_append(dev, method);
> >>> +
> >>> +    method = aml_method("_CRS", 0);
> >>> +    Aml *rbuf = aml_resource_template();
> >>> +    aml_append(rbuf,
> >>> +        aml_word_bus_number(aml_min_fixed, aml_max_fixed, aml_pos_decode,
> >>> +                            0x0000, 0x0000, info->nr_pcie_buses - 1,
> >>> +                            0x0000, info->nr_pcie_buses));
> >>> +    aml_append(rbuf,
> >>> +        aml_dword_memory(aml_pos_decode, aml_min_fixed, aml_max_fixed,
> >>> +                         aml_non_cacheable, aml_ReadWrite,
> >>> +                         0x0000, info->pcie_mmio.addr,
> >>> +                         info->pcie_mmio.addr + info->pcie_mmio.size - 1,
> >>> +                         0x0000, info->pcie_mmio.size));
> >>> +    aml_append(rbuf,
> >>> +        aml_dword_io(aml_min_fixed, aml_max_fixed,
> >>> +                     aml_pos_decode, aml_entire_range,
> >>> +                     0x0000, 0x0000, info->pcie_ioport.size - 1,
> >>> +                     info->pcie_ioport.addr, info->pcie_ioport.size));
> >>> +
> >>> +    aml_append(method, aml_name_decl("RBUF", rbuf));
> >>> +    aml_append(method, aml_return(rbuf));
> >>> +    aml_append(dev, method);
> >>> +
> >>> +    /* Declare an _OSC (OS Control Handoff) method */
> >>> +    aml_append(dev, aml_name_decl("SUPP", aml_int(0)));
> >>> +    aml_append(dev, aml_name_decl("CTRL", aml_int(0)));
> >>> +    method = aml_method("_OSC", 4);
> >>> +    aml_append(method,
> >>> +        aml_create_dword_field(aml_arg(3), aml_int(0), "CDW1"));
> >>> +
> >>> +    /* PCI Firmware Specification 3.0
> >>> +     * 4.5.1. _OSC Interface for PCI Host Bridge Devices
> >>> +     * The _OSC interface for a PCI/PCI-X/PCI Express hierarchy is
> >>> +     * identified by the Universal Unique IDentifier (UUID)
> >>> +     * 33db4d5b-1ff7-401c-9657-7441c03dd766
> >>> +     */
> >>> +    UUID = aml_touuid("33DB4D5B-1FF7-401C-9657-7441C03DD766");
> >>> +    ifctx = aml_if(aml_equal(aml_arg(0), UUID));
> >>> +    aml_append(ifctx,
> >>> +        aml_create_dword_field(aml_arg(3), aml_int(4), "CDW2"));
> >>> +    aml_append(ifctx,
> >>> +        aml_create_dword_field(aml_arg(3), aml_int(8), "CDW3"));
> >>> +    aml_append(ifctx, aml_store(aml_name("CDW2"), aml_name("SUPP")));
> >>> +    aml_append(ifctx, aml_store(aml_name("CDW3"), aml_name("CTRL")));
> >>> +    aml_append(ifctx, aml_store(aml_and(aml_name("CTRL"), aml_int(0x1D)),
> >>> +                                aml_name("CTRL")));
> >>> +
> >>> +    ifctx1 = aml_if(aml_not(aml_equal(aml_arg(1), aml_int(0x1))));
> >>> +    aml_append(ifctx1, aml_store(aml_or(aml_name("CDW1"), aml_int(0x08)),
> >>> +                                 aml_name("CDW1")));
> >>> +    aml_append(ifctx, ifctx1);
> >>> +
> >>> +    ifctx1 = aml_if(aml_not(aml_equal(aml_name("CDW3"), aml_name("CTRL"))));
> >>> +    aml_append(ifctx1, aml_store(aml_or(aml_name("CDW1"), aml_int(0x10)),
> >>> +                                 aml_name("CDW1")));
> >>> +    aml_append(ifctx, ifctx1);
> >>> +
> >>> +    aml_append(ifctx, aml_store(aml_name("CTRL"), aml_name("CDW3")));
> >>> +    aml_append(ifctx, aml_return(aml_arg(3)));
> >>> +    aml_append(method, ifctx);
> >>> +
> >>> +    elsectx = aml_else();
> >>> +    aml_append(elsectx, aml_store(aml_or(aml_name("CDW1"), aml_int(4)),
> >>> +                                  aml_name("CDW1")));
> >>> +    aml_append(elsectx, aml_return(aml_arg(3)));
> >>> +    aml_append(method, elsectx);
> >>> +    aml_append(dev, method);
> >>> +
> >>> +    method = aml_method("_DSM", 4);
> >>> +
> >>> +    /* PCI Firmware Specification 3.0
> >>> +     * 4.6.1. _DSM for PCI Express Slot Information
> >>> +     * The UUID in _DSM in this context is
> >>> +     * {E5C937D0-3553-4d7a-9117-EA4D19C3434D}
> >>> +     */
> >>> +    UUID = aml_touuid("E5C937D0-3553-4d7a-9117-EA4D19C3434D");
> >>> +    ifctx = aml_if(aml_equal(aml_arg(0), UUID));
> >>> +    ifctx1 = aml_if(aml_equal(aml_arg(2), aml_int(0)));
> >>> +    buf = aml_buffer();
> >>> +    build_append_int_noprefix(buf->buf, 0x01, 1);
> >> pls, do not modify internal buffer of Aml objects outside API,
> >> and do not export build_append_int_noprefix().
> >>
> >> Looking at spec _DSM returns bitfield buffer.
> >> for purposes you are using here i.e. set bit 1 to 1 or 0
> >> it's sufficient to use aml_int(1) or aml_int(0) as they are
> >> folded in OneOp /0x1/ and ZeroOp /0x0/ opcodes.
> >>
> >> However if you need to manipulate specific bitfields checkout 
> >> CreateBitField /ACPI5.0: 19.5.18 CreateBitField (Create 1-Bit Buffer Field)/
> >> Probably that's what you are looking for.
> >>
> >> Also aml_buffer() needs to be modified to handle BufferSize
> >> see /ACPI5.0: 19.5.10 Buffer (Declare Buffer Object)/
> >> so that space for bitfields could be reserved
> >> /* as workaround/hack one can add several aml_int(0) into it to
> >> reserve needed amount of bytes */
> > 
> > I dislike abusing aml_int like this, makes code hard to follow.
> > Best just do it properly.
> > Internally you can call append_byte.
> > 
> 
> I agree we could use append_byte to reserve BufferSize of bytes.
> But we still need a function to initialize the buffer. A new function
> wraps build_append_int_noprefix()?
just extend aml_buffer(BufferSize) and then use any appropriate
internal functions inside API.

Outside you'll call:

buf = aml_buffer(1);
aml_append(method, aml_name_decl("RET", buf);
aml_createbitfield("RET", 0, "FNEN"/* non 0 functions supported */);
...
ifctx1 = aml_if(aml_equal(aml_arg(2), aml_int(0)));
aml_append(ifctx1, aml_store(aml_name("FNEN", aml_int(1)));
...
/* create bit field for every supported function if supported */
...
aml_append(method, aml_return(aml_name("RET")));


> 
> >>
> >>> +    aml_append(ifctx1, aml_return(buf));
> >>> +    aml_append(ifctx, ifctx1);
> >>> +    aml_append(method, ifctx);
> >>> +
> >>> +    buf = aml_buffer();
> >>> +    build_append_int_noprefix(buf->buf, 0x00, 1);
> >>> +    aml_append(method, aml_return(buf));
> >>> +    aml_append(dev, method);
> >>> +
> >>> +    Aml *dev_rp0 = aml_device("%s", "RP0");
> >>> +    aml_append(dev_rp0, aml_name_decl("_ADR", aml_int(0)));
> >>> +    aml_append(dev, dev_rp0);
> >>> +    aml_append(scope, dev);
> >>> +}
> >>> +
> >>>  /* RSDP */
> >>>  static GArray *
> >>>  build_rsdp(GArray *rsdp_table, GArray *linker, unsigned rsdt)
> >>> @@ -318,6 +468,8 @@ build_dsdt(GArray *table_data, GArray *linker, VirtGuestInfo *guest_info)
> >>>      acpi_dsdt_add_flash(scope, info->flash_memmap);
> >>>      acpi_dsdt_add_virtio(scope, info->virtio_mmio_memmap,
> >>>               info->virtio_mmio_irq, info->virtio_mmio_num);
> >>> +    acpi_dsdt_add_pci(scope, guest_info->pcie_info);
> >>> +
> >>>      aml_append(dsdt, scope);
> >>>  
> >>>      /* copy AML table into ACPI tables blob and patch header there */
> > 
> > .
> > 
> 
>
Shannon Zhao April 28, 2015, 12:57 p.m. UTC | #5
On 2015/4/28 17:54, Igor Mammedov wrote:
> On Tue, 28 Apr 2015 17:06:00 +0800
> Shannon Zhao <zhaoshenglong@huawei.com> wrote:
> 
>> On 2015/4/28 16:47, Michael S. Tsirkin wrote:
>>> On Tue, Apr 28, 2015 at 10:42:25AM +0200, Igor Mammedov wrote:
>>>> On Wed, 15 Apr 2015 21:25:08 +0800
>>>> Shannon Zhao <zhaoshenglong@huawei.com> wrote:
>>>>
>>>>> From: Shannon Zhao <shannon.zhao@linaro.org>
>>>>>
>>>>> Add PCIe controller in ACPI DSDT table, so the guest can detect
>>>>> the PCIe.
>>>>>
>>>>> Signed-off-by: Shannon Zhao <zhaoshenglong@huawei.com>
>>>>> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
>>>>> ---
>>>>>  hw/arm/virt-acpi-build.c | 152 +++++++++++++++++++++++++++++++++++++++++++++++
>>>>>  1 file changed, 152 insertions(+)
>>>>>
>>>>> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
>>>>> index 85e8242..ceec405 100644
>>>>> --- a/hw/arm/virt-acpi-build.c
>>>>> +++ b/hw/arm/virt-acpi-build.c
>>>>> @@ -49,6 +49,8 @@
>>>>>  #include "qapi/qmp/qint.h"
>>>>>  #include "qom/qom-qobject.h"
>>>>>  #include "exec/ram_addr.h"
>>>>> +#include "hw/pci/pcie_host.h"
>>>>> +#include "hw/pci/pci.h"
>>>>>  
>>>>>  typedef struct VirtAcpiCpuInfo {
>>>>>      DECLARE_BITMAP(found_cpus, VIRT_ACPI_CPU_ID_LIMIT);
>>>>> @@ -160,6 +162,154 @@ static void acpi_dsdt_add_virtio(Aml *scope, const MemMap *virtio_mmio_memmap,
>>>>>      }
>>>>>  }
>>>>>  
>>>>> +static void acpi_dsdt_add_pci(Aml *scope, AcpiPcieInfo *info)
>>>>> +{
>>>>> +    Aml *method, *crs, *ifctx, *UUID, *ifctx1, *elsectx, *buf;
>>>>> +    int i, bus_no;
>>>>> +    int irq = *info->pcie_irq + 32;
>>>>> +
>>>>> +    Aml *dev = aml_device("%s", "PCI0");
>>>>> +    aml_append(dev, aml_name_decl("_HID", aml_string("PNP0A08")));
>>>>> +    aml_append(dev, aml_name_decl("_CID", aml_string("PNP0A03")));
>>>>> +    aml_append(dev, aml_name_decl("_SEG", aml_int(0)));
>>>>> +    aml_append(dev, aml_name_decl("_BBN", aml_int(0)));
>>>>> +    aml_append(dev, aml_name_decl("_ADR", aml_int(0)));
>>>>> +    aml_append(dev, aml_name_decl("_UID", aml_string("PCI0")));
>>>>> +    aml_append(dev, aml_name_decl("_STR", aml_string("PCIe 0 Device")));
>>>>> +
>>>>> +    /* Declare the PCI Routing Table. */
>>>>> +    Aml *rt_pkg = aml_package(info->nr_pcie_buses * PCI_NUM_PINS);
>>>>> +    for (bus_no = 0; bus_no < info->nr_pcie_buses; bus_no++) {
>>>>> +        for (i = 0; i < PCI_NUM_PINS; i++) {
>>>>> +            int gsi = (i + bus_no) % PCI_NUM_PINS;
>>>>> +            Aml *pkg = aml_package(4);
>>>>> +            aml_append(pkg, aml_int((bus_no << 16) | 0xFFFF));
>>>>> +            aml_append(pkg, aml_int(i));
>>>>> +            aml_append(pkg, aml_name("GSI%d", gsi));
>>>>> +            aml_append(pkg, aml_int(0));
>>>>> +            aml_append(rt_pkg, pkg);
>>>>> +        }
>>>>> +    }
>>>>> +    aml_append(dev, aml_name_decl("_PRT", rt_pkg));
>>>>> +
>>>>> +    /* Create GSI link device */
>>>>> +    for (i = 0; i < PCI_NUM_PINS; i++) {
>>>>> +        Aml *dev_gsi = aml_device("GSI%d", i);
>>>>> +        aml_append(dev_gsi, aml_name_decl("_HID", aml_string("PNP0C0F")));
>>>>> +        aml_append(dev_gsi, aml_name_decl("_UID", aml_int(0)));
>>>>> +        crs = aml_resource_template();
>>>>> +        aml_append(crs,
>>>>> +                   aml_interrupt(aml_consumer, aml_level, aml_active_high,
>>>>> +                   aml_exclusive, aml_not_wake_capable, irq + i));
>>>>> +        aml_append(dev_gsi, aml_name_decl("_PRS", crs));
>>>>> +        crs = aml_resource_template();
>>>>> +        aml_append(crs,
>>>>> +                   aml_interrupt(aml_consumer, aml_level, aml_active_high,
>>>>> +                   aml_exclusive, aml_not_wake_capable, irq + i));
>>>>> +        aml_append(dev_gsi, aml_name_decl("_CRS", crs));
>>>>> +        method = aml_method("_SRS", 1);
>>>>> +        aml_append(dev_gsi, method);
>>>>> +        aml_append(dev, dev_gsi);
>>>>> +    }
>>>>> +
>>>>> +    method = aml_method("_CBA", 0);
>>>>> +    aml_append(method, aml_return(aml_int(info->pcie_ecam.addr)));
>>>>> +    aml_append(dev, method);
>>>>> +
>>>>> +    method = aml_method("_CRS", 0);
>>>>> +    Aml *rbuf = aml_resource_template();
>>>>> +    aml_append(rbuf,
>>>>> +        aml_word_bus_number(aml_min_fixed, aml_max_fixed, aml_pos_decode,
>>>>> +                            0x0000, 0x0000, info->nr_pcie_buses - 1,
>>>>> +                            0x0000, info->nr_pcie_buses));
>>>>> +    aml_append(rbuf,
>>>>> +        aml_dword_memory(aml_pos_decode, aml_min_fixed, aml_max_fixed,
>>>>> +                         aml_non_cacheable, aml_ReadWrite,
>>>>> +                         0x0000, info->pcie_mmio.addr,
>>>>> +                         info->pcie_mmio.addr + info->pcie_mmio.size - 1,
>>>>> +                         0x0000, info->pcie_mmio.size));
>>>>> +    aml_append(rbuf,
>>>>> +        aml_dword_io(aml_min_fixed, aml_max_fixed,
>>>>> +                     aml_pos_decode, aml_entire_range,
>>>>> +                     0x0000, 0x0000, info->pcie_ioport.size - 1,
>>>>> +                     info->pcie_ioport.addr, info->pcie_ioport.size));
>>>>> +
>>>>> +    aml_append(method, aml_name_decl("RBUF", rbuf));
>>>>> +    aml_append(method, aml_return(rbuf));
>>>>> +    aml_append(dev, method);
>>>>> +
>>>>> +    /* Declare an _OSC (OS Control Handoff) method */
>>>>> +    aml_append(dev, aml_name_decl("SUPP", aml_int(0)));
>>>>> +    aml_append(dev, aml_name_decl("CTRL", aml_int(0)));
>>>>> +    method = aml_method("_OSC", 4);
>>>>> +    aml_append(method,
>>>>> +        aml_create_dword_field(aml_arg(3), aml_int(0), "CDW1"));
>>>>> +
>>>>> +    /* PCI Firmware Specification 3.0
>>>>> +     * 4.5.1. _OSC Interface for PCI Host Bridge Devices
>>>>> +     * The _OSC interface for a PCI/PCI-X/PCI Express hierarchy is
>>>>> +     * identified by the Universal Unique IDentifier (UUID)
>>>>> +     * 33db4d5b-1ff7-401c-9657-7441c03dd766
>>>>> +     */
>>>>> +    UUID = aml_touuid("33DB4D5B-1FF7-401C-9657-7441C03DD766");
>>>>> +    ifctx = aml_if(aml_equal(aml_arg(0), UUID));
>>>>> +    aml_append(ifctx,
>>>>> +        aml_create_dword_field(aml_arg(3), aml_int(4), "CDW2"));
>>>>> +    aml_append(ifctx,
>>>>> +        aml_create_dword_field(aml_arg(3), aml_int(8), "CDW3"));
>>>>> +    aml_append(ifctx, aml_store(aml_name("CDW2"), aml_name("SUPP")));
>>>>> +    aml_append(ifctx, aml_store(aml_name("CDW3"), aml_name("CTRL")));
>>>>> +    aml_append(ifctx, aml_store(aml_and(aml_name("CTRL"), aml_int(0x1D)),
>>>>> +                                aml_name("CTRL")));
>>>>> +
>>>>> +    ifctx1 = aml_if(aml_not(aml_equal(aml_arg(1), aml_int(0x1))));
>>>>> +    aml_append(ifctx1, aml_store(aml_or(aml_name("CDW1"), aml_int(0x08)),
>>>>> +                                 aml_name("CDW1")));
>>>>> +    aml_append(ifctx, ifctx1);
>>>>> +
>>>>> +    ifctx1 = aml_if(aml_not(aml_equal(aml_name("CDW3"), aml_name("CTRL"))));
>>>>> +    aml_append(ifctx1, aml_store(aml_or(aml_name("CDW1"), aml_int(0x10)),
>>>>> +                                 aml_name("CDW1")));
>>>>> +    aml_append(ifctx, ifctx1);
>>>>> +
>>>>> +    aml_append(ifctx, aml_store(aml_name("CTRL"), aml_name("CDW3")));
>>>>> +    aml_append(ifctx, aml_return(aml_arg(3)));
>>>>> +    aml_append(method, ifctx);
>>>>> +
>>>>> +    elsectx = aml_else();
>>>>> +    aml_append(elsectx, aml_store(aml_or(aml_name("CDW1"), aml_int(4)),
>>>>> +                                  aml_name("CDW1")));
>>>>> +    aml_append(elsectx, aml_return(aml_arg(3)));
>>>>> +    aml_append(method, elsectx);
>>>>> +    aml_append(dev, method);
>>>>> +
>>>>> +    method = aml_method("_DSM", 4);
>>>>> +
>>>>> +    /* PCI Firmware Specification 3.0
>>>>> +     * 4.6.1. _DSM for PCI Express Slot Information
>>>>> +     * The UUID in _DSM in this context is
>>>>> +     * {E5C937D0-3553-4d7a-9117-EA4D19C3434D}
>>>>> +     */
>>>>> +    UUID = aml_touuid("E5C937D0-3553-4d7a-9117-EA4D19C3434D");
>>>>> +    ifctx = aml_if(aml_equal(aml_arg(0), UUID));
>>>>> +    ifctx1 = aml_if(aml_equal(aml_arg(2), aml_int(0)));
>>>>> +    buf = aml_buffer();
>>>>> +    build_append_int_noprefix(buf->buf, 0x01, 1);
>>>> pls, do not modify internal buffer of Aml objects outside API,
>>>> and do not export build_append_int_noprefix().
>>>>
>>>> Looking at spec _DSM returns bitfield buffer.
>>>> for purposes you are using here i.e. set bit 1 to 1 or 0
>>>> it's sufficient to use aml_int(1) or aml_int(0) as they are
>>>> folded in OneOp /0x1/ and ZeroOp /0x0/ opcodes.
>>>>
>>>> However if you need to manipulate specific bitfields checkout 
>>>> CreateBitField /ACPI5.0: 19.5.18 CreateBitField (Create 1-Bit Buffer Field)/
>>>> Probably that's what you are looking for.
>>>>
>>>> Also aml_buffer() needs to be modified to handle BufferSize
>>>> see /ACPI5.0: 19.5.10 Buffer (Declare Buffer Object)/
>>>> so that space for bitfields could be reserved
>>>> /* as workaround/hack one can add several aml_int(0) into it to
>>>> reserve needed amount of bytes */
>>>
>>> I dislike abusing aml_int like this, makes code hard to follow.
>>> Best just do it properly.
>>> Internally you can call append_byte.
>>>
>>
>> I agree we could use append_byte to reserve BufferSize of bytes.
>> But we still need a function to initialize the buffer. A new function
>> wraps build_append_int_noprefix()?
> just extend aml_buffer(BufferSize) and then use any appropriate
> internal functions inside API.
> 
> Outside you'll call:
> 
> buf = aml_buffer(1);
> aml_append(method, aml_name_decl("RET", buf);
> aml_createbitfield("RET", 0, "FNEN"/* non 0 functions supported */);
> ...

Here I need to set the value of buffer to 1 or 0, we could
createbitfield, but if we want to set the value to non one or zero and
the BufferSize is large, how could we do? CreateByteField? It's a little
complex for user.

> ifctx1 = aml_if(aml_equal(aml_arg(2), aml_int(0)));
> aml_append(ifctx1, aml_store(aml_name("FNEN", aml_int(1)));
> ...
> /* create bit field for every supported function if supported */
> ...
> aml_append(method, aml_return(aml_name("RET")));
> 
> 
>>
>>>>
>>>>> +    aml_append(ifctx1, aml_return(buf));
>>>>> +    aml_append(ifctx, ifctx1);
>>>>> +    aml_append(method, ifctx);
>>>>> +
>>>>> +    buf = aml_buffer();
>>>>> +    build_append_int_noprefix(buf->buf, 0x00, 1);
>>>>> +    aml_append(method, aml_return(buf));
>>>>> +    aml_append(dev, method);
>>>>> +
>>>>> +    Aml *dev_rp0 = aml_device("%s", "RP0");
>>>>> +    aml_append(dev_rp0, aml_name_decl("_ADR", aml_int(0)));
>>>>> +    aml_append(dev, dev_rp0);
>>>>> +    aml_append(scope, dev);
>>>>> +}
>>>>> +
>>>>>  /* RSDP */
>>>>>  static GArray *
>>>>>  build_rsdp(GArray *rsdp_table, GArray *linker, unsigned rsdt)
>>>>> @@ -318,6 +468,8 @@ build_dsdt(GArray *table_data, GArray *linker, VirtGuestInfo *guest_info)
>>>>>      acpi_dsdt_add_flash(scope, info->flash_memmap);
>>>>>      acpi_dsdt_add_virtio(scope, info->virtio_mmio_memmap,
>>>>>               info->virtio_mmio_irq, info->virtio_mmio_num);
>>>>> +    acpi_dsdt_add_pci(scope, guest_info->pcie_info);
>>>>> +
>>>>>      aml_append(dsdt, scope);
>>>>>  
>>>>>      /* copy AML table into ACPI tables blob and patch header there */
>>>
>>> .
>>>
>>
>>
> 
> 
> .
>
Igor Mammedov April 28, 2015, 3:13 p.m. UTC | #6
On Tue, 28 Apr 2015 20:57:25 +0800
Shannon Zhao <zhaoshenglong@huawei.com> wrote:

> On 2015/4/28 17:54, Igor Mammedov wrote:
> > On Tue, 28 Apr 2015 17:06:00 +0800
> > Shannon Zhao <zhaoshenglong@huawei.com> wrote:
> > 
> >> On 2015/4/28 16:47, Michael S. Tsirkin wrote:
> >>> On Tue, Apr 28, 2015 at 10:42:25AM +0200, Igor Mammedov wrote:
> >>>> On Wed, 15 Apr 2015 21:25:08 +0800
> >>>> Shannon Zhao <zhaoshenglong@huawei.com> wrote:
> >>>>
> >>>>> From: Shannon Zhao <shannon.zhao@linaro.org>
> >>>>>
> >>>>> Add PCIe controller in ACPI DSDT table, so the guest can detect
> >>>>> the PCIe.
> >>>>>
> >>>>> Signed-off-by: Shannon Zhao <zhaoshenglong@huawei.com>
> >>>>> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
> >>>>> ---
> >>>>>  hw/arm/virt-acpi-build.c | 152 +++++++++++++++++++++++++++++++++++++++++++++++
> >>>>>  1 file changed, 152 insertions(+)
> >>>>>
> >>>>> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> >>>>> index 85e8242..ceec405 100644
> >>>>> --- a/hw/arm/virt-acpi-build.c
> >>>>> +++ b/hw/arm/virt-acpi-build.c
> >>>>> @@ -49,6 +49,8 @@
> >>>>>  #include "qapi/qmp/qint.h"
> >>>>>  #include "qom/qom-qobject.h"
> >>>>>  #include "exec/ram_addr.h"
> >>>>> +#include "hw/pci/pcie_host.h"
> >>>>> +#include "hw/pci/pci.h"
> >>>>>  
> >>>>>  typedef struct VirtAcpiCpuInfo {
> >>>>>      DECLARE_BITMAP(found_cpus, VIRT_ACPI_CPU_ID_LIMIT);
> >>>>> @@ -160,6 +162,154 @@ static void acpi_dsdt_add_virtio(Aml *scope, const MemMap *virtio_mmio_memmap,
> >>>>>      }
> >>>>>  }
> >>>>>  
> >>>>> +static void acpi_dsdt_add_pci(Aml *scope, AcpiPcieInfo *info)
> >>>>> +{
> >>>>> +    Aml *method, *crs, *ifctx, *UUID, *ifctx1, *elsectx, *buf;
> >>>>> +    int i, bus_no;
> >>>>> +    int irq = *info->pcie_irq + 32;
> >>>>> +
> >>>>> +    Aml *dev = aml_device("%s", "PCI0");
> >>>>> +    aml_append(dev, aml_name_decl("_HID", aml_string("PNP0A08")));
> >>>>> +    aml_append(dev, aml_name_decl("_CID", aml_string("PNP0A03")));
> >>>>> +    aml_append(dev, aml_name_decl("_SEG", aml_int(0)));
> >>>>> +    aml_append(dev, aml_name_decl("_BBN", aml_int(0)));
> >>>>> +    aml_append(dev, aml_name_decl("_ADR", aml_int(0)));
> >>>>> +    aml_append(dev, aml_name_decl("_UID", aml_string("PCI0")));
> >>>>> +    aml_append(dev, aml_name_decl("_STR", aml_string("PCIe 0 Device")));
> >>>>> +
> >>>>> +    /* Declare the PCI Routing Table. */
> >>>>> +    Aml *rt_pkg = aml_package(info->nr_pcie_buses * PCI_NUM_PINS);
> >>>>> +    for (bus_no = 0; bus_no < info->nr_pcie_buses; bus_no++) {
> >>>>> +        for (i = 0; i < PCI_NUM_PINS; i++) {
> >>>>> +            int gsi = (i + bus_no) % PCI_NUM_PINS;
> >>>>> +            Aml *pkg = aml_package(4);
> >>>>> +            aml_append(pkg, aml_int((bus_no << 16) | 0xFFFF));
> >>>>> +            aml_append(pkg, aml_int(i));
> >>>>> +            aml_append(pkg, aml_name("GSI%d", gsi));
> >>>>> +            aml_append(pkg, aml_int(0));
> >>>>> +            aml_append(rt_pkg, pkg);
> >>>>> +        }
> >>>>> +    }
> >>>>> +    aml_append(dev, aml_name_decl("_PRT", rt_pkg));
> >>>>> +
> >>>>> +    /* Create GSI link device */
> >>>>> +    for (i = 0; i < PCI_NUM_PINS; i++) {
> >>>>> +        Aml *dev_gsi = aml_device("GSI%d", i);
> >>>>> +        aml_append(dev_gsi, aml_name_decl("_HID", aml_string("PNP0C0F")));
> >>>>> +        aml_append(dev_gsi, aml_name_decl("_UID", aml_int(0)));
> >>>>> +        crs = aml_resource_template();
> >>>>> +        aml_append(crs,
> >>>>> +                   aml_interrupt(aml_consumer, aml_level, aml_active_high,
> >>>>> +                   aml_exclusive, aml_not_wake_capable, irq + i));
> >>>>> +        aml_append(dev_gsi, aml_name_decl("_PRS", crs));
> >>>>> +        crs = aml_resource_template();
> >>>>> +        aml_append(crs,
> >>>>> +                   aml_interrupt(aml_consumer, aml_level, aml_active_high,
> >>>>> +                   aml_exclusive, aml_not_wake_capable, irq + i));
> >>>>> +        aml_append(dev_gsi, aml_name_decl("_CRS", crs));
> >>>>> +        method = aml_method("_SRS", 1);
> >>>>> +        aml_append(dev_gsi, method);
> >>>>> +        aml_append(dev, dev_gsi);
> >>>>> +    }
> >>>>> +
> >>>>> +    method = aml_method("_CBA", 0);
> >>>>> +    aml_append(method, aml_return(aml_int(info->pcie_ecam.addr)));
> >>>>> +    aml_append(dev, method);
> >>>>> +
> >>>>> +    method = aml_method("_CRS", 0);
> >>>>> +    Aml *rbuf = aml_resource_template();
> >>>>> +    aml_append(rbuf,
> >>>>> +        aml_word_bus_number(aml_min_fixed, aml_max_fixed, aml_pos_decode,
> >>>>> +                            0x0000, 0x0000, info->nr_pcie_buses - 1,
> >>>>> +                            0x0000, info->nr_pcie_buses));
> >>>>> +    aml_append(rbuf,
> >>>>> +        aml_dword_memory(aml_pos_decode, aml_min_fixed, aml_max_fixed,
> >>>>> +                         aml_non_cacheable, aml_ReadWrite,
> >>>>> +                         0x0000, info->pcie_mmio.addr,
> >>>>> +                         info->pcie_mmio.addr + info->pcie_mmio.size - 1,
> >>>>> +                         0x0000, info->pcie_mmio.size));
> >>>>> +    aml_append(rbuf,
> >>>>> +        aml_dword_io(aml_min_fixed, aml_max_fixed,
> >>>>> +                     aml_pos_decode, aml_entire_range,
> >>>>> +                     0x0000, 0x0000, info->pcie_ioport.size - 1,
> >>>>> +                     info->pcie_ioport.addr, info->pcie_ioport.size));
> >>>>> +
> >>>>> +    aml_append(method, aml_name_decl("RBUF", rbuf));
> >>>>> +    aml_append(method, aml_return(rbuf));
> >>>>> +    aml_append(dev, method);
> >>>>> +
> >>>>> +    /* Declare an _OSC (OS Control Handoff) method */
> >>>>> +    aml_append(dev, aml_name_decl("SUPP", aml_int(0)));
> >>>>> +    aml_append(dev, aml_name_decl("CTRL", aml_int(0)));
> >>>>> +    method = aml_method("_OSC", 4);
> >>>>> +    aml_append(method,
> >>>>> +        aml_create_dword_field(aml_arg(3), aml_int(0), "CDW1"));
> >>>>> +
> >>>>> +    /* PCI Firmware Specification 3.0
> >>>>> +     * 4.5.1. _OSC Interface for PCI Host Bridge Devices
> >>>>> +     * The _OSC interface for a PCI/PCI-X/PCI Express hierarchy is
> >>>>> +     * identified by the Universal Unique IDentifier (UUID)
> >>>>> +     * 33db4d5b-1ff7-401c-9657-7441c03dd766
> >>>>> +     */
> >>>>> +    UUID = aml_touuid("33DB4D5B-1FF7-401C-9657-7441C03DD766");
> >>>>> +    ifctx = aml_if(aml_equal(aml_arg(0), UUID));
> >>>>> +    aml_append(ifctx,
> >>>>> +        aml_create_dword_field(aml_arg(3), aml_int(4), "CDW2"));
> >>>>> +    aml_append(ifctx,
> >>>>> +        aml_create_dword_field(aml_arg(3), aml_int(8), "CDW3"));
> >>>>> +    aml_append(ifctx, aml_store(aml_name("CDW2"), aml_name("SUPP")));
> >>>>> +    aml_append(ifctx, aml_store(aml_name("CDW3"), aml_name("CTRL")));
> >>>>> +    aml_append(ifctx, aml_store(aml_and(aml_name("CTRL"), aml_int(0x1D)),
> >>>>> +                                aml_name("CTRL")));
> >>>>> +
> >>>>> +    ifctx1 = aml_if(aml_not(aml_equal(aml_arg(1), aml_int(0x1))));
> >>>>> +    aml_append(ifctx1, aml_store(aml_or(aml_name("CDW1"), aml_int(0x08)),
> >>>>> +                                 aml_name("CDW1")));
> >>>>> +    aml_append(ifctx, ifctx1);
> >>>>> +
> >>>>> +    ifctx1 = aml_if(aml_not(aml_equal(aml_name("CDW3"), aml_name("CTRL"))));
> >>>>> +    aml_append(ifctx1, aml_store(aml_or(aml_name("CDW1"), aml_int(0x10)),
> >>>>> +                                 aml_name("CDW1")));
> >>>>> +    aml_append(ifctx, ifctx1);
> >>>>> +
> >>>>> +    aml_append(ifctx, aml_store(aml_name("CTRL"), aml_name("CDW3")));
> >>>>> +    aml_append(ifctx, aml_return(aml_arg(3)));
> >>>>> +    aml_append(method, ifctx);
> >>>>> +
> >>>>> +    elsectx = aml_else();
> >>>>> +    aml_append(elsectx, aml_store(aml_or(aml_name("CDW1"), aml_int(4)),
> >>>>> +                                  aml_name("CDW1")));
> >>>>> +    aml_append(elsectx, aml_return(aml_arg(3)));
> >>>>> +    aml_append(method, elsectx);
> >>>>> +    aml_append(dev, method);
> >>>>> +
> >>>>> +    method = aml_method("_DSM", 4);
> >>>>> +
> >>>>> +    /* PCI Firmware Specification 3.0
> >>>>> +     * 4.6.1. _DSM for PCI Express Slot Information
> >>>>> +     * The UUID in _DSM in this context is
> >>>>> +     * {E5C937D0-3553-4d7a-9117-EA4D19C3434D}
> >>>>> +     */
> >>>>> +    UUID = aml_touuid("E5C937D0-3553-4d7a-9117-EA4D19C3434D");
> >>>>> +    ifctx = aml_if(aml_equal(aml_arg(0), UUID));
> >>>>> +    ifctx1 = aml_if(aml_equal(aml_arg(2), aml_int(0)));
> >>>>> +    buf = aml_buffer();
> >>>>> +    build_append_int_noprefix(buf->buf, 0x01, 1);
> >>>> pls, do not modify internal buffer of Aml objects outside API,
> >>>> and do not export build_append_int_noprefix().
> >>>>
> >>>> Looking at spec _DSM returns bitfield buffer.
> >>>> for purposes you are using here i.e. set bit 1 to 1 or 0
> >>>> it's sufficient to use aml_int(1) or aml_int(0) as they are
> >>>> folded in OneOp /0x1/ and ZeroOp /0x0/ opcodes.
> >>>>
> >>>> However if you need to manipulate specific bitfields checkout 
> >>>> CreateBitField /ACPI5.0: 19.5.18 CreateBitField (Create 1-Bit Buffer Field)/
> >>>> Probably that's what you are looking for.
> >>>>
> >>>> Also aml_buffer() needs to be modified to handle BufferSize
> >>>> see /ACPI5.0: 19.5.10 Buffer (Declare Buffer Object)/
> >>>> so that space for bitfields could be reserved
> >>>> /* as workaround/hack one can add several aml_int(0) into it to
> >>>> reserve needed amount of bytes */
> >>>
> >>> I dislike abusing aml_int like this, makes code hard to follow.
> >>> Best just do it properly.
> >>> Internally you can call append_byte.
> >>>
> >>
> >> I agree we could use append_byte to reserve BufferSize of bytes.
> >> But we still need a function to initialize the buffer. A new function
> >> wraps build_append_int_noprefix()?
> > just extend aml_buffer(BufferSize) and then use any appropriate
> > internal functions inside API.
> > 
> > Outside you'll call:
> > 
> > buf = aml_buffer(1);
> > aml_append(method, aml_name_decl("RET", buf);
> > aml_createbitfield("RET", 0, "FNEN"/* non 0 functions supported */);
> > ...
> 
> Here I need to set the value of buffer to 1 or 0, we could
> createbitfield, but if we want to set the value to non one or zero and
> the BufferSize is large, how could we do? CreateByteField? It's a little
> complex for user.
that's what one would have to do writing it in ASL if bits
are flipped on/off dynamically.

In ASL you also can declare buffer with static initializer

   Buffer (0x01) { 0x03 }

and compiler is smart enough to set appropriate bits but it doesn't
allow you to do so with large values. For example:

   Buffer (0x01) { 0xAABBCCDD }

gives error:
Error 6139 - Constant out of range ^  (0xAABBCCDD, allowable: 0x0-0xFF)

If one wants to manipulate specific fields in Buffer, ASL has
a bunch of CreateFOOField operators, so lets follow spec and use
API consistently to avoid confusion.

BTW:
packaging value as int (even without prefix) is wrong since
its LE encoding will shuffle bytes and you won't get bits in
positions that you expect if value is more than 1 byte.

> 
> > ifctx1 = aml_if(aml_equal(aml_arg(2), aml_int(0)));
> > aml_append(ifctx1, aml_store(aml_name("FNEN", aml_int(1)));
> > ...
> > /* create bit field for every supported function if supported */
> > ...
> > aml_append(method, aml_return(aml_name("RET")));
> > 
> > 
> >>
> >>>>
> >>>>> +    aml_append(ifctx1, aml_return(buf));
> >>>>> +    aml_append(ifctx, ifctx1);
> >>>>> +    aml_append(method, ifctx);
> >>>>> +
> >>>>> +    buf = aml_buffer();
> >>>>> +    build_append_int_noprefix(buf->buf, 0x00, 1);
> >>>>> +    aml_append(method, aml_return(buf));
> >>>>> +    aml_append(dev, method);
> >>>>> +
> >>>>> +    Aml *dev_rp0 = aml_device("%s", "RP0");
> >>>>> +    aml_append(dev_rp0, aml_name_decl("_ADR", aml_int(0)));
> >>>>> +    aml_append(dev, dev_rp0);
> >>>>> +    aml_append(scope, dev);
> >>>>> +}
> >>>>> +
> >>>>>  /* RSDP */
> >>>>>  static GArray *
> >>>>>  build_rsdp(GArray *rsdp_table, GArray *linker, unsigned rsdt)
> >>>>> @@ -318,6 +468,8 @@ build_dsdt(GArray *table_data, GArray *linker, VirtGuestInfo *guest_info)
> >>>>>      acpi_dsdt_add_flash(scope, info->flash_memmap);
> >>>>>      acpi_dsdt_add_virtio(scope, info->virtio_mmio_memmap,
> >>>>>               info->virtio_mmio_irq, info->virtio_mmio_num);
> >>>>> +    acpi_dsdt_add_pci(scope, guest_info->pcie_info);
> >>>>> +
> >>>>>      aml_append(dsdt, scope);
> >>>>>  
> >>>>>      /* copy AML table into ACPI tables blob and patch header there */
> >>>
> >>> .
> >>>
> >>
> >>
> > 
> > 
> > .
> > 
> 
>
Michael S. Tsirkin April 28, 2015, 3:54 p.m. UTC | #7
On Tue, Apr 28, 2015 at 05:13:10PM +0200, Igor Mammedov wrote:
> > Here I need to set the value of buffer to 1 or 0, we could
> > createbitfield, but if we want to set the value to non one or zero and
> > the BufferSize is large, how could we do? CreateByteField? It's a little
> > complex for user.
> that's what one would have to do writing it in ASL if bits
> are flipped on/off dynamically.
> 
> In ASL you also can declare buffer with static initializer
> 
>    Buffer (0x01) { 0x03 }
> 
> and compiler is smart enough to set appropriate bits but it doesn't
> allow you to do so with large values. For example:
> 
>    Buffer (0x01) { 0xAABBCCDD }
> 
> gives error:
> Error 6139 - Constant out of range ^  (0xAABBCCDD, allowable: 0x0-0xFF)
> 
> If one wants to manipulate specific fields in Buffer, ASL has
> a bunch of CreateFOOField operators, so lets follow spec and use
> API consistently to avoid confusion.
> 
> BTW:
> packaging value as int (even without prefix) is wrong since
> its LE encoding will shuffle bytes and you won't get bits in
> positions that you expect if value is more than 1 byte.

I don't care about ASL, we are writing in C
But AML is same:
DefBuffer := BufferOp PkgLength BufferSize ByteList
BufferOp := 0x11
BufferSize := TermArg => Integer

So really just a bytelist.
We don't have any users for aml_buffer, maybe just add
const uint8_t *bytes, unsigned len as parameters.

Would that be enough?


> > 
> > > ifctx1 = aml_if(aml_equal(aml_arg(2), aml_int(0)));
> > > aml_append(ifctx1, aml_store(aml_name("FNEN", aml_int(1)));
> > > ...
> > > /* create bit field for every supported function if supported */
> > > ...
> > > aml_append(method, aml_return(aml_name("RET")));
> > > 
> > > 
> > >>
> > >>>>
> > >>>>> +    aml_append(ifctx1, aml_return(buf));
> > >>>>> +    aml_append(ifctx, ifctx1);
> > >>>>> +    aml_append(method, ifctx);
> > >>>>> +
> > >>>>> +    buf = aml_buffer();
> > >>>>> +    build_append_int_noprefix(buf->buf, 0x00, 1);
> > >>>>> +    aml_append(method, aml_return(buf));
> > >>>>> +    aml_append(dev, method);
> > >>>>> +
> > >>>>> +    Aml *dev_rp0 = aml_device("%s", "RP0");
> > >>>>> +    aml_append(dev_rp0, aml_name_decl("_ADR", aml_int(0)));
> > >>>>> +    aml_append(dev, dev_rp0);
> > >>>>> +    aml_append(scope, dev);
> > >>>>> +}
> > >>>>> +
> > >>>>>  /* RSDP */
> > >>>>>  static GArray *
> > >>>>>  build_rsdp(GArray *rsdp_table, GArray *linker, unsigned rsdt)
> > >>>>> @@ -318,6 +468,8 @@ build_dsdt(GArray *table_data, GArray *linker, VirtGuestInfo *guest_info)
> > >>>>>      acpi_dsdt_add_flash(scope, info->flash_memmap);
> > >>>>>      acpi_dsdt_add_virtio(scope, info->virtio_mmio_memmap,
> > >>>>>               info->virtio_mmio_irq, info->virtio_mmio_num);
> > >>>>> +    acpi_dsdt_add_pci(scope, guest_info->pcie_info);
> > >>>>> +
> > >>>>>      aml_append(dsdt, scope);
> > >>>>>  
> > >>>>>      /* copy AML table into ACPI tables blob and patch header there */
> > >>>
> > >>> .
> > >>>
> > >>
> > >>
> > > 
> > > 
> > > .
> > > 
> > 
> >
Shannon Zhao April 29, 2015, 3:12 a.m. UTC | #8
On 2015/4/28 23:54, Michael S. Tsirkin wrote:
> On Tue, Apr 28, 2015 at 05:13:10PM +0200, Igor Mammedov wrote:
>>> Here I need to set the value of buffer to 1 or 0, we could
>>> createbitfield, but if we want to set the value to non one or zero and
>>> the BufferSize is large, how could we do? CreateByteField? It's a little
>>> complex for user.
>> that's what one would have to do writing it in ASL if bits
>> are flipped on/off dynamically.
>>
>> In ASL you also can declare buffer with static initializer
>>
>>    Buffer (0x01) { 0x03 }
>>
>> and compiler is smart enough to set appropriate bits but it doesn't
>> allow you to do so with large values. For example:
>>
>>    Buffer (0x01) { 0xAABBCCDD }
>>
>> gives error:
>> Error 6139 - Constant out of range ^  (0xAABBCCDD, allowable: 0x0-0xFF)
>>
>> If one wants to manipulate specific fields in Buffer, ASL has
>> a bunch of CreateFOOField operators, so lets follow spec and use
>> API consistently to avoid confusion.
>>
>> BTW:
>> packaging value as int (even without prefix) is wrong since
>> its LE encoding will shuffle bytes and you won't get bits in
>> positions that you expect if value is more than 1 byte.
> 
> I don't care about ASL, we are writing in C
> But AML is same:
> DefBuffer := BufferOp PkgLength BufferSize ByteList
> BufferOp := 0x11
> BufferSize := TermArg => Integer
> 
> So really just a bytelist.
> We don't have any users for aml_buffer, maybe just add
> const uint8_t *bytes, unsigned len as parameters.
> 

Agree. It's consistent with the spec. If want to modify the value, could
use CreateFOOField.

So use following fuction to initialize Buffer?

/* ACPI 1.0b: 16.2.5.4 Type 2 Opcodes Encoding: DefBuffer */
Aml *aml_buffer(int buffer_size, uint8_t *byte_list)
{
    int i;
    Aml *var = aml_bundle(0x11 /* BufferOp */, AML_BUFFER);
    for (i = 0; i < buffer_size; i++) {
        build_append_byte(var->buf, *(byte_list + i));
    }
    return var;
}

> Would that be enough?
> 
> 
>>>
>>>> ifctx1 = aml_if(aml_equal(aml_arg(2), aml_int(0)));
>>>> aml_append(ifctx1, aml_store(aml_name("FNEN", aml_int(1)));
>>>> ...
>>>> /* create bit field for every supported function if supported */
>>>> ...
>>>> aml_append(method, aml_return(aml_name("RET")));
>>>>
>>>>
>>>>>
>>>>>>>
>>>>>>>> +    aml_append(ifctx1, aml_return(buf));
>>>>>>>> +    aml_append(ifctx, ifctx1);
>>>>>>>> +    aml_append(method, ifctx);
>>>>>>>> +
>>>>>>>> +    buf = aml_buffer();
>>>>>>>> +    build_append_int_noprefix(buf->buf, 0x00, 1);
>>>>>>>> +    aml_append(method, aml_return(buf));
>>>>>>>> +    aml_append(dev, method);
>>>>>>>> +
>>>>>>>> +    Aml *dev_rp0 = aml_device("%s", "RP0");
>>>>>>>> +    aml_append(dev_rp0, aml_name_decl("_ADR", aml_int(0)));
>>>>>>>> +    aml_append(dev, dev_rp0);
>>>>>>>> +    aml_append(scope, dev);
>>>>>>>> +}
>>>>>>>> +
>>>>>>>>  /* RSDP */
>>>>>>>>  static GArray *
>>>>>>>>  build_rsdp(GArray *rsdp_table, GArray *linker, unsigned rsdt)
>>>>>>>> @@ -318,6 +468,8 @@ build_dsdt(GArray *table_data, GArray *linker, VirtGuestInfo *guest_info)
>>>>>>>>      acpi_dsdt_add_flash(scope, info->flash_memmap);
>>>>>>>>      acpi_dsdt_add_virtio(scope, info->virtio_mmio_memmap,
>>>>>>>>               info->virtio_mmio_irq, info->virtio_mmio_num);
>>>>>>>> +    acpi_dsdt_add_pci(scope, guest_info->pcie_info);
>>>>>>>> +
>>>>>>>>      aml_append(dsdt, scope);
>>>>>>>>  
>>>>>>>>      /* copy AML table into ACPI tables blob and patch header there */
>>>>>>
>>>>>> .
>>>>>>
>>>>>
>>>>>
>>>>
>>>>
>>>> .
>>>>
>>>
>>>
> 
> .
>
Igor Mammedov April 29, 2015, 8:47 a.m. UTC | #9
On Wed, 29 Apr 2015 11:12:04 +0800
Shannon Zhao <zhaoshenglong@huawei.com> wrote:

> On 2015/4/28 23:54, Michael S. Tsirkin wrote:
> > On Tue, Apr 28, 2015 at 05:13:10PM +0200, Igor Mammedov wrote:
> >>> Here I need to set the value of buffer to 1 or 0, we could
> >>> createbitfield, but if we want to set the value to non one or zero and
> >>> the BufferSize is large, how could we do? CreateByteField? It's a little
> >>> complex for user.
> >> that's what one would have to do writing it in ASL if bits
> >> are flipped on/off dynamically.
> >>
> >> In ASL you also can declare buffer with static initializer
> >>
> >>    Buffer (0x01) { 0x03 }
> >>
> >> and compiler is smart enough to set appropriate bits but it doesn't
> >> allow you to do so with large values. For example:
> >>
> >>    Buffer (0x01) { 0xAABBCCDD }
> >>
> >> gives error:
> >> Error 6139 - Constant out of range ^  (0xAABBCCDD, allowable: 0x0-0xFF)
> >>
> >> If one wants to manipulate specific fields in Buffer, ASL has
> >> a bunch of CreateFOOField operators, so lets follow spec and use
> >> API consistently to avoid confusion.
> >>
> >> BTW:
> >> packaging value as int (even without prefix) is wrong since
> >> its LE encoding will shuffle bytes and you won't get bits in
> >> positions that you expect if value is more than 1 byte.
> > 
> > I don't care about ASL, we are writing in C
> > But AML is same:
> > DefBuffer := BufferOp PkgLength BufferSize ByteList
> > BufferOp := 0x11
> > BufferSize := TermArg => Integer
> > 
> > So really just a bytelist.
> > We don't have any users for aml_buffer, maybe just add
> > const uint8_t *bytes, unsigned len as parameters.
> > 
> 
> Agree. It's consistent with the spec. If want to modify the value, could
> use CreateFOOField.
> 
> So use following fuction to initialize Buffer?
> 
> /* ACPI 1.0b: 16.2.5.4 Type 2 Opcodes Encoding: DefBuffer */
> Aml *aml_buffer(int buffer_size, uint8_t *byte_list)
> {
>     int i;
>     Aml *var = aml_bundle(0x11 /* BufferOp */, AML_BUFFER);
>     for (i = 0; i < buffer_size; i++) {
>         build_append_byte(var->buf, *(byte_list + i));
>     }
>     return var;
> }
maybe

Aml *aml_buffer_initialized(int buffer_size, uint8_t *byte_list);
Aml *aml_buffer(int buffer_size);

the second one is needed for implementing code like:
Name(BUFF, Buffer(4){}) // Create SerialBus data buffer as BUFF
CreateByteField(BUFF, 0x00, STAT) // STAT = Status (Byte)
CreateWordField(BUFF, 0x02, DATA) // DATA = Data (Byte)

and could reuse aml_buffer_initialized() to reserve space.

> 
> > Would that be enough?
> > 
> > 
> >>>
> >>>> ifctx1 = aml_if(aml_equal(aml_arg(2), aml_int(0)));
> >>>> aml_append(ifctx1, aml_store(aml_name("FNEN", aml_int(1)));
> >>>> ...
> >>>> /* create bit field for every supported function if supported */
> >>>> ...
> >>>> aml_append(method, aml_return(aml_name("RET")));
> >>>>
> >>>>
> >>>>>
> >>>>>>>
> >>>>>>>> +    aml_append(ifctx1, aml_return(buf));
> >>>>>>>> +    aml_append(ifctx, ifctx1);
> >>>>>>>> +    aml_append(method, ifctx);
> >>>>>>>> +
> >>>>>>>> +    buf = aml_buffer();
> >>>>>>>> +    build_append_int_noprefix(buf->buf, 0x00, 1);
> >>>>>>>> +    aml_append(method, aml_return(buf));
> >>>>>>>> +    aml_append(dev, method);
> >>>>>>>> +
> >>>>>>>> +    Aml *dev_rp0 = aml_device("%s", "RP0");
> >>>>>>>> +    aml_append(dev_rp0, aml_name_decl("_ADR", aml_int(0)));
> >>>>>>>> +    aml_append(dev, dev_rp0);
> >>>>>>>> +    aml_append(scope, dev);
> >>>>>>>> +}
> >>>>>>>> +
> >>>>>>>>  /* RSDP */
> >>>>>>>>  static GArray *
> >>>>>>>>  build_rsdp(GArray *rsdp_table, GArray *linker, unsigned rsdt)
> >>>>>>>> @@ -318,6 +468,8 @@ build_dsdt(GArray *table_data, GArray *linker, VirtGuestInfo *guest_info)
> >>>>>>>>      acpi_dsdt_add_flash(scope, info->flash_memmap);
> >>>>>>>>      acpi_dsdt_add_virtio(scope, info->virtio_mmio_memmap,
> >>>>>>>>               info->virtio_mmio_irq, info->virtio_mmio_num);
> >>>>>>>> +    acpi_dsdt_add_pci(scope, guest_info->pcie_info);
> >>>>>>>> +
> >>>>>>>>      aml_append(dsdt, scope);
> >>>>>>>>  
> >>>>>>>>      /* copy AML table into ACPI tables blob and patch header there */
> >>>>>>
> >>>>>> .
> >>>>>>
> >>>>>
> >>>>>
> >>>>
> >>>>
> >>>> .
> >>>>
> >>>
> >>>
> > 
> > .
> > 
> 
>
Shannon Zhao April 29, 2015, 1:37 p.m. UTC | #10
On 2015/4/29 16:47, Igor Mammedov wrote:
> On Wed, 29 Apr 2015 11:12:04 +0800
> Shannon Zhao <zhaoshenglong@huawei.com> wrote:
>
>> On 2015/4/28 23:54, Michael S. Tsirkin wrote:
>>> On Tue, Apr 28, 2015 at 05:13:10PM +0200, Igor Mammedov wrote:
>>>>> Here I need to set the value of buffer to 1 or 0, we could
>>>>> createbitfield, but if we want to set the value to non one or zero and
>>>>> the BufferSize is large, how could we do? CreateByteField? It's a little
>>>>> complex for user.
>>>> that's what one would have to do writing it in ASL if bits
>>>> are flipped on/off dynamically.
>>>>
>>>> In ASL you also can declare buffer with static initializer
>>>>
>>>>     Buffer (0x01) { 0x03 }
>>>>
>>>> and compiler is smart enough to set appropriate bits but it doesn't
>>>> allow you to do so with large values. For example:
>>>>
>>>>     Buffer (0x01) { 0xAABBCCDD }
>>>>
>>>> gives error:
>>>> Error 6139 - Constant out of range ^  (0xAABBCCDD, allowable: 0x0-0xFF)
>>>>
>>>> If one wants to manipulate specific fields in Buffer, ASL has
>>>> a bunch of CreateFOOField operators, so lets follow spec and use
>>>> API consistently to avoid confusion.
>>>>
>>>> BTW:
>>>> packaging value as int (even without prefix) is wrong since
>>>> its LE encoding will shuffle bytes and you won't get bits in
>>>> positions that you expect if value is more than 1 byte.
>>>
>>> I don't care about ASL, we are writing in C
>>> But AML is same:
>>> DefBuffer := BufferOp PkgLength BufferSize ByteList
>>> BufferOp := 0x11
>>> BufferSize := TermArg => Integer
>>>
>>> So really just a bytelist.
>>> We don't have any users for aml_buffer, maybe just add
>>> const uint8_t *bytes, unsigned len as parameters.
>>>
>>
>> Agree. It's consistent with the spec. If want to modify the value, could
>> use CreateFOOField.
>>
>> So use following fuction to initialize Buffer?
>>
>> /* ACPI 1.0b: 16.2.5.4 Type 2 Opcodes Encoding: DefBuffer */
>> Aml *aml_buffer(int buffer_size, uint8_t *byte_list)
>> {
>>      int i;
>>      Aml *var = aml_bundle(0x11 /* BufferOp */, AML_BUFFER);
>>      for (i = 0; i < buffer_size; i++) {
>>          build_append_byte(var->buf, *(byte_list + i));
>>      }
>>      return var;
>> }
> maybe
>
> Aml *aml_buffer_initialized(int buffer_size, uint8_t *byte_list);
> Aml *aml_buffer(int buffer_size);
>
> the second one is needed for implementing code like:
> Name(BUFF, Buffer(4){}) // Create SerialBus data buffer as BUFF
> CreateByteField(BUFF, 0x00, STAT) // STAT = Status (Byte)
> CreateWordField(BUFF, 0x02, DATA) // DATA = Data (Byte)
>
> and could reuse aml_buffer_initialized() to reserve space.
>

maybe we could use only one. For the uninitialized buffer we can pass 
byte_list as NULL and within aml_buffer check byte_list, if byte_list is 
NULL, just reserve space.

/* ACPI 1.0b: 16.2.5.4 Type 2 Opcodes Encoding: DefBuffer */
Aml *aml_buffer(int buffer_size, uint8_t *byte_list)
{
     int i;
     Aml *var = aml_bundle(0x11 /* BufferOp */, AML_BUFFER);

     for (i = 0; i < buffer_size; i++) {
         if (byte_list == NULL)
	    build_append_byte(var->buf, 0);
         else
             build_append_byte(var->buf, *(byte_list + i));
    }

    return var;
}

>>
>>> Would that be enough?
>>>
>>>
>>>>>
>>>>>> ifctx1 = aml_if(aml_equal(aml_arg(2), aml_int(0)));
>>>>>> aml_append(ifctx1, aml_store(aml_name("FNEN", aml_int(1)));
>>>>>> ...
>>>>>> /* create bit field for every supported function if supported */
>>>>>> ...
>>>>>> aml_append(method, aml_return(aml_name("RET")));
>>>>>>
>>>>>>
>>>>>>>
>>>>>>>>>
>>>>>>>>>> +    aml_append(ifctx1, aml_return(buf));
>>>>>>>>>> +    aml_append(ifctx, ifctx1);
>>>>>>>>>> +    aml_append(method, ifctx);
>>>>>>>>>> +
>>>>>>>>>> +    buf = aml_buffer();
>>>>>>>>>> +    build_append_int_noprefix(buf->buf, 0x00, 1);
>>>>>>>>>> +    aml_append(method, aml_return(buf));
>>>>>>>>>> +    aml_append(dev, method);
>>>>>>>>>> +
>>>>>>>>>> +    Aml *dev_rp0 = aml_device("%s", "RP0");
>>>>>>>>>> +    aml_append(dev_rp0, aml_name_decl("_ADR", aml_int(0)));
>>>>>>>>>> +    aml_append(dev, dev_rp0);
>>>>>>>>>> +    aml_append(scope, dev);
>>>>>>>>>> +}
>>>>>>>>>> +
>>>>>>>>>>   /* RSDP */
>>>>>>>>>>   static GArray *
>>>>>>>>>>   build_rsdp(GArray *rsdp_table, GArray *linker, unsigned rsdt)
>>>>>>>>>> @@ -318,6 +468,8 @@ build_dsdt(GArray *table_data, GArray *linker, VirtGuestInfo *guest_info)
>>>>>>>>>>       acpi_dsdt_add_flash(scope, info->flash_memmap);
>>>>>>>>>>       acpi_dsdt_add_virtio(scope, info->virtio_mmio_memmap,
>>>>>>>>>>                info->virtio_mmio_irq, info->virtio_mmio_num);
>>>>>>>>>> +    acpi_dsdt_add_pci(scope, guest_info->pcie_info);
>>>>>>>>>> +
>>>>>>>>>>       aml_append(dsdt, scope);
>>>>>>>>>>
>>>>>>>>>>       /* copy AML table into ACPI tables blob and patch header there */
>>>>>>>>
>>>>>>>> .
>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>
>>>>>>
>>>>>> .
>>>>>>
>>>>>
>>>>>
>>>
>>> .
>>>
>>
>>
>
Igor Mammedov April 29, 2015, 1:58 p.m. UTC | #11
On Wed, 29 Apr 2015 21:37:11 +0800
Shannon Zhao <shannon.zhao@linaro.org> wrote:

> 
> 
> On 2015/4/29 16:47, Igor Mammedov wrote:
> > On Wed, 29 Apr 2015 11:12:04 +0800
> > Shannon Zhao <zhaoshenglong@huawei.com> wrote:
> >
> >> On 2015/4/28 23:54, Michael S. Tsirkin wrote:
> >>> On Tue, Apr 28, 2015 at 05:13:10PM +0200, Igor Mammedov wrote:
> >>>>> Here I need to set the value of buffer to 1 or 0, we could
> >>>>> createbitfield, but if we want to set the value to non one or zero and
> >>>>> the BufferSize is large, how could we do? CreateByteField? It's a little
> >>>>> complex for user.
> >>>> that's what one would have to do writing it in ASL if bits
> >>>> are flipped on/off dynamically.
> >>>>
> >>>> In ASL you also can declare buffer with static initializer
> >>>>
> >>>>     Buffer (0x01) { 0x03 }
> >>>>
> >>>> and compiler is smart enough to set appropriate bits but it doesn't
> >>>> allow you to do so with large values. For example:
> >>>>
> >>>>     Buffer (0x01) { 0xAABBCCDD }
> >>>>
> >>>> gives error:
> >>>> Error 6139 - Constant out of range ^  (0xAABBCCDD, allowable: 0x0-0xFF)
> >>>>
> >>>> If one wants to manipulate specific fields in Buffer, ASL has
> >>>> a bunch of CreateFOOField operators, so lets follow spec and use
> >>>> API consistently to avoid confusion.
> >>>>
> >>>> BTW:
> >>>> packaging value as int (even without prefix) is wrong since
> >>>> its LE encoding will shuffle bytes and you won't get bits in
> >>>> positions that you expect if value is more than 1 byte.
> >>>
> >>> I don't care about ASL, we are writing in C
> >>> But AML is same:
> >>> DefBuffer := BufferOp PkgLength BufferSize ByteList
> >>> BufferOp := 0x11
> >>> BufferSize := TermArg => Integer
> >>>
> >>> So really just a bytelist.
> >>> We don't have any users for aml_buffer, maybe just add
> >>> const uint8_t *bytes, unsigned len as parameters.
> >>>
> >>
> >> Agree. It's consistent with the spec. If want to modify the value, could
> >> use CreateFOOField.
> >>
> >> So use following fuction to initialize Buffer?
> >>
> >> /* ACPI 1.0b: 16.2.5.4 Type 2 Opcodes Encoding: DefBuffer */
> >> Aml *aml_buffer(int buffer_size, uint8_t *byte_list)
> >> {
> >>      int i;
> >>      Aml *var = aml_bundle(0x11 /* BufferOp */, AML_BUFFER);
> >>      for (i = 0; i < buffer_size; i++) {
> >>          build_append_byte(var->buf, *(byte_list + i));
> >>      }
> >>      return var;
> >> }
> > maybe
> >
> > Aml *aml_buffer_initialized(int buffer_size, uint8_t *byte_list);
> > Aml *aml_buffer(int buffer_size);
> >
> > the second one is needed for implementing code like:
> > Name(BUFF, Buffer(4){}) // Create SerialBus data buffer as BUFF
> > CreateByteField(BUFF, 0x00, STAT) // STAT = Status (Byte)
> > CreateWordField(BUFF, 0x02, DATA) // DATA = Data (Byte)
> >
> > and could reuse aml_buffer_initialized() to reserve space.
> >
> 
> maybe we could use only one. For the uninitialized buffer we can pass 
> byte_list as NULL and within aml_buffer check byte_list, if byte_list is 
> NULL, just reserve space.
works for me with doc comment

> 
> /* ACPI 1.0b: 16.2.5.4 Type 2 Opcodes Encoding: DefBuffer */
> Aml *aml_buffer(int buffer_size, uint8_t *byte_list)
> {
>      int i;
>      Aml *var = aml_bundle(0x11 /* BufferOp */, AML_BUFFER);
> 
>      for (i = 0; i < buffer_size; i++) {
>          if (byte_list == NULL)
> 	    build_append_byte(var->buf, 0);
>          else
>              build_append_byte(var->buf, *(byte_list + i));
>     }
> 
>     return var;
> }
> 
> >>
> >>> Would that be enough?
> >>>
> >>>
> >>>>>
> >>>>>> ifctx1 = aml_if(aml_equal(aml_arg(2), aml_int(0)));
> >>>>>> aml_append(ifctx1, aml_store(aml_name("FNEN", aml_int(1)));
> >>>>>> ...
> >>>>>> /* create bit field for every supported function if supported */
> >>>>>> ...
> >>>>>> aml_append(method, aml_return(aml_name("RET")));
> >>>>>>
> >>>>>>
> >>>>>>>
> >>>>>>>>>
> >>>>>>>>>> +    aml_append(ifctx1, aml_return(buf));
> >>>>>>>>>> +    aml_append(ifctx, ifctx1);
> >>>>>>>>>> +    aml_append(method, ifctx);
> >>>>>>>>>> +
> >>>>>>>>>> +    buf = aml_buffer();
> >>>>>>>>>> +    build_append_int_noprefix(buf->buf, 0x00, 1);
> >>>>>>>>>> +    aml_append(method, aml_return(buf));
> >>>>>>>>>> +    aml_append(dev, method);
> >>>>>>>>>> +
> >>>>>>>>>> +    Aml *dev_rp0 = aml_device("%s", "RP0");
> >>>>>>>>>> +    aml_append(dev_rp0, aml_name_decl("_ADR", aml_int(0)));
> >>>>>>>>>> +    aml_append(dev, dev_rp0);
> >>>>>>>>>> +    aml_append(scope, dev);
> >>>>>>>>>> +}
> >>>>>>>>>> +
> >>>>>>>>>>   /* RSDP */
> >>>>>>>>>>   static GArray *
> >>>>>>>>>>   build_rsdp(GArray *rsdp_table, GArray *linker, unsigned rsdt)
> >>>>>>>>>> @@ -318,6 +468,8 @@ build_dsdt(GArray *table_data, GArray *linker, VirtGuestInfo *guest_info)
> >>>>>>>>>>       acpi_dsdt_add_flash(scope, info->flash_memmap);
> >>>>>>>>>>       acpi_dsdt_add_virtio(scope, info->virtio_mmio_memmap,
> >>>>>>>>>>                info->virtio_mmio_irq, info->virtio_mmio_num);
> >>>>>>>>>> +    acpi_dsdt_add_pci(scope, guest_info->pcie_info);
> >>>>>>>>>> +
> >>>>>>>>>>       aml_append(dsdt, scope);
> >>>>>>>>>>
> >>>>>>>>>>       /* copy AML table into ACPI tables blob and patch header there */
> >>>>>>>>
> >>>>>>>> .
> >>>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>>
> >>>>>>
> >>>>>> .
> >>>>>>
> >>>>>
> >>>>>
> >>>
> >>> .
> >>>
> >>
> >>
> >
diff mbox

Patch

diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index 85e8242..ceec405 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -49,6 +49,8 @@ 
 #include "qapi/qmp/qint.h"
 #include "qom/qom-qobject.h"
 #include "exec/ram_addr.h"
+#include "hw/pci/pcie_host.h"
+#include "hw/pci/pci.h"
 
 typedef struct VirtAcpiCpuInfo {
     DECLARE_BITMAP(found_cpus, VIRT_ACPI_CPU_ID_LIMIT);
@@ -160,6 +162,154 @@  static void acpi_dsdt_add_virtio(Aml *scope, const MemMap *virtio_mmio_memmap,
     }
 }
 
+static void acpi_dsdt_add_pci(Aml *scope, AcpiPcieInfo *info)
+{
+    Aml *method, *crs, *ifctx, *UUID, *ifctx1, *elsectx, *buf;
+    int i, bus_no;
+    int irq = *info->pcie_irq + 32;
+
+    Aml *dev = aml_device("%s", "PCI0");
+    aml_append(dev, aml_name_decl("_HID", aml_string("PNP0A08")));
+    aml_append(dev, aml_name_decl("_CID", aml_string("PNP0A03")));
+    aml_append(dev, aml_name_decl("_SEG", aml_int(0)));
+    aml_append(dev, aml_name_decl("_BBN", aml_int(0)));
+    aml_append(dev, aml_name_decl("_ADR", aml_int(0)));
+    aml_append(dev, aml_name_decl("_UID", aml_string("PCI0")));
+    aml_append(dev, aml_name_decl("_STR", aml_string("PCIe 0 Device")));
+
+    /* Declare the PCI Routing Table. */
+    Aml *rt_pkg = aml_package(info->nr_pcie_buses * PCI_NUM_PINS);
+    for (bus_no = 0; bus_no < info->nr_pcie_buses; bus_no++) {
+        for (i = 0; i < PCI_NUM_PINS; i++) {
+            int gsi = (i + bus_no) % PCI_NUM_PINS;
+            Aml *pkg = aml_package(4);
+            aml_append(pkg, aml_int((bus_no << 16) | 0xFFFF));
+            aml_append(pkg, aml_int(i));
+            aml_append(pkg, aml_name("GSI%d", gsi));
+            aml_append(pkg, aml_int(0));
+            aml_append(rt_pkg, pkg);
+        }
+    }
+    aml_append(dev, aml_name_decl("_PRT", rt_pkg));
+
+    /* Create GSI link device */
+    for (i = 0; i < PCI_NUM_PINS; i++) {
+        Aml *dev_gsi = aml_device("GSI%d", i);
+        aml_append(dev_gsi, aml_name_decl("_HID", aml_string("PNP0C0F")));
+        aml_append(dev_gsi, aml_name_decl("_UID", aml_int(0)));
+        crs = aml_resource_template();
+        aml_append(crs,
+                   aml_interrupt(aml_consumer, aml_level, aml_active_high,
+                   aml_exclusive, aml_not_wake_capable, irq + i));
+        aml_append(dev_gsi, aml_name_decl("_PRS", crs));
+        crs = aml_resource_template();
+        aml_append(crs,
+                   aml_interrupt(aml_consumer, aml_level, aml_active_high,
+                   aml_exclusive, aml_not_wake_capable, irq + i));
+        aml_append(dev_gsi, aml_name_decl("_CRS", crs));
+        method = aml_method("_SRS", 1);
+        aml_append(dev_gsi, method);
+        aml_append(dev, dev_gsi);
+    }
+
+    method = aml_method("_CBA", 0);
+    aml_append(method, aml_return(aml_int(info->pcie_ecam.addr)));
+    aml_append(dev, method);
+
+    method = aml_method("_CRS", 0);
+    Aml *rbuf = aml_resource_template();
+    aml_append(rbuf,
+        aml_word_bus_number(aml_min_fixed, aml_max_fixed, aml_pos_decode,
+                            0x0000, 0x0000, info->nr_pcie_buses - 1,
+                            0x0000, info->nr_pcie_buses));
+    aml_append(rbuf,
+        aml_dword_memory(aml_pos_decode, aml_min_fixed, aml_max_fixed,
+                         aml_non_cacheable, aml_ReadWrite,
+                         0x0000, info->pcie_mmio.addr,
+                         info->pcie_mmio.addr + info->pcie_mmio.size - 1,
+                         0x0000, info->pcie_mmio.size));
+    aml_append(rbuf,
+        aml_dword_io(aml_min_fixed, aml_max_fixed,
+                     aml_pos_decode, aml_entire_range,
+                     0x0000, 0x0000, info->pcie_ioport.size - 1,
+                     info->pcie_ioport.addr, info->pcie_ioport.size));
+
+    aml_append(method, aml_name_decl("RBUF", rbuf));
+    aml_append(method, aml_return(rbuf));
+    aml_append(dev, method);
+
+    /* Declare an _OSC (OS Control Handoff) method */
+    aml_append(dev, aml_name_decl("SUPP", aml_int(0)));
+    aml_append(dev, aml_name_decl("CTRL", aml_int(0)));
+    method = aml_method("_OSC", 4);
+    aml_append(method,
+        aml_create_dword_field(aml_arg(3), aml_int(0), "CDW1"));
+
+    /* PCI Firmware Specification 3.0
+     * 4.5.1. _OSC Interface for PCI Host Bridge Devices
+     * The _OSC interface for a PCI/PCI-X/PCI Express hierarchy is
+     * identified by the Universal Unique IDentifier (UUID)
+     * 33db4d5b-1ff7-401c-9657-7441c03dd766
+     */
+    UUID = aml_touuid("33DB4D5B-1FF7-401C-9657-7441C03DD766");
+    ifctx = aml_if(aml_equal(aml_arg(0), UUID));
+    aml_append(ifctx,
+        aml_create_dword_field(aml_arg(3), aml_int(4), "CDW2"));
+    aml_append(ifctx,
+        aml_create_dword_field(aml_arg(3), aml_int(8), "CDW3"));
+    aml_append(ifctx, aml_store(aml_name("CDW2"), aml_name("SUPP")));
+    aml_append(ifctx, aml_store(aml_name("CDW3"), aml_name("CTRL")));
+    aml_append(ifctx, aml_store(aml_and(aml_name("CTRL"), aml_int(0x1D)),
+                                aml_name("CTRL")));
+
+    ifctx1 = aml_if(aml_not(aml_equal(aml_arg(1), aml_int(0x1))));
+    aml_append(ifctx1, aml_store(aml_or(aml_name("CDW1"), aml_int(0x08)),
+                                 aml_name("CDW1")));
+    aml_append(ifctx, ifctx1);
+
+    ifctx1 = aml_if(aml_not(aml_equal(aml_name("CDW3"), aml_name("CTRL"))));
+    aml_append(ifctx1, aml_store(aml_or(aml_name("CDW1"), aml_int(0x10)),
+                                 aml_name("CDW1")));
+    aml_append(ifctx, ifctx1);
+
+    aml_append(ifctx, aml_store(aml_name("CTRL"), aml_name("CDW3")));
+    aml_append(ifctx, aml_return(aml_arg(3)));
+    aml_append(method, ifctx);
+
+    elsectx = aml_else();
+    aml_append(elsectx, aml_store(aml_or(aml_name("CDW1"), aml_int(4)),
+                                  aml_name("CDW1")));
+    aml_append(elsectx, aml_return(aml_arg(3)));
+    aml_append(method, elsectx);
+    aml_append(dev, method);
+
+    method = aml_method("_DSM", 4);
+
+    /* PCI Firmware Specification 3.0
+     * 4.6.1. _DSM for PCI Express Slot Information
+     * The UUID in _DSM in this context is
+     * {E5C937D0-3553-4d7a-9117-EA4D19C3434D}
+     */
+    UUID = aml_touuid("E5C937D0-3553-4d7a-9117-EA4D19C3434D");
+    ifctx = aml_if(aml_equal(aml_arg(0), UUID));
+    ifctx1 = aml_if(aml_equal(aml_arg(2), aml_int(0)));
+    buf = aml_buffer();
+    build_append_int_noprefix(buf->buf, 0x01, 1);
+    aml_append(ifctx1, aml_return(buf));
+    aml_append(ifctx, ifctx1);
+    aml_append(method, ifctx);
+
+    buf = aml_buffer();
+    build_append_int_noprefix(buf->buf, 0x00, 1);
+    aml_append(method, aml_return(buf));
+    aml_append(dev, method);
+
+    Aml *dev_rp0 = aml_device("%s", "RP0");
+    aml_append(dev_rp0, aml_name_decl("_ADR", aml_int(0)));
+    aml_append(dev, dev_rp0);
+    aml_append(scope, dev);
+}
+
 /* RSDP */
 static GArray *
 build_rsdp(GArray *rsdp_table, GArray *linker, unsigned rsdt)
@@ -318,6 +468,8 @@  build_dsdt(GArray *table_data, GArray *linker, VirtGuestInfo *guest_info)
     acpi_dsdt_add_flash(scope, info->flash_memmap);
     acpi_dsdt_add_virtio(scope, info->virtio_mmio_memmap,
              info->virtio_mmio_irq, info->virtio_mmio_num);
+    acpi_dsdt_add_pci(scope, guest_info->pcie_info);
+
     aml_append(dsdt, scope);
 
     /* copy AML table into ACPI tables blob and patch header there */