Machine "ref405ep" maps its flash memory at address 2^32 - image size.
Image size is rounded up to the next multiple of 64KiB. Useless,
because pflash_cfi02_realize() fails with "failed to read the initial
flash content" unless the rounding is a no-op.
If the image size exceeds 0x80000 Bytes, we overlap first SRAM, then
other stuff. No idea how that would play out, but a useful outcomes
seem unlikely.
Map the flash memory at fixed address 0xFFF80000 with size 512KiB,
regardless of image size, to match the physical hardware.
Machine "taihu" maps its boot flash memory similarly. The code even
has a comment /* XXX: should check that size is 2MB */, followed by
disabled code to adjust the size to 2MiB regardless of image size.
Its code to map its application flash memory looks the same, except
there the XXX comment asks for 32MiB, and the code to adjust the size
isn't disabled. Note that pflash_cfi02_realize() fails with "failed
to read the initial flash content" for images smaller than 32MiB.
Map the boot flash memory at fixed address 0xFFE00000 with size 2MiB,
to match the physical hardware. Delete dead code from application
flash mapping, and simplify some.
Cc: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
hw/ppc/ppc405_boards.c | 53 +++++++++++++-----------------------------
1 file changed, 16 insertions(+), 37 deletions(-)
diff --git a/hw/ppc/ppc405_boards.c b/hw/ppc/ppc405_boards.c
index f47b15f10e..728154aebb 100644
--- a/hw/ppc/ppc405_boards.c
+++ b/hw/ppc/ppc405_boards.c
@@ -158,7 +158,7 @@ static void ref405ep_init(MachineState *machine)
target_ulong kernel_base, initrd_base;
long kernel_size, initrd_size;
int linux_boot;
- int fl_idx, fl_sectors, len;
+ int len;
DriveInfo *dinfo;
MemoryRegion *sysmem = get_system_memory();
@@ -185,26 +185,19 @@ static void ref405ep_init(MachineState *machine)
#ifdef DEBUG_BOARD_INIT
printf("%s: register BIOS\n", __func__);
#endif
- fl_idx = 0;
#ifdef USE_FLASH_BIOS
- dinfo = drive_get(IF_PFLASH, 0, fl_idx);
+ dinfo = drive_get(IF_PFLASH, 0, 0);
if (dinfo) {
- BlockBackend *blk = blk_by_legacy_dinfo(dinfo);
-
- bios_size = blk_getlength(blk);
- fl_sectors = (bios_size + 65535) >> 16;
#ifdef DEBUG_BOARD_INIT
- printf("Register parallel flash %d size %lx"
- " at addr %lx '%s' %d\n",
- fl_idx, bios_size, -bios_size,
- blk_name(blk), fl_sectors);
+ printf("Register parallel flash\n");
#endif
- pflash_cfi02_register((uint32_t)(-bios_size),
+ bios_size = 0x80000;
+ pflash_cfi02_register(0xFFF80000,
NULL, "ef405ep.bios", bios_size,
- blk, 65536, fl_sectors, 1,
+ dinfo ? blk_by_legacy_dinfo(dinfo) : NULL,
+ 65536, bios_size / 65536, 1,
2, 0x0001, 0x22DA, 0x0000, 0x0000, 0x555, 0x2AA,
1);
- fl_idx++;
} else
#endif
{
@@ -455,7 +448,7 @@ static void taihu_405ep_init(MachineState *machine)
target_ulong kernel_base, initrd_base;
long kernel_size, initrd_size;
int linux_boot;
- int fl_idx, fl_sectors;
+ int fl_idx;
DriveInfo *dinfo;
/* RAM is soldered to the board so the size cannot be changed */
@@ -486,21 +479,14 @@ static void taihu_405ep_init(MachineState *machine)
#if defined(USE_FLASH_BIOS)
dinfo = drive_get(IF_PFLASH, 0, fl_idx);
if (dinfo) {
- BlockBackend *blk = blk_by_legacy_dinfo(dinfo);
-
- bios_size = blk_getlength(blk);
- /* XXX: should check that size is 2MB */
- // bios_size = 2 * 1024 * 1024;
- fl_sectors = (bios_size + 65535) >> 16;
#ifdef DEBUG_BOARD_INIT
- printf("Register parallel flash %d size %lx"
- " at addr %lx '%s' %d\n",
- fl_idx, bios_size, -bios_size,
- blk_name(blk), fl_sectors);
+ printf("Register boot flash\n");
#endif
- pflash_cfi02_register((uint32_t)(-bios_size),
+ bios_size = 2 * MiB;
+ pflash_cfi02_register(0xFFE00000,
NULL, "taihu_405ep.bios", bios_size,
- blk, 65536, fl_sectors, 1,
+ dinfo ? blk_by_legacy_dinfo(dinfo) : NULL,
+ 65536, bios_size / 65536, 1,
4, 0x0001, 0x22DA, 0x0000, 0x0000, 0x555, 0x2AA,
1);
fl_idx++;
@@ -536,20 +522,13 @@ static void taihu_405ep_init(MachineState *machine)
/* Register Linux flash */
dinfo = drive_get(IF_PFLASH, 0, fl_idx);
if (dinfo) {
- BlockBackend *blk = blk_by_legacy_dinfo(dinfo);
-
- bios_size = blk_getlength(blk);
- /* XXX: should check that size is 32MB */
bios_size = 32 * MiB;
- fl_sectors = (bios_size + 65535) >> 16;
#ifdef DEBUG_BOARD_INIT
- printf("Register parallel flash %d size %lx"
- " at addr " TARGET_FMT_lx " '%s'\n",
- fl_idx, bios_size, (target_ulong)0xfc000000,
- blk_name(blk));
+ printf("Register application flash\n"
#endif
pflash_cfi02_register(0xfc000000, NULL, "taihu_405ep.flash", bios_size,
- blk, 65536, fl_sectors, 1,
+ dinfo ? blk_by_legacy_dinfo(dinfo) : NULL,
+ 65536, bios_size / 65536, 1,
4, 0x0001, 0x22DA, 0x0000, 0x0000, 0x555, 0x2AA,
1);
fl_idx++;
--
2.17.2
On Mon, Feb 18, 2019 at 01:56:10PM +0100, Markus Armbruster wrote:
> Machine "ref405ep" maps its flash memory at address 2^32 - image size.
> Image size is rounded up to the next multiple of 64KiB. Useless,
> because pflash_cfi02_realize() fails with "failed to read the initial
> flash content" unless the rounding is a no-op.
>
> If the image size exceeds 0x80000 Bytes, we overlap first SRAM, then
> other stuff. No idea how that would play out, but a useful outcomes
> seem unlikely.
>
> Map the flash memory at fixed address 0xFFF80000 with size 512KiB,
> regardless of image size, to match the physical hardware.
>
> Machine "taihu" maps its boot flash memory similarly. The code even
> has a comment /* XXX: should check that size is 2MB */, followed by
> disabled code to adjust the size to 2MiB regardless of image size.
>
> Its code to map its application flash memory looks the same, except
> there the XXX comment asks for 32MiB, and the code to adjust the size
> isn't disabled. Note that pflash_cfi02_realize() fails with "failed
> to read the initial flash content" for images smaller than 32MiB.
>
> Map the boot flash memory at fixed address 0xFFE00000 with size 2MiB,
> to match the physical hardware. Delete dead code from application
> flash mapping, and simplify some.
>
> Cc: David Gibson <david@gibson.dropbear.id.au>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
Acked-by: David Gibson <david@gibson.dropbear.id.au>
> ---
> hw/ppc/ppc405_boards.c | 53 +++++++++++++-----------------------------
> 1 file changed, 16 insertions(+), 37 deletions(-)
>
> diff --git a/hw/ppc/ppc405_boards.c b/hw/ppc/ppc405_boards.c
> index f47b15f10e..728154aebb 100644
> --- a/hw/ppc/ppc405_boards.c
> +++ b/hw/ppc/ppc405_boards.c
> @@ -158,7 +158,7 @@ static void ref405ep_init(MachineState *machine)
> target_ulong kernel_base, initrd_base;
> long kernel_size, initrd_size;
> int linux_boot;
> - int fl_idx, fl_sectors, len;
> + int len;
> DriveInfo *dinfo;
> MemoryRegion *sysmem = get_system_memory();
>
> @@ -185,26 +185,19 @@ static void ref405ep_init(MachineState *machine)
> #ifdef DEBUG_BOARD_INIT
> printf("%s: register BIOS\n", __func__);
> #endif
> - fl_idx = 0;
> #ifdef USE_FLASH_BIOS
> - dinfo = drive_get(IF_PFLASH, 0, fl_idx);
> + dinfo = drive_get(IF_PFLASH, 0, 0);
> if (dinfo) {
> - BlockBackend *blk = blk_by_legacy_dinfo(dinfo);
> -
> - bios_size = blk_getlength(blk);
> - fl_sectors = (bios_size + 65535) >> 16;
> #ifdef DEBUG_BOARD_INIT
> - printf("Register parallel flash %d size %lx"
> - " at addr %lx '%s' %d\n",
> - fl_idx, bios_size, -bios_size,
> - blk_name(blk), fl_sectors);
> + printf("Register parallel flash\n");
> #endif
> - pflash_cfi02_register((uint32_t)(-bios_size),
> + bios_size = 0x80000;
> + pflash_cfi02_register(0xFFF80000,
> NULL, "ef405ep.bios", bios_size,
> - blk, 65536, fl_sectors, 1,
> + dinfo ? blk_by_legacy_dinfo(dinfo) : NULL,
> + 65536, bios_size / 65536, 1,
> 2, 0x0001, 0x22DA, 0x0000, 0x0000, 0x555, 0x2AA,
> 1);
> - fl_idx++;
> } else
> #endif
> {
> @@ -455,7 +448,7 @@ static void taihu_405ep_init(MachineState *machine)
> target_ulong kernel_base, initrd_base;
> long kernel_size, initrd_size;
> int linux_boot;
> - int fl_idx, fl_sectors;
> + int fl_idx;
> DriveInfo *dinfo;
>
> /* RAM is soldered to the board so the size cannot be changed */
> @@ -486,21 +479,14 @@ static void taihu_405ep_init(MachineState *machine)
> #if defined(USE_FLASH_BIOS)
> dinfo = drive_get(IF_PFLASH, 0, fl_idx);
> if (dinfo) {
> - BlockBackend *blk = blk_by_legacy_dinfo(dinfo);
> -
> - bios_size = blk_getlength(blk);
> - /* XXX: should check that size is 2MB */
> - // bios_size = 2 * 1024 * 1024;
> - fl_sectors = (bios_size + 65535) >> 16;
> #ifdef DEBUG_BOARD_INIT
> - printf("Register parallel flash %d size %lx"
> - " at addr %lx '%s' %d\n",
> - fl_idx, bios_size, -bios_size,
> - blk_name(blk), fl_sectors);
> + printf("Register boot flash\n");
> #endif
> - pflash_cfi02_register((uint32_t)(-bios_size),
> + bios_size = 2 * MiB;
> + pflash_cfi02_register(0xFFE00000,
> NULL, "taihu_405ep.bios", bios_size,
> - blk, 65536, fl_sectors, 1,
> + dinfo ? blk_by_legacy_dinfo(dinfo) : NULL,
> + 65536, bios_size / 65536, 1,
> 4, 0x0001, 0x22DA, 0x0000, 0x0000, 0x555, 0x2AA,
> 1);
> fl_idx++;
> @@ -536,20 +522,13 @@ static void taihu_405ep_init(MachineState *machine)
> /* Register Linux flash */
> dinfo = drive_get(IF_PFLASH, 0, fl_idx);
> if (dinfo) {
> - BlockBackend *blk = blk_by_legacy_dinfo(dinfo);
> -
> - bios_size = blk_getlength(blk);
> - /* XXX: should check that size is 32MB */
> bios_size = 32 * MiB;
> - fl_sectors = (bios_size + 65535) >> 16;
> #ifdef DEBUG_BOARD_INIT
> - printf("Register parallel flash %d size %lx"
> - " at addr " TARGET_FMT_lx " '%s'\n",
> - fl_idx, bios_size, (target_ulong)0xfc000000,
> - blk_name(blk));
> + printf("Register application flash\n"
> #endif
> pflash_cfi02_register(0xfc000000, NULL, "taihu_405ep.flash", bios_size,
> - blk, 65536, fl_sectors, 1,
> + dinfo ? blk_by_legacy_dinfo(dinfo) : NULL,
> + 65536, bios_size / 65536, 1,
> 4, 0x0001, 0x22DA, 0x0000, 0x0000, 0x555, 0x2AA,
> 1);
> fl_idx++;
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
On Mon, 18 Feb 2019, Markus Armbruster wrote:
> Machine "ref405ep" maps its flash memory at address 2^32 - image size.
> Image size is rounded up to the next multiple of 64KiB. Useless,
> because pflash_cfi02_realize() fails with "failed to read the initial
> flash content" unless the rounding is a no-op.
>
> If the image size exceeds 0x80000 Bytes, we overlap first SRAM, then
> other stuff. No idea how that would play out, but a useful outcomes
> seem unlikely.
>
> Map the flash memory at fixed address 0xFFF80000 with size 512KiB,
> regardless of image size, to match the physical hardware.
If PPC405 behaves the same as PPC440 and starts at 0xfffffffc then this
won't boot. It's maybe better to keep 2^32 - image_size but assert image
is not bigger than 512kB. But I don't know anything about these boards so
just sharing this comment for your consideration. I'm fine with any
decision you take.
Regards,
BALATON Zoltan
> Machine "taihu" maps its boot flash memory similarly. The code even
> has a comment /* XXX: should check that size is 2MB */, followed by
> disabled code to adjust the size to 2MiB regardless of image size.
>
> Its code to map its application flash memory looks the same, except
> there the XXX comment asks for 32MiB, and the code to adjust the size
> isn't disabled. Note that pflash_cfi02_realize() fails with "failed
> to read the initial flash content" for images smaller than 32MiB.
>
> Map the boot flash memory at fixed address 0xFFE00000 with size 2MiB,
> to match the physical hardware. Delete dead code from application
> flash mapping, and simplify some.
>
> Cc: David Gibson <david@gibson.dropbear.id.au>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
> hw/ppc/ppc405_boards.c | 53 +++++++++++++-----------------------------
> 1 file changed, 16 insertions(+), 37 deletions(-)
>
> diff --git a/hw/ppc/ppc405_boards.c b/hw/ppc/ppc405_boards.c
> index f47b15f10e..728154aebb 100644
> --- a/hw/ppc/ppc405_boards.c
> +++ b/hw/ppc/ppc405_boards.c
> @@ -158,7 +158,7 @@ static void ref405ep_init(MachineState *machine)
> target_ulong kernel_base, initrd_base;
> long kernel_size, initrd_size;
> int linux_boot;
> - int fl_idx, fl_sectors, len;
> + int len;
> DriveInfo *dinfo;
> MemoryRegion *sysmem = get_system_memory();
>
> @@ -185,26 +185,19 @@ static void ref405ep_init(MachineState *machine)
> #ifdef DEBUG_BOARD_INIT
> printf("%s: register BIOS\n", __func__);
> #endif
> - fl_idx = 0;
> #ifdef USE_FLASH_BIOS
> - dinfo = drive_get(IF_PFLASH, 0, fl_idx);
> + dinfo = drive_get(IF_PFLASH, 0, 0);
> if (dinfo) {
> - BlockBackend *blk = blk_by_legacy_dinfo(dinfo);
> -
> - bios_size = blk_getlength(blk);
> - fl_sectors = (bios_size + 65535) >> 16;
> #ifdef DEBUG_BOARD_INIT
> - printf("Register parallel flash %d size %lx"
> - " at addr %lx '%s' %d\n",
> - fl_idx, bios_size, -bios_size,
> - blk_name(blk), fl_sectors);
> + printf("Register parallel flash\n");
> #endif
> - pflash_cfi02_register((uint32_t)(-bios_size),
> + bios_size = 0x80000;
> + pflash_cfi02_register(0xFFF80000,
> NULL, "ef405ep.bios", bios_size,
> - blk, 65536, fl_sectors, 1,
> + dinfo ? blk_by_legacy_dinfo(dinfo) : NULL,
> + 65536, bios_size / 65536, 1,
> 2, 0x0001, 0x22DA, 0x0000, 0x0000, 0x555, 0x2AA,
> 1);
> - fl_idx++;
> } else
> #endif
> {
> @@ -455,7 +448,7 @@ static void taihu_405ep_init(MachineState *machine)
> target_ulong kernel_base, initrd_base;
> long kernel_size, initrd_size;
> int linux_boot;
> - int fl_idx, fl_sectors;
> + int fl_idx;
> DriveInfo *dinfo;
>
> /* RAM is soldered to the board so the size cannot be changed */
> @@ -486,21 +479,14 @@ static void taihu_405ep_init(MachineState *machine)
> #if defined(USE_FLASH_BIOS)
> dinfo = drive_get(IF_PFLASH, 0, fl_idx);
> if (dinfo) {
> - BlockBackend *blk = blk_by_legacy_dinfo(dinfo);
> -
> - bios_size = blk_getlength(blk);
> - /* XXX: should check that size is 2MB */
> - // bios_size = 2 * 1024 * 1024;
> - fl_sectors = (bios_size + 65535) >> 16;
> #ifdef DEBUG_BOARD_INIT
> - printf("Register parallel flash %d size %lx"
> - " at addr %lx '%s' %d\n",
> - fl_idx, bios_size, -bios_size,
> - blk_name(blk), fl_sectors);
> + printf("Register boot flash\n");
> #endif
> - pflash_cfi02_register((uint32_t)(-bios_size),
> + bios_size = 2 * MiB;
> + pflash_cfi02_register(0xFFE00000,
> NULL, "taihu_405ep.bios", bios_size,
> - blk, 65536, fl_sectors, 1,
> + dinfo ? blk_by_legacy_dinfo(dinfo) : NULL,
> + 65536, bios_size / 65536, 1,
> 4, 0x0001, 0x22DA, 0x0000, 0x0000, 0x555, 0x2AA,
> 1);
> fl_idx++;
> @@ -536,20 +522,13 @@ static void taihu_405ep_init(MachineState *machine)
> /* Register Linux flash */
> dinfo = drive_get(IF_PFLASH, 0, fl_idx);
> if (dinfo) {
> - BlockBackend *blk = blk_by_legacy_dinfo(dinfo);
> -
> - bios_size = blk_getlength(blk);
> - /* XXX: should check that size is 32MB */
> bios_size = 32 * MiB;
> - fl_sectors = (bios_size + 65535) >> 16;
> #ifdef DEBUG_BOARD_INIT
> - printf("Register parallel flash %d size %lx"
> - " at addr " TARGET_FMT_lx " '%s'\n",
> - fl_idx, bios_size, (target_ulong)0xfc000000,
> - blk_name(blk));
> + printf("Register application flash\n"
> #endif
> pflash_cfi02_register(0xfc000000, NULL, "taihu_405ep.flash", bios_size,
> - blk, 65536, fl_sectors, 1,
> + dinfo ? blk_by_legacy_dinfo(dinfo) : NULL,
> + 65536, bios_size / 65536, 1,
> 4, 0x0001, 0x22DA, 0x0000, 0x0000, 0x555, 0x2AA,
> 1);
> fl_idx++;
>
BALATON Zoltan <balaton@eik.bme.hu> writes: > On Mon, 18 Feb 2019, Markus Armbruster wrote: >> Machine "ref405ep" maps its flash memory at address 2^32 - image size. >> Image size is rounded up to the next multiple of 64KiB. Useless, >> because pflash_cfi02_realize() fails with "failed to read the initial >> flash content" unless the rounding is a no-op. >> >> If the image size exceeds 0x80000 Bytes, we overlap first SRAM, then >> other stuff. No idea how that would play out, but a useful outcomes >> seem unlikely. >> >> Map the flash memory at fixed address 0xFFF80000 with size 512KiB, >> regardless of image size, to match the physical hardware. > > If PPC405 behaves the same as PPC440 and starts at 0xfffffffc then > this won't boot. It's maybe better to keep 2^32 - image_size but > assert image is not bigger than 512kB. But I don't know anything about > these boards so just sharing this comment for your consideration. I'm > fine with any decision you take. If the image is smaller than 512KiB, pflash_cfi02_realize() fails with "failed to read the initial flash content". Alex Bennée has a patch that'll make it fail for any size mismatch, with a much nicer error message. I like it; silently truncating firmware images is unlikely to be useful. For what it's worth, my patch brings this board into line with most other boards: create flash memory of fixed size at a fixed address, matching the physical machine we emulate.
Markus Armbruster <armbru@redhat.com> writes:
> Machine "ref405ep" maps its flash memory at address 2^32 - image size.
> Image size is rounded up to the next multiple of 64KiB. Useless,
> because pflash_cfi02_realize() fails with "failed to read the initial
> flash content" unless the rounding is a no-op.
>
> If the image size exceeds 0x80000 Bytes, we overlap first SRAM, then
> other stuff. No idea how that would play out, but a useful outcomes
> seem unlikely.
>
> Map the flash memory at fixed address 0xFFF80000 with size 512KiB,
> regardless of image size, to match the physical hardware.
>
> Machine "taihu" maps its boot flash memory similarly. The code even
> has a comment /* XXX: should check that size is 2MB */, followed by
> disabled code to adjust the size to 2MiB regardless of image size.
>
> Its code to map its application flash memory looks the same, except
> there the XXX comment asks for 32MiB, and the code to adjust the size
> isn't disabled. Note that pflash_cfi02_realize() fails with "failed
> to read the initial flash content" for images smaller than 32MiB.
>
> Map the boot flash memory at fixed address 0xFFE00000 with size 2MiB,
> to match the physical hardware. Delete dead code from application
> flash mapping, and simplify some.
>
> Cc: David Gibson <david@gibson.dropbear.id.au>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
> hw/ppc/ppc405_boards.c | 53 +++++++++++++-----------------------------
> 1 file changed, 16 insertions(+), 37 deletions(-)
>
> diff --git a/hw/ppc/ppc405_boards.c b/hw/ppc/ppc405_boards.c
> index f47b15f10e..728154aebb 100644
> --- a/hw/ppc/ppc405_boards.c
> +++ b/hw/ppc/ppc405_boards.c
> @@ -158,7 +158,7 @@ static void ref405ep_init(MachineState *machine)
> target_ulong kernel_base, initrd_base;
> long kernel_size, initrd_size;
> int linux_boot;
> - int fl_idx, fl_sectors, len;
> + int len;
> DriveInfo *dinfo;
> MemoryRegion *sysmem = get_system_memory();
>
> @@ -185,26 +185,19 @@ static void ref405ep_init(MachineState *machine)
> #ifdef DEBUG_BOARD_INIT
> printf("%s: register BIOS\n", __func__);
> #endif
> - fl_idx = 0;
> #ifdef USE_FLASH_BIOS
> - dinfo = drive_get(IF_PFLASH, 0, fl_idx);
> + dinfo = drive_get(IF_PFLASH, 0, 0);
> if (dinfo) {
> - BlockBackend *blk = blk_by_legacy_dinfo(dinfo);
> -
> - bios_size = blk_getlength(blk);
> - fl_sectors = (bios_size + 65535) >> 16;
> #ifdef DEBUG_BOARD_INIT
> - printf("Register parallel flash %d size %lx"
> - " at addr %lx '%s' %d\n",
> - fl_idx, bios_size, -bios_size,
> - blk_name(blk), fl_sectors);
> + printf("Register parallel flash\n");
> #endif
> - pflash_cfi02_register((uint32_t)(-bios_size),
> + bios_size = 0x80000;
bios_size = 8 * MiB?
> + pflash_cfi02_register(0xFFF80000,
> NULL, "ef405ep.bios", bios_size,
> - blk, 65536, fl_sectors, 1,
> + dinfo ? blk_by_legacy_dinfo(dinfo) : NULL,
> + 65536, bios_size / 65536, 1,
64 * KiB?
> 2, 0x0001, 0x22DA, 0x0000, 0x0000, 0x555, 0x2AA,
> 1);
> - fl_idx++;
> } else
> #endif
> {
> @@ -455,7 +448,7 @@ static void taihu_405ep_init(MachineState *machine)
> target_ulong kernel_base, initrd_base;
> long kernel_size, initrd_size;
> int linux_boot;
> - int fl_idx, fl_sectors;
> + int fl_idx;
> DriveInfo *dinfo;
>
> /* RAM is soldered to the board so the size cannot be changed */
> @@ -486,21 +479,14 @@ static void taihu_405ep_init(MachineState *machine)
> #if defined(USE_FLASH_BIOS)
> dinfo = drive_get(IF_PFLASH, 0, fl_idx);
> if (dinfo) {
> - BlockBackend *blk = blk_by_legacy_dinfo(dinfo);
> -
> - bios_size = blk_getlength(blk);
> - /* XXX: should check that size is 2MB */
> - // bios_size = 2 * 1024 * 1024;
> - fl_sectors = (bios_size + 65535) >> 16;
> #ifdef DEBUG_BOARD_INIT
> - printf("Register parallel flash %d size %lx"
> - " at addr %lx '%s' %d\n",
> - fl_idx, bios_size, -bios_size,
> - blk_name(blk), fl_sectors);
> + printf("Register boot flash\n");
> #endif
> - pflash_cfi02_register((uint32_t)(-bios_size),
> + bios_size = 2 * MiB;
> + pflash_cfi02_register(0xFFE00000,
> NULL, "taihu_405ep.bios", bios_size,
> - blk, 65536, fl_sectors, 1,
> + dinfo ? blk_by_legacy_dinfo(dinfo) : NULL,
> + 65536, bios_size / 65536, 1,
64 * KiB
> 4, 0x0001, 0x22DA, 0x0000, 0x0000, 0x555, 0x2AA,
> 1);
> fl_idx++;
> @@ -536,20 +522,13 @@ static void taihu_405ep_init(MachineState *machine)
> /* Register Linux flash */
> dinfo = drive_get(IF_PFLASH, 0, fl_idx);
> if (dinfo) {
> - BlockBackend *blk = blk_by_legacy_dinfo(dinfo);
> -
> - bios_size = blk_getlength(blk);
> - /* XXX: should check that size is 32MB */
> bios_size = 32 * MiB;
> - fl_sectors = (bios_size + 65535) >> 16;
> #ifdef DEBUG_BOARD_INIT
> - printf("Register parallel flash %d size %lx"
> - " at addr " TARGET_FMT_lx " '%s'\n",
> - fl_idx, bios_size, (target_ulong)0xfc000000,
> - blk_name(blk));
> + printf("Register application flash\n"
> #endif
> pflash_cfi02_register(0xfc000000, NULL, "taihu_405ep.flash", bios_size,
> - blk, 65536, fl_sectors, 1,
> + dinfo ? blk_by_legacy_dinfo(dinfo) : NULL,
> + 65536, bios_size / 65536, 1,
64 * KiB
> 4, 0x0001, 0x22DA, 0x0000, 0x0000, 0x555, 0x2AA,
> 1);
> fl_idx++;
--
Alex Bennée
Alex Bennée <alex.bennee@linaro.org> writes:
> Markus Armbruster <armbru@redhat.com> writes:
>
>> Machine "ref405ep" maps its flash memory at address 2^32 - image size.
>> Image size is rounded up to the next multiple of 64KiB. Useless,
>> because pflash_cfi02_realize() fails with "failed to read the initial
>> flash content" unless the rounding is a no-op.
>>
>> If the image size exceeds 0x80000 Bytes, we overlap first SRAM, then
>> other stuff. No idea how that would play out, but a useful outcomes
>> seem unlikely.
>>
>> Map the flash memory at fixed address 0xFFF80000 with size 512KiB,
>> regardless of image size, to match the physical hardware.
>>
>> Machine "taihu" maps its boot flash memory similarly. The code even
>> has a comment /* XXX: should check that size is 2MB */, followed by
>> disabled code to adjust the size to 2MiB regardless of image size.
>>
>> Its code to map its application flash memory looks the same, except
>> there the XXX comment asks for 32MiB, and the code to adjust the size
>> isn't disabled. Note that pflash_cfi02_realize() fails with "failed
>> to read the initial flash content" for images smaller than 32MiB.
>>
>> Map the boot flash memory at fixed address 0xFFE00000 with size 2MiB,
>> to match the physical hardware. Delete dead code from application
>> flash mapping, and simplify some.
>>
>> Cc: David Gibson <david@gibson.dropbear.id.au>
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>> hw/ppc/ppc405_boards.c | 53 +++++++++++++-----------------------------
>> 1 file changed, 16 insertions(+), 37 deletions(-)
>>
>> diff --git a/hw/ppc/ppc405_boards.c b/hw/ppc/ppc405_boards.c
>> index f47b15f10e..728154aebb 100644
>> --- a/hw/ppc/ppc405_boards.c
>> +++ b/hw/ppc/ppc405_boards.c
>> @@ -158,7 +158,7 @@ static void ref405ep_init(MachineState *machine)
>> target_ulong kernel_base, initrd_base;
>> long kernel_size, initrd_size;
>> int linux_boot;
>> - int fl_idx, fl_sectors, len;
>> + int len;
>> DriveInfo *dinfo;
>> MemoryRegion *sysmem = get_system_memory();
>>
>> @@ -185,26 +185,19 @@ static void ref405ep_init(MachineState *machine)
>> #ifdef DEBUG_BOARD_INIT
>> printf("%s: register BIOS\n", __func__);
>> #endif
>> - fl_idx = 0;
>> #ifdef USE_FLASH_BIOS
>> - dinfo = drive_get(IF_PFLASH, 0, fl_idx);
>> + dinfo = drive_get(IF_PFLASH, 0, 0);
>> if (dinfo) {
>> - BlockBackend *blk = blk_by_legacy_dinfo(dinfo);
>> -
>> - bios_size = blk_getlength(blk);
>> - fl_sectors = (bios_size + 65535) >> 16;
>> #ifdef DEBUG_BOARD_INIT
>> - printf("Register parallel flash %d size %lx"
>> - " at addr %lx '%s' %d\n",
>> - fl_idx, bios_size, -bios_size,
>> - blk_name(blk), fl_sectors);
>> + printf("Register parallel flash\n");
>> #endif
>> - pflash_cfi02_register((uint32_t)(-bios_size),
>> + bios_size = 0x80000;
>
> bios_size = 8 * MiB?
The next line has base address 0xFFF80000. I picked 0x80000 to make
0xFFF80000 + 0x80000 == 0 mod 2^32 more obvious.
If I change 0x80000 to 8 * MiB, the size is more obvious, but "at end of
32 bit address space" less so.
If I additionally change the base address back to ((uint32_t)-bios_size,
"at end of 32 bit address space" is obvious again, but the actual base
address less so.
I don't really care myself. David, you're the maintainer, do you have a
preference?
>> + pflash_cfi02_register(0xFFF80000,
>> NULL, "ef405ep.bios", bios_size,
>> - blk, 65536, fl_sectors, 1,
>> + dinfo ? blk_by_legacy_dinfo(dinfo) : NULL,
>> + 65536, bios_size / 65536, 1,
>
> 64 * KiB?
David, same question (two additional instances below).
>> 2, 0x0001, 0x22DA, 0x0000, 0x0000, 0x555, 0x2AA,
>> 1);
>> - fl_idx++;
>> } else
>> #endif
>> {
>> @@ -455,7 +448,7 @@ static void taihu_405ep_init(MachineState *machine)
>> target_ulong kernel_base, initrd_base;
>> long kernel_size, initrd_size;
>> int linux_boot;
>> - int fl_idx, fl_sectors;
>> + int fl_idx;
>> DriveInfo *dinfo;
>>
>> /* RAM is soldered to the board so the size cannot be changed */
>> @@ -486,21 +479,14 @@ static void taihu_405ep_init(MachineState *machine)
>> #if defined(USE_FLASH_BIOS)
>> dinfo = drive_get(IF_PFLASH, 0, fl_idx);
>> if (dinfo) {
>> - BlockBackend *blk = blk_by_legacy_dinfo(dinfo);
>> -
>> - bios_size = blk_getlength(blk);
>> - /* XXX: should check that size is 2MB */
>> - // bios_size = 2 * 1024 * 1024;
>> - fl_sectors = (bios_size + 65535) >> 16;
>> #ifdef DEBUG_BOARD_INIT
>> - printf("Register parallel flash %d size %lx"
>> - " at addr %lx '%s' %d\n",
>> - fl_idx, bios_size, -bios_size,
>> - blk_name(blk), fl_sectors);
>> + printf("Register boot flash\n");
>> #endif
>> - pflash_cfi02_register((uint32_t)(-bios_size),
>> + bios_size = 2 * MiB;
>> + pflash_cfi02_register(0xFFE00000,
>> NULL, "taihu_405ep.bios", bios_size,
>> - blk, 65536, fl_sectors, 1,
>> + dinfo ? blk_by_legacy_dinfo(dinfo) : NULL,
>> + 65536, bios_size / 65536, 1,
>
> 64 * KiB
>
>> 4, 0x0001, 0x22DA, 0x0000, 0x0000, 0x555, 0x2AA,
>> 1);
>> fl_idx++;
>> @@ -536,20 +522,13 @@ static void taihu_405ep_init(MachineState *machine)
>> /* Register Linux flash */
>> dinfo = drive_get(IF_PFLASH, 0, fl_idx);
>> if (dinfo) {
>> - BlockBackend *blk = blk_by_legacy_dinfo(dinfo);
>> -
>> - bios_size = blk_getlength(blk);
>> - /* XXX: should check that size is 32MB */
>> bios_size = 32 * MiB;
>> - fl_sectors = (bios_size + 65535) >> 16;
>> #ifdef DEBUG_BOARD_INIT
>> - printf("Register parallel flash %d size %lx"
>> - " at addr " TARGET_FMT_lx " '%s'\n",
>> - fl_idx, bios_size, (target_ulong)0xfc000000,
>> - blk_name(blk));
>> + printf("Register application flash\n"
>> #endif
>> pflash_cfi02_register(0xfc000000, NULL, "taihu_405ep.flash", bios_size,
>> - blk, 65536, fl_sectors, 1,
>> + dinfo ? blk_by_legacy_dinfo(dinfo) : NULL,
>> + 65536, bios_size / 65536, 1,
>
> 64 * KiB
>
>> 4, 0x0001, 0x22DA, 0x0000, 0x0000, 0x555, 0x2AA,
>> 1);
>> fl_idx++;
>
>
> --
> Alex Bennée
On Thu, Feb 21, 2019 at 05:31:30PM +0100, Markus Armbruster wrote:
> Alex Bennée <alex.bennee@linaro.org> writes:
>
> > Markus Armbruster <armbru@redhat.com> writes:
> >
> >> Machine "ref405ep" maps its flash memory at address 2^32 - image size.
> >> Image size is rounded up to the next multiple of 64KiB. Useless,
> >> because pflash_cfi02_realize() fails with "failed to read the initial
> >> flash content" unless the rounding is a no-op.
> >>
> >> If the image size exceeds 0x80000 Bytes, we overlap first SRAM, then
> >> other stuff. No idea how that would play out, but a useful outcomes
> >> seem unlikely.
> >>
> >> Map the flash memory at fixed address 0xFFF80000 with size 512KiB,
> >> regardless of image size, to match the physical hardware.
> >>
> >> Machine "taihu" maps its boot flash memory similarly. The code even
> >> has a comment /* XXX: should check that size is 2MB */, followed by
> >> disabled code to adjust the size to 2MiB regardless of image size.
> >>
> >> Its code to map its application flash memory looks the same, except
> >> there the XXX comment asks for 32MiB, and the code to adjust the size
> >> isn't disabled. Note that pflash_cfi02_realize() fails with "failed
> >> to read the initial flash content" for images smaller than 32MiB.
> >>
> >> Map the boot flash memory at fixed address 0xFFE00000 with size 2MiB,
> >> to match the physical hardware. Delete dead code from application
> >> flash mapping, and simplify some.
> >>
> >> Cc: David Gibson <david@gibson.dropbear.id.au>
> >> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> >> ---
> >> hw/ppc/ppc405_boards.c | 53 +++++++++++++-----------------------------
> >> 1 file changed, 16 insertions(+), 37 deletions(-)
> >>
> >> diff --git a/hw/ppc/ppc405_boards.c b/hw/ppc/ppc405_boards.c
> >> index f47b15f10e..728154aebb 100644
> >> --- a/hw/ppc/ppc405_boards.c
> >> +++ b/hw/ppc/ppc405_boards.c
> >> @@ -158,7 +158,7 @@ static void ref405ep_init(MachineState *machine)
> >> target_ulong kernel_base, initrd_base;
> >> long kernel_size, initrd_size;
> >> int linux_boot;
> >> - int fl_idx, fl_sectors, len;
> >> + int len;
> >> DriveInfo *dinfo;
> >> MemoryRegion *sysmem = get_system_memory();
> >>
> >> @@ -185,26 +185,19 @@ static void ref405ep_init(MachineState *machine)
> >> #ifdef DEBUG_BOARD_INIT
> >> printf("%s: register BIOS\n", __func__);
> >> #endif
> >> - fl_idx = 0;
> >> #ifdef USE_FLASH_BIOS
> >> - dinfo = drive_get(IF_PFLASH, 0, fl_idx);
> >> + dinfo = drive_get(IF_PFLASH, 0, 0);
> >> if (dinfo) {
> >> - BlockBackend *blk = blk_by_legacy_dinfo(dinfo);
> >> -
> >> - bios_size = blk_getlength(blk);
> >> - fl_sectors = (bios_size + 65535) >> 16;
> >> #ifdef DEBUG_BOARD_INIT
> >> - printf("Register parallel flash %d size %lx"
> >> - " at addr %lx '%s' %d\n",
> >> - fl_idx, bios_size, -bios_size,
> >> - blk_name(blk), fl_sectors);
> >> + printf("Register parallel flash\n");
> >> #endif
> >> - pflash_cfi02_register((uint32_t)(-bios_size),
> >> + bios_size = 0x80000;
> >
> > bios_size = 8 * MiB?
>
> The next line has base address 0xFFF80000. I picked 0x80000 to make
> 0xFFF80000 + 0x80000 == 0 mod 2^32 more obvious.
>
> If I change 0x80000 to 8 * MiB, the size is more obvious, but "at end of
> 32 bit address space" less so.
>
> If I additionally change the base address back to ((uint32_t)-bios_size,
> "at end of 32 bit address space" is obvious again, but the actual base
> address less so.
I have a weak preference for ((uint32_t)-bios_size), with bios_size =
8 * MiB.
>
> I don't really care myself. David, you're the maintainer, do you have a
> preference?
>
> >> + pflash_cfi02_register(0xFFF80000,
> >> NULL, "ef405ep.bios", bios_size,
> >> - blk, 65536, fl_sectors, 1,
> >> + dinfo ? blk_by_legacy_dinfo(dinfo) : NULL,
> >> + 65536, bios_size / 65536, 1,
> >
> > 64 * KiB?
>
> David, same question (two additional instances below).
Here I think 64 * KiB would be nice in each of those places. Again,
only a weak preference.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
David Gibson <david@gibson.dropbear.id.au> writes:
> On Thu, Feb 21, 2019 at 05:31:30PM +0100, Markus Armbruster wrote:
>> Alex Bennée <alex.bennee@linaro.org> writes:
>>
>> > Markus Armbruster <armbru@redhat.com> writes:
>> >
>> >> Machine "ref405ep" maps its flash memory at address 2^32 - image size.
>> >> Image size is rounded up to the next multiple of 64KiB. Useless,
>> >> because pflash_cfi02_realize() fails with "failed to read the initial
>> >> flash content" unless the rounding is a no-op.
>> >>
>> >> If the image size exceeds 0x80000 Bytes, we overlap first SRAM, then
>> >> other stuff. No idea how that would play out, but a useful outcomes
>> >> seem unlikely.
>> >>
>> >> Map the flash memory at fixed address 0xFFF80000 with size 512KiB,
>> >> regardless of image size, to match the physical hardware.
>> >>
>> >> Machine "taihu" maps its boot flash memory similarly. The code even
>> >> has a comment /* XXX: should check that size is 2MB */, followed by
>> >> disabled code to adjust the size to 2MiB regardless of image size.
>> >>
>> >> Its code to map its application flash memory looks the same, except
>> >> there the XXX comment asks for 32MiB, and the code to adjust the size
>> >> isn't disabled. Note that pflash_cfi02_realize() fails with "failed
>> >> to read the initial flash content" for images smaller than 32MiB.
>> >>
>> >> Map the boot flash memory at fixed address 0xFFE00000 with size 2MiB,
>> >> to match the physical hardware. Delete dead code from application
>> >> flash mapping, and simplify some.
>> >>
>> >> Cc: David Gibson <david@gibson.dropbear.id.au>
>> >> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> >> ---
>> >> hw/ppc/ppc405_boards.c | 53 +++++++++++++-----------------------------
>> >> 1 file changed, 16 insertions(+), 37 deletions(-)
>> >>
>> >> diff --git a/hw/ppc/ppc405_boards.c b/hw/ppc/ppc405_boards.c
>> >> index f47b15f10e..728154aebb 100644
>> >> --- a/hw/ppc/ppc405_boards.c
>> >> +++ b/hw/ppc/ppc405_boards.c
>> >> @@ -158,7 +158,7 @@ static void ref405ep_init(MachineState *machine)
>> >> target_ulong kernel_base, initrd_base;
>> >> long kernel_size, initrd_size;
>> >> int linux_boot;
>> >> - int fl_idx, fl_sectors, len;
>> >> + int len;
>> >> DriveInfo *dinfo;
>> >> MemoryRegion *sysmem = get_system_memory();
>> >>
>> >> @@ -185,26 +185,19 @@ static void ref405ep_init(MachineState *machine)
>> >> #ifdef DEBUG_BOARD_INIT
>> >> printf("%s: register BIOS\n", __func__);
>> >> #endif
>> >> - fl_idx = 0;
>> >> #ifdef USE_FLASH_BIOS
>> >> - dinfo = drive_get(IF_PFLASH, 0, fl_idx);
>> >> + dinfo = drive_get(IF_PFLASH, 0, 0);
>> >> if (dinfo) {
>> >> - BlockBackend *blk = blk_by_legacy_dinfo(dinfo);
>> >> -
>> >> - bios_size = blk_getlength(blk);
>> >> - fl_sectors = (bios_size + 65535) >> 16;
>> >> #ifdef DEBUG_BOARD_INIT
>> >> - printf("Register parallel flash %d size %lx"
>> >> - " at addr %lx '%s' %d\n",
>> >> - fl_idx, bios_size, -bios_size,
>> >> - blk_name(blk), fl_sectors);
>> >> + printf("Register parallel flash\n");
>> >> #endif
>> >> - pflash_cfi02_register((uint32_t)(-bios_size),
>> >> + bios_size = 0x80000;
>> >
>> > bios_size = 8 * MiB?
>>
>> The next line has base address 0xFFF80000. I picked 0x80000 to make
>> 0xFFF80000 + 0x80000 == 0 mod 2^32 more obvious.
>>
>> If I change 0x80000 to 8 * MiB, the size is more obvious, but "at end of
>> 32 bit address space" less so.
>>
>> If I additionally change the base address back to ((uint32_t)-bios_size,
>> "at end of 32 bit address space" is obvious again, but the actual base
>> address less so.
>
> I have a weak preference for ((uint32_t)-bios_size), with bios_size =
> 8 * MiB.
>
>>
>> I don't really care myself. David, you're the maintainer, do you have a
>> preference?
>>
>> >> + pflash_cfi02_register(0xFFF80000,
>> >> NULL, "ef405ep.bios", bios_size,
>> >> - blk, 65536, fl_sectors, 1,
>> >> + dinfo ? blk_by_legacy_dinfo(dinfo) : NULL,
>> >> + 65536, bios_size / 65536, 1,
>> >
>> > 64 * KiB?
>>
>> David, same question (two additional instances below).
>
> Here I think 64 * KiB would be nice in each of those places. Again,
> only a weak preference.
Your weak preference is enough to tip my scales. Thanks!
© 2016 - 2026 Red Hat, Inc.