| Message ID | 1453130745-25793-3-git-send-email-davidkiarie4@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 01/18/2016 05:25 PM, David Kiarie wrote: > Add IO MMU as a string to machine properties which > is used to control whether and the type of IO MMU > to emulate > > Signed-off-by: David Kiarie <davidkiarie4@gmail.com> > --- > hw/core/machine.c | 17 +++++++++-------- > include/hw/boards.h | 3 ++- > include/hw/i386/intel_iommu.h | 1 + > qemu-options.hx | 6 +++--- > util/qemu-config.c | 4 ++-- > vl.c | 8 ++++++++ > 6 files changed, 25 insertions(+), 14 deletions(-) > > diff --git a/hw/core/machine.c b/hw/core/machine.c > index c46ddc7..cb309aa 100644 > --- a/hw/core/machine.c > +++ b/hw/core/machine.c > @@ -283,18 +283,19 @@ 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); > > - ms->iommu = value; > + g_free(ms->iommu); > + ms->iommu = g_strdup(value); > } > > static void machine_set_suppress_vmdesc(Object *obj, bool value, Error **errp) > @@ -454,11 +455,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, > @@ -484,6 +484,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/include/hw/i386/intel_iommu.h b/include/hw/i386/intel_iommu.h > index 5dbadb7..0b32bd6 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 215d00d..ac327c8 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 IO MMU (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 IO MMU. 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 687fd34..f79b98c 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 IO MMU and sets the emulated type", > },{ > .name = "suppress-vmdesc", > .type = QEMU_OPT_BOOL, > diff --git a/vl.c b/vl.c > index b7a083e..4e39caa 100644 > --- a/vl.c > +++ b/vl.c > @@ -65,6 +65,8 @@ int main(int argc, char **argv) > #include "sysemu/accel.h" > #include "hw/usb.h" > #include "hw/i386/pc.h" > +#include "hw/i386/amd_iommu.h" > +#include "hw/i386/intel_iommu.h" > #include "hw/isa/isa.h" > #include "hw/bt.h" > #include "sysemu/watchdog.h" > @@ -3677,6 +3679,12 @@ int main(int argc, char **argv, char **envp) > if (!opts) { > exit(1); > } > + const char *iommu = qemu_opt_get(opts, "iommu"); > + if (iommu && !(g_strcmp0(iommu, AMD_IOMMU_STR) || > + g_strcmp0(iommu, INTEL_IOMMU_STR))) { Hi, I am sorry for bugging you, please don't re-post before you get more meaningful reviews, that being said: I don't know if the above will work as (g_strcmp0(iommu, AMD_IOMMU_STR) || g_strcmp0(iommu, INTEL_IOMMU_STR)) will always return 1. Right :) ? However, you should put this check in machine_set_iommu. You fill in the errp and you will get a nice error message when the machine properties are parsed. Thanks, Marcel > + fprintf(stderr, "Invalid IO MMU type %s\n", iommu); > + exit(1); > + } > break; > case QEMU_OPTION_no_kvm: > olist = qemu_find_opts("machine"); >
On 1/18/2016 7:21 PM, Marcel Apfelbaum wrote: > On 01/18/2016 05:25 PM, David Kiarie wrote: >> Add IO MMU as a string to machine properties which >> is used to control whether and the type of IO MMU >> to emulate >> >> Signed-off-by: David Kiarie <davidkiarie4@gmail.com> >> --- >> hw/core/machine.c | 17 +++++++++-------- >> include/hw/boards.h | 3 ++- >> include/hw/i386/intel_iommu.h | 1 + >> qemu-options.hx | 6 +++--- >> util/qemu-config.c | 4 ++-- >> vl.c | 8 ++++++++ >> 6 files changed, 25 insertions(+), 14 deletions(-) >> >> diff --git a/hw/core/machine.c b/hw/core/machine.c >> index c46ddc7..cb309aa 100644 >> --- a/hw/core/machine.c >> +++ b/hw/core/machine.c >> @@ -283,18 +283,19 @@ 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); >> >> - ms->iommu = value; >> + g_free(ms->iommu); >> + ms->iommu = g_strdup(value); >> } >> >> static void machine_set_suppress_vmdesc(Object *obj, bool value, >> Error **errp) >> @@ -454,11 +455,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, >> @@ -484,6 +484,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/include/hw/i386/intel_iommu.h >> b/include/hw/i386/intel_iommu.h >> index 5dbadb7..0b32bd6 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 215d00d..ac327c8 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 IO MMU (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 IO MMU. 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 687fd34..f79b98c 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 IO MMU and sets the emulated type", >> },{ >> .name = "suppress-vmdesc", >> .type = QEMU_OPT_BOOL, >> diff --git a/vl.c b/vl.c >> index b7a083e..4e39caa 100644 >> --- a/vl.c >> +++ b/vl.c >> @@ -65,6 +65,8 @@ int main(int argc, char **argv) >> #include "sysemu/accel.h" >> #include "hw/usb.h" >> #include "hw/i386/pc.h" >> +#include "hw/i386/amd_iommu.h" >> +#include "hw/i386/intel_iommu.h" >> #include "hw/isa/isa.h" >> #include "hw/bt.h" >> #include "sysemu/watchdog.h" >> @@ -3677,6 +3679,12 @@ int main(int argc, char **argv, char **envp) >> if (!opts) { >> exit(1); >> } >> + const char *iommu = qemu_opt_get(opts, "iommu"); >> + if (iommu && !(g_strcmp0(iommu, AMD_IOMMU_STR) || >> + g_strcmp0(iommu, INTEL_IOMMU_STR))) { > > Hi, > > I am sorry for bugging you, please don't re-post before > you get more meaningful reviews, that being said: > > I don't know if the above will work as > (g_strcmp0(iommu, AMD_IOMMU_STR) || g_strcmp0(iommu, INTEL_IOMMU_STR)) > will always return 1. Right :) ? Yep, always true. Above fails though if the string doesn't match any 'iommu' type string. > > However, you should put this check in machine_set_iommu. You fill in > the errp > and you will get a nice error message when the machine properties are > parsed. > > Thanks, > Marcel > > > >> + fprintf(stderr, "Invalid IO MMU type %s\n", iommu); >> + exit(1); >> + } >> break; >> case QEMU_OPTION_no_kvm: >> olist = qemu_find_opts("machine"); >> >
On 01/18/2016 06:48 PM, David Kiarie wrote: > > > On 1/18/2016 7:21 PM, Marcel Apfelbaum wrote: >> On 01/18/2016 05:25 PM, David Kiarie wrote: >>> Add IO MMU as a string to machine properties which >>> is used to control whether and the type of IO MMU >>> to emulate >>> >>> Signed-off-by: David Kiarie <davidkiarie4@gmail.com> >>> --- >>> hw/core/machine.c | 17 +++++++++-------- >>> include/hw/boards.h | 3 ++- >>> include/hw/i386/intel_iommu.h | 1 + >>> qemu-options.hx | 6 +++--- >>> util/qemu-config.c | 4 ++-- >>> vl.c | 8 ++++++++ >>> 6 files changed, 25 insertions(+), 14 deletions(-) >>> >>> diff --git a/hw/core/machine.c b/hw/core/machine.c >>> index c46ddc7..cb309aa 100644 >>> --- a/hw/core/machine.c >>> +++ b/hw/core/machine.c >>> @@ -283,18 +283,19 @@ 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); >>> >>> - ms->iommu = value; >>> + g_free(ms->iommu); >>> + ms->iommu = g_strdup(value); >>> } >>> >>> static void machine_set_suppress_vmdesc(Object *obj, bool value, Error **errp) >>> @@ -454,11 +455,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, >>> @@ -484,6 +484,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/include/hw/i386/intel_iommu.h b/include/hw/i386/intel_iommu.h >>> index 5dbadb7..0b32bd6 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 215d00d..ac327c8 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 IO MMU (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 IO MMU. 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 687fd34..f79b98c 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 IO MMU and sets the emulated type", >>> },{ >>> .name = "suppress-vmdesc", >>> .type = QEMU_OPT_BOOL, >>> diff --git a/vl.c b/vl.c >>> index b7a083e..4e39caa 100644 >>> --- a/vl.c >>> +++ b/vl.c >>> @@ -65,6 +65,8 @@ int main(int argc, char **argv) >>> #include "sysemu/accel.h" >>> #include "hw/usb.h" >>> #include "hw/i386/pc.h" >>> +#include "hw/i386/amd_iommu.h" >>> +#include "hw/i386/intel_iommu.h" >>> #include "hw/isa/isa.h" >>> #include "hw/bt.h" >>> #include "sysemu/watchdog.h" >>> @@ -3677,6 +3679,12 @@ int main(int argc, char **argv, char **envp) >>> if (!opts) { >>> exit(1); >>> } >>> + const char *iommu = qemu_opt_get(opts, "iommu"); >>> + if (iommu && !(g_strcmp0(iommu, AMD_IOMMU_STR) || >>> + g_strcmp0(iommu, INTEL_IOMMU_STR))) { >> >> Hi, >> >> I am sorry for bugging you, please don't re-post before >> you get more meaningful reviews, that being said: >> >> I don't know if the above will work as >> (g_strcmp0(iommu, AMD_IOMMU_STR) || g_strcmp0(iommu, INTEL_IOMMU_STR)) >> will always return 1. Right :) ? > > Yep, always true. > > Above fails though if the string doesn't match any 'iommu' type string. If you agree that the above is always true, !() is always false => if statement always false, meaning you never get the error message. Thanks, Marcel > >> >> However, you should put this check in machine_set_iommu. You fill in the errp >> and you will get a nice error message when the machine properties are parsed. >> >> Thanks, >> Marcel >> >> >> >>> + fprintf(stderr, "Invalid IO MMU type %s\n", iommu); >>> + exit(1); >>> + } >>> break; >>> case QEMU_OPTION_no_kvm: >>> olist = qemu_find_opts("machine"); >>> >> >
On 1/18/2016 8:27 PM, Marcel Apfelbaum wrote: > On 01/18/2016 06:48 PM, David Kiarie wrote: >> >> >> On 1/18/2016 7:21 PM, Marcel Apfelbaum wrote: >>> On 01/18/2016 05:25 PM, David Kiarie wrote: >>>> Add IO MMU as a string to machine properties which >>>> is used to control whether and the type of IO MMU >>>> to emulate >>>> >>>> Signed-off-by: David Kiarie <davidkiarie4@gmail.com> >>>> --- >>>> hw/core/machine.c | 17 +++++++++-------- >>>> include/hw/boards.h | 3 ++- >>>> include/hw/i386/intel_iommu.h | 1 + >>>> qemu-options.hx | 6 +++--- >>>> util/qemu-config.c | 4 ++-- >>>> vl.c | 8 ++++++++ >>>> 6 files changed, 25 insertions(+), 14 deletions(-) >>>> >>>> diff --git a/hw/core/machine.c b/hw/core/machine.c >>>> index c46ddc7..cb309aa 100644 >>>> --- a/hw/core/machine.c >>>> +++ b/hw/core/machine.c >>>> @@ -283,18 +283,19 @@ 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); >>>> >>>> - ms->iommu = value; >>>> + g_free(ms->iommu); >>>> + ms->iommu = g_strdup(value); >>>> } >>>> >>>> static void machine_set_suppress_vmdesc(Object *obj, bool value, >>>> Error **errp) >>>> @@ -454,11 +455,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, >>>> @@ -484,6 +484,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/include/hw/i386/intel_iommu.h >>>> b/include/hw/i386/intel_iommu.h >>>> index 5dbadb7..0b32bd6 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 215d00d..ac327c8 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 IO MMU (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 IO MMU. 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 687fd34..f79b98c 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 IO MMU and sets the emulated type", >>>> },{ >>>> .name = "suppress-vmdesc", >>>> .type = QEMU_OPT_BOOL, >>>> diff --git a/vl.c b/vl.c >>>> index b7a083e..4e39caa 100644 >>>> --- a/vl.c >>>> +++ b/vl.c >>>> @@ -65,6 +65,8 @@ int main(int argc, char **argv) >>>> #include "sysemu/accel.h" >>>> #include "hw/usb.h" >>>> #include "hw/i386/pc.h" >>>> +#include "hw/i386/amd_iommu.h" >>>> +#include "hw/i386/intel_iommu.h" >>>> #include "hw/isa/isa.h" >>>> #include "hw/bt.h" >>>> #include "sysemu/watchdog.h" >>>> @@ -3677,6 +3679,12 @@ int main(int argc, char **argv, char **envp) >>>> if (!opts) { >>>> exit(1); >>>> } >>>> + const char *iommu = qemu_opt_get(opts, "iommu"); >>>> + if (iommu && !(g_strcmp0(iommu, AMD_IOMMU_STR) || >>>> + g_strcmp0(iommu, INTEL_IOMMU_STR))) { >>> >>> Hi, >>> >>> I am sorry for bugging you, please don't re-post before >>> you get more meaningful reviews, that being said: >>> >>> I don't know if the above will work as >>> (g_strcmp0(iommu, AMD_IOMMU_STR) || g_strcmp0(iommu, >>> INTEL_IOMMU_STR)) >>> will always return 1. Right :) ? >> >> Yep, always true. >> >> Above fails though if the string doesn't match any 'iommu' type string. > > If you agree that the above is always true, !() is always false => > if statement always false, meaning you never get the error message. You right :) Queued for a fix. > > Thanks, > Marcel > >> >>> >>> However, you should put this check in machine_set_iommu. You fill in >>> the errp >>> and you will get a nice error message when the machine properties >>> are parsed. >>> >>> Thanks, >>> Marcel >>> >>> >>> >>>> + fprintf(stderr, "Invalid IO MMU type %s\n", >>>> iommu); >>>> + exit(1); >>>> + } >>>> break; >>>> case QEMU_OPTION_no_kvm: >>>> olist = qemu_find_opts("machine"); >>>> >>> >> >
On 01/18/2016 06:21 PM, Marcel Apfelbaum wrote: > On 01/18/2016 05:25 PM, David Kiarie wrote: >> Add IO MMU as a string to machine properties which >> is used to control whether and the type of IO MMU >> to emulate >> >> Signed-off-by: David Kiarie <davidkiarie4@gmail.com> >> --- [...] > Hi, > > I am sorry for bugging you, please don't re-post before > you get more meaningful reviews, I meant "meaningful reviews" *from others*, not from me :) If you have the time, please do re-post. Anyway, I will give it a try and read the AMD IOMMU spec, maybe I can help with patch [1/4] review. Thanks, Marcel [...]
diff --git a/hw/core/machine.c b/hw/core/machine.c index c46ddc7..cb309aa 100644 --- a/hw/core/machine.c +++ b/hw/core/machine.c @@ -283,18 +283,19 @@ 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); - ms->iommu = value; + g_free(ms->iommu); + ms->iommu = g_strdup(value); } static void machine_set_suppress_vmdesc(Object *obj, bool value, Error **errp) @@ -454,11 +455,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, @@ -484,6 +484,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/include/hw/i386/intel_iommu.h b/include/hw/i386/intel_iommu.h index 5dbadb7..0b32bd6 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 215d00d..ac327c8 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 IO MMU (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 IO MMU. 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 687fd34..f79b98c 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 IO MMU and sets the emulated type", },{ .name = "suppress-vmdesc", .type = QEMU_OPT_BOOL, diff --git a/vl.c b/vl.c index b7a083e..4e39caa 100644 --- a/vl.c +++ b/vl.c @@ -65,6 +65,8 @@ int main(int argc, char **argv) #include "sysemu/accel.h" #include "hw/usb.h" #include "hw/i386/pc.h" +#include "hw/i386/amd_iommu.h" +#include "hw/i386/intel_iommu.h" #include "hw/isa/isa.h" #include "hw/bt.h" #include "sysemu/watchdog.h" @@ -3677,6 +3679,12 @@ int main(int argc, char **argv, char **envp) if (!opts) { exit(1); } + const char *iommu = qemu_opt_get(opts, "iommu"); + if (iommu && !(g_strcmp0(iommu, AMD_IOMMU_STR) || + g_strcmp0(iommu, INTEL_IOMMU_STR))) { + fprintf(stderr, "Invalid IO MMU type %s\n", iommu); + exit(1); + } break; case QEMU_OPTION_no_kvm: olist = qemu_find_opts("machine");
Add IO MMU as a string to machine properties which is used to control whether and the type of IO MMU to emulate Signed-off-by: David Kiarie <davidkiarie4@gmail.com> --- hw/core/machine.c | 17 +++++++++-------- include/hw/boards.h | 3 ++- include/hw/i386/intel_iommu.h | 1 + qemu-options.hx | 6 +++--- util/qemu-config.c | 4 ++-- vl.c | 8 ++++++++ 6 files changed, 25 insertions(+), 14 deletions(-)