[PATCH 0/3] AMD invpcid exception fix

[PATCH 0/3] AMD invpcid exception fix

[Bandan Das]

The pcid-disabled test from kvm-unit-tests fails on a Milan host because the
processor injects a #GP while the test expects #UD. While setting the intercept
when the guest has it disabled seemed like the obvious thing to do, Babu Moger (AMD)
pointed me to an earlier discussion here - https://lkml.org/lkml/2020/6/11/949

Jim points out there that  #GP has precedence over the intercept bit when invpcid is
called with CPL > 0 and so even if we intercept invpcid, the guest would end up with getting
and "incorrect" exception. To inject the right exception, I created an entry for the instruction
in the emulator to decode it successfully and then inject a UD instead of a GP when
the guest has it disabled.

Bandan Das (3):
  KVM: Add a stub for invpcid in the emulator table
  KVM: SVM: Handle invpcid during gp interception
  KVM: SVM:  check if we need to track GP intercept for invpcid

 arch/x86/kvm/emulate.c |  3 ++-
 arch/x86/kvm/svm/svm.c | 22 +++++++++++++++++++++-
 2 files changed, 23 insertions(+), 2 deletions(-)

-- 
2.24.1

[PATCH 1/3] KVM: Add a stub for invpcid in the emulator table

[Bandan Das]

Upon an exception, this can be used to successfully
decode the instruction and will be used by the next patch
to inject the correct exception.

Signed-off-by: Bandan Das <bsd@redhat.com>
---
 arch/x86/kvm/emulate.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index 72a1bd04dfe1..78b47fe60239 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -4956,7 +4956,8 @@ static const struct opcode opcode_map_0f_38[256] = {
 	/* 0x00 - 0x7f */
 	X16(N), X16(N), X16(N), X16(N), X16(N), X16(N), X16(N), X16(N),
 	/* 0x80 - 0xef */
-	X16(N), X16(N), X16(N), X16(N), X16(N), X16(N), X16(N),
+	N, N, D(SrcNone | Prot), N, X4(N), X8(N),
+	X16(N), X16(N), X16(N), X16(N), X16(N), X16(N),
 	/* 0xf0 - 0xf1 */
 	GP(EmulateOnUD | ModRM, &three_byte_0f_38_f0),
 	GP(EmulateOnUD | ModRM, &three_byte_0f_38_f1),
-- 
2.24.1

[PATCH 2/3] KVM: SVM: Handle invpcid during gp interception

[Bandan Das]

Use the gp interception path to inject a #UD
to the guest if the guest has invpcid disabled.
This is required because for CPL > 0, #GP takes
precedence over the INVPCID intercept.

Signed-off-by: Bandan Das <bsd@redhat.com>
---
 arch/x86/kvm/svm/svm.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index 754e07538b4a..0e8ce7adb815 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -2170,6 +2170,7 @@ enum {
 	SVM_INSTR_VMRUN,
 	SVM_INSTR_VMLOAD,
 	SVM_INSTR_VMSAVE,
+	SVM_INSTR_INVPCID,
 };
 
 /* Return NONE_SVM_INSTR if not SVM instrs, otherwise return decode result */
@@ -2177,6 +2178,8 @@ static int svm_instr_opcode(struct kvm_vcpu *vcpu)
 {
 	struct x86_emulate_ctxt *ctxt = vcpu->arch.emulate_ctxt;
 
+	if (ctxt->b == 0x82)
+		return SVM_INSTR_INVPCID;
 	if (ctxt->b != 0x1 || ctxt->opcode_len != 2)
 		return NONE_SVM_INSTR;
 
@@ -2200,11 +2203,13 @@ static int emulate_svm_instr(struct kvm_vcpu *vcpu, int opcode)
 		[SVM_INSTR_VMRUN] = SVM_EXIT_VMRUN,
 		[SVM_INSTR_VMLOAD] = SVM_EXIT_VMLOAD,
 		[SVM_INSTR_VMSAVE] = SVM_EXIT_VMSAVE,
+		[SVM_INSTR_INVPCID] = SVM_EXIT_EXCP_BASE + UD_VECTOR,
 	};
 	int (*const svm_instr_handlers[])(struct kvm_vcpu *vcpu) = {
 		[SVM_INSTR_VMRUN] = vmrun_interception,
 		[SVM_INSTR_VMLOAD] = vmload_interception,
 		[SVM_INSTR_VMSAVE] = vmsave_interception,
+		[SVM_INSTR_INVPCID] = ud_interception,
 	};
 	struct vcpu_svm *svm = to_svm(vcpu);
 
@@ -2253,8 +2258,12 @@ static int gp_interception(struct kvm_vcpu *vcpu)
 		if (!is_guest_mode(vcpu))
 			return kvm_emulate_instruction(vcpu,
 				EMULTYPE_VMWARE_GP | EMULTYPE_NO_DECODE);
-	} else
+	} else {
+		if ((opcode == SVM_INSTR_INVPCID) &&
+		    guest_cpuid_has(vcpu, X86_FEATURE_INVPCID))
+			goto reinject;
 		return emulate_svm_instr(vcpu, opcode);
+	}
 
 reinject:
 	kvm_queue_exception_e(vcpu, GP_VECTOR, error_code);
