On 5/27/2025 7:20 PM, David Hildenbrand wrote:
> On 27.05.25 07:47, Chenyi Qiang wrote:
>>
>>
>> On 5/26/2025 5:08 PM, David Hildenbrand wrote:
>>> On 20.05.25 12:28, Chenyi Qiang wrote:
>>>> As guest_memfd is now managed by RamBlockAttribute with
>>>> RamDiscardManager, only block uncoordinated discard.
>>>>
>>>> Signed-off-by: Chenyi Qiang <chenyi.qi...@intel.com>
>>>> ---
>>>> Changes in v5:
>>>> - Revert to use RamDiscardManager.
>>>>
>>>> Changes in v4:
>>>> - Modify commit message (RamDiscardManager-
>>>> >PrivateSharedManager).
>>>>
>>>> Changes in v3:
>>>> - No change.
>>>>
>>>> Changes in v2:
>>>> - Change the ram_block_discard_require(false) to
>>>> ram_block_coordinated_discard_require(false).
>>>> ---
>>>> system/physmem.c | 6 +++---
>>>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/system/physmem.c b/system/physmem.c
>>>> index f05f7ff09a..58b7614660 100644
>>>> --- a/system/physmem.c
>>>> +++ b/system/physmem.c
>>>> @@ -1916,7 +1916,7 @@ static void ram_block_add(RAMBlock *new_block,
>>>> Error **errp)
>>>> }
>>>> assert(new_block->guest_memfd < 0);
>>>> - ret = ram_block_discard_require(true);
>>>> + ret = ram_block_coordinated_discard_require(true);
>>>> if (ret < 0) {
>>>> error_setg_errno(errp, -ret,
>>>> "cannot set up private guest memory:
>>>> discard currently blocked");
>>>> @@ -1939,7 +1939,7 @@ static void ram_block_add(RAMBlock *new_block,
>>>> Error **errp)
>>>> * ever develops a need to check for errors.
>>>> */
>>>> close(new_block->guest_memfd);
>>>> - ram_block_discard_require(false);
>>>> + ram_block_coordinated_discard_require(false);
>>>> qemu_mutex_unlock_ramlist();
>>>> goto out_free;
>>>> }
>>>> @@ -2302,7 +2302,7 @@ static void reclaim_ramblock(RAMBlock *block)
>>>> if (block->guest_memfd >= 0) {
>>>> ram_block_attribute_destroy(block->ram_shared);
>>>> close(block->guest_memfd);
>>>> - ram_block_discard_require(false);
>>>> + ram_block_coordinated_discard_require(false);
>>>> }
>>>> g_free(block);
>>>
>>>
>>> I think this patch should be squashed into the previous one, then the
>>> story in that single patch is consistent.
>>
>> I think this patch is a gate to allow device assignment with guest_memfd
>> and want to make it separately. Can we instead add some commit message
>> in previous one? like:
>>
>> "Using guest_memfd with vfio is still blocked via
>> ram_block_discard_disable()/ram_block_discard_require()."
>
> For the title it should probably be something like:
>
> "physmem: support coordinated discarding of RAM with guest_memdfd"
>
> Then explain how we install the RAMDiscardManager that will notify
> listeners (esp. vfio).
Make sense. Will do the squash and change the title. Thanks!
>
