Am 26.02.2013 um 03:01 schrieb David Gibson <da...@gibson.dropbear.id.au>:
> On Mon, Feb 25, 2013 at 01:19:48PM +0100, Alexander Graf wrote:
>>
>> On 21.02.2013, at 03:41, David Gibson wrote:
>>
>>> For PAPR guests, KVM tracks the various areas registered with the
>>> H_REGISTER_VPA hypercall. For full emulation, of course, these are tracked
>>> within qemu. At present these values are not synchronized. This is a
>>> problem for reset (qemu's reset of the VPA address is not pushed to KVM)
>>> and will also be a problem for savevm / migration.
>>>
>>> The kernel now supports accessing the VPA state via the ONE_REG interface,
>>> this patch adds code to qemu to use that interface to keep the qemu and
>>> KVM ideas of the VPA state synchronized.
>>>
>>> Signed-off-by: David Gibson <da...@gibson.dropbear.id.au>
>>> ---
>>> target-ppc/kvm.c | 122
>>> ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>> 1 file changed, 122 insertions(+)
>>>
>>> diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
>>> index 5a6f608..ac310c1 100644
>>> --- a/target-ppc/kvm.c
>>> +++ b/target-ppc/kvm.c
>>> @@ -62,6 +62,7 @@ static int cap_ppc_rma;
>>> static int cap_spapr_tce;
>>> static int cap_hior;
>>> static int cap_one_reg;
>>> +static int cap_papr;
>>>
>>> /* XXX We have a race condition where we actually have a level triggered
>>> * interrupt, but the infrastructure can't expose that yet, so the guest
>>> @@ -92,6 +93,8 @@ int kvm_arch_init(KVMState *s)
>>> cap_spapr_tce = kvm_check_extension(s, KVM_CAP_SPAPR_TCE);
>>> cap_one_reg = kvm_check_extension(s, KVM_CAP_ONE_REG);
>>> cap_hior = kvm_check_extension(s, KVM_CAP_PPC_HIOR);
>>> + /* Note: we don't set cap_papr here, because this capability is
>>> + * only activated after this by kvmppc_set_papr() */
>>>
>>> if (!cap_interrupt_level) {
>>> fprintf(stderr, "KVM: Couldn't find level irq capability. Expect the
>>> "
>>> @@ -647,6 +650,103 @@ static int kvm_get_fp(CPUState *cs)
>>> return 0;
>>> }
>>>
>>> +#if defined(TARGET_PPC64)
>>> +static int kvm_get_vpa(CPUState *cs)
>>> +{
>>> + PowerPCCPU *cpu = POWERPC_CPU(cs);
>>> + CPUPPCState *env = &cpu->env;
>>> + struct kvm_one_reg reg;
>>> + int ret;
>>> +
>>> + reg.id = KVM_REG_PPC_VPA_ADDR;
>>> + reg.addr = (uintptr_t)&env->vpa_addr;
>>> + ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, ®);
>>> + if (ret < 0) {
>>> + dprintf("Unable to get VPA address from KVM: %s\n",
>>> strerror(errno));
>>> + return ret;
>>> + }
>>> +
>>> + assert((uintptr_t)&env->slb_shadow_size
>>> + == ((uintptr_t)&env->slb_shadow_addr + 8));
>>> + reg.id = KVM_REG_PPC_VPA_SLB;
>>> + reg.addr = (uintptr_t)&env->slb_shadow_addr;
>>> + ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, ®);
>>> + if (ret < 0) {
>>> + dprintf("Unable to get SLB shadow state from KVM: %s\n",
>>> + strerror(errno));
>>> + return ret;
>>> + }
>>> +
>>> + assert((uintptr_t)&env->dtl_size == ((uintptr_t)&env->dtl_addr + 8));
>>> + reg.id = KVM_REG_PPC_VPA_DTL;
>>> + reg.addr = (uintptr_t)&env->dtl_addr;
>>> + ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, ®);
>>> + if (ret < 0) {
>>> + dprintf("Unable to get dispatch trace log state from KVM: %s\n",
>>> + strerror(errno));
>>> + return ret;
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int kvm_put_vpa(CPUState *cs)
>>> +{
>>> + PowerPCCPU *cpu = POWERPC_CPU(cs);
>>> + CPUPPCState *env = &cpu->env;
>>> + struct kvm_one_reg reg;
>>> + int ret;
>>> +
>>> + /* SLB shadow or DTL can't be registered unless a master VPA is
>>> + * registered. That means when restoring state, if a VPA *is*
>>> + * registered, we need to set that up first. If not, we need to
>>> + * deregister the others before deregistering the master VPA */
>>> + assert(env->vpa_addr || !(env->slb_shadow_addr || env->dtl_addr));
>>> +
>>> + if (env->vpa_addr) {
>>> + reg.id = KVM_REG_PPC_VPA_ADDR;
>>> + reg.addr = (uintptr_t)&env->vpa_addr;
>>> + ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, ®);
>>> + if (ret < 0) {
>>> + dprintf("Unable to set VPA address to KVM: %s\n",
>>> strerror(errno));
>>> + return ret;
>>> + }
>>> + }
>>> +
>>> + assert((uintptr_t)&env->slb_shadow_size
>>> + == ((uintptr_t)&env->slb_shadow_addr + 8));
>>> + reg.id = KVM_REG_PPC_VPA_SLB;
>>> + reg.addr = (uintptr_t)&env->slb_shadow_addr;
>>> + ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, ®);
>>> + if (ret < 0) {
>>> + dprintf("Unable to set SLB shadow state to KVM: %s\n",
>>> strerror(errno));
>>> + return ret;
>>> + }
>>> +
>>> + assert((uintptr_t)&env->dtl_size == ((uintptr_t)&env->dtl_addr + 8));
>>> + reg.id = KVM_REG_PPC_VPA_DTL;
>>> + reg.addr = (uintptr_t)&env->dtl_addr;
>>> + ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, ®);
>>> + if (ret < 0) {
>>> + dprintf("Unable to set dispatch trace log state to KVM: %s\n",
>>> + strerror(errno));
>>> + return ret;
>>> + }
>>> +
>>> + if (!env->vpa_addr) {
>>> + reg.id = KVM_REG_PPC_VPA_ADDR;
>>> + reg.addr = (uintptr_t)&env->vpa_addr;
>>> + ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, ®);
>>> + if (ret < 0) {
>>> + dprintf("Unable to set VPA address to KVM: %s\n",
>>> strerror(errno));
>>> + return ret;
>>> + }
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +#endif /* TARGET_PPC64 */
>>> +
>>> int kvm_arch_put_registers(CPUState *cs, int level)
>>> {
>>> PowerPCCPU *cpu = POWERPC_CPU(cs);
>>> @@ -747,6 +847,15 @@ int kvm_arch_put_registers(CPUState *cs, int level)
>>> kvm_put_one_spr(cs, id, i);
>>> }
>>> }
>>> +
>>> +#ifdef TARGET_PPC64
>>> + if (cap_papr) {
>>> + if (kvm_put_vpa(cs) < 0) {
>>> + fprintf(stderr,
>>> + "Warning: Unable to set VPA information to KVM\n");
>>
>> This makes newer QEMU on older host kernels unusable, right? It
>> would spill that warning all over the place on every register
>> synchronization. Are you sure this is what you want?
>
> Um.. I'm not sure why you single out this patch. The SPR and FPU sync
> patches introcued similar warnings,
No, they use dprintf :). You can also switch VPA to dprintf and thus get rid of
the problem.
> which can also spew on older
> hosts. The VPA is arguably the most harmful not to sync, since reset
> will fail without it.
>
>> How about adding a global to indicate that you've already emitted
>> the warning before? Only print it when you didn't.
>
> Yeah, I could. Doing that for FPU and SPRs as well starts to add up
> to a lot of global flags, though. Actually for SPRs, I guess I could
> just zero out the id field in the ppc_spr_t. Bit of a hack, but does
> the job.
Yup, sounds good :).
>
> --
> David Gibson | I'll have my music baroque, and my code
> david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
> | _way_ _around_!
> http://www.ozlabs.org/~dgibson