-- 
2.24.1

[PATCH 3/3] KVM: SVM: check if we need to track GP intercept for invpcid

[Bandan Das]

This is only set when the feature is available on the host
but the guest has disabled it, this will allow us to inject
the correct exception to the guest

Signed-off-by: Bandan Das <bsd@redhat.com>
---
 arch/x86/kvm/svm/svm.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index 0e8ce7adb815..2ecbf9bc929f 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -1102,6 +1102,7 @@ static u64 svm_write_l1_tsc_offset(struct kvm_vcpu *vcpu, u64 offset)
 
 static void svm_check_invpcid(struct vcpu_svm *svm)
 {
+	struct kvm_vcpu *vcpu = &svm->vcpu;
 	/*
 	 * Intercept INVPCID instruction only if shadow page table is
 	 * enabled. Interception is not required with nested page table
@@ -1112,6 +1113,16 @@ static void svm_check_invpcid(struct vcpu_svm *svm)
 			svm_set_intercept(svm, INTERCEPT_INVPCID);
 		else
 			svm_clr_intercept(svm, INTERCEPT_INVPCID);
+		/*
+		 * For CPL <> 0, #GP takes priority over intercepts.
+		 * This means that if invpcid is present but guest has disabled
+		 * it, it might end up getting a #GP instead of #UD
+		 * Let kvm inject the correct exception instead.
+		 */
+		if (!guest_cpuid_has(vcpu, X86_FEATURE_INVPCID))
+			set_exception_intercept(svm, GP_VECTOR);
+		else
+			clr_exception_intercept(svm, GP_VECTOR);
 	}
 }
 
-- 
2.24.1

Re: [PATCH 0/3] AMD invpcid exception fix

[Paolo Bonzini]

On 11/02/21 22:22, Bandan Das wrote:
> The pcid-disabled test from kvm-unit-tests fails on a Milan host because the
> processor injects a #GP while the test expects #UD. While setting the intercept
> when the guest has it disabled seemed like the obvious thing to do, Babu Moger (AMD)
> pointed me to an earlier discussion here - https://lkml.org/lkml/2020/6/11/949
> 
> Jim points out there that  #GP has precedence over the intercept bit when invpcid is
> called with CPL > 0 and so even if we intercept invpcid, the guest would end up with getting
> and "incorrect" exception. To inject the right exception, I created an entry for the instruction
> in the emulator to decode it successfully and then inject a UD instead of a GP when
> the guest has it disabled.
> 
> Bandan Das (3):
>    KVM: Add a stub for invpcid in the emulator table
>    KVM: SVM: Handle invpcid during gp interception
>    KVM: SVM:  check if we need to track GP intercept for invpcid
> 
>   arch/x86/kvm/emulate.c |  3 ++-
>   arch/x86/kvm/svm/svm.c | 22 +++++++++++++++++++++-
>   2 files changed, 23 insertions(+), 2 deletions(-)
> 

