Re: [Qemu-devel] [PATCH v2 6/6] q35: Allow only supported dynamic sysbus devices

[Eduardo Habkost <ehabkost@redhat.com>]

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