On 21/05/2024 6:58 am, Isaku Yamahata wrote:
On Mon, May 20, 2024 at 10:38:58AM +0000,
"Huang, Kai" <kai.huang@xxxxxxxxx> wrote:
On Sat, 2024-05-18 at 15:41 +0000, Edgecombe, Rick P wrote:
On Sat, 2024-05-18 at 05:42 +0000, Huang, Kai wrote:
No. I meant "using kvm_mmu_page.role.mirrored_pt to determine whether to
invoke kvm_x86_ops::xx_private_spt()" is not correct.
I agree this looks wrong.
Instead, we should
use fault->is_private to determine:
if (fault->is_private && kvm_x86_ops::xx_private_spt())
kvm_x86_ops::xx_private_spte();
else
// normal TDP MMU operation
The reason is this pattern works not just for TDX, but also for SNP (and
SW_PROTECTED_VM) if they ever need specific page table ops.
Do you want to split the concept from invoking hooks from mirrored PT
and to allow invoking hooks even for shared PT (probably without
mirrored PT)? So far I tied the mirrored PT to invoking the hooks as
those hooks are to reflect the changes on mirrored PT to private PT.
Is there any use case to allow hook for shared PT?
To be clear, my intention is to allow hook, if available, for "private
GPA". The point here is for "private GPA", but not "shared PT".
- SEV_SNP
Although I can't speak for SNP folks, I guess they don't need hooks.
I guess they want to stay away from directly modifying the TDP MMU
(to add TDP MMU hooks). Instead, They added hooks to guest_memfd.
RMP (Reverse mapping table) doesn't have to be consistent with NPT.
Anyway, I'll reply to
https://lore.kernel.org/lkml/20240501085210.2213060-1-michael.roth@xxxxxxx/T/#m8ca554a6d4bad7fa94dedefcf5914df19c9b8051
For SNP _ONLY_ I completely understand. The point is, TDX needs to
modify anyway. So if SNP can use hooks for TDX, and if in that case we
can avoid guest_memfd hooks, then I think it's better?
But I can certainly be, and probably am, wrong, because that
gmem_memfd() hooks have been there for long time.
TDX
I don't see immediate need to allow hooks for shared PT. >
SW_PROTECTED (today)
It uses only shared PT and don't need hooks.
SW_PROTECTED (with mirrored pt with shared mask in future in theory)
This would be similar to TDX, we wouldn't need hooks for shared PT.
SW_PROTECTED (shared PT only without mirrored pt in future in theory)
I don't see necessity hooks for shared PT.
(Or I don't see value of this SW_PROTECTED case.)
I don't think SW_PROTECTED VM will ever need to have any TDP MMU hook,
because there's no hardware feature backing behind it.
My intention is for SNP. Even if SNP doesn't need any TDP MMU hook
today, I think invoking hook depending on "private GPA", but not
"private page table" provides more flexibility. And this also works for
TDX, regardless whether SNP wants to implement any TDP MMU hook.
So conceptually speaking, I don't see any disadvantage of my proposal,
regardless whether SNP chooses to use any TDP MMU hook or not. On the
other hand, if we choose to "invoke hooks depending on page table type",
then this code will indeed be only for TDX.
