On 08.04.2011, at 23:48, Scott Wood wrote:
> Classic/server ppc has had SREGS for a while now (though I think not
> always?), but it's still missing for booke. Check the capability before
> calling KVM_SET_SREGS.
>
> Without this, booke kvm fails to boot as of commit
> 84b4915dd2c0eaa86c970ffc42a68ea8ba9e48b5 (kvm: Handle kvm_init_vcpu
> errors).
Oops :).
>
> Also, don't write random stack state into the non-PVR sregs fields --
> have kvm fill it in first.
>
> Eventually booke will have sregs and it will have its own capability to
> be tested here. However, we will want a way for platform code to request
> to look like the actual CPU we're running on, especially if SoC devices
> are being directly assigned.
>
> Signed-off-by: Scott Wood <scottw...@freescale.com>
> ---
> target-ppc/kvm.c | 33 ++++++++++++++++++++++++++++++---
> 1 files changed, 30 insertions(+), 3 deletions(-)
>
> diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
> index 2cfb24b..5401536 100644
> --- a/target-ppc/kvm.c
> +++ b/target-ppc/kvm.c
> @@ -77,13 +77,40 @@ int kvm_arch_init(KVMState *s)
> return 0;
> }
>
> -int kvm_arch_init_vcpu(CPUState *cenv)
> +static int kvm_arch_sync_sregs(CPUState *cenv)
huh? So what about the previous caller of this?
> {
> - int ret = 0;
> struct kvm_sregs sregs;
> + int ret;
Eh - this makes the patch less readable :)
> +
> +#ifdef TARGET_PPC
> +#ifdef KVM_CAP_PPC_SEGSTATE
This code never gets compiled without TARGET_PPC?
> + if (!kvm_check_extension(cenv->kvm_state, KVM_CAP_PPC_SEGSTATE)) {
> + return 0;
> + }
> +#else
> + return 0;
Doing a simple return 0 might lead to warnings (which become errors with
-Werror) due to unused variables. I'm not sure how to handle this well. Maybe
define KVM_CAP_PPC_SEGSTATE to something invalid when it's not defined? That
way the capability check would fail and we don't need the duplicate code paths.
> +#endif
> +#else /* TARGET_PPCEMB */
I guess you were #ifdefing on PPCEMB before? I don't think you really need to
care about PPCEMB. The only reason it exists is for page size < 4k, which you
don't hit on e500 IIUC.
> + return 0;
> +#endif
> +
> + ret = kvm_vcpu_ioctl(cenv, KVM_GET_SREGS, &sregs);
> + if (ret) {
> + return ret;
> + }
>
> sregs.pvr = cenv->spr[SPR_PVR];
> - ret = kvm_vcpu_ioctl(cenv, KVM_SET_SREGS, &sregs);
> + return kvm_vcpu_ioctl(cenv, KVM_SET_SREGS, &sregs);
> +}
> +
> +int kvm_arch_init_vcpu(CPUState *cenv)
> +{
> + int ret;
> +
> + ret = kvm_arch_sync_sregs(cenv);
> + if (ret) {
> + return ret;
> + }
>
> idle_timer = qemu_new_timer_ns(vm_clock, kvm_kick_env, cenv);
>
> --
> 1.7.1
>
Alex
