Re: [PATCH 2/4] x86/iommu: Make x86-iommu a singleton object

[Peter Xu <peterx@redhat.com>]

On Tue, Oct 29, 2024 at 10:47:06AM +0000, Daniel P. Berrangé wrote:
> On Thu, Oct 24, 2024 at 12:56:25PM -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.
> 
> Presumably the 'class' reported is the one that the user requested,
> but this would imply if we were to do
> 
>   qemu-system-x86_64 -device intel-iommu -device virtio-iommu
> 
> Then QEMU would report
> 
>    "Class 'virtio-iommu' only supports one instance"
> 
> at which point the user is wondering, huh, I only requested one virtio-iommu
> instance ?
> 
> IOW, the current error message would be better as it is not referring to a
> specific subclass, but rather to the more general fact that only a single
> IOMMU is permitted, no matter what it's impl is.

True.. though IIUC this is more or less a cosmetic change only.  E.g., if
we want (assuming after we could have object_new_allowed(Error **errp),
checking both abstract + singleton classes) we could make the error points
to the base class (rather than the top class to be initiated) that declared
TYPE_SINGLETON when it failed due to the singleton check.

One step further, we can even provide a custom Error for any singleton
class to say whatever it wants if it hits a duplicate.

So to me it's a separate issue from whether we would like to have a generic
way to define a singleton class.  I am still ok if we want to avoid
introducing the singleton, but just to mention I believe it can report
something similar as before if we want.

Thanks,

-- 
Peter Xu