Re: [PATCH] loongarch: Change the UEFI loading mode to loongarch

[lixianglai]

Hi  maobibo:

> > diff --git a/include/hw/loongarch/virt.h
> >         b/include/hw/loongarch/virt.h
> >         
> >         index 6ef9a92394..d1fba1204e 100644
> >         
> >         --- a/include/hw/loongarch/virt.h
> >         
> >         +++ b/include/hw/loongarch/virt.h
> >         
> >         @@ -20,8 +20,10 @@
> >         
> >           #define VIRT_BIOS_BASE          0x1c000000UL
> >         
> >           #define VIRT_BIOS_SIZE          (4 * MiB)
> >         
> >           #define VIRT_FLASH_SECTOR_SIZE  (128 * KiB)
> >         
> >         -#define VIRT_FLASH_BASE         0x1d000000UL
> >         
> >         -#define VIRT_FLASH_SIZE         (16 * MiB)
> >         
> >         +#define VIRT_FLASH0_BASE        VIRT_BIOS_BASE
> >         
> >         +#define VIRT_FLASH0_SIZE        VIRT_BIOS_SIZE
> >         
> >       
>       
>       Xianglai,
>       
>       
>       If there are two flash, what is size for flash0 16M
>       (VIRT_FLASH1_BASE - VIRT_FLASH0_BASE) or 4M (VIRT_BIOS_SIZE) ?
>       
>       
>       IIRC it should be 16M (VIRT_FLASH1_BASE -  VIRT_FLASH0_BASE).
>       
>       
>       Regards
>       
>       Bibo Mao
>       
>       
>

I referred to Philippe's last review suggestion:

https://lore.kernel.org/qemu-devel/b62401b2-3a12-e89d-6953-b40dd170b4ba@linaro.org/

> @@ -20,6 +21,9 @@
>   #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 + VIRT_BIOS_SIZE)

Do you really want the flash base addr to depend of the ROM size?
It could be safer/simpler to start with a fixed address, leaving
room for a bigger ROM if you think you might have to use one.

Thanks!

Xianglai

> > +#define VIRT_FLASH1_BASE       
> >         0x1d000000UL
> >         
> >         +#define VIRT_FLASH1_SIZE        (16 * MiB)
> >         
> >             #define VIRT_LOWMEM_BASE        0
> >         
> >           #define VIRT_LOWMEM_SIZE        0x10000000
> >         
> >         @@ -49,7 +51,7 @@ struct LoongArchMachineState {
> >         
> >               int          fdt_size;
> >         
> >               DeviceState *platform_bus_dev;
> >         
> >               PCIBus       *pci_bus;
> >         
> >         -    PFlashCFI01  *flash;
> >         
> >         +    PFlashCFI01  *flash[2];
> >         
> >               MemoryRegion system_iocsr;
> >         
> >               MemoryRegion iocsr_mem;
> >         
> >               AddressSpace as_iocsr;
> >         
> >         
> >       
>       
>