| Message ID | 20241029211607.2114845-6-peterx@redhat.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | QOM: Singleton interface | expand |
On Tue, Oct 29, 2024 at 05:16:05PM -0400, Peter Xu wrote: > X86 IOMMUs cannot be created more than one on a system yet. Make it a > singleton so it guards the system from accidentally create yet another > IOMMU object when one already presents. > > Now if someone tries to create more than one, e.g., via: > > ./qemu -M q35 -device intel-iommu -device intel-iommu > > The error will change from: > > qemu-system-x86_64: -device intel-iommu: QEMU does not support multiple vIOMMUs for x86 yet. > > To: > > qemu-system-x86_64: -device intel-iommu: Class 'intel-iommu' only supports one instance > > Unfortunately, yet we can't remove the singleton check in the machine > hook (pc_machine_device_pre_plug_cb), because there can also be > virtio-iommu involved, which doesn't share a common parent class yet. > > But with this, it should be closer to reach that goal to check singleton by > QOM one day. Looking at the other iommu impls, I noticed that they all have something in common, in that they call pci_setup_iommu from their realize() function to register their set of callback functions. This pci_setup_iommu can happily be called multiple times and just over-writes previously registered callbacks. I wonder if this is a better place to diagnose incorrect usage of multiple impls. If pci_setup_iommu raised an error, it wouldn't matter that virtio-iommu doesn't share a common parent with intel-iommu. This would also perhaps be better for a future heterogeneous machine types, where it might be valid to have multiple iommus concurrently. Checking at the resource setup/registration point reflects where the physical constraint comes from. With regards, Daniel
diff --git a/hw/i386/x86-iommu.c b/hw/i386/x86-iommu.c index 60af896225..ec45b80306 100644 --- a/hw/i386/x86-iommu.c +++ b/hw/i386/x86-iommu.c @@ -26,6 +26,7 @@ #include "qemu/error-report.h" #include "trace.h" #include "sysemu/kvm.h" +#include "qom/object_interfaces.h" void x86_iommu_iec_register_notifier(X86IOMMUState *iommu, iec_notify_fn fn, void *data) @@ -79,9 +80,15 @@ void x86_iommu_irq_to_msi_message(X86IOMMUIrq *irq, MSIMessage *msg_out) X86IOMMUState *x86_iommu_get_default(void) { - MachineState *ms = MACHINE(qdev_get_machine()); - PCMachineState *pcms = - PC_MACHINE(object_dynamic_cast(OBJECT(ms), TYPE_PC_MACHINE)); + Object *machine = qdev_get_machine(); + PCMachineState *pcms; + + /* If machine has not been created, so is the vIOMMU */ + if (!machine) { + return NULL; + } + + pcms = PC_MACHINE(object_dynamic_cast(machine, TYPE_PC_MACHINE)); if (pcms && object_dynamic_cast(OBJECT(pcms->iommu), TYPE_X86_IOMMU_DEVICE)) { @@ -133,10 +140,19 @@ static Property x86_iommu_properties[] = { DEFINE_PROP_END_OF_LIST(), }; +static Object *x86_iommu_get_instance(void) +{ + return object_ref(OBJECT(x86_iommu_get_default())); +} + static void x86_iommu_class_init(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); + SingletonClass *singleton = SINGLETON_CLASS(klass); + dc->realize = x86_iommu_realize; + singleton->get_instance = x86_iommu_get_instance; + device_class_set_props(dc, x86_iommu_properties); } @@ -152,6 +168,10 @@ static const TypeInfo x86_iommu_info = { .class_init = x86_iommu_class_init, .class_size = sizeof(X86IOMMUClass), .abstract = true, + .interfaces = (InterfaceInfo[]) { + { TYPE_SINGLETON }, + { } + } }; static void x86_iommu_register_types(void)
X86 IOMMUs cannot be created more than one on a system yet. Make it a singleton so it guards the system from accidentally create yet another IOMMU object when one already presents. Now if someone tries to create more than one, e.g., via: ./qemu -M q35 -device intel-iommu -device intel-iommu The error will change from: qemu-system-x86_64: -device intel-iommu: QEMU does not support multiple vIOMMUs for x86 yet. To: qemu-system-x86_64: -device intel-iommu: Class 'intel-iommu' only supports one instance Unfortunately, yet we can't remove the singleton check in the machine hook (pc_machine_device_pre_plug_cb), because there can also be virtio-iommu involved, which doesn't share a common parent class yet. But with this, it should be closer to reach that goal to check singleton by QOM one day. Note: after this patch, x86_iommu_get_default() might be called very early, even before machine is created (e.g., cmdline "-device intel-iommu,help"). In this case we need to be prepared with such, and that also means the default iommu is not yet around. Signed-off-by: Peter Xu <peterx@redhat.com> --- hw/i386/x86-iommu.c | 26 +++++++++++++++++++++++--- 1 file changed, 23 insertions(+), 3 deletions(-)