[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2] hw/block: report when pflash backing file is
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH v2] hw/block: report when pflash backing file isn't aligned |
Date: |
Fri, 15 Feb 2019 17:59:26 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux) |
Markus Armbruster <address@hidden> writes:
> Laszlo Ersek <address@hidden> writes:
>
>> On 02/15/19 13:28, Alex Bennée wrote:
>>> It looks like there was going to be code to check we had some sort of
>>> alignment so lets replace it with an actual check. This is a bit more
>>> useful than the enigmatic "failed to read the initial flash content"
>>> when we attempt to read the number of bytes the device should have.
>>>
>>> This is a potential confusing stumbling block when you move from using
>>> -bios to using -drive if=pflash,file=blob,format=raw,readonly for
>>> loading your firmware code.
>>>
>>> Signed-off-by: Alex Bennée <address@hidden>
>>>
>>> ---
>>> v2
>>> - use PRIu64 instead of PRId64
>>> - tweaked message output
>>> ---
>>> hw/block/pflash_cfi01.c | 20 ++++++++++++++------
>>> 1 file changed, 14 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c
>>> index bffb4c40e7..7532c8d8e8 100644
>>> --- a/hw/block/pflash_cfi01.c
>>> +++ b/hw/block/pflash_cfi01.c
>>> @@ -722,12 +722,20 @@ static void pflash_cfi01_realize(DeviceState *dev,
>>> Error **errp)
>>> }
>>> device_len = sector_len_per_device * blocks_per_device;
>>>
>>> - /* XXX: to be fixed */
>>> -#if 0
>>> - if (total_len != (8 * 1024 * 1024) && total_len != (16 * 1024 * 1024)
>>> &&
>>> - total_len != (32 * 1024 * 1024) && total_len != (64 * 1024 * 1024))
>>> - return NULL;
>>> -#endif
>
> pflash_cfi02_realize() has the same XXX.
>
> There's a pair of related XXXs in taihu_405ep_init(). Related because
> @total_len is computed like this
>
> total_len = pfl->sector_len * pfl->nb_blocs;
>
> and the two factors come from callers of pflash_cfi01_realize(),
> pflash_cfi02_realize(), or from ve_pflash_cfi01_register(),
> xtfpga_flash_init(). Aside: the latter two are slight variations of
> pflash_cfi01_realize() which I'm going to clean up. Some of these use
> fixed sizes (good, real machines do that, too). Some of them compute
> them from blk_getlength(), with varying levels of care.
Missed one: create_one_flash().
Let's review flash size computation.
Single fixed size per machine type:
collie_init() 0x02000000 (two times)
connex_init() 0x01000000
verdex_init() 0x02000000
mainstone_common_init() 0x02000000
sx1_init() (16 * 1024 * 1024) for sx1-v1
(32 * 1024 * 1024) for sx1
( 8 * 1024 * 1024) for sx1-v1
versatile_init() (64 * 1024 * 1024)
z2_init() 0x00800000
milkymist_init() 32 * MiB
petalogix_ml605_init() (32 * MiB)
petalogix_s3adsp1800_init() (16 * MiB)
mips_malta_init() (4 * MiB)
mips_r4k_init() 0x00400000
virtex_init() (16 * MiB)
digic4_add_k8p3215uqb_rom() (4 * 1024 * 1024)
zynq_init() (64 * 1024 * 1024)
lm32_evr_init() 32 * MiB
lm32_uclinux_init() 32 * MiB
taihu_405ep_init() 32 * MiB (but see below)
r2d_init() 0x02000000
ve_pflash_cfi01_register() (64 * 1024 * 1024)
xtfpga_flash_init() 0x00400000 for lx60*
0x01000000 for lx200*
0x01000000 for ml605*
0x08000000 for kc705*
create_one_flash() 0x08000000 / 2 (two times)
Set of fixed sizes:
musicpal_init() image size, either 8*1024*1024,
16*1024*1024 or 32*1024*1024
Troublemakers:
pc_system_flash_init() sum of image sizes, at most 8MiB
rejects images whose size is not a
multiple of 4KiB
sam460ex_load_uboot() image size rounded up to multiple of
64KiB
ref405ep_init() image size rounded up to multiple of
64KiB
taihu_405ep_init() image size rounded up to multiple of
64KiB
> I'm afraid we need to take all that into account.
We can ignore all but the four troublemakers.
[...]