[PATCH v4 5/7] hw/ppc/e500: Implement pflash handling

Bernhard Beschow posted 7 patches 3 years, 3 months ago
Maintainers: "Philippe Mathieu-Daudé" <philmd@linaro.org>, Kevin Wolf <kwolf@redhat.com>, Hanna Reitz <hreitz@redhat.com>, Bin Meng <bin.meng@windriver.com>
There is a newer version of this series
[PATCH v4 5/7] hw/ppc/e500: Implement pflash handling
Posted by Bernhard Beschow 3 years, 3 months ago
Allows e500 boards to have their root file system reside on flash using
only builtin devices located in the eLBC memory region.

Note that the flash memory area is only created when a -pflash argument is
given, and that the size is determined by the given file. The idea is to
put users into control.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 docs/system/ppc/ppce500.rst | 16 ++++++++
 hw/ppc/Kconfig              |  1 +
 hw/ppc/e500.c               | 79 +++++++++++++++++++++++++++++++++++++
 3 files changed, 96 insertions(+)

diff --git a/docs/system/ppc/ppce500.rst b/docs/system/ppc/ppce500.rst
index 7b5eb3c4ee..38f8ceb0cf 100644
--- a/docs/system/ppc/ppce500.rst
+++ b/docs/system/ppc/ppce500.rst
@@ -165,3 +165,19 @@ if “-device eTSEC” is given to QEMU:
 .. code-block:: bash
 
   -netdev tap,ifname=tap0,script=no,downscript=no,id=net0 -device eTSEC,netdev=net0
+
+Root file system on flash drive
+-------------------------------
+
+Rather than using a root file system on ram disk, it is possible to have it on
+CFI flash. Given an ext2 image whose size must be a power of two, it can be used
+as follows:
+
+.. code-block:: bash
+
+  $ qemu-system-ppc64 -M ppce500 -cpu e500mc -smp 4 -m 2G \
+      -display none -serial stdio \
+      -kernel vmlinux \
+      -drive if=pflash,file=/path/to/rootfs.ext2,format=raw \
+      -append "rootwait root=/dev/mtdblock0"
+
diff --git a/hw/ppc/Kconfig b/hw/ppc/Kconfig
index 791fe78a50..769a1ead1c 100644
--- a/hw/ppc/Kconfig
+++ b/hw/ppc/Kconfig
@@ -126,6 +126,7 @@ config E500
     select ETSEC
     select GPIO_MPC8XXX
     select OPENPIC
+    select PFLASH_CFI01
     select PLATFORM_BUS
     select PPCE500_PCI
     select SERIAL
diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
index 3e950ea3ba..73198adac8 100644
--- a/hw/ppc/e500.c
+++ b/hw/ppc/e500.c
@@ -23,8 +23,10 @@
 #include "e500-ccsr.h"
 #include "net/net.h"
 #include "qemu/config-file.h"
+#include "hw/block/flash.h"
 #include "hw/char/serial.h"
 #include "hw/pci/pci.h"
+#include "sysemu/block-backend-io.h"
 #include "sysemu/sysemu.h"
 #include "sysemu/kvm.h"
 #include "sysemu/reset.h"
@@ -267,6 +269,31 @@ static void sysbus_device_create_devtree(SysBusDevice *sbdev, void *opaque)
     }
 }
 
