Re: [PATCH] x86/virt/tdx: Warn on !4K level in tdh_mem_page_aug()
"Edgecombe, Rick P" <[email protected]>
| Newsgroups | org.kernel.vger.kvm,dev.linux.lists.linux-coco,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <[email protected]> |
On Tue, 2026-08-25 at 10:48 -0700, Sean Christopherson wrote: > > Since we have TDX huge pages coming so soon, it seems ok to leave it. But I > > just wanted to say having the level arg here originally was wrong. So this > > should not be precedent for pre-enabling of things years in the future. > > Eh, I would argue that in this particular case, including @level was ok from a > process perspective, the main issue is the implementation was buggy. Which is > very arguably _because_ there was no user to validate the code, but given all > the pre-existing KVM MMU code at play, at some point we'd have to insert code > to say "hugepages aren't supported". E.g. in this specific case, assuming > PG_LEVEL_4K could have been just as bad as having a partially-dead @level. These days (not back then), tdx_sept_set_private_spte() is a good spot to put all the "is this a supported S-EPT operation" checking. Having it in one place is good. While it is normal for the kernel to not include dead code, it is also a TDX process problem. We have a lot of people working together. Some on near things and some on far things. When you discuss your teammates patch, it can be hard to not think about how it affects your own work. But then you have a design influenced by hidden and unstable future projects. We run into this kind of thing all the time. So the slope is especially slippery. To work around it, we a firmish rule. The rule(s) is: - The design of a feature should be about the current state of upstream, not future work. - Except, you can think about long term uABI. Now, we got some different direction from you regarding DPAMT and TDX huge pages. You wanted them co-designed. And actually Dave was giving us the opposite direction to do them one at a time. So, partly why I'm elaborating on how we are doing this as a default.