Re: [PATCH v2 1/2] target/s390x/kvm: Turn KVM_CAP_SYNC_REGS into a hard requirement

2023-10-10 Thread Christian Borntraeger 




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 
---
  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 


Thanks,
   Thomas

Re: [PATCH v2 1/2] target/s390x/kvm: Turn KVM_CAP_SYNC_REGS into a hard requirement

2023-10-10 Thread 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 
---
  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 


Thanks,
  Thomas

Re: [PATCH v2 1/2] target/s390x/kvm: Turn KVM_CAP_SYNC_REGS into a hard requirement

2023-10-10 Thread Christian Borntraeger 




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 
---
  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 


  
-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);

[PATCH v2 1/2] target/s390x/kvm: Turn KVM_CAP_SYNC_REGS into a hard requirement

2023-10-09 Thread 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 
---
 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