We only need TCG (or QTest) to use TrustZone, whether
KVM or HVF are used is not relevant.

Reported-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
---
 hw/arm/virt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 99fde5836c9..b49d8579161 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -2203,7 +2203,7 @@ static void machvirt_init(MachineState *machine)
         exit(1);
     }
 
-    if (vms->secure && (kvm_enabled() || hvf_enabled())) {
+    if (vms->secure && !tcg_enabled() && !qtest_enabled()) {
         error_report("mach-virt: %s does not support providing "
                      "Security extensions (TrustZone) to the guest CPU",
                      current_accel_name());
-- 
2.49.0

On Mon, 23 Jun 2025 at 13:20, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>
> We only need TCG (or QTest) to use TrustZone, whether
> KVM or HVF are used is not relevant.
>
> Reported-by: Alex Bennée <alex.bennee@linaro.org>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  hw/arm/virt.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 99fde5836c9..b49d8579161 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -2203,7 +2203,7 @@ static void machvirt_init(MachineState *machine)
>          exit(1);
>      }
>
> -    if (vms->secure && (kvm_enabled() || hvf_enabled())) {
> +    if (vms->secure && !tcg_enabled() && !qtest_enabled()) {
>          error_report("mach-virt: %s does not support providing "
>                       "Security extensions (TrustZone) to the guest CPU",
>                       current_accel_name());

The change is fine, but the commit message is odd. You
only get to pick one accelerator. The reason for preferring
"fail unless accelerator A or B" over "fail if accelerator
C or D" is that if/when we add a new accelerator type E
we want the default to be "fail". Then the person implementing
the new accelerator can add E to the accept-list if they
implement support for an EL3 guest.

For the not-yet-implemented case of a hybrid hvf+TCG
accelerator, it's not clear what to do: in some cases
where we check the accelerator type you'll want it to
act like TCG, and sometimes like hvf.

I'll take these patches, with an updated commit message.

-- PMM

On 1/7/25 12:05, Peter Maydell wrote:
> On Mon, 23 Jun 2025 at 13:20, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>>
>> We only need TCG (or QTest) to use TrustZone, whether
>> KVM or HVF are used is not relevant.
>>
>> Reported-by: Alex Bennée <alex.bennee@linaro.org>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>> ---
>>   hw/arm/virt.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
>> index 99fde5836c9..b49d8579161 100644
>> --- a/hw/arm/virt.c
>> +++ b/hw/arm/virt.c
>> @@ -2203,7 +2203,7 @@ static void machvirt_init(MachineState *machine)
>>           exit(1);
>>       }
>>
>> -    if (vms->secure && (kvm_enabled() || hvf_enabled())) {
>> +    if (vms->secure && !tcg_enabled() && !qtest_enabled()) {
>>           error_report("mach-virt: %s does not support providing "
>>                        "Security extensions (TrustZone) to the guest CPU",
>>                        current_accel_name());
> 
> The change is fine, but the commit message is odd. You
> only get to pick one accelerator. The reason for preferring
> "fail unless accelerator A or B" over "fail if accelerator
> C or D" is that if/when we add a new accelerator type E
> we want the default to be "fail". Then the person implementing
> the new accelerator can add E to the accept-list if they
> implement support for an EL3 guest.
> 
> For the not-yet-implemented case of a hybrid hvf+TCG
> accelerator, it's not clear what to do: in some cases
> where we check the accelerator type you'll want it to
> act like TCG, and sometimes like hvf.

In that case we want to defer to the accelerator, not block
from the machine init.

BTW hybrid hw/sw accelerators *is* implemented, but not yet ready
to be merged:
https://lore.kernel.org/qemu-devel/20250620172751.94231-1-philmd@linaro.org/


> 
> I'll take these patches, with an updated commit message.

Thank you!