Hi Peter, On 09/10/2017 19:47, Peter Maydell wrote: > On 1 September 2017 at 18:21, Eric Auger <eric.au...@redhat.com> wrote: >> The VirtMachineState contains some dt phandles that will be used >> in some node creation functions. For instance we plan to use the >> PCI host controller phandle in the smmu node creation function. So >> let's pass the VirtMachineState handle down to the node creation >> functions by enhancing the involved datatypes. >> >> Signed-off-by: Eric Auger <eric.au...@redhat.com> >> --- >> hw/arm/sysbus-fdt.c | 3 +++ >> hw/arm/virt.c | 1 + >> include/hw/arm/sysbus-fdt.h | 2 ++ >> 3 files changed, 6 insertions(+) >> >> diff --git a/hw/arm/sysbus-fdt.c b/hw/arm/sysbus-fdt.c >> index d68e3dc..d92a983 100644 >> --- a/hw/arm/sysbus-fdt.c >> +++ b/hw/arm/sysbus-fdt.c >> @@ -36,6 +36,7 @@ >> #include "hw/vfio/vfio-platform.h" >> #include "hw/vfio/vfio-calxeda-xgmac.h" >> #include "hw/vfio/vfio-amd-xgbe.h" >> +#include "hw/arm/virt.h" >> #include "hw/arm/fdt.h" >> >> /* >> @@ -47,6 +48,7 @@ typedef struct PlatformBusFDTData { >> int irq_start; /* index of the first IRQ usable by platform bus devices >> */ >> const char *pbus_node_name; /* name of the platform bus node */ >> PlatformBusDevice *pbus; >> + VirtMachineState *vms; >> } PlatformBusFDTData; > > sysbus-fdt isn't virt specific, so this doesn't belong here. Correct. I plan to replace this by MachineState instead. > > More generally, why is sysbus-fdt involved in this at all? > I expected that instantiating and wiring up the SMMU would > be the job of hw/arm/virt.c, like any other device we > might have on the board. I wished to have the same type of option as for x86 where "-device intel-iommu" is passed to the QEMU command line. smmuv3 device being a SysBusDevice, a natural framework to handle its node creation function is sysbus-fdt. Having a -device approach is practical to pass other options to the device (this was typically the case for the "caching-mode" option). On Intel there are caching-mode, passthrough (pt) options.
Although the smmu caching-mode option may not survive in this form, we can foresee other options to come. In the future we may pass the PCI bus number we want to plug the smmu into. Also the invocation method would be identical for virtio-iommu. I explored the other (and simpler) alternative of passing an option to virt machine but I think this approach is less flexible. Also as you pointed out, by default, smmu would be on, if we stick to the existing convention. Given the perf overhead I don't think this is what we want. Thanks Eric > > thanks > -- PMM >