linux-kernel - Re: [PATCH v2 09/15] KVM: x86/tdp_mmu: Support mirror root for TDP MMU

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <9c5f7aae312325c0e880baf411f956d4cce3c6d1.camel@intel.com>
Date: Mon, 10 Jun 2024 00:08:56 +0000
From: "Edgecombe, Rick P" <rick.p.edgecombe@...el.com>
To: "pbonzini@...hat.com" <pbonzini@...hat.com>
CC: "seanjc@...gle.com" <seanjc@...gle.com>, "Huang, Kai"
	<kai.huang@...el.com>, "sagis@...gle.com" <sagis@...gle.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, "Zhao, Yan Y"
	<yan.y.zhao@...el.com>, "Aktas, Erdem" <erdemaktas@...gle.com>,
	"kvm@...r.kernel.org" <kvm@...r.kernel.org>, "dmatlack@...gle.com"
	<dmatlack@...gle.com>, "Yamahata, Isaku" <isaku.yamahata@...el.com>,
	"isaku.yamahata@...il.com" <isaku.yamahata@...il.com>
Subject: Re: [PATCH v2 09/15] KVM: x86/tdp_mmu: Support mirror root for TDP
 MMU

On Sat, 2024-06-08 at 11:13 +0200, Paolo Bonzini wrote:
> > I was not loving the amount of indirection here in the patch, but thought it
> > centralized the logic a bit better. This way seems good, given that the
> > actual
> > logic is not that complex.
> 
> My proposed implementation is a bit TDX-specific though... Something
> like this is more agnostic, and it exploits nicely the difference
> between fault->addr and fault->gfn:
> 
> if (!kvm_gfn_direct_mask(kvm) ||
>     (gpa_to_gfn(fault->addr) & kvm_gfn_direct_mask(kvm))
>   root_hpa = vcpu->arch.mmu->root.hpa;
> else
>   root_hpa = vcpu->arch.mmu->mirror_root_hpa;
> return root_to_sp(root_hpa);

Agreed that this is less TDX specific and it means that this part of the generic
MMU code doesn't need to know that the mirror/direct matches to private vs
shared. I don't love that it has such a complicated conditional for the normal
VM case, though. Just for readability.

The previous versions checked kvm_gfn_shared_mask() more readily in various open
coded spots. In this v2 we tried to reduce this and instead always rely on
the "private" concept to switch between the roots in the generic code. I think
it's arguably a little easier to understand if we stick to a single way of
deciding which root to use.

But I don't feel like any of these solutions discussed is perfectly clean. So
I'm ok taking the benefits you prefer. I guess doing bitwise operations when
possible is kind of the KVM way, haha. :)