| Message ID | 20200305043009.611636-3-david@gibson.dropbear.id.au (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | spapr: Use vIOMMU translation for virtio by default | expand |
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);
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); >
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); >> >
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); > >> > > >
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);
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(+)