Re: [Qemu-devel] [PATCH 4/5] hw/mips/malta: Only accept 'monitor' pflash of 4MiB

[Markus Armbruster]

Philippe Mathieu-Daudé <address@hidden> writes:

> The Malta 'mother' board can use various 'daughter' core cards [1].
> QEMU only models the CoreLV card.
>
> The CoreLV card provides [2] a Galileo GT64120 as North bridge,
> connecting the CPU via the 'CBUS'. The CBUS also connects a 'Monitor
> flash' memory and maps it to the CPU RESET vector.
> The Monitor flash size is exactly 4 MiB.
> Refuse Monitor pflash of different size.
>
> [1] https://www.linux-mips.org/wiki/MIPS_Malta#Core_cards
> [2] "Malta User's Manual"  rev. 01.05 (MIPS Technologies doc number: MD00048)
>
> Signed-off-by: Philippe Mathieu-Daudé <address@hidden>
> ---
>  hw/mips/mips_malta.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c
> index 396645b1a9..04788ff50a 100644
> --- a/hw/mips/mips_malta.c
> +++ b/hw/mips/mips_malta.c
> @@ -1267,6 +1267,11 @@ void mips_malta_init(MachineState *machine)
>      dinfo = drive_get(IF_PFLASH, 0, fl_idx);
>      if (dinfo) {
>          pflash_blk = blk_by_legacy_dinfo(dinfo);
> +
> +        if (blk_getlength(pflash_blk) != FLASH_SIZE) {
> +                error_report("Malta CoreLV card expects a bios of 4MB");
> +                exit(1);
> +        }
>  #ifdef DEBUG_BOARD_INIT
>          printf("Register parallel flash %d size " TARGET_FMT_lx " at "
>                 "addr %08llx '%s'\n",

I'm in favor of insisting on image size matching flash size, but I'd
rather do it in generic code.

We're already rejecting undersized images, with a sub-optimal error
message.  We're silently ignoring the tail of oversized images.  Alex
proposed a patch to generic code that does three things:

* We reject an undersized image with a sub-optimal error message.
  Improve that message.

* We silently ignore an oversized image's tail.  Warn instead.

* As a convenience feature, don't reject undersized read-only image, but
  pad it with 0xff instead, to simulate (1) above.

[v5] hw/block: better reporting on pflash backing file mismatch
Message-Id: <address@hidden>

I asked for the convenience feature split off into its own patch (and
offered to do it).

If we accept Alex's work (split or not), your patch merely kills the
convenience feature for this board.  I like killing it --- I consider it
a bad idea --- but if that's not in the cards, then I prefer all boards
to implement the same set of bad ideas.