hw/mips/mips_malta.c | 27 +++++++++++++++++---------- 1 file changed, 17 insertions(+), 10 deletions(-)
Hi Markus, this is a rework of your 'mips_malta: Clean up definition of flash memory size somewhat' patch: https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg07177.html Regards, Phil. Based-on: <20190226193408.23862-9-armbru@redhat.com> Markus Armbruster (1): mips_malta: Clean up definition of flash memory size somewhat Philippe Mathieu-Daudé (4): hw/mips/malta: Fix the DEBUG_BOARD_INIT code hw/mips/malta: Remove fl_sectors variable (used one single time) hw/mips/malta: Restrict 'bios_size' variable scope hw/mips/malta: Only accept 'monitor' pflash of 4MiB hw/mips/mips_malta.c | 27 +++++++++++++++++---------- 1 file changed, 17 insertions(+), 10 deletions(-) -- 2.20.1
On 3/5/19 8:28 AM, Philippe Mathieu-Daudé wrote: > Markus Armbruster (1): > mips_malta: Clean up definition of flash memory size somewhat > > Philippe Mathieu-Daudé (4): > hw/mips/malta: Fix the DEBUG_BOARD_INIT code > hw/mips/malta: Remove fl_sectors variable (used one single time) > hw/mips/malta: Restrict 'bios_size' variable scope > hw/mips/malta: Only accept 'monitor' pflash of 4MiB > > hw/mips/mips_malta.c | 27 +++++++++++++++++---------- > 1 file changed, 17 insertions(+), 10 deletions(-) Reviewed-by: Richard Henderson <richard.henderson@linaro.org> r~
Philippe Mathieu-Daudé <philmd@redhat.com> writes:
> Hi Markus,
>
> this is a rework of your 'mips_malta: Clean up definition of flash memory size somewhat' patch:
> https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg07177.html
Diff between my patch and your series:
diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c
index 9ade9b194c..2827074e9b 100644
--- a/hw/mips/mips_malta.c
+++ b/hw/mips/mips_malta.c
@@ -1269,12 +1269,12 @@ void mips_malta_init(MachineState *machine)
if (dinfo) {
printf("Register parallel flash %d size " TARGET_FMT_lx " at "
"addr %08llx '%s' %x\n",
- fl_idx, FLASH_SIZE, FLASH_ADDRESS,
+ fl_idx, bios_size, FLASH_ADDRESS,
blk_name(dinfo->bdrv), fl_sectors);
}
#endif
fl = pflash_cfi01_register(FLASH_ADDRESS, NULL, "mips_malta.bios",
- FLASH_SIZE,
+ BIOS_SIZE,
dinfo ? blk_by_legacy_dinfo(dinfo) : NULL,
65536, fl_sectors,
4, 0x0000, 0x0000, 0x0000, 0x0000, be);
We have in include/hw/mips/bios.h
#define BIOS_SIZE (4 * MiB)
and locally
#define FLASH_SIZE 0x400000
target_long bios_size = FLASH_SIZE;
Three names for the same value. Therefore, there's no functional
difference, just more cleanup. Good to know.
Markus Armbruster <armbru@redhat.com> writes:
> Philippe Mathieu-Daudé <philmd@redhat.com> writes:
>
>> Hi Markus,
>>
>> this is a rework of your 'mips_malta: Clean up definition of flash memory size somewhat' patch:
>> https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg07177.html
>
> Diff between my patch and your series:
>
> diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c
> index 9ade9b194c..2827074e9b 100644
> --- a/hw/mips/mips_malta.c
> +++ b/hw/mips/mips_malta.c
> @@ -1269,12 +1269,12 @@ void mips_malta_init(MachineState *machine)
> if (dinfo) {
> printf("Register parallel flash %d size " TARGET_FMT_lx " at "
> "addr %08llx '%s' %x\n",
> - fl_idx, FLASH_SIZE, FLASH_ADDRESS,
> + fl_idx, bios_size, FLASH_ADDRESS,
> blk_name(dinfo->bdrv), fl_sectors);
> }
> #endif
> fl = pflash_cfi01_register(FLASH_ADDRESS, NULL, "mips_malta.bios",
> - FLASH_SIZE,
> + BIOS_SIZE,
> dinfo ? blk_by_legacy_dinfo(dinfo) : NULL,
> 65536, fl_sectors,
> 4, 0x0000, 0x0000, 0x0000, 0x0000, be);
>
> We have in include/hw/mips/bios.h
>
> #define BIOS_SIZE (4 * MiB)
>
> and locally
>
> #define FLASH_SIZE 0x400000
>
> target_long bios_size = FLASH_SIZE;
>
> Three names for the same value. Therefore, there's no functional
> difference, just more cleanup. Good to know.
Wrong diff, sorry.
diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c
index 9ade9b194c..a5726edaa7 100644
--- a/hw/mips/mips_malta.c
+++ b/hw/mips/mips_malta.c
@@ -40,6 +40,7 @@
#include "hw/pci/pci.h"
#include "sysemu/sysemu.h"
#include "sysemu/arch_init.h"
+#include "sysemu/block-backend.h"
#include "qemu/log.h"
#include "hw/mips/bios.h"
#include "hw/ide.h"
@@ -1195,7 +1196,6 @@ void mips_malta_init(MachineState *machine)
MemoryRegion *ram_low_preio = g_new(MemoryRegion, 1);
MemoryRegion *ram_low_postio;
MemoryRegion *bios, *bios_copy = g_new(MemoryRegion, 1);
- target_long bios_size = FLASH_SIZE;
const size_t smbus_eeprom_size = 8 * 256;
uint8_t *smbus_eeprom_buf = g_malloc0(smbus_eeprom_size);
int64_t kernel_entry, bootloader_run_addr;
@@ -1205,10 +1205,10 @@ void mips_malta_init(MachineState *machine)
qemu_irq cbus_irq, i8259_irq;
int piix4_devfn;
I2CBus *smbus;
+ BlockBackend *pflash_blk = NULL;
DriveInfo *dinfo;
DriveInfo *hd[MAX_IDE_BUS * MAX_IDE_DEVS];
int fl_idx = 0;
- int fl_sectors = bios_size >> 16;
int be;
DeviceState *dev = qdev_create(NULL, TYPE_MIPS_MALTA);
@@ -1265,18 +1265,24 @@ void mips_malta_init(MachineState *machine)
/* Load firmware in flash / BIOS. */
dinfo = drive_get(IF_PFLASH, 0, fl_idx);
-#ifdef DEBUG_BOARD_INIT
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' %x\n",
- fl_idx, FLASH_SIZE, FLASH_ADDRESS,
- blk_name(dinfo->bdrv), fl_sectors);
- }
+ "addr %08llx '%s'\n",
+ fl_idx, blk_getlength(pflash_blk), FLASH_ADDRESS,
+ blk_name(pflash_blk));
#endif
+ }
fl = pflash_cfi01_register(FLASH_ADDRESS, NULL, "mips_malta.bios",
FLASH_SIZE,
- dinfo ? blk_by_legacy_dinfo(dinfo) : NULL,
- 65536, fl_sectors,
+ pflash_blk,
+ 65536, FLASH_SIZE >> 16,
4, 0x0000, 0x0000, 0x0000, 0x0000, be);
bios = pflash_cfi01_get_memory(fl);
fl_idx++;
@@ -1312,6 +1318,7 @@ void mips_malta_init(MachineState *machine)
bootloader_run_addr, kernel_entry);
}
} else {
+ target_long bios_size = FLASH_SIZE;
/* The flash region isn't executable from a KVM guest */
if (kvm_enabled()) {
error_report("KVM enabled but no -kernel argument was specified. "
On Tuesday, March 5, 2019, Philippe Mathieu-Daudé <philmd@redhat.com> wrote: > Hi Markus, > > this is a rework of your 'mips_malta: Clean up definition of flash memory > size somewhat' patch: > https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg07177.html > > Regards, > > Phil. Philippe, Could you summarize end-user-visible changes resulting from this series? Aleksandar > Based-on: <20190226193408.23862-9-armbru@redhat.com> > > Markus Armbruster (1): > mips_malta: Clean up definition of flash memory size somewhat > > Philippe Mathieu-Daudé (4): > hw/mips/malta: Fix the DEBUG_BOARD_INIT code > hw/mips/malta: Remove fl_sectors variable (used one single time) > hw/mips/malta: Restrict 'bios_size' variable scope > hw/mips/malta: Only accept 'monitor' pflash of 4MiB > > hw/mips/mips_malta.c | 27 +++++++++++++++++---------- > 1 file changed, 17 insertions(+), 10 deletions(-) > > -- > 2.20.1 > > >
Hi Aleksandar, On 3/6/19 7:38 AM, Aleksandar Markovic wrote: > On Tuesday, March 5, 2019, Philippe Mathieu-Daudé <philmd@redhat.com > <mailto:philmd@redhat.com>> wrote: > > Hi Markus, > > this is a rework of your 'mips_malta: Clean up definition of flash > memory size somewhat' patch: > https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg07177.html > <https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg07177.html> > > Regards, > > Phil. > > > Philippe, > > Could you summarize end-user-visible changes resulting from this series? Good maintainer reflex :) There is an end-user visible change, if he provides a file that is not exactly 4MiB he will now get a "Malta CoreLV card expects a bios of 4MB" error message and QEMU will exit to his shell. This change is introduced by patch 4/5. Now I rather expect this series to get integrated in Markus current work, because his subsequent patches change the PFlash API and it is easier he takes this (to avoid merge conflicts). Note: Markus series is expected to include the following patch from Alex Bennée: "hw/block: better reporting on pflash backing file mismatch" https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg07341.html which is a more important user visible change. I think the the changelog can be updated once, by Markus :) Meanwhile, if this series is taken by Markus can I have your Ack-by? Thanks, Phil.
Philippe Mathieu-Daudé <philmd@redhat.com> writes: > Hi Aleksandar, > > On 3/6/19 7:38 AM, Aleksandar Markovic wrote: >> On Tuesday, March 5, 2019, Philippe Mathieu-Daudé <philmd@redhat.com >> <mailto:philmd@redhat.com>> wrote: >> >> Hi Markus, >> >> this is a rework of your 'mips_malta: Clean up definition of flash >> memory size somewhat' patch: >> https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg07177.html >> <https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg07177.html> >> >> Regards, >> >> Phil. >> >> >> Philippe, >> >> Could you summarize end-user-visible changes resulting from this series? > > Good maintainer reflex :) > > There is an end-user visible change, if he provides a file that is not > exactly 4MiB he will now get a "Malta CoreLV card expects a bios of 4MB" > error message and QEMU will exit to his shell. This change is introduced > by patch 4/5. > > Now I rather expect this series to get integrated in Markus current > work, because his subsequent patches change the PFlash API and it is > easier he takes this (to avoid merge conflicts). Definitely. > Note: Markus series is expected to include the following patch from Alex > Bennée: "hw/block: better reporting on pflash backing file mismatch" > https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg07341.html > which is a more important user visible change. I think the the changelog > can be updated once, by Markus :) Yes. > Meanwhile, if this series is taken by Markus can I have your Ack-by? > > Thanks, > > Phil.
© 2016 - 2025 Red Hat, Inc.