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

[David Gibson <david@xxxxxxxxxxxxxxxxxxxxx>]

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é <philmd@xxxxxxxxxx>
> ---
> 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
> +++ b/hw/ppc/ppc440_bamboo.c
> @@ -298,12 +298,17 @@ static void bamboo_init(MachineState *machine)
>      }
>  }
>  
> +static const char *const valid_accels[] = {
> +    "tcg", "kvm", NULL
> +};
> +
>  static void bamboo_machine_init(MachineClass *mc)
>  {
>      mc->desc = "bamboo";
>      mc->init = bamboo_init;
>      mc->default_cpu_type = POWERPC_CPU_TYPE_NAME("440epb");
>      mc->default_ram_id = "ppc4xx.sdram";
> +    mc->valid_accelerators = valid_accels;
>  }
>  
>  DEFINE_MACHINE("bamboo", bamboo_machine_init)
> diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c
> index 7e72f6e4a9b..90d884b0883 100644
> --- a/hw/ppc/prep.c
> +++ b/hw/ppc/prep.c
> @@ -431,6 +431,10 @@ static void ibm_40p_init(MachineState *machine)
>      }
>  }
>  
> +static const char *const valid_accels[] = {
> +    "tcg", "kvm", NULL
> +};
> +
>  static void ibm_40p_machine_init(MachineClass *mc)
>  {
>      mc->desc = "IBM RS/6000 7020 (40p)",
> @@ -441,6 +445,7 @@ static void ibm_40p_machine_init(MachineClass *mc)
>      mc->default_boot_order = "c";
>      mc->default_cpu_type = POWERPC_CPU_TYPE_NAME("604");
>      mc->default_display = "std";
> +    mc->valid_accelerators = valid_accels;
>  }
>  
>  DEFINE_MACHINE("40p", ibm_40p_machine_init)
> diff --git a/hw/ppc/sam460ex.c b/hw/ppc/sam460ex.c
> index e459b43065b..79adb3352f0 100644
> --- a/hw/ppc/sam460ex.c
> +++ b/hw/ppc/sam460ex.c
> @@ -506,6 +506,10 @@ static void sam460ex_init(MachineState *machine)
>      boot_info->entry = entry;
>  }
>  
> +static const char *const valid_accels[] = {
> +    "tcg", "kvm", NULL
> +};
> +
>  static void sam460ex_machine_init(MachineClass *mc)
>  {
>      mc->desc = "aCube Sam460ex";
> @@ -513,6 +517,7 @@ static void sam460ex_machine_init(MachineClass *mc)
>      mc->default_cpu_type = POWERPC_CPU_TYPE_NAME("460exb");
>      mc->default_ram_size = 512 * MiB;
>      mc->default_ram_id = "ppc4xx.sdram";
> +    mc->valid_accelerators = valid_accels;
>  }
>  
>  DEFINE_MACHINE("sam460ex", sam460ex_machine_init)
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 85fe65f8947..c5f985f0187 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -4397,6 +4397,10 @@ static void spapr_cpu_exec_exit(PPCVirtualHypervisor 
> *vhyp, PowerPCCPU *cpu)
>      }
>  }
>  
> +static const char *const valid_accels[] = {
> +    "tcg", "kvm", NULL
> +};
> +
>  static void spapr_machine_class_init(ObjectClass *oc, void *data)
>  {
>      MachineClass *mc = MACHINE_CLASS(oc);
> @@ -4426,6 +4430,7 @@ static void spapr_machine_class_init(ObjectClass *oc, 
> void *data)
>      mc->default_ram_size = 512 * MiB;
>      mc->default_ram_id = "ppc_spapr.ram";
>      mc->default_display = "std";
> +    mc->valid_accelerators = valid_accels;
>      mc->kvm_type = spapr_kvm_type;
>      machine_class_allow_dynamic_sysbus_dev(mc, TYPE_SPAPR_PCI_HOST_BRIDGE);
>      mc->pci_allow_0_address = true;

-- 
David Gibson                    | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
                                | _way_ _around_!
http://www.ozlabs.org/~dgibson

Attachment: signature.asc
Description: PGP signature