Re: [PATCH v5 5/5] hw/arm/virt: Allow virt extensions with KVM

[Alyssa Ross <hi@alyssa.is>]

[-- Attachment #1: Type: text/plain, Size: 2830 bytes --]

Eric Auger <eric.auger@redhat.com> writes:

> Hi,
>
> On 6/17/25 5:50 PM, Miguel Luis wrote:
>> Hi Eric,
>>
>>> On 17 Jun 2025, at 15:41, Eric Auger <eric.auger@redhat.com> wrote:
>>>
>>>
>>>
>>> On 6/17/25 5:23 PM, Miguel Luis wrote:
>>>> Hi Alyssa,
>>>>
>>>>> On 17 Jun 2025, at 14:17, Alyssa Ross <hi@alyssa.is> wrote:
>>>>>
>>>>> Eric Auger <eric.auger@redhat.com> writes:
>>>>>
>>>>>> From: Haibo Xu <haibo.xu@linaro.org>
>>>>>>
>>>>>> Up to now virt support on guest has been only supported with TCG.
>>>>>> Now it becomes feasible to use it with KVM acceleration.
>>>>>>
>>>>>> Also check only in-kernel GICv3 is used along with KVM EL2.
>>>>>>
>>>>>> Signed-off-by: Haibo Xu <haibo.xu@linaro.org>
>>>>>> Signed-off-by: Miguel Luis <miguel.luis@oracle.com>
>>>>>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>>>>>> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>>>>> Hi!  From what I can tell, this will produce an error on hosts that
>>>>> don't support nested virtualization when QEMU is invoked with -accel
>>>>> kvm:tcg
>>>> I didn’t know '-acell kvm:tcg’ could be used as a fallback mechanism between
>>>> acceleration modes. May I ask whether do you manage the ‘-cpu’ type for ‘-accel
>>>> kvm:tcg’ with cpu ‘max’ ?
>>> Does it exist?
>>> qemu-system-aarch64: -accel kvm:tcg: invalid accelerator kvm:tcg
>> Maybe Alyssa is referring to ‘-M virt,accel=kvm:tcg,virtualization=on,gic-version=3’ ?
>>
>> The above didn’t triggered any error. Anyhow if the above does what Alyssa’s saying 
>> we would just be missing the check for || !tcg_enabled() in this patch, I believe.
>
> After discussion with Paolo, the lack of the CAP should be detected
> earlier in kvm_init/kvm_arch_init to allow the fallback to TCG.
> in target/arm/kvm.c kvm_arch_init() some generic caps are checked but
> none of them are related to machine settings and this code is virt arm
> machine agnostic.
>
> I checked and adding
>
>     if (object_dynamic_cast(OBJECT(ms), TYPE_VIRT_MACHINE)) {
>         VirtMachineState *vms = VIRT_MACHINE(ms);
>
>         if (vms->virt && !kvm_arm_el2_supported()) {
>             error_report("KVM does not support nested virtualization");
>             ret = -EINVAL;
>         }
>     }
>
> at the end of the function would do the job. But as I said previously
> this is not done for other virt arm machine options that are accel
> specific or require special KVM caps (secure, mte for instance) so it
> would be a change in the approach.

Right — if it's not done for other options it doesn't need to be done
here either; I wasn't aware of that.  It would be nice if QEMU worked
differently, but that's certainly not in scope of this series.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 227 bytes --]