[RFC PATCH v0 1/1] target/ppc: Support for H_RPT_INVALIDATE hcall

[RFC PATCH v0 1/1] target/ppc: Support for H_RPT_INVALIDATE hcall

[Bharata B Rao]

If KVM_CAP_RPT_INVALIDATE KVM capability is enabled, then

- indicate the availability of H_RPT_INVALIDATE hcall to the guest via
  ibm,hypertas-functions property.
- Enable the hcall

Both the above are done only if the new sPAPR machine capability
cap-rpt-invalidate is set.

Note: The KVM implementation of the hcall has been posted for upstream
review here:
https://lore.kernel.org/linuxppc-dev/20210105090557.2150104-1-bharata@linux.ibm.com/T/#t

Update to linux-headers/linux/kvm.h here is temporary, will be
done via header updates once the kernel change is accepted upstream.

Signed-off-by: Bharata B Rao <bharata@linux.ibm.com>
---
 hw/ppc/spapr.c            |  7 ++++++
 hw/ppc/spapr_caps.c       | 49 +++++++++++++++++++++++++++++++++++++++
 include/hw/ppc/spapr.h    |  8 +++++--
 linux-headers/linux/kvm.h |  1 +
 target/ppc/kvm.c          | 12 ++++++++++
 target/ppc/kvm_ppc.h      | 11 +++++++++
 6 files changed, 86 insertions(+), 2 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 489cefcb81..0228083800 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -890,6 +890,11 @@ static void spapr_dt_rtas(SpaprMachineState *spapr, void *fdt)
     add_str(hypertas, "hcall-copy");
     add_str(hypertas, "hcall-debug");
     add_str(hypertas, "hcall-vphn");
+    if (kvm_enabled() &&
+        (spapr_get_cap(spapr, SPAPR_CAP_RPT_INVALIDATE) == SPAPR_CAP_ON)) {
+        add_str(hypertas, "hcall-rpt-invalidate");
+    }
+
     add_str(qemu_hypertas, "hcall-memop1");
 
     if (!kvm_enabled() || kvmppc_spapr_use_multitce()) {
@@ -2021,6 +2026,7 @@ static const VMStateDescription vmstate_spapr = {
         &vmstate_spapr_cap_ccf_assist,
         &vmstate_spapr_cap_fwnmi,
         &vmstate_spapr_fwnmi,
+        &vmstate_spapr_cap_rpt_invalidate,
         NULL
     }
 };
@@ -4478,6 +4484,7 @@ static void spapr_machine_class_init(ObjectClass *oc, void *data)
     smc->default_caps.caps[SPAPR_CAP_LARGE_DECREMENTER] = SPAPR_CAP_ON;
     smc->default_caps.caps[SPAPR_CAP_CCF_ASSIST] = SPAPR_CAP_ON;
     smc->default_caps.caps[SPAPR_CAP_FWNMI] = SPAPR_CAP_ON;
+    smc->default_caps.caps[SPAPR_CAP_RPT_INVALIDATE] = SPAPR_CAP_OFF;
     spapr_caps_add_properties(smc);
     smc->irq = &spapr_irq_dual;
     smc->dr_phb_enabled = true;
diff --git a/hw/ppc/spapr_caps.c b/hw/ppc/spapr_caps.c
index 9341e9782a..ebf81e3b23 100644
--- a/hw/ppc/spapr_caps.c
+++ b/hw/ppc/spapr_caps.c
@@ -524,6 +524,45 @@ static void cap_fwnmi_apply(SpaprMachineState *spapr, uint8_t val,
     }
 }
 
