| Message ID | 20221003203142.24355-10-shentey@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ppc/e500: Add support for two types of flash, cleanup | expand |
On 3/10/22 22:31, 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 | 12 ++++++ > hw/ppc/Kconfig | 1 + > hw/ppc/e500.c | 76 +++++++++++++++++++++++++++++++++++++ > 3 files changed, 89 insertions(+) > @@ -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,45 @@ 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 (ctpop64(size) != 1) { > + error_report("Size of pflash file must be a power of two."); This is a PFLASH restriction (which you already fixed in the previous patch), not a board one. > + exit(1); > + } > + > + if (size > mmio_size) { > + error_report("Size of pflash file must not be bigger than %" PRIu64 > + " bytes.", mmio_size); There is no hardware limitation here, you can wire flash bigger than the memory aperture. What is above the aperture will simply be ignored. Should we display a warning here instead of a fatal error? > + exit(1); > + } > + > + assert(QEMU_IS_ALIGNED(size, sector_len)); Similarly, this doesn't seem a problem the board code should worry about: better to defer it to PFLASH realize(). > + 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"); > + s = SYS_BUS_DEVICE(dev); > + sysbus_realize_and_unref(s, &error_fatal); > + > + memory_region_add_subregion(&pms->pbus_dev->mmio, 0, > + sysbus_mmio_get_region(s, 0)); > + } > + > /* > * Smart firmware defaults ahead! > *
Am 3. Oktober 2022 21:21:15 UTC schrieb "Philippe Mathieu-Daudé" <f4bug@amsat.org>: >On 3/10/22 22:31, 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 | 12 ++++++ >> hw/ppc/Kconfig | 1 + >> hw/ppc/e500.c | 76 +++++++++++++++++++++++++++++++++++++ >> 3 files changed, 89 insertions(+) > >> @@ -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,45 @@ 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 (ctpop64(size) != 1) { >> + error_report("Size of pflash file must be a power of two."); > >This is a PFLASH restriction (which you already fixed in the previous >patch), not a board one. I agree that this check seems redundant to the one in cfi01. I added this one for clearer error messages since cfi01 only complains about the "device size" not being a power of two while this message at least gives a hint towards the source of the problem (the file given in the pflash option). Usually the size of the pflash area is hardcoded in the board while I choose to derive it from the size of the backing file in order to avoid hardcoding it. My idea is to put users into control by offering more flexibility. > >> + exit(1); >> + } >> + >> + if (size > mmio_size) { >> + error_report("Size of pflash file must not be bigger than %" PRIu64 >> + " bytes.", mmio_size); > >There is no hardware limitation here, you can wire flash bigger than the >memory aperture. What is above the aperture will simply be ignored. > >Should we display a warning here instead of a fatal error? While this is technically possible, is that what users would expect? Couldn't we just require users to truncate their files if they really want the "aperture" behavior? > >> + exit(1); >> + } >> + >> + assert(QEMU_IS_ALIGNED(size, sector_len)); > >Similarly, this doesn't seem a problem the board code should worry >about: better to defer it to PFLASH realize(). The reason for the assert() here is that size isn't stored directly in the cfi01 device. Instead, it must be calculated by the properties "num-blocks" times "sector-length". For this to work, size must be divisible by sector_len without remainder, which is checked by the assertion. We could theoretically add a "size" property which would violate the single point of truth principle, though. Do you see a different solution? Best regards, Bernhard > >> + 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"); >> + s = SYS_BUS_DEVICE(dev); >> + sysbus_realize_and_unref(s, &error_fatal); >> + >> + memory_region_add_subregion(&pms->pbus_dev->mmio, 0, >> + sysbus_mmio_get_region(s, 0)); >> + } >> + >> /* >> * Smart firmware defaults ahead! >> * >
On Tue, Oct 4, 2022 at 5:40 AM Bernhard Beschow <shentey@gmail.com> 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 | 12 ++++++ > hw/ppc/Kconfig | 1 + > hw/ppc/e500.c | 76 +++++++++++++++++++++++++++++++++++++ > 3 files changed, 89 insertions(+) > > diff --git a/docs/system/ppc/ppce500.rst b/docs/system/ppc/ppce500.rst > index ba6bcb7314..1ed6c36599 100644 > --- a/docs/system/ppc/ppce500.rst > +++ b/docs/system/ppc/ppce500.rst > @@ -119,6 +119,18 @@ To boot the 32-bit Linux kernel: > -initrd /path/to/rootfs.cpio \ > -append "root=/dev/ram" > > +Rather than using a root file system on ram disk, it is possible to have it on > +emulated flash. Given an ext2 image whose size must be a power of two, it can > +be used as follows: > + > +.. code-block:: bash > + > + $ qemu-system-ppc{64|32} -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" Could we add a separate sub-section "pflash" after the "networking" part you did before? > + > Running U-Boot > -------------- > > 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..2b1430fca4 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,45 @@ 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 (ctpop64(size) != 1) { > + 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); > + } > + > + assert(QEMU_IS_ALIGNED(size, sector_len)); > + > + 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"); > + s = SYS_BUS_DEVICE(dev); > + sysbus_realize_and_unref(s, &error_fatal); > + > + memory_region_add_subregion(&pms->pbus_dev->mmio, 0, > + sysbus_mmio_get_region(s, 0)); > + } > + > /* > * Smart firmware defaults ahead! > * Otherwise LGTM: Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
diff --git a/docs/system/ppc/ppce500.rst b/docs/system/ppc/ppce500.rst index ba6bcb7314..1ed6c36599 100644 --- a/docs/system/ppc/ppce500.rst +++ b/docs/system/ppc/ppce500.rst @@ -119,6 +119,18 @@ To boot the 32-bit Linux kernel: -initrd /path/to/rootfs.cpio \ -append "root=/dev/ram" +Rather than using a root file system on ram disk, it is possible to have it on +emulated flash. Given an ext2 image whose size must be a power of two, it can +be used as follows: + +.. code-block:: bash + + $ qemu-system-ppc{64|32} -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" + Running U-Boot -------------- 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..2b1430fca4 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,45 @@ 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 (ctpop64(size) != 1) { + 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); + } + + assert(QEMU_IS_ALIGNED(size, sector_len)); + + 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"); + s = SYS_BUS_DEVICE(dev); + sysbus_realize_and_unref(s, &error_fatal); + + memory_region_add_subregion(&pms->pbus_dev->mmio, 0, + sysbus_mmio_get_region(s, 0)); + } + /* * Smart firmware defaults ahead! *
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 | 12 ++++++ hw/ppc/Kconfig | 1 + hw/ppc/e500.c | 76 +++++++++++++++++++++++++++++++++++++ 3 files changed, 89 insertions(+)