On 05/25/20 17:58, Philippe Mathieu-Daudé wrote:
> As of this commit, the biggest CFI01 NOR flash documented is
> the Micron PC28F00BP33EF. Its size is 2 GiB (256 MiB).
I don't understand what "2 GiB (256 MiB)" means; please clarify.
>
> Actually this "2Gb device employs a virtual chip enable feature,
> which combines two 1Gb die with a common chip enable".
>
> Since we do not want to model unrealistic hardware, cap the
> current model to this maximum. At least we have a datasheet
> to refer.
>
> If a bigger flash is provided, the user get this warning:
>
> qemu-system-aarch64: Initialization of device cfi.pflash01 failed: Maximum
> supported CFI flash size is 16 MiB.
>
> Note, the sbsa-ref ARM machine introduced in commit 64580903c2b
> already uses a pair of 256 MiB flash devices.
>
> Signed-off-by: Philippe Mathieu-Daudé <phi...@redhat.com>
> ---
> hw/block/pflash_cfi01.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c
> index 11922c0f96..40f145dde7 100644
> --- a/hw/block/pflash_cfi01.c
> +++ b/hw/block/pflash_cfi01.c
> @@ -37,6 +37,8 @@
> */
>
> #include "qemu/osdep.h"
> +#include "qemu/units.h"
> +#include "qemu/cutils.h"
> #include "hw/block/block.h"
> #include "hw/block/flash.h"
> #include "hw/qdev-properties.h"
> @@ -68,6 +70,8 @@ do {
> \
> #define PFLASH_BE 0
> #define PFLASH_SECURE 1
>
> +#define PFLASH_SIZE_MAX (256 * MiB) /* Micron PC28F00BP33EF */
> +
> struct PFlashCFI01 {
> /*< private >*/
> SysBusDevice parent_obj;
> @@ -717,6 +721,12 @@ static void pflash_cfi01_realize(DeviceState *dev, Error
> **errp)
> }
>
> total_len = pfl->sector_len * pfl->nb_blocs;
> + if (total_len > PFLASH_SIZE_MAX) {
> + char *maxsz = size_to_str(PFLASH_SIZE_MAX);
> + error_setg(errp, "Maximum supported CFI flash size is %s.", maxsz);
> + g_free(maxsz);
> + return;
> + }
>
> /* These are only used to expose the parameters of each device
> * in the cfi_table[].
>
I'm unsure how strong the argument is, "we do not want to model
unrealistic hardware", but I do think the patch does what it says on the
tin (modulo the one part in the commit message that I don't understand).
With the commit message clarified
Reviewed-by: Laszlo Ersek <ler...@redhat.com>
(I expect & hope people will provide better reviews than mine.)
Thanks
Laszlo
