Re: [PATCH] tdx, memory hotplug: Check whole hot-adding memory range for TDX

[David Hildenbrand <david@redhat.com>]

On 01.10.24 10:08, Dan Williams wrote:
> David Hildenbrand wrote:
>> On 01.10.24 08:45, Dan Williams wrote:
>>> David Hildenbrand wrote:
>>>> On 30.09.24 07:51, Huang Ying wrote:
>>>>> On systems with TDX (Trust Domain eXtensions) enabled, memory ranges
>>>>> hot-added must be checked for compatibility by TDX.  This is currently
>>>>> implemented through memory hotplug notifiers for each memory_block.
>>>>> If a memory range which isn't TDX compatible is hot-added, for
>>>>> example, some CXL memory, the command line as follows,
>>>>>
>>>>>      $ echo 1 > /sys/devices/system/node/nodeX/memoryY/online
>>>>>
>>>>> will report something like,
>>>>>
>>>>>      bash: echo: write error: Operation not permitted
>>>>>
>>>>> If pr_debug() is enabled, the error message like below will be shown
>>>>> in the kernel log,
>>>>>
>>>>>      online_pages [mem 0xXXXXXXXXXX-0xXXXXXXXXXX] failed
>>>>>
>>>>> Both are too general to root cause the problem.  This will confuse
>>>>> users.  One solution is to print some error messages in the TDX memory
>>>>> hotplug notifier.  However, memory hotplug notifiers are called for
>>>>> each memory block, so this may lead to a large volume of messages in
>>>>> the kernel log if a large number of memory blocks are onlined with a
>>>>> script or automatically.  For example, the typical size of memory
>>>>> block is 128MB on x86_64, when online 64GB CXL memory, 512 messages
>>>>> will be logged.
>>>>
>>>> ratelimiting would likely help here a lot, but I agree that it is
>>>> suboptimal.
>>>>
>>>>>
>>>>> Therefore, in this patch, the whole hot-adding memory range is checked
>>>>> for TDX compatibility through a newly added architecture specific
>>>>> function (arch_check_hotplug_memory_range()).  If rejected, the memory
>>>>> hot-adding will be aborted with a proper kernel log message.  Which
>>>>> looks like something as below,
>>>>>
>>>>>      virt/tdx: Reject hot-adding memory range: 0xXXXXXXXX-0xXXXXXXXX for TDX compatibility.
>>>>    > > The target use case is to support CXL memory on TDX enabled systems.
>>>>> If the CXL memory isn't compatible with TDX, the whole CXL memory
>>>>> range hot-adding will be rejected.  While the CXL memory can still be
>>>>> used via devdax interface.
>>>>
>>>> I'm curious, why can that memory be used through devdax but not through
>>>> the buddy? I'm probably missing something important :)
>>>
>>> TDX requires memory that supports integrity and encryption. Until
>>> platforms and expanders with a technology called CXL TSP arrives, CXL
>>> memory is not able to join the TCB.
>>>
>>> The TDX code for simplicity assumes that only memory present at boot
>>> might be capable of TDX and that everything else is not.
>>
>> So is there ever a chance where add_memory() would actually work now
>> with TDX? Or can we just simplify and unconditionally reject
>> add_memory() if TDX is enabled?
> 
> Only if the memory address range is enumerated by the platform firmware
> (mcheck) at boot time.
> 
> This will eventually be possible with the CXL dynamic-capacity (DCD)
> capability once CXL TSP arrives. In that scenario the CXL DCD expander
> is brought into the TCB at boot time and assigned a fixed address range
> where future memory could arrive. I.e. the CXL device is brought into
> the TCB at boot, but the memory it provides can arrive later.
> 
>>> Confidential VMs use guest_mem_fd to allocate memory, and that only
>>> pulls from the page allocator as a backend.
>>>
>>> This ability to use devdax in an offline mode is a hack to not
>>
>> Thanks, I was missing the "hack" of it, and somehow (once again) assumed
>> that we would be hotplugging memory into confidential VMs.
> 
> When / if dynamic capacity and this security-protocol for CXL arrives
> that may yet happen. For now it is safe to block adding anything which
> mcheck does not like which is everything but memory present at boot
> (is_tdx_memory()).

Makes sense, thanks!

-- 
Cheers,

David / dhildenb