[Qemu-devel] [PATCH] hw/arm/allwinner-a10: Add the 'A' SRAM and the SRAM controller

Philippe Mathieu-Daudé posted 1 patch 5 years, 3 months ago
Test checkpatch passed
Test docker-quick@centos7 passed
Test docker-clang@ubuntu passed
Test docker-mingw@fedora passed
Test asan passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20181209193742.7582-1-f4bug@amsat.org
There is a newer version of this series
hw/arm/allwinner-a10.c         | 5 +++++
include/hw/arm/allwinner-a10.h | 1 +
2 files changed, 6 insertions(+)
[Qemu-devel] [PATCH] hw/arm/allwinner-a10: Add the 'A' SRAM and the SRAM controller
Posted by Philippe Mathieu-Daudé 5 years, 3 months ago
From the "A10 User Manual V1.20" p.29: "3.2. Memory Mapping" and:

 7. System Control
  7.1. Overview

  A10 embeds a high-speed SRAM which has been split into five segments.
  See detailed memory mapping in following table:

  Area          Address        Size (Bytes)
   A1    0x00000000-0x00003FFF 16K
   A2    0x00004000-0x00007FFF 16K
   A3    0x00008000-0x0000B3FF 13K
   A4    0x0000B400-0x0000BFFF  3K

Since for emulation purpose we don't need the segmentations, we simply define
the 'A' area as a single 48KB SRAM.

We don't implement the following others areas:
- 'B': 'Secure RAM' (64K),
- 'C': Debug/ISP SRAM
- 'D': USB SRAM

(qemu) info mtree
address-space: memory
  0000000000000000-ffffffffffffffff (prio 0, i/o): system
    0000000000000000-000000000000bfff (prio 0, ram): sram A
    0000000001c00000-0000000001c00fff (prio -1000, i/o): a10-sram-ctrl
    0000000001c0b000-0000000001c0bfff (prio 0, i/o): aw_emac
    0000000001c18000-0000000001c18fff (prio 0, i/o): ahci
      0000000001c18080-0000000001c180ff (prio 0, i/o): allwinner-ahci
    0000000001c20400-0000000001c207ff (prio 0, i/o): allwinner-a10-pic
    0000000001c20c00-0000000001c20fff (prio 0, i/o): allwinner-A10-timer
    0000000001c28000-0000000001c2801f (prio 0, i/o): serial
    0000000040000000-0000000047ffffff (prio 0, ram): cubieboard.ram

Reported-by: Charlie Smurthwaite <charlie@atech.media>
Tested-by: Charlie Smurthwaite <charlie@atech.media>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
There is an unmerged Linux patch where this SRAM seems used by the EMAC:
https://patchwork.kernel.org/patch/6515451/

 hw/arm/allwinner-a10.c         | 5 +++++
 include/hw/arm/allwinner-a10.h | 1 +
 2 files changed, 6 insertions(+)

diff --git a/hw/arm/allwinner-a10.c b/hw/arm/allwinner-a10.c
index 9fe875cdb5..0bb0aa3b64 100644
--- a/hw/arm/allwinner-a10.c
+++ b/hw/arm/allwinner-a10.c
@@ -22,6 +22,7 @@
 #include "hw/sysbus.h"
 #include "hw/devices.h"
 #include "hw/arm/allwinner-a10.h"
