Re: [PATCH v3 07/70] physmem: Relax the alignment check of host_startaddr in ram_block_discard_range()

Xiaoyao Li
2023-11-16T02:56:51Z

On 11/16/2023 2:20 AM, David Hildenbrand wrote:
> On 15.11.23 08:14, Xiaoyao Li wrote:
>> Commit d3a5038c461 ("exec: ram_block_discard_range") introduced
>> ram_block_discard_range() which grabs some code from
>> ram_discard_range(). However, during code movement, it changed alignment
>> check of host_startaddr from qemu_host_page_size to rb->page_size.
>>
>> When ramblock is back'ed by hugepage, it requires the startaddr to be
>> huge page size aligned, which is a overkill. e.g., TDX's private-shared
>> page conversion is done at 4KB granularity. Shared page is discarded
>> when it gets converts to private and when shared page back'ed by
>> hugepage it is going to fail on this check.
>>
>> So change to alignment check back to qemu_host_page_size.
>>
>> Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
>> ---
>> Changes in v3:
>>   - Newly added in v3;
>> ---
>>   system/physmem.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/system/physmem.c b/system/physmem.c
>> index c56b17e44df6..8a4e42c7cf60 100644
>> --- a/system/physmem.c
>> +++ b/system/physmem.c
>> @@ -3532,7 +3532,7 @@ int ram_block_discard_range(RAMBlock *rb,
>> uint64_t start, size_t length)
>>       uint8_t *host_startaddr = rb->host + start;
>> -    if (!QEMU_PTR_IS_ALIGNED(host_startaddr, rb->page_size)) {
>> +    if (!QEMU_PTR_IS_ALIGNED(host_startaddr, qemu_host_page_size)) {
>
> For your use cases, rb->page_size should always match qemu_host_page_size.
>
> IIRC, we only set rb->page_size to different values for hugetlb. And
> guest_memfd does not support hugetlb.
>
> Even if QEMU is using THP, rb->page_size should 4k.
>
> Please elaborate how you can actually trigger that. From what I recall,
> guest_memfd is not compatible with hugetlb.

It's the shared memory that can be back'ed by hugetlb.

Later patch 9 introduces ram_block_convert_page(), which will discard
shared memory when it gets converted to private. TD guest can request
convert a 4K to private while the page is previously back'ed by hugetlb
as 2M shared page.

> And the check here makes perfect sense for existing callers of
> ram_block_discard_range(): you cannot partially zap a hugetlb page.
>