On Thu, May 5, 2022 at 5:46 PM Kai Huang <kai.huang@xxxxxxxxx> wrote: > > On Thu, 2022-05-05 at 17:22 -0700, Dan Williams wrote: > > On Thu, May 5, 2022 at 3:14 PM Kai Huang <kai.huang@xxxxxxxxx> wrote: > > > > > > Thanks for feedback! > > > > > > On Thu, 2022-05-05 at 06:51 -0700, Dan Williams wrote: > > > > [ add Mike ] > > > > > > > > > > > > On Thu, May 5, 2022 at 2:54 AM Kai Huang <kai.huang@xxxxxxxxx> wrote: > > > > [..] > > > > > > > > > > Hi Dave, > > > > > > > > > > Sorry to ping (trying to close this). > > > > > > > > > > Given we don't need to consider kmem-hot-add legacy PMEM after TDX module > > > > > initialization, I think for now it's totally fine to exclude legacy PMEMs from > > > > > TDMRs. The worst case is when someone tries to use them as TD guest backend > > > > > directly, the TD will fail to create. IMO it's acceptable, as it is supposedly > > > > > that no one should just use some random backend to run TD. > > > > > > > > The platform will already do this, right? > > > > > > > > > > In the current v3 implementation, we don't have any code to handle memory > > > hotplug, therefore nothing prevents people from adding legacy PMEMs as system > > > RAM using kmem driver. In order to guarantee all pages managed by page > > > > That's the fundamental question I am asking why is "guarantee all > > pages managed by page allocator are TDX memory". That seems overkill > > compared to indicating the incompatibility after the fact. > > As I explained, the reason is I don't want to modify page allocator to > distinguish TDX and non-TDX allocation, for instance, having to have a ZONE_TDX > and GFP_TDX. Right, TDX details do not belong at that level, but it will work almost all the time if you do nothing to "guarantee" all TDX capable pages all the time. > KVM depends on host's page fault handler to allocate the page. In fact KVM only > consumes PFN from host's page tables. For now only RAM is TDX memory. By > guaranteeing all pages in page allocator is TDX memory, we can easily use > anonymous pages as TD guest memory. Again, TDX capable pages will be the overwhelming default, why are you worried about cluttering the memory hotplug path for nice corner cases. Consider the fact that end users can break the kernel by specifying invalid memmap= command line options. The memory hotplug code does not take any steps to add safety in those cases because there are already too many ways it can go wrong. TDX is just one more corner case where the memmap= user needs to be careful. Otherwise, it is up to the platform firmware to make sure everything in the base memory map is TDX capable, and then all you need is documentation about the failure mode when extending "System RAM" beyond that baseline. > shmem to support a new fd-based backend which doesn't require having to mmap() > TD guest memory to host userspace: > > https://lore.kernel.org/kvm/20220310140911.50924-1-chao.p.peng@xxxxxxxxxxxxxxx/ > > Also, besides TD guest memory, there are some per-TD control data structures > (which must be TDX memory too) need to be allocated for each TD. Normal memory > allocation APIs can be used for such allocation if we guarantee all pages in > page allocator is TDX memory. You don't need that guarantee, just check it after the fact and fail if that assertion fails. It should almost always be the case that it succeeds and if it doesn't then something special is happening with that system and the end user has effectively opt-ed out of TDX operation. > > > allocator are all TDX memory, the v3 implementation needs to always include > > > legacy PMEMs as TDX memory so that even people truly add legacy PMEMs as system > > > RAM, we can still guarantee all pages in page allocator are TDX memory. > > > > Why? > > If we don't include legacy PMEMs as TDX memory, then after they are hot-added as > system RAM using kmem driver, the assumption of "all pages in page allocator are > TDX memory" is broken. A TD can be killed during runtime. Yes, that is what the end user asked for. If they don't want that to happen then the policy decision about using kmem needs to be updated in userspace, not hard code that policy decision towards TDX inside the kernel.
