diff mbox

[v2,3/3] q35: allow dynamic sysbus

Message ID 1464898555-14914-4-git-send-email-marcel@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Marcel Apfelbaum June 2, 2016, 8:15 p.m. UTC
Allow adding sysbus devices with -device on Q35.

At first Q35 will support only intel-iommu to be added this way,
however the command line will support all sysbus devices.

Mark with 'cannot_instantiate_with_device_add_yet' the ones
causing immediate problems (e.g. crashes).

Signed-off-by: Marcel Apfelbaum <marcel@redhat.com>
---
 hw/i386/pc_q35.c                    | 1 +
 hw/pci-bridge/pci_expander_bridge.c | 1 +
 hw/pci-host/piix.c                  | 1 +
 hw/pci-host/q35.c                   | 1 +
 4 files changed, 4 insertions(+)

Comments

Markus Armbruster June 3, 2016, 6:33 a.m. UTC | #1
Marcel Apfelbaum <marcel@redhat.com> writes:

> Allow adding sysbus devices with -device on Q35.
>
> At first Q35 will support only intel-iommu to be added this way,
> however the command line will support all sysbus devices.
>
> Mark with 'cannot_instantiate_with_device_add_yet' the ones
> causing immediate problems (e.g. crashes).
>
> Signed-off-by: Marcel Apfelbaum <marcel@redhat.com>
> ---
>  hw/i386/pc_q35.c                    | 1 +
>  hw/pci-bridge/pci_expander_bridge.c | 1 +
>  hw/pci-host/piix.c                  | 1 +
>  hw/pci-host/q35.c                   | 1 +
>  4 files changed, 4 insertions(+)
>
> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> index 04aae89..431eaed 100644
> --- a/hw/i386/pc_q35.c
> +++ b/hw/i386/pc_q35.c
> @@ -281,6 +281,7 @@ static void pc_q35_machine_options(MachineClass *m)
>      m->default_machine_opts = "firmware=bios-256k.bin";
>      m->default_display = "std";
>      m->no_floppy = 1;
> +    m->has_dynamic_sysbus = true;
>  }
>  
>  static void pc_q35_2_6_machine_options(MachineClass *m)
> diff --git a/hw/pci-bridge/pci_expander_bridge.c b/hw/pci-bridge/pci_expander_bridge.c
> index ba320bd..40518a2 100644
> --- a/hw/pci-bridge/pci_expander_bridge.c
> +++ b/hw/pci-bridge/pci_expander_bridge.c
> @@ -149,6 +149,7 @@ static void pxb_host_class_init(ObjectClass *class, void *data)
>      PCIHostBridgeClass *hc = PCI_HOST_BRIDGE_CLASS(class);
>  
>      dc->fw_name = "pci";
> +    dc->cannot_instantiate_with_device_add_yet = true;
>      sbc->explicit_ofw_unit_address = pxb_host_ofw_unit_address;
>      hc->root_bus_path = pxb_host_root_bus_path;
>  }

Any assignment to cannot_instantiate_with_device_add_yet must have a
comment, like this:

       /* Reason: frobnicates some frobs backwards */
       dc->cannot_instantiate_with_device_add_yet = true;

We have one offender in master: hw/ppc/spapr_pci.c (commit 09aa9a52),
which I'll try to get fixed.  Please don't add more.

[...]
Marcel Apfelbaum June 3, 2016, 6:47 a.m. UTC | #2
On 06/03/2016 09:33 AM, Markus Armbruster wrote:
> Marcel Apfelbaum <marcel@redhat.com> writes:
>
>> Allow adding sysbus devices with -device on Q35.
>>
>> At first Q35 will support only intel-iommu to be added this way,
>> however the command line will support all sysbus devices.
>>
>> Mark with 'cannot_instantiate_with_device_add_yet' the ones
>> causing immediate problems (e.g. crashes).
>>
>> Signed-off-by: Marcel Apfelbaum <marcel@redhat.com>
>> ---
>>   hw/i386/pc_q35.c                    | 1 +
>>   hw/pci-bridge/pci_expander_bridge.c | 1 +
>>   hw/pci-host/piix.c                  | 1 +
>>   hw/pci-host/q35.c                   | 1 +
>>   4 files changed, 4 insertions(+)
>>
>> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
>> index 04aae89..431eaed 100644
>> --- a/hw/i386/pc_q35.c
>> +++ b/hw/i386/pc_q35.c
>> @@ -281,6 +281,7 @@ static void pc_q35_machine_options(MachineClass *m)
>>       m->default_machine_opts = "firmware=bios-256k.bin";
>>       m->default_display = "std";
>>       m->no_floppy = 1;
>> +    m->has_dynamic_sysbus = true;
>>   }
>>
>>   static void pc_q35_2_6_machine_options(MachineClass *m)
>> diff --git a/hw/pci-bridge/pci_expander_bridge.c b/hw/pci-bridge/pci_expander_bridge.c
>> index ba320bd..40518a2 100644
>> --- a/hw/pci-bridge/pci_expander_bridge.c
>> +++ b/hw/pci-bridge/pci_expander_bridge.c
>> @@ -149,6 +149,7 @@ static void pxb_host_class_init(ObjectClass *class, void *data)
>>       PCIHostBridgeClass *hc = PCI_HOST_BRIDGE_CLASS(class);
>>
>>       dc->fw_name = "pci";
>> +    dc->cannot_instantiate_with_device_add_yet = true;
>>       sbc->explicit_ofw_unit_address = pxb_host_ofw_unit_address;
>>       hc->root_bus_path = pxb_host_root_bus_path;
>>   }
>
> Any assignment to cannot_instantiate_with_device_add_yet must have a
> comment, like this:
>
>         /* Reason: frobnicates some frobs backwards */
>         dc->cannot_instantiate_with_device_add_yet = true;
>
> We have one offender in master: hw/ppc/spapr_pci.c (commit 09aa9a52),
> which I'll try to get fixed.  Please don't add more.
>

