| Message ID | 1416586732-26711-1-git-send-email-dslutz@verizon.com |
|---|---|
| State | New |
| Headers | show |
On Fri, Nov 21, 2014 at 11:18:52AM -0500, Don Slutz wrote: > c/s 9b23cfb76b3a5e9eb5cc899eaf2f46bc46d33ba4 > > or > > c/s b154537ad07598377ebf98252fb7d2aff127983b > > moved the testing of xen_enabled() from pc_init1() to > pc_machine_initfn(). > > xen_enabled() does not return the correct value in > pc_machine_initfn(). > > Changed vmport from a bool to an enum. Added the value "auto" to do > the old way. Move check of xen_enabled() back to pc_init1(). > > Signed-off-by: Don Slutz <dslutz@verizon.com> > Acked-by: Eric Blake <eblake@redhat.com> Reviewed-by: Eduardo Habkost <ehabkost@redhat.com> Thanks for your patience with the many requests for changes.
On 21/11/2014 17:18, Don Slutz wrote: > c/s 9b23cfb76b3a5e9eb5cc899eaf2f46bc46d33ba4 > > or > > c/s b154537ad07598377ebf98252fb7d2aff127983b > > moved the testing of xen_enabled() from pc_init1() to > pc_machine_initfn(). > > xen_enabled() does not return the correct value in > pc_machine_initfn(). > > Changed vmport from a bool to an enum. Added the value "auto" to do > the old way. Move check of xen_enabled() back to pc_init1(). > > Signed-off-by: Don Slutz <dslutz@verizon.com> > Acked-by: Eric Blake <eblake@redhat.com> > --- > v6: > > Added the sentence: "Move check of xen_enabled() back to pc_init1()." > to the commit message. > > I considered folding pc_machine_set_vmport and > pc_machine_get_vmport into 1 routine. Desided that for 2.2 I > would keep the extra safety of passing a copy of vmport in the get > routine. I do not think is is needed but without a comment > about this I took the simpler change. > > Eduardo Habkost: > Use visit_type_OnOffAuto not visit_type_enum. > Done -- Needed to add #include "qapi-visit.h" > Adjust usage of visit_type_OnOffAuto to avoid undefined > values. > Done > Change the property from "str" to "OnOffAuto" > Done. > > v5: > Eduardo Habkost: > What about changing pc_machine->vmport here instead of using a > no_vmport variable, so the actual vmport configuration may be > queried by anybody later using the QOM property? It would even > make the code shorter. > Done. > Eric Blake: > I've only reviewed the qapi/common.json and qemu-options.hx > files for QMP interface (and will leave the rest of the patch to > others), but I'm okay with the changes to those files. I guess > that means no R-b, since I didn't do a full review, so here's a > weaker acked-by. > > v4: > Michael S. Tsirkin, Eric Blake, Eduardo Habkost: > Rename vmport to OnOffAuto and move to qapi/common.json > Eduardo Habkost: > Simpler convert of enum to no_vmport. > Michael S. Tsirkin: > Add assert for ON_OFF_AUTO_MAX. > > hw/i386/pc.c | 22 +++++++++++++--------- > hw/i386/pc_piix.c | 7 ++++++- > hw/i386/pc_q35.c | 7 ++++++- > include/hw/i386/pc.h | 2 +- > qapi/common.json | 15 +++++++++++++++ > qemu-options.hx | 8 +++++--- > vl.c | 2 +- > 7 files changed, 47 insertions(+), 16 deletions(-) > > diff --git a/hw/i386/pc.c b/hw/i386/pc.c > index 1205db8..10c1fa5 100644 > --- a/hw/i386/pc.c > +++ b/hw/i386/pc.c > @@ -61,6 +61,7 @@ > #include "hw/mem/pc-dimm.h" > #include "trace.h" > #include "qapi/visitor.h" > +#include "qapi-visit.h" > > /* debug PC/ISA interrupts */ > //#define DEBUG_IRQ > @@ -1711,18 +1712,21 @@ static void pc_machine_set_max_ram_below_4g(Object *obj, Visitor *v, > pcms->max_ram_below_4g = value; > } > > -static bool pc_machine_get_vmport(Object *obj, Error **errp) > +static void pc_machine_get_vmport(Object *obj, Visitor *v, void *opaque, > + const char *name, Error **errp) > { > PCMachineState *pcms = PC_MACHINE(obj); > + OnOffAuto vmport = pcms->vmport; > > - return pcms->vmport; > + visit_type_OnOffAuto(v, &vmport, name, errp); > } > > -static void pc_machine_set_vmport(Object *obj, bool value, Error **errp) > +static void pc_machine_set_vmport(Object *obj, Visitor *v, void *opaque, > + const char *name, Error **errp) > { > PCMachineState *pcms = PC_MACHINE(obj); > > - pcms->vmport = value; > + visit_type_OnOffAuto(v, &pcms->vmport, name, errp); > } > > static void pc_machine_initfn(Object *obj) > @@ -1737,11 +1741,11 @@ static void pc_machine_initfn(Object *obj) > pc_machine_get_max_ram_below_4g, > pc_machine_set_max_ram_below_4g, > NULL, NULL, NULL); > - pcms->vmport = !xen_enabled(); > - object_property_add_bool(obj, PC_MACHINE_VMPORT, > - pc_machine_get_vmport, > - pc_machine_set_vmport, > - NULL); > + pcms->vmport = ON_OFF_AUTO_AUTO; > + object_property_add(obj, PC_MACHINE_VMPORT, "OnOffAuto", > + pc_machine_get_vmport, > + pc_machine_set_vmport, > + NULL, NULL, NULL); > } > > static void pc_machine_class_init(ObjectClass *oc, void *data) > diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c > index 7bb97a4..fffaab7 100644 > --- a/hw/i386/pc_piix.c > +++ b/hw/i386/pc_piix.c > @@ -234,9 +234,14 @@ static void pc_init1(MachineState *machine, > > pc_vga_init(isa_bus, pci_enabled ? pci_bus : NULL); > > + assert(pc_machine->vmport != ON_OFF_AUTO_MAX); > + if (pc_machine->vmport == ON_OFF_AUTO_AUTO) { > + pc_machine->vmport = xen_enabled() ? ON_OFF_AUTO_OFF : ON_OFF_AUTO_ON; > + } > + > /* init basic PC hardware */ > pc_basic_device_init(isa_bus, gsi, &rtc_state, &floppy, > - !pc_machine->vmport, 0x4); > + (pc_machine->vmport != ON_OFF_AUTO_ON), 0x4); > > pc_nic_init(isa_bus, pci_bus); > > diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c > index 598e679..88cee93 100644 > --- a/hw/i386/pc_q35.c > +++ b/hw/i386/pc_q35.c > @@ -242,9 +242,14 @@ static void pc_q35_init(MachineState *machine) > > pc_register_ferr_irq(gsi[13]); > > + assert(pc_machine->vmport != ON_OFF_AUTO_MAX); > + if (pc_machine->vmport == ON_OFF_AUTO_AUTO) { > + pc_machine->vmport = xen_enabled() ? ON_OFF_AUTO_OFF : ON_OFF_AUTO_ON; > + } > + > /* init basic PC hardware */ > pc_basic_device_init(isa_bus, gsi, &rtc_state, &floppy, > - !pc_machine->vmport, 0xff0104); > + (pc_machine->vmport != ON_OFF_AUTO_ON), 0xff0104); > > /* connect pm stuff to lpc */ > ich9_lpc_pm_init(lpc); > diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h > index 7c3731f..7f7d2d4 100644 > --- a/include/hw/i386/pc.h > +++ b/include/hw/i386/pc.h > @@ -37,7 +37,7 @@ struct PCMachineState { > ISADevice *rtc; > > uint64_t max_ram_below_4g; > - bool vmport; > + OnOffAuto vmport; > }; > > #define PC_MACHINE_ACPI_DEVICE_PROP "acpi-device" > diff --git a/qapi/common.json b/qapi/common.json > index 4e9a21f..63ef3b4 100644 > --- a/qapi/common.json > +++ b/qapi/common.json > @@ -87,3 +87,18 @@ > ## > { 'command': 'query-commands', 'returns': ['CommandInfo'] } > > +## > +# @OnOffAuto > +# > +# An enumeration of three options: on, off, and auto > +# > +# @auto: QEMU selects the value between on and off > +# > +# @on: Enabled > +# > +# @off: Disabled > +# > +# Since: 2.2 > +## > +{ 'enum': 'OnOffAuto', > + 'data': [ 'auto', 'on', 'off' ] } > diff --git a/qemu-options.hx b/qemu-options.hx > index da9851d..64af16d 100644 > --- a/qemu-options.hx > +++ b/qemu-options.hx > @@ -33,7 +33,7 @@ DEF("machine", HAS_ARG, QEMU_OPTION_machine, \ > " property accel=accel1[:accel2[:...]] selects accelerator\n" > " supported accelerators are kvm, xen, tcg (default: tcg)\n" > " kernel_irqchip=on|off controls accelerated irqchip support\n" > - " vmport=on|off controls emulation of vmport (default: on)\n" > + " vmport=on|off|auto controls emulation of vmport (default: auto)\n" > " 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" > @@ -52,8 +52,10 @@ than one accelerator specified, the next one is used if the previous one fails > to initialize. > @item kernel_irqchip=on|off > Enables in-kernel irqchip support for the chosen accelerator when available. > -@item vmport=on|off > -Enables emulation of VMWare IO port, for vmmouse etc. (enabled by default) > +@item vmport=on|off|auto > +Enables emulation of VMWare IO port, for vmmouse etc. auto says to select the > +value based on accel. For accel=xen the default is off otherwise the default > +is on. > @item kvm_shadow_mem=size > Defines the size of the KVM shadow MMU. > @item dump-guest-core=on|off > diff --git a/vl.c b/vl.c > index f4a6e5e..eb89d62 100644 > --- a/vl.c > +++ b/vl.c > @@ -381,7 +381,7 @@ static QemuOptsList qemu_machine_opts = { > .help = "maximum ram below the 4G boundary (32bit boundary)", > }, { > .name = PC_MACHINE_VMPORT, > - .type = QEMU_OPT_BOOL, > + .type = QEMU_OPT_STRING, > .help = "Enable vmport (pc & q35)", > },{ > .name = "iommu", > Thanks, applied. Paolo
diff --git a/hw/i386/pc.c b/hw/i386/pc.c index 1205db8..10c1fa5 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -61,6 +61,7 @@ #include "hw/mem/pc-dimm.h" #include "trace.h" #include "qapi/visitor.h" +#include "qapi-visit.h" /* debug PC/ISA interrupts */ //#define DEBUG_IRQ @@ -1711,18 +1712,21 @@ static void pc_machine_set_max_ram_below_4g(Object *obj, Visitor *v, pcms->max_ram_below_4g = value; } -static bool pc_machine_get_vmport(Object *obj, Error **errp) +static void pc_machine_get_vmport(Object *obj, Visitor *v, void *opaque, + const char *name, Error **errp) { PCMachineState *pcms = PC_MACHINE(obj); + OnOffAuto vmport = pcms->vmport; - return pcms->vmport; + visit_type_OnOffAuto(v, &vmport, name, errp); } -static void pc_machine_set_vmport(Object *obj, bool value, Error **errp) +static void pc_machine_set_vmport(Object *obj, Visitor *v, void *opaque, + const char *name, Error **errp) { PCMachineState *pcms = PC_MACHINE(obj); - pcms->vmport = value; + visit_type_OnOffAuto(v, &pcms->vmport, name, errp); } static void pc_machine_initfn(Object *obj) @@ -1737,11 +1741,11 @@ static void pc_machine_initfn(Object *obj) pc_machine_get_max_ram_below_4g, pc_machine_set_max_ram_below_4g, NULL, NULL, NULL); - pcms->vmport = !xen_enabled(); - object_property_add_bool(obj, PC_MACHINE_VMPORT, - pc_machine_get_vmport, - pc_machine_set_vmport, - NULL); + pcms->vmport = ON_OFF_AUTO_AUTO; + object_property_add(obj, PC_MACHINE_VMPORT, "OnOffAuto", + pc_machine_get_vmport, + pc_machine_set_vmport, + NULL, NULL, NULL); } static void pc_machine_class_init(ObjectClass *oc, void *data) diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c index 7bb97a4..fffaab7 100644 --- a/hw/i386/pc_piix.c +++ b/hw/i386/pc_piix.c @@ -234,9 +234,14 @@ static void pc_init1(MachineState *machine, pc_vga_init(isa_bus, pci_enabled ? pci_bus : NULL); + assert(pc_machine->vmport != ON_OFF_AUTO_MAX); + if (pc_machine->vmport == ON_OFF_AUTO_AUTO) { + pc_machine->vmport = xen_enabled() ? ON_OFF_AUTO_OFF : ON_OFF_AUTO_ON; + } + /* init basic PC hardware */ pc_basic_device_init(isa_bus, gsi, &rtc_state, &floppy, - !pc_machine->vmport, 0x4); + (pc_machine->vmport != ON_OFF_AUTO_ON), 0x4); pc_nic_init(isa_bus, pci_bus); diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c index 598e679..88cee93 100644 --- a/hw/i386/pc_q35.c +++ b/hw/i386/pc_q35.c @@ -242,9 +242,14 @@ static void pc_q35_init(MachineState *machine) pc_register_ferr_irq(gsi[13]); + assert(pc_machine->vmport != ON_OFF_AUTO_MAX); + if (pc_machine->vmport == ON_OFF_AUTO_AUTO) { + pc_machine->vmport = xen_enabled() ? ON_OFF_AUTO_OFF : ON_OFF_AUTO_ON; + } + /* init basic PC hardware */ pc_basic_device_init(isa_bus, gsi, &rtc_state, &floppy, - !pc_machine->vmport, 0xff0104); + (pc_machine->vmport != ON_OFF_AUTO_ON), 0xff0104); /* connect pm stuff to lpc */ ich9_lpc_pm_init(lpc); diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h index 7c3731f..7f7d2d4 100644 --- a/include/hw/i386/pc.h +++ b/include/hw/i386/pc.h @@ -37,7 +37,7 @@ struct PCMachineState { ISADevice *rtc; uint64_t max_ram_below_4g; - bool vmport; + OnOffAuto vmport; }; #define PC_MACHINE_ACPI_DEVICE_PROP "acpi-device" diff --git a/qapi/common.json b/qapi/common.json index 4e9a21f..63ef3b4 100644 --- a/qapi/common.json +++ b/qapi/common.json @@ -87,3 +87,18 @@ ## { 'command': 'query-commands', 'returns': ['CommandInfo'] } +## +# @OnOffAuto +# +# An enumeration of three options: on, off, and auto +# +# @auto: QEMU selects the value between on and off +# +# @on: Enabled +# +# @off: Disabled +# +# Since: 2.2 +## +{ 'enum': 'OnOffAuto', + 'data': [ 'auto', 'on', 'off' ] } diff --git a/qemu-options.hx b/qemu-options.hx index da9851d..64af16d 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -33,7 +33,7 @@ DEF("machine", HAS_ARG, QEMU_OPTION_machine, \ " property accel=accel1[:accel2[:...]] selects accelerator\n" " supported accelerators are kvm, xen, tcg (default: tcg)\n" " kernel_irqchip=on|off controls accelerated irqchip support\n" - " vmport=on|off controls emulation of vmport (default: on)\n" + " vmport=on|off|auto controls emulation of vmport (default: auto)\n" " 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" @@ -52,8 +52,10 @@ than one accelerator specified, the next one is used if the previous one fails to initialize. @item kernel_irqchip=on|off Enables in-kernel irqchip support for the chosen accelerator when available. -@item vmport=on|off -Enables emulation of VMWare IO port, for vmmouse etc. (enabled by default) +@item vmport=on|off|auto +Enables emulation of VMWare IO port, for vmmouse etc. auto says to select the +value based on accel. For accel=xen the default is off otherwise the default +is on. @item kvm_shadow_mem=size Defines the size of the KVM shadow MMU. @item dump-guest-core=on|off diff --git a/vl.c b/vl.c index f4a6e5e..eb89d62 100644 --- a/vl.c +++ b/vl.c @@ -381,7 +381,7 @@ static QemuOptsList qemu_machine_opts = { .help = "maximum ram below the 4G boundary (32bit boundary)", }, { .name = PC_MACHINE_VMPORT, - .type = QEMU_OPT_BOOL, + .type = QEMU_OPT_STRING, .help = "Enable vmport (pc & q35)", },{ .name = "iommu",