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

[address@hidden]

> 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