[Qemu-devel] [PATCH] hw/arm/virt-acpi-build: Fix wrong size of flash

[Qemu-devel] [PATCH] hw/arm/virt-acpi-build: Fix wrong size of flash

[shannon.zhao]

From: Shannon Zhao <shannon.zhao@linaro.org>

While virt machine creates two flash devices with total size 0x08000000,
it wrongly uses this total size for each one. So it will overlap other
MMIO spaces.

Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
---
 hw/arm/virt-acpi-build.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index 2073573..bc858c8 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -114,7 +114,7 @@ static void acpi_dsdt_add_flash(Aml *scope, const MemMapEntry *flash_memmap)
 {
     Aml *dev, *crs;
     hwaddr base = flash_memmap->base;
-    hwaddr size = flash_memmap->size;
+    hwaddr size = flash_memmap->size / 2;
 
     dev = aml_device("FLS0");
     aml_append(dev, aml_name_decl("_HID", aml_string("LNRO0015")));
-- 
2.1.0

Re: [Qemu-devel] [PATCH] hw/arm/virt-acpi-build: Fix wrong size of flash

[G Gregory]

This is an urgent fix as it completely breaks booting with ACPI.
Success is only a matter of luck with device probing order.

Tested-by: Graeme Gregory <graeme.gregory@linaro.org>

Graeme

On 17 September 2015 at 02:57,  <shannon.zhao@linaro.org> wrote:
> From: Shannon Zhao <shannon.zhao@linaro.org>
>
> While virt machine creates two flash devices with total size 0x08000000,
> it wrongly uses this total size for each one. So it will overlap other
> MMIO spaces.
>
> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
> ---
>  hw/arm/virt-acpi-build.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index 2073573..bc858c8 100644
> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -114,7 +114,7 @@ static void acpi_dsdt_add_flash(Aml *scope, const MemMapEntry *flash_memmap)
>  {
>      Aml *dev, *crs;
>      hwaddr base = flash_memmap->base;
> -    hwaddr size = flash_memmap->size;
> +    hwaddr size = flash_memmap->size / 2;
>
>      dev = aml_device("FLS0");
>      aml_append(dev, aml_name_decl("_HID", aml_string("LNRO0015")));
> --
> 2.1.0
>

Re: [Qemu-devel] [PATCH] hw/arm/virt-acpi-build: Fix wrong size of flash

[Andrew Jones]

On Thu, Sep 17, 2015 at 09:57:21AM +0800, shannon.zhao@linaro.org wrote:
> From: Shannon Zhao <shannon.zhao@linaro.org>
> 
> While virt machine creates two flash devices with total size 0x08000000,
> it wrongly uses this total size for each one. So it will overlap other
> MMIO spaces.
> 
> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
> ---
>  hw/arm/virt-acpi-build.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index 2073573..bc858c8 100644
> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -114,7 +114,7 @@ static void acpi_dsdt_add_flash(Aml *scope, const MemMapEntry *flash_memmap)
>  {
>      Aml *dev, *crs;
>      hwaddr base = flash_memmap->base;
> -    hwaddr size = flash_memmap->size;
> +    hwaddr size = flash_memmap->size / 2;
>  
>      dev = aml_device("FLS0");
>      aml_append(dev, aml_name_decl("_HID", aml_string("LNRO0015")));
> -- 
> 2.1.0
> 
>

A sentence in the commit message saying that the DT generation also
gives each device one half the total size would be nice

Reviewed-by: Andrew Jones <drjones@redhat.com>

Re: [Qemu-devel] [PATCH] hw/arm/virt-acpi-build: Fix wrong size of flash

[Wei Huang]

On 09/16/2015 08:57 PM, shannon.zhao@linaro.org wrote:
> From: Shannon Zhao <shannon.zhao@linaro.org>
> 
> While virt machine creates two flash devices with total size 0x08000000,
> it wrongly uses this total size for each one. So it will overlap other
> MMIO spaces.
> 
> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
> ---
>  hw/arm/virt-acpi-build.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index 2073573..bc858c8 100644
> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -114,7 +114,7 @@ static void acpi_dsdt_add_flash(Aml *scope, const MemMapEntry *flash_memmap)
>  {
>      Aml *dev, *crs;
>      hwaddr base = flash_memmap->base;
> -    hwaddr size = flash_memmap->size;
> +    hwaddr size = flash_memmap->size / 2;
>  
>      dev = aml_device("FLS0");
>      aml_append(dev, aml_name_decl("_HID", aml_string("LNRO0015")));
> 

In current code, it looks like both FLS0 and FLS1 are using up the whole
space. That would invade into the VIRT_CPUPERIPHS address space, which
apparently is wrong.

Reviewed-by: Wei Huang <wei@redhat.com>

Re: [Qemu-devel] [PATCH] hw/arm/virt-acpi-build: Fix wrong size of flash

[Peter Maydell]

On 17 September 2015 at 02:57,  <shannon.zhao@linaro.org> wrote:
> From: Shannon Zhao <shannon.zhao@linaro.org>
>
> While virt machine creates two flash devices with total size 0x08000000,
> it wrongly uses this total size for each one. So it will overlap other
> MMIO spaces.
>
> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>

Applied to target-arm.next, thanks.

Andrew suggested an update to the commit message, so I have edited it:

    hw/arm/virt-acpi-build: Fix wrong size of flash in ACPI table

    While virt machine creates two flash devices with total size 0x08000000,
    the ACPI table generation code was wrongly using this total size as the
    size of each flash device, so it would overlap other MMIO spaces.
    Make each device entry in the table half the total; this brings the
    ACPI table into line with the code which generates the device tree
    and which creates the flash devices themselves.

thanks
-- PMM