| Message ID | 20200207043055.218856-2-david@gibson.dropbear.id.au (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2] spapr: Disable legacy virtio devices for pseries-5.0 and later | expand |
On Fri, Feb 07, 2020 at 03:30:54PM +1100, David Gibson wrote: > PAPR specifies a kind of odd, paravirtualized PCI bus, which looks to > the guess mostly like classic PCI, even if some of the individual > devices on the bus are PCI Express. One consequence of that is that > virtio-pci devices still default to being in transitional mode, though > legacy mode is now disabled by default on current q35 x86 machine > types. > > Legacy mode virtio devices aren't really necessary any more, and are > causing some problems for future changes. Therefore, for the > pseries-5.0 machine type (and onwards), switch to modern-only > virtio-pci devices by default. It's worth noting in the commit log that this disables support for guests older than Linux 4.0. > > Signed-off-by: David Gibson <david@gibson.dropbear.id.au> > --- > hw/ppc/spapr.c | 14 +++++++++++++- > 1 file changed, 13 insertions(+), 1 deletion(-) > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > index c9b2e0a5e0..216d3b34dc 100644 > --- a/hw/ppc/spapr.c > +++ b/hw/ppc/spapr.c > @@ -65,6 +65,7 @@ > > #include "hw/pci/pci.h" > #include "hw/scsi/scsi.h" > +#include "hw/virtio/virtio-pci.h" > #include "hw/virtio/virtio-scsi.h" > #include "hw/virtio/vhost-scsi-common.h" > > @@ -4512,7 +4513,14 @@ static const TypeInfo spapr_machine_info = { > */ > static void spapr_machine_5_0_class_options(MachineClass *mc) > { > - /* Defaults for the latest behaviour inherited from the base class */ > + /* Most defaults for the latest behaviour are inherited from the > + * base class, but we need to override the (non ppc specific) > + * default behaviour for virtio */ > + static GlobalProperty compat[] = { > + { TYPE_VIRTIO_PCI, "disable-legacy", "on", }, > + }; > + > + compat_props_add(mc->compat_props, compat, G_N_ELEMENTS(compat)); > } > > DEFINE_SPAPR_MACHINE(5_0, "5.0", true); So this sets the defaults, right? Problem is we'll then need to remember to carry this in the latest call. If we forget we get a mess. How about adding a call to e.g. spapr_machine_latest_class_options in DEFINE_SPAPR_MACHINE? Then spapr_machine_latest_class_options can set the per-machine defaults. I send a patch for this: [PATCH] ppc: function to setup latest class options feel free to reuse. > @@ -4523,11 +4531,15 @@ DEFINE_SPAPR_MACHINE(5_0, "5.0", true); > static void spapr_machine_4_2_class_options(MachineClass *mc) > { > SpaprMachineClass *smc = SPAPR_MACHINE_CLASS(mc); > + static GlobalProperty compat[] = { > + { TYPE_VIRTIO_PCI, "disable-legacy", "auto" }, > + }; > > spapr_machine_5_0_class_options(mc); > compat_props_add(mc->compat_props, hw_compat_4_2, hw_compat_4_2_len); > smc->default_caps.caps[SPAPR_CAP_CCF_ASSIST] = SPAPR_CAP_OFF; > smc->default_caps.caps[SPAPR_CAP_FWNMI_MCE] = SPAPR_CAP_OFF; > + compat_props_add(mc->compat_props, compat, G_N_ELEMENTS(compat)); > } > > DEFINE_SPAPR_MACHINE(4_2, "4.2", false); > -- > 2.24.1
On Fri, Feb 07, 2020 at 01:54:26AM -0500, Michael S. Tsirkin wrote: > On Fri, Feb 07, 2020 at 03:30:54PM +1100, David Gibson wrote: > > PAPR specifies a kind of odd, paravirtualized PCI bus, which looks to > > the guess mostly like classic PCI, even if some of the individual > > devices on the bus are PCI Express. One consequence of that is that > > virtio-pci devices still default to being in transitional mode, though > > legacy mode is now disabled by default on current q35 x86 machine > > types. > > > > Legacy mode virtio devices aren't really necessary any more, and are > > causing some problems for future changes. Therefore, for the > > pseries-5.0 machine type (and onwards), switch to modern-only > > virtio-pci devices by default. > > It's worth noting in the commit log that this disables support > for guests older than Linux 4.0. Oof. That's more recent than I'd guessed. I'll have to do some checking to see if that's reasonable. > > > > > Signed-off-by: David Gibson <david@gibson.dropbear.id.au> > > --- > > hw/ppc/spapr.c | 14 +++++++++++++- > > 1 file changed, 13 insertions(+), 1 deletion(-) > > > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > > index c9b2e0a5e0..216d3b34dc 100644 > > --- a/hw/ppc/spapr.c > > +++ b/hw/ppc/spapr.c > > @@ -65,6 +65,7 @@ > > > > #include "hw/pci/pci.h" > > #include "hw/scsi/scsi.h" > > +#include "hw/virtio/virtio-pci.h" > > #include "hw/virtio/virtio-scsi.h" > > #include "hw/virtio/vhost-scsi-common.h" > > > > @@ -4512,7 +4513,14 @@ static const TypeInfo spapr_machine_info = { > > */ > > static void spapr_machine_5_0_class_options(MachineClass *mc) > > { > > - /* Defaults for the latest behaviour inherited from the base class */ > > + /* Most defaults for the latest behaviour are inherited from the > > + * base class, but we need to override the (non ppc specific) > > + * default behaviour for virtio */ > > + static GlobalProperty compat[] = { > > + { TYPE_VIRTIO_PCI, "disable-legacy", "on", }, > > + }; > > + > > + compat_props_add(mc->compat_props, compat, G_N_ELEMENTS(compat)); > > } > > > > DEFINE_SPAPR_MACHINE(5_0, "5.0", true); > > So this sets the defaults, right? > > Problem is we'll then need to remember to carry this in the latest > call. If we forget we get a mess. > > How about adding a call to e.g. spapr_machine_latest_class_options > in DEFINE_SPAPR_MACHINE? > Then spapr_machine_latest_class_options can set the per-machine > defaults. > > I send a patch for this: > [PATCH] ppc: function to setup latest class options > feel free to reuse. Good idea, thanks. I've merged that and will rebase these changes on it. > > > > @@ -4523,11 +4531,15 @@ DEFINE_SPAPR_MACHINE(5_0, "5.0", true); > > static void spapr_machine_4_2_class_options(MachineClass *mc) > > { > > SpaprMachineClass *smc = SPAPR_MACHINE_CLASS(mc); > > + static GlobalProperty compat[] = { > > + { TYPE_VIRTIO_PCI, "disable-legacy", "auto" }, > > + }; > > > > spapr_machine_5_0_class_options(mc); > > compat_props_add(mc->compat_props, hw_compat_4_2, hw_compat_4_2_len); > > smc->default_caps.caps[SPAPR_CAP_CCF_ASSIST] = SPAPR_CAP_OFF; > > smc->default_caps.caps[SPAPR_CAP_FWNMI_MCE] = SPAPR_CAP_OFF; > > + compat_props_add(mc->compat_props, compat, G_N_ELEMENTS(compat)); > > } > > > > DEFINE_SPAPR_MACHINE(4_2, "4.2", false); >
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index c9b2e0a5e0..216d3b34dc 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -65,6 +65,7 @@ #include "hw/pci/pci.h" #include "hw/scsi/scsi.h" +#include "hw/virtio/virtio-pci.h" #include "hw/virtio/virtio-scsi.h" #include "hw/virtio/vhost-scsi-common.h" @@ -4512,7 +4513,14 @@ static const TypeInfo spapr_machine_info = { */ static void spapr_machine_5_0_class_options(MachineClass *mc) { - /* Defaults for the latest behaviour inherited from the base class */ + /* Most defaults for the latest behaviour are inherited from the + * base class, but we need to override the (non ppc specific) + * default behaviour for virtio */ + static GlobalProperty compat[] = { + { TYPE_VIRTIO_PCI, "disable-legacy", "on", }, + }; + + compat_props_add(mc->compat_props, compat, G_N_ELEMENTS(compat)); } DEFINE_SPAPR_MACHINE(5_0, "5.0", true); @@ -4523,11 +4531,15 @@ DEFINE_SPAPR_MACHINE(5_0, "5.0", true); static void spapr_machine_4_2_class_options(MachineClass *mc) { SpaprMachineClass *smc = SPAPR_MACHINE_CLASS(mc); + static GlobalProperty compat[] = { + { TYPE_VIRTIO_PCI, "disable-legacy", "auto" }, + }; spapr_machine_5_0_class_options(mc); compat_props_add(mc->compat_props, hw_compat_4_2, hw_compat_4_2_len); smc->default_caps.caps[SPAPR_CAP_CCF_ASSIST] = SPAPR_CAP_OFF; smc->default_caps.caps[SPAPR_CAP_FWNMI_MCE] = SPAPR_CAP_OFF; + compat_props_add(mc->compat_props, compat, G_N_ELEMENTS(compat)); } DEFINE_SPAPR_MACHINE(4_2, "4.2", false);
PAPR specifies a kind of odd, paravirtualized PCI bus, which looks to the guess mostly like classic PCI, even if some of the individual devices on the bus are PCI Express. One consequence of that is that virtio-pci devices still default to being in transitional mode, though legacy mode is now disabled by default on current q35 x86 machine types. Legacy mode virtio devices aren't really necessary any more, and are causing some problems for future changes. Therefore, for the pseries-5.0 machine type (and onwards), switch to modern-only virtio-pci devices by default. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> --- hw/ppc/spapr.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-)