[Qemu-devel] [PATCH] PPC: Sync CPU state for KVM

[Qemu-devel] [PATCH] PPC: Sync CPU state for KVM

[Alexander Graf]

Some recent change made PPC guests always start at address 0x0 because env
isn't synced to kvm_state on first bootup.

I'm not sure if this is the correct bugfix, but at least it makes PPC boot
again with KVM enabled.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 hw/ppc_newworld.c |    4 ++++
 hw/ppc_oldworld.c |    4 ++++
 2 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/hw/ppc_newworld.c b/hw/ppc_newworld.c
index da868d3..2f2f091 100644
--- a/hw/ppc_newworld.c
+++ b/hw/ppc_newworld.c
@@ -38,6 +38,7 @@
 #include "ide.h"
 #include "loader.h"
 #include "elf.h"
+#include "kvm.h"
 
 #define MAX_IDE_BUS 2
 #define VGA_BIOS_SIZE 65536
@@ -134,6 +135,9 @@ static void ppc_core99_init (ram_addr_t ram_size,
         envs[i] = env;
     }
 
+    /* Make sure all register sets take effect */
+    cpu_synchronize_state(env);
+
     /* allocate RAM */
     ram_offset = qemu_ram_alloc(ram_size);
     cpu_register_physical_memory(0, ram_size, ram_offset);
diff --git a/hw/ppc_oldworld.c b/hw/ppc_oldworld.c
index 9b49a3d..4d89805 100644
--- a/hw/ppc_oldworld.c
+++ b/hw/ppc_oldworld.c
@@ -38,6 +38,7 @@
 #include "ide.h"
 #include "loader.h"
 #include "elf.h"
+#include "kvm.h"
 
 #define MAX_IDE_BUS 2
 #define VGA_BIOS_SIZE 65536
@@ -162,6 +163,9 @@ static void ppc_heathrow_init (ram_addr_t ram_size,
         envs[i] = env;
     }
 
