| Message ID | 1456078260-6669-3-git-send-email-davidkiarie4@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 2016-02-21 19:10, David Kiarie wrote: > diff --git a/qemu-options.hx b/qemu-options.hx > index 2f0465e..dad160f 100644 > --- a/qemu-options.hx > +++ b/qemu-options.hx > @@ -38,7 +38,7 @@ 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=amd|intel enables and selects the emulated IOMMU (default: off)\n" We should also support "iommu=off" or "none" to explicitly disable it. That is consistent with other switches, and maybe there will once be a machine type (chipset) with IOMMU enabled by default. Jan
On 21/02/16 23:09, Jan Kiszka wrote: > On 2016-02-21 19:10, David Kiarie wrote: >> diff --git a/qemu-options.hx b/qemu-options.hx >> index 2f0465e..dad160f 100644 >> --- a/qemu-options.hx >> +++ b/qemu-options.hx >> @@ -38,7 +38,7 @@ 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=amd|intel enables and selects the emulated IOMMU (default: off)\n" > We should also support "iommu=off" or "none" to explicitly disable it. > That is consistent with other switches, and maybe there will once be a > machine type (chipset) with IOMMU enabled by default. We could have such but this will not be referenced anywhere in the code as the IOMMU is 'off' by default. Most of the other such switches relate to properties that are 'on' by default. > > Jan >
On 03/02/2016 10:51 PM, David Kiarie wrote: > > > On 21/02/16 23:09, Jan Kiszka wrote: >> On 2016-02-21 19:10, David Kiarie wrote: >>> diff --git a/qemu-options.hx b/qemu-options.hx >>> index 2f0465e..dad160f 100644 >>> --- a/qemu-options.hx >>> +++ b/qemu-options.hx >>> @@ -38,7 +38,7 @@ 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=amd|intel enables and selects the emulated IOMMU (default: off)\n" >> We should also support "iommu=off" or "none" to explicitly disable it. >> That is consistent with other switches, and maybe there will once be a >> machine type (chipset) with IOMMU enabled by default. > > We could have such but this will not be referenced anywhere in the code as the IOMMU is 'off' by default. Most of the other such switches relate to properties that are 'on' by default. It doesn't matter if is not used, some management systems would prefer to pass an "off" value rather than not adding the property. Also you need a default value. Even in the description you mention it as "off". Thanks, Marcel > >> >> Jan >> >
On Sun, Feb 21, 2016 at 09:10:58PM +0300, David Kiarie wrote: > Add IOMMU as a string to machine properties which is > used to control whether and the type of IOMMU to emulate > > Signed-off-by: David Kiarie <davidkiarie4@gmail.com> > --- > hw/core/machine.c | 28 ++++++++++++++++++++-------- > include/hw/boards.h | 3 ++- > qemu-options.hx | 6 +++--- > util/qemu-config.c | 4 ++-- > 4 files changed, 27 insertions(+), 14 deletions(-) > > diff --git a/hw/core/machine.c b/hw/core/machine.c > index 6d1a0d8..001ace9 100644 > --- a/hw/core/machine.c > +++ b/hw/core/machine.c > @@ -14,6 +14,8 @@ > #include "hw/boards.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" > @@ -284,18 +286,28 @@ static void machine_set_firmware(Object *obj, const char *value, Error **errp) > ms->firmware = g_strdup(value); > } > > -static bool machine_get_iommu(Object *obj, Error **errp) > +static char *machine_get_iommu(Object *obj, Error **errp) > { > MachineState *ms = MACHINE(obj); > > - return ms->iommu; > + return g_strdup(ms->iommu); > } > > -static void machine_set_iommu(Object *obj, bool value, Error **errp) > +static void machine_set_iommu(Object *obj, const char *value, Error **errp) > { > MachineState *ms = MACHINE(obj); > + Error *err = NULL; > + > + g_free(ms->iommu); > + > + if (g_strcmp0(value, AMD_IOMMU_STR) && > + g_strcmp0(value, INTEL_IOMMU_STR)) { > + error_setg(errp, "Invalid IOMMU type %s", value); > + error_propagate(errp, err); > + return; > + } > > - ms->iommu = value; > + ms->iommu = g_strdup(value); > } > > static void machine_set_suppress_vmdesc(Object *obj, bool value, Error **errp) > @@ -455,11 +467,10 @@ static void machine_initfn(Object *obj) > object_property_set_description(obj, "firmware", > "Firmware image", > NULL); > - object_property_add_bool(obj, "iommu", > - machine_get_iommu, > - machine_set_iommu, NULL); > + object_property_add_str(obj, "iommu", > + machine_get_iommu, machine_set_iommu, NULL); > object_property_set_description(obj, "iommu", > - "Set on/off to enable/disable Intel IOMMU (VT-d)", > + "IOMMU list", > NULL); > object_property_add_bool(obj, "suppress-vmdesc", > machine_get_suppress_vmdesc, > @@ -485,6 +496,7 @@ static void machine_finalize(Object *obj) > g_free(ms->dumpdtb); > g_free(ms->dt_compatible); > g_free(ms->firmware); > + g_free(ms->iommu); > } > > bool machine_usb(MachineState *machine) > diff --git a/include/hw/boards.h b/include/hw/boards.h > index 0f30959..b119245 100644 > --- a/include/hw/boards.h > +++ b/include/hw/boards.h > @@ -36,6 +36,7 @@ bool machine_usb(MachineState *machine); > bool machine_kernel_irqchip_allowed(MachineState *machine); > bool machine_kernel_irqchip_required(MachineState *machine); > bool machine_kernel_irqchip_split(MachineState *machine); > +bool machine_amd_iommu(MachineState *machine); > int machine_kvm_shadow_mem(MachineState *machine); > int machine_phandle_start(MachineState *machine); > bool machine_dump_guest_core(MachineState *machine); > @@ -126,7 +127,7 @@ struct MachineState { > bool usb_disabled; > bool igd_gfx_passthru; > char *firmware; > - bool iommu; > + char *iommu; > bool suppress_vmdesc; > > ram_addr_t ram_size; > diff --git a/qemu-options.hx b/qemu-options.hx > index 2f0465e..dad160f 100644 > --- a/qemu-options.hx > +++ b/qemu-options.hx > @@ -38,7 +38,7 @@ 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=amd|intel enables and selects the emulated IOMMU (default: off)\n" Besides what Jan said (it says default:off so off should be available as an option), "on" should be available as an option too, to 1. make qemu select an automatic model if user does not care 2. keep compatibility with existing users Given this code is still kind of rough, how about a separate x-iommu-type property to override type from intel to AMD? > " 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" > @@ -72,8 +72,8 @@ Include guest memory in a core dump. The default is on. > 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. > +@item iommu=intel|amd > +Enables and selects the emulated IOMMU. The default is off. > @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..f1b5a3b 100644 > --- a/util/qemu-config.c > +++ b/util/qemu-config.c > @@ -213,8 +213,8 @@ 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_STRING, > + .help = "Enables IOMMU and sets the emulated type", > },{ > .name = "suppress-vmdesc", > .type = QEMU_OPT_BOOL, > -- > 2.1.4
diff --git a/hw/core/machine.c b/hw/core/machine.c index 6d1a0d8..001ace9 100644 --- a/hw/core/machine.c +++ b/hw/core/machine.c @@ -14,6 +14,8 @@ #include "hw/boards.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" @@ -284,18 +286,28 @@ static void machine_set_firmware(Object *obj, const char *value, Error **errp) ms->firmware = g_strdup(value); } -static bool machine_get_iommu(Object *obj, Error **errp) +static char *machine_get_iommu(Object *obj, Error **errp) { MachineState *ms = MACHINE(obj); - return ms->iommu; + return g_strdup(ms->iommu); } -static void machine_set_iommu(Object *obj, bool value, Error **errp) +static void machine_set_iommu(Object *obj, const char *value, Error **errp) { MachineState *ms = MACHINE(obj); + Error *err = NULL; + + g_free(ms->iommu); + + if (g_strcmp0(value, AMD_IOMMU_STR) && + g_strcmp0(value, INTEL_IOMMU_STR)) { + error_setg(errp, "Invalid IOMMU type %s", value); + error_propagate(errp, err); + return; + } - ms->iommu = value; + ms->iommu = g_strdup(value); } static void machine_set_suppress_vmdesc(Object *obj, bool value, Error **errp) @@ -455,11 +467,10 @@ static void machine_initfn(Object *obj) object_property_set_description(obj, "firmware", "Firmware image", NULL); - object_property_add_bool(obj, "iommu", - machine_get_iommu, - machine_set_iommu, NULL); + object_property_add_str(obj, "iommu", + machine_get_iommu, machine_set_iommu, NULL); object_property_set_description(obj, "iommu", - "Set on/off to enable/disable Intel IOMMU (VT-d)", + "IOMMU list", NULL); object_property_add_bool(obj, "suppress-vmdesc", machine_get_suppress_vmdesc, @@ -485,6 +496,7 @@ static void machine_finalize(Object *obj) g_free(ms->dumpdtb); g_free(ms->dt_compatible); g_free(ms->firmware); + g_free(ms->iommu); } bool machine_usb(MachineState *machine) diff --git a/include/hw/boards.h b/include/hw/boards.h index 0f30959..b119245 100644 --- a/include/hw/boards.h +++ b/include/hw/boards.h @@ -36,6 +36,7 @@ bool machine_usb(MachineState *machine); bool machine_kernel_irqchip_allowed(MachineState *machine); bool machine_kernel_irqchip_required(MachineState *machine); bool machine_kernel_irqchip_split(MachineState *machine); +bool machine_amd_iommu(MachineState *machine); int machine_kvm_shadow_mem(MachineState *machine); int machine_phandle_start(MachineState *machine); bool machine_dump_guest_core(MachineState *machine); @@ -126,7 +127,7 @@ struct MachineState { bool usb_disabled; bool igd_gfx_passthru; char *firmware; - bool iommu; + char *iommu; bool suppress_vmdesc; ram_addr_t ram_size; diff --git a/qemu-options.hx b/qemu-options.hx index 2f0465e..dad160f 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -38,7 +38,7 @@ 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=amd|intel enables and selects the emulated IOMMU (default: off)\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" @@ -72,8 +72,8 @@ Include guest memory in a core dump. The default is on. 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. +@item iommu=intel|amd +Enables and selects the emulated IOMMU. The default is off. @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..f1b5a3b 100644 --- a/util/qemu-config.c +++ b/util/qemu-config.c @@ -213,8 +213,8 @@ 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_STRING, + .help = "Enables IOMMU and sets the emulated type", },{ .name = "suppress-vmdesc", .type = QEMU_OPT_BOOL,
Add IOMMU as a string to machine properties which is used to control whether and the type of IOMMU to emulate Signed-off-by: David Kiarie <davidkiarie4@gmail.com> --- hw/core/machine.c | 28 ++++++++++++++++++++-------- include/hw/boards.h | 3 ++- qemu-options.hx | 6 +++--- util/qemu-config.c | 4 ++-- 4 files changed, 27 insertions(+), 14 deletions(-)