aboutsummaryrefslogtreecommitdiff
path: root/hw/i386/pc.c
AgeCommit message (Collapse)Author
2020-07-10error: Eliminate error_propagate() with Coccinelle, part 1Markus Armbruster
When all we do with an Error we receive into a local variable is propagating to somewhere else, we can just as well receive it there right away. Convert if (!foo(..., &err)) { ... error_propagate(errp, err); ... return ... } to if (!foo(..., errp)) { ... ... return ... } where nothing else needs @err. Coccinelle script: @rule1 forall@ identifier fun, err, errp, lbl; expression list args, args2; binary operator op; constant c1, c2; symbol false; @@ if ( ( - fun(args, &err, args2) + fun(args, errp, args2) | - !fun(args, &err, args2) + !fun(args, errp, args2) | - fun(args, &err, args2) op c1 + fun(args, errp, args2) op c1 ) ) { ... when != err when != lbl: when strict - error_propagate(errp, err); ... when != err ( return; | return c2; | return false; ) } @rule2 forall@ identifier fun, err, errp, lbl; expression list args, args2; expression var; binary operator op; constant c1, c2; symbol false; @@ - var = fun(args, &err, args2); + var = fun(args, errp, args2); ... when != err if ( ( var | !var | var op c1 ) ) { ... when != err when != lbl: when strict - error_propagate(errp, err); ... when != err ( return; | return c2; | return false; | return var; ) } @depends on rule1 || rule2@ identifier err; @@ - Error *err = NULL; ... when != err Not exactly elegant, I'm afraid. The "when != lbl:" is necessary to avoid transforming if (fun(args, &err)) { goto out } ... out: error_propagate(errp, err); even though other paths to label out still need the error_propagate(). For an actual example, see sclp_realize(). Without the "when strict", Coccinelle transforms vfio_msix_setup(), incorrectly. I don't know what exactly "when strict" does, only that it helps here. The match of return is narrower than what I want, but I can't figure out how to express "return where the operand doesn't use @err". For an example where it's too narrow, see vfio_intx_enable(). Silently fails to convert hw/arm/armsse.c, because Coccinelle gets confused by ARMSSE being used both as typedef and function-like macro there. Converted manually. Line breaks tidied up manually. One nested declaration of @local_err deleted manually. Preexisting unwanted blank line dropped in hw/riscv/sifive_e.c. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Message-Id: <20200707160613.848843-35-armbru@redhat.com>
2020-07-10error: Avoid unnecessary error_propagate() after error_setg()Markus Armbruster
Replace error_setg(&err, ...); error_propagate(errp, err); by error_setg(errp, ...); Related pattern: if (...) { error_setg(&err, ...); goto out; } ... out: error_propagate(errp, err); return; When all paths to label out are that way, replace by if (...) { error_setg(errp, ...); return; } and delete the label along with the error_propagate(). When we have at most one other path that actually needs to propagate, and maybe one at the end that where propagation is unnecessary, e.g. foo(..., &err); if (err) { goto out; } ... bar(..., &err); out: error_propagate(errp, err); return; move the error_propagate() to where it's needed, like if (...) { foo(..., &err); error_propagate(errp, err); return; } ... bar(..., errp); return; and transform the error_setg() as above. In some places, the transformation results in obviously unnecessary error_propagate(). The next few commits will eliminate them. Bonus: the elimination of gotos will make later patches in this series easier to review. Candidates for conversion tracked down with this Coccinelle script: @@ identifier err, errp; expression list args; @@ - error_setg(&err, args); + error_setg(errp, args); ... when != err error_propagate(errp, err); Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Message-Id: <20200707160613.848843-34-armbru@redhat.com>
2020-07-10qom: Put name parameter before value / visitor parameterMarkus Armbruster
The object_property_set_FOO() setters take property name and value in an unusual order: void object_property_set_FOO(Object *obj, FOO_TYPE value, const char *name, Error **errp) Having to pass value before name feels grating. Swap them. Same for object_property_set(), object_property_get(), and object_property_parse(). Convert callers with this Coccinelle script: @@ identifier fun = { object_property_get, object_property_parse, object_property_set_str, object_property_set_link, object_property_set_bool, object_property_set_int, object_property_set_uint, object_property_set, object_property_set_qobject }; expression obj, v, name, errp; @@ - fun(obj, v, name, errp) + fun(obj, name, v, errp) Chokes on hw/arm/musicpal.c's lcd_refresh() with the unhelpful error message "no position information". Convert that one manually. Fails to convert hw/arm/armsse.c, because Coccinelle gets confused by ARMSSE being used both as typedef and function-like macro there. Convert manually. Fails to convert hw/rx/rx-gdbsim.c, because Coccinelle gets confused by RXCPU being used both as typedef and function-like macro there. Convert manually. The other files using RXCPU that way don't need conversion. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20200707160613.848843-27-armbru@redhat.com> [Straightforwad conflict with commit 2336172d9b "audio: set default value for pcspk.iobase property" resolved]
2020-07-10qapi: Use returned bool to check for failure, Coccinelle partMarkus Armbruster
The previous commit enables conversion of visit_foo(..., &err); if (err) { ... } to if (!visit_foo(..., errp)) { ... } for visitor functions that now return true / false on success / error. Coccinelle script: @@ identifier fun =~ "check_list|input_type_enum|lv_start_struct|lv_type_bool|lv_type_int64|lv_type_str|lv_type_uint64|output_type_enum|parse_type_bool|parse_type_int64|parse_type_null|parse_type_number|parse_type_size|parse_type_str|parse_type_uint64|print_type_bool|print_type_int64|print_type_null|print_type_number|print_type_size|print_type_str|print_type_uint64|qapi_clone_start_alternate|qapi_clone_start_list|qapi_clone_start_struct|qapi_clone_type_bool|qapi_clone_type_int64|qapi_clone_type_null|qapi_clone_type_number|qapi_clone_type_str|qapi_clone_type_uint64|qapi_dealloc_start_list|qapi_dealloc_start_struct|qapi_dealloc_type_anything|qapi_dealloc_type_bool|qapi_dealloc_type_int64|qapi_dealloc_type_null|qapi_dealloc_type_number|qapi_dealloc_type_str|qapi_dealloc_type_uint64|qobject_input_check_list|qobject_input_check_struct|qobject_input_start_alternate|qobject_input_start_list|qobject_input_start_struct|qobject_input_type_any|qobject_input_type_bool|qobject_input_type_bool_keyval|qobject_input_type_int64|qobject_input_type_int64_keyval|qobject_input_type_null|qobject_input_type_number|qobject_input_type_number_keyval|qobject_input_type_size_keyval|qobject_input_type_str|qobject_input_type_str_keyval|qobject_input_type_uint64|qobject_input_type_uint64_keyval|qobject_output_start_list|qobject_output_start_struct|qobject_output_type_any|qobject_output_type_bool|qobject_output_type_int64|qobject_output_type_null|qobject_output_type_number|qobject_output_type_str|qobject_output_type_uint64|start_list|visit_check_list|visit_check_struct|visit_start_alternate|visit_start_list|visit_start_struct|visit_type_.*"; expression list args; typedef Error; Error *err; @@ - fun(args, &err); - if (err) + if (!fun(args, &err)) { ... } A few line breaks tidied up manually. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20200707160613.848843-19-armbru@redhat.com>
2020-07-08Merge remote-tracking branch ↵Peter Maydell
'remotes/kraxel/tags/audio-20200706-pull-request' into staging audio: deprecate -soundhw # gpg: Signature made Mon 06 Jul 2020 20:29:07 BST # gpg: using RSA key 4CB6D8EED3E87138 # gpg: Good signature from "Gerd Hoffmann (work) <kraxel@redhat.com>" [full] # gpg: aka "Gerd Hoffmann <gerd@kraxel.org>" [full] # gpg: aka "Gerd Hoffmann (private) <kraxel@gmail.com>" [full] # Primary key fingerprint: A032 8CFF B93A 17A7 9901 FE7D 4CB6 D8EE D3E8 7138 * remotes/kraxel/tags/audio-20200706-pull-request: audio: set default value for pcspk.iobase property pcspk: update docs/system/target-i386-desc.rst.inc audio: add soundhw deprecation notice audio: deprecate -soundhw pcspk audio: create pcspk device early audio: rework pcspk_init() softmmu: initialize spice and audio earlier pc_basic_device_init: drop no_vmport arg pc_basic_device_init: drop has_pit arg pc_basic_device_init: pass PCMachineState audio: deprecate -soundhw hda audio: deprecate -soundhw sb16 audio: deprecate -soundhw gus audio: deprecate -soundhw cs4231a audio: deprecate -soundhw adlib audio: deprecate -soundhw es1370 audio: deprecate -soundhw ac97 audio: add deprecated_register_soundhw stubs: add pci_create_simple stubs: add isa_create_simple Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
2020-07-06audio: deprecate -soundhw pcspkGerd Hoffmann
Add deprecation message to the audio init function. Factor out audio initialization and call that from both audio init and realize, so setting the audiodev property is enough to properly initialize pcspk. Add a property alias to the machine type to set the audio device, so pcspk can be initialized using: "-machine pcspk-audiodev=<name>" Using "-global isa-pcspk.audiodev=<name>" works too but is not recommended. Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> Message-id: 20200702132525.6849-18-kraxel@redhat.com
2020-07-06audio: create pcspk device earlyGerd Hoffmann
Create the pcspk device early, so it exists at machine type initialization time. Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> Message-id: 20200702132525.6849-17-kraxel@redhat.com
2020-07-06audio: rework pcspk_init()Gerd Hoffmann
Instead of creating and returning the pc speaker accept it as argument. That allows to rework the initialization workflow in followup patches. Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> Message-id: 20200702132525.6849-16-kraxel@redhat.com
2020-07-06pc_basic_device_init: drop no_vmport argGerd Hoffmann
Now that we pass pcms anyway, we don't need the no_vmport arg any more. No functional change. Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> Message-id: 20200702132525.6849-14-kraxel@redhat.com
2020-07-06pc_basic_device_init: drop has_pit argGerd Hoffmann
Now that we pass pcms anyway, we don't need the has_pit arg any more. No functional change. Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> Message-id: 20200702132525.6849-13-kraxel@redhat.com
2020-07-06pc_basic_device_init: pass PCMachineStateGerd Hoffmann
Need access to pcms for pcspk initialization. Just preparation, no functional change. Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> Message-id: 20200702132525.6849-12-kraxel@redhat.com
2020-07-03numa: Auto-enable NUMA when any memory devices are possibleDavid Hildenbrand
Let's auto-enable it also when maxmem is specified but no slots are defined. This will result in us properly creating ACPI srat tables, indicating the maximum possible PFN to the guest OS. Based on this, e.g., Linux will enable the swiotlb properly. This avoids having to manually force the switolb on (swiotlb=force) in Linux in case we're booting only using DMA memory (e.g., 2GB on x86-64), and virtio-mem adds memory later on that really needs the swiotlb to be used for DMA. Let's take care of backwards compatibility if somebody has a setup that specifies "maxram" without "slots". Reported-by: Alex Shi <alex.shi@linux.alibaba.com> Cc: Peter Maydell <peter.maydell@linaro.org> Cc: Eduardo Habkost <ehabkost@redhat.com> Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com> Cc: Sergio Lopez <slp@redhat.com> Cc: Paolo Bonzini <pbonzini@redhat.com> Cc: Richard Henderson <rth@twiddle.net> Cc: "Michael S. Tsirkin" <mst@redhat.com> Cc: Igor Mammedov <imammedo@redhat.com> Cc: qemu-arm@nongnu.org <qemu-arm@nongnu.org> Signed-off-by: David Hildenbrand <david@redhat.com> Message-Id: <20200626072248.78761-22-david@redhat.com> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
2020-07-03pc: Support for virtio-mem-pciDavid Hildenbrand
Let's wire it up similar to virtio-pmem. Also disallow unplug, so it's harder for users to shoot themselves into the foot. Reviewed-by: Pankaj Gupta <pankaj.gupta.linux@gmail.com> Cc: "Michael S. Tsirkin" <mst@redhat.com> Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com> Cc: Paolo Bonzini <pbonzini@redhat.com> Cc: Richard Henderson <rth@twiddle.net> Cc: Eduardo Habkost <ehabkost@redhat.com> Cc: Eric Blake <eblake@redhat.com> Cc: Markus Armbruster <armbru@redhat.com> Signed-off-by: David Hildenbrand <david@redhat.com> Message-Id: <20200626072248.78761-16-david@redhat.com> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
2020-07-02pc: Support coldplugging of virtio-pmem-pci devices on all busesDavid Hildenbrand
E.g., with "pc-q35-4.2", trying to coldplug a virtio-pmem-pci devices results in "virtio-pmem-pci not supported on this bus" Reasons is, that the bus does not support hotplug and, therefore, does not have a hotplug handler. Let's allow coldplugging virtio-pmem devices on such buses. The hotplug order is only relevant for virtio-pmem-pci when the guest is already alive and the device is visible before memory_device_plug() wired up the memory device bits. Hotplug attempts will still fail with: "Error: Bus 'pcie.0' does not support hotplugging" Hotunplug attempts will still fail with: "Error: Bus 'pcie.0' does not support hotplugging" Reported-by: Vivek Goyal <vgoyal@redhat.com> Reviewed-by: Pankaj Gupta <pankaj.gupta.linux@gmail.com> Cc: Pankaj Gupta <pankaj.gupta.linux@gmail.com> Cc: Igor Mammedov <imammedo@redhat.com> Cc: Paolo Bonzini <pbonzini@redhat.com> Cc: Richard Henderson <rth@twiddle.net> Cc: Eduardo Habkost <ehabkost@redhat.com> Cc: "Michael S. Tsirkin" <mst@redhat.com> Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com> Signed-off-by: David Hildenbrand <david@redhat.com> Message-Id: <20200626072248.78761-2-david@redhat.com> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
2020-06-26numa: forbid '-numa node, mem' for 5.1 and newer machine typesIgor Mammedov
Deprecation period is run out and it's a time to flip the switch introduced by cd5ff8333a. Disable legacy option for new machine types (since 5.1) and amend documentation. '-numa node,memdev' shall be used instead of disabled option with new machine types. Signed-off-by: Igor Mammedov <imammedo@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> Reviewed-by: Greg Kurz <groug@kaod.org> Message-Id: <20200609135635.761587-1-imammedo@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2020-06-25Merge remote-tracking branch 'remotes/mst/tags/for_upstream' into stagingPeter Maydell
virtio,acpi,pci: fixes, cleanups. Fixes, cleanups in ACPI, PCI, virtio. Signed-off-by: Michael S. Tsirkin <mst@redhat.com> # gpg: Signature made Thu 25 Jun 2020 07:48:47 BST # gpg: using RSA key 5D09FD0871C8F85B94CA8A0D281F0DB8D28D5469 # gpg: issuer "mst@redhat.com" # gpg: Good signature from "Michael S. Tsirkin <mst@kernel.org>" [full] # gpg: aka "Michael S. Tsirkin <mst@redhat.com>" [full] # Primary key fingerprint: 0270 606B 6F3C DF3D 0B17 0970 C350 3912 AFBE 8E67 # Subkey fingerprint: 5D09 FD08 71C8 F85B 94CA 8A0D 281F 0DB8 D28D 5469 * remotes/mst/tags/for_upstream: Rename use_acpi_pci_hotplug to more appropriate use_acpi_hotplug_bridge Stop vhost-user sending uninitialized mmap_offsets docs/specs/tpm: ACPI boot now supported for TPM/ARM arm/acpi: Add the TPM2.0 device under the DSDT acpi: Some build_tpm2() code reshape tests/acpi: update expected data files acpi: q35: drop _SB.PCI0.ISA.LPCD opregion. acpi: drop build_piix4_pm() acpi: drop serial/parallel enable bits from dsdt acpi: simplify build_isa_devices_aml() acpi: factor out fw_cfg_add_acpi_dsdt() acpi: move aml builder code for i8042 (kbd+mouse) device floppy: move cmos_get_fd_drive_type() from pc floppy: make isa_fdc_get_drive_max_chs static acpi: move aml builder code for floppy device acpi: bios-tables-test: show more context on asl diffs qtest: allow DSDT acpi table changes Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
2020-06-24floppy: move cmos_get_fd_drive_type() from pcGerd Hoffmann
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Acked-by: John Snow <jsnow@redhat.com> Message-Id: <20200619091905.21676-6-kraxel@redhat.com> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
2020-06-23fdc: Open-code fdctrl_init_isa()Markus Armbruster
Helper function fdctrl_init_isa() is less than helpful: one of three places creating "isa-fdc" devices use it. Open-code it there, and drop the function. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Message-Id: <20200622094227.1271650-6-armbru@redhat.com>
2020-06-17x86: move max-ram-below-4g to pcGerd Hoffmann
Move from X86MachineClass to PCMachineClass so it disappears from microvm machine type property list. Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Acked-by: Paolo Bonzini <pbonzini@redhat.com> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> Reviewed-by: Igor Mammedov <imammedo@redhat.com> Reviewed-by: Paul Durrant <paul@xen.org> Message-id: 20200529073957.8018-4-kraxel@redhat.com
2020-06-15sysbus: Convert to sysbus_realize() etc. with CoccinelleMarkus Armbruster
Convert from qdev_realize(), qdev_realize_and_unref() with null @bus argument to sysbus_realize(), sysbus_realize_and_unref(). Coccinelle script: @@ expression dev, errp; @@ - qdev_realize(DEVICE(dev), NULL, errp); + sysbus_realize(SYS_BUS_DEVICE(dev), errp); @@ expression sysbus_dev, dev, errp; @@ + sysbus_dev = SYS_BUS_DEVICE(dev); - qdev_realize_and_unref(dev, NULL, errp); + sysbus_realize_and_unref(sysbus_dev, errp); - sysbus_dev = SYS_BUS_DEVICE(dev); @@ expression sysbus_dev, dev, errp; expression expr; @@ sysbus_dev = SYS_BUS_DEVICE(dev); ... when != dev = expr; - qdev_realize_and_unref(dev, NULL, errp); + sysbus_realize_and_unref(sysbus_dev, errp); @@ expression dev, errp; @@ - qdev_realize_and_unref(DEVICE(dev), NULL, errp); + sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), errp); @@ expression dev, errp; @@ - qdev_realize_and_unref(dev, NULL, errp); + sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), errp); Whitespace changes minimized manually. Signed-off-by: Markus Armbruster <armbru@redhat.com> Acked-by: Alistair Francis <alistair.francis@wdc.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com> Message-Id: <20200610053247.1583243-46-armbru@redhat.com> [Conflicts in hw/misc/empty_slot.c and hw/sparc/leon3.c resolved]
2020-06-15isa: Convert uses of isa_create(), isa_try_create() manuallyMarkus Armbruster
Same transformation as in the previous commit. Manual, because convincing Coccinelle to transform these cases is not worthwhile. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com> Message-Id: <20200610053247.1583243-21-armbru@redhat.com>
2020-06-15qdev: Convert uses of qdev_create() manuallyMarkus Armbruster
Same transformation as in the previous commit. Manual, because convincing Coccinelle to transform these cases is somewhere between not worthwhile and infeasible (at least for me). Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com> Message-Id: <20200610053247.1583243-11-armbru@redhat.com>
2020-06-15qdev: Convert to qdev_unrealize() with CoccinelleMarkus Armbruster
For readability, and consistency with qbus_realize(). Coccinelle script: @ depends on !(file in "hw/core/qdev.c")@ typedef DeviceState; DeviceState *dev; symbol false, error_abort; @@ - object_property_set_bool(OBJECT(dev), false, "realized", &error_abort); + qdev_unrealize(dev); @ depends on !(file in "hw/core/qdev.c") && !(file in "hw/core/bus.c")@ expression dev; symbol false, error_abort; @@ - object_property_set_bool(OBJECT(dev), false, "realized", &error_abort); + qdev_unrealize(DEVICE(dev)); Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com> Message-Id: <20200610053247.1583243-8-armbru@redhat.com>
2020-06-10accel: Move Xen accelerator code under accel/xen/Philippe Mathieu-Daudé
This code is not related to hardware emulation. Move it under accel/ with the other hypervisors. Reviewed-by: Paul Durrant <paul@xen.org> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> Message-Id: <20200508100222.7112-1-philmd@redhat.com> Reviewed-by: Juan Quintela <quintela@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2020-06-10hw/i386/vmport: Introduce vmport.hLiran Alon
No functional change. This is mere refactoring. Suggested-by: Michael S. Tsirkin <mst@redhat.com> Signed-off-by: Liran Alon <liran.alon@oracle.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Message-Id: <20200312165431.82118-8-liran.alon@oracle.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2020-05-15qdev: Unrealize must not failMarkus Armbruster
Devices may have component devices and buses. Device realization may fail. Realization is recursive: a device's realize() method realizes its components, and device_set_realized() realizes its buses (which should in turn realize the devices on that bus, except bus_set_realized() doesn't implement that, yet). When realization of a component or bus fails, we need to roll back: unrealize everything we realized so far. If any of these unrealizes failed, the device would be left in an inconsistent state. Must not happen. device_set_realized() lets it happen: it ignores errors in the roll back code starting at label child_realize_fail. Since realization is recursive, unrealization must be recursive, too. But how could a partly failed unrealize be rolled back? We'd have to re-realize, which can fail. This design is fundamentally broken. device_set_realized() does not roll back at all. Instead, it keeps unrealizing, ignoring further errors. It can screw up even for a device with no buses: if the lone dc->unrealize() fails, it still unregisters vmstate, and calls listeners' unrealize() callback. bus_set_realized() does not roll back either. Instead, it stops unrealizing. Fortunately, no unrealize method can fail, as we'll see below. To fix the design error, drop parameter @errp from all the unrealize methods. Any unrealize method that uses @errp now needs an update. This leads us to unrealize() methods that can fail. Merely passing it to another unrealize method cannot cause failure, though. Here are the ones that do other things with @errp: * virtio_serial_device_unrealize() Fails when qbus_set_hotplug_handler() fails, but still does all the other work. On failure, the device would stay realized with its resources completely gone. Oops. Can't happen, because qbus_set_hotplug_handler() can't actually fail here. Pass &error_abort to qbus_set_hotplug_handler() instead. * hw/ppc/spapr_drc.c's unrealize() Fails when object_property_del() fails, but all the other work is already done. On failure, the device would stay realized with its vmstate registration gone. Oops. Can't happen, because object_property_del() can't actually fail here. Pass &error_abort to object_property_del() instead. * spapr_phb_unrealize() Fails and bails out when remove_drcs() fails, but other work is already done. On failure, the device would stay realized with some of its resources gone. Oops. remove_drcs() fails only when chassis_from_bus()'s object_property_get_uint() fails, and it can't here. Pass &error_abort to remove_drcs() instead. Therefore, no unrealize method can fail before this patch. device_set_realized()'s recursive unrealization via bus uses object_property_set_bool(). Can't drop @errp there, so pass &error_abort. We similarly unrealize with object_property_set_bool() elsewhere, always ignoring errors. Pass &error_abort instead. Several unrealize methods no longer handle errors from other unrealize methods: virtio_9p_device_unrealize(), virtio_input_device_unrealize(), scsi_qdev_unrealize(), ... Much of the deleted error handling looks wrong anyway. One unrealize methods no longer ignore such errors: usb_ehci_pci_exit(). Several realize methods no longer ignore errors when rolling back: v9fs_device_realize_common(), pci_qdev_unrealize(), spapr_phb_realize(), usb_qdev_realize(), vfio_ccw_realize(), virtio_device_realize(). Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com> Message-Id: <20200505152926.18877-17-armbru@redhat.com>
2020-05-15qom: Drop parameter @errp of object_property_add() & friendsMarkus Armbruster
The only way object_property_add() can fail is when a property with the same name already exists. Since our property names are all hardcoded, failure is a programming error, and the appropriate way to handle it is passing &error_abort. Same for its variants, except for object_property_add_child(), which additionally fails when the child already has a parent. Parentage is also under program control, so this is a programming error, too. We have a bit over 500 callers. Almost half of them pass &error_abort, slightly fewer ignore errors, one test case handles errors, and the remaining few callers pass them to their own callers. The previous few commits demonstrated once again that ignoring programming errors is a bad idea. Of the few ones that pass on errors, several violate the Error API. The Error ** argument must be NULL, &error_abort, &error_fatal, or a pointer to a variable containing NULL. Passing an argument of the latter kind twice without clearing it in between is wrong: if the first call sets an error, it no longer points to NULL for the second call. ich9_pm_add_properties(), sparc32_ledma_realize(), sparc32_dma_realize(), xilinx_axidma_realize(), xilinx_enet_realize() are wrong that way. When the one appropriate choice of argument is &error_abort, letting users pick the argument is a bad idea. Drop parameter @errp and assert the preconditions instead. There's one exception to "duplicate property name is a programming error": the way object_property_add() implements the magic (and undocumented) "automatic arrayification". Don't drop @errp there. Instead, rename object_property_add() to object_property_try_add(), and add the obvious wrapper object_property_add(). Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com> Message-Id: <20200505152926.18877-15-armbru@redhat.com> [Two semantic rebase conflicts resolved]
2020-05-15qom: Drop object_property_set_description() parameter @errpMarkus Armbruster
object_property_set_description() and object_class_property_set_description() fail only when property @name is not found. There are 85 calls of object_property_set_description() and object_class_property_set_description(). None of them can fail: * 84 immediately follow the creation of the property. * The one in spapr_rng_instance_init() refers to a property created in spapr_rng_class_init(), from spapr_rng_properties[]. Every one of them still gets to decide what to pass for @errp. 51 calls pass &error_abort, 32 calls pass NULL, one receives the error and propagates it to &error_abort, and one propagates it to &error_fatal. I'm actually surprised none of them violates the Error API. What are we gaining by letting callers handle the "property not found" error? Use when the property is not known to exist is simpler: you don't have to guard the call with a check. We haven't found such a use in 5+ years. Until we do, let's make life a bit simpler and drop the @errp parameter. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com> Message-Id: <20200505152926.18877-8-armbru@redhat.com> [One semantic rebase conflict resolved]
2020-05-06hw: add compat machines for 5.1Cornelia Huck
Add 5.1 machine types for arm/i440fx/q35/s390x/spapr. Acked-by: David Gibson <david@gibson.dropbear.id.au> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> Signed-off-by: Cornelia Huck <cohuck@redhat.com> Message-id: 20200429144605.7262-1-cohuck@redhat.com
2020-05-04hw/i386: Add 'vmport.h' local headerPhilippe Mathieu-Daudé
Move 'vmport' related declarations in a target-specific header. Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Message-Id: <20200504083342.24273-4-f4bug@amsat.org> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
2020-05-04hw/i386/pc: Create 'vmport' device in placePhilippe Mathieu-Daudé
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Message-Id: <20200504083342.24273-2-f4bug@amsat.org> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
2020-03-31i386: Fix pkg_id offset for EPYC cpu modelsBabu Moger
If the system is numa configured the pkg_offset needs to be adjusted for EPYC cpu models. Fix it calling the model specific handler. Signed-off-by: Babu Moger <babu.moger@amd.com> Reviewed-by: Igor Mammedov <imammedo@redhat.com> Acked-by: Michael S. Tsirkin <mst@redhat.com> Message-Id: <158396725589.58170.16424607815207074485.stgit@naples-babu.amd.com> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
2020-03-31hw/i386: Move arch_id decode inside x86_cpus_initBabu Moger
Apicid calculation depends on knowing the total number of numa nodes for EPYC cpu models. Right now, we are calculating the arch_id while parsing the numa(parse_numa). At this time, it is not known how many total numa nodes are configured in the system. Move the arch_id calculation inside x86_cpus_init. At this time, smp parse is already completed and numa node information is available. Override the handlers if use_epyc_apic_id_encoding is enabled in cpu model definition. Also replace the calling convention to use handlers from X86MachineState. Signed-off-by: Babu Moger <babu.moger@amd.com> Message-Id: <158396724217.58170.12256158354204870716.stgit@naples-babu.amd.com> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
2020-03-29acpi: add acpi=OnOffAuto machine property to x86 and arm virtGerd Hoffmann
Remove the global acpi_enabled bool and replace it with an acpi OnOffAuto machine property. qemu throws an error now if you use -no-acpi while the machine type you are using doesn't support acpi in the first place. Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> Message-Id: <20200320100136.11717-1-kraxel@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Acked-by: Paolo Bonzini <pbonzini@redhat.com> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
2020-03-17hw/i386: Rename apicid_from_topo_ids to x86_apicid_from_topo_idsBabu Moger
For consistency rename apicid_from_topo_ids to x86_apicid_from_topo_ids. No functional change. Signed-off-by: Babu Moger <babu.moger@amd.com> Reviewed-by: Igor Mammedov <imammedo@redhat.com> Acked-by: Michael S. Tsirkin <mst@redhat.com> Message-Id: <158396720748.58170.5335409429390890145.stgit@naples-babu.amd.com>
2020-03-17hw/i386: Update structures to save the number of nodes per packageBabu Moger
Update structures X86CPUTopoIDs and CPUX86State to hold the number of nodes per package. This is required to build EPYC mode topology. Signed-off-by: Babu Moger <babu.moger@amd.com> Reviewed-by: Igor Mammedov <imammedo@redhat.com> Acked-by: Michael S. Tsirkin <mst@redhat.com> Message-Id: <158396720035.58170.1973738805301006456.stgit@naples-babu.amd.com>
2020-03-17machine: Add SMP Sockets in CpuTopologyBabu Moger
Store the smp sockets in CpuTopology. The socket information required to build the apic id in EPYC mode. Right now socket information is not passed to down when decoding the apic id. Add the socket information here. Signed-off-by: Babu Moger <babu.moger@amd.com> Reviewed-by: Eduardo Habkost <ehabkost@redhat.com> Reviewed-by: Igor Mammedov <imammedo@redhat.com> Acked-by: Michael S. Tsirkin <mst@redhat.com> Message-Id: <158396718647.58170.2278448323151215741.stgit@naples-babu.amd.com>
2020-03-17hw/i386: Introduce X86CPUTopoInfo to contain topology infoBabu Moger
This is an effort to re-arrange few data structure for better readability. 1. Add X86CPUTopoInfo which will have all the topology informations required to build the cpu topology. There is no functional changes. 2. Introduce init_topo_info to initialize X86CPUTopoInfo members from X86MachineState. 3. Update x86 unit tests for new calling convention with parameter X86CPUTopoInfo There is no functional changes. Signed-off-by: Babu Moger <babu.moger@amd.com> Message-Id: <158396717251.58170.4499717831243474938.stgit@naples-babu.amd.com>
2020-03-17hw/i386: Rename X86CPUTopoInfo structure to X86CPUTopoIDsBabu Moger
Rename few data structures related to X86 topology. X86CPUTopoIDs will have individual arch ids. Next patch introduces X86CPUTopoInfo which will have all topology information(like cores, threads etc..). Signed-off-by: Babu Moger <babu.moger@amd.com> Reviewed-by: Eduardo Habkost <ehabkost@redhat.com> Message-Id: <158326541877.40452.17535023236841538507.stgit@naples-babu.amd.com> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
2020-03-09hw/i386: Include "hw/mem/nvdimm.h"Philippe Mathieu-Daudé
All this files use methods/definitions declared in the NVDIMM device header. Include it. This fixes (when modifying unrelated headers): hw/i386/acpi-build.c:2733:9: error: implicit declaration of function 'nvdimm_build_acpi' is invalid in C99 [-Werror,-Wimplicit-function-declaration] nvdimm_build_acpi(table_offsets, tables_blob, tables->linker, ^ hw/i386/pc.c:1996:61: error: use of undeclared identifier 'TYPE_NVDIMM' const bool is_nvdimm = object_dynamic_cast(OBJECT(dev), TYPE_NVDIMM); ^ hw/i386/pc.c:2032:55: error: use of undeclared identifier 'TYPE_NVDIMM' bool is_nvdimm = object_dynamic_cast(OBJECT(dev), TYPE_NVDIMM); ^ hw/i386/pc.c:2040:9: error: implicit declaration of function 'nvdimm_plug' is invalid in C99 [-Werror,-Wimplicit-function-declaration] nvdimm_plug(ms->nvdimms_state); ^ hw/i386/pc.c:2040:9: error: this function declaration is not a prototype [-Werror,-Wstrict-prototypes] nvdimm_plug(ms->nvdimms_state); ^ hw/i386/pc.c:2065:42: error: use of undeclared identifier 'TYPE_NVDIMM' if (object_dynamic_cast(OBJECT(dev), TYPE_NVDIMM)) { ^ hw/i386/pc_i440fx.c:307:9: error: implicit declaration of function 'nvdimm_init_acpi_state' is invalid in C99 [-Werror,-Wimplicit-function-declaration] nvdimm_init_acpi_state(machine->nvdimms_state, system_io, ^ hw/i386/pc_q35.c:332:9: error: implicit declaration of function 'nvdimm_init_acpi_state' is invalid in C99 [-Werror,-Wimplicit-function-declaration] nvdimm_init_acpi_state(machine->nvdimms_state, system_io, ^ Acked-by: John Snow <jsnow@redhat.com> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-Id: <20200228114649.12818-17-philmd@redhat.com> Signed-off-by: Laurent Vivier <laurent@vivier.eu>
2020-02-19x86/pc: use memdev for RAMIgor Mammedov
memory_region_allocate_system_memory() API is going away, so replace it with memdev allocated MemoryRegion. The later is initialized by generic code, so board only needs to opt in to memdev scheme by providing MachineClass::default_ram_id and using MachineState::ram instead of manually initializing RAM memory region. Signed-off-by: Igor Mammedov <imammedo@redhat.com> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-Id: <20200219160953.13771-44-imammedo@redhat.com>
2020-01-30add device_legacy_reset function to prepare for reset api changeDamien Hedde
Provide a temporary device_legacy_reset function doing what device_reset does to prepare for the transition with Resettable API. All occurrence of device_reset in the code tree are also replaced by device_legacy_reset. The new resettable API has different prototype and semantics (resetting child buses as well as the specified device). Subsequent commits will make the changeover for each call site individually; once that is complete device_legacy_reset() will be removed. Signed-off-by: Damien Hedde <damien.hedde@greensocs.com> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Acked-by: David Gibson <david@gibson.dropbear.id.au> Acked-by: Cornelia Huck <cohuck@redhat.com> Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Message-id: 20200123132823.1117486-2-damien.hedde@greensocs.com Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
2020-01-22q35: implement 128K SMRAM at default SMBASE addressIgor Mammedov
It's not what real HW does, implementing which would be overkill [**] and would require complex cross stack changes (QEMU+firmware) to make it work. So considering that SMRAM is owned by MCH, for simplicity (ab)use reserved Q35 register, which allows QEMU and firmware easily init and make RAM at SMBASE available only from SMM context. Patch uses commit (2f295167e0 q35/mch: implement extended TSEG sizes) for inspiration and uses reserved register in config space at 0x9c offset [*] to extend q35 pci-host with ability to use 128K at 0x30000 as SMRAM and hide it (like TSEG) from non-SMM context. Usage: 1: write 0xff in the register 2: if the feature is supported, follow up read from the register should return 0x01. At this point RAM at 0x30000 is still available for SMI handler configuration from non-SMM context 3: writing 0x02 in the register, locks SMBASE area, making its contents available only from SMM context. In non-SMM context, reads return 0xff and writes are ignored. Further writes into the register are ignored until the system reset. *) https://www.mail-archive.com/qemu-devel@nongnu.org/msg455991.html **) https://www.mail-archive.com/qemu-devel@nongnu.org/msg646965.html Signed-off-by: Igor Mammedov <imammedo@redhat.com> Message-Id: <1575896942-331151-3-git-send-email-imammedo@redhat.com> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> Signed-off-by: Michael S. Tsirkin <mst@redhat.com> Tested-by: Laszlo Ersek <lersek@redhat.com>
2020-01-07vmmouse: replace PROP_PTR with PROP_LINKMarc-André Lureau
While at it, use the expected type. Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
2020-01-07sysbus: remove unused sysbus_try_create*Marc-André Lureau
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
2019-12-17hw/i386/pc: Extract the port92 devicePhilippe Mathieu-Daudé
This device is only used by the PC machines. The pc.c file is already big enough, with 2255 lines. By removing 113 lines of it, we reduced it by 5%. It is now a bit easier to navigate the file. Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2019-12-17hw/i386/pc: Inline port92_init()Philippe Mathieu-Daudé
This one-line function is not very helpful, so remove it by inlining the call to qdev_connect_gpio_out_named(). Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2019-12-17hw/i386/pc: Use TYPE_PORT92 instead of hardcoded stringPhilippe Mathieu-Daudé
By using the TYPE_* definitions for devices, we can: - quickly find where devices are used with 'git-grep' - easily rename a device (one-line change). Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2019-12-17hw/i386: De-duplicate gsi_handler() to remove kvm_pc_gsi_handler()Philippe Mathieu-Daudé
Both gsi_handler() and kvm_pc_gsi_handler() have the same content, except one comment. Move the comment, and de-duplicate the code. Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2019-12-17x86: move more x86-generic functions out of PC filesPaolo Bonzini
These are needed by microvm too, so move them outside of PC-specific files. With this patch, microvm.c need not include pc.h anymore. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>