Isn't this the same thing that "[PATCH 1/3] KVM: SVM: Intercept INVPCID 
when it's disabled to inject #UD" also does?

Paolo

Re: [PATCH 0/3] AMD invpcid exception fix

[Bandan Das]

Paolo Bonzini <pbonzini@redhat.com> writes:

> On 11/02/21 22:22, Bandan Das wrote:
>> The pcid-disabled test from kvm-unit-tests fails on a Milan host because the
>> processor injects a #GP while the test expects #UD. While setting the intercept
>> when the guest has it disabled seemed like the obvious thing to do, Babu Moger (AMD)
>> pointed me to an earlier discussion here - https://lkml.org/lkml/2020/6/11/949
>>
>> Jim points out there that  #GP has precedence over the intercept bit when invpcid is
>> called with CPL > 0 and so even if we intercept invpcid, the guest would end up with getting
>> and "incorrect" exception. To inject the right exception, I created an entry for the instruction
>> in the emulator to decode it successfully and then inject a UD instead of a GP when
>> the guest has it disabled.
>>
>> Bandan Das (3):
>>    KVM: Add a stub for invpcid in the emulator table
>>    KVM: SVM: Handle invpcid during gp interception
>>    KVM: SVM:  check if we need to track GP intercept for invpcid
>>
>>   arch/x86/kvm/emulate.c |  3 ++-
>>   arch/x86/kvm/svm/svm.c | 22 +++++++++++++++++++++-
>>   2 files changed, 23 insertions(+), 2 deletions(-)
>>
>
> Isn't this the same thing that "[PATCH 1/3] KVM: SVM: Intercept
> INVPCID when it's disabled to inject #UD" also does?
>
Yeah, Babu pointed me to Sean's series after I posted mine.
1/3 indeed will fix the kvm-unit-test failure. IIUC, It doesn't look like it
handles the case for the guest executing invpcid at CPL > 0 when it's
disabled for the guest - #GP takes precedence over intercepts and will
be incorrectly injected instead of an #UD.

Bandan
> Paolo

Re: [PATCH 0/3] AMD invpcid exception fix

[Jim Mattson]

On Fri, Feb 12, 2021 at 6:49 AM Bandan Das <bsd@redhat.com> wrote:
>
> Paolo Bonzini <pbonzini@redhat.com> writes:
>
> > On 11/02/21 22:22, Bandan Das wrote:
> >> The pcid-disabled test from kvm-unit-tests fails on a Milan host because the
> >> processor injects a #GP while the test expects #UD. While setting the intercept
> >> when the guest has it disabled seemed like the obvious thing to do, Babu Moger (AMD)
> >> pointed me to an earlier discussion here - https://lkml.org/lkml/2020/6/11/949
> >>
> >> Jim points out there that  #GP has precedence over the intercept bit when invpcid is
> >> called with CPL > 0 and so even if we intercept invpcid, the guest would end up with getting
> >> and "incorrect" exception. To inject the right exception, I created an entry for the instruction
> >> in the emulator to decode it successfully and then inject a UD instead of a GP when
> >> the guest has it disabled.
> >>
> >> Bandan Das (3):
> >>    KVM: Add a stub for invpcid in the emulator table
> >>    KVM: SVM: Handle invpcid during gp interception
> >>    KVM: SVM:  check if we need to track GP intercept for invpcid
> >>
> >>   arch/x86/kvm/emulate.c |  3 ++-
> >>   arch/x86/kvm/svm/svm.c | 22 +++++++++++++++++++++-
> >>   2 files changed, 23 insertions(+), 2 deletions(-)
> >>
> >
> > Isn't this the same thing that "[PATCH 1/3] KVM: SVM: Intercept
> > INVPCID when it's disabled to inject #UD" also does?
> >
> Yeah, Babu pointed me to Sean's series after I posted mine.
> 1/3 indeed will fix the kvm-unit-test failure. IIUC, It doesn't look like it
> handles the case for the guest executing invpcid at CPL > 0 when it's
> disabled for the guest - #GP takes precedence over intercepts and will
> be incorrectly injected instead of an #UD.

