Re: [Qemu-devel] [PATCH] s390: use sync regs for register transfer

[Alexander Graf]

On 22.08.2012, at 13:54, Jens Freimann wrote:

> From: Christian Borntraeger <address@hidden>
> 
> 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 <address@hidden>
> Signed-off-by: Jens Freimann <address@hidden>
> ---
> 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, &regs);
> -    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, &regs);
> -    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