* [PATCH v2] KVM: PPC: Book3S HV: relax check on H_SVM_INIT_ABORT
@ 2020-05-20 17:43 ` Laurent Dufour
0 siblings, 0 replies; 24+ messages in thread
From: Laurent Dufour @ 2020-05-20 17:43 UTC (permalink / raw)
To: kvm-ppc, linuxppc-dev, linux-kernel, paulus; +Cc: groug, mpe, sukadev, linuxram
The commit 8c47b6ff29e3 ("KVM: PPC: Book3S HV: Check caller of H_SVM_*
Hcalls") added checks of secure bit of SRR1 to filter out the Hcall
reserved to the Ultravisor.
However, the Hcall H_SVM_INIT_ABORT is made by the Ultravisor passing the
context of the VM calling UV_ESM. This allows the Hypervisor to return to
the guest without going through the Ultravisor. Thus the Secure bit of SRR1
is not set in that particular case.
In the case a regular VM is calling H_SVM_INIT_ABORT, this hcall will be
filtered out in kvmppc_h_svm_init_abort() because kvm->arch.secure_guest is
not set in that case.
Fixes: 8c47b6ff29e3 ("KVM: PPC: Book3S HV: Check caller of H_SVM_* Hcalls")
Signed-off-by: Laurent Dufour <ldufour@linux.ibm.com>
---
arch/powerpc/kvm/book3s_hv.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index 93493f0cbfe8..6ad1a3b14300 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -1099,9 +1099,12 @@ int kvmppc_pseries_do_hcall(struct kvm_vcpu *vcpu)
ret = kvmppc_h_svm_init_done(vcpu->kvm);
break;
case H_SVM_INIT_ABORT:
- ret = H_UNSUPPORTED;
- if (kvmppc_get_srr1(vcpu) & MSR_S)
- ret = kvmppc_h_svm_init_abort(vcpu->kvm);
+ /*
+ * Even if that call is made by the Ultravisor, the SSR1 value
+ * is the guest context one, with the secure bit clear as it has
+ * not yet been secured. So we can't check it here.
+ */
+ ret = kvmppc_h_svm_init_abort(vcpu->kvm);
break;
default:
--
2.26.2
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH v2] KVM: PPC: Book3S HV: relax check on H_SVM_INIT_ABORT
@ 2020-05-20 17:43 ` Laurent Dufour
0 siblings, 0 replies; 24+ messages in thread
From: Laurent Dufour @ 2020-05-20 17:43 UTC (permalink / raw)
To: kvm-ppc, linuxppc-dev, linux-kernel, paulus; +Cc: sukadev, linuxram, groug
The commit 8c47b6ff29e3 ("KVM: PPC: Book3S HV: Check caller of H_SVM_*
Hcalls") added checks of secure bit of SRR1 to filter out the Hcall
reserved to the Ultravisor.
However, the Hcall H_SVM_INIT_ABORT is made by the Ultravisor passing the
context of the VM calling UV_ESM. This allows the Hypervisor to return to
the guest without going through the Ultravisor. Thus the Secure bit of SRR1
is not set in that particular case.
In the case a regular VM is calling H_SVM_INIT_ABORT, this hcall will be
filtered out in kvmppc_h_svm_init_abort() because kvm->arch.secure_guest is
not set in that case.
Fixes: 8c47b6ff29e3 ("KVM: PPC: Book3S HV: Check caller of H_SVM_* Hcalls")
Signed-off-by: Laurent Dufour <ldufour@linux.ibm.com>
---
arch/powerpc/kvm/book3s_hv.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index 93493f0cbfe8..6ad1a3b14300 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -1099,9 +1099,12 @@ int kvmppc_pseries_do_hcall(struct kvm_vcpu *vcpu)
ret = kvmppc_h_svm_init_done(vcpu->kvm);
break;
case H_SVM_INIT_ABORT:
- ret = H_UNSUPPORTED;
- if (kvmppc_get_srr1(vcpu) & MSR_S)
- ret = kvmppc_h_svm_init_abort(vcpu->kvm);
+ /*
+ * Even if that call is made by the Ultravisor, the SSR1 value
+ * is the guest context one, with the secure bit clear as it has
+ * not yet been secured. So we can't check it here.
+ */
+ ret = kvmppc_h_svm_init_abort(vcpu->kvm);
break;
default:
--
2.26.2
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH v2] KVM: PPC: Book3S HV: relax check on H_SVM_INIT_ABORT
2020-05-20 17:43 ` Laurent Dufour
(?)
@ 2020-05-20 18:23 ` Greg Kurz
-1 siblings, 0 replies; 24+ messages in thread
From: Greg Kurz @ 2020-05-20 18:23 UTC (permalink / raw)
To: Laurent Dufour
Cc: kvm-ppc, linuxppc-dev, linux-kernel, paulus, mpe, sukadev, linuxram
On Wed, 20 May 2020 19:43:08 +0200
Laurent Dufour <ldufour@linux.ibm.com> wrote:
> The commit 8c47b6ff29e3 ("KVM: PPC: Book3S HV: Check caller of H_SVM_*
> Hcalls") added checks of secure bit of SRR1 to filter out the Hcall
> reserved to the Ultravisor.
>
> However, the Hcall H_SVM_INIT_ABORT is made by the Ultravisor passing the
> context of the VM calling UV_ESM. This allows the Hypervisor to return to
> the guest without going through the Ultravisor. Thus the Secure bit of SRR1
> is not set in that particular case.
>
> In the case a regular VM is calling H_SVM_INIT_ABORT, this hcall will be
> filtered out in kvmppc_h_svm_init_abort() because kvm->arch.secure_guest is
> not set in that case.
>
> Fixes: 8c47b6ff29e3 ("KVM: PPC: Book3S HV: Check caller of H_SVM_* Hcalls")
> Signed-off-by: Laurent Dufour <ldufour@linux.ibm.com>
> ---
Reviewed-by: Greg Kurz <groug@kaod.org>
> arch/powerpc/kvm/book3s_hv.c | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
> index 93493f0cbfe8..6ad1a3b14300 100644
> --- a/arch/powerpc/kvm/book3s_hv.c
> +++ b/arch/powerpc/kvm/book3s_hv.c
> @@ -1099,9 +1099,12 @@ int kvmppc_pseries_do_hcall(struct kvm_vcpu *vcpu)
> ret = kvmppc_h_svm_init_done(vcpu->kvm);
> break;
> case H_SVM_INIT_ABORT:
> - ret = H_UNSUPPORTED;
> - if (kvmppc_get_srr1(vcpu) & MSR_S)
> - ret = kvmppc_h_svm_init_abort(vcpu->kvm);
> + /*
> + * Even if that call is made by the Ultravisor, the SSR1 value
> + * is the guest context one, with the secure bit clear as it has
> + * not yet been secured. So we can't check it here.
> + */
> + ret = kvmppc_h_svm_init_abort(vcpu->kvm);
> break;
>
> default:
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH v2] KVM: PPC: Book3S HV: relax check on H_SVM_INIT_ABORT
@ 2020-05-20 18:23 ` Greg Kurz
0 siblings, 0 replies; 24+ messages in thread
From: Greg Kurz @ 2020-05-20 18:23 UTC (permalink / raw)
To: Laurent Dufour
Cc: kvm-ppc, linuxppc-dev, linux-kernel, paulus, mpe, sukadev, linuxram
On Wed, 20 May 2020 19:43:08 +0200
Laurent Dufour <ldufour@linux.ibm.com> wrote:
> The commit 8c47b6ff29e3 ("KVM: PPC: Book3S HV: Check caller of H_SVM_*
> Hcalls") added checks of secure bit of SRR1 to filter out the Hcall
> reserved to the Ultravisor.
>
> However, the Hcall H_SVM_INIT_ABORT is made by the Ultravisor passing the
> context of the VM calling UV_ESM. This allows the Hypervisor to return to
> the guest without going through the Ultravisor. Thus the Secure bit of SRR1
> is not set in that particular case.
>
> In the case a regular VM is calling H_SVM_INIT_ABORT, this hcall will be
> filtered out in kvmppc_h_svm_init_abort() because kvm->arch.secure_guest is
> not set in that case.
>
> Fixes: 8c47b6ff29e3 ("KVM: PPC: Book3S HV: Check caller of H_SVM_* Hcalls")
> Signed-off-by: Laurent Dufour <ldufour@linux.ibm.com>
> ---
Reviewed-by: Greg Kurz <groug@kaod.org>
> arch/powerpc/kvm/book3s_hv.c | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
> index 93493f0cbfe8..6ad1a3b14300 100644
> --- a/arch/powerpc/kvm/book3s_hv.c
> +++ b/arch/powerpc/kvm/book3s_hv.c
> @@ -1099,9 +1099,12 @@ int kvmppc_pseries_do_hcall(struct kvm_vcpu *vcpu)
> ret = kvmppc_h_svm_init_done(vcpu->kvm);
> break;
> case H_SVM_INIT_ABORT:
> - ret = H_UNSUPPORTED;
> - if (kvmppc_get_srr1(vcpu) & MSR_S)
> - ret = kvmppc_h_svm_init_abort(vcpu->kvm);
> + /*
> + * Even if that call is made by the Ultravisor, the SSR1 value
> + * is the guest context one, with the secure bit clear as it has
> + * not yet been secured. So we can't check it here.
> + */
> + ret = kvmppc_h_svm_init_abort(vcpu->kvm);
> break;
>
> default:
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH v2] KVM: PPC: Book3S HV: relax check on H_SVM_INIT_ABORT
@ 2020-05-20 18:23 ` Greg Kurz
0 siblings, 0 replies; 24+ messages in thread
From: Greg Kurz @ 2020-05-20 18:23 UTC (permalink / raw)
To: Laurent Dufour
Cc: linuxram, linux-kernel, kvm-ppc, paulus, sukadev, linuxppc-dev
On Wed, 20 May 2020 19:43:08 +0200
Laurent Dufour <ldufour@linux.ibm.com> wrote:
> The commit 8c47b6ff29e3 ("KVM: PPC: Book3S HV: Check caller of H_SVM_*
> Hcalls") added checks of secure bit of SRR1 to filter out the Hcall
> reserved to the Ultravisor.
>
> However, the Hcall H_SVM_INIT_ABORT is made by the Ultravisor passing the
> context of the VM calling UV_ESM. This allows the Hypervisor to return to
> the guest without going through the Ultravisor. Thus the Secure bit of SRR1
> is not set in that particular case.
>
> In the case a regular VM is calling H_SVM_INIT_ABORT, this hcall will be
> filtered out in kvmppc_h_svm_init_abort() because kvm->arch.secure_guest is
> not set in that case.
>
> Fixes: 8c47b6ff29e3 ("KVM: PPC: Book3S HV: Check caller of H_SVM_* Hcalls")
> Signed-off-by: Laurent Dufour <ldufour@linux.ibm.com>
> ---
Reviewed-by: Greg Kurz <groug@kaod.org>
> arch/powerpc/kvm/book3s_hv.c | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
> index 93493f0cbfe8..6ad1a3b14300 100644
> --- a/arch/powerpc/kvm/book3s_hv.c
> +++ b/arch/powerpc/kvm/book3s_hv.c
> @@ -1099,9 +1099,12 @@ int kvmppc_pseries_do_hcall(struct kvm_vcpu *vcpu)
> ret = kvmppc_h_svm_init_done(vcpu->kvm);
> break;
> case H_SVM_INIT_ABORT:
> - ret = H_UNSUPPORTED;
> - if (kvmppc_get_srr1(vcpu) & MSR_S)
> - ret = kvmppc_h_svm_init_abort(vcpu->kvm);
> + /*
> + * Even if that call is made by the Ultravisor, the SSR1 value
> + * is the guest context one, with the secure bit clear as it has
> + * not yet been secured. So we can't check it here.
> + */
> + ret = kvmppc_h_svm_init_abort(vcpu->kvm);
> break;
>
> default:
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2] KVM: PPC: Book3S HV: relax check on H_SVM_INIT_ABORT
2020-05-20 17:43 ` Laurent Dufour
(?)
@ 2020-05-21 6:08 ` Ram Pai
-1 siblings, 0 replies; 24+ messages in thread
From: Ram Pai @ 2020-05-21 6:08 UTC (permalink / raw)
To: Laurent Dufour
Cc: kvm-ppc, linuxppc-dev, linux-kernel, paulus, groug, mpe, sukadev
On Wed, May 20, 2020 at 07:43:08PM +0200, Laurent Dufour wrote:
> The commit 8c47b6ff29e3 ("KVM: PPC: Book3S HV: Check caller of H_SVM_*
> Hcalls") added checks of secure bit of SRR1 to filter out the Hcall
> reserved to the Ultravisor.
>
> However, the Hcall H_SVM_INIT_ABORT is made by the Ultravisor passing the
> context of the VM calling UV_ESM. This allows the Hypervisor to return to
> the guest without going through the Ultravisor. Thus the Secure bit of SRR1
> is not set in that particular case.
>
> In the case a regular VM is calling H_SVM_INIT_ABORT, this hcall will be
> filtered out in kvmppc_h_svm_init_abort() because kvm->arch.secure_guest is
> not set in that case.
>
> Fixes: 8c47b6ff29e3 ("KVM: PPC: Book3S HV: Check caller of H_SVM_* Hcalls")
> Signed-off-by: Laurent Dufour <ldufour@linux.ibm.com>
Reviewed-by: Ram Pai <linuxram@us.ibm.com>
> ---
> arch/powerpc/kvm/book3s_hv.c | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
> index 93493f0cbfe8..6ad1a3b14300 100644
> --- a/arch/powerpc/kvm/book3s_hv.c
> +++ b/arch/powerpc/kvm/book3s_hv.c
> @@ -1099,9 +1099,12 @@ int kvmppc_pseries_do_hcall(struct kvm_vcpu *vcpu)
> ret = kvmppc_h_svm_init_done(vcpu->kvm);
> break;
> case H_SVM_INIT_ABORT:
> - ret = H_UNSUPPORTED;
> - if (kvmppc_get_srr1(vcpu) & MSR_S)
> - ret = kvmppc_h_svm_init_abort(vcpu->kvm);
> + /*
> + * Even if that call is made by the Ultravisor, the SSR1 value
> + * is the guest context one, with the secure bit clear as it has
> + * not yet been secured. So we can't check it here.
> + */
Frankly speaking, the comment above when read in isolation; i.e without
the delete code above, feels out of place. The reasoning for change is
anyway captured in the changelog. So, I think, we should delete this
comment.
Also the comment above assumes the Ultravisor will call H_SVM_INIT_ABORT
with SRR1(S) bit not set; which may or may not be true. Regardless of
who and how H_SVM_INIT_ABORT is called, we should just call
kvmppc_h_svm_init_abort() and let it deal with the complexities.
RP
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH v2] KVM: PPC: Book3S HV: relax check on H_SVM_INIT_ABORT
@ 2020-05-21 6:08 ` Ram Pai
0 siblings, 0 replies; 24+ messages in thread
From: Ram Pai @ 2020-05-21 6:08 UTC (permalink / raw)
To: Laurent Dufour
Cc: kvm-ppc, linuxppc-dev, linux-kernel, paulus, groug, mpe, sukadev
On Wed, May 20, 2020 at 07:43:08PM +0200, Laurent Dufour wrote:
> The commit 8c47b6ff29e3 ("KVM: PPC: Book3S HV: Check caller of H_SVM_*
> Hcalls") added checks of secure bit of SRR1 to filter out the Hcall
> reserved to the Ultravisor.
>
> However, the Hcall H_SVM_INIT_ABORT is made by the Ultravisor passing the
> context of the VM calling UV_ESM. This allows the Hypervisor to return to
> the guest without going through the Ultravisor. Thus the Secure bit of SRR1
> is not set in that particular case.
>
> In the case a regular VM is calling H_SVM_INIT_ABORT, this hcall will be
> filtered out in kvmppc_h_svm_init_abort() because kvm->arch.secure_guest is
> not set in that case.
>
> Fixes: 8c47b6ff29e3 ("KVM: PPC: Book3S HV: Check caller of H_SVM_* Hcalls")
> Signed-off-by: Laurent Dufour <ldufour@linux.ibm.com>
Reviewed-by: Ram Pai <linuxram@us.ibm.com>
> ---
> arch/powerpc/kvm/book3s_hv.c | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
> index 93493f0cbfe8..6ad1a3b14300 100644
> --- a/arch/powerpc/kvm/book3s_hv.c
> +++ b/arch/powerpc/kvm/book3s_hv.c
> @@ -1099,9 +1099,12 @@ int kvmppc_pseries_do_hcall(struct kvm_vcpu *vcpu)
> ret = kvmppc_h_svm_init_done(vcpu->kvm);
> break;
> case H_SVM_INIT_ABORT:
> - ret = H_UNSUPPORTED;
> - if (kvmppc_get_srr1(vcpu) & MSR_S)
> - ret = kvmppc_h_svm_init_abort(vcpu->kvm);
> + /*
> + * Even if that call is made by the Ultravisor, the SSR1 value
> + * is the guest context one, with the secure bit clear as it has
> + * not yet been secured. So we can't check it here.
> + */
Frankly speaking, the comment above when read in isolation; i.e without
the delete code above, feels out of place. The reasoning for change is
anyway captured in the changelog. So, I think, we should delete this
comment.
Also the comment above assumes the Ultravisor will call H_SVM_INIT_ABORT
with SRR1(S) bit not set; which may or may not be true. Regardless of
who and how H_SVM_INIT_ABORT is called, we should just call
kvmppc_h_svm_init_abort() and let it deal with the complexities.
RP
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH v2] KVM: PPC: Book3S HV: relax check on H_SVM_INIT_ABORT
@ 2020-05-21 6:08 ` Ram Pai
0 siblings, 0 replies; 24+ messages in thread
From: Ram Pai @ 2020-05-21 6:08 UTC (permalink / raw)
To: Laurent Dufour
Cc: linux-kernel, kvm-ppc, groug, paulus, sukadev, linuxppc-dev
On Wed, May 20, 2020 at 07:43:08PM +0200, Laurent Dufour wrote:
> The commit 8c47b6ff29e3 ("KVM: PPC: Book3S HV: Check caller of H_SVM_*
> Hcalls") added checks of secure bit of SRR1 to filter out the Hcall
> reserved to the Ultravisor.
>
> However, the Hcall H_SVM_INIT_ABORT is made by the Ultravisor passing the
> context of the VM calling UV_ESM. This allows the Hypervisor to return to
> the guest without going through the Ultravisor. Thus the Secure bit of SRR1
> is not set in that particular case.
>
> In the case a regular VM is calling H_SVM_INIT_ABORT, this hcall will be
> filtered out in kvmppc_h_svm_init_abort() because kvm->arch.secure_guest is
> not set in that case.
>
> Fixes: 8c47b6ff29e3 ("KVM: PPC: Book3S HV: Check caller of H_SVM_* Hcalls")
> Signed-off-by: Laurent Dufour <ldufour@linux.ibm.com>
Reviewed-by: Ram Pai <linuxram@us.ibm.com>
> ---
> arch/powerpc/kvm/book3s_hv.c | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
> index 93493f0cbfe8..6ad1a3b14300 100644
> --- a/arch/powerpc/kvm/book3s_hv.c
> +++ b/arch/powerpc/kvm/book3s_hv.c
> @@ -1099,9 +1099,12 @@ int kvmppc_pseries_do_hcall(struct kvm_vcpu *vcpu)
> ret = kvmppc_h_svm_init_done(vcpu->kvm);
> break;
> case H_SVM_INIT_ABORT:
> - ret = H_UNSUPPORTED;
> - if (kvmppc_get_srr1(vcpu) & MSR_S)
> - ret = kvmppc_h_svm_init_abort(vcpu->kvm);
> + /*
> + * Even if that call is made by the Ultravisor, the SSR1 value
> + * is the guest context one, with the secure bit clear as it has
> + * not yet been secured. So we can't check it here.
> + */
Frankly speaking, the comment above when read in isolation; i.e without
the delete code above, feels out of place. The reasoning for change is
anyway captured in the changelog. So, I think, we should delete this
comment.
Also the comment above assumes the Ultravisor will call H_SVM_INIT_ABORT
with SRR1(S) bit not set; which may or may not be true. Regardless of
who and how H_SVM_INIT_ABORT is called, we should just call
kvmppc_h_svm_init_abort() and let it deal with the complexities.
RP
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2] KVM: PPC: Book3S HV: relax check on H_SVM_INIT_ABORT
2020-05-20 17:43 ` Laurent Dufour
(?)
@ 2020-05-27 4:16 ` Paul Mackerras
-1 siblings, 0 replies; 24+ messages in thread
From: Paul Mackerras @ 2020-05-27 4:16 UTC (permalink / raw)
To: Laurent Dufour
Cc: kvm-ppc, linuxppc-dev, linux-kernel, groug, mpe, sukadev, linuxram
On Wed, May 20, 2020 at 07:43:08PM +0200, Laurent Dufour wrote:
> The commit 8c47b6ff29e3 ("KVM: PPC: Book3S HV: Check caller of H_SVM_*
> Hcalls") added checks of secure bit of SRR1 to filter out the Hcall
> reserved to the Ultravisor.
>
> However, the Hcall H_SVM_INIT_ABORT is made by the Ultravisor passing the
> context of the VM calling UV_ESM. This allows the Hypervisor to return to
> the guest without going through the Ultravisor. Thus the Secure bit of SRR1
> is not set in that particular case.
>
> In the case a regular VM is calling H_SVM_INIT_ABORT, this hcall will be
> filtered out in kvmppc_h_svm_init_abort() because kvm->arch.secure_guest is
> not set in that case.
>
> Fixes: 8c47b6ff29e3 ("KVM: PPC: Book3S HV: Check caller of H_SVM_* Hcalls")
> Signed-off-by: Laurent Dufour <ldufour@linux.ibm.com>
Thanks, applied to my kvm-ppc-next branch. I expanded the comment in
the code a little.
Paul.
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH v2] KVM: PPC: Book3S HV: relax check on H_SVM_INIT_ABORT
@ 2020-05-27 4:16 ` Paul Mackerras
0 siblings, 0 replies; 24+ messages in thread
From: Paul Mackerras @ 2020-05-27 4:16 UTC (permalink / raw)
To: Laurent Dufour
Cc: kvm-ppc, linuxppc-dev, linux-kernel, groug, mpe, sukadev, linuxram
On Wed, May 20, 2020 at 07:43:08PM +0200, Laurent Dufour wrote:
> The commit 8c47b6ff29e3 ("KVM: PPC: Book3S HV: Check caller of H_SVM_*
> Hcalls") added checks of secure bit of SRR1 to filter out the Hcall
> reserved to the Ultravisor.
>
> However, the Hcall H_SVM_INIT_ABORT is made by the Ultravisor passing the
> context of the VM calling UV_ESM. This allows the Hypervisor to return to
> the guest without going through the Ultravisor. Thus the Secure bit of SRR1
> is not set in that particular case.
>
> In the case a regular VM is calling H_SVM_INIT_ABORT, this hcall will be
> filtered out in kvmppc_h_svm_init_abort() because kvm->arch.secure_guest is
> not set in that case.
>
> Fixes: 8c47b6ff29e3 ("KVM: PPC: Book3S HV: Check caller of H_SVM_* Hcalls")
> Signed-off-by: Laurent Dufour <ldufour@linux.ibm.com>
Thanks, applied to my kvm-ppc-next branch. I expanded the comment in
the code a little.
Paul.
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH v2] KVM: PPC: Book3S HV: relax check on H_SVM_INIT_ABORT
@ 2020-05-27 4:16 ` Paul Mackerras
0 siblings, 0 replies; 24+ messages in thread
From: Paul Mackerras @ 2020-05-27 4:16 UTC (permalink / raw)
To: Laurent Dufour
Cc: linuxram, linux-kernel, kvm-ppc, groug, sukadev, linuxppc-dev
On Wed, May 20, 2020 at 07:43:08PM +0200, Laurent Dufour wrote:
> The commit 8c47b6ff29e3 ("KVM: PPC: Book3S HV: Check caller of H_SVM_*
> Hcalls") added checks of secure bit of SRR1 to filter out the Hcall
> reserved to the Ultravisor.
>
> However, the Hcall H_SVM_INIT_ABORT is made by the Ultravisor passing the
> context of the VM calling UV_ESM. This allows the Hypervisor to return to
> the guest without going through the Ultravisor. Thus the Secure bit of SRR1
> is not set in that particular case.
>
> In the case a regular VM is calling H_SVM_INIT_ABORT, this hcall will be
> filtered out in kvmppc_h_svm_init_abort() because kvm->arch.secure_guest is
> not set in that case.
>
> Fixes: 8c47b6ff29e3 ("KVM: PPC: Book3S HV: Check caller of H_SVM_* Hcalls")
> Signed-off-by: Laurent Dufour <ldufour@linux.ibm.com>
Thanks, applied to my kvm-ppc-next branch. I expanded the comment in
the code a little.
Paul.
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH v2] KVM: PPC: Book3S HV: relax check on H_SVM_INIT_ABORT
2020-05-27 4:16 ` Paul Mackerras
(?)
@ 2020-05-27 9:23 ` Laurent Dufour
-1 siblings, 0 replies; 24+ messages in thread
From: Laurent Dufour @ 2020-05-27 9:23 UTC (permalink / raw)
To: Paul Mackerras
Cc: kvm-ppc, linuxppc-dev, linux-kernel, groug, mpe, sukadev, linuxram
Le 27/05/2020 à 06:16, Paul Mackerras a écrit :
> On Wed, May 20, 2020 at 07:43:08PM +0200, Laurent Dufour wrote:
>> The commit 8c47b6ff29e3 ("KVM: PPC: Book3S HV: Check caller of H_SVM_*
>> Hcalls") added checks of secure bit of SRR1 to filter out the Hcall
>> reserved to the Ultravisor.
>>
>> However, the Hcall H_SVM_INIT_ABORT is made by the Ultravisor passing the
>> context of the VM calling UV_ESM. This allows the Hypervisor to return to
>> the guest without going through the Ultravisor. Thus the Secure bit of SRR1
>> is not set in that particular case.
>>
>> In the case a regular VM is calling H_SVM_INIT_ABORT, this hcall will be
>> filtered out in kvmppc_h_svm_init_abort() because kvm->arch.secure_guest is
>> not set in that case.
>>
>> Fixes: 8c47b6ff29e3 ("KVM: PPC: Book3S HV: Check caller of H_SVM_* Hcalls")
>> Signed-off-by: Laurent Dufour <ldufour@linux.ibm.com>
>
> Thanks, applied to my kvm-ppc-next branch. I expanded the comment in
> the code a little.
Thanks, the comment is more explicit now.
Laurent.
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH v2] KVM: PPC: Book3S HV: relax check on H_SVM_INIT_ABORT
@ 2020-05-27 9:23 ` Laurent Dufour
0 siblings, 0 replies; 24+ messages in thread
From: Laurent Dufour @ 2020-05-27 9:23 UTC (permalink / raw)
To: Paul Mackerras
Cc: kvm-ppc, linuxppc-dev, linux-kernel, groug, mpe, sukadev, linuxram
Le 27/05/2020 à 06:16, Paul Mackerras a écrit :
> On Wed, May 20, 2020 at 07:43:08PM +0200, Laurent Dufour wrote:
>> The commit 8c47b6ff29e3 ("KVM: PPC: Book3S HV: Check caller of H_SVM_*
>> Hcalls") added checks of secure bit of SRR1 to filter out the Hcall
>> reserved to the Ultravisor.
>>
>> However, the Hcall H_SVM_INIT_ABORT is made by the Ultravisor passing the
>> context of the VM calling UV_ESM. This allows the Hypervisor to return to
>> the guest without going through the Ultravisor. Thus the Secure bit of SRR1
>> is not set in that particular case.
>>
>> In the case a regular VM is calling H_SVM_INIT_ABORT, this hcall will be
>> filtered out in kvmppc_h_svm_init_abort() because kvm->arch.secure_guest is
>> not set in that case.
>>
>> Fixes: 8c47b6ff29e3 ("KVM: PPC: Book3S HV: Check caller of H_SVM_* Hcalls")
>> Signed-off-by: Laurent Dufour <ldufour@linux.ibm.com>
>
> Thanks, applied to my kvm-ppc-next branch. I expanded the comment in
> the code a little.
Thanks, the comment is more explicit now.
Laurent.
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH v2] KVM: PPC: Book3S HV: relax check on H_SVM_INIT_ABORT
@ 2020-05-27 9:23 ` Laurent Dufour
0 siblings, 0 replies; 24+ messages in thread
From: Laurent Dufour @ 2020-05-27 9:23 UTC (permalink / raw)
To: Paul Mackerras
Cc: linuxram, linux-kernel, kvm-ppc, groug, sukadev, linuxppc-dev
Le 27/05/2020 à 06:16, Paul Mackerras a écrit :
> On Wed, May 20, 2020 at 07:43:08PM +0200, Laurent Dufour wrote:
>> The commit 8c47b6ff29e3 ("KVM: PPC: Book3S HV: Check caller of H_SVM_*
>> Hcalls") added checks of secure bit of SRR1 to filter out the Hcall
>> reserved to the Ultravisor.
>>
>> However, the Hcall H_SVM_INIT_ABORT is made by the Ultravisor passing the
>> context of the VM calling UV_ESM. This allows the Hypervisor to return to
>> the guest without going through the Ultravisor. Thus the Secure bit of SRR1
>> is not set in that particular case.
>>
>> In the case a regular VM is calling H_SVM_INIT_ABORT, this hcall will be
>> filtered out in kvmppc_h_svm_init_abort() because kvm->arch.secure_guest is
>> not set in that case.
>>
>> Fixes: 8c47b6ff29e3 ("KVM: PPC: Book3S HV: Check caller of H_SVM_* Hcalls")
>> Signed-off-by: Laurent Dufour <ldufour@linux.ibm.com>
>
> Thanks, applied to my kvm-ppc-next branch. I expanded the comment in
> the code a little.
Thanks, the comment is more explicit now.
Laurent.
^ permalink raw reply [flat|nested] 24+ messages in thread