diff mbox

[V11,3/4] hw/core: provision for overriding emulated IOMMU

Message ID 1463912514-12658-4-git-send-email-davidkiarie4@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

David Kiarie May 22, 2016, 10:21 a.m. UTC
Added an enum, subject to review, to machine properties which
it used to override iommu emulated from Intel to AMD.

Signed-off-by: David Kiarie <davidkiarie4@gmail.com>
---
 hw/core/machine.c             | 29 ++++++++++++++++++++++++++---
 include/hw/boards.h           |  1 +
 include/hw/i386/intel_iommu.h |  1 +
 qemu-options.hx               |  7 +++++--
 util/qemu-config.c            |  8 ++++++--
 5 files changed, 39 insertions(+), 7 deletions(-)

Comments

Peter Xu May 24, 2016, 6:51 a.m. UTC | #1
On Sun, May 22, 2016 at 01:21:53PM +0300, David Kiarie wrote:
[...]
>  static void machine_set_suppress_vmdesc(Object *obj, bool value, Error **errp)
>  {
>      MachineState *ms = MACHINE(obj);
> @@ -473,10 +492,14 @@ static void machine_initfn(Object *obj)
>                                      "Firmware image",
>                                      NULL);
>      object_property_add_bool(obj, "iommu",
> -                             machine_get_iommu,
> -                             machine_set_iommu, NULL);
> +                             machine_get_iommu, machine_set_iommu, NULL);

Shall we avoid this line-merge change?