I know I was the one to complain about the #GP, but...

As a general rule, kvm cannot always guarantee a #UD for an
instruction that is hidden from the guest. Consider, for example,
popcnt, aesenc, vzeroall, movbe, addcx, clwb, ...
I'm pretty sure that Paolo has brought this up in the past when I've
made similar complaints.

Re: [PATCH 0/3] AMD invpcid exception fix

[Bandan Das]

Jim Mattson <jmattson@google.com> writes:

> On Fri, Feb 12, 2021 at 6:49 AM Bandan Das <bsd@redhat.com> wrote:
>>
>> Paolo Bonzini <pbonzini@redhat.com> writes:
>>
>> > On 11/02/21 22:22, Bandan Das wrote:
>> >> The pcid-disabled test from kvm-unit-tests fails on a Milan host because the
>> >> processor injects a #GP while the test expects #UD. While setting the intercept
>> >> when the guest has it disabled seemed like the obvious thing to do, Babu Moger (AMD)
>> >> pointed me to an earlier discussion here - https://lkml.org/lkml/2020/6/11/949
>> >>
>> >> Jim points out there that  #GP has precedence over the intercept bit when invpcid is
>> >> called with CPL > 0 and so even if we intercept invpcid, the guest would end up with getting
>> >> and "incorrect" exception. To inject the right exception, I created an entry for the instruction
>> >> in the emulator to decode it successfully and then inject a UD instead of a GP when
>> >> the guest has it disabled.
>> >>
>> >> Bandan Das (3):
>> >>    KVM: Add a stub for invpcid in the emulator table
>> >>    KVM: SVM: Handle invpcid during gp interception
>> >>    KVM: SVM:  check if we need to track GP intercept for invpcid
>> >>
>> >>   arch/x86/kvm/emulate.c |  3 ++-
>> >>   arch/x86/kvm/svm/svm.c | 22 +++++++++++++++++++++-
>> >>   2 files changed, 23 insertions(+), 2 deletions(-)
>> >>
>> >
>> > Isn't this the same thing that "[PATCH 1/3] KVM: SVM: Intercept
>> > INVPCID when it's disabled to inject #UD" also does?
>> >
>> Yeah, Babu pointed me to Sean's series after I posted mine.
>> 1/3 indeed will fix the kvm-unit-test failure. IIUC, It doesn't look like it
>> handles the case for the guest executing invpcid at CPL > 0 when it's
>> disabled for the guest - #GP takes precedence over intercepts and will
>> be incorrectly injected instead of an #UD.
>
> I know I was the one to complain about the #GP, but...
>
> As a general rule, kvm cannot always guarantee a #UD for an
> instruction that is hidden from the guest. Consider, for example,
> popcnt, aesenc, vzeroall, movbe, addcx, clwb, ...
> I'm pretty sure that Paolo has brought this up in the past when I've
> made similar complaints.

Ofcourse, even for vm instructions failures, the fixup table always jumps
to a ud2. I was just trying to address the concern because it is possible
to inject the correct exception via decoding the instruction.

Bandan

Re: [PATCH 0/3] AMD invpcid exception fix

[Jim Mattson]

