Re: [RFC PATCH v2 06/11] hw/ppc: Restrict KVM to various PPC machines

2021-02-22 Thread Philippe Mathieu-Daudé
On 2/22/21 6:59 AM, David Gibson wrote:
> On Fri, Feb 19, 2021 at 06:38:42PM +0100, Philippe Mathieu-Daudé wrote:
>> Restrit KVM to the following PPC machines:
>> - 40p
>> - bamboo
>> - g3beige
>> - mac99
>> - mpc8544ds
>> - ppce500
>> - pseries
>> - sam460ex
>> - virtex-ml507
> 
> Hrm.
> 
> The reason this list is kind of surprising is because there are 3
> different "flavours" of KVM on ppc: KVM HV ("pseries" only), KVM PR
> (almost any combination, theoretically, but kind of buggy in
> practice), and the Book E specific KVM (Book-E systems with HV
> extensions only).
> 
> But basically, qemu explicitly managing what accelerators are
> available for each machine seems the wrong way around to me.  The
> approach we've generally taken is that qemu requests the specific
> features it needs of KVM, and KVM tells us whether it can supply those
> or not (which may involve selecting between one of the several
> flavours).
> 
> That way we can extend KVM to cover more situations without needing
> corresponding changes in qemu every time.

OK thanks for the information. I'll wait the other patches
get reviewed (in particular the most important ones, 2 and
10) before respining including this information.

Regards,

Phil.




Re: [RFC PATCH v2 06/11] hw/ppc: Restrict KVM to various PPC machines

2021-02-21 Thread David Gibson
On Fri, Feb 19, 2021 at 06:38:42PM +0100, Philippe Mathieu-Daudé wrote:
> Restrit KVM to the following PPC machines:
> - 40p
> - bamboo
> - g3beige
> - mac99
> - mpc8544ds
> - ppce500
> - pseries
> - sam460ex
> - virtex-ml507

Hrm.

The reason this list is kind of surprising is because there are 3
different "flavours" of KVM on ppc: KVM HV ("pseries" only), KVM PR
(almost any combination, theoretically, but kind of buggy in
practice), and the Book E specific KVM (Book-E systems with HV
extensions only).

But basically, qemu explicitly managing what accelerators are
available for each machine seems the wrong way around to me.  The
approach we've generally taken is that qemu requests the specific
features it needs of KVM, and KVM tells us whether it can supply those
or not (which may involve selecting between one of the several
flavours).

That way we can extend KVM to cover more situations without needing
corresponding changes in qemu every time.