>      object_property_set_description(obj, "iommu",
> -                                    "Set on/off to enable/disable Intel IOMMU (VT-d)",
> +                                    "Set on to enable IOMMU emulation",
> +                                    NULL);
> +    object_property_add_str(obj, "x-iommu-type",
> +                            NULL, machine_set_iommu_override, NULL);
> +    object_property_set_description(obj, "x-iommu-type",
> +                                    "Set on to override emulated IOMMU to AMD IOMMU",

Here if we can still configure x-iommu-type to "intel", shall we
change it to something like:

"Configure specific type of IOMMU (intel/amd)"

I believe we just forgot to update this desc after we changed
x-iommu-type from boolean to string.

[...]
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 6106520..81217d3 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -38,7 +38,8 @@ DEF("machine", HAS_ARG, QEMU_OPTION_machine, \
>      "                kvm_shadow_mem=size of KVM shadow MMU\n"
>      "                dump-guest-core=on|off include guest memory in a core dump (default=on)\n"
>      "                mem-merge=on|off controls memory merge support (default: on)\n"
> -    "                iommu=on|off controls emulated Intel IOMMU (VT-d) support (default=off)\n"
> +    "                iommu=on|off controls emulated IOMMU support(default: off)\n"
> +    "                x-iommu-type=amd|intel overrides emulated IOMMU to AMD IOMMU (default: intel)\n"

Similar to above, how about:

"x-iommu-type=intel|amd configure emulated IOMMU type (default: intel)\n"

>      "                igd-passthru=on|off controls IGD GFX passthrough support (default=off)\n"
>      "                aes-key-wrap=on|off controls support for AES key wrapping (default=on)\n"
>      "                dea-key-wrap=on|off controls support for DEA key wrapping (default=on)\n"
> @@ -74,7 +75,9 @@ Enables or disables memory merge support. This feature, when supported by
>  the host, de-duplicates identical memory pages among VMs instances
>  (enabled by default).
>  @item iommu=on|off
> -Enables or disables emulated Intel IOMMU (VT-d) support. The default is off.
> +Enables and disables IOMMU emulation. The default is off.
> +@item x-iommu-type=on|off
> +Overrides emulated IOMMU from AMD IOMMU. By default Intel IOMMU is emulated.

Need to update here with latest codes as well:

"""
@item x-iommu-type=intel|amd
Configure IOMMU type (amd/intel).  By default Intel IOMMU is emulated.
"""

>  @item aes-key-wrap=on|off
>  Enables or disables AES key wrapping support on s390-ccw hosts. This feature
>  controls whether AES wrapping keys will be created to allow
> diff --git a/util/qemu-config.c b/util/qemu-config.c
> index fb97307..8886abf 100644
> --- a/util/qemu-config.c
> +++ b/util/qemu-config.c
> @@ -213,8 +213,12 @@ static QemuOptsList machine_opts = {
>              .help = "firmware image",
>          },{
>              .name = "iommu",
> -            .type = QEMU_OPT_BOOL,
> -            .help = "Set on/off to enable/disable Intel IOMMU (VT-d)",
> +            .type =  QEMU_OPT_BOOL,
                       ^
Meaningless extra space here?

Thanks,

-- peterx
Michael S. Tsirkin May 24, 2016, 11:49 a.m. UTC | #2
On Sun, May 22, 2016 at 01:21:53PM +0300, David Kiarie wrote:
> Added an enum, subject to review, to machine properties which
> it used to override iommu emulated from Intel to AMD.
> 
> Signed-off-by: David Kiarie <davidkiarie4@gmail.com>

Marcel's
 "enable iommu with -device"
seems like a better alternative.

> ---
>  hw/core/machine.c             | 29 ++++++++++++++++++++++++++---
>  include/hw/boards.h           |  1 +
>  include/hw/i386/intel_iommu.h |  1 +
>  qemu-options.hx               |  7 +++++--
>  util/qemu-config.c            |  8 ++++++--
>  5 files changed, 39 insertions(+), 7 deletions(-)
> 
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index 6dbbc85..fe44e25 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -15,6 +15,8 @@
>  #include "qapi/error.h"
>  #include "qapi-visit.h"
>  #include "qapi/visitor.h"
> +#include "hw/i386/amd_iommu.h"
> +#include "hw/i386/intel_iommu.h"
>  #include "hw/sysbus.h"
>  #include "sysemu/sysemu.h"
>  #include "qemu/error-report.h"
> @@ -297,9 +299,26 @@ static void machine_set_iommu(Object *obj, bool value, Error **errp)
>  {
>      MachineState *ms = MACHINE(obj);
>  
> +    ms->iommu_type = TYPE_INTEL;
>      ms->iommu = value;
>  }
>  
> +static void machine_set_iommu_override(Object *obj, const char *value,
> +                                       Error **errp)
> +{
> +    MachineState *ms = MACHINE(obj);
> +
> +    /* ensure a valid iommu type */
> +    if (g_strcmp0(value, AMD_IOMMU_STR) == 0) {
> +        ms->iommu_type = TYPE_AMD;
> +    } else if (g_strcmp0(value, INTEL_IOMMU_STR) == 0) {
> +        ms->iommu_type = TYPE_INTEL;
> +    } else {
> +        error_setg(errp, "Invalid IOMMU type %s", value);
> +        return;
> +    }
> +}
> +
>  static void machine_set_suppress_vmdesc(Object *obj, bool value, Error **errp)
>  {
>      MachineState *ms = MACHINE(obj);
> @@ -473,10 +492,14 @@ static void machine_initfn(Object *obj)
>                                      "Firmware image",
>                                      NULL);
>      object_property_add_bool(obj, "iommu",
> -                             machine_get_iommu,
> -                             machine_set_iommu, NULL);
> +                             machine_get_iommu, machine_set_iommu, NULL);
>      object_property_set_description(obj, "iommu",
> -                                    "Set on/off to enable/disable Intel IOMMU (VT-d)",
> +                                    "Set on to enable IOMMU emulation",
> +                                    NULL);
> +    object_property_add_str(obj, "x-iommu-type",
> +                            NULL, machine_set_iommu_override, NULL);
> +    object_property_set_description(obj, "x-iommu-type",
> +                                    "Set on to override emulated IOMMU to AMD IOMMU",
>                                      NULL);
>      object_property_add_bool(obj, "suppress-vmdesc",
>                               machine_get_suppress_vmdesc,
> diff --git a/include/hw/boards.h b/include/hw/boards.h
> index dbe6745..5b7eeda 100644
> --- a/include/hw/boards.h
> +++ b/include/hw/boards.h
> @@ -158,6 +158,7 @@ struct MachineState {
>      bool igd_gfx_passthru;
>      char *firmware;
>      bool iommu;
> +    IommuType iommu_type;
>      bool suppress_vmdesc;
>      bool enforce_config_section;
>  
> diff --git a/include/hw/i386/intel_iommu.h b/include/hw/i386/intel_iommu.h
> index b024ffa..539530c 100644
> --- a/include/hw/i386/intel_iommu.h
> +++ b/include/hw/i386/intel_iommu.h
> @@ -27,6 +27,7 @@
>  #define TYPE_INTEL_IOMMU_DEVICE "intel-iommu"
>  #define INTEL_IOMMU_DEVICE(obj) \
>       OBJECT_CHECK(IntelIOMMUState, (obj), TYPE_INTEL_IOMMU_DEVICE)
> +#define INTEL_IOMMU_STR "intel"
>  
>  /* DMAR Hardware Unit Definition address (IOMMU unit) */
>  #define Q35_HOST_BRIDGE_IOMMU_ADDR  0xfed90000ULL
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 6106520..81217d3 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -38,7 +38,8 @@ DEF("machine", HAS_ARG, QEMU_OPTION_machine, \
>      "                kvm_shadow_mem=size of KVM shadow MMU\n"
>      "                dump-guest-core=on|off include guest memory in a core dump (default=on)\n"
>      "                mem-merge=on|off controls memory merge support (default: on)\n"
> -    "                iommu=on|off controls emulated Intel IOMMU (VT-d) support (default=off)\n"
> +    "                iommu=on|off controls emulated IOMMU support(default: off)\n"
> +    "                x-iommu-type=amd|intel overrides emulated IOMMU to AMD IOMMU (default: intel)\n"
>      "                igd-passthru=on|off controls IGD GFX passthrough support (default=off)\n"
>      "                aes-key-wrap=on|off controls support for AES key wrapping (default=on)\n"
>      "                dea-key-wrap=on|off controls support for DEA key wrapping (default=on)\n"
> @@ -74,7 +75,9 @@ Enables or disables memory merge support. This feature, when supported by
>  the host, de-duplicates identical memory pages among VMs instances
>  (enabled by default).
>  @item iommu=on|off
> -Enables or disables emulated Intel IOMMU (VT-d) support. The default is off.
> +Enables and disables IOMMU emulation. The default is off.
> +@item x-iommu-type=on|off
> +Overrides emulated IOMMU from AMD IOMMU. By default Intel IOMMU is emulated.
>  @item aes-key-wrap=on|off
>  Enables or disables AES key wrapping support on s390-ccw hosts. This feature
>  controls whether AES wrapping keys will be created to allow
> diff --git a/util/qemu-config.c b/util/qemu-config.c
> index fb97307..8886abf 100644
> --- a/util/qemu-config.c
> +++ b/util/qemu-config.c
> @@ -213,8 +213,12 @@ static QemuOptsList machine_opts = {
>              .help = "firmware image",
>          },{
>              .name = "iommu",
> -            .type = QEMU_OPT_BOOL,
> -            .help = "Set on/off to enable/disable Intel IOMMU (VT-d)",
> +            .type =  QEMU_OPT_BOOL,
> +            .help = "Set on/off to enable iommu",
> +        },{
> +            .name = "x-iommu-type",
> +            .type =  QEMU_OPT_STRING,
> +            .help = "Overrides emulated IOMMU from Intel to AMD",
>          },{
>              .name = "suppress-vmdesc",
>              .type = QEMU_OPT_BOOL,
> -- 
> 2.1.4
Jan Kiszka May 24, 2016, 1:01 p.m. UTC | #3
On 2016-05-24 13:49, Michael S. Tsirkin wrote:
> On Sun, May 22, 2016 at 01:21:53PM +0300, David Kiarie wrote:
>> Added an enum, subject to review, to machine properties which
>> it used to override iommu emulated from Intel to AMD.
>>
>> Signed-off-by: David Kiarie <davidkiarie4@gmail.com>
> 
> Marcel's
>  "enable iommu with -device"
> seems like a better alternative.

If that works for both IOMMUs, it would be preferred. But the last time
I checked (granted: long ago), there were internal ordering issues
preventing this.

Bonus of moving IOMMU machine options over: They are getting more and
more, and many are now very IOMMU-specific (like the CM control for Intel).

Jan
Marcel Apfelbaum May 24, 2016, 2:23 p.m. UTC | #4
On 05/24/2016 04:01 PM, Jan Kiszka wrote:
> On 2016-05-24 13:49, Michael S. Tsirkin wrote:
>> On Sun, May 22, 2016 at 01:21:53PM +0300, David Kiarie wrote:
>>> Added an enum, subject to review, to machine properties which
>>> it used to override iommu emulated from Intel to AMD.
>>>
>>> Signed-off-by: David Kiarie <davidkiarie4@gmail.com>
>>
>> Marcel's
>>   "enable iommu with -device"
>> seems like a better alternative.
>
> If that works for both IOMMUs, it would be preferred. But the last time
> I checked (granted: long ago), there were internal ordering issues
> preventing this.
>

It will work for both. The reason is we delay the bus_master_region creation
to machine_done.

Thanks,
Marcel

> Bonus of moving IOMMU machine options over: They are getting more and
> more, and many are now very IOMMU-specific (like the CM control for Intel).
>
> Jan
>
>
diff mbox

Patch

diff --git a/hw/core/machine.c b/hw/core/machine.c
index 6dbbc85..fe44e25 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -15,6 +15,8 @@ 
 #include "qapi/error.h"
 #include "qapi-visit.h"
 #include "qapi/visitor.h"
+#include "hw/i386/amd_iommu.h"
+#include "hw/i386/intel_iommu.h"
 #include "hw/sysbus.h"
 #include "sysemu/sysemu.h"
 #include "qemu/error-report.h"
@@ -297,9 +299,26 @@  static void machine_set_iommu(Object *obj, bool value, Error **errp)
 {
     MachineState *ms = MACHINE(obj);
 
+    ms->iommu_type = TYPE_INTEL;
     ms->iommu = value;
 }
 
+static void machine_set_iommu_override(Object *obj, const char *value,
+                                       Error **errp)
+{
+    MachineState *ms = MACHINE(obj);
+
+    /* ensure a valid iommu type */
+    if (g_strcmp0(value, AMD_IOMMU_STR) == 0) {
+        ms->iommu_type = TYPE_AMD;
+    } else if (g_strcmp0(value, INTEL_IOMMU_STR) == 0) {
+        ms->iommu_type = TYPE_INTEL;
+    } else {
+        error_setg(errp, "Invalid IOMMU type %s", value);
+        return;
+    }
+}
+
 static void machine_set_suppress_vmdesc(Object *obj, bool value, Error **errp)
 {
     MachineState *ms = MACHINE(obj);
@@ -473,10 +492,14 @@  static void machine_initfn(Object *obj)
                                     "Firmware image",
                                     NULL);
     object_property_add_bool(obj, "iommu",
-                             machine_get_iommu,
-                             machine_set_iommu, NULL);
+                             machine_get_iommu, machine_set_iommu, NULL);
     object_property_set_description(obj, "iommu",
-                                    "Set on/off to enable/disable Intel IOMMU (VT-d)",
+                                    "Set on to enable IOMMU emulation",
+                                    NULL);
+    object_property_add_str(obj, "x-iommu-type",
+                            NULL, machine_set_iommu_override, NULL);
+    object_property_set_description(obj, "x-iommu-type",
+                                    "Set on to override emulated IOMMU to AMD IOMMU",
                                     NULL);
     object_property_add_bool(obj, "suppress-vmdesc",
                              machine_get_suppress_vmdesc,
diff --git a/include/hw/boards.h b/include/hw/boards.h
index dbe6745..5b7eeda 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -158,6 +158,7 @@  struct MachineState {
     bool igd_gfx_passthru;
     char *firmware;
     bool iommu;
+    IommuType iommu_type;
     bool suppress_vmdesc;
     bool enforce_config_section;
 
diff --git a/include/hw/i386/intel_iommu.h b/include/hw/i386/intel_iommu.h
index b024ffa..539530c 100644
--- a/include/hw/i386/intel_iommu.h
+++ b/include/hw/i386/intel_iommu.h
@@ -27,6 +27,7 @@ 
 #define TYPE_INTEL_IOMMU_DEVICE "intel-iommu"
 #define INTEL_IOMMU_DEVICE(obj) \
      OBJECT_CHECK(IntelIOMMUState, (obj), TYPE_INTEL_IOMMU_DEVICE)
+#define INTEL_IOMMU_STR "intel"
 
 /* DMAR Hardware Unit Definition address (IOMMU unit) */
 #define Q35_HOST_BRIDGE_IOMMU_ADDR  0xfed90000ULL
diff --git a/qemu-options.hx b/qemu-options.hx
index 6106520..81217d3 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -38,7 +38,8 @@  DEF("machine", HAS_ARG, QEMU_OPTION_machine, \
     "                kvm_shadow_mem=size of KVM shadow MMU\n"
     "                dump-guest-core=on|off include guest memory in a core dump (default=on)\n"
     "                mem-merge=on|off controls memory merge support (default: on)\n"
-    "                iommu=on|off controls emulated Intel IOMMU (VT-d) support (default=off)\n"
+    "                iommu=on|off controls emulated IOMMU support(default: off)\n"
+    "                x-iommu-type=amd|intel overrides emulated IOMMU to AMD IOMMU (default: intel)\n"
     "                igd-passthru=on|off controls IGD GFX passthrough support (default=off)\n"
     "                aes-key-wrap=on|off controls support for AES key wrapping (default=on)\n"
     "                dea-key-wrap=on|off controls support for DEA key wrapping (default=on)\n"
@@ -74,7 +75,9 @@  Enables or disables memory merge support. This feature, when supported by
 the host, de-duplicates identical memory pages among VMs instances
 (enabled by default).
 @item iommu=on|off
-Enables or disables emulated Intel IOMMU (VT-d) support. The default is off.
+Enables and disables IOMMU emulation. The default is off.
+@item x-iommu-type=on|off
+Overrides emulated IOMMU from AMD IOMMU. By default Intel IOMMU is emulated.
 @item aes-key-wrap=on|off
 Enables or disables AES key wrapping support on s390-ccw hosts. This feature
 controls whether AES wrapping keys will be created to allow
diff --git a/util/qemu-config.c b/util/qemu-config.c
index fb97307..8886abf 100644
--- a/util/qemu-config.c
+++ b/util/qemu-config.c
@@ -213,8 +213,12 @@  static QemuOptsList machine_opts = {
             .help = "firmware image",
         },{
             .name = "iommu",
-            .type = QEMU_OPT_BOOL,
-            .help = "Set on/off to enable/disable Intel IOMMU (VT-d)",
+            .type =  QEMU_OPT_BOOL,
+            .help = "Set on/off to enable iommu",
+        },{
+            .name = "x-iommu-type",
+            .type =  QEMU_OPT_STRING,
+            .help = "Overrides emulated IOMMU from Intel to AMD",
         },{
             .name = "suppress-vmdesc",
             .type = QEMU_OPT_BOOL,