On Fri, Feb 12, 2021 at 9:55 AM Bandan Das <bsd@redhat.com> wrote:
>
> Jim Mattson <jmattson@google.com> writes:
>
> > On Fri, Feb 12, 2021 at 6:49 AM Bandan Das <bsd@redhat.com> wrote:
> >>
> >> Paolo Bonzini <pbonzini@redhat.com> writes:
> >>
> >> > On 11/02/21 22:22, Bandan Das wrote:
> >> >> The pcid-disabled test from kvm-unit-tests fails on a Milan host because the
> >> >> processor injects a #GP while the test expects #UD. While setting the intercept
> >> >> when the guest has it disabled seemed like the obvious thing to do, Babu Moger (AMD)
> >> >> pointed me to an earlier discussion here - https://lkml.org/lkml/2020/6/11/949
> >> >>
> >> >> Jim points out there that  #GP has precedence over the intercept bit when invpcid is
> >> >> called with CPL > 0 and so even if we intercept invpcid, the guest would end up with getting
> >> >> and "incorrect" exception. To inject the right exception, I created an entry for the instruction
> >> >> in the emulator to decode it successfully and then inject a UD instead of a GP when
> >> >> the guest has it disabled.
> >> >>
> >> >> Bandan Das (3):
> >> >>    KVM: Add a stub for invpcid in the emulator table
> >> >>    KVM: SVM: Handle invpcid during gp interception
> >> >>    KVM: SVM:  check if we need to track GP intercept for invpcid
> >> >>
> >> >>   arch/x86/kvm/emulate.c |  3 ++-
> >> >>   arch/x86/kvm/svm/svm.c | 22 +++++++++++++++++++++-
> >> >>   2 files changed, 23 insertions(+), 2 deletions(-)
> >> >>
> >> >
> >> > Isn't this the same thing that "[PATCH 1/3] KVM: SVM: Intercept
> >> > INVPCID when it's disabled to inject #UD" also does?
> >> >
> >> Yeah, Babu pointed me to Sean's series after I posted mine.
> >> 1/3 indeed will fix the kvm-unit-test failure. IIUC, It doesn't look like it
> >> handles the case for the guest executing invpcid at CPL > 0 when it's
> >> disabled for the guest - #GP takes precedence over intercepts and will
> >> be incorrectly injected instead of an #UD.
> >
> > I know I was the one to complain about the #GP, but...
> >
> > As a general rule, kvm cannot always guarantee a #UD for an
> > instruction that is hidden from the guest. Consider, for example,
> > popcnt, aesenc, vzeroall, movbe, addcx, clwb, ...
> > I'm pretty sure that Paolo has brought this up in the past when I've
> > made similar complaints.
>
> Ofcourse, even for vm instructions failures, the fixup table always jumps
> to a ud2. I was just trying to address the concern because it is possible
> to inject the correct exception via decoding the instruction.

But kvm doesn't intercept #GP, except when enable_vmware_backdoor is
set, does it? I don't think it's worth intercepting #GP just to get
this #UD right.

Re: [PATCH 0/3] AMD invpcid exception fix

[Bandan Das]

Jim Mattson <jmattson@google.com> writes:

> On Fri, Feb 12, 2021 at 9:55 AM Bandan Das <bsd@redhat.com> wrote:
>>
>> Jim Mattson <jmattson@google.com> writes:
>>
>> > On Fri, Feb 12, 2021 at 6:49 AM Bandan Das <bsd@redhat.com> wrote:
>> >>
>> >> Paolo Bonzini <pbonzini@redhat.com> writes:
>> >>
>> >> > On 11/02/21 22:22, Bandan Das wrote:
>> >> >> The pcid-disabled test from kvm-unit-tests fails on a Milan host because the
>> >> >> processor injects a #GP while the test expects #UD. While setting the intercept
>> >> >> when the guest has it disabled seemed like the obvious thing to do, Babu Moger (AMD)
>> >> >> pointed me to an earlier discussion here - https://lkml.org/lkml/2020/6/11/949
>> >> >>
>> >> >> Jim points out there that  #GP has precedence over the intercept bit when invpcid is
>> >> >> called with CPL > 0 and so even if we intercept invpcid, the guest would end up with getting
>> >> >> and "incorrect" exception. To inject the right exception, I created an entry for the instruction
>> >> >> in the emulator to decode it successfully and then inject a UD instead of a GP when
>> >> >> the guest has it disabled.
>> >> >>
>> >> >> Bandan Das (3):
>> >> >>    KVM: Add a stub for invpcid in the emulator table
>> >> >>    KVM: SVM: Handle invpcid during gp interception
>> >> >>    KVM: SVM:  check if we need to track GP intercept for invpcid
>> >> >>
>> >> >>   arch/x86/kvm/emulate.c |  3 ++-
>> >> >>   arch/x86/kvm/svm/svm.c | 22 +++++++++++++++++++++-
>> >> >>   2 files changed, 23 insertions(+), 2 deletions(-)
>> >> >>
>> >> >
>> >> > Isn't this the same thing that "[PATCH 1/3] KVM: SVM: Intercept
>> >> > INVPCID when it's disabled to inject #UD" also does?
>> >> >
>> >> Yeah, Babu pointed me to Sean's series after I posted mine.
>> >> 1/3 indeed will fix the kvm-unit-test failure. IIUC, It doesn't look like it
>> >> handles the case for the guest executing invpcid at CPL > 0 when it's
>> >> disabled for the guest - #GP takes precedence over intercepts and will
>> >> be incorrectly injected instead of an #UD.
>> >
>> > I know I was the one to complain about the #GP, but...
>> >
>> > As a general rule, kvm cannot always guarantee a #UD for an
>> > instruction that is hidden from the guest. Consider, for example,
>> > popcnt, aesenc, vzeroall, movbe, addcx, clwb, ...
>> > I'm pretty sure that Paolo has brought this up in the past when I've
>> > made similar complaints.
>>
>> Ofcourse, even for vm instructions failures, the fixup table always jumps
>> to a ud2. I was just trying to address the concern because it is possible
>> to inject the correct exception via decoding the instruction.
>
> But kvm doesn't intercept #GP, except when enable_vmware_backdoor is
> set, does it? I don't think it's worth intercepting #GP just to get
> this #UD right.

