From 443df40cad0f5de602ac6f6ca1559e0923e180b3 Mon Sep 17 00:00:00 2001 From: Maksim Davydov Date: Tue, 19 Mar 2024 00:35:47 +0300 Subject: [PATCH 01/22] qom: add default value MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit qmp_qom_list_properties can print default values if they are available as qmp_device_list_properties does, because both of them use the ObjectPropertyInfo structure with default_value field. This can be useful when working with "not device" types (e.g. memory-backend). Signed-off-by: Maksim Davydov Reviewed-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Markus Armbruster Message-ID: <20240318213550.155573-2-davydov-max@yandex-team.ru> Signed-off-by: Philippe Mathieu-Daudé --- qom/qom-qmp-cmds.c | 1 + 1 file changed, 1 insertion(+) diff --git a/qom/qom-qmp-cmds.c b/qom/qom-qmp-cmds.c index 7c087299de..e91a235347 100644 --- a/qom/qom-qmp-cmds.c +++ b/qom/qom-qmp-cmds.c @@ -212,6 +212,7 @@ ObjectPropertyInfoList *qmp_qom_list_properties(const char *typename, info->name = g_strdup(prop->name); info->type = g_strdup(prop->type); info->description = g_strdup(prop->description); + info->default_value = qobject_ref(prop->defval); QAPI_LIST_PREPEND(prop_list, info); } From 236e9397b320518372a67c55777193c032b93d89 Mon Sep 17 00:00:00 2001 From: Maksim Davydov Date: Tue, 19 Mar 2024 00:35:48 +0300 Subject: [PATCH 02/22] qmp: add dump machine type compatibility properties MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit To control that creating new machine type doesn't affect the previous types (their compat_props) and to check complex compat_props inheritance we need qmp command to print machine type compatibility properties. This patch adds the ability to get list of all the compat_props of the corresponding supported machines for their comparison via new optional argument of "query-machines" command. Since information on compatibility properties can increase the command output by a factor of 40, add an argument to enable it, default off. Signed-off-by: Maksim Davydov Reviewed-by: Vladimir Sementsov-Ogievskiy Acked-by: Markus Armbruster Message-ID: <20240318213550.155573-3-davydov-max@yandex-team.ru> Signed-off-by: Philippe Mathieu-Daudé --- hw/core/machine-qmp-cmds.c | 23 ++++++++++++- qapi/machine.json | 67 +++++++++++++++++++++++++++++++++++-- tests/qtest/fuzz/qos_fuzz.c | 2 +- 3 files changed, 88 insertions(+), 4 deletions(-) diff --git a/hw/core/machine-qmp-cmds.c b/hw/core/machine-qmp-cmds.c index c20829b9ae..5972100b1f 100644 --- a/hw/core/machine-qmp-cmds.c +++ b/hw/core/machine-qmp-cmds.c @@ -64,7 +64,8 @@ CpuInfoFastList *qmp_query_cpus_fast(Error **errp) return head; } -MachineInfoList *qmp_query_machines(Error **errp) +MachineInfoList *qmp_query_machines(bool has_compat_props, bool compat_props, + Error **errp) { GSList *el, *machines = object_class_get_list(TYPE_MACHINE, false); MachineInfoList *mach_list = NULL; @@ -96,6 +97,26 @@ MachineInfoList *qmp_query_machines(Error **errp) info->default_ram_id = g_strdup(mc->default_ram_id); } + if (compat_props && mc->compat_props) { + int i; + info->compat_props = NULL; + CompatPropertyList **tail = &(info->compat_props); + info->has_compat_props = true; + + for (i = 0; i < mc->compat_props->len; i++) { + GlobalProperty *mt_prop = g_ptr_array_index(mc->compat_props, + i); + CompatProperty *prop; + + prop = g_malloc0(sizeof(*prop)); + prop->qom_type = g_strdup(mt_prop->driver); + prop->property = g_strdup(mt_prop->property); + prop->value = g_strdup(mt_prop->value); + + QAPI_LIST_APPEND(tail, prop); + } + } + QAPI_LIST_PREPEND(mach_list, info); } diff --git a/qapi/machine.json b/qapi/machine.json index 2df407e877..3e9cc3f17d 100644 --- a/qapi/machine.json +++ b/qapi/machine.json @@ -135,6 +135,26 @@ ## { 'command': 'query-cpus-fast', 'returns': [ 'CpuInfoFast' ] } +## +# @CompatProperty: +# +# Property default values specific to a machine type, for use by +# scripts/compare-machine-types. +# +# @qom-type: name of the QOM type to which the default applies +# +# @property: name of its property to which the default applies +# +# @value: the default value (machine-specific default can overwrite +# the "default" default, to avoid this use -machine none) +# +# Since: 9.1 +## +{ 'struct': 'CompatProperty', + 'data': { 'qom-type': 'str', + 'property': 'str', + 'value': 'str' } } + ## # @MachineInfo: # @@ -166,6 +186,14 @@ # # @acpi: machine type supports ACPI (since 8.0) # +# @compat-props: The machine type's compatibility properties. Only +# present when query-machines argument @compat-props is true. +# (since 9.1) +# +# Features: +# +# @unstable: Member @compat-props is experimental. +# # Since: 1.2 ## { 'struct': 'MachineInfo', @@ -173,18 +201,53 @@ '*is-default': 'bool', 'cpu-max': 'int', 'hotpluggable-cpus': 'bool', 'numa-mem-supported': 'bool', 'deprecated': 'bool', '*default-cpu-type': 'str', - '*default-ram-id': 'str', 'acpi': 'bool' } } + '*default-ram-id': 'str', 'acpi': 'bool', + '*compat-props': { 'type': ['CompatProperty'], + 'features': ['unstable'] } } } ## # @query-machines: # # Return a list of supported machines # +# @compat-props: if true, also return compatibility properties. +# (default: false) (since 9.1) +# +# Features: +# +# @unstable: Argument @compat-props is experimental. +# # Returns: a list of MachineInfo # # Since: 1.2 +# +# Example: +# +# -> { "execute": "query-machines", "arguments": { "compat-props": true } } +# <- { "return": [ +# { +# "hotpluggable-cpus": true, +# "name": "pc-q35-6.2", +# "compat-props": [ +# { +# "qom-type": "virtio-mem", +# "property": "unplugged-inaccessible", +# "value": "off" +# } +# ], +# "numa-mem-supported": false, +# "default-cpu-type": "qemu64-x86_64-cpu", +# "cpu-max": 288, +# "deprecated": false, +# "default-ram-id": "pc.ram" +# }, +# ... +# } ## -{ 'command': 'query-machines', 'returns': ['MachineInfo'] } +{ 'command': 'query-machines', + 'data': { '*compat-props': { 'type': 'bool', + 'features': [ 'unstable' ] } }, + 'returns': ['MachineInfo'] } ## # @CurrentMachineParams: diff --git a/tests/qtest/fuzz/qos_fuzz.c b/tests/qtest/fuzz/qos_fuzz.c index e403d373a0..b71e945c5f 100644 --- a/tests/qtest/fuzz/qos_fuzz.c +++ b/tests/qtest/fuzz/qos_fuzz.c @@ -46,7 +46,7 @@ static void qos_set_machines_devices_available(void) MachineInfoList *mach_info; ObjectTypeInfoList *type_info; - mach_info = qmp_query_machines(&error_abort); + mach_info = qmp_query_machines(false, false, &error_abort); machines_apply_to_node(mach_info); qapi_free_MachineInfoList(mach_info); From 33956e476802a6ae9b9a8e047f6a78e09e9ae180 Mon Sep 17 00:00:00 2001 From: Maksim Davydov Date: Tue, 19 Mar 2024 00:35:49 +0300 Subject: [PATCH 03/22] python/qemu/machine: add method to retrieve QEMUMachine::binary field MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add a supportive property to access the path to the QEMU binary Signed-off-by: Maksim Davydov Reviewed-by: John Snow Reviewed-by: Philippe Mathieu-Daudé Message-ID: <20240318213550.155573-4-davydov-max@yandex-team.ru> Signed-off-by: Philippe Mathieu-Daudé --- python/qemu/machine/machine.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/python/qemu/machine/machine.py b/python/qemu/machine/machine.py index 31cb9d617d..f648f6af45 100644 --- a/python/qemu/machine/machine.py +++ b/python/qemu/machine/machine.py @@ -328,6 +328,11 @@ def args(self) -> List[str]: """Returns the list of arguments given to the QEMU binary.""" return self._args + @property + def binary(self) -> str: + """Returns path to the QEMU binary""" + return self._binary + def _pre_launch(self) -> None: if self._qmp_set: if self._monitor_address is None: From b928505d39b634cdd3ba27b94561e6aadcd677af Mon Sep 17 00:00:00 2001 From: Maksim Davydov Date: Tue, 19 Mar 2024 00:35:50 +0300 Subject: [PATCH 04/22] scripts: add script to compare compatibility properties MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This script runs QEMU to obtain compat_props of machines and default values of different types of drivers to produce comparison table. This table can be used to compare machine types to choose the most suitable machine or compare binaries to be sure that migration to the newer version will save all device properties. Also the json or csv format of this table can be used to check does a new machine affect the previous ones by comparing tables with and without the new machine. Default values (that will be used without machine compat_props) of properties are needed to fill "holes" in the table (one machine has the property but another machine not. For instance, 2.12 machine has `{ "EPYC-" TYPE_X86_CPU, "xlevel", "0x8000000a" }`, but compat_pros of 3.1 machine doesn't have it. Thus, to compare these machines we need to get unknown value of "EPYC-x86_64-cpu-xlevel" for 3.1 machine. These unknown values in the table are called "holes". To get values for these "holes" the script uses list of appropriate methods.) Notes: * Some init values from the devices can't be available like properties from virtio-9p when configure has --disable-virtfs. This situations will be seen in the table as "unavailable driver". * Default values can be obtained in an unobvious way, like x86 features. If the script doesn't know how to get property default value to compare one machine with another it fills "holes" with "unavailable method". This is done because script uses whitelist model to get default values of different types. It means that the method that can't be applied to a new type that can crash this script. It is better to get an "unavailable driver" when creating a new machine with new compatible properties than to break this script. So it turns out a more stable and generic script. * If the default value can't be obtained because this property doesn't exist or because this property can't have default value, appropriate "hole" will be filled by "unknown property" or "no default value" * If the property is applied to the abstract class, the script collects default values from all child classes and prints all these classes * Raw table (--raw flag) should be used with json/csv parameters for scripts and etc. Human-readable (default) format contains transformed and simplified values and it doesn't contain lines with the same values in columns Example: ./scripts/compare-machine-types.py --mt pc-q35-6.2 pc-q35-7.1 ╒══════════════════╤══════════════════════════╤════════════════════════════╤════════════════════════════╕ │ Driver │ Property │ build/qemu-system-x86_64 │ build/qemu-system-x86_64 │ │ │ │ pc-q35-6.2 │ pc-q35-7.1 │ ╞══════════════════╪══════════════════════════╪════════════════════════════╪════════════════════════════╡ │ PIIX4_PM │ x-not-migrate-acpi-index │ True │ False │ ├──────────────────┼──────────────────────────┼────────────────────────────┼────────────────────────────┤ │ arm-gicv3-common │ force-8-bit-prio │ True │ unavailable driver │ ├──────────────────┼──────────────────────────┼────────────────────────────┼────────────────────────────┤ │ nvme-ns │ eui64-default │ True │ False │ ├──────────────────┼──────────────────────────┼────────────────────────────┼────────────────────────────┤ │ virtio-mem │ unplugged-inaccessible │ False │ auto │ ╘══════════════════╧══════════════════════════╧════════════════════════════╧════════════════════════════╛ Signed-off-by: Maksim Davydov Reviewed-by: Vladimir Sementsov-Ogievskiy Message-ID: <20240318213550.155573-5-davydov-max@yandex-team.ru> Signed-off-by: Philippe Mathieu-Daudé --- MAINTAINERS | 5 + scripts/compare-machine-types.py | 486 +++++++++++++++++++++++++++++++ 2 files changed, 491 insertions(+) create mode 100755 scripts/compare-machine-types.py diff --git a/MAINTAINERS b/MAINTAINERS index 8bb32f4a7e..118206c3e0 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -4229,3 +4229,8 @@ Code Coverage Tools M: Alex Bennée S: Odd Fixes F: scripts/coverage/ + +Machine development tool +M: Maksim Davydov +S: Supported +F: scripts/compare-machine-types.py diff --git a/scripts/compare-machine-types.py b/scripts/compare-machine-types.py new file mode 100755 index 0000000000..2af3995eb8 --- /dev/null +++ b/scripts/compare-machine-types.py @@ -0,0 +1,486 @@ +#!/usr/bin/env python3 +# +# Script to compare machine type compatible properties (include/hw/boards.h). +# compat_props are applied to the driver during initialization to change +# default values, for instance, to maintain compatibility. +# This script constructs table with machines and values of their compat_props +# to compare and to find places for improvements or places with bugs. If +# during the comparison, some machine type doesn't have a property (it is in +# the comparison table because another machine type has it), then the +# appropriate method will be used to obtain the default value of this driver +# property via qmp command (e.g. query-cpu-model-expansion for x86_64-cpu). +# These methods are defined below in qemu_property_methods. +# +# Copyright (c) Yandex Technologies LLC, 2023 +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, see . + +import sys +from os import path +from argparse import ArgumentParser, RawTextHelpFormatter, Namespace +import pandas as pd +from contextlib import ExitStack +from typing import Optional, List, Dict, Generator, Tuple, Union, Any, Set + +try: + qemu_dir = path.abspath(path.dirname(path.dirname(__file__))) + sys.path.append(path.join(qemu_dir, 'python')) + from qemu.machine import QEMUMachine +except ModuleNotFoundError as exc: + print(f"Module '{exc.name}' not found.") + print("Try export PYTHONPATH=top-qemu-dir/python or run from top-qemu-dir") + sys.exit(1) + + +default_qemu_args = '-enable-kvm -machine none' +default_qemu_binary = 'build/qemu-system-x86_64' + + +# Methods for gettig the right values of drivers properties +# +# Use these methods as a 'whitelist' and add entries only if necessary. It's +# important to be stable and predictable in analysis and tests. +# Be careful: +# * Class must be inherited from 'QEMUObject' and used in new_driver() +# * Class has to implement get_prop method in order to get values +# * Specialization always wins (with the given classes for 'device' and +# 'x86_64-cpu', method of 'x86_64-cpu' will be used for '486-x86_64-cpu') + +class Driver(): + def __init__(self, vm: QEMUMachine, name: str, abstract: bool) -> None: + self.vm = vm + self.name = name + self.abstract = abstract + self.parent: Optional[Driver] = None + self.property_getter: Optional[Driver] = None + + def get_prop(self, driver: str, prop: str) -> str: + if self.property_getter: + return self.property_getter.get_prop(driver, prop) + else: + return 'Unavailable method' + + def is_child_of(self, parent: 'Driver') -> bool: + """Checks whether self is (recursive) child of @parent""" + cur_parent = self.parent + while cur_parent: + if cur_parent is parent: + return True + cur_parent = cur_parent.parent + + return False + + def set_implementations(self, implementations: List['Driver']) -> None: + self.implementations = implementations + + +class QEMUObject(Driver): + def __init__(self, vm: QEMUMachine, name: str) -> None: + super().__init__(vm, name, True) + + def set_implementations(self, implementations: List[Driver]) -> None: + self.implementations = implementations + + # each implementation of the abstract driver has to use property getter + # of this abstract driver unless it has specialization. (e.g. having + # 'device' and 'x86_64-cpu', property getter of 'x86_64-cpu' will be + # used for '486-x86_64-cpu') + for impl in implementations: + if not impl.property_getter or\ + self.is_child_of(impl.property_getter): + impl.property_getter = self + + +class QEMUDevice(QEMUObject): + def __init__(self, vm: QEMUMachine) -> None: + super().__init__(vm, 'device') + self.cached: Dict[str, List[Dict[str, Any]]] = {} + + def get_prop(self, driver: str, prop_name: str) -> str: + if driver not in self.cached: + self.cached[driver] = self.vm.cmd('device-list-properties', + typename=driver) + for prop in self.cached[driver]: + if prop['name'] == prop_name: + return str(prop.get('default-value', 'No default value')) + + return 'Unknown property' + + +class QEMUx86CPU(QEMUObject): + def __init__(self, vm: QEMUMachine) -> None: + super().__init__(vm, 'x86_64-cpu') + self.cached: Dict[str, Dict[str, Any]] = {} + + def get_prop(self, driver: str, prop_name: str) -> str: + if not driver.endswith('-x86_64-cpu'): + return 'Wrong x86_64-cpu name' + + # crop last 11 chars '-x86_64-cpu' + name = driver[:-11] + if name not in self.cached: + self.cached[name] = self.vm.cmd( + 'query-cpu-model-expansion', type='full', + model={'name': name})['model']['props'] + return str(self.cached[name].get(prop_name, 'Unknown property')) + + +# Now it's stub, because all memory_backend types don't have default values +# but this behaviour can be changed +class QEMUMemoryBackend(QEMUObject): + def __init__(self, vm: QEMUMachine) -> None: + super().__init__(vm, 'memory-backend') + self.cached: Dict[str, List[Dict[str, Any]]] = {} + + def get_prop(self, driver: str, prop_name: str) -> str: + if driver not in self.cached: + self.cached[driver] = self.vm.cmd('qom-list-properties', + typename=driver) + for prop in self.cached[driver]: + if prop['name'] == prop_name: + return str(prop.get('default-value', 'No default value')) + + return 'Unknown property' + + +def new_driver(vm: QEMUMachine, name: str, is_abstr: bool) -> Driver: + if name == 'object': + return QEMUObject(vm, 'object') + elif name == 'device': + return QEMUDevice(vm) + elif name == 'x86_64-cpu': + return QEMUx86CPU(vm) + elif name == 'memory-backend': + return QEMUMemoryBackend(vm) + else: + return Driver(vm, name, is_abstr) +# End of methods definition + + +class VMPropertyGetter: + """It implements the relationship between drivers and how to get their + properties""" + def __init__(self, vm: QEMUMachine) -> None: + self.drivers: Dict[str, Driver] = {} + + qom_all_types = vm.cmd('qom-list-types', abstract=True) + self.drivers = {t['name']: new_driver(vm, t['name'], + t.get('abstract', False)) + for t in qom_all_types} + + for t in qom_all_types: + drv = self.drivers[t['name']] + if 'parent' in t: + drv.parent = self.drivers[t['parent']] + + for drv in self.drivers.values(): + imps = vm.cmd('qom-list-types', implements=drv.name) + # only implementations inherit property getter + drv.set_implementations([self.drivers[imp['name']] + for imp in imps]) + + def get_prop(self, driver: str, prop: str) -> str: + # wrong driver name or disabled in config driver + try: + drv = self.drivers[driver] + except KeyError: + return 'Unavailable driver' + + assert not drv.abstract + + return drv.get_prop(driver, prop) + + def get_implementations(self, driver: str) -> List[str]: + return [impl.name for impl in self.drivers[driver].implementations] + + +class Machine: + """A short QEMU machine type description. It contains only processed + compat_props (properties of abstract classes are applied to its + implementations) + """ + # raw_mt_dict - dict produced by `query-machines` + def __init__(self, raw_mt_dict: Dict[str, Any], + qemu_drivers: VMPropertyGetter) -> None: + self.name = raw_mt_dict['name'] + self.compat_props: Dict[str, Any] = {} + # properties are applied sequentially and can rewrite values like in + # QEMU. Also it has to resolve class relationships to apply appropriate + # values from abstract class to all implementations + for prop in raw_mt_dict['compat-props']: + driver = prop['qom-type'] + try: + # implementation adds only itself, abstract class adds + # lementation (abstract classes are uninterestiong) + impls = qemu_drivers.get_implementations(driver) + for impl in impls: + if impl not in self.compat_props: + self.compat_props[impl] = {} + self.compat_props[impl][prop['property']] = prop['value'] + except KeyError: + # QEMU doesn't know this driver thus it has to be saved + if driver not in self.compat_props: + self.compat_props[driver] = {} + self.compat_props[driver][prop['property']] = prop['value'] + + +class Configuration(): + """Class contains all necessary components to generate table and is used + to compare different binaries""" + def __init__(self, vm: QEMUMachine, + req_mt: List[str], all_mt: bool) -> None: + self._vm = vm + self._binary = vm.binary + self._qemu_args = args.qemu_args.split(' ') + + self._qemu_drivers = VMPropertyGetter(vm) + self.req_mt = get_req_mt(self._qemu_drivers, vm, req_mt, all_mt) + + def get_implementations(self, driver_name: str) -> List[str]: + return self._qemu_drivers.get_implementations(driver_name) + + def get_table(self, req_props: List[Tuple[str, str]]) -> pd.DataFrame: + table: List[pd.DataFrame] = [] + for mt in self.req_mt: + name = f'{self._binary}\n{mt.name}' + column = [] + for driver, prop in req_props: + try: + # values from QEMU machine type definitions + column.append(mt.compat_props[driver][prop]) + except KeyError: + # values from QEMU type definitions + column.append(self._qemu_drivers.get_prop(driver, prop)) + table.append(pd.DataFrame({name: column})) + + return pd.concat(table, axis=1) + + +script_desc = """Script to compare machine types (their compat_props). + +Examples: +* save info about all machines: ./scripts/compare-machine-types.py --all \ +--format csv --raw > table.csv +* compare machines: ./scripts/compare-machine-types.py --mt pc-q35-2.12 \ +pc-q35-3.0 +* compare binaries and machines: ./scripts/compare-machine-types.py \ +--mt pc-q35-6.2 pc-q35-7.0 --qemu-binary build/qemu-system-x86_64 \ +build/qemu-exp + ╒════════════╤══════════════════════════╤════════════════════════════\ +╤════════════════════════════╤══════════════════╤══════════════════╕ + │ Driver │ Property │ build/qemu-system-x86_64 \ +│ build/qemu-system-x86_64 │ build/qemu-exp │ build/qemu-exp │ + │ │ │ pc-q35-6.2 \ +│ pc-q35-7.0 │ pc-q35-6.2 │ pc-q35-7.0 │ + ╞════════════╪══════════════════════════╪════════════════════════════\ +╪════════════════════════════╪══════════════════╪══════════════════╡ + │ PIIX4_PM │ x-not-migrate-acpi-index │ True \ +│ False │ False │ False │ + ├────────────┼──────────────────────────┼────────────────────────────\ +┼────────────────────────────┼──────────────────┼──────────────────┤ + │ virtio-mem │ unplugged-inaccessible │ False \ +│ auto │ False │ auto │ + ╘════════════╧══════════════════════════╧════════════════════════════\ +╧════════════════════════════╧══════════════════╧══════════════════╛ + +If a property from QEMU machine defintion applies to an abstract class (e.g. \ +x86_64-cpu) this script will compare all implementations of this class. + +"Unavailable method" - means that this script doesn't know how to get \ +default values of the driver. To add method use the construction described \ +at the top of the script. +"Unavailable driver" - means that this script doesn't know this driver. \ +For instance, this can happen if you configure QEMU without this device or \ +if machine type definition has error. +"No default value" - means that the appropriate method can't get the default \ +value and most likely that this property doesn't have it. +"Unknown property" - means that the appropriate method can't find property \ +with this name.""" + + +def parse_args() -> Namespace: + parser = ArgumentParser(formatter_class=RawTextHelpFormatter, + description=script_desc) + parser.add_argument('--format', choices=['human-readable', 'json', 'csv'], + default='human-readable', + help='returns table in json format') + parser.add_argument('--raw', action='store_true', + help='prints ALL defined properties without value ' + 'transformation. By default, only rows ' + 'with different values will be printed and ' + 'values will be transformed(e.g. "on" -> True)') + parser.add_argument('--qemu-args', default=default_qemu_args, + help='command line to start qemu. ' + f'Default: "{default_qemu_args}"') + parser.add_argument('--qemu-binary', nargs="*", type=str, + default=[default_qemu_binary], + help='list of qemu binaries that will be compared. ' + f'Deafult: {default_qemu_binary}') + + mt_args_group = parser.add_mutually_exclusive_group() + mt_args_group.add_argument('--all', action='store_true', + help='prints all available machine types (list ' + 'of machine types will be ignored)') + mt_args_group.add_argument('--mt', nargs="*", type=str, + help='list of Machine Types ' + 'that will be compared') + + return parser.parse_args() + + +def mt_comp(mt: Machine) -> Tuple[str, int, int, int]: + """Function to compare and sort machine by names. + It returns socket_name, major version, minor version, revision""" + # none, microvm, x-remote and etc. + if '-' not in mt.name or '.' not in mt.name: + return mt.name, 0, 0, 0 + + socket, ver = mt.name.rsplit('-', 1) + ver_list = list(map(int, ver.split('.', 2))) + ver_list += [0] * (3 - len(ver_list)) + return socket, ver_list[0], ver_list[1], ver_list[2] + + +def get_mt_definitions(qemu_drivers: VMPropertyGetter, + vm: QEMUMachine) -> List[Machine]: + """Constructs list of machine definitions (primarily compat_props) via + info from QEMU""" + raw_mt_defs = vm.cmd('query-machines', compat_props=True) + mt_defs = [] + for raw_mt in raw_mt_defs: + mt_defs.append(Machine(raw_mt, qemu_drivers)) + + mt_defs.sort(key=mt_comp) + return mt_defs + + +def get_req_mt(qemu_drivers: VMPropertyGetter, vm: QEMUMachine, + req_mt: Optional[List[str]], all_mt: bool) -> List[Machine]: + """Returns list of requested by user machines""" + mt_defs = get_mt_definitions(qemu_drivers, vm) + if all_mt: + return mt_defs + + if req_mt is None: + print('Enter machine types for comparision') + exit(0) + + matched_mt = [] + for mt in mt_defs: + if mt.name in req_mt: + matched_mt.append(mt) + + return matched_mt + + +def get_affected_props(configs: List[Configuration]) -> Generator[Tuple[str, + str], + None, None]: + """Helps to go through all affected in machine definitions drivers + and properties""" + driver_props: Dict[str, Set[Any]] = {} + for config in configs: + for mt in config.req_mt: + compat_props = mt.compat_props + for driver, prop in compat_props.items(): + if driver not in driver_props: + driver_props[driver] = set() + driver_props[driver].update(prop.keys()) + + for driver, props in sorted(driver_props.items()): + for prop in sorted(props): + yield driver, prop + + +def transform_value(value: str) -> Union[str, bool]: + true_list = ['true', 'on'] + false_list = ['false', 'off'] + + out = value.lower() + + if out in true_list: + return True + + if out in false_list: + return False + + return value + + +def simplify_table(table: pd.DataFrame) -> pd.DataFrame: + """transforms values to make it easier to compare it and drops rows + with the same values for all columns""" + + table = table.map(transform_value) + + return table[~table.iloc[:, 3:].eq(table.iloc[:, 2], axis=0).all(axis=1)] + + +# constructs table in the format: +# +# Driver | Property | binary1 | binary1 | ... +# | | machine1 | machine2 | ... +# ------------------------------------------------------ ... +# driver1 | property1 | value1 | value2 | ... +# driver1 | property2 | value3 | value4 | ... +# driver2 | property3 | value5 | value6 | ... +# ... | ... | ... | ... | ... +# +def fill_prop_table(configs: List[Configuration], + is_raw: bool) -> pd.DataFrame: + req_props = list(get_affected_props(configs)) + if not req_props: + print('No drivers to compare. Check machine names') + exit(0) + + driver_col, prop_col = tuple(zip(*req_props)) + table = [pd.DataFrame({'Driver': driver_col}), + pd.DataFrame({'Property': prop_col})] + + table.extend([config.get_table(req_props) for config in configs]) + + df_table = pd.concat(table, axis=1) + + if is_raw: + return df_table + + return simplify_table(df_table) + + +def print_table(table: pd.DataFrame, table_format: str) -> None: + if table_format == 'json': + print(comp_table.to_json()) + elif table_format == 'csv': + print(comp_table.to_csv()) + else: + print(comp_table.to_markdown(index=False, stralign='center', + colalign=('center',), headers='keys', + tablefmt='fancy_grid', + disable_numparse=True)) + + +if __name__ == '__main__': + args = parse_args() + with ExitStack() as stack: + vms = [stack.enter_context(QEMUMachine(binary=binary, qmp_timer=15, + args=args.qemu_args.split(' '))) for binary in args.qemu_binary] + + configurations = [] + for vm in vms: + vm.launch() + configurations.append(Configuration(vm, args.mt, args.all)) + + comp_table = fill_prop_table(configurations, args.raw) + if not comp_table.empty: + print_table(comp_table, args.format) From 6c3b78532ca7b92eeeef756b4c3d8c5cc82ed3ab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Tue, 19 Mar 2024 19:29:50 +0100 Subject: [PATCH 05/22] hw/core: Remove check on NEED_CPU_H in tcg-cpu-ops.h MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Commit fd3f7d24d4 ("include/hw/core: Remove i386 conditional on fake_user_interrupt") remove the need to check on NEED_CPU_H. Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Richard Henderson Message-Id: <20240321154838.95771-3-philmd@linaro.org> --- include/hw/core/tcg-cpu-ops.h | 3 --- 1 file changed, 3 deletions(-) diff --git a/include/hw/core/tcg-cpu-ops.h b/include/hw/core/tcg-cpu-ops.h index bf8ff8e3ee..dc1f16a977 100644 --- a/include/hw/core/tcg-cpu-ops.h +++ b/include/hw/core/tcg-cpu-ops.h @@ -49,7 +49,6 @@ struct TCGCPUOps { /** @debug_excp_handler: Callback for handling debug exceptions */ void (*debug_excp_handler)(CPUState *cpu); -#ifdef NEED_CPU_H #ifdef CONFIG_USER_ONLY /** * @fake_user_interrupt: Callback for 'fake exception' handling. @@ -174,8 +173,6 @@ struct TCGCPUOps { */ bool (*need_replay_interrupt)(int interrupt_request); #endif /* !CONFIG_USER_ONLY */ -#endif /* NEED_CPU_H */ - }; #if defined(CONFIG_USER_ONLY) From 63073574e8d5551dc31a30b59830b886e9f9dbfe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Thu, 21 Mar 2024 14:29:11 +0100 Subject: [PATCH 06/22] target/i386: Move APIC related code to cpu-apic.c MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Move APIC related code split in cpu-sysemu.c and monitor.c to cpu-apic.c. Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Richard Henderson Reviewed-by: Zhao Liu Message-Id: <20240321154838.95771-4-philmd@linaro.org> --- target/i386/cpu-apic.c | 112 +++++++++++++++++++++++++++++++++++++++ target/i386/cpu-sysemu.c | 77 --------------------------- target/i386/meson.build | 1 + target/i386/monitor.c | 25 --------- 4 files changed, 113 insertions(+), 102 deletions(-) create mode 100644 target/i386/cpu-apic.c diff --git a/target/i386/cpu-apic.c b/target/i386/cpu-apic.c new file mode 100644 index 0000000000..d397ec94dc --- /dev/null +++ b/target/i386/cpu-apic.c @@ -0,0 +1,112 @@ +/* + * QEMU x86 CPU <-> APIC + * + * Copyright (c) 2003-2004 Fabrice Bellard + * + * SPDX-License-Identifier: MIT + */ + +#include "qemu/osdep.h" +#include "qapi/qmp/qdict.h" +#include "qapi/error.h" +#include "monitor/monitor.h" +#include "monitor/hmp-target.h" +#include "sysemu/hw_accel.h" +#include "sysemu/kvm.h" +#include "sysemu/xen.h" +#include "exec/address-spaces.h" +#include "hw/qdev-properties.h" +#include "hw/i386/apic_internal.h" +#include "cpu-internal.h" + +APICCommonClass *apic_get_class(Error **errp) +{ + const char *apic_type = "apic"; + + /* TODO: in-kernel irqchip for hvf */ + if (kvm_enabled()) { + if (!kvm_irqchip_in_kernel()) { + error_setg(errp, "KVM does not support userspace APIC"); + return NULL; + } + apic_type = "kvm-apic"; + } else if (xen_enabled()) { + apic_type = "xen-apic"; + } else if (whpx_apic_in_platform()) { + apic_type = "whpx-apic"; + } + + return APIC_COMMON_CLASS(object_class_by_name(apic_type)); +} + +void x86_cpu_apic_create(X86CPU *cpu, Error **errp) +{ + APICCommonState *apic; + APICCommonClass *apic_class = apic_get_class(errp); + + if (!apic_class) { + return; + } + + cpu->apic_state = DEVICE(object_new_with_class(OBJECT_CLASS(apic_class))); + object_property_add_child(OBJECT(cpu), "lapic", + OBJECT(cpu->apic_state)); + object_unref(OBJECT(cpu->apic_state)); + + /* TODO: convert to link<> */ + apic = APIC_COMMON(cpu->apic_state); + apic->cpu = cpu; + apic->apicbase = APIC_DEFAULT_ADDRESS | MSR_IA32_APICBASE_ENABLE; + + /* + * apic_common_set_id needs to check if the CPU has x2APIC + * feature in case APIC ID >= 255, so we need to set apic->cpu + * before setting APIC ID + */ + qdev_prop_set_uint32(cpu->apic_state, "id", cpu->apic_id); +} + +void x86_cpu_apic_realize(X86CPU *cpu, Error **errp) +{ + APICCommonState *apic; + static bool apic_mmio_map_once; + + if (cpu->apic_state == NULL) { + return; + } + qdev_realize(DEVICE(cpu->apic_state), NULL, errp); + + /* Map APIC MMIO area */ + apic = APIC_COMMON(cpu->apic_state); + if (!apic_mmio_map_once) { + memory_region_add_subregion_overlap(get_system_memory(), + apic->apicbase & + MSR_IA32_APICBASE_BASE, + &apic->io_memory, + 0x1000); + apic_mmio_map_once = true; + } +} + +void hmp_info_local_apic(Monitor *mon, const QDict *qdict) +{ + CPUState *cs; + + if (qdict_haskey(qdict, "apic-id")) { + int id = qdict_get_try_int(qdict, "apic-id", 0); + + cs = cpu_by_arch_id(id); + if (cs) { + cpu_synchronize_state(cs); + } + } else { + cs = mon_get_cpu(mon); + } + + + if (!cs) { + monitor_printf(mon, "No CPU available\n"); + return; + } + x86_cpu_dump_local_apic_state(cs, CPU_DUMP_FPU); +} diff --git a/target/i386/cpu-sysemu.c b/target/i386/cpu-sysemu.c index 3f9093d285..227ac021f6 100644 --- a/target/i386/cpu-sysemu.c +++ b/target/i386/cpu-sysemu.c @@ -19,19 +19,12 @@ #include "qemu/osdep.h" #include "cpu.h" -#include "sysemu/kvm.h" -#include "sysemu/xen.h" -#include "sysemu/whpx.h" #include "qapi/error.h" #include "qapi/qapi-visit-run-state.h" #include "qapi/qmp/qdict.h" #include "qapi/qobject-input-visitor.h" #include "qom/qom-qobject.h" #include "qapi/qapi-commands-machine-target.h" -#include "hw/qdev-properties.h" - -#include "exec/address-spaces.h" -#include "hw/i386/apic_internal.h" #include "cpu-internal.h" @@ -273,75 +266,6 @@ void x86_cpu_machine_reset_cb(void *opaque) cpu_reset(CPU(cpu)); } -APICCommonClass *apic_get_class(Error **errp) -{ - const char *apic_type = "apic"; - - /* TODO: in-kernel irqchip for hvf */ - if (kvm_enabled()) { - if (!kvm_irqchip_in_kernel()) { - error_setg(errp, "KVM does not support userspace APIC"); - return NULL; - } - apic_type = "kvm-apic"; - } else if (xen_enabled()) { - apic_type = "xen-apic"; - } else if (whpx_apic_in_platform()) { - apic_type = "whpx-apic"; - } - - return APIC_COMMON_CLASS(object_class_by_name(apic_type)); -} - -void x86_cpu_apic_create(X86CPU *cpu, Error **errp) -{ - APICCommonState *apic; - APICCommonClass *apic_class = apic_get_class(errp); - - if (!apic_class) { - return; - } - - cpu->apic_state = DEVICE(object_new_with_class(OBJECT_CLASS(apic_class))); - object_property_add_child(OBJECT(cpu), "lapic", - OBJECT(cpu->apic_state)); - object_unref(OBJECT(cpu->apic_state)); - - /* TODO: convert to link<> */ - apic = APIC_COMMON(cpu->apic_state); - apic->cpu = cpu; - apic->apicbase = APIC_DEFAULT_ADDRESS | MSR_IA32_APICBASE_ENABLE; - - /* - * apic_common_set_id needs to check if the CPU has x2APIC - * feature in case APIC ID >= 255, so we need to set apic->cpu - * before setting APIC ID - */ - qdev_prop_set_uint32(cpu->apic_state, "id", cpu->apic_id); -} - -void x86_cpu_apic_realize(X86CPU *cpu, Error **errp) -{ - APICCommonState *apic; - static bool apic_mmio_map_once; - - if (cpu->apic_state == NULL) { - return; - } - qdev_realize(DEVICE(cpu->apic_state), NULL, errp); - - /* Map APIC MMIO area */ - apic = APIC_COMMON(cpu->apic_state); - if (!apic_mmio_map_once) { - memory_region_add_subregion_overlap(get_system_memory(), - apic->apicbase & - MSR_IA32_APICBASE_BASE, - &apic->io_memory, - 0x1000); - apic_mmio_map_once = true; - } -} - GuestPanicInformation *x86_cpu_get_crash_info(CPUState *cs) { X86CPU *cpu = X86_CPU(cs); @@ -385,4 +309,3 @@ void x86_cpu_get_crash_info_qom(Object *obj, Visitor *v, errp); qapi_free_GuestPanicInformation(panic_info); } - diff --git a/target/i386/meson.build b/target/i386/meson.build index 8abce725f8..075117989b 100644 --- a/target/i386/meson.build +++ b/target/i386/meson.build @@ -18,6 +18,7 @@ i386_system_ss.add(files( 'arch_memory_mapping.c', 'machine.c', 'monitor.c', + 'cpu-apic.c', 'cpu-sysemu.c', )) i386_system_ss.add(when: 'CONFIG_SEV', if_true: files('sev.c'), if_false: files('sev-sysemu-stub.c')) diff --git a/target/i386/monitor.c b/target/i386/monitor.c index 3a281dab02..2d766b2637 100644 --- a/target/i386/monitor.c +++ b/target/i386/monitor.c @@ -28,8 +28,6 @@ #include "monitor/hmp-target.h" #include "monitor/hmp.h" #include "qapi/qmp/qdict.h" -#include "sysemu/hw_accel.h" -#include "sysemu/kvm.h" #include "qapi/error.h" #include "qapi/qapi-commands-misc-target.h" #include "qapi/qapi-commands-misc.h" @@ -647,26 +645,3 @@ const MonitorDef *target_monitor_defs(void) { return monitor_defs; } - -void hmp_info_local_apic(Monitor *mon, const QDict *qdict) -{ - CPUState *cs; - - if (qdict_haskey(qdict, "apic-id")) { - int id = qdict_get_try_int(qdict, "apic-id", 0); - - cs = cpu_by_arch_id(id); - if (cs) { - cpu_synchronize_state(cs); - } - } else { - cs = mon_get_cpu(mon); - } - - - if (!cs) { - monitor_printf(mon, "No CPU available\n"); - return; - } - x86_cpu_dump_local_apic_state(cs, CPU_DUMP_FPU); -} From a6ab7a98c9a8106cb8ef86cbc88a27fe69963423 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Mon, 8 Apr 2024 13:15:44 +0200 Subject: [PATCH 07/22] hw/misc/applesmc: Simplify DeviceReset handler MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Have applesmc_find_key() return a const pointer. Since the returned buffers are not modified in applesmc_io_data_write(), it is pointless to delete and re-add the keys in the DeviceReset handler. Add them once in DeviceRealize, and discard them in the DeviceUnrealize handler. Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Peter Maydell Message-Id: <20240410180819.92332-1-philmd@linaro.org> --- hw/misc/applesmc.c | 36 +++++++++++++++++++++--------------- 1 file changed, 21 insertions(+), 15 deletions(-) diff --git a/hw/misc/applesmc.c b/hw/misc/applesmc.c index 14e3ef667d..59a4899312 100644 --- a/hw/misc/applesmc.c +++ b/hw/misc/applesmc.c @@ -145,7 +145,7 @@ static void applesmc_io_cmd_write(void *opaque, hwaddr addr, uint64_t val, s->data_pos = 0; } -static struct AppleSMCData *applesmc_find_key(AppleSMCState *s) +static const struct AppleSMCData *applesmc_find_key(AppleSMCState *s) { struct AppleSMCData *d; @@ -161,7 +161,7 @@ static void applesmc_io_data_write(void *opaque, hwaddr addr, uint64_t val, unsigned size) { AppleSMCState *s = opaque; - struct AppleSMCData *d; + const struct AppleSMCData *d; smc_debug("DATA received: 0x%02x\n", (uint8_t)val); switch (s->cmd) { @@ -269,23 +269,10 @@ static void applesmc_add_key(AppleSMCState *s, const char *key, static void qdev_applesmc_isa_reset(DeviceState *dev) { AppleSMCState *s = APPLE_SMC(dev); - struct AppleSMCData *d, *next; - /* Remove existing entries */ - QLIST_FOREACH_SAFE(d, &s->data_def, node, next) { - QLIST_REMOVE(d, node); - g_free(d); - } s->status = 0x00; s->status_1e = 0x00; s->last_ret = 0x00; - - applesmc_add_key(s, "REV ", 6, "\x01\x13\x0f\x00\x00\x03"); - applesmc_add_key(s, "OSK0", 32, s->osk); - applesmc_add_key(s, "OSK1", 32, s->osk + 32); - applesmc_add_key(s, "NATJ", 1, "\0"); - applesmc_add_key(s, "MSSP", 1, "\0"); - applesmc_add_key(s, "MSSD", 1, "\0x3"); } static const MemoryRegionOps applesmc_data_io_ops = { @@ -343,6 +330,24 @@ static void applesmc_isa_realize(DeviceState *dev, Error **errp) } QLIST_INIT(&s->data_def); + applesmc_add_key(s, "REV ", 6, "\x01\x13\x0f\x00\x00\x03"); + applesmc_add_key(s, "OSK0", 32, s->osk); + applesmc_add_key(s, "OSK1", 32, s->osk + 32); + applesmc_add_key(s, "NATJ", 1, "\0"); + applesmc_add_key(s, "MSSP", 1, "\0"); + applesmc_add_key(s, "MSSD", 1, "\0x3"); +} + +static void applesmc_unrealize(DeviceState *dev) +{ + AppleSMCState *s = APPLE_SMC(dev); + struct AppleSMCData *d, *next; + + /* Remove existing entries */ + QLIST_FOREACH_SAFE(d, &s->data_def, node, next) { + QLIST_REMOVE(d, node); + g_free(d); + } } static Property applesmc_isa_properties[] = { @@ -377,6 +382,7 @@ static void qdev_applesmc_class_init(ObjectClass *klass, void *data) AcpiDevAmlIfClass *adevc = ACPI_DEV_AML_IF_CLASS(klass); dc->realize = applesmc_isa_realize; + dc->unrealize = applesmc_unrealize; dc->reset = qdev_applesmc_isa_reset; device_class_set_props(dc, applesmc_isa_properties); set_bit(DEVICE_CATEGORY_MISC, dc->categories); From ca4af17c5e4a55b73850163dfc64a7d3c69378be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Thu, 11 Apr 2024 12:31:14 +0200 Subject: [PATCH 08/22] hw/misc/imx: Replace sprintf() by snprintf() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit sprintf() is deprecated on Darwin since macOS 13.0 / XCode 14.1, resulting in painful developer experience. Use snprintf() instead. Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Peter Maydell Message-Id: <20240411104340.6617-6-philmd@linaro.org> Signed-off-by: Richard Henderson --- hw/misc/imx25_ccm.c | 2 +- hw/misc/imx31_ccm.c | 2 +- hw/misc/imx6_ccm.c | 4 ++-- hw/misc/imx6_src.c | 2 +- hw/misc/imx6ul_ccm.c | 4 ++-- hw/misc/imx7_src.c | 2 +- hw/net/imx_fec.c | 2 +- hw/ssi/imx_spi.c | 2 +- 8 files changed, 10 insertions(+), 10 deletions(-) diff --git a/hw/misc/imx25_ccm.c b/hw/misc/imx25_ccm.c index d888966014..faa726a86a 100644 --- a/hw/misc/imx25_ccm.c +++ b/hw/misc/imx25_ccm.c @@ -91,7 +91,7 @@ static const char *imx25_ccm_reg_name(uint32_t reg) case IMX25_CCM_LPIMR1_REG: return "lpimr1"; default: - sprintf(unknown, "[%u ?]", reg); + snprintf(unknown, sizeof(unknown), "[%u ?]", reg); return unknown; } } diff --git a/hw/misc/imx31_ccm.c b/hw/misc/imx31_ccm.c index a9059bb1f7..125d4fceeb 100644 --- a/hw/misc/imx31_ccm.c +++ b/hw/misc/imx31_ccm.c @@ -89,7 +89,7 @@ static const char *imx31_ccm_reg_name(uint32_t reg) case IMX31_CCM_PDR2_REG: return "PDR2"; default: - sprintf(unknown, "[%u ?]", reg); + snprintf(unknown, sizeof(unknown), "[%u ?]", reg); return unknown; } } diff --git a/hw/misc/imx6_ccm.c b/hw/misc/imx6_ccm.c index 56489d8b57..b1def7f05b 100644 --- a/hw/misc/imx6_ccm.c +++ b/hw/misc/imx6_ccm.c @@ -85,7 +85,7 @@ static const char *imx6_ccm_reg_name(uint32_t reg) case CCM_CMEOR: return "CMEOR"; default: - sprintf(unknown, "%u ?", reg); + snprintf(unknown, sizeof(unknown), "%u ?", reg); return unknown; } } @@ -224,7 +224,7 @@ static const char *imx6_analog_reg_name(uint32_t reg) case USB_ANALOG_DIGPROG: return "USB_ANALOG_DIGPROG"; default: - sprintf(unknown, "%u ?", reg); + snprintf(unknown, sizeof(unknown), "%u ?", reg); return unknown; } } diff --git a/hw/misc/imx6_src.c b/hw/misc/imx6_src.c index 0c6003559f..3766bdf561 100644 --- a/hw/misc/imx6_src.c +++ b/hw/misc/imx6_src.c @@ -68,7 +68,7 @@ static const char *imx6_src_reg_name(uint32_t reg) case SRC_GPR10: return "SRC_GPR10"; default: - sprintf(unknown, "%u ?", reg); + snprintf(unknown, sizeof(unknown), "%u ?", reg); return unknown; } } diff --git a/hw/misc/imx6ul_ccm.c b/hw/misc/imx6ul_ccm.c index bbc0be9921..0ac49ea34b 100644 --- a/hw/misc/imx6ul_ccm.c +++ b/hw/misc/imx6ul_ccm.c @@ -143,7 +143,7 @@ static const char *imx6ul_ccm_reg_name(uint32_t reg) case CCM_CMEOR: return "CMEOR"; default: - sprintf(unknown, "%u ?", reg); + snprintf(unknown, sizeof(unknown), "%u ?", reg); return unknown; } } @@ -274,7 +274,7 @@ static const char *imx6ul_analog_reg_name(uint32_t reg) case USB_ANALOG_DIGPROG: return "USB_ANALOG_DIGPROG"; default: - sprintf(unknown, "%u ?", reg); + snprintf(unknown, sizeof(unknown), "%u ?", reg); return unknown; } } diff --git a/hw/misc/imx7_src.c b/hw/misc/imx7_src.c index b3725ff6e7..d19f0450d4 100644 --- a/hw/misc/imx7_src.c +++ b/hw/misc/imx7_src.c @@ -75,7 +75,7 @@ static const char *imx7_src_reg_name(uint32_t reg) case SRC_GPR10: return "SRC_GPR10"; default: - sprintf(unknown, "%u ?", reg); + snprintf(unknown, sizeof(unknown), "%u ?", reg); return unknown; } } diff --git a/hw/net/imx_fec.c b/hw/net/imx_fec.c index cee84af7ba..8c91d20d44 100644 --- a/hw/net/imx_fec.c +++ b/hw/net/imx_fec.c @@ -41,7 +41,7 @@ static const char *imx_default_reg_name(IMXFECState *s, uint32_t index) { static char tmp[20]; - sprintf(tmp, "index %d", index); + snprintf(tmp, sizeof(tmp), "index %d", index); return tmp; } diff --git a/hw/ssi/imx_spi.c b/hw/ssi/imx_spi.c index d8a7583ff3..12d897d306 100644 --- a/hw/ssi/imx_spi.c +++ b/hw/ssi/imx_spi.c @@ -53,7 +53,7 @@ static const char *imx_spi_reg_name(uint32_t reg) case ECSPI_MSGDATA: return "ECSPI_MSGDATA"; default: - sprintf(unknown, "%u ?", reg); + snprintf(unknown, sizeof(unknown), "%u ?", reg); return unknown; } } From b8ff846ec88513112008bff3a4001c839fd50f03 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Thu, 11 Apr 2024 12:33:31 +0200 Subject: [PATCH 09/22] hw/riscv/virt: Replace sprintf by g_strdup_printf MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit sprintf() is deprecated on Darwin since macOS 13.0 / XCode 14.1. Use g_strdup_printf instead. Signed-off-by: Philippe Mathieu-Daudé [rth: Use g_strdup_printf] Signed-off-by: Richard Henderson Message-Id: <20240412073346.458116-26-richard.henderson@linaro.org> --- hw/riscv/virt.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c index d171e74f7b..4fdb660525 100644 --- a/hw/riscv/virt.c +++ b/hw/riscv/virt.c @@ -1617,10 +1617,8 @@ static void virt_machine_instance_init(Object *obj) static char *virt_get_aia_guests(Object *obj, Error **errp) { RISCVVirtState *s = RISCV_VIRT_MACHINE(obj); - char val[32]; - sprintf(val, "%d", s->aia_guests); - return g_strdup(val); + return g_strdup_printf("%d", s->aia_guests); } static void virt_set_aia_guests(Object *obj, const char *val, Error **errp) @@ -1741,7 +1739,6 @@ static void virt_machine_device_plug_cb(HotplugHandler *hotplug_dev, static void virt_machine_class_init(ObjectClass *oc, void *data) { - char str[128]; MachineClass *mc = MACHINE_CLASS(oc); HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(oc); @@ -1767,7 +1764,6 @@ static void virt_machine_class_init(ObjectClass *oc, void *data) machine_class_allow_dynamic_sysbus_dev(mc, TYPE_TPM_TIS_SYSBUS); #endif - object_class_property_add_bool(oc, "aclint", virt_get_aclint, virt_set_aclint); object_class_property_set_description(oc, "aclint", @@ -1785,9 +1781,14 @@ static void virt_machine_class_init(ObjectClass *oc, void *data) object_class_property_add_str(oc, "aia-guests", virt_get_aia_guests, virt_set_aia_guests); - sprintf(str, "Set number of guest MMIO pages for AIA IMSIC. Valid value " - "should be between 0 and %d.", VIRT_IRQCHIP_MAX_GUESTS); - object_class_property_set_description(oc, "aia-guests", str); + { + g_autofree char *str = + g_strdup_printf("Set number of guest MMIO pages for AIA IMSIC. " + "Valid value should be between 0 and %d.", + VIRT_IRQCHIP_MAX_GUESTS); + object_class_property_set_description(oc, "aia-guests", str); + } + object_class_property_add(oc, "acpi", "OnOffAuto", virt_get_acpi, virt_set_acpi, NULL, NULL); From c1c350dc2ccbf92524754694547909e1455e4eef Mon Sep 17 00:00:00 2001 From: Thomas Huth Date: Mon, 15 Apr 2024 08:56:54 +0200 Subject: [PATCH 10/22] hw: Fix problem with the A*MPCORE switches in the Kconfig files MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A9MPCORE, ARM11MPCORE and A15MPCORE are defined twice, once in hw/cpu/Kconfig and once in hw/arm/Kconfig. This is only possible by accident, since hw/cpu/Kconfig is never included from hw/Kconfig. Fix it by declaring the switches only in hw/cpu/Kconfig (since the related files reside in the hw/cpu/ folder) and by making sure that the file hw/cpu/Kconfig is now properly included from hw/Kconfig. Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: Thomas Huth Message-ID: <20240415065655.130099-2-thuth@redhat.com> Reviewed-by: Richard Henderson Signed-off-by: Philippe Mathieu-Daudé --- hw/Kconfig | 1 + hw/arm/Kconfig | 15 --------------- hw/cpu/Kconfig | 12 +++++++++--- 3 files changed, 10 insertions(+), 18 deletions(-) diff --git a/hw/Kconfig b/hw/Kconfig index b1cc40d6be..f7866e76f7 100644 --- a/hw/Kconfig +++ b/hw/Kconfig @@ -47,6 +47,7 @@ source watchdog/Kconfig # arch Kconfig source arm/Kconfig +source cpu/Kconfig source alpha/Kconfig source avr/Kconfig source cris/Kconfig diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig index 893a7bff66..d97015c45c 100644 --- a/hw/arm/Kconfig +++ b/hw/arm/Kconfig @@ -678,21 +678,6 @@ config ZAURUS select NAND select ECC -config A9MPCORE - bool - select A9_GTIMER - select A9SCU # snoop control unit - select ARM_GIC - select ARM_MPTIMER - -config A15MPCORE - bool - select ARM_GIC - -config ARM11MPCORE - bool - select ARM11SCU - config ARMSSE bool select ARM_V7M diff --git a/hw/cpu/Kconfig b/hw/cpu/Kconfig index 1767d028ac..f776e884cd 100644 --- a/hw/cpu/Kconfig +++ b/hw/cpu/Kconfig @@ -1,8 +1,14 @@ -config ARM11MPCORE - bool - config A9MPCORE bool + select A9_GTIMER + select A9SCU # snoop control unit + select ARM_GIC + select ARM_MPTIMER config A15MPCORE bool + select ARM_GIC + +config ARM11MPCORE + bool + select ARM11SCU From 259181d29f81aa72a489dddc7d59517894b51e0f Mon Sep 17 00:00:00 2001 From: Thomas Huth Date: Mon, 15 Apr 2024 08:56:55 +0200 Subject: [PATCH 11/22] hw: Add a Kconfig switch for the TYPE_CPU_CLUSTER device MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The cpu-cluster device is only needed for some few arm and riscv machines. Let's avoid compiling and linking it if it is not really necessary. Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: Thomas Huth Message-ID: <20240415065655.130099-3-thuth@redhat.com> Reviewed-by: Richard Henderson Signed-off-by: Philippe Mathieu-Daudé --- hw/arm/Kconfig | 3 +++ hw/cpu/Kconfig | 3 +++ hw/cpu/meson.build | 3 ++- hw/riscv/Kconfig | 2 ++ 4 files changed, 10 insertions(+), 1 deletion(-) diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig index d97015c45c..5d4015b75a 100644 --- a/hw/arm/Kconfig +++ b/hw/arm/Kconfig @@ -485,6 +485,7 @@ config XLNX_ZYNQMP_ARM select AHCI select ARM_GIC select CADENCE + select CPU_CLUSTER select DDC select DPCD select SDHCI @@ -503,6 +504,7 @@ config XLNX_VERSAL default y depends on TCG && AARCH64 select ARM_GIC + select CPU_CLUSTER select PL011 select CADENCE select VIRTIO_MMIO @@ -688,6 +690,7 @@ config ARMSSE select CMSDK_APB_DUALTIMER select CMSDK_APB_UART select CMSDK_APB_WATCHDOG + select CPU_CLUSTER select IOTKIT_SECCTL select IOTKIT_SYSCTL select IOTKIT_SYSINFO diff --git a/hw/cpu/Kconfig b/hw/cpu/Kconfig index f776e884cd..baff478e1b 100644 --- a/hw/cpu/Kconfig +++ b/hw/cpu/Kconfig @@ -12,3 +12,6 @@ config A15MPCORE config ARM11MPCORE bool select ARM11SCU + +config CPU_CLUSTER + bool diff --git a/hw/cpu/meson.build b/hw/cpu/meson.build index 38cdcfbe57..9d36bf8ae2 100644 --- a/hw/cpu/meson.build +++ b/hw/cpu/meson.build @@ -1,4 +1,5 @@ -system_ss.add(files('core.c', 'cluster.c')) +system_ss.add(files('core.c')) +system_ss.add(when: 'CONFIG_CPU_CLUSTER', if_true: files('cluster.c')) system_ss.add(when: 'CONFIG_ARM11MPCORE', if_true: files('arm11mpcore.c')) system_ss.add(when: 'CONFIG_REALVIEW', if_true: files('realview_mpcore.c')) diff --git a/hw/riscv/Kconfig b/hw/riscv/Kconfig index 5d644eb7b1..fc72ef0379 100644 --- a/hw/riscv/Kconfig +++ b/hw/riscv/Kconfig @@ -9,6 +9,7 @@ config IBEX config MICROCHIP_PFSOC bool select CADENCE_SDHCI + select CPU_CLUSTER select MCHP_PFSOC_DMC select MCHP_PFSOC_IOSCB select MCHP_PFSOC_MMUART @@ -68,6 +69,7 @@ config SIFIVE_E config SIFIVE_U bool select CADENCE + select CPU_CLUSTER select RISCV_ACLINT select SIFIVE_GPIO select SIFIVE_PDMA From 2c5b2d9128b22fa5b8eccc3993170cc49b414d29 Mon Sep 17 00:00:00 2001 From: Zhao Liu Date: Thu, 18 Apr 2024 18:04:31 +0800 Subject: [PATCH 12/22] hw/cxl/cxl-cdat: Make ct3_load_cdat() return boolean MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit As error.h suggested, the best practice for callee is to return something to indicate success / failure. So make ct3_load_cdat() return boolean, and this is the preparation for cxl_doe_cdat_init() returning boolean. Suggested-by: Markus Armbruster Signed-off-by: Zhao Liu Reviewed-by: Philippe Mathieu-Daudé Acked-by: Jonathan Cameron Message-ID: <20240418100433.1085447-2-zhao1.liu@linux.intel.com> Signed-off-by: Philippe Mathieu-Daudé --- hw/cxl/cxl-cdat.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/hw/cxl/cxl-cdat.c b/hw/cxl/cxl-cdat.c index 551545f782..b3e496857a 100644 --- a/hw/cxl/cxl-cdat.c +++ b/hw/cxl/cxl-cdat.c @@ -111,7 +111,7 @@ static void ct3_build_cdat(CDATObject *cdat, Error **errp) cdat->entry = g_steal_pointer(&cdat_st); } -static void ct3_load_cdat(CDATObject *cdat, Error **errp) +static bool ct3_load_cdat(CDATObject *cdat, Error **errp) { g_autofree CDATEntry *cdat_st = NULL; g_autofree uint8_t *buf = NULL; @@ -127,11 +127,11 @@ static void ct3_load_cdat(CDATObject *cdat, Error **errp) &file_size, &error)) { error_setg(errp, "CDAT: File read failed: %s", error->message); g_error_free(error); - return; + return false; } if (file_size < sizeof(CDATTableHeader)) { error_setg(errp, "CDAT: File too short"); - return; + return false; } i = sizeof(CDATTableHeader); num_ent = 1; @@ -139,19 +139,19 @@ static void ct3_load_cdat(CDATObject *cdat, Error **errp) hdr = (CDATSubHeader *)(buf + i); if (i + sizeof(CDATSubHeader) > file_size) { error_setg(errp, "CDAT: Truncated table"); - return; + return false; } cdat_len_check(hdr, errp); i += hdr->length; if (i > file_size) { error_setg(errp, "CDAT: Truncated table"); - return; + return false; } num_ent++; } if (i != file_size) { error_setg(errp, "CDAT: File length mismatch"); - return; + return false; } cdat_st = g_new0(CDATEntry, num_ent); @@ -185,6 +185,7 @@ static void ct3_load_cdat(CDATObject *cdat, Error **errp) cdat->entry_len = num_ent; cdat->entry = g_steal_pointer(&cdat_st); cdat->buf = g_steal_pointer(&buf); + return true; } void cxl_doe_cdat_init(CXLComponentState *cxl_cstate, Error **errp) From a133d207a8fefe934eb808c2b1ee8f2c695cb528 Mon Sep 17 00:00:00 2001 From: Zhao Liu Date: Thu, 18 Apr 2024 18:04:32 +0800 Subject: [PATCH 13/22] hw/cxl/cxl-cdat: Make ct3_build_cdat() return boolean MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit As error.h suggested, the best practice for callee is to return something to indicate success / failure. So make ct3_build_cdat() return boolean, and this is the preparation for cxl_doe_cdat_init() returning boolean. Suggested-by: Markus Armbruster Signed-off-by: Zhao Liu Reviewed-by: Philippe Mathieu-Daudé Acked-by: Jonathan Cameron Message-ID: <20240418100433.1085447-3-zhao1.liu@linux.intel.com> Signed-off-by: Philippe Mathieu-Daudé --- hw/cxl/cxl-cdat.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/hw/cxl/cxl-cdat.c b/hw/cxl/cxl-cdat.c index b3e496857a..e7bc1380bf 100644 --- a/hw/cxl/cxl-cdat.c +++ b/hw/cxl/cxl-cdat.c @@ -44,7 +44,7 @@ static void cdat_len_check(CDATSubHeader *hdr, Error **errp) } } -static void ct3_build_cdat(CDATObject *cdat, Error **errp) +static bool ct3_build_cdat(CDATObject *cdat, Error **errp) { g_autofree CDATTableHeader *cdat_header = NULL; g_autofree CDATEntry *cdat_st = NULL; @@ -58,7 +58,7 @@ static void ct3_build_cdat(CDATObject *cdat, Error **errp) cdat_header = g_malloc0(sizeof(*cdat_header)); if (!cdat_header) { error_setg(errp, "Failed to allocate CDAT header"); - return; + return false; } cdat->built_buf_len = cdat->build_cdat_table(&cdat->built_buf, @@ -67,14 +67,14 @@ static void ct3_build_cdat(CDATObject *cdat, Error **errp) if (cdat->built_buf_len <= 0) { /* Build later as not all data available yet */ cdat->to_update = true; - return; + return true; } cdat->to_update = false; cdat_st = g_malloc0(sizeof(*cdat_st) * (cdat->built_buf_len + 1)); if (!cdat_st) { error_setg(errp, "Failed to allocate CDAT entry array"); - return; + return false; } /* Entry 0 for CDAT header, starts with Entry 1 */ @@ -109,6 +109,7 @@ static void ct3_build_cdat(CDATObject *cdat, Error **errp) cdat_st[0].length = sizeof(*cdat_header); cdat->entry_len = 1 + cdat->built_buf_len; cdat->entry = g_steal_pointer(&cdat_st); + return true; } static bool ct3_load_cdat(CDATObject *cdat, Error **errp) From e0ddabc6d4cfef4a5c7f154f0b0ad00dbf9a18d0 Mon Sep 17 00:00:00 2001 From: Zhao Liu Date: Thu, 18 Apr 2024 18:04:33 +0800 Subject: [PATCH 14/22] hw/cxl/cxl-cdat: Make cxl_doe_cdat_init() return boolean MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit As error.h suggested, the best practice for callee is to return something to indicate success / failure. With returned boolean, there's no need to dereference @errp to check failure case. Suggested-by: Markus Armbruster Signed-off-by: Zhao Liu Reviewed-by: Philippe Mathieu-Daudé Acked-by: Jonathan Cameron Message-ID: <20240418100433.1085447-4-zhao1.liu@linux.intel.com> Signed-off-by: Philippe Mathieu-Daudé --- hw/cxl/cxl-cdat.c | 6 +++--- hw/mem/cxl_type3.c | 3 +-- hw/pci-bridge/cxl_upstream.c | 3 +-- include/hw/cxl/cxl_component.h | 2 +- 4 files changed, 6 insertions(+), 8 deletions(-) diff --git a/hw/cxl/cxl-cdat.c b/hw/cxl/cxl-cdat.c index e7bc1380bf..959a55518e 100644 --- a/hw/cxl/cxl-cdat.c +++ b/hw/cxl/cxl-cdat.c @@ -189,14 +189,14 @@ static bool ct3_load_cdat(CDATObject *cdat, Error **errp) return true; } -void cxl_doe_cdat_init(CXLComponentState *cxl_cstate, Error **errp) +bool cxl_doe_cdat_init(CXLComponentState *cxl_cstate, Error **errp) { CDATObject *cdat = &cxl_cstate->cdat; if (cdat->filename) { - ct3_load_cdat(cdat, errp); + return ct3_load_cdat(cdat, errp); } else { - ct3_build_cdat(cdat, errp); + return ct3_build_cdat(cdat, errp); } } diff --git a/hw/mem/cxl_type3.c b/hw/mem/cxl_type3.c index b0a7e9f11b..3e42490b6c 100644 --- a/hw/mem/cxl_type3.c +++ b/hw/mem/cxl_type3.c @@ -705,8 +705,7 @@ static void ct3_realize(PCIDevice *pci_dev, Error **errp) cxl_cstate->cdat.build_cdat_table = ct3_build_cdat_table; cxl_cstate->cdat.free_cdat_table = ct3_free_cdat_table; cxl_cstate->cdat.private = ct3d; - cxl_doe_cdat_init(cxl_cstate, errp); - if (*errp) { + if (!cxl_doe_cdat_init(cxl_cstate, errp)) { goto err_free_special_ops; } diff --git a/hw/pci-bridge/cxl_upstream.c b/hw/pci-bridge/cxl_upstream.c index 783fa6adac..e51221a5f3 100644 --- a/hw/pci-bridge/cxl_upstream.c +++ b/hw/pci-bridge/cxl_upstream.c @@ -338,8 +338,7 @@ static void cxl_usp_realize(PCIDevice *d, Error **errp) cxl_cstate->cdat.build_cdat_table = build_cdat_table; cxl_cstate->cdat.free_cdat_table = free_default_cdat_table; cxl_cstate->cdat.private = d; - cxl_doe_cdat_init(cxl_cstate, errp); - if (*errp) { + if (!cxl_doe_cdat_init(cxl_cstate, errp)) { goto err_cap; } diff --git a/include/hw/cxl/cxl_component.h b/include/hw/cxl/cxl_component.h index 5012fab6f7..945ee6ffd0 100644 --- a/include/hw/cxl/cxl_component.h +++ b/include/hw/cxl/cxl_component.h @@ -273,7 +273,7 @@ hwaddr cxl_decode_ig(int ig); CXLComponentState *cxl_get_hb_cstate(PCIHostState *hb); bool cxl_get_hb_passthrough(PCIHostState *hb); -void cxl_doe_cdat_init(CXLComponentState *cxl_cstate, Error **errp); +bool cxl_doe_cdat_init(CXLComponentState *cxl_cstate, Error **errp); void cxl_doe_cdat_release(CXLComponentState *cxl_cstate); void cxl_doe_cdat_update(CXLComponentState *cxl_cstate, Error **errp); From 159fb790e48992f74644b0b0876615e8caacbfe4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Thu, 18 Apr 2024 16:49:16 +0200 Subject: [PATCH 15/22] hw/elf_ops: Rename elf_ops.h -> elf_ops.h.inc MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Since commit 139c1837db ("meson: rename included C source files to .c.inc"), QEMU standard procedure for included C files is to use *.c.inc. Besides, since commit 6a0057aa22 ("docs/devel: make a statement about includes") this is documented in the Coding Style: If you do use template header files they should be named with the ``.c.inc`` or ``.h.inc`` suffix to make it clear they are being included for expansion. Therefore rename "hw/elf_ops.h" as "hw/elf_ops.h.inc". Signed-off-by: Philippe Mathieu-Daudé Acked-by: Richard Henderson Message-Id: <20240424173333.96148-2-philmd@linaro.org> --- bsd-user/elfload.c | 2 +- hw/core/loader.c | 4 ++-- include/hw/{elf_ops.h => elf_ops.h.inc} | 0 linux-user/elfload.c | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) rename include/hw/{elf_ops.h => elf_ops.h.inc} (100%) diff --git a/bsd-user/elfload.c b/bsd-user/elfload.c index baf2f63d2f..833fa3bd05 100644 --- a/bsd-user/elfload.c +++ b/bsd-user/elfload.c @@ -383,7 +383,7 @@ static const char *lookup_symbolxx(struct syminfo *s, uint64_t orig_addr) return ""; } -/* FIXME: This should use elf_ops.h */ +/* FIXME: This should use elf_ops.h.inc */ static int symcmp(const void *s0, const void *s1) { struct elf_sym *sym0 = (struct elf_sym *)s0; diff --git a/hw/core/loader.c b/hw/core/loader.c index b8e52f3fb0..2f8105d7de 100644 --- a/hw/core/loader.c +++ b/hw/core/loader.c @@ -305,7 +305,7 @@ static void *load_at(int fd, off_t offset, size_t size) #define elf_word uint32_t #define elf_sword int32_t #define bswapSZs bswap32s -#include "hw/elf_ops.h" +#include "hw/elf_ops.h.inc" #undef elfhdr #undef elf_phdr @@ -327,7 +327,7 @@ static void *load_at(int fd, off_t offset, size_t size) #define elf_sword int64_t #define bswapSZs bswap64s #define SZ 64 -#include "hw/elf_ops.h" +#include "hw/elf_ops.h.inc" const char *load_elf_strerror(ssize_t error) { diff --git a/include/hw/elf_ops.h b/include/hw/elf_ops.h.inc similarity index 100% rename from include/hw/elf_ops.h rename to include/hw/elf_ops.h.inc diff --git a/linux-user/elfload.c b/linux-user/elfload.c index f4a0b78c75..a0999dac15 100644 --- a/linux-user/elfload.c +++ b/linux-user/elfload.c @@ -3572,7 +3572,7 @@ static const char *lookup_symbolxx(struct syminfo *s, uint64_t orig_addr) return ""; } -/* FIXME: This should use elf_ops.h */ +/* FIXME: This should use elf_ops.h.inc */ static int symcmp(const void *s0, const void *s1) { struct elf_sym *sym0 = (struct elf_sym *)s0; From 206e562c5aa5db55362bff0e88ba220250858f35 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Thu, 18 Apr 2024 17:07:03 +0200 Subject: [PATCH 16/22] hw/xtensa: Include missing 'exec/cpu-common.h' in 'bootparam.h' MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit cpu_physical_memory_write() is declared in "exec/cpu-common.h". Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Anton Johansson Message-Id: <20240418192525.97451-21-philmd@linaro.org> --- hw/xtensa/bootparam.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/hw/xtensa/bootparam.h b/hw/xtensa/bootparam.h index ade7891ec5..f57ff850bc 100644 --- a/hw/xtensa/bootparam.h +++ b/hw/xtensa/bootparam.h @@ -1,6 +1,8 @@ #ifndef HW_XTENSA_BOOTPARAM_H #define HW_XTENSA_BOOTPARAM_H +#include "exec/cpu-common.h" + #define BP_TAG_COMMAND_LINE 0x1001 /* command line (0-terminated string)*/ #define BP_TAG_INITRD 0x1002 /* ramdisk addr and size (bp_meminfo) */ #define BP_TAG_MEMORY 0x1003 /* memory addr and size (bp_meminfo) */ From 4f88e5215a74a97563c958d63634b107eff30c9f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?In=C3=A8s=20Varhol?= Date: Sun, 21 Apr 2024 16:14:23 +0200 Subject: [PATCH 17/22] hw/misc : Correct 5 spaces indents in stm32l4x5_exti MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Inès Varhol Reviewed-by: Philippe Mathieu-Daudé Message-ID: <20240421141455.116548-1-ines.varhol@telecom-paris.fr> Signed-off-by: Philippe Mathieu-Daudé --- hw/misc/stm32l4x5_exti.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/hw/misc/stm32l4x5_exti.c b/hw/misc/stm32l4x5_exti.c index 9fd859160d..5c55ee4268 100644 --- a/hw/misc/stm32l4x5_exti.c +++ b/hw/misc/stm32l4x5_exti.c @@ -59,22 +59,22 @@ static const uint32_t exti_romask[EXTI_NUM_REGISTER] = { static unsigned regbank_index_by_irq(unsigned irq) { - return irq >= EXTI_MAX_IRQ_PER_BANK ? 1 : 0; + return irq >= EXTI_MAX_IRQ_PER_BANK ? 1 : 0; } static unsigned regbank_index_by_addr(hwaddr addr) { - return addr >= EXTI_IMR2 ? 1 : 0; + return addr >= EXTI_IMR2 ? 1 : 0; } static unsigned valid_mask(unsigned bank) { - return MAKE_64BIT_MASK(0, irqs_per_bank[bank]); + return MAKE_64BIT_MASK(0, irqs_per_bank[bank]); } static unsigned configurable_mask(unsigned bank) { - return valid_mask(bank) & ~exti_romask[bank]; + return valid_mask(bank) & ~exti_romask[bank]; } static void stm32l4x5_exti_reset_hold(Object *obj) From f4b63768b91811cdcf1fb7b270587123251dfea5 Mon Sep 17 00:00:00 2001 From: Bernhard Beschow Date: Mon, 22 Apr 2024 22:06:22 +0200 Subject: [PATCH 18/22] hw/i386/pc_sysfw: Remove unused parameter from pc_isa_bios_init() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Bernhard Beschow Reviewed-by: Philippe Mathieu-Daudé Message-ID: <20240422200625.2768-2-shentey@gmail.com> Signed-off-by: Philippe Mathieu-Daudé --- hw/i386/pc_sysfw.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/hw/i386/pc_sysfw.c b/hw/i386/pc_sysfw.c index 3efabbbab2..87b5bf59d6 100644 --- a/hw/i386/pc_sysfw.c +++ b/hw/i386/pc_sysfw.c @@ -41,8 +41,7 @@ #define FLASH_SECTOR_SIZE 4096 static void pc_isa_bios_init(MemoryRegion *rom_memory, - MemoryRegion *flash_mem, - int ram_size) + MemoryRegion *flash_mem) { int isa_bios_size; MemoryRegion *isa_bios; @@ -186,7 +185,7 @@ static void pc_system_flash_map(PCMachineState *pcms, if (i == 0) { flash_mem = pflash_cfi01_get_memory(system_flash); - pc_isa_bios_init(rom_memory, flash_mem, size); + pc_isa_bios_init(rom_memory, flash_mem); /* Encrypt the pflash boot ROM */ if (sev_enabled()) { From dcba73b4453b7ed74d2ae24c5c8b273431c4484c Mon Sep 17 00:00:00 2001 From: Zhao Liu Date: Wed, 24 Apr 2024 23:49:09 +0800 Subject: [PATCH 19/22] hw/core/machine: Introduce the module as a CPU topology level MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In x86, module is the topology level above core, which contains a set of cores that share certain resources (in current products, the resource usually includes L2 cache, as well as module scoped features and MSRs). Though smp.clusters could also share the L2 cache resource [1], there are following reasons that drive us to introduce the new smp.modules: * As the CPU topology abstraction in device tree [2], cluster supports nesting (though currently QEMU hasn't support that). In contrast, (x86) module does not support nesting. * Due to nesting, there is great flexibility in sharing resources on cluster, rather than narrowing cluster down to sharing L2 (and L3 tags) as the lowest topology level that contains cores. * Flexible nesting of cluster allows it to correspond to any level between the x86 package and core. * In Linux kernel, x86's cluster only represents the L2 cache domain but QEMU's smp.clusters is the CPU topology level. Linux kernel will also expose module level topology information in sysfs for x86. To avoid cluster ambiguity and keep a consistent CPU topology naming style with the Linux kernel, we introduce module level for x86. The module is, in existing hardware practice, the lowest layer that contains the core, while the cluster is able to have a higher topological scope than the module due to its nesting. Therefore, place the module between the cluster and the core: drawer/book/socket/die/cluster/module/core/thread With the above topological hierarchy order, introduce module level support in MachineState and MachineClass. [1]: https://lore.kernel.org/qemu-devel/c3d68005-54e0-b8fe-8dc1-5989fe3c7e69@huawei.com/ [2]: https://www.kernel.org/doc/Documentation/devicetree/bindings/cpu/cpu-topology.txt Suggested-by: Xiaoyao Li Tested-by: Yongwei Ma Signed-off-by: Zhao Liu Tested-by: Babu Moger Message-ID: <20240424154929.1487382-2-zhao1.liu@intel.com> Signed-off-by: Philippe Mathieu-Daudé --- hw/core/machine-smp.c | 2 +- hw/core/machine.c | 1 + include/hw/boards.h | 4 ++++ 3 files changed, 6 insertions(+), 1 deletion(-) diff --git a/hw/core/machine-smp.c b/hw/core/machine-smp.c index 27864c9507..2e68fcfdfd 100644 --- a/hw/core/machine-smp.c +++ b/hw/core/machine-smp.c @@ -266,7 +266,7 @@ void machine_parse_smp_config(MachineState *ms, unsigned int machine_topo_get_cores_per_socket(const MachineState *ms) { - return ms->smp.cores * ms->smp.clusters * ms->smp.dies; + return ms->smp.cores * ms->smp.modules * ms->smp.clusters * ms->smp.dies; } unsigned int machine_topo_get_threads_per_socket(const MachineState *ms) diff --git a/hw/core/machine.c b/hw/core/machine.c index 582c2df37a..9966641159 100644 --- a/hw/core/machine.c +++ b/hw/core/machine.c @@ -1157,6 +1157,7 @@ static void machine_initfn(Object *obj) ms->smp.sockets = 1; ms->smp.dies = 1; ms->smp.clusters = 1; + ms->smp.modules = 1; ms->smp.cores = 1; ms->smp.threads = 1; diff --git a/include/hw/boards.h b/include/hw/boards.h index 69c1ba45cf..2fa800f11a 100644 --- a/include/hw/boards.h +++ b/include/hw/boards.h @@ -144,6 +144,7 @@ typedef struct { * provided SMP configuration * @books_supported - whether books are supported by the machine * @drawers_supported - whether drawers are supported by the machine + * @modules_supported - whether modules are supported by the machine */ typedef struct { bool prefer_sockets; @@ -152,6 +153,7 @@ typedef struct { bool has_clusters; bool books_supported; bool drawers_supported; + bool modules_supported; } SMPCompatProps; /** @@ -339,6 +341,7 @@ typedef struct DeviceMemoryState { * @sockets: the number of sockets in one book * @dies: the number of dies in one socket * @clusters: the number of clusters in one die + * @modules: the number of modules in one cluster * @cores: the number of cores in one cluster * @threads: the number of threads in one core * @max_cpus: the maximum number of logical processors on the machine @@ -350,6 +353,7 @@ typedef struct CpuTopology { unsigned int sockets; unsigned int dies; unsigned int clusters; + unsigned int modules; unsigned int cores; unsigned int threads; unsigned int max_cpus; From 8ec0a46347987c74464fe67c42030d074fe8c0f0 Mon Sep 17 00:00:00 2001 From: Zhao Liu Date: Wed, 24 Apr 2024 23:49:10 +0800 Subject: [PATCH 20/22] hw/core/machine: Support modules in -smp MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add "modules" parameter parsing support in -smp. Suggested-by: Xiaoyao Li Tested-by: Yongwei Ma Signed-off-by: Zhao Liu Tested-by: Babu Moger Acked-by: Markus Armbruster Message-ID: <20240424154929.1487382-3-zhao1.liu@intel.com> Signed-off-by: Philippe Mathieu-Daudé --- hw/core/machine-smp.c | 39 +++++++++++++++++++++++++++++++++------ hw/core/machine.c | 1 + qapi/machine.json | 3 +++ system/vl.c | 3 +++ 4 files changed, 40 insertions(+), 6 deletions(-) diff --git a/hw/core/machine-smp.c b/hw/core/machine-smp.c index 2e68fcfdfd..2b93fa99c9 100644 --- a/hw/core/machine-smp.c +++ b/hw/core/machine-smp.c @@ -51,6 +51,10 @@ static char *cpu_hierarchy_to_string(MachineState *ms) g_string_append_printf(s, " * clusters (%u)", ms->smp.clusters); } + if (mc->smp_props.modules_supported) { + g_string_append_printf(s, " * modules (%u)", ms->smp.modules); + } + g_string_append_printf(s, " * cores (%u)", ms->smp.cores); g_string_append_printf(s, " * threads (%u)", ms->smp.threads); @@ -88,6 +92,7 @@ void machine_parse_smp_config(MachineState *ms, unsigned sockets = config->has_sockets ? config->sockets : 0; unsigned dies = config->has_dies ? config->dies : 0; unsigned clusters = config->has_clusters ? config->clusters : 0; + unsigned modules = config->has_modules ? config->modules : 0; unsigned cores = config->has_cores ? config->cores : 0; unsigned threads = config->has_threads ? config->threads : 0; unsigned maxcpus = config->has_maxcpus ? config->maxcpus : 0; @@ -103,6 +108,7 @@ void machine_parse_smp_config(MachineState *ms, (config->has_sockets && config->sockets == 0) || (config->has_dies && config->dies == 0) || (config->has_clusters && config->clusters == 0) || + (config->has_modules && config->modules == 0) || (config->has_cores && config->cores == 0) || (config->has_threads && config->threads == 0) || (config->has_maxcpus && config->maxcpus == 0)) { @@ -115,6 +121,20 @@ void machine_parse_smp_config(MachineState *ms, * If not supported by the machine, a topology parameter must be * omitted. */ + if (!mc->smp_props.modules_supported && config->has_modules) { + if (config->modules > 1) { + error_setg(errp, "modules not supported by this " + "machine's CPU topology"); + return; + } else { + /* Here modules only equals 1 since we've checked zero case. */ + warn_report("Deprecated CPU topology (considered invalid): " + "Unsupported modules parameter mustn't be " + "specified as 1"); + } + } + modules = modules > 0 ? modules : 1; + if (!mc->smp_props.clusters_supported && config->has_clusters) { if (config->clusters > 1) { error_setg(errp, "clusters not supported by this " @@ -185,11 +205,13 @@ void machine_parse_smp_config(MachineState *ms, cores = cores > 0 ? cores : 1; threads = threads > 0 ? threads : 1; sockets = maxcpus / - (drawers * books * dies * clusters * cores * threads); + (drawers * books * dies * clusters * + modules * cores * threads); } else if (cores == 0) { threads = threads > 0 ? threads : 1; cores = maxcpus / - (drawers * books * sockets * dies * clusters * threads); + (drawers * books * sockets * dies * + clusters * modules * threads); } } else { /* prefer cores over sockets since 6.2 */ @@ -197,22 +219,26 @@ void machine_parse_smp_config(MachineState *ms, sockets = sockets > 0 ? sockets : 1; threads = threads > 0 ? threads : 1; cores = maxcpus / - (drawers * books * sockets * dies * clusters * threads); + (drawers * books * sockets * dies * + clusters * modules * threads); } else if (sockets == 0) { threads = threads > 0 ? threads : 1; sockets = maxcpus / - (drawers * books * dies * clusters * cores * threads); + (drawers * books * dies * clusters * + modules * cores * threads); } } /* try to calculate omitted threads at last */ if (threads == 0) { threads = maxcpus / - (drawers * books * sockets * dies * clusters * cores); + (drawers * books * sockets * dies * + clusters * modules * cores); } } - total_cpus = drawers * books * sockets * dies * clusters * cores * threads; + total_cpus = drawers * books * sockets * dies * + clusters * modules * cores * threads; maxcpus = maxcpus > 0 ? maxcpus : total_cpus; cpus = cpus > 0 ? cpus : maxcpus; @@ -222,6 +248,7 @@ void machine_parse_smp_config(MachineState *ms, ms->smp.sockets = sockets; ms->smp.dies = dies; ms->smp.clusters = clusters; + ms->smp.modules = modules; ms->smp.cores = cores; ms->smp.threads = threads; ms->smp.max_cpus = maxcpus; diff --git a/hw/core/machine.c b/hw/core/machine.c index 9966641159..494b712a76 100644 --- a/hw/core/machine.c +++ b/hw/core/machine.c @@ -881,6 +881,7 @@ static void machine_get_smp(Object *obj, Visitor *v, const char *name, .has_sockets = true, .sockets = ms->smp.sockets, .has_dies = true, .dies = ms->smp.dies, .has_clusters = true, .clusters = ms->smp.clusters, + .has_modules = true, .modules = ms->smp.modules, .has_cores = true, .cores = ms->smp.cores, .has_threads = true, .threads = ms->smp.threads, .has_maxcpus = true, .maxcpus = ms->smp.max_cpus, diff --git a/qapi/machine.json b/qapi/machine.json index 3e9cc3f17d..48f98e7d9f 100644 --- a/qapi/machine.json +++ b/qapi/machine.json @@ -1696,6 +1696,8 @@ # # @clusters: number of clusters per parent container (since 7.0) # +# @modules: number of modules per parent container (since 9.1) +# # @cores: number of cores per parent container # # @threads: number of threads per core @@ -1709,6 +1711,7 @@ '*sockets': 'int', '*dies': 'int', '*clusters': 'int', + '*modules': 'int', '*cores': 'int', '*threads': 'int', '*maxcpus': 'int' } } diff --git a/system/vl.c b/system/vl.c index c644222982..7756eac81e 100644 --- a/system/vl.c +++ b/system/vl.c @@ -741,6 +741,9 @@ static QemuOptsList qemu_smp_opts = { }, { .name = "clusters", .type = QEMU_OPT_NUMBER, + }, { + .name = "modules", + .type = QEMU_OPT_NUMBER, }, { .name = "cores", .type = QEMU_OPT_NUMBER, From 989bb312b021d66927db8e5f443503d63722b38d Mon Sep 17 00:00:00 2001 From: Zhao Liu Date: Wed, 24 Apr 2024 23:49:11 +0800 Subject: [PATCH 21/22] hw/core: Introduce module-id as the topology subindex MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add module-id in CpuInstanceProperties, to locate the CPU with module level. Suggested-by: Xiaoyao Li Tested-by: Yongwei Ma Signed-off-by: Zhao Liu Tested-by: Babu Moger Acked-by: Markus Armbruster Message-ID: <20240424154929.1487382-4-zhao1.liu@intel.com> Signed-off-by: Philippe Mathieu-Daudé --- hw/core/machine-hmp-cmds.c | 4 ++++ qapi/machine.json | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/hw/core/machine-hmp-cmds.c b/hw/core/machine-hmp-cmds.c index a6ff6a4875..8701f00cc7 100644 --- a/hw/core/machine-hmp-cmds.c +++ b/hw/core/machine-hmp-cmds.c @@ -87,6 +87,10 @@ void hmp_hotpluggable_cpus(Monitor *mon, const QDict *qdict) monitor_printf(mon, " cluster-id: \"%" PRIu64 "\"\n", c->cluster_id); } + if (c->has_module_id) { + monitor_printf(mon, " module-id: \"%" PRIu64 "\"\n", + c->module_id); + } if (c->has_core_id) { monitor_printf(mon, " core-id: \"%" PRIu64 "\"\n", c->core_id); } diff --git a/qapi/machine.json b/qapi/machine.json index 48f98e7d9f..bce6e1bbc4 100644 --- a/qapi/machine.json +++ b/qapi/machine.json @@ -988,6 +988,9 @@ # @cluster-id: cluster number within the parent container the CPU # belongs to (since 7.1) # +# @module-id: module number within the parent container the CPU belongs +# to (since 9.1) +# # @core-id: core number within the parent container the CPU belongs to # # @thread-id: thread number within the core the CPU belongs to @@ -1005,6 +1008,7 @@ '*socket-id': 'int', '*die-id': 'int', '*cluster-id': 'int', + '*module-id': 'int', '*core-id': 'int', '*thread-id': 'int' } From 098de99aad1aa911b4950b47b55d2e2bcc4f9c0c Mon Sep 17 00:00:00 2001 From: Zhao Liu Date: Wed, 24 Apr 2024 23:49:12 +0800 Subject: [PATCH 22/22] hw/core: Support module-id in numa configuration MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Module is a level above the core, thereby supporting numa configuration on the module level can bring user more numa flexibility. This is the natural further support for module level. Add module level support in numa configuration. Tested-by: Yongwei Ma Signed-off-by: Zhao Liu Tested-by: Babu Moger Message-ID: <20240424154929.1487382-5-zhao1.liu@intel.com> Signed-off-by: Philippe Mathieu-Daudé --- hw/core/machine.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/hw/core/machine.c b/hw/core/machine.c index 494b712a76..0dec48e802 100644 --- a/hw/core/machine.c +++ b/hw/core/machine.c @@ -800,6 +800,11 @@ void machine_set_cpu_numa_node(MachineState *machine, return; } + if (props->has_module_id && !slot->props.has_module_id) { + error_setg(errp, "module-id is not supported"); + return; + } + if (props->has_cluster_id && !slot->props.has_cluster_id) { error_setg(errp, "cluster-id is not supported"); return; @@ -824,6 +829,11 @@ void machine_set_cpu_numa_node(MachineState *machine, continue; } + if (props->has_module_id && + props->module_id != slot->props.module_id) { + continue; + } + if (props->has_cluster_id && props->cluster_id != slot->props.cluster_id) { continue; @@ -1226,6 +1236,12 @@ static char *cpu_slot_to_string(const CPUArchId *cpu) } g_string_append_printf(s, "cluster-id: %"PRId64, cpu->props.cluster_id); } + if (cpu->props.has_module_id) { + if (s->len) { + g_string_append_printf(s, ", "); + } + g_string_append_printf(s, "module-id: %"PRId64, cpu->props.module_id); + } if (cpu->props.has_core_id) { if (s->len) { g_string_append_printf(s, ", ");