The firmware may be specified with -bios
and the plfash0 device with option -drive if=pflash.
If both options are applied, it is intented that the pflash0 will
store the secure variable and the firmware be the one specified
by -bios. Explicitly specify "-bios none" if choose to boot from
pflash0

Signed-off-by: Yong Li <yong.li@intel.com>
Cc: "Zhiwei Liu" <zhiwei_liu@linux.alibaba.com>
---
 hw/riscv/virt.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index 4e3efbee16..b38b41e685 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -1296,10 +1296,12 @@ static void virt_machine_done(Notifier *notifier, void *data)
         kernel_entry = 0;
     }
 
-    if (drive_get(IF_PFLASH, 0, 0)) {
+    if (drive_get(IF_PFLASH, 0, 0) &&
+            !strcmp(machine->firmware, "none")) {
         /*
-         * Pflash was supplied, let's overwrite the address we jump to after
-         * reset to the base of the flash.
+         * If pflash (unit 0) was supplied and at the same time the -bois
+         * is not specified, then let's overwrite the address we jump to
+         * after reset to the base of the flash.
          */
         start_addr = virt_memmap[VIRT_FLASH].base;
     }
-- 
2.25.1

On Sun, Apr 23, 2023 at 11:39 PM Yong Li <yong.li@intel.com> wrote:
>
> The firmware may be specified with -bios
> and the plfash0 device with option -drive if=pflash.
> If both options are applied, it is intented that the pflash0 will
> store the secure variable and the firmware be the one specified
> by -bios. Explicitly specify "-bios none" if choose to boot from
> pflash0

This seems like the right approach.

Can you update the docs/system/riscv/virt.rst docs to include this information?

Alistair

>
> Signed-off-by: Yong Li <yong.li@intel.com>
> Cc: "Zhiwei Liu" <zhiwei_liu@linux.alibaba.com>
> ---
>  hw/riscv/virt.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> index 4e3efbee16..b38b41e685 100644
> --- a/hw/riscv/virt.c
> +++ b/hw/riscv/virt.c
> @@ -1296,10 +1296,12 @@ static void virt_machine_done(Notifier *notifier, void *data)
>          kernel_entry = 0;
>      }
>
> -    if (drive_get(IF_PFLASH, 0, 0)) {
> +    if (drive_get(IF_PFLASH, 0, 0) &&
> +            !strcmp(machine->firmware, "none")) {
>          /*
> -         * Pflash was supplied, let's overwrite the address we jump to after
> -         * reset to the base of the flash.
> +         * If pflash (unit 0) was supplied and at the same time the -bois
> +         * is not specified, then let's overwrite the address we jump to
> +         * after reset to the base of the flash.
>           */
>          start_addr = virt_memmap[VIRT_FLASH].base;
>      }
> --
> 2.25.1
>
>

On 2023/5/17 10:27, Alistair Francis wrote:
> On Sun, Apr 23, 2023 at 11:39 PM Yong Li<yong.li@intel.com>  wrote:
>> The firmware may be specified with -bios
>> and the plfash0 device with option -drive if=pflash.
>> If both options are applied, it is intented that the pflash0 will
>> store the secure variable and the firmware be the one specified
>> by -bios. Explicitly specify "-bios none" if choose to boot from
>> pflash0
> This seems like the right approach.
>
> Can you update the docs/system/riscv/virt.rst docs to include this information?
>
> Alistair

Hi Alistair

Thanks for your review, after syncing with Sunil from ventena team.
Sunil is also working with for thishttps://www.mail-archive.com/qemu-devel@nongnu.org/msg959882.html,
and he will handle this change in his patch. Thanks


>> Signed-off-by: Yong Li<yong.li@intel.com>
>> Cc: "Zhiwei Liu"<zhiwei_liu@linux.alibaba.com>
>> ---
>>   hw/riscv/virt.c | 8 +++++---
>>   1 file changed, 5 insertions(+), 3 deletions(-)
>>
>> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
>> index 4e3efbee16..b38b41e685 100644
>> --- a/hw/riscv/virt.c
>> +++ b/hw/riscv/virt.c
>> @@ -1296,10 +1296,12 @@ static void virt_machine_done(Notifier *notifier, void *data)
>>           kernel_entry = 0;
>>       }
>>
>> -    if (drive_get(IF_PFLASH, 0, 0)) {
>> +    if (drive_get(IF_PFLASH, 0, 0) &&
>> +            !strcmp(machine->firmware, "none")) {
>>           /*
>> -         * Pflash was supplied, let's overwrite the address we jump to after
>> -         * reset to the base of the flash.
>> +         * If pflash (unit 0) was supplied and at the same time the -bois
>> +         * is not specified, then let's overwrite the address we jump to
>> +         * after reset to the base of the flash.
>>            */
>>           start_addr = virt_memmap[VIRT_FLASH].base;
>>       }
>> --
>> 2.25.1
>>
>>