I prefer following the spec wherever we can.
Otoh, if kvm can't guarantee injecting the right exception,
we should change kvm-unit-tests to just check for exceptions and not a specific
exception that adheres to the spec. This one's fine though, as long as we don't add
a CPL > 0 invpcid test, the other patch that was posted fixes it.

Bandan

Re: [PATCH 0/3] AMD invpcid exception fix

[Jim Mattson]

On Fri, Feb 12, 2021 at 10:35 AM Bandan Das <bsd@redhat.com> wrote:
>
> Jim Mattson <jmattson@google.com> writes:
>
> > On Fri, Feb 12, 2021 at 9:55 AM Bandan Das <bsd@redhat.com> wrote:
> >>
> >> Jim Mattson <jmattson@google.com> writes:
> >>
> >> > On Fri, Feb 12, 2021 at 6:49 AM Bandan Das <bsd@redhat.com> wrote:
> >> >>
> >> >> Paolo Bonzini <pbonzini@redhat.com> writes:
> >> >>
> >> >> > On 11/02/21 22:22, Bandan Das wrote:
> >> >> >> The pcid-disabled test from kvm-unit-tests fails on a Milan host because the
> >> >> >> processor injects a #GP while the test expects #UD. While setting the intercept
> >> >> >> when the guest has it disabled seemed like the obvious thing to do, Babu Moger (AMD)
> >> >> >> pointed me to an earlier discussion here - https://lkml.org/lkml/2020/6/11/949
> >> >> >>
> >> >> >> Jim points out there that  #GP has precedence over the intercept bit when invpcid is
> >> >> >> called with CPL > 0 and so even if we intercept invpcid, the guest would end up with getting
> >> >> >> and "incorrect" exception. To inject the right exception, I created an entry for the instruction
> >> >> >> in the emulator to decode it successfully and then inject a UD instead of a GP when
> >> >> >> the guest has it disabled.
> >> >> >>
> >> >> >> Bandan Das (3):
> >> >> >>    KVM: Add a stub for invpcid in the emulator table
> >> >> >>    KVM: SVM: Handle invpcid during gp interception
> >> >> >>    KVM: SVM:  check if we need to track GP intercept for invpcid
> >> >> >>
> >> >> >>   arch/x86/kvm/emulate.c |  3 ++-
> >> >> >>   arch/x86/kvm/svm/svm.c | 22 +++++++++++++++++++++-
> >> >> >>   2 files changed, 23 insertions(+), 2 deletions(-)
> >> >> >>
> >> >> >
> >> >> > Isn't this the same thing that "[PATCH 1/3] KVM: SVM: Intercept
> >> >> > INVPCID when it's disabled to inject #UD" also does?
> >> >> >
> >> >> Yeah, Babu pointed me to Sean's series after I posted mine.
> >> >> 1/3 indeed will fix the kvm-unit-test failure. IIUC, It doesn't look like it
> >> >> handles the case for the guest executing invpcid at CPL > 0 when it's
> >> >> disabled for the guest - #GP takes precedence over intercepts and will
> >> >> be incorrectly injected instead of an #UD.
> >> >
> >> > I know I was the one to complain about the #GP, but...
> >> >
> >> > As a general rule, kvm cannot always guarantee a #UD for an
> >> > instruction that is hidden from the guest. Consider, for example,
> >> > popcnt, aesenc, vzeroall, movbe, addcx, clwb, ...
> >> > I'm pretty sure that Paolo has brought this up in the past when I've
> >> > made similar complaints.
> >>
> >> Ofcourse, even for vm instructions failures, the fixup table always jumps
> >> to a ud2. I was just trying to address the concern because it is possible
> >> to inject the correct exception via decoding the instruction.
> >
> > But kvm doesn't intercept #GP, except when enable_vmware_backdoor is
> > set, does it? I don't think it's worth intercepting #GP just to get
> > this #UD right.
>
> I prefer following the spec wherever we can.

