Re: [PATCH 3/4] python/qmp-shell: relicense as LGPLv2+
Hi! On Fri, 25 Mar 2022 at 16:04, John Snow wrote: > > qmp-shell is presently licensed as GPLv2 (only). I intend to include > this tool as an add-on to an LGPLv2+ library package hosted on > PyPI.org. I've selected LGPLv2+ to maximize compatibility with other > licenses while retaining a copyleft license. > > To keep licensing matters simple, I'd like to relicense this tool as > LGPLv2+ as well in order to keep the resultant license of the hosted > release files simple -- even if library users won't "link against" this > command line tool. > > Therefore, I am asking permission from the current authors of this > tool to loosen the license. At present, those people are: > > - John Snow (me!), 411/609 > - Luiz Capitulino, Author, 97/609 > - Daniel Berrangé, 81/609 > - Eduardo Habkost, 10/609 > - Marc-André Lureau, 6/609 > - Fam Zheng, 3/609 > - Cleber Rosa, 1/609 > > (All of which appear to have been written under redhat.com addresses.) > > Eduardo's fixes are largely automated from 2to3 conversion tools and may > not necessarily constitute authorship, but his signature would put to > rest any questions. > > Cleber's changes concern a single import statement change. Also won't > hurt to ask. > > CC: Luiz Capitulino > CC: Daniel Berrange > CC: Eduardo Habkost > CC: Marc-André Lureau > CC: Fam Zheng > CC: Cleber Rosa > > Signed-off-by: John Snow Acked-by: Eduardo Habkost
Re: [PATCH] MAINTAINERS: python - remove ehabkost and add bleal
On Mon, 7 Feb 2022 at 19:05, John Snow wrote: > > Eduardo Habkost has left Red Hat and has other daily responsibilities to > attend to. In order to stop spamming him on every series, remove him as > "Reviewer" for the python/ library dir and add Beraldo Leal instead. > > For the "python scripts" stanza (which is separate due to level of > support), replace Eduardo as maintainer with myself. > > (Thanks for all of your hard work, Eduardo!) Thank you! And my apologies for not sending the MAINTAINERS patch myself. I'm being unable to deal with the amount of QEMU-related traffic directed to my email address. Acked-by: Eduardo Habkost -- Eduardo
Re: [PULL 0/1] MAINTAINERS update
On Wed, Dec 1, 2021 at 01:19 Richard Henderson wrote: > On 11/30/21 9:47 PM, Eduardo Habkost wrote: > > * MAINTAINERS: Change my email address (Eduardo Habkost) > > > > Eduardo Habkost (1): > >MAINTAINERS: Change my email address > > > > MAINTAINERS | 12 ++-- > > 1 file changed, 6 insertions(+), 6 deletions(-) > > > > Not a pull request. But since it's just one patch that needs no > regression testing, I > have applied it directly. Oops! I was in a hurry and used the wrong git-publish options, sorry about that, and thanks for applying it.
[PULL 0/1] MAINTAINERS update
* MAINTAINERS: Change my email address (Eduardo Habkost) Eduardo Habkost (1): MAINTAINERS: Change my email address MAINTAINERS | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) -- 2.32.0
[PULL 1/1] MAINTAINERS: Change my email address
The ehabk...@redhat.com email address will stop working on 2021-12-01, change it to my personal email address. Signed-off-by: Eduardo Habkost Message-Id: <20211129163053.2506734-1-ehabk...@redhat.com> Signed-off-by: Eduardo Habkost --- MAINTAINERS | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/MAINTAINERS b/MAINTAINERS index d3879aa3c12..7e8a586b2ae 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -324,7 +324,7 @@ F: disas/sparc.c X86 TCG CPUs M: Paolo Bonzini M: Richard Henderson -M: Eduardo Habkost +M: Eduardo Habkost S: Maintained F: target/i386/tcg/ F: tests/tcg/i386/ @@ -1628,7 +1628,7 @@ F: include/hw/i386/microvm.h F: pc-bios/bios-microvm.bin Machine core -M: Eduardo Habkost +M: Eduardo Habkost M: Marcel Apfelbaum R: Philippe Mathieu-Daudé S: Supported @@ -2648,13 +2648,13 @@ F: backends/cryptodev*.c Python library M: John Snow M: Cleber Rosa -R: Eduardo Habkost +R: Eduardo Habkost S: Maintained F: python/ T: git https://gitlab.com/jsnow/qemu.git python Python scripts -M: Eduardo Habkost +M: Eduardo Habkost M: Cleber Rosa S: Odd Fixes F: scripts/*.py @@ -2730,7 +2730,7 @@ T: git https://github.com/mdroth/qemu.git qga QOM M: Paolo Bonzini R: Daniel P. Berrange -R: Eduardo Habkost +R: Eduardo Habkost S: Supported F: docs/qdev-device-use.txt F: hw/core/qdev* @@ -2750,7 +2750,7 @@ F: tests/unit/check-qom-proplist.c F: tests/unit/test-qdev-global-props.c QOM boilerplate conversion script -M: Eduardo Habkost +M: Eduardo Habkost S: Maintained F: scripts/codeconverter/ -- 2.32.0
[PATCH] MAINTAINERS: Change my email address
The ehabk...@redhat.com email address will stop working on 2021-12-01, change it to my personal email address. Signed-off-by: Eduardo Habkost --- Note: I will probably step down as maintainer of some areas, but I will do this later because I will need a few weeks to figure out how much time I will be able to dedicate to QEMU. --- MAINTAINERS | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/MAINTAINERS b/MAINTAINERS index d3879aa3c12..7e8a586b2ae 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -324,7 +324,7 @@ F: disas/sparc.c X86 TCG CPUs M: Paolo Bonzini M: Richard Henderson -M: Eduardo Habkost +M: Eduardo Habkost S: Maintained F: target/i386/tcg/ F: tests/tcg/i386/ @@ -1628,7 +1628,7 @@ F: include/hw/i386/microvm.h F: pc-bios/bios-microvm.bin Machine core -M: Eduardo Habkost +M: Eduardo Habkost M: Marcel Apfelbaum R: Philippe Mathieu-Daudé S: Supported @@ -2648,13 +2648,13 @@ F: backends/cryptodev*.c Python library M: John Snow M: Cleber Rosa -R: Eduardo Habkost +R: Eduardo Habkost S: Maintained F: python/ T: git https://gitlab.com/jsnow/qemu.git python Python scripts -M: Eduardo Habkost +M: Eduardo Habkost M: Cleber Rosa S: Odd Fixes F: scripts/*.py @@ -2730,7 +2730,7 @@ T: git https://github.com/mdroth/qemu.git qga QOM M: Paolo Bonzini R: Daniel P. Berrange -R: Eduardo Habkost +R: Eduardo Habkost S: Supported F: docs/qdev-device-use.txt F: hw/core/qdev* @@ -2750,7 +2750,7 @@ F: tests/unit/check-qom-proplist.c F: tests/unit/test-qdev-global-props.c QOM boilerplate conversion script -M: Eduardo Habkost +M: Eduardo Habkost S: Maintained F: scripts/codeconverter/ -- 2.32.0
Re: [PATCH v3 3/3] hw/i386: expose a "smbios-entry-point-type" PC machine property
On Tue, Nov 02, 2021 at 07:25:25AM -0400, Michael S. Tsirkin wrote: > On Tue, Nov 02, 2021 at 09:51:35AM +0100, Philippe Mathieu-Daudé wrote: > > On 10/26/21 17:11, Eduardo Habkost wrote: > > > The i440fx and Q35 machine types are both hardcoded to use the > > > legacy SMBIOS 2.1 (32-bit) entry point. This is a sensible > > > conservative choice because SeaBIOS only supports SMBIOS 2.1 > > > > > > EDK2, however, can also support SMBIOS 3.0 (64-bit) entry points, > > > and QEMU already uses this on the ARM virt machine type. > > > > > > This adds a property to allow the choice of SMBIOS entry point > > > versions For example to opt in to 64-bit SMBIOS entry point: > > > > > >$QEMU -machine q35,smbios-entry-point-type=64 > > > > It would be nice to have a test for this... > > > > Otherwise, > > Reviewed-by: Philippe Mathieu-Daudé > > Can we update seabios and the switch the default? > Maybe just for q35? > Or are there more considerations? We can switch the default, but SeaBIOS maintainers won't include the SMBIOS 3.0 code I had submitted[1] until this is supported by QEMU. After we patch SeaBIOS to support SMBIOS 3.0 and update the SeaBIOS binaries in the QEMU tree, we can switch the default in Q35 and/or i440fx to SMBIOS 3.0. [1] https://www.mail-archive.com/seabios@seabios.org/msg12415.html https://www.mail-archive.com/seabios@seabios.org/msg12438.html > > > > > Based on a patch submitted by Daniel Berrangé. > > > > > > Signed-off-by: Daniel P. Berrangé > > > Signed-off-by: Eduardo Habkost > > > --- > > > This is patch was previously submitted at: > > > https://lore.kernel.org/qemu-devel/20200908165438.1008942-6-berra...@redhat.com > > > > > > Changes from v2: > > > * Rename "smbios-ep" to "smbios-entry-point-type" > > > > > > Changes from v1: > > > * Include qapi-visit-smbios.h instead of qapi-visit-machine.h > > > * Commit message fix: s/smbios_ep/smbios-ep/ > > > --- > > > include/hw/i386/pc.h | 4 > > > hw/i386/pc.c | 26 ++ > > > hw/i386/pc_piix.c| 2 +- > > > hw/i386/pc_q35.c | 2 +- > > > 4 files changed, 32 insertions(+), 2 deletions(-) > -- Eduardo
Re: [PATCH v3 0/3] Dynamic sysbus device check error report
On Fri, Oct 29, 2021 at 04:22:55PM +0200, Damien Hedde wrote: > Hi, > > Dynamic sysbus devices are allowed by a per-machine basis. > Right now, the allowance check is done during an machine_init_done > notifier, well after such devices are created. > > This series move the check at the right place (during the handling > of a QMP device_add command or -device CLI option) so that we can > report the error right away. > > This was initially part of my RFC (hence the v3) about allowing to > create devices during the machine initialized phase (link is below). > But it seems to me these patches make sense already as a standalone > cleanup. > > Only patch 1 miss a review. > > Thanks, > Damien > > v3: > + standalone series > + minor tweaks > > v2 was part of: > https://lists.gnu.org/archive/html/qemu-devel/2021-09/msg05683.html Acked-by: Eduardo Habkost -- Eduardo
[PULL 1/1] target/i386: Remove core-capability in Snowridge CPU model
From: Chenyi Qiang
Because core-capability releated features are model-specific and KVM
won't support it, remove the core-capability in CPU model to avoid the
warning message.
Signed-off-by: Chenyi Qiang
Message-Id: <20210827064818.4698-3-chenyi.qi...@intel.com>
Signed-off-by: Eduardo Habkost
---
target/i386/cpu.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index fc3ed80ef1e..598d451dcf0 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -3749,9 +3749,10 @@ static const X86CPUDefinition builtin_x86_defs[] = {
},
{
.version = 4,
-.note = "no split lock detect",
+.note = "no split lock detect, no core-capability",
.props = (PropValue[]) {
{ "split-lock-detect", "off" },
+{ "core-capability", "off" },
{ /* end of list */ },
},
},
--
2.32.0
[PULL 0/1] x86 CPU model queue, 2021-10-29
I should have flushed the queue a long time ago. Apologies for taking so long. A single patch, by now. Probably there are others I missed on the mailing list, and if necessary I will send another pull request. The following changes since commit a92cecba2791cd408d2bca04ce181dc2abaf9695: Merge remote-tracking branch 'remotes/rth/tags/pull-tcg-20211028' into staging (2021-10-29 08:39:44 -0700) are available in the Git repository at: https://gitlab.com/ehabkost/qemu.git tags/x86-next-pull-request for you to fetch changes up to 07db29f20a9a845c8408df11936889e5411ff44f: target/i386: Remove core-capability in Snowridge CPU model (2021-10-29 15:02:30 -0400) x86 queue, 2021-10-29 Bug fixes: * Remove core-capability in Snowridge CPU model Chenyi Qiang (1): target/i386: Remove core-capability in Snowridge CPU model target/i386/cpu.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) -- 2.32.0
Re: [PATCH 0/4] hw/core: Restrict qdev-hotplug to sysemu
On Thu, Oct 28, 2021 at 05:05:17PM +0200, Philippe Mathieu-Daudé wrote: > Restrict various hw/core/ files to sysemu, > add stubs for qdev-hotplug. > > Philippe Mathieu-Daudé (4): > hw/core: Restrict sysemu specific files > hw/core: Declare meson source set > hw/core: Extract hotplug-related functions to qdev-hotplug.c > hw/core: Restrict hotplug to system emulation Acked-by: Eduardo Habkost -- Eduardo
Re: [PATCH v4 0/2] hw/core/machine: Add an unit test for smp_parse
On Thu, Oct 28, 2021 at 05:09:11PM +0200, Philippe Mathieu-Daudé wrote: > Respin of Yanan Wang v3, based on > "hw/core: Restrict qdev-hotplug to sysemu" > > Based-on: 20211028150521.1973821-1-phi...@redhat.com > https://lore.kernel.org/qemu-devel/20211028150521.1973821-1-phi...@redhat.com > > git-backport-diff: > Key: > [] : patches are identical > [] : number of functional differences between upstream/downstream patch > [down] : patch is downstream-only > The flags [FC] indicate (F)unctional and (C)ontextual differences, > respectively > > 001/2:[0001] [FC] 'hw/core/machine: Split out the smp parsing code' > 002/2:[] [--] 'tests/unit: Add an unit test for smp parsing' Acked-by: Eduardo Habkost -- Eduardo
Re: [PATCH v3 1/3] smbios: Rename SMBIOS_ENTRY_POINT_* enums
On Wed, Oct 27, 2021 at 03:43:43AM -0400, Michael S. Tsirkin wrote: > On Tue, Oct 26, 2021 at 11:10:58AM -0400, Eduardo Habkost wrote: > > Rename the enums to match the naming style used by QAPI, and to > > use "32" and "64" instead of "20" and "31". This will allow us > > to more easily move the enum to the QAPI schema later. > > > > About the naming choice: "SMBIOS 2.1 entry point"/"SMBIO 3.0 > > typo in commit log I'll fix it, thanks! > > > entry point" and "32-bit entry point"/"64-bit entry point" are > > synonymous in the SMBIOS specification. However, the phrases > > "32-bit entry point" and "64-bit entry point" are used more often. > > > > The new names also avoid confusion between the entry point format > > and the actual SMBIOS version reported in the entry point > > structure. For example: currently the 32-bit entry point > > actually report SMBIOS 2.8 support, not 2.1. > > > > Based on portions of a patch submitted by Daniel P. Berrangé. > > I think you need the original S.O.B here too then. I'm not sure it is appropriate here, as zero lines of code from the original patch remain here. > > > > > Signed-off-by: Eduardo Habkost > > --- > > First version of this code was submitted at: > > https://lore.kernel.org/qemu-devel/20200908165438.1008942-5-berra...@redhat.com > > > > Changes from v2: > > * Use "32" and "64" instead of "2_0" and "3_1" > > > > Changes from v1: > > * Patch was split in two > > * Hunks included this patch are not changed from v1 -- Eduardo
[PATCH v3 3/3] hw/i386: expose a "smbios-entry-point-type" PC machine property
The i440fx and Q35 machine types are both hardcoded to use the
legacy SMBIOS 2.1 (32-bit) entry point. This is a sensible
conservative choice because SeaBIOS only supports SMBIOS 2.1
EDK2, however, can also support SMBIOS 3.0 (64-bit) entry points,
and QEMU already uses this on the ARM virt machine type.
This adds a property to allow the choice of SMBIOS entry point
versions For example to opt in to 64-bit SMBIOS entry point:
$QEMU -machine q35,smbios-entry-point-type=64
Based on a patch submitted by Daniel Berrangé.
Signed-off-by: Daniel P. Berrangé
Signed-off-by: Eduardo Habkost
---
This is patch was previously submitted at:
https://lore.kernel.org/qemu-devel/20200908165438.1008942-6-berra...@redhat.com
Changes from v2:
* Rename "smbios-ep" to "smbios-entry-point-type"
Changes from v1:
* Include qapi-visit-smbios.h instead of qapi-visit-machine.h
* Commit message fix: s/smbios_ep/smbios-ep/
---
include/hw/i386/pc.h | 4
hw/i386/pc.c | 26 ++
hw/i386/pc_piix.c| 2 +-
hw/i386/pc_q35.c | 2 +-
4 files changed, 32 insertions(+), 2 deletions(-)
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 11426e26dc3..95f7f55cdc6 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -13,6 +13,7 @@
#include "hw/hotplug.h"
#include "qom/object.h"
#include "hw/i386/sgx-epc.h"
+#include "hw/firmware/smbios.h"
#define HPET_INTCAP "hpet-intcap"
@@ -39,6 +40,7 @@ typedef struct PCMachineState {
/* Configuration options: */
uint64_t max_ram_below_4g;
OnOffAuto vmport;
+SmbiosEntryPointType smbios_entry_point_type;
bool acpi_build_enabled;
bool smbus_enabled;
@@ -62,6 +64,8 @@ typedef struct PCMachineState {
#define PC_MACHINE_SATA "sata"
#define PC_MACHINE_PIT "pit"
#define PC_MACHINE_MAX_FW_SIZE "max-fw-size"
+#define PC_MACHINE_SMBIOS_EP"smbios-entry-point-type"
+
/**
* PCMachineClass:
*
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 86223acfd34..bbeae19fa2f 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -77,6 +77,7 @@
#include "hw/mem/nvdimm.h"
#include "qapi/error.h"
#include "qapi/qapi-visit-common.h"
+#include "qapi/qapi-visit-machine.h"
#include "qapi/visitor.h"
#include "hw/core/cpu.h"
#include "hw/usb.h"
@@ -1494,6 +1495,23 @@ static void
pc_machine_set_default_bus_bypass_iommu(Object *obj, bool value,
pcms->default_bus_bypass_iommu = value;
}
+static void pc_machine_get_smbios_ep(Object *obj, Visitor *v, const char *name,
+ void *opaque, Error **errp)
+{
+PCMachineState *pcms = PC_MACHINE(obj);
+SmbiosEntryPointType smbios_entry_point_type =
pcms->smbios_entry_point_type;
+
+visit_type_SmbiosEntryPointType(v, name, _entry_point_type, errp);
+}
+
+static void pc_machine_set_smbios_ep(Object *obj, Visitor *v, const char *name,
+ void *opaque, Error **errp)
+{
+PCMachineState *pcms = PC_MACHINE(obj);
+
+visit_type_SmbiosEntryPointType(v, name, >smbios_entry_point_type,
errp);
+}
+
static void pc_machine_get_max_ram_below_4g(Object *obj, Visitor *v,
const char *name, void *opaque,
Error **errp)
@@ -1584,6 +1602,8 @@ static void pc_machine_initfn(Object *obj)
pcms->vmport = ON_OFF_AUTO_OFF;
#endif /* CONFIG_VMPORT */
pcms->max_ram_below_4g = 0; /* use default */
+pcms->smbios_entry_point_type = SMBIOS_ENTRY_POINT_TYPE_32;
+
/* acpi build is enabled by default if machine supports it */
pcms->acpi_build_enabled = PC_MACHINE_GET_CLASS(pcms)->has_acpi_build;
pcms->smbus_enabled = true;
@@ -1727,6 +1747,12 @@ static void pc_machine_class_init(ObjectClass *oc, void
*data)
NULL, NULL);
object_class_property_set_description(oc, PC_MACHINE_MAX_FW_SIZE,
"Maximum combined firmware size");
+
+object_class_property_add(oc, PC_MACHINE_SMBIOS_EP, "str",
+pc_machine_get_smbios_ep, pc_machine_set_smbios_ep,
+NULL, NULL);
+object_class_property_set_description(oc, PC_MACHINE_SMBIOS_EP,
+"SMBIOS Entry Point type [32, 64]");
}
static const TypeInfo pc_machine_info = {
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 17c050694f5..45e3c760915 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -177,7 +177,7 @@ static void pc_init1(MachineState *machine,
smbios_set_defaults("QEMU", "Standard PC (i440FX + PIIX, 1996)",
mc->name, pcmc->smbios_legacy_mode,
pcmc->smbios_uuid_encoded,
-SMBIOS_ENTRY_POINT_TYPE_32);
+
[PATCH v3 2/3] hw/smbios: Use qapi for SmbiosEntryPointType
This prepares for exposing the SMBIOS entry point type as a
machine property on x86.
Based on a patch from Daniel P. Berrangé.
Signed-off-by: Daniel P. Berrangé
Signed-off-by: Eduardo Habkost
---
First version of this code was submitted at:
https://lore.kernel.org/qemu-devel/20200908165438.1008942-5-berra...@redhat.com
Changes from v2:
* Rename "2_0"/"3_1" to "32"/"64", to
make the names more QAPI-friendly (as underscores and dots are
not allowed by QAPI)
* Move definition from smbios.json back to machine.json
(no need for a separate file just for one enum)
Changes from v1:
* Patch was split in two
* Moved definition to smbios.json
---
include/hw/firmware/smbios.h | 10 ++
qapi/machine.json| 12
2 files changed, 14 insertions(+), 8 deletions(-)
diff --git a/include/hw/firmware/smbios.h b/include/hw/firmware/smbios.h
index d916baed6a9..4b7ad77a44f 100644
--- a/include/hw/firmware/smbios.h
+++ b/include/hw/firmware/smbios.h
@@ -1,6 +1,8 @@
#ifndef QEMU_SMBIOS_H
#define QEMU_SMBIOS_H
+#include "qapi/qapi-types-machine.h"
+
/*
* SMBIOS Support
*
@@ -23,14 +25,6 @@ struct smbios_phys_mem_area {
uint64_t length;
};
-/*
- * SMBIOS spec defined tables
- */
-typedef enum SmbiosEntryPointType {
-SMBIOS_ENTRY_POINT_TYPE_32,
-SMBIOS_ENTRY_POINT_TYPE_64,
-} SmbiosEntryPointType;
-
/* SMBIOS Entry Point
* There are two types of entry points defined in the SMBIOS specification
* (see below). BIOS must place the entry point(s) at a 16-byte-aligned
diff --git a/qapi/machine.json b/qapi/machine.json
index 5db54df298f..0a13579275f 100644
--- a/qapi/machine.json
+++ b/qapi/machine.json
@@ -1411,3 +1411,15 @@
'*cores': 'int',
'*threads': 'int',
'*maxcpus': 'int' } }
+
+##
+# @SmbiosEntryPointType:
+#
+# @32: SMBIOS version 2.1 (32-bit) Entry Point
+#
+# @64: SMBIOS version 3.0 (64-bit) Entry Point
+#
+# Since: 6.1
+##
+{ 'enum': 'SmbiosEntryPointType',
+ 'data': [ '32', '64' ] }
--
2.32.0
[PATCH v3 1/3] smbios: Rename SMBIOS_ENTRY_POINT_* enums
Rename the enums to match the naming style used by QAPI, and to
use "32" and "64" instead of "20" and "31". This will allow us
to more easily move the enum to the QAPI schema later.
About the naming choice: "SMBIOS 2.1 entry point"/"SMBIO 3.0
entry point" and "32-bit entry point"/"64-bit entry point" are
synonymous in the SMBIOS specification. However, the phrases
"32-bit entry point" and "64-bit entry point" are used more often.
The new names also avoid confusion between the entry point format
and the actual SMBIOS version reported in the entry point
structure. For example: currently the 32-bit entry point
actually report SMBIOS 2.8 support, not 2.1.
Based on portions of a patch submitted by Daniel P. Berrangé.
Signed-off-by: Eduardo Habkost
---
First version of this code was submitted at:
https://lore.kernel.org/qemu-devel/20200908165438.1008942-5-berra...@redhat.com
Changes from v2:
* Use "32" and "64" instead of "2_0" and "3_1"
Changes from v1:
* Patch was split in two
* Hunks included this patch are not changed from v1
---
include/hw/firmware/smbios.h | 4 ++--
hw/arm/virt.c| 2 +-
hw/i386/pc_piix.c| 2 +-
hw/i386/pc_q35.c | 2 +-
hw/smbios/smbios.c | 8
5 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/include/hw/firmware/smbios.h b/include/hw/firmware/smbios.h
index 5a0dd0c8cff..d916baed6a9 100644
--- a/include/hw/firmware/smbios.h
+++ b/include/hw/firmware/smbios.h
@@ -27,8 +27,8 @@ struct smbios_phys_mem_area {
* SMBIOS spec defined tables
*/
typedef enum SmbiosEntryPointType {
-SMBIOS_ENTRY_POINT_21,
-SMBIOS_ENTRY_POINT_30,
+SMBIOS_ENTRY_POINT_TYPE_32,
+SMBIOS_ENTRY_POINT_TYPE_64,
} SmbiosEntryPointType;
/* SMBIOS Entry Point
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index ca433adb5b1..2bd73d501da 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -1589,7 +1589,7 @@ static void virt_build_smbios(VirtMachineState *vms)
smbios_set_defaults("QEMU", product,
vmc->smbios_old_sys_ver ? "1.0" : mc->name, false,
-true, SMBIOS_ENTRY_POINT_30);
+true, SMBIOS_ENTRY_POINT_TYPE_64);
smbios_get_tables(MACHINE(vms), NULL, 0,
_tables, _tables_len,
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 6ad0d763c57..17c050694f5 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -177,7 +177,7 @@ static void pc_init1(MachineState *machine,
smbios_set_defaults("QEMU", "Standard PC (i440FX + PIIX, 1996)",
mc->name, pcmc->smbios_legacy_mode,
pcmc->smbios_uuid_encoded,
-SMBIOS_ENTRY_POINT_21);
+SMBIOS_ENTRY_POINT_TYPE_32);
}
/* allocate ram and load rom/bios */
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index fcc6e4eb2b8..48419ebfd5f 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -199,7 +199,7 @@ static void pc_q35_init(MachineState *machine)
smbios_set_defaults("QEMU", "Standard PC (Q35 + ICH9, 2009)",
mc->name, pcmc->smbios_legacy_mode,
pcmc->smbios_uuid_encoded,
-SMBIOS_ENTRY_POINT_21);
+SMBIOS_ENTRY_POINT_TYPE_32);
}
/* allocate ram and load rom/bios */
diff --git a/hw/smbios/smbios.c b/hw/smbios/smbios.c
index 7397e567373..6013df1698e 100644
--- a/hw/smbios/smbios.c
+++ b/hw/smbios/smbios.c
@@ -62,7 +62,7 @@ uint8_t *smbios_tables;
size_t smbios_tables_len;
unsigned smbios_table_max;
unsigned smbios_table_cnt;
-static SmbiosEntryPointType smbios_ep_type = SMBIOS_ENTRY_POINT_21;
+static SmbiosEntryPointType smbios_ep_type = SMBIOS_ENTRY_POINT_TYPE_32;
static SmbiosEntryPoint ep;
@@ -432,7 +432,7 @@ static void smbios_validate_table(MachineState *ms)
exit(1);
}
-if (smbios_ep_type == SMBIOS_ENTRY_POINT_21 &&
+if (smbios_ep_type == SMBIOS_ENTRY_POINT_TYPE_32 &&
smbios_tables_len > SMBIOS_21_MAX_TABLES_LEN) {
error_report("SMBIOS 2.1 table length %zu exceeds %d",
smbios_tables_len, SMBIOS_21_MAX_TABLES_LEN);
@@ -927,7 +927,7 @@ void smbios_set_defaults(const char *manufacturer, const
char *product,
static void smbios_entry_point_setup(void)
{
switch (smbios_ep_type) {
-case SMBIOS_ENTRY_POINT_21:
+case SMBIOS_ENTRY_POINT_TYPE_32:
memcpy(ep.ep21.anchor_string, "_SM_", 4);
memcpy(ep.ep21.intermediate_anchor_string, "_DMI_", 5);
ep.ep21.length = sizeof(struct smbios_21_entry_point);
@@ -950,7 +950,7 @@ static void smbios_entry_point_
[PATCH v3 0/3] pc: Support configuration of SMBIOS entry point type
This includes code previously submitted[1] by Daniel P. Berrangé to add a "smbios-ep" machine property on PC. SMBIOS 3.0 is necessary to support more than ~720 VCPUs, as a large number of VCPUs can easily hit the table size limit of SMBIOS 2.1 entry points. Changes from v2: * Renamed option to "smbios-entry-point-type" for clarity * Renamed option values to "32" and "64", for two reasons: * The option is not about reporting an exact SMBIOS version, but just the entry point format. FWIW, the SMBIOS specification uses the phrases "32-bit entry point" and "64-bit entry point" more often than "2.1 entry point" and "3.0 entry point". * QAPI doesn't allow us to use enum member names with dots or underscores [1] https://lore.kernel.org/qemu-devel/20200908165438.1008942-5-berra...@redhat.com https://lore.kernel.org/qemu-devel/20200908165438.1008942-6-berra...@redhat.com Eduardo Habkost (3): smbios: Rename SMBIOS_ENTRY_POINT_* enums hw/smbios: Use qapi for SmbiosEntryPointType hw/i386: expose a "smbios-entry-point-type" PC machine property include/hw/firmware/smbios.h | 10 ++ include/hw/i386/pc.h | 4 hw/arm/virt.c| 2 +- hw/i386/pc.c | 26 ++ hw/i386/pc_piix.c| 2 +- hw/i386/pc_q35.c | 2 +- hw/smbios/smbios.c | 8 qapi/machine.json| 12 8 files changed, 51 insertions(+), 15 deletions(-) -- 2.32.0
Re: [PATCH v3 03/22] host-utils: introduce uabs64()
On Thu, Oct 21, 2021 at 4:04 PM Richard Henderson wrote: > > On 9/10/21 4:26 AM, Luis Pires wrote: > > Introduce uabs64(), a function that returns the absolute value of > > a 64-bit int as an unsigned value. This avoids the undefined behavior > > for common abs implementations, where abs of the most negative value is > > undefined. > > I do question the comment there wrt undefined. We compile with -fwrapv, which > means that > *no* overflow is undefined; we always have properly truncated twos-compliment > values. Can we really assume that -fwrapv would make llabs(LLONG_MIN) not undefined? We would be calling a function compiled by somebody else (possibly without -fwrapv). -- Eduardo
Re: [PATCH] hw/qdev-core: Add compatibility for (non)-transitional devs
On Wed, Oct 20, 2021 at 10:58:12AM -0400, Michael S. Tsirkin wrote:
> On Wed, Oct 20, 2021 at 10:09:17AM -0400, Eduardo Habkost wrote:
> > On Wed, Oct 20, 2021 at 01:02:24PM +0800, Jason Wang wrote:
> > > On Wed, Oct 20, 2021 at 9:31 AM Jason Wang wrote:
> > > >
> > > > On Wed, Oct 20, 2021 at 12:56 AM Eduardo Habkost
> > > > wrote:
> > > > >
> > > > > On Tue, Oct 19, 2021 at 12:13:17PM -0400, Michael S. Tsirkin wrote:
> > > > > > On Tue, Oct 19, 2021 at 11:29:13AM -0400, Eduardo Habkost wrote:
> > > > > > > On Tue, Oct 19, 2021 at 06:59:09AM -0400, Michael S. Tsirkin
> > > > > > > wrote:
> > > > > > > > On Tue, Oct 19, 2021 at 11:46:17AM +0100, Stefan Hajnoczi wrote:
> > > > > > > > > On Tue, Oct 12, 2021 at 10:36:01AM +0200, Jean-Louis Dupond
> > > > > > > > > wrote:
> > > > > > > > > > Forgot to CC maintainers.
> > > > > > > > >
> > > > > > > > > Also CCing Jason Wang and Michael Tsirkin for VIRTIO.
> > > > > > > > >
> > > > > > > > > Stefan
> > > > > > > >
> > > > > > > > OMG
> > > > > > > > where all compat properties broken all the time?
> > > > > > >
> > > > > > > Compat properties that existed when commit f6e501a28ef9 ("virtio:
> > > > > > > Provide version-specific variants of virtio PCI devices") was
> > > > > > > merged are not broken, because virtio-*-transitional and
> > > > > > > virtio-*-non-transitional were brand new QOM types (so there's no
> > > > > > > compatibility to be kept with old QEMU versions).
> > > > > > >
> > > > > > > Compat properties referencing "virtio-*-pci" instead of
> > > > > > > "virtio-*-pci-base" added after commit f6e501a28ef9 are probably
> > > > > > > broken, yes.
> > > > > > >
> > > > > > > --
> > > > > > > Eduardo
> > > > > >
> > > > > > Oh. So just this one:
> > > > > > { "virtio-net-pci", "vectors", "3"},
> > > > > >
> > > > > > right?
> > > > >
> > > > > I think so. That's the only post-4.0 virtio-*-pci compat property I
> > > > > see in
> > > > > hw/core/machine.c.
> > > > >
> > > > > pc.c doesn't have any post-4.0 virtio-*-pci compat props. I didn't
> > > > > see any
> > > > > virtio compat props on spapr.c and s390-virtio-ccw.c.
> > > > >
> > > > > >
> > > > > > about the patch: how do people feel about virtio specific
> > > > > > stuff in qdev core? Ok by everyone?
> > > > >
> > > > > Not OK, if we have a mechanism to avoid that, already (the
> > > > > "virtio-net-pci-base" type name). I wonder what we can do to
> > > > > make this kind of mistake less likely, though.
> > > > >
> > > > > Jean-Louis, Jason, does the following fix work?
> > > >
> > > > Yes.
> > > >
> > > > Acked-by: Jason Wang
> > > >
> > > > Thanks
> > > >
> > > > >
> > > > > Signed-off-by: Eduardo Habkost
> > > > > ---
> > > > > diff --git a/hw/core/machine.c b/hw/core/machine.c
> > > > > index b8d95eec32d..bd9c6156c1a 100644
> > > > > --- a/hw/core/machine.c
> > > > > +++ b/hw/core/machine.c
> > > > > @@ -56,7 +56,7 @@ GlobalProperty hw_compat_5_2[] = {
> > > > > { "ICH9-LPC", "smm-compat", "on"},
> > > > > { "PIIX4_PM", "smm-compat", "on"},
> > > > > { "virtio-blk-device", "report-discard-granularity", "off" },
> > > > > -{ "virtio-net-pci", "vectors", "3"},
> > > > > +{ "virtio-net-pci-base", "vectors", "3"},
> > >
> > > Rethink about this, any chance that we can use "virtio-net-pci" as the
> > > base_name? It looks to me this can cause less confusion and consis
Re: [PATCH] via-ide: Avoid expensive operations in irq handler
On Wed, Oct 20, 2021 at 04:58:58PM +0200, BALATON Zoltan wrote:
> On Wed, 20 Oct 2021, Eduardo Habkost wrote:
> > On Mon, Oct 18, 2021 at 12:10:04PM +0200, Philippe Mathieu-Daudé wrote:
> > > On 10/18/21 11:51, BALATON Zoltan wrote:
> > > > On Mon, 18 Oct 2021, Philippe Mathieu-Daudé wrote:
> > > > > On 10/18/21 03:36, BALATON Zoltan wrote:
> > > > > > Cache the pointer to PCI function 0 (ISA bridge, that this IDE
> > > > > > device
> > > > > > has to use for IRQs) in the PCIIDEState and pass that as the opaque
> > > > > > data for the interrupt handler to eliminate both the need to look up
> > > > > > function 0 at every interrupt and also a QOM type cast of the opaque
> > > > > > pointer as that's also expensive (mainly due to qom-debug being
> > > > > > enabled by default).
> > > > > >
> > > > > > Suggested-by: Philippe Mathieu-Daudé
> > > > > > Signed-off-by: BALATON Zoltan
> > > > > > ---
> > > > > > hw/ide/via.c | 11 ++-
> > > > > > include/hw/ide/pci.h | 1 +
> > > > > > 2 files changed, 7 insertions(+), 5 deletions(-)
> > > > > >
> > > > > > diff --git a/hw/ide/via.c b/hw/ide/via.c
> > > > > > index 82def819c4..30566bc409 100644
> > > > > > --- a/hw/ide/via.c
> > > > > > +++ b/hw/ide/via.c
> > > > > > @@ -104,15 +104,15 @@ static void bmdma_setup_bar(PCIIDEState *d)
> > > > > >
> > > > > > static void via_ide_set_irq(void *opaque, int n, int level)
> > > > > > {
> > > > > > - PCIDevice *d = PCI_DEVICE(opaque);
> > > > > > + PCIIDEState *d = opaque;
> > > > > >
> > > > > > if (level) {
> > > > > > - d->config[0x70 + n * 8] |= 0x80;
> > > > > > + d->parent_obj.config[0x70 + n * 8] |= 0x80;
> > > > > > } else {
> > > > > > - d->config[0x70 + n * 8] &= ~0x80;
> > > > > > + d->parent_obj.config[0x70 + n * 8] &= ~0x80;
> > > > > > }
> > > > >
> > > > > Better not to access parent_obj, so I'd rather keep the previous
> > > > > code as it. The rest is OK, thanks. If you don't want to respin
> > > > > I can fix and take via mips-next.
> > > >
> > > > Why not? If it's OK to access other fields why not parent_obj? That
> > > > avoids the QOM cast PCI_DEVICE(opaque) or PCI_DEVICE(d) after this
> > > > patch. We know it's a PCIIDEState and has PCIDevice parent_obj field
> > > > because we set the opaque pointer when adding this callback so I think
> > > > this works and is the less expensive way. But feel free to change it any
> > > > way you like and use it that way. I'd keep it as it is.
> > >
> > > My understanding of QOM is we shouldn't access internal states that
> > > way, because 1/ this makes object refactors harder and 2/ this is
> > > not the style/example we want in the codebase, but it doesn't seem
> > > documented, so Cc'ing Markus/Eduardo for confirmation.
> >
> > `PCI_DEVICE(d)` is preferred instead `of d.parent_obj` (parent_obj is
> > just a QOM implementation detail). If there are real performance
> > reasons to avoid it, we need numbers to support that.
>
> OK I can try to do some measurement or go back to PCI_DEVICE() then.
>
> > Also, note that `OBJECT_CHECK(obj)` is just `return obj` if
> > CONFIG_QOM_CAST_DEBUG is disabled.
>
> But configure enables it by default even without --enable-debug so I think
> most people don't even know and it's enabled practically everywhere
> (probably even in distros). Why can't we have it disabled by default if it's
> a developer debug option and enable it only for developers where it's useful
> to catch bugs?
It's a trade-off, I don't think there's a right answer for
everybody. Personally, I'd say the benefits outweigh the risks
of disabling it for most users (especially the ones building QEMU
directly from source).
I don't mean to say it's OK for CONFIG_QOM_CAST_DEBUG=y builds to
have visibly poor performance. If the typecast above causes a
measurable performance impact, it might be reasonable to have a
workaround like:
static void via_ide_set_irq(void *opaque, int n, int level)
{
PCIIDEState *ide = opaque;
PCIDevice *pci = opaque;
...
}
--
Eduardo
Re: [PATCH] hw/qdev-core: Add compatibility for (non)-transitional devs
On Wed, Oct 20, 2021 at 10:55:59AM -0400, Michael S. Tsirkin wrote:
> On Wed, Oct 20, 2021 at 09:57:37AM -0400, Eduardo Habkost wrote:
> > On Wed, Oct 20, 2021 at 03:41:38AM -0400, Michael S. Tsirkin wrote:
> > > On Tue, Oct 19, 2021 at 12:56:11PM -0400, Eduardo Habkost wrote:
> > > > On Tue, Oct 19, 2021 at 12:13:17PM -0400, Michael S. Tsirkin wrote:
> > > > > On Tue, Oct 19, 2021 at 11:29:13AM -0400, Eduardo Habkost wrote:
> > > > > > On Tue, Oct 19, 2021 at 06:59:09AM -0400, Michael S. Tsirkin wrote:
> > > > > > > On Tue, Oct 19, 2021 at 11:46:17AM +0100, Stefan Hajnoczi wrote:
> > > > > > > > On Tue, Oct 12, 2021 at 10:36:01AM +0200, Jean-Louis Dupond
> > > > > > > > wrote:
> > > > > > > > > Forgot to CC maintainers.
> > > > > > > >
> > > > > > > > Also CCing Jason Wang and Michael Tsirkin for VIRTIO.
> > > > > > > >
> > > > > > > > Stefan
> > > > > > >
> > > > > > > OMG
> > > > > > > where all compat properties broken all the time?
> > > > > >
> > > > > > Compat properties that existed when commit f6e501a28ef9 ("virtio:
> > > > > > Provide version-specific variants of virtio PCI devices") was
> > > > > > merged are not broken, because virtio-*-transitional and
> > > > > > virtio-*-non-transitional were brand new QOM types (so there's no
> > > > > > compatibility to be kept with old QEMU versions).
> > > > > >
> > > > > > Compat properties referencing "virtio-*-pci" instead of
> > > > > > "virtio-*-pci-base" added after commit f6e501a28ef9 are probably
> > > > > > broken, yes.
> > > > > >
> > > > > > --
> > > > > > Eduardo
> > > > >
> > > > > Oh. So just this one:
> > > > > { "virtio-net-pci", "vectors", "3"},
> > > > >
> > > > > right?
> > > >
> > > > I think so. That's the only post-4.0 virtio-*-pci compat property I
> > > > see in
> > > > hw/core/machine.c.
> > > >
> > > > pc.c doesn't have any post-4.0 virtio-*-pci compat props. I didn't see
> > > > any
> > > > virtio compat props on spapr.c and s390-virtio-ccw.c.
> > > >
> > > > >
> > > > > about the patch: how do people feel about virtio specific
> > > > > stuff in qdev core? Ok by everyone?
> > > >
> > > > Not OK, if we have a mechanism to avoid that, already (the
> > > > "virtio-net-pci-base" type name). I wonder what we can do to
> > > > make this kind of mistake less likely, though.
> > > >
> > > > Jean-Louis, Jason, does the following fix work?
> > > >
> > > > Signed-off-by: Eduardo Habkost
> > > > ---
> > > > diff --git a/hw/core/machine.c b/hw/core/machine.c
> > > > index b8d95eec32d..bd9c6156c1a 100644
> > > > --- a/hw/core/machine.c
> > > > +++ b/hw/core/machine.c
> > > > @@ -56,7 +56,7 @@ GlobalProperty hw_compat_5_2[] = {
> > > > { "ICH9-LPC", "smm-compat", "on"},
> > > > { "PIIX4_PM", "smm-compat", "on"},
> > > > { "virtio-blk-device", "report-discard-granularity", "off" },
> > > > -{ "virtio-net-pci", "vectors", "3"},
> > > > +{ "virtio-net-pci-base", "vectors", "3"},
> > > > };
> > > > const size_t hw_compat_5_2_len = G_N_ELEMENTS(hw_compat_5_2);
> > >
> > > Hmm I'm a bit confused at this point, as to why does
> > > specifying properties for virtio-net-pci on command
> > > line with -global work, but in compat list doesn't. Do others
> > > understand?
> >
> > I don't think that's the case. -global behaves similarly to compat_props.
> >
> > Running an unpatched QEMU 6.1.0 binary:
> >
> > $ echo -e 'info qtree\nquit' | qemu-system-x86_64 -device virtio-net-pci
> > -machine pc-q35-5.2 -monitor stdio | grep vectors
> > vectors = 3 (0x3)
> > $ echo -e 'info qtree\nquit' | qemu-system-x86_64 -device
> > virtio-net-pci-non-transitional -machine pc-q35-5.2 -monitor stdio | grep
> > vectors
> > vectors = 4 (0x4)
> > $ echo -e 'info qtree\nquit' | qemu-system-x86_64 -device
> > virtio-net-pci-non-transitional -global virtio-net-pci.vectors=3 -monitor
> > stdio | grep vectors
> > vectors = 4 (0x4)
> > $ echo -e 'info qtree\nquit' | qemu-system-x86_64 -device
> > virtio-net-pci-non-transitional -global virtio-net-pci-base.vectors=3
> > -monitor stdio | grep vectors
> > vectors = 3 (0x3)
>
> OK so ... that's another breakage then. Suggestions how to fix?
What exactly is another breakage? virtio-net-pci,
virtio-net-pci-non-transitional, and virtio-net-pci-transitional
are three distinct device types.
--
Eduardo
Re: [RFC PATCH 3/5] target/i386: Enable host pass through of Intel PT
On Mon, Oct 18, 2021 at 01:37:12PM +0800, Xiaoyao Li wrote: > On 10/18/2021 11:46 AM, Xiaoyao Li wrote: > > On 10/16/2021 4:22 AM, Eduardo Habkost wrote: > > > On Thu, Sep 09, 2021 at 10:41:48PM +0800, Xiaoyao Li wrote: [...] > > > > +#define INTEL_PT_DEFAULT_0_EBX (CPUID_14_0_EBX_CR3_FILTER | \ > > > > + CPUID_14_0_EBX_PSB | CPUID_14_0_EBX_IP_FILTER | > > > > CPUID_14_0_EBX_MTC) > > > > +#define INTEL_PT_DEFAULT_0_ECX (CPUID_14_0_ECX_TOPA | \ > > > > + CPUID_14_0_ECX_MULTI_ENTRIES | CPUID_14_0_ECX_SINGLE_RANGE) > > > > +#define INTEL_PT_DEFAULT_1_EAX (INTEL_PT_DEFAULT_MTC_BITMAP << 16 | \ > > > > + INTEL_PT_DEFAULT_ADDR_RANGES_NUM) > > > > +#define INTEL_PT_DEFAULT_1_EBX (INTEL_PT_DEFAULT_PSB_BITMAP << 16 | \ > > > > + INTEL_PT_DEFAULT_CYCLE_BITMAP) > > > > > > I like these new macros because they make the code at > > > x86_cpu_filter_features() clearer. > > I tried it. But I find it doesn't make the code at x86_cpu_filter_features() > clearer. It just introduces more code churn. Don't worry, this is not a requirement for getting the code accepted, but just a suggestion to make the more complex parts of the series smaller and easier to review. -- Eduardo
Re: [PATCH] via-ide: Avoid expensive operations in irq handler
On Mon, Oct 18, 2021 at 12:10:04PM +0200, Philippe Mathieu-Daudé wrote:
> On 10/18/21 11:51, BALATON Zoltan wrote:
> > On Mon, 18 Oct 2021, Philippe Mathieu-Daudé wrote:
> >> On 10/18/21 03:36, BALATON Zoltan wrote:
> >>> Cache the pointer to PCI function 0 (ISA bridge, that this IDE device
> >>> has to use for IRQs) in the PCIIDEState and pass that as the opaque
> >>> data for the interrupt handler to eliminate both the need to look up
> >>> function 0 at every interrupt and also a QOM type cast of the opaque
> >>> pointer as that's also expensive (mainly due to qom-debug being
> >>> enabled by default).
> >>>
> >>> Suggested-by: Philippe Mathieu-Daudé
> >>> Signed-off-by: BALATON Zoltan
> >>> ---
> >>> hw/ide/via.c | 11 ++-
> >>> include/hw/ide/pci.h | 1 +
> >>> 2 files changed, 7 insertions(+), 5 deletions(-)
> >>>
> >>> diff --git a/hw/ide/via.c b/hw/ide/via.c
> >>> index 82def819c4..30566bc409 100644
> >>> --- a/hw/ide/via.c
> >>> +++ b/hw/ide/via.c
> >>> @@ -104,15 +104,15 @@ static void bmdma_setup_bar(PCIIDEState *d)
> >>>
> >>> static void via_ide_set_irq(void *opaque, int n, int level)
> >>> {
> >>> - PCIDevice *d = PCI_DEVICE(opaque);
> >>> + PCIIDEState *d = opaque;
> >>>
> >>> if (level) {
> >>> - d->config[0x70 + n * 8] |= 0x80;
> >>> + d->parent_obj.config[0x70 + n * 8] |= 0x80;
> >>> } else {
> >>> - d->config[0x70 + n * 8] &= ~0x80;
> >>> + d->parent_obj.config[0x70 + n * 8] &= ~0x80;
> >>> }
> >>
> >> Better not to access parent_obj, so I'd rather keep the previous
> >> code as it. The rest is OK, thanks. If you don't want to respin
> >> I can fix and take via mips-next.
> >
> > Why not? If it's OK to access other fields why not parent_obj? That
> > avoids the QOM cast PCI_DEVICE(opaque) or PCI_DEVICE(d) after this
> > patch. We know it's a PCIIDEState and has PCIDevice parent_obj field
> > because we set the opaque pointer when adding this callback so I think
> > this works and is the less expensive way. But feel free to change it any
> > way you like and use it that way. I'd keep it as it is.
>
> My understanding of QOM is we shouldn't access internal states that
> way, because 1/ this makes object refactors harder and 2/ this is
> not the style/example we want in the codebase, but it doesn't seem
> documented, so Cc'ing Markus/Eduardo for confirmation.
`PCI_DEVICE(d)` is preferred instead `of d.parent_obj` (parent_obj is
just a QOM implementation detail). If there are real performance
reasons to avoid it, we need numbers to support that.
Also, note that `OBJECT_CHECK(obj)` is just `return obj` if
CONFIG_QOM_CAST_DEBUG is disabled.
--
Eduardo
Re: [PATCH] hw/qdev-core: Add compatibility for (non)-transitional devs
On Wed, Oct 20, 2021 at 01:02:24PM +0800, Jason Wang wrote:
> On Wed, Oct 20, 2021 at 9:31 AM Jason Wang wrote:
> >
> > On Wed, Oct 20, 2021 at 12:56 AM Eduardo Habkost
> > wrote:
> > >
> > > On Tue, Oct 19, 2021 at 12:13:17PM -0400, Michael S. Tsirkin wrote:
> > > > On Tue, Oct 19, 2021 at 11:29:13AM -0400, Eduardo Habkost wrote:
> > > > > On Tue, Oct 19, 2021 at 06:59:09AM -0400, Michael S. Tsirkin wrote:
> > > > > > On Tue, Oct 19, 2021 at 11:46:17AM +0100, Stefan Hajnoczi wrote:
> > > > > > > On Tue, Oct 12, 2021 at 10:36:01AM +0200, Jean-Louis Dupond wrote:
> > > > > > > > Forgot to CC maintainers.
> > > > > > >
> > > > > > > Also CCing Jason Wang and Michael Tsirkin for VIRTIO.
> > > > > > >
> > > > > > > Stefan
> > > > > >
> > > > > > OMG
> > > > > > where all compat properties broken all the time?
> > > > >
> > > > > Compat properties that existed when commit f6e501a28ef9 ("virtio:
> > > > > Provide version-specific variants of virtio PCI devices") was
> > > > > merged are not broken, because virtio-*-transitional and
> > > > > virtio-*-non-transitional were brand new QOM types (so there's no
> > > > > compatibility to be kept with old QEMU versions).
> > > > >
> > > > > Compat properties referencing "virtio-*-pci" instead of
> > > > > "virtio-*-pci-base" added after commit f6e501a28ef9 are probably
> > > > > broken, yes.
> > > > >
> > > > > --
> > > > > Eduardo
> > > >
> > > > Oh. So just this one:
> > > > { "virtio-net-pci", "vectors", "3"},
> > > >
> > > > right?
> > >
> > > I think so. That's the only post-4.0 virtio-*-pci compat property I see
> > > in
> > > hw/core/machine.c.
> > >
> > > pc.c doesn't have any post-4.0 virtio-*-pci compat props. I didn't see
> > > any
> > > virtio compat props on spapr.c and s390-virtio-ccw.c.
> > >
> > > >
> > > > about the patch: how do people feel about virtio specific
> > > > stuff in qdev core? Ok by everyone?
> > >
> > > Not OK, if we have a mechanism to avoid that, already (the
> > > "virtio-net-pci-base" type name). I wonder what we can do to
> > > make this kind of mistake less likely, though.
> > >
> > > Jean-Louis, Jason, does the following fix work?
> >
> > Yes.
> >
> > Acked-by: Jason Wang
> >
> > Thanks
> >
> > >
> > > Signed-off-by: Eduardo Habkost
> > > ---
> > > diff --git a/hw/core/machine.c b/hw/core/machine.c
> > > index b8d95eec32d..bd9c6156c1a 100644
> > > --- a/hw/core/machine.c
> > > +++ b/hw/core/machine.c
> > > @@ -56,7 +56,7 @@ GlobalProperty hw_compat_5_2[] = {
> > > { "ICH9-LPC", "smm-compat", "on"},
> > > { "PIIX4_PM", "smm-compat", "on"},
> > > { "virtio-blk-device", "report-discard-granularity", "off" },
> > > -{ "virtio-net-pci", "vectors", "3"},
> > > +{ "virtio-net-pci-base", "vectors", "3"},
>
> Rethink about this, any chance that we can use "virtio-net-pci" as the
> base_name? It looks to me this can cause less confusion and consistent
> with the existing compat properties.
It's probably too late now: we can't change the semantics of
"-global virtio-net-pci" without breaking compatibility.
The original reasoning for making generic_name != base_name is at
this comment in struct VirtioPCIDeviceTypeInfo:
/*
* Common base class for the subclasses below.
*
* Required only if transitional_name or non_transitional_name is set.
*
* We need a separate base type instead of making all types
* inherit from generic_name for two reasons:
* 1) generic_name implements INTERFACE_PCIE_DEVICE, but
*transitional_name does not.
* 2) generic_name has the "disable-legacy" and "disable-modern"
*properties, transitional_name and non_transitional name don't.
*/
const char *base_name;
(I had to look it up. I didn't remember the original reason for that)
--
Eduardo
Re: [PATCH] hw/qdev-core: Add compatibility for (non)-transitional devs
On Wed, Oct 20, 2021 at 03:41:38AM -0400, Michael S. Tsirkin wrote:
> On Tue, Oct 19, 2021 at 12:56:11PM -0400, Eduardo Habkost wrote:
> > On Tue, Oct 19, 2021 at 12:13:17PM -0400, Michael S. Tsirkin wrote:
> > > On Tue, Oct 19, 2021 at 11:29:13AM -0400, Eduardo Habkost wrote:
> > > > On Tue, Oct 19, 2021 at 06:59:09AM -0400, Michael S. Tsirkin wrote:
> > > > > On Tue, Oct 19, 2021 at 11:46:17AM +0100, Stefan Hajnoczi wrote:
> > > > > > On Tue, Oct 12, 2021 at 10:36:01AM +0200, Jean-Louis Dupond wrote:
> > > > > > > Forgot to CC maintainers.
> > > > > >
> > > > > > Also CCing Jason Wang and Michael Tsirkin for VIRTIO.
> > > > > >
> > > > > > Stefan
> > > > >
> > > > > OMG
> > > > > where all compat properties broken all the time?
> > > >
> > > > Compat properties that existed when commit f6e501a28ef9 ("virtio:
> > > > Provide version-specific variants of virtio PCI devices") was
> > > > merged are not broken, because virtio-*-transitional and
> > > > virtio-*-non-transitional were brand new QOM types (so there's no
> > > > compatibility to be kept with old QEMU versions).
> > > >
> > > > Compat properties referencing "virtio-*-pci" instead of
> > > > "virtio-*-pci-base" added after commit f6e501a28ef9 are probably
> > > > broken, yes.
> > > >
> > > > --
> > > > Eduardo
> > >
> > > Oh. So just this one:
> > > { "virtio-net-pci", "vectors", "3"},
> > >
> > > right?
> >
> > I think so. That's the only post-4.0 virtio-*-pci compat property I see in
> > hw/core/machine.c.
> >
> > pc.c doesn't have any post-4.0 virtio-*-pci compat props. I didn't see any
> > virtio compat props on spapr.c and s390-virtio-ccw.c.
> >
> > >
> > > about the patch: how do people feel about virtio specific
> > > stuff in qdev core? Ok by everyone?
> >
> > Not OK, if we have a mechanism to avoid that, already (the
> > "virtio-net-pci-base" type name). I wonder what we can do to
> > make this kind of mistake less likely, though.
> >
> > Jean-Louis, Jason, does the following fix work?
> >
> > Signed-off-by: Eduardo Habkost
> > ---
> > diff --git a/hw/core/machine.c b/hw/core/machine.c
> > index b8d95eec32d..bd9c6156c1a 100644
> > --- a/hw/core/machine.c
> > +++ b/hw/core/machine.c
> > @@ -56,7 +56,7 @@ GlobalProperty hw_compat_5_2[] = {
> > { "ICH9-LPC", "smm-compat", "on"},
> > { "PIIX4_PM", "smm-compat", "on"},
> > { "virtio-blk-device", "report-discard-granularity", "off" },
> > -{ "virtio-net-pci", "vectors", "3"},
> > +{ "virtio-net-pci-base", "vectors", "3"},
> > };
> > const size_t hw_compat_5_2_len = G_N_ELEMENTS(hw_compat_5_2);
>
> Hmm I'm a bit confused at this point, as to why does
> specifying properties for virtio-net-pci on command
> line with -global work, but in compat list doesn't. Do others
> understand?
I don't think that's the case. -global behaves similarly to compat_props.
Running an unpatched QEMU 6.1.0 binary:
$ echo -e 'info qtree\nquit' | qemu-system-x86_64 -device virtio-net-pci
-machine pc-q35-5.2 -monitor stdio | grep vectors
vectors = 3 (0x3)
$ echo -e 'info qtree\nquit' | qemu-system-x86_64 -device
virtio-net-pci-non-transitional -machine pc-q35-5.2 -monitor stdio | grep
vectors
vectors = 4 (0x4)
$ echo -e 'info qtree\nquit' | qemu-system-x86_64 -device
virtio-net-pci-non-transitional -global virtio-net-pci.vectors=3 -monitor stdio
| grep vectors
vectors = 4 (0x4)
$ echo -e 'info qtree\nquit' | qemu-system-x86_64 -device
virtio-net-pci-non-transitional -global virtio-net-pci-base.vectors=3 -monitor
stdio | grep vectors
vectors = 3 (0x3)
--
Eduardo
Re: [PATCH] hw/qdev-core: Add compatibility for (non)-transitional devs
On Tue, Oct 19, 2021 at 12:13:17PM -0400, Michael S. Tsirkin wrote:
> On Tue, Oct 19, 2021 at 11:29:13AM -0400, Eduardo Habkost wrote:
> > On Tue, Oct 19, 2021 at 06:59:09AM -0400, Michael S. Tsirkin wrote:
> > > On Tue, Oct 19, 2021 at 11:46:17AM +0100, Stefan Hajnoczi wrote:
> > > > On Tue, Oct 12, 2021 at 10:36:01AM +0200, Jean-Louis Dupond wrote:
> > > > > Forgot to CC maintainers.
> > > >
> > > > Also CCing Jason Wang and Michael Tsirkin for VIRTIO.
> > > >
> > > > Stefan
> > >
> > > OMG
> > > where all compat properties broken all the time?
> >
> > Compat properties that existed when commit f6e501a28ef9 ("virtio:
> > Provide version-specific variants of virtio PCI devices") was
> > merged are not broken, because virtio-*-transitional and
> > virtio-*-non-transitional were brand new QOM types (so there's no
> > compatibility to be kept with old QEMU versions).
> >
> > Compat properties referencing "virtio-*-pci" instead of
> > "virtio-*-pci-base" added after commit f6e501a28ef9 are probably
> > broken, yes.
> >
> > --
> > Eduardo
>
> Oh. So just this one:
> { "virtio-net-pci", "vectors", "3"},
>
> right?
I think so. That's the only post-4.0 virtio-*-pci compat property I see in
hw/core/machine.c.
pc.c doesn't have any post-4.0 virtio-*-pci compat props. I didn't see any
virtio compat props on spapr.c and s390-virtio-ccw.c.
>
> about the patch: how do people feel about virtio specific
> stuff in qdev core? Ok by everyone?
Not OK, if we have a mechanism to avoid that, already (the
"virtio-net-pci-base" type name). I wonder what we can do to
make this kind of mistake less likely, though.
Jean-Louis, Jason, does the following fix work?
Signed-off-by: Eduardo Habkost
---
diff --git a/hw/core/machine.c b/hw/core/machine.c
index b8d95eec32d..bd9c6156c1a 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -56,7 +56,7 @@ GlobalProperty hw_compat_5_2[] = {
{ "ICH9-LPC", "smm-compat", "on"},
{ "PIIX4_PM", "smm-compat", "on"},
{ "virtio-blk-device", "report-discard-granularity", "off" },
-{ "virtio-net-pci", "vectors", "3"},
+{ "virtio-net-pci-base", "vectors", "3"},
};
const size_t hw_compat_5_2_len = G_N_ELEMENTS(hw_compat_5_2);
--
Eduardo
Re: [PATCH] hw/qdev-core: Add compatibility for (non)-transitional devs
On Tue, Oct 19, 2021 at 06:59:09AM -0400, Michael S. Tsirkin wrote:
> On Tue, Oct 19, 2021 at 11:46:17AM +0100, Stefan Hajnoczi wrote:
> > On Tue, Oct 12, 2021 at 10:36:01AM +0200, Jean-Louis Dupond wrote:
> > > Forgot to CC maintainers.
> >
> > Also CCing Jason Wang and Michael Tsirkin for VIRTIO.
> >
> > Stefan
>
> OMG
> where all compat properties broken all the time?
Compat properties that existed when commit f6e501a28ef9 ("virtio:
Provide version-specific variants of virtio PCI devices") was
merged are not broken, because virtio-*-transitional and
virtio-*-non-transitional were brand new QOM types (so there's no
compatibility to be kept with old QEMU versions).
Compat properties referencing "virtio-*-pci" instead of
"virtio-*-pci-base" added after commit f6e501a28ef9 are probably
broken, yes.
--
Eduardo
Re: [PATCH] hw/qdev-core: Add compatibility for (non)-transitional devs
On Tue, Oct 12, 2021 at 10:24:28AM +0200, Jean-Louis Dupond wrote:
> hw_compat modes only take into account their base name.
What do you mean by "base name"?
> But if a device is created with (non)-transitional, then the compat
> values are not used, causing migrating issues.
>
> This commit adds their (non)-transitional entries with the same settings
> as the base entry.
Wouldn't it be easier to fix the incorrect compat_props arrays to
use "virtio-*-pci-base" instead?
If a piece of code is supposed to affect/support both
non-transitional and transitional subclasses, that's why
VirtioPCIDeviceTypeInfo.base_name exists.
>
> Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1999141
>
> Signed-off-by: Jean-Louis Dupond
> ---
> include/hw/qdev-core.h | 34 ++
> 1 file changed, 34 insertions(+)
>
> diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
> index 4ff19c714b..5726825c2d 100644
> --- a/include/hw/qdev-core.h
> +++ b/include/hw/qdev-core.h
> @@ -293,6 +293,30 @@ typedef struct GlobalProperty {
> bool optional;
> } GlobalProperty;
>
> +
> +/**
> + * Helper to add (non)transitional compat properties
> + */
> +static inline void
> +compat_props_add_transitional(GPtrArray *arr, GlobalProperty *prop)
> +{
> +GlobalProperty *transitional = g_new0(typeof(*transitional), 1);
> +transitional->driver = g_strdup_printf("%s-transitional", prop->driver);
> +transitional->property = g_strdup(prop->property);
> +transitional->value = g_strdup(prop->value);
> +transitional->used = prop->used;
> +transitional->optional = prop->optional;
> +g_ptr_array_add(arr, (void *)transitional);
> +
> +GlobalProperty *non_transitional = g_new0(typeof(*non_transitional), 1);
> +non_transitional->driver = g_strdup_printf("%s-non-transitional",
> prop->driver);
> +non_transitional->property = g_strdup(prop->property);
> +non_transitional->value = g_strdup(prop->value);
> +non_transitional->used = prop->used;
> +non_transitional->optional = prop->optional;
> +g_ptr_array_add(arr, (void *)non_transitional);
> +}
> +
> static inline void
> compat_props_add(GPtrArray *arr,
> GlobalProperty props[], size_t nelem)
> @@ -300,6 +324,16 @@ compat_props_add(GPtrArray *arr,
> int i;
> for (i = 0; i < nelem; i++) {
> g_ptr_array_add(arr, (void *)[i]);
> +if (g_str_equal(props[i].driver, "vhost-user-blk-pci") ||
> +g_str_equal(props[i].driver, "virtio-scsi-pci") ||
> +g_str_equal(props[i].driver, "virtio-blk-pci") ||
> +g_str_equal(props[i].driver, "virtio-balloon-pci") ||
> +g_str_equal(props[i].driver, "virtio-serial-pci") ||
> +g_str_equal(props[i].driver, "virtio-9p-pci") ||
> +g_str_equal(props[i].driver, "virtio-net-pci") ||
> +g_str_equal(props[i].driver, "virtio-rng-pci")) {
> +compat_props_add_transitional(arr, [i]);
> +}
> }
> }
>
> --
> 2.33.0
>
>
--
Eduardo
Re: [RFC PATCH 3/5] target/i386: Enable host pass through of Intel PT
On Thu, Sep 09, 2021 at 10:41:48PM +0800, Xiaoyao Li wrote:
> commit e37a5c7fa459 ("i386: Add Intel Processor Trace feature support")
> added the support of Intel PT by making CPUID[14] of PT as fixed feature
> set (from ICX) for any CPU model on any host.
>
> This truly breaks the PT exposing on Intel SPR platform because SPR has
> less supported bitmap CPUID(0x14,1):EBX[15:0] than ICX.
>
> This patch enables passing through host's PT capabilities for the case
> "-cpu host/max" while ensure named CPU model still has the fixed
> PT feature set to not break the live migration.
>
> It introduces @has_specific_intel_pt_feature_set field for name CPU
> model. If a named CPU model has this field as false, it will use fixed
> PT feature set of ICX. Besides same to previous behavior, if fixed PT
> feature set of ICX cannot be satisfied/supported by host, it disables PT
> instead of adjusting the feature set based on host's capabilities.
>
> In the future, new named CPU model, e.g., Sapphire Rapids, can define
> its own PT feature set by setting @has_specific_intel_pt_feature_set to
> true and defines it's own FEAT_14_0_EBX, FEAT_14_0_ECX, FEAT_14_1_EAX
> and FEAT_14_1_EBX.
>
> Signed-off-by: Xiaoyao Li
> ---
> target/i386/cpu.c | 106 --
> target/i386/cpu.h | 1 +
> 2 files changed, 47 insertions(+), 60 deletions(-)
>
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index 58e98210f219..00c4ad23110d 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -543,34 +543,24 @@ static CPUCacheInfo legacy_l3_cache = {
> #define L2_ITLB_4K_ASSOC 4
> #define L2_ITLB_4K_ENTRIES 512
>
> -/* CPUID Leaf 0x14 constants: */
> -#define INTEL_PT_MAX_SUBLEAF 0x1
> -/*
> - * bit[00]: IA32_RTIT_CTL.CR3 filter can be set to 1 and IA32_RTIT_CR3_MATCH
> - * MSR can be accessed;
> - * bit[01]: Support Configurable PSB and Cycle-Accurate Mode;
> - * bit[02]: Support IP Filtering, TraceStop filtering, and preservation
> - * of Intel PT MSRs across warm reset;
> - * bit[03]: Support MTC timing packet and suppression of COFI-based packets;
> - */
> -#define INTEL_PT_MINIMAL_EBX 0xf
> -/*
> - * bit[00]: Tracing can be enabled with IA32_RTIT_CTL.ToPA = 1 and
> - * IA32_RTIT_OUTPUT_BASE and IA32_RTIT_OUTPUT_MASK_PTRS MSRs can be
> - * accessed;
> - * bit[01]: ToPA tables can hold any number of output entries, up to the
> - * maximum allowed by the MaskOrTableOffset field of
> - * IA32_RTIT_OUTPUT_MASK_PTRS;
> - * bit[02]: Support Single-Range Output scheme;
> - */
> -#define INTEL_PT_MINIMAL_ECX 0x7
> -/* generated packets which contain IP payloads have LIP values */
> -#define INTEL_PT_IP_LIP (1 << 31)
> -#define INTEL_PT_ADDR_RANGES_NUM 0x2 /* Number of configurable address
> ranges */
> -#define INTEL_PT_ADDR_RANGES_NUM_MASK 0x7
> -#define INTEL_PT_MTC_BITMAP (0x0249 << 16) /* Support ART(0,3,6,9) */
> -#define INTEL_PT_CYCLE_BITMAP0x1fff /* Support 0,2^(0~11) */
> -#define INTEL_PT_PSB_BITMAP (0x003f << 16) /* Support
> 2K,4K,8K,16K,32K,64K */
> +#define INTEL_PT_MAX_SUBLEAF0x1
> +#define INTEL_PT_DEFAULT_ADDR_RANGES_NUM0x2
> +#define INTEL_PT_ADDR_RANGES_NUM_MASK 0x7
> +/* Support ART(0,3,6,9) */
> +#define INTEL_PT_DEFAULT_MTC_BITMAP 0x0249
> +/* Support 0,2^(0~11) */
> +#define INTEL_PT_DEFAULT_CYCLE_BITMAP 0x1fff
> +/* Support 2K,4K,8K,16K,32K,64K */
> +#define INTEL_PT_DEFAULT_PSB_BITMAP 0x003f
> +
> +#define INTEL_PT_DEFAULT_0_EBX (CPUID_14_0_EBX_CR3_FILTER | \
> +CPUID_14_0_EBX_PSB | CPUID_14_0_EBX_IP_FILTER |
> CPUID_14_0_EBX_MTC)
> +#define INTEL_PT_DEFAULT_0_ECX (CPUID_14_0_ECX_TOPA | \
> +CPUID_14_0_ECX_MULTI_ENTRIES | CPUID_14_0_ECX_SINGLE_RANGE)
> +#define INTEL_PT_DEFAULT_1_EAX (INTEL_PT_DEFAULT_MTC_BITMAP << 16 | \
> + INTEL_PT_DEFAULT_ADDR_RANGES_NUM)
> +#define INTEL_PT_DEFAULT_1_EBX (INTEL_PT_DEFAULT_PSB_BITMAP << 16 | \
> + INTEL_PT_DEFAULT_CYCLE_BITMAP)
I like these new macros because they make the code at
x86_cpu_filter_features() clearer. Can we do this in a separate
patch, to be applied before "Introduce FeatureWordInfo for Intel
PT CPUID leaf 0xD"?
>
> void x86_cpu_vendor_words2str(char *dst, uint32_t vendor1,
>uint32_t vendor2, uint32_t vendor3)
> @@ -1517,6 +1507,7 @@ typedef struct X86CPUDefinition {
> int family;
> int model;
> int stepping;
> +bool has_specific_intel_pt_feature_set;
> FeatureWordArray features;
> const char *model_id;
> const CPUCaches *const cache_info;
> @@ -5061,6 +5052,14 @@ static void x86_cpu_load_model(X86CPU *cpu,
> X86CPUModel *model)
> env->features[w] = def->features[w];
> }
>
> +if (!def->has_specific_intel_pt_feature_set) {
> +env->use_default_intel_pt = true;
> +
Re: [RFC PATCH 2/5] target/i386: Introduce FeatureWordInfo for Intel PT CPUID leaf 0xD
Hi,
Apologies for the delay. Comments below:
On Thu, Sep 09, 2021 at 10:41:47PM +0800, Xiaoyao Li wrote:
> CPUID leaf 0x14 subleaf 0x0 and 0x1 enumerate the resource and
> capability of Intel PT.
>
> Introduce FeatureWord FEAT_14_0_EBX, FEAT_14_1_EAX and FEAT_14_1_EBX,
> and complete FEAT_14_0_ECX. Thus all the features of Intel PT can be
> expanded when "-cpu host/max" and can be configured in named CPU model.
>
> Regarding FEAT_14_1_EAX and FEAT_14_1_EBX, don't define the name for
> them since each bit of them doesn't represent a standalone sub-feature
> of Intel PT. However, explicitly mark them as migratable. So the
> meaningful bits of them can be autoenabled in x86_cpu_expand_features().
>
> It has special handling for FEAT_14_1_EAX[2:0], because the 3 bits as a
> whole represents the number of PT ADDRn_CFG ranges. Thus it has special
> handling in mark_unavailable_features() and x86_cpu_filter_features().
>
> Signed-off-by: Xiaoyao Li
> ---
> target/i386/cpu.c | 87 +--
> target/i386/cpu.h | 37 +++-
> 2 files changed, 112 insertions(+), 12 deletions(-)
>
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index a06473c9e84c..58e98210f219 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -567,7 +567,7 @@ static CPUCacheInfo legacy_l3_cache = {
> /* generated packets which contain IP payloads have LIP values */
> #define INTEL_PT_IP_LIP (1 << 31)
> #define INTEL_PT_ADDR_RANGES_NUM 0x2 /* Number of configurable address
> ranges */
> -#define INTEL_PT_ADDR_RANGES_NUM_MASK 0x3
> +#define INTEL_PT_ADDR_RANGES_NUM_MASK 0x7
> #define INTEL_PT_MTC_BITMAP (0x0249 << 16) /* Support ART(0,3,6,9) */
> #define INTEL_PT_CYCLE_BITMAP0x1fff /* Support 0,2^(0~11) */
> #define INTEL_PT_PSB_BITMAP (0x003f << 16) /* Support
> 2K,4K,8K,16K,32K,64K */
> @@ -1161,17 +1161,32 @@ FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
> }
> },
>
> +[FEAT_14_0_EBX] = {
> +.type = CPUID_FEATURE_WORD,
> +.feat_names = {
> +[0] = "intel-pt-cr3-filter",
> +[1] = "intel-pt-PSB",
I suggest using lowercase for all feature names, as it is the
usual convention for QOM property names.
> +[2] = "intel-pt-ip-filter",
> +[3] = "intel-pt-mtc",
> +[4] = "intel-pt-ptwrite",
> +[5] = "intel-pt-power-event",
> +[6] = "intel-pt-psb-pmi-preservation",
This has a side effect: live migration with those flags enabled
will become possible.
All bits allow enumerate support for an optional feature to be
enabled by the OS, so it means we can emulate a CPU with bit=0
CPU on a bit=1 host. So it will be safe as long as there's no
additional state that needs to be live migrated when those
features are used. Do we have any additional state, or all MSRs
currently being migrated are enough?
> +},
> +.cpuid = {
> +.eax = 0x14,
> +.needs_ecx = true, .ecx = 0,
> +.reg = R_EBX,
> +},
> +},
> +
> [FEAT_14_0_ECX] = {
> .type = CPUID_FEATURE_WORD,
> .feat_names = {
> -NULL, NULL, NULL, NULL,
> -NULL, NULL, NULL, NULL,
> -NULL, NULL, NULL, NULL,
> -NULL, NULL, NULL, NULL,
> -NULL, NULL, NULL, NULL,
> -NULL, NULL, NULL, NULL,
> -NULL, NULL, NULL, NULL,
> -NULL, NULL, NULL, "intel-pt-lip",
> +[0] = "intel-pt-topa",
> +[1] = "intel-pt-multi-topa-entries",
> +[2] = "intel-pt-single-range",
> +[3] = "intel-pt-trace-transport-subsystem",
All bits above are also optional features to be enabled
explicitly by the OS, so it also seems OK.
> +[31] = "intel-pt-lip",
This one is trickier because its value must match the host, but
it is already present in the existing code. We already have an
explicit check for host LIP == guest LIP, so it's OK.
> },
> .cpuid = {
> .eax = 0x14,
> @@ -1181,6 +1196,26 @@ FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
> .tcg_features = TCG_14_0_ECX_FEATURES,
> },
>
> +[FEAT_14_1_EAX] = {
> +.type = CPUID_FEATURE_WORD,
> +.cpuid = {
> +.eax = 0x14,
> +.needs_ecx = true, .ecx = 1,
> +.reg = R_EAX,
> +},
> +.migratable_flags = ~0ull,
This one is trickier. I see a few potential issues:
* Bits 15:3 are documented as reserved on my version of the Intel
SDM (June 2021). If they are reserved, I don't think we should
make them migratable yet. If they aren't, do you have a
pointer to the documentation?
* Bits 2:0 is a number, not a simple boolean flag. You are
handling this as a special case in x86_cpu_filter_features()
below, so it should be OK.
* The flags are migratable but have no names. The only existing
case of non-zero
Re: [RFC PATCH 1/5] target/i386: Print CPUID subleaf info for unsupported feature
On Thu, Sep 09, 2021 at 10:41:46PM +0800, Xiaoyao Li wrote: > Some CPUID leaves have meaningful subleaf index. Print the subleaf info > in feature_word_description for CPUID features. > > Signed-off-by: Xiaoyao Li Reviewed-by: Eduardo Habkost -- Eduardo
Re: [PATCH v2 0/2] Remove unsupported features in SNR CPU model
On Fri, Aug 27, 2021 at 02:48:16PM +0800, Chenyi Qiang wrote: > Patch 1: > https://lore.kernel.org/qemu-devel/20210825195438.914387-2-ehabk...@redhat.com/ > > Patch 2 removes one more feature (core-capability) in Snowridge-v4 CPU > model based on previous patch. > > Chenyi Qiang (2): > target/i386: Remove split lock detect in Snowridge CPU model > target/i386: Remove core-capability in Snowridge CPU model Patch 2/2 queued (patch 1/2 was already merged). Thanks and apologies for the delay. -- Eduardo
Re: [PATCH v0 0/2] virtio-blk and vhost-user-blk cross-device migration
On Tue, Oct 05, 2021 at 03:01:05PM +0100, Dr. David Alan Gilbert wrote:
> * Michael S. Tsirkin (m...@redhat.com) wrote:
> > On Tue, Oct 05, 2021 at 02:18:40AM +0300, Roman Kagan wrote:
> > > On Mon, Oct 04, 2021 at 11:11:00AM -0400, Michael S. Tsirkin wrote:
> > > > On Mon, Oct 04, 2021 at 06:07:29PM +0300, Denis Plotnikov wrote:
> > > > > It might be useful for the cases when a slow block layer should be
> > > > > replaced
> > > > > with a more performant one on running VM without stopping, i.e. with
> > > > > very low
> > > > > downtime comparable with the one on migration.
> > > > >
> > > > > It's possible to achive that for two reasons:
> > > > >
> > > > > 1.The VMStates of "virtio-blk" and "vhost-user-blk" are almost the
> > > > > same.
> > > > > They consist of the identical VMSTATE_VIRTIO_DEVICE and differs from
> > > > > each other in the values of migration service fields only.
> > > > > 2.The device driver used in the guest is the same: virtio-blk
> > > > >
> > > > > In the series cross-migration is achieved by adding a new type.
> > > > > The new type uses virtio-blk VMState instead of vhost-user-blk
> > > > > specific
> > > > > VMstate, also it implements migration save/load callbacks to be
> > > > > compatible
> > > > > with migration stream produced by "virtio-blk" device.
> > > > >
> > > > > Adding the new type instead of modifying the existing one is
> > > > > convenent.
> > > > > It ease to differ the new virtio-blk-compatible vhost-user-blk
> > > > > device from the existing non-compatible one using qemu machinery
> > > > > without any
> > > > > other modifiactions. That gives all the variety of qemu device related
> > > > > constraints out of box.
> > > >
> > > > Hmm I'm not sure I understand. What is the advantage for the user?
> > > > What if vhost-user-blk became an alias for vhost-user-virtio-blk?
> > > > We could add some hacks to make it compatible for old machine types.
> > >
> > > The point is that virtio-blk and vhost-user-blk are not
> > > migration-compatible ATM. OTOH they are the same device from the guest
> > > POV so there's nothing fundamentally preventing the migration between
> > > the two. In particular, we see it as a means to switch between the
> > > storage backend transports via live migration without disrupting the
> > > guest.
> > >
> > > Migration-wise virtio-blk and vhost-user-blk have in common
> > >
> > > - the content of the VMState -- VMSTATE_VIRTIO_DEVICE
> > >
> > > The two differ in
> > >
> > > - the name and the version of the VMStateDescription
> > >
> > > - virtio-blk has an extra migration section (via .save/.load callbacks
> > > on VirtioDeviceClass) containing requests in flight
> > >
> > > It looks like to become migration-compatible with virtio-blk,
> > > vhost-user-blk has to start using VMStateDescription of virtio-blk and
> > > provide compatible .save/.load callbacks. It isn't entirely obvious how
> > > to make this machine-type-dependent, so we came up with a simpler idea
> > > of defining a new device that shares most of the implementation with the
> > > original vhost-user-blk except for the migration stuff. We're certainly
> > > open to suggestions on how to reconcile this under a single
> > > vhost-user-blk device, as this would be more user-friendly indeed.
> > >
> > > We considered using a class property for this and defining the
> > > respective compat clause, but IIUC the class constructors (where .vmsd
> > > and .save/.load are defined) are not supposed to depend on class
> > > properties.
> > >
> > > Thanks,
> > > Roman.
> >
> > So the question is how to make vmsd depend on machine type.
> > CC Eduardo who poked at this kind of compat stuff recently,
> > paolo who looked at qom things most recently and dgilbert
> > for advice on migration.
>
> I don't think I've seen anyone change vmsd name dependent on machine
> type; making fields appear/disappear is easy - that just ends up as a
> property on the device that's checked; I guess if that property is
> global (rather than per instance) then you can check it in
> vhost_user_blk_class_init and swing the dc->vmsd pointer?
class_init can be called very early during QEMU initialization,
so it's too early to make decisions based on machine type.
Making a specific vmsd appear/disappear based on machine
configuration or state is "easy", by implementing
VMStateDescription.needed. But this would require registering
both vmsds (one of them would need to be registered manually
instead of using DeviceClass.vmsd).
I don't remember what are the consequences of not using
DeviceClass.vmsd to register a vmsd, I only remember it was
subtle. See commit b170fce3dd06 ("cpu: Register
VMStateDescription through CPUState") and related threads. CCing
Philippe, who might remember the details here.
If that's an important use case, I would suggest allowing devices
to implement a DeviceClass.get_vmsd method, which would override
DeviceClass.vmsd if necessary. Is the
Re: [PATCH v3 1/2] docs: remove non-reference uses of single backticks
On Thu, Sep 23, 2021 at 03:13:22PM -0400, John Snow wrote: > The single backtick markup in ReST is the "default role". Currently, > Sphinx's default role is called "content". Sphinx suggests you can use > the "Any" role instead to turn any single-backtick enclosed item into a > cross-reference. > > This is useful for things like autodoc for Python docstrings, where it's > often nicer to reference other types with `foo` instead of the more > laborious :py:meth:`foo`. It's also useful in multi-domain cases to > easily reference definitions from other Sphinx domains, such as > referencing C code definitions from outside of kerneldoc comments. > > Before we do that, though, we'll need to turn all existing usages of the > "content" role to inline verbatim markup wherever it does not correctly > resolve into a cross-refernece by using double backticks instead. > > Signed-off-by: John Snow > --- > docs/devel/fuzzing.rst | 9 + > docs/devel/tcg-plugins.rst | 2 +- > docs/interop/live-block-operations.rst | 2 +- > docs/system/guest-loader.rst | 2 +- > qapi/block-core.json | 4 ++-- > include/qemu/module.h | 6 +++--- > qemu-options.hx| 4 ++-- > 7 files changed, 15 insertions(+), 14 deletions(-) > > diff --git a/docs/devel/fuzzing.rst b/docs/devel/fuzzing.rst > index 2749bb9bed3..784ecb99e66 100644 > --- a/docs/devel/fuzzing.rst > +++ b/docs/devel/fuzzing.rst > @@ -182,10 +182,11 @@ The output should contain a complete list of matched > MemoryRegions. > > OSS-Fuzz > > -QEMU is continuously fuzzed on `OSS-Fuzz` > __(https://github.com/google/oss-fuzz). > -By default, the OSS-Fuzz build will try to fuzz every fuzz-target. Since the > -generic-fuzz target requires additional information provided in environment > -variables, we pre-define some generic-fuzz configs in > +QEMU is continuously fuzzed on `OSS-Fuzz > +<https://github.com/google/oss-fuzz>`_. By default, the OSS-Fuzz build Gosh, I think I'll never understand the syntax for links in reStructuredText. > +will try to fuzz every fuzz-target. Since the generic-fuzz target > +requires additional information provided in environment variables, we > +pre-define some generic-fuzz configs in > ``tests/qtest/fuzz/generic_fuzz_configs.h``. Each config must specify: [...] Reviewed-by: Eduardo Habkost -- Eduardo
Re: [PATCH v3 2/2] docs/sphinx: change default role to "any"
On Thu, Sep 23, 2021 at 03:13:23PM -0400, John Snow wrote: > This interprets single-backtick syntax in all of our Sphinx docs as a > cross-reference to *something*, including Python symbols. > > From here on out, new uses of `backticks` will cause a build failure if > the target cannot be referenced. > > Signed-off-by: John Snow Patch 1/2 demonstrates why patch 2/2 is useful. Reviewed-by: Eduardo Habkost > --- > docs/conf.py | 5 + > 1 file changed, 5 insertions(+) > > diff --git a/docs/conf.py b/docs/conf.py > index ff6e92c6e2e..4d9f56601fc 100644 > --- a/docs/conf.py > +++ b/docs/conf.py > @@ -85,6 +85,11 @@ > # The master toctree document. > master_doc = 'index' > > +# Interpret `single-backticks` to be a cross-reference to any kind of > +# referenceable object. Unresolvable or ambiguous references will emit a > +# warning at build time. > +default_role = 'any' > + > # General information about the project. > project = u'QEMU' > copyright = u'2021, The QEMU Project Developers' > -- > 2.31.1 > -- Eduardo
Re: [PATCH v2 1/2] docs: remove non-reference uses of single backticks
On Thu, Sep 23, 2021 at 02:22:03PM -0400, John Snow wrote: > The single backtick markup in ReST is the "default role". Currently, > Sphinx's default role is called "content". Sphinx suggests you can use > the "Any" role instead to turn any single-backtick enclosed item into a > cross-reference. > > This is useful for things like autodoc for Python docstrings, where it's > often nicer to reference other types with `foo` instead of the more > laborious :py:meth:`foo`. It's also useful in multi-domain cases to > easily reference definitions from other Sphinx domains, such as > referencing C code definitions from outside of kerneldoc comments. > > Before we do that, though, we'll need to turn all existing usages of the > "content" role to inline verbatim markup wherever it does not correctly > resolve into a cross-refernece by using double backticks instead. > > Signed-off-by: John Snow Clear demonstration of the usefulness of patch 2/2 (these occurrences of `foo` wouldn't have been added if the default role was "any" because "any" errors out on invalid references). However, it looks like there are unrelated changes: [...] > diff --git a/docs/devel/migration.rst b/docs/devel/migration.rst > index 24012534827..6b1230f2d7f 100644 > --- a/docs/devel/migration.rst > +++ b/docs/devel/migration.rst > @@ -403,8 +403,8 @@ version_id. And the function ``load_state_old()`` (if > present) is able to > load state from minimum_version_id_old to minimum_version_id. This > function is deprecated and will be removed when no more users are left. > > -There are *_V* forms of many ``VMSTATE_`` macros to load fields for version > dependent fields, > -e.g. > +There are *_V* forms of many ``VMSTATE_`` macros to load fields for > +version dependent fields, e.g. Unrelated? Line wrapping change only. > > .. code:: c > > @@ -819,9 +819,9 @@ Postcopy now works with hugetlbfs backed memory: > Postcopy with shared memory > --- > > -Postcopy migration with shared memory needs explicit support from the other > -processes that share memory and from QEMU. There are restrictions on the > type of > -memory that userfault can support shared. > +Postcopy migration with shared memory needs explicit support from the > +other processes that share memory and from QEMU. There are restrictions > +on the type of memory that userfault can support shared. Unrelated? Line wrapping change only. Reviewed-by: Eduardo Habkost # if unrelated line wrapping changes are dropped -- Eduardo
Re: [PATCH 2/2] python: pylint 2.11 support
On Thu, Sep 16, 2021 at 02:22:48PM -0400, John Snow wrote: > We're not ready to enforce f-strings everywhere, so just silence this > new warning. > > Signed-off-by: John Snow Reviewed-by: Eduardo Habkost -- Eduardo
Re: [PATCH 1/2] python: Update for pylint 2.10
On Thu, Sep 16, 2021 at 02:22:47PM -0400, John Snow wrote: > A few new annoyances. Of note is the new warning for an unspecified > encoding when opening a text file, which actually does indicate a > potentially real problem; see > https://www.python.org/dev/peps/pep-0597/#motivation > > Use LC_CTYPE to determine an encoding to use for interpreting QEMU's > terminal output. Note that Python states: "language code and encoding > may be None if their values cannot be determined" -- use a platform > default as a backup. > > Notes: Passing encoding=None will generate a suppressed warning on > Python 3.10+ that 'None' should not be passed as the encoding > argument. This behavior may be deprecated in the future and the default > switched to be a ubiquitous UTF-8. Opting in to the locale default will > be done by passing the encoding 'locale', but that isn't available in > 3.6 through 3.9. Presumably this warning will be unsuppressed some time > prior to the actual switch and we can re-investigate these issues at > that time if necessary. So, in the very worst case this will trigger a warning that is currently suppressed. And that will happen only if we are in the unlikely situation where we have absolutely no information about the encoding being used by other parts of the system. Sounds reasonable to me, so: Reviewed-by: Eduardo Habkost > > Signed-off-by: John Snow > --- > python/qemu/machine/machine.py | 7 ++- > python/setup.cfg | 1 + > 2 files changed, 7 insertions(+), 1 deletion(-) > > diff --git a/python/qemu/machine/machine.py b/python/qemu/machine/machine.py > index a7081b1845..34131884a5 100644 > --- a/python/qemu/machine/machine.py > +++ b/python/qemu/machine/machine.py > @@ -19,6 +19,7 @@ > > import errno > from itertools import chain > +import locale > import logging > import os > import shutil > @@ -290,8 +291,12 @@ def get_pid(self) -> Optional[int]: > return self._subp.pid > > def _load_io_log(self) -> None: > +# Assume that the output encoding of QEMU's terminal output is > +# defined by our locale. If indeterminate, allow open() to fall > +# back to the platform default. > +_, encoding = locale.getlocale() > if self._qemu_log_path is not None: > -with open(self._qemu_log_path, "r") as iolog: > +with open(self._qemu_log_path, "r", encoding=encoding) as iolog: > self._iolog = iolog.read() > > @property > diff --git a/python/setup.cfg b/python/setup.cfg > index 83909c1c97..0f0cab098f 100644 > --- a/python/setup.cfg > +++ b/python/setup.cfg > @@ -104,6 +104,7 @@ good-names=i, > [pylint.similarities] > # Ignore imports when computing similarities. > ignore-imports=yes > +ignore-signatures=yes > > # Minimum lines number of a similarity. > # TODO: Remove after we opt in to Pylint 2.8.3. See commit msg. > -- > 2.31.1 > -- Eduardo
Re: [PATCH v2 02/19] host-utils: move abs64() to host-utils as uabs64()
On Tue, Aug 31, 2021 at 01:39:50PM -0300, Luis Pires wrote: > Move abs64 to host-utils as uabs64, so it can be used elsewhere. > The function was renamed to uabs64 and modified to return an > unsigned value. This avoids the undefined behavior for common > abs implementations, where abs of the most negative value is > undefined. > > Signed-off-by: Luis Pires Reviewed-by: Eduardo Habkost In case you need to send a new version of the series, I'd suggest separating this into two patches: creation of the uabs64() function in host-utils, and then changing the KVM PIT code to use the new uabs64() function. This way, the rest of your series won't depend on the KVM PIT changes, helping bisectability and backportability. -- Eduardo
Re: Live migration regarding Intel PT
On Thu, Aug 26, 2021 at 10:16:26AM +0800, Xiaoyao Li wrote: > On 8/26/2021 4:48 AM, Eduardo Habkost wrote: > > On Wed, Aug 25, 2021 at 02:59:37PM +0800, Xiaoyao Li wrote: > > > Hi Eduardo, > > > > > > I have some question regrading Intel PT live migration. > > > > > > Commit "e37a5c7fa459 (i386: Add Intel Processor Trace feature support)" > > > expose Intel PT with a fixed capabilities of CPUID 0x14 for live > > > migration. > > > And the fixed capabilities are the value reported on ICX(IceLake). > > > However, > > > the upcoming SPR(Sapphire Rapids) has less capabilities of > > > INTEL_PT_CYCLE_BITMAP than ICX. It fails to enable PT in guest on SPR > > > machine. > > > > > > If change the check on INTEL_PT_CYCLE_BITMAP to allow different value to > > > allow it work on SPR. I think it breaks live migration, right? > > > > If I understand your description correctly, I don't think that > > would break live migration. > > > > Generating different CPUID data for the same CPU model+flags > > would break live migration. > > > > However, merely allowing a wider set of configurations to work > > wouldn't break live migration, as long as the same CPU > > model+flags generates the same CPUID data on any host. > > The easy fix in my brain to make PT work on SPR guest surely will lead to > different CPUID data between ICX and SPR. That's why I wrote the email. > Yes, but I don't see where the problem is. We only need to generate the same CPUID data if you are running the same CPU model + flags. It's OK (and expected) to have "-cpu Icelake" and "-cpu SapphireRapids" result in different CPUID data. > Other questions about live migration. I think a named CPU model is the > pre-condition for live migration, right? Yes. More precisely, it needs a migration-safe CPU model (in x86 it means all named CPU models except "max" and "host"). > > Then is "same QEMU version/binary" the pre-condition for live migration as > well? Not at all. We support migration to different QEMU binaries/versions. But "same machine type" and "same -cpu option" are both preconditions. > > > > > > > > > For me, not making each sub-function of PT as configurable in QEMU indeed > > > makes it hard for live migration. [...] > > > > Making all sub-functions of PT configurable would be welcome. > > actually. That would allow us to support a wider range of > > configurations and keep live migration working at the same time. > > I think it will break old QEMU? Is it OK? If it's a new feature that requires a new command-line option, it is completely OK. > > > > > > [...] Why not make PT as unmigratable in the > > > first place when introducing the support in QEMU? > > > > I don't know, this was not my decision. Now we support live > > migration in a few specific cases (ICX hosts), and I assume we > > don't want to stop supporting it. > > > > If you just want to support a larger set of hosts when live > > migration is not needed, you can add support to that use case > > too. e.g.: "-cpu host,migratable=off" and/or > > "-cpu ...,intel-pt-passthrough=on" could do host passthrough of > > intel-pt sub-leaves (and block live migration). > > > > That will make things complicated that old use case "-cpu ...,+intel-pt" > still means supporting live migration. And when use "-cpu ...,+intel-pt", > QEMU needs to generate fixed PT capability as previous? Yes, you need to keep the existing use cases working unless you deprecate the existing migration-safe use case (which I don't think you want to). But a "if (cpu->intel_pt_passthrough)" check in cpu_x86_cpuid() would solve that. Anyway, you only need to do that if you want to (if you believe that's an important and useful use case). -- Eduardo
Re: [PATCH 02/19] host-utils: move abs64() to host-utils
On Wed, Aug 25, 2021 at 08:37:17PM +, Luis Fernando Fujita Pires wrote: > From: Eduardo Habkost > > > > Right, that's true of any standard implementation of abs(). > > > I thought about making it return uint64_t, but that could make it > > > weird for other uses of abs64(), where callers wouldn't expect a type > > > change from int64_t to uint64_t. Maybe create a separate uabs64() that > > > returns uint64_t? Or is that even weirder? :) > > > > Which users of abs64 would expect it to return int64_t? > > kvm_pit_update_clock_offset() doesn't seem to. > > Oh, I wasn't referring to any specific users. What I meant is > that, if we make abs64() generically available from host-utils, > callers could expect it to behave the same way as abs() in > stdlib, for example. That would be surprising, but do you think there are cases where that would be a bad surprise? I don't think anybody who is aware of the abs(INT_MIN), labs(LONG_MIN), and llabs(LLONG_MIN) edge cases actually _like_ that behaviour. If you really want to avoid surprises, providing a saner function with a different name seems better than trying to emulate the edge cases of abs()/labs()/llabs(). -- Eduardo
Re: Live migration regarding Intel PT
On Wed, Aug 25, 2021 at 02:59:37PM +0800, Xiaoyao Li wrote: > Hi Eduardo, > > I have some question regrading Intel PT live migration. > > Commit "e37a5c7fa459 (i386: Add Intel Processor Trace feature support)" > expose Intel PT with a fixed capabilities of CPUID 0x14 for live migration. > And the fixed capabilities are the value reported on ICX(IceLake). However, > the upcoming SPR(Sapphire Rapids) has less capabilities of > INTEL_PT_CYCLE_BITMAP than ICX. It fails to enable PT in guest on SPR > machine. > > If change the check on INTEL_PT_CYCLE_BITMAP to allow different value to > allow it work on SPR. I think it breaks live migration, right? If I understand your description correctly, I don't think that would break live migration. Generating different CPUID data for the same CPU model+flags would break live migration. However, merely allowing a wider set of configurations to work wouldn't break live migration, as long as the same CPU model+flags generates the same CPUID data on any host. > > For me, not making each sub-function of PT as configurable in QEMU indeed > makes it hard for live migration. [...] Making all sub-functions of PT configurable would be welcome. actually. That would allow us to support a wider range of configurations and keep live migration working at the same time. > [...] Why not make PT as unmigratable in the > first place when introducing the support in QEMU? I don't know, this was not my decision. Now we support live migration in a few specific cases (ICX hosts), and I assume we don't want to stop supporting it. If you just want to support a larger set of hosts when live migration is not needed, you can add support to that use case too. e.g.: "-cpu host,migratable=off" and/or "-cpu ...,intel-pt-passthrough=on" could do host passthrough of intel-pt sub-leaves (and block live migration). -- Eduardo
Re: [PATCH 02/19] host-utils: move abs64() to host-utils
On Wed, Aug 25, 2021 at 12:48:35PM +, Luis Fernando Fujita Pires wrote: > From: David Gibson > > Hrm.. I'm a bit concerned about mkaing this a more widespread function, > > because it has a nasty edge case... which is basically unavoidable in an > > abs64() > > implementation. Specifically: > > > > abs64(0x800___0) == 0x800___ < 0 > > > > At least in the most likely 2's complement implementation. > > Right, that's true of any standard implementation of abs(). > I thought about making it return uint64_t, but that could make > it weird for other uses of abs64(), where callers wouldn't > expect a type change from int64_t to uint64_t. Maybe create a > separate uabs64() that returns uint64_t? Or is that even > weirder? :) Which users of abs64 would expect it to return int64_t? kvm_pit_update_clock_offset() doesn't seem to. -- Eduardo
[PULL 1/2] target/i386: Remove split lock detect in Snowridge CPU model
From: Chenyi Qiang
At present, there's no mechanism intelligent enough to virtualize split
lock detection correctly. Remove it in Snowridge CPU model to avoid the
feature exposure.
Signed-off-by: Chenyi Qiang
Message-Id: <20210630012053.10098-1-chenyi.qi...@intel.com>
Signed-off-by: Eduardo Habkost
---
target/i386/cpu.c | 8
1 file changed, 8 insertions(+)
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 34a7ce865bb..aebf81d9c98 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -3682,6 +3682,14 @@ static const X86CPUDefinition builtin_x86_defs[] = {
{ /* end of list */ },
},
},
+{
+.version = 4,
+.note = "no split lock detect",
+.props = (PropValue[]) {
+{ "split-lock-detect", "off" },
+{ /* end of list */ },
+},
+},
{ /* end of list */ },
},
},
--
2.31.1
[PULL 2/2] i386/cpu: Remove AVX_VNNI feature from Cooperlake cpu model
From: Yang Zhong
The AVX_VNNI feature is not in Cooperlake platform, remove it
from cpu model.
Signed-off-by: Yang Zhong
Message-Id: <20210820054611.84303-1-yang.zh...@intel.com>
Fixes: c1826ea6a052 ("i386/cpu: Expose AVX_VNNI instruction to guest")
Cc: qemu-sta...@nongnu.org
Signed-off-by: Eduardo Habkost
---
target/i386/cpu.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index aebf81d9c98..97e250e8760 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -3102,7 +3102,7 @@ static const X86CPUDefinition builtin_x86_defs[] = {
MSR_ARCH_CAP_SKIP_L1DFL_VMENTRY | MSR_ARCH_CAP_MDS_NO |
MSR_ARCH_CAP_PSCHANGE_MC_NO | MSR_ARCH_CAP_TAA_NO,
.features[FEAT_7_1_EAX] =
-CPUID_7_1_EAX_AVX_VNNI | CPUID_7_1_EAX_AVX512_BF16,
+CPUID_7_1_EAX_AVX512_BF16,
/* XSAVES is added in version 2 */
.features[FEAT_XSAVE] =
CPUID_XSAVE_XSAVEOPT | CPUID_XSAVE_XSAVEC |
--
2.31.1
[PULL 0/2] x86 queue, 2021-08-25
The following changes since commit d42685765653ec155fdf60910662f8830bdb2cef: Open 6.2 development tree (2021-08-25 10:25:12 +0100) are available in the Git repository at: https://gitlab.com/ehabkost/qemu.git tags/x86-next-pull-request for you to fetch changes up to f429dbf8fc526a9cacf531176b28d0c65701475a: i386/cpu: Remove AVX_VNNI feature from Cooperlake cpu model (2021-08-25 12:36:49 -0400) x86 queue, 2021-08-25 Bug fixes: * Remove split lock detect in Snowridge CPU model (Chenyi Qiang) * Remove AVX_VNNI feature from Cooperlake cpu model (Yang Zhong) Chenyi Qiang (1): target/i386: Remove split lock detect in Snowridge CPU model Yang Zhong (1): i386/cpu: Remove AVX_VNNI feature from Cooperlake cpu model target/i386/cpu.c | 10 +- 1 file changed, 9 insertions(+), 1 deletion(-) -- 2.31.1
Re: [PATCH v2 2/3] hw/usb/hcd-xhci-pci: Abort if setting link property failed
On Tue, Aug 24, 2021 at 01:16:40PM +0100, Peter Maydell wrote:
> On Tue, 24 Aug 2021 at 13:05, Markus Armbruster wrote:
> > When you know that all callers handle errors like _fatal does, use
> > of _fatal doesn't produce wrong behavior. It's still kind of
> > wrong, because relying on such a non-local argument without a genuine
> > need is.
>
> Not using error_fatal results in quite a bit of extra boilerplate
> for all those extra explicit "check for failure, print the error
> message and exit" points.
I don't get it. There's no need for extra boilerplate if the
caller is using _fatal.
> If the MachineState init function took
> an Error** that might be a mild encouragement to "pass an Error
> upward rather than exiting"; but it doesn't.
Agreed.
>
> Right now we have nearly a thousand instances of error_fatal
> in the codebase, incidentally.
It looks like 73 of them are in functions that take an Error**
argument.
Coccinelle patch for finding them:
@@
typedef Error;
type T;
identifier errp, fn;
@@
T fn(..., Error **errp)
{
...
*_fatal
...
}
Coccinelle output:
diff -u -p ./hw/pci-host/pnv_phb3.c /tmp/nothing/hw/pci-host/pnv_phb3.c
--- ./hw/pci-host/pnv_phb3.c
+++ /tmp/nothing/hw/pci-host/pnv_phb3.c
@@ -1054,7 +1054,6 @@ static void pnv_phb3_realize(DeviceState
/* Add a single Root port */
qdev_prop_set_uint8(DEVICE(>root), "chassis", phb->chip_id);
qdev_prop_set_uint16(DEVICE(>root), "slot", phb->phb_id);
-qdev_realize(DEVICE(>root), BUS(pci->bus), _fatal);
}
void pnv_phb3_update_regions(PnvPHB3 *phb)
diff -u -p ./hw/pci-host/q35.c /tmp/nothing/hw/pci-host/q35.c
--- ./hw/pci-host/q35.c
+++ /tmp/nothing/hw/pci-host/q35.c
@@ -67,7 +67,6 @@ static void q35_host_realize(DeviceState
PC_MACHINE(qdev_get_machine())->bus = pci->bus;
pci->bypass_iommu =
PC_MACHINE(qdev_get_machine())->default_bus_bypass_iommu;
-qdev_realize(DEVICE(>mch), BUS(pci->bus), _fatal);
}
static const char *q35_host_root_bus_path(PCIHostState *host_bridge,
diff -u -p ./hw/pci-host/mv64361.c /tmp/nothing/hw/pci-host/mv64361.c
--- ./hw/pci-host/mv64361.c
+++ /tmp/nothing/hw/pci-host/mv64361.c
@@ -875,7 +875,6 @@ static void mv64361_realize(DeviceState
TYPE_MV64361_PCI);
DeviceState *pci = DEVICE(>pci[i]);
qdev_prop_set_uint8(pci, "index", i);
-sysbus_realize_and_unref(SYS_BUS_DEVICE(pci), _fatal);
}
sysbus_init_irq(SYS_BUS_DEVICE(dev), >cpu_irq);
qdev_init_gpio_in_named(dev, mv64361_gpp_irq, "gpp", 32);
diff -u -p ./hw/pci-host/designware.c /tmp/nothing/hw/pci-host/designware.c
--- ./hw/pci-host/designware.c
+++ /tmp/nothing/hw/pci-host/designware.c
@@ -707,7 +707,6 @@ static void designware_pcie_host_realize
"pcie-bus-address-space");
pci_setup_iommu(pci->bus, designware_pcie_host_set_iommu, s);
-qdev_realize(DEVICE(>root), BUS(pci->bus), _fatal);
}
static const VMStateDescription vmstate_designware_pcie_host = {
diff -u -p ./hw/pci-host/raven.c /tmp/nothing/hw/pci-host/raven.c
--- ./hw/pci-host/raven.c
+++ /tmp/nothing/hw/pci-host/raven.c
@@ -335,7 +335,6 @@ static void raven_realize(PCIDevice *d,
d->config[0x34] = 0x00; // capabilities_pointer
memory_region_init_rom_nomigrate(>bios, OBJECT(s), "bios", BIOS_SIZE,
- _fatal);
memory_region_add_subregion(get_system_memory(), (uint32_t)(-BIOS_SIZE),
>bios);
if (s->bios_name) {
diff -u -p ./hw/pci-host/gpex.c /tmp/nothing/hw/pci-host/gpex.c
--- ./hw/pci-host/gpex.c
+++ /tmp/nothing/hw/pci-host/gpex.c
@@ -138,7 +138,6 @@ static void gpex_host_realize(DeviceStat
>io_ioport, 0, 4, TYPE_PCIE_BUS);
pci_bus_set_route_irq_fn(pci->bus, gpex_route_intx_pin_to_irq);
-qdev_realize(DEVICE(>gpex_root), BUS(pci->bus), _fatal);
}
static const char *gpex_host_root_bus_path(PCIHostState *host_bridge,
diff -u -p ./hw/pci-host/xilinx-pcie.c /tmp/nothing/hw/pci-host/xilinx-pcie.c
--- ./hw/pci-host/xilinx-pcie.c
+++ /tmp/nothing/hw/pci-host/xilinx-pcie.c
@@ -137,7 +137,6 @@ static void xilinx_pcie_host_realize(Dev
pci_swizzle_map_irq_fn, s, >mmio,
>io, 0, 4, TYPE_PCIE_BUS);
-qdev_realize(DEVICE(>root), BUS(pci->bus), _fatal);
}
static const char *xilinx_pcie_host_root_bus_path(PCIHostState *host_bridge,
diff -u -p ./hw/ppc/spapr_irq.c /tmp/nothing/hw/ppc/spapr_irq.c
--- ./hw/ppc/spapr_irq.c
+++ /tmp/nothing/hw/ppc/spapr_irq.c
@@ -337,7 +337,6 @@ void spapr_irq_init(SpaprMachineState *s
qdev_prop_set_uint32(dev, "nr-ends", nr_servers << 3);
object_property_set_link(OBJECT(dev), "xive-fabric", OBJECT(spapr),
Re: [PATCH 4/4] vl: Prioritize realizations of devices
On Mon, Aug 23, 2021 at 05:31:46PM -0400, Peter Xu wrote: > On Mon, Aug 23, 2021 at 05:07:03PM -0400, Eduardo Habkost wrote: > > To give just one example: > > > > $ (echo 'info pci';echo quit;) | qemu-system-x86_64 -device virtio-net-pci > > -device e1000e -monitor stdio | tail -n 20 > > Bus 0, device 4, function 0: > > Ethernet controller: PCI device 1af4:1000 > > PCI subsystem 1af4:0001 > > IRQ 0, pin A > > BAR0: I/O at 0x [0x001e]. > > BAR1: 32 bit memory at 0x [0x0ffe]. > > BAR4: 64 bit prefetchable memory at 0x [0x3ffe]. > > BAR6: 32 bit memory at 0x [0x0003fffe]. > > id "" > > Bus 0, device 5, function 0: > > Ethernet controller: PCI device 8086:10d3 > > PCI subsystem 8086: > > IRQ 0, pin A > > BAR0: 32 bit memory at 0x [0x0001fffe]. > > BAR1: 32 bit memory at 0x [0x0001fffe]. > > BAR2: I/O at 0x [0x001e]. > > BAR3: 32 bit memory at 0x [0x3ffe]. > > BAR6: 32 bit memory at 0x [0x0003fffe]. > > id "" > > (qemu) quit > > $ (echo 'info pci';echo quit;) | qemu-system-x86_64 -device e1000e -device > > virtio-net-pci -monitor stdio | tail -n 20 > > Bus 0, device 4, function 0: > > Ethernet controller: PCI device 8086:10d3 > > PCI subsystem 8086: > > IRQ 0, pin A > > BAR0: 32 bit memory at 0x [0x0001fffe]. > > BAR1: 32 bit memory at 0x [0x0001fffe]. > > BAR2: I/O at 0x [0x001e]. > > BAR3: 32 bit memory at 0x [0x3ffe]. > > BAR6: 32 bit memory at 0x [0x0003fffe]. > > id "" > > Bus 0, device 5, function 0: > > Ethernet controller: PCI device 1af4:1000 > > PCI subsystem 1af4:0001 > > IRQ 0, pin A > > BAR0: I/O at 0x [0x001e]. > > BAR1: 32 bit memory at 0x [0x0ffe]. > > BAR4: 64 bit prefetchable memory at 0x [0x3ffe]. > > BAR6: 32 bit memory at 0x [0x0003fffe]. > > id "" > > (qemu) quit > > > > > > If the order of the -device arguments changes, the devices are assigned to > > different PCI slots. > > Thanks for the example. > > Initially I thought about this and didn't think it an issue (because serious > users will always specify addr=XXX for -device; I thought libvirt always does > that), but I do remember that guest OS could identify its hardware config with > devfn number, so nmcli may mess up its config with before/after this change > indeed.. > > I can use a custom sort to replace qsort() to guarantee that. > > Do you have other examples in mind that I may have overlooked, especially I > may > not be able to fix by a custom sort with only moving priority>=1 devices? I don't have any other example, but I assume address assignment based on ordering is a common pattern in device code. I would take a very close and careful look at the devices with non-default vmsd priority. If you can prove that the 13 device types with non-default priority are all order-insensitive, a custom sort function as you describe might be safe. -- Eduardo
Re: [PATCH 4/4] vl: Prioritize realizations of devices
On Mon, Aug 23, 2021 at 03:18:51PM -0400, Peter Xu wrote: > On Mon, Aug 23, 2021 at 02:49:12PM -0400, Eduardo Habkost wrote: > > On Wed, Aug 18, 2021 at 03:43:18PM -0400, Peter Xu wrote: > > > QEMU creates -device objects in order as specified by the user's cmdline. > > > However that ordering may not be the ideal order. For example, some > > > platform > > > devices (vIOMMUs) may want to be created earlier than most of the rest > > > devices (e.g., vfio-pci, virtio). > > > > > > This patch orders the QemuOptsList of '-device's so they'll be sorted > > > first > > > before kicking off the device realizations. This will allow the device > > > realization code to be able to use APIs like > > > pci_device_iommu_address_space() > > > correctly, because those functions rely on the platfrom devices being > > > realized. > > > > > > Now we rely on vmsd->priority which is defined as MigrationPriority to > > > provide > > > the ordering, as either VM init and migration completes will need such an > > > ordering. In the future we can move that priority information out of > > > vmsd. > > > > > > Signed-off-by: Peter Xu > > > > Can we be 100% sure that changing the ordering of every single > > device being created won't affect guest ABI? (I don't think we can) > > That's a good question, however I doubt whether there's any real-world guest > ABI for that. As a developer, I normally specify cmdline parameter in an > adhoc > way, so that I assume most parameters are not sensitive to ordering and I can > tune the ordering as wish. I'm not sure whether that's common for qemu users, > I would expect so, but I may have missed something that I'm not aware of. To give just one example: $ (echo 'info pci';echo quit;) | qemu-system-x86_64 -device virtio-net-pci -device e1000e -monitor stdio | tail -n 20 Bus 0, device 4, function 0: Ethernet controller: PCI device 1af4:1000 PCI subsystem 1af4:0001 IRQ 0, pin A BAR0: I/O at 0x [0x001e]. BAR1: 32 bit memory at 0x [0x0ffe]. BAR4: 64 bit prefetchable memory at 0x [0x3ffe]. BAR6: 32 bit memory at 0x [0x0003fffe]. id "" Bus 0, device 5, function 0: Ethernet controller: PCI device 8086:10d3 PCI subsystem 8086: IRQ 0, pin A BAR0: 32 bit memory at 0x [0x0001fffe]. BAR1: 32 bit memory at 0x [0x0001fffe]. BAR2: I/O at 0x [0x001e]. BAR3: 32 bit memory at 0x [0x3ffe]. BAR6: 32 bit memory at 0x [0x0003fffe]. id "" (qemu) quit $ (echo 'info pci';echo quit;) | qemu-system-x86_64 -device e1000e -device virtio-net-pci -monitor stdio | tail -n 20 Bus 0, device 4, function 0: Ethernet controller: PCI device 8086:10d3 PCI subsystem 8086: IRQ 0, pin A BAR0: 32 bit memory at 0x [0x0001fffe]. BAR1: 32 bit memory at 0x [0x0001fffe]. BAR2: I/O at 0x [0x001e]. BAR3: 32 bit memory at 0x [0x3ffe]. BAR6: 32 bit memory at 0x [0x0003fffe]. id "" Bus 0, device 5, function 0: Ethernet controller: PCI device 1af4:1000 PCI subsystem 1af4:0001 IRQ 0, pin A BAR0: I/O at 0x [0x001e]. BAR1: 32 bit memory at 0x [0x0ffe]. BAR4: 64 bit prefetchable memory at 0x [0x3ffe]. BAR6: 32 bit memory at 0x [0x0003fffe]. id "" (qemu) quit If the order of the -device arguments changes, the devices are assigned to different PCI slots. > > Per my knowledge the only "guest ABI" change is e.g. when we specify > "vfio-pci" > to be before "intel-iommu": it'll be constantly broken before this patchset, > while after this series it'll be working. It's just that I don't think those > "guest ABI" is necessary to be kept, and that's exactly what I want to fix > with > the patchset.. If the only ordering changes caused by this patch were intentional and affected only configurations that are known to be broken (like vfio-pci vs intel-iommu), I would agree. However, if we are reordering every single -device option in an unspecified way (like qsort() does when elements compare as equal), we are probably breaking guest ABI and creating a completely different machine (like in the PCI example above). > > > > > How many device types in QEMU have non-default vmsd priority? > > Not so much; here's the list of prioriti
Re: [PATCH 4/4] vl: Prioritize realizations of devices
On Wed, Aug 18, 2021 at 03:43:18PM -0400, Peter Xu wrote: > QEMU creates -device objects in order as specified by the user's cmdline. > However that ordering may not be the ideal order. For example, some platform > devices (vIOMMUs) may want to be created earlier than most of the rest > devices (e.g., vfio-pci, virtio). > > This patch orders the QemuOptsList of '-device's so they'll be sorted first > before kicking off the device realizations. This will allow the device > realization code to be able to use APIs like pci_device_iommu_address_space() > correctly, because those functions rely on the platfrom devices being > realized. > > Now we rely on vmsd->priority which is defined as MigrationPriority to provide > the ordering, as either VM init and migration completes will need such an > ordering. In the future we can move that priority information out of vmsd. > > Signed-off-by: Peter Xu Can we be 100% sure that changing the ordering of every single device being created won't affect guest ABI? (I don't think we can) How many device types in QEMU have non-default vmsd priority? Can we at least ensure devices with the same priority won't be reordered, just to be safe? (qsort() doesn't guarantee that) If very few device types have non-default vmsd priority and devices with the same priority aren't reordered, the risk of compatibility breakage would be much smaller. -- Eduardo
Re: [PATCH v2 2/3] hw/usb/hcd-xhci-pci: Abort if setting link property failed
+Markus
On Thu, Aug 19, 2021 at 07:15:46PM +0200, Philippe Mathieu-Daudé wrote:
> Do not ignore eventual error if we failed at setting the 'host'
> property of the TYPE_XHCI model.
>
> Signed-off-by: Philippe Mathieu-Daudé
> ---
> hw/usb/hcd-xhci-pci.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/usb/hcd-xhci-pci.c b/hw/usb/hcd-xhci-pci.c
> index e934b1a5b1f..71f6629ccde 100644
> --- a/hw/usb/hcd-xhci-pci.c
> +++ b/hw/usb/hcd-xhci-pci.c
> @@ -115,7 +115,7 @@ static void usb_xhci_pci_realize(struct PCIDevice *dev,
> Error **errp)
> dev->config[PCI_CACHE_LINE_SIZE] = 0x10;
> dev->config[0x60] = 0x30; /* release number */
>
> -object_property_set_link(OBJECT(>xhci), "host", OBJECT(s), NULL);
> +object_property_set_link(OBJECT(>xhci), "host", OBJECT(s),
> _fatal);
If this fails, it's due to programmer error, isn't? Shouldn't we
use _abort on that case?
> s->xhci.intr_update = xhci_pci_intr_update;
> s->xhci.intr_raise = xhci_pci_intr_raise;
> if (!qdev_realize(DEVICE(>xhci), NULL, errp)) {
> --
> 2.31.1
>
--
Eduardo
Re: [PATCH v12] qapi: introduce 'query-x86-cpuid' QMP command.
On Mon, Aug 23, 2021 at 9:35 AM Markus Armbruster wrote: > > Eduardo Habkost writes: > > > On Wed, Aug 11, 2021 at 9:44 AM Thomas Huth wrote: > >> > >> On 11/08/2021 15.40, Eduardo Habkost wrote: > >> > On Wed, Aug 11, 2021 at 2:10 AM Thomas Huth wrote: > >> >> > >> >> On 10/08/2021 20.56, Eduardo Habkost wrote: > >> >>> On Sat, Aug 07, 2021 at 04:22:42PM +0200, Markus Armbruster wrote: > >> >>>> Is this intended to be a stable interface? Interfaces intended just > >> >>>> for > >> >>>> debugging usually aren't. > >> >>> > >> >>> I don't think we need to make it a stable interface, but I won't > >> >>> mind if we declare it stable. > >> >> > >> >> If we don't feel 100% certain yet, it's maybe better to introduce this > >> >> with > >> >> a "x-" prefix first, isn't it? I.e. "x-query-x86-cpuid" ... then it's > >> >> clear > >> >> that this is only experimental/debugging/not-stable yet. Just my 0.02 €. > >> > > >> > That would be my expectation. Is this a documented policy? > >> > > >> > >> According to docs/interop/qmp-spec.txt : > >> > >> Any command or member name beginning with "x-" is deemed > >> experimental, and may be withdrawn or changed in an incompatible > >> manner in a future release. > > > > Thanks! I had looked at other QMP docs, but not qmp-spec.txt. > > > > In my reply above, please read "make it a stable interface" as > > "declare it as supported by not using the 'x-' prefix". > > > > I don't think we have to make it stable, but I won't argue against it > > if the current proposal is deemed acceptable by other maintainers. > > > > Personally, I'm still frustrated by the complexity of the current > > proposal, but I don't want to block it just because of my frustration. > > Is this a case of "there must be a simpler way", or did you actually > propose a simpler way? I don't remember... > I did propose a simpler way at https://lore.kernel.org/qemu-devel/20210810195053.6vsjadglrexf6...@habkost.net/ -- Eduardo
Re: [PATCH] i386/cpu: Remove AVX_VNNI feature from Cooperlake cpu model
On Fri, Aug 20, 2021 at 01:46:11PM +0800, Yang Zhong wrote:
> The AVX_VNNI feature is not in Cooperlake platform, remove it
> from cpu model.
>
> Signed-off-by: Yang Zhong
Fixes: c1826ea6a052 ("i386/cpu: Expose AVX_VNNI instruction to guest")
Queued, thanks!
--
Eduardo
Re: [PATCH v3 50/66] hw/core/cpu: Move cpu properties to cpu-sysemu.c
On Thu, Aug 19, 2021 at 04:26:10PM +0100, Peter Maydell wrote: > On Wed, 18 Aug 2021 at 21:09, Richard Henderson > wrote: > > > > The comment in cpu-common.c is absolutely correct, we can't > > rely on the ifdef in a file built once. This was only "working" > > because we used ifndef. > > > > Signed-off-by: Richard Henderson > > Fixes: 1b36e4f5a5de585 > > which moved the properties out of cpu.c and into cpu-common.c > with the remark "There's no reason to keep the property list > separate from the CPU class code" despite there being a big > fat warning comment saying why it can't go in a compiled-once > source file ! Ouch. Sorry about that. :( > > Is there a reason to prefer this patch over just reverting > 1b36e4f5a5de585 ? I agree with reverting the commit. -- Eduardo
Re: [PATCH v3 09/25] python/aqmp: add AsyncProtocol.accept() method
On Thu, Aug 19, 2021 at 11:48:16AM -0400, John Snow wrote:
> On Thu, Aug 19, 2021 at 10:50 AM Eric Blake wrote:
>
> > On Wed, Aug 18, 2021 at 10:24:52AM -0400, John Snow wrote:
> > > > >
> > > > > +@upper_half
> > > > > +@require(Runstate.IDLE)
> > > > > +async def accept(self, address: Union[str, Tuple[str, int]],
> > > > > + ssl: Optional[SSLContext] = None) -> None:
> > > > > +"""
> > > > > +Accept a connection and begin processing message queues.
> > > > > +
> > > > > +If this call fails, `runstate` is guaranteed to be set back
> > to
> > > > `IDLE`.
> > > > > +
> > > > > +:param address:
> > > > > +Address to listen to; UNIX socket path or TCP
> > address/port.
> > > >
> > > > Can't TCP use a well-known port name instead of an int? But limiting
> > > > clients to just int port for now isn't fatal to the patch.
> > > >
> > > >
> > > The old QMP library didn't support this, and I used the old library as my
> > > template here. I'm willing to change the address format and types to be
> > > more comprehensive, but I was thinking that it should probably try to
> > match
> > > or adhere to some standard; de-facto or otherwise. I wasn't sure which to
> > > pick, and we use a few different ones in QEMU itself. Any recommendations
> > > for me?
> >
> > I asked because I know QAPI specifies TCP as string/string (the
> > hostname as a string makes absolute sense, but the port number as a
> > string is because of the less-used feature of a well-known port name).
> > I'm fine if the initial patch uses an int for the port number here; we
> > can always add support for more formats down the road when someone
> > actually has a use for them.
> >
> >
> https://docs.python.org/3/library/socket.html#socket-families
>
> "A pair (host, port) is used for the AF_INET address family, where host is
> a string representing either a hostname in Internet domain notation like '
> daring.cwi.nl' or an IPv4 address like '100.50.200.5', and port is an
> integer."
>
> The docs seem to suggest that I am actually limited only to integers here.
> Do you have an example of using a string for a port number? I have to admit
> I am not well acquainted with it.
QEMU uses getaddrinfo() at inet_parse_connect_saddr() to translate the
string/string pair to a socket address.
Python equivalent:
>> socket.getaddrinfo('localhost', 'ssh')
[(, , 6, '', ('::1', 22,
0, 0)), (, , 17, '',
('::1', 22, 0, 0)), (, ,
132, '', ('::1', 22, 0, 0)), (, , 132, '', ('::1', 22, 0,
0)), (, , 6, '',
('127.0.0.1', 22)), (, ,
17, '', ('127.0.0.1', 22)), (, , 132, '', ('127.0.0.1', 22)),
(, , 132, '',
('127.0.0.1', 22))]
Translating this to the correct arguments to socket.socket() and
socket.socket.connect() seems overly complicated, though.
--
Eduardo
[PULL 1/1] machine: Disallow specifying topology parameters as zero
From: Yanan Wang
In the SMP configuration, we should either provide a topology
parameter with a reasonable value (greater than zero) or just
omit it and QEMU will compute the missing value. Users should
have never provided a configuration with parameters as zero
(e.g. -smp 8,sockets=0) which should be treated as invalid.
But commit 1e63fe68580 (machine: pass QAPI struct to mc->smp_parse)
has added some doc which implied that 'anything=0' is valid and
has the same semantics as omitting a parameter.
To avoid meaningless configurations possibly introduced by users
in the future and consequently a necessary deprecation process,
fix the doc and also add the corresponding sanity check.
Fixes: 1e63fe68580 (machine: pass QAPI struct to mc->smp_parse)
Suggested-by: Andrew Jones
Signed-off-by: Yanan Wang
Reviewed-by: Daniel P. Berrange
Tested-by: Daniel P. Berrange
Reviewed-by: Andrew Jones
Reviewed-by: Cornelia Huck
Message-Id: <20210816024522.143124-2-wangyana...@huawei.com>
Signed-off-by: Eduardo Habkost
---
hw/core/machine.c | 14 ++
qapi/machine.json | 6 +++---
qemu-options.hx | 12 +++-
3 files changed, 24 insertions(+), 8 deletions(-)
diff --git a/hw/core/machine.c b/hw/core/machine.c
index 54e040587dd..a7e119469aa 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -832,6 +832,20 @@ static void machine_set_smp(Object *obj, Visitor *v, const
char *name,
return;
}
+/*
+ * A specified topology parameter must be greater than zero,
+ * explicit configuration like "cpus=0" is not allowed.
+ */
+if ((config->has_cpus && config->cpus == 0) ||
+(config->has_sockets && config->sockets == 0) ||
+(config->has_dies && config->dies == 0) ||
+(config->has_cores && config->cores == 0) ||
+(config->has_threads && config->threads == 0) ||
+(config->has_maxcpus && config->maxcpus == 0)) {
+error_setg(errp, "CPU topology parameters must be greater than zero");
+goto out_free;
+}
+
mc->smp_parse(ms, config, errp);
if (*errp) {
goto out_free;
diff --git a/qapi/machine.json b/qapi/machine.json
index c3210ee1fb2..9272cb3cf8b 100644
--- a/qapi/machine.json
+++ b/qapi/machine.json
@@ -1288,8 +1288,8 @@
##
# @SMPConfiguration:
#
-# Schema for CPU topology configuration. "0" or a missing value lets
-# QEMU figure out a suitable value based on the ones that are provided.
+# Schema for CPU topology configuration. A missing value lets QEMU
+# figure out a suitable value based on the ones that are provided.
#
# @cpus: number of virtual CPUs in the virtual machine
#
@@ -1297,7 +1297,7 @@
#
# @dies: number of dies per socket in the CPU topology
#
-# @cores: number of cores per thread in the CPU topology
+# @cores: number of cores per die in the CPU topology
#
# @threads: number of threads per core in the CPU topology
#
diff --git a/qemu-options.hx b/qemu-options.hx
index 83aa59a920f..aee622f577d 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -227,11 +227,13 @@ SRST
of computing the CPU maximum count.
Either the initial CPU count, or at least one of the topology parameters
-must be specified. Values for any omitted parameters will be computed
-from those which are given. Historically preference was given to the
-coarsest topology parameters when computing missing values (ie sockets
-preferred over cores, which were preferred over threads), however, this
-behaviour is considered liable to change.
+must be specified. The specified parameters must be greater than zero,
+explicit configuration like "cpus=0" is not allowed. Values for any
+omitted parameters will be computed from those which are given.
+Historically preference was given to the coarsest topology parameters
+when computing missing values (ie sockets preferred over cores, which
+were preferred over threads), however, this behaviour is considered
+liable to change.
ERST
DEF("numa", HAS_ARG, QEMU_OPTION_numa,
--
2.31.1
[PULL 0/1] Last minute fix for -rc4
The following changes since commit bd44d64a3879bb6b0ca19bff3be16e0093502fac: Merge remote-tracking branch 'remotes/thuth-gitlab/tags/pull-request-2021-08-11' into staging (2021-08-15 16:46:23 +0100) are available in the Git repository at: https://gitlab.com/ehabkost/qemu.git tags/machine-next-pull-request for you to fetch changes up to bbd0db9dc4751b6ab0884e92421fa4b2c3d3b532: machine: Disallow specifying topology parameters as zero (2021-08-16 16:55:39 -0400) Last minute fix for -rc4 Bug fix: * Disallow specifying topology parameters as zero (Yanan Wang) Yanan Wang (1): machine: Disallow specifying topology parameters as zero hw/core/machine.c | 14 ++ qapi/machine.json | 6 +++--- qemu-options.hx | 12 +++- 3 files changed, 24 insertions(+), 8 deletions(-) -- 2.31.1
[PULL 0/1] Bug fix for -rc4
This is a bug fix to be included in case we are going to have a 6.1.0-rc4. I don't think this bug alone should delay the release of QEMU 6.1.0. The following changes since commit 703e8cd6189cf699c8d5c094bc68b5f3afa6ad71: Update version for v6.1.0-rc3 release (2021-08-10 19:08:09 +0100) are available in the Git repository at: https://gitlab.com/ehabkost/qemu.git tags/machine-next-pull-request for you to fetch changes up to 0fa1eecc092feb5a4a373ff1fa761ad3a03ea2d9: hw/core: fix error checkig in smp_parse (2021-08-12 14:58:50 -0400) Bug fix for -rc4 Bug fix: * Fix error checkig in smp_parse (Daniel P. Berrangé) Daniel P. Berrangé (1): hw/core: fix error checkig in smp_parse hw/core/machine.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) -- 2.31.1
[PULL 1/1] hw/core: fix error checkig in smp_parse
From: Daniel P. Berrangé
The machine_set_smp() mistakenly checks 'errp' not '*errp',
and so thinks there is an error every single time it runs.
This causes it to jump to the end of the method, skipping
the max CPUs checks. The caller meanwhile sees no error
and so carries on execution. The result of all this is:
$ qemu-system-x86_64 -smp -1
qemu-system-x86_64: GLib: ../glib/gmem.c:142: failed to allocate 481036337048
bytes
instead of
$ qemu-system-x86_64 -smp -1
qemu-system-x86_64: Invalid SMP CPUs -1. The max CPUs supported by machine
'pc-i440fx-6.1' is 255
This is a regression from
commit fe68090e8fbd6e831aaf3fc3bb0459c5cccf14cf
Author: Paolo Bonzini
Date: Thu May 13 09:03:48 2021 -0400
machine: add smp compound property
Closes: https://gitlab.com/qemu-project/qemu/-/issues/524
Signed-off-by: Daniel P. Berrangé
Message-Id: <20210812175353.4128471-1-berra...@redhat.com>
Signed-off-by: Eduardo Habkost
---
hw/core/machine.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/core/machine.c b/hw/core/machine.c
index 943974d411c..ab4fca6546a 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -832,7 +832,7 @@ static void machine_set_smp(Object *obj, Visitor *v, const
char *name,
}
mc->smp_parse(ms, config, errp);
-if (errp) {
+if (*errp) {
goto out_free;
}
--
2.31.1
Re: [PATCH for-6.1 ?] hw/core: fix error checkig in smp_parse
On Thu, Aug 12, 2021 at 06:53:53PM +0100, Daniel P. Berrangé wrote:
> The machine_set_smp() mistakenly checks 'errp' not '*errp',
> and so thinks there is an error every single time it runs.
> This causes it to jump to the end of the method, skipping
> the max CPUs checks. The caller meanwhile sees no error
> and so carries on execution. The result of all this is:
>
> $ qemu-system-x86_64 -smp -1
> qemu-system-x86_64: GLib: ../glib/gmem.c:142: failed to allocate
> 481036337048 bytes
>
> instead of
>
> $ qemu-system-x86_64 -smp -1
> qemu-system-x86_64: Invalid SMP CPUs -1. The max CPUs supported by machine
> 'pc-i440fx-6.1' is 255
>
> This is a regression from
>
> commit fe68090e8fbd6e831aaf3fc3bb0459c5cccf14cf
> Author: Paolo Bonzini
> Date: Thu May 13 09:03:48 2021 -0400
>
> machine: add smp compound property
>
> Closes: https://gitlab.com/qemu-project/qemu/-/issues/524
> Signed-off-by: Daniel P. Berrangé
I will prepare a pull request with this, just in case we are
already going to have a -rc4. I don't think this bug alone
should delay release of 6.1, though.
> ---
> hw/core/machine.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index 943974d411..ab4fca6546 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -832,7 +832,7 @@ static void machine_set_smp(Object *obj, Visitor *v,
> const char *name,
> }
>
> mc->smp_parse(ms, config, errp);
> -if (errp) {
> +if (*errp) {
> goto out_free;
> }
>
> --
> 2.31.1
>
--
Eduardo
Re: [PATCH] target/i386: Remove split lock detect in Snowridge CPU model
On Wed, Jun 30, 2021 at 09:20:53AM +0800, Chenyi Qiang wrote: > At present, there's no mechanism intelligent enough to virtualize split > lock detection correctly. Remove it in Snowridge CPU model to avoid the > feature exposure. > > Signed-off-by: Chenyi Qiang Sorry I have missed this before 6.1 soft freeze. I'm queueing this for 6.2. Thanks! -- Eduardo
Re: [PATCH v12] qapi: introduce 'query-x86-cpuid' QMP command.
On Wed, Aug 11, 2021 at 9:44 AM Thomas Huth wrote: > > On 11/08/2021 15.40, Eduardo Habkost wrote: > > On Wed, Aug 11, 2021 at 2:10 AM Thomas Huth wrote: > >> > >> On 10/08/2021 20.56, Eduardo Habkost wrote: > >>> On Sat, Aug 07, 2021 at 04:22:42PM +0200, Markus Armbruster wrote: > >>>> Is this intended to be a stable interface? Interfaces intended just for > >>>> debugging usually aren't. > >>> > >>> I don't think we need to make it a stable interface, but I won't > >>> mind if we declare it stable. > >> > >> If we don't feel 100% certain yet, it's maybe better to introduce this with > >> a "x-" prefix first, isn't it? I.e. "x-query-x86-cpuid" ... then it's clear > >> that this is only experimental/debugging/not-stable yet. Just my 0.02 €. > > > > That would be my expectation. Is this a documented policy? > > > > According to docs/interop/qmp-spec.txt : > > Any command or member name beginning with "x-" is deemed > experimental, and may be withdrawn or changed in an incompatible > manner in a future release. Thanks! I had looked at other QMP docs, but not qmp-spec.txt. In my reply above, please read "make it a stable interface" as "declare it as supported by not using the 'x-' prefix". I don't think we have to make it stable, but I won't argue against it if the current proposal is deemed acceptable by other maintainers. Personally, I'm still frustrated by the complexity of the current proposal, but I don't want to block it just because of my frustration. -- Eduardo
Re: [PATCH v12] qapi: introduce 'query-x86-cpuid' QMP command.
On Wed, Aug 11, 2021 at 2:10 AM Thomas Huth wrote: > > On 10/08/2021 20.56, Eduardo Habkost wrote: > > On Sat, Aug 07, 2021 at 04:22:42PM +0200, Markus Armbruster wrote: > >> Is this intended to be a stable interface? Interfaces intended just for > >> debugging usually aren't. > > > > I don't think we need to make it a stable interface, but I won't > > mind if we declare it stable. > > If we don't feel 100% certain yet, it's maybe better to introduce this with > a "x-" prefix first, isn't it? I.e. "x-query-x86-cpuid" ... then it's clear > that this is only experimental/debugging/not-stable yet. Just my 0.02 €. That would be my expectation. Is this a documented policy? -- Eduardo
Re: [PATCH v14] qapi: introduce 'query-x86-cpuid' QMP command.
On Tue, Aug 10, 2021 at 09:51:31AM +0300, Valeriy Vdovin wrote: > From: Valeriy Vdovin > > Introducing new QMP command 'query-x86-cpuid'. This command can be used to > get virtualized cpu model info generated by QEMU during VM initialization in > the form of cpuid representation. > > Diving into more details about virtual CPU generation: QEMU first parses > '-cpu' > command line option. From there it takes the name of the model as the basis > for > feature set of the new virtual CPU. After that it uses trailing '-cpu' > options, > that state if additional cpu features should be present on the virtual CPU or > excluded from it (tokens '+'/'-' or '=on'/'=off'). > After that QEMU checks if the host's cpu can actually support the derived > feature set and applies host limitations to it. > After this initialization procedure, virtual CPU has it's model and > vendor names, and a working feature set and is ready for identification > instructions such as CPUID. > > To learn exactly how virtual CPU is presented to the guest machine via CPUID > instruction, new QMP command can be used. By calling 'query-x86-cpuid' > command, one can get a full listing of all CPUID leaves with subleaves which > are > supported by the initialized virtual CPU. > > Other than debug, the command is useful in cases when we would like to > utilize QEMU's virtual CPU initialization routines and put the retrieved > values into kernel CPUID overriding mechanics for more precise control > over how various processes perceive its underlying hardware with > container processes as a good example. > > The command is specific to x86. It is currenly only implemented for KVM > acceleator. > > Output format: > The output is a plain list of leaf/subleaf argument combinations, that > return 4 words in registers EAX, EBX, ECX, EDX. > [...] Based on the effort being required from you to make sure this patch is in good shape, maybe you could reconsider my suggestion from a while ago for a single-CPUID-leaf interface, as discussed at: https://lore.kernel.org/qemu-devel/20210421201759.utsmhuopdmlhg...@habkost.net/ A single-CPUID-leaf qmp_query_x86_cpuid() function that is generic and not KVM-specific can probably be implemented in ~5 lines of code. I'm not against the interface proposed here, but you are surely going to get more friction and more complexity to deal with. -- Eduardo
Re: [PATCH v12] qapi: introduce 'query-x86-cpuid' QMP command.
On Sat, Aug 07, 2021 at 04:22:42PM +0200, Markus Armbruster wrote: > Is this intended to be a stable interface? Interfaces intended just for > debugging usually aren't. I don't think we need to make it a stable interface, but I won't mind if we declare it stable. > > Eduardo, what's your take on this version? > I sent my comments as reply to v14: https://lore.kernel.org/qemu-devel/20210810185245.kivvmrmvew6e5...@habkost.net/ -- Eduardo
Re: [PATCH v14] qapi: introduce 'query-x86-cpuid' QMP command.
On Tue, Aug 10, 2021 at 09:51:31AM +0300, Valeriy Vdovin wrote:
> From: Valeriy Vdovin
>
> Introducing new QMP command 'query-x86-cpuid'. This command can be used to
> get virtualized cpu model info generated by QEMU during VM initialization in
> the form of cpuid representation.
>
> Diving into more details about virtual CPU generation: QEMU first parses
> '-cpu'
> command line option. From there it takes the name of the model as the basis
> for
> feature set of the new virtual CPU. After that it uses trailing '-cpu'
> options,
> that state if additional cpu features should be present on the virtual CPU or
> excluded from it (tokens '+'/'-' or '=on'/'=off').
> After that QEMU checks if the host's cpu can actually support the derived
> feature set and applies host limitations to it.
> After this initialization procedure, virtual CPU has it's model and
> vendor names, and a working feature set and is ready for identification
> instructions such as CPUID.
>
> To learn exactly how virtual CPU is presented to the guest machine via CPUID
> instruction, new QMP command can be used. By calling 'query-x86-cpuid'
> command, one can get a full listing of all CPUID leaves with subleaves which
> are
> supported by the initialized virtual CPU.
>
> Other than debug, the command is useful in cases when we would like to
> utilize QEMU's virtual CPU initialization routines and put the retrieved
> values into kernel CPUID overriding mechanics for more precise control
> over how various processes perceive its underlying hardware with
> container processes as a good example.
>
> The command is specific to x86. It is currenly only implemented for KVM
> acceleator.
>
> Output format:
> The output is a plain list of leaf/subleaf argument combinations, that
> return 4 words in registers EAX, EBX, ECX, EDX.
>
> Use example:
> qmp_request: {
> "execute": "query-x86-cpuid"
> }
>
> qmp_response: {
> "return": [
> {
> "eax": 1073741825,
> "edx": 77,
> "in-eax": 1073741824,
> "ecx": 1447775574,
> "ebx": 1263359563
> },
> {
> "eax": 16777339,
> "edx": 0,
> "in-eax": 1073741825,
> "ecx": 0,
> "ebx": 0
> },
> {
> "eax": 13,
> "edx": 1231384169,
> "in-eax": 0,
> "ecx": 1818588270,
> "ebx": 1970169159
> },
> {
> "eax": 198354,
> "edx": 126614527,
> "in-eax": 1,
> "ecx": 2176328193,
> "ebx": 2048
> },
>
> {
> "eax": 12328,
> "edx": 0,
> "in-eax": 2147483656,
> "ecx": 0,
> "ebx": 0
> }
> ]
> }
>
> Signed-off-by: Valeriy Vdovin
> ---
> v2: - Removed leaf/subleaf iterators.
> - Modified cpu_x86_cpuid to return false in cases when count is
> greater than supported subleaves.
> v3: - Fixed structure name coding style.
> - Added more comments
> - Ensured buildability for non-x86 targets.
> v4: - Fixed cpu_x86_cpuid return value logic and handling of 0xA leaf.
> - Fixed comments.
> - Removed target check in qmp_query_cpu_model_cpuid.
> v5: - Added error handling code in qmp_query_cpu_model_cpuid
> v6: - Fixed error handling code. Added method to query_error_class
> v7: - Changed implementation in favor of cached cpuid_data for
> KVM_SET_CPUID2
> v8: - Renamed qmp method to query-kvm-cpuid and some fields in response.
> - Modified documentation to qmp method
> - Removed helper struct declaration
> v9: - Renamed 'in_eax' / 'in_ecx' fields to 'in-eax' / 'in-ecx'
> - Pasted more complete response to commit message.
> v10:
> - Subject changed
> - Fixes in commit message
> - Small fixes in QMP command docs
> v11:
> - Added explanation about CONFIG_KVM to the commit message.
> v12:
> - Changed title from query-kvm-cpuid to query-x86-cpuid
> - Removed CONFIG_KVM ifdefs
> - Added detailed error messages for some stub/unimplemented cases.
> v13:
> - Tagged with since 6.2
> v14:
> - Rebased to latest master 632eda54043d6f26ff87dac16233e14b4708b967
> - Added note about error return cases in api documentation.
>
> qapi/machine-target.json | 46 ++
> softmmu/cpus.c | 2 +-
> target/i386/kvm/kvm-stub.c | 10
> target/i386/kvm/kvm.c | 51 ++
> tests/qtest/qmp-cmd-test.c | 1 +
> 5 files changed, 109 insertions(+), 1 deletion(-)
>
> diff --git a/qapi/machine-target.json b/qapi/machine-target.json
> index e7811654b7..71648a4f56 100644
> --- a/qapi/machine-target.json
> +++ b/qapi/machine-target.json
> @@ -329,3 +329,49 @@
> ##
> { 'command': 'query-cpu-definitions', 'returns': ['CpuDefinitionInfo'],
>'if': 'defined(TARGET_PPC) || defined(TARGET_ARM) || defined(TARGET_I386)
> || defined(TARGET_S390X) || defined(TARGET_MIPS)' }
> +
> +##
> +# @CpuidEntry:
> +#
> +# A single entry of a CPUID response.
> +#
> +# One entry holds full set of information (leaf)
Re: [PATCH for-6.2 02/12] qom: Use DEVICE_*CLASS instead of OBJECT_*CLASS
On Tue, Aug 10, 2021 at 01:56:25PM +0200, Juan Quintela wrote: > > -DeviceClass *dc = OBJECT_CLASS_CHECK(DeviceClass, list->data, > > - TYPE_DEVICE); > > +DeviceClass *dc = DEVICE_CLASS(list->data); > > Finding where DEVICE_CLASS is defined is interesting. That's a valid concern, but I wonder what we can do to address this. The existing practice of defining all macros manually leads to a high number of mistakes and inconsistencies[1]. Now, once all QOM types are converted to the new macros (which is work in progress), maybe we could replace: DEVICE_CLASS(oc) DEVICE_GET_CLASS(dev) with more grep-friendly expressions like: CLASS(DEVICE, oc) GET_CLASS(DEVICE, dev) The type of those expressions would still be (DeviceClass*). --- [1] These are some of the fixes for bugs or inconsistencies that were already merged to qemu.git: 6a567fbcf0b8 nubus: Delete unused NUBUS_BRIDGE macro 98b49b2bea15 spapr: Remove unnecessary DRC type-checker macros 08e14bb7e060 platform-bus: Delete macros for non-existing typedef 5c8b0f2cc799 can_emu: Delete macros for non-existing typedef f58b770fbbd9 virtio-ccw: Fix definition of VIRTIO_CCW_BUS_GET_CLASS These are fixes for broken QOM macros I submitted recently: [PATCH for-6.2 1/6] acpi: Delete broken ACPI_GED_X86 macro [PATCH for-6.2 2/6] sbsa_gwdt: Delete broken SBSA_*CLASS macros And these are some other inconsistencies that are still in the current tree, that need to be addressed: hw/i386/kvm/i8254.c:45:1: type name mismatch: TYPE_KVM_I8254 vs KVM_PIT hw/net/e1000.c:158:1: type name mismatch: TYPE_E1000_BASE vs E1000 hw/rtc/m48t59-isa.c:38:1: mismatching class type for M48TXX_ISA (M48txxISADeviceClass) hw/rtc/m48t59-isa.c:131:1: class type declared here (None) hw/rtc/m48t59.c:47:1: mismatching class type for M48TXX_SYS_BUS (M48txxSysBusDeviceClass) hw/rtc/m48t59.c:654:1: class type declared here (None) hw/s390x/virtio-ccw.h:63:1: typedef name mismatch: VirtioCcwBusState is defined as struct VirtioBusState hw/s390x/virtio-ccw.h:59:1: typedef is here hw/scsi/megasas.c:137:1: type name mismatch: TYPE_MEGASAS_BASE vs MEGASAS hw/virtio/virtio-pci.h:29:1: typedef name mismatch: VirtioPCIBusState is defined as struct VirtioBusState hw/virtio/virtio-pci.h:25:1: typedef is here include/exec/memory.h:48:1: mismatching instance type for RAM_DISCARD_MANAGER (RamDiscardManager) softmmu/memory.c:3418:1: instance type declared here (None) include/hw/isa/superio.h:20:1: mismatching instance type for ISA_SUPERIO (ISASuperIODevice) hw/isa/isa-superio.c:180:1: instance type declared here (None) include/hw/s390x/event-facility.h:197:1: type name mismatch: TYPE_SCLP_EVENT_FACILITY vs EVENT_FACILITY include/hw/s390x/s390-ccw.h:22:1: type name mismatch: TYPE_S390_CCW vs S390_CCW_DEVICE include/hw/vfio/vfio-amd-xgbe.h:43:1: type name mismatch: TYPE_VFIO_AMD_XGBE vs VFIO_AMD_XGBE_DEVICE include/hw/vfio/vfio-calxeda-xgmac.h:40:1: type name mismatch: TYPE_VFIO_CALXEDA_XGMAC vs VFIO_CALXEDA_XGMAC_DEVICE include/hw/vfio/vfio-platform.h:73:1: type name mismatch: TYPE_VFIO_PLATFORM vs VFIO_PLATFORM_DEVICE include/hw/watchdog/wdt_diag288.h:10:1: type name mismatch: TYPE_WDT_DIAG288 vs DIAG288 migration/migration.h:141:1: type name mismatch: TYPE_MIGRATION vs MIGRATION_OBJ target/ppc/cpu.h:1253:1: mismatching instance type for PPC_VIRTUAL_HYPERVISOR (PPCVirtualHypervisor) target/ppc/cpu_init.c:9090:1: instance type declared here (None) -- Eduardo
Re: [PATCH for-6.2 05/12] [automated] Move QOM typedefs and add missing includes
On Tue, Aug 10, 2021 at 02:01:40PM +0200, Juan Quintela wrote:
> Eduardo Habkost wrote:
> > Some typedefs and macros are defined after the type check macros.
> > This makes it difficult to automatically replace their
> > definitions with OBJECT_DECLARE_TYPE.
> >
> > Patch generated using:
> >
> > $ ./scripts/codeconverter/converter.py -i --pattern=MoveSymbols \
> > $(git grep -l '' -- '*.[ch]')
> >
> > which will:
> > - split "typdef struct { ... } TypedefName" declarations
> > - move the typedefs and #defines above the type check macros
> > - add missing #include "qom/object.h" lines if necessary
> >
> > Signed-off-by: Eduardo Habkost
>
> Reviewed-by: Juan Quintela
Thanks!
>
> Just curious, how did my name ended on the CC'd list? I don't see any
> file that I touched or that is migration related.
include/hw/vmstate-if.h is in the migration section in
MAINTAINERS.
--
Eduardo
[PATCH for-6.2 v3 1/2] qom: DECLARE_INTERFACE_CHECKER macro
The new macro will be similar to DECLARE_INSTANCE_CHECKER,
but for interface types (that use INTEFACE_CHECK instead of
OBJECT_CHECK).
Signed-off-by: Eduardo Habkost
---
Changes series v1 -> v2: none
Change series v2 -> v3:
* Rebased on top of
[PATCH for-6.2 00/12] qom: Get rid of all manual usage of OBJECT_CHECK &
friends
---
Cc: Paolo Bonzini
Cc: "Daniel P. Berrangé"
Cc: Eduardo Habkost
Cc: qemu-devel@nongnu.org
---
include/qom/object.h | 14 ++
1 file changed, 14 insertions(+)
diff --git a/include/qom/object.h b/include/qom/object.h
index faae0d841fe..4a9d7017d9f 100644
--- a/include/qom/object.h
+++ b/include/qom/object.h
@@ -178,6 +178,20 @@ struct Object
OBJ_NAME(const void *obj) \
{ return OBJECT_CHECK(InstanceType, obj, TYPENAME); }
+/**
+ * DECLARE_INTERFACE_CHECKER:
+ * @InstanceType: instance type
+ * @OBJ_NAME: the object name in uppercase with underscore separators
+ * @TYPENAME: type name
+ *
+ * This macro will provide the instance type cast functions for a
+ * QOM interface type.
+ */
+#define DECLARE_INTERFACE_CHECKER(InstanceType, OBJ_NAME, TYPENAME) \
+static inline G_GNUC_UNUSED InstanceType * \
+OBJ_NAME(const void *obj) \
+{ return INTERFACE_CHECK(InstanceType, obj, TYPENAME); }
+
/**
* DECLARE_CLASS_CHECKERS:
* @ClassType: class struct name
--
2.31.1
[PATCH for-6.2 v3 2/2] [autoamted] Use DECLARE_INTERFACE_CHECKER macro
Replace manual INTERFACE_CHECK macros with
DECLARE_INTERFACE_CHECKER, which is less error prone.
Generated using:
$ ./scripts/codeconverter/converter.py -i \
--pattern=InterfaceCheckMacro $(g grep -l '' -- '*.[ch]')
Signed-off-by: Eduardo Habkost
---
include/hw/acpi/acpi_dev_interface.h | 5 ++---
include/hw/arm/linux-boot-if.h | 4 ++--
include/hw/fw-path-provider.h| 4 ++--
include/hw/hotplug.h | 4 ++--
include/hw/intc/intc.h | 5 ++---
include/hw/ipmi/ipmi.h | 4 ++--
include/hw/isa/isa.h | 4 ++--
include/hw/mem/memory-device.h | 4 ++--
include/hw/nmi.h | 4 ++--
include/hw/ppc/pnv_xscom.h | 4 ++--
include/hw/ppc/spapr_irq.h | 4 ++--
include/hw/ppc/xics.h| 4 ++--
include/hw/ppc/xive.h| 12 ++--
include/hw/rdma/rdma.h | 5 ++---
include/hw/rtc/m48t59.h | 4 ++--
include/hw/stream.h | 4 ++--
include/hw/vmstate-if.h | 4 ++--
include/qom/object_interfaces.h | 5 ++---
include/sysemu/tpm.h | 4 ++--
target/arm/idau.h| 4 ++--
tests/unit/check-qom-interface.c | 4 ++--
21 files changed, 46 insertions(+), 50 deletions(-)
diff --git a/include/hw/acpi/acpi_dev_interface.h
b/include/hw/acpi/acpi_dev_interface.h
index c9c7c17e043..f99b8b380eb 100644
--- a/include/hw/acpi/acpi_dev_interface.h
+++ b/include/hw/acpi/acpi_dev_interface.h
@@ -22,9 +22,8 @@ typedef struct AcpiDeviceIfClass AcpiDeviceIfClass;
DECLARE_CLASS_CHECKERS(AcpiDeviceIfClass, ACPI_DEVICE_IF,
TYPE_ACPI_DEVICE_IF)
typedef struct AcpiDeviceIf AcpiDeviceIf;
-#define ACPI_DEVICE_IF(obj) \
- INTERFACE_CHECK(AcpiDeviceIf, (obj), \
- TYPE_ACPI_DEVICE_IF)
+DECLARE_INTERFACE_CHECKER(AcpiDeviceIf, ACPI_DEVICE_IF,
+ TYPE_ACPI_DEVICE_IF)
void acpi_send_event(DeviceState *dev, AcpiEventStatusBits event);
diff --git a/include/hw/arm/linux-boot-if.h b/include/hw/arm/linux-boot-if.h
index 295e282c36e..17b8083f954 100644
--- a/include/hw/arm/linux-boot-if.h
+++ b/include/hw/arm/linux-boot-if.h
@@ -13,8 +13,8 @@ typedef struct ARMLinuxBootIfClass ARMLinuxBootIfClass;
DECLARE_CLASS_CHECKERS(ARMLinuxBootIfClass, ARM_LINUX_BOOT_IF,
TYPE_ARM_LINUX_BOOT_IF)
typedef struct ARMLinuxBootIf ARMLinuxBootIf;
-#define ARM_LINUX_BOOT_IF(obj) \
-INTERFACE_CHECK(ARMLinuxBootIf, (obj), TYPE_ARM_LINUX_BOOT_IF)
+DECLARE_INTERFACE_CHECKER(ARMLinuxBootIf, ARM_LINUX_BOOT_IF,
+ TYPE_ARM_LINUX_BOOT_IF)
struct ARMLinuxBootIfClass {
diff --git a/include/hw/fw-path-provider.h b/include/hw/fw-path-provider.h
index 33d91daed52..639fe9d821a 100644
--- a/include/hw/fw-path-provider.h
+++ b/include/hw/fw-path-provider.h
@@ -26,8 +26,8 @@ typedef struct FWPathProviderClass FWPathProviderClass;
DECLARE_CLASS_CHECKERS(FWPathProviderClass, FW_PATH_PROVIDER,
TYPE_FW_PATH_PROVIDER)
typedef struct FWPathProvider FWPathProvider;
-#define FW_PATH_PROVIDER(obj) \
- INTERFACE_CHECK(FWPathProvider, (obj), TYPE_FW_PATH_PROVIDER)
+DECLARE_INTERFACE_CHECKER(FWPathProvider, FW_PATH_PROVIDER,
+ TYPE_FW_PATH_PROVIDER)
struct FWPathProviderClass {
diff --git a/include/hw/hotplug.h b/include/hw/hotplug.h
index 75d32d69e2b..5dc7435a4c5 100644
--- a/include/hw/hotplug.h
+++ b/include/hw/hotplug.h
@@ -20,8 +20,8 @@ typedef struct HotplugHandlerClass HotplugHandlerClass;
DECLARE_CLASS_CHECKERS(HotplugHandlerClass, HOTPLUG_HANDLER,
TYPE_HOTPLUG_HANDLER)
typedef struct HotplugHandler HotplugHandler;
-#define HOTPLUG_HANDLER(obj) \
- INTERFACE_CHECK(HotplugHandler, (obj), TYPE_HOTPLUG_HANDLER)
+DECLARE_INTERFACE_CHECKER(HotplugHandler, HOTPLUG_HANDLER,
+ TYPE_HOTPLUG_HANDLER)
/**
diff --git a/include/hw/intc/intc.h b/include/hw/intc/intc.h
index 7c57c3a0379..a31b5341acb 100644
--- a/include/hw/intc/intc.h
+++ b/include/hw/intc/intc.h
@@ -9,9 +9,8 @@ typedef struct InterruptStatsProviderClass
InterruptStatsProviderClass;
DECLARE_CLASS_CHECKERS(InterruptStatsProviderClass, INTERRUPT_STATS_PROVIDER,
TYPE_INTERRUPT_STATS_PROVIDER)
typedef struct InterruptStatsProvider InterruptStatsProvider;
-#define INTERRUPT_STATS_PROVIDER(obj) \
-INTERFACE_CHECK(InterruptStatsProvider, (obj), \
-TYPE_INTERRUPT_STATS_PROVIDER)
+DECLARE_INTERFACE_CHECKER(InterruptStatsProvider, INTERRUPT_STATS_PROVIDER,
+ TYPE_INTERRUPT_STATS_PROVIDER)
struct InterruptStatsProviderClass {
diff --git a/include/hw/ipmi/ipmi.h b/include/hw/ipmi/ipmi.h
index d655352fa95..983e7366e75 100644
--- a/include/hw/ipmi/ipmi.h
+++ b/include/hw/ipmi/ipmi.h
@@ -110,8 +110,8 @@ uint32_t ipmi_next_uuid(void);
*/
#define TYPE_IPMI_INTERFACE
Re: [PATCH] docs/devel: fix missing antislash
On Mon, Aug 09, 2021 at 07:31:41PM +0200, Alexandre Iooss wrote: > Signed-off-by: Alexandre Iooss > --- > docs/devel/qom.rst | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/docs/devel/qom.rst b/docs/devel/qom.rst > index e5fe3597cd..b9568c0fb8 100644 > --- a/docs/devel/qom.rst > +++ b/docs/devel/qom.rst > @@ -309,7 +309,7 @@ This is equivalent to the following: > OBJECT_GET_CLASS(MyDeviceClass, obj, TYPE_MY_DEVICE) > #define MY_DEVICE_CLASS(void *klass) \ > OBJECT_CLASS_CHECK(MyDeviceClass, klass, TYPE_MY_DEVICE) > - #define MY_DEVICE(void *obj) > + #define MY_DEVICE(void *obj) \ > OBJECT_CHECK(MyDevice, obj, TYPE_MY_DEVICE) Oops, nice catch! However, the code above is already going to be deleted by: https://lore.kernel.org/qemu-devel/20210729175554.686474-9-ehabk...@redhat.com -- Eduardo
[PATCH for-6.2 v3 0/2] qom: DECLARE_INTERFACE_CHECKER macro
This is an alternative to the series: Subject: [PATCH 0/3] qom: Replace INTERFACE_CHECK with OBJECT_CHECK https://lore.kernel.org/qemu-devel/20200916193101.511600-1-ehabk...@redhat.com/ Instead of removing INTERFACE_CHECK completely, keep it but use a DECLARE_INTERFACE_CHECKER macro to define the type checking functions for interface types. Maybe one day INTERFACE_CHECK/DECLARE_INTERFACE_CHECKER will be completely replaced with OBJECT_CHECK/DECLARE_INSTANCE_CHECKER, but by now it might be useful to keep the distinction between regular objects and interface types. See discussion at https://lore.kernel.org/qemu-devel/20200916221347.gl7...@habkost.net Based-on: <20210806211127.646908-1-ehabk...@redhat.com> Changes v2 -> v3: * Rebased on top of Subject: [PATCH for-6.2 00/12] qom: Get rid of all manual usage of OBJECT_CHECK & friends Date: Fri, 6 Aug 2021 17:11:15 -0400 Message-Id: <20210806211127.646908-1-ehabk...@redhat.com> https://lore.kernel.org/qemu-devel/20210806211127.646908-1-ehabk...@redhat.com Changes v1 -> v2: * Move declaration after typedefs, so the code actually compiles Links to previous versions: v1: https://lore.kernel.org/qemu-devel/20200916223258.599367-1-ehabk...@redhat.com v2: https://lore.kernel.org/qemu-devel/20200917024947.707586-1-ehabk...@redhat.com Eduardo Habkost (2): qom: DECLARE_INTERFACE_CHECKER macro [autoamted] Use DECLARE_INTERFACE_CHECKER macro include/hw/acpi/acpi_dev_interface.h | 5 ++--- include/hw/arm/linux-boot-if.h | 4 ++-- include/hw/fw-path-provider.h| 4 ++-- include/hw/hotplug.h | 4 ++-- include/hw/intc/intc.h | 5 ++--- include/hw/ipmi/ipmi.h | 4 ++-- include/hw/isa/isa.h | 4 ++-- include/hw/mem/memory-device.h | 4 ++-- include/hw/nmi.h | 4 ++-- include/hw/ppc/pnv_xscom.h | 4 ++-- include/hw/ppc/spapr_irq.h | 4 ++-- include/hw/ppc/xics.h| 4 ++-- include/hw/ppc/xive.h| 12 ++-- include/hw/rdma/rdma.h | 5 ++--- include/hw/rtc/m48t59.h | 4 ++-- include/hw/stream.h | 4 ++-- include/hw/vmstate-if.h | 4 ++-- include/qom/object.h | 14 ++ include/qom/object_interfaces.h | 5 ++--- include/sysemu/tpm.h | 4 ++-- target/arm/idau.h| 4 ++-- tests/unit/check-qom-interface.c | 4 ++-- 22 files changed, 60 insertions(+), 50 deletions(-) -- 2.31.1
Re: [PATCH for-6.2 00/12] qom: Get rid of all manual usage of OBJECT_CHECK & friends
On Sat, Aug 07, 2021 at 10:15:52AM +0200, Philippe Mathieu-Daudé wrote: > On 8/6/21 11:11 PM, Eduardo Habkost wrote: > > This series gets rid of all manual usage of OBJECT_CHECK, > > OBJECT_CLASS_CHECK, and OBJECT_GET_CLASS. > > > > All type check macros defined manually are replaced with > > DEFINE_*CHECKER* or OBJECT_DECLARE* macros. > > > > All manual usage of OBJECT_CHECK/OBJECT_CLASS_CHECK/OBJECT_GET_CLASS > > is manually replaced with the corresponding type-specific wrappers. > > Is INTERFACE_CHECK already converted / in good shape? Not yet. I need to refresh my memory by looking at mailing list archives, but I have a work in progress branch that I haven't touched in a while (except for rebasing it) at: https://gitlab.com/ehabkost/qemu/-/commits/work/qom-declare-interface-type/ Basically it introduces a DECLARE_INTERFACE_CHECKER macro instead of reusing OBJECT_CHECK/DECLARE_INSTANCE_CHECKER. -- Eduardo
[PATCH for-6.2 07/12] [automated] Use DECLARE_*CHECKER* macros when possible
Converting existing QOM types to OBJECT_DECLARE_TYPE is not
always trivial (due to inconsistent type/macro naming schemes),
but at least converting existing manual QOM type checking macros
to use DECLARE_*CHECKER* is a simpler process, and should at
least discourage people from defining new QOM type checker macros
manually.
Generated using:
$ ./scripts/codeconverter/converter.py -i \
--pattern=TypeCheckMacro $(git grep -l '' -- '*.[ch]')
Signed-off-by: Eduardo Habkost
---
Cc: "Marc-André Lureau"
Cc: Paolo Bonzini
Cc: Patrick Venture
Cc: Thomas Huth
Cc: "Michael S. Tsirkin"
Cc: Igor Mammedov
Cc: Alexander Bulekov
Cc: Bandan Das
Cc: Stefan Hajnoczi
Cc: Keith Busch
Cc: Klaus Jensen
Cc: Cornelia Huck
Cc: Halil Pasic
Cc: Christian Borntraeger
Cc: Richard Henderson
Cc: David Hildenbrand
Cc: Gerd Hoffmann
Cc: Havard Skinnemoen
Cc: Tyrone Ting
Cc: Vijai Kumar K
Cc: Alistair Francis
Cc: Bin Meng
Cc: Palmer Dabbelt
Cc: "Edgar E. Iglesias"
Cc: Peter Maydell
Cc: Laurent Vivier
Cc: "Cédric Le Goater"
Cc: Andrew Jeffery
Cc: Joel Stanley
Cc: Jason Wang
Cc: Vikram Garhwal
Cc: Francisco Iglesias
Cc: David Gibson
Cc: Greg Kurz
Cc: Bastian Koppelmann
Cc: Taylor Simpson
Cc: qemu-devel@nongnu.org
Cc: qemu-bl...@nongnu.org
Cc: qemu-s3...@nongnu.org
Cc: qemu-...@nongnu.org
Cc: qemu-ri...@nongnu.org
Cc: qemu-...@nongnu.org
---
hw/nvme/nvme.h | 12 ++--
hw/usb/hcd-xhci-pci.h | 4 ++--
hw/usb/hcd-xhci-sysbus.h| 4 ++--
hw/usb/u2f.h| 8 ++--
include/hw/adc/npcm7xx_adc.h| 4 ++--
include/hw/arm/npcm7xx.h| 15 ---
include/hw/char/shakti_uart.h | 4 ++--
include/hw/dma/sifive_pdma.h| 4 ++--
include/hw/dma/xlnx_csu_dma.h | 4 ++--
include/hw/gpio/npcm7xx_gpio.h | 4 ++--
include/hw/i2c/npcm7xx_smbus.h | 4 ++--
include/hw/intc/m68k_irqc.h | 4 ++--
include/hw/intc/sifive_clint.h | 4 ++--
include/hw/mem/npcm7xx_mc.h | 3 ++-
include/hw/misc/aspeed_lpc.h| 3 ++-
include/hw/misc/mchp_pfsoc_dmc.h| 10 --
include/hw/misc/mchp_pfsoc_ioscb.h | 4 ++--
include/hw/misc/mchp_pfsoc_sysreg.h | 5 ++---
include/hw/misc/npcm7xx_clk.h | 3 ++-
include/hw/misc/npcm7xx_gcr.h | 3 ++-
include/hw/misc/npcm7xx_mft.h | 4 ++--
include/hw/misc/npcm7xx_pwm.h | 4 ++--
include/hw/misc/npcm7xx_rng.h | 3 ++-
include/hw/misc/xlnx-versal-xramc.h | 4 ++--
include/hw/net/npcm7xx_emc.h| 4 ++--
include/hw/net/xlnx-zynqmp-can.h| 4 ++--
include/hw/nvram/npcm7xx_otp.h | 3 ++-
include/hw/ppc/spapr_drc.h | 13 -
include/hw/ppc/spapr_xive.h | 7 ++-
include/hw/riscv/microchip_pfsoc.h | 9 -
include/hw/riscv/shakti_c.h | 8
include/hw/riscv/sifive_e.h | 8
include/hw/riscv/sifive_u.h | 8
include/hw/sd/cadence_sdhci.h | 4 ++--
include/hw/ssi/npcm7xx_fiu.h| 3 ++-
include/hw/ssi/sifive_spi.h | 3 ++-
include/hw/timer/npcm7xx_timer.h| 4 ++--
include/hw/tricore/tricore_testdevice.h | 4 ++--
include/hw/usb/hcd-dwc3.h | 4 ++--
include/hw/usb/xlnx-usb-subsystem.h | 4 ++--
include/hw/usb/xlnx-versal-usb2-ctrl-regs.h | 4 ++--
include/hw/watchdog/sbsa_gwdt.h | 4 ++--
include/qemu/accel.h| 8 ++--
target/hexagon/cpu.h| 8 ++--
chardev/char-parallel.c | 6 ++
hw/i2c/i2c_mux_pca954x.c| 4 ++--
hw/m68k/mcf5206.c | 3 ++-
hw/mem/sparse-mem.c | 3 ++-
hw/misc/sbsa_ec.c | 3 ++-
hw/s390x/vhost-user-fs-ccw.c| 4 ++--
hw/sensor/adm1272.c | 3 ++-
hw/sensor/max34451.c| 3 ++-
hw/usb/u2f-emulated.c | 4 ++--
hw/usb/u2f-passthru.c | 4 ++--
54 files changed, 126 insertions(+), 146 deletions(-)
diff --git a/hw/nvme/nvme.h b/hw/nvme/nvme.h
index 0840f585d6e..c4c43da5c17 100644
--- a/hw/nvme/nvme.h
+++ b/hw/nvme/nvme.h
@@ -43,8 +43,8 @@ typedef struct NvmeBus {
#define TYPE_NVME_SUBSYS "nvme-subsys"
typedef struct NvmeSubsystem NvmeSubsystem;
-#define NVME_SUBSYS(obj) \
-OBJECT_CHECK(NvmeSubsystem, (obj), TYPE_NVME_SUBSYS)
+DECLARE_INSTANCE_CHECKER(NvmeSubsystem, NVME_SUBSYS,
+ TYPE_NVME_SUBSYS)
struct NvmeSubsystem {
DeviceState parent_
[PATCH for-6.2 12/12] [automated] Use OBJECT_DECLARE_SIMPLE_TYPE when possible
Replace typedef + DECLARE_INSTANCE_CHECKER with
equivalent OBJECT_DECLARE_SIMPLE_TYPE macro.
Generated using:
$ ./scripts/codeconverter/converter.py -i \
--pattern=AddObjectDeclareSimpleType $(git grep -l '' -- '*.[ch]')
Signed-off-by: Eduardo Habkost
---
Cc: Thomas Huth
Cc: Paul Burton
Cc: Aleksandar Rikalo
Cc: "Philippe Mathieu-Daudé"
Cc: Aurelien Jarno
Cc: Jiaxun Yang
Cc: Havard Skinnemoen
Cc: Tyrone Ting
Cc: Pavel Pisa
Cc: Vikram Garhwal
Cc: Jason Wang
Cc: Keith Busch
Cc: Klaus Jensen
Cc: "Michael S. Tsirkin"
Cc: Richard Henderson
Cc: David Hildenbrand
Cc: Cornelia Huck
Cc: Halil Pasic
Cc: Christian Borntraeger
Cc: Gerd Hoffmann
Cc: Vijai Kumar K
Cc: Alistair Francis
Cc: Bin Meng
Cc: Palmer Dabbelt
Cc: "Edgar E. Iglesias"
Cc: Peter Maydell
Cc: Laurent Vivier
Cc: "Cédric Le Goater"
Cc: Andrew Jeffery
Cc: Joel Stanley
Cc: Andrew Baumann
Cc: Francisco Iglesias
Cc: David Gibson
Cc: Greg Kurz
Cc: Bastian Koppelmann
Cc: qemu-devel@nongnu.org
Cc: qemu-...@nongnu.org
Cc: qemu-bl...@nongnu.org
Cc: qemu-s3...@nongnu.org
Cc: qemu-ri...@nongnu.org
Cc: qemu-...@nongnu.org
---
hw/nvme/nvme.h | 10 +++---
hw/usb/hcd-xhci-pci.h | 4 +---
hw/usb/hcd-xhci-sysbus.h| 4 +---
include/hw/adc/npcm7xx_adc.h| 4 +---
include/hw/char/shakti_uart.h | 4 +---
include/hw/dma/sifive_pdma.h| 4 +---
include/hw/dma/xlnx_csu_dma.h | 4 +---
include/hw/gpio/sifive_gpio.h | 4 +---
include/hw/intc/m68k_irqc.h | 4 +---
include/hw/intc/sifive_clint.h | 4 +---
include/hw/intc/sifive_plic.h | 4 +---
include/hw/misc/aspeed_lpc.h| 4 +---
include/hw/misc/bcm2835_cprman_internals.h | 12
include/hw/misc/led.h | 3 +--
include/hw/misc/mchp_pfsoc_dmc.h| 8 ++--
include/hw/misc/mchp_pfsoc_ioscb.h | 4 +---
include/hw/misc/mchp_pfsoc_sysreg.h | 4 +---
include/hw/misc/npcm7xx_clk.h | 3 +--
include/hw/misc/npcm7xx_gcr.h | 4 +---
include/hw/misc/npcm7xx_mft.h | 4 +---
include/hw/misc/npcm7xx_pwm.h | 3 +--
include/hw/misc/sifive_e_prci.h | 4 +---
include/hw/misc/sifive_test.h | 4 +---
include/hw/misc/sifive_u_otp.h | 4 +---
include/hw/misc/sifive_u_prci.h | 4 +---
include/hw/misc/xlnx-versal-xramc.h | 4 +---
include/hw/net/npcm7xx_emc.h| 4 +---
include/hw/net/xlnx-zynqmp-can.h| 4 +---
include/hw/ppc/spapr_drc.h | 4 +---
include/hw/register.h | 3 +--
include/hw/riscv/microchip_pfsoc.h | 4 +---
include/hw/riscv/shakti_c.h | 8 ++--
include/hw/riscv/sifive_e.h | 4 +---
include/hw/riscv/sifive_u.h | 4 +---
include/hw/sd/cadence_sdhci.h | 4 +---
include/hw/ssi/sifive_spi.h | 4 +---
include/hw/timer/npcm7xx_timer.h| 3 +--
include/hw/tricore/tricore_testdevice.h | 4 +---
include/hw/usb/hcd-dwc3.h | 4 +---
include/hw/usb/xlnx-versal-usb2-ctrl-regs.h | 4 +---
hw/m68k/mcf5206.c | 4 +---
hw/mips/boston.c| 4 +---
hw/misc/npcm7xx_clk.c | 9 +++--
hw/net/can/ctucan_pci.c | 4 +---
hw/s390x/vhost-user-fs-ccw.c| 4 +---
hw/sensor/adm1272.c | 4 +---
hw/sensor/max34451.c| 4 +---
47 files changed, 56 insertions(+), 154 deletions(-)
diff --git a/hw/nvme/nvme.h b/hw/nvme/nvme.h
index c4c43da5c17..a9ee5c4f1de 100644
--- a/hw/nvme/nvme.h
+++ b/hw/nvme/nvme.h
@@ -42,9 +42,7 @@ typedef struct NvmeBus {
} NvmeBus;
#define TYPE_NVME_SUBSYS "nvme-subsys"
-typedef struct NvmeSubsystem NvmeSubsystem;
-DECLARE_INSTANCE_CHECKER(NvmeSubsystem, NVME_SUBSYS,
- TYPE_NVME_SUBSYS)
+OBJECT_DECLARE_SIMPLE_TYPE(NvmeSubsystem, NVME_SUBSYS)
struct NvmeSubsystem {
DeviceState parent_obj;
@@ -83,8 +81,7 @@ static inline NvmeNamespace *nvme_subsys_ns(NvmeSubsystem
*subsys,
}
#define TYPE_NVME_NS "nvme-ns"
-DECLARE_INSTANCE_CHECKER(NvmeNamespace, NVME_NS,
- TYPE_NVME_NS)
+OBJECT_DECLARE_SIMPLE_TYPE(NvmeNamespace, NVME_NS)
typedef struct NvmeZone {
NvmeZoneDescr d;
@@ -377,8 +374,7 @@ typedef struct NvmeCQueue {
} NvmeCQueue;
#define TYPE_NVME "nvme"
-DECLARE_INSTANCE_CHECKER(NvmeCtrl, NVME,
- TYPE_NVME)
+OBJECT_DECLARE_SIMPLE_TYPE(NvmeCtrl, NVME)
typedef struct NvmeParams {
char *serial;
diff --git a/hw/usb/hcd-xhci-pci.h b/hw/usb/hcd-xhci-pci.h
index d83aad82e04
[PATCH for-6.2 09/12] npcm7xx_otp: Use DECLARE_CLASS_CHECKERS
Use DECLARE_CLASS_CHECKERS instead of defining the
NPCM7XX_OTP_CLASS and NPCM7XX_OTP_GET_CLASS macros manually.
These changes had to be done manually because the typedef and the
existing macro definitions were in different files.
Signed-off-by: Eduardo Habkost
---
Cc: Havard Skinnemoen
Cc: Tyrone Ting
Cc: qemu-...@nongnu.org
Cc: qemu-devel@nongnu.org
---
include/hw/nvram/npcm7xx_otp.h | 5 +++--
hw/nvram/npcm7xx_otp.c | 5 -
2 files changed, 3 insertions(+), 7 deletions(-)
diff --git a/include/hw/nvram/npcm7xx_otp.h b/include/hw/nvram/npcm7xx_otp.h
index 4cfc6577e87..0a3ebb091d5 100644
--- a/include/hw/nvram/npcm7xx_otp.h
+++ b/include/hw/nvram/npcm7xx_otp.h
@@ -60,12 +60,13 @@ typedef struct NPCM7xxOTPState NPCM7xxOTPState;
#define TYPE_NPCM7XX_OTP "npcm7xx-otp"
DECLARE_INSTANCE_CHECKER(NPCM7xxOTPState, NPCM7XX_OTP,
TYPE_NPCM7XX_OTP)
+typedef struct NPCM7xxOTPClass NPCM7xxOTPClass;
+DECLARE_CLASS_CHECKERS(NPCM7xxOTPClass, NPCM7XX_OTP,
+ TYPE_NPCM7XX_OTP)
#define TYPE_NPCM7XX_KEY_STORAGE "npcm7xx-key-storage"
#define TYPE_NPCM7XX_FUSE_ARRAY "npcm7xx-fuse-array"
-typedef struct NPCM7xxOTPClass NPCM7xxOTPClass;
-
/**
* npcm7xx_otp_array_write - ECC encode and write data to OTP array.
* @s: OTP module.
diff --git a/hw/nvram/npcm7xx_otp.c b/hw/nvram/npcm7xx_otp.c
index 52b9482419e..61085c5228b 100644
--- a/hw/nvram/npcm7xx_otp.c
+++ b/hw/nvram/npcm7xx_otp.c
@@ -73,11 +73,6 @@ struct NPCM7xxOTPClass {
const MemoryRegionOps *mmio_ops;
};
-#define NPCM7XX_OTP_CLASS(klass) \
-OBJECT_CLASS_CHECK(NPCM7xxOTPClass, (klass), TYPE_NPCM7XX_OTP)
-#define NPCM7XX_OTP_GET_CLASS(obj) \
-OBJECT_GET_CLASS(NPCM7xxOTPClass, (obj), TYPE_NPCM7XX_OTP)
-
static uint8_t ecc_encode_nibble(uint8_t n)
{
uint8_t result = n;
--
2.31.1
[PATCH for-6.2 10/12] [automated] Use DECLARE_OBJ_CHECKERS when possible
This automatically convert
DECLARE_INSTANCE_CHECKER/DECLARE_CLASS_CHECKER pairs to
DECLARE_OBJ_CHECKERS.
Generated using:
$ ./scripts/codeconverter/converter.py -i \
--pattern=AddDeclareObjCheckers $(git grep -l '' -- '*.[ch]')
Signed-off-by: Eduardo Habkost
---
Cc: Havard Skinnemoen
Cc: Tyrone Ting
Cc: qemu-...@nongnu.org
Cc: qemu-devel@nongnu.org
---
include/hw/nvram/npcm7xx_otp.h | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/include/hw/nvram/npcm7xx_otp.h b/include/hw/nvram/npcm7xx_otp.h
index 0a3ebb091d5..377cf462090 100644
--- a/include/hw/nvram/npcm7xx_otp.h
+++ b/include/hw/nvram/npcm7xx_otp.h
@@ -58,11 +58,9 @@ struct NPCM7xxOTPState {
typedef struct NPCM7xxOTPState NPCM7xxOTPState;
#define TYPE_NPCM7XX_OTP "npcm7xx-otp"
-DECLARE_INSTANCE_CHECKER(NPCM7xxOTPState, NPCM7XX_OTP,
- TYPE_NPCM7XX_OTP)
typedef struct NPCM7xxOTPClass NPCM7xxOTPClass;
-DECLARE_CLASS_CHECKERS(NPCM7xxOTPClass, NPCM7XX_OTP,
- TYPE_NPCM7XX_OTP)
+DECLARE_OBJ_CHECKERS(NPCM7xxOTPState, NPCM7xxOTPClass,
+ NPCM7XX_OTP, TYPE_NPCM7XX_OTP)
#define TYPE_NPCM7XX_KEY_STORAGE "npcm7xx-key-storage"
#define TYPE_NPCM7XX_FUSE_ARRAY "npcm7xx-fuse-array"
--
2.31.1
[PATCH for-6.2 11/12] [automated] Use OBJECT_DECLARE_TYPE when possible
Replace typedefs + DECLARE_OBJ_CHECKERS with equivalent
OBJECT_DECLARE_TYPE macro.
Generated using:
$ ./scripts/codeconverter/converter.py -i \
--pattern=AddObjectDeclareType $(git grep -l '' -- '*.[ch]')
Signed-off-by: Eduardo Habkost
---
Cc: John Snow
Cc: Kevin Wolf
Cc: Max Reitz
Cc: Gerd Hoffmann
Cc: "Daniel P. Berrangé"
Cc: David Gibson
Cc: Greg Kurz
Cc: "Cédric Le Goater"
Cc: Richard Henderson
Cc: Paolo Bonzini
Cc: qemu-bl...@nongnu.org
Cc: qemu-devel@nongnu.org
Cc: qemu-...@nongnu.org
---
hw/usb/u2f.h| 6 +-
include/crypto/tlscreds.h | 5 +
include/hw/ppc/spapr_drc.h | 5 +
include/hw/ppc/spapr_xive.h | 5 +
include/qemu/accel.h| 4 +---
hw/block/fdc-sysbus.c | 5 +
6 files changed, 6 insertions(+), 24 deletions(-)
diff --git a/hw/usb/u2f.h b/hw/usb/u2f.h
index 5767ca8fac9..e6e3ead40f1 100644
--- a/hw/usb/u2f.h
+++ b/hw/usb/u2f.h
@@ -32,13 +32,10 @@
#define U2FHID_PACKET_SIZE 64
#define U2FHID_PENDING_IN_NUM 32
-typedef struct U2FKeyState U2FKeyState;
typedef struct U2FKeyInfo U2FKeyInfo;
#define TYPE_U2F_KEY "u2f-key"
-typedef struct U2FKeyClass U2FKeyClass;
-DECLARE_OBJ_CHECKERS(U2FKeyState, U2FKeyClass,
- U2F_KEY, TYPE_U2F_KEY)
+OBJECT_DECLARE_TYPE(U2FKeyState, U2FKeyClass, U2F_KEY)
/*
* Callbacks to be used by the U2F key base device (i.e. hw/u2f.c)
@@ -69,7 +66,6 @@ struct U2FKeyState {
uint8_t pending_in_end;
uint8_t pending_in_num;
};
-typedef struct U2FKeyState U2FKeyState;
/*
* API to be used by the U2F key device variants (i.e. hw/u2f-*.c)
diff --git a/include/crypto/tlscreds.h b/include/crypto/tlscreds.h
index 2a8a8570109..617979a3986 100644
--- a/include/crypto/tlscreds.h
+++ b/include/crypto/tlscreds.h
@@ -25,10 +25,7 @@
#include "qom/object.h"
#define TYPE_QCRYPTO_TLS_CREDS "tls-creds"
-typedef struct QCryptoTLSCreds QCryptoTLSCreds;
-typedef struct QCryptoTLSCredsClass QCryptoTLSCredsClass;
-DECLARE_OBJ_CHECKERS(QCryptoTLSCreds, QCryptoTLSCredsClass, QCRYPTO_TLS_CREDS,
- TYPE_QCRYPTO_TLS_CREDS)
+OBJECT_DECLARE_TYPE(QCryptoTLSCreds, QCryptoTLSCredsClass, QCRYPTO_TLS_CREDS)
#define QCRYPTO_TLS_CREDS_DH_PARAMS "dh-params.pem"
diff --git a/include/hw/ppc/spapr_drc.h b/include/hw/ppc/spapr_drc.h
index 540439812f0..ff876fd74ca 100644
--- a/include/hw/ppc/spapr_drc.h
+++ b/include/hw/ppc/spapr_drc.h
@@ -20,10 +20,7 @@
#include "qapi/error.h"
#define TYPE_SPAPR_DR_CONNECTOR "spapr-dr-connector"
-typedef struct SpaprDrc SpaprDrc;
-typedef struct SpaprDrcClass SpaprDrcClass;
-DECLARE_OBJ_CHECKERS(SpaprDrc, SpaprDrcClass,
- SPAPR_DR_CONNECTOR, TYPE_SPAPR_DR_CONNECTOR)
+OBJECT_DECLARE_TYPE(SpaprDrc, SpaprDrcClass, SPAPR_DR_CONNECTOR)
#define TYPE_SPAPR_DRC_PHYSICAL "spapr-drc-physical"
typedef struct SpaprDrcPhysical SpaprDrcPhysical;
diff --git a/include/hw/ppc/spapr_xive.h b/include/hw/ppc/spapr_xive.h
index 333095c3fd9..9e7c46c801f 100644
--- a/include/hw/ppc/spapr_xive.h
+++ b/include/hw/ppc/spapr_xive.h
@@ -15,10 +15,7 @@
#include "qom/object.h"
#define TYPE_SPAPR_XIVE "spapr-xive"
-typedef struct SpaprXive SpaprXive;
-typedef struct SpaprXiveClass SpaprXiveClass;
-DECLARE_OBJ_CHECKERS(SpaprXive, SpaprXiveClass,
- SPAPR_XIVE, TYPE_SPAPR_XIVE)
+OBJECT_DECLARE_TYPE(SpaprXive, SpaprXiveClass, SPAPR_XIVE)
struct SpaprXive {
XiveRouterparent;
diff --git a/include/qemu/accel.h b/include/qemu/accel.h
index 8dc4ccc39ef..1b7696f9fbc 100644
--- a/include/qemu/accel.h
+++ b/include/qemu/accel.h
@@ -54,15 +54,13 @@ struct AccelClass {
*/
GPtrArray *compat_props;
};
-typedef struct AccelClass AccelClass;
#define TYPE_ACCEL_BASE "accel"
#define ACCEL_CLASS_SUFFIX "-" TYPE_ACCEL_BASE
#define ACCEL_CLASS_NAME(a) (a ACCEL_CLASS_SUFFIX)
-DECLARE_OBJ_CHECKERS(AccelState, AccelClass,
- ACCEL_BASE, TYPE_ACCEL_BASE)
+OBJECT_DECLARE_TYPE(AccelState, AccelClass, ACCEL_BASE)
AccelClass *accel_find(const char *opt_name);
AccelState *current_accel(void);
diff --git a/hw/block/fdc-sysbus.c b/hw/block/fdc-sysbus.c
index 57fc8773f12..ecce27db34e 100644
--- a/hw/block/fdc-sysbus.c
+++ b/hw/block/fdc-sysbus.c
@@ -33,10 +33,7 @@
#include "trace.h"
#define TYPE_SYSBUS_FDC "base-sysbus-fdc"
-typedef struct FDCtrlSysBusClass FDCtrlSysBusClass;
-typedef struct FDCtrlSysBus FDCtrlSysBus;
-DECLARE_OBJ_CHECKERS(FDCtrlSysBus, FDCtrlSysBusClass,
- SYSBUS_FDC, TYPE_SYSBUS_FDC)
+OBJECT_DECLARE_TYPE(FDCtrlSysBus, FDCtrlSysBusClass, SYSBUS_FDC)
struct FDCtrlSysBusClass {
/*< private >*/
--
2.31.1
[PATCH for-6.2 03/12] scripts/codeconverter: Update to latest version
I'm not documenting every single change in the codeconverter
script because most of that code will be deleted once we finish
the QOM code conversion. This patch updates the script to the
latest version that was used to perform changes in the QOM code.
Signed-off-by: Eduardo Habkost
---
.../codeconverter/codeconverter/patching.py | 18 +-
.../codeconverter/codeconverter/qom_macros.py | 187 +++--
.../codeconverter/qom_type_info.py| 247 +-
.../codeconverter/codeconverter/regexps.py| 3 +-
4 files changed, 405 insertions(+), 50 deletions(-)
diff --git a/scripts/codeconverter/codeconverter/patching.py
b/scripts/codeconverter/codeconverter/patching.py
index 9e92505d394..b4f0029f17e 100644
--- a/scripts/codeconverter/codeconverter/patching.py
+++ b/scripts/codeconverter/codeconverter/patching.py
@@ -59,11 +59,11 @@ def name(self) -> str:
def compiled_re(klass):
return re.compile(klass.regexp, re.MULTILINE)
-def start(self) -> int:
-return self.match.start()
+def start(self, *args) -> int:
+return self.match.start(*args)
-def end(self) -> int:
-return self.match.end()
+def end(self, *args) -> int:
+return self.match.end(*args)
def line_col(self) -> LineAndColumn:
return self.file.line_col(self.start())
@@ -114,9 +114,6 @@ def make_patch(self, replacement: str) -> 'Patch':
"""Make patch replacing the content of this match"""
return Patch(self.start(), self.end(), replacement)
-def make_subpatch(self, start: int, end: int, replacement: str) -> 'Patch':
-return Patch(self.start() + start, self.start() + end, replacement)
-
def make_removal_patch(self) -> 'Patch':
"""Make patch removing contents of match completely"""
return self.make_patch('')
@@ -377,8 +374,8 @@ def scan_for_matches(self, class_names:
Optional[List[str]]=None) -> Iterable[Fi
DBG("class_names: %r", class_names)
for cn in class_names:
matches = self.matches_of_type(class_dict[cn])
-DBG('%d matches found for %s: %s',
-len(matches), cn, ' '.join(names(matches)))
+INFO('%d matches found for %s: %s',
+ len(matches), cn, ' '.join(names(matches)))
yield from matches
def apply_patches(self) -> None:
@@ -407,9 +404,6 @@ def show_diff(self) -> None:
f.flush()
subprocess.call(['diff', '-u', self.filename, f.name])
-def ref(self):
-return TypeInfoReference
-
class FileList(RegexpScanner):
def __init__(self):
super().__init__()
diff --git a/scripts/codeconverter/codeconverter/qom_macros.py
b/scripts/codeconverter/codeconverter/qom_macros.py
index 2d2f2055a3d..61570620bd8 100644
--- a/scripts/codeconverter/codeconverter/qom_macros.py
+++ b/scripts/codeconverter/codeconverter/qom_macros.py
@@ -112,7 +112,9 @@ class SimpleTypedefMatch(TypedefMatch):
# This doesn't parse the struct definitions completely, it just assumes
# the closing brackets are going to be in an unindented line:
-RE_FULL_STRUCT = S('struct', SP, M(RE_IDENTIFIER, n='?', name='structname'),
SP,
+RE_FULL_STRUCT = S(NAMED('structtype',
+ 'struct', M(SP, NAMED('structname', RE_IDENTIFIER),
n='?')),
+ SP,
NAMED('body', r'{\n',
# acceptable inside the struct body:
# - lines starting with space or tab
@@ -165,6 +167,10 @@ def split_typedef(self) -> Iterator[Patch]:
yield self.strip_typedef()
yield self.append(self.make_simple_typedef())
+def add_struct_name(self, structname: str) -> Iterator[Patch]:
+yield Patch(self.start('structtype'), self.end('structtype'),
+ f'struct {structname}')
+
class StructTypedefSplit(FullStructTypedefMatch):
"""split struct+typedef declaration"""
def gen_patches(self) -> Iterator[Patch]:
@@ -191,32 +197,42 @@ def gen_patches(self) -> Iterable[Patch]:
if self.group('typedef_type') == 'struct
'+td.group('structname'):
yield td.strip_typedef()
+def typedef_used_by_qom(files: FileList, typedef_name: str) -> bool:
+qom_macros = [TypeCheckMacro, DeclareInstanceChecker,
DeclareInterfaceChecker, DeclareClassCheckers, DeclareObjCheckers,
DeclareInterfaceCheckers]
+qom_matches = chain(*(files.matches_of_type(t) for t in qom_macros))
+in_use = any(RequiredIdentifier('type', typedef_name) in
m.required_identifiers()
+ for m in qom_matches)
+return in_use
+
class QOMDuplicatedTypedefs(DuplicatedTypedefs):
"""Delete duplicate typedefs if used by QOM type"""
def gen_patches(self) -&g
[PATCH for-6.2 06/12] [automated] Split QOM "typedef struct T { ... } T" declarations
Automatically split struct definition and typedef declaration in
separate declarations, using a codeconverter rule. The rule will
only touch declarations of structs/typedefs actually used by QOM
types.
This will make automated changes to use OBJECT_DECLARE* macros
easier to implement, because automated removal of typedef lines
will be easier and safer.
Generated using:
$ ./scripts/codeconverter/converter.py -i \
--pattern=QOMStructTypedefSplit $(git grep -l '' -- '*.[ch]')
Signed-off-by: Eduardo Habkost
---
Cc: "Marc-André Lureau"
Cc: Paolo Bonzini
Cc: Patrick Venture
Cc: Thomas Huth
Cc: Keith Busch
Cc: Klaus Jensen
Cc: "Michael S. Tsirkin"
Cc: Cornelia Huck
Cc: Halil Pasic
Cc: Christian Borntraeger
Cc: Richard Henderson
Cc: David Hildenbrand
Cc: Gerd Hoffmann
Cc: Havard Skinnemoen
Cc: Tyrone Ting
Cc: Alistair Francis
Cc: Bin Meng
Cc: Palmer Dabbelt
Cc: "Edgar E. Iglesias"
Cc: Peter Maydell
Cc: Andrew Baumann
Cc: "Philippe Mathieu-Daudé"
Cc: "Cédric Le Goater"
Cc: David Gibson
Cc: Greg Kurz
Cc: Laurent Vivier
Cc: qemu-devel@nongnu.org
Cc: qemu-bl...@nongnu.org
Cc: qemu-s3...@nongnu.org
Cc: qemu-...@nongnu.org
Cc: qemu-ri...@nongnu.org
Cc: qemu-...@nongnu.org
---
hw/nvme/nvme.h | 10 ++
hw/usb/hcd-uhci.h | 5 +++--
hw/usb/u2f.h| 5 +++--
include/hw/adc/npcm7xx_adc.h| 5 +++--
include/hw/arm/npcm7xx.h| 10 ++
include/hw/core/accel-cpu.h | 5 +++--
include/hw/dma/sifive_pdma.h| 5 +++--
include/hw/dma/xlnx_csu_dma.h | 5 +++--
include/hw/gpio/npcm7xx_gpio.h | 5 +++--
include/hw/i2c/npcm7xx_smbus.h | 5 +++--
include/hw/mem/npcm7xx_mc.h | 5 +++--
include/hw/misc/bcm2835_cprman.h| 20
include/hw/misc/mchp_pfsoc_dmc.h| 10 ++
include/hw/misc/mchp_pfsoc_ioscb.h | 5 +++--
include/hw/misc/mchp_pfsoc_sysreg.h | 5 +++--
include/hw/misc/npcm7xx_clk.h | 15 +--
include/hw/misc/npcm7xx_gcr.h | 5 +++--
include/hw/misc/npcm7xx_mft.h | 5 +++--
include/hw/misc/npcm7xx_rng.h | 5 +++--
include/hw/nvram/npcm7xx_otp.h | 5 +++--
include/hw/riscv/microchip_pfsoc.h | 10 ++
include/hw/riscv/sifive_e.h | 5 +++--
include/hw/sd/cadence_sdhci.h | 5 +++--
include/qemu/accel.h| 10 ++
chardev/char-parallel.c | 10 ++
hw/i2c/i2c_mux_pca954x.c| 5 +++--
hw/m68k/mcf5206.c | 5 +++--
hw/misc/sbsa_ec.c | 5 +++--
hw/s390x/vhost-user-fs-ccw.c| 5 +++--
tests/qtest/pnv-xscom-test.c| 5 +++--
30 files changed, 123 insertions(+), 82 deletions(-)
diff --git a/hw/nvme/nvme.h b/hw/nvme/nvme.h
index 5ddcd783055..0840f585d6e 100644
--- a/hw/nvme/nvme.h
+++ b/hw/nvme/nvme.h
@@ -118,7 +118,7 @@ typedef struct NvmeNamespaceParams {
uint32_t zd_extension_size;
} NvmeNamespaceParams;
-typedef struct NvmeNamespace {
+struct NvmeNamespace {
DeviceState parent_obj;
BlockConfblkconf;
int32_t bootindex;
@@ -153,7 +153,8 @@ typedef struct NvmeNamespace {
struct {
uint32_t err_rec;
} features;
-} NvmeNamespace;
+};
+typedef struct NvmeNamespace NvmeNamespace;
static inline uint32_t nvme_nsid(NvmeNamespace *ns)
{
@@ -395,7 +396,7 @@ typedef struct NvmeParams {
bool legacy_cmb;
} NvmeParams;
-typedef struct NvmeCtrl {
+struct NvmeCtrl {
PCIDeviceparent_obj;
MemoryRegion bar0;
MemoryRegion iomem;
@@ -462,7 +463,8 @@ typedef struct NvmeCtrl {
};
uint32_tasync_config;
} features;
-} NvmeCtrl;
+};
+typedef struct NvmeCtrl NvmeCtrl;
static inline NvmeNamespace *nvme_ns(NvmeCtrl *n, uint32_t nsid)
{
diff --git a/hw/usb/hcd-uhci.h b/hw/usb/hcd-uhci.h
index 57d0d574644..2597a6d2eb6 100644
--- a/hw/usb/hcd-uhci.h
+++ b/hw/usb/hcd-uhci.h
@@ -43,7 +43,7 @@ typedef struct UHCIPort {
uint16_t ctrl;
} UHCIPort;
-typedef struct UHCIState {
+struct UHCIState {
PCIDevice dev;
MemoryRegion io_bar;
USBBus bus; /* Note unused when we're a companion controller */
@@ -73,7 +73,8 @@ typedef struct UHCIState {
char *masterbus;
uint32_t firstport;
uint32_t maxframes;
-} UHCIState;
+};
+typedef struct UHCIState UHCIState;
#define TYPE_UHCI "pci-uhci-usb"
DECLARE_INSTANCE_CHECKER(UHCIState, UHCI, TYPE_UHCI)
diff --git a/hw/usb/u2f.h b/hw/usb/u2f.h
index 705d5c43ce6..7191a23ee1a 100644
--- a/hw/usb/u2f.h
+++ b/hw/usb/u2f.h
@@ -62,7 +62,7 @@ struct U2FKeyClass {
/*
* State of the U2F key base device (i.e. hw/u2f.c)
*/
-typedef struct U2FKeyState {
+struct U2FKeyState {
USBDevice dev;
USBEndpoint *ep;
uint8_t idle;
@@ -72,7 +72,8 @@ typedef struct U2FKeyState {
uint8_t pending_in_start;
uint8_t pending_in_end;
uint8_t pen
[PATCH for-6.2 08/12] npcm7xx_clk: Use DECLARE_INSTANCE_CHECKER
Use DECLARE_INSTANCE_CHECKER instead of defining the
NPCM7XX_CLOCK_PLL, NPCM7XX_CLOCK_SEL, and NPCM7XX_CLOCK_DIVIDER
macros manually.
These changes had to be done manually because the codeconverter
script isn't smart enough to figure out that the typedefs exist
in a separate header.
Signed-off-by: Eduardo Habkost
---
Cc: Havard Skinnemoen
Cc: Tyrone Ting
Cc: qemu-...@nongnu.org
Cc: qemu-devel@nongnu.org
---
hw/misc/npcm7xx_clk.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/hw/misc/npcm7xx_clk.c b/hw/misc/npcm7xx_clk.c
index da6b14c545d..5247acfeb5a 100644
--- a/hw/misc/npcm7xx_clk.c
+++ b/hw/misc/npcm7xx_clk.c
@@ -110,14 +110,14 @@ static const uint32_t
cold_reset_values[NPCM7XX_CLK_NR_REGS] = {
/* Clock converter functions */
#define TYPE_NPCM7XX_CLOCK_PLL "npcm7xx-clock-pll"
-#define NPCM7XX_CLOCK_PLL(obj) OBJECT_CHECK(NPCM7xxClockPLLState, \
-(obj), TYPE_NPCM7XX_CLOCK_PLL)
+DECLARE_INSTANCE_CHECKER(NPCM7xxClockPLLState, NPCM7XX_CLOCK_PLL,
+ TYPE_NPCM7XX_CLOCK_PLL)
#define TYPE_NPCM7XX_CLOCK_SEL "npcm7xx-clock-sel"
-#define NPCM7XX_CLOCK_SEL(obj) OBJECT_CHECK(NPCM7xxClockSELState, \
-(obj), TYPE_NPCM7XX_CLOCK_SEL)
+DECLARE_INSTANCE_CHECKER(NPCM7xxClockSELState, NPCM7XX_CLOCK_SEL,
+ TYPE_NPCM7XX_CLOCK_SEL)
#define TYPE_NPCM7XX_CLOCK_DIVIDER "npcm7xx-clock-divider"
-#define NPCM7XX_CLOCK_DIVIDER(obj) OBJECT_CHECK(NPCM7xxClockDividerState, \
-(obj), TYPE_NPCM7XX_CLOCK_DIVIDER)
+DECLARE_INSTANCE_CHECKER(NPCM7xxClockDividerState, NPCM7XX_CLOCK_DIVIDER,
+ TYPE_NPCM7XX_CLOCK_DIVIDER)
static void npcm7xx_clk_update_pll(void *opaque)
{
--
2.31.1
[PATCH for-6.2 01/12] accel: Rename TYPE_ACCEL to TYPE_ACCEL_BASE
The ACCEL name conflicts with a Windows API typedef name, and it
is difficult to work around this because windows.h needs to be
included by osdep.h. This prevents us from replacing the
existing ACCEL macro with an inline function generated by
OBJECT_DEFINE_TYPE.
Work around the conflict by renaming TYPE_ACCEL to TYPE_ACCEL_BASE.
Note that the actual QOM type name is still "accel", because QOM
type names are user-visible and I don't want to make any
user-visible change here.
Signed-off-by: Eduardo Habkost
---
Notes about name alternatives:
I have considered using the name "CPU_ACCEL" as Daniel suggested,
but this would lead to a parent class named CPU_ACCEL, while
the subclasses would be still named KVM_ACCEL, HVF_ACCEL,
TCG_ACCEL, etc. I that this would be confusing, because it
would look like CPU_ACCEL is just another type of accel, not the
parent type of all other *_ACCEL types.
Renaming KVM_ACCEL/HVF_ACCEL/TCG_ACCEL to
KVM_CPU_ACCEL/HVF_CPU_ACCEL/TCG_CPU_ACCEL would be clearer, but I
believe it would be too intrusive and the resulting type names
would be too long.
Renaming the base classe ACCEL_BASE sounds like a reasonable
alternative, as there are other examples in QEMU where abstract
base classes are called *_BASE: TYPE_VIRTIO_GPU_PCI_BASE,
TYPE_VIRTIO_GPU_BASE, TYPE_E1000_BASE, TYPE_RISCV_CPU_BASE,
TYPE_MEGASAS_BASE, TYPE_SCSI_DISK_BASE, etc.
---
Cc: Richard Henderson
Cc: Paolo Bonzini
Cc: Cameron Esfahani
Cc: Roman Bolshakov
Cc: Thomas Huth
Cc: Laurent Vivier
Cc: Stefano Stabellini
Cc: Anthony Perard
Cc: Paul Durrant
Cc: Warner Losh
Cc: Kyle Evans
Cc: Eduardo Habkost
Cc: Marcel Apfelbaum
Cc: Peter Xu
Cc: David Hildenbrand
Cc: Wenchao Wang
Cc: Colin Xu
Cc: Kamil Rytarowski
Cc: Reinoud Zandijk
Cc: Sunil Muthuswamy
Cc: qemu-devel@nongnu.org
Cc: k...@vger.kernel.org
Cc: xen-de...@lists.xenproject.org
Cc: haxm-t...@intel.com
---
hw/core/machine.c | 2 +-
include/qemu/accel.h| 16
accel/accel-common.c| 4 ++--
accel/accel-softmmu.c | 4 ++--
accel/accel-user.c | 2 +-
accel/hvf/hvf-accel-ops.c | 4 ++--
accel/kvm/kvm-all.c | 4 ++--
accel/qtest/qtest.c | 4 ++--
accel/tcg/tcg-all.c | 4 ++--
accel/xen/xen-all.c | 4 ++--
bsd-user/main.c | 2 +-
linux-user/main.c | 2 +-
softmmu/memory.c| 2 +-
softmmu/vl.c| 6 +++---
target/i386/hax/hax-all.c | 4 ++--
target/i386/nvmm/nvmm-all.c | 4 ++--
target/i386/whpx/whpx-all.c | 4 ++--
17 files changed, 36 insertions(+), 36 deletions(-)
diff --git a/hw/core/machine.c b/hw/core/machine.c
index 943974d411c..fdb1f886ce8 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -1268,7 +1268,7 @@ void machine_run_board_init(MachineState *machine)
"on", false);
}
-accel_init_interfaces(ACCEL_GET_CLASS(machine->accelerator));
+accel_init_interfaces(ACCEL_BASE_GET_CLASS(machine->accelerator));
machine_class->init(machine);
phase_advance(PHASE_MACHINE_INITIALIZED);
}
diff --git a/include/qemu/accel.h b/include/qemu/accel.h
index 4f4c283f6fc..cc915720494 100644
--- a/include/qemu/accel.h
+++ b/include/qemu/accel.h
@@ -54,17 +54,17 @@ typedef struct AccelClass {
GPtrArray *compat_props;
} AccelClass;
-#define TYPE_ACCEL "accel"
+#define TYPE_ACCEL_BASE "accel"
-#define ACCEL_CLASS_SUFFIX "-" TYPE_ACCEL
+#define ACCEL_CLASS_SUFFIX "-" TYPE_ACCEL_BASE
#define ACCEL_CLASS_NAME(a) (a ACCEL_CLASS_SUFFIX)
-#define ACCEL_CLASS(klass) \
-OBJECT_CLASS_CHECK(AccelClass, (klass), TYPE_ACCEL)
-#define ACCEL(obj) \
-OBJECT_CHECK(AccelState, (obj), TYPE_ACCEL)
-#define ACCEL_GET_CLASS(obj) \
-OBJECT_GET_CLASS(AccelClass, (obj), TYPE_ACCEL)
+#define ACCEL_BASE_CLASS(klass) \
+OBJECT_CLASS_CHECK(AccelClass, (klass), TYPE_ACCEL_BASE)
+#define ACCEL_BASE(obj) \
+OBJECT_CHECK(AccelState, (obj), TYPE_ACCEL_BASE)
+#define ACCEL_BASE_GET_CLASS(obj) \
+OBJECT_GET_CLASS(AccelClass, (obj), TYPE_ACCEL_BASE)
AccelClass *accel_find(const char *opt_name);
AccelState *current_accel(void);
diff --git a/accel/accel-common.c b/accel/accel-common.c
index 7b8ec7e0f72..c4e268c8a74 100644
--- a/accel/accel-common.c
+++ b/accel/accel-common.c
@@ -34,7 +34,7 @@
#endif /* !CONFIG_USER_ONLY */
static const TypeInfo accel_type = {
-.name = TYPE_ACCEL,
+.name = TYPE_ACCEL_BASE,
.parent = TYPE_OBJECT,
.class_size = sizeof(AccelClass),
.instance_size = sizeof(AccelState),
@@ -44,7 +44,7 @@ static const TypeInfo accel_type = {
AccelClass *accel_find(const char *opt_name)
{
char *class_name = g_strdup_printf(ACCEL_CLASS_NAME("%s"), opt_name);
-AccelClass *ac = ACCEL_CLASS(module_object_class_by_name(class_name));
+AccelClass *ac = ACCEL_BASE_CLASS(module_object_class_by_name(class_name));
[PATCH for-6.2 05/12] [automated] Move QOM typedefs and add missing includes
Some typedefs and macros are defined after the type check macros.
This makes it difficult to automatically replace their
definitions with OBJECT_DECLARE_TYPE.
Patch generated using:
$ ./scripts/codeconverter/converter.py -i --pattern=MoveSymbols \
$(git grep -l '' -- '*.[ch]')
which will:
- split "typdef struct { ... } TypedefName" declarations
- move the typedefs and #defines above the type check macros
- add missing #include "qom/object.h" lines if necessary
Signed-off-by: Eduardo Habkost
---
Cc: Richard Henderson
Cc: Paolo Bonzini
Cc: "Marc-André Lureau"
Cc: Peter Maydell
Cc: Andrew Baumann
Cc: "Philippe Mathieu-Daudé"
Cc: Thomas Huth
Cc: "Michael S. Tsirkin"
Cc: Igor Mammedov
Cc: Alexander Bulekov
Cc: Bandan Das
Cc: Stefan Hajnoczi
Cc: Huacai Chen
Cc: Jiaxun Yang
Cc: Aurelien Jarno
Cc: Aleksandar Rikalo
Cc: Havard Skinnemoen
Cc: Tyrone Ting
Cc: Pavel Pisa
Cc: Vikram Garhwal
Cc: Jason Wang
Cc: Keith Busch
Cc: Klaus Jensen
Cc: Cornelia Huck
Cc: Halil Pasic
Cc: Christian Borntraeger
Cc: David Hildenbrand
Cc: Gerd Hoffmann
Cc: Vijai Kumar K
Cc: Alistair Francis
Cc: Bin Meng
Cc: Palmer Dabbelt
Cc: "Edgar E. Iglesias"
Cc: Laurent Vivier
Cc: Corey Minyard
Cc: "Cédric Le Goater"
Cc: Andrew Jeffery
Cc: Joel Stanley
Cc: Francisco Iglesias
Cc: David Gibson
Cc: Greg Kurz
Cc: Alexey Kardashevskiy
Cc: "Hervé Poussineau"
Cc: Bastian Koppelmann
Cc: Juan Quintela
Cc: "Dr. David Alan Gilbert"
Cc: "Daniel P. Berrangé"
Cc: Eduardo Habkost
Cc: Stefan Berger
Cc: Taylor Simpson
Cc: qemu-devel@nongnu.org
Cc: qemu-...@nongnu.org
Cc: qemu-bl...@nongnu.org
Cc: qemu-s3...@nongnu.org
Cc: qemu-ri...@nongnu.org
Cc: qemu-...@nongnu.org
---
hw/nvme/nvme.h | 6 --
hw/usb/hcd-uhci.h | 1 +
hw/usb/hcd-xhci-pci.h | 6 --
hw/usb/hcd-xhci-sysbus.h| 6 --
hw/usb/u2f.h| 6 --
include/hw/acpi/acpi_dev_interface.h| 2 +-
include/hw/adc/npcm7xx_adc.h| 1 +
include/hw/arm/linux-boot-if.h | 2 +-
include/hw/arm/npcm7xx.h| 11 +++
include/hw/char/shakti_uart.h | 6 --
include/hw/core/accel-cpu.h | 1 +
include/hw/dma/sifive_pdma.h| 1 +
include/hw/dma/xlnx_csu_dma.h | 1 +
include/hw/fw-path-provider.h | 2 +-
include/hw/gpio/npcm7xx_gpio.h | 1 +
include/hw/hotplug.h| 2 +-
include/hw/i2c/npcm7xx_smbus.h | 1 +
include/hw/intc/intc.h | 2 +-
include/hw/intc/m68k_irqc.h | 6 --
include/hw/intc/sifive_clint.h | 6 --
include/hw/ipmi/ipmi.h | 2 +-
include/hw/mem/memory-device.h | 2 +-
include/hw/mem/npcm7xx_mc.h | 1 +
include/hw/misc/aspeed_lpc.h| 6 --
include/hw/misc/bcm2835_cprman.h| 1 +
include/hw/misc/bcm2835_cprman_internals.h | 1 +
include/hw/misc/mchp_pfsoc_dmc.h| 1 +
include/hw/misc/mchp_pfsoc_ioscb.h | 1 +
include/hw/misc/mchp_pfsoc_sysreg.h | 1 +
include/hw/misc/npcm7xx_clk.h | 1 +
include/hw/misc/npcm7xx_gcr.h | 1 +
include/hw/misc/npcm7xx_pwm.h | 1 +
include/hw/misc/npcm7xx_rng.h | 1 +
include/hw/misc/xlnx-versal-xramc.h | 6 --
include/hw/net/npcm7xx_emc.h| 1 +
include/hw/net/xlnx-zynqmp-can.h| 6 --
include/hw/nmi.h| 2 +-
include/hw/nvram/npcm7xx_otp.h | 1 +
include/hw/ppc/spapr_drc.h | 15 +--
include/hw/ppc/spapr_xive.h | 11 +++
include/hw/ppc/vof.h| 1 +
include/hw/rdma/rdma.h | 2 +-
include/hw/riscv/microchip_pfsoc.h | 1 +
include/hw/riscv/shakti_c.h | 11 +++
include/hw/riscv/sifive_e.h | 6 --
include/hw/riscv/sifive_u.h | 11 +++
include/hw/rtc/m48t59.h | 2 +-
include/hw/sd/cadence_sdhci.h | 1 +
include/hw/ssi/npcm7xx_fiu.h| 1 +
include/hw/ssi/sifive_spi.h | 6 --
include/hw/stream.h | 2 +-
include/hw/timer/npcm7xx_timer.h| 1 +
include/hw/tricore/tricore_testdevice.h | 6 --
include/hw/usb/hcd-dwc3.h | 6 --
include/hw/usb/msd.h| 1 +
include/hw/usb/xlnx-usb-subsystem.h | 6 --
include/hw/usb/xlnx-versal-usb2-ctrl-regs.h | 6 --
include/hw/vmstate-if.h | 2 +-
include/hw/watchdog/sbsa_gwdt.
[PATCH for-6.2 04/12] [automated] Add struct names to typedefs used by QOM types
Anonymous structs on QOM typedefs make the code harder to convert
to OBJECT_DEFINE* macros, as the macros expect the struct name to
exist.
Use a codeconverter rule to automatically add names to the
structs used in QOM typedefs.
Generated using:
$ ./scripts/codeconverter/converter.py -i \
--pattern=AddNamesToTypedefs $(git grep -l '' -- '*.[ch]')
Signed-off-by: Eduardo Habkost
---
Cc: "Marc-André Lureau"
Cc: Paolo Bonzini
Cc: Thomas Huth
Cc: Havard Skinnemoen
Cc: Tyrone Ting
Cc: Vijai Kumar K
Cc: Bastian Koppelmann
Cc: qemu-devel@nongnu.org
Cc: qemu-...@nongnu.org
Cc: qemu-ri...@nongnu.org
---
include/hw/adc/npcm7xx_adc.h| 2 +-
include/hw/char/shakti_uart.h | 2 +-
include/hw/tricore/tricore_testdevice.h | 2 +-
chardev/char-parallel.c | 4 ++--
hw/m68k/mcf5206.c | 2 +-
hw/misc/sbsa_ec.c | 2 +-
6 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/include/hw/adc/npcm7xx_adc.h b/include/hw/adc/npcm7xx_adc.h
index 7d8442107ae..8e5a1897b4b 100644
--- a/include/hw/adc/npcm7xx_adc.h
+++ b/include/hw/adc/npcm7xx_adc.h
@@ -42,7 +42,7 @@
* @iref: The internal reference voltage, initialized at launch time.
* @rv: The calibrated output values of 0.5V and 1.5V for the ADC.
*/
-typedef struct {
+typedef struct NPCM7xxADCState {
SysBusDevice parent;
MemoryRegion iomem;
diff --git a/include/hw/char/shakti_uart.h b/include/hw/char/shakti_uart.h
index 526c408233f..25f7cbcaa55 100644
--- a/include/hw/char/shakti_uart.h
+++ b/include/hw/char/shakti_uart.h
@@ -51,7 +51,7 @@
#define SHAKTI_UART(obj) \
OBJECT_CHECK(ShaktiUartState, (obj), TYPE_SHAKTI_UART)
-typedef struct {
+typedef struct ShaktiUartState {
/* */
SysBusDevice parent_obj;
diff --git a/include/hw/tricore/tricore_testdevice.h
b/include/hw/tricore/tricore_testdevice.h
index 2c56c51bcb8..e93c883872d 100644
--- a/include/hw/tricore/tricore_testdevice.h
+++ b/include/hw/tricore/tricore_testdevice.h
@@ -26,7 +26,7 @@
#define TRICORE_TESTDEVICE(obj) \
OBJECT_CHECK(TriCoreTestDeviceState, (obj), TYPE_TRICORE_TESTDEVICE)
-typedef struct {
+typedef struct TriCoreTestDeviceState {
/* */
SysBusDevice parent_obj;
diff --git a/chardev/char-parallel.c b/chardev/char-parallel.c
index 05e7efbd6ca..acf9fb8afa0 100644
--- a/chardev/char-parallel.c
+++ b/chardev/char-parallel.c
@@ -49,7 +49,7 @@
#if defined(__linux__)
-typedef struct {
+typedef struct ParallelChardev {
Chardev parent;
int fd;
int mode;
@@ -177,7 +177,7 @@ static void qemu_chr_open_pp_fd(Chardev *chr,
#if defined(__FreeBSD__) || defined(__FreeBSD_kernel__) ||
defined(__DragonFly__)
-typedef struct {
+typedef struct ParallelChardev {
Chardev parent;
int fd;
} ParallelChardev;
diff --git a/hw/m68k/mcf5206.c b/hw/m68k/mcf5206.c
index 6d93d761a5e..72a815dbbd0 100644
--- a/hw/m68k/mcf5206.c
+++ b/hw/m68k/mcf5206.c
@@ -160,7 +160,7 @@ static m5206_timer_state *m5206_timer_init(qemu_irq irq)
/* System Integration Module. */
-typedef struct {
+typedef struct m5206_mbar_state {
SysBusDevice parent_obj;
M68kCPU *cpu;
diff --git a/hw/misc/sbsa_ec.c b/hw/misc/sbsa_ec.c
index 83020fe9ac9..9e3c40a23dc 100644
--- a/hw/misc/sbsa_ec.c
+++ b/hw/misc/sbsa_ec.c
@@ -16,7 +16,7 @@
#include "hw/sysbus.h"
#include "sysemu/runstate.h"
-typedef struct {
+typedef struct SECUREECState {
SysBusDevice parent_obj;
MemoryRegion iomem;
} SECUREECState;
--
2.31.1
[PATCH for-6.2 02/12] qom: Use DEVICE_*CLASS instead of OBJECT_*CLASS
There are multiple functions where OBJECT_GET_CLASS or
OBJECT_CLASS_CHECK are being used directly for
DeviceClass/TYPE_DEVICE, instead of the DEVICE_GET_CLASS or
DEVICE_CLASS wrappers. There's no reason to not use the
wrappers, so use them.
Signed-off-by: Eduardo Habkost
---
Cc: "Michael S. Tsirkin"
Cc: Marcel Apfelbaum
Cc: Gerd Hoffmann
Cc: Juan Quintela
Cc: "Dr. David Alan Gilbert"
Cc: Markus Armbruster
Cc: Paolo Bonzini
Cc: "Daniel P. Berrangé"
Cc: Eduardo Habkost
Cc: qemu-devel@nongnu.org
---
hw/pci/pci.c | 3 +--
hw/usb/hcd-ehci-pci.c | 2 +-
migration/savevm.c | 3 +--
monitor/misc.c | 3 +--
softmmu/qdev-monitor.c | 3 +--
5 files changed, 5 insertions(+), 9 deletions(-)
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 23d2ae2ab23..9af32ef4cb8 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -1915,8 +1915,7 @@ PCIDevice *pci_nic_init_nofail(NICInfo *nd, PCIBus
*rootbus,
list = object_class_get_list_sorted(TYPE_PCI_DEVICE, false);
pci_nic_models = g_ptr_array_new();
while (list) {
-DeviceClass *dc = OBJECT_CLASS_CHECK(DeviceClass, list->data,
- TYPE_DEVICE);
+DeviceClass *dc = DEVICE_CLASS(list->data);
GSList *next;
if (test_bit(DEVICE_CATEGORY_NETWORK, dc->categories) &&
dc->user_creatable) {
diff --git a/hw/usb/hcd-ehci-pci.c b/hw/usb/hcd-ehci-pci.c
index 4c37c8e2271..345444a5739 100644
--- a/hw/usb/hcd-ehci-pci.c
+++ b/hw/usb/hcd-ehci-pci.c
@@ -74,7 +74,7 @@ static void usb_ehci_pci_realize(PCIDevice *dev, Error **errp)
static void usb_ehci_pci_init(Object *obj)
{
-DeviceClass *dc = OBJECT_GET_CLASS(DeviceClass, obj, TYPE_DEVICE);
+DeviceClass *dc = DEVICE_GET_CLASS(obj);
EHCIPCIState *i = PCI_EHCI(obj);
EHCIState *s = >ehci;
diff --git a/migration/savevm.c b/migration/savevm.c
index 7b7b64bd13e..23cc55b8533 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -663,8 +663,7 @@ void dump_vmstate_json_to_file(FILE *out_file)
first = true;
list = object_class_get_list(TYPE_DEVICE, true);
for (elt = list; elt; elt = elt->next) {
-DeviceClass *dc = OBJECT_CLASS_CHECK(DeviceClass, elt->data,
- TYPE_DEVICE);
+DeviceClass *dc = DEVICE_CLASS(elt->data);
const char *name;
int indent = 2;
diff --git a/monitor/misc.c b/monitor/misc.c
index ffe79668706..98202d12e7f 100644
--- a/monitor/misc.c
+++ b/monitor/misc.c
@@ -1549,8 +1549,7 @@ void device_add_completion(ReadLineState *rs, int
nb_args, const char *str)
list = elt = object_class_get_list(TYPE_DEVICE, false);
while (elt) {
const char *name;
-DeviceClass *dc = OBJECT_CLASS_CHECK(DeviceClass, elt->data,
- TYPE_DEVICE);
+DeviceClass *dc = DEVICE_CLASS(elt->data);
name = object_class_get_name(OBJECT_CLASS(dc));
if (dc->user_creatable
diff --git a/softmmu/qdev-monitor.c b/softmmu/qdev-monitor.c
index 721dec2d820..82d164c6539 100644
--- a/softmmu/qdev-monitor.c
+++ b/softmmu/qdev-monitor.c
@@ -165,8 +165,7 @@ static void qdev_print_devinfos(bool show_no_user)
for (i = 0; i <= DEVICE_CATEGORY_MAX; i++) {
cat_printed = false;
for (elt = list; elt; elt = elt->next) {
-DeviceClass *dc = OBJECT_CLASS_CHECK(DeviceClass, elt->data,
- TYPE_DEVICE);
+DeviceClass *dc = DEVICE_CLASS(elt->data);
if ((i < DEVICE_CATEGORY_MAX
? !test_bit(i, dc->categories)
: !bitmap_empty(dc->categories, DEVICE_CATEGORY_MAX))
--
2.31.1
[PATCH for-6.2 00/12] qom: Get rid of all manual usage of OBJECT_CHECK & friends
This series gets rid of all manual usage of OBJECT_CHECK,
OBJECT_CLASS_CHECK, and OBJECT_GET_CLASS.
All type check macros defined manually are replaced with
DEFINE_*CHECKER* or OBJECT_DECLARE* macros.
All manual usage of OBJECT_CHECK/OBJECT_CLASS_CHECK/OBJECT_GET_CLASS
is manually replaced with the corresponding type-specific wrappers.
For a git tree containing all dependencies, see:
https://gitlab.com/ehabkost/qemu.git work/qom-automated-declare-type-pass2
This series is based on multiple other series I have submitted
recently, including:
Date: Thu, 5 Aug 2021 15:34:25 -0400
Subject: [PATCH for-6.2 0/6] qom: Fix broken OBJECT_CHECK usage
Message-Id: <20210805193431.307761-1-ehabk...@redhat.com>
https://lore.kernel.org/qemu-devel/20210805193431.307761-1-ehabk...@redhat.com
Date: Thu, 5 Aug 2021 17:47:39 -0400
Subject: [PATCH] bcm2836: Remove redundant typedef and macros
Message-Id: <20210805214739.390613-1-ehabk...@redhat.com>
https://lore.kernel.org/qemu-devel/20210805214739.390613-1-ehabk...@redhat.com
Date: Thu, 5 Aug 2021 22:31:19 -0400
Subject: [PATCH for-6.2] sbsa-ref: Rename SBSA_GWDT enum value
Message-Id: <20210806023119.431680-1-ehabk...@redhat.com>
https://lore.kernel.org/qemu-devel/20210806023119.431680-1-ehabk...@redhat.com
Based-on: <20210805193431.307761-1-ehabk...@redhat.com>
Based-on: <20210805214739.390613-1-ehabk...@redhat.com>
Based-on: <20210806023119.431680-1-ehabk...@redhat.com>
Eduardo Habkost (12):
accel: Rename TYPE_ACCEL to TYPE_ACCEL_BASE
qom: Use DEVICE_*CLASS instead of OBJECT_*CLASS
scripts/codeconverter: Update to latest version
[automated] Add struct names to typedefs used by QOM types
[automated] Move QOM typedefs and add missing includes
[automated] Split QOM "typedef struct T { ... } T" declarations
[automated] Use DECLARE_*CHECKER* macros when possible
npcm7xx_clk: Use DECLARE_INSTANCE_CHECKER
npcm7xx_otp: Use DECLARE_CLASS_CHECKERS
[automated] Use DECLARE_OBJ_CHECKERS when possible
[automated] Use OBJECT_DECLARE_TYPE when possible
[automated] Use OBJECT_DECLARE_SIMPLE_TYPE when possible
hw/core/machine.c | 2 +-
hw/nvme/nvme.h| 24 +-
hw/usb/hcd-uhci.h | 6 +-
hw/usb/hcd-xhci-pci.h | 8 +-
hw/usb/hcd-xhci-sysbus.h | 8 +-
hw/usb/u2f.h | 17 +-
include/crypto/tlscreds.h | 5 +-
include/hw/acpi/acpi_dev_interface.h | 2 +-
include/hw/adc/npcm7xx_adc.h | 8 +-
include/hw/arm/linux-boot-if.h| 2 +-
include/hw/arm/npcm7xx.h | 36 ++-
include/hw/char/shakti_uart.h | 8 +-
include/hw/core/accel-cpu.h | 6 +-
include/hw/dma/sifive_pdma.h | 8 +-
include/hw/dma/xlnx_csu_dma.h | 8 +-
include/hw/fw-path-provider.h | 2 +-
include/hw/gpio/npcm7xx_gpio.h| 10 +-
include/hw/gpio/sifive_gpio.h | 4 +-
include/hw/hotplug.h | 2 +-
include/hw/i2c/npcm7xx_smbus.h| 10 +-
include/hw/intc/intc.h| 2 +-
include/hw/intc/m68k_irqc.h | 8 +-
include/hw/intc/sifive_clint.h| 8 +-
include/hw/intc/sifive_plic.h | 4 +-
include/hw/ipmi/ipmi.h| 2 +-
include/hw/mem/memory-device.h| 2 +-
include/hw/mem/npcm7xx_mc.h | 9 +-
include/hw/misc/aspeed_lpc.h | 7 +-
include/hw/misc/bcm2835_cprman.h | 21 +-
include/hw/misc/bcm2835_cprman_internals.h| 13 +-
include/hw/misc/led.h | 3 +-
include/hw/misc/mchp_pfsoc_dmc.h | 17 +-
include/hw/misc/mchp_pfsoc_ioscb.h| 8 +-
include/hw/misc/mchp_pfsoc_sysreg.h | 9 +-
include/hw/misc/npcm7xx_clk.h | 18 +-
include/hw/misc/npcm7xx_gcr.h | 7 +-
include/hw/misc/npcm7xx_mft.h | 7 +-
include/hw/misc/npcm7xx_pwm.h | 4 +-
include/hw/misc/npcm7xx_rng.h | 9 +-
include/hw/misc/sifive_e_prci.h | 4 +-
include/hw/misc/sifive_test.h | 4 +-
include/hw/misc/sifive_u_otp.h| 4 +-
include/hw/misc/sifive_u_prci.h | 4 +-
include/hw/misc/xlnx-versal-xramc.h | 8 +-
include/hw/net/npcm7xx_emc.h | 5 +-
include/hw/net/xlnx-zynqmp-can.h | 8 +-
include/hw/nmi.h | 2 +-
include/hw/nvram/npcm7xx_otp.h| 12 +-
include/hw/ppc/spapr_drc.h| 23 +-
include/hw/ppc/spapr_xive.h | 15 +-
include/hw/ppc/vof.h
Win32 and ACCEL macro/function
Hello, I'm looking for help dealing with a naming conflict when building QEMU for Windows hosts. The summary is: I'm trying to replace the ACCEL() macro in include/qemu/accel.h with an inline function, but the ACCEL name conflicts with a symbol provided by winuser.h: In file included from /builds/ehabkost/qemu/include/exec/memory.h:28, from /builds/ehabkost/qemu/hw/ppc/mac.h:30, from ../hw/pci-host/uninorth.c:27: /builds/ehabkost/qemu/include/qemu/accel.h:63:45: error: 'ACCEL' redeclared as different kind of symbol 63 | OBJECT_DECLARE_TYPE(AccelState, AccelClass, ACCEL) | ^ /builds/ehabkost/qemu/include/qom/object.h:178:5: note: in definition of macro 'DECLARE_INSTANCE_CHECKER' 178 | OBJ_NAME(const void *obj) \ | ^~~~ /builds/ehabkost/qemu/include/qom/object.h:240:5: note: in expansion of macro 'DECLARE_OBJ_CHECKERS' 240 | DECLARE_OBJ_CHECKERS(InstanceType, ClassType, \ | ^~~~ /builds/ehabkost/qemu/include/qemu/accel.h:63:1: note: in expansion of macro 'OBJECT_DECLARE_TYPE' 63 | OBJECT_DECLARE_TYPE(AccelState, AccelClass, ACCEL) | ^~~ In file included from /usr/x86_64-w64-mingw32/sys-root/mingw/include/windows.h:72, from /usr/x86_64-w64-mingw32/sys-root/mingw/include/winsock2.h:23, from /builds/ehabkost/qemu/include/sysemu/os-win32.h:29, from /builds/ehabkost/qemu/include/qemu/osdep.h:135, from ../hw/pci-host/uninorth.c:25: /usr/x86_64-w64-mingw32/sys-root/mingw/include/winuser.h:1757:5: note: previous declaration of 'ACCEL' was here 1757 | } ACCEL,*LPACCEL; | ^ [338/4278] Compiling C object libqemuutil.a.p/meson-generated_.._trace_trace-scsi.c.obj ninja: build stopped: subcommand failed. make: *** [Makefile:156: run-ninja] Error 1 (Full log at https://gitlab.com/ehabkost/qemu/-/jobs/1481978645) Does anybody more experienced with Win32 have a suggestion on how to deal with this? Do we really need to include winsock2.h / windows.h / winuser.h from qemu/osdep.h? -- Eduardo
[PATCH for-6.2 v2] s390-sclp-events-bus: Use OBJECT_DECLARE_SIMPLE_TYPE
We have a SCLPEventsBus struct type defined, but no QOM type
checkers are declared for the type.
Use OBJECT_DECLARE_SIMPLE_TYPE to declare the struct type and
have a SCLP_EVENT_BUS typecast wrapper defined.
Signed-off-by: Eduardo Habkost
---
Changes v1 -> v2:
* v1 was previously submitted as part of series:
[PATCH for-6.2 0/6] qom: Fix broken OBJECT_CHECK usage
at
https://lore.kernel.org/qemu-devel/20210805193431.307761-5-ehabk...@redhat.com
* Fix typo (s/SCLP_EVENT_BUS/SCLP_EVENTS_BUS/)
Cc: Richard Henderson
Cc: David Hildenbrand
Cc: Cornelia Huck
Cc: Halil Pasic
Cc: Christian Borntraeger
Cc: Thomas Huth
Cc: qemu-s3...@nongnu.org
Cc: qemu-devel@nongnu.org
---
hw/s390x/event-facility.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/hw/s390x/event-facility.c b/hw/s390x/event-facility.c
index ed92ce510d9..4bfd0b194b4 100644
--- a/hw/s390x/event-facility.c
+++ b/hw/s390x/event-facility.c
@@ -328,6 +328,7 @@ static void write_event_mask(SCLPEventFacility *ef, SCCB
*sccb)
/* qemu object creation and initialization functions */
#define TYPE_SCLP_EVENTS_BUS "s390-sclp-events-bus"
+OBJECT_DECLARE_SIMPLE_TYPE(SCLPEventsBus, SCLP_EVENTS_BUS)
static const TypeInfo sclp_events_bus_info = {
.name = TYPE_SCLP_EVENTS_BUS,
--
2.31.1
Re: [PATCH for-6.2 4/6] s390-sclp-events-bus: Use OBJECT_DECLARE_SIMPLE_TYPE
On Thu, Aug 05, 2021 at 03:34:29PM -0400, Eduardo Habkost wrote: > We have a SCLPEventsBus struct type defined, but no QOM type > checkers are declared for the type. > > Use OBJECT_DECLARE_SIMPLE_TYPE to declare the struct type and > have a SCLP_EVENT_BUS typecast wrapper defined. > > Signed-off-by: Eduardo Habkost > --- > Cc: Richard Henderson > Cc: David Hildenbrand > Cc: Cornelia Huck > Cc: Halil Pasic > Cc: Christian Borntraeger > Cc: Thomas Huth > Cc: qemu-s3...@nongnu.org > Cc: qemu-devel@nongnu.org > --- > hw/s390x/event-facility.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/hw/s390x/event-facility.c b/hw/s390x/event-facility.c > index 0a65e16cdd9..9f7883d6e20 100644 > --- a/hw/s390x/event-facility.c > +++ b/hw/s390x/event-facility.c > @@ -328,6 +328,7 @@ static void write_event_mask(SCLPEventFacility *ef, SCCB > *sccb) > /* qemu object creation and initialization functions */ > > #define TYPE_SCLP_EVENTS_BUS "s390-sclp-events-bus" > +OBJECT_DECLARE_SIMPLE_TYPE(SCLPEventsBus, SCLP_EVENT_BUS) Oops, a typo (should be SCLP_EVENTS_BUS instead). I will submit v2 later as a standalone patch. -- Eduardo
[PATCH for-6.2] sbsa-ref: Rename SBSA_GWDT enum value
The SBSA_GWDT enum value conflicts with the SBSA_GWDT() QOM type
checking helper, preventing us from using a OBJECT_DEFINE* or
DEFINE_INSTANCE_CHECKER macro for the SBSA_GWDT() wrapper.
If I understand the SBSA 6.0 specification correctly, the signal
being connected to IRQ 16 is the WS0 output signal from the
Generic Watchdog. Rename the enum value to SBSA_GWDT_WS0 to be
more explicit and avoid the name conflict.
Signed-off-by: Eduardo Habkost
---
Cc: Radoslaw Biernacki
Cc: Peter Maydell
Cc: Leif Lindholm
Cc: Shashi Mallela
Cc: qemu-...@nongnu.org
Cc: qemu-devel@nongnu.org
---
hw/arm/sbsa-ref.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/hw/arm/sbsa-ref.c b/hw/arm/sbsa-ref.c
index c1629df6031..509c5f09b4e 100644
--- a/hw/arm/sbsa-ref.c
+++ b/hw/arm/sbsa-ref.c
@@ -65,7 +65,7 @@ enum {
SBSA_GIC_DIST,
SBSA_GIC_REDIST,
SBSA_SECURE_EC,
-SBSA_GWDT,
+SBSA_GWDT_WS0,
SBSA_GWDT_REFRESH,
SBSA_GWDT_CONTROL,
SBSA_SMMU,
@@ -140,7 +140,7 @@ static const int sbsa_ref_irqmap[] = {
[SBSA_AHCI] = 10,
[SBSA_EHCI] = 11,
[SBSA_SMMU] = 12, /* ... to 15 */
-[SBSA_GWDT] = 16,
+[SBSA_GWDT_WS0] = 16,
};
static const char * const valid_cpus[] = {
@@ -481,7 +481,7 @@ static void create_wdt(const SBSAMachineState *sms)
hwaddr cbase = sbsa_ref_memmap[SBSA_GWDT_CONTROL].base;
DeviceState *dev = qdev_new(TYPE_WDT_SBSA);
SysBusDevice *s = SYS_BUS_DEVICE(dev);
-int irq = sbsa_ref_irqmap[SBSA_GWDT];
+int irq = sbsa_ref_irqmap[SBSA_GWDT_WS0];
sysbus_realize_and_unref(s, _fatal);
sysbus_mmio_map(s, 0, rbase);
--
2.31.1
[PATCH] bcm2836: Remove redundant typedef and macros
commit 58b350280e97 ("hw/arm/bcm2836: Restrict BCM283XInfo
declaration to C source") didn't just move the struct
BCM283XClass definition to bcm2836.c. It also introduced a
typedef (BCM283XClass) and two type checking macros
(BCM283X_CLASS, BCM283X_GET_CLASS).
The typedef and macros duplicate what is already defined by the
OBJECT_DECLARE_TYPE declaration at bcm2836.h. Remove the
redundant declarations.
Signed-off-by: Eduardo Habkost
---
Cc: Philippe Mathieu-Daudé
Cc: Luc Michel
Cc: Peter Maydell
---
hw/arm/bcm2836.c | 9 ++---
1 file changed, 2 insertions(+), 7 deletions(-)
diff --git a/hw/arm/bcm2836.c b/hw/arm/bcm2836.c
index 24354338cad..f894338fc6a 100644
--- a/hw/arm/bcm2836.c
+++ b/hw/arm/bcm2836.c
@@ -16,7 +16,7 @@
#include "hw/arm/raspi_platform.h"
#include "hw/sysbus.h"
-typedef struct BCM283XClass {
+struct BCM283XClass {
/*< private >*/
DeviceClass parent_class;
/*< public >*/
@@ -26,12 +26,7 @@ typedef struct BCM283XClass {
hwaddr peri_base; /* Peripheral base address seen by the CPU */
hwaddr ctrl_base; /* Interrupt controller and mailboxes etc. */
int clusterid;
-} BCM283XClass;
-
-#define BCM283X_CLASS(klass) \
-OBJECT_CLASS_CHECK(BCM283XClass, (klass), TYPE_BCM283X)
-#define BCM283X_GET_CLASS(obj) \
-OBJECT_GET_CLASS(BCM283XClass, (obj), TYPE_BCM283X)
+};
static Property bcm2836_enabled_cores_property =
DEFINE_PROP_UINT32("enabled-cpus", BCM283XState, enabled_cpus, 0);
--
2.31.1
[PATCH for-6.2 4/6] s390-sclp-events-bus: Use OBJECT_DECLARE_SIMPLE_TYPE
We have a SCLPEventsBus struct type defined, but no QOM type
checkers are declared for the type.
Use OBJECT_DECLARE_SIMPLE_TYPE to declare the struct type and
have a SCLP_EVENT_BUS typecast wrapper defined.
Signed-off-by: Eduardo Habkost
---
Cc: Richard Henderson
Cc: David Hildenbrand
Cc: Cornelia Huck
Cc: Halil Pasic
Cc: Christian Borntraeger
Cc: Thomas Huth
Cc: qemu-s3...@nongnu.org
Cc: qemu-devel@nongnu.org
---
hw/s390x/event-facility.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/hw/s390x/event-facility.c b/hw/s390x/event-facility.c
index 0a65e16cdd9..9f7883d6e20 100644
--- a/hw/s390x/event-facility.c
+++ b/hw/s390x/event-facility.c
@@ -328,6 +328,7 @@ static void write_event_mask(SCLPEventFacility *ef, SCCB
*sccb)
/* qemu object creation and initialization functions */
#define TYPE_SCLP_EVENTS_BUS "s390-sclp-events-bus"
+OBJECT_DECLARE_SIMPLE_TYPE(SCLPEventsBus, SCLP_EVENT_BUS)
static const TypeInfo sclp_events_bus_info = {
.name = TYPE_SCLP_EVENTS_BUS,
--
2.31.1
[PATCH for-6.2 6/6] Use PCI_HOST_BRIDGE macro
OBJECT_CHECK(PciHostState, ..., TYPE_PCI_HOST_BRIDGE) is exactly
what the PCI_HOST_BRIDGE macro does. We can just use the macro
instead of using OBJECT_CHECK manually.
Signed-off-by: Eduardo Habkost
---
Cc: "Michael S. Tsirkin"
Cc: Igor Mammedov
Cc: Paolo Bonzini
Cc: Richard Henderson
Cc: Eduardo Habkost
Cc: Marcel Apfelbaum
Cc: qemu-devel@nongnu.org
---
hw/i386/acpi-build.c | 8 ++--
hw/pci-host/i440fx.c | 4 +---
2 files changed, 3 insertions(+), 9 deletions(-)
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index a33ac8b91e1..3c6bbb1beb3 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -303,13 +303,9 @@ Object *acpi_get_i386_pci_host(void)
{
PCIHostState *host;
-host = OBJECT_CHECK(PCIHostState,
-object_resolve_path("/machine/i440fx", NULL),
-TYPE_PCI_HOST_BRIDGE);
+host = PCI_HOST_BRIDGE(object_resolve_path("/machine/i440fx", NULL));
if (!host) {
-host = OBJECT_CHECK(PCIHostState,
-object_resolve_path("/machine/q35", NULL),
-TYPE_PCI_HOST_BRIDGE);
+host = PCI_HOST_BRIDGE(object_resolve_path("/machine/q35", NULL));
}
return OBJECT(host);
diff --git a/hw/pci-host/i440fx.c b/hw/pci-host/i440fx.c
index 28c9bae8994..cd87e21a9b2 100644
--- a/hw/pci-host/i440fx.c
+++ b/hw/pci-host/i440fx.c
@@ -316,9 +316,7 @@ PCIBus *i440fx_init(const char *host_type, const char
*pci_type,
PCIBus *find_i440fx(void)
{
-PCIHostState *s = OBJECT_CHECK(PCIHostState,
- object_resolve_path("/machine/i440fx",
NULL),
- TYPE_PCI_HOST_BRIDGE);
+PCIHostState *s = PCI_HOST_BRIDGE(object_resolve_path("/machine/i440fx",
NULL));
return s ? s->bus : NULL;
}
--
2.31.1
[PATCH for-6.2 3/6] s390-sclp-events-bus: Set instance_size
We have a SCLPEventsBus struct defined, but the struct is not
used at the TypeInfo definition. This works today but will break
silently if anybody adds a new field to SCLPEventsBus.
Set instance_size properly to avoid problems in the future.
Signed-off-by: Eduardo Habkost
---
Cc: Cornelia Huck
Cc: Thomas Huth
Cc: Halil Pasic
Cc: Christian Borntraeger
Cc: Richard Henderson
Cc: David Hildenbrand
Cc: qemu-s3...@nongnu.org
Cc: qemu-devel@nongnu.org
---
hw/s390x/event-facility.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/hw/s390x/event-facility.c b/hw/s390x/event-facility.c
index ed92ce510d9..0a65e16cdd9 100644
--- a/hw/s390x/event-facility.c
+++ b/hw/s390x/event-facility.c
@@ -332,6 +332,7 @@ static void write_event_mask(SCLPEventFacility *ef, SCCB
*sccb)
static const TypeInfo sclp_events_bus_info = {
.name = TYPE_SCLP_EVENTS_BUS,
.parent = TYPE_BUS,
+.instance_size = sizeof(SCLPEventsBus),
};
static void command_handler(SCLPEventFacility *ef, SCCB *sccb, uint64_t code)
--
2.31.1
[PATCH for-6.2 5/6] s390x: event-facility: Use SCLP_EVENT_BUS macro
Use the SCLP_EVENT_BUS macro instead of manually calling
OBJECT_CHECK.
Signed-off-by: Eduardo Habkost
---
Cc: Cornelia Huck
Cc: Thomas Huth
Cc: Halil Pasic
Cc: Christian Borntraeger
Cc: Richard Henderson
Cc: David Hildenbrand
Cc: qemu-s3...@nongnu.org
Cc: qemu-devel@nongnu.org
---
hw/s390x/event-facility.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/s390x/event-facility.c b/hw/s390x/event-facility.c
index 9f7883d6e20..bc706bd19b4 100644
--- a/hw/s390x/event-facility.c
+++ b/hw/s390x/event-facility.c
@@ -532,7 +532,7 @@ BusState *sclp_get_event_facility_bus(void)
SCLPEventsBus *sbus;
busobj = object_resolve_path_type("", TYPE_SCLP_EVENTS_BUS, NULL);
-sbus = OBJECT_CHECK(SCLPEventsBus, busobj, TYPE_SCLP_EVENTS_BUS);
+sbus = SCLP_EVENT_BUS(busobj);
if (!sbus) {
return NULL;
}
--
2.31.1
[PATCH for-6.2 2/6] sbsa_gwdt: Delete broken SBSA_*CLASS macros
Those macros never worked and never will, because the SBSA_GWDTClass type never existed. Signed-off-by: Eduardo Habkost --- Cc: qemu-devel@nongnu.org Cc: Peter Maydell Cc: Shashi Mallela --- include/hw/watchdog/sbsa_gwdt.h | 4 1 file changed, 4 deletions(-) diff --git a/include/hw/watchdog/sbsa_gwdt.h b/include/hw/watchdog/sbsa_gwdt.h index 70b137de301..dcb13bc29dc 100644 --- a/include/hw/watchdog/sbsa_gwdt.h +++ b/include/hw/watchdog/sbsa_gwdt.h @@ -19,10 +19,6 @@ #define TYPE_WDT_SBSA "sbsa_gwdt" #define SBSA_GWDT(obj) \ OBJECT_CHECK(SBSA_GWDTState, (obj), TYPE_WDT_SBSA) -#define SBSA_GWDT_CLASS(klass) \ -OBJECT_CLASS_CHECK(SBSA_GWDTClass, (klass), TYPE_WDT_SBSA) -#define SBSA_GWDT_GET_CLASS(obj) \ -OBJECT_GET_CLASS(SBSA_GWDTClass, (obj), TYPE_WDT_SBSA) /* SBSA Generic Watchdog register definitions */ /* refresh frame */ -- 2.31.1
[PATCH for-6.2 1/6] acpi: Delete broken ACPI_GED_X86 macro
The macro never worked and never will, because the AcpiGedX86State type never existed. Signed-off-by: Eduardo Habkost --- Cc: "Michael S. Tsirkin" Cc: Igor Mammedov Cc: qemu-devel@nongnu.org --- include/hw/acpi/generic_event_device.h | 2 -- 1 file changed, 2 deletions(-) diff --git a/include/hw/acpi/generic_event_device.h b/include/hw/acpi/generic_event_device.h index 6bed92e8fc5..d49217c445f 100644 --- a/include/hw/acpi/generic_event_device.h +++ b/include/hw/acpi/generic_event_device.h @@ -70,8 +70,6 @@ OBJECT_DECLARE_SIMPLE_TYPE(AcpiGedState, ACPI_GED) #define TYPE_ACPI_GED_X86 "acpi-ged-x86" -#define ACPI_GED_X86(obj) \ -OBJECT_CHECK(AcpiGedX86State, (obj), TYPE_ACPI_GED_X86) #define ACPI_GED_EVT_SEL_OFFSET0x0 #define ACPI_GED_EVT_SEL_LEN 0x4 -- 2.31.1
[PATCH for-6.2 0/6] qom: Fix broken OBJECT_CHECK usage
This series removes some broken OBJECT_CHECK macros and fix cases where OBJECT_CHECK is being used directly in the code. Eduardo Habkost (6): acpi: Delete broken ACPI_GED_X86 macro sbsa_gwdt: Delete broken SBSA_*CLASS macros s390-sclp-events-bus: Set instance_size s390-sclp-events-bus: Use OBJECT_DECLARE_SIMPLE_TYPE s390x: event-facility: Use SCLP_EVENT_BUS macro Use PCI_HOST_BRIDGE macro include/hw/acpi/generic_event_device.h | 2 -- include/hw/watchdog/sbsa_gwdt.h| 4 hw/i386/acpi-build.c | 8 ++-- hw/pci-host/i440fx.c | 4 +--- hw/s390x/event-facility.c | 4 +++- 5 files changed, 6 insertions(+), 16 deletions(-) -- 2.31.1
Re: [PATCH for-6.2 01/10] docs: qom: Replace old GTK-Doc #symbol syntax with `symbol`
On Thu, Aug 05, 2021 at 12:36:11PM -0400, Eduardo Habkost wrote: > On Wed, Aug 04, 2021 at 08:26:10PM -0400, John Snow wrote: > > On Wed, Aug 4, 2021 at 5:00 PM Eduardo Habkost wrote: > > > > > On Wed, Aug 04, 2021 at 09:42:24PM +0100, Peter Maydell wrote: > > > > Is there a sensible default-role we can use as the default for the whole > > > manual, > > > > rather than only declaring it in individual .rst files ? One of the > > > > things I don't > > > > like about the change here is that it means that `thing` in this > > > individual .rst > > > > file is different from `thing` in every other .rst file in our docs. > > > > > > I believe "any" would be a very sensible default role for all > > > documents, but I don't know how to set default-role globally. > > > I'll try to find out. > > > > Oh, I actually fixed that issue I referenced there back in May -- I keep a > > patchset up to date whenever I work on modernizing the QAPI python code > > that actually DOES switch our default role to "any" ... I updated it just > > today, in fact. I will send it to the list if there's an appetite for it > > now. > > If you already have a patch that makes it possible to change the > default role to "any" globally, I'd be glad to include it in v2 > of this series. John had submitted the patches at: https://lore.kernel.org/qemu-devel/20210805004837.1775306-1-js...@redhat.com/ (Thanks!) If John's patches are merged, the only change needed in this series is the removal of the "default-role" line in patch 01/10. Instead of submitting v2 for a one-line change, I'm hoping I can just get Reviewed-by lines for this version. (the reviews can be conditional on the removal of the "default-role" line in patch 01/10) -- Eduardo
Re: [PATCH for-6.2 01/10] docs: qom: Replace old GTK-Doc #symbol syntax with `symbol`
On Wed, Aug 04, 2021 at 08:26:10PM -0400, John Snow wrote: > On Wed, Aug 4, 2021 at 5:00 PM Eduardo Habkost wrote: > > > On Wed, Aug 04, 2021 at 09:42:24PM +0100, Peter Maydell wrote: > > > Is there a sensible default-role we can use as the default for the whole > > manual, > > > rather than only declaring it in individual .rst files ? One of the > > > things I don't > > > like about the change here is that it means that `thing` in this > > individual .rst > > > file is different from `thing` in every other .rst file in our docs. > > > > I believe "any" would be a very sensible default role for all > > documents, but I don't know how to set default-role globally. > > I'll try to find out. > > Oh, I actually fixed that issue I referenced there back in May -- I keep a > patchset up to date whenever I work on modernizing the QAPI python code > that actually DOES switch our default role to "any" ... I updated it just > today, in fact. I will send it to the list if there's an appetite for it > now. If you already have a patch that makes it possible to change the default role to "any" globally, I'd be glad to include it in v2 of this series. -- Eduardo
