* [PATCH v3 0/2] spapr: Use vIOMMU translation for virtio by default @ 2020-03-05 4:30 David Gibson 2020-03-05 4:30 ` [PATCH v3 1/2] spapr: Disable legacy virtio devices for pseries-5.0 and later David Gibson ` (3 more replies) 0 siblings, 4 replies; 29+ messages in thread From: David Gibson @ 2020-03-05 4:30 UTC (permalink / raw) To: groug, pair, qemu-devel, clg Cc: mst, aik, paulus, mdroth, qemu-ppc, David Gibson Upcoming Secure VM support for pSeries machines introduces some complications for virtio, since the transfer buffers need to be explicitly shared so that the hypervisor can access them. While it's not strictly speaking dependent on it, the fact that virtio devices bypass normal platform IOMMU translation complicates the issue on the guest side. Since there are some significan downsides to bypassing the vIOMMU anyway, let's just disable that. There's already a flag to do this in virtio, just turn it on by default for forthcoming pseries machine types. Any opinions on whether dropping support for the older guest kernels is acceptable at this point? Changes since v2: * Rebase and improve some comments Changes since v1: * Added information on which guest kernel versions will no longer work with these changes * Use Michael Tsirkin's suggested better way of handling the machine type change David Gibson (2): spapr: Disable legacy virtio devices for pseries-5.0 and later spapr: Enable virtio iommu_platform=on by default hw/ppc/spapr.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) -- 2.24.1 ^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH v3 1/2] spapr: Disable legacy virtio devices for pseries-5.0 and later 2020-03-05 4:30 [PATCH v3 0/2] spapr: Use vIOMMU translation for virtio by default David Gibson @ 2020-03-05 4:30 ` David Gibson 2020-03-05 10:31 ` Greg Kurz 2020-03-10 11:56 ` Daniel P. Berrangé 2020-03-05 4:30 ` [PATCH v3 2/2] spapr: Enable virtio iommu_platform=on by default David Gibson ` (2 subsequent siblings) 3 siblings, 2 replies; 29+ messages in thread From: David Gibson @ 2020-03-05 4:30 UTC (permalink / raw) To: groug, pair, qemu-devel, clg Cc: mst, aik, paulus, mdroth, qemu-ppc, David Gibson 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. This does mean we no longer support guest kernels prior to 4.0, unless they have modern virtio support backported (which some distro kernels like that in RHEL7 do). Signed-off-by: David Gibson <david@gibson.dropbear.id.au> --- hw/ppc/spapr.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index 2eb0d8f70d..3cfc98ac61 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" @@ -4566,8 +4567,20 @@ static const TypeInfo spapr_machine_info = { static void spapr_machine_latest_class_options(MachineClass *mc) { + /* + * 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. We can't do that from the base + * class since it doesn't have a compat_props. + */ + static GlobalProperty compat[] = { + { TYPE_VIRTIO_PCI, "disable-legacy", "on", }, + }; + mc->alias = "pseries"; mc->is_default = true; + + compat_props_add(mc->compat_props, compat, G_N_ELEMENTS(compat)); } #define DEFINE_SPAPR_MACHINE(suffix, verstr, latest) \ @@ -4607,6 +4620,9 @@ 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); @@ -4614,6 +4630,7 @@ static void spapr_machine_4_2_class_options(MachineClass *mc) smc->default_caps.caps[SPAPR_CAP_FWNMI_MCE] = SPAPR_CAP_OFF; smc->rma_limit = 16 * GiB; mc->nvdimm_supported = false; + compat_props_add(mc->compat_props, compat, G_N_ELEMENTS(compat)); } DEFINE_SPAPR_MACHINE(4_2, "4.2", false); -- 2.24.1 ^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [PATCH v3 1/2] spapr: Disable legacy virtio devices for pseries-5.0 and later 2020-03-05 4:30 ` [PATCH v3 1/2] spapr: Disable legacy virtio devices for pseries-5.0 and later David Gibson @ 2020-03-05 10:31 ` Greg Kurz 2020-03-10 9:43 ` Greg Kurz 2020-03-10 11:56 ` Daniel P. Berrangé 1 sibling, 1 reply; 29+ messages in thread From: Greg Kurz @ 2020-03-05 10:31 UTC (permalink / raw) To: David Gibson; +Cc: pair, mst, aik, qemu-devel, paulus, clg, mdroth, qemu-ppc On Thu, 5 Mar 2020 15:30:08 +1100 David Gibson <david@gibson.dropbear.id.au> 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. > > This does mean we no longer support guest kernels prior to 4.0, unless > they have modern virtio support backported (which some distro kernels > like that in RHEL7 do). > > Signed-off-by: David Gibson <david@gibson.dropbear.id.au> > --- Reviewed-by: Greg Kurz <groug@kaod.org> FWIW, I could test the following: - allows a RHEL7 guest with pre 4.0 kernel to boot, as mentioned in the changelog - breaks boot of older RHEL 6.10 guests as expected - allows migration of older machine types to/from QEMU 4.2 Tested-by: Greg Kurz <groug@kaod.org> > hw/ppc/spapr.c | 17 +++++++++++++++++ > 1 file changed, 17 insertions(+) > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > index 2eb0d8f70d..3cfc98ac61 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" > > @@ -4566,8 +4567,20 @@ static const TypeInfo spapr_machine_info = { > > static void spapr_machine_latest_class_options(MachineClass *mc) > { > + /* > + * 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. We can't do that from the base > + * class since it doesn't have a compat_props. > + */ > + static GlobalProperty compat[] = { > + { TYPE_VIRTIO_PCI, "disable-legacy", "on", }, > + }; > + > mc->alias = "pseries"; > mc->is_default = true; > + > + compat_props_add(mc->compat_props, compat, G_N_ELEMENTS(compat)); > } > > #define DEFINE_SPAPR_MACHINE(suffix, verstr, latest) \ > @@ -4607,6 +4620,9 @@ 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); > @@ -4614,6 +4630,7 @@ static void spapr_machine_4_2_class_options(MachineClass *mc) > smc->default_caps.caps[SPAPR_CAP_FWNMI_MCE] = SPAPR_CAP_OFF; > smc->rma_limit = 16 * GiB; > mc->nvdimm_supported = false; > + compat_props_add(mc->compat_props, compat, G_N_ELEMENTS(compat)); > } > > DEFINE_SPAPR_MACHINE(4_2, "4.2", false); ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v3 1/2] spapr: Disable legacy virtio devices for pseries-5.0 and later 2020-03-05 10:31 ` Greg Kurz @ 2020-03-10 9:43 ` Greg Kurz 2020-03-10 9:57 ` Michael S. Tsirkin 2020-03-10 11:03 ` Michael S. Tsirkin 0 siblings, 2 replies; 29+ messages in thread From: Greg Kurz @ 2020-03-10 9:43 UTC (permalink / raw) To: David Gibson; +Cc: pair, mst, aik, qemu-devel, paulus, clg, mdroth, qemu-ppc On Thu, 5 Mar 2020 11:31:46 +0100 Greg Kurz <groug@kaod.org> wrote: > On Thu, 5 Mar 2020 15:30:08 +1100 > David Gibson <david@gibson.dropbear.id.au> 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. > > > > This does mean we no longer support guest kernels prior to 4.0, unless > > they have modern virtio support backported (which some distro kernels > > like that in RHEL7 do). > > > > Signed-off-by: David Gibson <david@gibson.dropbear.id.au> > > --- > > Reviewed-by: Greg Kurz <groug@kaod.org> > > FWIW, I could test the following: > - allows a RHEL7 guest with pre 4.0 kernel to boot, as mentioned > in the changelog > - breaks boot of older RHEL 6.10 guests as expected > - allows migration of older machine types to/from QEMU 4.2 > > Tested-by: Greg Kurz <groug@kaod.org> > Wait... I gave a try to virtiofsd and there's a problem: $ ./ppc64-softmmu/qemu-system-ppc64 -device vhost-user-fs-pci Unexpected error in object_property_find() at /home/greg/Work/qemu/qemu-ppc/qom/object.c:1231: qemu-system-ppc64: -device vhost-user-fs-pci: can't apply global virtio-pci.disable-legacy=on: Property '.disable-legacy' not found Aborted (core dumped) It is still not possible to set the disable-legacy prop on the vhost-user-fs-pci device, even without this patch, but QEMU doesn't abort: $ ./ppc64-softmmu/qemu-system-ppc64 -device vhost-user-fs-pci -global virtio-pci.disable-legacy=on qemu-system-ppc64: -device vhost-user-fs-pci: can't apply global virtio-pci.disable-legacy=on: Property '.disable-legacy' not found $ It seems to be related to the fact that vhost-user-fs-pci is a non-transitional only device, as shown with this workaround: --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -4574,7 +4574,7 @@ static void spapr_machine_latest_class_options(MachineClass *mc) * class since it doesn't have a compat_props. */ static GlobalProperty compat[] = { - { TYPE_VIRTIO_PCI, "disable-legacy", "on", }, + { TYPE_VIRTIO_PCI "-transitional", "disable-legacy", "on", }, }; mc->alias = "pseries"; but there's probably a better way to address this. MST, Any suggestion ? > > hw/ppc/spapr.c | 17 +++++++++++++++++ > > 1 file changed, 17 insertions(+) > > > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > > index 2eb0d8f70d..3cfc98ac61 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" > > > > @@ -4566,8 +4567,20 @@ static const TypeInfo spapr_machine_info = { > > > > static void spapr_machine_latest_class_options(MachineClass *mc) > > { > > + /* > > + * 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. We can't do that from the base > > + * class since it doesn't have a compat_props. > > + */ > > + static GlobalProperty compat[] = { > > + { TYPE_VIRTIO_PCI, "disable-legacy", "on", }, > > + }; > > + > > mc->alias = "pseries"; > > mc->is_default = true; > > + > > + compat_props_add(mc->compat_props, compat, G_N_ELEMENTS(compat)); > > } > > > > #define DEFINE_SPAPR_MACHINE(suffix, verstr, latest) \ > > @@ -4607,6 +4620,9 @@ 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); > > @@ -4614,6 +4630,7 @@ static void spapr_machine_4_2_class_options(MachineClass *mc) > > smc->default_caps.caps[SPAPR_CAP_FWNMI_MCE] = SPAPR_CAP_OFF; > > smc->rma_limit = 16 * GiB; > > mc->nvdimm_supported = false; > > + compat_props_add(mc->compat_props, compat, G_N_ELEMENTS(compat)); > > } > > > > DEFINE_SPAPR_MACHINE(4_2, "4.2", false); > ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v3 1/2] spapr: Disable legacy virtio devices for pseries-5.0 and later 2020-03-10 9:43 ` Greg Kurz @ 2020-03-10 9:57 ` Michael S. Tsirkin 2020-03-10 11:03 ` Michael S. Tsirkin 1 sibling, 0 replies; 29+ messages in thread From: Michael S. Tsirkin @ 2020-03-10 9:57 UTC (permalink / raw) To: Greg Kurz Cc: pair, aik, qemu-devel, paulus, clg, mdroth, qemu-ppc, David Gibson On Tue, Mar 10, 2020 at 10:43:29AM +0100, Greg Kurz wrote: > On Thu, 5 Mar 2020 11:31:46 +0100 > Greg Kurz <groug@kaod.org> wrote: > > > On Thu, 5 Mar 2020 15:30:08 +1100 > > David Gibson <david@gibson.dropbear.id.au> 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. > > > > > > This does mean we no longer support guest kernels prior to 4.0, unless > > > they have modern virtio support backported (which some distro kernels > > > like that in RHEL7 do). > > > > > > Signed-off-by: David Gibson <david@gibson.dropbear.id.au> > > > --- > > > > Reviewed-by: Greg Kurz <groug@kaod.org> > > > > FWIW, I could test the following: > > - allows a RHEL7 guest with pre 4.0 kernel to boot, as mentioned > > in the changelog > > - breaks boot of older RHEL 6.10 guests as expected > > - allows migration of older machine types to/from QEMU 4.2 > > > > Tested-by: Greg Kurz <groug@kaod.org> > > > > Wait... I gave a try to virtiofsd and there's a problem: > > $ ./ppc64-softmmu/qemu-system-ppc64 -device vhost-user-fs-pci > Unexpected error in object_property_find() at /home/greg/Work/qemu/qemu-ppc/qom/object.c:1231: > qemu-system-ppc64: -device vhost-user-fs-pci: can't apply global virtio-pci.disable-legacy=on: Property '.disable-legacy' not found > Aborted (core dumped) > > It is still not possible to set the disable-legacy prop on the > vhost-user-fs-pci device, even without this patch, but QEMU > doesn't abort: > > $ ./ppc64-softmmu/qemu-system-ppc64 -device vhost-user-fs-pci -global virtio-pci.disable-legacy=on > qemu-system-ppc64: -device vhost-user-fs-pci: can't apply global virtio-pci.disable-legacy=on: Property '.disable-legacy' not found > $ > > It seems to be related to the fact that vhost-user-fs-pci is a non-transitional > only device, as shown with this workaround: > > --- a/hw/ppc/spapr.c > +++ b/hw/ppc/spapr.c > @@ -4574,7 +4574,7 @@ static void spapr_machine_latest_class_options(MachineClass *mc) > * class since it doesn't have a compat_props. > */ > static GlobalProperty compat[] = { > - { TYPE_VIRTIO_PCI, "disable-legacy", "on", }, > + { TYPE_VIRTIO_PCI "-transitional", "disable-legacy", "on", }, > }; > > mc->alias = "pseries"; > > > but there's probably a better way to address this. > > MST, Any suggestion ? Ugh looks like we have two types named virtio-pci? Something's very wrong ... > > > hw/ppc/spapr.c | 17 +++++++++++++++++ > > > 1 file changed, 17 insertions(+) > > > > > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > > > index 2eb0d8f70d..3cfc98ac61 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" > > > > > > @@ -4566,8 +4567,20 @@ static const TypeInfo spapr_machine_info = { > > > > > > static void spapr_machine_latest_class_options(MachineClass *mc) > > > { > > > + /* > > > + * 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. We can't do that from the base > > > + * class since it doesn't have a compat_props. > > > + */ > > > + static GlobalProperty compat[] = { > > > + { TYPE_VIRTIO_PCI, "disable-legacy", "on", }, > > > + }; > > > + > > > mc->alias = "pseries"; > > > mc->is_default = true; > > > + > > > + compat_props_add(mc->compat_props, compat, G_N_ELEMENTS(compat)); > > > } > > > > > > #define DEFINE_SPAPR_MACHINE(suffix, verstr, latest) \ > > > @@ -4607,6 +4620,9 @@ 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); > > > @@ -4614,6 +4630,7 @@ static void spapr_machine_4_2_class_options(MachineClass *mc) > > > smc->default_caps.caps[SPAPR_CAP_FWNMI_MCE] = SPAPR_CAP_OFF; > > > smc->rma_limit = 16 * GiB; > > > mc->nvdimm_supported = false; > > > + compat_props_add(mc->compat_props, compat, G_N_ELEMENTS(compat)); > > > } > > > > > > DEFINE_SPAPR_MACHINE(4_2, "4.2", false); > > ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v3 1/2] spapr: Disable legacy virtio devices for pseries-5.0 and later 2020-03-10 9:43 ` Greg Kurz 2020-03-10 9:57 ` Michael S. Tsirkin @ 2020-03-10 11:03 ` Michael S. Tsirkin 2020-03-10 12:24 ` Greg Kurz 1 sibling, 1 reply; 29+ messages in thread From: Michael S. Tsirkin @ 2020-03-10 11:03 UTC (permalink / raw) To: Greg Kurz Cc: pair, aik, qemu-devel, paulus, clg, mdroth, qemu-ppc, David Gibson On Tue, Mar 10, 2020 at 10:43:29AM +0100, Greg Kurz wrote: > On Thu, 5 Mar 2020 11:31:46 +0100 > Greg Kurz <groug@kaod.org> wrote: > > > On Thu, 5 Mar 2020 15:30:08 +1100 > > David Gibson <david@gibson.dropbear.id.au> 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. > > > > > > This does mean we no longer support guest kernels prior to 4.0, unless > > > they have modern virtio support backported (which some distro kernels > > > like that in RHEL7 do). > > > > > > Signed-off-by: David Gibson <david@gibson.dropbear.id.au> > > > --- > > > > Reviewed-by: Greg Kurz <groug@kaod.org> > > > > FWIW, I could test the following: > > - allows a RHEL7 guest with pre 4.0 kernel to boot, as mentioned > > in the changelog > > - breaks boot of older RHEL 6.10 guests as expected > > - allows migration of older machine types to/from QEMU 4.2 > > > > Tested-by: Greg Kurz <groug@kaod.org> > > > > Wait... I gave a try to virtiofsd and there's a problem: > > $ ./ppc64-softmmu/qemu-system-ppc64 -device vhost-user-fs-pci > Unexpected error in object_property_find() at /home/greg/Work/qemu/qemu-ppc/qom/object.c:1231: > qemu-system-ppc64: -device vhost-user-fs-pci: can't apply global virtio-pci.disable-legacy=on: Property '.disable-legacy' not found > Aborted (core dumped) > > It is still not possible to set the disable-legacy prop on the > vhost-user-fs-pci device, even without this patch, but QEMU > doesn't abort: > > $ ./ppc64-softmmu/qemu-system-ppc64 -device vhost-user-fs-pci -global virtio-pci.disable-legacy=on > qemu-system-ppc64: -device vhost-user-fs-pci: can't apply global virtio-pci.disable-legacy=on: Property '.disable-legacy' not found > $ > > It seems to be related to the fact that vhost-user-fs-pci is a non-transitional > only device, as shown with this workaround: > > --- a/hw/ppc/spapr.c > +++ b/hw/ppc/spapr.c > @@ -4574,7 +4574,7 @@ static void spapr_machine_latest_class_options(MachineClass *mc) > * class since it doesn't have a compat_props. > */ > static GlobalProperty compat[] = { > - { TYPE_VIRTIO_PCI, "disable-legacy", "on", }, > + { TYPE_VIRTIO_PCI "-transitional", "disable-legacy", "on", }, > }; > > mc->alias = "pseries"; Does this actually help? There's no type virtio-pci-transitional, is there? > > but there's probably a better way to address this. > > MST, Any suggestion ? Hmm I'm not sure how to fix this properly. The only idea that comes to mind is a new internal-only "x-disable-legacy" that would be on virtio-pci, duplicating functionality of disable-legacy but intended for globals like this. > > > hw/ppc/spapr.c | 17 +++++++++++++++++ > > > 1 file changed, 17 insertions(+) > > > > > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > > > index 2eb0d8f70d..3cfc98ac61 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" > > > > > > @@ -4566,8 +4567,20 @@ static const TypeInfo spapr_machine_info = { > > > > > > static void spapr_machine_latest_class_options(MachineClass *mc) > > > { > > > + /* > > > + * 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. We can't do that from the base > > > + * class since it doesn't have a compat_props. > > > + */ > > > + static GlobalProperty compat[] = { > > > + { TYPE_VIRTIO_PCI, "disable-legacy", "on", }, > > > + }; > > > + > > > mc->alias = "pseries"; > > > mc->is_default = true; > > > + > > > + compat_props_add(mc->compat_props, compat, G_N_ELEMENTS(compat)); > > > } > > > > > > #define DEFINE_SPAPR_MACHINE(suffix, verstr, latest) \ > > > @@ -4607,6 +4620,9 @@ 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); > > > @@ -4614,6 +4630,7 @@ static void spapr_machine_4_2_class_options(MachineClass *mc) > > > smc->default_caps.caps[SPAPR_CAP_FWNMI_MCE] = SPAPR_CAP_OFF; > > > smc->rma_limit = 16 * GiB; > > > mc->nvdimm_supported = false; > > > + compat_props_add(mc->compat_props, compat, G_N_ELEMENTS(compat)); > > > } > > > > > > DEFINE_SPAPR_MACHINE(4_2, "4.2", false); > > ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v3 1/2] spapr: Disable legacy virtio devices for pseries-5.0 and later 2020-03-10 11:03 ` Michael S. Tsirkin @ 2020-03-10 12:24 ` Greg Kurz 0 siblings, 0 replies; 29+ messages in thread From: Greg Kurz @ 2020-03-10 12:24 UTC (permalink / raw) To: Michael S. Tsirkin Cc: pair, aik, qemu-devel, paulus, clg, mdroth, qemu-ppc, David Gibson On Tue, 10 Mar 2020 07:03:34 -0400 "Michael S. Tsirkin" <mst@redhat.com> wrote: > On Tue, Mar 10, 2020 at 10:43:29AM +0100, Greg Kurz wrote: > > On Thu, 5 Mar 2020 11:31:46 +0100 > > Greg Kurz <groug@kaod.org> wrote: > > > > > On Thu, 5 Mar 2020 15:30:08 +1100 > > > David Gibson <david@gibson.dropbear.id.au> 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. > > > > > > > > This does mean we no longer support guest kernels prior to 4.0, unless > > > > they have modern virtio support backported (which some distro kernels > > > > like that in RHEL7 do). > > > > > > > > Signed-off-by: David Gibson <david@gibson.dropbear.id.au> > > > > --- > > > > > > Reviewed-by: Greg Kurz <groug@kaod.org> > > > > > > FWIW, I could test the following: > > > - allows a RHEL7 guest with pre 4.0 kernel to boot, as mentioned > > > in the changelog > > > - breaks boot of older RHEL 6.10 guests as expected > > > - allows migration of older machine types to/from QEMU 4.2 > > > > > > Tested-by: Greg Kurz <groug@kaod.org> > > > > > > > Wait... I gave a try to virtiofsd and there's a problem: > > > > $ ./ppc64-softmmu/qemu-system-ppc64 -device vhost-user-fs-pci > > Unexpected error in object_property_find() at /home/greg/Work/qemu/qemu-ppc/qom/object.c:1231: > > qemu-system-ppc64: -device vhost-user-fs-pci: can't apply global virtio-pci.disable-legacy=on: Property '.disable-legacy' not found > > Aborted (core dumped) > > > > It is still not possible to set the disable-legacy prop on the > > vhost-user-fs-pci device, even without this patch, but QEMU > > doesn't abort: > > > > $ ./ppc64-softmmu/qemu-system-ppc64 -device vhost-user-fs-pci -global virtio-pci.disable-legacy=on > > qemu-system-ppc64: -device vhost-user-fs-pci: can't apply global virtio-pci.disable-legacy=on: Property '.disable-legacy' not found > > $ > > > > It seems to be related to the fact that vhost-user-fs-pci is a non-transitional > > only device, as shown with this workaround: > > > > --- a/hw/ppc/spapr.c > > +++ b/hw/ppc/spapr.c > > @@ -4574,7 +4574,7 @@ static void spapr_machine_latest_class_options(MachineClass *mc) > > * class since it doesn't have a compat_props. > > */ > > static GlobalProperty compat[] = { > > - { TYPE_VIRTIO_PCI, "disable-legacy", "on", }, > > + { TYPE_VIRTIO_PCI "-transitional", "disable-legacy", "on", }, > > }; > > > > mc->alias = "pseries"; > > Does this actually help? There's no type virtio-pci-transitional, is > there? > Oops you're right. There's no such type and it doesn't help in the end... I should have tried that with -global before posting -global virtio-pci-transitional.disable-legacy=on qemu-system-ppc64: warning: global virtio-pci-transitional.disable-legacy has invalid class name Maybe worth adding a warning as well when done from the machine code, but this is another story. Sorry for the noise. > > > > but there's probably a better way to address this. > > > > MST, Any suggestion ? > > Hmm I'm not sure how to fix this properly. The only idea that comes > to mind is a new internal-only "x-disable-legacy" that would be on virtio-pci, > duplicating functionality of disable-legacy but intended for > globals like this. > Maybe but Daniel P. Berrangé jumped in and has some arguments against what we're trying to achieve with this series... A suivre. > > > > > > hw/ppc/spapr.c | 17 +++++++++++++++++ > > > > 1 file changed, 17 insertions(+) > > > > > > > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > > > > index 2eb0d8f70d..3cfc98ac61 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" > > > > > > > > @@ -4566,8 +4567,20 @@ static const TypeInfo spapr_machine_info = { > > > > > > > > static void spapr_machine_latest_class_options(MachineClass *mc) > > > > { > > > > + /* > > > > + * 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. We can't do that from the base > > > > + * class since it doesn't have a compat_props. > > > > + */ > > > > + static GlobalProperty compat[] = { > > > > + { TYPE_VIRTIO_PCI, "disable-legacy", "on", }, > > > > + }; > > > > + > > > > mc->alias = "pseries"; > > > > mc->is_default = true; > > > > + > > > > + compat_props_add(mc->compat_props, compat, G_N_ELEMENTS(compat)); > > > > } > > > > > > > > #define DEFINE_SPAPR_MACHINE(suffix, verstr, latest) \ > > > > @@ -4607,6 +4620,9 @@ 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); > > > > @@ -4614,6 +4630,7 @@ static void spapr_machine_4_2_class_options(MachineClass *mc) > > > > smc->default_caps.caps[SPAPR_CAP_FWNMI_MCE] = SPAPR_CAP_OFF; > > > > smc->rma_limit = 16 * GiB; > > > > mc->nvdimm_supported = false; > > > > + compat_props_add(mc->compat_props, compat, G_N_ELEMENTS(compat)); > > > > } > > > > > > > > DEFINE_SPAPR_MACHINE(4_2, "4.2", false); > > > > ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v3 1/2] spapr: Disable legacy virtio devices for pseries-5.0 and later 2020-03-05 4:30 ` [PATCH v3 1/2] spapr: Disable legacy virtio devices for pseries-5.0 and later David Gibson 2020-03-05 10:31 ` Greg Kurz @ 2020-03-10 11:56 ` Daniel P. Berrangé 2020-03-11 0:58 ` David Gibson 1 sibling, 1 reply; 29+ messages in thread From: Daniel P. Berrangé @ 2020-03-10 11:56 UTC (permalink / raw) To: David Gibson Cc: pair, mst, aik, groug, qemu-devel, paulus, clg, mdroth, qemu-ppc On Thu, Mar 05, 2020 at 03:30:08PM +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. Two things to note here x86 defaults to the i440fx machine type, and so defaults to transitional mode. AFAIK, only RHEL-8 downstream changed x86 to defualt to q35 With q35 whether you get transitional mode or not is actually dependent on where you place the device. If it is placed into a PCIe root port, then it is modern-only. If it is placed into a PCI bridge, then it is transitional still. > Legacy mode virtio devices aren't really necessary any more, and are Legacy mode is required for RHEL-6 which has not reached EOL yet. > 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. The challenge I see with pseries, as compared to x86 is around how apps deal with mgmt / guest setup. With x86 there are distinct machine types i440fx / q35, so mgmt apps could decide what todo based on the chipset & device support. eg they can determine whether the guest supports PCIe at all, and they can determine whether the guest supports virtio-1.0. Thus they can decide between three options - Use i440fx - Use q35 with placement in PCI bridge - Use q35 with placement in PCIe root port These rules applies no matter what version of q35/i440fx is in use. With PPC, we're changing behaviour of the existing pseries machine type in a minor version. Management apps need to avoid creating logic that depends on a specific minor version because these version numbers are all changed by downstream distro vendors. IOW, as a comparison to x86, this change is akin to altering behaviour of the i440fx machine type so that it disables legacy mode despite still being PCI, and not PCIe. Is there any liklihood we'll ever introduce a true PCIe based machine type for PPC, so we get something much closer to x86/aarch64 machine types in terms of PCIe architecture ? > This does mean we no longer support guest kernels prior to 4.0, unless > they have modern virtio support backported (which some distro kernels > like that in RHEL7 do). > > Signed-off-by: David Gibson <david@gibson.dropbear.id.au> > --- > hw/ppc/spapr.c | 17 +++++++++++++++++ > 1 file changed, 17 insertions(+) > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > index 2eb0d8f70d..3cfc98ac61 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" > > @@ -4566,8 +4567,20 @@ static const TypeInfo spapr_machine_info = { > > static void spapr_machine_latest_class_options(MachineClass *mc) > { > + /* > + * 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. We can't do that from the base > + * class since it doesn't have a compat_props. > + */ > + static GlobalProperty compat[] = { > + { TYPE_VIRTIO_PCI, "disable-legacy", "on", }, > + }; > + > mc->alias = "pseries"; > mc->is_default = true; > + > + compat_props_add(mc->compat_props, compat, G_N_ELEMENTS(compat)); > } > > #define DEFINE_SPAPR_MACHINE(suffix, verstr, latest) \ > @@ -4607,6 +4620,9 @@ 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); > @@ -4614,6 +4630,7 @@ static void spapr_machine_4_2_class_options(MachineClass *mc) > smc->default_caps.caps[SPAPR_CAP_FWNMI_MCE] = SPAPR_CAP_OFF; > smc->rma_limit = 16 * GiB; > mc->nvdimm_supported = false; > + compat_props_add(mc->compat_props, compat, G_N_ELEMENTS(compat)); > } > > DEFINE_SPAPR_MACHINE(4_2, "4.2", false); > -- > 2.24.1 > > Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :| ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v3 1/2] spapr: Disable legacy virtio devices for pseries-5.0 and later 2020-03-10 11:56 ` Daniel P. Berrangé @ 2020-03-11 0:58 ` David Gibson 2020-03-11 7:11 ` Michael S. Tsirkin 0 siblings, 1 reply; 29+ messages in thread From: David Gibson @ 2020-03-11 0:58 UTC (permalink / raw) To: Daniel P. Berrangé Cc: pair, mst, aik, groug, qemu-devel, paulus, clg, mdroth, qemu-ppc [-- Attachment #1: Type: text/plain, Size: 7190 bytes --] On Tue, Mar 10, 2020 at 11:56:11AM +0000, Daniel P. Berrangé wrote: > On Thu, Mar 05, 2020 at 03:30:08PM +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. > > Two things to note here > > x86 defaults to the i440fx machine type, and so defaults > to transitional mode. AFAIK, only RHEL-8 downstream changed > x86 to defualt to q35 > > With q35 whether you get transitional mode or not is actually > dependent on where you place the device. If it is placed into > a PCIe root port, then it is modern-only. If it is placed into > a PCI bridge, then it is transitional still. Yes, I'm aware. > > Legacy mode virtio devices aren't really necessary any more, and are > > Legacy mode is required for RHEL-6 which has not reached EOL yet. Yeah.. I'm concerned about this, but I'm not sure what to do about it. > > 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. > > The challenge I see with pseries, as compared to x86 is around > how apps deal with mgmt / guest setup. With x86 there are > distinct machine types i440fx / q35, so mgmt apps could decide > what todo based on the chipset & device support. eg they can > determine whether the guest supports PCIe at all, and they > can determine whether the guest supports virtio-1.0. Thus > they can decide between three options > > - Use i440fx > - Use q35 with placement in PCI bridge > - Use q35 with placement in PCIe root port > > These rules applies no matter what version of q35/i440fx > is in use. Yeah.. x86 also has the advantage of enough visibility that it can reasonbly easily get mgmt layers to do stuff about it. This is much harder for POWER :/. > With PPC, we're changing behaviour of the existing pseries > machine type in a minor version. Management apps need to > avoid creating logic that depends on a specific minor version > because these version numbers are all changed by downstream > distro vendors. IOW, as a comparison to x86, this change is > akin to altering behaviour of the i440fx machine type so > that it disables legacy mode despite still being PCI, and > not PCIe. > > Is there any liklihood we'll ever introduce a true PCIe > based machine type for PPC, so we get something much > closer to x86/aarch64 machine types in terms of PCIe > architecture ? It doesn't really make sense to do so. For x86 - or more strictly for pc - the change from PCI to PCIe is a pretty fundamental system change with affects in lots of places, which makes a whole new versioned series of machine types a reasonable option. For pseries - that's not really the case. PCI under PAPR is paravirtualized, and it always has been. The interface we're matching is not real hardware, but the PAPR spec and to a lesser extent the existing PowerVM implementation of it. [Aside: you've made a subtle but common x86-centric assumption above that there's only one important platform design per ISA. There is a real PCIe based PPC machine type in "pnv" (and maybe some of the embedded ones as well), but that's not the environment we care about for guests in production, since we can't use KVM with it] What PAPR has is an odd hybrid - individual devices can be PCIe (we have calls to access extended config space) - but the overall bus structure is more-or-less like vanilla PCI. I think it would be possible to kind of expose a more PCIe like structure, but a) it would be weirdly artificial, b) it doesn't match the PAPR interfaces very well, c) it would make our behaviour different from PowerVM. It would certainly be possible to better handle PCIe devices on a root bus. That's been on my todo someday list for ages, but I've kept putting it off because the tangible benefits are pretty minimal. Note that several things that I believe are now in the PCIe spec, but really derive more from PC legacy considerations, don't apply at all for PAPR. e.g. there's no meaningful distinction between integrated and slotted devices, multiple independent host bridges is routine and doesn't require any (virtual) hardware visible domain numbers. > > This does mean we no longer support guest kernels prior to 4.0, unless > > they have modern virtio support backported (which some distro kernels > > like that in RHEL7 do). > > > > Signed-off-by: David Gibson <david@gibson.dropbear.id.au> > > --- > > hw/ppc/spapr.c | 17 +++++++++++++++++ > > 1 file changed, 17 insertions(+) > > > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > > index 2eb0d8f70d..3cfc98ac61 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" > > > > @@ -4566,8 +4567,20 @@ static const TypeInfo spapr_machine_info = { > > > > static void spapr_machine_latest_class_options(MachineClass *mc) > > { > > + /* > > + * 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. We can't do that from the base > > + * class since it doesn't have a compat_props. > > + */ > > + static GlobalProperty compat[] = { > > + { TYPE_VIRTIO_PCI, "disable-legacy", "on", }, > > + }; > > + > > mc->alias = "pseries"; > > mc->is_default = true; > > + > > + compat_props_add(mc->compat_props, compat, G_N_ELEMENTS(compat)); > > } > > > > #define DEFINE_SPAPR_MACHINE(suffix, verstr, latest) \ > > @@ -4607,6 +4620,9 @@ 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); > > @@ -4614,6 +4630,7 @@ static void spapr_machine_4_2_class_options(MachineClass *mc) > > smc->default_caps.caps[SPAPR_CAP_FWNMI_MCE] = SPAPR_CAP_OFF; > > smc->rma_limit = 16 * GiB; > > mc->nvdimm_supported = false; > > + compat_props_add(mc->compat_props, compat, G_N_ELEMENTS(compat)); > > } > > > > DEFINE_SPAPR_MACHINE(4_2, "4.2", false); > > Regards, > Daniel -- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v3 1/2] spapr: Disable legacy virtio devices for pseries-5.0 and later 2020-03-11 0:58 ` David Gibson @ 2020-03-11 7:11 ` Michael S. Tsirkin 2020-03-12 1:14 ` David Gibson 0 siblings, 1 reply; 29+ messages in thread From: Michael S. Tsirkin @ 2020-03-11 7:11 UTC (permalink / raw) To: David Gibson Cc: pair, Daniel P. Berrangé, aik, groug, qemu-devel, paulus, clg, mdroth, qemu-ppc On Wed, Mar 11, 2020 at 11:58:57AM +1100, David Gibson wrote: > Note that several things that I believe are now in the PCIe spec, but > really derive more from PC legacy considerations, don't apply at all > for PAPR. e.g. there's no meaningful distinction between integrated > and slotted devices, multiple independent host bridges is routine and > doesn't require any (virtual) hardware visible domain numbers. Domain numbers are a Linux thing, not a PCIe thing. On x86 they come from ACPI segment numbers. As such they aren't usually hardware visible on x86, they are supplied by firmware. -- MST ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v3 1/2] spapr: Disable legacy virtio devices for pseries-5.0 and later 2020-03-11 7:11 ` Michael S. Tsirkin @ 2020-03-12 1:14 ` David Gibson 2020-03-12 6:41 ` Michael S. Tsirkin 0 siblings, 1 reply; 29+ messages in thread From: David Gibson @ 2020-03-12 1:14 UTC (permalink / raw) To: Michael S. Tsirkin Cc: pair, Daniel P. Berrangé, aik, groug, qemu-devel, paulus, clg, mdroth, qemu-ppc [-- Attachment #1: Type: text/plain, Size: 1392 bytes --] On Wed, Mar 11, 2020 at 03:11:16AM -0400, Michael S. Tsirkin wrote: > On Wed, Mar 11, 2020 at 11:58:57AM +1100, David Gibson wrote: > > Note that several things that I believe are now in the PCIe spec, but > > really derive more from PC legacy considerations, don't apply at all > > for PAPR. e.g. there's no meaningful distinction between integrated > > and slotted devices, multiple independent host bridges is routine and > > doesn't require any (virtual) hardware visible domain numbers. > > Domain numbers are a Linux thing, not a PCIe thing. On x86 they come > from ACPI segment numbers. As such they aren't usually hardware > visible on x86, they are supplied by firmware. Oh, ok. I thought that at least on the standard IO 0xcf8 host bridge controller the domain number was written into certain registers to select the relevant root bus. On POWER the domain numbers are arbitrarily assigned within Linux. "Hardware" (well, the firmware/hypervisor) uses a different identifier, called the BUID (generally a large, 64-bit pseudo-address) in the device tree and hypercalls. [As an aside, this means the use of domain numbers in libvirt XML is complete bogosity] -- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v3 1/2] spapr: Disable legacy virtio devices for pseries-5.0 and later 2020-03-12 1:14 ` David Gibson @ 2020-03-12 6:41 ` Michael S. Tsirkin 0 siblings, 0 replies; 29+ messages in thread From: Michael S. Tsirkin @ 2020-03-12 6:41 UTC (permalink / raw) To: David Gibson Cc: pair, Daniel P. Berrangé, aik, groug, qemu-devel, paulus, clg, mdroth, qemu-ppc On Thu, Mar 12, 2020 at 12:14:20PM +1100, David Gibson wrote: > On Wed, Mar 11, 2020 at 03:11:16AM -0400, Michael S. Tsirkin wrote: > > On Wed, Mar 11, 2020 at 11:58:57AM +1100, David Gibson wrote: > > > Note that several things that I believe are now in the PCIe spec, but > > > really derive more from PC legacy considerations, don't apply at all > > > for PAPR. e.g. there's no meaningful distinction between integrated > > > and slotted devices, multiple independent host bridges is routine and > > > doesn't require any (virtual) hardware visible domain numbers. > > > > Domain numbers are a Linux thing, not a PCIe thing. On x86 they come > > from ACPI segment numbers. As such they aren't usually hardware > > visible on x86, they are supplied by firmware. > > Oh, ok. I thought that at least on the standard IO 0xcf8 host bridge > controller the domain number was written into certain registers to > select the relevant root bus. standard 0xcf8 can only access 256 bus numbers. software does not much care on which root bus these are though. > On POWER the domain numbers are arbitrarily assigned within Linux. > "Hardware" (well, the firmware/hypervisor) uses a different > identifier, called the BUID (generally a large, 64-bit pseudo-address) > in the device tree and hypercalls. > > [As an aside, this means the use of domain numbers in libvirt XML is > complete bogosity] For fun, they aren't actually used either. And of course using the word "domain" in a domain XML format means you can't search for it anywhere without getting a million unrelated hits. > -- > David Gibson | I'll have my music baroque, and my code > david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ > | _way_ _around_! > http://www.ozlabs.org/~dgibson ^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH v3 2/2] spapr: Enable virtio iommu_platform=on by default 2020-03-05 4:30 [PATCH v3 0/2] spapr: Use vIOMMU translation for virtio by default David Gibson 2020-03-05 4:30 ` [PATCH v3 1/2] spapr: Disable legacy virtio devices for pseries-5.0 and later David Gibson @ 2020-03-05 4:30 ` David Gibson 2020-03-05 11:59 ` Greg Kurz 2020-03-10 11:43 ` Upstream QEMU guest support policy ? Re: [PATCH v3 0/2] spapr: Use vIOMMU translation for virtio " Daniel P. Berrangé 2020-03-11 17:19 ` Greg Kurz 3 siblings, 1 reply; 29+ messages in thread From: David Gibson @ 2020-03-05 4:30 UTC (permalink / raw) To: groug, pair, qemu-devel, clg Cc: mst, aik, paulus, mdroth, qemu-ppc, David Gibson Traditionally, virtio devices don't do DMA by the usual path on the guest platform. In particular they usually bypass any virtual IOMMU the guest has, using hypervisor magic to access untranslated guest physical addresses. There's now the optional iommu_platform flag which can tell virtio devices to use the platform's normal DMA path, including any IOMMUs. That flag was motiviated for the case of hardware virtio implementations, but there are other reasons to want it. Specifically, the fact that the virtio device doesn't use vIOMMU translation means that virtio devices are unsafe to pass to nested guests, or to use with VFIO userspace drivers inside the guest. This is particularly noticeable on the pseries platform which *always* has a guest-visible vIOMMU. Not using the normal DMA path also causes difficulties for the guest side driver when using the upcoming POWER Secure VMs (a.k.a. PEF). While it's theoretically possible to handle this on the guest side, it's really fiddly. Given the other problems with the non-translated virtio device, let's just enable vIOMMU translation for virtio devices by default in the pseries-5.0 (and later) machine types. This does mean the new machine type will no longer support guest kernels older than 4.8, unless they have support for the virtio IOMMU_PLATFORM flag backported (which some distro kernels like RHEL7 do). Signed-off-by: David Gibson <david@gibson.dropbear.id.au> --- hw/ppc/spapr.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index 3cfc98ac61..5ef099536e 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -4575,6 +4575,7 @@ static void spapr_machine_latest_class_options(MachineClass *mc) */ static GlobalProperty compat[] = { { TYPE_VIRTIO_PCI, "disable-legacy", "on", }, + { TYPE_VIRTIO_DEVICE, "iommu_platform", "on", }, }; mc->alias = "pseries"; @@ -4622,6 +4623,7 @@ 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" }, + { TYPE_VIRTIO_DEVICE, "iommu_platform", "off", }, }; spapr_machine_5_0_class_options(mc); -- 2.24.1 ^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [PATCH v3 2/2] spapr: Enable virtio iommu_platform=on by default 2020-03-05 4:30 ` [PATCH v3 2/2] spapr: Enable virtio iommu_platform=on by default David Gibson @ 2020-03-05 11:59 ` Greg Kurz 2020-03-10 10:43 ` Greg Kurz 0 siblings, 1 reply; 29+ messages in thread From: Greg Kurz @ 2020-03-05 11:59 UTC (permalink / raw) To: David Gibson; +Cc: pair, mst, aik, qemu-devel, paulus, clg, mdroth, qemu-ppc On Thu, 5 Mar 2020 15:30:09 +1100 David Gibson <david@gibson.dropbear.id.au> wrote: > Traditionally, virtio devices don't do DMA by the usual path on the > guest platform. In particular they usually bypass any virtual IOMMU > the guest has, using hypervisor magic to access untranslated guest > physical addresses. > > There's now the optional iommu_platform flag which can tell virtio > devices to use the platform's normal DMA path, including any IOMMUs. > That flag was motiviated for the case of hardware virtio > implementations, but there are other reasons to want it. > > Specifically, the fact that the virtio device doesn't use vIOMMU > translation means that virtio devices are unsafe to pass to nested > guests, or to use with VFIO userspace drivers inside the guest. This > is particularly noticeable on the pseries platform which *always* has > a guest-visible vIOMMU. > > Not using the normal DMA path also causes difficulties for the guest > side driver when using the upcoming POWER Secure VMs (a.k.a. PEF). > While it's theoretically possible to handle this on the guest side, > it's really fiddly. Given the other problems with the non-translated > virtio device, let's just enable vIOMMU translation for virtio devices > by default in the pseries-5.0 (and later) machine types. > > This does mean the new machine type will no longer support guest > kernels older than 4.8, unless they have support for the virtio > IOMMU_PLATFORM flag backported (which some distro kernels like RHEL7 > do). > > Signed-off-by: David Gibson <david@gibson.dropbear.id.au> > --- The patch looks good but I'm not sure if we're quite ready to merge it yet. With this applied, I get zero output on a virtio-serial based console: ie. -chardev stdio,id=con0 -device virtio-serial -device virtconsole,chardev=con0 FYI, virtio-serial is a bit broken for spapr with iommu_platform=off already: (1) pressing a key in the console during SLOF or grub has no effect (2) the guest kernel boot stays stuck around quiesce These are regressions introduced by this SLOF update: a363e9ed8731f45674260932a340a0d81c4b0a6f is the first bad commit commit a363e9ed8731f45674260932a340a0d81c4b0a6f Author: Alexey Kardashevskiy <aik@ozlabs.ru> Date: Tue Dec 17 11:31:54 2019 +1100 pseries: Update SLOF firmware image A trivial fix was already posted on the SLOF list for (1) : https://patchwork.ozlabs.org/patch/1249338/ (2) is still under investigation but the console is _at least_ functional until the guest OS takes control. This is no longer the case with this patch. > hw/ppc/spapr.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > index 3cfc98ac61..5ef099536e 100644 > --- a/hw/ppc/spapr.c > +++ b/hw/ppc/spapr.c > @@ -4575,6 +4575,7 @@ static void spapr_machine_latest_class_options(MachineClass *mc) > */ > static GlobalProperty compat[] = { > { TYPE_VIRTIO_PCI, "disable-legacy", "on", }, > + { TYPE_VIRTIO_DEVICE, "iommu_platform", "on", }, > }; > > mc->alias = "pseries"; > @@ -4622,6 +4623,7 @@ 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" }, > + { TYPE_VIRTIO_DEVICE, "iommu_platform", "off", }, > }; > > spapr_machine_5_0_class_options(mc); ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v3 2/2] spapr: Enable virtio iommu_platform=on by default 2020-03-05 11:59 ` Greg Kurz @ 2020-03-10 10:43 ` Greg Kurz 2020-03-12 4:14 ` Alexey Kardashevskiy 0 siblings, 1 reply; 29+ messages in thread From: Greg Kurz @ 2020-03-10 10:43 UTC (permalink / raw) To: David Gibson; +Cc: pair, mst, aik, qemu-devel, paulus, clg, mdroth, qemu-ppc On Thu, 5 Mar 2020 12:59:03 +0100 Greg Kurz <groug@kaod.org> wrote: > On Thu, 5 Mar 2020 15:30:09 +1100 > David Gibson <david@gibson.dropbear.id.au> wrote: > > > Traditionally, virtio devices don't do DMA by the usual path on the > > guest platform. In particular they usually bypass any virtual IOMMU > > the guest has, using hypervisor magic to access untranslated guest > > physical addresses. > > > > There's now the optional iommu_platform flag which can tell virtio > > devices to use the platform's normal DMA path, including any IOMMUs. > > That flag was motiviated for the case of hardware virtio > > implementations, but there are other reasons to want it. > > > > Specifically, the fact that the virtio device doesn't use vIOMMU > > translation means that virtio devices are unsafe to pass to nested > > guests, or to use with VFIO userspace drivers inside the guest. This > > is particularly noticeable on the pseries platform which *always* has > > a guest-visible vIOMMU. > > > > Not using the normal DMA path also causes difficulties for the guest > > side driver when using the upcoming POWER Secure VMs (a.k.a. PEF). > > While it's theoretically possible to handle this on the guest side, > > it's really fiddly. Given the other problems with the non-translated > > virtio device, let's just enable vIOMMU translation for virtio devices > > by default in the pseries-5.0 (and later) machine types. > > > > This does mean the new machine type will no longer support guest > > kernels older than 4.8, unless they have support for the virtio > > IOMMU_PLATFORM flag backported (which some distro kernels like RHEL7 > > do). > > > > Signed-off-by: David Gibson <david@gibson.dropbear.id.au> > > --- > > The patch looks good but I'm not sure if we're quite ready to merge > it yet. With this applied, I get zero output on a virtio-serial based > console: > > ie. > -chardev stdio,id=con0 -device virtio-serial -device virtconsole,chardev=con0 > > FYI, virtio-serial is a bit broken for spapr with iommu_platform=off already: > > (1) pressing a key in the console during SLOF or grub has no effect > > (2) the guest kernel boot stays stuck around quiesce > > These are regressions introduced by this SLOF update: > > a363e9ed8731f45674260932a340a0d81c4b0a6f is the first bad commit > commit a363e9ed8731f45674260932a340a0d81c4b0a6f > Author: Alexey Kardashevskiy <aik@ozlabs.ru> > Date: Tue Dec 17 11:31:54 2019 +1100 > pseries: Update SLOF firmware image > > A trivial fix was already posted on the SLOF list for (1) : > > https://patchwork.ozlabs.org/patch/1249338/ > > (2) is still under investigation but the console is _at least_ > functional until the guest OS takes control. This is no longer > the case with this patch. > Some progress was made on the SLOF front: https://patchwork.ozlabs.org/project/slof/list/?series=163314 With these series applied to SLOF, I can now boot a fedora31 guest with a virtio-serial console and iommu_platform=on... but now I'm trying out other virtio devices supported by SLOF and I'm running into issues around virtio-pci.disable-legacy as mentioned in some other mail... It seems we may not be ready to merge this series yet. > > hw/ppc/spapr.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > > index 3cfc98ac61..5ef099536e 100644 > > --- a/hw/ppc/spapr.c > > +++ b/hw/ppc/spapr.c > > @@ -4575,6 +4575,7 @@ static void spapr_machine_latest_class_options(MachineClass *mc) > > */ > > static GlobalProperty compat[] = { > > { TYPE_VIRTIO_PCI, "disable-legacy", "on", }, > > + { TYPE_VIRTIO_DEVICE, "iommu_platform", "on", }, > > }; > > > > mc->alias = "pseries"; > > @@ -4622,6 +4623,7 @@ 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" }, > > + { TYPE_VIRTIO_DEVICE, "iommu_platform", "off", }, > > }; > > > > spapr_machine_5_0_class_options(mc); > ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v3 2/2] spapr: Enable virtio iommu_platform=on by default 2020-03-10 10:43 ` Greg Kurz @ 2020-03-12 4:14 ` Alexey Kardashevskiy 2020-03-12 8:02 ` Greg Kurz 0 siblings, 1 reply; 29+ messages in thread From: Alexey Kardashevskiy @ 2020-03-12 4:14 UTC (permalink / raw) To: Greg Kurz, David Gibson Cc: pair, mst, qemu-devel, paulus, clg, mdroth, qemu-ppc On 10/03/2020 21:43, Greg Kurz wrote: > On Thu, 5 Mar 2020 12:59:03 +0100 > Greg Kurz <groug@kaod.org> wrote: > >> On Thu, 5 Mar 2020 15:30:09 +1100 >> David Gibson <david@gibson.dropbear.id.au> wrote: >> >>> Traditionally, virtio devices don't do DMA by the usual path on the >>> guest platform. In particular they usually bypass any virtual IOMMU >>> the guest has, using hypervisor magic to access untranslated guest >>> physical addresses. >>> >>> There's now the optional iommu_platform flag which can tell virtio >>> devices to use the platform's normal DMA path, including any IOMMUs. >>> That flag was motiviated for the case of hardware virtio >>> implementations, but there are other reasons to want it. >>> >>> Specifically, the fact that the virtio device doesn't use vIOMMU >>> translation means that virtio devices are unsafe to pass to nested >>> guests, or to use with VFIO userspace drivers inside the guest. This >>> is particularly noticeable on the pseries platform which *always* has >>> a guest-visible vIOMMU. >>> >>> Not using the normal DMA path also causes difficulties for the guest >>> side driver when using the upcoming POWER Secure VMs (a.k.a. PEF). >>> While it's theoretically possible to handle this on the guest side, >>> it's really fiddly. Given the other problems with the non-translated >>> virtio device, let's just enable vIOMMU translation for virtio devices >>> by default in the pseries-5.0 (and later) machine types. >>> >>> This does mean the new machine type will no longer support guest >>> kernels older than 4.8, unless they have support for the virtio >>> IOMMU_PLATFORM flag backported (which some distro kernels like RHEL7 >>> do). >>> >>> Signed-off-by: David Gibson <david@gibson.dropbear.id.au> >>> --- >> >> The patch looks good but I'm not sure if we're quite ready to merge >> it yet. With this applied, I get zero output on a virtio-serial based >> console: >> >> ie. >> -chardev stdio,id=con0 -device virtio-serial -device virtconsole,chardev=con0 >> >> FYI, virtio-serial is a bit broken for spapr with iommu_platform=off already: >> >> (1) pressing a key in the console during SLOF or grub has no effect >> >> (2) the guest kernel boot stays stuck around quiesce >> >> These are regressions introduced by this SLOF update: >> >> a363e9ed8731f45674260932a340a0d81c4b0a6f is the first bad commit >> commit a363e9ed8731f45674260932a340a0d81c4b0a6f >> Author: Alexey Kardashevskiy <aik@ozlabs.ru> >> Date: Tue Dec 17 11:31:54 2019 +1100 >> pseries: Update SLOF firmware image >> >> A trivial fix was already posted on the SLOF list for (1) : >> >> https://patchwork.ozlabs.org/patch/1249338/ >> >> (2) is still under investigation but the console is _at least_ >> functional until the guest OS takes control. This is no longer >> the case with this patch. >> > > Some progress was made on the SLOF front: > > https://patchwork.ozlabs.org/project/slof/list/?series=163314 > > With these series applied to SLOF, I can now boot a fedora31 guest > with a virtio-serial console and iommu_platform=on... but now > I'm trying out other virtio devices supported by SLOF and I'm > running into issues around virtio-pci.disable-legacy as mentioned > in some other mail... > > It seems we may not be ready to merge this series yet. fwiw I sent a pull request: https://lore.kernel.org/qemu-devel/20200312041010.16229-1-aik@ozlabs.ru/T/#u > >>> hw/ppc/spapr.c | 2 ++ >>> 1 file changed, 2 insertions(+) >>> >>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c >>> index 3cfc98ac61..5ef099536e 100644 >>> --- a/hw/ppc/spapr.c >>> +++ b/hw/ppc/spapr.c >>> @@ -4575,6 +4575,7 @@ static void spapr_machine_latest_class_options(MachineClass *mc) >>> */ >>> static GlobalProperty compat[] = { >>> { TYPE_VIRTIO_PCI, "disable-legacy", "on", }, >>> + { TYPE_VIRTIO_DEVICE, "iommu_platform", "on", }, >>> }; >>> >>> mc->alias = "pseries"; >>> @@ -4622,6 +4623,7 @@ 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" }, >>> + { TYPE_VIRTIO_DEVICE, "iommu_platform", "off", }, >>> }; >>> >>> spapr_machine_5_0_class_options(mc); >> > -- Alexey ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v3 2/2] spapr: Enable virtio iommu_platform=on by default 2020-03-12 4:14 ` Alexey Kardashevskiy @ 2020-03-12 8:02 ` Greg Kurz 0 siblings, 0 replies; 29+ messages in thread From: Greg Kurz @ 2020-03-12 8:02 UTC (permalink / raw) To: Alexey Kardashevskiy Cc: pair, mst, qemu-devel, paulus, clg, mdroth, qemu-ppc, David Gibson On Thu, 12 Mar 2020 15:14:06 +1100 Alexey Kardashevskiy <aik@ozlabs.ru> wrote: > > > On 10/03/2020 21:43, Greg Kurz wrote: > > On Thu, 5 Mar 2020 12:59:03 +0100 > > Greg Kurz <groug@kaod.org> wrote: > > > >> On Thu, 5 Mar 2020 15:30:09 +1100 > >> David Gibson <david@gibson.dropbear.id.au> wrote: > >> > >>> Traditionally, virtio devices don't do DMA by the usual path on the > >>> guest platform. In particular they usually bypass any virtual IOMMU > >>> the guest has, using hypervisor magic to access untranslated guest > >>> physical addresses. > >>> > >>> There's now the optional iommu_platform flag which can tell virtio > >>> devices to use the platform's normal DMA path, including any IOMMUs. > >>> That flag was motiviated for the case of hardware virtio > >>> implementations, but there are other reasons to want it. > >>> > >>> Specifically, the fact that the virtio device doesn't use vIOMMU > >>> translation means that virtio devices are unsafe to pass to nested > >>> guests, or to use with VFIO userspace drivers inside the guest. This > >>> is particularly noticeable on the pseries platform which *always* has > >>> a guest-visible vIOMMU. > >>> > >>> Not using the normal DMA path also causes difficulties for the guest > >>> side driver when using the upcoming POWER Secure VMs (a.k.a. PEF). > >>> While it's theoretically possible to handle this on the guest side, > >>> it's really fiddly. Given the other problems with the non-translated > >>> virtio device, let's just enable vIOMMU translation for virtio devices > >>> by default in the pseries-5.0 (and later) machine types. > >>> > >>> This does mean the new machine type will no longer support guest > >>> kernels older than 4.8, unless they have support for the virtio > >>> IOMMU_PLATFORM flag backported (which some distro kernels like RHEL7 > >>> do). > >>> > >>> Signed-off-by: David Gibson <david@gibson.dropbear.id.au> > >>> --- > >> > >> The patch looks good but I'm not sure if we're quite ready to merge > >> it yet. With this applied, I get zero output on a virtio-serial based > >> console: > >> > >> ie. > >> -chardev stdio,id=con0 -device virtio-serial -device virtconsole,chardev=con0 > >> > >> FYI, virtio-serial is a bit broken for spapr with iommu_platform=off already: > >> > >> (1) pressing a key in the console during SLOF or grub has no effect > >> > >> (2) the guest kernel boot stays stuck around quiesce > >> > >> These are regressions introduced by this SLOF update: > >> > >> a363e9ed8731f45674260932a340a0d81c4b0a6f is the first bad commit > >> commit a363e9ed8731f45674260932a340a0d81c4b0a6f > >> Author: Alexey Kardashevskiy <aik@ozlabs.ru> > >> Date: Tue Dec 17 11:31:54 2019 +1100 > >> pseries: Update SLOF firmware image > >> > >> A trivial fix was already posted on the SLOF list for (1) : > >> > >> https://patchwork.ozlabs.org/patch/1249338/ > >> > >> (2) is still under investigation but the console is _at least_ > >> functional until the guest OS takes control. This is no longer > >> the case with this patch. > >> > > > > Some progress was made on the SLOF front: > > > > https://patchwork.ozlabs.org/project/slof/list/?series=163314 > > > > With these series applied to SLOF, I can now boot a fedora31 guest > > with a virtio-serial console and iommu_platform=on... but now > > I'm trying out other virtio devices supported by SLOF and I'm > > running into issues around virtio-pci.disable-legacy as mentioned > > in some other mail... > > > > It seems we may not be ready to merge this series yet. > > > fwiw I sent a pull request: > > https://lore.kernel.org/qemu-devel/20200312041010.16229-1-aik@ozlabs.ru/T/#u > Great ! Thanks mate ! :) > > > > > >>> hw/ppc/spapr.c | 2 ++ > >>> 1 file changed, 2 insertions(+) > >>> > >>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > >>> index 3cfc98ac61..5ef099536e 100644 > >>> --- a/hw/ppc/spapr.c > >>> +++ b/hw/ppc/spapr.c > >>> @@ -4575,6 +4575,7 @@ static void spapr_machine_latest_class_options(MachineClass *mc) > >>> */ > >>> static GlobalProperty compat[] = { > >>> { TYPE_VIRTIO_PCI, "disable-legacy", "on", }, > >>> + { TYPE_VIRTIO_DEVICE, "iommu_platform", "on", }, > >>> }; > >>> > >>> mc->alias = "pseries"; > >>> @@ -4622,6 +4623,7 @@ 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" }, > >>> + { TYPE_VIRTIO_DEVICE, "iommu_platform", "off", }, > >>> }; > >>> > >>> spapr_machine_5_0_class_options(mc); > >> > > > ^ permalink raw reply [flat|nested] 29+ messages in thread
* Upstream QEMU guest support policy ? Re: [PATCH v3 0/2] spapr: Use vIOMMU translation for virtio by default 2020-03-05 4:30 [PATCH v3 0/2] spapr: Use vIOMMU translation for virtio by default David Gibson 2020-03-05 4:30 ` [PATCH v3 1/2] spapr: Disable legacy virtio devices for pseries-5.0 and later David Gibson 2020-03-05 4:30 ` [PATCH v3 2/2] spapr: Enable virtio iommu_platform=on by default David Gibson @ 2020-03-10 11:43 ` Daniel P. Berrangé 2020-03-11 1:12 ` David Gibson 2020-03-11 17:19 ` Greg Kurz 3 siblings, 1 reply; 29+ messages in thread From: Daniel P. Berrangé @ 2020-03-10 11:43 UTC (permalink / raw) To: David Gibson Cc: pair, mst, aik, groug, qemu-devel, paulus, clg, mdroth, qemu-ppc On Thu, Mar 05, 2020 at 03:30:07PM +1100, David Gibson wrote: > Upcoming Secure VM support for pSeries machines introduces some > complications for virtio, since the transfer buffers need to be > explicitly shared so that the hypervisor can access them. > > While it's not strictly speaking dependent on it, the fact that virtio > devices bypass normal platform IOMMU translation complicates the issue > on the guest side. Since there are some significan downsides to > bypassing the vIOMMU anyway, let's just disable that. > > There's already a flag to do this in virtio, just turn it on by > default for forthcoming pseries machine types. Breaking existing guest OS to support a new secure VM feature that may not even be used/wanted doesn't seems like a sensible tradeoff for default out of the box behaviour. IOW, if Secure VM needs this, can we tie the change in virtio and IOMMU defaults to the machine type flag that enables the use of Secure VM. That way the changed virtio defaults only take effect if a user/mgmt app has explicitly opted in to the new Secure VM feature, and existing users won't be broken by a new feature they don't even use. > Any opinions on whether dropping support for the older guest kernels > is acceptable at this point? I think this question has different answers depending on whether you are considering downstream vendor policy, current upstream policy, and a possible new downstream policy on guest support. IOW a bit of a can of worms... In the case of RHEL downstream there is a very narrow matrix for what guest OS are considered supported. In the case of current upstream, there has essentially never been any documented guest matrix. The unwritten implicit rule upstream has followed is to not change defaults in a way that would break ability to run existing guest OS. As an example, on x86 upstream defaults to i440fx and thus still uses virtio devices in transitional mode by default, while downstream RHEL used its narrow support matrix as a justification for why it was ok to switch to q35 by default & loose guest support in many cases. Even that was problematic though, because RHEL still needed to support RHEL-6 guest which are broken by default with q35 since they only support legacy mode virtio. Thus we needed work in management apps to enable RHEL-6 to continue working with q35 chipset, by placing the devices onto a PCI bridge, instead of a PCIe root port, or by explicitly using i440fx instead. Thus if we follow our *current* upstream guest support policy, I don't think it is acceptable to break existing guests with the new machine type upstream. It is reasonable to do it downstream if the downstream is willing to sacrifice these guests, or invest to make any mgmt apps add workaround/revert QEMU changes. With that all said, I do *NOT* think current upstream practice is good or sustainable long term (though I admit I've argued on the other side in the past). This policy is why we're still using a machine designed in 1995 on x86 by default, in order that we avoid breaking the popular guest OS of the day, like Windows 95. This is similar to the problem we had with host build platforms, where we afraid to make any change which would break an existing build platform, or on the flipside made arbitrary ad-hoc changes with no consistent approach across different subsystems. I think that we should aim define some clearer policy around how we want to support guest OS in upstream. As we did with our host build platforms policy, any guest support policy should aim to move forward at a reasonable pace so that we are not locked at a specific point in time forever. I can imagine three tiers 1. Recommended. Expected to work well with machine type defaults 2. Supported. Should work with QEMU but may need special settings applied 3. Unsupported. Will not work with QEMU regardless of settings I don't have an opinion right now on what guest OS should fall in which category. One possible way to classify them would be to look at whether the vendor themselves still supported the OS. IOW, to be in the "Recommended" criteria, it must be actively supported by the vendor. Once EOL by the vendor it would be capped at the "Supported" tier. That definition wouldn't help your pseries issue though, because RHEL-6 is still considered a supported OS. Another possible way to classify guest OS would be to look purely at the original release date, and set a cap of "$TODAY - 5 years" for the "Recommended" tier. That would exclude RHEL-6. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :| ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: Upstream QEMU guest support policy ? Re: [PATCH v3 0/2] spapr: Use vIOMMU translation for virtio by default 2020-03-10 11:43 ` Upstream QEMU guest support policy ? Re: [PATCH v3 0/2] spapr: Use vIOMMU translation for virtio " Daniel P. Berrangé @ 2020-03-11 1:12 ` David Gibson 2020-03-11 7:33 ` Michael S. Tsirkin 2020-03-11 10:01 ` Daniel P. Berrangé 0 siblings, 2 replies; 29+ messages in thread From: David Gibson @ 2020-03-11 1:12 UTC (permalink / raw) To: Daniel P. Berrangé Cc: pair, mst, aik, groug, qemu-devel, paulus, clg, mdroth, qemu-ppc [-- Attachment #1: Type: text/plain, Size: 6941 bytes --] On Tue, Mar 10, 2020 at 11:43:43AM +0000, Daniel P. Berrangé wrote: > On Thu, Mar 05, 2020 at 03:30:07PM +1100, David Gibson wrote: > > Upcoming Secure VM support for pSeries machines introduces some > > complications for virtio, since the transfer buffers need to be > > explicitly shared so that the hypervisor can access them. > > > > While it's not strictly speaking dependent on it, the fact that virtio > > devices bypass normal platform IOMMU translation complicates the issue > > on the guest side. Since there are some significan downsides to > > bypassing the vIOMMU anyway, let's just disable that. > > > > There's already a flag to do this in virtio, just turn it on by > > default for forthcoming pseries machine types. > > Breaking existing guest OS to support a new secure VM feature that > may not even be used/wanted doesn't seems like a sensible tradeoff > for default out of the box behaviour. > > IOW, if Secure VM needs this, can we tie the change in virtio and > IOMMU defaults to the machine type flag that enables the use of > Secure VM. There is no such flag. In the POWER secure VM model, the secure mode option isn't something that's constructed in when the hypervisor builds the VM. Instead the VM is started normally and transitions itself to secure mode by talking directly with the ultravisor (it then uses TPM shenannigans to safely get the keys to its real storage backend(s)). > That way the changed virtio defaults only take effect if a user/mgmt > app has explicitly opted in to the new Secure VM feature, and existing > users won't be broken by a new feature they don't even use. Sure, but qemu has no natural way to know if secure VM is in use, until too late. I am wondering if we have to introduce an "svm=on" flag anyway. It's pretty ugly, since all it would be doing is changing defaults here and there for compatibilty with a possible future SVM transition, but maybe it's the best we can do :/. > > Any opinions on whether dropping support for the older guest kernels > > is acceptable at this point? > > > I think this question has different answers depending on whether you > are considering downstream vendor policy, current upstream policy, > and a possible new downstream policy on guest support. IOW a bit of a > can of worms... > > > In the case of RHEL downstream there is a very narrow matrix for > what guest OS are considered supported. > > In the case of current upstream, there has essentially never been > any documented guest matrix. The unwritten implicit rule upstream > has followed is to not change defaults in a way that would break > ability to run existing guest OS. Hrm, ok, that's not how I've been treating it for for pseries, though previous breakages have been for much older / rarer cases. We broke support for guests that don't call "ibm,client-architecture-support" long, long ago (but such guests are really, really ancient). We broke support (without workarounds) for guests with 4kiB standard page size more recently, but those are at least a decade old for common downstream distros (you can build such kernels today, but approximately nobody does). > As an example, on x86 upstream defaults to i440fx and thus still > uses virtio devices in transitional mode by default, while downstream > RHEL used its narrow support matrix as a justification for why it was > ok to switch to q35 by default & loose guest support in many cases. > Even that was problematic though, because RHEL still needed to support > RHEL-6 guest which are broken by default with q35 since they only > support legacy mode virtio. Thus we needed work in management apps > to enable RHEL-6 to continue working with q35 chipset, by placing > the devices onto a PCI bridge, instead of a PCIe root port, or by > explicitly using i440fx instead. Yeah, and here's where x86's visibility with mgmt because a big thing. Most of these changes are easily enough worked around with machine type options - and there's no inherent reason those are harder to work with than whole machine types, or other config details. But getting mgmt apps to support machine option based workarounds for us is a real challenge. > Thus if we follow our *current* upstream guest support policy, I don't > think it is acceptable to break existing guests with the new machine > type upstream. It is reasonable to do it downstream if the downstream > is willing to sacrifice these guests, or invest to make any mgmt apps > add workaround/revert QEMU changes. > > > With that all said, I do *NOT* think current upstream practice is good > or sustainable long term (though I admit I've argued on the other side > in the past). > > This policy is why we're still using a machine designed in 1995 on x86 > by default, in order that we avoid breaking the popular guest OS of the > day, like Windows 95. > > This is similar to the problem we had with host build platforms, where > we afraid to make any change which would break an existing build platform, > or on the flipside made arbitrary ad-hoc changes with no consistent > approach across different subsystems. > > > I think that we should aim define some clearer policy around how we > want to support guest OS in upstream. As we did with our host build > platforms policy, any guest support policy should aim to move forward > at a reasonable pace so that we are not locked at a specific point in > time forever. > > I can imagine three tiers > > 1. Recommended. Expected to work well with machine type defaults > 2. Supported. Should work with QEMU but may need special settings applied > 3. Unsupported. Will not work with QEMU regardless of settings > > I don't have an opinion right now on what guest OS should fall in which > category. One possible way to classify them would be to look at whether > the vendor themselves still supported the OS. IOW, to be in the > "Recommended" criteria, it must be actively supported by the vendor. > Once EOL by the vendor it would be capped at the "Supported" tier. > > That definition wouldn't help your pseries issue though, because RHEL-6 > is still considered a supported OS. > > Another possible way to classify guest OS would be to look purely at > the original release date, and set a cap of "$TODAY - 5 years" for > the "Recommended" tier. That would exclude RHEL-6. That seems fairly reasonable to me, but it doesn't really help me move forward right now. Right now, we have no sane way of distinguishing early enough between a RHEL-6 guest that needs legacy virtio and a guest that wants to go to secure mode, and can't have legacy virtio. :( -- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: Upstream QEMU guest support policy ? Re: [PATCH v3 0/2] spapr: Use vIOMMU translation for virtio by default 2020-03-11 1:12 ` David Gibson @ 2020-03-11 7:33 ` Michael S. Tsirkin 2020-03-12 1:10 ` David Gibson 2020-03-11 10:01 ` Daniel P. Berrangé 1 sibling, 1 reply; 29+ messages in thread From: Michael S. Tsirkin @ 2020-03-11 7:33 UTC (permalink / raw) To: David Gibson Cc: pair, Daniel P. Berrangé, aik, groug, qemu-devel, paulus, clg, mdroth, qemu-ppc On Wed, Mar 11, 2020 at 12:12:47PM +1100, David Gibson wrote: > I am wondering if we have to introduce an "svm=on" flag anyway. It's > pretty ugly, since all it would be doing is changing defaults here and > there for compatibilty with a possible future SVM transition, but > maybe it's the best we can do :/. Frankly I'm surprised there's no way for the hypervisor to block VM transition to secure mode. To me an inability to disable DRM looks like a security problem. Does not the ultravisor somehow allow enabling/disabling this functionality from the hypervisor? It would be even better if the hypervisor could block the guest from poking at the ultravisor completely but I guess that would be too much to hope for. ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: Upstream QEMU guest support policy ? Re: [PATCH v3 0/2] spapr: Use vIOMMU translation for virtio by default 2020-03-11 7:33 ` Michael S. Tsirkin @ 2020-03-12 1:10 ` David Gibson 2020-03-12 6:32 ` Michael S. Tsirkin 0 siblings, 1 reply; 29+ messages in thread From: David Gibson @ 2020-03-12 1:10 UTC (permalink / raw) To: Michael S. Tsirkin Cc: pair, Daniel P. Berrangé, aik, groug, qemu-devel, paulus, clg, mdroth, qemu-ppc [-- Attachment #1: Type: text/plain, Size: 1293 bytes --] On Wed, Mar 11, 2020 at 03:33:59AM -0400, Michael S. Tsirkin wrote: > On Wed, Mar 11, 2020 at 12:12:47PM +1100, David Gibson wrote: > > I am wondering if we have to introduce an "svm=on" flag anyway. It's > > pretty ugly, since all it would be doing is changing defaults here and > > there for compatibilty with a possible future SVM transition, but > > maybe it's the best we can do :/. > > Frankly I'm surprised there's no way for the hypervisor to block VM > transition to secure mode. To me an inability to disable DRM looks like > a security problem. Uh.. I don't immediately see how it's a security problem, though I'm certainly convinced it's a problem in other ways. > Does not the ultravisor somehow allow > enabling/disabling this functionality from the hypervisor? Not at present, but as mentioned on the other thread, Paul and I came up with a tentative plan to change that. > It would be > even better if the hypervisor could block the guest from poking at the > ultravisor completely but I guess that would be too much to hope for. Yeah, probably :/. -- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: Upstream QEMU guest support policy ? Re: [PATCH v3 0/2] spapr: Use vIOMMU translation for virtio by default 2020-03-12 1:10 ` David Gibson @ 2020-03-12 6:32 ` Michael S. Tsirkin 2020-03-16 3:06 ` David Gibson 0 siblings, 1 reply; 29+ messages in thread From: Michael S. Tsirkin @ 2020-03-12 6:32 UTC (permalink / raw) To: David Gibson Cc: pair, Daniel P. Berrangé, aik, groug, qemu-devel, paulus, clg, mdroth, qemu-ppc On Thu, Mar 12, 2020 at 12:10:49PM +1100, David Gibson wrote: > On Wed, Mar 11, 2020 at 03:33:59AM -0400, Michael S. Tsirkin wrote: > > On Wed, Mar 11, 2020 at 12:12:47PM +1100, David Gibson wrote: > > > I am wondering if we have to introduce an "svm=on" flag anyway. It's > > > pretty ugly, since all it would be doing is changing defaults here and > > > there for compatibilty with a possible future SVM transition, but > > > maybe it's the best we can do :/. > > > > Frankly I'm surprised there's no way for the hypervisor to block VM > > transition to secure mode. To me an inability to disable DRM looks like > > a security problem. > > Uh.. I don't immediately see how it's a security problem, though I'm > certainly convinced it's a problem in other ways. Well for one it breaks introspection, allowing guests to hide malicious code from hypervisors. > > Does not the ultravisor somehow allow > > enabling/disabling this functionality from the hypervisor? > > Not at present, but as mentioned on the other thread, Paul and I came > up with a tentative plan to change that. > > > It would be > > even better if the hypervisor could block the guest from poking at the > > ultravisor completely but I guess that would be too much to hope for. > > Yeah, probably :/. > > -- > David Gibson | I'll have my music baroque, and my code > david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ > | _way_ _around_! > http://www.ozlabs.org/~dgibson ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: Upstream QEMU guest support policy ? Re: [PATCH v3 0/2] spapr: Use vIOMMU translation for virtio by default 2020-03-12 6:32 ` Michael S. Tsirkin @ 2020-03-16 3:06 ` David Gibson 0 siblings, 0 replies; 29+ messages in thread From: David Gibson @ 2020-03-16 3:06 UTC (permalink / raw) To: Michael S. Tsirkin Cc: pair, Daniel P. Berrangé, aik, groug, qemu-devel, paulus, clg, mdroth, qemu-ppc [-- Attachment #1: Type: text/plain, Size: 1818 bytes --] On Thu, Mar 12, 2020 at 02:32:11AM -0400, Michael S. Tsirkin wrote: > On Thu, Mar 12, 2020 at 12:10:49PM +1100, David Gibson wrote: > > On Wed, Mar 11, 2020 at 03:33:59AM -0400, Michael S. Tsirkin wrote: > > > On Wed, Mar 11, 2020 at 12:12:47PM +1100, David Gibson wrote: > > > > I am wondering if we have to introduce an "svm=on" flag anyway. It's > > > > pretty ugly, since all it would be doing is changing defaults here and > > > > there for compatibilty with a possible future SVM transition, but > > > > maybe it's the best we can do :/. > > > > > > Frankly I'm surprised there's no way for the hypervisor to block VM > > > transition to secure mode. To me an inability to disable DRM looks like > > > a security problem. > > > > Uh.. I don't immediately see how it's a security problem, though I'm > > certainly convinced it's a problem in other ways. > > Well for one it breaks introspection, allowing guests to hide > malicious code from hypervisors. Hm, ok. Is that much used in practice? (Aside: I don't think I'd call that "introspection" since it's one thing examining another, not something examining itself). > > > > Does not the ultravisor somehow allow > > > enabling/disabling this functionality from the hypervisor? > > > > Not at present, but as mentioned on the other thread, Paul and I came > > up with a tentative plan to change that. > > > > > It would be > > > even better if the hypervisor could block the guest from poking at the > > > ultravisor completely but I guess that would be too much to hope for. > > > > Yeah, probably :/. > > > > -- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: Upstream QEMU guest support policy ? Re: [PATCH v3 0/2] spapr: Use vIOMMU translation for virtio by default 2020-03-11 1:12 ` David Gibson 2020-03-11 7:33 ` Michael S. Tsirkin @ 2020-03-11 10:01 ` Daniel P. Berrangé 2020-03-11 11:48 ` Michael S. Tsirkin 2020-03-12 1:08 ` David Gibson 1 sibling, 2 replies; 29+ messages in thread From: Daniel P. Berrangé @ 2020-03-11 10:01 UTC (permalink / raw) To: David Gibson Cc: pair, mst, aik, qemu-devel, groug, paulus, clg, mdroth, qemu-ppc On Wed, Mar 11, 2020 at 12:12:47PM +1100, David Gibson wrote: > On Tue, Mar 10, 2020 at 11:43:43AM +0000, Daniel P. Berrangé wrote: > > On Thu, Mar 05, 2020 at 03:30:07PM +1100, David Gibson wrote: > > > Upcoming Secure VM support for pSeries machines introduces some > > > complications for virtio, since the transfer buffers need to be > > > explicitly shared so that the hypervisor can access them. > > > > > > While it's not strictly speaking dependent on it, the fact that virtio > > > devices bypass normal platform IOMMU translation complicates the issue > > > on the guest side. Since there are some significan downsides to > > > bypassing the vIOMMU anyway, let's just disable that. > > > > > > There's already a flag to do this in virtio, just turn it on by > > > default for forthcoming pseries machine types. > > > > Breaking existing guest OS to support a new secure VM feature that > > may not even be used/wanted doesn't seems like a sensible tradeoff > > for default out of the box behaviour. > > > > IOW, if Secure VM needs this, can we tie the change in virtio and > > IOMMU defaults to the machine type flag that enables the use of > > Secure VM. > > There is no such flag. > > In the POWER secure VM model, the secure mode option isn't something > that's constructed in when the hypervisor builds the VM. Instead the > VM is started normally and transitions itself to secure mode by > talking directly with the ultravisor (it then uses TPM shenannigans to > safely get the keys to its real storage backend(s)). This is pretty suprising to me. The ability to use secure VM mode surely depends on host hardware features. We would need to be able to block the use of this, in order to allow VMs to be live migrated to hosts which lack the feature. Automatically & silently enabling a feature that has a hardware dependancy is something we aim to avoid, unless the user has opted in via some flag (such as -cpu host, or a -cpu $NAME, that implies the feature). > > > Any opinions on whether dropping support for the older guest kernels > > > is acceptable at this point? > > > > > > I think this question has different answers depending on whether you > > are considering downstream vendor policy, current upstream policy, > > and a possible new downstream policy on guest support. IOW a bit of a > > can of worms... > > > > > > In the case of RHEL downstream there is a very narrow matrix for > > what guest OS are considered supported. > > > > In the case of current upstream, there has essentially never been > > any documented guest matrix. The unwritten implicit rule upstream > > has followed is to not change defaults in a way that would break > > ability to run existing guest OS. > > Hrm, ok, that's not how I've been treating it for for pseries, though > previous breakages have been for much older / rarer cases. We broke > support for guests that don't call "ibm,client-architecture-support" > long, long ago (but such guests are really, really ancient). We broke > support (without workarounds) for guests with 4kiB standard page size > more recently, but those are at least a decade old for common > downstream distros (you can build such kernels today, but > approximately nobody does). > > > As an example, on x86 upstream defaults to i440fx and thus still > > uses virtio devices in transitional mode by default, while downstream > > RHEL used its narrow support matrix as a justification for why it was > > ok to switch to q35 by default & loose guest support in many cases. > > Even that was problematic though, because RHEL still needed to support > > RHEL-6 guest which are broken by default with q35 since they only > > support legacy mode virtio. Thus we needed work in management apps > > to enable RHEL-6 to continue working with q35 chipset, by placing > > the devices onto a PCI bridge, instead of a PCIe root port, or by > > explicitly using i440fx instead. > > Yeah, and here's where x86's visibility with mgmt because a big > thing. Most of these changes are easily enough worked around with > machine type options - and there's no inherent reason those are harder > to work with than whole machine types, or other config details. But > getting mgmt apps to support machine option based workarounds for us > is a real challenge. > > > Thus if we follow our *current* upstream guest support policy, I don't > > think it is acceptable to break existing guests with the new machine > > type upstream. It is reasonable to do it downstream if the downstream > > is willing to sacrifice these guests, or invest to make any mgmt apps > > add workaround/revert QEMU changes. > > > > > > With that all said, I do *NOT* think current upstream practice is good > > or sustainable long term (though I admit I've argued on the other side > > in the past). > > > > This policy is why we're still using a machine designed in 1995 on x86 > > by default, in order that we avoid breaking the popular guest OS of the > > day, like Windows 95. > > > > This is similar to the problem we had with host build platforms, where > > we afraid to make any change which would break an existing build platform, > > or on the flipside made arbitrary ad-hoc changes with no consistent > > approach across different subsystems. > > > > > > I think that we should aim define some clearer policy around how we > > want to support guest OS in upstream. As we did with our host build > > platforms policy, any guest support policy should aim to move forward > > at a reasonable pace so that we are not locked at a specific point in > > time forever. > > > > I can imagine three tiers > > > > 1. Recommended. Expected to work well with machine type defaults > > 2. Supported. Should work with QEMU but may need special settings applied > > 3. Unsupported. Will not work with QEMU regardless of settings > > > > I don't have an opinion right now on what guest OS should fall in which > > category. One possible way to classify them would be to look at whether > > the vendor themselves still supported the OS. IOW, to be in the > > "Recommended" criteria, it must be actively supported by the vendor. > > Once EOL by the vendor it would be capped at the "Supported" tier. > > > > That definition wouldn't help your pseries issue though, because RHEL-6 > > is still considered a supported OS. > > > > Another possible way to classify guest OS would be to look purely at > > the original release date, and set a cap of "$TODAY - 5 years" for > > the "Recommended" tier. That would exclude RHEL-6. > > That seems fairly reasonable to me, but it doesn't really help me move > forward right now. Right now, we have no sane way of distinguishing > early enough between a RHEL-6 guest that needs legacy virtio and a > guest that wants to go to secure mode, and can't have legacy virtio. :( Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :| ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: Upstream QEMU guest support policy ? Re: [PATCH v3 0/2] spapr: Use vIOMMU translation for virtio by default 2020-03-11 10:01 ` Daniel P. Berrangé @ 2020-03-11 11:48 ` Michael S. Tsirkin 2020-03-12 1:09 ` David Gibson 2020-03-12 1:08 ` David Gibson 1 sibling, 1 reply; 29+ messages in thread From: Michael S. Tsirkin @ 2020-03-11 11:48 UTC (permalink / raw) To: Daniel P. Berrangé Cc: pair, aik, qemu-devel, groug, paulus, clg, mdroth, qemu-ppc, David Gibson On Wed, Mar 11, 2020 at 10:01:27AM +0000, Daniel P. Berrangé wrote: > On Wed, Mar 11, 2020 at 12:12:47PM +1100, David Gibson wrote: > > On Tue, Mar 10, 2020 at 11:43:43AM +0000, Daniel P. Berrangé wrote: > > > On Thu, Mar 05, 2020 at 03:30:07PM +1100, David Gibson wrote: > > > > Upcoming Secure VM support for pSeries machines introduces some > > > > complications for virtio, since the transfer buffers need to be > > > > explicitly shared so that the hypervisor can access them. > > > > > > > > While it's not strictly speaking dependent on it, the fact that virtio > > > > devices bypass normal platform IOMMU translation complicates the issue > > > > on the guest side. Since there are some significan downsides to > > > > bypassing the vIOMMU anyway, let's just disable that. > > > > > > > > There's already a flag to do this in virtio, just turn it on by > > > > default for forthcoming pseries machine types. > > > > > > Breaking existing guest OS to support a new secure VM feature that > > > may not even be used/wanted doesn't seems like a sensible tradeoff > > > for default out of the box behaviour. > > > > > > IOW, if Secure VM needs this, can we tie the change in virtio and > > > IOMMU defaults to the machine type flag that enables the use of > > > Secure VM. > > > > There is no such flag. > > > > In the POWER secure VM model, the secure mode option isn't something > > that's constructed in when the hypervisor builds the VM. Instead the > > VM is started normally and transitions itself to secure mode by > > talking directly with the ultravisor (it then uses TPM shenannigans to > > safely get the keys to its real storage backend(s)). > > This is pretty suprising to me. The ability to use secure VM mode surely > depends on host hardware features. We would need to be able to block the > use of this, in order to allow VMs to be live migrated to hosts which > lack the feature. Automatically & silently enabling a feature that > has a hardware dependancy is something we aim to avoid, unless the user > has opted in via some flag (such as -cpu host, or a -cpu $NAME, that > implies the feature). That's something I don't know. Is migration supported in this mode? -- MST ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: Upstream QEMU guest support policy ? Re: [PATCH v3 0/2] spapr: Use vIOMMU translation for virtio by default 2020-03-11 11:48 ` Michael S. Tsirkin @ 2020-03-12 1:09 ` David Gibson 0 siblings, 0 replies; 29+ messages in thread From: David Gibson @ 2020-03-12 1:09 UTC (permalink / raw) To: Michael S. Tsirkin Cc: pair, Daniel P. Berrangé, aik, groug, qemu-devel, paulus, clg, mdroth, qemu-ppc [-- Attachment #1: Type: text/plain, Size: 2644 bytes --] On Wed, Mar 11, 2020 at 07:48:26AM -0400, Michael S. Tsirkin wrote: > On Wed, Mar 11, 2020 at 10:01:27AM +0000, Daniel P. Berrangé wrote: > > On Wed, Mar 11, 2020 at 12:12:47PM +1100, David Gibson wrote: > > > On Tue, Mar 10, 2020 at 11:43:43AM +0000, Daniel P. Berrangé wrote: > > > > On Thu, Mar 05, 2020 at 03:30:07PM +1100, David Gibson wrote: > > > > > Upcoming Secure VM support for pSeries machines introduces some > > > > > complications for virtio, since the transfer buffers need to be > > > > > explicitly shared so that the hypervisor can access them. > > > > > > > > > > While it's not strictly speaking dependent on it, the fact that virtio > > > > > devices bypass normal platform IOMMU translation complicates the issue > > > > > on the guest side. Since there are some significan downsides to > > > > > bypassing the vIOMMU anyway, let's just disable that. > > > > > > > > > > There's already a flag to do this in virtio, just turn it on by > > > > > default for forthcoming pseries machine types. > > > > > > > > Breaking existing guest OS to support a new secure VM feature that > > > > may not even be used/wanted doesn't seems like a sensible tradeoff > > > > for default out of the box behaviour. > > > > > > > > IOW, if Secure VM needs this, can we tie the change in virtio and > > > > IOMMU defaults to the machine type flag that enables the use of > > > > Secure VM. > > > > > > There is no such flag. > > > > > > In the POWER secure VM model, the secure mode option isn't something > > > that's constructed in when the hypervisor builds the VM. Instead the > > > VM is started normally and transitions itself to secure mode by > > > talking directly with the ultravisor (it then uses TPM shenannigans to > > > safely get the keys to its real storage backend(s)). > > > > This is pretty suprising to me. The ability to use secure VM mode surely > > depends on host hardware features. We would need to be able to block the > > use of this, in order to allow VMs to be live migrated to hosts which > > lack the feature. Automatically & silently enabling a feature that > > has a hardware dependancy is something we aim to avoid, unless the user > > has opted in via some flag (such as -cpu host, or a -cpu $NAME, that > > implies the feature). > > That's something I don't know. Is migration supported in this mode? Not at this stage, though there's plans for it later. -- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: Upstream QEMU guest support policy ? Re: [PATCH v3 0/2] spapr: Use vIOMMU translation for virtio by default 2020-03-11 10:01 ` Daniel P. Berrangé 2020-03-11 11:48 ` Michael S. Tsirkin @ 2020-03-12 1:08 ` David Gibson 2020-03-12 9:47 ` Daniel P. Berrangé 1 sibling, 1 reply; 29+ messages in thread From: David Gibson @ 2020-03-12 1:08 UTC (permalink / raw) To: Daniel P. Berrangé Cc: pair, mst, aik, qemu-devel, groug, paulus, clg, mdroth, qemu-ppc [-- Attachment #1: Type: text/plain, Size: 3094 bytes --] On Wed, Mar 11, 2020 at 10:01:27AM +0000, Daniel P. Berrangé wrote: 65;5803;1c> On Wed, Mar 11, 2020 at 12:12:47PM +1100, David Gibson wrote: > > On Tue, Mar 10, 2020 at 11:43:43AM +0000, Daniel P. Berrangé wrote: > > > On Thu, Mar 05, 2020 at 03:30:07PM +1100, David Gibson wrote: > > > > Upcoming Secure VM support for pSeries machines introduces some > > > > complications for virtio, since the transfer buffers need to be > > > > explicitly shared so that the hypervisor can access them. > > > > > > > > While it's not strictly speaking dependent on it, the fact that virtio > > > > devices bypass normal platform IOMMU translation complicates the issue > > > > on the guest side. Since there are some significan downsides to > > > > bypassing the vIOMMU anyway, let's just disable that. > > > > > > > > There's already a flag to do this in virtio, just turn it on by > > > > default for forthcoming pseries machine types. > > > > > > Breaking existing guest OS to support a new secure VM feature that > > > may not even be used/wanted doesn't seems like a sensible tradeoff > > > for default out of the box behaviour. > > > > > > IOW, if Secure VM needs this, can we tie the change in virtio and > > > IOMMU defaults to the machine type flag that enables the use of > > > Secure VM. > > > > There is no such flag. > > > > In the POWER secure VM model, the secure mode option isn't something > > that's constructed in when the hypervisor builds the VM. Instead the > > VM is started normally and transitions itself to secure mode by > > talking directly with the ultravisor (it then uses TPM shenannigans to > > safely get the keys to its real storage backend(s)). > > This is pretty suprising to me. The ability to use secure VM mode surely > depends on host hardware features. We would need to be able to block the > use of this, in order to allow VMs to be live migrated to hosts which > lack the feature. Automatically & silently enabling a feature that > has a hardware dependancy is something we aim to avoid, unless the user > has opted in via some flag (such as -cpu host, or a -cpu $NAME, that > implies the feature). That is an excellent point, which I had not previously considered. I have confirmed that there is indeed not, at present, a way to disable the secure transition. But, it looks like it's not too late to fix it. I've discussed with Paul Mackerras, and early in the secure transition apparently the UV makes a call to the HV, which is allowed to fail. So, we're looking at adding another KVM capability for secure mode. It will default to disabled, and until it is explicitly enabled, KVM will always fail that call from the UV, effectively preventing guests from going into secure mode. We can then wire that up to a new spapr cap in qemu, which we can also use to configure these virtio defaults. -- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: Upstream QEMU guest support policy ? Re: [PATCH v3 0/2] spapr: Use vIOMMU translation for virtio by default 2020-03-12 1:08 ` David Gibson @ 2020-03-12 9:47 ` Daniel P. Berrangé 0 siblings, 0 replies; 29+ messages in thread From: Daniel P. Berrangé @ 2020-03-12 9:47 UTC (permalink / raw) To: David Gibson Cc: pair, mst, aik, qemu-devel, groug, paulus, clg, mdroth, qemu-ppc On Thu, Mar 12, 2020 at 12:08:47PM +1100, David Gibson wrote: > On Wed, Mar 11, 2020 at 10:01:27AM +0000, Daniel P. Berrangé wrote: > 65;5803;1c> On Wed, Mar 11, 2020 at 12:12:47PM +1100, David Gibson wrote: > > > On Tue, Mar 10, 2020 at 11:43:43AM +0000, Daniel P. Berrangé wrote: > > > > On Thu, Mar 05, 2020 at 03:30:07PM +1100, David Gibson wrote: > > > > > Upcoming Secure VM support for pSeries machines introduces some > > > > > complications for virtio, since the transfer buffers need to be > > > > > explicitly shared so that the hypervisor can access them. > > > > > > > > > > While it's not strictly speaking dependent on it, the fact that virtio > > > > > devices bypass normal platform IOMMU translation complicates the issue > > > > > on the guest side. Since there are some significan downsides to > > > > > bypassing the vIOMMU anyway, let's just disable that. > > > > > > > > > > There's already a flag to do this in virtio, just turn it on by > > > > > default for forthcoming pseries machine types. > > > > > > > > Breaking existing guest OS to support a new secure VM feature that > > > > may not even be used/wanted doesn't seems like a sensible tradeoff > > > > for default out of the box behaviour. > > > > > > > > IOW, if Secure VM needs this, can we tie the change in virtio and > > > > IOMMU defaults to the machine type flag that enables the use of > > > > Secure VM. > > > > > > There is no such flag. > > > > > > In the POWER secure VM model, the secure mode option isn't something > > > that's constructed in when the hypervisor builds the VM. Instead the > > > VM is started normally and transitions itself to secure mode by > > > talking directly with the ultravisor (it then uses TPM shenannigans to > > > safely get the keys to its real storage backend(s)). > > > > This is pretty suprising to me. The ability to use secure VM mode surely > > depends on host hardware features. We would need to be able to block the > > use of this, in order to allow VMs to be live migrated to hosts which > > lack the feature. Automatically & silently enabling a feature that > > has a hardware dependancy is something we aim to avoid, unless the user > > has opted in via some flag (such as -cpu host, or a -cpu $NAME, that > > implies the feature). > > That is an excellent point, which I had not previously considered. > > I have confirmed that there is indeed not, at present, a way to > disable the secure transition. But, it looks like it's not too late > to fix it. > > I've discussed with Paul Mackerras, and early in the secure transition > apparently the UV makes a call to the HV, which is allowed to fail. > > So, we're looking at adding another KVM capability for secure mode. > It will default to disabled, and until it is explicitly enabled, KVM > will always fail that call from the UV, effectively preventing guests > from going into secure mode. > > We can then wire that up to a new spapr cap in qemu, which we can also > use to configure these virtio defaults. Great, that sounds viable to me. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :| ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v3 0/2] spapr: Use vIOMMU translation for virtio by default 2020-03-05 4:30 [PATCH v3 0/2] spapr: Use vIOMMU translation for virtio by default David Gibson ` (2 preceding siblings ...) 2020-03-10 11:43 ` Upstream QEMU guest support policy ? Re: [PATCH v3 0/2] spapr: Use vIOMMU translation for virtio " Daniel P. Berrangé @ 2020-03-11 17:19 ` Greg Kurz 3 siblings, 0 replies; 29+ messages in thread From: Greg Kurz @ 2020-03-11 17:19 UTC (permalink / raw) To: David Gibson Cc: pair, mst, aik, Jason Wang, qemu-devel, paulus, clg, mdroth, qemu-ppc On Thu, 5 Mar 2020 15:30:07 +1100 David Gibson <david@gibson.dropbear.id.au> wrote: > Upcoming Secure VM support for pSeries machines introduces some > complications for virtio, since the transfer buffers need to be > explicitly shared so that the hypervisor can access them. > > While it's not strictly speaking dependent on it, the fact that virtio > devices bypass normal platform IOMMU translation complicates the issue > on the guest side. Since there are some significan downsides to > bypassing the vIOMMU anyway, let's just disable that. > > There's already a flag to do this in virtio, just turn it on by > default for forthcoming pseries machine types. > > Any opinions on whether dropping support for the older guest kernels > is acceptable at this point? > > Changes since v2: > * Rebase and improve some comments > Changes since v1: > * Added information on which guest kernel versions will no longer > work with these changes > * Use Michael Tsirkin's suggested better way of handling the machine > type change > > David Gibson (2): > spapr: Disable legacy virtio devices for pseries-5.0 and later This disables legacy AND transitional devices. IIUC this first patch is needed because only non-transitional (pure virtio-1) devices support iommu_platform=on, ie. this check in virtio_pci_device_plugged(): if (legacy) { if (virtio_host_has_feature(vdev, VIRTIO_F_IOMMU_PLATFORM)) { error_setg(errp, "VIRTIO_F_IOMMU_PLATFORM was supported by" " neither legacy nor transitional device"); return ; } It certainly looks right for legacy devices but what about transitional ones ? I couldn't find any indication in the spec or in the QEMU archives that explains why IOMMU should only be used with non-transitional devices... Jason or Michael, can you explain ? > spapr: Enable virtio iommu_platform=on by default > > hw/ppc/spapr.c | 19 +++++++++++++++++++ > 1 file changed, 19 insertions(+) > ^ permalink raw reply [flat|nested] 29+ messages in thread
end of thread, other threads:[~2020-03-16 3:18 UTC | newest] Thread overview: 29+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-03-05 4:30 [PATCH v3 0/2] spapr: Use vIOMMU translation for virtio by default David Gibson 2020-03-05 4:30 ` [PATCH v3 1/2] spapr: Disable legacy virtio devices for pseries-5.0 and later David Gibson 2020-03-05 10:31 ` Greg Kurz 2020-03-10 9:43 ` Greg Kurz 2020-03-10 9:57 ` Michael S. Tsirkin 2020-03-10 11:03 ` Michael S. Tsirkin 2020-03-10 12:24 ` Greg Kurz 2020-03-10 11:56 ` Daniel P. Berrangé 2020-03-11 0:58 ` David Gibson 2020-03-11 7:11 ` Michael S. Tsirkin 2020-03-12 1:14 ` David Gibson 2020-03-12 6:41 ` Michael S. Tsirkin 2020-03-05 4:30 ` [PATCH v3 2/2] spapr: Enable virtio iommu_platform=on by default David Gibson 2020-03-05 11:59 ` Greg Kurz 2020-03-10 10:43 ` Greg Kurz 2020-03-12 4:14 ` Alexey Kardashevskiy 2020-03-12 8:02 ` Greg Kurz 2020-03-10 11:43 ` Upstream QEMU guest support policy ? Re: [PATCH v3 0/2] spapr: Use vIOMMU translation for virtio " Daniel P. Berrangé 2020-03-11 1:12 ` David Gibson 2020-03-11 7:33 ` Michael S. Tsirkin 2020-03-12 1:10 ` David Gibson 2020-03-12 6:32 ` Michael S. Tsirkin 2020-03-16 3:06 ` David Gibson 2020-03-11 10:01 ` Daniel P. Berrangé 2020-03-11 11:48 ` Michael S. Tsirkin 2020-03-12 1:09 ` David Gibson 2020-03-12 1:08 ` David Gibson 2020-03-12 9:47 ` Daniel P. Berrangé 2020-03-11 17:19 ` Greg Kurz
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.