> On 19 Mar 2026, at 4:23 PM, Philippe Mathieu-Daudé <[email protected]> wrote:
>
> Hi Ani,
>
> On 19/3/26 11:11, Ani Sinha wrote:
>> Add a new call hyperv_enable_synic() that can be called whether or not
>> CONFIG_HYPERV is enabled. This way genetic code in i396/kvm.c can call this
>> function to enable synic for hyperv. For non-hyperv cases, the stub will
>> be a noop.
>> Reported-by: Michale Tokarev <[email protected]>
>> Signed-off-by: Ani Sinha <[email protected]>
>> ---
>> target/i386/kvm/hyperv-stub.c | 5 +++++
>> target/i386/kvm/hyperv.c | 9 +++++++++
>> target/i386/kvm/hyperv.h | 1 +
>> target/i386/kvm/kvm.c | 12 +++++-------
>> 4 files changed, 20 insertions(+), 7 deletions(-)
>> diff --git a/target/i386/kvm/hyperv-stub.c b/target/i386/kvm/hyperv-stub.c
>> index 5836f53c23..767a4c7e1a 100644
>> --- a/target/i386/kvm/hyperv-stub.c
>> +++ b/target/i386/kvm/hyperv-stub.c
>> @@ -61,3 +61,8 @@ uint64_t hyperv_syndbg_query_options(void)
>> {
>> return 0;
>> }
>> +
>> +int hyperv_enable_synic(X86CPU *cpu)
>> +{
>> + return 0;
>> +}
>> diff --git a/target/i386/kvm/hyperv.c b/target/i386/kvm/hyperv.c
>> index f7a81bd270..bd3c26d02b 100644
>> --- a/target/i386/kvm/hyperv.c
>> +++ b/target/i386/kvm/hyperv.c
>> @@ -24,6 +24,15 @@ int hyperv_x86_synic_add(X86CPU *cpu)
>> return 0;
>> }
>> +int hyperv_enable_synic(X86CPU *cpu)
>> +{
>> + int ret = 0;
>> + if (!hyperv_is_synic_enabled()) {
>> + ret = hyperv_x86_synic_add(cpu);
>> + }
>> + return ret;
>> +}
>> +
>> /*
>> * All devices possibly using SynIC have to be reset before calling this to
>> let
>> * them remove their SINT routes first.
>> diff --git a/target/i386/kvm/hyperv.h b/target/i386/kvm/hyperv.h
>> index e45a4512fe..a393a5d428 100644
>> --- a/target/i386/kvm/hyperv.h
>> +++ b/target/i386/kvm/hyperv.h
>> @@ -23,6 +23,7 @@ int kvm_hv_handle_exit(X86CPU *cpu, struct kvm_hyperv_exit
>> *exit);
>> #endif
>> int hyperv_x86_synic_add(X86CPU *cpu);
>> +int hyperv_enable_synic(X86CPU *cpu);
>> void hyperv_x86_synic_reset(X86CPU *cpu);
>> void hyperv_x86_synic_update(X86CPU *cpu);
>> diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
>> index a29f757c16..9e352882c8 100644
>> --- a/target/i386/kvm/kvm.c
>> +++ b/target/i386/kvm/kvm.c
>> @@ -1754,13 +1754,11 @@ static int hyperv_init_vcpu(X86CPU *cpu)
>> return ret;
>> }
>> - if (!hyperv_is_synic_enabled()) {
>> - ret = hyperv_x86_synic_add(cpu);
>> - if (ret < 0) {
>> - error_report("failed to create HyperV SynIC: %s",
>> - strerror(-ret));
>> - return ret;
>> - }
>> + ret = hyperv_enable_synic(cpu);
>> + if (ret < 0) {
>> + error_report("failed to create HyperV SynIC: %s",
>> + strerror(-ret));
>> + return ret;
>> }
>> }
>>
>
> Why not simply add a hyperv_is_synic_enabled() stub?
That would be wrong unless I assert in hyperv_is_synic_enabled() stub function
which would break minimal microvms?
The other reason is I do not like hyperv_x86_synic_add() returning -ENOSYS. If
hyperv is not enabled we do not need to call those functions at all.
