diff mbox series

[4/5] hw/mips/malta: Only accept 'monitor' pflash of 4MiB

Message ID 20190305162829.20079-5-philmd@redhat.com (mailing list archive)
State New, archived
Headers show
Series mips_malta: Clean up definition of flash memory size | expand

Commit Message

Philippe Mathieu-Daudé March 5, 2019, 4:28 p.m. UTC
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é <philmd@redhat.com>
---
 hw/mips/mips_malta.c | 5 +++++
 1 file changed, 5 insertions(+)

Comments

Richard Henderson March 5, 2019, 11:53 p.m. UTC | #1
On 3/5/19 8:28 AM, Philippe Mathieu-Daudé wrote:
> +
> +        if (blk_getlength(pflash_blk) != FLASH_SIZE) {
> +                error_report("Malta CoreLV card expects a bios of 4MB");
> +                exit(1);
> +        }

Indentation is off, somehow.  Tabs or extra spaces?


r~
Philippe Mathieu-Daudé March 6, 2019, 8:01 a.m. UTC | #2
On 3/6/19 12:53 AM, Richard Henderson wrote:
> On 3/5/19 8:28 AM, Philippe Mathieu-Daudé wrote:
>> +
>> +        if (blk_getlength(pflash_blk) != FLASH_SIZE) {
>> +                error_report("Malta CoreLV card expects a bios of 4MB");
>> +                exit(1);
>> +        }
> 
> Indentation is off, somehow.  Tabs or extra spaces?

Oops... Probably extra spaces, since checkpatch didn't complain.

I'll let Markus fix this if it takes this series ;)

Thanks!

Phil.
Markus Armbruster March 6, 2019, 1:31 p.m. UTC | #3
Philippe Mathieu-Daudé <philmd@redhat.com> 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é <philmd@redhat.com>
> ---
>  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: <20190227111347.15063-1-alex.bennee@linaro.org>

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.
diff mbox series

Patch

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",