Re: [PATCH] hw/riscv: microchip_pfsoc: specify XIP image

[Bin Meng]

+Alistair Francis

On Sat, Dec 19, 2020 at 8:24 AM Vitaly Wool <vitaly.wool@konsulko.com> wrote:
>
> Add command line parameter to microchip_pfsoc machine to be able
> to specify XIP kernel image file. To pass over XIP image file, it
> will be enough to run
>
> $ qemu-system-riscv64 -M microchip-icicle-kit,xipImage=<image> ...
>
> Signed-off-by: Vitaly Wool <vitaly.wool@konsulko.com>
> ---
>  hw/riscv/microchip_pfsoc.c         | 42 +++++++++++++++++++++++++++---
>  include/hw/riscv/microchip_pfsoc.h |  1 +
>  2 files changed, 39 insertions(+), 4 deletions(-)
>
> diff --git a/hw/riscv/microchip_pfsoc.c b/hw/riscv/microchip_pfsoc.c
> index e952b49e8c..04d81d52fe 100644
> --- a/hw/riscv/microchip_pfsoc.c
> +++ b/hw/riscv/microchip_pfsoc.c
> @@ -181,6 +181,7 @@ static void microchip_pfsoc_soc_instance_init(Object *obj)
>  static void microchip_pfsoc_soc_realize(DeviceState *dev, Error **errp)
>  {
>      MachineState *ms = MACHINE(qdev_get_machine());
> +    MicrochipIcicleKitState *mks = MICROCHIP_ICICLE_KIT_MACHINE(ms);
>      MicrochipPFSoCState *s = MICROCHIP_PFSOC(dev);
>      const struct MemmapEntry *memmap = microchip_pfsoc_memmap;
>      MemoryRegion *system_memory = get_system_memory();
> @@ -415,10 +416,19 @@ static void microchip_pfsoc_soc_realize(DeviceState 
> *dev, Error **errp)
>                      memmap[MICROCHIP_PFSOC_IOSCB].base);
>
>      /* QSPI Flash */
> -    memory_region_init_rom(qspi_xip_mem, OBJECT(dev),

I believe we only need to change this to memory_region_init_ram(),
then use the device loader the load the XIP image into this space. The
remove the need to introduce the XIP image property as you did.

> -                           "microchip.pfsoc.qspi_xip",
> -                           memmap[MICROCHIP_PFSOC_QSPI_XIP].size,
> -                           &error_fatal);
> +    if (mks->xip_image) {
> +        memory_region_init_ram_from_file(qspi_xip_mem, OBJECT(dev),
> +                                         "microchip.pfsoc.qspi_xip",
> +                                         
> memmap[MICROCHIP_PFSOC_QSPI_XIP].size,
> +                                         0x10000 /* align */, 0 /* ram_flags 
> */,
> +                                         mks->xip_image, &error_fatal);
> +        qspi_xip_mem->readonly = true;
> +    } else {
> +        memory_region_init_rom(qspi_xip_mem, OBJECT(dev),
> +                               "microchip.pfsoc.qspi_xip",
> +                               memmap[MICROCHIP_PFSOC_QSPI_XIP].size,
> +                               &error_fatal);
> +    }
>      memory_region_add_subregion(system_memory,
>                                  memmap[MICROCHIP_PFSOC_QSPI_XIP].base,
>                                  qspi_xip_mem);
> @@ -517,6 +527,24 @@ static void 
> microchip_icicle_kit_machine_init(MachineState *machine)
>      }
>  }
>

Regards,
Bin