All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] KVM: PPC: Book3S PR: SCV fixes
@ 2022-01-24 10:24 Nicholas Piggin
  2022-01-24 10:24 ` [PATCH 1/2] KVM: PPC: Book3S PR: Disable SCV when running AIL is disabled Nicholas Piggin
  2022-01-24 10:24 ` [PATCH 2/2] KVM: PPC: Book3S PR: Disallow AIL != 0 Nicholas Piggin
  0 siblings, 2 replies; 6+ messages in thread
From: Nicholas Piggin @ 2022-01-24 10:24 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Nicholas Piggin

These patches seem to be the quickest and easiest way to avoid the
host crash and to get recent Linux images (that use scv) to run under
PR.

These are independent of the syscall emulation series. Those just came
about when looking at ways to fix the host problem and seeing emulation
was broken.

Thanks,
Nick

Nicholas Piggin (2):
  KVM: PPC: Book3S PR: Disable SCV when running AIL is disabled
  KVM: PPC: Book3S PR: Disallow AIL != 0

 arch/powerpc/kernel/exceptions-64s.S |  4 ++++
 arch/powerpc/kernel/setup_64.c       | 15 +++++++++++++++
 arch/powerpc/kvm/book3s_pr.c         | 20 ++++++++++++++------
 arch/powerpc/kvm/book3s_pr_papr.c    | 20 ++++++++++++++++++++
 4 files changed, 53 insertions(+), 6 deletions(-)

-- 
2.23.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 1/2] KVM: PPC: Book3S PR: Disable SCV when running AIL is disabled
  2022-01-24 10:24 [PATCH 0/2] KVM: PPC: Book3S PR: SCV fixes Nicholas Piggin
