[v2] spapr: fallback to raw mode if best compat mode cannot be set during CAS

[Qemu-devel] [PATCH v2] spapr: fallback to raw mode if best compat mode cannot be set during CAS

Posted by Greg Kurz 6 years, 8 months ago

KVM PR doesn't allow to set a compat mode. This causes ppc_set_compat_all()
to fail and we return H_HARDWARE to the guest right away.

This is excessive: even if we favor compat mode since commit 152ef803ceb19,
we should at least fallback to raw mode if the guest supports it.

This patch modifies cas_check_pvr() so that it also reports that the real
PVR was found in the table supplied by the guest. Note that this is only
makes sense if raw mode isn't explicitely disabled (ie, the user didn't
set the machine "max-cpu-compat" property). If this is the case, we can
simply ignore ppc_set_compat_all() failures, and let the guest run in raw
mode.

Signed-off-by: Greg Kurz <groug@kaod.org>
---
v2: - initialize raw_mode_supported to silent patchew
---
 hw/ppc/spapr_hcall.c |   18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index 07b3da8dc4cd..2f4c4f59e110 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -1441,7 +1441,8 @@ static target_ulong h_signal_sys_reset(PowerPCCPU *cpu,
 }
 
 static uint32_t cas_check_pvr(sPAPRMachineState *spapr, PowerPCCPU *cpu,
-                              target_ulong *addr, Error **errp)
+                              target_ulong *addr, bool *raw_mode_supported,
+                              Error **errp)
 {
     bool explicit_match = false; /* Matched the CPU's real PVR */
     uint32_t max_compat = spapr->max_compat_pvr;
@@ -1481,6 +1482,8 @@ static uint32_t cas_check_pvr(sPAPRMachineState *spapr, PowerPCCPU *cpu,
         return 0;
     }
 
+    *raw_mode_supported = explicit_match;
+
     /* Parsing finished */
     trace_spapr_cas_pvr(cpu->compat_pvr, explicit_match, best_compat);
 
@@ -1499,8 +1502,9 @@ static target_ulong h_client_architecture_support(PowerPCCPU *cpu,
     sPAPROptionVector *ov1_guest, *ov5_guest, *ov5_cas_old, *ov5_updates;
     bool guest_radix;
     Error *local_err = NULL;
+    bool raw_mode_supported = false;
 
-    cas_pvr = cas_check_pvr(spapr, cpu, &addr, &local_err);
+    cas_pvr = cas_check_pvr(spapr, cpu, &addr, &raw_mode_supported, &local_err);
     if (local_err) {
         error_report_err(local_err);
         return H_HARDWARE;
@@ -1510,8 +1514,14 @@ static target_ulong h_client_architecture_support(PowerPCCPU *cpu,
     if (cpu->compat_pvr != cas_pvr) {
         ppc_set_compat_all(cas_pvr, &local_err);
         if (local_err) {
-            error_report_err(local_err);
-            return H_HARDWARE;
+            /* We fail to set compat mode (likely because running with KVM PR),
+             * but maybe we can fallback to raw mode if the guest supports it.
+             */
+            if (!raw_mode_supported) {
+                error_report_err(local_err);
+                return H_HARDWARE;
+            }
+            local_err = NULL;
         }
     }

Re: [Qemu-devel] [Qemu-ppc] [PATCH v2] spapr: fallback to raw mode if best compat mode cannot be set during CAS

Posted by Greg Kurz 6 years, 7 months ago

On Thu, 17 Aug 2017 13:23:50 +0200
Greg Kurz <groug@kaod.org> wrote:

> KVM PR doesn't allow to set a compat mode. This causes ppc_set_compat_all()
> to fail and we return H_HARDWARE to the guest right away.
> 
> This is excessive: even if we favor compat mode since commit 152ef803ceb19,
> we should at least fallback to raw mode if the guest supports it.
> 
> This patch modifies cas_check_pvr() so that it also reports that the real
> PVR was found in the table supplied by the guest. Note that this is only
> makes sense if raw mode isn't explicitely disabled (ie, the user didn't
> set the machine "max-cpu-compat" property). If this is the case, we can
> simply ignore ppc_set_compat_all() failures, and let the guest run in raw
> mode.
> 
> Signed-off-by: Greg Kurz <groug@kaod.org>
> ---
> v2: - initialize raw_mode_supported to silent patchew
> ---

Ping ?

>  hw/ppc/spapr_hcall.c |   18 ++++++++++++++----
>  1 file changed, 14 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> index 07b3da8dc4cd..2f4c4f59e110 100644
> --- a/hw/ppc/spapr_hcall.c
> +++ b/hw/ppc/spapr_hcall.c
> @@ -1441,7 +1441,8 @@ static target_ulong h_signal_sys_reset(PowerPCCPU *cpu,
>  }
>  
>  static uint32_t cas_check_pvr(sPAPRMachineState *spapr, PowerPCCPU *cpu,
> -                              target_ulong *addr, Error **errp)
> +                              target_ulong *addr, bool *raw_mode_supported,
> +                              Error **errp)
>  {
>      bool explicit_match = false; /* Matched the CPU's real PVR */
>      uint32_t max_compat = spapr->max_compat_pvr;
> @@ -1481,6 +1482,8 @@ static uint32_t cas_check_pvr(sPAPRMachineState *spapr, PowerPCCPU *cpu,
>          return 0;
>      }
>  
> +    *raw_mode_supported = explicit_match;
> +
>      /* Parsing finished */
>      trace_spapr_cas_pvr(cpu->compat_pvr, explicit_match, best_compat);
>  
> @@ -1499,8 +1502,9 @@ static target_ulong h_client_architecture_support(PowerPCCPU *cpu,
>      sPAPROptionVector *ov1_guest, *ov5_guest, *ov5_cas_old, *ov5_updates;
>      bool guest_radix;
>      Error *local_err = NULL;
> +    bool raw_mode_supported = false;
>  
> -    cas_pvr = cas_check_pvr(spapr, cpu, &addr, &local_err);
> +    cas_pvr = cas_check_pvr(spapr, cpu, &addr, &raw_mode_supported, &local_err);
>      if (local_err) {
>          error_report_err(local_err);
>          return H_HARDWARE;
> @@ -1510,8 +1514,14 @@ static target_ulong h_client_architecture_support(PowerPCCPU *cpu,
>      if (cpu->compat_pvr != cas_pvr) {
>          ppc_set_compat_all(cas_pvr, &local_err);
>          if (local_err) {
> -            error_report_err(local_err);
> -            return H_HARDWARE;
> +            /* We fail to set compat mode (likely because running with KVM PR),
> +             * but maybe we can fallback to raw mode if the guest supports it.
> +             */
> +            if (!raw_mode_supported) {
> +                error_report_err(local_err);
> +                return H_HARDWARE;
> +            }
> +            local_err = NULL;
>          }
>      }
>  
> 
>

Re: [Qemu-devel] [PATCH v2] spapr: fallback to raw mode if best compat mode cannot be set during CAS

Posted by David Gibson 6 years, 7 months ago

On Thu, Aug 17, 2017 at 01:23:50PM +0200, Greg Kurz wrote:
> KVM PR doesn't allow to set a compat mode. This causes ppc_set_compat_all()
> to fail and we return H_HARDWARE to the guest right away.
> 
> This is excessive: even if we favor compat mode since commit 152ef803ceb19,
> we should at least fallback to raw mode if the guest supports it.
> 
> This patch modifies cas_check_pvr() so that it also reports that the real
> PVR was found in the table supplied by the guest. Note that this is only
> makes sense if raw mode isn't explicitely disabled (ie, the user didn't
> set the machine "max-cpu-compat" property). If this is the case, we can
> simply ignore ppc_set_compat_all() failures, and let the guest run in raw
> mode.
> 
> Signed-off-by: Greg Kurz <groug@kaod.org>
> ---
> v2: - initialize raw_mode_supported to silent patchew

I hesitated about this, because making guest visible changes based on
host abilities can get really messy.  However we already have a bunch
of things like that in CAS, and we already migrate the compatibility
state, so I think it should be ok.

Applied to ppc-for-2.11.

> ---
>  hw/ppc/spapr_hcall.c |   18 ++++++++++++++----
>  1 file changed, 14 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> index 07b3da8dc4cd..2f4c4f59e110 100644
> --- a/hw/ppc/spapr_hcall.c
> +++ b/hw/ppc/spapr_hcall.c
> @@ -1441,7 +1441,8 @@ static target_ulong h_signal_sys_reset(PowerPCCPU *cpu,
>  }
>  
>  static uint32_t cas_check_pvr(sPAPRMachineState *spapr, PowerPCCPU *cpu,
> -                              target_ulong *addr, Error **errp)
> +                              target_ulong *addr, bool *raw_mode_supported,
> +                              Error **errp)
>  {
>      bool explicit_match = false; /* Matched the CPU's real PVR */
>      uint32_t max_compat = spapr->max_compat_pvr;
> @@ -1481,6 +1482,8 @@ static uint32_t cas_check_pvr(sPAPRMachineState *spapr, PowerPCCPU *cpu,
>          return 0;
>      }
>  
> +    *raw_mode_supported = explicit_match;
> +
>      /* Parsing finished */
>      trace_spapr_cas_pvr(cpu->compat_pvr, explicit_match, best_compat);
>  
> @@ -1499,8 +1502,9 @@ static target_ulong h_client_architecture_support(PowerPCCPU *cpu,
>      sPAPROptionVector *ov1_guest, *ov5_guest, *ov5_cas_old, *ov5_updates;
>      bool guest_radix;
>      Error *local_err = NULL;
> +    bool raw_mode_supported = false;
>  
> -    cas_pvr = cas_check_pvr(spapr, cpu, &addr, &local_err);
> +    cas_pvr = cas_check_pvr(spapr, cpu, &addr, &raw_mode_supported, &local_err);
>      if (local_err) {
>          error_report_err(local_err);
>          return H_HARDWARE;
> @@ -1510,8 +1514,14 @@ static target_ulong h_client_architecture_support(PowerPCCPU *cpu,
>      if (cpu->compat_pvr != cas_pvr) {
>          ppc_set_compat_all(cas_pvr, &local_err);
>          if (local_err) {
> -            error_report_err(local_err);
> -            return H_HARDWARE;
> +            /* We fail to set compat mode (likely because running with KVM PR),
> +             * but maybe we can fallback to raw mode if the guest supports it.
> +             */
> +            if (!raw_mode_supported) {
> +                error_report_err(local_err);
> +                return H_HARDWARE;
> +            }
> +            local_err = NULL;
>          }
>      }
>  
> 

-- 
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