* [PATCH v2 0/2] target/s390x/kvm: Simplify the synchronization code
@ 2023-10-09 17:07 Thomas Huth
2023-10-09 17:07 ` [PATCH v2 1/2] target/s390x/kvm: Turn KVM_CAP_SYNC_REGS into a hard requirement Thomas Huth
2023-10-09 17:07 ` [PATCH v2 2/2] target/s390x/kvm: Simplify the GPRs, ACRs, CRs and prefix synchronization code Thomas Huth
0 siblings, 2 replies; 8+ messages in thread
From: Thomas Huth @ 2023-10-09 17:07 UTC (permalink / raw)
To: qemu-devel, Halil Pasic, Christian Borntraeger
Cc: qemu-s390x, David Hildenbrand, Cédric Le Goater
KVM_SYNC_GPRS, KVM_SYNC_ACRS, KVM_SYNC_CRS and KVM_SYNC_PREFIX are
available since kernel 3.10. Since we already require at least kernel
3.15 in the s390x KVM code, we can also assume that the KVM_CAP_SYNC_REGS
sync code is always possible for these registers, and remove the
related checks and fallbacks via KVM_SET_REGS and KVM_GET_REGS.
v2:
- Split the patch from v1 into two patches
- Use a #define KVM_SYNC_REQUIRED_BITS for the required sync bits
- Use memcpy() instead of for-loops for copying the registers
Thomas Huth (2):
target/s390x/kvm: Turn KVM_CAP_SYNC_REGS into a hard requirement
target/s390x/kvm: Simplify the GPRs, ACRs, CRs and prefix
synchronization code
target/s390x/kvm/kvm.c | 120 ++++++++++++-----------------------------
1 file changed, 33 insertions(+), 87 deletions(-)
--
2.41.0
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2 1/2] target/s390x/kvm: Turn KVM_CAP_SYNC_REGS into a hard requirement
2023-10-09 17:07 [PATCH v2 0/2] target/s390x/kvm: Simplify the synchronization code Thomas Huth
@ 2023-10-09 17:07 ` Thomas Huth
2023-10-10 11:02 ` Christian Borntraeger
2023-10-09 17:07 ` [PATCH v2 2/2] target/s390x/kvm: Simplify the GPRs, ACRs, CRs and prefix synchronization code Thomas Huth
1 sibling, 1 reply; 8+ messages in thread
From: Thomas Huth @ 2023-10-09 17:07 UTC (permalink / raw)
To: qemu-devel, Halil Pasic, Christian Borntraeger
Cc: qemu-s390x, David Hildenbrand, Cédric Le Goater
Since we already require at least kernel 3.15 in the s390x KVM code,
we can assume that the KVM_CAP_SYNC_REGS capability is always there.
Thus turn this into a hard requirement now.
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
target/s390x/kvm/kvm.c | 20 ++++++++++++++------
1 file changed, 14 insertions(+), 6 deletions(-)
diff --git a/target/s390x/kvm/kvm.c b/target/s390x/kvm/kvm.c
index bc5c56a305..b3e2eaa2eb 100644
--- a/target/s390x/kvm/kvm.c
+++ b/target/s390x/kvm/kvm.c
@@ -337,21 +337,29 @@ int kvm_arch_get_default_type(MachineState *ms)
int kvm_arch_init(MachineState *ms, KVMState *s)
{
+ int required_caps[] = {
+ KVM_CAP_DEVICE_CTRL,
+ KVM_CAP_SYNC_REGS,
+ };
+
+ for (int i = 0; i < ARRAY_SIZE(required_caps); i++) {
+ if (!kvm_check_extension(s, required_caps[i])) {
+ error_report("KVM is missing capability #%d - "
+ "please use kernel 3.15 or newer", required_caps[i]);
+ return -1;
+ }
+ }
+
object_class_foreach(ccw_machine_class_foreach, TYPE_S390_CCW_MACHINE,
false, NULL);
- if (!kvm_check_extension(kvm_state, KVM_CAP_DEVICE_CTRL)) {
- error_report("KVM is missing capability KVM_CAP_DEVICE_CTRL - "
- "please use kernel 3.15 or newer");
- return -1;
- }
if (!kvm_check_extension(s, KVM_CAP_S390_COW)) {
error_report("KVM is missing capability KVM_CAP_S390_COW - "
"unsupported environment");
return -1;
}
- cap_sync_regs = kvm_check_extension(s, KVM_CAP_SYNC_REGS);
+ cap_sync_regs = true;
cap_async_pf = kvm_check_extension(s, KVM_CAP_ASYNC_PF);
cap_mem_op = kvm_check_extension(s, KVM_CAP_S390_MEM_OP);
cap_mem_op_extension = kvm_check_extension(s, KVM_CAP_S390_MEM_OP_EXTENSION);
--
2.41.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v2 2/2] target/s390x/kvm: Simplify the GPRs, ACRs, CRs and prefix synchronization code
2023-10-09 17:07 [PATCH v2 0/2] target/s390x/kvm: Simplify the synchronization code Thomas Huth
2023-10-09 17:07 ` [PATCH v2 1/2] target/s390x/kvm: Turn KVM_CAP_SYNC_REGS into a hard requirement Thomas Huth
@ 2023-10-09 17:07 ` Thomas Huth
2023-10-10 11:41 ` Christian Borntraeger
1 sibling, 1 reply; 8+ messages in thread
From: Thomas Huth @ 2023-10-09 17:07 UTC (permalink / raw)
To: qemu-devel, Halil Pasic, Christian Borntraeger
Cc: qemu-s390x, David Hildenbrand, Cédric Le Goater
KVM_SYNC_GPRS, KVM_SYNC_ACRS, KVM_SYNC_CRS and KVM_SYNC_PREFIX are
available since kernel 3.10. Since we already require at least kernel
3.15 in the s390x KVM code, we can also assume that the KVM_CAP_SYNC_REGS
sync code is always possible for these registers, and remove the
related checks and fallbacks via KVM_SET_REGS and KVM_GET_REGS.
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
target/s390x/kvm/kvm.c | 102 ++++++++---------------------------------
1 file changed, 20 insertions(+), 82 deletions(-)
diff --git a/target/s390x/kvm/kvm.c b/target/s390x/kvm/kvm.c
index b3e2eaa2eb..b988fc3abb 100644
--- a/target/s390x/kvm/kvm.c
+++ b/target/s390x/kvm/kvm.c
@@ -138,7 +138,6 @@ const KVMCapabilityInfo kvm_arch_required_capabilities[] = {
KVM_CAP_LAST_INFO
};
-static int cap_sync_regs;
static int cap_async_pf;
static int cap_mem_op;
static int cap_mem_op_extension;
@@ -359,7 +358,6 @@ int kvm_arch_init(MachineState *ms, KVMState *s)
return -1;
}
- cap_sync_regs = true;
cap_async_pf = kvm_check_extension(s, KVM_CAP_ASYNC_PF);
cap_mem_op = kvm_check_extension(s, KVM_CAP_S390_MEM_OP);
cap_mem_op_extension = kvm_check_extension(s, KVM_CAP_S390_MEM_OP_EXTENSION);
@@ -466,15 +464,16 @@ void kvm_s390_reset_vcpu_normal(S390CPU *cpu)
static int can_sync_regs(CPUState *cs, int regs)
{
- return cap_sync_regs && (cs->kvm_run->kvm_valid_regs & regs) == regs;
+ return (cs->kvm_run->kvm_valid_regs & regs) == regs;
}
+#define KVM_SYNC_REQUIRED_BITS (KVM_SYNC_GPRS | KVM_SYNC_ACRS | \
+ KVM_SYNC_CRS | KVM_SYNC_PREFIX)
+
int kvm_arch_put_registers(CPUState *cs, int level)
{
S390CPU *cpu = S390_CPU(cs);
CPUS390XState *env = &cpu->env;
- struct kvm_sregs sregs;
- struct kvm_regs regs;
struct kvm_fpu fpu = {};
int r;
int i;
@@ -483,20 +482,14 @@ int kvm_arch_put_registers(CPUState *cs, int level)
cs->kvm_run->psw_addr = env->psw.addr;
cs->kvm_run->psw_mask = env->psw.mask;
- if (can_sync_regs(cs, KVM_SYNC_GPRS)) {
- for (i = 0; i < 16; i++) {
- cs->kvm_run->s.regs.gprs[i] = env->regs[i];
- cs->kvm_run->kvm_dirty_regs |= KVM_SYNC_GPRS;
- }
- } else {
- for (i = 0; i < 16; i++) {
- regs.gprs[i] = env->regs[i];
- }
- r = kvm_vcpu_ioctl(cs, KVM_SET_REGS, ®s);
- if (r < 0) {
- return r;
- }
- }
+ g_assert((cs->kvm_run->kvm_valid_regs & KVM_SYNC_REQUIRED_BITS) ==
+ KVM_SYNC_REQUIRED_BITS);
+ cs->kvm_run->kvm_dirty_regs |= KVM_SYNC_REQUIRED_BITS;
+ memcpy(cs->kvm_run->s.regs.gprs, env->regs, sizeof(cs->kvm_run->s.regs.gprs));
+ memcpy(cs->kvm_run->s.regs.acrs, env->aregs, sizeof(cs->kvm_run->s.regs.acrs));
+ memcpy(cs->kvm_run->s.regs.crs, env->cregs, sizeof(cs->kvm_run->s.regs.crs));
+
+ cs->kvm_run->s.regs.prefix = env->psa;
if (can_sync_regs(cs, KVM_SYNC_VRS)) {
for (i = 0; i < 32; i++) {
@@ -575,25 +568,6 @@ int kvm_arch_put_registers(CPUState *cs, int level)
}
}
- /* access registers and control registers*/
- if (can_sync_regs(cs, KVM_SYNC_ACRS | KVM_SYNC_CRS)) {
- for (i = 0; i < 16; i++) {
- cs->kvm_run->s.regs.acrs[i] = env->aregs[i];
- cs->kvm_run->s.regs.crs[i] = env->cregs[i];
- }
- cs->kvm_run->kvm_dirty_regs |= KVM_SYNC_ACRS;
- cs->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];
- }
- r = kvm_vcpu_ioctl(cs, KVM_SET_SREGS, &sregs);
- if (r < 0) {
- return r;
- }
- }
-
if (can_sync_regs(cs, KVM_SYNC_GSCB)) {
memcpy(cs->kvm_run->s.regs.gscb, env->gscb, 32);
cs->kvm_run->kvm_dirty_regs |= KVM_SYNC_GSCB;
@@ -615,13 +589,6 @@ int kvm_arch_put_registers(CPUState *cs, int level)
cs->kvm_run->kvm_dirty_regs |= KVM_SYNC_DIAG318;
}
- /* Finally the prefix */
- if (can_sync_regs(cs, KVM_SYNC_PREFIX)) {
- cs->kvm_run->s.regs.prefix = env->psa;
- cs->kvm_run->kvm_dirty_regs |= KVM_SYNC_PREFIX;
- } else {
- /* prefix is only supported via sync regs */
- }
return 0;
}
@@ -629,8 +596,6 @@ int kvm_arch_get_registers(CPUState *cs)
{
S390CPU *cpu = S390_CPU(cs);
CPUS390XState *env = &cpu->env;
- struct kvm_sregs sregs;
- struct kvm_regs regs;
struct kvm_fpu fpu;
int i, r;
@@ -638,37 +603,15 @@ int kvm_arch_get_registers(CPUState *cs)
env->psw.addr = cs->kvm_run->psw_addr;
env->psw.mask = cs->kvm_run->psw_mask;
- /* the GPRS */
- if (can_sync_regs(cs, KVM_SYNC_GPRS)) {
- for (i = 0; i < 16; i++) {
- env->regs[i] = cs->kvm_run->s.regs.gprs[i];
- }
- } else {
- r = kvm_vcpu_ioctl(cs, KVM_GET_REGS, ®s);
- if (r < 0) {
- return r;
- }
- for (i = 0; i < 16; i++) {
- env->regs[i] = regs.gprs[i];
- }
- }
+ /* the GPRS, ACRS and CRS */
+ g_assert((cs->kvm_run->kvm_valid_regs & KVM_SYNC_REQUIRED_BITS) ==
+ KVM_SYNC_REQUIRED_BITS);
+ memcpy(env->regs, cs->kvm_run->s.regs.gprs, sizeof(env->regs));
+ memcpy(env->aregs, cs->kvm_run->s.regs.acrs, sizeof(env->aregs));
+ memcpy(env->cregs, cs->kvm_run->s.regs.crs, sizeof(env->cregs));
- /* The ACRS and CRS */
- if (can_sync_regs(cs, KVM_SYNC_ACRS | KVM_SYNC_CRS)) {
- for (i = 0; i < 16; i++) {
- env->aregs[i] = cs->kvm_run->s.regs.acrs[i];
- env->cregs[i] = cs->kvm_run->s.regs.crs[i];
- }
- } else {
- r = kvm_vcpu_ioctl(cs, KVM_GET_SREGS, &sregs);
- if (r < 0) {
- return r;
- }
- for (i = 0; i < 16; i++) {
- env->aregs[i] = sregs.acrs[i];
- env->cregs[i] = sregs.crs[i];
- }
- }
+ /* The prefix */
+ env->psa = cs->kvm_run->s.regs.prefix;
/* Floating point and vector registers */
if (can_sync_regs(cs, KVM_SYNC_VRS)) {
@@ -693,11 +636,6 @@ int kvm_arch_get_registers(CPUState *cs)
env->fpc = fpu.fpc;
}
- /* The prefix */
- if (can_sync_regs(cs, KVM_SYNC_PREFIX)) {
- env->psa = cs->kvm_run->s.regs.prefix;
- }
-
if (can_sync_regs(cs, KVM_SYNC_ARCH0)) {
env->cputm = cs->kvm_run->s.regs.cputm;
env->ckc = cs->kvm_run->s.regs.ckc;
--
2.41.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/2] target/s390x/kvm: Turn KVM_CAP_SYNC_REGS into a hard requirement
2023-10-09 17:07 ` [PATCH v2 1/2] target/s390x/kvm: Turn KVM_CAP_SYNC_REGS into a hard requirement Thomas Huth
@ 2023-10-10 11:02 ` Christian Borntraeger
2023-10-10 11:12 ` Thomas Huth
0 siblings, 1 reply; 8+ messages in thread
From: Christian Borntraeger @ 2023-10-10 11:02 UTC (permalink / raw)
To: Thomas Huth, qemu-devel, Halil Pasic
Cc: qemu-s390x, David Hildenbrand, Cédric Le Goater
Am 09.10.23 um 19:07 schrieb Thomas Huth:
> Since we already require at least kernel 3.15 in the s390x KVM code,
> we can assume that the KVM_CAP_SYNC_REGS capability is always there.
> Thus turn this into a hard requirement now.
>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
> target/s390x/kvm/kvm.c | 20 ++++++++++++++------
> 1 file changed, 14 insertions(+), 6 deletions(-)
>
> diff --git a/target/s390x/kvm/kvm.c b/target/s390x/kvm/kvm.c
> index bc5c56a305..b3e2eaa2eb 100644
> --- a/target/s390x/kvm/kvm.c
> +++ b/target/s390x/kvm/kvm.c
> @@ -337,21 +337,29 @@ int kvm_arch_get_default_type(MachineState *ms)
>
> int kvm_arch_init(MachineState *ms, KVMState *s)
> {
> + int required_caps[] = {
> + KVM_CAP_DEVICE_CTRL,
> + KVM_CAP_SYNC_REGS,
> + };
> +
> + for (int i = 0; i < ARRAY_SIZE(required_caps); i++) {
> + if (!kvm_check_extension(s, required_caps[i])) {
> + error_report("KVM is missing capability #%d - "
> + "please use kernel 3.15 or newer", required_caps[i]);
> + return -1;
> + }
> + }
> +
> object_class_foreach(ccw_machine_class_foreach, TYPE_S390_CCW_MACHINE,
> false, NULL);
>
> - if (!kvm_check_extension(kvm_state, KVM_CAP_DEVICE_CTRL)) {
> - error_report("KVM is missing capability KVM_CAP_DEVICE_CTRL - "
> - "please use kernel 3.15 or newer");
> - return -1;
> - }
> if (!kvm_check_extension(s, KVM_CAP_S390_COW)) {
> error_report("KVM is missing capability KVM_CAP_S390_COW - "
> "unsupported environment");
> return -1;
> }
Not sure if we also want to move KVM_CAP_S390_COW somehow. The message would be different.
Aparch from that:
Reviewed-by: Christian Borntraeger <borntraeger@linux.ibm.com>
>
> - cap_sync_regs = kvm_check_extension(s, KVM_CAP_SYNC_REGS);
> + cap_sync_regs = true;
> cap_async_pf = kvm_check_extension(s, KVM_CAP_ASYNC_PF);
> cap_mem_op = kvm_check_extension(s, KVM_CAP_S390_MEM_OP);
> cap_mem_op_extension = kvm_check_extension(s, KVM_CAP_S390_MEM_OP_EXTENSION);
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/2] target/s390x/kvm: Turn KVM_CAP_SYNC_REGS into a hard requirement
2023-10-10 11:02 ` Christian Borntraeger
@ 2023-10-10 11:12 ` Thomas Huth
2023-10-10 11:36 ` Christian Borntraeger
0 siblings, 1 reply; 8+ messages in thread
From: Thomas Huth @ 2023-10-10 11:12 UTC (permalink / raw)
To: Christian Borntraeger, qemu-devel, Halil Pasic
Cc: qemu-s390x, David Hildenbrand, Cédric Le Goater
On 10/10/2023 13.02, Christian Borntraeger wrote:
>
>
> Am 09.10.23 um 19:07 schrieb Thomas Huth:
>> Since we already require at least kernel 3.15 in the s390x KVM code,
>> we can assume that the KVM_CAP_SYNC_REGS capability is always there.
>> Thus turn this into a hard requirement now.
>>
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> ---
>> target/s390x/kvm/kvm.c | 20 ++++++++++++++------
>> 1 file changed, 14 insertions(+), 6 deletions(-)
>>
>> diff --git a/target/s390x/kvm/kvm.c b/target/s390x/kvm/kvm.c
>> index bc5c56a305..b3e2eaa2eb 100644
>> --- a/target/s390x/kvm/kvm.c
>> +++ b/target/s390x/kvm/kvm.c
>> @@ -337,21 +337,29 @@ int kvm_arch_get_default_type(MachineState *ms)
>> int kvm_arch_init(MachineState *ms, KVMState *s)
>> {
>> + int required_caps[] = {
>> + KVM_CAP_DEVICE_CTRL,
>> + KVM_CAP_SYNC_REGS,
>> + };
>> +
>> + for (int i = 0; i < ARRAY_SIZE(required_caps); i++) {
>> + if (!kvm_check_extension(s, required_caps[i])) {
>> + error_report("KVM is missing capability #%d - "
>> + "please use kernel 3.15 or newer",
>> required_caps[i]);
>> + return -1;
>> + }
>> + }
>> +
>> object_class_foreach(ccw_machine_class_foreach, TYPE_S390_CCW_MACHINE,
>> false, NULL);
>> - if (!kvm_check_extension(kvm_state, KVM_CAP_DEVICE_CTRL)) {
>> - error_report("KVM is missing capability KVM_CAP_DEVICE_CTRL - "
>> - "please use kernel 3.15 or newer");
>> - return -1;
>> - }
>> if (!kvm_check_extension(s, KVM_CAP_S390_COW)) {
>> error_report("KVM is missing capability KVM_CAP_S390_COW - "
>> "unsupported environment");
>> return -1;
>> }
>
> Not sure if we also want to move KVM_CAP_S390_COW somehow. The message would
> be different.
IIRC that error could happen when you ran KVM within an older version of
z/VM, so the "please use kernel 3.15 or newer" message would be completely
misleading there.
> Aparch from that:
> Reviewed-by: Christian Borntraeger <borntraeger@linux.ibm.com>
Thanks,
Thomas
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/2] target/s390x/kvm: Turn KVM_CAP_SYNC_REGS into a hard requirement
2023-10-10 11:12 ` Thomas Huth
@ 2023-10-10 11:36 ` Christian Borntraeger
0 siblings, 0 replies; 8+ messages in thread
From: Christian Borntraeger @ 2023-10-10 11:36 UTC (permalink / raw)
To: Thomas Huth, qemu-devel, Halil Pasic
Cc: qemu-s390x, David Hildenbrand, Cédric Le Goater
Am 10.10.23 um 13:12 schrieb Thomas Huth:
> On 10/10/2023 13.02, Christian Borntraeger wrote:
>>
>>
>> Am 09.10.23 um 19:07 schrieb Thomas Huth:
>>> Since we already require at least kernel 3.15 in the s390x KVM code,
>>> we can assume that the KVM_CAP_SYNC_REGS capability is always there.
>>> Thus turn this into a hard requirement now.
>>>
>>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>>> ---
>>> target/s390x/kvm/kvm.c | 20 ++++++++++++++------
>>> 1 file changed, 14 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/target/s390x/kvm/kvm.c b/target/s390x/kvm/kvm.c
>>> index bc5c56a305..b3e2eaa2eb 100644
>>> --- a/target/s390x/kvm/kvm.c
>>> +++ b/target/s390x/kvm/kvm.c
>>> @@ -337,21 +337,29 @@ int kvm_arch_get_default_type(MachineState *ms)
>>> int kvm_arch_init(MachineState *ms, KVMState *s)
>>> {
>>> + int required_caps[] = {
>>> + KVM_CAP_DEVICE_CTRL,
>>> + KVM_CAP_SYNC_REGS,
>>> + };
>>> +
>>> + for (int i = 0; i < ARRAY_SIZE(required_caps); i++) {
>>> + if (!kvm_check_extension(s, required_caps[i])) {
>>> + error_report("KVM is missing capability #%d - "
>>> + "please use kernel 3.15 or newer", required_caps[i]);
>>> + return -1;
>>> + }
>>> + }
>>> +
>>> object_class_foreach(ccw_machine_class_foreach, TYPE_S390_CCW_MACHINE,
>>> false, NULL);
>>> - if (!kvm_check_extension(kvm_state, KVM_CAP_DEVICE_CTRL)) {
>>> - error_report("KVM is missing capability KVM_CAP_DEVICE_CTRL - "
>>> - "please use kernel 3.15 or newer");
>>> - return -1;
>>> - }
>>> if (!kvm_check_extension(s, KVM_CAP_S390_COW)) {
>>> error_report("KVM is missing capability KVM_CAP_S390_COW - "
>>> "unsupported environment");
>>> return -1;
>>> }
>>
>> Not sure if we also want to move KVM_CAP_S390_COW somehow. The message would be different.
>
> IIRC that error could happen when you ran KVM within an older version of z/VM, so the "please use kernel 3.15 or newer" message would be completely misleading there.
Yes, thats what I was trying to say, we would need a different message.
Lets go with this patch.
>
>> Aparch from that:
>> Reviewed-by: Christian Borntraeger <borntraeger@linux.ibm.com>
>
> Thanks,
> Thomas
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 2/2] target/s390x/kvm: Simplify the GPRs, ACRs, CRs and prefix synchronization code
2023-10-09 17:07 ` [PATCH v2 2/2] target/s390x/kvm: Simplify the GPRs, ACRs, CRs and prefix synchronization code Thomas Huth
@ 2023-10-10 11:41 ` Christian Borntraeger
2023-10-10 11:46 ` Thomas Huth
0 siblings, 1 reply; 8+ messages in thread
From: Christian Borntraeger @ 2023-10-10 11:41 UTC (permalink / raw)
To: Thomas Huth, qemu-devel, Halil Pasic
Cc: qemu-s390x, David Hildenbrand, Cédric Le Goater
Am 09.10.23 um 19:07 schrieb Thomas Huth:
[...]
> @@ -483,20 +482,14 @@ int kvm_arch_put_registers(CPUState *cs, int level)
> cs->kvm_run->psw_addr = env->psw.addr;
> cs->kvm_run->psw_mask = env->psw.mask;
>
> - if (can_sync_regs(cs, KVM_SYNC_GPRS)) {
> - for (i = 0; i < 16; i++) {
> - cs->kvm_run->s.regs.gprs[i] = env->regs[i];
> - cs->kvm_run->kvm_dirty_regs |= KVM_SYNC_GPRS;
> - }
> - } else {
> - for (i = 0; i < 16; i++) {
> - regs.gprs[i] = env->regs[i];
> - }
> - r = kvm_vcpu_ioctl(cs, KVM_SET_REGS, ®s);
> - if (r < 0) {
> - return r;
> - }
> - }
> + g_assert((cs->kvm_run->kvm_valid_regs & KVM_SYNC_REQUIRED_BITS) ==
> + KVM_SYNC_REQUIRED_BITS);
> + cs->kvm_run->kvm_dirty_regs |= KVM_SYNC_REQUIRED_BITS;
> + memcpy(cs->kvm_run->s.regs.gprs, env->regs, sizeof(cs->kvm_run->s.regs.gprs));
> + memcpy(cs->kvm_run->s.regs.acrs, env->aregs, sizeof(cs->kvm_run->s.regs.acrs));
> + memcpy(cs->kvm_run->s.regs.crs, env->cregs, sizeof(cs->kvm_run->s.regs.crs));
> +
> + cs->kvm_run->s.regs.prefix = env->psa;
These 3 have only been saved for level> KVM_PUT_RUNTIME_STATE. By moving the memcpy
into this position you would always write them. Probably ok but a change in
behaviour. Was this intentional?
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 2/2] target/s390x/kvm: Simplify the GPRs, ACRs, CRs and prefix synchronization code
2023-10-10 11:41 ` Christian Borntraeger
@ 2023-10-10 11:46 ` Thomas Huth
0 siblings, 0 replies; 8+ messages in thread
From: Thomas Huth @ 2023-10-10 11:46 UTC (permalink / raw)
To: Christian Borntraeger, qemu-devel, Halil Pasic
Cc: qemu-s390x, David Hildenbrand, Cédric Le Goater
On 10/10/2023 13.41, Christian Borntraeger wrote:
>
>
> Am 09.10.23 um 19:07 schrieb Thomas Huth:
> [...]
>
>> @@ -483,20 +482,14 @@ int kvm_arch_put_registers(CPUState *cs, int level)
>> cs->kvm_run->psw_addr = env->psw.addr;
>> cs->kvm_run->psw_mask = env->psw.mask;
>> - if (can_sync_regs(cs, KVM_SYNC_GPRS)) {
>> - for (i = 0; i < 16; i++) {
>> - cs->kvm_run->s.regs.gprs[i] = env->regs[i];
>> - cs->kvm_run->kvm_dirty_regs |= KVM_SYNC_GPRS;
>> - }
>> - } else {
>> - for (i = 0; i < 16; i++) {
>> - regs.gprs[i] = env->regs[i];
>> - }
>> - r = kvm_vcpu_ioctl(cs, KVM_SET_REGS, ®s);
>> - if (r < 0) {
>> - return r;
>> - }
>> - }
>> + g_assert((cs->kvm_run->kvm_valid_regs & KVM_SYNC_REQUIRED_BITS) ==
>> + KVM_SYNC_REQUIRED_BITS);
>> + cs->kvm_run->kvm_dirty_regs |= KVM_SYNC_REQUIRED_BITS;
>> + memcpy(cs->kvm_run->s.regs.gprs, env->regs,
>> sizeof(cs->kvm_run->s.regs.gprs));
>
>
>> + memcpy(cs->kvm_run->s.regs.acrs, env->aregs,
>> sizeof(cs->kvm_run->s.regs.acrs));
>> + memcpy(cs->kvm_run->s.regs.crs, env->cregs,
>> sizeof(cs->kvm_run->s.regs.crs));
>> +
>> + cs->kvm_run->s.regs.prefix = env->psa;
>
> These 3 have only been saved for level> KVM_PUT_RUNTIME_STATE. By moving the
> memcpy
> into this position you would always write them. Probably ok but a change in
> behaviour. Was this intentional?
Oops, no, I missed that early return in the function. I'll fix it and send a v3.
Thanks!
Thomas
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2023-10-10 11:47 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-10-09 17:07 [PATCH v2 0/2] target/s390x/kvm: Simplify the synchronization code Thomas Huth
2023-10-09 17:07 ` [PATCH v2 1/2] target/s390x/kvm: Turn KVM_CAP_SYNC_REGS into a hard requirement Thomas Huth
2023-10-10 11:02 ` Christian Borntraeger
2023-10-10 11:12 ` Thomas Huth
2023-10-10 11:36 ` Christian Borntraeger
2023-10-09 17:07 ` [PATCH v2 2/2] target/s390x/kvm: Simplify the GPRs, ACRs, CRs and prefix synchronization code Thomas Huth
2023-10-10 11:41 ` Christian Borntraeger
2023-10-10 11:46 ` Thomas Huth
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).