Re: [PATCH] hw/loongarch: Add cfi01 pflash device

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

On 15/11/22 12:56, Xiaojuan Yang wrote:
> Add cfi01 pflash device for LoongArch virt machine

So the subject prefix should be "hw/loongarch/virt:".

> Signed-off-by: Xiaojuan Yang <yangxiaojuan@loongson.cn>
> ---
>   hw/loongarch/Kconfig        |   1 +
>   hw/loongarch/acpi-build.c   |  39 +++++++++++
>   hw/loongarch/virt.c         | 130 +++++++++++++++++++++++++++++++++---
>   include/hw/loongarch/virt.h |   7 ++
>   4 files changed, 168 insertions(+), 9 deletions(-)

>   static bool memhp_type_supported(DeviceState *dev)
> diff --git a/include/hw/loongarch/virt.h b/include/hw/loongarch/virt.h
> index 45c383f5a7..4ec4a7b4fe 100644
> --- a/include/hw/loongarch/virt.h
> +++ b/include/hw/loongarch/virt.h
> @@ -12,6 +12,7 @@
>   #include "hw/boards.h"
>   #include "qemu/queue.h"
>   #include "hw/intc/loongarch_ipi.h"
> +#include "hw/block/flash.h"
>   
>   #define LOONGARCH_MAX_VCPUS     4
>   
> @@ -20,6 +21,11 @@
>   #define VIRT_FWCFG_BASE         0x1e020000UL
>   #define VIRT_BIOS_BASE          0x1c000000UL
>   #define VIRT_BIOS_SIZE          (4 * MiB)
> +#define VIRT_FLASH_SECTOR_SIZE  (128 * KiB)
> +#define VIRT_FLASH0_BASE        VIRT_BIOS_BASE
> +#define VIRT_FLASH0_SIZE        (4 * MiB)
> +#define VIRT_FLASH1_BASE        (VIRT_FLASH0_BASE + VIRT_FLASH0_SIZE)
> +#define VIRT_FLASH1_SIZE        (4 * MiB)
>   
>   #define VIRT_LOWMEM_BASE        0
>   #define VIRT_LOWMEM_SIZE        0x10000000
> @@ -48,6 +54,7 @@ struct LoongArchMachineState {
>       int          fdt_size;
>       DeviceState *platform_bus_dev;
>       PCIBus       *pci_bus;
> +    PFlashCFI01  *flash[2];
>   };

Since you are starting a virtual machine from scratch, you should take
the opportunity to learn from other early mistakes. X86 ended that way
due to 1/ old firmwares back-compability and 2/ QEMU pflash block
protections not being implemented. IIUC if we were starting with a
UEFI firmware today, the layout design (still using QEMU) would be
to map the CODE area in a dumb ROM device, and the VARSTORE area
in a PFlash device. Since Virt machines don't need to use Capsule
update, having the CODE area in ROM drastically simplifies the design
and maintainance.

Regards,

Phil.