On 5/1/24 10:51, Michael Roth wrote:
> In the case of SEV-SNP, whether or not a 2MB page can be mapped via a
> 2MB mapping in the guest's nested page table depends on whether or not
> any subpages within the range have already been initialized as private
> in the RMP table. The existing mixed-attribute tracking in KVM is
> insufficient here, for instance:
>
> - gmem allocates 2MB page
> - guest issues PVALIDATE on 2MB page
> - guest later converts a subpage to shared
> - SNP host code issues PSMASH to split 2MB RMP mapping to 4K
> - KVM MMU splits NPT mapping to 4K
> - guest later converts that shared page back to private
>
> At this point there are no mixed attributes, and KVM would normally
> allow for 2MB NPT mappings again, but this is actually not allowed
> because the RMP table mappings are 4K and cannot be promoted on the
> hypervisor side, so the NPT mappings must still be limited to 4K to
> match this.
>
> Add a hook to determine the max NPT mapping size in situations like
> this.
>
> Suggested-by: Sean Christopherson <[email protected]>
> Signed-off-by: Michael Roth <[email protected]>
> ---
> arch/x86/include/asm/kvm-x86-ops.h | 1 +
> arch/x86/include/asm/kvm_host.h | 1 +
> arch/x86/kvm/mmu/mmu.c | 18 ++++++++++++++++--
> 3 files changed, 18 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/include/asm/kvm-x86-ops.h b/arch/x86/include/asm/kvm-x86-ops.h
> index c81990937ab4..566d19b02483 100644
> --- a/arch/x86/include/asm/kvm-x86-ops.h
> +++ b/arch/x86/include/asm/kvm-x86-ops.h
> @@ -140,6 +140,7 @@ KVM_X86_OP_OPTIONAL_RET0(vcpu_get_apicv_inhibit_reasons);
> KVM_X86_OP_OPTIONAL(get_untagged_addr)
> KVM_X86_OP_OPTIONAL(alloc_apic_backing_page)
> KVM_X86_OP_OPTIONAL_RET0(gmem_prepare)
> +KVM_X86_OP_OPTIONAL_RET0(private_max_mapping_level)
> KVM_X86_OP_OPTIONAL(gmem_invalidate)
>
> #undef KVM_X86_OP
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index c6c5018376be..87265b73906a 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -1816,6 +1816,7 @@ struct kvm_x86_ops {
> void *(*alloc_apic_backing_page)(struct kvm_vcpu *vcpu);
> int (*gmem_prepare)(struct kvm *kvm, kvm_pfn_t pfn, gfn_t gfn, int max_order);
> void (*gmem_invalidate)(kvm_pfn_t start, kvm_pfn_t end);
> + int (*private_max_mapping_level)(struct kvm *kvm, kvm_pfn_t pfn);
> };
>
> struct kvm_x86_nested_ops {
> diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
> index 510eb1117012..0d556da052f6 100644
> --- a/arch/x86/kvm/mmu/mmu.c
> +++ b/arch/x86/kvm/mmu/mmu.c
> @@ -4271,6 +4271,20 @@ static inline u8 kvm_max_level_for_order(int order)
> return PG_LEVEL_4K;
> }
>
> +static u8 kvm_max_private_mapping_level(struct kvm *kvm, kvm_pfn_t pfn,
> + u8 max_level, int gmem_order)
> +{
> + if (max_level == PG_LEVEL_4K)
> + return PG_LEVEL_4K;
> +
> + max_level = min(kvm_max_level_for_order(gmem_order), max_level);
> + if (max_level == PG_LEVEL_4K)
> + return PG_LEVEL_4K;
> +
> + return min(max_level,
> + static_call(kvm_x86_private_max_mapping_level)(kvm, pfn));
> +}
Since you're returning 0 both as a default and, later in the series, for non-SNP guests, you need to treat 0 as "don't care":
diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
index de35dee25bf6..62ad38b2a8c9 100644
--- a/arch/x86/kvm/mmu/mmu.c
+++ b/arch/x86/kvm/mmu/mmu.c
@@ -4274,6 +4274,8 @@ static inline u8 kvm_max_level_for_order(int order)
static u8 kvm_max_private_mapping_level(struct kvm *kvm, kvm_pfn_t pfn,
u8 max_level, int gmem_order)
{
+ u8 req_max_level;
+
if (max_level == PG_LEVEL_4K)
return PG_LEVEL_4K;
@@ -4281,8 +4283,11 @@ static u8 kvm_max_private_mapping_level(struct kvm *kvm, kvm_pfn_t pfn,
if (max_level == PG_LEVEL_4K)
return PG_LEVEL_4K;
- return min(max_level,
- static_call(kvm_x86_private_max_mapping_level)(kvm, pfn));
+ req_max_level = static_call(kvm_x86_private_max_mapping_level)(kvm, pfn);
+ if (req_max_level)
+ max_level = min(max_level, req_max_level);
+
+ return req_max_level;
}
static int kvm_faultin_pfn_private(struct kvm_vcpu *vcpu,
Not beautiful but it does the job I guess.
Paolo