Re: [PATCH v4 07/11] pc: Allow instantiating a virtio-iommu device

[Jean-Philippe Brucker]

On Wed, Oct 06, 2021 at 09:19:54AM +0200, Igor Mammedov wrote:
> > @@ -1367,8 +1368,11 @@ static void pc_virtio_md_pci_unplug(HotplugHandler 
> > *hotplug_dev,
> >  static void pc_machine_device_pre_plug_cb(HotplugHandler *hotplug_dev,
> >                                            DeviceState *dev, Error **errp)
> >  {
> > -    if (object_dynamic_cast(OBJECT(dev), TYPE_X86_IOMMU_DEVICE) &&
> > -        x86_iommu_get_default()) {
> > +    PCMachineState *pcms = PC_MACHINE(hotplug_dev);
> > +
> > +    if ((object_dynamic_cast(OBJECT(dev), TYPE_X86_IOMMU_DEVICE) ||
> > +         object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_IOMMU_PCI)) &&
> > +        (x86_iommu_get_default() || pcms->virtio_iommu)) {
> 
> this check is getting uglier,
> may be instead of introducing pcms->virtio_iommu boolean, better approach
> would be adding 'Device* PCMachineState::iommu' and setting it to IOMMU
> so the check would reduce to:
>       if ((object_dynamic_cast(OBJECT(dev), TYPE_X86_IOMMU_DEVICE) ||
>            object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_IOMMU_PCI)))
>       {
>           if (pcms->iommu)
>             err
>           else set pcms->iommu in plug handler or here
>       }
>       
> that also will let to cleanup/get rid of x86_iommu_[s|g]et_default()
> and x86_iommu_default 'global'.
> Maybe replace previous patch with one that would remove
> x86_iommu_[s|g]et_default().

Ok, I can't figure out a nice way to do this at the moment, will think
more about it. Callers of x86_iommu_get_default() still need a helper, and
most but not all assume that the returned object is for IRQ remapping. At
least adding Device *iommu to pcms should be nicer, but not sure about
removing the x86_iommu_get/set_default helpers.

Thanks,
Jean