> Hi Daniel,
>
>> Top level libvirt device representation in XML is based on the device
>> *class*, not the specific device impl. Adding a <nestedSmmuv3> device
>> type XML element in libvirt is totally inappropriate. Any configuration
>> must be done beneath the <iommu> element.
> Would keeping track of PXB <=> host SMMU nodes be better represented with a
> <nestedSmmuv3> PXB attribute like below, when the "nestedSmmuv3"
IOMMU model
> is specified? This method would be simplest IMO because we could omit
> keeping track of the nestedSmmuv3 bus number in the virDomainDef struct
> since its association with a PXB controller would already be baked-in.
>
> <devices>
> ...
> <controller type='pci' index='1'
model='pcie-expander-bus'>
> <model name='pxb-pcie'/>
> <target busNr='254'/>
> <nestedSmmuv3>smmu3.0x0000000012000000</nestedSmmuv3>
> <address type='pci' domain='0x0000' bus='0x00'
slot='0x01' function='0x0'/>
> </controller>
> ...
> <iommu model='nestedSmmuv3'/>
> </devices>
The '<nestedSmmuv3>smmu3.0x0000000012000000</nestedSmmuv3>' bit
is also wierd, because it never appears in the QEMU command line
at all. Some kind of implicit association seems to be taking
place behind the scenes and that is unlikely to be a desirable
thing. We need to model associations between devices explicitly
and directly pass this on to QEMU.
If the nestedSmmuv3 is assocaited with a host SMMUv3 device,
then that association should be represented on the <iommu>
device, and this mapping passed to QEMU.
The '<nestedSmmuv3>smmu3.0x0000000012000000</nestedSmmuv3>' bit will
be
moved to the <iommu> definition in the next revision.
I agree that explicitly specifying the host SMMU node name in QEMU would
make the host to guest SMMU association more clear. But the current QEMU
implementation appears more flexible in allowing us to create
nestedSmmuv3 instances and then imply the association to a host SMMU
node based on the PCIe topology isolating devices under different PXBs.
I think we should discuss this further on the QEMU thread with Shameer
before he sends out the second revision of his series, then adjust the
libvirt implementation based on the next QEMU revision.
> Or would it still be preferred to purely contain the host SMMU
node names
> and bus numbers within the <iommu> element? If so, the virDomainDef struct
> is setup to only have a single virDomainIOMMUDef member, but we need to keep
> track of multiple host SMMU node names and bus numbers. Would we setup
> multiple virDomainIOMMUDef members? Or add "char** nestedSmmuv3" and
"size_t
> *nestedSmmuv3Bus" members to the virDomainIOMMUDef struct to keep track of
> these multiple host SMMU node names and bus numbers?
>
> Or we could modify the device info member of virDomainIOMMUDef to be a
> variable-length array of device info structs instead of the "size_t
> *nestedSmmuv3Bus" member. But I'm not convinced this would be the cleanest
> approach, and the qemu command line doesn't specify a slot and function to
> arm-smmuv3-nested devices - it just specifies bus number to keep track of
> which PXB is associated with each SMMU node.
>
> If we need a way to associate PXB to SMMU node, we have multiple possible
> approaches, listed below in order of best to worst in my opinion:
>
> 1. Adding a <nestedSmmuv3> attribute for PXB controller.
> 2. Having a single virDomainIOMMUDef struct for virDomainDef. Adding
> variable-length array members to virDomainIOMMUDef for multiple SMMU node
> names and bus numbers.
> 3. Having a single virDomainIOMMUDef struct for virDomainDef. Adding a
> variable-length array member to virDomainIOMMUDef for SMMU node names.
> Changing the single virDomainDeviceInfo struct to a variable-length array of
> virDomainDeviceInfo structs for multiple nested SMMU bus numbers.
> 4. Supporting multiple virDomainIOMMUDef structs for virDomainDef.
We need (4). Each device must be represented as a distinct virDomainIOMMUDef,
and then associated to the controller and host SMMU.
Makes sense, I will proceed with this in the next revision. Thanks!