Re: [PATCH v5 15/17] aspeed/soc: fix incorrect dram size for AST2700

2024-06-03 Thread Cédric Le Goater

On 6/4/24 07:44, Jamin Lin wrote:

AST2700 dram size calculation is not back compatible AST2600.
According to the DDR capacity hardware behavior,
if users write the data to the address which is beyond the ram size,
it would write the data to the "address % ram_size".
For example:
a. sdram base address "0x4 "
b. sdram size 1 GiB
The available address range is from "0x4 " to "0x4 3FFF".
If users write 0x12345678 to address "0x5 ",
the value of DRAM address 0 (base address 0x4 ) will be 0x12345678.

Add aspeed_soc_ast2700_dram_init to calculate the dram size and add
memory I/O whose address range is from "max_ram_size - ram_size" to max_ram_size
and its read/write handler to emulate DDR capacity hardware behavior.

Signed-off-by: Troy Lee 
Signed-off-by: Jamin Lin 



Reviewed-by: Cédric Le Goater 

Thanks,

C.



---
  hw/arm/aspeed_ast27x0.c | 87 -
  include/hw/arm/aspeed_soc.h |  2 +
  2 files changed, 88 insertions(+), 1 deletion(-)

diff --git a/hw/arm/aspeed_ast27x0.c b/hw/arm/aspeed_ast27x0.c
index 29e75072c4..b6876b4862 100644
--- a/hw/arm/aspeed_ast27x0.c
+++ b/hw/arm/aspeed_ast27x0.c
@@ -20,6 +20,7 @@
  #include "sysemu/sysemu.h"
  #include "hw/intc/arm_gicv3.h"
  #include "qapi/qmp/qlist.h"
+#include "qemu/log.h"
  
  static const hwaddr aspeed_soc_ast2700_memmap[] = {

  [ASPEED_DEV_SPI_BOOT]  =  0x4,
@@ -191,6 +192,90 @@ static qemu_irq aspeed_soc_ast2700_get_irq(AspeedSoCState 
*s, int dev)
  return qdev_get_gpio_in(DEVICE(&a->gic), sc->irqmap[dev]);
  }
  
