On Thu, 12 Mar 2020 15:14:06 +1100 Alexey Kardashevskiy <a...@ozlabs.ru> wrote:
> > > On 10/03/2020 21:43, Greg Kurz wrote: > > On Thu, 5 Mar 2020 12:59:03 +0100 > > Greg Kurz <gr...@kaod.org> wrote: > > > >> On Thu, 5 Mar 2020 15:30:09 +1100 > >> David Gibson <da...@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 <da...@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 <a...@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-...@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); > >> > > >
