Re: [PATCH v4 2/3] riscv/virt: Support using pflash via -blockdev option

[Philippe Mathieu-Daudé]

On 25/5/23 18:48, Sunil V L wrote:
> Currently, pflash devices can be configured only via -pflash
> or if=pflash options. This is the legacy way and the
> better way is to use -blockdev as in other architectures.
> libvirt also has moved to -blockdev method.
>
> To support -blockdev option, pflash devices need to be
> created in instance_init itself. So, update the code to
> move the virt_flash_create() to instance_init. Also, use
> standard interfaces to detect whether pflash0 is
> configured or not.
>
> Signed-off-by: Sunil V L <sunilvl@ventanamicro.com>
> Reported-by: Andrea Bolognani <abologna@redhat.com>
> ---
>   hw/riscv/virt.c | 15 +++++++++------
>   1 file changed, 9 insertions(+), 6 deletions(-)


> @@ -1265,21 +1267,22 @@ static void virt_machine_done(Notifier *notifier, void 
> *data)
>   
>       firmware_end_addr = riscv_find_and_load_firmware(machine, firmware_name,
>                                                        start_addr, NULL);
> -
> -    if (drive_get(IF_PFLASH, 0, 0)) {
> +    pflash_blk0 = pflash_cfi01_get_blk(s->flash[0]);
> +    if (pflash_blk0) {
> +        flash_mem = pflash_cfi01_get_memory(s->flash[0]);
>           if (machine->firmware && !strcmp(machine->firmware, "none")) {
>               /*
>                * Pflash was supplied but bios is none, let's overwrite the
>                * address we jump to after reset to the base of the flash.
>                */
> -            start_addr = virt_memmap[VIRT_FLASH].base;
> +            start_addr = flash_mem->addr;

I don't understand this change. Besides you access MemoryRegion::addr
which is an internal API field.

>           } else {
>               /*
>                * Pflash was supplied but bios is not none. In this case,
>                * base of the flash would contain S-mode payload.
>                */
>               riscv_setup_firmware_boot(machine);
> -            kernel_entry = virt_memmap[VIRT_FLASH].base;
> +            kernel_entry = flash_mem->addr;
>           }
>       }