+static uint64_t aspeed_ram_capacity_read(void *opaque, hwaddr addr,

+unsigned int size)
+{
+qemu_log_mask(LOG_GUEST_ERROR,
+  "%s: DRAM read out of ram size, addr:0x%" PRIx64 "\n",
+   __func__, addr);
+return 0;
+}
+
+static void aspeed_ram_capacity_write(void *opaque, hwaddr addr, uint64_t data,
+unsigned int size)
+{
+AspeedSoCState *s = ASPEED_SOC(opaque);
+ram_addr_t ram_size;
+MemTxResult result;
+
+ram_size = object_property_get_uint(OBJECT(&s->sdmc), "ram-size",
+&error_abort);
+
+/*
+ * Emulate ddr capacity hardware behavior.
+ * If writes the data to the address which is beyond the ram size,
+ * it would write the data to the "address % ram_size".
+ */
+result = address_space_write(&s->dram_as, addr % ram_size,
+ MEMTXATTRS_UNSPECIFIED, &data, 4);
+if (result != MEMTX_OK) {
+qemu_log_mask(LOG_GUEST_ERROR,
+  "%s: DRAM write failed, addr:0x%" HWADDR_PRIx
+  ", data :0x%" PRIx64  "\n",
+  __func__, addr % ram_size, data);
+}
+}
+
+static const MemoryRegionOps aspeed_ram_capacity_ops = {
+.read = aspeed_ram_capacity_read,
+.write = aspeed_ram_capacity_write,
+.endianness = DEVICE_LITTLE_ENDIAN,
+.valid = {
+.min_access_size = 1,
+.max_access_size = 8,
+},
+};
+
+/*
+ * SDMC should be realized first to get correct RAM size and max size
+ * values
+ */
+static bool aspeed_soc_ast2700_dram_init(DeviceState *dev, Error **errp)
+{
+ram_addr_t ram_size, max_ram_size;
+Aspeed27x0SoCState *a = ASPEED27X0_SOC(dev);
+AspeedSoCState *s = ASPEED_SOC(dev);
+AspeedSoCClass *sc = ASPEED_SOC_GET_CLASS(s);
+
+ram_size = object_property_get_uint(OBJECT(&s->sdmc), "ram-size",
+&error_abort);
+max_ram_size = object_property_get_uint(OBJECT(&s->sdmc), "max-ram-size",
+&error_abort);
+
+memory_region_init(&s->dram_container, OBJECT(s), "ram-container",
+   ram_size);
+memory_region_add_subregion(&s->dram_container, 0, s->dram_mr);
+address_space_init(&s->dram_as, s->dram_mr, "dram");
+
+/*
+ * Add a memory region beyond the RAM region to emulate
+ * ddr capacity hardware behavior.
+ */
+if (ram_size < max_ram_size) {
+memory_region_init_io(&a->dram_empty, OBJECT(s),
+  &aspeed_ram_capacity_ops, s,
+  "ram-empty", max_ram_size - ram_size);
+
+memory_region_add_subregion(s->memory,
+sc->memmap[ASPEED_DEV_SDRAM] + ram_size,
+&a->dram_empty);
+}
+
+memory_region_add_subregion(s->memory,
+  sc->memmap[ASPEED_DEV_SDRAM], &s->dram_container);
+return true;
+}
+
  static void aspeed_soc_ast2700_init(Object *obj)
  {
  Aspeed27x0SoCState *a = ASPEED27X0_SOC(obj);
@@ -461,7 +546,7 @@ static void aspeed_soc_ast2700_realize(DeviceState *dev, 
Error **errp)
  sc->memmap[ASPEED_DEV_SDMC]);
  
  /* RAM */