One has to wonder why userspace is even trying to execute a privileged
instruction not enumerated by CPUID, unless it's just trying to expose
virtualization inconsistencies. Perhaps this could be controlled by a
new module parameter: "pedantic."

> Otoh, if kvm can't guarantee injecting the right exception,
> we should change kvm-unit-tests to just check for exceptions and not a specific
> exception that adheres to the spec. This one's fine though, as long as we don't add
> a CPL > 0 invpcid test, the other patch that was posted fixes it.

KVM *can* guarantee the correct exception, but it requires
intercepting all #GPs. That's probably not a big deal, but it is a
non-zero cost. Is it the right tradeoff to make?

Re: [PATCH 0/3] AMD invpcid exception fix

[Bandan Das]

Jim Mattson <jmattson@google.com> writes:
...
> On>> >> > I know I was the one to complain about the #GP, but...
>> >> >
>> >> > As a general rule, kvm cannot always guarantee a #UD for an
>> >> > instruction that is hidden from the guest. Consider, for example,
>> >> > popcnt, aesenc, vzeroall, movbe, addcx, clwb, ...
>> >> > I'm pretty sure that Paolo has brought this up in the past when I've
>> >> > made similar complaints.
>> >>
>> >> Ofcourse, even for vm instructions failures, the fixup table always jumps
>> >> to a ud2. I was just trying to address the concern because it is possible
>> >> to inject the correct exception via decoding the instruction.
>> >
>> > But kvm doesn't intercept #GP, except when enable_vmware_backdoor is
>> > set, does it? I don't think it's worth intercepting #GP just to get
>> > this #UD right.
>>
>> I prefer following the spec wherever we can.
>
> One has to wonder why userspace is even trying to execute a privileged
> instruction not enumerated by CPUID, unless it's just trying to expose
> virtualization inconsistencies. Perhaps this could be controlled by a
> new module parameter: "pedantic."
>
Yeah, fair point.

>> Otoh, if kvm can't guarantee injecting the right exception,
>> we should change kvm-unit-tests to just check for exceptions and not a specific
>> exception that adheres to the spec. This one's fine though, as long as we don't add
>> a CPL > 0 invpcid test, the other patch that was posted fixes it.
>
> KVM *can* guarantee the correct exception, but it requires
> intercepting all #GPs. That's probably not a big deal, but it is a
> non-zero cost. Is it the right tradeoff to make?

Not all, we intercept GPs only under a specific condition - just as we
do for vmware_backdoor and for the recent amd errata. IMO, I think it's the right
tradeoff to make to get guest exceptions right.

Bandan

Re: [PATCH 0/3] AMD invpcid exception fix

[Jim Mattson]

