The only user-creatable sysbus devices in qemu-system-x86_64 are
amd-iommu, intel-iommu, and xen-backend.  xen-backend is handled
by xen_set_dynamic_sysbus(), so we only need to add amd-iommu and
intel-iommu.

Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Marcel Apfelbaum <marcel.a@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
Changes series v1 -> v2:
* New patch added to series
---
 hw/i386/pc_q35.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index d0b0e5b422..db2bebb357 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -42,6 +42,8 @@
 #include "exec/address-spaces.h"
 #include "hw/i386/pc.h"
 #include "hw/i386/ich9.h"
+#include "hw/i386/amd_iommu.h"
+#include "hw/i386/intel_iommu.h"
 #include "hw/smbios/smbios.h"
 #include "hw/ide/pci.h"
 #include "hw/ide/ahci.h"
@@ -299,8 +301,8 @@ static void pc_q35_machine_options(MachineClass *m)
     m->default_machine_opts = "firmware=bios-256k.bin";
     m->default_display = "std";
     m->no_floppy = 1;
-    /*TODO: allow only sysbus devices that really work with this machine */
-    machine_class_allow_dynamic_sysbus_dev(m, TYPE_SYS_BUS_DEVICE);
+    machine_class_allow_dynamic_sysbus_dev(m, TYPE_AMD_IOMMU_DEVICE);
+    machine_class_allow_dynamic_sysbus_dev(m, TYPE_INTEL_IOMMU_DEVICE);
     m->max_cpus = 288;
 }
 
-- 
2.13.6

On 25/11/2017 17:16, Eduardo Habkost wrote:
> The only user-creatable sysbus devices in qemu-system-x86_64 are
> amd-iommu, intel-iommu, and xen-backend.  xen-backend is handled
> by xen_set_dynamic_sysbus(), so we only need to add amd-iommu and
> intel-iommu.
> 
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Cc: Marcel Apfelbaum <marcel.a@redhat.com>
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
> Changes series v1 -> v2:
> * New patch added to series
> ---
>   hw/i386/pc_q35.c | 6 ++++--
>   1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> index d0b0e5b422..db2bebb357 100644
> --- a/hw/i386/pc_q35.c
> +++ b/hw/i386/pc_q35.c
> @@ -42,6 +42,8 @@
>   #include "exec/address-spaces.h"
>   #include "hw/i386/pc.h"
>   #include "hw/i386/ich9.h"
> +#include "hw/i386/amd_iommu.h"
> +#include "hw/i386/intel_iommu.h"

Is a pity we have to add the AMD/Intel header files
only for the type, maybe we should have a header only
with the type names, anyway out of the scope of this series.

Another question, what about I440FX?
I think the AMD vIOMMU can theoretically work with the
conventional PCI machines, I am not sure if it was tested
or intended to work with it.

Reviewed-by: Marcel Apfelbaum <marcel@redhat.com>

Thanks,
Marcel

>   #include "hw/smbios/smbios.h"
>   #include "hw/ide/pci.h"
>   #include "hw/ide/ahci.h"
> @@ -299,8 +301,8 @@ static void pc_q35_machine_options(MachineClass *m)
>       m->default_machine_opts = "firmware=bios-256k.bin";
>       m->default_display = "std";
>       m->no_floppy = 1;
> -    /*TODO: allow only sysbus devices that really work with this machine */
> -    machine_class_allow_dynamic_sysbus_dev(m, TYPE_SYS_BUS_DEVICE);
> +    machine_class_allow_dynamic_sysbus_dev(m, TYPE_AMD_IOMMU_DEVICE);
> +    machine_class_allow_dynamic_sysbus_dev(m, TYPE_INTEL_IOMMU_DEVICE);
>       m->max_cpus = 288;
>   }
>   
> 

On Mon, Nov 27, 2017 at 10:44:34AM +0200, Marcel Apfelbaum wrote:
> On 25/11/2017 17:16, Eduardo Habkost wrote:
> > The only user-creatable sysbus devices in qemu-system-x86_64 are
> > amd-iommu, intel-iommu, and xen-backend.  xen-backend is handled
> > by xen_set_dynamic_sysbus(), so we only need to add amd-iommu and
> > intel-iommu.
> > 
> > Cc: "Michael S. Tsirkin" <mst@redhat.com>
> > Cc: Marcel Apfelbaum <marcel.a@redhat.com>
> > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> > ---
> > Changes series v1 -> v2:
> > * New patch added to series
> > ---
> >   hw/i386/pc_q35.c | 6 ++++--
> >   1 file changed, 4 insertions(+), 2 deletions(-)
> > 
> > diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> > index d0b0e5b422..db2bebb357 100644
> > --- a/hw/i386/pc_q35.c
> > +++ b/hw/i386/pc_q35.c
> > @@ -42,6 +42,8 @@
> >   #include "exec/address-spaces.h"
> >   #include "hw/i386/pc.h"
> >   #include "hw/i386/ich9.h"
> > +#include "hw/i386/amd_iommu.h"
> > +#include "hw/i386/intel_iommu.h"
> 
> Is a pity we have to add the AMD/Intel header files
> only for the type, maybe we should have a header only
> with the type names, anyway out of the scope of this series.

Another option is to use the type name strings directly instead
of the TYPE_* macros (which I never really liked).  But I don't
want to deviate from the current practice.

> 
> Another question, what about I440FX?
> I think the AMD vIOMMU can theoretically work with the
> conventional PCI machines, I am not sure if it was tested
> or intended to work with it.

i440fx still has has_dynamic_sysbus=false.  With this series, we
can now add amd-iommu to its allowed list, if somebody is willing
to support/test it.


> 
> Reviewed-by: Marcel Apfelbaum <marcel@redhat.com>

Thanks!

> 
> Thanks,
> Marcel
> 
> >   #include "hw/smbios/smbios.h"
> >   #include "hw/ide/pci.h"
> >   #include "hw/ide/ahci.h"
> > @@ -299,8 +301,8 @@ static void pc_q35_machine_options(MachineClass *m)
> >       m->default_machine_opts = "firmware=bios-256k.bin";
> >       m->default_display = "std";
> >       m->no_floppy = 1;
> > -    /*TODO: allow only sysbus devices that really work with this machine */
> > -    machine_class_allow_dynamic_sysbus_dev(m, TYPE_SYS_BUS_DEVICE);
> > +    machine_class_allow_dynamic_sysbus_dev(m, TYPE_AMD_IOMMU_DEVICE);
> > +    machine_class_allow_dynamic_sysbus_dev(m, TYPE_INTEL_IOMMU_DEVICE);
> >       m->max_cpus = 288;
> >   }
> > 
> 

-- 
Eduardo