Re: [PATCH v4 03/12] kvm: Provide explicit error for kvm_create_guest_memfd()

On 14/8/26 14:44, Peter Xu wrote:
> On Fri, Aug 14, 2026 at 07:56:42AM +0200, Philippe Mathieu-Daudé wrote:
>> Hi Peter, Michael,
>>
>> On 12/8/26 22:16, Michael Roth wrote:
>>> From: Peter Xu <[email protected]>
>>>
>>> So that there will be a verbal string returned when kvm not enabled, or
>>> kvm not compiled.
>>>
>>> Signed-off-by: Peter Xu <[email protected]>
>>> Reviewed-by: Xiaoyao Li <[email protected]>
>>> Reviewed-by: Fabiano Rosas <[email protected]>
>>> Reviewed-by: Michael Roth <[email protected]>
>>> Signed-off-by: Michael Roth <[email protected]>
>>> ---
>>>    accel/kvm/kvm-all.c    | 5 +++++
>>>    accel/stubs/kvm-stub.c | 1 +
>>>    2 files changed, 6 insertions(+)


>> So maybe what we want is:
>>
>> -- >8 --
>> diff --git a/system/physmem.c b/system/physmem.c
>> index b97016b1303..66ff74541aa 100644
>> --- a/system/physmem.c
>> +++ b/system/physmem.c
>> @@ -2824,6 +2824,8 @@ int ram_block_rebind(Error **errp)
>>   {
>>       RAMBlock *block;
>>
>> +    assert(kvm_enabled()); /* Only supported by KVM so far */
>> +
>>       qemu_mutex_lock_ramlist();
>>
>>       RAMBLOCK_FOREACH(block) {
>> ---
>>
>> Or less aggressive:
>>
>> -- >8 --
>> diff --git a/system/physmem.c b/system/physmem.c
>> index b97016b1303..2988d1dd6c9 100644
>> --- a/system/physmem.c
>> +++ b/system/physmem.c
>> @@ -2824,6 +2824,11 @@ int ram_block_rebind(Error **errp)
>>   {
>>       RAMBlock *block;
>>
>> +    if (!kvm_enabled()) {
>> +        error_setg(errp, "guest-memfd requires KVM accelerator");
>> +        return -1;
>> +    }
>> +
>>       qemu_mutex_lock_ramlist();
>>
>>       RAMBLOCK_FOREACH(block) {
>> ---
>>
>> WDYT?
> 
> Fine by me.
> 
> IMHO it's normally more of an issue the other way round, if we used an
> assert() where we should use error_setg() (hence, user triggerable
> assert()s).  Here we expect it to never happen, so either way should not
> happen..
> 
> If so, we could also assert() in ram_block_rebind(), as it's only used in
> kvm_reset_vmfd() only, so I don't see how it can be reached if KVM is not
> enabled first..

I'd rather avoid kvm_enabled() checks in kvm-specific API (accel/kvm/).

If ram_block_rebind() is KVM-specific, why expose it as RAMBlock API?