+static void cap_rpt_invalidate_apply(SpaprMachineState *spapr,
+                                     uint8_t val, Error **errp)
+{
+    ERRP_GUARD();
+    PowerPCCPU *cpu = POWERPC_CPU(first_cpu);
+
+    if (!val) {
+        /* capability disabled by default */
+        return;
+    }
+
+    if (tcg_enabled()) {
+        error_setg(errp, "No H_RPT_INVALIDATE support in TCG");
+        error_append_hint(errp, "Try appending -machine cap-rpt-invalidate=off\n");
+    } else if (kvm_enabled()) {
+        if (!ppc_check_compat(cpu, CPU_POWERPC_LOGICAL_3_00, 0,
+                              spapr->max_compat_pvr)) {
+            error_setg(errp, "H_RPT_INVALIDATE only supported on POWER9");
+            error_append_hint(errp,
+                              "Try appending -machine max-cpu-compat=power9\n");
+            return;
+        }
+
+        if (!kvmppc_has_cap_mmu_radix()) {
+            error_setg(errp, "H_RPT_INVALIDATE only supported on Radix");
+            return;
+        }
+
+        if (!kvmppc_has_cap_rpt_invalidate()) {
+            error_setg(errp,
+                       "KVM implementation does not support H_RPT_INVALIDATE");
+            error_append_hint(errp,
+                              "Try appending -machine cap-rpt-invalidate=off\n");
+        } else {
+            kvmppc_enable_h_rpt_invalidate();
+        }
+    }
+}
+
 SpaprCapabilityInfo capability_table[SPAPR_CAP_NUM] = {
     [SPAPR_CAP_HTM] = {
         .name = "htm",
@@ -632,6 +671,15 @@ SpaprCapabilityInfo capability_table[SPAPR_CAP_NUM] = {
         .type = "bool",
         .apply = cap_fwnmi_apply,
     },
+    [SPAPR_CAP_RPT_INVALIDATE] = {
+        .name = "rpt-invalidate",
+        .description = "Allow H_RPT_INVALIDATE",
+        .index = SPAPR_CAP_RPT_INVALIDATE,
+        .get = spapr_cap_get_bool,
+        .set = spapr_cap_set_bool,
+        .type = "bool",
+        .apply = cap_rpt_invalidate_apply,
+    },
 };
 
 static SpaprCapabilities default_caps_with_cpu(SpaprMachineState *spapr,
@@ -772,6 +820,7 @@ SPAPR_CAP_MIG_STATE(nested_kvm_hv, SPAPR_CAP_NESTED_KVM_HV);
 SPAPR_CAP_MIG_STATE(large_decr, SPAPR_CAP_LARGE_DECREMENTER);
 SPAPR_CAP_MIG_STATE(ccf_assist, SPAPR_CAP_CCF_ASSIST);
 SPAPR_CAP_MIG_STATE(fwnmi, SPAPR_CAP_FWNMI);
+SPAPR_CAP_MIG_STATE(rpt_invalidate, SPAPR_CAP_RPT_INVALIDATE);
 
 void spapr_caps_init(SpaprMachineState *spapr)
 {
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index e0f10f252c..0931b5b8e8 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -74,8 +74,10 @@ typedef enum {
 #define SPAPR_CAP_CCF_ASSIST            0x09
 /* Implements PAPR FWNMI option */
 #define SPAPR_CAP_FWNMI                 0x0A
+/* Support H_RPT_INVALIDATE */
+#define SPAPR_CAP_RPT_INVALIDATE        0x0B
 /* Num Caps */
-#define SPAPR_CAP_NUM                   (SPAPR_CAP_FWNMI + 1)
+#define SPAPR_CAP_NUM                   (SPAPR_CAP_RPT_INVALIDATE + 1)
 
 /*
  * Capability Values
@@ -535,8 +537,9 @@ struct SpaprMachineState {
 #define H_SCM_BIND_MEM          0x3EC
 #define H_SCM_UNBIND_MEM        0x3F0
 #define H_SCM_UNBIND_ALL        0x3FC
+#define H_RPT_INVALIDATE        0x448
 
-#define MAX_HCALL_OPCODE        H_SCM_UNBIND_ALL
+#define MAX_HCALL_OPCODE        H_RPT_INVALIDATE
 
 /* The hcalls above are standardized in PAPR and implemented by pHyp
  * as well.
@@ -923,6 +926,7 @@ extern const VMStateDescription vmstate_spapr_cap_nested_kvm_hv;
 extern const VMStateDescription vmstate_spapr_cap_large_decr;
 extern const VMStateDescription vmstate_spapr_cap_ccf_assist;
 extern const VMStateDescription vmstate_spapr_cap_fwnmi;
+extern const VMStateDescription vmstate_spapr_cap_rpt_invalidate;
 
 static inline uint8_t spapr_get_cap(SpaprMachineState *spapr, int cap)
 {
diff --git a/linux-headers/linux/kvm.h b/linux-headers/linux/kvm.h
index 56ce14ad20..2b762157e0 100644
--- a/linux-headers/linux/kvm.h
+++ b/linux-headers/linux/kvm.h
@@ -1053,6 +1053,7 @@ struct kvm_ppc_resize_hpt {
 #define KVM_CAP_X86_USER_SPACE_MSR 188
 #define KVM_CAP_X86_MSR_FILTER 189
 #define KVM_CAP_ENFORCE_PV_FEATURE_CPUID 190
+#define KVM_CAP_PPC_RPT_INVALIDATE 193
 
 #ifdef KVM_CAP_IRQ_ROUTING
 
diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
index daf690a678..1c51951ae2 100644
--- a/target/ppc/kvm.c
+++ b/target/ppc/kvm.c
@@ -89,6 +89,7 @@ static int cap_ppc_count_cache_flush_assist;
 static int cap_ppc_nested_kvm_hv;
 static int cap_large_decr;
 static int cap_fwnmi;
+static int cap_rpt_invalidate;
 
 static uint32_t debug_inst_opcode;
 
@@ -152,6 +153,7 @@ int kvm_arch_init(MachineState *ms, KVMState *s)
         exit(1);
     }
 
+    cap_rpt_invalidate = kvm_vm_check_extension(s, KVM_CAP_PPC_RPT_INVALIDATE);
     kvm_ppc_register_host_cpu_type();
 
     return 0;
@@ -2027,6 +2029,11 @@ void kvmppc_enable_h_page_init(void)
     kvmppc_enable_hcall(kvm_state, H_PAGE_INIT);
 }
 
+void kvmppc_enable_h_rpt_invalidate(void)
+{
+    kvmppc_enable_hcall(kvm_state, H_RPT_INVALIDATE);
+}
+
 void kvmppc_set_papr(PowerPCCPU *cpu)
 {
     CPUState *cs = CPU(cpu);
@@ -2538,6 +2545,11 @@ int kvmppc_enable_cap_large_decr(PowerPCCPU *cpu, int enable)
     return 0;
 }
 
+int kvmppc_has_cap_rpt_invalidate(void)
+{
+    return cap_rpt_invalidate;
+}
+
 PowerPCCPUClass *kvm_ppc_get_host_cpu_class(void)
 {
     uint32_t host_pvr = mfpvr();
diff --git a/target/ppc/kvm_ppc.h b/target/ppc/kvm_ppc.h
index 73ce2bc951..8e27f8421f 100644
--- a/target/ppc/kvm_ppc.h
+++ b/target/ppc/kvm_ppc.h
@@ -24,6 +24,7 @@ void kvmppc_enable_logical_ci_hcalls(void);
 void kvmppc_enable_set_mode_hcall(void);
 void kvmppc_enable_clear_ref_mod_hcalls(void);
 void kvmppc_enable_h_page_init(void);
+void kvmppc_enable_h_rpt_invalidate(void);
 void kvmppc_set_papr(PowerPCCPU *cpu);
 int kvmppc_set_compat(PowerPCCPU *cpu, uint32_t compat_pvr);
 void kvmppc_set_mpic_proxy(PowerPCCPU *cpu, int mpic_proxy);
@@ -72,6 +73,7 @@ bool kvmppc_has_cap_nested_kvm_hv(void);
 int kvmppc_set_cap_nested_kvm_hv(int enable);
 int kvmppc_get_cap_large_decr(void);
 int kvmppc_enable_cap_large_decr(PowerPCCPU *cpu, int enable);
+int kvmppc_has_cap_rpt_invalidate(void);
 int kvmppc_enable_hwrng(void);
 int kvmppc_put_books_sregs(PowerPCCPU *cpu);
 PowerPCCPUClass *kvm_ppc_get_host_cpu_class(void);
@@ -151,6 +153,10 @@ static inline void kvmppc_enable_h_page_init(void)
 {
 }
 
+static inline void kvmppc_enable_h_rpt_invalidate(void)
+{
+}
+
 static inline void kvmppc_set_papr(PowerPCCPU *cpu)
 {
 }
@@ -387,6 +393,11 @@ static inline int kvmppc_enable_cap_large_decr(PowerPCCPU *cpu, int enable)
     return -1;
 }
 
+static inline int kvmppc_has_cap_rpt_invalidate(void)
+{
+    return false;
+}
+
 static inline int kvmppc_enable_hwrng(void)
 {
     return -1;
-- 
2.26.2

Re: [RFC PATCH v0 1/1] target/ppc: Support for H_RPT_INVALIDATE hcall

[Daniel Henrique Barboza]

On 1/6/21 5:59 AM, Bharata B Rao wrote:
> If KVM_CAP_RPT_INVALIDATE KVM capability is enabled, then
> 
> - indicate the availability of H_RPT_INVALIDATE hcall to the guest via
>    ibm,hypertas-functions property.
> - Enable the hcall
> 
> Both the above are done only if the new sPAPR machine capability
> cap-rpt-invalidate is set.
> 
> Note: The KVM implementation of the hcall has been posted for upstream
> review here:
> https://lore.kernel.org/linuxppc-dev/20210105090557.2150104-1-bharata@linux.ibm.com/T/#t
> 
> Update to linux-headers/linux/kvm.h here is temporary, will be
> done via header updates once the kernel change is accepted upstream.
> 
> Signed-off-by: Bharata B Rao <bharata@linux.ibm.com>
> ---

Code LGTM.

Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>


A few questions about the logic:

- does it work only on Power 9 like you mentioned in the error message
down below? If it's supported on Power 10 as well then we would want the
error message to read "H_RPT_INVALIDATE only supported on POWER9 and newer"
to contemplate it.

- Does it make sense to expose "rpt-invalidate" to Libvirt? I see that the
capability is turned off by default, which may indicate that even if kernel
and QEMU support is present the user might want to not enable it. Is there
some sort of drawback/compromise when activating this cap?


Thanks,


DHB



>   hw/ppc/spapr.c            |  7 ++++++
>   hw/ppc/spapr_caps.c       | 49 +++++++++++++++++++++++++++++++++++++++
>   include/hw/ppc/spapr.h    |  8 +++++--
>   linux-headers/linux/kvm.h |  1 +
>   target/ppc/kvm.c          | 12 ++++++++++
>   target/ppc/kvm_ppc.h      | 11 +++++++++
>   6 files changed, 86 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 489cefcb81..0228083800 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -890,6 +890,11 @@ static void spapr_dt_rtas(SpaprMachineState *spapr, void *fdt)
>       add_str(hypertas, "hcall-copy");
>       add_str(hypertas, "hcall-debug");
>       add_str(hypertas, "hcall-vphn");
> +    if (kvm_enabled() &&
> +        (spapr_get_cap(spapr, SPAPR_CAP_RPT_INVALIDATE) == SPAPR_CAP_ON)) {
> +        add_str(hypertas, "hcall-rpt-invalidate");
> +    }
> +
>       add_str(qemu_hypertas, "hcall-memop1");
>   
>       if (!kvm_enabled() || kvmppc_spapr_use_multitce()) {
> @@ -2021,6 +2026,7 @@ static const VMStateDescription vmstate_spapr = {
>           &vmstate_spapr_cap_ccf_assist,
>           &vmstate_spapr_cap_fwnmi,
>           &vmstate_spapr_fwnmi,
> +        &vmstate_spapr_cap_rpt_invalidate,
>           NULL
>       }
>   };
> @@ -4478,6 +4484,7 @@ static void spapr_machine_class_init(ObjectClass *oc, void *data)
>       smc->default_caps.caps[SPAPR_CAP_LARGE_DECREMENTER] = SPAPR_CAP_ON;
>       smc->default_caps.caps[SPAPR_CAP_CCF_ASSIST] = SPAPR_CAP_ON;
>       smc->default_caps.caps[SPAPR_CAP_FWNMI] = SPAPR_CAP_ON;
> +    smc->default_caps.caps[SPAPR_CAP_RPT_INVALIDATE] = SPAPR_CAP_OFF;
>       spapr_caps_add_properties(smc);
>       smc->irq = &spapr_irq_dual;
>       smc->dr_phb_enabled = true;
> diff --git a/hw/ppc/spapr_caps.c b/hw/ppc/spapr_caps.c
> index 9341e9782a..ebf81e3b23 100644
> --- a/hw/ppc/spapr_caps.c
> +++ b/hw/ppc/spapr_caps.c
> @@ -524,6 +524,45 @@ static void cap_fwnmi_apply(SpaprMachineState *spapr, uint8_t val,
>       }
>   }
>   
> +static void cap_rpt_invalidate_apply(SpaprMachineState *spapr,
> +                                     uint8_t val, Error **errp)
> +{
> +    ERRP_GUARD();
> +    PowerPCCPU *cpu = POWERPC_CPU(first_cpu);
> +
> +    if (!val) {
> +        /* capability disabled by default */
> +        return;
> +    }
> +
> +    if (tcg_enabled()) {
> +        error_setg(errp, "No H_RPT_INVALIDATE support in TCG");
> +        error_append_hint(errp, "Try appending -machine cap-rpt-invalidate=off\n");
> +    } else if (kvm_enabled()) {
> +        if (!ppc_check_compat(cpu, CPU_POWERPC_LOGICAL_3_00, 0,
> +                              spapr->max_compat_pvr)) {
> +            error_setg(errp, "H_RPT_INVALIDATE only supported on POWER9");
> +            error_append_hint(errp,
> +                              "Try appending -machine max-cpu-compat=power9\n");
> +            return;
> +        }
> +
> +        if (!kvmppc_has_cap_mmu_radix()) {
> +            error_setg(errp, "H_RPT_INVALIDATE only supported on Radix");
> +            return;
> +        }
> +
> +        if (!kvmppc_has_cap_rpt_invalidate()) {
> +            error_setg(errp,
> +                       "KVM implementation does not support H_RPT_INVALIDATE");
> +            error_append_hint(errp,
> +                              "Try appending -machine cap-rpt-invalidate=off\n");
> +        } else {
> +            kvmppc_enable_h_rpt_invalidate();
> +        }
> +    }
> +}
> +
>   SpaprCapabilityInfo capability_table[SPAPR_CAP_NUM] = {
>       [SPAPR_CAP_HTM] = {
>           .name = "htm",
> @@ -632,6 +671,15 @@ SpaprCapabilityInfo capability_table[SPAPR_CAP_NUM] = {
>           .type = "bool",
>           .apply = cap_fwnmi_apply,
>       },
> +    [SPAPR_CAP_RPT_INVALIDATE] = {
> +        .name = "rpt-invalidate",
> +        .description = "Allow H_RPT_INVALIDATE",
> +        .index = SPAPR_CAP_RPT_INVALIDATE,
> +        .get = spapr_cap_get_bool,
> +        .set = spapr_cap_set_bool,
> +        .type = "bool",
> +        .apply = cap_rpt_invalidate_apply,
> +    },
>   };
>   
>   static SpaprCapabilities default_caps_with_cpu(SpaprMachineState *spapr,
> @@ -772,6 +820,7 @@ SPAPR_CAP_MIG_STATE(nested_kvm_hv, SPAPR_CAP_NESTED_KVM_HV);
>   SPAPR_CAP_MIG_STATE(large_decr, SPAPR_CAP_LARGE_DECREMENTER);
>   SPAPR_CAP_MIG_STATE(ccf_assist, SPAPR_CAP_CCF_ASSIST);
>   SPAPR_CAP_MIG_STATE(fwnmi, SPAPR_CAP_FWNMI);
> +SPAPR_CAP_MIG_STATE(rpt_invalidate, SPAPR_CAP_RPT_INVALIDATE);
>   
>   void spapr_caps_init(SpaprMachineState *spapr)
>   {
> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> index e0f10f252c..0931b5b8e8 100644
> --- a/include/hw/ppc/spapr.h
> +++ b/include/hw/ppc/spapr.h
> @@ -74,8 +74,10 @@ typedef enum {
>   #define SPAPR_CAP_CCF_ASSIST            0x09
>   /* Implements PAPR FWNMI option */
>   #define SPAPR_CAP_FWNMI                 0x0A
> +/* Support H_RPT_INVALIDATE */
> +#define SPAPR_CAP_RPT_INVALIDATE        0x0B
>   /* Num Caps */
> -#define SPAPR_CAP_NUM                   (SPAPR_CAP_FWNMI + 1)
> +#define SPAPR_CAP_NUM                   (SPAPR_CAP_RPT_INVALIDATE + 1)
>   
>   /*
>    * Capability Values
> @@ -535,8 +537,9 @@ struct SpaprMachineState {
>   #define H_SCM_BIND_MEM          0x3EC
>   #define H_SCM_UNBIND_MEM        0x3F0
>   #define H_SCM_UNBIND_ALL        0x3FC
> +#define H_RPT_INVALIDATE        0x448
>   
> -#define MAX_HCALL_OPCODE        H_SCM_UNBIND_ALL
> +#define MAX_HCALL_OPCODE        H_RPT_INVALIDATE
>   
>   /* The hcalls above are standardized in PAPR and implemented by pHyp
>    * as well.
> @@ -923,6 +926,7 @@ extern const VMStateDescription vmstate_spapr_cap_nested_kvm_hv;
>   extern const VMStateDescription vmstate_spapr_cap_large_decr;
>   extern const VMStateDescription vmstate_spapr_cap_ccf_assist;
>   extern const VMStateDescription vmstate_spapr_cap_fwnmi;
> +extern const VMStateDescription vmstate_spapr_cap_rpt_invalidate;
>   
>   static inline uint8_t spapr_get_cap(SpaprMachineState *spapr, int cap)
>   {
> diff --git a/linux-headers/linux/kvm.h b/linux-headers/linux/kvm.h
> index 56ce14ad20..2b762157e0 100644
> --- a/linux-headers/linux/kvm.h
> +++ b/linux-headers/linux/kvm.h
> @@ -1053,6 +1053,7 @@ struct kvm_ppc_resize_hpt {
>   #define KVM_CAP_X86_USER_SPACE_MSR 188
>   #define KVM_CAP_X86_MSR_FILTER 189
>   #define KVM_CAP_ENFORCE_PV_FEATURE_CPUID 190
> +#define KVM_CAP_PPC_RPT_INVALIDATE 193
>   
>   #ifdef KVM_CAP_IRQ_ROUTING
>   
> diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
> index daf690a678..1c51951ae2 100644
> --- a/target/ppc/kvm.c
> +++ b/target/ppc/kvm.c
> @@ -89,6 +89,7 @@ static int cap_ppc_count_cache_flush_assist;
>   static int cap_ppc_nested_kvm_hv;
>   static int cap_large_decr;
>   static int cap_fwnmi;
> +static int cap_rpt_invalidate;
>   
>   static uint32_t debug_inst_opcode;
>   
> @@ -152,6 +153,7 @@ int kvm_arch_init(MachineState *ms, KVMState *s)
>           exit(1);
>       }
>   
> +    cap_rpt_invalidate = kvm_vm_check_extension(s, KVM_CAP_PPC_RPT_INVALIDATE);
>       kvm_ppc_register_host_cpu_type();
>   
>       return 0;
> @@ -2027,6 +2029,11 @@ void kvmppc_enable_h_page_init(void)
>       kvmppc_enable_hcall(kvm_state, H_PAGE_INIT);
>   }
>   
> +void kvmppc_enable_h_rpt_invalidate(void)
> +{
> +    kvmppc_enable_hcall(kvm_state, H_RPT_INVALIDATE);
> +}
> +
>   void kvmppc_set_papr(PowerPCCPU *cpu)
>   {
>       CPUState *cs = CPU(cpu);
> @@ -2538,6 +2545,11 @@ int kvmppc_enable_cap_large_decr(PowerPCCPU *cpu, int enable)
>       return 0;
>   }
>   
> +int kvmppc_has_cap_rpt_invalidate(void)
> +{
> +    return cap_rpt_invalidate;
> +}
> +
>   PowerPCCPUClass *kvm_ppc_get_host_cpu_class(void)
>   {
>       uint32_t host_pvr = mfpvr();
> diff --git a/target/ppc/kvm_ppc.h b/target/ppc/kvm_ppc.h
> index 73ce2bc951..8e27f8421f 100644
> --- a/target/ppc/kvm_ppc.h
> +++ b/target/ppc/kvm_ppc.h
> @@ -24,6 +24,7 @@ void kvmppc_enable_logical_ci_hcalls(void);
>   void kvmppc_enable_set_mode_hcall(void);
>   void kvmppc_enable_clear_ref_mod_hcalls(void);
>   void kvmppc_enable_h_page_init(void);
> +void kvmppc_enable_h_rpt_invalidate(void);
>   void kvmppc_set_papr(PowerPCCPU *cpu);
>   int kvmppc_set_compat(PowerPCCPU *cpu, uint32_t compat_pvr);
>   void kvmppc_set_mpic_proxy(PowerPCCPU *cpu, int mpic_proxy);
> @@ -72,6 +73,7 @@ bool kvmppc_has_cap_nested_kvm_hv(void);
>   int kvmppc_set_cap_nested_kvm_hv(int enable);
>   int kvmppc_get_cap_large_decr(void);
>   int kvmppc_enable_cap_large_decr(PowerPCCPU *cpu, int enable);
> +int kvmppc_has_cap_rpt_invalidate(void);
>   int kvmppc_enable_hwrng(void);
>   int kvmppc_put_books_sregs(PowerPCCPU *cpu);
>   PowerPCCPUClass *kvm_ppc_get_host_cpu_class(void);
> @@ -151,6 +153,10 @@ static inline void kvmppc_enable_h_page_init(void)
>   {
>   }
>   
> +static inline void kvmppc_enable_h_rpt_invalidate(void)
> +{
> +}
> +
>   static inline void kvmppc_set_papr(PowerPCCPU *cpu)
>   {
>   }
> @@ -387,6 +393,11 @@ static inline int kvmppc_enable_cap_large_decr(PowerPCCPU *cpu, int enable)
>       return -1;
>   }
>   
> +static inline int kvmppc_has_cap_rpt_invalidate(void)
> +{
> +    return false;
> +}
> +
>   static inline int kvmppc_enable_hwrng(void)
>   {
>       return -1;
> 

Re: [RFC PATCH v0 1/1] target/ppc: Support for H_RPT_INVALIDATE hcall

[Bharata B Rao]

On Tue, Jan 12, 2021 at 10:16:30AM -0300, Daniel Henrique Barboza wrote:
> 
> 
> On 1/6/21 5:59 AM, Bharata B Rao wrote:
> > If KVM_CAP_RPT_INVALIDATE KVM capability is enabled, then
> > 
> > - indicate the availability of H_RPT_INVALIDATE hcall to the guest via
> >    ibm,hypertas-functions property.
> > - Enable the hcall
> > 
> > Both the above are done only if the new sPAPR machine capability
> > cap-rpt-invalidate is set.
> > 
> > Note: The KVM implementation of the hcall has been posted for upstream
> > review here:
> > https://lore.kernel.org/linuxppc-dev/20210105090557.2150104-1-bharata@linux.ibm.com/T/#t
> > 
> > Update to linux-headers/linux/kvm.h here is temporary, will be
> > done via header updates once the kernel change is accepted upstream.
> > 
> > Signed-off-by: Bharata B Rao <bharata@linux.ibm.com>
> > ---
> 
> Code LGTM.
> 
> Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
> 
> 
> A few questions about the logic:
> 
> - does it work only on Power 9 like you mentioned in the error message
> down below? If it's supported on Power 10 as well then we would want the
> error message to read "H_RPT_INVALIDATE only supported on POWER9 and newer"
> to contemplate it.

Making it conditional to Power 9 was an oversight, will remove in the
next iteration.

> 
> - Does it make sense to expose "rpt-invalidate" to Libvirt? I see that the
> capability is turned off by default, which may indicate that even if kernel
> and QEMU support is present the user might want to not enable it. Is there
> some sort of drawback/compromise when activating this cap?

I have added this to take care of migration compatibility between
source and target when hcall is present in target and not present
in source or vice versa. I wonder if there is any other preferred
method than introducing a new machine capability like cap-rpt-invalidate.

Regards,
Bharata.

Re: [RFC PATCH v0 1/1] target/ppc: Support for H_RPT_INVALIDATE hcall

[Greg Kurz]

Hi Bharata,

On Wed,  6 Jan 2021 14:29:10 +0530
Bharata B Rao <bharata@linux.ibm.com> wrote:

> If KVM_CAP_RPT_INVALIDATE KVM capability is enabled, then
> 
> - indicate the availability of H_RPT_INVALIDATE hcall to the guest via
>   ibm,hypertas-functions property.
> - Enable the hcall
> 
> Both the above are done only if the new sPAPR machine capability
> cap-rpt-invalidate is set.
> 
> Note: The KVM implementation of the hcall has been posted for upstream
> review here:
> https://lore.kernel.org/linuxppc-dev/20210105090557.2150104-1-bharata@linux.ibm.com/T/#t
> 
> Update to linux-headers/linux/kvm.h here is temporary, will be
> done via header updates once the kernel change is accepted upstream.
> 
> Signed-off-by: Bharata B Rao <bharata@linux.ibm.com>
> ---

Patch looks mostly fine. A few remarks below.

>  hw/ppc/spapr.c            |  7 ++++++
>  hw/ppc/spapr_caps.c       | 49 +++++++++++++++++++++++++++++++++++++++
>  include/hw/ppc/spapr.h    |  8 +++++--
>  linux-headers/linux/kvm.h |  1 +
>  target/ppc/kvm.c          | 12 ++++++++++
>  target/ppc/kvm_ppc.h      | 11 +++++++++
>  6 files changed, 86 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 489cefcb81..0228083800 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -890,6 +890,11 @@ static void spapr_dt_rtas(SpaprMachineState *spapr, void *fdt)
>      add_str(hypertas, "hcall-copy");
>      add_str(hypertas, "hcall-debug");
>      add_str(hypertas, "hcall-vphn");
> +    if (kvm_enabled() &&

You shouldn't check KVM here. The capability is enough to decide if we
should expose "hcall-rpt-invalidate" or not. FWIW we won't even reach
this code when running with anything but KVM.

> +        (spapr_get_cap(spapr, SPAPR_CAP_RPT_INVALIDATE) == SPAPR_CAP_ON)) {
> +        add_str(hypertas, "hcall-rpt-invalidate");
> +    }
> +
>      add_str(qemu_hypertas, "hcall-memop1");
>  
>      if (!kvm_enabled() || kvmppc_spapr_use_multitce()) {
> @@ -2021,6 +2026,7 @@ static const VMStateDescription vmstate_spapr = {
>          &vmstate_spapr_cap_ccf_assist,
>          &vmstate_spapr_cap_fwnmi,
>          &vmstate_spapr_fwnmi,
> +        &vmstate_spapr_cap_rpt_invalidate,
>          NULL
>      }
>  };
> @@ -4478,6 +4484,7 @@ static void spapr_machine_class_init(ObjectClass *oc, void *data)
>      smc->default_caps.caps[SPAPR_CAP_LARGE_DECREMENTER] = SPAPR_CAP_ON;
>      smc->default_caps.caps[SPAPR_CAP_CCF_ASSIST] = SPAPR_CAP_ON;
>      smc->default_caps.caps[SPAPR_CAP_FWNMI] = SPAPR_CAP_ON;
> +    smc->default_caps.caps[SPAPR_CAP_RPT_INVALIDATE] = SPAPR_CAP_OFF;

Any reason for not enabling this for the default machine type and
disabling it for existing machine types only ?

>      spapr_caps_add_properties(smc);
>      smc->irq = &spapr_irq_dual;
>      smc->dr_phb_enabled = true;
> diff --git a/hw/ppc/spapr_caps.c b/hw/ppc/spapr_caps.c
> index 9341e9782a..ebf81e3b23 100644
> --- a/hw/ppc/spapr_caps.c
> +++ b/hw/ppc/spapr_caps.c
> @@ -524,6 +524,45 @@ static void cap_fwnmi_apply(SpaprMachineState *spapr, uint8_t val,
>      }
>  }
>  
> +static void cap_rpt_invalidate_apply(SpaprMachineState *spapr,
> +                                     uint8_t val, Error **errp)
> +{
> +    ERRP_GUARD();
> +    PowerPCCPU *cpu = POWERPC_CPU(first_cpu);
> +
> +    if (!val) {
> +        /* capability disabled by default */
> +        return;
> +    }
> +
> +    if (tcg_enabled()) {
> +        error_setg(errp, "No H_RPT_INVALIDATE support in TCG");
> +        error_append_hint(errp, "Try appending -machine cap-rpt-invalidate=off\n");
> +    } else if (kvm_enabled()) {
> +        if (!ppc_check_compat(cpu, CPU_POWERPC_LOGICAL_3_00, 0,
> +                              spapr->max_compat_pvr)) {
> +            error_setg(errp, "H_RPT_INVALIDATE only supported on POWER9");
> +            error_append_hint(errp,
> +                              "Try appending -machine max-cpu-compat=power9\n");
> +            return;
> +        }
> +
> +        if (!kvmppc_has_cap_mmu_radix()) {
> +            error_setg(errp, "H_RPT_INVALIDATE only supported on Radix");
> +            return;
> +        }
> +
> +        if (!kvmppc_has_cap_rpt_invalidate()) {
> +            error_setg(errp,
> +                       "KVM implementation does not support H_RPT_INVALIDATE");
> +            error_append_hint(errp,
> +                              "Try appending -machine cap-rpt-invalidate=off\n");
> +        } else {
> +            kvmppc_enable_h_rpt_invalidate();
> +        }
> +    }
> +}
> +
>  SpaprCapabilityInfo capability_table[SPAPR_CAP_NUM] = {
>      [SPAPR_CAP_HTM] = {
>          .name = "htm",
> @@ -632,6 +671,15 @@ SpaprCapabilityInfo capability_table[SPAPR_CAP_NUM] = {
>          .type = "bool",
>          .apply = cap_fwnmi_apply,
>      },
> +    [SPAPR_CAP_RPT_INVALIDATE] = {
> +        .name = "rpt-invalidate",
> +        .description = "Allow H_RPT_INVALIDATE",
> +        .index = SPAPR_CAP_RPT_INVALIDATE,
> +        .get = spapr_cap_get_bool,
> +        .set = spapr_cap_set_bool,
> +        .type = "bool",
> +        .apply = cap_rpt_invalidate_apply,
> +    },
>  };
>  
>  static SpaprCapabilities default_caps_with_cpu(SpaprMachineState *spapr,
> @@ -772,6 +820,7 @@ SPAPR_CAP_MIG_STATE(nested_kvm_hv, SPAPR_CAP_NESTED_KVM_HV);
>  SPAPR_CAP_MIG_STATE(large_decr, SPAPR_CAP_LARGE_DECREMENTER);
>  SPAPR_CAP_MIG_STATE(ccf_assist, SPAPR_CAP_CCF_ASSIST);
>  SPAPR_CAP_MIG_STATE(fwnmi, SPAPR_CAP_FWNMI);
> +SPAPR_CAP_MIG_STATE(rpt_invalidate, SPAPR_CAP_RPT_INVALIDATE);
>  
>  void spapr_caps_init(SpaprMachineState *spapr)
>  {
> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> index e0f10f252c..0931b5b8e8 100644
> --- a/include/hw/ppc/spapr.h
> +++ b/include/hw/ppc/spapr.h
> @@ -74,8 +74,10 @@ typedef enum {
>  #define SPAPR_CAP_CCF_ASSIST            0x09
>  /* Implements PAPR FWNMI option */
>  #define SPAPR_CAP_FWNMI                 0x0A
> +/* Support H_RPT_INVALIDATE */
> +#define SPAPR_CAP_RPT_INVALIDATE        0x0B
>  /* Num Caps */
> -#define SPAPR_CAP_NUM                   (SPAPR_CAP_FWNMI + 1)
> +#define SPAPR_CAP_NUM                   (SPAPR_CAP_RPT_INVALIDATE + 1)
>  
>  /*
>   * Capability Values
> @@ -535,8 +537,9 @@ struct SpaprMachineState {
>  #define H_SCM_BIND_MEM          0x3EC
>  #define H_SCM_UNBIND_MEM        0x3F0
>  #define H_SCM_UNBIND_ALL        0x3FC
> +#define H_RPT_INVALIDATE        0x448
>  
> -#define MAX_HCALL_OPCODE        H_SCM_UNBIND_ALL
> +#define MAX_HCALL_OPCODE        H_RPT_INVALIDATE
>  
>  /* The hcalls above are standardized in PAPR and implemented by pHyp
>   * as well.
> @@ -923,6 +926,7 @@ extern const VMStateDescription vmstate_spapr_cap_nested_kvm_hv;
>  extern const VMStateDescription vmstate_spapr_cap_large_decr;
>  extern const VMStateDescription vmstate_spapr_cap_ccf_assist;
>  extern const VMStateDescription vmstate_spapr_cap_fwnmi;
> +extern const VMStateDescription vmstate_spapr_cap_rpt_invalidate;
>  
>  static inline uint8_t spapr_get_cap(SpaprMachineState *spapr, int cap)
>  {
> diff --git a/linux-headers/linux/kvm.h b/linux-headers/linux/kvm.h
> index 56ce14ad20..2b762157e0 100644
> --- a/linux-headers/linux/kvm.h
> +++ b/linux-headers/linux/kvm.h
> @@ -1053,6 +1053,7 @@ struct kvm_ppc_resize_hpt {
>  #define KVM_CAP_X86_USER_SPACE_MSR 188
>  #define KVM_CAP_X86_MSR_FILTER 189
>  #define KVM_CAP_ENFORCE_PV_FEATURE_CPUID 190
> +#define KVM_CAP_PPC_RPT_INVALIDATE 193
>  
>  #ifdef KVM_CAP_IRQ_ROUTING
>  
> diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
> index daf690a678..1c51951ae2 100644
> --- a/target/ppc/kvm.c
> +++ b/target/ppc/kvm.c
> @@ -89,6 +89,7 @@ static int cap_ppc_count_cache_flush_assist;
>  static int cap_ppc_nested_kvm_hv;
>  static int cap_large_decr;
>  static int cap_fwnmi;
> +static int cap_rpt_invalidate;
>  
>  static uint32_t debug_inst_opcode;
>  
> @@ -152,6 +153,7 @@ int kvm_arch_init(MachineState *ms, KVMState *s)
>          exit(1);
>      }
>  
> +    cap_rpt_invalidate = kvm_vm_check_extension(s, KVM_CAP_PPC_RPT_INVALIDATE);
>      kvm_ppc_register_host_cpu_type();
>  
>      return 0;
> @@ -2027,6 +2029,11 @@ void kvmppc_enable_h_page_init(void)
>      kvmppc_enable_hcall(kvm_state, H_PAGE_INIT);
>  }
>  
> +void kvmppc_enable_h_rpt_invalidate(void)
> +{
> +    kvmppc_enable_hcall(kvm_state, H_RPT_INVALIDATE);
> +}
> +
>  void kvmppc_set_papr(PowerPCCPU *cpu)
>  {
>      CPUState *cs = CPU(cpu);
> @@ -2538,6 +2545,11 @@ int kvmppc_enable_cap_large_decr(PowerPCCPU *cpu, int enable)
>      return 0;
>  }
>  
> +int kvmppc_has_cap_rpt_invalidate(void)
> +{
> +    return cap_rpt_invalidate;
> +}
> +
>  PowerPCCPUClass *kvm_ppc_get_host_cpu_class(void)
>  {
>      uint32_t host_pvr = mfpvr();
> diff --git a/target/ppc/kvm_ppc.h b/target/ppc/kvm_ppc.h
> index 73ce2bc951..8e27f8421f 100644
> --- a/target/ppc/kvm_ppc.h
> +++ b/target/ppc/kvm_ppc.h
> @@ -24,6 +24,7 @@ void kvmppc_enable_logical_ci_hcalls(void);
>  void kvmppc_enable_set_mode_hcall(void);
>  void kvmppc_enable_clear_ref_mod_hcalls(void);
>  void kvmppc_enable_h_page_init(void);
> +void kvmppc_enable_h_rpt_invalidate(void);
>  void kvmppc_set_papr(PowerPCCPU *cpu);
>  int kvmppc_set_compat(PowerPCCPU *cpu, uint32_t compat_pvr);
>  void kvmppc_set_mpic_proxy(PowerPCCPU *cpu, int mpic_proxy);
> @@ -72,6 +73,7 @@ bool kvmppc_has_cap_nested_kvm_hv(void);
>  int kvmppc_set_cap_nested_kvm_hv(int enable);
>  int kvmppc_get_cap_large_decr(void);
>  int kvmppc_enable_cap_large_decr(PowerPCCPU *cpu, int enable);
> +int kvmppc_has_cap_rpt_invalidate(void);
>  int kvmppc_enable_hwrng(void);
>  int kvmppc_put_books_sregs(PowerPCCPU *cpu);
>  PowerPCCPUClass *kvm_ppc_get_host_cpu_class(void);
> @@ -151,6 +153,10 @@ static inline void kvmppc_enable_h_page_init(void)
>  {
>  }
>  
> +static inline void kvmppc_enable_h_rpt_invalidate(void)
> +{

g_assert_not_reached() ?

> +}
> +
>  static inline void kvmppc_set_papr(PowerPCCPU *cpu)
>  {
>  }
> @@ -387,6 +393,11 @@ static inline int kvmppc_enable_cap_large_decr(PowerPCCPU *cpu, int enable)
>      return -1;
>  }
>  
> +static inline int kvmppc_has_cap_rpt_invalidate(void)
> +{
> +    return false;
> +}
> +
>  static inline int kvmppc_enable_hwrng(void)
>  {
>      return -1;

Re: [RFC PATCH v0 1/1] target/ppc: Support for H_RPT_INVALIDATE hcall

[Bharata B Rao]

On Wed, Jan 13, 2021 at 05:22:56PM +0100, Greg Kurz wrote:
> Hi Bharata,
> 
> On Wed,  6 Jan 2021 14:29:10 +0530
> Bharata B Rao <bharata@linux.ibm.com> wrote:
> 
> > If KVM_CAP_RPT_INVALIDATE KVM capability is enabled, then
> > 
> > - indicate the availability of H_RPT_INVALIDATE hcall to the guest via
> >   ibm,hypertas-functions property.
> > - Enable the hcall
> > 
> > Both the above are done only if the new sPAPR machine capability
> > cap-rpt-invalidate is set.
> > 
> > Note: The KVM implementation of the hcall has been posted for upstream
> > review here:
> > https://lore.kernel.org/linuxppc-dev/20210105090557.2150104-1-bharata@linux.ibm.com/T/#t
> > 
> > Update to linux-headers/linux/kvm.h here is temporary, will be
> > done via header updates once the kernel change is accepted upstream.
> > 
> > Signed-off-by: Bharata B Rao <bharata@linux.ibm.com>
> > ---
> 
> Patch looks mostly fine. A few remarks below.
> 
> >  hw/ppc/spapr.c            |  7 ++++++
> >  hw/ppc/spapr_caps.c       | 49 +++++++++++++++++++++++++++++++++++++++
> >  include/hw/ppc/spapr.h    |  8 +++++--
> >  linux-headers/linux/kvm.h |  1 +
> >  target/ppc/kvm.c          | 12 ++++++++++
> >  target/ppc/kvm_ppc.h      | 11 +++++++++
> >  6 files changed, 86 insertions(+), 2 deletions(-)
> > 
> > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > index 489cefcb81..0228083800 100644
> > --- a/hw/ppc/spapr.c
> > +++ b/hw/ppc/spapr.c
> > @@ -890,6 +890,11 @@ static void spapr_dt_rtas(SpaprMachineState *spapr, void *fdt)
> >      add_str(hypertas, "hcall-copy");
> >      add_str(hypertas, "hcall-debug");
> >      add_str(hypertas, "hcall-vphn");
> > +    if (kvm_enabled() &&
> 
> You shouldn't check KVM here. The capability is enough to decide if we
> should expose "hcall-rpt-invalidate" or not. FWIW we won't even reach
> this code when running with anything but KVM.

Correct, the capability itself can be only for KVM case.

> 
> > +        (spapr_get_cap(spapr, SPAPR_CAP_RPT_INVALIDATE) == SPAPR_CAP_ON)) {
> > +        add_str(hypertas, "hcall-rpt-invalidate");
> > +    }
> > +
> >      add_str(qemu_hypertas, "hcall-memop1");
> >  
> >      if (!kvm_enabled() || kvmppc_spapr_use_multitce()) {
> > @@ -2021,6 +2026,7 @@ static const VMStateDescription vmstate_spapr = {
> >          &vmstate_spapr_cap_ccf_assist,
> >          &vmstate_spapr_cap_fwnmi,
> >          &vmstate_spapr_fwnmi,
> > +        &vmstate_spapr_cap_rpt_invalidate,
> >          NULL
> >      }
> >  };
> > @@ -4478,6 +4484,7 @@ static void spapr_machine_class_init(ObjectClass *oc, void *data)
> >      smc->default_caps.caps[SPAPR_CAP_LARGE_DECREMENTER] = SPAPR_CAP_ON;
> >      smc->default_caps.caps[SPAPR_CAP_CCF_ASSIST] = SPAPR_CAP_ON;
> >      smc->default_caps.caps[SPAPR_CAP_FWNMI] = SPAPR_CAP_ON;
> > +    smc->default_caps.caps[SPAPR_CAP_RPT_INVALIDATE] = SPAPR_CAP_OFF;
> 
> Any reason for not enabling this for the default machine type and
> disabling it for existing machine types only ?

If this capability is enabled, then

1. First level guest (L1) can off-load the TLB invalidations to the
new hcall if the platform has disabled LPCR[GTSE].

2. Nested guest (L2) will switch to this new hcall rather than using
the old H_TLB_INVALIDATE hcall.

Case 2 is optional and case 1 makes sense only if LPCR[GTSE]=off.
Hence I thought keeping it off by default and expecting the
user to turn it on only if required would be correct.

Please note that turning this capability ON will result in the
new hcall being exposed to the guest. I hope this is the right
usage of spapr-caps?

> > diff --git a/target/ppc/kvm_ppc.h b/target/ppc/kvm_ppc.h
> > index 73ce2bc951..8e27f8421f 100644
> > --- a/target/ppc/kvm_ppc.h
> > +++ b/target/ppc/kvm_ppc.h
> > @@ -24,6 +24,7 @@ void kvmppc_enable_logical_ci_hcalls(void);
> >  void kvmppc_enable_set_mode_hcall(void);
> >  void kvmppc_enable_clear_ref_mod_hcalls(void);
> >  void kvmppc_enable_h_page_init(void);
> > +void kvmppc_enable_h_rpt_invalidate(void);
> >  void kvmppc_set_papr(PowerPCCPU *cpu);
> >  int kvmppc_set_compat(PowerPCCPU *cpu, uint32_t compat_pvr);
> >  void kvmppc_set_mpic_proxy(PowerPCCPU *cpu, int mpic_proxy);
> > @@ -72,6 +73,7 @@ bool kvmppc_has_cap_nested_kvm_hv(void);
> >  int kvmppc_set_cap_nested_kvm_hv(int enable);
> >  int kvmppc_get_cap_large_decr(void);
> >  int kvmppc_enable_cap_large_decr(PowerPCCPU *cpu, int enable);
> > +int kvmppc_has_cap_rpt_invalidate(void);
> >  int kvmppc_enable_hwrng(void);
> >  int kvmppc_put_books_sregs(PowerPCCPU *cpu);
> >  PowerPCCPUClass *kvm_ppc_get_host_cpu_class(void);
> > @@ -151,6 +153,10 @@ static inline void kvmppc_enable_h_page_init(void)
> >  {
> >  }
> >  
> > +static inline void kvmppc_enable_h_rpt_invalidate(void)
> > +{
> 
> g_assert_not_reached() ?

Don't see many others doing that, is that a new preferred
way?

Regards,
Bharata.

Re: [RFC PATCH v0 1/1] target/ppc: Support for H_RPT_INVALIDATE hcall

[Greg Kurz]

On Fri, 15 Jan 2021 14:01:28 +0530
Bharata B Rao <bharata@linux.ibm.com> wrote:

> On Wed, Jan 13, 2021 at 05:22:56PM +0100, Greg Kurz wrote:
> > Hi Bharata,
> > 
> > On Wed,  6 Jan 2021 14:29:10 +0530
> > Bharata B Rao <bharata@linux.ibm.com> wrote:
> > 
> > > If KVM_CAP_RPT_INVALIDATE KVM capability is enabled, then
> > > 
> > > - indicate the availability of H_RPT_INVALIDATE hcall to the guest via
> > >   ibm,hypertas-functions property.
> > > - Enable the hcall
> > > 
> > > Both the above are done only if the new sPAPR machine capability
> > > cap-rpt-invalidate is set.
> > > 
> > > Note: The KVM implementation of the hcall has been posted for upstream
> > > review here:
> > > https://lore.kernel.org/linuxppc-dev/20210105090557.2150104-1-bharata@linux.ibm.com/T/#t
> > > 
> > > Update to linux-headers/linux/kvm.h here is temporary, will be
> > > done via header updates once the kernel change is accepted upstream.
> > > 
> > > Signed-off-by: Bharata B Rao <bharata@linux.ibm.com>
> > > ---
> > 
> > Patch looks mostly fine. A few remarks below.
> > 
> > >  hw/ppc/spapr.c            |  7 ++++++
> > >  hw/ppc/spapr_caps.c       | 49 +++++++++++++++++++++++++++++++++++++++
> > >  include/hw/ppc/spapr.h    |  8 +++++--
> > >  linux-headers/linux/kvm.h |  1 +
> > >  target/ppc/kvm.c          | 12 ++++++++++
> > >  target/ppc/kvm_ppc.h      | 11 +++++++++
> > >  6 files changed, 86 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > > index 489cefcb81..0228083800 100644
> > > --- a/hw/ppc/spapr.c
> > > +++ b/hw/ppc/spapr.c
> > > @@ -890,6 +890,11 @@ static void spapr_dt_rtas(SpaprMachineState *spapr, void *fdt)
> > >      add_str(hypertas, "hcall-copy");
> > >      add_str(hypertas, "hcall-debug");
> > >      add_str(hypertas, "hcall-vphn");
> > > +    if (kvm_enabled() &&
> > 
> > You shouldn't check KVM here. The capability is enough to decide if we
> > should expose "hcall-rpt-invalidate" or not. FWIW we won't even reach
> > this code when running with anything but KVM.
> 
> Correct, the capability itself can be only for KVM case.
> 
> > 
> > > +        (spapr_get_cap(spapr, SPAPR_CAP_RPT_INVALIDATE) == SPAPR_CAP_ON)) {
> > > +        add_str(hypertas, "hcall-rpt-invalidate");
> > > +    }
> > > +
> > >      add_str(qemu_hypertas, "hcall-memop1");
> > >  
> > >      if (!kvm_enabled() || kvmppc_spapr_use_multitce()) {
> > > @@ -2021,6 +2026,7 @@ static const VMStateDescription vmstate_spapr = {
> > >          &vmstate_spapr_cap_ccf_assist,
> > >          &vmstate_spapr_cap_fwnmi,
> > >          &vmstate_spapr_fwnmi,
> > > +        &vmstate_spapr_cap_rpt_invalidate,
> > >          NULL
> > >      }
> > >  };
> > > @@ -4478,6 +4484,7 @@ static void spapr_machine_class_init(ObjectClass *oc, void *data)
> > >      smc->default_caps.caps[SPAPR_CAP_LARGE_DECREMENTER] = SPAPR_CAP_ON;
> > >      smc->default_caps.caps[SPAPR_CAP_CCF_ASSIST] = SPAPR_CAP_ON;
> > >      smc->default_caps.caps[SPAPR_CAP_FWNMI] = SPAPR_CAP_ON;
> > > +    smc->default_caps.caps[SPAPR_CAP_RPT_INVALIDATE] = SPAPR_CAP_OFF;
> > 
> > Any reason for not enabling this for the default machine type and
> > disabling it for existing machine types only ?
> 
> If this capability is enabled, then
> 
> 1. First level guest (L1) can off-load the TLB invalidations to the
> new hcall if the platform has disabled LPCR[GTSE].
> 
> 2. Nested guest (L2) will switch to this new hcall rather than using
> the old H_TLB_INVALIDATE hcall.
> 
> Case 2 is optional and case 1 makes sense only if LPCR[GTSE]=off.

I don't think this is relevant, as the importance of each case can change,
e.g. nested is gaining momentum.

> Hence I thought keeping it off by default and expecting the
> user to turn it on only if required would be correct.
> 

If the feature is an improvement, even for what is considered a corner
case now, and it doesn't do harm to setups that won't use it, then it
should be enabled IMHO.

> Please note that turning this capability ON will result in the
> new hcall being exposed to the guest. I hope this is the right
> usage of spapr-caps?
> 

That's perfectly fine and this is why we should set it to ON
for the default machine type only.

> > > diff --git a/target/ppc/kvm_ppc.h b/target/ppc/kvm_ppc.h
> > > index 73ce2bc951..8e27f8421f 100644
> > > --- a/target/ppc/kvm_ppc.h
> > > +++ b/target/ppc/kvm_ppc.h
> > > @@ -24,6 +24,7 @@ void kvmppc_enable_logical_ci_hcalls(void);
> > >  void kvmppc_enable_set_mode_hcall(void);
> > >  void kvmppc_enable_clear_ref_mod_hcalls(void);
> > >  void kvmppc_enable_h_page_init(void);
> > > +void kvmppc_enable_h_rpt_invalidate(void);
> > >  void kvmppc_set_papr(PowerPCCPU *cpu);
> > >  int kvmppc_set_compat(PowerPCCPU *cpu, uint32_t compat_pvr);
> > >  void kvmppc_set_mpic_proxy(PowerPCCPU *cpu, int mpic_proxy);
> > > @@ -72,6 +73,7 @@ bool kvmppc_has_cap_nested_kvm_hv(void);
> > >  int kvmppc_set_cap_nested_kvm_hv(int enable);
> > >  int kvmppc_get_cap_large_decr(void);
> > >  int kvmppc_enable_cap_large_decr(PowerPCCPU *cpu, int enable);
> > > +int kvmppc_has_cap_rpt_invalidate(void);
> > >  int kvmppc_enable_hwrng(void);
> > >  int kvmppc_put_books_sregs(PowerPCCPU *cpu);
> > >  PowerPCCPUClass *kvm_ppc_get_host_cpu_class(void);
> > > @@ -151,6 +153,10 @@ static inline void kvmppc_enable_h_page_init(void)
> > >  {
> > >  }
> > >  
> > > +static inline void kvmppc_enable_h_rpt_invalidate(void)
> > > +{
> > 
> > g_assert_not_reached() ?
> 
> Don't see many others doing that, is that a new preferred
> way?
> 

I don't know if this is preferred or not, but it feels that
this should never be called when !CONFIG_KVM.

Cheers,

--
Greg

> Regards,
> Bharata.

Re: [RFC PATCH v0 1/1] target/ppc: Support for H_RPT_INVALIDATE hcall

[David Gibson]

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

On Fri, Jan 15, 2021 at 02:01:28PM +0530, Bharata B Rao wrote:
> On Wed, Jan 13, 2021 at 05:22:56PM +0100, Greg Kurz wrote:
> > Hi Bharata,
> > 
> > On Wed,  6 Jan 2021 14:29:10 +0530
> > Bharata B Rao <bharata@linux.ibm.com> wrote:
> > 
> > > If KVM_CAP_RPT_INVALIDATE KVM capability is enabled, then
> > > 
> > > - indicate the availability of H_RPT_INVALIDATE hcall to the guest via
> > >   ibm,hypertas-functions property.
> > > - Enable the hcall
> > > 
> > > Both the above are done only if the new sPAPR machine capability
> > > cap-rpt-invalidate is set.
> > > 
> > > Note: The KVM implementation of the hcall has been posted for upstream
> > > review here:
> > > https://lore.kernel.org/linuxppc-dev/20210105090557.2150104-1-bharata@linux.ibm.com/T/#t
> > > 
> > > Update to linux-headers/linux/kvm.h here is temporary, will be
> > > done via header updates once the kernel change is accepted upstream.
> > > 
> > > Signed-off-by: Bharata B Rao <bharata@linux.ibm.com>
> > > ---
> > 
> > Patch looks mostly fine. A few remarks below.
> > 
> > >  hw/ppc/spapr.c            |  7 ++++++
> > >  hw/ppc/spapr_caps.c       | 49 +++++++++++++++++++++++++++++++++++++++
> > >  include/hw/ppc/spapr.h    |  8 +++++--
> > >  linux-headers/linux/kvm.h |  1 +
> > >  target/ppc/kvm.c          | 12 ++++++++++
> > >  target/ppc/kvm_ppc.h      | 11 +++++++++
> > >  6 files changed, 86 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > > index 489cefcb81..0228083800 100644
> > > --- a/hw/ppc/spapr.c
> > > +++ b/hw/ppc/spapr.c
> > > @@ -890,6 +890,11 @@ static void spapr_dt_rtas(SpaprMachineState *spapr, void *fdt)
> > >      add_str(hypertas, "hcall-copy");
> > >      add_str(hypertas, "hcall-debug");
> > >      add_str(hypertas, "hcall-vphn");
> > > +    if (kvm_enabled() &&
> > 
> > You shouldn't check KVM here. The capability is enough to decide if we
> > should expose "hcall-rpt-invalidate" or not. FWIW we won't even reach
> > this code when running with anything but KVM.
> 
> Correct, the capability itself can be only for KVM case.

Hrm.. that's kind of a problem in itself.  Enabling KVM should not
change the guest visible environment.

> 
> > 
> > > +        (spapr_get_cap(spapr, SPAPR_CAP_RPT_INVALIDATE) == SPAPR_CAP_ON)) {
> > > +        add_str(hypertas, "hcall-rpt-invalidate");
> > > +    }
> > > +
> > >      add_str(qemu_hypertas, "hcall-memop1");
> > >  
> > >      if (!kvm_enabled() || kvmppc_spapr_use_multitce()) {
> > > @@ -2021,6 +2026,7 @@ static const VMStateDescription vmstate_spapr = {
> > >          &vmstate_spapr_cap_ccf_assist,
> > >          &vmstate_spapr_cap_fwnmi,
> > >          &vmstate_spapr_fwnmi,
> > > +        &vmstate_spapr_cap_rpt_invalidate,
> > >          NULL
> > >      }
> > >  };
> > > @@ -4478,6 +4484,7 @@ static void spapr_machine_class_init(ObjectClass *oc, void *data)
> > >      smc->default_caps.caps[SPAPR_CAP_LARGE_DECREMENTER] = SPAPR_CAP_ON;
> > >      smc->default_caps.caps[SPAPR_CAP_CCF_ASSIST] = SPAPR_CAP_ON;
> > >      smc->default_caps.caps[SPAPR_CAP_FWNMI] = SPAPR_CAP_ON;
> > > +    smc->default_caps.caps[SPAPR_CAP_RPT_INVALIDATE] = SPAPR_CAP_OFF;
> > 
> > Any reason for not enabling this for the default machine type and
> > disabling it for existing machine types only ?
> 
> If this capability is enabled, then
> 
> 1. First level guest (L1) can off-load the TLB invalidations to the
> new hcall if the platform has disabled LPCR[GTSE].
> 
> 2. Nested guest (L2) will switch to this new hcall rather than using
> the old H_TLB_INVALIDATE hcall.
> 
> Case 2 is optional and case 1 makes sense only if LPCR[GTSE]=off.
> Hence I thought keeping it off by default and expecting the
> user to turn it on only if required would be correct.
> 
> Please note that turning this capability ON will result in the
> new hcall being exposed to the guest. I hope this is the right
> usage of spapr-caps?
> 
> > > diff --git a/target/ppc/kvm_ppc.h b/target/ppc/kvm_ppc.h
> > > index 73ce2bc951..8e27f8421f 100644
> > > --- a/target/ppc/kvm_ppc.h
> > > +++ b/target/ppc/kvm_ppc.h
> > > @@ -24,6 +24,7 @@ void kvmppc_enable_logical_ci_hcalls(void);
> > >  void kvmppc_enable_set_mode_hcall(void);
> > >  void kvmppc_enable_clear_ref_mod_hcalls(void);
> > >  void kvmppc_enable_h_page_init(void);
> > > +void kvmppc_enable_h_rpt_invalidate(void);
> > >  void kvmppc_set_papr(PowerPCCPU *cpu);
> > >  int kvmppc_set_compat(PowerPCCPU *cpu, uint32_t compat_pvr);
> > >  void kvmppc_set_mpic_proxy(PowerPCCPU *cpu, int mpic_proxy);
> > > @@ -72,6 +73,7 @@ bool kvmppc_has_cap_nested_kvm_hv(void);
> > >  int kvmppc_set_cap_nested_kvm_hv(int enable);
> > >  int kvmppc_get_cap_large_decr(void);
> > >  int kvmppc_enable_cap_large_decr(PowerPCCPU *cpu, int enable);
> > > +int kvmppc_has_cap_rpt_invalidate(void);
> > >  int kvmppc_enable_hwrng(void);
> > >  int kvmppc_put_books_sregs(PowerPCCPU *cpu);
> > >  PowerPCCPUClass *kvm_ppc_get_host_cpu_class(void);
> > > @@ -151,6 +153,10 @@ static inline void kvmppc_enable_h_page_init(void)
> > >  {
> > >  }
> > >  
> > > +static inline void kvmppc_enable_h_rpt_invalidate(void)
> > > +{
> > 
> > g_assert_not_reached() ?
> 
> Don't see many others doing that, is that a new preferred
> way?

Pretty much, yes.

> 
> Regards,
> Bharata.
> 

-- 
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: [RFC PATCH v0 1/1] target/ppc: Support for H_RPT_INVALIDATE hcall

[Bharata B Rao]

On Fri, Jan 15, 2021 at 06:30:05PM +0100, Greg Kurz wrote:
> On Fri, 15 Jan 2021 14:01:28 +0530
> Bharata B Rao <bharata@linux.ibm.com> wrote:
> 
> > On Wed, Jan 13, 2021 at 05:22:56PM +0100, Greg Kurz wrote:
> > > Hi Bharata,
> > > 
> > > On Wed,  6 Jan 2021 14:29:10 +0530
> > > Bharata B Rao <bharata@linux.ibm.com> wrote:
> > > 
> > > > If KVM_CAP_RPT_INVALIDATE KVM capability is enabled, then
> > > > 
> > > > - indicate the availability of H_RPT_INVALIDATE hcall to the guest via
> > > >   ibm,hypertas-functions property.
> > > > - Enable the hcall
> > > > 
> > > > Both the above are done only if the new sPAPR machine capability
> > > > cap-rpt-invalidate is set.
> > > > 
> > > > Note: The KVM implementation of the hcall has been posted for upstream
> > > > review here:
> > > > https://lore.kernel.org/linuxppc-dev/20210105090557.2150104-1-bharata@linux.ibm.com/T/#t
> > > > 
> > > > Update to linux-headers/linux/kvm.h here is temporary, will be
> > > > done via header updates once the kernel change is accepted upstream.
> > > > 
> > > > Signed-off-by: Bharata B Rao <bharata@linux.ibm.com>
> > > > ---
> > > 
> > > Patch looks mostly fine. A few remarks below.
> > > 
> > > >  hw/ppc/spapr.c            |  7 ++++++
> > > >  hw/ppc/spapr_caps.c       | 49 +++++++++++++++++++++++++++++++++++++++
> > > >  include/hw/ppc/spapr.h    |  8 +++++--
> > > >  linux-headers/linux/kvm.h |  1 +
> > > >  target/ppc/kvm.c          | 12 ++++++++++
> > > >  target/ppc/kvm_ppc.h      | 11 +++++++++
> > > >  6 files changed, 86 insertions(+), 2 deletions(-)
> > > > 
> > > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > > > index 489cefcb81..0228083800 100644
> > > > --- a/hw/ppc/spapr.c
> > > > +++ b/hw/ppc/spapr.c
> > > > @@ -890,6 +890,11 @@ static void spapr_dt_rtas(SpaprMachineState *spapr, void *fdt)
> > > >      add_str(hypertas, "hcall-copy");
> > > >      add_str(hypertas, "hcall-debug");
> > > >      add_str(hypertas, "hcall-vphn");
> > > > +    if (kvm_enabled() &&
> > > 
> > > You shouldn't check KVM here. The capability is enough to decide if we
> > > should expose "hcall-rpt-invalidate" or not. FWIW we won't even reach
> > > this code when running with anything but KVM.
> > 
> > Correct, the capability itself can be only for KVM case.
> > 
> > > 
> > > > +        (spapr_get_cap(spapr, SPAPR_CAP_RPT_INVALIDATE) == SPAPR_CAP_ON)) {
> > > > +        add_str(hypertas, "hcall-rpt-invalidate");
> > > > +    }
> > > > +
> > > >      add_str(qemu_hypertas, "hcall-memop1");
> > > >  
> > > >      if (!kvm_enabled() || kvmppc_spapr_use_multitce()) {
> > > > @@ -2021,6 +2026,7 @@ static const VMStateDescription vmstate_spapr = {
> > > >          &vmstate_spapr_cap_ccf_assist,
> > > >          &vmstate_spapr_cap_fwnmi,
> > > >          &vmstate_spapr_fwnmi,
> > > > +        &vmstate_spapr_cap_rpt_invalidate,
> > > >          NULL
> > > >      }
> > > >  };
> > > > @@ -4478,6 +4484,7 @@ static void spapr_machine_class_init(ObjectClass *oc, void *data)
> > > >      smc->default_caps.caps[SPAPR_CAP_LARGE_DECREMENTER] = SPAPR_CAP_ON;
> > > >      smc->default_caps.caps[SPAPR_CAP_CCF_ASSIST] = SPAPR_CAP_ON;
> > > >      smc->default_caps.caps[SPAPR_CAP_FWNMI] = SPAPR_CAP_ON;
> > > > +    smc->default_caps.caps[SPAPR_CAP_RPT_INVALIDATE] = SPAPR_CAP_OFF;
> > > 
> > > Any reason for not enabling this for the default machine type and
> > > disabling it for existing machine types only ?
> > 
> > If this capability is enabled, then
> > 
> > 1. First level guest (L1) can off-load the TLB invalidations to the
> > new hcall if the platform has disabled LPCR[GTSE].
> > 
> > 2. Nested guest (L2) will switch to this new hcall rather than using
> > the old H_TLB_INVALIDATE hcall.
> > 
> > Case 2 is optional and case 1 makes sense only if LPCR[GTSE]=off.
> 
> I don't think this is relevant, as the importance of each case can change,
> e.g. nested is gaining momentum.
> 
> > Hence I thought keeping it off by default and expecting the
> > user to turn it on only if required would be correct.
> > 
> 
> If the feature is an improvement, even for what is considered a corner
> case now, and it doesn't do harm to setups that won't use it, then it
> should be enabled IMHO.
> 
> > Please note that turning this capability ON will result in the
> > new hcall being exposed to the guest. I hope this is the right
> > usage of spapr-caps?
> > 
> 
> That's perfectly fine and this is why we should set it to ON
> for the default machine type only.

The property can be turned ON only when the hypervisor supports
the hcall. So if it set to ON for default machine type, then
it may fail if the host doesn't have this hcall. Hence I thought
it should be OFF by default and turning ON should be left to the
user.

Regards,
Bharata.

Re: [RFC PATCH v0 1/1] target/ppc: Support for H_RPT_INVALIDATE hcall

[Greg Kurz]

On Tue, 19 Jan 2021 10:14:55 +0530
Bharata B Rao <bharata@linux.ibm.com> wrote:

> On Fri, Jan 15, 2021 at 06:30:05PM +0100, Greg Kurz wrote:
> > On Fri, 15 Jan 2021 14:01:28 +0530
> > Bharata B Rao <bharata@linux.ibm.com> wrote:
> > 
> > > On Wed, Jan 13, 2021 at 05:22:56PM +0100, Greg Kurz wrote:
> > > > Hi Bharata,
> > > > 
> > > > On Wed,  6 Jan 2021 14:29:10 +0530
> > > > Bharata B Rao <bharata@linux.ibm.com> wrote:
> > > > 
> > > > > If KVM_CAP_RPT_INVALIDATE KVM capability is enabled, then
> > > > > 
> > > > > - indicate the availability of H_RPT_INVALIDATE hcall to the guest via
> > > > >   ibm,hypertas-functions property.
> > > > > - Enable the hcall
> > > > > 
> > > > > Both the above are done only if the new sPAPR machine capability
> > > > > cap-rpt-invalidate is set.
> > > > > 
> > > > > Note: The KVM implementation of the hcall has been posted for upstream
> > > > > review here:
> > > > > https://lore.kernel.org/linuxppc-dev/20210105090557.2150104-1-bharata@linux.ibm.com/T/#t
> > > > > 
> > > > > Update to linux-headers/linux/kvm.h here is temporary, will be
> > > > > done via header updates once the kernel change is accepted upstream.
> > > > > 
> > > > > Signed-off-by: Bharata B Rao <bharata@linux.ibm.com>
> > > > > ---
> > > > 
> > > > Patch looks mostly fine. A few remarks below.
> > > > 
> > > > >  hw/ppc/spapr.c            |  7 ++++++
> > > > >  hw/ppc/spapr_caps.c       | 49 +++++++++++++++++++++++++++++++++++++++
> > > > >  include/hw/ppc/spapr.h    |  8 +++++--
> > > > >  linux-headers/linux/kvm.h |  1 +
> > > > >  target/ppc/kvm.c          | 12 ++++++++++
> > > > >  target/ppc/kvm_ppc.h      | 11 +++++++++
> > > > >  6 files changed, 86 insertions(+), 2 deletions(-)
> > > > > 
> > > > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > > > > index 489cefcb81..0228083800 100644
> > > > > --- a/hw/ppc/spapr.c
> > > > > +++ b/hw/ppc/spapr.c
> > > > > @@ -890,6 +890,11 @@ static void spapr_dt_rtas(SpaprMachineState *spapr, void *fdt)
> > > > >      add_str(hypertas, "hcall-copy");
> > > > >      add_str(hypertas, "hcall-debug");
> > > > >      add_str(hypertas, "hcall-vphn");
> > > > > +    if (kvm_enabled() &&
> > > > 
> > > > You shouldn't check KVM here. The capability is enough to decide if we
> > > > should expose "hcall-rpt-invalidate" or not. FWIW we won't even reach
> > > > this code when running with anything but KVM.
> > > 
> > > Correct, the capability itself can be only for KVM case.
> > > 
> > > > 
> > > > > +        (spapr_get_cap(spapr, SPAPR_CAP_RPT_INVALIDATE) == SPAPR_CAP_ON)) {
> > > > > +        add_str(hypertas, "hcall-rpt-invalidate");
> > > > > +    }
> > > > > +
> > > > >      add_str(qemu_hypertas, "hcall-memop1");
> > > > >  
> > > > >      if (!kvm_enabled() || kvmppc_spapr_use_multitce()) {
> > > > > @@ -2021,6 +2026,7 @@ static const VMStateDescription vmstate_spapr = {
> > > > >          &vmstate_spapr_cap_ccf_assist,
> > > > >          &vmstate_spapr_cap_fwnmi,
> > > > >          &vmstate_spapr_fwnmi,
> > > > > +        &vmstate_spapr_cap_rpt_invalidate,
> > > > >          NULL
> > > > >      }
> > > > >  };
> > > > > @@ -4478,6 +4484,7 @@ static void spapr_machine_class_init(ObjectClass *oc, void *data)
> > > > >      smc->default_caps.caps[SPAPR_CAP_LARGE_DECREMENTER] = SPAPR_CAP_ON;
> > > > >      smc->default_caps.caps[SPAPR_CAP_CCF_ASSIST] = SPAPR_CAP_ON;
> > > > >      smc->default_caps.caps[SPAPR_CAP_FWNMI] = SPAPR_CAP_ON;
> > > > > +    smc->default_caps.caps[SPAPR_CAP_RPT_INVALIDATE] = SPAPR_CAP_OFF;
> > > > 
> > > > Any reason for not enabling this for the default machine type and
> > > > disabling it for existing machine types only ?
> > > 
> > > If this capability is enabled, then
> > > 
> > > 1. First level guest (L1) can off-load the TLB invalidations to the
> > > new hcall if the platform has disabled LPCR[GTSE].
> > > 
> > > 2. Nested guest (L2) will switch to this new hcall rather than using
> > > the old H_TLB_INVALIDATE hcall.
> > > 
> > > Case 2 is optional and case 1 makes sense only if LPCR[GTSE]=off.
> > 
> > I don't think this is relevant, as the importance of each case can change,
> > e.g. nested is gaining momentum.
> > 
> > > Hence I thought keeping it off by default and expecting the
> > > user to turn it on only if required would be correct.
> > > 
> > 
> > If the feature is an improvement, even for what is considered a corner
> > case now, and it doesn't do harm to setups that won't use it, then it
> > should be enabled IMHO.
> > 
> > > Please note that turning this capability ON will result in the
> > > new hcall being exposed to the guest. I hope this is the right
> > > usage of spapr-caps?
> > > 
> > 
> > That's perfectly fine and this is why we should set it to ON
> > for the default machine type only.
> 
> The property can be turned ON only when the hypervisor supports
> the hcall. So if it set to ON for default machine type, then
> it may fail if the host doesn't have this hcall. Hence I thought
> it should be OFF by default and turning ON should be left to the
> user.
> 

Ok. This can be changed later when H_RPT_INVALIDATE support is
more widely available. BTW, if users are expected to manually
set this, I think you should add some documentation so that
they know how/when to use it.

> Regards,
> Bharata.