Subject: Re: [PATCH v3 1/6] KVM: PPC: Use getters and setters for vcpu register state

From: Michael Ellerman <[email-masked]>
Date: Thu, 17 Aug 2023 13:25:58 +1000

Jordan Niethe <[email-masked]> writes:
> There are already some getter and setter functions used for accessing
> vcpu register state, e.g. kvmppc_get_pc(). There are also more
> complicated examples that are generated by macros like
> kvmppc_get_sprg0() which are generated by the SHARED_SPRNG_WRAPPER()
> macro.
>

...
> diff --git a/arch/powerpc/include/asm/kvm_book3s.h b/arch/powerpc/include/asm/kvm_book3s.h
> index bbf5e2c5fe09..1a7e837ea2d5 100644
> --- a/arch/powerpc/include/asm/kvm_book3s.h
> +++ b/arch/powerpc/include/asm/kvm_book3s.h
> @@ -403,10 +413,121 @@ static inline ulong kvmppc_get_fault_dar(struct kvm_vcpu *vcpu)
...
> +
> +#ifdef CONFIG_VSX
> +static inline void kvmppc_get_vsx_vr(struct kvm_vcpu *vcpu, int i, vector128 *v)
> +{
> +	*v =  vcpu->arch.vr.vr[i];
> +}

This is causing build errors if VSX is disabled.

I'm using g5_defconfig plus:

  CONFIG_VIRTUALIZATION=y
  CONFIG_KVM_BOOK3S_64=y
  CONFIG_KVM_BOOK3S_64_PR=y

Which gives me:

  ../arch/powerpc/kvm/powerpc.c: In function ‘kvmppc_set_vmx_dword’:
  ../arch/powerpc/kvm/powerpc.c:1061:9: error: implicit declaration of function ‘kvmppc_get_vsx_vr’; did you mean ‘kvmppc_get_vsx_fpr’? [-Werror=implicit-function-declaration]
   1061 |         kvmppc_get_vsx_vr(vcpu, index, &val.vval);
        |         ^~~~~~~~~~~~~~~~~
        |         kvmppc_get_vsx_fpr
  ../arch/powerpc/kvm/powerpc.c:1063:9: error: implicit declaration of function ‘kvmppc_set_vsx_vr’; did you mean ‘kvmppc_set_vsx_fpr’? [-Werror=implicit-function-declaration]
   1063 |         kvmppc_set_vsx_vr(vcpu, index, &val.vval);
        |         ^~~~~~~~~~~~~~~~~
        |         kvmppc_set_vsx_fpr
  In file included from ../arch/powerpc/kvm/powerpc.c:25:
  ../arch/powerpc/kvm/powerpc.c: In function ‘kvm_vcpu_ioctl_get_one_reg’:
  ../arch/powerpc/kvm/powerpc.c:1729:52: error: implicit declaration of function ‘kvmppc_get_vscr’; did you mean ‘kvmppc_get_sr’? [-Werror=implicit-function-declaration]
   1729 |                         val = get_reg_val(reg->id, kvmppc_get_vscr(vcpu));
        |                                                    ^~~~~~~~~~~~~~~
  ../arch/powerpc/include/asm/kvm_ppc.h:412:29: note: in definition of macro ‘get_reg_val’
    412 |         case 4: __u.wval = (reg); break;        \
        |                             ^~~
  ../arch/powerpc/kvm/powerpc.c: In function ‘kvm_vcpu_ioctl_set_one_reg’:
  ../arch/powerpc/kvm/powerpc.c:1780:25: error: implicit declaration of function ‘kvmppc_set_vscr’; did you mean ‘kvmppc_set_fscr’? [-Werror=implicit-function-declaration]
   1780 |                         kvmppc_set_vscr(vcpu, set_reg_val(reg->id, val));
        |                         ^~~~~~~~~~~~~~~
        |                         kvmppc_set_fscr

Looking at kvm_vcpu_arch, the thread_vr_state and members are guarded by
CONFIG_ALTIVEC, not CONFIG_VSX.

Switching to that fixes the build.

Whether it makes sense to be getting/setting those registers when VSX=n
is not immediately clear, but is a separate problem.

cheers