[PATCH 6/9] ppc/sam460ex: Report an error when run with KVM

Cédric Le Goater posted 9 patches 2 years, 7 months ago
Maintainers: BALATON Zoltan <balaton@eik.bme.hu>, "Cédric Le Goater" <clg@kaod.org>, "Frédéric Barrat" <fbarrat@linux.ibm.com>, Nicholas Piggin <npiggin@gmail.com>, "Hervé Poussineau" <hpoussin@reactos.org>, Daniel Henrique Barboza <danielhb413@gmail.com>, David Gibson <david@gibson.dropbear.id.au>, Greg Kurz <groug@kaod.org>, Paolo Bonzini <pbonzini@redhat.com>
[PATCH 6/9] ppc/sam460ex: Report an error when run with KVM
Posted by Cédric Le Goater 2 years, 7 months ago
The 'sam460ex' machine never supported KVM. This piece of code was
inherited from another model.

Cc: BALATON Zoltan <balaton@eik.bme.hu>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/ppc/sam460ex.c | 15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/hw/ppc/sam460ex.c b/hw/ppc/sam460ex.c
index cf065aae0eae..24f25e5897b7 100644
--- a/hw/ppc/sam460ex.c
+++ b/hw/ppc/sam460ex.c
@@ -18,7 +18,6 @@
 #include "qapi/error.h"
 #include "hw/boards.h"
 #include "sysemu/kvm.h"
-#include "kvm_ppc.h"
 #include "sysemu/device_tree.h"
 #include "sysemu/block-backend.h"
 #include "hw/loader.h"