@ 2022-01-24 10:24 ` Nicholas Piggin
  2022-01-24 22:49   ` Fabiano Rosas
  2022-01-24 10:24 ` [PATCH 2/2] KVM: PPC: Book3S PR: Disallow AIL != 0 Nicholas Piggin
  1 sibling, 1 reply; 6+ messages in thread
From: Nicholas Piggin @ 2022-01-24 10:24 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Nicholas Piggin

PR KVM does not support running with AIL enabled, and SCV does is not
supported with AIL disabled.

Fix this by ensuring the SCV facility is disabled with FSCR while a
CPU can be running with AIL=0. PowerNV host supports disabling AIL on a
per-CPU basis, so SCV just needs to be disabled when a vCPU is run.

The pSeries machine can only switch AIL on a system-wide basis, so it
must disable SCV support at boot if the configuration can potentially
run a PR KVM guest.

SCV is not emulated for the PR guest at the moment, this just fixes the
host crashes.

Alternatives considered and rejected:
- SCV support can not be disabled by PR KVM after boot, because it is
  advertised to userspace with HWCAP.
- AIL can not be disabled on a per-CPU basis. At least when running on
  pseries it is a per-LPAR setting.
- Support for real-mode SCV vectors will not be added because they are
  at 0x17000 so making such a large fixed head space causes immediate
  value limits to be exceeded, requiring a lot rework and more code.
- Disabling SCV for any PR KVM possible kernel will cause a slowdown
  when not using PR KVM.
- A boot time option to disable SCV to use PR KVM is user-hostile.
- System call instruction emulation for SCV facility unavailable
  instructions is too complex and old emulation code was subtly broken
  and removed.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 arch/powerpc/kernel/exceptions-64s.S |  4 ++++
 arch/powerpc/kernel/setup_64.c       | 15 +++++++++++++++
 arch/powerpc/kvm/book3s_pr.c         | 20 ++++++++++++++------
 3 files changed, 33 insertions(+), 6 deletions(-)

diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S
index 55caeee37c08..b66dd6f775a4 100644
--- a/arch/powerpc/kernel/exceptions-64s.S
+++ b/arch/powerpc/kernel/exceptions-64s.S
@@ -809,6 +809,10 @@ __start_interrupts:
  * - MSR_EE|MSR_RI is clear (no reentrant exceptions)
  * - Standard kernel environment is set up (stack, paca, etc)
  *
+ * KVM:
+ * These interrupts do not elevate HV 0->1, so HV is not involved. PR KVM
+ * ensures that FSCR[SCV] is disabled whenever it has to force AIL off.
+ *
  * Call convention:
  *
  * syscall register convention is in Documentation/powerpc/syscall64-abi.rst
diff --git a/arch/powerpc/kernel/setup_64.c b/arch/powerpc/kernel/setup_64.c
index be8577ac9397..ac52c69a3811 100644
--- a/arch/powerpc/kernel/setup_64.c
+++ b/arch/powerpc/kernel/setup_64.c
@@ -197,6 +197,21 @@ static void __init configure_exceptions(void)
 
 	/* Under a PAPR hypervisor, we need hypercalls */
 	if (firmware_has_feature(FW_FEATURE_SET_MODE)) {
+		/*
+		 * PR KVM does not support AIL mode interrupts in the host, and
+		 * SCV system call interrupt vectors are only implemented for
+		 * AIL mode. Under pseries, AIL mode can only be enabled and
+		 * disabled system-wide so when PR KVM is loaded, all CPUs in
+		 * the host are set to AIL=0 mode. SCV can not be disabled
+		 * dynamically because the feature is advertised to host
+		 * userspace, so SCV support must not be enabled if PR KVM can
+		 * possibly be run.
+		 */
+		if (IS_ENABLED(CONFIG_KVM_BOOK3S_PR_POSSIBLE) && !radix_enabled()) {
+			init_task.thread.fscr &= ~FSCR_SCV;
+			cur_cpu_spec->cpu_user_features2 &= ~PPC_FEATURE2_SCV;
+		}
+
 		/* Enable AIL if possible */
 		if (!pseries_enable_reloc_on_exc()) {
 			init_task.thread.fscr &= ~FSCR_SCV;
diff --git a/arch/powerpc/kvm/book3s_pr.c b/arch/powerpc/kvm/book3s_pr.c
index 34a801c3604a..4d1c84b94b77 100644
--- a/arch/powerpc/kvm/book3s_pr.c
+++ b/arch/powerpc/kvm/book3s_pr.c
@@ -140,9 +140,12 @@ static void kvmppc_core_vcpu_load_pr(struct kvm_vcpu *vcpu, int cpu)
 #endif
 
 	/* Disable AIL if supported */
-	if (cpu_has_feature(CPU_FTR_HVMODE) &&
-	    cpu_has_feature(CPU_FTR_ARCH_207S))
-		mtspr(SPRN_LPCR, mfspr(SPRN_LPCR) & ~LPCR_AIL);
+	if (cpu_has_feature(CPU_FTR_HVMODE)) {
+		if (cpu_has_feature(CPU_FTR_ARCH_207S))
+			mtspr(SPRN_LPCR, mfspr(SPRN_LPCR) & ~LPCR_AIL);
+		if (cpu_has_feature(CPU_FTR_ARCH_300) && (current->thread.fscr & FSCR_SCV))
+			mtspr(SPRN_FSCR, mfspr(SPRN_FSCR) & ~FSCR_SCV);
+	}
 
 	vcpu->cpu = smp_processor_id();
 #ifdef CONFIG_PPC_BOOK3S_32
@@ -175,9 +178,12 @@ static void kvmppc_core_vcpu_put_pr(struct kvm_vcpu *vcpu)
 	kvmppc_save_tm_pr(vcpu);
 
 	/* Enable AIL if supported */
-	if (cpu_has_feature(CPU_FTR_HVMODE) &&
-	    cpu_has_feature(CPU_FTR_ARCH_207S))
-		mtspr(SPRN_LPCR, mfspr(SPRN_LPCR) | LPCR_AIL_3);
+	if (cpu_has_feature(CPU_FTR_HVMODE)) {
+		if (cpu_has_feature(CPU_FTR_ARCH_207S))
+			mtspr(SPRN_LPCR, mfspr(SPRN_LPCR) | LPCR_AIL_3);
+		if (cpu_has_feature(CPU_FTR_ARCH_300) && (current->thread.fscr & FSCR_SCV))
+			mtspr(SPRN_FSCR, mfspr(SPRN_FSCR) | FSCR_SCV);
+	}
 
 	vcpu->cpu = -1;
 }
@@ -1037,6 +1043,8 @@ static int kvmppc_handle_fac(struct kvm_vcpu *vcpu, ulong fac)
 
 void kvmppc_set_fscr(struct kvm_vcpu *vcpu, u64 fscr)
 {
+	if (fscr & FSCR_SCV)
+		fscr &= ~FSCR_SCV; /* SCV must not be enabled */
 	if ((vcpu->arch.fscr & FSCR_TAR) && !(fscr & FSCR_TAR)) {
 		/* TAR got dropped, drop it in shadow too */
 		kvmppc_giveup_fac(vcpu, FSCR_TAR_LG);
-- 
2.23.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 2/2] KVM: PPC: Book3S PR: Disallow AIL != 0
  2022-01-24 10:24 [PATCH 0/2] KVM: PPC: Book3S PR: SCV fixes Nicholas Piggin
  2022-01-24 10:24 ` [PATCH 1/2] KVM: PPC: Book3S PR: Disable SCV when running AIL is disabled Nicholas Piggin