On Fri, Feb 12, 2021 at 12:10 PM Bandan Das <bsd@redhat.com> wrote:
>
> Jim Mattson <jmattson@google.com> writes:
> ...
> > On>> >> > I know I was the one to complain about the #GP, but...
> >> >> >
> >> >> > As a general rule, kvm cannot always guarantee a #UD for an
> >> >> > instruction that is hidden from the guest. Consider, for example,
> >> >> > popcnt, aesenc, vzeroall, movbe, addcx, clwb, ...
> >> >> > I'm pretty sure that Paolo has brought this up in the past when I've
> >> >> > made similar complaints.
> >> >>
> >> >> Ofcourse, even for vm instructions failures, the fixup table always jumps
> >> >> to a ud2. I was just trying to address the concern because it is possible
> >> >> to inject the correct exception via decoding the instruction.
> >> >
> >> > But kvm doesn't intercept #GP, except when enable_vmware_backdoor is
> >> > set, does it? I don't think it's worth intercepting #GP just to get
> >> > this #UD right.
> >>
> >> I prefer following the spec wherever we can.
> >
> > One has to wonder why userspace is even trying to execute a privileged
> > instruction not enumerated by CPUID, unless it's just trying to expose
> > virtualization inconsistencies. Perhaps this could be controlled by a
> > new module parameter: "pedantic."
> >
> Yeah, fair point.
>
> >> Otoh, if kvm can't guarantee injecting the right exception,
> >> we should change kvm-unit-tests to just check for exceptions and not a specific
> >> exception that adheres to the spec. This one's fine though, as long as we don't add
> >> a CPL > 0 invpcid test, the other patch that was posted fixes it.
> >
> > KVM *can* guarantee the correct exception, but it requires
> > intercepting all #GPs. That's probably not a big deal, but it is a
> > non-zero cost. Is it the right tradeoff to make?
>
> Not all, we intercept GPs only under a specific condition - just as we
> do for vmware_backdoor and for the recent amd errata. IMO, I think it's the right
> tradeoff to make to get guest exceptions right.

It sounds like I need to get you in my corner to help put a stop to
all of the incorrect #UDs that kvm is going to be raising in lieu of
#PF when narrow physical address width emulation is enabled!

Re: [PATCH 0/3] AMD invpcid exception fix

[Paolo Bonzini]

On 12/02/21 21:56, Jim Mattson wrote:
>> Not all, we intercept GPs only under a specific condition - just as we
>> do for vmware_backdoor and for the recent amd errata. IMO, I think it's the right
>> tradeoff to make to get guest exceptions right.
> It sounds like I need to get you in my corner to help put a stop to
> all of the incorrect #UDs that kvm is going to be raising in lieu of
> #PF when narrow physical address width emulation is enabled!

Ahah :)  Apart from the question of when you've entered diminishing 
returns, one important thing to consider is what the code looks like. 
This series is not especially pretty, and that's not your fault.  The 
whole idea of special decoding for #GP is a necessary evil for the 
address-check errata, but is it worth extending it to the corner case of 
INVPCID for CPL>0?

Paolo

Re: [PATCH 0/3] AMD invpcid exception fix

[Bandan Das]

Paolo Bonzini <pbonzini@redhat.com> writes:

> On 12/02/21 21:56, Jim Mattson wrote:
>>> Not all, we intercept GPs only under a specific condition - just as we
>>> do for vmware_backdoor and for the recent amd errata. IMO, I think it's the right
>>> tradeoff to make to get guest exceptions right.
>> It sounds like I need to get you in my corner to help put a stop to
>> all of the incorrect #UDs that kvm is going to be raising in lieu of
>> #PF when narrow physical address width emulation is enabled!
>
> Ahah :)  Apart from the question of when you've entered diminishing
> returns, one important thing to consider is what the code looks
> like. This series is not especially pretty, and that's not your fault.
> The whole idea of special decoding for #GP is a necessary evil for the
> address-check errata, but is it worth extending it to the corner case
> of INVPCID for CPL>0?
>
Sure, no worries. I will have fond memories of the time I spent extra time
on a trivial patch to address Jim's concerns(ofcourse valid!) only to find out
now he has changed his mind.

Bandan

> Paolo