-if (!aspeed_soc_dram_init(s, errp)) {
+if (!aspeed

[PATCH v5 15/17] aspeed/soc: fix incorrect dram size for AST2700

2024-06-03 Thread Jamin Lin via
AST2700 dram size calculation is not back compatible AST2600.
According to the DDR capacity hardware behavior,
if users write the data to the address which is beyond the ram size,
it would write the data to the "address % ram_size".
For example:
a. sdram base address "0x4 "
b. sdram size 1 GiB
The available address range is from "0x4 " to "0x4 3FFF".
If users write 0x12345678 to address "0x5 ",
the value of DRAM address 0 (base address 0x4 ) will be 0x12345678.

Add aspeed_soc_ast2700_dram_init to calculate the dram size and add
memory I/O whose address range is from "max_ram_size - ram_size" to max_ram_size
and its read/write handler to emulate DDR capacity hardware behavior.

Signed-off-by: Troy Lee 
Signed-off-by: Jamin Lin 
---
 hw/arm/aspeed_ast27x0.c | 87 -
 include/hw/arm/aspeed_soc.h |  2 +
 2 files changed, 88 insertions(+), 1 deletion(-)

diff --git a/hw/arm/aspeed_ast27x0.c b/hw/arm/aspeed_ast27x0.c
index 29e75072c4..b6876b4862 100644
--- a/hw/arm/aspeed_ast27x0.c
+++ b/hw/arm/aspeed_ast27x0.c
@@ -20,6 +20,7 @@
 #include "sysemu/sysemu.h"
 #include "hw/intc/arm_gicv3.h"
 #include "qapi/qmp/qlist.h"
+#include "qemu/log.h"
 
 static const hwaddr aspeed_soc_ast2700_memmap[] = {
 [ASPEED_DEV_SPI_BOOT]  =  0x4,
@@ -191,6 +192,90 @@ static qemu_irq aspeed_soc_ast2700_get_irq(AspeedSoCState 
*s, int dev)
 return qdev_get_gpio_in(DEVICE(&a->gic), sc->irqmap[dev]);
 }
 
+static uint64_t aspeed_ram_capacity_read(void *opaque, hwaddr addr,
+unsigned int size)
+{
+qemu_log_mask(LOG_GUEST_ERROR,
+  "%s: DRAM read out of ram size, addr:0x%" PRIx64 "\n",
+   __func__, addr);
+return 0;
+}
+
+static void aspeed_ram_capacity_write(void *opaque, hwaddr addr, uint64_t data,
+unsigned int size)
+{
+AspeedSoCState *s = ASPEED_SOC(opaque);
+ram_addr_t ram_size;
+MemTxResult result;
+
+ram_size = object_property_get_uint(OBJECT(&s->sdmc), "ram-size",
+&error_abort);
+
+/*
+ * Emulate ddr capacity hardware behavior.
+ * If writes the data to the address which is beyond the ram size,
+ * it would write the data to the "address % ram_size".
+ */
+result = address_space_write(&s->dram_as, addr % ram_size,
+ MEMTXATTRS_UNSPECIFIED, &data, 4);
+if (result != MEMTX_OK) {
+qemu_log_mask(LOG_GUEST_ERROR,
+  "%s: DRAM write failed, addr:0x%" HWADDR_PRIx
+  ", data :0x%" PRIx64  "\n",
+  __func__, addr % ram_size, data);
+}
+}
+
+static const MemoryRegionOps aspeed_ram_capacity_ops = {
+.read = aspeed_ram_capacity_read,
+.write = aspeed_ram_capacity_write,
+.endianness = DEVICE_LITTLE_ENDIAN,
+.valid = {
+.min_access_size = 1,
+.max_access_size = 8,
+},
+};
+
+/*
+ * SDMC should be realized first to get correct RAM size and max size
+ * values
+ */
+static bool aspeed_soc_ast2700_dram_init(DeviceState *dev, Error **errp)
+{
+ram_addr_t ram_size, max_ram_size;
+Aspeed27x0SoCState *a = ASPEED27X0_SOC(dev);
+AspeedSoCState *s = ASPEED_SOC(dev);
+AspeedSoCClass *sc = ASPEED_SOC_GET_CLASS(s);
+
+ram_size = object_property_get_uint(OBJECT(&s->sdmc), "ram-size",
+&error_abort);
+max_ram_size = object_property_get_uint(OBJECT(&s->sdmc), "max-ram-size",
+&error_abort);
+
+memory_region_init(&s->dram_container, OBJECT(s), "ram-container",
+   ram_size);
+memory_region_add_subregion(&s->dram_container, 0, s->dram_mr);
+address_space_init(&s->dram_as, s->dram_mr, "dram");
+
+/*
+ * Add a memory region beyond the RAM region to emulate
+ * ddr capacity hardware behavior.
+ */
+if (ram_size < max_ram_size) {
+memory_region_init_io(&a->dram_empty, OBJECT(s),
+  &aspeed_ram_capacity_ops, s,
+  "ram-empty", max_ram_size - ram_size);
+
+memory_region_add_subregion(s->memory,
+sc->memmap[ASPEED_DEV_SDRAM] + ram_size,
+&a->dram_empty);
+}
+
+memory_region_add_subregion(s->memory,
+  sc->memmap[ASPEED_DEV_SDRAM], &s->dram_container);
+return true;
+}
+
 static void aspeed_soc_ast2700_init(Object *obj)
 {
 Aspeed27x0SoCState *a = ASPEED27X0_SOC(obj);
@@ -461,7 +546,7 @@ static void aspeed_soc_ast2700_realize(DeviceState *dev, 
Error **errp)
 sc->memmap[ASPEED_DEV_SDMC]);
 
 /* RAM */
-if (!aspeed_soc_dram_init(s, errp)) {
+if (!aspeed_soc_ast2700_dram_init(dev, errp)) {
 return;
 }
 
diff --git a/include/hw/arm/aspeed_soc.h b/i