+    /* Make sure all register sets take effect */
+    cpu_synchronize_state(env);
+
     /* allocate RAM */
     if (ram_size > (2047 << 20)) {
         fprintf(stderr,
-- 
1.6.0.2

[Qemu-devel] [PATCH] PPC: Get MMU state on register sync

[Alexander Graf]

While x86 only needs to sync cr0-4 to know all about its MMU state and enable
qemu to resolve virtual to physical addresses, we need to sync all of the
segment registers on PPC to know which mapping we're in.

So let's grab the segment register contents to be able to use the "x" monitor
command and also enable the gdbstub to resolve virtual addresses.

I sent the corresponding KVM patch to the KVM ML some minutes ago.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 target-ppc/kvm.c |   30 ++++++++++++++++++++++++++++++
 1 files changed, 30 insertions(+), 0 deletions(-)

diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
index 4e1c65f..566513f 100644
--- a/target-ppc/kvm.c
+++ b/target-ppc/kvm.c
@@ -98,12 +98,17 @@ int kvm_arch_put_registers(CPUState *env)
 int kvm_arch_get_registers(CPUState *env)
 {
     struct kvm_regs regs;
+    struct kvm_sregs sregs;
     uint32_t i, ret;
 
     ret = kvm_vcpu_ioctl(env, KVM_GET_REGS, &regs);
     if (ret < 0)
         return ret;
 
+    ret = kvm_vcpu_ioctl(env, KVM_GET_SREGS, &sregs);
+    if (ret < 0)
+        return ret;
+
     env->ctr = regs.ctr;
     env->lr = regs.lr;
     env->xer = regs.xer;
@@ -125,6 +130,31 @@ int kvm_arch_get_registers(CPUState *env)
     for (i = 0;i < 32; i++)
         env->gpr[i] = regs.gpr[i];
 
+#ifdef KVM_CAP_PPC_SEGSTATE
+    if (kvm_check_extension(env->kvm_state, KVM_CAP_PPC_SEGSTATE)) {
+        env->sdr1 = sregs.sdr1;
+    
+        /* Sync SLB */
+        for (i = 0; i < 64; i++) {
+            ppc_store_slb(env, sregs.ppc64.slb[i].slbe,
+                               sregs.ppc64.slb[i].slbv);
+        }
+    
+        /* Sync SRs */
+        for (i = 0; i < 16; i++) {
+            env->sr[i] = sregs.ppc32.sr[i];
+        }
+    
+        /* Sync BATs */
+        for (i = 0; i < 8; i++) {
+            env->DBAT[0][i] = sregs.ppc32.dbat[i] & 0xffffffff;
+            env->DBAT[1][i] = sregs.ppc32.dbat[i] >> 32;
+            env->IBAT[0][i] = sregs.ppc32.ibat[i] & 0xffffffff;
+            env->IBAT[1][i] = sregs.ppc32.ibat[i] >> 32;
+        }
+    }
+#endif
+
     return 0;
 }
 
-- 
1.6.0.2

[Qemu-devel] Re: [PATCH] PPC: Get MMU state on register sync

[Jan Kiszka]

Alexander Graf wrote:
> While x86 only needs to sync cr0-4 to know all about its MMU state and enable
> qemu to resolve virtual to physical addresses, we need to sync all of the
> segment registers on PPC to know which mapping we're in.
> 
> So let's grab the segment register contents to be able to use the "x" monitor
> command and also enable the gdbstub to resolve virtual addresses.
> 
> I sent the corresponding KVM patch to the KVM ML some minutes ago.
> 
> Signed-off-by: Alexander Graf <agraf@suse.de>
> ---
>  target-ppc/kvm.c |   30 ++++++++++++++++++++++++++++++
>  1 files changed, 30 insertions(+), 0 deletions(-)
> 
> diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
> index 4e1c65f..566513f 100644
> --- a/target-ppc/kvm.c
> +++ b/target-ppc/kvm.c
> @@ -98,12 +98,17 @@ int kvm_arch_put_registers(CPUState *env)
>  int kvm_arch_get_registers(CPUState *env)
>  {
>      struct kvm_regs regs;
> +    struct kvm_sregs sregs;
>      uint32_t i, ret;
>  
>      ret = kvm_vcpu_ioctl(env, KVM_GET_REGS, &regs);
>      if (ret < 0)
>          return ret;
>  
> +    ret = kvm_vcpu_ioctl(env, KVM_GET_SREGS, &sregs);
> +    if (ret < 0)
> +        return ret;
> +
>      env->ctr = regs.ctr;
>      env->lr = regs.lr;
>      env->xer = regs.xer;
> @@ -125,6 +130,31 @@ int kvm_arch_get_registers(CPUState *env)
>      for (i = 0;i < 32; i++)
>          env->gpr[i] = regs.gpr[i];
>  
> +#ifdef KVM_CAP_PPC_SEGSTATE
> +    if (kvm_check_extension(env->kvm_state, KVM_CAP_PPC_SEGSTATE)) {
> +        env->sdr1 = sregs.sdr1;
> +    
> +        /* Sync SLB */
> +        for (i = 0; i < 64; i++) {
> +            ppc_store_slb(env, sregs.ppc64.slb[i].slbe,
> +                               sregs.ppc64.slb[i].slbv);
> +        }
> +    
> +        /* Sync SRs */
> +        for (i = 0; i < 16; i++) {
> +            env->sr[i] = sregs.ppc32.sr[i];
> +        }
> +    
> +        /* Sync BATs */
> +        for (i = 0; i < 8; i++) {
> +            env->DBAT[0][i] = sregs.ppc32.dbat[i] & 0xffffffff;
> +            env->DBAT[1][i] = sregs.ppc32.dbat[i] >> 32;
> +            env->IBAT[0][i] = sregs.ppc32.ibat[i] & 0xffffffff;
> +            env->IBAT[1][i] = sregs.ppc32.ibat[i] >> 32;
> +        }
> +    }
> +#endif
> +
>      return 0;
>  }
>  

What about KVM_SET_SREGS in kvm_arch_put_registers? E.g. to play back
potential changes to that special registers someone did via gdb?

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

[Qemu-devel] Re: [PATCH] PPC: Get MMU state on register sync

[Alexander Graf]

On 24.11.2009, at 19:01, Jan Kiszka wrote:

> Alexander Graf wrote:
>> While x86 only needs to sync cr0-4 to know all about its MMU state and enable
>> qemu to resolve virtual to physical addresses, we need to sync all of the
>> segment registers on PPC to know which mapping we're in.
>> 
>> So let's grab the segment register contents to be able to use the "x" monitor
>> command and also enable the gdbstub to resolve virtual addresses.
>> 
>> I sent the corresponding KVM patch to the KVM ML some minutes ago.
>> 
>> Signed-off-by: Alexander Graf <agraf@suse.de>
>> ---
>> target-ppc/kvm.c |   30 ++++++++++++++++++++++++++++++
>> 1 files changed, 30 insertions(+), 0 deletions(-)
>> 
>> diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
>> index 4e1c65f..566513f 100644
>> --- a/target-ppc/kvm.c
>> +++ b/target-ppc/kvm.c
>> @@ -98,12 +98,17 @@ int kvm_arch_put_registers(CPUState *env)
>> int kvm_arch_get_registers(CPUState *env)
>> {
>>     struct kvm_regs regs;
>> +    struct kvm_sregs sregs;
>>     uint32_t i, ret;
>> 
>>     ret = kvm_vcpu_ioctl(env, KVM_GET_REGS, &regs);
>>     if (ret < 0)
>>         return ret;
>> 
>> +    ret = kvm_vcpu_ioctl(env, KVM_GET_SREGS, &sregs);
>> +    if (ret < 0)
>> +        return ret;
>> +
>>     env->ctr = regs.ctr;
>>     env->lr = regs.lr;
>>     env->xer = regs.xer;
>> @@ -125,6 +130,31 @@ int kvm_arch_get_registers(CPUState *env)
>>     for (i = 0;i < 32; i++)
>>         env->gpr[i] = regs.gpr[i];
>> 
>> +#ifdef KVM_CAP_PPC_SEGSTATE
>> +    if (kvm_check_extension(env->kvm_state, KVM_CAP_PPC_SEGSTATE)) {
>> +        env->sdr1 = sregs.sdr1;
>> +    
>> +        /* Sync SLB */
>> +        for (i = 0; i < 64; i++) {
>> +            ppc_store_slb(env, sregs.ppc64.slb[i].slbe,
>> +                               sregs.ppc64.slb[i].slbv);
>> +        }
>> +    
>> +        /* Sync SRs */
>> +        for (i = 0; i < 16; i++) {
>> +            env->sr[i] = sregs.ppc32.sr[i];
>> +        }
>> +    
>> +        /* Sync BATs */
>> +        for (i = 0; i < 8; i++) {
>> +            env->DBAT[0][i] = sregs.ppc32.dbat[i] & 0xffffffff;
>> +            env->DBAT[1][i] = sregs.ppc32.dbat[i] >> 32;
>> +            env->IBAT[0][i] = sregs.ppc32.ibat[i] & 0xffffffff;
>> +            env->IBAT[1][i] = sregs.ppc32.ibat[i] >> 32;
>> +        }
>> +    }
>> +#endif
>> +
>>     return 0;
>> }
>> 
> 
> What about KVM_SET_SREGS in kvm_arch_put_registers? E.g. to play back
> potential changes to that special registers someone did via gdb?

I don't think you can actually change the segment values. At least I can't imagine why.

I definitely will implement SET_SREGS as soon as your sync split is in, as that's IMHO only really required on migration.


Alex

[Qemu-devel] Re: [PATCH] PPC: Get MMU state on register sync

[Jan Kiszka]

Alexander Graf wrote:
> On 24.11.2009, at 19:01, Jan Kiszka wrote:
> 
>> Alexander Graf wrote:
>>> While x86 only needs to sync cr0-4 to know all about its MMU state and enable
>>> qemu to resolve virtual to physical addresses, we need to sync all of the
>>> segment registers on PPC to know which mapping we're in.
>>>
>>> So let's grab the segment register contents to be able to use the "x" monitor
>>> command and also enable the gdbstub to resolve virtual addresses.
>>>
>>> I sent the corresponding KVM patch to the KVM ML some minutes ago.
>>>
>>> Signed-off-by: Alexander Graf <agraf@suse.de>
>>> ---
>>> target-ppc/kvm.c |   30 ++++++++++++++++++++++++++++++
>>> 1 files changed, 30 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
>>> index 4e1c65f..566513f 100644
>>> --- a/target-ppc/kvm.c
>>> +++ b/target-ppc/kvm.c
>>> @@ -98,12 +98,17 @@ int kvm_arch_put_registers(CPUState *env)
>>> int kvm_arch_get_registers(CPUState *env)
>>> {
>>>     struct kvm_regs regs;
>>> +    struct kvm_sregs sregs;
>>>     uint32_t i, ret;
>>>
>>>     ret = kvm_vcpu_ioctl(env, KVM_GET_REGS, &regs);
>>>     if (ret < 0)
>>>         return ret;
>>>
>>> +    ret = kvm_vcpu_ioctl(env, KVM_GET_SREGS, &sregs);
>>> +    if (ret < 0)
>>> +        return ret;
>>> +
>>>     env->ctr = regs.ctr;
>>>     env->lr = regs.lr;
>>>     env->xer = regs.xer;
>>> @@ -125,6 +130,31 @@ int kvm_arch_get_registers(CPUState *env)
>>>     for (i = 0;i < 32; i++)
>>>         env->gpr[i] = regs.gpr[i];
>>>
>>> +#ifdef KVM_CAP_PPC_SEGSTATE
>>> +    if (kvm_check_extension(env->kvm_state, KVM_CAP_PPC_SEGSTATE)) {
>>> +        env->sdr1 = sregs.sdr1;
>>> +    
>>> +        /* Sync SLB */
>>> +        for (i = 0; i < 64; i++) {
>>> +            ppc_store_slb(env, sregs.ppc64.slb[i].slbe,
>>> +                               sregs.ppc64.slb[i].slbv);
>>> +        }
>>> +    
>>> +        /* Sync SRs */
>>> +        for (i = 0; i < 16; i++) {
>>> +            env->sr[i] = sregs.ppc32.sr[i];
>>> +        }
>>> +    
>>> +        /* Sync BATs */
>>> +        for (i = 0; i < 8; i++) {
>>> +            env->DBAT[0][i] = sregs.ppc32.dbat[i] & 0xffffffff;
>>> +            env->DBAT[1][i] = sregs.ppc32.dbat[i] >> 32;
>>> +            env->IBAT[0][i] = sregs.ppc32.ibat[i] & 0xffffffff;
>>> +            env->IBAT[1][i] = sregs.ppc32.ibat[i] >> 32;
>>> +        }
>>> +    }
>>> +#endif
>>> +
>>>     return 0;
>>> }
>>>
>> What about KVM_SET_SREGS in kvm_arch_put_registers? E.g. to play back
>> potential changes to that special registers someone did via gdb?
> 
> I don't think you can actually change the segment values. At least I can't imagine why.

Dunno about PPC in this regard and how much value it has, but we have
segment register access via gdb for x86.

> 
> I definitely will implement SET_SREGS as soon as your sync split is in, as that's IMHO only really required on migration.
> 

Migration is, of course, the major use case.

Still I wonder why not making this API symmetric when already touching it.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

[Qemu-devel] Re: [PATCH] PPC: Get MMU state on register sync

[Alexander Graf]

On 24.11.2009, at 19:12, Jan Kiszka wrote:

> Alexander Graf wrote:
>> On 24.11.2009, at 19:01, Jan Kiszka wrote:
>> 
>>> Alexander Graf wrote:
>>>> While x86 only needs to sync cr0-4 to know all about its MMU state and enable
>>>> qemu to resolve virtual to physical addresses, we need to sync all of the
>>>> segment registers on PPC to know which mapping we're in.
>>>> 
>>>> So let's grab the segment register contents to be able to use the "x" monitor
>>>> command and also enable the gdbstub to resolve virtual addresses.
>>>> 
>>>> I sent the corresponding KVM patch to the KVM ML some minutes ago.
>>>> 
>>>> Signed-off-by: Alexander Graf <agraf@suse.de>
>>>> ---
>>>> target-ppc/kvm.c |   30 ++++++++++++++++++++++++++++++
>>>> 1 files changed, 30 insertions(+), 0 deletions(-)
>>>> 
>>>> diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
>>>> index 4e1c65f..566513f 100644
>>>> --- a/target-ppc/kvm.c
>>>> +++ b/target-ppc/kvm.c
>>>> @@ -98,12 +98,17 @@ int kvm_arch_put_registers(CPUState *env)
>>>> int kvm_arch_get_registers(CPUState *env)
>>>> {
>>>>    struct kvm_regs regs;
>>>> +    struct kvm_sregs sregs;
>>>>    uint32_t i, ret;
>>>> 
>>>>    ret = kvm_vcpu_ioctl(env, KVM_GET_REGS, &regs);
>>>>    if (ret < 0)
>>>>        return ret;
>>>> 
>>>> +    ret = kvm_vcpu_ioctl(env, KVM_GET_SREGS, &sregs);
>>>> +    if (ret < 0)
>>>> +        return ret;
>>>> +
>>>>    env->ctr = regs.ctr;
>>>>    env->lr = regs.lr;
>>>>    env->xer = regs.xer;
>>>> @@ -125,6 +130,31 @@ int kvm_arch_get_registers(CPUState *env)
>>>>    for (i = 0;i < 32; i++)
>>>>        env->gpr[i] = regs.gpr[i];
>>>> 
>>>> +#ifdef KVM_CAP_PPC_SEGSTATE
>>>> +    if (kvm_check_extension(env->kvm_state, KVM_CAP_PPC_SEGSTATE)) {
>>>> +        env->sdr1 = sregs.sdr1;
>>>> +    
>>>> +        /* Sync SLB */
>>>> +        for (i = 0; i < 64; i++) {
>>>> +            ppc_store_slb(env, sregs.ppc64.slb[i].slbe,
>>>> +                               sregs.ppc64.slb[i].slbv);
>>>> +        }
>>>> +    
>>>> +        /* Sync SRs */
>>>> +        for (i = 0; i < 16; i++) {
>>>> +            env->sr[i] = sregs.ppc32.sr[i];
>>>> +        }
>>>> +    
>>>> +        /* Sync BATs */
>>>> +        for (i = 0; i < 8; i++) {
>>>> +            env->DBAT[0][i] = sregs.ppc32.dbat[i] & 0xffffffff;
>>>> +            env->DBAT[1][i] = sregs.ppc32.dbat[i] >> 32;
>>>> +            env->IBAT[0][i] = sregs.ppc32.ibat[i] & 0xffffffff;
>>>> +            env->IBAT[1][i] = sregs.ppc32.ibat[i] >> 32;
>>>> +        }
>>>> +    }
>>>> +#endif
>>>> +
>>>>    return 0;
>>>> }
>>>> 
>>> What about KVM_SET_SREGS in kvm_arch_put_registers? E.g. to play back
>>> potential changes to that special registers someone did via gdb?
>> 
>> I don't think you can actually change the segment values. At least I can't imagine why.
> 
> Dunno about PPC in this regard and how much value it has, but we have
> segment register access via gdb for x86.

The segments here are more like PLM4 on x86.

>> I definitely will implement SET_SREGS as soon as your sync split is in, as that's IMHO only really required on migration.
>> 
> 
> Migration is, of course, the major use case.
> 
> Still I wonder why not making this API symmetric when already touching it.

I was afraid to introduce performance regressions - setting the segments means flushing the complete shadow MMU.


Alex

[Qemu-devel] Re: [PATCH] PPC: Get MMU state on register sync

[Jan Kiszka]

Alexander Graf wrote:
> On 24.11.2009, at 19:12, Jan Kiszka wrote:
> 
>> Alexander Graf wrote:
>>> On 24.11.2009, at 19:01, Jan Kiszka wrote:
>>>
>>>> Alexander Graf wrote:
>>>>> While x86 only needs to sync cr0-4 to know all about its MMU state and enable
>>>>> qemu to resolve virtual to physical addresses, we need to sync all of the
>>>>> segment registers on PPC to know which mapping we're in.
>>>>>
>>>>> So let's grab the segment register contents to be able to use the "x" monitor
>>>>> command and also enable the gdbstub to resolve virtual addresses.
>>>>>
>>>>> I sent the corresponding KVM patch to the KVM ML some minutes ago.
>>>>>
>>>>> Signed-off-by: Alexander Graf <agraf@suse.de>
>>>>> ---
>>>>> target-ppc/kvm.c |   30 ++++++++++++++++++++++++++++++
>>>>> 1 files changed, 30 insertions(+), 0 deletions(-)
>>>>>
>>>>> diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
>>>>> index 4e1c65f..566513f 100644
>>>>> --- a/target-ppc/kvm.c
>>>>> +++ b/target-ppc/kvm.c
>>>>> @@ -98,12 +98,17 @@ int kvm_arch_put_registers(CPUState *env)
>>>>> int kvm_arch_get_registers(CPUState *env)
>>>>> {
>>>>>    struct kvm_regs regs;
>>>>> +    struct kvm_sregs sregs;
>>>>>    uint32_t i, ret;
>>>>>
>>>>>    ret = kvm_vcpu_ioctl(env, KVM_GET_REGS, &regs);
>>>>>    if (ret < 0)
>>>>>        return ret;
>>>>>
>>>>> +    ret = kvm_vcpu_ioctl(env, KVM_GET_SREGS, &sregs);
>>>>> +    if (ret < 0)
>>>>> +        return ret;
>>>>> +
>>>>>    env->ctr = regs.ctr;
>>>>>    env->lr = regs.lr;
>>>>>    env->xer = regs.xer;
>>>>> @@ -125,6 +130,31 @@ int kvm_arch_get_registers(CPUState *env)
>>>>>    for (i = 0;i < 32; i++)
>>>>>        env->gpr[i] = regs.gpr[i];
>>>>>
>>>>> +#ifdef KVM_CAP_PPC_SEGSTATE
>>>>> +    if (kvm_check_extension(env->kvm_state, KVM_CAP_PPC_SEGSTATE)) {
>>>>> +        env->sdr1 = sregs.sdr1;
>>>>> +    
>>>>> +        /* Sync SLB */
>>>>> +        for (i = 0; i < 64; i++) {
>>>>> +            ppc_store_slb(env, sregs.ppc64.slb[i].slbe,
>>>>> +                               sregs.ppc64.slb[i].slbv);
>>>>> +        }
>>>>> +    
>>>>> +        /* Sync SRs */
>>>>> +        for (i = 0; i < 16; i++) {
>>>>> +            env->sr[i] = sregs.ppc32.sr[i];
>>>>> +        }
>>>>> +    
>>>>> +        /* Sync BATs */
>>>>> +        for (i = 0; i < 8; i++) {
>>>>> +            env->DBAT[0][i] = sregs.ppc32.dbat[i] & 0xffffffff;
>>>>> +            env->DBAT[1][i] = sregs.ppc32.dbat[i] >> 32;
>>>>> +            env->IBAT[0][i] = sregs.ppc32.ibat[i] & 0xffffffff;
>>>>> +            env->IBAT[1][i] = sregs.ppc32.ibat[i] >> 32;
>>>>> +        }
>>>>> +    }
>>>>> +#endif
>>>>> +
>>>>>    return 0;
>>>>> }
>>>>>
>>>> What about KVM_SET_SREGS in kvm_arch_put_registers? E.g. to play back
>>>> potential changes to that special registers someone did via gdb?
>>> I don't think you can actually change the segment values. At least I can't imagine why.
>> Dunno about PPC in this regard and how much value it has, but we have
>> segment register access via gdb for x86.
> 
> The segments here are more like PLM4 on x86.

Even that will be settable one day (gdb just do not yet know much about
x86 system management registers).

> 
>>> I definitely will implement SET_SREGS as soon as your sync split is in, as that's IMHO only really required on migration.
>>>
>> Migration is, of course, the major use case.
>>
>> Still I wonder why not making this API symmetric when already touching it.
> 
> I was afraid to introduce performance regressions - setting the segments means flushing the complete shadow MMU.
> 

Unless it costs milliseconds, not really critical, given how often
registers are synchronized.

BTW, I noticed that ppc only syncs the SREGS once on init, not on reset
- are they static?

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

[Qemu-devel] Re: [PATCH] PPC: Get MMU state on register sync

[Alexander Graf]

On 24.11.2009, at 19:33, Jan Kiszka wrote:

> Alexander Graf wrote:
>> On 24.11.2009, at 19:12, Jan Kiszka wrote:
>> 
>>> Alexander Graf wrote:
>>>> On 24.11.2009, at 19:01, Jan Kiszka wrote:
>>>> 
>>>>> Alexander Graf wrote:
>>>>>> While x86 only needs to sync cr0-4 to know all about its MMU state and enable
>>>>>> qemu to resolve virtual to physical addresses, we need to sync all of the
>>>>>> segment registers on PPC to know which mapping we're in.
>>>>>> 
>>>>>> So let's grab the segment register contents to be able to use the "x" monitor
>>>>>> command and also enable the gdbstub to resolve virtual addresses.
>>>>>> 
>>>>>> I sent the corresponding KVM patch to the KVM ML some minutes ago.
>>>>>> 
>>>>>> Signed-off-by: Alexander Graf <agraf@suse.de>
>>>>>> ---
>>>>>> target-ppc/kvm.c |   30 ++++++++++++++++++++++++++++++
>>>>>> 1 files changed, 30 insertions(+), 0 deletions(-)
>>>>>> 
>>>>>> diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
>>>>>> index 4e1c65f..566513f 100644
>>>>>> --- a/target-ppc/kvm.c
>>>>>> +++ b/target-ppc/kvm.c
>>>>>> @@ -98,12 +98,17 @@ int kvm_arch_put_registers(CPUState *env)
>>>>>> int kvm_arch_get_registers(CPUState *env)
>>>>>> {
>>>>>>   struct kvm_regs regs;
>>>>>> +    struct kvm_sregs sregs;
>>>>>>   uint32_t i, ret;
>>>>>> 
>>>>>>   ret = kvm_vcpu_ioctl(env, KVM_GET_REGS, &regs);
>>>>>>   if (ret < 0)
>>>>>>       return ret;
>>>>>> 
>>>>>> +    ret = kvm_vcpu_ioctl(env, KVM_GET_SREGS, &sregs);
>>>>>> +    if (ret < 0)
>>>>>> +        return ret;
>>>>>> +
>>>>>>   env->ctr = regs.ctr;
>>>>>>   env->lr = regs.lr;
>>>>>>   env->xer = regs.xer;
>>>>>> @@ -125,6 +130,31 @@ int kvm_arch_get_registers(CPUState *env)
>>>>>>   for (i = 0;i < 32; i++)
>>>>>>       env->gpr[i] = regs.gpr[i];
>>>>>> 
>>>>>> +#ifdef KVM_CAP_PPC_SEGSTATE
>>>>>> +    if (kvm_check_extension(env->kvm_state, KVM_CAP_PPC_SEGSTATE)) {
>>>>>> +        env->sdr1 = sregs.sdr1;
>>>>>> +    
>>>>>> +        /* Sync SLB */
>>>>>> +        for (i = 0; i < 64; i++) {
>>>>>> +            ppc_store_slb(env, sregs.ppc64.slb[i].slbe,
>>>>>> +                               sregs.ppc64.slb[i].slbv);
>>>>>> +        }
>>>>>> +    
>>>>>> +        /* Sync SRs */
>>>>>> +        for (i = 0; i < 16; i++) {
>>>>>> +            env->sr[i] = sregs.ppc32.sr[i];
>>>>>> +        }
>>>>>> +    
>>>>>> +        /* Sync BATs */
>>>>>> +        for (i = 0; i < 8; i++) {
>>>>>> +            env->DBAT[0][i] = sregs.ppc32.dbat[i] & 0xffffffff;
>>>>>> +            env->DBAT[1][i] = sregs.ppc32.dbat[i] >> 32;
>>>>>> +            env->IBAT[0][i] = sregs.ppc32.ibat[i] & 0xffffffff;
>>>>>> +            env->IBAT[1][i] = sregs.ppc32.ibat[i] >> 32;
>>>>>> +        }
>>>>>> +    }
>>>>>> +#endif
>>>>>> +
>>>>>>   return 0;
>>>>>> }
>>>>>> 
>>>>> What about KVM_SET_SREGS in kvm_arch_put_registers? E.g. to play back
>>>>> potential changes to that special registers someone did via gdb?
>>>> I don't think you can actually change the segment values. At least I can't imagine why.
>>> Dunno about PPC in this regard and how much value it has, but we have
>>> segment register access via gdb for x86.
>> 
>> The segments here are more like PLM4 on x86.
> 
> Even that will be settable one day (gdb just do not yet know much about
> x86 system management registers).
> 
>> 
>>>> I definitely will implement SET_SREGS as soon as your sync split is in, as that's IMHO only really required on migration.
>>>> 
>>> Migration is, of course, the major use case.
>>> 
>>> Still I wonder why not making this API symmetric when already touching it.
>> 
>> I was afraid to introduce performance regressions - setting the segments means flushing the complete shadow MMU.
>> 
> 
> Unless it costs milliseconds, not really critical, given how often
> registers are synchronized.
> 
> BTW, I noticed that ppc only syncs the SREGS once on init, not on reset
> - are they static?

So far SREGS are only used for setting the PVR (cpuid in x86 speech). There's no need to reset that on reset :-).


Alex

[Qemu-devel] Re: [PATCH] PPC: Get MMU state on register sync

[Jan Kiszka]

Alexander Graf wrote:
> On 24.11.2009, at 19:33, Jan Kiszka wrote:
> 
>> Alexander Graf wrote:
>>> On 24.11.2009, at 19:12, Jan Kiszka wrote:
>>>
>>>> Alexander Graf wrote:
>>>>> On 24.11.2009, at 19:01, Jan Kiszka wrote:
>>>>>
>>>>>> Alexander Graf wrote:
>>>>>>> While x86 only needs to sync cr0-4 to know all about its MMU state and enable
>>>>>>> qemu to resolve virtual to physical addresses, we need to sync all of the
>>>>>>> segment registers on PPC to know which mapping we're in.
>>>>>>>
>>>>>>> So let's grab the segment register contents to be able to use the "x" monitor
>>>>>>> command and also enable the gdbstub to resolve virtual addresses.
>>>>>>>
>>>>>>> I sent the corresponding KVM patch to the KVM ML some minutes ago.
>>>>>>>
>>>>>>> Signed-off-by: Alexander Graf <agraf@suse.de>
>>>>>>> ---
>>>>>>> target-ppc/kvm.c |   30 ++++++++++++++++++++++++++++++
>>>>>>> 1 files changed, 30 insertions(+), 0 deletions(-)
>>>>>>>
>>>>>>> diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
>>>>>>> index 4e1c65f..566513f 100644
>>>>>>> --- a/target-ppc/kvm.c
>>>>>>> +++ b/target-ppc/kvm.c
>>>>>>> @@ -98,12 +98,17 @@ int kvm_arch_put_registers(CPUState *env)
>>>>>>> int kvm_arch_get_registers(CPUState *env)
>>>>>>> {
>>>>>>>   struct kvm_regs regs;
>>>>>>> +    struct kvm_sregs sregs;
>>>>>>>   uint32_t i, ret;
>>>>>>>
>>>>>>>   ret = kvm_vcpu_ioctl(env, KVM_GET_REGS, &regs);
>>>>>>>   if (ret < 0)
>>>>>>>       return ret;
>>>>>>>
>>>>>>> +    ret = kvm_vcpu_ioctl(env, KVM_GET_SREGS, &sregs);
>>>>>>> +    if (ret < 0)
>>>>>>> +        return ret;
>>>>>>> +
>>>>>>>   env->ctr = regs.ctr;
>>>>>>>   env->lr = regs.lr;
>>>>>>>   env->xer = regs.xer;
>>>>>>> @@ -125,6 +130,31 @@ int kvm_arch_get_registers(CPUState *env)
>>>>>>>   for (i = 0;i < 32; i++)
>>>>>>>       env->gpr[i] = regs.gpr[i];
>>>>>>>
>>>>>>> +#ifdef KVM_CAP_PPC_SEGSTATE
>>>>>>> +    if (kvm_check_extension(env->kvm_state, KVM_CAP_PPC_SEGSTATE)) {
>>>>>>> +        env->sdr1 = sregs.sdr1;
>>>>>>> +    
>>>>>>> +        /* Sync SLB */
>>>>>>> +        for (i = 0; i < 64; i++) {
>>>>>>> +            ppc_store_slb(env, sregs.ppc64.slb[i].slbe,
>>>>>>> +                               sregs.ppc64.slb[i].slbv);
>>>>>>> +        }
>>>>>>> +    
>>>>>>> +        /* Sync SRs */
>>>>>>> +        for (i = 0; i < 16; i++) {
>>>>>>> +            env->sr[i] = sregs.ppc32.sr[i];
>>>>>>> +        }
>>>>>>> +    
>>>>>>> +        /* Sync BATs */
>>>>>>> +        for (i = 0; i < 8; i++) {
>>>>>>> +            env->DBAT[0][i] = sregs.ppc32.dbat[i] & 0xffffffff;
>>>>>>> +            env->DBAT[1][i] = sregs.ppc32.dbat[i] >> 32;
>>>>>>> +            env->IBAT[0][i] = sregs.ppc32.ibat[i] & 0xffffffff;
>>>>>>> +            env->IBAT[1][i] = sregs.ppc32.ibat[i] >> 32;
>>>>>>> +        }
>>>>>>> +    }
>>>>>>> +#endif
>>>>>>> +
>>>>>>>   return 0;
>>>>>>> }
>>>>>>>
>>>>>> What about KVM_SET_SREGS in kvm_arch_put_registers? E.g. to play back
>>>>>> potential changes to that special registers someone did via gdb?
>>>>> I don't think you can actually change the segment values. At least I can't imagine why.
>>>> Dunno about PPC in this regard and how much value it has, but we have
>>>> segment register access via gdb for x86.
>>> The segments here are more like PLM4 on x86.
>> Even that will be settable one day (gdb just do not yet know much about
>> x86 system management registers).
>>
>>>>> I definitely will implement SET_SREGS as soon as your sync split is in, as that's IMHO only really required on migration.
>>>>>
>>>> Migration is, of course, the major use case.
>>>>
>>>> Still I wonder why not making this API symmetric when already touching it.
>>> I was afraid to introduce performance regressions - setting the segments means flushing the complete shadow MMU.
>>>
>> Unless it costs milliseconds, not really critical, given how often
>> registers are synchronized.
>>
>> BTW, I noticed that ppc only syncs the SREGS once on init, not on reset
>> - are they static?
> 
> So far SREGS are only used for setting the PVR (cpuid in x86 speech). There's no need to reset that on reset :-).

Then I don't get why you need to re-read them during runtime - user
space should know the state and should be able push it into the CPUState
on init.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

[Qemu-devel] Re: [PATCH] PPC: Get MMU state on register sync

[Alexander Graf]

On 24.11.2009, at 19:49, Jan Kiszka wrote:

> Alexander Graf wrote:
>> On 24.11.2009, at 19:33, Jan Kiszka wrote:
>> 
>>> Alexander Graf wrote:
>>>> On 24.11.2009, at 19:12, Jan Kiszka wrote:
>>>> 
>>>>> Alexander Graf wrote:
>>>>>> On 24.11.2009, at 19:01, Jan Kiszka wrote:
>>>>>> 
>>>>>>> Alexander Graf wrote:
>>>>>>>> While x86 only needs to sync cr0-4 to know all about its MMU state and enable
>>>>>>>> qemu to resolve virtual to physical addresses, we need to sync all of the
>>>>>>>> segment registers on PPC to know which mapping we're in.
>>>>>>>> 
>>>>>>>> So let's grab the segment register contents to be able to use the "x" monitor
>>>>>>>> command and also enable the gdbstub to resolve virtual addresses.
>>>>>>>> 
>>>>>>>> I sent the corresponding KVM patch to the KVM ML some minutes ago.
>>>>>>>> 
>>>>>>>> Signed-off-by: Alexander Graf <agraf@suse.de>
>>>>>>>> ---
>>>>>>>> target-ppc/kvm.c |   30 ++++++++++++++++++++++++++++++
>>>>>>>> 1 files changed, 30 insertions(+), 0 deletions(-)
>>>>>>>> 
>>>>>>>> diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
>>>>>>>> index 4e1c65f..566513f 100644
>>>>>>>> --- a/target-ppc/kvm.c
>>>>>>>> +++ b/target-ppc/kvm.c
>>>>>>>> @@ -98,12 +98,17 @@ int kvm_arch_put_registers(CPUState *env)
>>>>>>>> int kvm_arch_get_registers(CPUState *env)
>>>>>>>> {
>>>>>>>>  struct kvm_regs regs;
>>>>>>>> +    struct kvm_sregs sregs;
>>>>>>>>  uint32_t i, ret;
>>>>>>>> 
>>>>>>>>  ret = kvm_vcpu_ioctl(env, KVM_GET_REGS, &regs);
>>>>>>>>  if (ret < 0)
>>>>>>>>      return ret;
>>>>>>>> 
>>>>>>>> +    ret = kvm_vcpu_ioctl(env, KVM_GET_SREGS, &sregs);
>>>>>>>> +    if (ret < 0)
>>>>>>>> +        return ret;
>>>>>>>> +
>>>>>>>>  env->ctr = regs.ctr;
>>>>>>>>  env->lr = regs.lr;
>>>>>>>>  env->xer = regs.xer;
>>>>>>>> @@ -125,6 +130,31 @@ int kvm_arch_get_registers(CPUState *env)
>>>>>>>>  for (i = 0;i < 32; i++)
>>>>>>>>      env->gpr[i] = regs.gpr[i];
>>>>>>>> 
>>>>>>>> +#ifdef KVM_CAP_PPC_SEGSTATE
>>>>>>>> +    if (kvm_check_extension(env->kvm_state, KVM_CAP_PPC_SEGSTATE)) {
>>>>>>>> +        env->sdr1 = sregs.sdr1;
>>>>>>>> +    
>>>>>>>> +        /* Sync SLB */
>>>>>>>> +        for (i = 0; i < 64; i++) {
>>>>>>>> +            ppc_store_slb(env, sregs.ppc64.slb[i].slbe,
>>>>>>>> +                               sregs.ppc64.slb[i].slbv);
>>>>>>>> +        }
>>>>>>>> +    
>>>>>>>> +        /* Sync SRs */
>>>>>>>> +        for (i = 0; i < 16; i++) {
>>>>>>>> +            env->sr[i] = sregs.ppc32.sr[i];
>>>>>>>> +        }
>>>>>>>> +    
>>>>>>>> +        /* Sync BATs */
>>>>>>>> +        for (i = 0; i < 8; i++) {
>>>>>>>> +            env->DBAT[0][i] = sregs.ppc32.dbat[i] & 0xffffffff;
>>>>>>>> +            env->DBAT[1][i] = sregs.ppc32.dbat[i] >> 32;
>>>>>>>> +            env->IBAT[0][i] = sregs.ppc32.ibat[i] & 0xffffffff;
>>>>>>>> +            env->IBAT[1][i] = sregs.ppc32.ibat[i] >> 32;
>>>>>>>> +        }
>>>>>>>> +    }
>>>>>>>> +#endif
>>>>>>>> +
>>>>>>>>  return 0;
>>>>>>>> }
>>>>>>>> 
>>>>>>> What about KVM_SET_SREGS in kvm_arch_put_registers? E.g. to play back
>>>>>>> potential changes to that special registers someone did via gdb?
>>>>>> I don't think you can actually change the segment values. At least I can't imagine why.
>>>>> Dunno about PPC in this regard and how much value it has, but we have
>>>>> segment register access via gdb for x86.
>>>> The segments here are more like PLM4 on x86.
>>> Even that will be settable one day (gdb just do not yet know much about
>>> x86 system management registers).
>>> 
>>>>>> I definitely will implement SET_SREGS as soon as your sync split is in, as that's IMHO only really required on migration.
>>>>>> 
>>>>> Migration is, of course, the major use case.
>>>>> 
>>>>> Still I wonder why not making this API symmetric when already touching it.
>>>> I was afraid to introduce performance regressions - setting the segments means flushing the complete shadow MMU.
>>>> 
>>> Unless it costs milliseconds, not really critical, given how often
>>> registers are synchronized.
>>> 
>>> BTW, I noticed that ppc only syncs the SREGS once on init, not on reset
>>> - are they static?
>> 
>> So far SREGS are only used for setting the PVR (cpuid in x86 speech). There's no need to reset that on reset :-).
> 
> Then I don't get why you need to re-read them during runtime - user
> space should know the state and should be able push it into the CPUState
> on init.

Eeh. The SREGS contain:

- PVR
- Segment register contents
- BATs (another MMU thing for linear direct mapping)

On init we send SREGS to set PVR. Later on sync we get SREGS to get the segment registers.

You think it would have been better to create a new ioctl?

Alex

[Qemu-devel] Re: [PATCH] PPC: Get MMU state on register sync

[Jan Kiszka]

Alexander Graf wrote:
> On 24.11.2009, at 19:49, Jan Kiszka wrote:
> 
>> Alexander Graf wrote:
>>> On 24.11.2009, at 19:33, Jan Kiszka wrote:
>>>
>>>> Alexander Graf wrote:
>>>>> On 24.11.2009, at 19:12, Jan Kiszka wrote:
>>>>>
>>>>>> Alexander Graf wrote:
>>>>>>> On 24.11.2009, at 19:01, Jan Kiszka wrote:
>>>>>>>
>>>>>>>> Alexander Graf wrote:
>>>>>>>>> While x86 only needs to sync cr0-4 to know all about its MMU state and enable
>>>>>>>>> qemu to resolve virtual to physical addresses, we need to sync all of the
>>>>>>>>> segment registers on PPC to know which mapping we're in.
>>>>>>>>>
>>>>>>>>> So let's grab the segment register contents to be able to use the "x" monitor
>>>>>>>>> command and also enable the gdbstub to resolve virtual addresses.
>>>>>>>>>
>>>>>>>>> I sent the corresponding KVM patch to the KVM ML some minutes ago.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Alexander Graf <agraf@suse.de>
>>>>>>>>> ---
>>>>>>>>> target-ppc/kvm.c |   30 ++++++++++++++++++++++++++++++
>>>>>>>>> 1 files changed, 30 insertions(+), 0 deletions(-)
>>>>>>>>>
>>>>>>>>> diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
>>>>>>>>> index 4e1c65f..566513f 100644
>>>>>>>>> --- a/target-ppc/kvm.c
>>>>>>>>> +++ b/target-ppc/kvm.c
>>>>>>>>> @@ -98,12 +98,17 @@ int kvm_arch_put_registers(CPUState *env)
>>>>>>>>> int kvm_arch_get_registers(CPUState *env)
>>>>>>>>> {
>>>>>>>>>  struct kvm_regs regs;
>>>>>>>>> +    struct kvm_sregs sregs;
>>>>>>>>>  uint32_t i, ret;
>>>>>>>>>
>>>>>>>>>  ret = kvm_vcpu_ioctl(env, KVM_GET_REGS, &regs);
>>>>>>>>>  if (ret < 0)
>>>>>>>>>      return ret;
>>>>>>>>>
>>>>>>>>> +    ret = kvm_vcpu_ioctl(env, KVM_GET_SREGS, &sregs);
>>>>>>>>> +    if (ret < 0)
>>>>>>>>> +        return ret;
>>>>>>>>> +
>>>>>>>>>  env->ctr = regs.ctr;
>>>>>>>>>  env->lr = regs.lr;
>>>>>>>>>  env->xer = regs.xer;
>>>>>>>>> @@ -125,6 +130,31 @@ int kvm_arch_get_registers(CPUState *env)
>>>>>>>>>  for (i = 0;i < 32; i++)
>>>>>>>>>      env->gpr[i] = regs.gpr[i];
>>>>>>>>>
>>>>>>>>> +#ifdef KVM_CAP_PPC_SEGSTATE
>>>>>>>>> +    if (kvm_check_extension(env->kvm_state, KVM_CAP_PPC_SEGSTATE)) {
>>>>>>>>> +        env->sdr1 = sregs.sdr1;
>>>>>>>>> +    
>>>>>>>>> +        /* Sync SLB */
>>>>>>>>> +        for (i = 0; i < 64; i++) {
>>>>>>>>> +            ppc_store_slb(env, sregs.ppc64.slb[i].slbe,
>>>>>>>>> +                               sregs.ppc64.slb[i].slbv);
>>>>>>>>> +        }
>>>>>>>>> +    
>>>>>>>>> +        /* Sync SRs */
>>>>>>>>> +        for (i = 0; i < 16; i++) {
>>>>>>>>> +            env->sr[i] = sregs.ppc32.sr[i];
>>>>>>>>> +        }
>>>>>>>>> +    
>>>>>>>>> +        /* Sync BATs */
>>>>>>>>> +        for (i = 0; i < 8; i++) {
>>>>>>>>> +            env->DBAT[0][i] = sregs.ppc32.dbat[i] & 0xffffffff;
>>>>>>>>> +            env->DBAT[1][i] = sregs.ppc32.dbat[i] >> 32;
>>>>>>>>> +            env->IBAT[0][i] = sregs.ppc32.ibat[i] & 0xffffffff;
>>>>>>>>> +            env->IBAT[1][i] = sregs.ppc32.ibat[i] >> 32;
>>>>>>>>> +        }
>>>>>>>>> +    }
>>>>>>>>> +#endif
>>>>>>>>> +
>>>>>>>>>  return 0;
>>>>>>>>> }
>>>>>>>>>
>>>>>>>> What about KVM_SET_SREGS in kvm_arch_put_registers? E.g. to play back
>>>>>>>> potential changes to that special registers someone did via gdb?
>>>>>>> I don't think you can actually change the segment values. At least I can't imagine why.
>>>>>> Dunno about PPC in this regard and how much value it has, but we have
>>>>>> segment register access via gdb for x86.
>>>>> The segments here are more like PLM4 on x86.
>>>> Even that will be settable one day (gdb just do not yet know much about
>>>> x86 system management registers).
>>>>
>>>>>>> I definitely will implement SET_SREGS as soon as your sync split is in, as that's IMHO only really required on migration.
>>>>>>>
>>>>>> Migration is, of course, the major use case.
>>>>>>
>>>>>> Still I wonder why not making this API symmetric when already touching it.
>>>>> I was afraid to introduce performance regressions - setting the segments means flushing the complete shadow MMU.
>>>>>
>>>> Unless it costs milliseconds, not really critical, given how often
>>>> registers are synchronized.
>>>>
>>>> BTW, I noticed that ppc only syncs the SREGS once on init, not on reset
>>>> - are they static?
>>> So far SREGS are only used for setting the PVR (cpuid in x86 speech). There's no need to reset that on reset :-).
>> Then I don't get why you need to re-read them during runtime - user
>> space should know the state and should be able push it into the CPUState
>> on init.
> 
> Eeh. The SREGS contain:
> 
> - PVR
> - Segment register contents
> - BATs (another MMU thing for linear direct mapping)
> 
> On init we send SREGS to set PVR. Later on sync we get SREGS to get the segment registers.
> 
> You think it would have been better to create a new ioctl?

No, but I think you might miss a proper reset of some SREGS elements
when the VM goes through reset. If those states may change during guest
runtime, a hard reset should send them back into their hard reset state.

You can do this by adding yet another extraordinary SET_SREGS to the
reset callback or - that was my original point - by symmetrically adding
GET_SREGS and SET_SREGS to the register state sync.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

[Qemu-devel] Re: [PATCH] PPC: Get MMU state on register sync

[Alexander Graf]

On 24.11.2009, at 20:03, Jan Kiszka wrote:

> Alexander Graf wrote:
>> On 24.11.2009, at 19:49, Jan Kiszka wrote:
>> 
>>> Alexander Graf wrote:
>>>> On 24.11.2009, at 19:33, Jan Kiszka wrote:
>>>> 
>>>>> Alexander Graf wrote:
>>>>>> On 24.11.2009, at 19:12, Jan Kiszka wrote:
>>>>>> 
>>>>>>> Alexander Graf wrote:
>>>>>>>> On 24.11.2009, at 19:01, Jan Kiszka wrote:
>>>>>>>> 
>>>>>>>>> Alexander Graf wrote:
>>>>>>>>>> While x86 only needs to sync cr0-4 to know all about its MMU state and enable
>>>>>>>>>> qemu to resolve virtual to physical addresses, we need to sync all of the
>>>>>>>>>> segment registers on PPC to know which mapping we're in.
>>>>>>>>>> 
>>>>>>>>>> So let's grab the segment register contents to be able to use the "x" monitor
>>>>>>>>>> command and also enable the gdbstub to resolve virtual addresses.
>>>>>>>>>> 
>>>>>>>>>> I sent the corresponding KVM patch to the KVM ML some minutes ago.
>>>>>>>>>> 
>>>>>>>>>> Signed-off-by: Alexander Graf <agraf@suse.de>
>>>>>>>>>> ---
>>>>>>>>>> target-ppc/kvm.c |   30 ++++++++++++++++++++++++++++++
>>>>>>>>>> 1 files changed, 30 insertions(+), 0 deletions(-)
>>>>>>>>>> 
>>>>>>>>>> diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
>>>>>>>>>> index 4e1c65f..566513f 100644
>>>>>>>>>> --- a/target-ppc/kvm.c
>>>>>>>>>> +++ b/target-ppc/kvm.c
>>>>>>>>>> @@ -98,12 +98,17 @@ int kvm_arch_put_registers(CPUState *env)
>>>>>>>>>> int kvm_arch_get_registers(CPUState *env)
>>>>>>>>>> {
>>>>>>>>>> struct kvm_regs regs;
>>>>>>>>>> +    struct kvm_sregs sregs;
>>>>>>>>>> uint32_t i, ret;
>>>>>>>>>> 
>>>>>>>>>> ret = kvm_vcpu_ioctl(env, KVM_GET_REGS, &regs);
>>>>>>>>>> if (ret < 0)
>>>>>>>>>>     return ret;
>>>>>>>>>> 
>>>>>>>>>> +    ret = kvm_vcpu_ioctl(env, KVM_GET_SREGS, &sregs);
>>>>>>>>>> +    if (ret < 0)
>>>>>>>>>> +        return ret;
>>>>>>>>>> +
>>>>>>>>>> env->ctr = regs.ctr;
>>>>>>>>>> env->lr = regs.lr;
>>>>>>>>>> env->xer = regs.xer;
>>>>>>>>>> @@ -125,6 +130,31 @@ int kvm_arch_get_registers(CPUState *env)
>>>>>>>>>> for (i = 0;i < 32; i++)
>>>>>>>>>>     env->gpr[i] = regs.gpr[i];
>>>>>>>>>> 
>>>>>>>>>> +#ifdef KVM_CAP_PPC_SEGSTATE
>>>>>>>>>> +    if (kvm_check_extension(env->kvm_state, KVM_CAP_PPC_SEGSTATE)) {
>>>>>>>>>> +        env->sdr1 = sregs.sdr1;
>>>>>>>>>> +    
>>>>>>>>>> +        /* Sync SLB */
>>>>>>>>>> +        for (i = 0; i < 64; i++) {
>>>>>>>>>> +            ppc_store_slb(env, sregs.ppc64.slb[i].slbe,
>>>>>>>>>> +                               sregs.ppc64.slb[i].slbv);
>>>>>>>>>> +        }
>>>>>>>>>> +    
>>>>>>>>>> +        /* Sync SRs */
>>>>>>>>>> +        for (i = 0; i < 16; i++) {
>>>>>>>>>> +            env->sr[i] = sregs.ppc32.sr[i];
>>>>>>>>>> +        }
>>>>>>>>>> +    
>>>>>>>>>> +        /* Sync BATs */
>>>>>>>>>> +        for (i = 0; i < 8; i++) {
>>>>>>>>>> +            env->DBAT[0][i] = sregs.ppc32.dbat[i] & 0xffffffff;
>>>>>>>>>> +            env->DBAT[1][i] = sregs.ppc32.dbat[i] >> 32;
>>>>>>>>>> +            env->IBAT[0][i] = sregs.ppc32.ibat[i] & 0xffffffff;
>>>>>>>>>> +            env->IBAT[1][i] = sregs.ppc32.ibat[i] >> 32;
>>>>>>>>>> +        }
>>>>>>>>>> +    }
>>>>>>>>>> +#endif
>>>>>>>>>> +
>>>>>>>>>> return 0;
>>>>>>>>>> }
>>>>>>>>>> 
>>>>>>>>> What about KVM_SET_SREGS in kvm_arch_put_registers? E.g. to play back
>>>>>>>>> potential changes to that special registers someone did via gdb?
>>>>>>>> I don't think you can actually change the segment values. At least I can't imagine why.
>>>>>>> Dunno about PPC in this regard and how much value it has, but we have
>>>>>>> segment register access via gdb for x86.
>>>>>> The segments here are more like PLM4 on x86.
>>>>> Even that will be settable one day (gdb just do not yet know much about
>>>>> x86 system management registers).
>>>>> 
>>>>>>>> I definitely will implement SET_SREGS as soon as your sync split is in, as that's IMHO only really required on migration.
>>>>>>>> 
>>>>>>> Migration is, of course, the major use case.
>>>>>>> 
>>>>>>> Still I wonder why not making this API symmetric when already touching it.
>>>>>> I was afraid to introduce performance regressions - setting the segments means flushing the complete shadow MMU.
>>>>>> 
>>>>> Unless it costs milliseconds, not really critical, given how often
>>>>> registers are synchronized.
>>>>> 
>>>>> BTW, I noticed that ppc only syncs the SREGS once on init, not on reset
>>>>> - are they static?
>>>> So far SREGS are only used for setting the PVR (cpuid in x86 speech). There's no need to reset that on reset :-).
>>> Then I don't get why you need to re-read them during runtime - user
>>> space should know the state and should be able push it into the CPUState
>>> on init.
>> 
>> Eeh. The SREGS contain:
>> 
>> - PVR
>> - Segment register contents
>> - BATs (another MMU thing for linear direct mapping)
>> 
>> On init we send SREGS to set PVR. Later on sync we get SREGS to get the segment registers.
>> 
>> You think it would have been better to create a new ioctl?
> 
> No, but I think you might miss a proper reset of some SREGS elements
> when the VM goes through reset. If those states may change during guest
> runtime, a hard reset should send them back into their hard reset state.
> 
> You can do this by adding yet another extraordinary SET_SREGS to the
> reset callback or - that was my original point - by symmetrically adding
> GET_SREGS and SET_SREGS to the register state sync.

At least with the firmware we have now, we start in real mode anyways, then set all 16 SRs (or 16 SLB fake entries) and then go into paged mode.

So we're safe for now. As I said I will implement the SET_SREGS in the sync when your split is done and I can limit sregs sets to reset.

Alex

[Qemu-devel] [PATCH] PPC: Sync CPU state for KVM

[Alexander Graf]

Some recent change made PPC guests always start at address 0x0 because env
isn't synced to kvm_state on first bootup.

I'm not sure if this is the correct bugfix, but at least it makes PPC boot
again with KVM enabled.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 hw/ppc_newworld.c |    4 ++++
 hw/ppc_oldworld.c |    4 ++++
 2 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/hw/ppc_newworld.c b/hw/ppc_newworld.c
index da868d3..2f2f091 100644
--- a/hw/ppc_newworld.c
+++ b/hw/ppc_newworld.c
@@ -38,6 +38,7 @@
 #include "ide.h"
 #include "loader.h"
 #include "elf.h"
+#include "kvm.h"
 
 #define MAX_IDE_BUS 2
 #define VGA_BIOS_SIZE 65536
@@ -134,6 +135,9 @@ static void ppc_core99_init (ram_addr_t ram_size,
         envs[i] = env;
     }
 
+    /* Make sure all register sets take effect */
+    cpu_synchronize_state(env);
+
     /* allocate RAM */
     ram_offset = qemu_ram_alloc(ram_size);
     cpu_register_physical_memory(0, ram_size, ram_offset);
diff --git a/hw/ppc_oldworld.c b/hw/ppc_oldworld.c
index 9b49a3d..4d89805 100644
--- a/hw/ppc_oldworld.c
+++ b/hw/ppc_oldworld.c
@@ -38,6 +38,7 @@
 #include "ide.h"
 #include "loader.h"
 #include "elf.h"
+#include "kvm.h"
 
 #define MAX_IDE_BUS 2
 #define VGA_BIOS_SIZE 65536
@@ -162,6 +163,9 @@ static void ppc_heathrow_init (ram_addr_t ram_size,
         envs[i] = env;
     }
 
+    /* Make sure all register sets take effect */
+    cpu_synchronize_state(env);
+
     /* allocate RAM */
     if (ram_size > (2047 << 20)) {
         fprintf(stderr,
-- 
1.6.0.2