Re: [Qemu-devel] [PATCH v4 00/20] Generate ACPI v5.1 tables and expose it to guest over fw_cfg on ARM

[Shannon Zhao <zhaoshenglong@huawei.com>]

On 2015/4/7 17:19, Peter Maydell wrote:
> On 7 April 2015 at 03:43, Shannon Zhao <zhaoshenglong@huawei.com> wrote:
>> The dts node is:
>>                 ranges = <0x1000000 0x0 0x0 0x0 0x3eff0000 0x0 0x10000
>>                           0x2000000 0x0 0x10000000 0x0 0x10000000 0x0 0x2eff0000>;
>>                 reg = <0x0 0x3f000000 0x0 0x1000000>;
>>                 bus-range = <0x0 0xf>;
>>
>> The ACPI table entry:
>>             Method (_CBA, 0, NotSerialized)  // _CBA: Configuration Base Address
>>             {
>>                 Return (0x3F000000)
>>             }
>>             Method (_CRS, 0, NotSerialized)  // _CRS: Current Resource Settings
>>             {
>>                 Name (RBUF, ResourceTemplate ()
>>                 {
>>                     WordBusNumber (ResourceProducer, MinFixed, MaxFixed, PosDecode,
>>                         0x0000,             // Granularity
>>                         0x0000,             // Range Minimum
>>                         0x000F,             // Range Maximum
>>                         0x0000,             // Translation Offset
>>                         0x0010,             // Length
>>                         ,, )
>>                     DWordMemory (ResourceProducer, PosDecode, MinFixed, MaxFixed, Cacheable, ReadWrite,
> 
> Is this claiming that the non-cacheable PCI MMIO region is cacheable?
> If so that isn't right...

ok, will fix this.

> 
>>                         0x00000000,         // Granularity
>>                         0x10000000,         // Range Minimum
>>                         0x3EFF0000,         // Range Maximum
>>                         0x00000000,         // Translation Offset
>>                         0x2EFF0000,         // Length
>>                         ,, , AddressRangeMemory, TypeStatic)
>>                     DWordIO (ResourceProducer, MinFixed, MaxFixed, PosDecode, EntireRange,
>>                         0x00000000,         // Granularity
>>                         0x3EFF0000,         // Range Minimum
>>                         0x3F000000,         // Range Maximum
>>                         0x00000000,         // Translation Offset
> 
> I rather suspect this is wrong, since (my guess without looking
> at the spec) it looks like it defines a 1:1 mapping between
> the addresses used to interact with the PCIe IO window and
> the IO addresses, which is obviously not what you want.
> My guess is you need to set the translation offset at least,
> but check the spec.
> 

Yes, it's not right. I think it should be the following:

DWordIO (ResourceProducer, MinFixed, MaxFixed, PosDecode, EntireRange,
    0x00000000,         // Granularity
    0x00000000,         // Range Minimum
    0x0000FFFF,         // Range Maximum
    0x3EFF0000,         // Translation Offset
    0x00010000,         // Length
    ,, , TypeStatic)


With following patch on top of this patchset, the guest with e1000 and virtio-pci works well.

diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index 11574c9..faa5042 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -236,16 +236,15 @@ static void acpi_dsdt_add_pci(Aml *scope, acpi_pcie_info *info)
                             0x0000, info->nr_pcie_buses));
     aml_append(rbuf,
         aml_dword_memory(aml_pos_decode, aml_min_fixed, aml_max_fixed,
-                         aml_cacheable, aml_ReadWrite,
+                         aml_non_cacheable, aml_ReadWrite,
                          0x0000, info->pcie_mmio_base,
                          info->pcie_mmio_base + 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, info->pcie_ioport_base,
-                     info->pcie_ioport_base + info->pcie_ioport_size - 1,
-                     0x0000, info->pcie_ioport_size));
+                     0x0000, 0x0000,info->pcie_ioport_size - 1,
+                     info->pcie_ioport_base, info->pcie_ioport_size));


Thanks,
Shannon