These two functions are essentially called by code that is only
compiled when CONFIG_KVM=y, with the notable exception of the
two users in the sPAPR code:

$ git grep -E -l 'kvm_arch_vcpu_id|kvm_vcpu_id_is_valid'
accel/kvm/kvm-all.c
hw/intc/openpic_kvm.c
hw/intc/xics_kvm.c
hw/ppc/spapr.c
include/sysemu/kvm.h
target/arm/kvm.c
target/i386/kvm.c
target/mips/kvm.c
target/ppc/kvm.c
target/s390x/kvm.c

In hw/ppc/spapr.c:

    if (kvm_enabled()) {
        return kvm_arch_vcpu_id(cs);
    } else {
        return cs->cpu_index;
    }

and

        if (kvm_enabled() && !kvm_vcpu_id_is_valid(cpu->vcpu_id)) {
            ...
        }

This code happens to compile without CONFIG_KVM=y simply because
kvm_enabled() expands to (0) and the compiler optimizes the dead
code away. Unless this was done on purpose to indicate no stubs
are required, and we'd rather break the build if calling these
from KVM agnostic code, it seems better practice to have stubs.

It doesn't really make sense to call these when kvm_enabled() is
false, since they were introduced to allow the architecture to
control VCPU ids passed to the KVM_CREATE_VCPU ioctl.

This being said, I see no reason for not giving the stub a friendly
default behaviour:
- VCPU id is cpu_index
- a VCPU id is always valid

Signed-off-by: Greg Kurz <groug@kaod.org>
---
 accel/stubs/kvm-stub.c |   10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/accel/stubs/kvm-stub.c b/accel/stubs/kvm-stub.c
index c964af3e1c97..d3e7d39ff9a9 100644
--- a/accel/stubs/kvm-stub.c
+++ b/accel/stubs/kvm-stub.c
@@ -160,4 +160,14 @@ bool kvm_arm_supports_user_irq(void)
 {
     return false;
 }
+
+bool kvm_vcpu_id_is_valid(int vcpu_id)
+{
+    return true;
+}
+
+unsigned long kvm_arch_vcpu_id(CPUState *cpu)
+{
+    return cpu->cpu_index;
+}
 #endif

On 16/02/2018 19:11, Greg Kurz wrote:
> These two functions are essentially called by code that is only
> compiled when CONFIG_KVM=y, with the notable exception of the
> two users in the sPAPR code:
> 
> $ git grep -E -l 'kvm_arch_vcpu_id|kvm_vcpu_id_is_valid'
> accel/kvm/kvm-all.c
> hw/intc/openpic_kvm.c
> hw/intc/xics_kvm.c
> hw/ppc/spapr.c
> include/sysemu/kvm.h
> target/arm/kvm.c
> target/i386/kvm.c
> target/mips/kvm.c
> target/ppc/kvm.c
> target/s390x/kvm.c
> 
> In hw/ppc/spapr.c:
> 
>     if (kvm_enabled()) {
>         return kvm_arch_vcpu_id(cs);
>     } else {
>         return cs->cpu_index;
>     }
> 
> and
> 
>         if (kvm_enabled() && !kvm_vcpu_id_is_valid(cpu->vcpu_id)) {
>             ...
>         }
> 
> This code happens to compile without CONFIG_KVM=y simply because
> kvm_enabled() expands to (0) and the compiler optimizes the dead
> code away. Unless this was done on purpose

Yes, it is.  There are more examples, the first I saw is:

    uint32_t tbfreq = kvm_enabled() ? kvmppc_get_tbfreq()
        : SPAPR_TIMEBASE_FREQ;
    uint32_t cpufreq = kvm_enabled() ? kvmppc_get_clockfreq() : 1000000000;

> to indicate no stubs
> are required, and we'd rather break the build if calling these
> from KVM agnostic code

That's the idea. :)

Paolo