@ 2022-01-24 10:24 ` Nicholas Piggin
  2022-01-24 22:50   ` Fabiano Rosas
  1 sibling, 1 reply; 6+ messages in thread
From: Nicholas Piggin @ 2022-01-24 10:24 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Nicholas Piggin

KVM PR does not implement address translation modes on interrupt, so it
must not allow H_SET_MODE to succeed.

This is not compatible with QEMU behaviour. The solution might be to
have a cap-ail for this, but now it's broken either way so fix it in
KVM to start with.

This allows PR Linux guests that are using the SCV facility to boot and
run, because Linux disables the use of SCV if AIL can not be set to 3.
This isn't a real fix because Linux or another OS could implement real
mode SCV vectors and try to enable it. The right solution is for KVM to
emulate scv interrupts from the facility unavailable interrupt.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 arch/powerpc/kvm/book3s_pr_papr.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/arch/powerpc/kvm/book3s_pr_papr.c b/arch/powerpc/kvm/book3s_pr_papr.c
index 1f10e7dfcdd0..dc4f51ac84bc 100644
--- a/arch/powerpc/kvm/book3s_pr_papr.c
+++ b/arch/powerpc/kvm/book3s_pr_papr.c
@@ -281,6 +281,22 @@ static int kvmppc_h_pr_logical_ci_store(struct kvm_vcpu *vcpu)
 	return EMULATE_DONE;
 }
 
