[PATCH] hw/arm/virt: Default enable the virtualization option(VHE) on A64FX

[PATCH] hw/arm/virt: Default enable the virtualization option(VHE) on A64FX

[Shuuichirou Ishii]

At A64FX specification, VHE is enabled by default, so
the virtualization option should be enabled when -cpu a64fx
is specified.

Signed-off-by: Shuuichirou Ishii <ishii.shuuichir@fujitsu.com>
---
 hw/arm/virt.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index b45b52c90e..56acf5cc7e 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -1932,6 +1932,12 @@ static void machvirt_init(MachineState *machine)
     firmware_loaded = virt_firmware_init(vms, sysmem,
                                          secure_sysmem ?: sysmem);
 
+    /* A64FX proceesor is supported the ARMv8.1-VHE */
+    if (!vms->virt &&
+            !strncmp(machine->cpu_type, ARM_CPU_TYPE_NAME("a64fx"), 13)) {
+        vms->virt = true;
+    }
+
     /* If we have an EL3 boot ROM then the assumption is that it will
      * implement PSCI itself, so disable QEMU's internal implementation
      * so it doesn't get in the way. Instead of starting secondary
-- 
2.27.0

Re: [PATCH] hw/arm/virt: Default enable the virtualization option(VHE) on A64FX

[Andrew Jones]

On Fri, Jan 14, 2022 at 04:24:37PM +0900, Shuuichirou Ishii wrote:
> At A64FX specification, VHE is enabled by default, so
> the virtualization option should be enabled when -cpu a64fx
> is specified.
> 
> Signed-off-by: Shuuichirou Ishii <ishii.shuuichir@fujitsu.com>
> ---
>  hw/arm/virt.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index b45b52c90e..56acf5cc7e 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -1932,6 +1932,12 @@ static void machvirt_init(MachineState *machine)
>      firmware_loaded = virt_firmware_init(vms, sysmem,
>                                           secure_sysmem ?: sysmem);
>  
> +    /* A64FX proceesor is supported the ARMv8.1-VHE */
> +    if (!vms->virt &&
> +            !strncmp(machine->cpu_type, ARM_CPU_TYPE_NAME("a64fx"), 13)) {
> +        vms->virt = true;
> +    }
> +
>      /* If we have an EL3 boot ROM then the assumption is that it will
>       * implement PSCI itself, so disable QEMU's internal implementation
>       * so it doesn't get in the way. Instead of starting secondary
> -- 
> 2.27.0
>

a64fx isn't the only CPU type the virt machine type supports which has
EL2 support. In fact, all the CPU types supported by mach-virt do, unless
KVM is enabled. So, while I understand the sentiment of this patch, it
doesn't fit the current model. And, since we don't want to start
accumulating hacks like this one in mach-virt, then the options are either
a) push the problem up the stack to whatever constructs the QEMU command
line or b) consider/propose a different machine type.

Thanks,
drew

RE: [PATCH] hw/arm/virt: Default enable the virtualization option(VHE) on A64FX

[ishii.shuuichir]

> a64fx isn't the only CPU type the virt machine type supports which has
> EL2 support. In fact, all the CPU types supported by mach-virt do, unless
> KVM is enabled. So, while I understand the sentiment of this patch, it
> doesn't fit the current model. And, since we don't want to start
> accumulating hacks like this one in mach-virt, then the options are either
> a) push the problem up the stack to whatever constructs the QEMU command
> line or b) consider/propose a different machine type.

Hi, Andrew,
Thank you for your comments.
Also, thank you for the suggested revisions.
We would like to review how to fix this again.

Best regards,
Shuuichirou.

> -----Original Message-----
> From: Andrew Jones <drjones@redhat.com>
> Sent: Friday, January 14, 2022 5:46 PM
> To: Ishii, Shuuichirou/石井 周一郎 <ishii.shuuichir@fujitsu.com>
> Cc: peter.maydell@linaro.org; mst@redhat.com; richard.henderson@linaro.org;
> f4bug@amsat.org; qemu-arm@nongnu.org; qemu-devel@nongnu.org
> Subject: Re: [PATCH] hw/arm/virt: Default enable the virtualization option(VHE)
> on A64FX
> 
> On Fri, Jan 14, 2022 at 04:24:37PM +0900, Shuuichirou Ishii wrote:
> > At A64FX specification, VHE is enabled by default, so the
> > virtualization option should be enabled when -cpu a64fx is specified.
> >
> > Signed-off-by: Shuuichirou Ishii <ishii.shuuichir@fujitsu.com>
> > ---
> >  hw/arm/virt.c | 6 ++++++
> >  1 file changed, 6 insertions(+)
> >
> > diff --git a/hw/arm/virt.c b/hw/arm/virt.c index
> > b45b52c90e..56acf5cc7e 100644
> > --- a/hw/arm/virt.c
> > +++ b/hw/arm/virt.c
> > @@ -1932,6 +1932,12 @@ static void machvirt_init(MachineState *machine)
> >      firmware_loaded = virt_firmware_init(vms, sysmem,
> >                                           secure_sysmem ?: sysmem);
> >
> > +    /* A64FX proceesor is supported the ARMv8.1-VHE */
> > +    if (!vms->virt &&
> > +            !strncmp(machine->cpu_type,
> ARM_CPU_TYPE_NAME("a64fx"), 13)) {
> > +        vms->virt = true;
> > +    }
> > +
> >      /* If we have an EL3 boot ROM then the assumption is that it will
> >       * implement PSCI itself, so disable QEMU's internal implementation
> >       * so it doesn't get in the way. Instead of starting secondary
> > --
> > 2.27.0
> >
> 
> a64fx isn't the only CPU type the virt machine type supports which has
> EL2 support. In fact, all the CPU types supported by mach-virt do, unless
> KVM is enabled. So, while I understand the sentiment of this patch, it
> doesn't fit the current model. And, since we don't want to start
> accumulating hacks like this one in mach-virt, then the options are either
> a) push the problem up the stack to whatever constructs the QEMU command
> line or b) consider/propose a different machine type.
> 
> Thanks,
> drew