+#include "hw/misc/unimp.h"
 
 static void aw_a10_init(Object *obj)
 {
@@ -85,6 +86,10 @@ static void aw_a10_realize(DeviceState *dev, Error **errp)
     sysbus_connect_irq(sysbusdev, 4, s->irq[67]);
     sysbus_connect_irq(sysbusdev, 5, s->irq[68]);
 
+    memory_region_init_ram(&s->sram_a, NULL, "sram A", 48 * KiB, &error_fatal);
+    memory_region_add_subregion(get_system_memory(), 0x00000000, &s->sram_a);
+    create_unimplemented_device("a10-sram-ctrl", 0x01c00000, 4 * KiB);
+
     /* FIXME use qdev NIC properties instead of nd_table[] */
     if (nd_table[0].used) {
         qemu_check_nic_model(&nd_table[0], TYPE_AW_EMAC);
diff --git a/include/hw/arm/allwinner-a10.h b/include/hw/arm/allwinner-a10.h
index efb8fc8123..389e128d0f 100644
--- a/include/hw/arm/allwinner-a10.h
+++ b/include/hw/arm/allwinner-a10.h
@@ -35,6 +35,7 @@ typedef struct AwA10State {
     AwA10PICState intc;
     AwEmacState emac;
     AllwinnerAHCIState sata;
+    MemoryRegion sram_a;
 } AwA10State;
 
 #define ALLWINNER_H_
-- 
2.17.2


Re: [Qemu-devel] [PATCH] hw/arm/allwinner-a10: Add the 'A' SRAM and the SRAM controller
Posted by Peter Maydell 5 years, 2 months ago
On Sun, 9 Dec 2018 at 19:37, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> From the "A10 User Manual V1.20" p.29: "3.2. Memory Mapping" and:
>
>  7. System Control
>   7.1. Overview
>
>   A10 embeds a high-speed SRAM which has been split into five segments.
>   See detailed memory mapping in following table:
>
>   Area          Address        Size (Bytes)
>    A1    0x00000000-0x00003FFF 16K
>    A2    0x00004000-0x00007FFF 16K
>    A3    0x00008000-0x0000B3FF 13K
>    A4    0x0000B400-0x0000BFFF  3K
>
> Since for emulation purpose we don't need the segmentations, we simply define
> the 'A' area as a single 48KB SRAM.

>  static void aw_a10_init(Object *obj)
>  {
> @@ -85,6 +86,10 @@ static void aw_a10_realize(DeviceState *dev, Error **errp)
>      sysbus_connect_irq(sysbusdev, 4, s->irq[67]);
>      sysbus_connect_irq(sysbusdev, 5, s->irq[68]);
>
> +    memory_region_init_ram(&s->sram_a, NULL, "sram A", 48 * KiB, &error_fatal);

This creates the memory region with a NULL owner, which is
OK for board model code, but since we're a device object
here we should use OBJECT(dev) as the owner.

This is more important if the region creation happens
in the instance_init function, in which case it will
result in a leak of the memory region after the device
is destroyed (see for instance commit 09d98b69804cfd9e),
but it's worth getting right here anyway.

> +    memory_region_add_subregion(get_system_memory(), 0x00000000, &s->sram_a);
> +    create_unimplemented_device("a10-sram-ctrl", 0x01c00000, 4 * KiB);
> +
>      /* FIXME use qdev NIC properties instead of nd_table[] */
>      if (nd_table[0].used) {
>          qemu_check_nic_model(&nd_table[0], TYPE_AW_EMAC);

thanks
-- PMM

Re: [Qemu-devel] [PATCH] hw/arm/allwinner-a10: Add the 'A' SRAM and the SRAM controller
Posted by Philippe Mathieu-Daudé 5 years, 2 months ago
On Fri, Jan 4, 2019 at 3:10 PM Peter Maydell <peter.maydell@linaro.org> wrote:
> On Sun, 9 Dec 2018 at 19:37, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> > From the "A10 User Manual V1.20" p.29: "3.2. Memory Mapping" and:
> >
> >  7. System Control
> >   7.1. Overview
> >
> >   A10 embeds a high-speed SRAM which has been split into five segments.
> >   See detailed memory mapping in following table:
> >
> >   Area          Address        Size (Bytes)
> >    A1    0x00000000-0x00003FFF 16K
> >    A2    0x00004000-0x00007FFF 16K
> >    A3    0x00008000-0x0000B3FF 13K
> >    A4    0x0000B400-0x0000BFFF  3K
> >
> > Since for emulation purpose we don't need the segmentations, we simply define
> > the 'A' area as a single 48KB SRAM.
>
> >  static void aw_a10_init(Object *obj)
> >  {
> > @@ -85,6 +86,10 @@ static void aw_a10_realize(DeviceState *dev, Error **errp)
> >      sysbus_connect_irq(sysbusdev, 4, s->irq[67]);
> >      sysbus_connect_irq(sysbusdev, 5, s->irq[68]);
> >
> > +    memory_region_init_ram(&s->sram_a, NULL, "sram A", 48 * KiB, &error_fatal);
>
> This creates the memory region with a NULL owner, which is
> OK for board model code, but since we're a device object
> here we should use OBJECT(dev) as the owner.
>
> This is more important if the region creation happens
> in the instance_init function, in which case it will
> result in a leak of the memory region after the device
> is destroyed (see for instance commit 09d98b69804cfd9e),
> but it's worth getting right here anyway.

Thanks, good to know.
I'll fix.
Regards,

Phil.

> > +    memory_region_add_subregion(get_system_memory(), 0x00000000, &s->sram_a);
> > +    create_unimplemented_device("a10-sram-ctrl", 0x01c00000, 4 * KiB);
> > +
> >      /* FIXME use qdev NIC properties instead of nd_table[] */
> >      if (nd_table[0].used) {
> >          qemu_check_nic_model(&nd_table[0], TYPE_AW_EMAC);
>
> thanks
> -- PMM