+static int kvmppc_h_pr_set_mode(struct kvm_vcpu *vcpu)
+{
+	unsigned long mflags = kvmppc_get_gpr(vcpu, 4);
+	unsigned long resource = kvmppc_get_gpr(vcpu, 5);
+
+	if (resource == H_SET_MODE_RESOURCE_ADDR_TRANS_MODE) {
+		/* KVM PR does not provide AIL!=0 to guests */
+		if (mflags == 0)
+			kvmppc_set_gpr(vcpu, 3, H_SUCCESS);
+		else
+			kvmppc_set_gpr(vcpu, 3, H_UNSUPPORTED_FLAG_START - 63);
+		return EMULATE_DONE;
+	}
+	return EMULATE_FAIL;
+}
+
 #ifdef CONFIG_SPAPR_TCE_IOMMU
 static int kvmppc_h_pr_put_tce(struct kvm_vcpu *vcpu)
 {
@@ -384,6 +400,8 @@ int kvmppc_h_pr(struct kvm_vcpu *vcpu, unsigned long cmd)
 		return kvmppc_h_pr_logical_ci_load(vcpu);
 	case H_LOGICAL_CI_STORE:
 		return kvmppc_h_pr_logical_ci_store(vcpu);
+	case H_SET_MODE:
+		return kvmppc_h_pr_set_mode(vcpu);
 	case H_XIRR:
 	case H_CPPR:
 	case H_EOI:
@@ -421,6 +439,7 @@ int kvmppc_hcall_impl_pr(unsigned long cmd)
 	case H_CEDE:
 	case H_LOGICAL_CI_LOAD:
 	case H_LOGICAL_CI_STORE:
+	case H_SET_MODE:
 #ifdef CONFIG_KVM_XICS
 	case H_XIRR:
 	case H_CPPR:
@@ -447,6 +466,7 @@ static unsigned int default_hcall_list[] = {
 	H_BULK_REMOVE,
 	H_PUT_TCE,
 	H_CEDE,
+	H_SET_MODE,
 #ifdef CONFIG_KVM_XICS
 	H_XIRR,
 	H_CPPR,
-- 
2.23.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] KVM: PPC: Book3S PR: Disable SCV when running AIL is disabled
  2022-01-24 10:24 ` [PATCH 1/2] KVM: PPC: Book3S PR: Disable SCV when running AIL is disabled Nicholas Piggin
@ 2022-01-24 22:49   ` Fabiano Rosas
  2022-01-25  3:55     ` Nicholas Piggin
  0 siblings, 1 reply; 6+ messages in thread
From: Fabiano Rosas @ 2022-01-24 22:49 UTC (permalink / raw)
  To: Nicholas Piggin, linuxppc-dev; +Cc: Nicholas Piggin

Nicholas Piggin <npiggin@gmail.com> writes:

> PR KVM does not support running with AIL enabled, and SCV does is not
> supported with AIL disabled.
>
> Fix this by ensuring the SCV facility is disabled with FSCR while a
> CPU can be running with AIL=0. PowerNV host supports disabling AIL on a
> per-CPU basis, so SCV just needs to be disabled when a vCPU is run.
>
> The pSeries machine can only switch AIL on a system-wide basis, so it
> must disable SCV support at boot if the configuration can potentially
> run a PR KVM guest.
>
> SCV is not emulated for the PR guest at the moment, this just fixes the
> host crashes.
>
> Alternatives considered and rejected:
> - SCV support can not be disabled by PR KVM after boot, because it is
>   advertised to userspace with HWCAP.
> - AIL can not be disabled on a per-CPU basis. At least when running on
>   pseries it is a per-LPAR setting.
> - Support for real-mode SCV vectors will not be added because they are
>   at 0x17000 so making such a large fixed head space causes immediate
>   value limits to be exceeded, requiring a lot rework and more code.
> - Disabling SCV for any PR KVM possible kernel will cause a slowdown
>   when not using PR KVM.
> - A boot time option to disable SCV to use PR KVM is user-hostile.
> - System call instruction emulation for SCV facility unavailable
>   instructions is too complex and old emulation code was subtly broken
>   and removed.
>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>  arch/powerpc/kernel/exceptions-64s.S |  4 ++++
>  arch/powerpc/kernel/setup_64.c       | 15 +++++++++++++++
>  arch/powerpc/kvm/book3s_pr.c         | 20 ++++++++++++++------
>  3 files changed, 33 insertions(+), 6 deletions(-)
>
> diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S
> index 55caeee37c08..b66dd6f775a4 100644
> --- a/arch/powerpc/kernel/exceptions-64s.S
> +++ b/arch/powerpc/kernel/exceptions-64s.S
> @@ -809,6 +809,10 @@ __start_interrupts:
>   * - MSR_EE|MSR_RI is clear (no reentrant exceptions)
>   * - Standard kernel environment is set up (stack, paca, etc)
>   *
> + * KVM:
> + * These interrupts do not elevate HV 0->1, so HV is not involved. PR KVM
> + * ensures that FSCR[SCV] is disabled whenever it has to force AIL off.
> + *
>   * Call convention:
>   *
>   * syscall register convention is in Documentation/powerpc/syscall64-abi.rst
> diff --git a/arch/powerpc/kernel/setup_64.c b/arch/powerpc/kernel/setup_64.c
> index be8577ac9397..ac52c69a3811 100644
> --- a/arch/powerpc/kernel/setup_64.c
> +++ b/arch/powerpc/kernel/setup_64.c
> @@ -197,6 +197,21 @@ static void __init configure_exceptions(void)
>
>  	/* Under a PAPR hypervisor, we need hypercalls */
>  	if (firmware_has_feature(FW_FEATURE_SET_MODE)) {
> +		/*
> +		 * PR KVM does not support AIL mode interrupts in the host, and
> +		 * SCV system call interrupt vectors are only implemented for
> +		 * AIL mode. Under pseries, AIL mode can only be enabled and
> +		 * disabled system-wide so when PR KVM is loaded, all CPUs in
> +		 * the host are set to AIL=0 mode. SCV can not be disabled
> +		 * dynamically because the feature is advertised to host
> +		 * userspace, so SCV support must not be enabled if PR KVM can
> +		 * possibly be run.
> +		 */
> +		if (IS_ENABLED(CONFIG_KVM_BOOK3S_PR_POSSIBLE) && !radix_enabled()) {
> +			init_task.thread.fscr &= ~FSCR_SCV;
> +			cur_cpu_spec->cpu_user_features2 &= ~PPC_FEATURE2_SCV;
> +		}
> +

"Under pseries, AIL mode can only be enabled and disabled system-wide so
 when PR KVM is loaded, all CPUs in the host are set to AIL=0 mode."

Loaded as in 'modprobe kvm_pr'? And host as in "nested host"
surely. Unless I completely misunderstood the patch (likely).

Is there a way to make this less unexpected to users? Maybe a few words
in the Kconfig entry for PR_POSSIBLE saying "if you enable this and run
a Hash MMU guest, you lose SCV"?

>  		/* Enable AIL if possible */
>  		if (!pseries_enable_reloc_on_exc()) {
>  			init_task.thread.fscr &= ~FSCR_SCV;
> diff --git a/arch/powerpc/kvm/book3s_pr.c b/arch/powerpc/kvm/book3s_pr.c
> index 34a801c3604a..4d1c84b94b77 100644
> --- a/arch/powerpc/kvm/book3s_pr.c
> +++ b/arch/powerpc/kvm/book3s_pr.c
> @@ -140,9 +140,12 @@ static void kvmppc_core_vcpu_load_pr(struct kvm_vcpu *vcpu, int cpu)
>  #endif
>
>  	/* Disable AIL if supported */
> -	if (cpu_has_feature(CPU_FTR_HVMODE) &&
> -	    cpu_has_feature(CPU_FTR_ARCH_207S))
> -		mtspr(SPRN_LPCR, mfspr(SPRN_LPCR) & ~LPCR_AIL);
> +	if (cpu_has_feature(CPU_FTR_HVMODE)) {
> +		if (cpu_has_feature(CPU_FTR_ARCH_207S))
> +			mtspr(SPRN_LPCR, mfspr(SPRN_LPCR) & ~LPCR_AIL);
> +		if (cpu_has_feature(CPU_FTR_ARCH_300) && (current->thread.fscr & FSCR_SCV))
> +			mtspr(SPRN_FSCR, mfspr(SPRN_FSCR) & ~FSCR_SCV);
> +	}
>
>  	vcpu->cpu = smp_processor_id();
>  #ifdef CONFIG_PPC_BOOK3S_32
> @@ -175,9 +178,12 @@ static void kvmppc_core_vcpu_put_pr(struct kvm_vcpu *vcpu)
>  	kvmppc_save_tm_pr(vcpu);
>
>  	/* Enable AIL if supported */
> -	if (cpu_has_feature(CPU_FTR_HVMODE) &&
> -	    cpu_has_feature(CPU_FTR_ARCH_207S))
> -		mtspr(SPRN_LPCR, mfspr(SPRN_LPCR) | LPCR_AIL_3);
> +	if (cpu_has_feature(CPU_FTR_HVMODE)) {
> +		if (cpu_has_feature(CPU_FTR_ARCH_207S))
> +			mtspr(SPRN_LPCR, mfspr(SPRN_LPCR) | LPCR_AIL_3);
> +		if (cpu_has_feature(CPU_FTR_ARCH_300) && (current->thread.fscr & FSCR_SCV))
> +			mtspr(SPRN_FSCR, mfspr(SPRN_FSCR) | FSCR_SCV);
> +	}
>
>  	vcpu->cpu = -1;
>  }
> @@ -1037,6 +1043,8 @@ static int kvmppc_handle_fac(struct kvm_vcpu *vcpu, ulong fac)
>
>  void kvmppc_set_fscr(struct kvm_vcpu *vcpu, u64 fscr)
>  {
> +	if (fscr & FSCR_SCV)
> +		fscr &= ~FSCR_SCV; /* SCV must not be enabled */
>  	if ((vcpu->arch.fscr & FSCR_TAR) && !(fscr & FSCR_TAR)) {
>  		/* TAR got dropped, drop it in shadow too */
>  		kvmppc_giveup_fac(vcpu, FSCR_TAR_LG);

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/2] KVM: PPC: Book3S PR: Disallow AIL != 0
  2022-01-24 10:24 ` [PATCH 2/2] KVM: PPC: Book3S PR: Disallow AIL != 0 Nicholas Piggin
@ 2022-01-24 22:50   ` Fabiano Rosas
  0 siblings, 0 replies; 6+ messages in thread
From: Fabiano Rosas @ 2022-01-24 22:50 UTC (permalink / raw)
  To: Nicholas Piggin, linuxppc-dev; +Cc: Nicholas Piggin

Nicholas Piggin <npiggin@gmail.com> writes:

> KVM PR does not implement address translation modes on interrupt, so it
> must not allow H_SET_MODE to succeed.
>
> This is not compatible with QEMU behaviour. The solution might be to
> have a cap-ail for this, but now it's broken either way so fix it in
> KVM to start with.
>
> This allows PR Linux guests that are using the SCV facility to boot and
> run, because Linux disables the use of SCV if AIL can not be set to 3.
> This isn't a real fix because Linux or another OS could implement real
> mode SCV vectors and try to enable it. The right solution is for KVM to
> emulate scv interrupts from the facility unavailable interrupt.
>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---

Reviewed-by: Fabiano Rosas <farosas@linux.ibm.com>

>  arch/powerpc/kvm/book3s_pr_papr.c | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
>
> diff --git a/arch/powerpc/kvm/book3s_pr_papr.c b/arch/powerpc/kvm/book3s_pr_papr.c
> index 1f10e7dfcdd0..dc4f51ac84bc 100644
> --- a/arch/powerpc/kvm/book3s_pr_papr.c
> +++ b/arch/powerpc/kvm/book3s_pr_papr.c
> @@ -281,6 +281,22 @@ static int kvmppc_h_pr_logical_ci_store(struct kvm_vcpu *vcpu)
>  	return EMULATE_DONE;
>  }
>
> +static int kvmppc_h_pr_set_mode(struct kvm_vcpu *vcpu)
> +{
> +	unsigned long mflags = kvmppc_get_gpr(vcpu, 4);
> +	unsigned long resource = kvmppc_get_gpr(vcpu, 5);
> +
> +	if (resource == H_SET_MODE_RESOURCE_ADDR_TRANS_MODE) {
> +		/* KVM PR does not provide AIL!=0 to guests */
> +		if (mflags == 0)
> +			kvmppc_set_gpr(vcpu, 3, H_SUCCESS);
> +		else
> +			kvmppc_set_gpr(vcpu, 3, H_UNSUPPORTED_FLAG_START - 63);
> +		return EMULATE_DONE;
> +	}
> +	return EMULATE_FAIL;
> +}
> +
>  #ifdef CONFIG_SPAPR_TCE_IOMMU
>  static int kvmppc_h_pr_put_tce(struct kvm_vcpu *vcpu)
>  {
> @@ -384,6 +400,8 @@ int kvmppc_h_pr(struct kvm_vcpu *vcpu, unsigned long cmd)
>  		return kvmppc_h_pr_logical_ci_load(vcpu);
>  	case H_LOGICAL_CI_STORE:
>  		return kvmppc_h_pr_logical_ci_store(vcpu);
> +	case H_SET_MODE:
> +		return kvmppc_h_pr_set_mode(vcpu);
>  	case H_XIRR:
>  	case H_CPPR:
>  	case H_EOI:
> @@ -421,6 +439,7 @@ int kvmppc_hcall_impl_pr(unsigned long cmd)
>  	case H_CEDE:
>  	case H_LOGICAL_CI_LOAD:
>  	case H_LOGICAL_CI_STORE:
> +	case H_SET_MODE:
>  #ifdef CONFIG_KVM_XICS
>  	case H_XIRR:
>  	case H_CPPR:
> @@ -447,6 +466,7 @@ static unsigned int default_hcall_list[] = {
>  	H_BULK_REMOVE,
>  	H_PUT_TCE,
>  	H_CEDE,
> +	H_SET_MODE,
>  #ifdef CONFIG_KVM_XICS
>  	H_XIRR,
>  	H_CPPR,

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] KVM: PPC: Book3S PR: Disable SCV when running AIL is disabled
  2022-01-24 22:49   ` Fabiano Rosas
@ 2022-01-25  3:55     ` Nicholas Piggin
  0 siblings, 0 replies; 6+ messages in thread
From: Nicholas Piggin @ 2022-01-25  3:55 UTC (permalink / raw)
  To: Fabiano Rosas, linuxppc-dev

Excerpts from Fabiano Rosas's message of January 25, 2022 8:49 am:
> Nicholas Piggin <npiggin@gmail.com> writes:
> 
>> PR KVM does not support running with AIL enabled, and SCV does is not
>> supported with AIL disabled.
>>
>> Fix this by ensuring the SCV facility is disabled with FSCR while a
>> CPU can be running with AIL=0. PowerNV host supports disabling AIL on a
>> per-CPU basis, so SCV just needs to be disabled when a vCPU is run.
>>
>> The pSeries machine can only switch AIL on a system-wide basis, so it
>> must disable SCV support at boot if the configuration can potentially
>> run a PR KVM guest.
>>
>> SCV is not emulated for the PR guest at the moment, this just fixes the
>> host crashes.
>>
>> Alternatives considered and rejected:
>> - SCV support can not be disabled by PR KVM after boot, because it is
>>   advertised to userspace with HWCAP.
>> - AIL can not be disabled on a per-CPU basis. At least when running on
>>   pseries it is a per-LPAR setting.
>> - Support for real-mode SCV vectors will not be added because they are
>>   at 0x17000 so making such a large fixed head space causes immediate
>>   value limits to be exceeded, requiring a lot rework and more code.
>> - Disabling SCV for any PR KVM possible kernel will cause a slowdown
>>   when not using PR KVM.
>> - A boot time option to disable SCV to use PR KVM is user-hostile.
>> - System call instruction emulation for SCV facility unavailable
>>   instructions is too complex and old emulation code was subtly broken
>>   and removed.
>>
>> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
>> ---
>>  arch/powerpc/kernel/exceptions-64s.S |  4 ++++
>>  arch/powerpc/kernel/setup_64.c       | 15 +++++++++++++++
>>  arch/powerpc/kvm/book3s_pr.c         | 20 ++++++++++++++------
>>  3 files changed, 33 insertions(+), 6 deletions(-)
>>
>> diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S
>> index 55caeee37c08..b66dd6f775a4 100644
>> --- a/arch/powerpc/kernel/exceptions-64s.S
>> +++ b/arch/powerpc/kernel/exceptions-64s.S
>> @@ -809,6 +809,10 @@ __start_interrupts:
>>   * - MSR_EE|MSR_RI is clear (no reentrant exceptions)
>>   * - Standard kernel environment is set up (stack, paca, etc)
>>   *
>> + * KVM:
>> + * These interrupts do not elevate HV 0->1, so HV is not involved. PR KVM
>> + * ensures that FSCR[SCV] is disabled whenever it has to force AIL off.
>> + *
>>   * Call convention:
>>   *
>>   * syscall register convention is in Documentation/powerpc/syscall64-abi.rst
>> diff --git a/arch/powerpc/kernel/setup_64.c b/arch/powerpc/kernel/setup_64.c
>> index be8577ac9397..ac52c69a3811 100644
>> --- a/arch/powerpc/kernel/setup_64.c
>> +++ b/arch/powerpc/kernel/setup_64.c
>> @@ -197,6 +197,21 @@ static void __init configure_exceptions(void)
>>
>>  	/* Under a PAPR hypervisor, we need hypercalls */
>>  	if (firmware_has_feature(FW_FEATURE_SET_MODE)) {
>> +		/*
>> +		 * PR KVM does not support AIL mode interrupts in the host, and
>> +		 * SCV system call interrupt vectors are only implemented for
>> +		 * AIL mode. Under pseries, AIL mode can only be enabled and
>> +		 * disabled system-wide so when PR KVM is loaded, all CPUs in
>> +		 * the host are set to AIL=0 mode. SCV can not be disabled
>> +		 * dynamically because the feature is advertised to host
>> +		 * userspace, so SCV support must not be enabled if PR KVM can
>> +		 * possibly be run.
>> +		 */
>> +		if (IS_ENABLED(CONFIG_KVM_BOOK3S_PR_POSSIBLE) && !radix_enabled()) {
>> +			init_task.thread.fscr &= ~FSCR_SCV;
>> +			cur_cpu_spec->cpu_user_features2 &= ~PPC_FEATURE2_SCV;
>> +		}
>> +
> 
> "Under pseries, AIL mode can only be enabled and disabled system-wide so
>  when PR KVM is loaded, all CPUs in the host are set to AIL=0 mode."
> 
> Loaded as in 'modprobe kvm_pr'?

In kvmppc_core_init_vm_pr(), so while there is a PR guest running in the 
system.

> And host as in "nested host"
> surely. Unless I completely misunderstood the patch (likely).

Yes the PR KVM host. I didn't want to say nested because it runs in 
supervisor mode so is basically no difference whether under a HV or
not so I'm not sure if that's the right term for PR KVM or could
confused with nested HV.

I will see if I can make it a bit clearer.

> Is there a way to make this less unexpected to users? Maybe a few words
> in the Kconfig entry for PR_POSSIBLE saying "if you enable this and run
> a Hash MMU guest, you lose SCV"?

That's not a bad idea, also if you run a PR guest under it you lose
AIL in the host which also slows down interrupts and system calls.

Thanks,
Nick

> 
>>  		/* Enable AIL if possible */
>>  		if (!pseries_enable_reloc_on_exc()) {
>>  			init_task.thread.fscr &= ~FSCR_SCV;
>> diff --git a/arch/powerpc/kvm/book3s_pr.c b/arch/powerpc/kvm/book3s_pr.c
>> index 34a801c3604a..4d1c84b94b77 100644
>> --- a/arch/powerpc/kvm/book3s_pr.c
>> +++ b/arch/powerpc/kvm/book3s_pr.c
>> @@ -140,9 +140,12 @@ static void kvmppc_core_vcpu_load_pr(struct kvm_vcpu *vcpu, int cpu)
>>  #endif
>>
>>  	/* Disable AIL if supported */
>> -	if (cpu_has_feature(CPU_FTR_HVMODE) &&
>> -	    cpu_has_feature(CPU_FTR_ARCH_207S))
>> -		mtspr(SPRN_LPCR, mfspr(SPRN_LPCR) & ~LPCR_AIL);
>> +	if (cpu_has_feature(CPU_FTR_HVMODE)) {
>> +		if (cpu_has_feature(CPU_FTR_ARCH_207S))
>> +			mtspr(SPRN_LPCR, mfspr(SPRN_LPCR) & ~LPCR_AIL);
>> +		if (cpu_has_feature(CPU_FTR_ARCH_300) && (current->thread.fscr & FSCR_SCV))
>> +			mtspr(SPRN_FSCR, mfspr(SPRN_FSCR) & ~FSCR_SCV);
>> +	}
>>
>>  	vcpu->cpu = smp_processor_id();
>>  #ifdef CONFIG_PPC_BOOK3S_32
>> @@ -175,9 +178,12 @@ static void kvmppc_core_vcpu_put_pr(struct kvm_vcpu *vcpu)
>>  	kvmppc_save_tm_pr(vcpu);
>>
>>  	/* Enable AIL if supported */
>> -	if (cpu_has_feature(CPU_FTR_HVMODE) &&
>> -	    cpu_has_feature(CPU_FTR_ARCH_207S))
>> -		mtspr(SPRN_LPCR, mfspr(SPRN_LPCR) | LPCR_AIL_3);
>> +	if (cpu_has_feature(CPU_FTR_HVMODE)) {
>> +		if (cpu_has_feature(CPU_FTR_ARCH_207S))
>> +			mtspr(SPRN_LPCR, mfspr(SPRN_LPCR) | LPCR_AIL_3);
>> +		if (cpu_has_feature(CPU_FTR_ARCH_300) && (current->thread.fscr & FSCR_SCV))
>> +			mtspr(SPRN_FSCR, mfspr(SPRN_FSCR) | FSCR_SCV);
>> +	}
>>
>>  	vcpu->cpu = -1;
>>  }
>> @@ -1037,6 +1043,8 @@ static int kvmppc_handle_fac(struct kvm_vcpu *vcpu, ulong fac)
>>
>>  void kvmppc_set_fscr(struct kvm_vcpu *vcpu, u64 fscr)
>>  {
>> +	if (fscr & FSCR_SCV)
>> +		fscr &= ~FSCR_SCV; /* SCV must not be enabled */
>>  	if ((vcpu->arch.fscr & FSCR_TAR) && !(fscr & FSCR_TAR)) {
>>  		/* TAR got dropped, drop it in shadow too */
>>  		kvmppc_giveup_fac(vcpu, FSCR_TAR_LG);
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2022-01-25  3:56 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-24 10:24 [PATCH 0/2] KVM: PPC: Book3S PR: SCV fixes Nicholas Piggin
2022-01-24 10:24 ` [PATCH 1/2] KVM: PPC: Book3S PR: Disable SCV when running AIL is disabled Nicholas Piggin
2022-01-24 22:49   ` Fabiano Rosas
2022-01-25  3:55     ` Nicholas Piggin
2022-01-24 10:24 ` [PATCH 2/2] KVM: PPC: Book3S PR: Disallow AIL != 0 Nicholas Piggin
2022-01-24 22:50   ` Fabiano Rosas

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.