q35: change defaults for kernel irqchip and IR

This patch allows the kvm_arch_irqchip_create() to return 0 if the split irqchip is specified but not forced by the user. Also, modify kvm_irqchip_create() similiarly.

This patch should have no functional change for existing code since currently if split is specified it must be forced by the user so we'll always have machine_kernel_irqchip_required() returns true. However it could potentially be used in follow up patches when we want to turn split kernel irqchip as default for QEMU 4.0 which could trigger the case that kernel_irqchip_required=N while kernel_irqchip_split=Y. When with that, we'll first try with split irqchip, and falls back to normal kernel irqchip when split capability is not provided by the kernel.

This brings us benefit that we can even run a default QEMU 4.0 on old kernels that does not support split irqchip (<4.4) but at the same time enable split irqchip for new kernels (>=4.4) as default.

Signed-off-by Peter Xu <[email protected]>

---
 accel/kvm/kvm-all.c | 3 ++-
 target/i386/kvm.c   | 6 +++---
 2 files changed, 5 insertions(+), 4 deletions(-)

accel/kvm/kvm-all.c +2 -1

index 4880a05399..b008364041 100644
@@ -1468,7 +1468,8 @@ static void kvm_irqchip_create(MachineState *machine, KVMState *s)
      * in-kernel irqchip for us */
     ret = kvm_arch_irqchip_create(machine, s);
     if (ret == 0) {
-        if (machine_kernel_irqchip_split(machine)) {
+        if (machine_kernel_irqchip_required(machine) &&
+            machine_kernel_irqchip_split(machine)) {
             perror("Split IRQ chip mode not supported.");
             exit(1);
         } else {

target/i386/kvm.c +3 -3

index 739cf8c8ea..8f919f8f9f 100644
@@ -3685,9 +3685,9 @@ int kvm_arch_irqchip_create(MachineState *ms, KVMState *s)
     if (machine_kernel_irqchip_split(ms)) {
         ret = kvm_vm_enable_cap(s, KVM_CAP_SPLIT_IRQCHIP, 0, 24);
         if (ret) {
-            error_report("Could not enable split irqchip mode: %s",
-                         strerror(-ret));
-            exit(1);
+            assert(ret < 0);
+            /* If split not required, return 0 instead to retry */
+            return machine_kernel_irqchip_required(ms) ? ret : 0;
         } else {
             DPRINTF("Enabled KVM_CAP_SPLIT_IRQCHIP\n");
             kvm_split_irqchip = true;

Signature · 1 line

2.17.1

Quoted 14 lines · Peter Xu

This patch allows the kvm_arch_irqchip_create() to return 0 if the split irqchip is specified but not forced by the user. Also, modify kvm_irqchip_create() similiarly.

This patch should have no functional change for existing code since currently if split is specified it must be forced by the user so we'll always have machine_kernel_irqchip_required() returns true. However it could potentially be used in follow up patches when we want to turn split kernel irqchip as default for QEMU 4.0 which could trigger the case that kernel_irqchip_required=N while kernel_irqchip_split=Y. When with that, we'll first try with split irqchip, and falls back to normal kernel irqchip when split capability is not provided by the kernel.

Quoted 5 lines

This brings us benefit that we can even run a default QEMU 4.0 on old kernels that does not support split irqchip (<4.4) but at the same time enable split irqchip for new kernels (>=4.4) as default.

Signed-off-by Peter Xu <[email protected]>

Paolo, if you can ack this one, I can merge the rest.

Quoted 39 lines

---
 accel/kvm/kvm-all.c | 3 ++-
 target/i386/kvm.c   | 6 +++---
 2 files changed, 5 insertions(+), 4 deletions(-)

accel/kvm/kvm-all.c +2 -1

index 4880a05399..b008364041 100644
@@ -1468,7 +1468,8 @@ static void kvm_irqchip_create(MachineState *machine, KVMState *s)
      * in-kernel irqchip for us */
     ret = kvm_arch_irqchip_create(machine, s);
     if (ret == 0) {
-        if (machine_kernel_irqchip_split(machine)) {
+        if (machine_kernel_irqchip_required(machine) &&
+            machine_kernel_irqchip_split(machine)) {
             perror("Split IRQ chip mode not supported.");
             exit(1);
         } else {

target/i386/kvm.c +3 -3

index 739cf8c8ea..8f919f8f9f 100644
@@ -3685,9 +3685,9 @@ int kvm_arch_irqchip_create(MachineState *ms, KVMState *s)
     if (machine_kernel_irqchip_split(ms)) {
         ret = kvm_vm_enable_cap(s, KVM_CAP_SPLIT_IRQCHIP, 0, 24);
         if (ret) {
-            error_report("Could not enable split irqchip mode: %s",
-                         strerror(-ret));
-            exit(1);
+            assert(ret < 0);
+            /* If split not required, return 0 instead to retry */
+            return machine_kernel_irqchip_required(ms) ? ret : 0;
         } else {
             DPRINTF("Enabled KVM_CAP_SPLIT_IRQCHIP\n");
             kvm_split_irqchip = true;

Signature · 1 line

2.17.1

Starting from QEMU 4.0, let's specify "split" as the default value for kernel-irqchip.

So for QEMU>=4.0 we'll have: allowed=Y,required=N,split=Y
for QEMU<=3.1 we'll have: allowed=Y,required=N,split=N (omitting all the "kernel_irqchip_" prefix)

Note that this "split" is optional - we'll first try to enable split kernel irqchip, and we'll fall back to complete kernel irqchip if we found that the kernel capability is missing.

Signed-off-by Peter Xu <[email protected]>

---
 hw/core/machine.c   | 2 ++
 hw/i386/pc_q35.c    | 2 ++
 include/hw/boards.h | 1 +
 3 files changed, 5 insertions(+)

hw/core/machine.c +2 -0

index c51423b647..4439ea663f 100644
@@ -653,8 +653,10 @@ static void machine_class_base_init(ObjectClass *oc, void *data)
 static void machine_initfn(Object *obj)
 {
     MachineState *ms = MACHINE(obj);
+    MachineClass *mc = MACHINE_GET_CLASS(obj);
 
     ms->kernel_irqchip_allowed = true;
+    ms->kernel_irqchip_split = mc->default_kernel_irqchip_split;
     ms->kvm_shadow_mem = -1;
     ms->dump_guest_core = true;
     ms->mem_merge = true;

hw/i386/pc_q35.c +2 -0

index 58459bdab5..d2fb0fa49f 100644
@@ -304,6 +304,7 @@ static void pc_q35_machine_options(MachineClass *m)
     m->units_per_default_bus = 1;
     m->default_machine_opts = "firmware=bios-256k.bin";
     m->default_display = "std";
+    m->default_kernel_irqchip_split = true;
     m->no_floppy = 1;
     machine_class_allow_dynamic_sysbus_dev(m, TYPE_AMD_IOMMU_DEVICE);
     machine_class_allow_dynamic_sysbus_dev(m, TYPE_INTEL_IOMMU_DEVICE);

@@ -323,6 +324,7 @@ DEFINE_Q35_MACHINE(v4_0, "pc-q35-4.0", NULL,
 static void pc_q35_3_1_machine_options(MachineClass *m)
 {
     pc_q35_4_0_machine_options(m);
+    m->default_kernel_irqchip_split = false;
     m->alias = NULL;
     SET_MACHINE_COMPAT(m, PC_COMPAT_3_1);
 }

include/hw/boards.h +1 -0

index f82f28468b..362384815e 100644
@@ -195,6 +195,7 @@ struct MachineClass {
     const char *hw_version;
     ram_addr_t default_ram_size;
     const char *default_cpu_type;
+    bool default_kernel_irqchip_split;
     bool option_rom_has_mr;
     bool rom_file_has_mr;
     int minimum_page_bits;

Signature · 1 line

2.17.1

Quoted 11 lines · Peter Xu

Starting from QEMU 4.0, let's specify "split" as the default value for kernel-irqchip.

So for QEMU>=4.0 we'll have: allowed=Y,required=N,split=Y
for QEMU<=3.1 we'll have: allowed=Y,required=N,split=N (omitting all the "kernel_irqchip_" prefix)

Note that this "split" is optional - we'll first try to enable split kernel irqchip, and we'll fall back to complete kernel irqchip if we found that the kernel capability is missing.

Quoted 1 line

Signed-off-by Peter Xu <[email protected]>

I'm split on this one ;)
On the one hand why not make pc and q35 are consistent here? On the other hand we really should work to leave PC alone as much as we can ...

Paolo, what's your opinion?

Starting from QEMU 4.0, let's specify "split" as the default value for kernel-irqchip.

So for QEMU>=4.0 we'll have: allowed=Y,required=N,split=Y
for QEMU<=3.1 we'll have: allowed=Y,required=N,split=N (omitting all the "kernel_irqchip_" prefix)

Note that this "split" is optional - we'll first try to enable split kernel irqchip, and we'll fall back to complete kernel irqchip if we found that the kernel capability is missing.

Quoted 7 lines · Peter Xu

Starting from QEMU 4.0, let's specify "split" as the default value for kernel-irqchip.

So for QEMU>=4.0 we'll have: allowed=Y,required=N,split=Y
for QEMU<=3.1 we'll have: allowed=Y,required=N,split=N (omitting all the "kernel_irqchip_" prefix)

Quoted 3 lines

Note that this "split" is optional - we'll first try to enable split kernel irqchip, and we'll fall back to complete kernel irqchip if we found that the kernel capability is missing.

Please just fail completely and require a new kernel for the 4.0 machine type. There are subtle differences between kernel and QEMU irqchip, I don't think we want to open that can of worms.

Quoted 8 lines · Paolo Bonzini

Quoted 7 lines · Peter Xu

Starting from QEMU 4.0, let's specify "split" as the default value for kernel-irqchip.

So for QEMU>=4.0 we'll have: allowed=Y,required=N,split=Y
for QEMU<=3.1 we'll have: allowed=Y,required=N,split=N (omitting all the "kernel_irqchip_" prefix)

Quoted 7 lines

Quoted 3 lines

Note that this "split" is optional - we'll first try to enable split kernel irqchip, and we'll fall back to complete kernel irqchip if we found that the kernel capability is missing.

Please just fail completely and require a new kernel for the 4.0 machine type. There are subtle differences between kernel and QEMU irqchip, I don't think we want to open that can of worms.

This would make existing VMs that are runnable with pc-q35-3.1.0 not runnable by only updating the machine-type.

The good news is that we can make this a non-issue by clearly documenting that QEMU needs a more recent kernel (just like we'll do for RDTSCP[1]).

Quoted 17 lines · Eduardo Habkost

Quoted 15 lines · Paolo Bonzini

Quoted 10 lines · Peter Xu

Starting from QEMU 4.0, let's specify "split" as the default value for kernel-irqchip.

So for QEMU>=4.0 we'll have: allowed=Y,required=N,split=Y
for QEMU<=3.1 we'll have: allowed=Y,required=N,split=N (omitting all the "kernel_irqchip_" prefix)

Note that this "split" is optional - we'll first try to enable split kernel irqchip, and we'll fall back to complete kernel irqchip if we found that the kernel capability is missing.

Please just fail completely and require a new kernel for the 4.0 machine type. There are subtle differences between kernel and QEMU irqchip, I don't think we want to open that can of worms.

Quoted 6 lines

This would make existing VMs that are runnable with pc-q35-3.1.0 not runnable by only updating the machine-type.

The good news is that we can make this a non-issue by clearly documenting that QEMU needs a more recent kernel (just like we'll do for RDTSCP[1]).

Right, RDTSCP is exactly what came to mind.