@@ -165,14 +164,6 @@ static int sam460ex_load_device_tree(MachineState *machine,
     qemu_fdt_setprop_string(fdt, "/chosen", "bootargs",
                             machine->kernel_cmdline);
 
-    /* Copy data from the host device tree into the guest. Since the guest can
-     * directly access the timebase without host involvement, we must expose
-     * the correct frequencies. */
-    if (kvm_enabled()) {
-        tb_freq = kvmppc_get_tbfreq();
-        clock_freq = kvmppc_get_clockfreq();
-    }
-
     qemu_fdt_setprop_cell(fdt, "/cpus/cpu@0", "clock-frequency",
                               clock_freq);
     qemu_fdt_setprop_cell(fdt, "/cpus/cpu@0", "timebase-frequency",
@@ -283,6 +274,12 @@ static void sam460ex_init(MachineState *machine)
     uint8_t *spd_data;
     int success;
 
+    if (kvm_enabled()) {
+        error_report("machine %s does not support the KVM accelerator",
+                     MACHINE_GET_CLASS(machine)->name);
+        exit(EXIT_FAILURE);
+    }
+
     cpu = POWERPC_CPU(cpu_create(machine->cpu_type));
     env = &cpu->env;
     if (env->mmu_model != POWERPC_MMU_BOOKE) {
-- 
2.41.0


Re: [PATCH 6/9] ppc/sam460ex: Report an error when run with KVM
Posted by BALATON Zoltan 2 years, 7 months ago
On Tue, 20 Jun 2023, Cédric Le Goater wrote:
> The 'sam460ex' machine never supported KVM. This piece of code was
> inherited from another model.

This is the same as for pegasos2, it might work on a BookE host if KVM-PR 
on that host is still supported so please keep this around unless there's 
a known problem that can't be fixed.

Regards,
BALATON Zoltan

> Cc: BALATON Zoltan <balaton@eik.bme.hu>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
> hw/ppc/sam460ex.c | 15 ++++++---------
> 1 file changed, 6 insertions(+), 9 deletions(-)
>
> diff --git a/hw/ppc/sam460ex.c b/hw/ppc/sam460ex.c
> index cf065aae0eae..24f25e5897b7 100644
> --- a/hw/ppc/sam460ex.c
> +++ b/hw/ppc/sam460ex.c
> @@ -18,7 +18,6 @@
> #include "qapi/error.h"
> #include "hw/boards.h"
> #include "sysemu/kvm.h"
> -#include "kvm_ppc.h"
> #include "sysemu/device_tree.h"
> #include "sysemu/block-backend.h"
> #include "hw/loader.h"
> @@ -165,14 +164,6 @@ static int sam460ex_load_device_tree(MachineState *machine,
>     qemu_fdt_setprop_string(fdt, "/chosen", "bootargs",
>                             machine->kernel_cmdline);
>
> -    /* Copy data from the host device tree into the guest. Since the guest can
> -     * directly access the timebase without host involvement, we must expose
> -     * the correct frequencies. */
> -    if (kvm_enabled()) {
> -        tb_freq = kvmppc_get_tbfreq();
> -        clock_freq = kvmppc_get_clockfreq();
> -    }
> -
>     qemu_fdt_setprop_cell(fdt, "/cpus/cpu@0", "clock-frequency",
>                               clock_freq);
>     qemu_fdt_setprop_cell(fdt, "/cpus/cpu@0", "timebase-frequency",
> @@ -283,6 +274,12 @@ static void sam460ex_init(MachineState *machine)
>     uint8_t *spd_data;
>     int success;
>
> +    if (kvm_enabled()) {
> +        error_report("machine %s does not support the KVM accelerator",
> +                     MACHINE_GET_CLASS(machine)->name);
> +        exit(EXIT_FAILURE);
> +    }
> +
>     cpu = POWERPC_CPU(cpu_create(machine->cpu_type));
>     env = &cpu->env;
>     if (env->mmu_model != POWERPC_MMU_BOOKE) {
>
Re: [PATCH 6/9] ppc/sam460ex: Report an error when run with KVM
Posted by Cédric Le Goater 2 years, 7 months ago
On 6/20/23 12:42, BALATON Zoltan wrote:
> On Tue, 20 Jun 2023, Cédric Le Goater wrote:
>> The 'sam460ex' machine never supported KVM. This piece of code was
>> inherited from another model.
> 
> This is the same as for pegasos2, it might work on a BookE host if KVM-PR on that host is still supported so please keep this around unless there's a known problem that can't be fixed.

That sounds like a lot of 'if' for 2 decade old HW and an abandoned HV
implementation. Nevertheless, if KVM is supported one day, we can remove
easily the couple of lines below. I am sure there will be more changes.

Same comment for the pegasos2,

Thanks,

C.

> 
> Regards,
> BALATON Zoltan
> 
>> Cc: BALATON Zoltan <balaton@eik.bme.hu>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> ---
>> hw/ppc/sam460ex.c | 15 ++++++---------
>> 1 file changed, 6 insertions(+), 9 deletions(-)
>>
>> diff --git a/hw/ppc/sam460ex.c b/hw/ppc/sam460ex.c
>> index cf065aae0eae..24f25e5897b7 100644
>> --- a/hw/ppc/sam460ex.c
>> +++ b/hw/ppc/sam460ex.c
>> @@ -18,7 +18,6 @@
>> #include "qapi/error.h"
>> #include "hw/boards.h"
>> #include "sysemu/kvm.h"
>> -#include "kvm_ppc.h"
>> #include "sysemu/device_tree.h"
>> #include "sysemu/block-backend.h"
>> #include "hw/loader.h"
>> @@ -165,14 +164,6 @@ static int sam460ex_load_device_tree(MachineState *machine,
>>     qemu_fdt_setprop_string(fdt, "/chosen", "bootargs",
>>                             machine->kernel_cmdline);
>>
>> -    /* Copy data from the host device tree into the guest. Since the guest can
>> -     * directly access the timebase without host involvement, we must expose
>> -     * the correct frequencies. */
>> -    if (kvm_enabled()) {
>> -        tb_freq = kvmppc_get_tbfreq();
>> -        clock_freq = kvmppc_get_clockfreq();
>> -    }
>> -
>>     qemu_fdt_setprop_cell(fdt, "/cpus/cpu@0", "clock-frequency",
>>                               clock_freq);
>>     qemu_fdt_setprop_cell(fdt, "/cpus/cpu@0", "timebase-frequency",
>> @@ -283,6 +274,12 @@ static void sam460ex_init(MachineState *machine)
>>     uint8_t *spd_data;
>>     int success;
>>
>> +    if (kvm_enabled()) {
>> +        error_report("machine %s does not support the KVM accelerator",
>> +                     MACHINE_GET_CLASS(machine)->name);
>> +        exit(EXIT_FAILURE);
>> +    }
>> +
>>     cpu = POWERPC_CPU(cpu_create(machine->cpu_type));
>>     env = &cpu->env;
>>     if (env->mmu_model != POWERPC_MMU_BOOKE) {
>>


Re: [PATCH 6/9] ppc/sam460ex: Report an error when run with KVM
Posted by BALATON Zoltan 2 years, 7 months ago
On Tue, 20 Jun 2023, Cédric Le Goater wrote:
> On 6/20/23 12:42, BALATON Zoltan wrote:
>> On Tue, 20 Jun 2023, Cédric Le Goater wrote:
>>> The 'sam460ex' machine never supported KVM. This piece of code was
>>> inherited from another model.
>> 
>> This is the same as for pegasos2, it might work on a BookE host if KVM-PR 
>> on that host is still supported so please keep this around unless there's a 
>> known problem that can't be fixed.
>
> That sounds like a lot of 'if' for 2 decade old HW and an abandoned HV
> implementation. Nevertheless, if KVM is supported one day, we can remove
> easily the couple of lines below. I am sure there will be more changes.
>
> Same comment for the pegasos2,

If you disable it then nobody can test it so it can't be supported. I need 
people to be able to test it to support it so please leave this enabled. 
(People using these machines don't normally compile their binaries from 
source and don't want to.)

Regards,
BALATON Zoltan

> Thanks,
>
> C.
>
>> 
>> Regards,
>> BALATON Zoltan
>> 
>>> Cc: BALATON Zoltan <balaton@eik.bme.hu>
>>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>>> ---
>>> hw/ppc/sam460ex.c | 15 ++++++---------
>>> 1 file changed, 6 insertions(+), 9 deletions(-)
>>> 
>>> diff --git a/hw/ppc/sam460ex.c b/hw/ppc/sam460ex.c
>>> index cf065aae0eae..24f25e5897b7 100644
>>> --- a/hw/ppc/sam460ex.c
>>> +++ b/hw/ppc/sam460ex.c
>>> @@ -18,7 +18,6 @@
>>> #include "qapi/error.h"
>>> #include "hw/boards.h"
>>> #include "sysemu/kvm.h"
>>> -#include "kvm_ppc.h"
>>> #include "sysemu/device_tree.h"
>>> #include "sysemu/block-backend.h"
>>> #include "hw/loader.h"
>>> @@ -165,14 +164,6 @@ static int sam460ex_load_device_tree(MachineState 
>>> *machine,
>>>     qemu_fdt_setprop_string(fdt, "/chosen", "bootargs",
>>>                             machine->kernel_cmdline);
>>> 
>>> -    /* Copy data from the host device tree into the guest. Since the 
>>> guest can
>>> -     * directly access the timebase without host involvement, we must 
>>> expose
>>> -     * the correct frequencies. */
>>> -    if (kvm_enabled()) {
>>> -        tb_freq = kvmppc_get_tbfreq();
>>> -        clock_freq = kvmppc_get_clockfreq();
>>> -    }
>>> -
>>>     qemu_fdt_setprop_cell(fdt, "/cpus/cpu@0", "clock-frequency",
>>>                               clock_freq);
>>>     qemu_fdt_setprop_cell(fdt, "/cpus/cpu@0", "timebase-frequency",
>>> @@ -283,6 +274,12 @@ static void sam460ex_init(MachineState *machine)
>>>     uint8_t *spd_data;
>>>     int success;
>>> 
>>> +    if (kvm_enabled()) {
>>> +        error_report("machine %s does not support the KVM accelerator",
>>> +                     MACHINE_GET_CLASS(machine)->name);
>>> +        exit(EXIT_FAILURE);
>>> +    }
>>> +
>>>     cpu = POWERPC_CPU(cpu_create(machine->cpu_type));
>>>     env = &cpu->env;
>>>     if (env->mmu_model != POWERPC_MMU_BOOKE) {
>>> 
>
>
>
Re: [PATCH 6/9] ppc/sam460ex: Report an error when run with KVM
Posted by Richard Henderson 2 years, 7 months ago
On 6/20/23 07:59, Cédric Le Goater wrote:
> The 'sam460ex' machine never supported KVM. This piece of code was
> inherited from another model.
> 
> Cc: BALATON Zoltan<balaton@eik.bme.hu>
> Signed-off-by: Cédric Le Goater<clg@kaod.org>
> ---
>   hw/ppc/sam460ex.c | 15 ++++++---------
>   1 file changed, 6 insertions(+), 9 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~