+static void create_devtree_flash(SysBusDevice *sbdev,
+                                 PlatformDevtreeData *data)
+{
+    g_autofree char *name = NULL;
+    uint64_t num_blocks = object_property_get_uint(OBJECT(sbdev),
+                                                   "num-blocks",
+                                                   &error_fatal);
+    uint64_t sector_length = object_property_get_uint(OBJECT(sbdev),
+                                                      "sector-length",
+                                                      &error_fatal);
+    uint64_t bank_width = object_property_get_uint(OBJECT(sbdev),
+                                                   "width",
+                                                   &error_fatal);
+    hwaddr flashbase = 0;
+    hwaddr flashsize = num_blocks * sector_length;
+    void *fdt = data->fdt;
+
+    name = g_strdup_printf("%s/nor@%" PRIx64, data->node, flashbase);
+    qemu_fdt_add_subnode(fdt, name);
+    qemu_fdt_setprop_string(fdt, name, "compatible", "cfi-flash");
+    qemu_fdt_setprop_sized_cells(fdt, name, "reg",
+                                 1, flashbase, 1, flashsize);
+    qemu_fdt_setprop_cell(fdt, name, "bank-width", bank_width);
+}
+
 static void platform_bus_create_devtree(PPCE500MachineState *pms,
                                         void *fdt, const char *mpic)
 {
@@ -276,6 +303,8 @@ static void platform_bus_create_devtree(PPCE500MachineState *pms,
     uint64_t addr = pmc->platform_bus_base;
     uint64_t size = pmc->platform_bus_size;
     int irq_start = pmc->platform_bus_first_irq;
+    SysBusDevice *sbdev;
+    bool ambiguous;
 
     /* Create a /platform node that we can put all devices into */
 
@@ -302,6 +331,13 @@ static void platform_bus_create_devtree(PPCE500MachineState *pms,
     /* Loop through all dynamic sysbus devices and create nodes for them */
     foreach_dynamic_sysbus_device(sysbus_device_create_devtree, &data);
 
+    sbdev = SYS_BUS_DEVICE(object_resolve_path_type("", TYPE_PFLASH_CFI01,
+                                                    &ambiguous));
+    if (sbdev) {
+        assert(!ambiguous);
+        create_devtree_flash(sbdev, &data);
+    }
+
     g_free(node);
 }
 
@@ -856,6 +892,7 @@ void ppce500_init(MachineState *machine)
     unsigned int pci_irq_nrs[PCI_NUM_PINS] = {1, 2, 3, 4};
     IrqLines *irqs;
     DeviceState *dev, *mpicdev;
+    DriveInfo *dinfo;
     CPUPPCState *firstenv = NULL;
     MemoryRegion *ccsr_addr_space;
     SysBusDevice *s;
@@ -1024,6 +1061,48 @@ void ppce500_init(MachineState *machine)
                                 pmc->platform_bus_base,
                                 &pms->pbus_dev->mmio);
 
+    dinfo = drive_get(IF_PFLASH, 0, 0);
+    if (dinfo) {
+        BlockBackend *blk = blk_by_legacy_dinfo(dinfo);
+        BlockDriverState *bs = blk_bs(blk);
+        uint64_t size = bdrv_getlength(bs);
+        uint64_t mmio_size = pms->pbus_dev->mmio.size;
+        uint32_t sector_len = 64 * KiB;
+
+        if (!is_power_of_2(size)) {
+            error_report("Size of pflash file must be a power of two.");
+            exit(1);
+        }
+
+        if (size > mmio_size) {
+            error_report("Size of pflash file must not be bigger than %" PRIu64
+                         " bytes.", mmio_size);
+            exit(1);
+        }
+
+        if (!QEMU_IS_ALIGNED(size, sector_len)) {
+            error_report("Size of pflash file must be a multiple of %" PRIu32
+                         ".", sector_len);
+            exit(1);
+        }
+
+        dev = qdev_new(TYPE_PFLASH_CFI01);
+        qdev_prop_set_drive(dev, "drive", blk);
+        qdev_prop_set_uint32(dev, "num-blocks", size / sector_len);
+        qdev_prop_set_uint64(dev, "sector-length", sector_len);
+        qdev_prop_set_uint8(dev, "width", 2);
+        qdev_prop_set_bit(dev, "big-endian", true);
+        qdev_prop_set_uint16(dev, "id0", 0x89);
+        qdev_prop_set_uint16(dev, "id1", 0x18);
+        qdev_prop_set_uint16(dev, "id2", 0x0000);
+        qdev_prop_set_uint16(dev, "id3", 0x0);
+        qdev_prop_set_string(dev, "name", "e500.flash");
+        sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
+
+        memory_region_add_subregion(&pms->pbus_dev->mmio, 0,
+                                    pflash_cfi01_get_memory(PFLASH_CFI01(dev)));
+    }
+
     /*
      * Smart firmware defaults ahead!
      *
-- 
2.38.0


Re: [PATCH v4 5/7] hw/ppc/e500: Implement pflash handling
Posted by Daniel Henrique Barboza 3 years, 3 months ago
Bernhard,

The 32 builds aren't fancying this patch. The issue is down there:

On 10/18/22 18:01, Bernhard Beschow wrote:
> Allows e500 boards to have their root file system reside on flash using
> only builtin devices located in the eLBC memory region.
> 
> Note that the flash memory area is only created when a -pflash argument is
> given, and that the size is determined by the given file. The idea is to
> put users into control.
> 
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> ---
>   docs/system/ppc/ppce500.rst | 16 ++++++++
>   hw/ppc/Kconfig              |  1 +
>   hw/ppc/e500.c               | 79 +++++++++++++++++++++++++++++++++++++
>   3 files changed, 96 insertions(+)
> 
> diff --git a/docs/system/ppc/ppce500.rst b/docs/system/ppc/ppce500.rst
> index 7b5eb3c4ee..38f8ceb0cf 100644
> --- a/docs/system/ppc/ppce500.rst
> +++ b/docs/system/ppc/ppce500.rst
> @@ -165,3 +165,19 @@ if “-device eTSEC” is given to QEMU:
>   .. code-block:: bash
>   
>     -netdev tap,ifname=tap0,script=no,downscript=no,id=net0 -device eTSEC,netdev=net0
> +
> +Root file system on flash drive
> +-------------------------------
> +
> +Rather than using a root file system on ram disk, it is possible to have it on
> +CFI flash. Given an ext2 image whose size must be a power of two, it can be used
> +as follows:
> +
> +.. code-block:: bash
> +
> +  $ qemu-system-ppc64 -M ppce500 -cpu e500mc -smp 4 -m 2G \
> +      -display none -serial stdio \
> +      -kernel vmlinux \
> +      -drive if=pflash,file=/path/to/rootfs.ext2,format=raw \
> +      -append "rootwait root=/dev/mtdblock0"
> +
> diff --git a/hw/ppc/Kconfig b/hw/ppc/Kconfig
> index 791fe78a50..769a1ead1c 100644
> --- a/hw/ppc/Kconfig
> +++ b/hw/ppc/Kconfig
> @@ -126,6 +126,7 @@ config E500
>       select ETSEC
>       select GPIO_MPC8XXX
>       select OPENPIC
> +    select PFLASH_CFI01
>       select PLATFORM_BUS
>       select PPCE500_PCI
>       select SERIAL
> diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
> index 3e950ea3ba..73198adac8 100644
> --- a/hw/ppc/e500.c
> +++ b/hw/ppc/e500.c
> @@ -23,8 +23,10 @@
>   #include "e500-ccsr.h"
>   #include "net/net.h"
>   #include "qemu/config-file.h"
> +#include "hw/block/flash.h"
>   #include "hw/char/serial.h"
>   #include "hw/pci/pci.h"
> +#include "sysemu/block-backend-io.h"
>   #include "sysemu/sysemu.h"
>   #include "sysemu/kvm.h"
>   #include "sysemu/reset.h"
> @@ -267,6 +269,31 @@ static void sysbus_device_create_devtree(SysBusDevice *sbdev, void *opaque)
>       }
>   }
>   
> +static void create_devtree_flash(SysBusDevice *sbdev,
> +                                 PlatformDevtreeData *data)
> +{
> +    g_autofree char *name = NULL;
> +    uint64_t num_blocks = object_property_get_uint(OBJECT(sbdev),
> +                                                   "num-blocks",
> +                                                   &error_fatal);
> +    uint64_t sector_length = object_property_get_uint(OBJECT(sbdev),
> +                                                      "sector-length",
> +                                                      &error_fatal);
> +    uint64_t bank_width = object_property_get_uint(OBJECT(sbdev),
> +                                                   "width",
> +                                                   &error_fatal);
> +    hwaddr flashbase = 0;
> +    hwaddr flashsize = num_blocks * sector_length;
> +    void *fdt = data->fdt;
> +
> +    name = g_strdup_printf("%s/nor@%" PRIx64, data->node, flashbase);
> +    qemu_fdt_add_subnode(fdt, name);
> +    qemu_fdt_setprop_string(fdt, name, "compatible", "cfi-flash");
> +    qemu_fdt_setprop_sized_cells(fdt, name, "reg",
> +                                 1, flashbase, 1, flashsize);
> +    qemu_fdt_setprop_cell(fdt, name, "bank-width", bank_width);
> +}
> +
>   static void platform_bus_create_devtree(PPCE500MachineState *pms,
>                                           void *fdt, const char *mpic)
>   {
> @@ -276,6 +303,8 @@ static void platform_bus_create_devtree(PPCE500MachineState *pms,
>       uint64_t addr = pmc->platform_bus_base;
>       uint64_t size = pmc->platform_bus_size;
>       int irq_start = pmc->platform_bus_first_irq;
> +    SysBusDevice *sbdev;
> +    bool ambiguous;
>   
>       /* Create a /platform node that we can put all devices into */
>   
> @@ -302,6 +331,13 @@ static void platform_bus_create_devtree(PPCE500MachineState *pms,
>       /* Loop through all dynamic sysbus devices and create nodes for them */
>       foreach_dynamic_sysbus_device(sysbus_device_create_devtree, &data);
>   
> +    sbdev = SYS_BUS_DEVICE(object_resolve_path_type("", TYPE_PFLASH_CFI01,
> +                                                    &ambiguous));
> +    if (sbdev) {
> +        assert(!ambiguous);
> +        create_devtree_flash(sbdev, &data);
> +    }
> +
>       g_free(node);
>   }
>   
> @@ -856,6 +892,7 @@ void ppce500_init(MachineState *machine)
>       unsigned int pci_irq_nrs[PCI_NUM_PINS] = {1, 2, 3, 4};
>       IrqLines *irqs;
>       DeviceState *dev, *mpicdev;
> +    DriveInfo *dinfo;
>       CPUPPCState *firstenv = NULL;
>       MemoryRegion *ccsr_addr_space;
>       SysBusDevice *s;
> @@ -1024,6 +1061,48 @@ void ppce500_init(MachineState *machine)
>                                   pmc->platform_bus_base,
>                                   &pms->pbus_dev->mmio);
>   
> +    dinfo = drive_get(IF_PFLASH, 0, 0);
> +    if (dinfo) {
> +        BlockBackend *blk = blk_by_legacy_dinfo(dinfo);
> +        BlockDriverState *bs = blk_bs(blk);
> +        uint64_t size = bdrv_getlength(bs);
> +        uint64_t mmio_size = pms->pbus_dev->mmio.size;

^ here. The issue is that on a 32 bit system it is not possible to cast the
Int128 type to uint64_t:

FAILED: libqemu-ppc64-softmmu.fa.p/hw_ppc_e500.c.o
3746cc -m32 -Ilibqemu-ppc64-softmmu.fa.p -I. -I.. -Itarget/ppc -I../target/ppc -I../dtc/libfdt -Iqapi -Itrace -Iui -Iui/shader -I/usr/include/pixman-1 -I/usr/include/glib-2.0 -I/usr/lib/glib-2.0/include -I/usr/include/sysprof-4 -fdiagnostics-color=auto -Wall -Winvalid-pch -Werror -std=gnu11 -O2 -g -isystem /builds/danielhb/qemu/linux-headers -isystem linux-headers -iquote . -iquote /builds/danielhb/qemu -iquote /builds/danielhb/qemu/include -iquote /builds/danielhb/qemu/tcg/i386 -pthread -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wundef -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fno-common -fwrapv -Wold-style-declaration -Wold-style-definition -Wtype-limits -Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers -Wempty-body -Wnested-externs -Wendif-labels -Wexpansion-to-defined -Wimplicit-fallthrough=2 -Wno-missing-include-dirs -Wno-shift-negative-value -Wno-psabi -fstack-protector-strong -fPIE -isystem../linux-headers -isystemlinux-headers -DNEED_CPU_H '-DCONFIG_TARGET="ppc64-softmmu-config-target.h"' '-DCONFIG_DEVICES="ppc64-softmmu-config-devices.h"' -MD -MQ libqemu-ppc64-softmmu.fa.p/hw_ppc_e500.c.o -MF libqemu-ppc64-softmmu.fa.p/hw_ppc_e500.c.o.d -o libqemu-ppc64-softmmu.fa.p/hw_ppc_e500.c.o -c ../hw/ppc/e500.c
3747../hw/ppc/e500.c: In function 'ppce500_init':
3748../hw/ppc/e500.c:1069:30: error: incompatible types when initializing type 'uint64_t' {aka 'long long unsigned int'} using type 'Int128'
3749 1069 |         uint64_t mmio_size = pms->pbus_dev->mmio.size;
3750      |                              ^~~
3751[3207/5331] Compiling C object libqemu-ppc64-softmmu.fa.p/hw_ppc_mpc8544_guts.c.o


What I did to solve the problem is this:


+         uint64_t mmio_size = int128_get64(pms->pbus_dev->mmio.size);


This will get the lower 64 bits and return an uint64_t.

Note that this function will assert if mmio.size is bigger than UINT64_MAX, but
since you're doing an error(1) on the "if size > mmio_size" conditional, this
assert() is not introducing a new side effect. We'll just fail earlier with
a different error message.


Let me know if this is acceptable for you.


Daniel



> +        uint32_t sector_len = 64 * KiB;
> +
> +        if (!is_power_of_2(size)) {
> +            error_report("Size of pflash file must be a power of two.");
> +            exit(1);
> +        }
> +
> +        if (size > mmio_size) {
> +            error_report("Size of pflash file must not be bigger than %" PRIu64
> +                         " bytes.", mmio_size);
> +            exit(1);
> +        }
> +
> +        if (!QEMU_IS_ALIGNED(size, sector_len)) {
> +            error_report("Size of pflash file must be a multiple of %" PRIu32
> +                         ".", sector_len);
> +            exit(1);
> +        }
> +
> +        dev = qdev_new(TYPE_PFLASH_CFI01);
> +        qdev_prop_set_drive(dev, "drive", blk);
> +        qdev_prop_set_uint32(dev, "num-blocks", size / sector_len);
> +        qdev_prop_set_uint64(dev, "sector-length", sector_len);
> +        qdev_prop_set_uint8(dev, "width", 2);
> +        qdev_prop_set_bit(dev, "big-endian", true);
> +        qdev_prop_set_uint16(dev, "id0", 0x89);
> +        qdev_prop_set_uint16(dev, "id1", 0x18);
> +        qdev_prop_set_uint16(dev, "id2", 0x0000);
> +        qdev_prop_set_uint16(dev, "id3", 0x0);
> +        qdev_prop_set_string(dev, "name", "e500.flash");
> +        sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
> +
> +        memory_region_add_subregion(&pms->pbus_dev->mmio, 0,
> +                                    pflash_cfi01_get_memory(PFLASH_CFI01(dev)));
> +    }
> +
>       /*
>        * Smart firmware defaults ahead!
>        *
Re: [PATCH v4 5/7] hw/ppc/e500: Implement pflash handling
Posted by Philippe Mathieu-Daudé 3 years, 3 months ago
On 28/10/22 17:09, Daniel Henrique Barboza wrote:
> Bernhard,
> 
> The 32 builds aren't fancying this patch. The issue is down there:
> 
> On 10/18/22 18:01, Bernhard Beschow wrote:
>> Allows e500 boards to have their root file system reside on flash using
>> only builtin devices located in the eLBC memory region.
>>
>> Note that the flash memory area is only created when a -pflash 
>> argument is
>> given, and that the size is determined by the given file. The idea is to
>> put users into control.
>>
>> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
>> ---
>>   docs/system/ppc/ppce500.rst | 16 ++++++++
>>   hw/ppc/Kconfig              |  1 +
>>   hw/ppc/e500.c               | 79 +++++++++++++++++++++++++++++++++++++
>>   3 files changed, 96 insertions(+)

>> @@ -1024,6 +1061,48 @@ void ppce500_init(MachineState *machine)
>>                                   pmc->platform_bus_base,
>>                                   &pms->pbus_dev->mmio);
>> +    dinfo = drive_get(IF_PFLASH, 0, 0);
>> +    if (dinfo) {
>> +        BlockBackend *blk = blk_by_legacy_dinfo(dinfo);
>> +        BlockDriverState *bs = blk_bs(blk);
>> +        uint64_t size = bdrv_getlength(bs);
>> +        uint64_t mmio_size = pms->pbus_dev->mmio.size;
> 
> ^ here. The issue is that on a 32 bit system it is not possible to cast the
> Int128 type to uint64_t:
> 
> FAILED: libqemu-ppc64-softmmu.fa.p/hw_ppc_e500.c.o
> 3746cc -m32 -Ilibqemu-ppc64-softmmu.fa.p -I. -I.. -Itarget/ppc 
> -I../target/ppc -I../dtc/libfdt -Iqapi -Itrace -Iui -Iui/shader 
> -I/usr/include/pixman-1 -I/usr/include/glib-2.0 
> -I/usr/lib/glib-2.0/include -I/usr/include/sysprof-4 
> -fdiagnostics-color=auto -Wall -Winvalid-pch -Werror -std=gnu11 -O2 -g 
> -isystem /builds/danielhb/qemu/linux-headers -isystem linux-headers 
> -iquote . -iquote /builds/danielhb/qemu -iquote 
> /builds/danielhb/qemu/include -iquote /builds/danielhb/qemu/tcg/i386 
> -pthread -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -D_GNU_SOURCE 
> -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes 
> -Wredundant-decls -Wundef -Wwrite-strings -Wmissing-prototypes 
> -fno-strict-aliasing -fno-common -fwrapv -Wold-style-declaration 
> -Wold-style-definition -Wtype-limits -Wformat-security -Wformat-y2k 
> -Winit-self -Wignored-qualifiers -Wempty-body -Wnested-externs 
> -Wendif-labels -Wexpansion-to-defined -Wimplicit-fallthrough=2 
> -Wno-missing-include-dirs -Wno-shift-negative-value -Wno-psabi 
> -fstack-protector-strong -fPIE -isystem../linux-headers 
> -isystemlinux-headers -DNEED_CPU_H 
> '-DCONFIG_TARGET="ppc64-softmmu-config-target.h"' 
> '-DCONFIG_DEVICES="ppc64-softmmu-config-devices.h"' -MD -MQ 
> libqemu-ppc64-softmmu.fa.p/hw_ppc_e500.c.o -MF 
> libqemu-ppc64-softmmu.fa.p/hw_ppc_e500.c.o.d -o 
> libqemu-ppc64-softmmu.fa.p/hw_ppc_e500.c.o -c ../hw/ppc/e500.c
> 3747../hw/ppc/e500.c: In function 'ppce500_init':
> 3748../hw/ppc/e500.c:1069:30: error: incompatible types when 
> initializing type 'uint64_t' {aka 'long long unsigned int'} using type 
> 'Int128'
> 3749 1069 |         uint64_t mmio_size = pms->pbus_dev->mmio.size;
> 3750      |                              ^~~
> 3751[3207/5331] Compiling C object 
> libqemu-ppc64-softmmu.fa.p/hw_ppc_mpc8544_guts.c.o
> 
> 
> What I did to solve the problem is this:
> 
> 
> +         uint64_t mmio_size = int128_get64(pms->pbus_dev->mmio.size); >
> This will get the lower 64 bits and return an uint64_t.
> 
> Note that this function will assert if mmio.size is bigger than 
> UINT64_MAX, but
> since you're doing an error(1) on the "if size > mmio_size" conditional, 
> this
> assert() is not introducing a new side effect. We'll just fail earlier with
> a different error message.

Simply use:

   memory_region_size(pms->pbus_dev->mmio);


Re: [PATCH v4 5/7] hw/ppc/e500: Implement pflash handling
Posted by Daniel Henrique Barboza 3 years, 3 months ago

On 10/28/22 19:42, Philippe Mathieu-Daudé wrote:
> On 28/10/22 17:09, Daniel Henrique Barboza wrote:
>> Bernhard,
>>
>> The 32 builds aren't fancying this patch. The issue is down there:
>>
>> On 10/18/22 18:01, Bernhard Beschow wrote:
>>> Allows e500 boards to have their root file system reside on flash using
>>> only builtin devices located in the eLBC memory region.
>>>
>>> Note that the flash memory area is only created when a -pflash argument is
>>> given, and that the size is determined by the given file. The idea is to
>>> put users into control.
>>>
>>> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
>>> ---
>>>   docs/system/ppc/ppce500.rst | 16 ++++++++
>>>   hw/ppc/Kconfig              |  1 +
>>>   hw/ppc/e500.c               | 79 +++++++++++++++++++++++++++++++++++++
>>>   3 files changed, 96 insertions(+)
> 
>>> @@ -1024,6 +1061,48 @@ void ppce500_init(MachineState *machine)
>>>                                   pmc->platform_bus_base,
>>>                                   &pms->pbus_dev->mmio);
>>> +    dinfo = drive_get(IF_PFLASH, 0, 0);
>>> +    if (dinfo) {
>>> +        BlockBackend *blk = blk_by_legacy_dinfo(dinfo);
>>> +        BlockDriverState *bs = blk_bs(blk);
>>> +        uint64_t size = bdrv_getlength(bs);
>>> +        uint64_t mmio_size = pms->pbus_dev->mmio.size;
>>
>> ^ here. The issue is that on a 32 bit system it is not possible to cast the
>> Int128 type to uint64_t:
>>
>> FAILED: libqemu-ppc64-softmmu.fa.p/hw_ppc_e500.c.o
>> 3746cc -m32 -Ilibqemu-ppc64-softmmu.fa.p -I. -I.. -Itarget/ppc -I../target/ppc -I../dtc/libfdt -Iqapi -Itrace -Iui -Iui/shader -I/usr/include/pixman-1 -I/usr/include/glib-2.0 -I/usr/lib/glib-2.0/include -I/usr/include/sysprof-4 -fdiagnostics-color=auto -Wall -Winvalid-pch -Werror -std=gnu11 -O2 -g -isystem /builds/danielhb/qemu/linux-headers -isystem linux-headers -iquote . -iquote /builds/danielhb/qemu -iquote /builds/danielhb/qemu/include -iquote /builds/danielhb/qemu/tcg/i386 -pthread -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wundef -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fno-common -fwrapv -Wold-style-declaration -Wold-style-definition -Wtype-limits -Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers -Wempty-body -Wnested-externs -Wendif-labels -Wexpansion-to-defined -Wimplicit-fallthrough=2 -Wno-missing-include-dirs -Wno-shift-negative-value 
>> -Wno-psabi -fstack-protector-strong -fPIE -isystem../linux-headers -isystemlinux-headers -DNEED_CPU_H '-DCONFIG_TARGET="ppc64-softmmu-config-target.h"' '-DCONFIG_DEVICES="ppc64-softmmu-config-devices.h"' -MD -MQ libqemu-ppc64-softmmu.fa.p/hw_ppc_e500.c.o -MF libqemu-ppc64-softmmu.fa.p/hw_ppc_e500.c.o.d -o libqemu-ppc64-softmmu.fa.p/hw_ppc_e500.c.o -c ../hw/ppc/e500.c
>> 3747../hw/ppc/e500.c: In function 'ppce500_init':
>> 3748../hw/ppc/e500.c:1069:30: error: incompatible types when initializing type 'uint64_t' {aka 'long long unsigned int'} using type 'Int128'
>> 3749 1069 |         uint64_t mmio_size = pms->pbus_dev->mmio.size;
>> 3750      |                              ^~~
>> 3751[3207/5331] Compiling C object libqemu-ppc64-softmmu.fa.p/hw_ppc_mpc8544_guts.c.o
>>
>>
>> What I did to solve the problem is this:
>>
>>
>> +         uint64_t mmio_size = int128_get64(pms->pbus_dev->mmio.size); >
>> This will get the lower 64 bits and return an uint64_t.
>>
>> Note that this function will assert if mmio.size is bigger than UINT64_MAX, but
>> since you're doing an error(1) on the "if size > mmio_size" conditional, this
>> assert() is not introducing a new side effect. We'll just fail earlier with
>> a different error message.
> 
> Simply use:
> 
>    memory_region_size(pms->pbus_dev->mmio);

Nice! I'll change it in-tree before re-sending the PR.


Daniel

> 

Re: [PATCH v4 5/7] hw/ppc/e500: Implement pflash handling
Posted by Bernhard Beschow 3 years, 3 months ago
Am 29. Oktober 2022 09:29:33 UTC schrieb Daniel Henrique Barboza <danielhb413@gmail.com>:
>
>
>On 10/28/22 19:42, Philippe Mathieu-Daudé wrote:
>> On 28/10/22 17:09, Daniel Henrique Barboza wrote:
>>> Bernhard,
>>> 
>>> The 32 builds aren't fancying this patch. The issue is down there:
>>> 
>>> On 10/18/22 18:01, Bernhard Beschow wrote:
>>>> Allows e500 boards to have their root file system reside on flash using
>>>> only builtin devices located in the eLBC memory region.
>>>> 
>>>> Note that the flash memory area is only created when a -pflash argument is
>>>> given, and that the size is determined by the given file. The idea is to
>>>> put users into control.
>>>> 
>>>> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
>>>> ---
>>>>   docs/system/ppc/ppce500.rst | 16 ++++++++
>>>>   hw/ppc/Kconfig              |  1 +
>>>>   hw/ppc/e500.c               | 79 +++++++++++++++++++++++++++++++++++++
>>>>   3 files changed, 96 insertions(+)
>> 
>>>> @@ -1024,6 +1061,48 @@ void ppce500_init(MachineState *machine)
>>>>                                   pmc->platform_bus_base,
>>>>                                   &pms->pbus_dev->mmio);
>>>> +    dinfo = drive_get(IF_PFLASH, 0, 0);
>>>> +    if (dinfo) {
>>>> +        BlockBackend *blk = blk_by_legacy_dinfo(dinfo);
>>>> +        BlockDriverState *bs = blk_bs(blk);
>>>> +        uint64_t size = bdrv_getlength(bs);
>>>> +        uint64_t mmio_size = pms->pbus_dev->mmio.size;
>>> 
>>> ^ here. The issue is that on a 32 bit system it is not possible to cast the
>>> Int128 type to uint64_t:
>>> 
>>> FAILED: libqemu-ppc64-softmmu.fa.p/hw_ppc_e500.c.o
>>> 3746cc -m32 -Ilibqemu-ppc64-softmmu.fa.p -I. -I.. -Itarget/ppc -I../target/ppc -I../dtc/libfdt -Iqapi -Itrace -Iui -Iui/shader -I/usr/include/pixman-1 -I/usr/include/glib-2.0 -I/usr/lib/glib-2.0/include -I/usr/include/sysprof-4 -fdiagnostics-color=auto -Wall -Winvalid-pch -Werror -std=gnu11 -O2 -g -isystem /builds/danielhb/qemu/linux-headers -isystem linux-headers -iquote . -iquote /builds/danielhb/qemu -iquote /builds/danielhb/qemu/include -iquote /builds/danielhb/qemu/tcg/i386 -pthread -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wundef -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fno-common -fwrapv -Wold-style-declaration -Wold-style-definition -Wtype-limits -Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers -Wempty-body -Wnested-externs -Wendif-labels -Wexpansion-to-defined -Wimplicit-fallthrough=2 -Wno-missing-include-dirs -Wno-shift-negative-value -Wno-psabi -fstack-protector-strong -fPIE -isystem../linux-headers -isystemlinux-headers -DNEED_CPU_H '-DCONFIG_TARGET="ppc64-softmmu-config-target.h"' '-DCONFIG_DEVICES="ppc64-softmmu-config-devices.h"' -MD -MQ libqemu-ppc64-softmmu.fa.p/hw_ppc_e500.c.o -MF libqemu-ppc64-softmmu.fa.p/hw_ppc_e500.c.o.d -o libqemu-ppc64-softmmu.fa.p/hw_ppc_e500.c.o -c ../hw/ppc/e500.c
>>> 3747../hw/ppc/e500.c: In function 'ppce500_init':
>>> 3748../hw/ppc/e500.c:1069:30: error: incompatible types when initializing type 'uint64_t' {aka 'long long unsigned int'} using type 'Int128'
>>> 3749 1069 |         uint64_t mmio_size = pms->pbus_dev->mmio.size;
>>> 3750      |                              ^~~
>>> 3751[3207/5331] Compiling C object libqemu-ppc64-softmmu.fa.p/hw_ppc_mpc8544_guts.c.o
>>> 
>>> 
>>> What I did to solve the problem is this:
>>> 
>>> 
>>> +         uint64_t mmio_size = int128_get64(pms->pbus_dev->mmio.size); >
>>> This will get the lower 64 bits and return an uint64_t.
>>> 
>>> Note that this function will assert if mmio.size is bigger than UINT64_MAX, but
>>> since you're doing an error(1) on the "if size > mmio_size" conditional, this
>>> assert() is not introducing a new side effect. We'll just fail earlier with
>>> a different error message.
>> 
>> Simply use:
>> 
>>    memory_region_size(pms->pbus_dev->mmio);
>
>Nice! I'll change it in-tree before re-sending the PR.

Thanks Daniel!

Bernhard
>
>
>Daniel
>
>> 
Re: [PATCH v4 5/7] hw/ppc/e500: Implement pflash handling
Posted by B 3 years, 3 months ago

Am 28. Oktober 2022 15:09:50 UTC schrieb Daniel Henrique Barboza <danielhb413@gmail.com>:
>Bernhard,

Hi Daniel,

>
>The 32 builds aren't fancying this patch. The issue is down there:
>
>On 10/18/22 18:01, Bernhard Beschow wrote:
>> Allows e500 boards to have their root file system reside on flash using
>> only builtin devices located in the eLBC memory region.
>> 
>> Note that the flash memory area is only created when a -pflash argument is
>> given, and that the size is determined by the given file. The idea is to
>> put users into control.
>> 
>> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
>> ---
>>   docs/system/ppc/ppce500.rst | 16 ++++++++
>>   hw/ppc/Kconfig              |  1 +
>>   hw/ppc/e500.c               | 79 +++++++++++++++++++++++++++++++++++++
>>   3 files changed, 96 insertions(+)
>> 
>> diff --git a/docs/system/ppc/ppce500.rst b/docs/system/ppc/ppce500.rst
>> index 7b5eb3c4ee..38f8ceb0cf 100644
>> --- a/docs/system/ppc/ppce500.rst
>> +++ b/docs/system/ppc/ppce500.rst
>> @@ -165,3 +165,19 @@ if “-device eTSEC” is given to QEMU:
>>   .. code-block:: bash
>>       -netdev tap,ifname=tap0,script=no,downscript=no,id=net0 -device eTSEC,netdev=net0
>> +
>> +Root file system on flash drive
>> +-------------------------------
>> +
>> +Rather than using a root file system on ram disk, it is possible to have it on
>> +CFI flash. Given an ext2 image whose size must be a power of two, it can be used
>> +as follows:
>> +
>> +.. code-block:: bash
>> +
>> +  $ qemu-system-ppc64 -M ppce500 -cpu e500mc -smp 4 -m 2G \
>> +      -display none -serial stdio \
>> +      -kernel vmlinux \
>> +      -drive if=pflash,file=/path/to/rootfs.ext2,format=raw \
>> +      -append "rootwait root=/dev/mtdblock0"
>> +
>> diff --git a/hw/ppc/Kconfig b/hw/ppc/Kconfig
>> index 791fe78a50..769a1ead1c 100644
>> --- a/hw/ppc/Kconfig
>> +++ b/hw/ppc/Kconfig
>> @@ -126,6 +126,7 @@ config E500
>>       select ETSEC
>>       select GPIO_MPC8XXX
>>       select OPENPIC
>> +    select PFLASH_CFI01
>>       select PLATFORM_BUS
>>       select PPCE500_PCI
>>       select SERIAL
>> diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
>> index 3e950ea3ba..73198adac8 100644
>> --- a/hw/ppc/e500.c
>> +++ b/hw/ppc/e500.c
>> @@ -23,8 +23,10 @@
>>   #include "e500-ccsr.h"
>>   #include "net/net.h"
>>   #include "qemu/config-file.h"
>> +#include "hw/block/flash.h"
>>   #include "hw/char/serial.h"
>>   #include "hw/pci/pci.h"
>> +#include "sysemu/block-backend-io.h"
>>   #include "sysemu/sysemu.h"
>>   #include "sysemu/kvm.h"
>>   #include "sysemu/reset.h"
>> @@ -267,6 +269,31 @@ static void sysbus_device_create_devtree(SysBusDevice *sbdev, void *opaque)
>>       }
>>   }
>>   +static void create_devtree_flash(SysBusDevice *sbdev,
>> +                                 PlatformDevtreeData *data)
>> +{
>> +    g_autofree char *name = NULL;
>> +    uint64_t num_blocks = object_property_get_uint(OBJECT(sbdev),
>> +                                                   "num-blocks",
>> +                                                   &error_fatal);
>> +    uint64_t sector_length = object_property_get_uint(OBJECT(sbdev),
>> +                                                      "sector-length",
>> +                                                      &error_fatal);
>> +    uint64_t bank_width = object_property_get_uint(OBJECT(sbdev),
>> +                                                   "width",
>> +                                                   &error_fatal);
>> +    hwaddr flashbase = 0;
>> +    hwaddr flashsize = num_blocks * sector_length;
>> +    void *fdt = data->fdt;
>> +
>> +    name = g_strdup_printf("%s/nor@%" PRIx64, data->node, flashbase);
>> +    qemu_fdt_add_subnode(fdt, name);
>> +    qemu_fdt_setprop_string(fdt, name, "compatible", "cfi-flash");
>> +    qemu_fdt_setprop_sized_cells(fdt, name, "reg",
>> +                                 1, flashbase, 1, flashsize);
>> +    qemu_fdt_setprop_cell(fdt, name, "bank-width", bank_width);
>> +}
>> +
>>   static void platform_bus_create_devtree(PPCE500MachineState *pms,
>>                                           void *fdt, const char *mpic)
>>   {
>> @@ -276,6 +303,8 @@ static void platform_bus_create_devtree(PPCE500MachineState *pms,
>>       uint64_t addr = pmc->platform_bus_base;
>>       uint64_t size = pmc->platform_bus_size;
>>       int irq_start = pmc->platform_bus_first_irq;
>> +    SysBusDevice *sbdev;
>> +    bool ambiguous;
>>         /* Create a /platform node that we can put all devices into */
>>   @@ -302,6 +331,13 @@ static void platform_bus_create_devtree(PPCE500MachineState *pms,
>>       /* Loop through all dynamic sysbus devices and create nodes for them */
>>       foreach_dynamic_sysbus_device(sysbus_device_create_devtree, &data);
>>   +    sbdev = SYS_BUS_DEVICE(object_resolve_path_type("", TYPE_PFLASH_CFI01,
>> +                                                    &ambiguous));
>> +    if (sbdev) {
>> +        assert(!ambiguous);
>> +        create_devtree_flash(sbdev, &data);
>> +    }
>> +
>>       g_free(node);
>>   }
>>   @@ -856,6 +892,7 @@ void ppce500_init(MachineState *machine)
>>       unsigned int pci_irq_nrs[PCI_NUM_PINS] = {1, 2, 3, 4};
>>       IrqLines *irqs;
>>       DeviceState *dev, *mpicdev;
>> +    DriveInfo *dinfo;
>>       CPUPPCState *firstenv = NULL;
>>       MemoryRegion *ccsr_addr_space;
>>       SysBusDevice *s;
>> @@ -1024,6 +1061,48 @@ void ppce500_init(MachineState *machine)
>>                                   pmc->platform_bus_base,
>>                                   &pms->pbus_dev->mmio);
>>   +    dinfo = drive_get(IF_PFLASH, 0, 0);
>> +    if (dinfo) {
>> +        BlockBackend *blk = blk_by_legacy_dinfo(dinfo);
>> +        BlockDriverState *bs = blk_bs(blk);
>> +        uint64_t size = bdrv_getlength(bs);
>> +        uint64_t mmio_size = pms->pbus_dev->mmio.size;
>
>^ here. The issue is that on a 32 bit system it is not possible to cast the
>Int128 type to uint64_t:
>
>FAILED: libqemu-ppc64-softmmu.fa.p/hw_ppc_e500.c.o
>3746cc -m32 -Ilibqemu-ppc64-softmmu.fa.p -I. -I.. -Itarget/ppc -I../target/ppc -I../dtc/libfdt -Iqapi -Itrace -Iui -Iui/shader -I/usr/include/pixman-1 -I/usr/include/glib-2.0 -I/usr/lib/glib-2.0/include -I/usr/include/sysprof-4 -fdiagnostics-color=auto -Wall -Winvalid-pch -Werror -std=gnu11 -O2 -g -isystem /builds/danielhb/qemu/linux-headers -isystem linux-headers -iquote . -iquote /builds/danielhb/qemu -iquote /builds/danielhb/qemu/include -iquote /builds/danielhb/qemu/tcg/i386 -pthread -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wundef -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fno-common -fwrapv -Wold-style-declaration -Wold-style-definition -Wtype-limits -Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers -Wempty-body -Wnested-externs -Wendif-labels -Wexpansion-to-defined -Wimplicit-fallthrough=2 -Wno-missing-include-dirs -Wno-shift-negative-value -Wno-psabi -fstack-protector-strong -fPIE -isystem../linux-headers -isystemlinux-headers -DNEED_CPU_H '-DCONFIG_TARGET="ppc64-softmmu-config-target.h"' '-DCONFIG_DEVICES="ppc64-softmmu-config-devices.h"' -MD -MQ libqemu-ppc64-softmmu.fa.p/hw_ppc_e500.c.o -MF libqemu-ppc64-softmmu.fa.p/hw_ppc_e500.c.o.d -o libqemu-ppc64-softmmu.fa.p/hw_ppc_e500.c.o -c ../hw/ppc/e500.c
>3747../hw/ppc/e500.c: In function 'ppce500_init':
>3748../hw/ppc/e500.c:1069:30: error: incompatible types when initializing type 'uint64_t' {aka 'long long unsigned int'} using type 'Int128'
>3749 1069 |         uint64_t mmio_size = pms->pbus_dev->mmio.size;
>3750      |                              ^~~
>3751[3207/5331] Compiling C object libqemu-ppc64-softmmu.fa.p/hw_ppc_mpc8544_guts.c.o

Whoops.
>
>
>What I did to solve the problem is this:
>
>
>+         uint64_t mmio_size = int128_get64(pms->pbus_dev->mmio.size);
>
>
>This will get the lower 64 bits and return an uint64_t.
>
>Note that this function will assert if mmio.size is bigger than UINT64_MAX, but
>since you're doing an error(1) on the "if size > mmio_size" conditional, this
>assert() is not introducing a new side effect. We'll just fail earlier with
>a different error message.

Yes, sounds reasonable.
>
>
>Let me know if this is acceptable for you.

Yes, that's fine with me. Thanks for the fix!

Best regards,
Bernhard
>
>
>Daniel
>
>
>
>> +        uint32_t sector_len = 64 * KiB;
>> +
>> +        if (!is_power_of_2(size)) {
>> +            error_report("Size of pflash file must be a power of two.");
>> +            exit(1);
>> +        }
>> +
>> +        if (size > mmio_size) {
>> +            error_report("Size of pflash file must not be bigger than %" PRIu64
>> +                         " bytes.", mmio_size);
>> +            exit(1);
>> +        }
>> +
>> +        if (!QEMU_IS_ALIGNED(size, sector_len)) {
>> +            error_report("Size of pflash file must be a multiple of %" PRIu32
>> +                         ".", sector_len);
>> +            exit(1);
>> +        }
>> +
>> +        dev = qdev_new(TYPE_PFLASH_CFI01);
>> +        qdev_prop_set_drive(dev, "drive", blk);
>> +        qdev_prop_set_uint32(dev, "num-blocks", size / sector_len);
>> +        qdev_prop_set_uint64(dev, "sector-length", sector_len);
>> +        qdev_prop_set_uint8(dev, "width", 2);
>> +        qdev_prop_set_bit(dev, "big-endian", true);
>> +        qdev_prop_set_uint16(dev, "id0", 0x89);
>> +        qdev_prop_set_uint16(dev, "id1", 0x18);
>> +        qdev_prop_set_uint16(dev, "id2", 0x0000);
>> +        qdev_prop_set_uint16(dev, "id3", 0x0);
>> +        qdev_prop_set_string(dev, "name", "e500.flash");
>> +        sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
>> +
>> +        memory_region_add_subregion(&pms->pbus_dev->mmio, 0,
>> +                                    pflash_cfi01_get_memory(PFLASH_CFI01(dev)));
>> +    }
>> +
>>       /*
>>        * Smart firmware defaults ahead!
>>        *
Re: [PATCH v4 5/7] hw/ppc/e500: Implement pflash handling
Posted by Philippe Mathieu-Daudé 3 years, 3 months ago
On 18/10/22 23:01, Bernhard Beschow wrote:
> Allows e500 boards to have their root file system reside on flash using
> only builtin devices located in the eLBC memory region.
> 
> Note that the flash memory area is only created when a -pflash argument is
> given, and that the size is determined by the given file. The idea is to
> put users into control.
> 
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> ---
>   docs/system/ppc/ppce500.rst | 16 ++++++++
>   hw/ppc/Kconfig              |  1 +
>   hw/ppc/e500.c               | 79 +++++++++++++++++++++++++++++++++++++
>   3 files changed, 96 insertions(+)

> @@ -1024,6 +1061,48 @@ void ppce500_init(MachineState *machine)
>                                   pmc->platform_bus_base,
>                                   &pms->pbus_dev->mmio);
>   
> +    dinfo = drive_get(IF_PFLASH, 0, 0);
> +    if (dinfo) {
> +        BlockBackend *blk = blk_by_legacy_dinfo(dinfo);
> +        BlockDriverState *bs = blk_bs(blk);
> +        uint64_t size = bdrv_getlength(bs);
> +        uint64_t mmio_size = pms->pbus_dev->mmio.size;
> +        uint32_t sector_len = 64 * KiB;
> +
> +        if (!is_power_of_2(size)) {
> +            error_report("Size of pflash file must be a power of two.");
> +            exit(1);
> +        }
> +
> +        if (size > mmio_size) {
> +            error_report("Size of pflash file must not be bigger than %" PRIu64
> +                         " bytes.", mmio_size);
> +            exit(1);
> +        }
> +
> +        if (!QEMU_IS_ALIGNED(size, sector_len)) {
> +            error_report("Size of pflash file must be a multiple of %" PRIu32
> +                         ".", sector_len);
> +            exit(1);
> +        }

Again, this check is unrelated to the board code and belong to the flash 
device (the board has no idea of the underlying flash restrictions).

(see below)

> +        dev = qdev_new(TYPE_PFLASH_CFI01);
> +        qdev_prop_set_drive(dev, "drive", blk);
> +        qdev_prop_set_uint32(dev, "num-blocks", size / sector_len);
> +        qdev_prop_set_uint64(dev, "sector-length", sector_len);
> +        qdev_prop_set_uint8(dev, "width", 2);
> +        qdev_prop_set_bit(dev, "big-endian", true);
> +        qdev_prop_set_uint16(dev, "id0", 0x89);
> +        qdev_prop_set_uint16(dev, "id1", 0x18);
> +        qdev_prop_set_uint16(dev, "id2", 0x0000);
> +        qdev_prop_set_uint16(dev, "id3", 0x0);
> +        qdev_prop_set_string(dev, "name", "e500.flash");
> +        sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);

If you want to report the error differently, you can use a local Error*
and display it with error_report_err() before exiting.

Anyhow can be cleaned later, so:
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

> +        memory_region_add_subregion(&pms->pbus_dev->mmio, 0,
> +                                    pflash_cfi01_get_memory(PFLASH_CFI01(dev)));
> +    }


Re: [PATCH v4 5/7] hw/ppc/e500: Implement pflash handling
Posted by Daniel Henrique Barboza 3 years, 3 months ago

On 10/18/22 18:01, Bernhard Beschow wrote:
> Allows e500 boards to have their root file system reside on flash using
> only builtin devices located in the eLBC memory region.
> 
> Note that the flash memory area is only created when a -pflash argument is
> given, and that the size is determined by the given file. The idea is to
> put users into control.
> 
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> ---

Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>

>   docs/system/ppc/ppce500.rst | 16 ++++++++
>   hw/ppc/Kconfig              |  1 +
>   hw/ppc/e500.c               | 79 +++++++++++++++++++++++++++++++++++++
>   3 files changed, 96 insertions(+)
> 
> diff --git a/docs/system/ppc/ppce500.rst b/docs/system/ppc/ppce500.rst
> index 7b5eb3c4ee..38f8ceb0cf 100644
> --- a/docs/system/ppc/ppce500.rst
> +++ b/docs/system/ppc/ppce500.rst
> @@ -165,3 +165,19 @@ if “-device eTSEC” is given to QEMU:
>   .. code-block:: bash
>   
>     -netdev tap,ifname=tap0,script=no,downscript=no,id=net0 -device eTSEC,netdev=net0
> +
> +Root file system on flash drive
> +-------------------------------
> +
> +Rather than using a root file system on ram disk, it is possible to have it on
> +CFI flash. Given an ext2 image whose size must be a power of two, it can be used
> +as follows:
> +
> +.. code-block:: bash
> +
> +  $ qemu-system-ppc64 -M ppce500 -cpu e500mc -smp 4 -m 2G \
> +      -display none -serial stdio \
> +      -kernel vmlinux \
> +      -drive if=pflash,file=/path/to/rootfs.ext2,format=raw \
> +      -append "rootwait root=/dev/mtdblock0"
> +
> diff --git a/hw/ppc/Kconfig b/hw/ppc/Kconfig
> index 791fe78a50..769a1ead1c 100644
> --- a/hw/ppc/Kconfig
> +++ b/hw/ppc/Kconfig
> @@ -126,6 +126,7 @@ config E500
>       select ETSEC
>       select GPIO_MPC8XXX
>       select OPENPIC
> +    select PFLASH_CFI01
>       select PLATFORM_BUS
>       select PPCE500_PCI
>       select SERIAL
> diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
> index 3e950ea3ba..73198adac8 100644
> --- a/hw/ppc/e500.c
> +++ b/hw/ppc/e500.c
> @@ -23,8 +23,10 @@
>   #include "e500-ccsr.h"
>   #include "net/net.h"
>   #include "qemu/config-file.h"
> +#include "hw/block/flash.h"
>   #include "hw/char/serial.h"
>   #include "hw/pci/pci.h"
> +#include "sysemu/block-backend-io.h"
>   #include "sysemu/sysemu.h"
>   #include "sysemu/kvm.h"
>   #include "sysemu/reset.h"
> @@ -267,6 +269,31 @@ static void sysbus_device_create_devtree(SysBusDevice *sbdev, void *opaque)
>       }
>   }
>   
> +static void create_devtree_flash(SysBusDevice *sbdev,
> +                                 PlatformDevtreeData *data)
> +{
> +    g_autofree char *name = NULL;
> +    uint64_t num_blocks = object_property_get_uint(OBJECT(sbdev),
> +                                                   "num-blocks",
> +                                                   &error_fatal);
> +    uint64_t sector_length = object_property_get_uint(OBJECT(sbdev),
> +                                                      "sector-length",
> +                                                      &error_fatal);
> +    uint64_t bank_width = object_property_get_uint(OBJECT(sbdev),
> +                                                   "width",
> +                                                   &error_fatal);
> +    hwaddr flashbase = 0;
> +    hwaddr flashsize = num_blocks * sector_length;
> +    void *fdt = data->fdt;
> +
> +    name = g_strdup_printf("%s/nor@%" PRIx64, data->node, flashbase);
> +    qemu_fdt_add_subnode(fdt, name);
> +    qemu_fdt_setprop_string(fdt, name, "compatible", "cfi-flash");
> +    qemu_fdt_setprop_sized_cells(fdt, name, "reg",
> +                                 1, flashbase, 1, flashsize);
> +    qemu_fdt_setprop_cell(fdt, name, "bank-width", bank_width);
> +}
> +
>   static void platform_bus_create_devtree(PPCE500MachineState *pms,
>                                           void *fdt, const char *mpic)
>   {
> @@ -276,6 +303,8 @@ static void platform_bus_create_devtree(PPCE500MachineState *pms,
>       uint64_t addr = pmc->platform_bus_base;
>       uint64_t size = pmc->platform_bus_size;
>       int irq_start = pmc->platform_bus_first_irq;
> +    SysBusDevice *sbdev;
> +    bool ambiguous;
>   
>       /* Create a /platform node that we can put all devices into */
>   
> @@ -302,6 +331,13 @@ static void platform_bus_create_devtree(PPCE500MachineState *pms,
>       /* Loop through all dynamic sysbus devices and create nodes for them */
>       foreach_dynamic_sysbus_device(sysbus_device_create_devtree, &data);
>   
> +    sbdev = SYS_BUS_DEVICE(object_resolve_path_type("", TYPE_PFLASH_CFI01,
> +                                                    &ambiguous));
> +    if (sbdev) {
> +        assert(!ambiguous);
> +        create_devtree_flash(sbdev, &data);
> +    }
> +
>       g_free(node);
>   }
>   
> @@ -856,6 +892,7 @@ void ppce500_init(MachineState *machine)
>       unsigned int pci_irq_nrs[PCI_NUM_PINS] = {1, 2, 3, 4};
>       IrqLines *irqs;
>       DeviceState *dev, *mpicdev;
> +    DriveInfo *dinfo;
>       CPUPPCState *firstenv = NULL;
>       MemoryRegion *ccsr_addr_space;
>       SysBusDevice *s;
> @@ -1024,6 +1061,48 @@ void ppce500_init(MachineState *machine)
>                                   pmc->platform_bus_base,
>                                   &pms->pbus_dev->mmio);
>   
> +    dinfo = drive_get(IF_PFLASH, 0, 0);
> +    if (dinfo) {
> +        BlockBackend *blk = blk_by_legacy_dinfo(dinfo);
> +        BlockDriverState *bs = blk_bs(blk);
> +        uint64_t size = bdrv_getlength(bs);
> +        uint64_t mmio_size = pms->pbus_dev->mmio.size;
> +        uint32_t sector_len = 64 * KiB;
> +
> +        if (!is_power_of_2(size)) {
> +            error_report("Size of pflash file must be a power of two.");
> +            exit(1);
> +        }
> +
> +        if (size > mmio_size) {
> +            error_report("Size of pflash file must not be bigger than %" PRIu64
> +                         " bytes.", mmio_size);
> +            exit(1);
> +        }
> +
> +        if (!QEMU_IS_ALIGNED(size, sector_len)) {
> +            error_report("Size of pflash file must be a multiple of %" PRIu32
> +                         ".", sector_len);
> +            exit(1);
> +        }
> +
> +        dev = qdev_new(TYPE_PFLASH_CFI01);
> +        qdev_prop_set_drive(dev, "drive", blk);
> +        qdev_prop_set_uint32(dev, "num-blocks", size / sector_len);
> +        qdev_prop_set_uint64(dev, "sector-length", sector_len);
> +        qdev_prop_set_uint8(dev, "width", 2);
> +        qdev_prop_set_bit(dev, "big-endian", true);
> +        qdev_prop_set_uint16(dev, "id0", 0x89);
> +        qdev_prop_set_uint16(dev, "id1", 0x18);
> +        qdev_prop_set_uint16(dev, "id2", 0x0000);
> +        qdev_prop_set_uint16(dev, "id3", 0x0);
> +        qdev_prop_set_string(dev, "name", "e500.flash");
> +        sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
> +
> +        memory_region_add_subregion(&pms->pbus_dev->mmio, 0,
> +                                    pflash_cfi01_get_memory(PFLASH_CFI01(dev)));
> +    }
> +
>       /*
>        * Smart firmware defaults ahead!
>        *