Sure, I'll take care of it and send a V2.

Thanks,
Marcel


> [...]
>
Peter Xu June 8, 2016, 2:56 a.m. UTC | #3
On Thu, Jun 02, 2016 at 11:15:55PM +0300, Marcel Apfelbaum wrote:
> Allow adding sysbus devices with -device on Q35.
> 
> At first Q35 will support only intel-iommu to be added this way,
> however the command line will support all sysbus devices.
> 
> Mark with 'cannot_instantiate_with_device_add_yet' the ones
> causing immediate problems (e.g. crashes).

What happens if we do dynamic device_add for IOMMU? Guessing we should
not allow that as well?

-- peterx
Marcel Apfelbaum June 8, 2016, 11:18 a.m. UTC | #4
On 06/08/2016 05:56 AM, Peter Xu wrote:
> On Thu, Jun 02, 2016 at 11:15:55PM +0300, Marcel Apfelbaum wrote:
>> Allow adding sysbus devices with -device on Q35.
>>
>> At first Q35 will support only intel-iommu to be added this way,
>> however the command line will support all sysbus devices.
>>
>> Mark with 'cannot_instantiate_with_device_add_yet' the ones
>> causing immediate problems (e.g. crashes).
>
> What happens if we do dynamic device_add for IOMMU? Guessing we should
> not allow that as well?
>

Sure, hot-plug is not supported, at least for now.
I can think about a device that has an IOMMU built-in,
but this is not our use-case.

Thanks,
Marcel


> -- peterx
>
diff mbox

Patch

diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 04aae89..431eaed 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -281,6 +281,7 @@  static void pc_q35_machine_options(MachineClass *m)
     m->default_machine_opts = "firmware=bios-256k.bin";
     m->default_display = "std";
     m->no_floppy = 1;
+    m->has_dynamic_sysbus = true;
 }
 
 static void pc_q35_2_6_machine_options(MachineClass *m)
diff --git a/hw/pci-bridge/pci_expander_bridge.c b/hw/pci-bridge/pci_expander_bridge.c
index ba320bd..40518a2 100644
--- a/hw/pci-bridge/pci_expander_bridge.c
+++ b/hw/pci-bridge/pci_expander_bridge.c
@@ -149,6 +149,7 @@  static void pxb_host_class_init(ObjectClass *class, void *data)
     PCIHostBridgeClass *hc = PCI_HOST_BRIDGE_CLASS(class);
 
     dc->fw_name = "pci";
+    dc->cannot_instantiate_with_device_add_yet = true;
     sbc->explicit_ofw_unit_address = pxb_host_ofw_unit_address;
     hc->root_bus_path = pxb_host_root_bus_path;
 }
diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
index df2b0e2..fea7f59 100644
--- a/hw/pci-host/piix.c
+++ b/hw/pci-host/piix.c
@@ -865,6 +865,7 @@  static void i440fx_pcihost_class_init(ObjectClass *klass, void *data)
     dc->realize = i440fx_pcihost_realize;
     dc->fw_name = "pci";
     dc->props = i440fx_props;
+    dc->cannot_instantiate_with_device_add_yet = true;
 }
 
 static const TypeInfo i440fx_pcihost_info = {
diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
index ea684c7..9d82d30 100644
--- a/hw/pci-host/q35.c
+++ b/hw/pci-host/q35.c
@@ -138,6 +138,7 @@  static void q35_host_class_init(ObjectClass *klass, void *data)
     hc->root_bus_path = q35_host_root_bus_path;
     dc->realize = q35_host_realize;
     dc->props = mch_props;
+    dc->cannot_instantiate_with_device_add_yet = true;
     set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
     dc->fw_name = "pci";
 }