> Signed-off-by: Philippe Mathieu-Daudé 
> ---
> RFC: I'm surprise by this list, but this is the result of
>  auditing calls to kvm_enabled() checks.
> 
>  hw/ppc/e500plat.c  | 5 +
>  hw/ppc/mac_newworld.c  | 6 ++
>  hw/ppc/mac_oldworld.c  | 5 +
>  hw/ppc/mpc8544ds.c | 5 +
>  hw/ppc/ppc440_bamboo.c | 5 +
>  hw/ppc/prep.c  | 5 +
>  hw/ppc/sam460ex.c  | 5 +
>  hw/ppc/spapr.c | 5 +
>  8 files changed, 41 insertions(+)
> 
> diff --git a/hw/ppc/e500plat.c b/hw/ppc/e500plat.c
> index bddd5e7c48f..9701dbc2231 100644
> --- a/hw/ppc/e500plat.c
> +++ b/hw/ppc/e500plat.c
> @@ -67,6 +67,10 @@ HotplugHandler 
> *e500plat_machine_get_hotpug_handler(MachineState *machine,
>  
>  #define TYPE_E500PLAT_MACHINE  MACHINE_TYPE_NAME("ppce500")
>  
> +static const char *const valid_accels[] = {
> +"tcg", "kvm", NULL
> +};
> +
>  static void e500plat_machine_class_init(ObjectClass *oc, void *data)
>  {
>  PPCE500MachineClass *pmc = PPCE500_MACHINE_CLASS(oc);
> @@ -98,6 +102,7 @@ static void e500plat_machine_class_init(ObjectClass *oc, 
> void *data)
>  mc->max_cpus = 32;
>  mc->default_cpu_type = POWERPC_CPU_TYPE_NAME("e500v2_v30");
>  mc->default_ram_id = "mpc8544ds.ram";
> +mc->valid_accelerators = valid_accels;
>  machine_class_allow_dynamic_sysbus_dev(mc, TYPE_ETSEC_COMMON);
>   }
>  
> diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c
> index e991db4addb..634f5ad19a0 100644
> --- a/hw/ppc/mac_newworld.c
> +++ b/hw/ppc/mac_newworld.c
> @@ -578,6 +578,11 @@ static char *core99_fw_dev_path(FWPathProvider *p, 
> BusState *bus,
>  
>  return NULL;
>  }
> +
> +static const char *const valid_accels[] = {
> +"tcg", "kvm", NULL
> +};
> +
>  static int core99_kvm_type(MachineState *machine, const char *arg)
>  {
>  /* Always force PR KVM */
> @@ -595,6 +600,7 @@ static void core99_machine_class_init(ObjectClass *oc, 
> void *data)
>  mc->max_cpus = MAX_CPUS;
>  mc->default_boot_order = "cd";
>  mc->default_display = "std";
> +mc->valid_accelerators = valid_accels;
>  mc->kvm_type = core99_kvm_type;
>  #ifdef TARGET_PPC64
>  mc->default_cpu_type = POWERPC_CPU_TYPE_NAME("970fx_v3.1");
> diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
> index 44ee99be886..2c58f73b589 100644
> --- a/hw/ppc/mac_oldworld.c
> +++ b/hw/ppc/mac_oldworld.c
> @@ -424,6 +424,10 @@ static char *heathrow_fw_dev_path(FWPathProvider *p, 
> BusState *bus,
>  return NULL;
>  }
>  
> +static const char *const valid_accels[] = {
> +"tcg", "kvm", NULL
> +};
> +
>  static int heathrow_kvm_type(MachineState *machine, const char *arg)
>  {
>  /* Always force PR KVM */
> @@ -444,6 +448,7 @@ static void heathrow_class_init(ObjectClass *oc, void 
> *data)
>  #endif
>  /* TOFIX "cad" when Mac floppy is implemented */
>  mc->default_boot_order = "cd";
> +mc->valid_accelerators = valid_accels;
>  mc->kvm_type = heathrow_kvm_type;
>  mc->default_cpu_type = POWERPC_CPU_TYPE_NAME("750_v3.1");
>  mc->default_display = "std";
> diff --git a/hw/ppc/mpc8544ds.c b/hw/ppc/mpc8544ds.c
> index 81177505f02..92b0e926c1b 100644
> --- a/hw/ppc/mpc8544ds.c
> +++ b/hw/ppc/mpc8544ds.c
> @@ -36,6 +36,10 @@ static void mpc8544ds_init(MachineState *machine)
>  ppce500_init(machine);
>  }
>  
> +static const char *const valid_accels[] = {
> +"tcg", "kvm", NULL
> +};
> +
>  static void e500plat_machine_class_init(ObjectClass *oc, void *data)
>  {
>  MachineClass *mc = MACHINE_CLASS(oc);
> @@ -56,6 +60,7 @@ static void e500plat_machine_class_init(ObjectClass *oc, 
> void *data)
>  mc->max_cpus = 15;
>  mc->default_cpu_type = POWERPC_CPU_TYPE_NAME("e500v2_v30");
>  mc->default_ram_id = "mpc8544ds.ram";
> +mc->valid_accelerators = valid_accels;
>  }
>  
>  #define TYPE_MPC8544DS_MACHINE  MACHINE_TYPE_NAME("mpc8544ds")
> diff --git a/hw/ppc/ppc440_bamboo.c b/hw/ppc/ppc440_bamboo.c
> index b156bcb9990..02501f489e4 100644
> --- a/hw/ppc/ppc440_bamboo.c
>