[PATCH 0/3] spapr: Nested TCG is TCG only

[PATCH 0/3] spapr: Nested TCG is TCG only

[Fabiano Rosas]

In a KVM-only scenario that does not support nested KVM, a well
written guest would never try to call nested KVM hypercalls. However,
if a buggy or malicious guest calls the nested KVM API even without
nested KVM support, the L0 would redirect the hypercalls into
QEMU. Previously this would have caused an H_FUNCTION return for every
call, but now that QEMU knows about the nested KVM API, it tries to
service the calls. This is incorrect because the spapr virtual
hypervisor implementation of the nested KVM API depends on the first
level guest to be emulated by TCG.

So add guards against that and move the whole code under CONFIG_TCG.

Fabiano Rosas (3):
  spapr: Ignore nested KVM hypercalls when not running TCG
  spapr: Move hypercall_register_softmmu
  spapr: Move nested KVM hypercalls under a TCG only config.

 hw/ppc/spapr_hcall.c | 76 ++++++++++++++++++++++++++------------------
 1 file changed, 45 insertions(+), 31 deletions(-)

-- 
2.34.1

[PATCH 1/3] spapr: Ignore nested KVM hypercalls when not running TCG

[Fabiano Rosas]

It is possible that nested KVM hypercalls reach QEMU while we're
running KVM. The spapr virtual hypervisor implementation of the nested
KVM API only works when the L1 is running under TCG. So return
H_FUNCTION if we are under KVM.

Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com>
---
 hw/ppc/spapr_hcall.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index f008290787..119baa1d2d 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -1508,7 +1508,7 @@ static target_ulong h_set_ptbl(PowerPCCPU *cpu,
 {
     target_ulong ptcr = args[0];
 
-    if (!spapr_get_cap(spapr, SPAPR_CAP_NESTED_KVM_HV)) {
+    if (!spapr_get_cap(spapr, SPAPR_CAP_NESTED_KVM_HV) || !tcg_enabled()) {
         return H_FUNCTION;
     }
 
@@ -1532,6 +1532,10 @@ static target_ulong h_tlb_invalidate(PowerPCCPU *cpu,
      * across L1<->L2 transitions, so nothing is required here.
      */
 
+    if (!tcg_enabled()) {
+        return H_FUNCTION;
+    }
+
     return H_SUCCESS;
 }
 
@@ -1572,6 +1576,10 @@ static target_ulong h_enter_nested(PowerPCCPU *cpu,
     uint64_t cr;
     int i;
 
+    if (!tcg_enabled()) {
+        return H_FUNCTION;
+    }
+
     if (spapr->nested_ptcr == 0) {
         return H_NOT_AVAILABLE;
     }
-- 
2.34.1

[PATCH 2/3] spapr: Move hypercall_register_softmmu

[Fabiano Rosas]

Need to move this because next patch will add calls to the functions
that are below it.

Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com>
---
 hw/ppc/spapr_hcall.c | 50 ++++++++++++++++++++++----------------------
 1 file changed, 25 insertions(+), 25 deletions(-)

diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index 119baa1d2d..c0bfc4bc9c 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -1473,31 +1473,6 @@ target_ulong spapr_hypercall(PowerPCCPU *cpu, target_ulong opcode,
     return H_FUNCTION;
 }
 
-#ifndef CONFIG_TCG
-static target_ulong h_softmmu(PowerPCCPU *cpu, SpaprMachineState *spapr,
-                            target_ulong opcode, target_ulong *args)
-{
-    g_assert_not_reached();
-}
-
-static void hypercall_register_softmmu(void)
-{
-    /* hcall-pft */
-    spapr_register_hypercall(H_ENTER, h_softmmu);
-    spapr_register_hypercall(H_REMOVE, h_softmmu);
-    spapr_register_hypercall(H_PROTECT, h_softmmu);
-    spapr_register_hypercall(H_READ, h_softmmu);
-
-    /* hcall-bulk */
-    spapr_register_hypercall(H_BULK_REMOVE, h_softmmu);
-}
-#else
-static void hypercall_register_softmmu(void)
-{
-    /* DO NOTHING */
-}
-#endif
-
 /* TCG only */
 #define PRTS_MASK      0x1f
 
@@ -1833,6 +1808,31 @@ out_restore_l1:
     spapr_cpu->nested_host_state = NULL;
 }
 
+#ifndef CONFIG_TCG
+static target_ulong h_softmmu(PowerPCCPU *cpu, SpaprMachineState *spapr,
+                            target_ulong opcode, target_ulong *args)
+{
+    g_assert_not_reached();
+}
+
+static void hypercall_register_softmmu(void)
+{
+    /* hcall-pft */
+    spapr_register_hypercall(H_ENTER, h_softmmu);
+    spapr_register_hypercall(H_REMOVE, h_softmmu);
+    spapr_register_hypercall(H_PROTECT, h_softmmu);
+    spapr_register_hypercall(H_READ, h_softmmu);
+
+    /* hcall-bulk */
+    spapr_register_hypercall(H_BULK_REMOVE, h_softmmu);
+}
+#else
+static void hypercall_register_softmmu(void)
+{
+    /* DO NOTHING */
+}
+#endif
+
 static void hypercall_register_types(void)
 {
     hypercall_register_softmmu();
-- 
2.34.1

[PATCH 3/3] spapr: Move nested KVM hypercalls under a TCG only config.

[Fabiano Rosas]

These are the spapr virtual hypervisor implementation of the nested
KVM API. They only make sense when running with TCG.

Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com>
---
 hw/ppc/spapr_hcall.c | 20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index c0bfc4bc9c..f2c802c155 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -2,6 +2,7 @@
 #include "qemu/cutils.h"
 #include "qapi/error.h"
 #include "sysemu/hw_accel.h"
+#include "sysemu/tcg.h"
 #include "sysemu/runstate.h"
 #include "qemu/log.h"
 #include "qemu/main-loop.h"
@@ -1473,7 +1474,8 @@ target_ulong spapr_hypercall(PowerPCCPU *cpu, target_ulong opcode,
     return H_FUNCTION;
 }
 
-/* TCG only */
+#ifdef CONFIG_TCG
+
 #define PRTS_MASK      0x1f
 
 static target_ulong h_set_ptbl(PowerPCCPU *cpu,
@@ -1807,6 +1809,12 @@ out_restore_l1:
     g_free(spapr_cpu->nested_host_state);
     spapr_cpu->nested_host_state = NULL;
 }
+#else
+void spapr_exit_nested(PowerPCCPU *cpu, int excp)
+{
+    g_assert_not_reached();
+}
+#endif
 
 #ifndef CONFIG_TCG
 static target_ulong h_softmmu(PowerPCCPU *cpu, SpaprMachineState *spapr,
@@ -1829,7 +1837,10 @@ static void hypercall_register_softmmu(void)
 #else
 static void hypercall_register_softmmu(void)
 {
-    /* DO NOTHING */
+    spapr_register_hypercall(KVMPPC_H_SET_PARTITION_TABLE, h_set_ptbl);
+    spapr_register_hypercall(KVMPPC_H_ENTER_NESTED, h_enter_nested);
+    spapr_register_hypercall(KVMPPC_H_TLB_INVALIDATE, h_tlb_invalidate);
+    spapr_register_hypercall(KVMPPC_H_COPY_TOFROM_GUEST, h_copy_tofrom_guest);
 }
 #endif
 
@@ -1888,11 +1899,6 @@ static void hypercall_register_types(void)
     spapr_register_hypercall(KVMPPC_H_CAS, h_client_architecture_support);
 
     spapr_register_hypercall(KVMPPC_H_UPDATE_DT, h_update_dt);
-
-    spapr_register_hypercall(KVMPPC_H_SET_PARTITION_TABLE, h_set_ptbl);
-    spapr_register_hypercall(KVMPPC_H_ENTER_NESTED, h_enter_nested);
-    spapr_register_hypercall(KVMPPC_H_TLB_INVALIDATE, h_tlb_invalidate);
-    spapr_register_hypercall(KVMPPC_H_COPY_TOFROM_GUEST, h_copy_tofrom_guest);
 }
 
 type_init(hypercall_register_types)
-- 
2.34.1

Re: [PATCH 1/3] spapr: Ignore nested KVM hypercalls when not running TCG

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 2381 bytes --]

On Thu, Mar 17, 2022 at 02:20:47PM -0300, Fabiano Rosas wrote:
> It is possible that nested KVM hypercalls reach QEMU while we're
> running KVM. The spapr virtual hypervisor implementation of the nested
> KVM API only works when the L1 is running under TCG. So return
> H_FUNCTION if we are under KVM.
> 
> Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com>
> ---
>  hw/ppc/spapr_hcall.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> index f008290787..119baa1d2d 100644
> --- a/hw/ppc/spapr_hcall.c
> +++ b/hw/ppc/spapr_hcall.c
> @@ -1508,7 +1508,7 @@ static target_ulong h_set_ptbl(PowerPCCPU *cpu,
>  {
>      target_ulong ptcr = args[0];
>  
> -    if (!spapr_get_cap(spapr, SPAPR_CAP_NESTED_KVM_HV)) {
> +    if (!spapr_get_cap(spapr, SPAPR_CAP_NESTED_KVM_HV) || !tcg_enabled()) {

I was about to nack this on the grounds that it changes guest visible
behaviour based on host properties.  Then I realized that's not the
case, because in the KVM + SPAPR_CAP_NESTED_KVM_HV case the hypercall
should be caught by KVM first and never reach here.

So at the very least I think this needs a comment explaining that.

However, I'm still kind of confused how we would get here in the first
place.  If SPAPR_CAP_NESTED_KVM_HV is set, but KVM doesn't support it,
we should fail outright in cap_nested_kvm_hv_apply().  So how *do* we
get here?  Is the kernel not doing what we expect of it?  If so, we
should probably abort, rather than just returning H_FUNCTION.


>          return H_FUNCTION;
>      }
>  
> @@ -1532,6 +1532,10 @@ static target_ulong h_tlb_invalidate(PowerPCCPU *cpu,
>       * across L1<->L2 transitions, so nothing is required here.
>       */
>  
> +    if (!tcg_enabled()) {
> +        return H_FUNCTION;
> +    }
> +
>      return H_SUCCESS;
>  }
>  
> @@ -1572,6 +1576,10 @@ static target_ulong h_enter_nested(PowerPCCPU *cpu,
>      uint64_t cr;
>      int i;
>  
> +    if (!tcg_enabled()) {
> +        return H_FUNCTION;
> +    }
> +
>      if (spapr->nested_ptcr == 0) {
>          return H_NOT_AVAILABLE;
>      }

-- 
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 #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 3/3] spapr: Move nested KVM hypercalls under a TCG only config.

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 2801 bytes --]

On Thu, Mar 17, 2022 at 02:20:49PM -0300, Fabiano Rosas wrote:
> These are the spapr virtual hypervisor implementation of the nested
> KVM API. They only make sense when running with TCG.
> 
> Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com>
> ---
>  hw/ppc/spapr_hcall.c | 20 +++++++++++++-------
>  1 file changed, 13 insertions(+), 7 deletions(-)
> 
> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> index c0bfc4bc9c..f2c802c155 100644
> --- a/hw/ppc/spapr_hcall.c
> +++ b/hw/ppc/spapr_hcall.c
> @@ -2,6 +2,7 @@
>  #include "qemu/cutils.h"
>  #include "qapi/error.h"
>  #include "sysemu/hw_accel.h"
> +#include "sysemu/tcg.h"
>  #include "sysemu/runstate.h"
>  #include "qemu/log.h"
>  #include "qemu/main-loop.h"
> @@ -1473,7 +1474,8 @@ target_ulong spapr_hypercall(PowerPCCPU *cpu, target_ulong opcode,
>      return H_FUNCTION;
>  }
>  
> -/* TCG only */
> +#ifdef CONFIG_TCG
> +
>  #define PRTS_MASK      0x1f
>  
>  static target_ulong h_set_ptbl(PowerPCCPU *cpu,
> @@ -1807,6 +1809,12 @@ out_restore_l1:
>      g_free(spapr_cpu->nested_host_state);
>      spapr_cpu->nested_host_state = NULL;
>  }
> +#else
> +void spapr_exit_nested(PowerPCCPU *cpu, int excp)
> +{
> +    g_assert_not_reached();
> +}
> +#endif
>  
>  #ifndef CONFIG_TCG
>  static target_ulong h_softmmu(PowerPCCPU *cpu, SpaprMachineState *spapr,
> @@ -1829,7 +1837,10 @@ static void hypercall_register_softmmu(void)
>  #else
>  static void hypercall_register_softmmu(void)
>  {
> -    /* DO NOTHING */
> +    spapr_register_hypercall(KVMPPC_H_SET_PARTITION_TABLE, h_set_ptbl);
> +    spapr_register_hypercall(KVMPPC_H_ENTER_NESTED, h_enter_nested);
> +    spapr_register_hypercall(KVMPPC_H_TLB_INVALIDATE, h_tlb_invalidate);
> +    spapr_register_hypercall(KVMPPC_H_COPY_TOFROM_GUEST, h_copy_tofrom_guest);

This doesn't fit.  This is specifically about the MMU hypercalls - if
you want to put other things in there it needs a name change at least.

>  }
>  #endif
>  
> @@ -1888,11 +1899,6 @@ static void hypercall_register_types(void)
>      spapr_register_hypercall(KVMPPC_H_CAS, h_client_architecture_support);
>  
>      spapr_register_hypercall(KVMPPC_H_UPDATE_DT, h_update_dt);
> -
> -    spapr_register_hypercall(KVMPPC_H_SET_PARTITION_TABLE, h_set_ptbl);
> -    spapr_register_hypercall(KVMPPC_H_ENTER_NESTED, h_enter_nested);
> -    spapr_register_hypercall(KVMPPC_H_TLB_INVALIDATE, h_tlb_invalidate);
> -    spapr_register_hypercall(KVMPPC_H_COPY_TOFROM_GUEST, h_copy_tofrom_guest);
>  }
>  
>  type_init(hypercall_register_types)

-- 
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 #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 1/3] spapr: Ignore nested KVM hypercalls when not running TCG

[Fabiano Rosas]

David Gibson <david@gibson.dropbear.id.au> writes:

> On Thu, Mar 17, 2022 at 02:20:47PM -0300, Fabiano Rosas wrote:
>> It is possible that nested KVM hypercalls reach QEMU while we're
>> running KVM. The spapr virtual hypervisor implementation of the nested
>> KVM API only works when the L1 is running under TCG. So return
>> H_FUNCTION if we are under KVM.
>> 
>> Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com>
>> ---
>>  hw/ppc/spapr_hcall.c | 10 +++++++++-
>>  1 file changed, 9 insertions(+), 1 deletion(-)
>> 
>> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
>> index f008290787..119baa1d2d 100644
>> --- a/hw/ppc/spapr_hcall.c
>> +++ b/hw/ppc/spapr_hcall.c
>> @@ -1508,7 +1508,7 @@ static target_ulong h_set_ptbl(PowerPCCPU *cpu,
>>  {
>>      target_ulong ptcr = args[0];
>>  
>> -    if (!spapr_get_cap(spapr, SPAPR_CAP_NESTED_KVM_HV)) {
>> +    if (!spapr_get_cap(spapr, SPAPR_CAP_NESTED_KVM_HV) || !tcg_enabled()) {
>
> I was about to nack this on the grounds that it changes guest visible
> behaviour based on host properties.  Then I realized that's not the
> case, because in the KVM + SPAPR_CAP_NESTED_KVM_HV case the hypercall
> should be caught by KVM first and never reach here.
>
> So at the very least I think this needs a comment explaining that.

Ok.

> However, I'm still kind of confused how we would get here in the first
> place.  If SPAPR_CAP_NESTED_KVM_HV is set, but KVM doesn't support it,
> we should fail outright in cap_nested_kvm_hv_apply().  So how *do* we
> get here?  Is the kernel not doing what we expect of it?  If so, we
> should probably abort, rather than just returning H_FUNCTION.

Indeed, If all parts are functioning this should never happen. I was
hacking in L0 and accidentally let some hcalls through. So I'm just
being overly cautions with this patch. If that will end up causing too
much confusion, we could drop this one.

>>          return H_FUNCTION;
>>      }
>>  
>> @@ -1532,6 +1532,10 @@ static target_ulong h_tlb_invalidate(PowerPCCPU *cpu,
>>       * across L1<->L2 transitions, so nothing is required here.
>>       */
>>  
>> +    if (!tcg_enabled()) {
>> +        return H_FUNCTION;
>> +    }
>> +
>>      return H_SUCCESS;
>>  }
>>  
>> @@ -1572,6 +1576,10 @@ static target_ulong h_enter_nested(PowerPCCPU *cpu,
>>      uint64_t cr;
>>      int i;
>>  
>> +    if (!tcg_enabled()) {
>> +        return H_FUNCTION;
>> +    }
>> +
>>      if (spapr->nested_ptcr == 0) {
>>          return H_NOT_AVAILABLE;
>>      }

Re: [PATCH 3/3] spapr: Move nested KVM hypercalls under a TCG only config.

[Fabiano Rosas]

David Gibson <david@gibson.dropbear.id.au> writes:

> On Thu, Mar 17, 2022 at 02:20:49PM -0300, Fabiano Rosas wrote:
>> These are the spapr virtual hypervisor implementation of the nested
>> KVM API. They only make sense when running with TCG.
>> 
>> Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com>
>> ---
>>  hw/ppc/spapr_hcall.c | 20 +++++++++++++-------
>>  1 file changed, 13 insertions(+), 7 deletions(-)
>> 
>> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
>> index c0bfc4bc9c..f2c802c155 100644
>> --- a/hw/ppc/spapr_hcall.c
>> +++ b/hw/ppc/spapr_hcall.c
>> @@ -2,6 +2,7 @@
>>  #include "qemu/cutils.h"
>>  #include "qapi/error.h"
>>  #include "sysemu/hw_accel.h"
>> +#include "sysemu/tcg.h"
>>  #include "sysemu/runstate.h"
>>  #include "qemu/log.h"
>>  #include "qemu/main-loop.h"
>> @@ -1473,7 +1474,8 @@ target_ulong spapr_hypercall(PowerPCCPU *cpu, target_ulong opcode,
>>      return H_FUNCTION;
>>  }
>>  
>> -/* TCG only */
>> +#ifdef CONFIG_TCG
>> +
>>  #define PRTS_MASK      0x1f
>>  
>>  static target_ulong h_set_ptbl(PowerPCCPU *cpu,
>> @@ -1807,6 +1809,12 @@ out_restore_l1:
>>      g_free(spapr_cpu->nested_host_state);
>>      spapr_cpu->nested_host_state = NULL;
>>  }
>> +#else
>> +void spapr_exit_nested(PowerPCCPU *cpu, int excp)
>> +{
>> +    g_assert_not_reached();
>> +}
>> +#endif
>>  
>>  #ifndef CONFIG_TCG
>>  static target_ulong h_softmmu(PowerPCCPU *cpu, SpaprMachineState *spapr,
>> @@ -1829,7 +1837,10 @@ static void hypercall_register_softmmu(void)
>>  #else
>>  static void hypercall_register_softmmu(void)
>>  {
>> -    /* DO NOTHING */
>> +    spapr_register_hypercall(KVMPPC_H_SET_PARTITION_TABLE, h_set_ptbl);
>> +    spapr_register_hypercall(KVMPPC_H_ENTER_NESTED, h_enter_nested);
>> +    spapr_register_hypercall(KVMPPC_H_TLB_INVALIDATE, h_tlb_invalidate);
>> +    spapr_register_hypercall(KVMPPC_H_COPY_TOFROM_GUEST, h_copy_tofrom_guest);
>
> This doesn't fit.  This is specifically about the MMU hypercalls - if
> you want to put other things in there it needs a name change at least.

Thanks, I really overlooked that. I'll put this somewhere else.

>>  }
>>  #endif
>>  
>> @@ -1888,11 +1899,6 @@ static void hypercall_register_types(void)
>>      spapr_register_hypercall(KVMPPC_H_CAS, h_client_architecture_support);
>>  
>>      spapr_register_hypercall(KVMPPC_H_UPDATE_DT, h_update_dt);
>> -
>> -    spapr_register_hypercall(KVMPPC_H_SET_PARTITION_TABLE, h_set_ptbl);
>> -    spapr_register_hypercall(KVMPPC_H_ENTER_NESTED, h_enter_nested);
>> -    spapr_register_hypercall(KVMPPC_H_TLB_INVALIDATE, h_tlb_invalidate);
>> -    spapr_register_hypercall(KVMPPC_H_COPY_TOFROM_GUEST, h_copy_tofrom_guest);
>>  }
>>  
>>  type_init(hypercall_register_types)

Re: [PATCH 1/3] spapr: Ignore nested KVM hypercalls when not running TCG

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 3125 bytes --]

On Fri, Mar 18, 2022 at 10:41:19AM -0300, Fabiano Rosas wrote:
> David Gibson <david@gibson.dropbear.id.au> writes:
> 
> > On Thu, Mar 17, 2022 at 02:20:47PM -0300, Fabiano Rosas wrote:
> >> It is possible that nested KVM hypercalls reach QEMU while we're
> >> running KVM. The spapr virtual hypervisor implementation of the nested
> >> KVM API only works when the L1 is running under TCG. So return
> >> H_FUNCTION if we are under KVM.
> >> 
> >> Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com>
> >> ---
> >>  hw/ppc/spapr_hcall.c | 10 +++++++++-
> >>  1 file changed, 9 insertions(+), 1 deletion(-)
> >> 
> >> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> >> index f008290787..119baa1d2d 100644
> >> --- a/hw/ppc/spapr_hcall.c
> >> +++ b/hw/ppc/spapr_hcall.c
> >> @@ -1508,7 +1508,7 @@ static target_ulong h_set_ptbl(PowerPCCPU *cpu,
> >>  {
> >>      target_ulong ptcr = args[0];
> >>  
> >> -    if (!spapr_get_cap(spapr, SPAPR_CAP_NESTED_KVM_HV)) {
> >> +    if (!spapr_get_cap(spapr, SPAPR_CAP_NESTED_KVM_HV) || !tcg_enabled()) {
> >
> > I was about to nack this on the grounds that it changes guest visible
> > behaviour based on host properties.  Then I realized that's not the
> > case, because in the KVM + SPAPR_CAP_NESTED_KVM_HV case the hypercall
> > should be caught by KVM first and never reach here.
> >
> > So at the very least I think this needs a comment explaining that.
> 
> Ok.
> 
> > However, I'm still kind of confused how we would get here in the first
> > place.  If SPAPR_CAP_NESTED_KVM_HV is set, but KVM doesn't support it,
> > we should fail outright in cap_nested_kvm_hv_apply().  So how *do* we
> > get here?  Is the kernel not doing what we expect of it?  If so, we
> > should probably abort, rather than just returning H_FUNCTION.
> 
> Indeed, If all parts are functioning this should never happen. I was
> hacking in L0 and accidentally let some hcalls through. So I'm just
> being overly cautions with this patch. If that will end up causing too
> much confusion, we could drop this one.

Ok, having something check that case is reasonable - but as a "can't
happen" it should abort, rather than returning something sensible to
the guest.

> 
> >>          return H_FUNCTION;
> >>      }
> >>  
> >> @@ -1532,6 +1532,10 @@ static target_ulong h_tlb_invalidate(PowerPCCPU *cpu,
> >>       * across L1<->L2 transitions, so nothing is required here.
> >>       */
> >>  
> >> +    if (!tcg_enabled()) {
> >> +        return H_FUNCTION;
> >> +    }
> >> +
> >>      return H_SUCCESS;
> >>  }
> >>  
> >> @@ -1572,6 +1576,10 @@ static target_ulong h_enter_nested(PowerPCCPU *cpu,
> >>      uint64_t cr;
> >>      int i;
> >>  
> >> +    if (!tcg_enabled()) {
> >> +        return H_FUNCTION;
> >> +    }
> >> +
> >>      if (spapr->nested_ptcr == 0) {
> >>          return H_NOT_AVAILABLE;
> >>      }
> 

-- 
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 #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]