Re: [pve-devel] [PATCH docs v4 4/5] added vIOMMU documentation

[Wolfgang Bumiller <w.bumiller@proxmox.com>]

On Fri, Jan 13, 2023 at 02:31:36PM +0100, Markus Frank wrote:
> 
> 
> On 1/13/23 11:09, Wolfgang Bumiller wrote:
> > On Fri, Nov 25, 2022 at 03:08:56PM +0100, Markus Frank wrote:
> > > Signed-off-by: Markus Frank <m.frank@proxmox.com>
> > > ---
> > >   qm-pci-passthrough.adoc | 25 +++++++++++++++++++++++++
> > >   1 file changed, 25 insertions(+)
> > > 
> > > diff --git a/qm-pci-passthrough.adoc b/qm-pci-passthrough.adoc
> > > index fa6ba35..7ed4d49 100644
> > > --- a/qm-pci-passthrough.adoc
> > > +++ b/qm-pci-passthrough.adoc
> > > @@ -389,6 +389,31 @@ Example configuration with an `Intel GVT-g vGPU` (`Intel Skylake 6700k`):
> > >   With this set, {pve} automatically creates such a device on VM start, and
> > >   cleans it up again when the VM stops.
> > > +[[qm_pci_viommu]]
> > > +vIOMMU
> > > +~~~~~~
> > > +
> > > +vIOMMU enables the option to passthrough pci devices to Level-2 VMs
> > > +in Level-1 VMs via Nested Virtualisation.
> > > +
> > > +Host-Requirement: Set `intel_iommu=on` or `amd_iommu=on` depending on your
> > > +CPU.
> > 
> > And by "CPU" you mean kernel command line? ;-)
> 
> Host-Requirement: Add `intel_iommu=on` or `amd_iommu=on`
> depending on your CPU to your kernel command line.
> 
> like this?
> > 
> > > +
> > > +VM-Requirement: For both Intel and AMD CPUs you will have to set
> > > +`intel_iommu=on` as a Linux boot parameter in the vIOMMU-enabled-VM, because
> > > +Qemu implements the Intel variant.
> > 
> > ^ As mentioned, there does appear to be an amd_iommu device in the qemu
> > code, so would the amd variant work?
> > 
> > In my reply to the code patch I mentioned checking the host arch. But if
> > you say we can use intel_iommu on AMD as well, I'd say, if both work,
> > give the user a choice, otherwise we can of course just stick to the one
> > that works ;-)
> 
> intel_iommu works better on my AMD CPU than amd_iommu ;)

Can you define "better"?
My main concern is that if we don't give users the option to choose, the
only data point we have is yours ;-)
If we explicitly mention that you can use one on the other in the docs,
people can try it themselves and maybe we'll see some feedback on the
forums etc.

However, I'm fine with a patch for only the intel version for now as we
can always add an option later.

> Moreover it adds an extra AMDVI-PCI device that is using the first pci address.
> `kvm: -device VGA,id=vga,bus=pcie.0,addr=0x1: PCI: slot 1 function 0 not available for VGA, in use by AMDVI-PCI,id=(null)`

For that I'd say, try to add the AMDVI-PCI device manually to an
explicitly chosen slot. We need to avoid automatically added devices
like the plague, because moving them later can break live snapshots (and
windows).

> 
> I cannot find any good documentation for amd_iommu but it also seems like
> it has less features.

Less, or just not configurable? ;-)
I mean, if it works it works ;-)

> 
> $ qemu-system-x86_64 -device 'amd-iommu,help'
> amd-iommu options:
>   device-iotlb=<bool>    -  (default: false)
>   intremap=<OnOffAuto>   - on/off/auto (default: "auto")
>   pt=<bool>              -  (default: true)
> $ qemu-system-x86_64 -device 'intel-iommu,help'
> intel-iommu options:
>   aw-bits=<uint8>        -  (default: 39)
>   caching-mode=<bool>    -  (default: false)
>   device-iotlb=<bool>    -  (default: false)
>   dma-drain=<bool>       -  (default: true)
>   dma-translation=<bool> -  (default: true)
>   eim=<OnOffAuto>        - on/off/auto (default: "auto")
>   intremap=<OnOffAuto>   - on/off/auto (default: "auto")
>   pt=<bool>              -  (default: true)
>   snoop-control=<bool>   -  (default: false)
>   version=<uint32>       -  (default: 0)
>   x-buggy-eim=<bool>     -  (default: false)
>   x-pasid-mode=<bool>    -  (default: false)
>   x-scalable-mode=<bool> -  (default: false)