* [Qemu-devel] [PATCH] s390: use sync regs for register transfer
@ 2012-08-22 11:54 Jens Freimann
2012-09-20 12:19 ` Alexander Graf
0 siblings, 1 reply; 6+ messages in thread
From: Jens Freimann @ 2012-08-22 11:54 UTC (permalink / raw)
To: Alexander Graf
Cc: Heinz Graalfs, qemu-devel, Christian Borntraeger, Jens Freimann,
Cornelia Huck, Einar Lueck
From: Christian Borntraeger <borntraeger@de.ibm.com>
Newer kernels provide the guest registers in kvm_run. Lets use
those if available. This avoids ioctls on cpu_synchronize_state
making intercepts faster.
In addition, we have now the prefix register, the access registers
the control registers up to date. This helps in certain cases,
e.g. for resolving kernel module addresses with gdb on a guest.
On return, we only update the gprs, since qemu does not change
prefix, crs and acrs. Blindly updating those might cause some
expensive flushing in the kernel.
Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
Signed-off-by: Jens Freimann <jfrei@linux.vnet.ibm.com>
---
target-s390x/kvm.c | 75 +++++++++++++++++++++++++++++++++++++-----------------
1 file changed, 51 insertions(+), 24 deletions(-)
diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c
index 07edf93..4598923 100644
--- a/target-s390x/kvm.c
+++ b/target-s390x/kvm.c
@@ -88,50 +88,77 @@ void kvm_arch_reset_vcpu(CPUS390XState *env)
/* FIXME: add code to reset vcpu. */
}
+/* we want to have the prefix, the GPRS, the ACRS and the CRS up to date */
+#define QEMU_NEEDED_REGS (KVM_SYNC_PREFIX | KVM_SYNC_GPRS | \
+ KVM_SYNC_ACRS | KVM_SYNC_CRS)
+
+/* But qemu only changes the GPRS */
+#define QEMU_DIRTY_REGS (KVM_SYNC_GPRS)
+
int kvm_arch_put_registers(CPUS390XState *env, int level)
{
struct kvm_regs regs;
int ret;
int i;
- ret = kvm_vcpu_ioctl(env, KVM_GET_REGS, ®s);
- if (ret < 0) {
- return ret;
- }
-
- for (i = 0; i < 16; i++) {
- regs.gprs[i] = env->regs[i];
- }
-
- ret = kvm_vcpu_ioctl(env, KVM_SET_REGS, ®s);
- if (ret < 0) {
- return ret;
- }
-
env->kvm_run->psw_addr = env->psw.addr;
env->kvm_run->psw_mask = env->psw.mask;
- return ret;
+ if ((env->kvm_run->kvm_valid_regs & QEMU_NEEDED_REGS) == QEMU_NEEDED_REGS) {
+ env->kvm_run->s.regs.prefix = env->psa;
+ for (i = 0; i < 16; i++) {
+ env->kvm_run->s.regs.gprs[i] = env->regs[i];
+ env->kvm_run->kvm_dirty_regs |= QEMU_DIRTY_REGS;
+ }
+ } else {
+ for (i = 0; i < 16; i++) {
+ regs.gprs[i] = env->regs[i];
+ }
+ ret = kvm_vcpu_ioctl(env, KVM_SET_REGS, ®s);
+ if (ret < 0) {
+ return ret;
+ }
+ /* no prefix available */
+ }
+ /* sregs unchanged */
+
+ return 0;
}
int kvm_arch_get_registers(CPUS390XState *env)
{
int ret;
struct kvm_regs regs;
+ struct kvm_sregs sregs;
int i;
- ret = kvm_vcpu_ioctl(env, KVM_GET_REGS, ®s);
- if (ret < 0) {
- return ret;
- }
-
- for (i = 0; i < 16; i++) {
- env->regs[i] = regs.gprs[i];
- }
-
env->psw.addr = env->kvm_run->psw_addr;
env->psw.mask = env->kvm_run->psw_mask;
+ if ((env->kvm_run->kvm_valid_regs & QEMU_NEEDED_REGS) == QEMU_NEEDED_REGS) {
+ env->psa = env->kvm_run->s.regs.prefix;
+ for (i = 0; i < 16; i++) {
+ env->regs[i] = env->kvm_run->s.regs.gprs[i];
+ env->cregs[i] = env->kvm_run->s.regs.crs[i];
+ env->aregs[i] = env->kvm_run->s.regs.acrs[i];
+ }
+ } else {
+ ret = kvm_vcpu_ioctl(env, KVM_GET_REGS, ®s);
+ if (ret < 0) {
+ return ret;
+ }
+ ret = kvm_vcpu_ioctl(env, KVM_GET_SREGS, &sregs);
+ if (ret < 0) {
+ return ret;
+ }
+ for (i = 0; i < 16; i++) {
+ env->regs[i] = regs.gprs[i];
+ env->cregs[i] = sregs.crs[i];
+ env->aregs[i] = sregs.acrs[i];
+ }
+ /* no prefix available */
+ }
+
return 0;
}
--
1.7.11.5
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [Qemu-devel] [PATCH] s390: use sync regs for register transfer
2012-08-22 11:54 [Qemu-devel] [PATCH] s390: use sync regs for register transfer Jens Freimann
@ 2012-09-20 12:19 ` Alexander Graf
2012-09-20 12:49 ` Christian Borntraeger
0 siblings, 1 reply; 6+ messages in thread
From: Alexander Graf @ 2012-09-20 12:19 UTC (permalink / raw)
To: Jens Freimann
Cc: Cornelia Huck, Christian Borntraeger, Heinz Graalfs, qemu-devel,
Einar Lueck
On 22.08.2012, at 13:54, Jens Freimann wrote:
> From: Christian Borntraeger <borntraeger@de.ibm.com>
>
> Newer kernels provide the guest registers in kvm_run. Lets use
> those if available. This avoids ioctls on cpu_synchronize_state
> making intercepts faster.
>
> In addition, we have now the prefix register, the access registers
> the control registers up to date. This helps in certain cases,
> e.g. for resolving kernel module addresses with gdb on a guest.
>
> On return, we only update the gprs, since qemu does not change
> prefix, crs and acrs. Blindly updating those might cause some
> expensive flushing in the kernel.
>
> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
> Signed-off-by: Jens Freimann <jfrei@linux.vnet.ibm.com>
> ---
> target-s390x/kvm.c | 75 +++++++++++++++++++++++++++++++++++++-----------------
> 1 file changed, 51 insertions(+), 24 deletions(-)
>
> diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c
> index 07edf93..4598923 100644
> --- a/target-s390x/kvm.c
> +++ b/target-s390x/kvm.c
> @@ -88,50 +88,77 @@ void kvm_arch_reset_vcpu(CPUS390XState *env)
> /* FIXME: add code to reset vcpu. */
> }
>
> +/* we want to have the prefix, the GPRS, the ACRS and the CRS up to date */
> +#define QEMU_NEEDED_REGS (KVM_SYNC_PREFIX | KVM_SYNC_GPRS | \
> + KVM_SYNC_ACRS | KVM_SYNC_CRS)
> +
> +/* But qemu only changes the GPRS */
> +#define QEMU_DIRTY_REGS (KVM_SYNC_GPRS)
> +
> int kvm_arch_put_registers(CPUS390XState *env, int level)
> {
> struct kvm_regs regs;
> int ret;
> int i;
>
> - ret = kvm_vcpu_ioctl(env, KVM_GET_REGS, ®s);
> - if (ret < 0) {
> - return ret;
> - }
> -
> - for (i = 0; i < 16; i++) {
> - regs.gprs[i] = env->regs[i];
> - }
> -
> - ret = kvm_vcpu_ioctl(env, KVM_SET_REGS, ®s);
> - if (ret < 0) {
> - return ret;
> - }
> -
> env->kvm_run->psw_addr = env->psw.addr;
> env->kvm_run->psw_mask = env->psw.mask;
>
> - return ret;
> + if ((env->kvm_run->kvm_valid_regs & QEMU_NEEDED_REGS) == QEMU_NEEDED_REGS) {
Isn't this also missing a check for KVM_CAP_SYNC_REGS?
Also, on reset we probably also want to write the other registers back, right?
Alex
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [Qemu-devel] [PATCH] s390: use sync regs for register transfer
2012-09-20 12:19 ` Alexander Graf
@ 2012-09-20 12:49 ` Christian Borntraeger
2012-09-20 14:13 ` Alexander Graf
0 siblings, 1 reply; 6+ messages in thread
From: Christian Borntraeger @ 2012-09-20 12:49 UTC (permalink / raw)
To: Alexander Graf
Cc: Cornelia Huck, Jens Freimann, Heinz Graalfs, qemu-devel, Einar Lueck
[...]
>> --- a/target-s390x/kvm.c
>> +++ b/target-s390x/kvm.c
>> @@ -88,50 +88,77 @@ void kvm_arch_reset_vcpu(CPUS390XState *env)
>> /* FIXME: add code to reset vcpu. */
>> }
>>
>> +/* we want to have the prefix, the GPRS, the ACRS and the CRS up to date */
>> +#define QEMU_NEEDED_REGS (KVM_SYNC_PREFIX | KVM_SYNC_GPRS | \
>> + KVM_SYNC_ACRS | KVM_SYNC_CRS)
>> +
>> +/* But qemu only changes the GPRS */
>> +#define QEMU_DIRTY_REGS (KVM_SYNC_GPRS)
>> +
>> int kvm_arch_put_registers(CPUS390XState *env, int level)
>> {
>> struct kvm_regs regs;
>> int ret;
>> int i;
>>
>> - ret = kvm_vcpu_ioctl(env, KVM_GET_REGS, ®s);
>> - if (ret < 0) {
>> - return ret;
>> - }
>> -
>> - for (i = 0; i < 16; i++) {
>> - regs.gprs[i] = env->regs[i];
>> - }
>> -
>> - ret = kvm_vcpu_ioctl(env, KVM_SET_REGS, ®s);
>> - if (ret < 0) {
>> - return ret;
>> - }
>> -
>> env->kvm_run->psw_addr = env->psw.addr;
>> env->kvm_run->psw_mask = env->psw.mask;
>>
>> - return ret;
>> + if ((env->kvm_run->kvm_valid_regs & QEMU_NEEDED_REGS) == QEMU_NEEDED_REGS) {
>
> Isn't this also missing a check for KVM_CAP_SYNC_REGS?
I ommitted the check for two reasons:
- since kvm_check_extension is an ioctl by itself, this would counter the original idea of sync regs
- the kvm_run structure contains zeroes at the sync reg location in kernels that dont support that,
(the area was unused and the page was allocated with GFP_ZERO)
So by having any of the kvm_valid_regs != 0 we know that SYNC REGS must be supported
> Also, on reset we probably also want to write the other registers back, right?
Well, on reset we call the initial cpu reset ioctl, which does reset the other registers mandated
by a cpu reset. Furthermore, it is not different to the current implementation....
What we might want to do is to also consider a clear reset (which also zeroes the FPRs and ARs as
well as memory etc) as an additional option. But this should not be the default, e.g. to make
kdump or standalone dump possible.
I think this would then be an addon-patch by itself.
> Alex
Christian
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [Qemu-devel] [PATCH] s390: use sync regs for register transfer
2012-09-20 12:49 ` Christian Borntraeger
@ 2012-09-20 14:13 ` Alexander Graf
2012-09-20 15:21 ` Christian Borntraeger
0 siblings, 1 reply; 6+ messages in thread
From: Alexander Graf @ 2012-09-20 14:13 UTC (permalink / raw)
To: Christian Borntraeger
Cc: Cornelia Huck, Jens Freimann, Heinz Graalfs, qemu-devel, Einar Lueck
On 20.09.2012, at 14:49, Christian Borntraeger <borntraeger@de.ibm.com> wrote:
> [...]
>>> --- a/target-s390x/kvm.c
>>> +++ b/target-s390x/kvm.c
>>> @@ -88,50 +88,77 @@ void kvm_arch_reset_vcpu(CPUS390XState *env)
>>> /* FIXME: add code to reset vcpu. */
>>> }
>>>
>>> +/* we want to have the prefix, the GPRS, the ACRS and the CRS up to date */
>>> +#define QEMU_NEEDED_REGS (KVM_SYNC_PREFIX | KVM_SYNC_GPRS | \
>>> + KVM_SYNC_ACRS | KVM_SYNC_CRS)
>>> +
>>> +/* But qemu only changes the GPRS */
>>> +#define QEMU_DIRTY_REGS (KVM_SYNC_GPRS)
>>> +
>>> int kvm_arch_put_registers(CPUS390XState *env, int level)
>>> {
>>> struct kvm_regs regs;
>>> int ret;
>>> int i;
>>>
>>> - ret = kvm_vcpu_ioctl(env, KVM_GET_REGS, ®s);
>>> - if (ret < 0) {
>>> - return ret;
>>> - }
>>> -
>>> - for (i = 0; i < 16; i++) {
>>> - regs.gprs[i] = env->regs[i];
>>> - }
>>> -
>>> - ret = kvm_vcpu_ioctl(env, KVM_SET_REGS, ®s);
>>> - if (ret < 0) {
>>> - return ret;
>>> - }
>>> -
>>> env->kvm_run->psw_addr = env->psw.addr;
>>> env->kvm_run->psw_mask = env->psw.mask;
>>>
>>> - return ret;
>>> + if ((env->kvm_run->kvm_valid_regs & QEMU_NEEDED_REGS) == QEMU_NEEDED_REGS) {
>>
>> Isn't this also missing a check for KVM_CAP_SYNC_REGS?
>
> I ommitted the check for two reasons:
> - since kvm_check_extension is an ioctl by itself, this would counter the original idea of sync regs
> - the kvm_run structure contains zeroes at the sync reg location in kernels that dont support that,
> (the area was unused and the page was allocated with GFP_ZERO)
> So by having any of the kvm_valid_regs != 0 we know that SYNC REGS must be supported
But the spec explicitly says that the fields are only valid when the CAP is present. So if you think that is unnecessary, please patch the spec.
The way we get around constant ioctls for cap checks is that we usually check only once and remember the result in a global variable. Please check other arch's kvm.c files for reference.
>
>
>> Also, on reset we probably also want to write the other registers back, right?
>
> Well, on reset we call the initial cpu reset ioctl, which does reset the other registers mandated
> by a cpu reset. Furthermore, it is not different to the current implementation....
The difference is that get_registers fetches more registers, so not putting them looks awkward.
>
> What we might want to do is to also consider a clear reset (which also zeroes the FPRs and ARs as
> well as memory etc) as an additional option. But this should not be the default, e.g. to make
> kdump or standalone dump possible.
> I think this would then be an addon-patch by itself.
What does reset on normal systems look like? Does it clear register state?
Alex
>
>> Alex
>
> Christian
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [Qemu-devel] [PATCH] s390: use sync regs for register transfer
2012-09-20 14:13 ` Alexander Graf
@ 2012-09-20 15:21 ` Christian Borntraeger
0 siblings, 0 replies; 6+ messages in thread
From: Christian Borntraeger @ 2012-09-20 15:21 UTC (permalink / raw)
To: Alexander Graf
Cc: Cornelia Huck, Jens Freimann, Heinz Graalfs, qemu-devel, Einar Lueck
On 20/09/12 16:13, Alexander Graf wrote:
>
>
> On 20.09.2012, at 14:49, Christian Borntraeger <borntraeger@de.ibm.com> wrote:
>
>> [...]
>>>> --- a/target-s390x/kvm.c
>>>> +++ b/target-s390x/kvm.c
>>>> @@ -88,50 +88,77 @@ void kvm_arch_reset_vcpu(CPUS390XState *env)
>>>> /* FIXME: add code to reset vcpu. */
>>>> }
>>>>
>>>> +/* we want to have the prefix, the GPRS, the ACRS and the CRS up to date */
>>>> +#define QEMU_NEEDED_REGS (KVM_SYNC_PREFIX | KVM_SYNC_GPRS | \
>>>> + KVM_SYNC_ACRS | KVM_SYNC_CRS)
>>>> +
>>>> +/* But qemu only changes the GPRS */
>>>> +#define QEMU_DIRTY_REGS (KVM_SYNC_GPRS)
>>>> +
>>>> int kvm_arch_put_registers(CPUS390XState *env, int level)
>>>> {
>>>> struct kvm_regs regs;
>>>> int ret;
>>>> int i;
>>>>
>>>> - ret = kvm_vcpu_ioctl(env, KVM_GET_REGS, ®s);
>>>> - if (ret < 0) {
>>>> - return ret;
>>>> - }
>>>> -
>>>> - for (i = 0; i < 16; i++) {
>>>> - regs.gprs[i] = env->regs[i];
>>>> - }
>>>> -
>>>> - ret = kvm_vcpu_ioctl(env, KVM_SET_REGS, ®s);
>>>> - if (ret < 0) {
>>>> - return ret;
>>>> - }
>>>> -
>>>> env->kvm_run->psw_addr = env->psw.addr;
>>>> env->kvm_run->psw_mask = env->psw.mask;
>>>>
>>>> - return ret;
>>>> + if ((env->kvm_run->kvm_valid_regs & QEMU_NEEDED_REGS) == QEMU_NEEDED_REGS) {
>>>
>>> Isn't this also missing a check for KVM_CAP_SYNC_REGS?
>>
>> I ommitted the check for two reasons:
>> - since kvm_check_extension is an ioctl by itself, this would counter the original idea of sync regs
>> - the kvm_run structure contains zeroes at the sync reg location in kernels that dont support that,
>> (the area was unused and the page was allocated with GFP_ZERO)
>> So by having any of the kvm_valid_regs != 0 we know that SYNC REGS must be supported
>
> But the spec explicitly says that the fields are only valid when the CAP is present. So if you think that is unnecessary, please patch the spec.
Maybe I should do both. Update the spec that the CAP defines if this is any
useful, if not it will start out with zeroes,but also having a single CAP check
during startup to have the code cleaner.
> The way we get around constant ioctls for cap checks is that we usually check only once and remember the result in a global variable. Please check other arch's kvm.c files for reference.
I looked for that in i386, but seems that only ppc does it. So I will look more
into ppc....
>
>>
>>
>>> Also, on reset we probably also want to write the other registers back, right?
>>
>> Well, on reset we call the initial cpu reset ioctl, which does reset the other registers mandated
>> by a cpu reset. Furthermore, it is not different to the current implementation....
>
> The difference is that get_registers fetches more registers, so not putting them looks awkward.
Yep, obviously the commenting was not enough to explain the why.
>
>>
>> What we might want to do is to also consider a clear reset (which also zeroes the FPRs and ARs as
>> well as memory etc) as an additional option. But this should not be the default, e.g. to make
>> kdump or standalone dump possible.
>> I think this would then be an addon-patch by itself.
>
> What does reset on normal systems look like? Does it clear register state?
A reset does not, a reset clear does.
But you just got me convinced, that we should really follow the level statement
for the put function and simply write everything for KVM_PUT_FULL_STATE and
KVM_PUT_RESET_STATE. The KVM_EXIT_S390_RESET handler must then do the right
thing (e.g. making sure that the general purpose register content for a
non-clear reset is not changed). Will look if that works out.
Christian
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH 1/5] s390: use sync regs for register transfer
@ 2012-10-03 20:08 Blue Swirl
2012-10-04 7:56 ` [Qemu-devel] [PATCH] " Jens Freimann
0 siblings, 1 reply; 6+ messages in thread
From: Blue Swirl @ 2012-10-03 20:08 UTC (permalink / raw)
To: Jens Freimann
Cc: Heinz Graalfs, Alexander Graf, qemu-devel, Christian Borntraeger,
Cornelia Huck, Einar Lueck
On Tue, Oct 2, 2012 at 7:33 AM, Jens Freimann <jfrei@linux.vnet.ibm.com> wrote:
> From: Christian Borntraeger <borntraeger@de.ibm.com>
>
> Newer kernels provide the guest registers in kvm_run. Lets use
> those if available (i.e. the capability is set). This avoids
> ioctls on cpu_synchronize_state making intercepts faster.
>
> In addition, we have now the prefix register, the access registers
> the control registers up to date. This helps in certain cases,
> e.g. for resolving kernel module addresses with gdb on a guest.
>
> On return, we update the registers according to the level statement,
> i.e. we put all registers for KVM_PUT_FULL_STATE and _RESET_STATE.
>
> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
> Signed-off-by: Jens Freimann <jfrei@linux.vnet.ibm.com>
> ---
> target-s390x/kvm.c | 119 ++++++++++++++++++++++++++++++++++++++++++++---------
> 1 file changed, 99 insertions(+), 20 deletions(-)
>
> diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c
> index 07edf93..cd57da3 100644
> --- a/target-s390x/kvm.c
> +++ b/target-s390x/kvm.c
> @@ -67,8 +67,11 @@ const KVMCapabilityInfo kvm_arch_required_capabilities[] = {
> KVM_CAP_LAST_INFO
> };
>
> +int cap_sync_regs;
static
> +
> int kvm_arch_init(KVMState *s)
> {
> + cap_sync_regs = kvm_check_extension(s, KVM_CAP_SYNC_REGS);
> return 0;
> }
>
> @@ -88,49 +91,125 @@ void kvm_arch_reset_vcpu(CPUS390XState *env)
> /* FIXME: add code to reset vcpu. */
> }
>
> +/* we want to have the prefix, the GPRS, the ACRS and the CRS up to date */
> +#define QEMU_NEEDED_REGS (KVM_SYNC_PREFIX | KVM_SYNC_GPRS | \
> + KVM_SYNC_ACRS | KVM_SYNC_CRS)
> +
> +/* But qemu only changes the GPRS */
> +#define QEMU_DIRTY_REGS (KVM_SYNC_GPRS)
> +
> int kvm_arch_put_registers(CPUS390XState *env, int level)
> {
> + struct kvm_sregs sregs;
> struct kvm_regs regs;
> int ret;
> int i;
>
> - ret = kvm_vcpu_ioctl(env, KVM_GET_REGS, ®s);
> - if (ret < 0) {
> - return ret;
> - }
> + /* always save the PSW and the GPRS*/
> + env->kvm_run->psw_addr = env->psw.addr;
> + env->kvm_run->psw_mask = env->psw.mask;
>
> - for (i = 0; i < 16; i++) {
> - regs.gprs[i] = env->regs[i];
> + if (cap_sync_regs && env->kvm_run->kvm_valid_regs & KVM_SYNC_GPRS) {
> + for (i = 0; i < 16; i++) {
> + env->kvm_run->s.regs.gprs[i] = env->regs[i];
> + env->kvm_run->kvm_dirty_regs |= KVM_SYNC_GPRS;
> + }
> + } else {
> + for (i = 0; i < 16; i++) {
> + regs.gprs[i] = env->regs[i];
> + }
> + ret = kvm_vcpu_ioctl(env, KVM_SET_REGS, ®s);
> + if (ret < 0) {
> + return ret;
> + }
> }
>
> - ret = kvm_vcpu_ioctl(env, KVM_SET_REGS, ®s);
> - if (ret < 0) {
> - return ret;
> + /* Do we need to save more than that? */
> + if (level == KVM_PUT_RUNTIME_STATE) {
> + return 0;
> }
>
> - env->kvm_run->psw_addr = env->psw.addr;
> - env->kvm_run->psw_mask = env->psw.mask;
> + if (cap_sync_regs &&
> + env->kvm_run->kvm_valid_regs & KVM_SYNC_ACRS &&
> + env->kvm_run->kvm_valid_regs & KVM_SYNC_CRS) {
> + for (i = 0; i < 16; i++) {
> + env->kvm_run->s.regs.acrs[i] = env->aregs[i];
> + env->kvm_run->s.regs.crs[i] = env->cregs[i];
> + }
> + env->kvm_run->kvm_dirty_regs |= KVM_SYNC_ACRS;
> + env->kvm_run->kvm_dirty_regs |= KVM_SYNC_CRS;
> + } else {
> + for (i = 0; i < 16; i++) {
> + sregs.acrs[i] = env->aregs[i];
> + sregs.crs[i] = env->cregs[i];
> + }
> + ret = kvm_vcpu_ioctl(env, KVM_SET_SREGS, &sregs);
> + if (ret < 0) {
> + return ret;
> + }
> + }
>
> - return ret;
> + /* Finally the prefix */
> + if (cap_sync_regs && env->kvm_run->kvm_valid_regs & KVM_SYNC_PREFIX) {
> + env->kvm_run->s.regs.prefix = env->psa;
> + env->kvm_run->kvm_dirty_regs |= KVM_SYNC_PREFIX;
> + } else {
> + /* prefix is only supported via sync regs */
> + }
> + return 0;
> }
>
> int kvm_arch_get_registers(CPUS390XState *env)
> {
> - int ret;
> + struct kvm_sregs sregs;
> struct kvm_regs regs;
> + int ret;
> int i;
>
> - ret = kvm_vcpu_ioctl(env, KVM_GET_REGS, ®s);
> - if (ret < 0) {
> - return ret;
> + /* get the PSW */
> + env->psw.addr = env->kvm_run->psw_addr;
> + env->psw.mask = env->kvm_run->psw_mask;
> +
> + /* the GPRS */
> + if (cap_sync_regs && env->kvm_run->kvm_valid_regs & KVM_SYNC_GPRS) {
> + for (i = 0; i < 16; i++) {
> + env->regs[i] = env->kvm_run->s.regs.gprs[i];
> + }
> + } else {
> + ret = kvm_vcpu_ioctl(env, KVM_GET_REGS, ®s);
> + if (ret < 0) {
> + return ret;
> + }
> + for (i = 0; i < 16; i++) {
> + env->regs[i] = regs.gprs[i];
> + }
> }
>
> - for (i = 0; i < 16; i++) {
> - env->regs[i] = regs.gprs[i];
> + /* The ACRS and CRS */
> + if (cap_sync_regs &&
> + env->kvm_run->kvm_valid_regs & KVM_SYNC_ACRS &&
> + env->kvm_run->kvm_valid_regs & KVM_SYNC_CRS) {
> + for (i = 0; i < 16; i++) {
> + env->aregs[i] = env->kvm_run->s.regs.acrs[i];
> + env->cregs[i] = env->kvm_run->s.regs.crs[i];
> + }
> + } else {
> + ret = kvm_vcpu_ioctl(env, KVM_GET_SREGS, &sregs);
> + if (ret < 0) {
> + return ret;
> + }
> + for (i = 0; i < 16; i++) {
> + env->aregs[i] = sregs.acrs[i];
> + env->cregs[i] = sregs.crs[i];
> + }
> }
>
> - env->psw.addr = env->kvm_run->psw_addr;
> - env->psw.mask = env->kvm_run->psw_mask;
> + /* Finally the prefix */
> + if (cap_sync_regs && env->kvm_run->kvm_valid_regs & KVM_SYNC_PREFIX) {
> + env->psa = env->kvm_run->s.regs.prefix;
> + } else {
> + /* no prefix without sync regs */
> + }
>
> return 0;
> }
> --
> 1.7.11.7
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread* [Qemu-devel] [PATCH] s390: use sync regs for register transfer
2012-10-03 20:08 [Qemu-devel] [PATCH 1/5] " Blue Swirl
@ 2012-10-04 7:56 ` Jens Freimann
0 siblings, 0 replies; 6+ messages in thread
From: Jens Freimann @ 2012-10-04 7:56 UTC (permalink / raw)
To: Alexander Graf
Cc: Heinz Graalfs, qemu-devel, Christian Borntraeger, Jens Freimann,
Cornelia Huck, Einar Lueck
From: Christian Borntraeger <borntraeger@de.ibm.com>
Newer kernels provide the guest registers in kvm_run. Lets use
those if available (i.e. the capability is set). This avoids
ioctls on cpu_synchronize_state making intercepts faster.
In addition, we have now the prefix register, the access registers
the control registers up to date. This helps in certain cases,
e.g. for resolving kernel module addresses with gdb on a guest.
On return, we update the registers according to the level statement,
i.e. we put all registers for KVM_PUT_FULL_STATE and _RESET_STATE.
Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
Signed-off-by: Jens Freimann <jfrei@linux.vnet.ibm.com>
---
target-s390x/kvm.c | 119 ++++++++++++++++++++++++++++++++++++++++++++---------
1 file changed, 99 insertions(+), 20 deletions(-)
diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c
index 07edf93..321ab49 100644
--- a/target-s390x/kvm.c
+++ b/target-s390x/kvm.c
@@ -67,8 +67,11 @@ const KVMCapabilityInfo kvm_arch_required_capabilities[] = {
KVM_CAP_LAST_INFO
};
+static int cap_sync_regs;
+
int kvm_arch_init(KVMState *s)
{
+ cap_sync_regs = kvm_check_extension(s, KVM_CAP_SYNC_REGS);
return 0;
}
@@ -88,49 +91,125 @@ void kvm_arch_reset_vcpu(CPUS390XState *env)
/* FIXME: add code to reset vcpu. */
}
+/* we want to have the prefix, the GPRS, the ACRS and the CRS up to date */
+#define QEMU_NEEDED_REGS (KVM_SYNC_PREFIX | KVM_SYNC_GPRS | \
+ KVM_SYNC_ACRS | KVM_SYNC_CRS)
+
+/* But qemu only changes the GPRS */
+#define QEMU_DIRTY_REGS (KVM_SYNC_GPRS)
+
int kvm_arch_put_registers(CPUS390XState *env, int level)
{
+ struct kvm_sregs sregs;
struct kvm_regs regs;
int ret;
int i;
- ret = kvm_vcpu_ioctl(env, KVM_GET_REGS, ®s);
- if (ret < 0) {
- return ret;
- }
+ /* always save the PSW and the GPRS*/
+ env->kvm_run->psw_addr = env->psw.addr;
+ env->kvm_run->psw_mask = env->psw.mask;
- for (i = 0; i < 16; i++) {
- regs.gprs[i] = env->regs[i];
+ if (cap_sync_regs && env->kvm_run->kvm_valid_regs & KVM_SYNC_GPRS) {
+ for (i = 0; i < 16; i++) {
+ env->kvm_run->s.regs.gprs[i] = env->regs[i];
+ env->kvm_run->kvm_dirty_regs |= KVM_SYNC_GPRS;
+ }
+ } else {
+ for (i = 0; i < 16; i++) {
+ regs.gprs[i] = env->regs[i];
+ }
+ ret = kvm_vcpu_ioctl(env, KVM_SET_REGS, ®s);
+ if (ret < 0) {
+ return ret;
+ }
}
- ret = kvm_vcpu_ioctl(env, KVM_SET_REGS, ®s);
- if (ret < 0) {
- return ret;
+ /* Do we need to save more than that? */
+ if (level == KVM_PUT_RUNTIME_STATE) {
+ return 0;
}
- env->kvm_run->psw_addr = env->psw.addr;
- env->kvm_run->psw_mask = env->psw.mask;
+ if (cap_sync_regs &&
+ env->kvm_run->kvm_valid_regs & KVM_SYNC_ACRS &&
+ env->kvm_run->kvm_valid_regs & KVM_SYNC_CRS) {
+ for (i = 0; i < 16; i++) {
+ env->kvm_run->s.regs.acrs[i] = env->aregs[i];
+ env->kvm_run->s.regs.crs[i] = env->cregs[i];
+ }
+ env->kvm_run->kvm_dirty_regs |= KVM_SYNC_ACRS;
+ env->kvm_run->kvm_dirty_regs |= KVM_SYNC_CRS;
+ } else {
+ for (i = 0; i < 16; i++) {
+ sregs.acrs[i] = env->aregs[i];
+ sregs.crs[i] = env->cregs[i];
+ }
+ ret = kvm_vcpu_ioctl(env, KVM_SET_SREGS, &sregs);
+ if (ret < 0) {
+ return ret;
+ }
+ }
- return ret;
+ /* Finally the prefix */
+ if (cap_sync_regs && env->kvm_run->kvm_valid_regs & KVM_SYNC_PREFIX) {
+ env->kvm_run->s.regs.prefix = env->psa;
+ env->kvm_run->kvm_dirty_regs |= KVM_SYNC_PREFIX;
+ } else {
+ /* prefix is only supported via sync regs */
+ }
+ return 0;
}
int kvm_arch_get_registers(CPUS390XState *env)
{
- int ret;
+ struct kvm_sregs sregs;
struct kvm_regs regs;
+ int ret;
int i;
- ret = kvm_vcpu_ioctl(env, KVM_GET_REGS, ®s);
- if (ret < 0) {
- return ret;
+ /* get the PSW */
+ env->psw.addr = env->kvm_run->psw_addr;
+ env->psw.mask = env->kvm_run->psw_mask;
+
+ /* the GPRS */
+ if (cap_sync_regs && env->kvm_run->kvm_valid_regs & KVM_SYNC_GPRS) {
+ for (i = 0; i < 16; i++) {
+ env->regs[i] = env->kvm_run->s.regs.gprs[i];
+ }
+ } else {
+ ret = kvm_vcpu_ioctl(env, KVM_GET_REGS, ®s);
+ if (ret < 0) {
+ return ret;
+ }
+ for (i = 0; i < 16; i++) {
+ env->regs[i] = regs.gprs[i];
+ }
}
- for (i = 0; i < 16; i++) {
- env->regs[i] = regs.gprs[i];
+ /* The ACRS and CRS */
+ if (cap_sync_regs &&
+ env->kvm_run->kvm_valid_regs & KVM_SYNC_ACRS &&
+ env->kvm_run->kvm_valid_regs & KVM_SYNC_CRS) {
+ for (i = 0; i < 16; i++) {
+ env->aregs[i] = env->kvm_run->s.regs.acrs[i];
+ env->cregs[i] = env->kvm_run->s.regs.crs[i];
+ }
+ } else {
+ ret = kvm_vcpu_ioctl(env, KVM_GET_SREGS, &sregs);
+ if (ret < 0) {
+ return ret;
+ }
+ for (i = 0; i < 16; i++) {
+ env->aregs[i] = sregs.acrs[i];
+ env->cregs[i] = sregs.crs[i];
+ }
}
- env->psw.addr = env->kvm_run->psw_addr;
- env->psw.mask = env->kvm_run->psw_mask;
+ /* Finally the prefix */
+ if (cap_sync_regs && env->kvm_run->kvm_valid_regs & KVM_SYNC_PREFIX) {
+ env->psa = env->kvm_run->s.regs.prefix;
+ } else {
+ /* no prefix without sync regs */
+ }
return 0;
}
--
1.7.11.7
^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2012-10-04 7:57 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-08-22 11:54 [Qemu-devel] [PATCH] s390: use sync regs for register transfer Jens Freimann
2012-09-20 12:19 ` Alexander Graf
2012-09-20 12:49 ` Christian Borntraeger
2012-09-20 14:13 ` Alexander Graf
2012-09-20 15:21 ` Christian Borntraeger
2012-10-03 20:08 [Qemu-devel] [PATCH 1/5] " Blue Swirl
2012-10-04 7:56 ` [Qemu-devel] [PATCH] " Jens Freimann
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.