| Message ID | 20181219085038.7729-3-peterx@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | q35: change defaults for kernel irqchip and IR | expand |
On Wed, Dec 19, 2018 at 04:50:36PM +0800, Peter Xu wrote: > 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 <peterx@redhat.com> 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? > --- > hw/core/machine.c | 2 ++ > hw/i386/pc_q35.c | 2 ++ > include/hw/boards.h | 1 + > 3 files changed, 5 insertions(+) > > diff --git a/hw/core/machine.c b/hw/core/machine.c > index c51423b647..4439ea663f 100644 > --- a/hw/core/machine.c > +++ b/hw/core/machine.c > @@ -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; > diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c > index 58459bdab5..d2fb0fa49f 100644 > --- a/hw/i386/pc_q35.c > +++ b/hw/i386/pc_q35.c > @@ -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); > } > diff --git a/include/hw/boards.h b/include/hw/boards.h > index f82f28468b..362384815e 100644 > --- a/include/hw/boards.h > +++ b/include/hw/boards.h > @@ -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; > -- > 2.17.1
On 19/12/18 09:50, Peter Xu wrote: > 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. Paolo
On 19/12/18 16:52, Michael S. Tsirkin wrote: > On Wed, Dec 19, 2018 at 04:50:36PM +0800, Peter Xu wrote: >> 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 <peterx@redhat.com> > > 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? The idea was to avoid bumping the minimal kernel version for PC, only for Q35. PC can still use split irqchip for security purposes, but only Q35 needs it for features. Paolo
On Wed, Dec 19, 2018 at 09:12:55PM +0100, Paolo Bonzini wrote: > On 19/12/18 09:50, Peter Xu wrote: > > 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. 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]). [1] https://lore.kernel.org/lkml/20181210181328.GA762@zn.tnic/
On 19/12/18 22:24, Eduardo Habkost wrote: > On Wed, Dec 19, 2018 at 09:12:55PM +0100, Paolo Bonzini wrote: >> On 19/12/18 09:50, Peter Xu wrote: >>> 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. > > 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. Paolo
On Wed, Dec 19, 2018 at 10:45:40PM +0100, Paolo Bonzini wrote: > On 19/12/18 22:24, Eduardo Habkost wrote: > > On Wed, Dec 19, 2018 at 09:12:55PM +0100, Paolo Bonzini wrote: > >> On 19/12/18 09:50, Peter Xu wrote: > >>> 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. > > > > 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. Ok so I think I'll just make it even simpler by dropping patch 1. Also I noticed that the documentation on linux kernel version requirement has not yet reached master but I'll assume it'll be there some day very soon so I'll ignore that part. Thanks everyone! I'll repost soon.
diff --git a/hw/core/machine.c b/hw/core/machine.c index c51423b647..4439ea663f 100644 --- a/hw/core/machine.c +++ b/hw/core/machine.c @@ -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; diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c index 58459bdab5..d2fb0fa49f 100644 --- a/hw/i386/pc_q35.c +++ b/hw/i386/pc_q35.c @@ -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); } diff --git a/include/hw/boards.h b/include/hw/boards.h index f82f28468b..362384815e 100644 --- a/include/hw/boards.h +++ b/include/hw/boards.h @@ -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;
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 <peterx@redhat.com> --- hw/core/machine.c | 2 ++ hw/i386/pc_q35.c | 2 ++ include/hw/boards.h | 1 + 3 files changed, 5 insertions(+)