Patch 01 reverts the C-bit truncation patch as the reserved #PF was
confirmed to be due to a magic HyperTransport region (how many magic
addresses are there!?!). Hopefully the original patch simply be dropped,
but just in case...
Patch 02 reverts the C-bit clearing in the #NPF handler. If that somehow
turns out to be incorrect, i.e. there are flows where the CPU doesn't
mask off the C-bit, then it can be conditional on a SEV guest.
I'll be offline for the next two weeks, fingers crossed I've undone all
the damage. :-)
Thanks!
Sean Christopherson (2):
Revert "KVM: x86: Truncate reported guest MAXPHYADDR to C-bit if SEV
is supported"
KVM: SVM: Revert clearing of C-bit on GPA in #NPF handler
arch/x86/kvm/cpuid.c | 11 -----------
arch/x86/kvm/svm/svm.c | 39 +++++++++------------------------------
arch/x86/kvm/x86.c | 3 ---
arch/x86/kvm/x86.h | 1 -
4 files changed, 9 insertions(+), 45 deletions(-)
--
2.32.0.93.g670b81a890-goog
Don't clear the C-bit in the #NPF handler, as it is a legal GPA bit for
non-SEV guests, and for SEV guests the C-bit is dropped before the GPA
hits the NPT in hardware. Clearing the bit for non-SEV guests causes KVM
to mishandle #NPFs with that collide with the host's C-bit.
Although the APM doesn't explicitly state that the C-bit is not reserved
for non-SEV, Tom Lendacky confirmed that the following snippet about the
effective reduction due to the C-bit does indeed apply only to SEV guests.
Note that because guest physical addresses are always translated
through the nested page tables, the size of the guest physical address
space is not impacted by any physical address space reduction indicated
in CPUID 8000_001F[EBX]. If the C-bit is a physical address bit however,
the guest physical address space is effectively reduced by 1 bit.
And for SEV guests, the APM clearly states that the bit is dropped before
walking the nested page tables.
If the C-bit is an address bit, this bit is masked from the guest
physical address when it is translated through the nested page tables.
Consequently, the hypervisor does not need to be aware of which pages
the guest has chosen to mark private.
Note, the bogus C-bit clearing was removed from legacy #PF handler in
commit 6d1b867d0456 ("KVM: SVM: Don't strip the C-bit from CR2 on #PF
interception").
Fixes: 0ede79e13224 ("KVM: SVM: Clear C-bit from the page fault address")
Cc: Peter Gonda <[email protected]>
Cc: Brijesh Singh <[email protected]>
Cc: Tom Lendacky <[email protected]>
Cc: [email protected]
Signed-off-by: Sean Christopherson <[email protected]>
---
arch/x86/kvm/svm/svm.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index 8834822c00cd..ca5614a48b21 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -1923,7 +1923,7 @@ static int npf_interception(struct kvm_vcpu *vcpu)
{
struct vcpu_svm *svm = to_svm(vcpu);
- u64 fault_address = __sme_clr(svm->vmcb->control.exit_info_2);
+ u64 fault_address = svm->vmcb->control.exit_info_2;
u64 error_code = svm->vmcb->control.exit_info_1;
trace_kvm_page_fault(fault_address, error_code);
--
2.32.0.93.g670b81a890-goog
On 6/24/21 9:03 PM, Sean Christopherson wrote:
> Don't clear the C-bit in the #NPF handler, as it is a legal GPA bit for
> non-SEV guests, and for SEV guests the C-bit is dropped before the GPA
> hits the NPT in hardware. Clearing the bit for non-SEV guests causes KVM
> to mishandle #NPFs with that collide with the host's C-bit.
>
> Although the APM doesn't explicitly state that the C-bit is not reserved
> for non-SEV, Tom Lendacky confirmed that the following snippet about the
> effective reduction due to the C-bit does indeed apply only to SEV guests.
>
> Note that because guest physical addresses are always translated
> through the nested page tables, the size of the guest physical address
> space is not impacted by any physical address space reduction indicated
> in CPUID 8000_001F[EBX]. If the C-bit is a physical address bit however,
> the guest physical address space is effectively reduced by 1 bit.
>
> And for SEV guests, the APM clearly states that the bit is dropped before
> walking the nested page tables.
>
> If the C-bit is an address bit, this bit is masked from the guest
> physical address when it is translated through the nested page tables.
> Consequently, the hypervisor does not need to be aware of which pages
> the guest has chosen to mark private.
>
> Note, the bogus C-bit clearing was removed from legacy #PF handler in
> commit 6d1b867d0456 ("KVM: SVM: Don't strip the C-bit from CR2 on #PF
> interception").
>
> Fixes: 0ede79e13224 ("KVM: SVM: Clear C-bit from the page fault address")
> Cc: Peter Gonda <[email protected]>
> Cc: Brijesh Singh <[email protected]>
> Cc: Tom Lendacky <[email protected]>
> Cc: [email protected]
> Signed-off-by: Sean Christopherson <[email protected]>
Yep, definitely wasn't correct to be using an SME based macro on a guest
address. And as the APM states, the encryption bit is stripped for SEV
guests, so looks correct to me.
Acked-by: Tom Lendacky <[email protected]>
> ---
> arch/x86/kvm/svm/svm.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> index 8834822c00cd..ca5614a48b21 100644
> --- a/arch/x86/kvm/svm/svm.c
> +++ b/arch/x86/kvm/svm/svm.c
> @@ -1923,7 +1923,7 @@ static int npf_interception(struct kvm_vcpu *vcpu)
> {
> struct vcpu_svm *svm = to_svm(vcpu);
>
> - u64 fault_address = __sme_clr(svm->vmcb->control.exit_info_2);
> + u64 fault_address = svm->vmcb->control.exit_info_2;
> u64 error_code = svm->vmcb->control.exit_info_1;
>
> trace_kvm_page_fault(fault_address, error_code);
>
On 25/06/21 04:03, Sean Christopherson wrote:
> Patch 01 reverts the C-bit truncation patch as the reserved #PF was
> confirmed to be due to a magic HyperTransport region (how many magic
> addresses are there!?!). Hopefully the original patch simply be dropped,
> but just in case...
>
> Patch 02 reverts the C-bit clearing in the #NPF handler. If that somehow
> turns out to be incorrect, i.e. there are flows where the CPU doesn't
> mask off the C-bit, then it can be conditional on a SEV guest.
>
> I'll be offline for the next two weeks, fingers crossed I've undone all
> the damage. :-)
>
> Thanks!
>
> Sean Christopherson (2):
> Revert "KVM: x86: Truncate reported guest MAXPHYADDR to C-bit if SEV
> is supported"
> KVM: SVM: Revert clearing of C-bit on GPA in #NPF handler
>
> arch/x86/kvm/cpuid.c | 11 -----------
> arch/x86/kvm/svm/svm.c | 39 +++++++++------------------------------
> arch/x86/kvm/x86.c | 3 ---
> arch/x86/kvm/x86.h | 1 -
> 4 files changed, 9 insertions(+), 45 deletions(-)
>
Queued, thanks.
Paolo