qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v5 19/20] hw/arm/virt-acpi-build: Add PCIe contr


From: Igor Mammedov
Subject: Re: [Qemu-devel] [PATCH v5 19/20] hw/arm/virt-acpi-build: Add PCIe controller in ACPI DSDT table
Date: Tue, 28 Apr 2015 17:13:10 +0200

On Tue, 28 Apr 2015 20:57:25 +0800
Shannon Zhao <address@hidden> wrote:

> On 2015/4/28 17:54, Igor Mammedov wrote:
> > On Tue, 28 Apr 2015 17:06:00 +0800
> > Shannon Zhao <address@hidden> 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 <address@hidden> wrote:
> >>>>
> >>>>> From: Shannon Zhao <address@hidden>
> >>>>>
> >>>>> Add PCIe controller in ACPI DSDT table, so the guest can detect
> >>>>> the PCIe.
> >>>>>
> >>>>> Signed-off-by: Shannon Zhao <address@hidden>
> >>>>> Signed-off-by: Shannon Zhao <address@hidden>
> >>>>> ---
> >>>>>  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 */
> >>>
> >>> .
> >>>
> >>
> >>
> > 
> > 
> > .
> > 
> 
> 




reply via email to

[Prev in Thread] Current Thread [Next in Thread]