Re: [PATCH v4 0/4] qapi: generalize special features
Queued with imports tidied up. Thanks!
[PULL 5/6] qapi: rename 'special_features' to 'features'
From: Daniel P. Berrangé
This updates the QAPI code generation to refer to 'features' instead
of 'special_features', in preparation for generalizing their exposure.
Signed-off-by: Daniel P. Berrangé
Message-ID: <20250205123550.2754387-4-berra...@redhat.com>
Reviewed-by: Markus Armbruster
[Imports tidied up with isort]
Signed-off-by: Markus Armbruster
---
scripts/qapi/commands.py | 4 ++--
scripts/qapi/gen.py | 8
scripts/qapi/types.py| 14 +-
scripts/qapi/visit.py| 18 +++---
4 files changed, 18 insertions(+), 26 deletions(-)
diff --git a/scripts/qapi/commands.py b/scripts/qapi/commands.py
index 79951a841f..d629d2d97e 100644
--- a/scripts/qapi/commands.py
+++ b/scripts/qapi/commands.py
@@ -25,7 +25,7 @@
QAPIGenC,
QAPISchemaModularCVisitor,
build_params,
-gen_special_features,
+gen_features,
ifcontext,
)
from .schema import (
@@ -298,7 +298,7 @@ def gen_register_command(name: str,
''',
name=name, c_name=c_name(name),
opts=' | '.join(options) or 0,
-feats=gen_special_features(features))
+feats=gen_features(features))
return ret
diff --git a/scripts/qapi/gen.py b/scripts/qapi/gen.py
index c53ca72950..b51f8d955e 100644
--- a/scripts/qapi/gen.py
+++ b/scripts/qapi/gen.py
@@ -41,10 +41,10 @@
from .source import QAPISourceInfo
-def gen_special_features(features: Sequence[QAPISchemaFeature]) -> str:
-special_features = [f"1u << {c_enum_const('qapi', feat.name)}"
-for feat in features if feat.is_special()]
-return ' | '.join(special_features) or '0'
+def gen_features(features: Sequence[QAPISchemaFeature]) -> str:
+featenum = [f"1u << {c_enum_const('qapi', feat.name)}"
+for feat in features if feat.is_special()]
+return ' | '.join(featenum) or '0'
class QAPIGen:
diff --git a/scripts/qapi/types.py b/scripts/qapi/types.py
index 7bc3f8241f..e4a1bb9f85 100644
--- a/scripts/qapi/types.py
+++ b/scripts/qapi/types.py
@@ -16,11 +16,7 @@
from typing import List, Optional
from .common import c_enum_const, c_name, mcgen
-from .gen import (
-QAPISchemaModularCVisitor,
-gen_special_features,
-ifcontext,
-)
+from .gen import QAPISchemaModularCVisitor, gen_features, ifcontext
from .schema import (
QAPISchema,
QAPISchemaAlternatives,
@@ -61,12 +57,12 @@ def gen_enum_lookup(name: str,
index=index, name=memb.name)
ret += memb.ifcond.gen_endif()
-special_features = gen_special_features(memb.features)
-if special_features != '0':
+features = gen_features(memb.features)
+if features != '0':
feats += mcgen('''
-[%(index)s] = %(special_features)s,
+[%(index)s] = %(features)s,
''',
- index=index, special_features=special_features)
+ index=index, features=features)
if feats:
ret += mcgen('''
diff --git a/scripts/qapi/visit.py b/scripts/qapi/visit.py
index 12f92e429f..928273b9bb 100644
--- a/scripts/qapi/visit.py
+++ b/scripts/qapi/visit.py
@@ -21,11 +21,7 @@
indent,
mcgen,
)
-from .gen import (
-QAPISchemaModularCVisitor,
-gen_special_features,
-ifcontext,
-)
+from .gen import QAPISchemaModularCVisitor, gen_features, ifcontext
from .schema import (
QAPISchema,
QAPISchemaAlternatives,
@@ -103,15 +99,15 @@ def gen_visit_object_members(name: str,
''',
name=memb.name, has=has)
indent.increase()
-special_features = gen_special_features(memb.features)
-if special_features != '0':
+features = gen_features(memb.features)
+if features != '0':
ret += mcgen('''
-if (visit_policy_reject(v, "%(name)s", %(special_features)s, errp)) {
+if (visit_policy_reject(v, "%(name)s", %(features)s, errp)) {
return false;
}
-if (!visit_policy_skip(v, "%(name)s", %(special_features)s)) {
+if (!visit_policy_skip(v, "%(name)s", %(features)s)) {
''',
- name=memb.name, special_features=special_features)
+ name=memb.name, features=features)
indent.increase()
ret += mcgen('''
if (!visit_type_%(c_type)s(v, "%(name)s", &obj->%(c_name)s, errp)) {
@@ -120,7 +116,7 @@ def gen_visit_object_members(name: str,
''',
c_type=memb.type.c_name(), name=memb.name,
c_name=c_name(memb.name))
-if special_features != '0':
+if features != '0':
indent.decrease()
ret += mcgen('''
}
--
2.48.1
[PULL 2/6] qapi/ui: Fix documentation of upper bound value in InputMoveEvent
From: Zhang Boyang The upper bound of pointer position in InputMoveEvent should be 0x7fff, according to INPUT_EVENT_ABS_MAX. Signed-off-by: Zhang Boyang Message-ID: <20250116104433.12114-1-zhangboyang...@gmail.com> Acked-by: Markus Armbruster [Phrasing tweak squashed in] Signed-off-by: Markus Armbruster --- qapi/ui.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/qapi/ui.json b/qapi/ui.json index 460a26b981..c536d4e524 100644 --- a/qapi/ui.json +++ b/qapi/ui.json @@ -1133,7 +1133,7 @@ # @axis: Which axis is referenced by @value. # # @value: Pointer position. For absolute coordinates the valid range -# is 0 -> 0x7 +# is 0 to 0x7fff. # # Since: 2.0 ## -- 2.48.1
[PULL 4/6] qapi: change 'unsigned special_features' to 'uint64_t features'
From: Daniel P. Berrangé
The "special_features" field / parameter holds the subset of schema
features that are for internal code use. Specifically 'DEPRECATED'
and 'UNSTABLE'.
This special casing of internal features is going to be removed, so
prepare for that by renaming to 'features'. Using a fixed size type
is also best practice for bit fields.
Reviewed-by: Philippe Mathieu-Daudé
Signed-off-by: Daniel P. Berrangé
Message-ID: <20250205123550.2754387-3-berra...@redhat.com>
Reviewed-by: Markus Armbruster
Signed-off-by: Markus Armbruster
---
include/qapi/compat-policy.h | 2 +-
include/qapi/qmp/dispatch.h | 4 ++--
include/qapi/util.h | 2 +-
include/qapi/visitor-impl.h | 4 ++--
include/qapi/visitor.h| 12 ++--
qapi/qapi-forward-visitor.c | 8
qapi/qapi-util.c | 6 +++---
qapi/qapi-visit-core.c| 12 ++--
qapi/qmp-dispatch.c | 2 +-
qapi/qmp-registry.c | 4 ++--
qapi/qobject-input-visitor.c | 4 ++--
qapi/qobject-output-visitor.c | 6 +++---
scripts/qapi/types.py | 2 +-
13 files changed, 34 insertions(+), 34 deletions(-)
diff --git a/include/qapi/compat-policy.h b/include/qapi/compat-policy.h
index 8b7b25c0b5..ea65e10744 100644
--- a/include/qapi/compat-policy.h
+++ b/include/qapi/compat-policy.h
@@ -18,7 +18,7 @@
extern CompatPolicy compat_policy;
-bool compat_policy_input_ok(unsigned special_features,
+bool compat_policy_input_ok(uint64_t features,
const CompatPolicy *policy,
ErrorClass error_class,
const char *kind, const char *name,
diff --git a/include/qapi/qmp/dispatch.h b/include/qapi/qmp/dispatch.h
index f2e956813a..e0ee1ad3ac 100644
--- a/include/qapi/qmp/dispatch.h
+++ b/include/qapi/qmp/dispatch.h
@@ -33,7 +33,7 @@ typedef struct QmpCommand
/* Runs in coroutine context if QCO_COROUTINE is set */
QmpCommandFunc *fn;
QmpCommandOptions options;
-unsigned special_features;
+uint64_t features;
QTAILQ_ENTRY(QmpCommand) node;
bool enabled;
const char *disable_reason;
@@ -43,7 +43,7 @@ typedef QTAILQ_HEAD(QmpCommandList, QmpCommand)
QmpCommandList;
void qmp_register_command(QmpCommandList *cmds, const char *name,
QmpCommandFunc *fn, QmpCommandOptions options,
- unsigned special_features);
+ uint64_t features);
const QmpCommand *qmp_find_command(const QmpCommandList *cmds,
const char *name);
void qmp_disable_command(QmpCommandList *cmds, const char *name,
diff --git a/include/qapi/util.h b/include/qapi/util.h
index b8254247b8..29bc4eb865 100644
--- a/include/qapi/util.h
+++ b/include/qapi/util.h
@@ -18,7 +18,7 @@ typedef enum {
typedef struct QEnumLookup {
const char *const *array;
-const unsigned char *const special_features;
+const uint64_t *const features;
const int size;
} QEnumLookup;
diff --git a/include/qapi/visitor-impl.h b/include/qapi/visitor-impl.h
index 2badec5ba4..7beb0dbfa5 100644
--- a/include/qapi/visitor-impl.h
+++ b/include/qapi/visitor-impl.h
@@ -115,11 +115,11 @@ struct Visitor
/* Optional */
bool (*policy_reject)(Visitor *v, const char *name,
- unsigned special_features, Error **errp);
+ uint64_t features, Error **errp);
/* Optional */
bool (*policy_skip)(Visitor *v, const char *name,
-unsigned special_features);
+uint64_t features);
/* Must be set */
VisitorType type;
diff --git a/include/qapi/visitor.h b/include/qapi/visitor.h
index 27b85d4700..f6a9b0743f 100644
--- a/include/qapi/visitor.h
+++ b/include/qapi/visitor.h
@@ -463,29 +463,29 @@ bool visit_optional(Visitor *v, const char *name, bool
*present);
/*
* Should we reject member @name due to policy?
*
- * @special_features is the member's special features encoded as a
- * bitset of QapiSpecialFeature.
+ * @features is the member's special features encoded as a
+ * bitset of QapiFeature.
*
* @name must not be NULL. This function is only useful between
* visit_start_struct() and visit_end_struct(), since only objects
* have deprecated members.
*/
bool visit_policy_reject(Visitor *v, const char *name,
- unsigned special_features, Error **errp);
+ uint64_t features, Error **errp);
/*
*
* Should we skip member @name due to policy?
*
- * @special_features is the member's special features encoded as a
- * bitset of QapiSpecialFeature.
+ * @features is the member's special features encoded as a
+ * bitset of QapiFeature.
*
* @name must not be NULL. This function is only useful between
* visit_start_struct() and visit_end_struct(), since only objects
* have deprecated members.
*/
bool visit_policy_skip(Visitor *v, const char *name,
[PULL 6/6] qapi: expose all schema features to code
From: Daniel P. Berrangé
This replaces use of the constants from the QapiSpecialFeatures
enum, with constants from the auto-generate QapiFeatures enum
in qapi-features.h
The 'deprecated' and 'unstable' features still have a little bit of
special handling, being force defined to be the 1st + 2nd features
in the enum, regardless of whether they're used in the schema. This
retains compatibility with common code that references the features
via the QapiSpecialFeatures constants.
Signed-off-by: Daniel P. Berrangé
Message-ID: <20250205123550.2754387-5-berra...@redhat.com>
Reviewed-by: Markus Armbruster
[Imports tidied up with isort]
Signed-off-by: Markus Armbruster
---
meson.build | 1 +
scripts/qapi/commands.py | 1 +
scripts/qapi/features.py | 48
scripts/qapi/gen.py | 6 +--
scripts/qapi/main.py | 2 +
scripts/qapi/schema.py | 31 ++-
scripts/qapi/types.py| 7 +++-
scripts/qapi/visit.py| 3 +-
tests/meson.build| 2 +
tests/qapi-schema/features-too-many.err | 2 +
tests/qapi-schema/features-too-many.json | 13 +++
tests/qapi-schema/features-too-many.out | 0
tests/qapi-schema/meson.build| 1 +
13 files changed, 110 insertions(+), 7 deletions(-)
create mode 100644 scripts/qapi/features.py
create mode 100644 tests/qapi-schema/features-too-many.err
create mode 100644 tests/qapi-schema/features-too-many.json
create mode 100644 tests/qapi-schema/features-too-many.out
diff --git a/meson.build b/meson.build
index 131b2225ab..ee59d647e4 100644
--- a/meson.build
+++ b/meson.build
@@ -3444,6 +3444,7 @@ qapi_gen_depends = [ meson.current_source_dir() /
'scripts/qapi/__init__.py',
meson.current_source_dir() / 'scripts/qapi/schema.py',
meson.current_source_dir() / 'scripts/qapi/source.py',
meson.current_source_dir() / 'scripts/qapi/types.py',
+ meson.current_source_dir() / 'scripts/qapi/features.py',
meson.current_source_dir() / 'scripts/qapi/visit.py',
meson.current_source_dir() / 'scripts/qapi-gen.py'
]
diff --git a/scripts/qapi/commands.py b/scripts/qapi/commands.py
index d629d2d97e..bf88bfc442 100644
--- a/scripts/qapi/commands.py
+++ b/scripts/qapi/commands.py
@@ -355,6 +355,7 @@ def visit_begin(self, schema: QAPISchema) -> None:
#include "qemu/osdep.h"
#include "%(prefix)sqapi-commands.h"
#include "%(prefix)sqapi-init-commands.h"
+#include "%(prefix)sqapi-features.h"
void %(c_prefix)sqmp_init_marshal(QmpCommandList *cmds)
{
diff --git a/scripts/qapi/features.py b/scripts/qapi/features.py
new file mode 100644
index 00..57563207a8
--- /dev/null
+++ b/scripts/qapi/features.py
@@ -0,0 +1,48 @@
+"""
+QAPI features generator
+
+Copyright 2024 Red Hat
+
+This work is licensed under the terms of the GNU GPL, version 2.
+# See the COPYING file in the top-level directory.
+"""
+
+from typing import ValuesView
+
+from .common import c_enum_const, c_name
+from .gen import QAPISchemaMonolithicCVisitor
+from .schema import QAPISchema, QAPISchemaFeature
+
+
+class QAPISchemaGenFeatureVisitor(QAPISchemaMonolithicCVisitor):
+
+def __init__(self, prefix: str):
+super().__init__(
+prefix, 'qapi-features',
+' * Schema-defined QAPI features',
+__doc__)
+
+self.features: ValuesView[QAPISchemaFeature]
+
+def visit_begin(self, schema: QAPISchema) -> None:
+self.features = schema.features()
+self._genh.add("#include \"qapi/util.h\"\n\n")
+
+def visit_end(self) -> None:
+self._genh.add("typedef enum {\n")
+for f in self.features:
+self._genh.add(f"{c_enum_const('qapi_feature', f.name)}")
+if f.name in QAPISchemaFeature.SPECIAL_NAMES:
+self._genh.add(f" = {c_enum_const('qapi', f.name)},\n")
+else:
+self._genh.add(",\n")
+
+self._genh.add("} " + c_name('QapiFeature') + ";\n")
+
+
+def gen_features(schema: QAPISchema,
+ output_dir: str,
+ prefix: str) -> None:
+vis = QAPISchemaGenFeatureVisitor(prefix)
+schema.visit(vis)
+vis.write(output_dir)
diff --git a/scripts/qapi/gen.py b/scripts/qapi/gen.py
index b51f8d955e..d3c56d45c8 100644
--- a/scripts/qapi/gen.py
+++ b/scripts/qapi/gen.py
@@ -42,9 +42,9 @@
def gen_features(features: Sequence[QAPISchemaFeature]) -> str:
-featenum = [f"1u << {c_enum_const('qapi', feat.name)}"
-for feat in features if feat.is_special()]
-return ' | '.join(featenum) or '0'
+feats = [f"1u << {c_enum_const('qapi_feature', feat.name)}"
+ for feat in features]
+return ' | '.join(feats) or '0'
class QAPIGen:
diff --git a/scripts/qapi/main.py b/scripts/qapi
Re: [PATCH 4/5] hw/arm/smmuv3: Move reset to exit phase
Hi Peter,
On 2/7/25 5:58 PM, Peter Maydell wrote:
> On Fri, 7 Feb 2025 at 16:50, Eric Auger wrote:
>>
>>
>>
>> On 2/7/25 5:37 PM, Peter Maydell wrote:
>>> On Thu, 6 Feb 2025 at 14:23, Eric Auger wrote:
Currently the iommu may be reset before the devices
it protects. For example this happens with virtio-scsi-pci.
when system_reset is issued from qmp monitor, spurious
"virtio: zero sized buffers are not allowed" warnings can
be observed.
Signed-off-by: Eric Auger
---
hw/arm/smmuv3.c | 9 +
hw/arm/trace-events | 1 +
2 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
index c0cf5df0f6..7522c32b24 100644
--- a/hw/arm/smmuv3.c
+++ b/hw/arm/smmuv3.c
@@ -1870,13 +1870,14 @@ static void smmu_init_irq(SMMUv3State *s,
SysBusDevice *dev)
}
}
-static void smmu_reset_hold(Object *obj, ResetType type)
+static void smmu_reset_exit(Object *obj, ResetType type)
{
SMMUv3State *s = ARM_SMMUV3(obj);
SMMUv3Class *c = ARM_SMMUV3_GET_CLASS(s);
-if (c->parent_phases.hold) {
-c->parent_phases.hold(obj, type);
+trace_smmu_reset_exit();
+if (c->parent_phases.exit) {
+c->parent_phases.exit(obj, type);
}
>>> If we need to do something unexpected like reset
>>> register values in the exit phase rather than the
>>> hold phase, it's a good idea to have a comment explaining
>>> why, to avoid somebody coming along afterwards and tidying
>>> it up into the more usual arrangement.
>> sure
>>> If I understand correctly we need to keep the whole IOMMU
>>> config intact until the exit phase? What's the thing the
>>> device behind the IOMMU is trying to do during its reset
>>> that triggers the warning?
>> The virtio-pci-net continues to perform DMA requests and this causes
>> some weird messages such as:
>> "virtio: bogus descriptor or out of resources"
>>
>> Also VFIO devices may continue issuing DMAs causing translation faults
> Hmm, right. I guess this only happens with KVM, or can you
> trigger it in a TCG setup too? Anyway, presumably we can
I have only tested with KVM acceleration for now.
> rely on the devices quiescing all their outstanding DMA
> by the time the hold phase comes along.
>
> (I wonder if we ought to suggest quiescing outstanding
> DMA in the enter phase? But it's probably easier to fix
> the iommus like this series does than try to get every
> dma-capable pci device to do something different.)
at least we can document this, as Peter later suggests. Indeed fixing it
at vIOMMU level looked much easier for me, hoping that no other DMA
capable device would stop DMAs at exit phase
Eric
>
> thanks
> -- PMM
>
[PULL 3/6] qapi: cope with feature names containing a '-'
From: Daniel P. Berrangé
When we shortly expose all feature names to code, it will be valid to
include a '-', which must be translated to a '_' for the enum constants.
Signed-off-by: Daniel P. Berrangé
Message-ID: <20250205123550.2754387-2-berra...@redhat.com>
Reviewed-by: Markus Armbruster
Signed-off-by: Markus Armbruster
---
scripts/qapi/gen.py | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/scripts/qapi/gen.py b/scripts/qapi/gen.py
index 6a8abe0041..c53ca72950 100644
--- a/scripts/qapi/gen.py
+++ b/scripts/qapi/gen.py
@@ -24,6 +24,7 @@
)
from .common import (
+c_enum_const,
c_fname,
c_name,
guardend,
@@ -41,7 +42,7 @@
def gen_special_features(features: Sequence[QAPISchemaFeature]) -> str:
-special_features = [f"1u << QAPI_{feat.name.upper()}"
+special_features = [f"1u << {c_enum_const('qapi', feat.name)}"
for feat in features if feat.is_special()]
return ' | '.join(special_features) or '0'
--
2.48.1
[PULL 1/6] qapi: fix colon in Since tag section
From: Victor Toso
As described in docs/devel/qapi-code-gen.rst line 998,
there should be no space between "Since" and ":".
Signed-off-by: Victor Toso
Message-ID: <20241217091504.16416-1-victort...@redhat.com>
Reviewed-by: Markus Armbruster
Signed-off-by: Markus Armbruster
---
qapi/cxl.json | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/qapi/cxl.json b/qapi/cxl.json
index 9f65589bce..dd947d3bbc 100644
--- a/qapi/cxl.json
+++ b/qapi/cxl.json
@@ -460,7 +460,7 @@
#
# @unstable: For now this command is subject to change.
#
-# Since : 9.1
+# Since: 9.1
##
{ 'command': 'cxl-add-dynamic-capacity',
'data': { 'path': 'str',
@@ -539,7 +539,7 @@
#
# @unstable: For now this command is subject to change.
#
-# Since : 9.1
+# Since: 9.1
##
{ 'command': 'cxl-release-dynamic-capacity',
'data': { 'path': 'str',
--
2.48.1
[PULL 0/6] QAPI patches patches for 2025-02-10
The following changes since commit 04d3d0e9f54d4c42759f3810aa135ce314d98dc4: Merge tag 'hppa-system-for-v10-diva-artist-pull-request' of https://github.com/hdeller/qemu-hppa into staging (2025-02-08 09:00:57 -0500) are available in the Git repository at: https://repo.or.cz/qemu/armbru.git tags/pull-qapi-2025-02-10 for you to fetch changes up to 9c339601b652edf91d74a626999285e3245b2589: qapi: expose all schema features to code (2025-02-10 09:14:14 +0100) QAPI patches patches for 2025-02-10 Daniel P. Berrangé (4): qapi: cope with feature names containing a '-' qapi: change 'unsigned special_features' to 'uint64_t features' qapi: rename 'special_features' to 'features' qapi: expose all schema features to code Victor Toso (1): qapi: fix colon in Since tag section Zhang Boyang (1): qapi/ui: Fix documentation of upper bound value in InputMoveEvent meson.build | 1 + qapi/cxl.json| 4 +-- qapi/ui.json | 2 +- include/qapi/compat-policy.h | 2 +- include/qapi/qmp/dispatch.h | 4 +-- include/qapi/util.h | 2 +- include/qapi/visitor-impl.h | 4 +-- include/qapi/visitor.h | 12 qapi/qapi-forward-visitor.c | 8 +++--- qapi/qapi-util.c | 6 ++-- qapi/qapi-visit-core.c | 12 qapi/qmp-dispatch.c | 2 +- qapi/qmp-registry.c | 4 +-- qapi/qobject-input-visitor.c | 4 +-- qapi/qobject-output-visitor.c| 6 ++-- scripts/qapi/commands.py | 5 ++-- scripts/qapi/features.py | 48 scripts/qapi/gen.py | 9 +++--- scripts/qapi/main.py | 2 ++ scripts/qapi/schema.py | 31 - scripts/qapi/types.py| 23 --- scripts/qapi/visit.py| 21 ++ tests/meson.build| 2 ++ tests/qapi-schema/features-too-many.err | 2 ++ tests/qapi-schema/features-too-many.json | 13 + tests/qapi-schema/features-too-many.out | 0 tests/qapi-schema/meson.build| 1 + 27 files changed, 163 insertions(+), 67 deletions(-) create mode 100644 scripts/qapi/features.py create mode 100644 tests/qapi-schema/features-too-many.err create mode 100644 tests/qapi-schema/features-too-many.json create mode 100644 tests/qapi-schema/features-too-many.out -- 2.48.1
Re: [PATCH 0/5] Fix vIOMMU reset order
Hi, On 2/7/25 6:31 PM, Peter Xu wrote: > On Fri, Feb 07, 2025 at 05:06:20PM +, Peter Maydell wrote: >> On Fri, 7 Feb 2025 at 16:54, Peter Xu wrote: >>> On Thu, Feb 06, 2025 at 03:21:51PM +0100, Eric Auger wrote: This is a follow-up of Peter's attempt to fix the fact that vIOMMUs are likely to be reset before the device they protect: [PATCH 0/4] intel_iommu: Reset vIOMMU after all the rest of devices https://lore.kernel.org/all/20240117091559.144730-1-pet...@redhat.com/ This is especially observed with virtio devices when a qmp system_reset command is sent but also with VFIO devices. This series puts the vIOMMU reset in the 3-phase exit callback. This scheme was tested successful with virtio-devices and some VFIO devices. Nevertheless not all the topologies have been tested yet. >>> Eric, >>> >>> It's great to know that we seem to be able to fix everything in such small >>> changeset! >>> >>> I would like to double check two things with you here: >>> >>> - For VFIO's reset hook, looks like we have landed more changes so that >>> vfio's reset function is now a TYPE_LEGACY_RESET, and it always do the >>> reset during "hold" phase only (via legacy_reset_hold()). That part >>> will make sure vIOMMU (if switching to exit()-only reset) will order >>> properly with VFIO. Is my understanding correct here? >> Yes, we now do a reset of the whole system as a three-phase setup, >> and the old pre-three-phase reset APIs like qemu_register_reset() and >> device_class_set_legacy_reset() all happen during the "hold" phase. >> >>> - Is it possible if some PCIe devices that will provide its own >>> phase.exit(), would it matter on the order of PCIe device's >>> phase.exit() and vIOMMU's phase.exit() (if vIOMMUs switch to use >>> exit()-only approach like this one)? >> It's certainly possible for a PCIe device to implement >> a three-phase reset which does things in the exit phase. However >> I think I would say that such a device which didn't cancel all >> outstanding DMA operations during either 'enter' or 'hold' >> phases would be broken. If it did some other things during >> the 'exit' phase I don't think the ordering of those vs the >> iommu 'exit' handling should matter. > Yes, this sounds fair. > >> (To some extent the splitting into three phases is trying >> to set up a consistent model as outlined in docs/devel/reset.rst >> and to some extent it's just a convenient way to get a basic >> "this reset thing I need to do must happen after some other >> device has done its reset things" which you can achieve >> by ad-hoc putting them in different phases. Ideally we get >> mostly the former and a little pragmatic dose of the latter, >> but the consistent model is not very solidly nailed down >> so I have a feeling the proportions may not be quite as >> lopsided as we'd like :-) ) > Yes, it's a good move that we can have other ways to fix all the problems > without major surgery, and it also looks solid and clean if we have plan to > fix any outlier PCIe devices. > > If there will be a repost after all, not sure if Eric would like to add > some of above discussions into either some commit messages or cover letter. > Or some comment in the code might be even better. Yes I will definitively augment commit msgs/cover letter with all those considerations. Thank you very much for this rich discussion! Eric > > Thanks! >
[PATCH v3 0/7] physmem: teach cpu_memory_rw_debug() to write to more memory regions
This is a follow-up to [1], implementing it by avoiding the use of address_space_write_rom() in cpu_memory_rw_debug() completely, and teaching address_space_write() about debug access instead, the can also write to ROM. The goal is to let GDB via cpu_memory_rw_debug() to also properly write to MMIO device regions, not just RAM/ROM. It's worth noting that other users of address_space_write_rom() are left unchanged. Maybe hw/core/loader.c and friends could now be converted to to a debug access via address_space_write() instead? Survives a basic gitlab CI build/check. [1] https://lore.kernel.org/all/20241220195923.314208-1-...@zabka.it/ v2 -> v3: * Rebased, only a minor conflict in the last patch. v1 -> v2: * Split up "physmem: disallow direct access to RAM DEVICE in address_space_write_rom()" into 4 patches Cc: Paolo Bonzini Cc: Peter Xu Cc: Philippe Mathieu-Daudé Cc: Peter Maydell Cc: Alex Bennée Cc: Alex Williamson Cc: Eduardo Habkost Cc: Marcel Apfelbaum Cc: Elena Ufimtseva Cc: Jagannathan Raman Cc: "Dr. David Alan Gilbert" Cc: Stefan Zabka David Hildenbrand (7): physmem: factor out memory_region_is_ram_device() check in memory_access_is_direct() physmem: factor out RAM/ROMD check in memory_access_is_direct() physmem: factor out direct access check into memory_region_supports_direct_access() physmem: disallow direct access to RAM DEVICE in address_space_write_rom() memory: pass MemTxAttrs to memory_access_is_direct() hmp: use cpu_get_phys_page_debug() in hmp_gva2gpa() physmem: teach cpu_memory_rw_debug() to write to more memory regions hw/core/cpu-system.c | 13 + hw/core/loader.c | 2 +- hw/remote/vfio-user-obj.c | 2 +- include/exec/memattrs.h | 5 - include/exec/memory.h | 35 +++ monitor/hmp-cmds-target.c | 3 +-- system/memory_ldst.c.inc | 18 +- system/physmem.c | 24 +--- 8 files changed, 61 insertions(+), 41 deletions(-) -- 2.48.1
[PATCH v3 4/7] physmem: disallow direct access to RAM DEVICE in address_space_write_rom()
As documented in commit 4a2e242bbb306 ("memory: Don't use memcpy for
ram_device regions"), we disallow direct access to RAM DEVICE regions.
This change implies that address_space_write_rom() and
cpu_memory_rw_debug() won't be able to write to RAM DEVICE regions. It
will also affect cpu_flush_icache_range(), but it's only used by
hw/core/loader.c after writing to ROM, so it is expected to not apply
here with RAM DEVICE.
This fixes direct access to these regions where we don't want direct
access. We'll extend cpu_memory_rw_debug() next to also be able to write to
these (and IO) regions.
This is a preparation for further changes.
Cc: Alex Williamson
Reviewed-by: Peter Xu
Signed-off-by: David Hildenbrand
---
system/physmem.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/system/physmem.c b/system/physmem.c
index 67c9db9daa..7cfcc6cafa 100644
--- a/system/physmem.c
+++ b/system/physmem.c
@@ -3137,8 +3137,7 @@ static inline MemTxResult
address_space_write_rom_internal(AddressSpace *as,
l = len;
mr = address_space_translate(as, addr, &addr1, &l, true, attrs);
-if (!(memory_region_is_ram(mr) ||
- memory_region_is_romd(mr))) {
+if (!memory_region_supports_direct_access(mr)) {
l = memory_access_size(mr, l, addr1);
} else {
/* ROM/RAM case */
--
2.48.1
[PATCH v3 1/7] physmem: factor out memory_region_is_ram_device() check in memory_access_is_direct()
As documented in commit 4a2e242bbb306 ("memory: Don't use memcpy for
ram_device regions"), we disallow direct access to RAM DEVICE regions.
Let's make this clearer to prepare for further changes. Note that romd
regions will never be RAM DEVICE at the same time.
Reviewed-by: Peter Xu
Signed-off-by: David Hildenbrand
---
include/exec/memory.h | 13 ++---
1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/include/exec/memory.h b/include/exec/memory.h
index 9f73b59867..5cd7574c60 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -2997,12 +2997,19 @@ bool prepare_mmio_access(MemoryRegion *mr);
static inline bool memory_access_is_direct(MemoryRegion *mr, bool is_write)
{
+/*
+ * RAM DEVICE regions can be accessed directly using memcpy, but it might
+ * be MMIO and access using mempy can be wrong (e.g., using instructions
not
+ * intended for MMIO access). So we treat this as IO.
+ */
+if (memory_region_is_ram_device(mr)) {
+return false;
+}
if (is_write) {
return memory_region_is_ram(mr) && !mr->readonly &&
- !mr->rom_device && !memory_region_is_ram_device(mr);
+ !mr->rom_device;
} else {
-return (memory_region_is_ram(mr) && !memory_region_is_ram_device(mr))
||
- memory_region_is_romd(mr);
+return memory_region_is_ram(mr) || memory_region_is_romd(mr);
}
}
--
2.48.1
[PATCH v3 3/7] physmem: factor out direct access check into memory_region_supports_direct_access()
Let's factor the complete "directly accessible" check independent of
the "write" condition out so we can reuse it next.
We can now split up the checks RAM and ROMD check, so we really only check
for RAM DEVICE in case of RAM -- ROM DEVICE is neither RAM not RAM DEVICE.
Reviewed-by: Peter Xu
Signed-off-by: David Hildenbrand
---
include/exec/memory.h | 14 +++---
1 file changed, 11 insertions(+), 3 deletions(-)
diff --git a/include/exec/memory.h b/include/exec/memory.h
index cb35c38402..4e2cf95ab6 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -2995,10 +2995,13 @@ MemTxResult
address_space_write_cached_slow(MemoryRegionCache *cache,
int memory_access_size(MemoryRegion *mr, unsigned l, hwaddr addr);
bool prepare_mmio_access(MemoryRegion *mr);
-static inline bool memory_access_is_direct(MemoryRegion *mr, bool is_write)
+static inline bool memory_region_supports_direct_access(MemoryRegion *mr)
{
/* ROM DEVICE regions only allow direct access if in ROMD mode. */
-if (!memory_region_is_ram(mr) && !memory_region_is_romd(mr)) {
+if (memory_region_is_romd(mr)) {
+return true;
+}
+if (!memory_region_is_ram(mr)) {
return false;
}
/*
@@ -3006,7 +3009,12 @@ static inline bool memory_access_is_direct(MemoryRegion
*mr, bool is_write)
* be MMIO and access using mempy can be wrong (e.g., using instructions
not
* intended for MMIO access). So we treat this as IO.
*/
-if (memory_region_is_ram_device(mr)) {
+return !memory_region_is_ram_device(mr);
+}
+
+static inline bool memory_access_is_direct(MemoryRegion *mr, bool is_write)
+{
+if (!memory_region_supports_direct_access(mr)) {
return false;
}
if (is_write) {
--
2.48.1
[PATCH v3 7/7] physmem: teach cpu_memory_rw_debug() to write to more memory regions
Right now, we only allow for writing to memory regions that allow direct
access using memcpy etc; all other writes are simply ignored. This
implies that debugging guests will not work as expected when writing
to MMIO device regions.
Let's extend cpu_memory_rw_debug() to write to more memory regions,
including MMIO device regions. Reshuffle the condition in
memory_access_is_direct() to make it easier to read and add a comment.
While this change implies that debug access can now also write to MMIO
devices, we now are also permit ELF image loads and similar users of
cpu_memory_rw_debug() to write to MMIO devices; currently we ignore
these writes.
Peter assumes [1] that there's probably a class of guest images, which
will start writing junk (likely zeroes) into device model registers; we
previously would silently ignore any such bogus ELF sections. Likely
these images are of questionable correctness and this can be ignored. If
ever a problem, we could make these cases use address_space_write_rom()
instead, which is left unchanged for now.
This patch is based on previous work by Stefan Zabka.
[1]
https://lore.kernel.org/all/CAFEAcA_2CEJKFyjvbwmpt=on=GgMVamQ5hiiVt+zUr6AY3X=x...@mail.gmail.com/
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/213
Reviewed-by: Peter Xu
Signed-off-by: David Hildenbrand
---
hw/core/cpu-system.c| 13 +
include/exec/memattrs.h | 5 -
include/exec/memory.h | 3 ++-
system/physmem.c| 9 ++---
4 files changed, 17 insertions(+), 13 deletions(-)
diff --git a/hw/core/cpu-system.c b/hw/core/cpu-system.c
index 6aae28a349..6e307c8959 100644
--- a/hw/core/cpu-system.c
+++ b/hw/core/cpu-system.c
@@ -51,13 +51,18 @@ hwaddr cpu_get_phys_page_attrs_debug(CPUState *cpu, vaddr
addr,
MemTxAttrs *attrs)
{
CPUClass *cc = CPU_GET_CLASS(cpu);
+hwaddr paddr;
if (cc->sysemu_ops->get_phys_page_attrs_debug) {
-return cc->sysemu_ops->get_phys_page_attrs_debug(cpu, addr, attrs);
+paddr = cc->sysemu_ops->get_phys_page_attrs_debug(cpu, addr, attrs);
+} else {
+/* Fallback for CPUs which don't implement the _attrs_ hook */
+*attrs = MEMTXATTRS_UNSPECIFIED;
+paddr = cc->sysemu_ops->get_phys_page_debug(cpu, addr);
}
-/* Fallback for CPUs which don't implement the _attrs_ hook */
-*attrs = MEMTXATTRS_UNSPECIFIED;
-return cc->sysemu_ops->get_phys_page_debug(cpu, addr);
+/* Indicate that this is a debug access. */
+attrs->debug = 1;
+return paddr;
}
hwaddr cpu_get_phys_page_debug(CPUState *cpu, vaddr addr)
diff --git a/include/exec/memattrs.h b/include/exec/memattrs.h
index 060b7e7131..8db1d30464 100644
--- a/include/exec/memattrs.h
+++ b/include/exec/memattrs.h
@@ -44,6 +44,8 @@ typedef struct MemTxAttrs {
* (see MEMTX_ACCESS_ERROR).
*/
unsigned int memory:1;
+/* Debug access that can even write to ROM. */
+unsigned int debug:1;
/* Requester ID (for MSI for example) */
unsigned int requester_id:16;
@@ -56,7 +58,8 @@ typedef struct MemTxAttrs {
* Bus masters which don't specify any attributes will get this
* (via the MEMTXATTRS_UNSPECIFIED constant), so that we can
* distinguish "all attributes deliberately clear" from
- * "didn't specify" if necessary.
+ * "didn't specify" if necessary. "debug" can be set alongside
+ * "unspecified".
*/
bool unspecified;
diff --git a/include/exec/memory.h b/include/exec/memory.h
index b18ecf933e..78c4e0aec8 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -3018,7 +3018,8 @@ static inline bool memory_access_is_direct(MemoryRegion
*mr, bool is_write,
if (!memory_region_supports_direct_access(mr)) {
return false;
}
-if (is_write) {
+/* Debug access can write to ROM. */
+if (is_write && !attrs.debug) {
return !mr->readonly && !mr->rom_device;
}
return true;
diff --git a/system/physmem.c b/system/physmem.c
index 9766c6d2e0..486316b651 100644
--- a/system/physmem.c
+++ b/system/physmem.c
@@ -3680,13 +3680,8 @@ int cpu_memory_rw_debug(CPUState *cpu, vaddr addr,
if (l > len)
l = len;
phys_addr += (addr & ~TARGET_PAGE_MASK);
-if (is_write) {
-res = address_space_write_rom(cpu->cpu_ases[asidx].as, phys_addr,
- attrs, buf, l);
-} else {
-res = address_space_read(cpu->cpu_ases[asidx].as, phys_addr,
- attrs, buf, l);
-}
+res = address_space_rw(cpu->cpu_ases[asidx].as, phys_addr, attrs, buf,
+ l, is_write);
if (res != MEMTX_OK) {
return -1;
}
--
2.48.1
[PATCH v3 5/7] memory: pass MemTxAttrs to memory_access_is_direct()
We want to pass another flag that will be stored in MemTxAttrs. So pass
MemTxAttrs directly.
Reviewed-by: Peter Xu
Reviewed-by: Philippe Mathieu-Daudé
Signed-off-by: David Hildenbrand
---
hw/core/loader.c | 2 +-
hw/remote/vfio-user-obj.c | 2 +-
include/exec/memory.h | 5 +++--
system/memory_ldst.c.inc | 18 +-
system/physmem.c | 12 ++--
5 files changed, 20 insertions(+), 19 deletions(-)
diff --git a/hw/core/loader.c b/hw/core/loader.c
index fd25c5e01b..332b879a0b 100644
--- a/hw/core/loader.c
+++ b/hw/core/loader.c
@@ -144,7 +144,7 @@ ssize_t load_image_mr(const char *filename, MemoryRegion
*mr)
{
ssize_t size;
-if (!memory_access_is_direct(mr, false)) {
+if (!memory_access_is_direct(mr, false, MEMTXATTRS_UNSPECIFIED)) {
/* Can only load an image into RAM or ROM */
return -1;
}
diff --git a/hw/remote/vfio-user-obj.c b/hw/remote/vfio-user-obj.c
index 9e5ff6d87a..6e51a92856 100644
--- a/hw/remote/vfio-user-obj.c
+++ b/hw/remote/vfio-user-obj.c
@@ -358,7 +358,7 @@ static int vfu_object_mr_rw(MemoryRegion *mr, uint8_t *buf,
hwaddr offset,
int access_size;
uint64_t val;
-if (memory_access_is_direct(mr, is_write)) {
+if (memory_access_is_direct(mr, is_write, MEMTXATTRS_UNSPECIFIED)) {
/**
* Some devices expose a PCI expansion ROM, which could be buffer
* based as compared to other regions which are primarily based on
diff --git a/include/exec/memory.h b/include/exec/memory.h
index 4e2cf95ab6..b18ecf933e 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -3012,7 +3012,8 @@ static inline bool
memory_region_supports_direct_access(MemoryRegion *mr)
return !memory_region_is_ram_device(mr);
}
-static inline bool memory_access_is_direct(MemoryRegion *mr, bool is_write)
+static inline bool memory_access_is_direct(MemoryRegion *mr, bool is_write,
+ MemTxAttrs attrs)
{
if (!memory_region_supports_direct_access(mr)) {
return false;
@@ -3053,7 +3054,7 @@ MemTxResult address_space_read(AddressSpace *as, hwaddr
addr,
fv = address_space_to_flatview(as);
l = len;
mr = flatview_translate(fv, addr, &addr1, &l, false, attrs);
-if (len == l && memory_access_is_direct(mr, false)) {
+if (len == l && memory_access_is_direct(mr, false, attrs)) {
ptr = qemu_map_ram_ptr(mr->ram_block, addr1);
memcpy(buf, ptr, len);
} else {
diff --git a/system/memory_ldst.c.inc b/system/memory_ldst.c.inc
index 0e6f3940a9..7f32d3d9ff 100644
--- a/system/memory_ldst.c.inc
+++ b/system/memory_ldst.c.inc
@@ -34,7 +34,7 @@ static inline uint32_t glue(address_space_ldl_internal,
SUFFIX)(ARG1_DECL,
RCU_READ_LOCK();
mr = TRANSLATE(addr, &addr1, &l, false, attrs);
-if (l < 4 || !memory_access_is_direct(mr, false)) {
+if (l < 4 || !memory_access_is_direct(mr, false, attrs)) {
release_lock |= prepare_mmio_access(mr);
/* I/O case */
@@ -103,7 +103,7 @@ static inline uint64_t glue(address_space_ldq_internal,
SUFFIX)(ARG1_DECL,
RCU_READ_LOCK();
mr = TRANSLATE(addr, &addr1, &l, false, attrs);
-if (l < 8 || !memory_access_is_direct(mr, false)) {
+if (l < 8 || !memory_access_is_direct(mr, false, attrs)) {
release_lock |= prepare_mmio_access(mr);
/* I/O case */
@@ -170,7 +170,7 @@ uint8_t glue(address_space_ldub, SUFFIX)(ARG1_DECL,
RCU_READ_LOCK();
mr = TRANSLATE(addr, &addr1, &l, false, attrs);
-if (!memory_access_is_direct(mr, false)) {
+if (!memory_access_is_direct(mr, false, attrs)) {
release_lock |= prepare_mmio_access(mr);
/* I/O case */
@@ -207,7 +207,7 @@ static inline uint16_t glue(address_space_lduw_internal,
SUFFIX)(ARG1_DECL,
RCU_READ_LOCK();
mr = TRANSLATE(addr, &addr1, &l, false, attrs);
-if (l < 2 || !memory_access_is_direct(mr, false)) {
+if (l < 2 || !memory_access_is_direct(mr, false, attrs)) {
release_lock |= prepare_mmio_access(mr);
/* I/O case */
@@ -277,7 +277,7 @@ void glue(address_space_stl_notdirty, SUFFIX)(ARG1_DECL,
RCU_READ_LOCK();
mr = TRANSLATE(addr, &addr1, &l, true, attrs);
-if (l < 4 || !memory_access_is_direct(mr, true)) {
+if (l < 4 || !memory_access_is_direct(mr, true, attrs)) {
release_lock |= prepare_mmio_access(mr);
r = memory_region_dispatch_write(mr, addr1, val, MO_32, attrs);
@@ -314,7 +314,7 @@ static inline void glue(address_space_stl_internal,
SUFFIX)(ARG1_DECL,
RCU_READ_LOCK();
mr = TRANSLATE(addr, &addr1, &l, true, attrs);
-if (l < 4 || !memory_access_is_direct(mr, true)) {
+if (l < 4 || !memory_access_is_direct(mr, true, attrs)) {
release_lock |= prepare_mmio_access(mr);
r = memory_region_dispatch_write(mr, addr1, val,
Re: [PATCH 4/5] hw/arm/smmuv3: Move reset to exit phase
Hi Peter, On 2/7/25 7:18 PM, Peter Maydell wrote: > On Fri, 7 Feb 2025 at 17:48, Peter Xu wrote: >> On Fri, Feb 07, 2025 at 04:58:39PM +, Peter Maydell wrote: >>> (I wonder if we ought to suggest quiescing outstanding >>> DMA in the enter phase? But it's probably easier to fix >>> the iommus like this series does than try to get every >>> dma-capable pci device to do something different.) >> I wonder if we should provide some generic helper to register vIOMMU reset >> callbacks, so that we'll be sure any vIOMMU model impl that will register >> at exit() phase only, and do nothing during the initial two phases. Then >> we can put some rich comment on that helper on why. >> >> Looks like it means the qemu reset model in the future can be a combination >> of device tree (which resets depth-first) and the three phases model. We >> will start to use different approach to solve different problems. > The tree of QOM devices (i.e. the one based on the qbus buses > and rooted at the sysbus) resets depth-first, but it does so in > three phases: first we traverse everything doing 'enter'; then > we traverse everything doing 'hold'; then we traverse everything > doing 'exit'. There *used* to be an awkward mix of some things > being three-phase and some not, but we have now got rid of all > of those so a system reset does a single three-phase reset run > which resets everything. Thank you Peter. This is reassuring. I will add such kind of description in the commit msg/cover letter. Eric > > -- PMM >
[PATCH v3 6/7] hmp: use cpu_get_phys_page_debug() in hmp_gva2gpa()
We don't need the MemTxAttrs, so let's simply use the simpler function
variant.
Reviewed-by: Peter Xu
Reviewed-by: Philippe Mathieu-Daudé
Signed-off-by: David Hildenbrand
---
monitor/hmp-cmds-target.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/monitor/hmp-cmds-target.c b/monitor/hmp-cmds-target.c
index 0300faa8a2..0d2e9dce69 100644
--- a/monitor/hmp-cmds-target.c
+++ b/monitor/hmp-cmds-target.c
@@ -301,7 +301,6 @@ void hmp_gpa2hva(Monitor *mon, const QDict *qdict)
void hmp_gva2gpa(Monitor *mon, const QDict *qdict)
{
target_ulong addr = qdict_get_int(qdict, "addr");
-MemTxAttrs attrs;
CPUState *cs = mon_get_cpu(mon);
hwaddr gpa;
@@ -310,7 +309,7 @@ void hmp_gva2gpa(Monitor *mon, const QDict *qdict)
return;
}
-gpa = cpu_get_phys_page_attrs_debug(cs, addr & TARGET_PAGE_MASK, &attrs);
+gpa = cpu_get_phys_page_debug(cs, addr & TARGET_PAGE_MASK);
if (gpa == -1) {
monitor_printf(mon, "Unmapped\n");
} else {
--
2.48.1
[PATCH v3 2/7] physmem: factor out RAM/ROMD check in memory_access_is_direct()
Let's factor more of the generic "is this directly accessible" check,
independent of the "write" condition out.
Note that the "!mr->rom_device" check in the write case essentially
disallows the memory_region_is_romd() condition again. Further note that
RAM DEVICE regions are also RAM regions, so we can check for RAM+ROMD
first.
This is a preparation for further changes.
Reviewed-by: Peter Xu
Signed-off-by: David Hildenbrand
---
include/exec/memory.h | 10 ++
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/include/exec/memory.h b/include/exec/memory.h
index 5cd7574c60..cb35c38402 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -2997,6 +2997,10 @@ bool prepare_mmio_access(MemoryRegion *mr);
static inline bool memory_access_is_direct(MemoryRegion *mr, bool is_write)
{
+/* ROM DEVICE regions only allow direct access if in ROMD mode. */
+if (!memory_region_is_ram(mr) && !memory_region_is_romd(mr)) {
+return false;
+}
/*
* RAM DEVICE regions can be accessed directly using memcpy, but it might
* be MMIO and access using mempy can be wrong (e.g., using instructions
not
@@ -3006,11 +3010,9 @@ static inline bool memory_access_is_direct(MemoryRegion
*mr, bool is_write)
return false;
}
if (is_write) {
-return memory_region_is_ram(mr) && !mr->readonly &&
- !mr->rom_device;
-} else {
-return memory_region_is_ram(mr) || memory_region_is_romd(mr);
+return !mr->readonly && !mr->rom_device;
}
+return true;
}
/**
--
2.48.1
Re: [PATCH 0/5] Fix vIOMMU reset order
Hi Cédric, On 2/7/25 6:25 PM, Cédric Le Goater wrote: > On 2/7/25 17:54, Peter Xu wrote: >> On Thu, Feb 06, 2025 at 03:21:51PM +0100, Eric Auger wrote: >>> This is a follow-up of Peter's attempt to fix the fact that >>> vIOMMUs are likely to be reset before the device they protect: >>> >>> [PATCH 0/4] intel_iommu: Reset vIOMMU after all the rest of devices >>> https://lore.kernel.org/all/20240117091559.144730-1-pet...@redhat.com/ >>> >>> This is especially observed with virtio devices when a qmp system_reset >>> command is sent but also with VFIO devices. >>> >>> This series puts the vIOMMU reset in the 3-phase exit callback. >>> >>> This scheme was tested successful with virtio-devices and some >>> VFIO devices. Nevertheless not all the topologies have been >>> tested yet. >> >> Eric, >> >> It's great to know that we seem to be able to fix everything in such >> small >> changeset! >> >> I would like to double check two things with you here: >> >> - For VFIO's reset hook, looks like we have landed more changes so >> that >> vfio's reset function is now a TYPE_LEGACY_RESET, and it always >> do the >> reset during "hold" phase only (via legacy_reset_hold()). That >> part >> will make sure vIOMMU (if switching to exit()-only reset) will >> order >> properly with VFIO. Is my understanding correct here? > > > Eric, > > We were still seeing DMA errors from VFIO devices : > > VFIO_MAP_DMA failed: Bad address > > with this series at shutdown (machine or OS) when using an intel_iommu > device. We could see that the VIOMMU was reset and the device DMAs > were still alive. Do you know why now ? I have started debugging this other case. At first sight this looks like a different problem. First this occurs on a qmp system_powerdown The error messages do not occur on qemu reset but rather as a result of the guest disabling the intel iommu anc curiously when the aliased IOMMU MR (nodma) is re-enabled. I need more time to debug this. Eric > > Thanks, > > C. > > >> >> - Is it possible if some PCIe devices that will provide its own >> phase.exit(), would it matter on the order of PCIe device's >> phase.exit() and vIOMMU's phase.exit() (if vIOMMUs switch to use >> exit()-only approach like this one)? >> >> PS: it would be great to attach such information in either cover >> letter or >> commit message. But definitely not a request to repost the patchset, if >> Michael would have Message-ID when merge that'll be far enough to help >> anyone find this discussion again. >> >> Thanks! >> >
Re: [PATCH v4 4/4] qapi: expose all schema features to code
John Snow writes: > On Fri, Feb 7, 2025, 6:57 AM Markus Armbruster wrote: > >> Daniel P. Berrangé writes: >> >> > This replaces use of the constants from the QapiSpecialFeatures >> > enum, with constants from the auto-generate QapiFeatures enum >> > in qapi-features.h >> > >> > The 'deprecated' and 'unstable' features still have a little bit of >> > special handling, being force defined to be the 1st + 2nd features >> > in the enum, regardless of whether they're used in the schema. This >> > retains compatibility with common code that references the features >> > via the QapiSpecialFeatures constants. >> > >> > Signed-off-by: Daniel P. Berrangé >> >> Daniel, feel free to ignore this at least for now. I'm trying to learn >> some typing lore from John. >> >> v3 made mypy unhappy. I asked John for advice, and also posted a >> solution involving ValuesView I hacked up myself. Daniel took it for >> v4. >> >> John suggested to use List. >> >> I now wonder whether could use Iterable. >> >> I'll show the three solutions inline. >> >> John, thoughts? >> > > ValuesView works just fine. It accurately describes what that function > returns. I only avoided it in my fixup because it's a more obscure type and > generally list is easier to work with as a first-class built in primitive > type to the language. > > (read as: I didn't have to consult any docs to fix it up using List and I'm > lazy.) Aside: John later shared a useful technique on IRC: "you can write reveal_type(foo) to get mypy to spill the beans on what it thinks". > Your solution describes precisely the type being returned (always good) and > avoids any re-copying of data. > > Do be aware by caching the values view object in another object that you > are keeping a "live reference" to the list of dict values that I think can > change if the source dict changes. Yes. >I doubt it matters, but you should know > about that. I believe it's just fine. > The only design consideration you have now is what type you actually want > to return and why. I think it barely matters, and I'm always going to opt > for whatever is the least annoying for the patch author so I don't have to > bore/torture them with python minutiae. Since the typing in Daniel's patch is fine, I'll refrain from messing with it. > As long as the tests pass (my first three patches in the dan-fixup branch I > posted based on v3) I'm more than content. Thanks!
Re: [PATCH v4 0/4] qapi: generalize special features
Daniel P. Berrangé writes: > This series is a spin-off from > > https://lists.nongnu.org/archive/html/qemu-devel/2024-06/msg00807.html > > That series introduced a pragma allowing a schema to declare extra > features that would be exposed to code. > > Following Markus' suggestion: > > https://lists.nongnu.org/archive/html/qemu-devel/2024-07/msg03765.html > > I've changed impl such that we expose all features to the code > regardless of whether they are special, and don't require any pragma. > > I've split it from the QGA patches since it makes more sense to work > on this bit in isolation. Series Reviewed-by: Markus Armbruster
Re: [PATCH 4/5] hw/arm/smmuv3: Move reset to exit phase
Hi Peter, On 2/7/25 6:47 PM, Peter Xu wrote: > On Fri, Feb 07, 2025 at 04:58:39PM +, Peter Maydell wrote: >> (I wonder if we ought to suggest quiescing outstanding >> DMA in the enter phase? But it's probably easier to fix >> the iommus like this series does than try to get every >> dma-capable pci device to do something different.) > I wonder if we should provide some generic helper to register vIOMMU reset > callbacks, so that we'll be sure any vIOMMU model impl that will register > at exit() phase only, and do nothing during the initial two phases. Then > we can put some rich comment on that helper on why. As discussed with Cédric, I think it shall think about having eventually a base class for vIOMMU. Maybe this is something we can handle afterwards though. > > Looks like it means the qemu reset model in the future can be a combination > of device tree (which resets depth-first) and the three phases model. We > will start to use different approach to solve different problems. > > Maybe after we settle our mind, we should update the reset document, > e.g. for device emulation developers, we need to be clear on where to > quiesce the DMAs, and it must not happen at exit(). Both all devices and > all iommu impls need to follow the rules to make it work like the plan. The 3 phase documentation already states that you shouldn't do anything in enter phase that can have side-effect on other devices. However I agree we can add another example besides the qemu_irq line one. Eric > > Thanks, >
Re: [RFC] target/i386: sev: Add cmdline option to enable the Allowed SEV Features feature
On Fri, Feb 07, 2025 at 05:33:27PM -0600, Kim Phillips wrote:
> The Allowed SEV Features feature allows the host kernel to control
> which SEV features it does not want the guest to enable [1].
>
> This has to be explicitly opted-in by the user because it has the
> ability to break existing VMs if it were set automatically.
>
> Currently, both the PmcVirtualization and SecureAvic features
> require the Allowed SEV Features feature to be set.
>
> Based on a similar patch written for Secure TSC [2].
>
> [1] Section 15.36.20 "Allowed SEV Features", AMD64 Architecture
> Programmer's Manual, Pub. 24593 Rev. 3.42 - March 2024:
> https://bugzilla.kernel.org/attachment.cgi?id=306250
>
> [2]
> https://github.com/qemu/qemu/commit/4b2288dc6025ba32519ee8d202ca72d565cbbab7
Despite that URL, that commit also does not appear to be merged into
the QEMU git repo, and indeed I can't find any record of it even being
posted as a patch for review on qemu-devel.
This is horribly misleading to reviewers, suggesting that the referenced
patch was already accepted :-(
>
> Signed-off-by: Kim Phillips
> ---
> qapi/qom.json | 6 -
> target/i386/sev.c | 60 +++
> target/i386/sev.h | 2 ++
> 3 files changed, 67 insertions(+), 1 deletion(-)
>
> diff --git a/qapi/qom.json b/qapi/qom.json
> index 28ce24cd8d..113b44ad74 100644
> --- a/qapi/qom.json
> +++ b/qapi/qom.json
> @@ -948,13 +948,17 @@
> # designated guest firmware page for measured boot with -kernel
> # (default: false) (since 6.2)
> #
> +# @allowed-sev-features: true if secure allowed-sev-features feature
> +# is to be enabled in an SEV-ES or SNP guest. (default: false)
Missing 'since' annotation.
> +#
> # Since: 9.1
> ##
> { 'struct': 'SevCommonProperties',
>'data': { '*sev-device': 'str',
> '*cbitpos': 'uint32',
> 'reduced-phys-bits': 'uint32',
> -'*kernel-hashes': 'bool' } }
> +'*kernel-hashes': 'bool',
> +'*allowed-sev-features': 'bool' } }
>
> ##
> # @SevGuestProperties:
> diff --git a/target/i386/sev.c b/target/i386/sev.c
> index 0e1dbb6959..85ad73f9a0 100644
> --- a/target/i386/sev.c
> +++ b/target/i386/sev.c
> @@ -98,6 +98,7 @@ struct SevCommonState {
> uint32_t cbitpos;
> uint32_t reduced_phys_bits;
> bool kernel_hashes;
> +uint64_t vmsa_features;
>
> /* runtime state */
> uint8_t api_major;
> @@ -411,6 +412,33 @@ sev_get_reduced_phys_bits(void)
> return sev_common ? sev_common->reduced_phys_bits : 0;
> }
>
> +static __u64
> +sev_supported_vmsa_features(void)
> +{
> +uint64_t supported_vmsa_features = 0;
> +struct kvm_device_attr attr = {
> +.group = KVM_X86_GRP_SEV,
> +.attr = KVM_X86_SEV_VMSA_FEATURES,
> +.addr = (unsigned long) &supported_vmsa_features
> +};
> +
> +bool sys_attr = kvm_check_extension(kvm_state, KVM_CAP_SYS_ATTRIBUTES);
> +if (!sys_attr) {
> +return 0;
> +}
> +
> +int rc = kvm_ioctl(kvm_state, KVM_GET_DEVICE_ATTR, &attr);
> +if (rc < 0) {
> +if (rc != -ENXIO) {
> +warn_report("KVM_GET_DEVICE_ATTR(0, KVM_X86_SEV_VMSA_FEATURES) "
> +"error: %d", rc);
> +}
> +return 0;
> +}
> +
> +return supported_vmsa_features;
> +}
> +
> static SevInfo *sev_get_info(void)
> {
> SevInfo *info;
> @@ -1524,6 +1552,20 @@ static int
> sev_common_kvm_init(ConfidentialGuestSupport *cgs, Error **errp)
> case KVM_X86_SNP_VM: {
> struct kvm_sev_init args = { 0 };
>
> +if (sev_es_enabled()) {
> +__u64 vmsa_features, supported_vmsa_features;
> +
> +supported_vmsa_features = sev_supported_vmsa_features();
> +vmsa_features = sev_common->vmsa_features;
> +if ((vmsa_features & supported_vmsa_features) != vmsa_features) {
> +error_setg(errp, "%s: requested sev feature mask (0x%llx) "
> + "contains bits not supported by the host kernel "
> + " (0x%llx)", __func__, vmsa_features,
> + supported_vmsa_features);
This logic is being applied unconditionally, and not connected to
the setting of the new 'allowed-sev-features' flag value. Is that
correct ?
Will this end up breaking existing deployed guests, or is this a
scenario that would have been blocked with an error later on
regardless ?
> +return -1;
Malformed indentation.
> +}
> +args.vmsa_features = vmsa_features;
> +}
> ret = sev_ioctl(sev_common->sev_fd, KVM_SEV_INIT2, &args, &fw_error);
> break;
> }
> @@ -2044,6 +2086,19 @@ static void sev_common_set_kernel_hashes(Object *obj,
> bool value, Error **errp)
> SEV_COMMON(obj)->kernel_hashes = value;
> }
>
> +static bool
> +sev_snp_guest_get_allowed_sev_features(Object *obj, Error **errp)
> +{
> +return SE
Re: [PATCH 2/7] target/i386/kvm: introduce 'pmu-cap-disabled' to set KVM_PMU_CAP_DISABLE
On 2/10/2025 4:12 AM, dongli.zh...@oracle.com wrote: > Hi Dapeng, > > On 2/7/25 1:52 AM, Mi, Dapeng wrote: >> On 11/21/2024 6:06 PM, Mi, Dapeng wrote: >>> On 11/8/2024 7:44 AM, dongli.zh...@oracle.com wrote: Hi Zhao, On 11/6/24 11:52 PM, Zhao Liu wrote: > (+Dapang & Zide) > > Hi Dongli, > > On Mon, Nov 04, 2024 at 01:40:17AM -0800, Dongli Zhang wrote: >> Date: Mon, 4 Nov 2024 01:40:17 -0800 >> From: Dongli Zhang >> Subject: [PATCH 2/7] target/i386/kvm: introduce 'pmu-cap-disabled' to set >> KVM_PMU_CAP_DISABLE >> X-Mailer: git-send-email 2.43.5 >> >> The AMD PMU virtualization is not disabled when configuring >> "-cpu host,-pmu" in the QEMU command line on an AMD server. Neither >> "-cpu host,-pmu" nor "-cpu EPYC" effectively disables AMD PMU >> virtualization in such an environment. >> >> As a result, VM logs typically show: >> >> [0.510611] Performance Events: Fam17h+ core perfctr, AMD PMU driver. >> >> whereas the expected logs should be: >> >> [0.596381] Performance Events: PMU not available due to >> virtualization, using software events only. >> [0.600972] NMI watchdog: Perf NMI watchdog permanently disabled >> >> This discrepancy occurs because AMD PMU does not use CPUID to determine >> whether PMU virtualization is supported. > Intel platform doesn't have this issue since Linux kernel fails to check > the CPU family & model when "-cpu *,-pmu" option clears PMU version. > > The difference between Intel and AMD platforms, however, is that it seems > Intel hardly ever reaches the “...due virtualization” message, but > instead reports an error because it recognizes a mismatched family/model. > > This may be a drawback of the PMU driver's print message, but the result > is the same, it prevents the PMU driver from enabling. > > So, please mention that KVM_PMU_CAP_DISABLE doesn't change the PMU > behavior on Intel platform because current "pmu" property works as > expected. Sure. I will mention this in v2. >> To address this, we introduce a new property, 'pmu-cap-disabled', for KVM >> acceleration. This property sets KVM_PMU_CAP_DISABLE if >> KVM_CAP_PMU_CAPABILITY is supported. Note that this feature currently >> supports only x86 hosts, as KVM_CAP_PMU_CAPABILITY is used exclusively >> for >> x86 systems. >> >> Signed-off-by: Dongli Zhang >> --- >> Another previous solution to re-use '-cpu host,-pmu': >> https://urldefense.com/v3/__https://lore.kernel.org/all/20221119122901.2469-1-dongli.zh...@oracle.com/__;!!ACWV5N9M2RV99hQ!Nm8Db-mwBoMIwKkRqzC9kgNi5uZ7SCIf43zUBn92Ar_NEbLXq-ZkrDDvpvDQ4cnS2i4VyKAp6CRVE12bRkMF$ >> > IMO, I prefer the previous version. This VM-level KVM property is > difficult to integrate with the existing CPU properties. Pls refer later > comments for reasons. > >> accel/kvm/kvm-all.c| 1 + >> include/sysemu/kvm_int.h | 1 + >> qemu-options.hx| 9 ++- >> target/i386/cpu.c | 2 +- >> target/i386/kvm/kvm.c | 52 ++ >> target/i386/kvm/kvm_i386.h | 2 ++ >> 6 files changed, 65 insertions(+), 2 deletions(-) >> >> diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c >> index 801cff16a5..8b5ba45cf7 100644 >> --- a/accel/kvm/kvm-all.c >> +++ b/accel/kvm/kvm-all.c >> @@ -3933,6 +3933,7 @@ static void kvm_accel_instance_init(Object *obj) >> s->xen_evtchn_max_pirq = 256; >> s->device = NULL; >> s->msr_energy.enable = false; >> +s->pmu_cap_disabled = false; >> } > The CPU property "pmu" also defaults to "false"...but: > > * max CPU would override this and try to enable PMU by default in >max_x86_cpu_initfn(). > > * Other named CPU models keep the default setting to avoid affecting >the migration. > > The pmu_cap_disabled and “pmu” property look unbound and unassociated, > so this can cause the conflict when they are not synchronized. For > example, > > -cpu host -accel kvm,pmu-cap-disabled=on > > The above options will fail to launch a VM (on Intel platform). > > Ideally, the “pmu” property and pmu-cap-disabled should be bound to each > other and be consistent. But it's not easy because: > - There is no proper way to have pmu_cap_disabled set different default >values (e.g., "false" for max CPU and "true" for named CPU models) >based on different CPU models. > - And, no proper place to check the consistency of pmu_cap_disabled and >enable_pmu. > > Therefore, I prefer your previous approach, to reuse current CPU "pmu" > property. Thank you very much for the suggestion and reasons. I am going to follow your sugges
Re: [RFC PATCH v3 4/8] io: Add flags argument to qio_channel_readv_full_all_eof
On Fri, Feb 07, 2025 at 04:53:55PM -0300, Fabiano Rosas wrote: > We want to pass flags into qio_channel_tls_readv() but > qio_channel_readv_full_all_eof() doesn't take a flags argument. > > No functional change. > > Signed-off-by: Fabiano Rosas > --- > hw/remote/mpqemu-link.c | 2 +- > include/io/channel.h| 2 ++ > io/channel.c| 9 ++--- > 3 files changed, 9 insertions(+), 4 deletions(-) Reviewed-by: Daniel P. Berrangé Acked-by: Daniel P. Berrangé With regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [PATCH v3] hw/net: cadence_gem: feat: add logic for the DISABLE_MASK bit in type2_compare_x_word_1
On 4/2/25 15:37, Peter Maydell wrote:
On Thu, 30 Jan 2025 at 22:31, Edgar E. Iglesias
wrote:
On Mon, Jan 27, 2025 at 8:40 AM Peter Maydell wrote:
On Thu, 19 Dec 2024 at 06:17, Andrew.Yuan wrote:
-rx_cmp = rxbuf_ptr[offset] << 8 | rxbuf_ptr[offset];
-mask = FIELD_EX32(cr0, TYPE2_COMPARE_0_WORD_0, MASK_VALUE);
-compare = FIELD_EX32(cr0, TYPE2_COMPARE_0_WORD_0, COMPARE_VALUE);
+disable_mask =
+FIELD_EX32(cr1, TYPE2_COMPARE_0_WORD_1, DISABLE_MASK);
+if (disable_mask) {
+/*
+ * If disable_mask is set,
+ * mask_value is used as an additional 2 byte Compare Value.
+ * To simple, set mask = 0x, if disable_mask is set.
+ */
+rx_cmp = ldl_le_p(rxbuf_ptr + offset);
+mask = 0x;
+compare = cr0;
+} else {
+rx_cmp = lduw_le_p(rxbuf_ptr + offset);
Is the change in behaviour in the !disable_mask codepath here
intentional? Previously we use one byte from rxbuf_ptr[offset],
duplicated into both halves of rx_cmp; now we will load two
different bytes from rxbuf_ptr[offset] and rxbuf_ptr[offset + 1].
If this is intended, we should say so in the commit message.
I agree that it should be mentioned (looks like a correct bugfix).
Thanks. I've expanded the commit message:
hw/net/cadence_gem: Fix the mask/compare/disable-mask logic
Our current handling of the mask/compare logic in the Cadence
GEM ethernet device is wrong:
(1) we load the same byte twice from rx_buf when
creating the compare value
(2) we ignore the DISABLE_MASK flag
The "Cadence IP for Gigabit Ethernet MAC Part Number: IP7014 IP Rev:
R1p12 - Doc Rev: 1.3 User Guide" states that if the DISABLE_MASK bit
in type2_compare_x_word_1 is set, the mask_value field in
type2_compare_x_word_0 is used as an additional 2 byte Compare Value.
Correct these bugs:
* in the !disable_mask codepath, use lduw_le_p() so we
correctly load a 16-bit value for comparison
* in the disable_mask codepath, we load a full 4-byte value
from rx_buf for the comparison, set the compare value to
the whole of the cr0 register (i.e. the concatenation of
the mask and compare fields), and set mask to 0x
to force a 32-bit comparison
and also tweaked the comment a bit:
+/*
+ * If disable_mask is set, mask_value is used as an
+ * additional 2 byte Compare Value; that is equivalent
+ * to using the whole cr0 register as the comparison value.
+ * Load 32 bits of data from rx_buf, and set mask to
+ * all-ones so we compare all 32 bits.
+ */
and applied this to target-arm.next.
Other than that this patch looks good to me!
Can I call that a Reviewed-by (with the above changes)?
Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH v2 0/7] hw/loongarch/virt: CPU irq routing enhancement
Cc'ing Igor for vCPU hotplugging expertise. On 10/2/25 10:36, Bibo Mao wrote: Interrupt controller ipi and extioi on LoongArch system can send intterrupt to multiple CPUs, physical cpu id is used to route interrupt for CPUs. With cpu hotplug feature in future, notification with ipi and extioi interrupt controller is required. Since there is common Notifier API for CPU hotplug, cpu hotplug interface is added on ipi and extioi class for notification usage. With CPU hotplug event notfication, gpio irq line is connected to cpu irq line, and irq routing for irqchip is setup. --- v1 .. v2: 1. Combine patchset ipi and extioi irq routing enhancement together 2. Rebase patch based on latest version --- Bibo Mao (7): hw/intc/loongarch_ipi: Add basic hotplug framework hw/intc/loongarch_ipi: Implment cpu hotplug interface hw/intc/loongarch_ipi: Notify ipi object when cpu is plugged hw/intc/loongarch_extioi: Move gpio irq initial to common code hw/intc/loongarch_extioi: Add basic hotplug framework hw/intc/loongarch_extioi: Implment cpu hotplug interface hw/intc/loongarch_extioi: Use cpu plug notification hw/intc/loongarch_extioi.c| 8 +-- hw/intc/loongarch_extioi_common.c | 84 ++- hw/intc/loongarch_ipi.c | 71 ++ hw/loongarch/virt.c | 17 ++- 4 files changed, 159 insertions(+), 21 deletions(-)
Re: [PATCH 3/6] hw/mips/boston: Check for error return from boston_fdt_filter()
On 6/2/25 16:12, Peter Maydell wrote: The function boston_fdt_filter() can return NULL on errors (in which case it will print an error message). When we call this from the non-FIT-image codepath, we aren't checking the return value, so we will plough on with a NULL pointer, and segfault in fdt_totalsize(). Check for errors here. Signed-off-by: Peter Maydell --- hw/mips/boston.c | 4 1 file changed, 4 insertions(+) Reviewed-by: Philippe Mathieu-Daudé
vtables and procedural macros (was Re: [PATCH] rust: pl011: convert pl011_create to safe Rust)
On 2/10/25 10:59, Zhao Liu wrote:
On Thu, Feb 06, 2025 at 12:15:14PM +0100, Paolo Bonzini wrote:
Date: Thu, 6 Feb 2025 12:15:14 +0100
From: Paolo Bonzini
Subject: [PATCH] rust: pl011: convert pl011_create to safe Rust
X-Mailer: git-send-email 2.48.1
Not a major change but, as a small but significant step in creating
qdev bindings, show how pl011_create can be written without "unsafe"
calls (apart from converting pointers to references).
This also provides a starting point for creating Error** bindings.
Signed-off-by: Paolo Bonzini
---
rust/hw/char/pl011/src/device.rs | 39
rust/qemu-api/src/sysbus.rs | 34 +---
2 files changed, 50 insertions(+), 23 deletions(-)
...
+fn realize(&self) {
What about renaming this as "realize_with_sysbus"?
Elsewhere, the device's own realize method is often used to set
DeviceImpl::REALIZE.
I *think* this is not a problem in practice because trait methods are
public (if the trait is in scope) whereas the device's realize method if
private.
I agree that the naming conflict is unfortunate though, if only because
it can cause confusion. I don't know if this can be solved by
procedural macros; for example a #[vtable] attribute that changes
#[qemu_api_macros::vtable]
fn realize(...) {
}
into
const fn REALIZE: ... = Some({
fn realize(...) {
}
realize
}
This way, the REALIZE item would be included in the "impl DeviceImpl for
PL011State" block instead of "impl PL011State". It's always a fine line
between procedural macros cleaning vs. messing things up, which is why
until now I wanted to see what things look like with pure Rust code; but
I guess now it's the time to evaluate this kind of thing.
Adding Junjie since he contributed the initial proc macro infrastructure
and may have opinions on this.
Paolo
Re: [PATCH 4/6] hw/mips/boston: Support dumpdtb monitor commands
Hi Peter,
On 6/2/25 16:12, Peter Maydell wrote:
The boston machine doesn't set MachineState::fdt to the DTB blob that
it has loaded or created, which means that the QMP/HMP dumpdtb
monitor commands don't work.
Setting MachineState::fdt is easy in the non-FIT codepath: we can
simply do so immediately before loading the DTB into guest memory.
The FIT codepath is a bit more awkward as currently the FIT loader
throws away the memory that the FDT was in after it loads it into
guest memory. So we add a void *pfdt argument to load_fit() for it
to store the FDT pointer into.
There is some readjustment required of the pointer handling in
loader-fit.c, so that it applies 'const' only where it should (e.g.
the data pointer we get back from fdt_getprop() is const, because
it's into the middle of the input FDT data, but the pointer that
fit_load_image_alloc() should not be const, because it's freshly
allocated memory that the caller can change if it likes).
Signed-off-by: Peter Maydell
---
include/hw/loader-fit.h | 21 ++---
hw/core/loader-fit.c| 38 +-
hw/mips/boston.c| 11 +++
3 files changed, 46 insertions(+), 24 deletions(-)
diff --git a/include/hw/loader-fit.h b/include/hw/loader-fit.h
index 0832e379dc9..9a43490ed63 100644
--- a/include/hw/loader-fit.h
+++ b/include/hw/loader-fit.h
@@ -30,12 +30,27 @@ struct fit_loader_match {
struct fit_loader {
const struct fit_loader_match *matches;
hwaddr (*addr_to_phys)(void *opaque, uint64_t addr);
-const void *(*fdt_filter)(void *opaque, const void *fdt,
- const void *match_data, hwaddr *load_addr);
+void *(*fdt_filter)(void *opaque, const void *fdt,
+const void *match_data, hwaddr *load_addr);
const void *(*kernel_filter)(void *opaque, const void *kernel,
hwaddr *load_addr, hwaddr *entry_addr);
};
-int load_fit(const struct fit_loader *ldr, const char *filename, void *opaque);
+/**
+ * load_fit: load a FIT format image
+ * @ldr: structure defining board specific properties and hooks
+ * @filename: image to load
+ * @pfdt: pointer to update with address of FDT blob
It is not clear this field is optional or mandatory.
Looking at other docstrings, optional is not always precised,
and code often consider optional if not precised. Mandatory is
mentioned explicitly.
+ * @opaque: opaque value passed back to the hook functions in @ldr
+ * Returns: 0 on success, or a negative errno on failure
+ *
+ * @pfdt is used to tell the caller about the FDT blob. On return, it
+ * has been set to point to the FDT blob, and it is now the caller's
+ * responsibility to free that memory with g_free(). Usually the caller
+ * will want to pass in &machine->fdt here, to record the FDT blob for
+ * the dumpdtb option and QMP/HMP commands.
+ */
+int load_fit(const struct fit_loader *ldr, const char *filename, void **pfdt,
+ void *opaque);
static int fit_load_fdt(const struct fit_loader *ldr, const void *itb,
int cfg, void *opaque, const void *match_data,
-hwaddr kernel_end, Error **errp)
+hwaddr kernel_end, void **pfdt, Error **errp)
{
ERRP_GUARD();
Error *err = NULL;
const char *name;
-const void *data;
-const void *load_data;
+void *data;
hwaddr load_addr;
int img_off;
size_t sz;
@@ -194,7 +193,7 @@ static int fit_load_fdt(const struct fit_loader *ldr, const
void *itb,
return 0;
}
-load_data = data = fit_load_image_alloc(itb, name, &img_off, &sz, errp);
+data = fit_load_image_alloc(itb, name, &img_off, &sz, errp);
if (!data) {
error_prepend(errp, "unable to load FDT image from FIT: ");
return -EINVAL;
@@ -211,19 +210,23 @@ static int fit_load_fdt(const struct fit_loader *ldr,
const void *itb,
}
if (ldr->fdt_filter) {
-load_data = ldr->fdt_filter(opaque, data, match_data, &load_addr);
+void *filtered_data;
+
+filtered_data = ldr->fdt_filter(opaque, data, match_data, &load_addr);
+if (filtered_data != data) {
+g_free(data);
+data = filtered_data;
+}
}
load_addr = ldr->addr_to_phys(opaque, load_addr);
-sz = fdt_totalsize(load_data);
-rom_add_blob_fixed(name, load_data, sz, load_addr);
+sz = fdt_totalsize(data);
+rom_add_blob_fixed(name, data, sz, load_addr);
-ret = 0;
+*pfdt = data;
Here pfdt is assumed to be non-NULL, so a mandatory field.
Could you update the documentation? Otherwise consider adding
a non-NULL check.
Either ways:
Reviewed-by: Philippe Mathieu-Daudé
+return 0;
out:
g_free((void *) data);
-if (data != load_data) {
-g_free((void *) load_data);
-}
return ret;
}
@@ -259,7 +262,8 @@ out:
return ret;
}
Re: [PATCH 1/6] monitor/hmp-cmds.c: Clean up hmp_dumpdtb printf
On 6/2/25 16:12, Peter Maydell wrote: In hmp_dumpdtb(), we print a message when the command succeeds. This message is missing the trailing \n, so the HMP command prompt is printed immediately after it. We also weren't capitalizing 'DTB', or quoting the filename in the message. Fix these nits. Signed-off-by: Peter Maydell --- monitor/hmp-cmds.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH 2/6] hw/openrisc: Support monitor dumpdtb command
On 6/2/25 16:12, Peter Maydell wrote: The openrisc machines don't set MachineState::fdt to point to their DTB blob. This means that although the command line '-machine dumpdtb=file.dtb' option works, the equivalent QMP and HMP monitor commands do not, but instead produce the error "This machine doesn't have a FDT". Set MachineState::fdt in openrisc_load_fdt(), when we write it to guest memory. Signed-off-by: Peter Maydell --- include/hw/openrisc/boot.h | 3 ++- hw/openrisc/boot.c | 7 +-- hw/openrisc/openrisc_sim.c | 2 +- hw/openrisc/virt.c | 2 +- 4 files changed, 9 insertions(+), 5 deletions(-) Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH 03/10] target/xtensa: Finalize config in xtensa_register_core()
On Mon, Feb 10, 2025 at 2:26 AM Philippe Mathieu-Daudé
wrote:
>
> Only modify XtensaConfig within xtensa_register_core(),
> when the class is registered, not when it is initialized.
>
> Signed-off-by: Philippe Mathieu-Daudé
> ---
> Cc: Max Filippov
> ---
> target/xtensa/helper.c | 8 +---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/target/xtensa/helper.c b/target/xtensa/helper.c
> index 2978c471c1f..c4735989714 100644
> --- a/target/xtensa/helper.c
> +++ b/target/xtensa/helper.c
> @@ -173,9 +173,8 @@ static void xtensa_core_class_init(ObjectClass *oc, void
> *data)
> {
> CPUClass *cc = CPU_CLASS(oc);
> XtensaCPUClass *xcc = XTENSA_CPU_CLASS(oc);
> -XtensaConfig *config = data;
> +const XtensaConfig *config = data;
>
> -xtensa_finalize_config(config);
It was here to only do potentially expensive finalization once for the
actually used core, but I guess there's nothing that expensive there.
> xcc->config = config;
>
> /*
> @@ -189,12 +188,15 @@ static void xtensa_core_class_init(ObjectClass *oc,
> void *data)
>
> void xtensa_register_core(XtensaConfigList *node)
> {
> +XtensaConfig *config = g_memdup2(node->config, sizeof(config));
The structures pointed to by the node->config are not const, I'm not sure
why the pointer is const. I'd say that rather than making a copy here the
XtensaConfigList should lose the const qualifier in the config definition.
> TypeInfo type = {
> .parent = TYPE_XTENSA_CPU,
> .class_init = xtensa_core_class_init,
> -.class_data = (void *)node->config,
> +.class_data = config,
> };
>
> +xtensa_finalize_config(config);
> +
> node->next = xtensa_cores;
> xtensa_cores = node;
> type.name = g_strdup_printf(XTENSA_CPU_TYPE_NAME("%s"),
> node->config->name);
--
Thanks.
-- Max
Re: [PATCH RFC 0/4] hvf: use TCG emulation to handle data aborts
On Sun, 9 Feb 2025 at 03:33, Joelle van Dyne wrote: > > When the VM exits with an data abort, we check the ISV field in the ESR and > when > ISV=1, that means the processor has filled the remaining fields with > information > needed to determine the access that caused the abort: address, access width, > and > the register operand. However, only a limited set of instructions which can > cause a data abort is nice enough for the processor to decode this way. Many > instructions such as LDP/STP and SIMD can cause an data abort with ISV=0 and > for > that the hypervisor needs to manually decode the instruction, find the > operands, > and emulate the access. > > QEMU already ships with the ability to do this: TCG. However, TCG currently > operates as a stand-alone accelerator. This patch set enables HVF to call into > TCG when needed in order to perform a memory access that caused the abort. So one problem with this is that it immediately puts all of TCG onto the security boundary with the VM. We don't claim any kind of security or can't-escape guarantees for TCG, and it's a lot of code, some of which is old and some of which wasn't written with security as a top-of-mind concern. Our approach to these instructions with KVM on Arm is to say "don't do those in the guest to MMIO regions". Most sensible guest code doesn't do weird instruction forms for device accesses, and the performance is going to be bad anyway if you need to fully emulate them. (This includes in the past that Windows got fixed to not do this kind of insn to a device in at least one case.) > One thing this enables is the ability to use virtio-vga with Windows for > ARM64. > Currently, graphics support for Windows is flakey because you must first boot > with ramfb to get to the desktop where you can then install the virtio-gpu > drivers and then start up with virtio-gpu. Even then, there is a known issue > where Windows mistakingly thinks there are two monitors connected because the > boot display does not share a framebuffer with the GPU. This results in > sometimes a black screen when updating Windows. It's not really a good idea to use virtio-vga in an Arm VM, because it requires FEAT_S2FWB in the host CPU to make it work properly, and not every CPU has that, at least in the KVM world. So you need to use virtio-gpu anyhow. thanks -- PMM
[PULL 2/9] rust: include rust_version in Cargo.toml
Tell clippy the minimum supported Rust version for QEMU.
Signed-off-by: Paolo Bonzini
---
rust/hw/char/pl011/Cargo.toml | 1 +
rust/hw/char/pl011/src/device_class.rs | 1 -
rust/qemu-api-macros/Cargo.toml| 1 +
rust/qemu-api/Cargo.toml | 1 +
4 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/rust/hw/char/pl011/Cargo.toml b/rust/hw/char/pl011/Cargo.toml
index 2b4097864df..f2296cad58b 100644
--- a/rust/hw/char/pl011/Cargo.toml
+++ b/rust/hw/char/pl011/Cargo.toml
@@ -9,6 +9,7 @@ resolver = "2"
publish = false
keywords = []
categories = []
+rust-version = "1.63.0"
[lib]
crate-type = ["staticlib"]
diff --git a/rust/hw/char/pl011/src/device_class.rs
b/rust/hw/char/pl011/src/device_class.rs
index 8a157a663fb..dbef93f6cb3 100644
--- a/rust/hw/char/pl011/src/device_class.rs
+++ b/rust/hw/char/pl011/src/device_class.rs
@@ -12,7 +12,6 @@
use crate::device::{PL011Registers, PL011State};
-#[allow(clippy::missing_const_for_fn)]
extern "C" fn pl011_clock_needed(opaque: *mut c_void) -> bool {
let state = NonNull::new(opaque).unwrap().cast::();
unsafe { state.as_ref().migrate_clock }
diff --git a/rust/qemu-api-macros/Cargo.toml b/rust/qemu-api-macros/Cargo.toml
index b9b4baecddb..89dee1cfb39 100644
--- a/rust/qemu-api-macros/Cargo.toml
+++ b/rust/qemu-api-macros/Cargo.toml
@@ -9,6 +9,7 @@ resolver = "2"
publish = false
keywords = []
categories = []
+rust-version = "1.63.0"
[lib]
proc-macro = true
diff --git a/rust/qemu-api/Cargo.toml b/rust/qemu-api/Cargo.toml
index 4aa22f31986..a51dd142852 100644
--- a/rust/qemu-api/Cargo.toml
+++ b/rust/qemu-api/Cargo.toml
@@ -12,6 +12,7 @@ resolver = "2"
publish = false
keywords = []
categories = []
+rust-version = "1.63.0"
[dependencies]
qemu_api_macros = { path = "../qemu-api-macros" }
--
2.48.1
[PULL 7/9] tcg/optimize: optimize TSTNE using smask and zmask
Generalize the existing optimization of "TSTNE x,sign" and "TSTNE x,-1".
This can be useful for example in the i386 frontend, which will generate
tests of zero-extended registers against 0x.
Ironically, on x86 hosts this is a very slight pessimization in the very
case it's meant to optimize because
brcond_i64 cc_dst,$0x,tsteq,$L1
(test %ebx, %ebx) is 1 byte smaller than
brcond_i64 cc_dst,$0x0,eq,$L1
(test %rbx, %rbx). However, in general it is an improvement, especially
if it avoids placing a large immediate in the constant pool.
Signed-off-by: Paolo Bonzini
---
tcg/optimize.c | 13 -
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/tcg/optimize.c b/tcg/optimize.c
index 8c6303e3afa..bca11cc427b 100644
--- a/tcg/optimize.c
+++ b/tcg/optimize.c
@@ -766,6 +766,7 @@ static int do_constant_folding_cond1(OptContext *ctx, TCGOp
*op, TCGArg dest,
TCGArg *p1, TCGArg *p2, TCGArg *pcond)
{
TCGCond cond;
+TempOptInfo *i1;
bool swap;
int r;
@@ -783,19 +784,21 @@ static int do_constant_folding_cond1(OptContext *ctx,
TCGOp *op, TCGArg dest,
return -1;
}
+i1 = arg_info(*p1);
+
/*
* TSTNE x,x -> NE x,0
- * TSTNE x,-1 -> NE x,0
+ * TSTNE x,i -> NE x,0 if i includes all nonzero bits of x
*/
-if (args_are_copies(*p1, *p2) || arg_is_const_val(*p2, -1)) {
+if (args_are_copies(*p1, *p2) ||
+(arg_is_const(*p2) && (i1->z_mask & ~arg_info(*p2)->val) == 0)) {
*p2 = arg_new_constant(ctx, 0);
*pcond = tcg_tst_eqne_cond(cond);
return -1;
}
-/* TSTNE x,sign -> LT x,0 */
-if (arg_is_const_val(*p2, (ctx->type == TCG_TYPE_I32
- ? INT32_MIN : INT64_MIN))) {
+/* TSTNE x,i -> LT x,0 if i only includes sign bit copies */
+if (arg_is_const(*p2) && (arg_info(*p2)->val & ~i1->s_mask) == 0) {
*p2 = arg_new_constant(ctx, 0);
*pcond = tcg_tst_ltge_cond(cond);
return -1;
--
2.48.1
[PULL 9/9] rust: restrict missing_const_for_fn to qemu_api crate
missing_const_for_fn is not necessarily useful or good. For example in
a private API you can always add const later, and in a public API
it can be unnecessarily restrictive to annotate everything with const
(blocking further improvements to the API).
Nevertheless, QEMU turns it on because qemu_api uses const quite
aggressively and therefore it can be handy to have as much as possible
annotated with const. Outside qemu_api though, not so much: devices
are self contained consumers and if there is nothing that could use
their functions in const contexts that were not anticipated.
Since missing_const_for_fn can be a bit noisy and trigger on trivial
functions that no one would ever call in const context, do not
turn it on everywhere and only keep it in qemu_api as a special case.
Signed-off-by: Paolo Bonzini
---
rust/Cargo.toml | 1 -
rust/qemu-api/src/lib.rs | 1 +
2 files changed, 1 insertion(+), 1 deletion(-)
diff --git a/rust/Cargo.toml b/rust/Cargo.toml
index 5b6b6ca4382..5b0cb559286 100644
--- a/rust/Cargo.toml
+++ b/rust/Cargo.toml
@@ -52,7 +52,6 @@ empty_structs_with_brackets = "deny"
ignored_unit_patterns = "deny"
implicit_clone = "deny"
macro_use_imports = "deny"
-missing_const_for_fn = "deny"
missing_safety_doc = "deny"
multiple_crate_versions = "deny"
mut_mut = "deny"
diff --git a/rust/qemu-api/src/lib.rs b/rust/qemu-api/src/lib.rs
index 83c6a987c05..3cf9371cff0 100644
--- a/rust/qemu-api/src/lib.rs
+++ b/rust/qemu-api/src/lib.rs
@@ -3,6 +3,7 @@
// SPDX-License-Identifier: GPL-2.0-or-later
#![cfg_attr(not(MESON), doc = include_str!("../README.md"))]
+#![deny(clippy::missing_const_for_fn)]
#[rustfmt::skip]
pub mod bindings;
--
2.48.1
[PULL 3/9] rust: add docs
Signed-off-by: Paolo Bonzini --- docs/devel/index-process.rst | 1 + docs/devel/rust.rst | 430 +++ 2 files changed, 431 insertions(+) create mode 100644 docs/devel/rust.rst diff --git a/docs/devel/index-process.rst b/docs/devel/index-process.rst index 362f97ee300..cb7c6640fd2 100644 --- a/docs/devel/index-process.rst +++ b/docs/devel/index-process.rst @@ -17,3 +17,4 @@ Notes about how to interact with the community and how and where to submit patch stable-process submitting-a-pull-request secure-coding-practices + rust diff --git a/docs/devel/rust.rst b/docs/devel/rust.rst new file mode 100644 index 000..390aae43866 --- /dev/null +++ b/docs/devel/rust.rst @@ -0,0 +1,430 @@ +.. |msrv| replace:: 1.63.0 + +Rust in QEMU + + +Rust in QEMU is a project to enable using the Rust programming language +to add new functionality to QEMU. + +Right now, the focus is on making it possible to write devices that inherit +from ``SysBusDevice`` in `*safe*`__ Rust. Later, it may become possible +to write other kinds of devices (e.g. PCI devices that can do DMA), +complete boards, or backends (e.g. block device formats). + +__ https://doc.rust-lang.org/nomicon/meet-safe-and-unsafe.html + +Building the Rust in QEMU code +-- + +The Rust in QEMU code is included in the emulators via Meson. Meson +invokes rustc directly, building static libraries that are then linked +together with the C code. This is completely automatic when you run +``make`` or ``ninja``. + +However, QEMU's build system also tries to be easy to use for people who +are accustomed to the more "normal" Cargo-based development workflow. +In particular: + +* the set of warnings and lints that are used to build QEMU always + comes from the ``rust/Cargo.toml`` workspace file + +* it is also possible to use ``cargo`` for common Rust-specific coding + tasks, in particular to invoke ``clippy``, ``rustfmt`` and ``rustdoc``. + +To this end, QEMU includes a ``build.rs`` build script that picks up +generated sources from QEMU's build directory and puts it in Cargo's +output directory (typically ``rust/target/``). A vanilla invocation +of Cargo will complain that it cannot find the generated sources, +which can be fixed in different ways: + +* by using special shorthand targets in the QEMU build directory:: + +make clippy +make rustfmt +make rustdoc + +* by invoking ``cargo`` through the Meson `development environment`__ + feature:: + +pyvenv/bin/meson devenv -w ../rust cargo clippy --tests +pyvenv/bin/meson devenv -w ../rust cargo fmt + + If you are going to use ``cargo`` repeatedly, ``pyvenv/bin/meson devenv`` + will enter a shell where commands like ``cargo clippy`` just work. + +__ https://mesonbuild.com/Commands.html#devenv + +* by pointing the ``MESON_BUILD_ROOT`` to the top of your QEMU build + tree. This third method is useful if you are using ``rust-analyzer``; + you can set the environment variable through the + ``rust-analyzer.cargo.extraEnv`` setting. + +As shown above, you can use the ``--tests`` option as usual to operate on test +code. Note however that you cannot *build* or run tests via ``cargo``, because +they need support C code from QEMU that Cargo does not know about. Tests can +be run via ``meson test`` or ``make``:: + + make check-rust + +Building Rust code with ``--enable-modules`` is not supported yet. + +Supported tools +''' + +QEMU supports rustc version 1.63.0 and newer. Notably, the following features +are missing: + +* ``core::ffi`` (1.64.0). Use ``std::os::raw`` and ``std::ffi`` instead. + +* ``cast_mut()``/``cast_const()`` (1.65.0). Use ``as`` instead. + +* "let ... else" (1.65.0). Use ``if let`` instead. This is currently patched + in QEMU's vendored copy of the bilge crate. + +* Generic Associated Types (1.65.0) + +* ``CStr::from_bytes_with_nul()`` as a ``const`` function (1.72.0). + +* "Return position ``impl Trait`` in Traits" (1.75.0, blocker for including + the pinned-init create). + +* ``MaybeUninit::zeroed()`` as a ``const`` function (1.75.0). QEMU's + ``Zeroable`` trait can be implemented without ``MaybeUninit::zeroed()``, + so this would be just a cleanup. + +* ``c"" literals`` (stable in 1.77.0). QEMU provides a ``c_str!()`` macro + to define ``CStr`` constants easily + +* ``offset_of!`` (stable in 1.77.0). QEMU uses ``offset_of!()`` heavily; it + provides a replacement in the ``qemu_api`` crate, but it does not support + lifetime parameters and therefore ``&'a Something`` fields in the struct + may have to be replaced by ``NonNull``. *Nested* ``offset_of!`` + was only stabilized in Rust 1.82.0, but it is not used. + +* inline const expression (stable in 1.79.0), currently worked around with + associated constants in the ``FnCall`` trait. + +* associated constants have to be explicitly marked ``'static`` (`changed in + 1.81.0`__) + +* ``&raw`` (stable in 1.
[PULL 8/9] rust: pl011: use default set of lints
Being the first crate added to QEMU, pl011 has a rather restrictive
Clippy setup. This can be sometimes a bit too heavy on its suggestions,
for example
error: this could be a `const fn`
--> hw/char/pl011/src/device.rs:382:5
|
382 | / fn set_read_trigger(&mut self) {
383 | | self.read_trigger = 1;
384 | | }
| |_^
Just use the standard set that is present in rust/Cargo.toml, with
just a small adjustment to allow upper case acronyms which are used
for register names.
Reported-by: Stefan Hajnoczi
Signed-off-by: Paolo Bonzini
---
rust/hw/char/pl011/src/lib.rs | 9 -
1 file changed, 9 deletions(-)
diff --git a/rust/hw/char/pl011/src/lib.rs b/rust/hw/char/pl011/src/lib.rs
index e704daf6e3e..3c72f1221ff 100644
--- a/rust/hw/char/pl011/src/lib.rs
+++ b/rust/hw/char/pl011/src/lib.rs
@@ -12,16 +12,7 @@
//! See [`PL011State`](crate::device::PL011State) for the device model type and
//! the [`registers`] module for register types.
-#![deny(
-clippy::correctness,
-clippy::suspicious,
-clippy::complexity,
-clippy::perf,
-clippy::nursery,
-clippy::style
-)]
#![allow(clippy::upper_case_acronyms)]
-#![allow(clippy::result_unit_err)]
use qemu_api::c_str;
--
2.48.1
[PULL 6/9] tests/tcg/x86_64/fma: Test some x86 fused-multiply-add cases
From: Peter Maydell
Add a test case which tests some corner case behaviour of
fused-multiply-add on x86:
* 0 * Inf + SNaN should raise Invalid
* 0 * Inf + QNaN shouldh not raise Invalid
* tininess should be detected after rounding
There is also one currently-disabled test case:
* flush-to-zero should be done after rounding
This is disabled because QEMU's emulation currently does this
incorrectly (and so would fail the test). The test case is kept in
but disabled, as the justification for why the test running harness
has support for testing both with and without FTZ set.
Signed-off-by: Peter Maydell
Reviewed-by: Richard Henderson
Link:
https://lore.kernel.org/r/20250116112536.4117889-3-peter.mayd...@linaro.org
Signed-off-by: Paolo Bonzini
---
tests/tcg/x86_64/fma.c | 109 +++
tests/tcg/x86_64/Makefile.target | 1 +
2 files changed, 110 insertions(+)
create mode 100644 tests/tcg/x86_64/fma.c
diff --git a/tests/tcg/x86_64/fma.c b/tests/tcg/x86_64/fma.c
new file mode 100644
index 000..09c622ebc00
--- /dev/null
+++ b/tests/tcg/x86_64/fma.c
@@ -0,0 +1,109 @@
+/*
+ * Test some fused multiply add corner cases.
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+#include
+#include
+#include
+#include
+
+#define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
+
+/*
+ * Perform one "n * m + a" operation using the vfmadd insn and return
+ * the result; on return *mxcsr_p is set to the bottom 6 bits of MXCSR
+ * (the Flag bits). If ftz is true then we set MXCSR.FTZ while doing
+ * the operation.
+ * We print the operation and its results to stdout.
+ */
+static uint64_t do_fmadd(uint64_t n, uint64_t m, uint64_t a,
+ bool ftz, uint32_t *mxcsr_p)
+{
+uint64_t r;
+uint32_t mxcsr = 0;
+uint32_t ftz_bit = ftz ? (1 << 15) : 0;
+uint32_t saved_mxcsr = 0;
+
+asm volatile("stmxcsr %[saved_mxcsr]\n"
+ "stmxcsr %[mxcsr]\n"
+ "andl $0x7fc0, %[mxcsr]\n"
+ "orl %[ftz_bit], %[mxcsr]\n"
+ "ldmxcsr %[mxcsr]\n"
+ "movq %[a], %%xmm0\n"
+ "movq %[m], %%xmm1\n"
+ "movq %[n], %%xmm2\n"
+ /* xmm0 = xmm0 + xmm2 * xmm1 */
+ "vfmadd231sd %%xmm1, %%xmm2, %%xmm0\n"
+ "movq %%xmm0, %[r]\n"
+ "stmxcsr %[mxcsr]\n"
+ "ldmxcsr %[saved_mxcsr]\n"
+ : [r] "=r" (r), [mxcsr] "=m" (mxcsr),
+ [saved_mxcsr] "=m" (saved_mxcsr)
+ : [n] "r" (n), [m] "r" (m), [a] "r" (a),
+ [ftz_bit] "r" (ftz_bit)
+ : "xmm0", "xmm1", "xmm2");
+*mxcsr_p = mxcsr & 0x3f;
+printf("vfmadd132sd 0x%" PRIx64 " 0x%" PRIx64 " 0x%" PRIx64
+ " = 0x%" PRIx64 " MXCSR flags 0x%" PRIx32 "\n",
+ n, m, a, r, *mxcsr_p);
+return r;
+}
+
+typedef struct testdata {
+/* Input n, m, a */
+uint64_t n;
+uint64_t m;
+uint64_t a;
+bool ftz;
+/* Expected result */
+uint64_t expected_r;
+/* Expected low 6 bits of MXCSR (the Flag bits) */
+uint32_t expected_mxcsr;
+} testdata;
+
+static testdata tests[] = {
+{ 0, 0x7ff0, 0x7ff0, false, /* 0 * Inf + SNaN */
+ 0x7ff8, 1 }, /* Should be QNaN and does raise Invalid */
+{ 0, 0x7ff0, 0x7ff8, false, /* 0 * Inf + QNaN */
+ 0x7ff8, 0 }, /* Should be QNaN and does *not* raise Invalid
*/
+/*
+ * These inputs give a result which is tiny before rounding but which
+ * becomes non-tiny after rounding. x86 is a "detect tininess after
+ * rounding" architecture, so it should give a non-denormal result and
+ * not set the Underflow flag (only the Precision flag for an inexact
+ * result).
+ */
+{ 0x3fdf, 0x001f, 0x801f, false,
+ 0x8010, 0x20 },
+/*
+ * Flushing of denormal outputs to zero should also happen after
+ * rounding, so setting FTZ should not affect the result or the flags.
+ * QEMU currently does not emulate this correctly because we do the
+ * flush-to-zero check before rounding, so we incorrectly produce a
+ * zero result and set Underflow as well as Precision.
+ */
+#ifdef ENABLE_FAILING_TESTS
+{ 0x3fdf, 0x001f, 0x801f, true,
+ 0x8010, 0x20 }, /* Enabling FTZ shouldn't change flags */
+#endif
+};
+
+int main(void)
+{
+bool passed = true;
+for (int i = 0; i < ARRAY_SIZE(tests); i++) {
+uint32_t mxcsr;
+uint64_t r = do_fmadd(tests[i].n, tests[i].m, tests[i].a,
+ tests[i].ftz, &mxcsr);
+if (r != tests[i].expected_r) {
+printf("expected result 0x%" PRIx64 "\n", tests[i].expected_r);
+passed = false;
+}
+if (mxcsr != tests[i].expected_mxcsr) {
+printf
[PULL 5/9] target/i386: Do not raise Invalid for 0 * Inf + QNaN
From: Peter Maydell
In commit 8adcff4ae7 ("fpu: handle raising Invalid for infzero in
pick_nan_muladd") we changed the handling of 0 * Inf + QNaN to always
raise the Invalid exception regardless of target architecture. (This
was a change affecting hppa, i386, sh4 and tricore.) However, this
was incorrect for i386, which documents in the SDM section 14.5.2
that for the 0 * Inf + NaN case that it will only raise the Invalid
exception when the input is an SNaN. (This is permitted by the IEEE
754-2008 specification, which documents that whether we raise Invalid
for 0 * Inf + QNaN is implementation defined.)
Adjust the softfloat pick_nan_muladd code to allow the target to
suppress the raising of Invalid for the inf * zero + NaN case (as an
extra flag orthogonal to its choice for when to use the default NaN),
and enable that for x86.
We do not revert here the behaviour change for hppa, sh4 or tricore:
* The sh4 manual is clear that it should signal Invalid
* The tricore manual is a bit vague but doesn't say it shouldn't
* The hppa manual doesn't talk about fused multiply-add corner
cases at all
Cc: qemu-sta...@nongnu.org
Fixes: 8adcff4ae7 (""fpu: handle raising Invalid for infzero in
pick_nan_muladd")
Signed-off-by: Peter Maydell
Reviewed-by: Richard Henderson
Link:
https://lore.kernel.org/r/20250116112536.4117889-2-peter.mayd...@linaro.org
Signed-off-by: Paolo Bonzini
---
include/fpu/softfloat-types.h | 16 +---
target/i386/tcg/fpu_helper.c | 5 -
fpu/softfloat-parts.c.inc | 5 +++--
3 files changed, 20 insertions(+), 6 deletions(-)
diff --git a/include/fpu/softfloat-types.h b/include/fpu/softfloat-types.h
index 616c290145f..2e43d1dd9e6 100644
--- a/include/fpu/softfloat-types.h
+++ b/include/fpu/softfloat-types.h
@@ -280,11 +280,21 @@ typedef enum __attribute__((__packed__)) {
/* No propagation rule specified */
float_infzeronan_none = 0,
/* Result is never the default NaN (so always the input NaN) */
-float_infzeronan_dnan_never,
+float_infzeronan_dnan_never = 1,
/* Result is always the default NaN */
-float_infzeronan_dnan_always,
+float_infzeronan_dnan_always = 2,
/* Result is the default NaN if the input NaN is quiet */
-float_infzeronan_dnan_if_qnan,
+float_infzeronan_dnan_if_qnan = 3,
+/*
+ * Don't raise Invalid for 0 * Inf + NaN. Default is to raise.
+ * IEEE 754-2008 section 7.2 makes it implementation defined whether
+ * 0 * Inf + QNaN raises Invalid or not. Note that 0 * Inf + SNaN will
+ * raise the Invalid flag for the SNaN anyway.
+ *
+ * This is a flag which can be ORed in with any of the above
+ * DNaN behaviour options.
+ */
+float_infzeronan_suppress_invalid = (1 << 7),
} FloatInfZeroNaNRule;
/*
diff --git a/target/i386/tcg/fpu_helper.c b/target/i386/tcg/fpu_helper.c
index 3d764bc138d..de6d0b252ec 100644
--- a/target/i386/tcg/fpu_helper.c
+++ b/target/i386/tcg/fpu_helper.c
@@ -178,8 +178,11 @@ void cpu_init_fp_statuses(CPUX86State *env)
* "Fused-Multiply-ADD (FMA) Numeric Behavior" the NaN handling is
* specified -- for 0 * inf + NaN the input NaN is selected, and if
* there are multiple input NaNs they are selected in the order a, b, c.
+ * We also do not raise Invalid for the 0 * inf + (Q)NaN case.
*/
-set_float_infzeronan_rule(float_infzeronan_dnan_never, &env->sse_status);
+set_float_infzeronan_rule(float_infzeronan_dnan_never |
+ float_infzeronan_suppress_invalid,
+ &env->sse_status);
set_float_3nan_prop_rule(float_3nan_prop_abc, &env->sse_status);
/* Default NaN: sign bit set, most significant frac bit set */
set_float_default_nan_pattern(0b1100, &env->fp_status);
diff --git a/fpu/softfloat-parts.c.inc b/fpu/softfloat-parts.c.inc
index fee05d0a863..73621f4a970 100644
--- a/fpu/softfloat-parts.c.inc
+++ b/fpu/softfloat-parts.c.inc
@@ -126,7 +126,8 @@ static FloatPartsN *partsN(pick_nan_muladd)(FloatPartsN *a,
FloatPartsN *b,
float_raise(float_flag_invalid | float_flag_invalid_snan, s);
}
-if (infzero) {
+if (infzero &&
+!(s->float_infzeronan_rule & float_infzeronan_suppress_invalid)) {
/* This is (0 * inf) + NaN or (inf * 0) + NaN */
float_raise(float_flag_invalid | float_flag_invalid_imz, s);
}
@@ -144,7 +145,7 @@ static FloatPartsN *partsN(pick_nan_muladd)(FloatPartsN *a,
FloatPartsN *b,
* Inf * 0 + NaN -- some implementations return the
* default NaN here, and some return the input NaN.
*/
-switch (s->float_infzeronan_rule) {
+switch (s->float_infzeronan_rule & ~float_infzeronan_suppress_invalid)
{
case float_infzeronan_dnan_never:
break;
case float_infzeronan_dnan_always:
--
2.48.1
[PULL 1/9] rust: remove unnecessary Cargo.toml metadata
Some items of Cargo.toml (readme, homepage, repository) are
only present because of clippy::cargo warnings being enabled in
rust/hw/char/pl011/src/lib.rs. But these items are not
particularly useful and would be all the same for all Cargo.toml
files in the QEMU workspace. Clean them up.
Signed-off-by: Paolo Bonzini
---
rust/hw/char/pl011/Cargo.toml | 3 ---
rust/hw/char/pl011/README.md| 31 ---
rust/hw/char/pl011/src/lib.rs | 14 ++
rust/qemu-api-macros/Cargo.toml | 3 ---
rust/qemu-api-macros/README.md | 1 -
5 files changed, 6 insertions(+), 46 deletions(-)
delete mode 100644 rust/hw/char/pl011/README.md
delete mode 100644 rust/qemu-api-macros/README.md
diff --git a/rust/hw/char/pl011/Cargo.toml b/rust/hw/char/pl011/Cargo.toml
index 58f3e859f7e..2b4097864df 100644
--- a/rust/hw/char/pl011/Cargo.toml
+++ b/rust/hw/char/pl011/Cargo.toml
@@ -4,10 +4,7 @@ version = "0.1.0"
edition = "2021"
authors = ["Manos Pitsidianakis "]
license = "GPL-2.0-or-later"
-readme = "README.md"
-homepage = "https://www.qemu.org";
description = "pl011 device model for QEMU"
-repository = "https://gitlab.com/epilys/rust-for-qemu";
resolver = "2"
publish = false
keywords = []
diff --git a/rust/hw/char/pl011/README.md b/rust/hw/char/pl011/README.md
deleted file mode 100644
index cd7dea31634..000
--- a/rust/hw/char/pl011/README.md
+++ /dev/null
@@ -1,31 +0,0 @@
-# PL011 QEMU Device Model
-
-This library implements a device model for the PrimeCell® UART (PL011)
-device in QEMU.
-
-## Build static lib
-
-Host build target must be explicitly specified:
-
-```sh
-cargo build --target x86_64-unknown-linux-gnu
-```
-
-Replace host target triplet if necessary.
-
-## Generate Rust documentation
-
-To generate docs for this crate, including private items:
-
-```sh
-cargo doc --no-deps --document-private-items --target x86_64-unknown-linux-gnu
-```
-
-To include direct dependencies like `bilge` (bitmaps for register types):
-
-```sh
-cargo tree --depth 1 -e normal --prefix none \
- | cut -d' ' -f1 \
- | xargs printf -- '-p %s\n' \
- | xargs cargo doc --no-deps --document-private-items --target
x86_64-unknown-linux-gnu
-```
diff --git a/rust/hw/char/pl011/src/lib.rs b/rust/hw/char/pl011/src/lib.rs
index e2df4586bcc..e704daf6e3e 100644
--- a/rust/hw/char/pl011/src/lib.rs
+++ b/rust/hw/char/pl011/src/lib.rs
@@ -1,13 +1,12 @@
// Copyright 2024, Linaro Limited
// Author(s): Manos Pitsidianakis
// SPDX-License-Identifier: GPL-2.0-or-later
-//
-// PL011 QEMU Device Model
-//
-// This library implements a device model for the PrimeCell® UART (PL011)
-// device in QEMU.
-//
-#![doc = include_str!("../README.md")]
+
+//! PL011 QEMU Device Model
+//!
+//! This library implements a device model for the PrimeCell® UART (PL011)
+//! device in QEMU.
+//!
//! # Library crate
//!
//! See [`PL011State`](crate::device::PL011State) for the device model type and
@@ -18,7 +17,6 @@
clippy::suspicious,
clippy::complexity,
clippy::perf,
-clippy::cargo,
clippy::nursery,
clippy::style
)]
diff --git a/rust/qemu-api-macros/Cargo.toml b/rust/qemu-api-macros/Cargo.toml
index 5a27b52ee6e..b9b4baecddb 100644
--- a/rust/qemu-api-macros/Cargo.toml
+++ b/rust/qemu-api-macros/Cargo.toml
@@ -4,10 +4,7 @@ version = "0.1.0"
edition = "2021"
authors = ["Manos Pitsidianakis "]
license = "GPL-2.0-or-later"
-readme = "README.md"
-homepage = "https://www.qemu.org";
description = "Rust bindings for QEMU - Utility macros"
-repository = "https://gitlab.com/qemu-project/qemu/";
resolver = "2"
publish = false
keywords = []
diff --git a/rust/qemu-api-macros/README.md b/rust/qemu-api-macros/README.md
deleted file mode 100644
index f60f54ac4be..000
--- a/rust/qemu-api-macros/README.md
+++ /dev/null
@@ -1 +0,0 @@
-# `qemu-api-macros` - Utility macros for defining QEMU devices
--
2.48.1
[PULL 4/9] rust: add clippy configuration file
Configure the minimum supported Rust version (though strictly speaking that's redundant with Cargo.toml), and the list of CamelCase identifiers that are not Rust types. Signed-off-by: Paolo Bonzini --- rust/clippy.toml | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 rust/clippy.toml diff --git a/rust/clippy.toml b/rust/clippy.toml new file mode 100644 index 000..5d190f91dec --- /dev/null +++ b/rust/clippy.toml @@ -0,0 +1,2 @@ +doc-valid-idents = ["PrimeCell", ".."] +msrv = "1.63.0" -- 2.48.1
[PULL v2 0/9] Rust, TCG, x86 patches for 2025-02-07
The following changes since commit 131c58469f6fb68c89b38fee6aba8bbb20c7f4bf: rust: add --rust-target option for bindgen (2025-02-06 13:51:46 -0500) are available in the Git repository at: https://gitlab.com/bonzini/qemu.git tags/for-upstream for you to fetch changes up to 476d6e4c9c4965734d6f47ee299ac9f84440a9b3: rust: restrict missing_const_for_fn to qemu_api crate (2025-02-10 11:18:32 +0100) * tcg/optimize: optimize TSTNE using smask and zmask * target/i386: fix exceptions for 0 * Inf + QNaN * rust: cleanups to the configuration and the warnings * rust: add developer docs v1->v2: add fix for check-rust-tools-nightly failure remove stray lcitool update some touchups to the Rust docs Paolo Bonzini (7): rust: remove unnecessary Cargo.toml metadata rust: include rust_version in Cargo.toml rust: add docs rust: add clippy configuration file tcg/optimize: optimize TSTNE using smask and zmask rust: pl011: use default set of lints rust: restrict missing_const_for_fn to qemu_api crate Peter Maydell (2): target/i386: Do not raise Invalid for 0 * Inf + QNaN tests/tcg/x86_64/fma: Test some x86 fused-multiply-add cases docs/devel/index-process.rst | 1 + docs/devel/rust.rst| 430 + include/fpu/softfloat-types.h | 16 +- target/i386/tcg/fpu_helper.c | 5 +- tcg/optimize.c | 13 +- tests/tcg/x86_64/fma.c | 109 + fpu/softfloat-parts.c.inc | 5 +- rust/Cargo.toml| 1 - rust/clippy.toml | 2 + rust/hw/char/pl011/Cargo.toml | 4 +- rust/hw/char/pl011/README.md | 31 --- rust/hw/char/pl011/src/device_class.rs | 1 - rust/hw/char/pl011/src/lib.rs | 23 +- rust/qemu-api-macros/Cargo.toml| 4 +- rust/qemu-api-macros/README.md | 1 - rust/qemu-api/Cargo.toml | 1 + rust/qemu-api/src/lib.rs | 1 + tests/tcg/x86_64/Makefile.target | 1 + 18 files changed, 581 insertions(+), 68 deletions(-) create mode 100644 docs/devel/rust.rst create mode 100644 tests/tcg/x86_64/fma.c create mode 100644 rust/clippy.toml delete mode 100644 rust/hw/char/pl011/README.md delete mode 100644 rust/qemu-api-macros/README.md -- 2.48.1
[PATCH 04/10] target/riscv: Declare RISCVCPUClass::misa_mxl_max as RISCVMXL
Signed-off-by: Philippe Mathieu-Daudé
---
Cc: qemu-ri...@nongnu.org
---
target/riscv/cpu.h | 2 +-
target/riscv/cpu.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 97713681cbe..fbe5548cf5a 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -529,7 +529,7 @@ struct RISCVCPUClass {
DeviceRealize parent_realize;
ResettablePhases parent_phases;
-uint32_t misa_mxl_max; /* max mxl for this cpu */
+RISCVMXL misa_mxl_max; /* max mxl for this cpu */
};
static inline int riscv_has_ext(CPURISCVState *env, target_ulong ext)
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 3d4bd157d2c..f3ad7f88f0e 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -2955,7 +2955,7 @@ static void riscv_cpu_class_init(ObjectClass *c, void
*data)
{
RISCVCPUClass *mcc = RISCV_CPU_CLASS(c);
-mcc->misa_mxl_max = (uint32_t)(uintptr_t)data;
+mcc->misa_mxl_max = (RISCVMXL)(uintptr_t)data;
riscv_cpu_validate_misa_mxl(mcc);
}
--
2.47.1
[PATCH 00/10] qom: Constify class_data
Following Richard's suggestion [*], make QOM class data *const*. Note, rust code not modified... [*] https://lore.kernel.org/qemu-devel/f4ec871d-e759-44bc-a10b-872322330...@linaro.org/ Philippe Mathieu-Daudé (10): target/i386: Constify X86CPUModel uses target/sparc: Constify SPARCCPUClass::cpu_def target/xtensa: Finalize config in xtensa_register_core() target/riscv: Declare RISCVCPUClass::misa_mxl_max as RISCVMXL target/riscv: Convert misa_mxl_max using GLib macros hw: Declare various const data as 'const' hw: Make class data 'const' qom: Have class_base_init() take a const data argument qom: Have class_init() take a const data argument qom: Constify class_data docs/devel/qom.rst| 8 +- docs/devel/reset.rst | 2 +- docs/devel/virtio-backends.rst| 2 +- hw/sd/sdhci-internal.h| 2 +- hw/usb/hcd-uhci.h | 2 +- include/hw/boards.h | 2 +- include/hw/core/cpu.h | 2 +- include/hw/i386/pc.h | 5 +- include/hw/virtio/virtio-pci.h| 2 +- include/qom/object.h | 8 +- target/arm/cpu.h | 2 +- target/i386/cpu.h | 2 +- target/ppc/cpu.h | 3 +- target/riscv/cpu.h| 2 +- target/sparc/cpu.h| 2 +- accel/hvf/hvf-accel-ops.c | 4 +- accel/kvm/kvm-accel-ops.c | 2 +- accel/kvm/kvm-all.c | 2 +- accel/qtest/qtest.c | 4 +- accel/tcg/tcg-accel-ops.c | 2 +- accel/tcg/tcg-all.c | 2 +- accel/xen/xen-all.c | 4 +- authz/list.c | 2 +- authz/listfile.c | 2 +- authz/pamacct.c | 2 +- authz/simple.c| 2 +- backends/confidential-guest-support.c | 3 +- backends/cryptodev-builtin.c | 2 +- backends/cryptodev-lkcf.c | 2 +- backends/cryptodev-vhost-user.c | 2 +- backends/cryptodev.c | 2 +- backends/dbus-vmstate.c | 2 +- backends/host_iommu_device.c | 2 +- backends/hostmem-epc.c| 2 +- backends/hostmem-file.c | 2 +- backends/hostmem-memfd.c | 2 +- backends/hostmem-ram.c| 2 +- backends/hostmem-shm.c| 2 +- backends/hostmem.c| 2 +- backends/iommufd.c| 4 +- backends/rng-builtin.c| 2 +- backends/rng-egd.c| 2 +- backends/rng-random.c | 2 +- backends/rng.c| 2 +- backends/tpm/tpm_emulator.c | 2 +- backends/tpm/tpm_passthrough.c| 2 +- backends/vhost-user.c | 2 +- block/throttle-groups.c | 3 +- chardev/baum.c| 2 +- chardev/char-console.c| 2 +- chardev/char-fd.c | 2 +- chardev/char-file.c | 2 +- chardev/char-hub.c| 2 +- chardev/char-mux.c| 2 +- chardev/char-null.c | 2 +- chardev/char-parallel.c | 2 +- chardev/char-pipe.c | 2 +- chardev/char-pty.c| 2 +- chardev/char-ringbuf.c| 2 +- chardev/char-serial.c | 2 +- chardev/char-socket.c | 2 +- chardev/char-stdio.c | 2 +- chardev/char-udp.c| 2 +- chardev/char-win-stdio.c | 2 +- chardev/char-win.c| 2 +- chardev/char.c| 2 +- chardev/msmouse.c | 2 +- chardev/spice.c | 6 +- chardev/testdev.c | 2 +- chardev/wctablet.c| 2 +- crypto/secret.c | 2 +- crypto/secret_common.c| 2 +- crypto/secret_keyring.c | 2 +- crypto/tls-cipher-suites.c| 3 +- crypto/tlscreds.c | 2 +- crypto/tlscredsanon.c | 2 +- crypto/tlscredspsk.c | 2 +- crypto/tlscredsx509.c | 2 +- event-loop-b
[PATCH 03/10] target/xtensa: Finalize config in xtensa_register_core()
Only modify XtensaConfig within xtensa_register_core(),
when the class is registered, not when it is initialized.
Signed-off-by: Philippe Mathieu-Daudé
---
Cc: Max Filippov
---
target/xtensa/helper.c | 8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/target/xtensa/helper.c b/target/xtensa/helper.c
index 2978c471c1f..c4735989714 100644
--- a/target/xtensa/helper.c
+++ b/target/xtensa/helper.c
@@ -173,9 +173,8 @@ static void xtensa_core_class_init(ObjectClass *oc, void
*data)
{
CPUClass *cc = CPU_CLASS(oc);
XtensaCPUClass *xcc = XTENSA_CPU_CLASS(oc);
-XtensaConfig *config = data;
+const XtensaConfig *config = data;
-xtensa_finalize_config(config);
xcc->config = config;
/*
@@ -189,12 +188,15 @@ static void xtensa_core_class_init(ObjectClass *oc, void
*data)
void xtensa_register_core(XtensaConfigList *node)
{
+XtensaConfig *config = g_memdup2(node->config, sizeof(config));
TypeInfo type = {
.parent = TYPE_XTENSA_CPU,
.class_init = xtensa_core_class_init,
-.class_data = (void *)node->config,
+.class_data = config,
};
+xtensa_finalize_config(config);
+
node->next = xtensa_cores;
xtensa_cores = node;
type.name = g_strdup_printf(XTENSA_CPU_TYPE_NAME("%s"),
node->config->name);
--
2.47.1
[PATCH 07/10] hw: Make class data 'const'
When the %data argument is not modified, we can declare it const.
Signed-off-by: Philippe Mathieu-Daudé
---
hw/sd/sdhci-internal.h | 2 +-
hw/sd/sdhci.c| 2 +-
hw/sensor/emc141x.c | 2 +-
hw/sensor/isl_pmbus_vr.c | 2 +-
4 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/hw/sd/sdhci-internal.h b/hw/sd/sdhci-internal.h
index 5f3765f12d2..9f768c418e0 100644
--- a/hw/sd/sdhci-internal.h
+++ b/hw/sd/sdhci-internal.h
@@ -322,6 +322,6 @@ void sdhci_initfn(SDHCIState *s);
void sdhci_uninitfn(SDHCIState *s);
void sdhci_common_realize(SDHCIState *s, Error **errp);
void sdhci_common_unrealize(SDHCIState *s);
-void sdhci_common_class_init(ObjectClass *klass, void *data);
+void sdhci_common_class_init(ObjectClass *klass, const void *data);
#endif
diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index 99dd4a4e952..1f45a77566c 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -1542,7 +1542,7 @@ const VMStateDescription sdhci_vmstate = {
},
};
-void sdhci_common_class_init(ObjectClass *klass, void *data)
+void sdhci_common_class_init(ObjectClass *klass, const void *data)
{
DeviceClass *dc = DEVICE_CLASS(klass);
diff --git a/hw/sensor/emc141x.c b/hw/sensor/emc141x.c
index aeccd2a3c94..33c1bd330fd 100644
--- a/hw/sensor/emc141x.c
+++ b/hw/sensor/emc141x.c
@@ -265,7 +265,7 @@ static void emc141x_initfn(Object *obj)
emc141x_set_temperature, NULL, NULL);
}
-static void emc141x_class_init(ObjectClass *klass, void *data)
+static void emc141x_class_init(ObjectClass *klass, const void *data)
{
DeviceClass *dc = DEVICE_CLASS(klass);
I2CSlaveClass *k = I2C_SLAVE_CLASS(klass);
diff --git a/hw/sensor/isl_pmbus_vr.c b/hw/sensor/isl_pmbus_vr.c
index 304a66ea8b0..c60282cfe77 100644
--- a/hw/sensor/isl_pmbus_vr.c
+++ b/hw/sensor/isl_pmbus_vr.c
@@ -233,7 +233,7 @@ static void raa228000_init(Object *obj)
isl_pmbus_vr_add_props(obj, flags, 1);
}
-static void isl_pmbus_vr_class_init(ObjectClass *klass, void *data,
+static void isl_pmbus_vr_class_init(ObjectClass *klass, const void *data,
uint8_t pages)
{
PMBusDeviceClass *k = PMBUS_DEVICE_CLASS(klass);
--
2.47.1
[PATCH 02/10] target/sparc: Constify SPARCCPUClass::cpu_def
Signed-off-by: Philippe Mathieu-Daudé
---
target/sparc/cpu.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/target/sparc/cpu.h b/target/sparc/cpu.h
index dda811503b5..462bcb6c0e6 100644
--- a/target/sparc/cpu.h
+++ b/target/sparc/cpu.h
@@ -574,7 +574,7 @@ struct SPARCCPUClass {
DeviceRealize parent_realize;
ResettablePhases parent_phases;
-sparc_def_t *cpu_def;
+const sparc_def_t *cpu_def;
};
#ifndef CONFIG_USER_ONLY
--
2.47.1
[PATCH 10/10] qom: Constify class_data
All callers now correctly expect a const class data.
Signed-off-by: Philippe Mathieu-Daudé
---
Cc: qemu-r...@nongnu.org
---
include/qom/object.h| 2 +-
hw/arm/armsse.c | 2 +-
hw/block/m25p80.c | 2 +-
hw/isa/vt82c686.c | 4 ++--
hw/net/e1000.c | 2 +-
hw/ppc/spapr_cpu_core.c | 2 +-
hw/scsi/megasas.c | 2 +-
hw/sensor/tmp421.c | 2 +-
hw/virtio/virtio-pci.c | 4 ++--
qom/object.c| 2 +-
target/arm/cpu.c| 2 +-
target/arm/cpu64.c | 2 +-
target/mips/cpu.c | 2 +-
target/s390x/cpu_models.c | 4 ++--
target/sparc/cpu.c | 2 +-
scripts/codeconverter/codeconverter/test_regexps.py | 2 +-
16 files changed, 19 insertions(+), 19 deletions(-)
diff --git a/include/qom/object.h b/include/qom/object.h
index 2fb86f00a68..42b75d10a43 100644
--- a/include/qom/object.h
+++ b/include/qom/object.h
@@ -488,7 +488,7 @@ struct TypeInfo
void (*class_init)(ObjectClass *klass, const void *data);
void (*class_base_init)(ObjectClass *klass, const void *data);
-void *class_data;
+const void *class_data;
InterfaceInfo *interfaces;
};
diff --git a/hw/arm/armsse.c b/hw/arm/armsse.c
index d65a46b8d8d..9403b65ddb5 100644
--- a/hw/arm/armsse.c
+++ b/hw/arm/armsse.c
@@ -1730,7 +1730,7 @@ static void armsse_register_types(void)
.name = armsse_variants[i].name,
.parent = TYPE_ARM_SSE,
.class_init = armsse_class_init,
-.class_data = (void *)&armsse_variants[i],
+.class_data = &armsse_variants[i],
};
type_register_static(&ti);
}
diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
index 236fa798c34..eee7bedd6b3 100644
--- a/hw/block/m25p80.c
+++ b/hw/block/m25p80.c
@@ -1891,7 +1891,7 @@ static void m25p80_register_types(void)
.name = known_devices[i].part_name,
.parent = TYPE_M25P80,
.class_init = m25p80_class_init,
-.class_data = (void *)&known_devices[i],
+.class_data = &known_devices[i],
};
type_register_static(&ti);
}
diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
index 80366aaf647..c62afc907b2 100644
--- a/hw/isa/vt82c686.c
+++ b/hw/isa/vt82c686.c
@@ -259,7 +259,7 @@ static const TypeInfo vt82c686b_pm_info = {
.name = TYPE_VT82C686B_PM,
.parent= TYPE_VIA_PM,
.class_init= via_pm_class_init,
-.class_data= (void *)&vt82c686b_pm_init_info,
+.class_data= &vt82c686b_pm_init_info,
};
static const ViaPMInitInfo vt8231_pm_init_info = {
@@ -272,7 +272,7 @@ static const TypeInfo vt8231_pm_info = {
.name = TYPE_VT8231_PM,
.parent= TYPE_VIA_PM,
.class_init= via_pm_class_init,
-.class_data= (void *)&vt8231_pm_init_info,
+.class_data= &vt8231_pm_init_info,
};
diff --git a/hw/net/e1000.c b/hw/net/e1000.c
index d49730f4ad4..13814e84d18 100644
--- a/hw/net/e1000.c
+++ b/hw/net/e1000.c
@@ -1770,7 +1770,7 @@ static void e1000_register_types(void)
type_info.name = info->name;
type_info.parent = TYPE_E1000_BASE;
-type_info.class_data = (void *)info;
+type_info.class_data = info;
type_info.class_init = e1000_class_init;
type_register_static(&type_info);
diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
index c1964d3dc8a..e1929a546a3 100644
--- a/hw/ppc/spapr_cpu_core.c
+++ b/hw/ppc/spapr_cpu_core.c
@@ -380,7 +380,7 @@ static void spapr_cpu_core_class_init(ObjectClass *oc,
const void *data)
#define DEFINE_SPAPR_CPU_CORE_TYPE(cpu_model) \
{ \
.parent = TYPE_SPAPR_CPU_CORE, \
-.class_data = (void *) POWERPC_CPU_TYPE_NAME(cpu_model), \
+.class_data = POWERPC_CPU_TYPE_NAME(cpu_model), \
.class_init = spapr_cpu_core_class_init,\
.name = SPAPR_CPU_CORE_TYPE_NAME(cpu_model),\
}
diff --git a/hw/scsi/megasas.c b/hw/scsi/megasas.c
index cfa5516b96c..6104d4202aa 100644
--- a/hw/scsi/megasas.c
+++ b/hw/scsi/megasas.c
@@ -2573,7 +2573,7 @@ static void megasas_register_types(void)
type_info.name = info->name;
type_info.parent = TYPE_MEGASAS_BASE;
-type_info.class_data = (void *)info;
+type_info.class_data = info;
type_info.class_init = megasas_class_init;
type_info.interfaces = info->interfaces;
diff --git a/hw/sensor/tmp421.c b/hw/sensor/tmp421.c
index 263bfa1bbda..3421c440869 100644
--- a/hw/sensor/tmp421.c
[PATCH 06/10] hw: Declare various const data as 'const'
Signed-off-by: Philippe Mathieu-Daudé
---
hw/isa/vt82c686.c | 2 +-
hw/rtc/m48t59-isa.c | 2 +-
hw/rtc/m48t59.c | 2 +-
hw/sensor/tmp421.c| 2 +-
hw/usb/hcd-ehci-pci.c | 2 +-
hw/usb/hcd-uhci.c | 2 +-
6 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
index 6f44b381a5f..43bd67eeef2 100644
--- a/hw/isa/vt82c686.c
+++ b/hw/isa/vt82c686.c
@@ -224,7 +224,7 @@ static void via_pm_class_init(ObjectClass *klass, void
*data)
{
DeviceClass *dc = DEVICE_CLASS(klass);
PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
-ViaPMInitInfo *info = data;
+const ViaPMInitInfo *info = data;
k->realize = via_pm_realize;
k->config_write = pm_write_config;
diff --git a/hw/rtc/m48t59-isa.c b/hw/rtc/m48t59-isa.c
index 38bc8dcf100..9c3855a3ef1 100644
--- a/hw/rtc/m48t59-isa.c
+++ b/hw/rtc/m48t59-isa.c
@@ -129,7 +129,7 @@ static void m48txx_isa_class_init(ObjectClass *klass, void
*data)
static void m48txx_isa_concrete_class_init(ObjectClass *klass, void *data)
{
M48txxISADeviceClass *u = M48TXX_ISA_CLASS(klass);
-M48txxInfo *info = data;
+const M48txxInfo *info = data;
u->info = *info;
}
diff --git a/hw/rtc/m48t59.c b/hw/rtc/m48t59.c
index c9bd6f878fe..3fb2f27d9d1 100644
--- a/hw/rtc/m48t59.c
+++ b/hw/rtc/m48t59.c
@@ -639,7 +639,7 @@ static void m48txx_sysbus_class_init(ObjectClass *klass,
void *data)
static void m48txx_sysbus_concrete_class_init(ObjectClass *klass, void *data)
{
M48txxSysBusDeviceClass *u = M48TXX_SYS_BUS_CLASS(klass);
-M48txxInfo *info = data;
+const M48txxInfo *info = data;
u->info = *info;
}
diff --git a/hw/sensor/tmp421.c b/hw/sensor/tmp421.c
index 82e604279c5..007f7cd018b 100644
--- a/hw/sensor/tmp421.c
+++ b/hw/sensor/tmp421.c
@@ -68,7 +68,7 @@ struct TMP421State {
struct TMP421Class {
I2CSlaveClass parent_class;
-DeviceInfo *dev;
+const DeviceInfo *dev;
};
#define TYPE_TMP421 "tmp421-generic"
diff --git a/hw/usb/hcd-ehci-pci.c b/hw/usb/hcd-ehci-pci.c
index d410c38a8a2..e00316721ac 100644
--- a/hw/usb/hcd-ehci-pci.c
+++ b/hw/usb/hcd-ehci-pci.c
@@ -182,7 +182,7 @@ static void ehci_data_class_init(ObjectClass *klass, void
*data)
{
PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
DeviceClass *dc = DEVICE_CLASS(klass);
-EHCIPCIInfo *i = data;
+const EHCIPCIInfo *i = data;
k->vendor_id = i->vendor_id;
k->device_id = i->device_id;
diff --git a/hw/usb/hcd-uhci.c b/hw/usb/hcd-uhci.c
index 8528d493d63..0561a6d801a 100644
--- a/hw/usb/hcd-uhci.c
+++ b/hw/usb/hcd-uhci.c
@@ -1289,7 +1289,7 @@ void uhci_data_class_init(ObjectClass *klass, void *data)
PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
DeviceClass *dc = DEVICE_CLASS(klass);
UHCIPCIDeviceClass *u = UHCI_CLASS(klass);
-UHCIInfo *info = data;
+const UHCIInfo *info = data;
k->realize = info->realize ? info->realize : usb_uhci_common_realize;
k->exit = info->unplug ? usb_uhci_exit : NULL;
--
2.47.1
[PATCH 01/10] target/i386: Constify X86CPUModel uses
Signed-off-by: Philippe Mathieu-Daudé
---
target/i386/cpu.h | 2 +-
target/i386/cpu.c | 8
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index c67b42d34fc..f9ce6970ee1 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -2288,7 +2288,7 @@ struct X86CPUClass {
* CPU definition, automatically loaded by instance_init if not NULL.
* Should be eventually replaced by subclass-specific property defaults.
*/
-X86CPUModel *model;
+const X86CPUModel *model;
bool host_cpuid_required;
int ordering;
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index b5dd60d2812..4b2da45366b 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -6436,7 +6436,7 @@ void x86_cpu_apply_props(X86CPU *cpu, PropValue *props)
* Only for builtin_x86_defs models initialized with x86_register_cpudef_types.
*/
-static void x86_cpu_apply_version_props(X86CPU *cpu, X86CPUModel *model)
+static void x86_cpu_apply_version_props(X86CPU *cpu, const X86CPUModel *model)
{
const X86CPUVersionDefinition *vdef;
X86CPUVersion version = x86_cpu_model_resolve_version(model);
@@ -6465,7 +6465,7 @@ static void x86_cpu_apply_version_props(X86CPU *cpu,
X86CPUModel *model)
}
static const CPUCaches *x86_cpu_get_versioned_cache_info(X86CPU *cpu,
- X86CPUModel *model)
+ const X86CPUModel
*model)
{
const X86CPUVersionDefinition *vdef;
X86CPUVersion version = x86_cpu_model_resolve_version(model);
@@ -6493,7 +6493,7 @@ static const CPUCaches
*x86_cpu_get_versioned_cache_info(X86CPU *cpu,
* Load data from X86CPUDefinition into a X86CPU object.
* Only for builtin_x86_defs models initialized with x86_register_cpudef_types.
*/
-static void x86_cpu_load_model(X86CPU *cpu, X86CPUModel *model)
+static void x86_cpu_load_model(X86CPU *cpu, const X86CPUModel *model)
{
const X86CPUDefinition *def = model->cpudef;
CPUX86State *env = &cpu->env;
@@ -6563,7 +6563,7 @@ static const gchar *x86_gdb_arch_name(CPUState *cs)
static void x86_cpu_cpudef_class_init(ObjectClass *oc, void *data)
{
-X86CPUModel *model = data;
+const X86CPUModel *model = data;
X86CPUClass *xcc = X86_CPU_CLASS(oc);
CPUClass *cc = CPU_CLASS(oc);
--
2.47.1
Re: [PATCH 00/14] target/arm: Clean up some corner cases of sysreg traps
Ping for review on patches 2, 3, 9, 10, 12, 14, please?
thanks
-- PMM
On Thu, 30 Jan 2025 at 18:23, Peter Maydell wrote:
>
> While reviewing Alex's recent secure timer patchset, I noticed a
> bug where it was using CP_ACCESS_TRAP when CP_ACCESS_TRAP_UNCATEGORIZED
> was wanted, and that we were making the same mistake elsewhere in
> our existing code. This series started out as an attempt to fix
> that and also rearrange the API so that it's harder to introduce
> other instances of this bug in future. In the process I noticed
> a different bug, where we weren't handling traps to AArch32
> Monitor mode correctly, so the series fixes those as well.
>
> The first four patches are fixes for the places where we were
> using CP_ACCESS_TRAP when we wanted CP_ACCESS_TRAP_UNCATEGORIZED.
> These are all situations where an attempt to access a sysreg
> should UNDEF and we were incorrectly reporting it with a
> syndrome value for a system register access trap. I've cc'd
> these to stable as they are technically bugs, but really the impact
> s very limited because I can't see a reason why any code except
> test code would deliberately attempt a sysreg access that they
> knew would take an exception and then look at the syndrome
> value for it.
>
> Patches 5, 6 and 7 together fix bugs in our handling of sysreg
> traps that should go to AArch32 Monitor mode:
> * we were incorrectly triggering an UNDEF exception for these
>rather than a Monitor Trap, so the exception would go to
>the wrong vector table and the wrong CPU mode
> * in most cases we weren't trapping accesses from EL3
>non-Monitor modes to Monitor mode
> This affects only:
> * trapping of ERRIDR via SCR.TERR
> * trapping of the debug channel registers via SDCR.TDCC
> * trapping of GICv3 registers via SCR.IRQ and SCR.FIQ
> because most "trap sysreg access to EL3" situations are either for
> AArch64 only registers or for trap bits that are AArch64 only.
>
> Patch 8 is a trivial one removing an unnecessary clause in
> an if() condition in the GIC cpuif access functions.
>
> Patches 9-13 are the API change that tries to make the bug harder to
> write: we add CP_ACCESS_TRAP_EL1 for "trap a sysreg access direct to
> EL1". After all the bugfixes in the first half of the series, the
> remaining uses of CP_ACCESS_TRAP are all in contexts where we know the
> current EL is 0, so we can directly replace them with
> CP_ACCESS_TRAP_EL1, and remove CP_ACCESS_TRAP entirely. We also rename
> CP_ACCESS_TRAP_UNCATEGORIZED to CP_ACCESS_UNDEFINED, to be a clearer
> parallel with the pseudocode's use of "UNDEFINED" in this situation.
> The resulting
> API is that an accessfn can only return:
> CP_ACCESS_OK for a success
> CP_ACCESS_UNDEFINED for an UNDEF
> CP_ACCESS_TRAP_EL{1,2,3} for a sysreg trap direct to an EL
> and everything else is invalid. UNCATEGORIZED traps never go to a
> specified target EL, and sysreg-traps always go to a specified target
> EL, matching the pseudocode which either uses "UNDEFINED" or some kind
> of SystemAccessTrap(ELx, ...).
>
> Patch 14 fixes some issues with the WFI/WFX trap code, some of
> which are like the above sysreg access check bugs (not using
> Monitor Trap, not honouring traps in EL3 not-Monitor-mode),
> plus one which I noticed while working on the code (it doesn't
> use arm_sctlr() so will look at the wrong SCTLR when in EL2&0).
>
> I've cc'd the relevant patches to stable, but I don't think
> it's very likely that anybody ever ran into these bugs, because
> they're all cases of "we did the wrong thing if code at an EL
> below EL3 tried to do something it shouldn't". I don't think that
> EL3 code generally uses the trap bits for trap-and-emulate
> of functionality, only to prevent the lower ELs messing with
> state it should not, and everything here with the exception of
> SCR.IRQ and SCR.FIQ is pretty obscure anyway.
>
> (Tested somewhat lightly, because I don't have any test images
> that test AArch32 EL3 really.)
>
> thanks
> -- PMM
>
> Peter Maydell (14):
> target/arm: Report correct syndrome for UNDEFINED CNTPS_*_EL1 from EL2
> and NS EL1
> target/arm: Report correct syndrome for UNDEFINED AT ops with wrong
> NSE,NS
> target/arm: Report correct syndrome for UNDEFINED S1E2 AT ops at EL3
> target/arm: Report correct syndrome for UNDEFINED LOR sysregs when
> NS=0
> target/arm: Make CP_ACCESS_TRAPs to AArch32 EL3 be Monitor traps
> hw/intc/arm_gicv3_cpuif: Don't downgrade monitor traps for AArch32 EL3
> target/arm: Honour SDCR.TDCC and SCR.TERR in AArch32 EL3 non-Monitor
> modes
> hw/intc/arm_gicv3_cpuif(): Remove redundant tests of is_a64()
> target/arm: Support CP_ACCESS_TRAP_EL1 as a CPAccessResult
> target/arm: Use CP_ACCESS_TRAP_EL1 for traps that are always to EL1
> target/arm: Use TRAP_UNCATEGORIZED for XScale CPAR traps
> target/arm: Remove CP_ACCESS_TRAP handling
> target/arm: Rename CP_ACCESS_TRAP_UNCATEGORIZED to CP_ACCESS_UNDEFINED
> target/arm: Correct errors
[PATCH 08/10] qom: Have class_base_init() take a const data argument
Signed-off-by: Philippe Mathieu-Daudé
---
Cc: qemu-r...@nongnu.org
---
include/qom/object.h | 2 +-
hw/core/machine.c| 2 +-
hw/core/qdev.c | 2 +-
hw/pci/pci.c | 2 +-
qom/object.c | 2 +-
5 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/include/qom/object.h b/include/qom/object.h
index 9192265db76..7bb14ca7067 100644
--- a/include/qom/object.h
+++ b/include/qom/object.h
@@ -487,7 +487,7 @@ struct TypeInfo
size_t class_size;
void (*class_init)(ObjectClass *klass, void *data);
-void (*class_base_init)(ObjectClass *klass, void *data);
+void (*class_base_init)(ObjectClass *klass, const void *data);
void *class_data;
InterfaceInfo *interfaces;
diff --git a/hw/core/machine.c b/hw/core/machine.c
index 254cc20c4cb..7bdde9286c2 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -1236,7 +1236,7 @@ static void machine_class_init(ObjectClass *oc, void
*data)
"Memory size configuration");
}
-static void machine_class_base_init(ObjectClass *oc, void *data)
+static void machine_class_base_init(ObjectClass *oc, const void *data)
{
MachineClass *mc = MACHINE_CLASS(oc);
mc->max_cpus = mc->max_cpus ?: 1;
diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 82bbdcb654e..54578299147 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -693,7 +693,7 @@ static void device_finalize(Object *obj)
g_free(dev->id);
}
-static void device_class_base_init(ObjectClass *class, void *data)
+static void device_class_base_init(ObjectClass *class, const void *data)
{
DeviceClass *klass = DEVICE_CLASS(class);
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 2afa423925c..00f50e6f2cc 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -2712,7 +2712,7 @@ static void pci_device_class_init(ObjectClass *klass,
void *data)
"access to indirect DMA memory");
}
-static void pci_device_class_base_init(ObjectClass *klass, void *data)
+static void pci_device_class_base_init(ObjectClass *klass, const void *data)
{
if (!object_class_is_abstract(klass)) {
ObjectClass *conventional =
diff --git a/qom/object.c b/qom/object.c
index ec447f14a78..61ac8cd4842 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -55,7 +55,7 @@ struct TypeImpl
size_t instance_align;
void (*class_init)(ObjectClass *klass, void *data);
-void (*class_base_init)(ObjectClass *klass, void *data);
+void (*class_base_init)(ObjectClass *klass, const void *data);
void *class_data;
--
2.47.1
Re: [PULL 7/9] tcg/optimize: optimize TSTNE using smask and zmask
On 10/2/25 11:22, Paolo Bonzini wrote: Generalize the existing optimization of "TSTNE x,sign" and "TSTNE x,-1". This can be useful for example in the i386 frontend, which will generate tests of zero-extended registers against 0x. Ironically, on x86 hosts this is a very slight pessimization in the very case it's meant to optimize because brcond_i64 cc_dst,$0x,tsteq,$L1 (test %ebx, %ebx) is 1 byte smaller than brcond_i64 cc_dst,$0x0,eq,$L1 (test %rbx, %rbx). However, in general it is an improvement, especially if it avoids placing a large immediate in the constant pool. Signed-off-by: Paolo Bonzini --- tcg/optimize.c | 13 - 1 file changed, 8 insertions(+), 5 deletions(-) Reviewed-by: Richard Henderson https://lore.kernel.org/qemu-devel/484b6063-e21f-48d6-9121-6c0c64396...@linaro.org/
[PATCH 05/10] target/riscv: Convert misa_mxl_max using GLib macros
Use GLib conversion macros to pass misa_mxl_max as
riscv_cpu_class_init() class data.
Signed-off-by: Philippe Mathieu-Daudé
---
Cc: qemu-ri...@nongnu.org
---
target/riscv/cpu.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index f3ad7f88f0e..9fe1b23a297 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -2955,7 +2955,7 @@ static void riscv_cpu_class_init(ObjectClass *c, void
*data)
{
RISCVCPUClass *mcc = RISCV_CPU_CLASS(c);
-mcc->misa_mxl_max = (RISCVMXL)(uintptr_t)data;
+mcc->misa_mxl_max = (RISCVMXL)GPOINTER_TO_UINT(data);
riscv_cpu_validate_misa_mxl(mcc);
}
@@ -3057,7 +3057,7 @@ void riscv_isa_write_fdt(RISCVCPU *cpu, void *fdt, char
*nodename)
.parent = TYPE_RISCV_CPU, \
.instance_init = (initfn), \
.class_init = riscv_cpu_class_init, \
-.class_data = (void *)(misa_mxl_max)\
+.class_data = GUINT_TO_POINTER(misa_mxl_max)\
}
#define DEFINE_DYNAMIC_CPU(type_name, misa_mxl_max, initfn) \
@@ -3066,7 +3066,7 @@ void riscv_isa_write_fdt(RISCVCPU *cpu, void *fdt, char
*nodename)
.parent = TYPE_RISCV_DYNAMIC_CPU, \
.instance_init = (initfn), \
.class_init = riscv_cpu_class_init, \
-.class_data = (void *)(misa_mxl_max)\
+.class_data = GUINT_TO_POINTER(misa_mxl_max)\
}
#define DEFINE_VENDOR_CPU(type_name, misa_mxl_max, initfn) \
@@ -3075,7 +3075,7 @@ void riscv_isa_write_fdt(RISCVCPU *cpu, void *fdt, char
*nodename)
.parent = TYPE_RISCV_VENDOR_CPU,\
.instance_init = (initfn), \
.class_init = riscv_cpu_class_init, \
-.class_data = (void *)(misa_mxl_max)\
+.class_data = GUINT_TO_POINTER(misa_mxl_max)\
}
#define DEFINE_BARE_CPU(type_name, misa_mxl_max, initfn)\
@@ -3084,7 +3084,7 @@ void riscv_isa_write_fdt(RISCVCPU *cpu, void *fdt, char
*nodename)
.parent = TYPE_RISCV_BARE_CPU, \
.instance_init = (initfn), \
.class_init = riscv_cpu_class_init, \
-.class_data = (void *)(misa_mxl_max)\
+.class_data = GUINT_TO_POINTER(misa_mxl_max)\
}
#define DEFINE_PROFILE_CPU(type_name, misa_mxl_max, initfn) \
@@ -3093,7 +3093,7 @@ void riscv_isa_write_fdt(RISCVCPU *cpu, void *fdt, char
*nodename)
.parent = TYPE_RISCV_BARE_CPU, \
.instance_init = (initfn), \
.class_init = riscv_cpu_class_init, \
-.class_data = (void *)(misa_mxl_max)\
+.class_data = GUINT_TO_POINTER(misa_mxl_max)\
}
static const TypeInfo riscv_cpu_type_infos[] = {
--
2.47.1
Re: [PULL 1/9] rust: remove unnecessary Cargo.toml metadata
On 10/2/25 11:22, Paolo Bonzini wrote: Some items of Cargo.toml (readme, homepage, repository) are only present because of clippy::cargo warnings being enabled in rust/hw/char/pl011/src/lib.rs. But these items are not particularly useful and would be all the same for all Cargo.toml files in the QEMU workspace. Clean them up. Signed-off-by: Paolo Bonzini --- rust/hw/char/pl011/Cargo.toml | 3 --- rust/hw/char/pl011/README.md| 31 --- rust/hw/char/pl011/src/lib.rs | 14 ++ rust/qemu-api-macros/Cargo.toml | 3 --- rust/qemu-api-macros/README.md | 1 - 5 files changed, 6 insertions(+), 46 deletions(-) delete mode 100644 rust/hw/char/pl011/README.md delete mode 100644 rust/qemu-api-macros/README.md Reviewed-by: Zhao Liu https://lore.kernel.org/qemu-devel/z6ssbl3j9dugd...@intel.com/
Re: [PATCH] rust: pl011: convert pl011_create to safe Rust
On Thu, Feb 06, 2025 at 12:15:14PM +0100, Paolo Bonzini wrote:
> Date: Thu, 6 Feb 2025 12:15:14 +0100
> From: Paolo Bonzini
> Subject: [PATCH] rust: pl011: convert pl011_create to safe Rust
> X-Mailer: git-send-email 2.48.1
>
> Not a major change but, as a small but significant step in creating
> qdev bindings, show how pl011_create can be written without "unsafe"
> calls (apart from converting pointers to references).
>
> This also provides a starting point for creating Error** bindings.
>
> Signed-off-by: Paolo Bonzini
> ---
> rust/hw/char/pl011/src/device.rs | 39
> rust/qemu-api/src/sysbus.rs | 34 +---
> 2 files changed, 50 insertions(+), 23 deletions(-)
...
> +fn realize(&self) {
What about renaming this as "realize_with_sysbus"?
Elsewhere, the device's own realize method is often used to set
DeviceImpl::REALIZE.
And this realize here is meant to call the realize() method defined on
the C side, so to avoid confusion we can avoid the same name? It's up to
you.
> +// TODO: return an Error
> +assert!(bql_locked());
> +unsafe {
> +bindings::sysbus_realize(self.as_mut_ptr(),
> addr_of_mut!(bindings::error_fatal));
> +}
> +}
This is a nice patch that shows more about how to use Owned<>!
(BTW, I guess this patch is not the stable material. :-) )
Reviewed-by: Zhao Liu
[PATCH v2 5/7] hw/intc/loongarch_extioi: Add basic hotplug framework
LoongArch extioi interrupt controller routes peripheral interrupt
to multiple CPUs, physical cpu id is used in interrupt routing table.
Here hotplug interface is added for extioi object, so that parent irq
line can be connected, and routing table can be added for new created
cpu.
Here only basic hotplug framework is added, it is stub function.
Signed-off-by: Bibo Mao
---
hw/intc/loongarch_extioi_common.c | 33 +++
1 file changed, 33 insertions(+)
diff --git a/hw/intc/loongarch_extioi_common.c
b/hw/intc/loongarch_extioi_common.c
index e3a38b318a..19e19a9f73 100644
--- a/hw/intc/loongarch_extioi_common.c
+++ b/hw/intc/loongarch_extioi_common.c
@@ -4,11 +4,37 @@
* Copyright (C) 2024 Loongson Technology Corporation Limited
*/
#include "qemu/osdep.h"
+#include "qemu/error-report.h"
#include "qemu/module.h"
#include "qapi/error.h"
#include "hw/qdev-properties.h"
#include "hw/intc/loongarch_extioi_common.h"
#include "migration/vmstate.h"
+#include "target/loongarch/cpu.h"
+
+static void loongarch_extioi_cpu_plug(HotplugHandler *hotplug_dev,
+ DeviceState *dev, Error **errp)
+{
+Object *obj = OBJECT(dev);
+
+if (!object_dynamic_cast(obj, TYPE_LOONGARCH_CPU)) {
+warn_report("LoongArch extioi: Invalid %s device type",
+ object_get_typename(obj));
+return;
+}
+}
+
+static void loongarch_extioi_cpu_unplug(HotplugHandler *hotplug_dev,
+DeviceState *dev, Error **errp)
+{
+Object *obj = OBJECT(dev);
+
+if (!object_dynamic_cast(obj, TYPE_LOONGARCH_CPU)) {
+warn_report("LoongArch extioi: Invalid %s device type",
+ object_get_typename(obj));
+return;
+}
+}
static void loongarch_extioi_common_realize(DeviceState *dev, Error **errp)
{
@@ -107,11 +133,14 @@ static void
loongarch_extioi_common_class_init(ObjectClass *klass, void *data)
{
DeviceClass *dc = DEVICE_CLASS(klass);
LoongArchExtIOICommonClass *lecc = LOONGARCH_EXTIOI_COMMON_CLASS(klass);
+HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(klass);
device_class_set_parent_realize(dc, loongarch_extioi_common_realize,
&lecc->parent_realize);
device_class_set_props(dc, extioi_properties);
dc->vmsd = &vmstate_loongarch_extioi;
+hc->plug = loongarch_extioi_cpu_plug;
+hc->unplug = loongarch_extioi_cpu_unplug;
}
static const TypeInfo loongarch_extioi_common_types[] = {
@@ -121,6 +150,10 @@ static const TypeInfo loongarch_extioi_common_types[] = {
.instance_size = sizeof(LoongArchExtIOICommonState),
.class_size = sizeof(LoongArchExtIOICommonClass),
.class_init = loongarch_extioi_common_class_init,
+.interfaces = (InterfaceInfo[]) {
+{ TYPE_HOTPLUG_HANDLER },
+{ }
+},
.abstract = true,
}
};
--
2.39.3
[PATCH v2 4/7] hw/intc/loongarch_extioi: Move gpio irq initial to common code
When cpu is added, it will connect gpio irq line to cpu irq.
And cpu hot-add is put in common code, move gpio irq initial
part into common code.
Signed-off-by: Bibo Mao
---
hw/intc/loongarch_extioi.c| 8 +---
hw/intc/loongarch_extioi_common.c | 6 +-
2 files changed, 6 insertions(+), 8 deletions(-)
diff --git a/hw/intc/loongarch_extioi.c b/hw/intc/loongarch_extioi.c
index f3055ec4d2..a51a215e6e 100644
--- a/hw/intc/loongarch_extioi.c
+++ b/hw/intc/loongarch_extioi.c
@@ -343,7 +343,7 @@ static void loongarch_extioi_realize(DeviceState *dev,
Error **errp)
LoongArchExtIOIClass *lec = LOONGARCH_EXTIOI_GET_CLASS(dev);
SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
Error *local_err = NULL;
-int i, pin;
+int i;
lec->parent_realize(dev, &local_err);
if (local_err) {
@@ -368,12 +368,6 @@ static void loongarch_extioi_realize(DeviceState *dev,
Error **errp)
} else {
s->status |= BIT(EXTIOI_ENABLE);
}
-
-for (i = 0; i < s->num_cpu; i++) {
-for (pin = 0; pin < LS3A_INTC_IP; pin++) {
-qdev_init_gpio_out(dev, &s->cpu[i].parent_irq[pin], 1);
-}
-}
}
static void loongarch_extioi_unrealize(DeviceState *dev)
diff --git a/hw/intc/loongarch_extioi_common.c
b/hw/intc/loongarch_extioi_common.c
index fd56253d10..e3a38b318a 100644
--- a/hw/intc/loongarch_extioi_common.c
+++ b/hw/intc/loongarch_extioi_common.c
@@ -16,7 +16,7 @@ static void loongarch_extioi_common_realize(DeviceState *dev,
Error **errp)
MachineState *machine = MACHINE(qdev_get_machine());
MachineClass *mc = MACHINE_GET_CLASS(machine);
const CPUArchIdList *id_list;
-int i;
+int i, pin;
assert(mc->possible_cpu_arch_ids);
id_list = mc->possible_cpu_arch_ids(machine);
@@ -30,6 +30,10 @@ static void loongarch_extioi_common_realize(DeviceState
*dev, Error **errp)
for (i = 0; i < s->num_cpu; i++) {
s->cpu[i].arch_id = id_list->cpus[i].arch_id;
s->cpu[i].cpu = CPU(id_list->cpus[i].cpu);
+
+for (pin = 0; pin < LS3A_INTC_IP; pin++) {
+qdev_init_gpio_out(dev, &s->cpu[i].parent_irq[pin], 1);
+}
}
}
--
2.39.3
[PATCH v2 1/7] hw/intc/loongarch_ipi: Add basic hotplug framework
LoongArch ipi can send interrupt to multiple CPUs, interrupt routing
to CPU comes from destination physical cpu id. Here hotplug interface
is added for IPI object, so that parent irq line can be connected, and
routing table can be added for new created cpu.
Here only basic hotplug framework is added, it is stub function.
Signed-off-by: Bibo Mao
---
hw/intc/loongarch_ipi.c | 32
1 file changed, 32 insertions(+)
diff --git a/hw/intc/loongarch_ipi.c b/hw/intc/loongarch_ipi.c
index 5376f1e084..90bbb7ac6e 100644
--- a/hw/intc/loongarch_ipi.c
+++ b/hw/intc/loongarch_ipi.c
@@ -6,6 +6,7 @@
*/
#include "qemu/osdep.h"
+#include "qemu/error-report.h"
#include "hw/boards.h"
#include "qapi/error.h"
#include "hw/intc/loongarch_ipi.h"
@@ -76,9 +77,34 @@ static void loongarch_ipi_realize(DeviceState *dev, Error
**errp)
}
}
+static void loongarch_ipi_cpu_plug(HotplugHandler *hotplug_dev,
+ DeviceState *dev, Error **errp)
+{
+Object *obj = OBJECT(dev);
+
+if (!object_dynamic_cast(obj, TYPE_LOONGARCH_CPU)) {
+warn_report("LoongArch extioi: Invalid %s device type",
+ object_get_typename(obj));
+return;
+}
+}
+
+static void loongarch_ipi_cpu_unplug(HotplugHandler *hotplug_dev,
+ DeviceState *dev, Error **errp)
+{
+Object *obj = OBJECT(dev);
+
+if (!object_dynamic_cast(obj, TYPE_LOONGARCH_CPU)) {
+warn_report("LoongArch extioi: Invalid %s device type",
+ object_get_typename(obj));
+return;
+}
+}
+
static void loongarch_ipi_class_init(ObjectClass *klass, void *data)
{
LoongsonIPICommonClass *licc = LOONGSON_IPI_COMMON_CLASS(klass);
+HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(klass);
LoongarchIPIClass *lic = LOONGARCH_IPI_CLASS(klass);
DeviceClass *dc = DEVICE_CLASS(klass);
@@ -86,6 +112,8 @@ static void loongarch_ipi_class_init(ObjectClass *klass,
void *data)
&lic->parent_realize);
licc->get_iocsr_as = get_iocsr_as;
licc->cpu_by_arch_id = loongarch_cpu_by_arch_id;
+hc->plug = loongarch_ipi_cpu_plug;
+hc->unplug = loongarch_ipi_cpu_unplug;
}
static const TypeInfo loongarch_ipi_types[] = {
@@ -95,6 +123,10 @@ static const TypeInfo loongarch_ipi_types[] = {
.instance_size = sizeof(LoongarchIPIState),
.class_size = sizeof(LoongarchIPIClass),
.class_init = loongarch_ipi_class_init,
+.interfaces = (InterfaceInfo[]) {
+{ TYPE_HOTPLUG_HANDLER },
+{ }
+},
}
};
--
2.39.3
Re: [PATCH] target/arm/helper: Fix timer interrupt masking when HCR_EL2.E2H == 0
On Fri, 7 Feb 2025 at 18:29, Alex Bennée wrote: > > Richard Henderson writes: > > > On 2/7/25 07:45, Peter Maydell wrote: > >> This is where things go wrong -- icount_start_warp_timer() > >> notices that all CPU threads are currently idle, and > >> decides it needs to warp the timer forwards to the > >> next deadline, which is at the end of time -- INT64_MAX. > >> But once timer_mod_ns() returns, the generic timer code > >> is going to raise an interrupt (this goes through the GIC > >> code and comes back into the CPU which calls cpu_interrupt()), > >> so we don't want to warp the timer at all. The clock should > >> stay exactly at the value it has and the CPU is going to > >> have more work to do. > >> How is this supposed to work? Shouldn't we only be doing > >> the "start moving the icount forward to the next deadline" > >> once we've completed all the "run timers and AIO stuff" that > >> icount_handle_deadline() triggers, not randomly in the middle > >> of that when this timer callback or some other one might do > >> something to trigger an interrupt? > > > > I don't understand timer warping at all. And you're right, it doesn't > > seem like this should happen outside of a specific point in the main > > loop. > > This has come up before - and the conclusion was we don't know what > sleep=on/off is meant to mean. If the processor is asleep and there are > no timers to fire then nothing will happen. > > It was off-list though: > > Subject: Re: qemu-system-aarch64 & icount behavior No, that was a different situation. That thread was about when there genuinely is nothing to do (all CPUs asleep and no timers active) for the rest of the life of the simulation. The bug in this thread is that icount incorrectly prematurely decides it should warp the timer forwards, when in fact there is going to be something the CPU should be doing right now (i.e. responding to the interrupt the timer callback is about to raise). It becomes very obvious when there's no other timer callback, because the place that icount incorrectly warps us to is end-of-time, but I'm pretty sure that even when there is another timer active icount will still be wrongly warping time -- it will just be less obvious because the interrupt gets incorrectly delayed to whatever that subsequent timer callback time is, rather than forever. thanks -- PMM
[PATCH v2 7/7] hw/intc/loongarch_extioi: Use cpu plug notification
Use hotplug_handler_plug() to nofity extioi object when cold-plug
cpu is created, so that extioi can set and configure irq routing
to new cpu.
Signed-off-by: Bibo Mao
---
hw/loongarch/virt.c | 12 ++--
1 file changed, 2 insertions(+), 10 deletions(-)
diff --git a/hw/loongarch/virt.c b/hw/loongarch/virt.c
index ad1467c6f8..355be14ccc 100644
--- a/hw/loongarch/virt.c
+++ b/hw/loongarch/virt.c
@@ -320,7 +320,7 @@ static void virt_devices_init(DeviceState *pch_pic,
static void virt_cpu_irq_init(LoongArchVirtMachineState *lvms)
{
-int num, pin;
+int num;
MachineState *ms = MACHINE(lvms);
MachineClass *mc = MACHINE_GET_CLASS(ms);
const CPUArchIdList *possible_cpus;
@@ -336,15 +336,7 @@ static void virt_cpu_irq_init(LoongArchVirtMachineState
*lvms)
}
hotplug_handler_plug(HOTPLUG_HANDLER(lvms->ipi), DEVICE(cs), &err);
-
-/*
- * connect ext irq to the cpu irq
- * cpu_pin[9:2] <= intc_pin[7:0]
- */
-for (pin = 0; pin < LS3A_INTC_IP; pin++) {
-qdev_connect_gpio_out(lvms->extioi, (num * LS3A_INTC_IP + pin),
- qdev_get_gpio_in(DEVICE(cs), pin + 2));
-}
+hotplug_handler_plug(HOTPLUG_HANDLER(lvms->extioi), DEVICE(cs), &err);
}
}
--
2.39.3
[PATCH v2 2/7] hw/intc/loongarch_ipi: Implment cpu hotplug interface
Add logic cpu allocation and cpu mapping with cpu hotplug interface.
When cpu is added, connect ipi gpio irq to CPU IRQ_IPI irq pin.
Signed-off-by: Bibo Mao
---
hw/intc/loongarch_ipi.c | 39 +++
1 file changed, 39 insertions(+)
diff --git a/hw/intc/loongarch_ipi.c b/hw/intc/loongarch_ipi.c
index 90bbb7ac6e..b10641dd03 100644
--- a/hw/intc/loongarch_ipi.c
+++ b/hw/intc/loongarch_ipi.c
@@ -49,6 +49,22 @@ static int loongarch_cpu_by_arch_id(LoongsonIPICommonState
*lics,
return MEMTX_ERROR;
}
+static IPICore *loongarch_ipi_get_cpu(LoongsonIPICommonState *lics,
+ DeviceState *dev)
+{
+CPUClass *k = CPU_GET_CLASS(dev);
+uint64_t arch_id = k->get_arch_id(CPU(dev));
+int i;
+
+for (i = 0; i < lics->num_cpu; i++) {
+if (lics->cpu[i].arch_id == arch_id) {
+return &lics->cpu[i];
+}
+}
+
+return NULL;
+}
+
static void loongarch_ipi_realize(DeviceState *dev, Error **errp)
{
LoongsonIPICommonState *lics = LOONGSON_IPI_COMMON(dev);
@@ -80,25 +96,48 @@ static void loongarch_ipi_realize(DeviceState *dev, Error
**errp)
static void loongarch_ipi_cpu_plug(HotplugHandler *hotplug_dev,
DeviceState *dev, Error **errp)
{
+LoongsonIPICommonState *lics = LOONGSON_IPI_COMMON(hotplug_dev);
Object *obj = OBJECT(dev);
+IPICore *core;
+int index;
if (!object_dynamic_cast(obj, TYPE_LOONGARCH_CPU)) {
warn_report("LoongArch extioi: Invalid %s device type",
object_get_typename(obj));
return;
}
+
+core = loongarch_ipi_get_cpu(lics, dev);
+if (!core) {
+return;
+}
+
+core->cpu = CPU(dev);
+index = core - lics->cpu;
+
+/* connect ipi irq to cpu irq */
+qdev_connect_gpio_out(DEVICE(lics), index, qdev_get_gpio_in(dev, IRQ_IPI));
}
static void loongarch_ipi_cpu_unplug(HotplugHandler *hotplug_dev,
DeviceState *dev, Error **errp)
{
+LoongsonIPICommonState *lics = LOONGSON_IPI_COMMON(hotplug_dev);
Object *obj = OBJECT(dev);
+IPICore *core;
if (!object_dynamic_cast(obj, TYPE_LOONGARCH_CPU)) {
warn_report("LoongArch extioi: Invalid %s device type",
object_get_typename(obj));
return;
}
+
+core = loongarch_ipi_get_cpu(lics, dev);
+if (!core) {
+return;
+}
+
+core->cpu = NULL;
}
static void loongarch_ipi_class_init(ObjectClass *klass, void *data)
--
2.39.3
[PATCH v2 3/7] hw/intc/loongarch_ipi: Notify ipi object when cpu is plugged
Use hotplug_handler_plug() to nofity ipi object when cold-plug cpu is created, so that ipi can set and configure irq routing to new cpu. Signed-off-by: Bibo Mao --- hw/loongarch/virt.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/hw/loongarch/virt.c b/hw/loongarch/virt.c index f2aa0a9782..ad1467c6f8 100644 --- a/hw/loongarch/virt.c +++ b/hw/loongarch/virt.c @@ -325,6 +325,7 @@ static void virt_cpu_irq_init(LoongArchVirtMachineState *lvms) MachineClass *mc = MACHINE_GET_CLASS(ms); const CPUArchIdList *possible_cpus; CPUState *cs; +Error *err = NULL; /* cpu nodes */ possible_cpus = mc->possible_cpu_arch_ids(ms); @@ -334,9 +335,7 @@ static void virt_cpu_irq_init(LoongArchVirtMachineState *lvms) continue; } -/* connect ipi irq to cpu irq */ -qdev_connect_gpio_out(lvms->ipi, num, - qdev_get_gpio_in(DEVICE(cs), IRQ_IPI)); +hotplug_handler_plug(HOTPLUG_HANDLER(lvms->ipi), DEVICE(cs), &err); /* * connect ext irq to the cpu irq -- 2.39.3
[PATCH v2 6/7] hw/intc/loongarch_extioi: Implment cpu hotplug interface
When cpu is added, connect extioi gpio irq to CPU irq pin.
Signed-off-by: Bibo Mao
---
hw/intc/loongarch_extioi_common.c | 45 +++
1 file changed, 45 insertions(+)
diff --git a/hw/intc/loongarch_extioi_common.c
b/hw/intc/loongarch_extioi_common.c
index 19e19a9f73..ff3974f2a1 100644
--- a/hw/intc/loongarch_extioi_common.c
+++ b/hw/intc/loongarch_extioi_common.c
@@ -12,28 +12,73 @@
#include "migration/vmstate.h"
#include "target/loongarch/cpu.h"
+static ExtIOICore *loongarch_extioi_get_cpu(LoongArchExtIOICommonState *s,
+DeviceState *dev)
+{
+CPUClass *k = CPU_GET_CLASS(dev);
+uint64_t arch_id = k->get_arch_id(CPU(dev));
+int i;
+
+for (i = 0; i < s->num_cpu; i++) {
+if (s->cpu[i].arch_id == arch_id) {
+return &s->cpu[i];
+}
+}
+
+return NULL;
+}
+
static void loongarch_extioi_cpu_plug(HotplugHandler *hotplug_dev,
DeviceState *dev, Error **errp)
{
+LoongArchExtIOICommonState *s = LOONGARCH_EXTIOI_COMMON(hotplug_dev);
Object *obj = OBJECT(dev);
+ExtIOICore *core;
+int pin, index;
if (!object_dynamic_cast(obj, TYPE_LOONGARCH_CPU)) {
warn_report("LoongArch extioi: Invalid %s device type",
object_get_typename(obj));
return;
}
+
+core = loongarch_extioi_get_cpu(s, dev);
+if (!core) {
+return;
+}
+
+core->cpu = CPU(dev);
+index = core - s->cpu;
+
+/*
+ * connect extioi irq to the cpu irq
+ * cpu_pin[LS3A_INTC_IP + 2 : 2] <= intc_pin[LS3A_INTC_IP : 0]
+ */
+for (pin = 0; pin < LS3A_INTC_IP; pin++) {
+qdev_connect_gpio_out(DEVICE(s), index * LS3A_INTC_IP + pin,
+ qdev_get_gpio_in(dev, pin + 2));
+}
}
static void loongarch_extioi_cpu_unplug(HotplugHandler *hotplug_dev,
DeviceState *dev, Error **errp)
{
+LoongArchExtIOICommonState *s = LOONGARCH_EXTIOI_COMMON(hotplug_dev);
Object *obj = OBJECT(dev);
+ExtIOICore *core;
if (!object_dynamic_cast(obj, TYPE_LOONGARCH_CPU)) {
warn_report("LoongArch extioi: Invalid %s device type",
object_get_typename(obj));
return;
}
+
+core = loongarch_extioi_get_cpu(s, dev);
+if (!core) {
+return;
+}
+
+core->cpu = NULL;
}
static void loongarch_extioi_common_realize(DeviceState *dev, Error **errp)
--
2.39.3
[PATCH v2 0/7] hw/loongarch/virt: CPU irq routing enhancement
Interrupt controller ipi and extioi on LoongArch system can send intterrupt to multiple CPUs, physical cpu id is used to route interrupt for CPUs. With cpu hotplug feature in future, notification with ipi and extioi interrupt controller is required. Since there is common Notifier API for CPU hotplug, cpu hotplug interface is added on ipi and extioi class for notification usage. With CPU hotplug event notfication, gpio irq line is connected to cpu irq line, and irq routing for irqchip is setup. --- v1 .. v2: 1. Combine patchset ipi and extioi irq routing enhancement together 2. Rebase patch based on latest version --- Bibo Mao (7): hw/intc/loongarch_ipi: Add basic hotplug framework hw/intc/loongarch_ipi: Implment cpu hotplug interface hw/intc/loongarch_ipi: Notify ipi object when cpu is plugged hw/intc/loongarch_extioi: Move gpio irq initial to common code hw/intc/loongarch_extioi: Add basic hotplug framework hw/intc/loongarch_extioi: Implment cpu hotplug interface hw/intc/loongarch_extioi: Use cpu plug notification hw/intc/loongarch_extioi.c| 8 +-- hw/intc/loongarch_extioi_common.c | 84 ++- hw/intc/loongarch_ipi.c | 71 ++ hw/loongarch/virt.c | 17 ++- 4 files changed, 159 insertions(+), 21 deletions(-) -- 2.39.3
Re: [PATCH v2 05/10] rust: add bindings for memattrs
> +/// A special `MemTxAttrs` constant, used to indicate that no memary
typo... s/memary/memory/
> +/// attributes are specified.
> +///
> +/// Bus masters which don't specify any attributes will get this,
> +/// which has all attribute bits clear except the topmost one
> +/// (so that we can distinguish "all attributes deliberately clear"
> +/// from "didn't specify" if necessary).
> +pub const MEMTXATTRS_UNSPECIFIED: MemTxAttrs = MemTxAttrs {
> +unspecified: true,
> +..Zeroable::ZERO
> +};
Re: [PATCH 10/10] rust: bindings for MemoryRegionOps
On 6/2/25 11:19, Paolo Bonzini wrote: On 2/6/25 11:02, Philippe Mathieu-Daudé wrote: Could we always make .valid_sizes() explicit? Yes (for example build() could even fail to compile if you don't have impl_sizes/valid_sizes set), but why do you want that? I'm not even sure that all cases of .valid.max_access_size=4 are correct... Exactly for that :) Not have implicit default values, so correct values are reviewed when models are added. But I wouldn't bet that those that we have in C are reviewed and correct... They are incorrect if the hardware accepts 8-byte writes, either discarding the top 4 bytes (then impl must both be 8) or writing to both registers (then impl must be 4). Are you saying in general or for the pl011 model? What I'm asking is to have all rust models explicit the min/max sizes, regardless of whether the C implementations are correct or not. For rust models, we won't rely on default and will have to check the valid range in the specs.
Re: [PATCH 4/6] hw/mips/boston: Support dumpdtb monitor commands
On Mon, 10 Feb 2025 at 10:56, Philippe Mathieu-Daudé wrote: > > Hi Peter, > > On 6/2/25 16:12, Peter Maydell wrote: > > -int load_fit(const struct fit_loader *ldr, const char *filename, void > > *opaque); > > +/** > > + * load_fit: load a FIT format image > > + * @ldr: structure defining board specific properties and hooks > > + * @filename: image to load > > + * @pfdt: pointer to update with address of FDT blob > > It is not clear this field is optional or mandatory. > > Looking at other docstrings, optional is not always precised, > and code often consider optional if not precised. Mandatory is > mentioned explicitly. I did go back and forth while I was writing this on whether to make it optional or not (and the versions where it is optional, I had a note in the documentation about that). But there is exactly one caller of this function, and that callsite wants to pass a non-NULL pointer, so I ended up deciding that it was pointless to make the argument optional and include the code to handle "pfdt is NULL". If you get it wrong you'll find out very quickly because your code will segfault :-) Generally we (should) say arguments are optional when they're optional; in those cases we also should document what the behaviour when they're omitted is. So I think "mandatory" is the default. In this function, ldr and filename also are mandatory. If we mark every mandatory argument as "mandatory" then we will end up with a lot of boilerplate markup for most arguments, I think. > Could you update the documentation? Otherwise consider adding > a non-NULL check. > > Either ways: > Reviewed-by: Philippe Mathieu-Daudé thanks -- PMM
Re: [RFC v4 0/5] Add packed virtqueue to shadow virtqueue
Hi, On 2/6/25 8:47 PM, Sahil Siddiq wrote: On 2/6/25 12:42 PM, Eugenio Perez Martin wrote: On Thu, Feb 6, 2025 at 6:26 AM Sahil Siddiq wrote: On 2/4/25 11:45 PM, Eugenio Perez Martin wrote: PS: Please note that you can check packed_vq SVQ implementation already without CVQ, as these features are totally orthogonal :). Right. Now that I can ping with the ctrl features turned off, I think this should take precedence. There's another issue specific to the packed virtqueue case. It causes the kernel to crash. I have been investigating this and the situation here looks very similar to what's explained in Jason Wang's mail [2]. My plan of action is to apply his changes in L2's kernel and check if that resolves the problem. The details of the crash can be found in this mail [3]. If you're testing this series without changes, I think that is caused by not implementing the packed version of vhost_svq_get_buf. https://lists.nongnu.org/archive/html/qemu-devel/2024-12/msg01902.html Oh, apologies, I think I had misunderstood your response in the linked mail. Until now, I thought they were unrelated. In that case, I'll implement the packed version of vhost_svq_get_buf. Hopefully that fixes it :). I noticed one thing while testing some of the changes that I have made. I haven't finished making the relevant changes to all the functions which will have to handle split and packed vq differently. L2's kernel crashes when I launch L0-QEMU with ctrl_vq=on,ctrl_rx=on. However, when I start L0-QEMU with ctrl_vq=off,ctrl_rx=off,ctrl_vlan=off,ctrl_mac_addr=off, L2's kernel boots successfully. Tracing L2-QEMU also confirms that the packed feature is enabled. With all the ctrl features disabled, I think pinging will also be possible once I finish implementing the packed versions of the other functions. There's another thing that I am confused about regarding the current implementation (in the master branch). In hw/virtio/vhost-shadow-virtqueue.c:vhost_svq_vring_write_descs() [1], svq->free_head saves the descriptor in the specified format using "le16_to_cpu" (line 171). On the other hand, the value of i is stored in the native endianness using "cpu_to_le16" (line 168). If "i" is to be stored in the native endianness (little endian in this case), then should svq->free_head first be converted to little endian before being assigned to "i" at the start of the function (line 142)? Thanks, Sahil [1] https://gitlab.com/qemu-project/qemu/-/blob/master/hw/virtio/vhost-shadow-virtqueue.c
Re: [PATCH v3 0/2] include: move include/qapi/qmp/ to include/qobject/
Hi Markus,
These patches seem to have got lost/delayed along the way. Are
you able to send a pull for them soon ?
On Mon, Nov 18, 2024 at 04:12:33PM +0100, Markus Armbruster wrote:
> To repeat the 1st patch commit message...
>
> The general expectation is that header files should follow the same
> file/path naming scheme as the corresponding source file. There are
> various historical exceptions to this practice in QEMU, with one of
> the most notable being the include/qapi/qmp/ directory. Most of the
> headers there correspond to source files in qobject/.
>
> This patch corrects that inconsistency by creating include/qobject/.
> The only outlier is include/qapi/qmp/dispatch.h which gets renamed
> to include/qapi/qmp-registry.h.
>
> Changed in v3:
>
> - Rebased, trivial semantic conflict with commit 34fdd734c5d resolved
>
> - Drop extra blank line [Zhao Liu]
>
> - Instead of doing both the move to qobject/ and the rename of
>qmp/dispatch.h to qmp-registry.h together in one patch per
>top-level directory, do them separately, but tree-wide.
>
> Changed in v2:
>
> - Don't move include/qapi/qmp/qerror.h, as it is not
>the same kind of thing as the other qobject related
>headers, and this header is deprecated & (slowly)
>getting eliminated anyway
>
> - Tacked on two trivial patches removing redundant
>imports of qerror.h. Strictly they're independant
>of this series, so could just go to qemu-trivial
>on their own
>
> Daniel P. Berrangé (2):
> qapi: Move and rename qapi/qmp/dispatch.h to qapi/qmp-registry.h
> qapi: Move include/qapi/qmp/ to include/qobject/
>
> MAINTAINERS | 5 +
> docs/devel/qapi-code-gen.rst| 4 ++--
> include/block/qdict.h | 2 +-
> include/qapi/{qmp/dispatch.h => qmp-registry.h} | 0
> include/{qapi/qmp => qobject}/json-parser.h | 0
> include/{qapi/qmp => qobject}/json-writer.h | 0
> include/{qapi/qmp => qobject}/qbool.h | 2 +-
> include/{qapi/qmp => qobject}/qdict.h | 2 +-
> include/{qapi/qmp => qobject}/qjson.h | 0
> include/{qapi/qmp => qobject}/qlist.h | 2 +-
> include/{qapi/qmp => qobject}/qlit.h| 0
> include/{qapi/qmp => qobject}/qnull.h | 2 +-
> include/{qapi/qmp => qobject}/qnum.h| 2 +-
> include/{qapi/qmp => qobject}/qobject.h | 2 +-
> include/{qapi/qmp => qobject}/qstring.h | 2 +-
> migration/migration.h | 2 +-
> monitor/monitor-internal.h | 4 ++--
> qga/guest-agent-core.h | 2 +-
> qobject/json-parser-int.h | 2 +-
> qobject/qobject-internal.h | 2 +-
> tests/qtest/libqmp.h| 2 +-
> tests/qtest/libqtest.h | 4 ++--
> audio/audio-hmp-cmds.c | 2 +-
> audio/audio.c | 2 +-
> authz/listfile.c| 4 ++--
> backends/cryptodev-hmp-cmds.c | 2 +-
> block.c | 8
> block/blkdebug.c| 6 +++---
> block/blkio.c | 2 +-
> block/blklogwrites.c| 4 ++--
> block/blkverify.c | 4 ++--
> block/copy-before-write.c | 2 +-
> block/copy-on-read.c| 2 +-
> block/curl.c| 4 ++--
> block/file-posix.c | 4 ++--
> block/file-win32.c | 4 ++--
> block/gluster.c | 2 +-
> block/iscsi.c | 4 ++--
> block/monitor/block-hmp-cmds.c | 2 +-
> block/nbd.c | 2 +-
> block/nfs.c | 4 ++--
> block/null.c| 4 ++--
> block/nvme.c| 4 ++--
> block/parallels.c | 2 +-
> block/qapi-sysemu.c | 2 +-
> block/qapi.c| 10 +-
> block/qcow.c| 4 ++--
> block/qcow2.c | 4 ++--
> block/qed.c | 2 +-
> block/quorum.c | 6 +++---
> block/rbd.c | 8
> block/replication.c | 2 +-
> block/snapshot.c| 4 ++--
> block/ssh.c | 4 ++--
> block/stream.c | 2 +-
> block/vhdx.c| 2 +-
> block/vm
[PATCH] linux-user: Move TARGET_SA_RESTORER out of generic/signal.h
SA_RESTORER and the associated sa_restorer field of struct sigaction are an obsolete feature, not expected to be used by future architectures. They are also absent on RISC-V, LoongArch, Hexagon and OpenRISC, but defined due to their use of generic/signal.h. This leads to corrupted data and out-of-bounds accesses. Move the definition of TARGET_SA_RESTORER out of generic/signal.h into the target_signal.h files that need it. Note that m68k has the sa_restorer field, but does not use it and does not define SA_RESTORER. Reported-by: Thomas Weißschuh Signed-off-by: Andreas Schwab --- linux-user/aarch64/target_signal.h| 2 ++ linux-user/arm/target_signal.h| 2 ++ linux-user/generic/signal.h | 1 - linux-user/i386/target_signal.h | 2 ++ linux-user/m68k/target_signal.h | 1 + linux-user/microblaze/target_signal.h | 2 ++ linux-user/ppc/target_signal.h| 2 ++ linux-user/s390x/target_signal.h | 2 ++ linux-user/sh4/target_signal.h| 2 ++ linux-user/x86_64/target_signal.h | 2 ++ linux-user/xtensa/target_signal.h | 2 ++ 11 files changed, 19 insertions(+), 1 deletion(-) diff --git a/linux-user/aarch64/target_signal.h b/linux-user/aarch64/target_signal.h index 40e399d990..6f66a50bfd 100644 --- a/linux-user/aarch64/target_signal.h +++ b/linux-user/aarch64/target_signal.h @@ -3,6 +3,8 @@ #include "../generic/signal.h" +#define TARGET_SA_RESTORER 0x0400 + #define TARGET_SEGV_MTEAERR 8 /* Asynchronous ARM MTE error */ #define TARGET_SEGV_MTESERR 9 /* Synchronous ARM MTE exception */ diff --git a/linux-user/arm/target_signal.h b/linux-user/arm/target_signal.h index 0e6351d9f7..ff1810b1fe 100644 --- a/linux-user/arm/target_signal.h +++ b/linux-user/arm/target_signal.h @@ -3,6 +3,8 @@ #include "../generic/signal.h" +#define TARGET_SA_RESTORER 0x0400 + #define TARGET_ARCH_HAS_SETUP_FRAME #define TARGET_ARCH_HAS_SIGTRAMP_PAGE 1 diff --git a/linux-user/generic/signal.h b/linux-user/generic/signal.h index 6fd05b77bb..b34740258e 100644 --- a/linux-user/generic/signal.h +++ b/linux-user/generic/signal.h @@ -15,7 +15,6 @@ #define TARGET_SA_RESTART 0x1000 #define TARGET_SA_NODEFER 0x4000 #define TARGET_SA_RESETHAND 0x8000 -#define TARGET_SA_RESTORER 0x0400 #define TARGET_SIGHUP1 #define TARGET_SIGINT2 diff --git a/linux-user/i386/target_signal.h b/linux-user/i386/target_signal.h index 9315cba241..eee792ef63 100644 --- a/linux-user/i386/target_signal.h +++ b/linux-user/i386/target_signal.h @@ -3,6 +3,8 @@ #include "../generic/signal.h" +#define TARGET_SA_RESTORER 0x0400 + #define TARGET_ARCH_HAS_SETUP_FRAME #define TARGET_ARCH_HAS_SIGTRAMP_PAGE 1 diff --git a/linux-user/m68k/target_signal.h b/linux-user/m68k/target_signal.h index 6e0f4b74e3..b05b9303b0 100644 --- a/linux-user/m68k/target_signal.h +++ b/linux-user/m68k/target_signal.h @@ -3,6 +3,7 @@ #include "../generic/signal.h" +#define TARGET_ARCH_HAS_SA_RESTORER 1 #define TARGET_ARCH_HAS_SETUP_FRAME #define TARGET_ARCH_HAS_SIGTRAMP_PAGE 1 diff --git a/linux-user/microblaze/target_signal.h b/linux-user/microblaze/target_signal.h index 7dc5c45f00..ffe4442213 100644 --- a/linux-user/microblaze/target_signal.h +++ b/linux-user/microblaze/target_signal.h @@ -3,6 +3,8 @@ #include "../generic/signal.h" +#define TARGET_SA_RESTORER 0x0400 + #define TARGET_ARCH_HAS_SIGTRAMP_PAGE 1 #endif /* MICROBLAZE_TARGET_SIGNAL_H */ diff --git a/linux-user/ppc/target_signal.h b/linux-user/ppc/target_signal.h index 5be24e152b..53fae473f3 100644 --- a/linux-user/ppc/target_signal.h +++ b/linux-user/ppc/target_signal.h @@ -3,6 +3,8 @@ #include "../generic/signal.h" +#define TARGET_SA_RESTORER 0x0400 + #if !defined(TARGET_PPC64) #define TARGET_ARCH_HAS_SETUP_FRAME #endif diff --git a/linux-user/s390x/target_signal.h b/linux-user/s390x/target_signal.h index 41e0e34a55..738e0673f4 100644 --- a/linux-user/s390x/target_signal.h +++ b/linux-user/s390x/target_signal.h @@ -3,6 +3,8 @@ #include "../generic/signal.h" +#define TARGET_SA_RESTORER 0x0400 + #define TARGET_ARCH_HAS_SETUP_FRAME #define TARGET_ARCH_HAS_SIGTRAMP_PAGE 1 diff --git a/linux-user/sh4/target_signal.h b/linux-user/sh4/target_signal.h index eee6a1a7cd..0bde417fd1 100644 --- a/linux-user/sh4/target_signal.h +++ b/linux-user/sh4/target_signal.h @@ -3,6 +3,8 @@ #include "../generic/signal.h" +#define TARGET_SA_RESTORER 0x0400 + #define TARGET_ARCH_HAS_SETUP_FRAME #define TARGET_ARCH_HAS_SIGTRAMP_PAGE 1 diff --git a/linux-user/x86_64/target_signal.h b/linux-user/x86_64/target_signal.h index 9d9717406f..0af100c661 100644 --- a/linux-user/x86_64/target_signal.h +++ b/linux-user/x86_64/target_signal.h @@ -3,6 +3,8 @@ #include "../generic/signal.h" +#define TARGET_SA_RESTORER 0x0400 + /* For x86_64, use of SA_RESTORER is mandatory. */ #define TARGET_ARCH_HAS_SIGTR
Re: [PATCH v4 1/2] s390x/pci: add support for guests that request direct mapping
On 2/10/25 14:12, Niklas Schnelle wrote:
On Fri, 2025-02-07 at 15:56 -0500, Matthew Rosato wrote:
When receiving a guest mpcifc(4) or mpcifc(6) instruction without the T
bit set, treat this as a request to perform direct mapping instead of
address translation. In order to facilitate this, pin the entirety of
guest memory into the host iommu.
Pinning for the direct mapping case is handled via vfio and its memory
listener. Additionally, ram discard settings are inherited from vfio:
coordinated discards (e.g. virtio-mem) are allowed while uncoordinated
discards (e.g. virtio-balloon) are disabled.
Subsequent guest DMA operations are all expected to be of the format
guest_phys+sdma, allowing them to be used as lookup into the host
iommu table.
Signed-off-by: Matthew Rosato
---
hw/s390x/s390-pci-bus.c | 38 +++--
hw/s390x/s390-pci-inst.c| 13 +--
hw/s390x/s390-pci-vfio.c| 23
hw/s390x/s390-virtio-ccw.c | 5 +
include/hw/s390x/s390-pci-bus.h | 4
5 files changed, 75 insertions(+), 8 deletions(-)
---8<---
static const VMStateDescription s390_pci_device_vmstate = {
diff --git a/hw/s390x/s390-pci-inst.c b/hw/s390x/s390-pci-inst.c
index e386d75d58..8cdeb6cb7f 100644
--- a/hw/s390x/s390-pci-inst.c
+++ b/hw/s390x/s390-pci-inst.c
@@ -16,6 +16,7 @@
#include "exec/memory.h"
#include "qemu/error-report.h"
#include "system/hw_accel.h"
+#include "hw/boards.h"
#include "hw/pci/pci_device.h"
#include "hw/s390x/s390-pci-inst.h"
#include "hw/s390x/s390-pci-bus.h"
@@ -1008,17 +1009,25 @@ static int reg_ioat(CPUS390XState *env,
S390PCIBusDevice *pbdev, ZpciFib fib,
}
/* currently we only support designation type 1 with translation */
-if (!(dt == ZPCI_IOTA_RTTO && t)) {
+if (t && dt != ZPCI_IOTA_RTTO) {
error_report("unsupported ioat dt %d t %d", dt, t);
s390_program_interrupt(env, PGM_OPERAND, ra);
return -EINVAL;
+} else if (!t && !pbdev->rtr_avail) {
+error_report("relaxed translation not allowed");
+s390_program_interrupt(env, PGM_OPERAND, ra);
+return -EINVAL;
}
iommu->pba = pba;
iommu->pal = pal;
iommu->g_iota = g_iota;
-s390_pci_iommu_enable(iommu);
+if (t) {
+s390_pci_iommu_enable(iommu);
+} else {
+s390_pci_iommu_direct_map_enable(iommu);
+}
return 0;
}
diff --git a/hw/s390x/s390-pci-vfio.c b/hw/s390x/s390-pci-vfio.c
index 7dbbc76823..443e222912 100644
--- a/hw/s390x/s390-pci-vfio.c
+++ b/hw/s390x/s390-pci-vfio.c
@@ -131,13 +131,28 @@ static void s390_pci_read_base(S390PCIBusDevice *pbdev,
/* Store function type separately for type-specific behavior */
pbdev->pft = cap->pft;
+/*
+ * If the device is a passthrough ISM device, disallow relaxed
+ * translation.
+ */
+if (pbdev->pft == ZPCI_PFT_ISM) {
+pbdev->rtr_avail = false;
+}
Just a note for external readers. The ISM device does work without the
above but having explicit guest IOMMU mappings plus the respective
shadow mappings in the host would catch driver misbehavior more easily.
At the same time ISM uses long lived IOMMU mappings so the cost of
shadowing its mappings is low.
+
/*
* If appropriate, reduce the size of the supported DMA aperture reported
- * to the guest based upon the vfio DMA limit.
+ * to the guest based upon the vfio DMA limit. This is applicable for
+ * devices that are guaranteed to not use relaxed translation. If the
+ * device is capable of relaxed translation then we must advertise the
+ * full aperture. In this case, if translation is used then we will
+ * rely on the vfio DMA limit counting and use RPCIT CC1 / status 16
+ * to request that the guest free DMA mappings as necessary.
*/
-vfio_size = pbdev->iommu->max_dma_limit << TARGET_PAGE_BITS;
-if (vfio_size > 0 && vfio_size < cap->end_dma - cap->start_dma + 1) {
-pbdev->zpci_fn.edma = cap->start_dma + vfio_size - 1;
+if (!pbdev->rtr_avail) {
+vfio_size = pbdev->iommu->max_dma_limit << TARGET_PAGE_BITS;
+if (vfio_size > 0 && vfio_size < cap->end_dma - cap->start_dma + 1) {
+pbdev->zpci_fn.edma = cap->start_dma + vfio_size - 1;
+}
}
}
diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index d9e683c5b4..6a6cb39808 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -937,8 +937,13 @@ static void ccw_machine_9_2_instance_options(MachineState
*machine)
static void ccw_machine_9_2_class_options(MachineClass *mc)
{
+static GlobalProperty compat[] = {
+{ TYPE_S390_PCI_DEVICE, "relaxed-translation", "off", },
+};
+
ccw_machine_10_0_class_options(mc);
compat_props_add(mc->compat_props, hw_compat_9_2, hw_compat_9_2_len);
+compat_props_add(mc->compat_props, compat, G_
Re: [PATCH 00/10] qom: Constify class_data
On 10/2/25 13:30, Paolo Bonzini wrote:
On 2/10/25 11:25, Philippe Mathieu-Daudé wrote:
Following Richard's suggestion [*], make QOM class data *const*.
Note, rust code not modified...
Untested but it should be something like
diff --git a/rust/qemu-api/src/qom.rs b/rust/qemu-api/src/qom.rs
index f36be2831eb..8603f7cc657 100644
--- a/rust/qemu-api/src/qom.rs
+++ b/rust/qemu-api/src/qom.rs
@@ -180,7 +180,7 @@ fn fmt(&self, f: &mut fmt::Formatter<'_>) ->
Result<(), fmt::Error> {
unsafe extern "C" fn rust_class_initClassInitImpl>(
klass: *mut ObjectClass,
- _data: *mut c_void,
+ _data: *const c_void,
) {
let mut klass = NonNull::new(klass)
.unwrap()
@@ -523,7 +523,7 @@ pub trait ObjectImpl: ObjectType +
ClassInitImpl {
/// the effects of copying the contents of the parent's class struct
/// to the descendants.
const CLASS_BASE_INIT: Option<
- unsafe extern "C" fn(klass: *mut ObjectClass, data: *mut c_void),
+ unsafe extern "C" fn(klass: *mut ObjectClass, data: *const
c_void),
> = None;
const TYPE_INFO: TypeInfo = TypeInfo {
@@ -544,7 +544,7 @@ pub trait ObjectImpl: ObjectType +
ClassInitImpl {
class_size: core::mem::size_of::(),
class_init: Some(rust_class_init::),
class_base_init: Self::CLASS_BASE_INIT,
- class_data: core::ptr::null_mut(),
+ class_data: core::ptr::null(),
interfaces: core::ptr::null_mut(),
};
to be split across patches 8-10.
Thanks!
Re: [PATCH v4 2/2] s390x/pci: indicate QEMU supports relaxed translation for passthrough
On Fri, 2025-02-07 at 15:56 -0500, Matthew Rosato wrote:
> Specifying this bit in the guest CLP response indicates that the guest
> can optionally choose to skip translation and instead use
> identity-mapped operations.
>
> Signed-off-by: Matthew Rosato
> ---
> hw/s390x/s390-pci-vfio.c| 5 -
> include/hw/s390x/s390-pci-clp.h | 1 +
> 2 files changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/hw/s390x/s390-pci-vfio.c b/hw/s390x/s390-pci-vfio.c
> index 443e222912..6236ac7f1e 100644
> --- a/hw/s390x/s390-pci-vfio.c
> +++ b/hw/s390x/s390-pci-vfio.c
> @@ -238,8 +238,11 @@ static void s390_pci_read_group(S390PCIBusDevice *pbdev,
> pbdev->pci_group = s390_group_create(pbdev->zpci_fn.pfgid,
> start_gid);
>
> resgrp = &pbdev->pci_group->zpci_group;
> +if (pbdev->rtr_avail) {
> +resgrp->fr |= CLP_RSP_QPCIG_MASK_RTR;
> +}
> if (cap->flags & VFIO_DEVICE_INFO_ZPCI_FLAG_REFRESH) {
> -resgrp->fr = 1;
> +resgrp->fr |= CLP_RSP_QPCIG_MASK_REFRESH;
> }
> resgrp->dasm = cap->dasm;
> resgrp->msia = cap->msi_addr;
> diff --git a/include/hw/s390x/s390-pci-clp.h b/include/hw/s390x/s390-pci-clp.h
> index 03b7f9ba5f..6a635d693b 100644
> --- a/include/hw/s390x/s390-pci-clp.h
> +++ b/include/hw/s390x/s390-pci-clp.h
> @@ -158,6 +158,7 @@ typedef struct ClpRspQueryPciGrp {
> #define CLP_RSP_QPCIG_MASK_NOI 0xfff
> uint16_t i;
> uint8_t version;
> +#define CLP_RSP_QPCIG_MASK_RTR 0x20
> #define CLP_RSP_QPCIG_MASK_FRAME 0x2
> #define CLP_RSP_QPCIG_MASK_REFRESH 0x1
> uint8_t fr;
Looks good to me!
Tested-by: Niklas Schnelle
Reviewed-by: Niklas Schnelle
[PATCH v2 00/11] qom: Constify class_data
Since v1: - Make XtensaConfigList::config not const (Max) - Update / test rust (Paolo) - Constify InterfaceInfo[] Following Richard's suggestion [*], make QOM class data *const*. [*] https://lore.kernel.org/qemu-devel/f4ec871d-e759-44bc-a10b-872322330...@linaro.org/ Philippe Mathieu-Daudé (11): target/i386: Constify X86CPUModel uses target/sparc: Constify SPARCCPUClass::cpu_def target/xtensa: Finalize config in xtensa_register_core() target/riscv: Declare RISCVCPUClass::misa_mxl_max as RISCVMXL target/riscv: Convert misa_mxl_max using GLib macros hw: Declare various const data as 'const' hw: Make class data 'const' qom: Have class_base_init() take a const data argument qom: Have class_init() take a const data argument qom: Constify TypeInfo::class_data qom: Constify InterfaceInfo[] interfaces docs/devel/qom.rst| 8 +- docs/devel/reset.rst | 2 +- docs/devel/virtio-backends.rst| 2 +- hw/sd/sdhci-internal.h| 2 +- hw/usb/hcd-uhci.h | 2 +- include/hw/boards.h | 2 +- include/hw/core/cpu.h | 2 +- include/hw/i386/pc.h | 5 +- include/hw/virtio/virtio-pci.h| 4 +- include/qom/object.h | 12 +-- target/arm/cpu.h | 2 +- target/i386/cpu.h | 2 +- target/ppc/cpu.h | 3 +- target/riscv/cpu.h| 2 +- target/sparc/cpu.h| 2 +- target/xtensa/cpu.h | 2 +- accel/hvf/hvf-accel-ops.c | 4 +- accel/kvm/kvm-accel-ops.c | 2 +- accel/kvm/kvm-all.c | 2 +- accel/qtest/qtest.c | 4 +- accel/tcg/tcg-accel-ops.c | 2 +- accel/tcg/tcg-all.c | 2 +- accel/xen/xen-all.c | 4 +- authz/list.c | 4 +- authz/listfile.c | 4 +- authz/pamacct.c | 4 +- authz/simple.c| 4 +- backends/confidential-guest-support.c | 3 +- backends/cryptodev-builtin.c | 2 +- backends/cryptodev-lkcf.c | 2 +- backends/cryptodev-vhost-user.c | 2 +- backends/cryptodev.c | 4 +- backends/dbus-vmstate.c | 4 +- backends/host_iommu_device.c | 2 +- backends/hostmem-epc.c| 2 +- backends/hostmem-file.c | 2 +- backends/hostmem-memfd.c | 2 +- backends/hostmem-ram.c| 2 +- backends/hostmem-shm.c| 2 +- backends/hostmem.c| 4 +- backends/iommufd.c| 6 +- backends/rng-builtin.c| 2 +- backends/rng-egd.c| 2 +- backends/rng-random.c | 2 +- backends/rng.c| 4 +- backends/tpm/tpm_emulator.c | 2 +- backends/tpm/tpm_passthrough.c| 2 +- backends/vhost-user.c | 2 +- block/throttle-groups.c | 5 +- chardev/baum.c| 2 +- chardev/char-console.c| 2 +- chardev/char-fd.c | 2 +- chardev/char-file.c | 2 +- chardev/char-hub.c| 2 +- chardev/char-mux.c| 2 +- chardev/char-null.c | 2 +- chardev/char-parallel.c | 2 +- chardev/char-pipe.c | 2 +- chardev/char-pty.c| 2 +- chardev/char-ringbuf.c| 2 +- chardev/char-serial.c | 2 +- chardev/char-socket.c | 2 +- chardev/char-stdio.c | 2 +- chardev/char-udp.c| 2 +- chardev/char-win-stdio.c | 2 +- chardev/char-win.c| 2 +- chardev/char.c| 2 +- chardev/msmouse.c | 2 +- chardev/spice.c | 6 +- chardev/testdev.c | 2 +- chardev/wctablet.c| 2 +- crypto/secret.c | 2 +- crypto/secret_common.c| 4 +- crypto/secret_keyring.c | 2 +- crypto/tls-cipher-suites.c| 5 +- crypto/tlscreds.c
[PATCH v2 05/11] target/riscv: Convert misa_mxl_max using GLib macros
Use GLib conversion macros to pass misa_mxl_max as
riscv_cpu_class_init() class data.
Signed-off-by: Philippe Mathieu-Daudé
---
Cc: qemu-ri...@nongnu.org
---
target/riscv/cpu.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index f3ad7f88f0e..9fe1b23a297 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -2955,7 +2955,7 @@ static void riscv_cpu_class_init(ObjectClass *c, void
*data)
{
RISCVCPUClass *mcc = RISCV_CPU_CLASS(c);
-mcc->misa_mxl_max = (RISCVMXL)(uintptr_t)data;
+mcc->misa_mxl_max = (RISCVMXL)GPOINTER_TO_UINT(data);
riscv_cpu_validate_misa_mxl(mcc);
}
@@ -3057,7 +3057,7 @@ void riscv_isa_write_fdt(RISCVCPU *cpu, void *fdt, char
*nodename)
.parent = TYPE_RISCV_CPU, \
.instance_init = (initfn), \
.class_init = riscv_cpu_class_init, \
-.class_data = (void *)(misa_mxl_max)\
+.class_data = GUINT_TO_POINTER(misa_mxl_max)\
}
#define DEFINE_DYNAMIC_CPU(type_name, misa_mxl_max, initfn) \
@@ -3066,7 +3066,7 @@ void riscv_isa_write_fdt(RISCVCPU *cpu, void *fdt, char
*nodename)
.parent = TYPE_RISCV_DYNAMIC_CPU, \
.instance_init = (initfn), \
.class_init = riscv_cpu_class_init, \
-.class_data = (void *)(misa_mxl_max)\
+.class_data = GUINT_TO_POINTER(misa_mxl_max)\
}
#define DEFINE_VENDOR_CPU(type_name, misa_mxl_max, initfn) \
@@ -3075,7 +3075,7 @@ void riscv_isa_write_fdt(RISCVCPU *cpu, void *fdt, char
*nodename)
.parent = TYPE_RISCV_VENDOR_CPU,\
.instance_init = (initfn), \
.class_init = riscv_cpu_class_init, \
-.class_data = (void *)(misa_mxl_max)\
+.class_data = GUINT_TO_POINTER(misa_mxl_max)\
}
#define DEFINE_BARE_CPU(type_name, misa_mxl_max, initfn)\
@@ -3084,7 +3084,7 @@ void riscv_isa_write_fdt(RISCVCPU *cpu, void *fdt, char
*nodename)
.parent = TYPE_RISCV_BARE_CPU, \
.instance_init = (initfn), \
.class_init = riscv_cpu_class_init, \
-.class_data = (void *)(misa_mxl_max)\
+.class_data = GUINT_TO_POINTER(misa_mxl_max)\
}
#define DEFINE_PROFILE_CPU(type_name, misa_mxl_max, initfn) \
@@ -3093,7 +3093,7 @@ void riscv_isa_write_fdt(RISCVCPU *cpu, void *fdt, char
*nodename)
.parent = TYPE_RISCV_BARE_CPU, \
.instance_init = (initfn), \
.class_init = riscv_cpu_class_init, \
-.class_data = (void *)(misa_mxl_max)\
+.class_data = GUINT_TO_POINTER(misa_mxl_max)\
}
static const TypeInfo riscv_cpu_type_infos[] = {
--
2.47.1
[PATCH v2 06/11] hw: Declare various const data as 'const'
Signed-off-by: Philippe Mathieu-Daudé
---
hw/isa/vt82c686.c | 2 +-
hw/rtc/m48t59-isa.c | 2 +-
hw/rtc/m48t59.c | 2 +-
hw/sensor/tmp421.c| 2 +-
hw/usb/hcd-ehci-pci.c | 2 +-
hw/usb/hcd-uhci.c | 2 +-
6 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
index 6f44b381a5f..43bd67eeef2 100644
--- a/hw/isa/vt82c686.c
+++ b/hw/isa/vt82c686.c
@@ -224,7 +224,7 @@ static void via_pm_class_init(ObjectClass *klass, void
*data)
{
DeviceClass *dc = DEVICE_CLASS(klass);
PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
-ViaPMInitInfo *info = data;
+const ViaPMInitInfo *info = data;
k->realize = via_pm_realize;
k->config_write = pm_write_config;
diff --git a/hw/rtc/m48t59-isa.c b/hw/rtc/m48t59-isa.c
index 38bc8dcf100..9c3855a3ef1 100644
--- a/hw/rtc/m48t59-isa.c
+++ b/hw/rtc/m48t59-isa.c
@@ -129,7 +129,7 @@ static void m48txx_isa_class_init(ObjectClass *klass, void
*data)
static void m48txx_isa_concrete_class_init(ObjectClass *klass, void *data)
{
M48txxISADeviceClass *u = M48TXX_ISA_CLASS(klass);
-M48txxInfo *info = data;
+const M48txxInfo *info = data;
u->info = *info;
}
diff --git a/hw/rtc/m48t59.c b/hw/rtc/m48t59.c
index c9bd6f878fe..3fb2f27d9d1 100644
--- a/hw/rtc/m48t59.c
+++ b/hw/rtc/m48t59.c
@@ -639,7 +639,7 @@ static void m48txx_sysbus_class_init(ObjectClass *klass,
void *data)
static void m48txx_sysbus_concrete_class_init(ObjectClass *klass, void *data)
{
M48txxSysBusDeviceClass *u = M48TXX_SYS_BUS_CLASS(klass);
-M48txxInfo *info = data;
+const M48txxInfo *info = data;
u->info = *info;
}
diff --git a/hw/sensor/tmp421.c b/hw/sensor/tmp421.c
index 82e604279c5..007f7cd018b 100644
--- a/hw/sensor/tmp421.c
+++ b/hw/sensor/tmp421.c
@@ -68,7 +68,7 @@ struct TMP421State {
struct TMP421Class {
I2CSlaveClass parent_class;
-DeviceInfo *dev;
+const DeviceInfo *dev;
};
#define TYPE_TMP421 "tmp421-generic"
diff --git a/hw/usb/hcd-ehci-pci.c b/hw/usb/hcd-ehci-pci.c
index d410c38a8a2..e00316721ac 100644
--- a/hw/usb/hcd-ehci-pci.c
+++ b/hw/usb/hcd-ehci-pci.c
@@ -182,7 +182,7 @@ static void ehci_data_class_init(ObjectClass *klass, void
*data)
{
PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
DeviceClass *dc = DEVICE_CLASS(klass);
-EHCIPCIInfo *i = data;
+const EHCIPCIInfo *i = data;
k->vendor_id = i->vendor_id;
k->device_id = i->device_id;
diff --git a/hw/usb/hcd-uhci.c b/hw/usb/hcd-uhci.c
index 8528d493d63..0561a6d801a 100644
--- a/hw/usb/hcd-uhci.c
+++ b/hw/usb/hcd-uhci.c
@@ -1289,7 +1289,7 @@ void uhci_data_class_init(ObjectClass *klass, void *data)
PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
DeviceClass *dc = DEVICE_CLASS(klass);
UHCIPCIDeviceClass *u = UHCI_CLASS(klass);
-UHCIInfo *info = data;
+const UHCIInfo *info = data;
k->realize = info->realize ? info->realize : usb_uhci_common_realize;
k->exit = info->unplug ? usb_uhci_exit : NULL;
--
2.47.1
[PATCH v2 01/11] target/i386: Constify X86CPUModel uses
Signed-off-by: Philippe Mathieu-Daudé
---
target/i386/cpu.h | 2 +-
target/i386/cpu.c | 8
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index c67b42d34fc..f9ce6970ee1 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -2288,7 +2288,7 @@ struct X86CPUClass {
* CPU definition, automatically loaded by instance_init if not NULL.
* Should be eventually replaced by subclass-specific property defaults.
*/
-X86CPUModel *model;
+const X86CPUModel *model;
bool host_cpuid_required;
int ordering;
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index b5dd60d2812..4b2da45366b 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -6436,7 +6436,7 @@ void x86_cpu_apply_props(X86CPU *cpu, PropValue *props)
* Only for builtin_x86_defs models initialized with x86_register_cpudef_types.
*/
-static void x86_cpu_apply_version_props(X86CPU *cpu, X86CPUModel *model)
+static void x86_cpu_apply_version_props(X86CPU *cpu, const X86CPUModel *model)
{
const X86CPUVersionDefinition *vdef;
X86CPUVersion version = x86_cpu_model_resolve_version(model);
@@ -6465,7 +6465,7 @@ static void x86_cpu_apply_version_props(X86CPU *cpu,
X86CPUModel *model)
}
static const CPUCaches *x86_cpu_get_versioned_cache_info(X86CPU *cpu,
- X86CPUModel *model)
+ const X86CPUModel
*model)
{
const X86CPUVersionDefinition *vdef;
X86CPUVersion version = x86_cpu_model_resolve_version(model);
@@ -6493,7 +6493,7 @@ static const CPUCaches
*x86_cpu_get_versioned_cache_info(X86CPU *cpu,
* Load data from X86CPUDefinition into a X86CPU object.
* Only for builtin_x86_defs models initialized with x86_register_cpudef_types.
*/
-static void x86_cpu_load_model(X86CPU *cpu, X86CPUModel *model)
+static void x86_cpu_load_model(X86CPU *cpu, const X86CPUModel *model)
{
const X86CPUDefinition *def = model->cpudef;
CPUX86State *env = &cpu->env;
@@ -6563,7 +6563,7 @@ static const gchar *x86_gdb_arch_name(CPUState *cs)
static void x86_cpu_cpudef_class_init(ObjectClass *oc, void *data)
{
-X86CPUModel *model = data;
+const X86CPUModel *model = data;
X86CPUClass *xcc = X86_CPU_CLASS(oc);
CPUClass *cc = CPU_CLASS(oc);
--
2.47.1
[PATCH v2 02/11] target/sparc: Constify SPARCCPUClass::cpu_def
Signed-off-by: Philippe Mathieu-Daudé
---
target/sparc/cpu.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/target/sparc/cpu.h b/target/sparc/cpu.h
index dda811503b5..462bcb6c0e6 100644
--- a/target/sparc/cpu.h
+++ b/target/sparc/cpu.h
@@ -574,7 +574,7 @@ struct SPARCCPUClass {
DeviceRealize parent_realize;
ResettablePhases parent_phases;
-sparc_def_t *cpu_def;
+const sparc_def_t *cpu_def;
};
#ifndef CONFIG_USER_ONLY
--
2.47.1
[PATCH v2 11/11] qom: Constify InterfaceInfo[] interfaces
Mechanical change using gsed. Signed-off-by: Philippe Mathieu-Daudé --- include/hw/virtio/virtio-pci.h | 2 +- include/qom/object.h| 4 ++-- authz/list.c| 2 +- authz/listfile.c| 2 +- authz/pamacct.c | 2 +- authz/simple.c | 2 +- backends/cryptodev.c| 2 +- backends/dbus-vmstate.c | 2 +- backends/hostmem.c | 2 +- backends/iommufd.c | 2 +- backends/rng.c | 2 +- block/throttle-groups.c | 2 +- crypto/secret_common.c | 2 +- crypto/tls-cipher-suites.c | 2 +- crypto/tlscredsanon.c | 2 +- crypto/tlscredspsk.c| 2 +- crypto/tlscredsx509.c | 2 +- event-loop-base.c | 2 +- hw/acpi/erst.c | 2 +- hw/acpi/generic_event_device.c | 2 +- hw/acpi/piix4.c | 2 +- hw/arm/armsse.c | 2 +- hw/arm/mps2-tz.c| 2 +- hw/arm/virt.c | 2 +- hw/audio/ac97.c | 2 +- hw/audio/es1370.c | 2 +- hw/audio/intel-hda.c| 2 +- hw/audio/via-ac97.c | 4 ++-- hw/block/fdc-isa.c | 2 +- hw/char/diva-gsp.c | 4 ++-- hw/char/parallel.c | 2 +- hw/char/serial-isa.c| 2 +- hw/char/serial-pci-multi.c | 4 ++-- hw/char/serial-pci.c| 2 +- hw/char/virtio-serial-bus.c | 2 +- hw/core/bus.c | 2 +- hw/core/qdev.c | 2 +- hw/cxl/switch-mailbox-cci.c | 2 +- hw/display/ati.c| 2 +- hw/display/bochs-display.c | 2 +- hw/display/cirrus_vga.c | 2 +- hw/display/qxl.c| 2 +- hw/display/sm501.c | 2 +- hw/display/vga-pci.c| 2 +- hw/display/virtio-gpu-pci-rutabaga.c| 2 +- hw/display/vmware_vga.c | 2 +- hw/dma/i8257.c | 2 +- hw/dma/xilinx_axidma.c | 4 ++-- hw/dma/xlnx_csu_dma.c | 2 +- hw/hppa/machine.c | 4 ++-- hw/i2c/smbus_ich9.c | 2 +- hw/i386/amd_iommu.c | 2 +- hw/i386/microvm.c | 2 +- hw/i386/pc.c| 2 +- hw/i386/sgx-epc.c | 2 +- hw/i386/x86.c | 2 +- hw/i386/xen/xen_platform.c | 2 +- hw/i386/xen/xen_pvdevice.c | 2 +- hw/ide/ich.c| 2 +- hw/ide/pci.c| 2 +- hw/input/pckbd.c| 2 +- hw/intc/arm_gic_common.c| 2 +- hw/intc/arm_gicv3_common.c | 2 +- hw/intc/goldfish_pic.c | 2 +- hw/intc/i8259_common.c | 2 +- hw/intc/ioapic_common.c | 2 +- hw/intc/m68k_irqc.c | 2 +- hw/intc/pnv_xive.c | 2 +- hw/intc/pnv_xive2.c | 2 +- hw/intc/slavio_intctl.c | 2 +- hw/intc/spapr_xive.c| 2 +- hw/intc/xics_spapr.c| 2 +- hw/intc/xive.c | 2 +- hw/intc/xive2.c | 2 +- hw/ipack/tpci200.c | 2 +- hw/ipmi/isa_ipmi_bt.c | 2 +- hw/ipmi/isa_ipmi_kcs.c | 2 +- hw/ipmi/pci_ipmi_bt.c | 2 +- hw/ipmi/pci_ipmi_kcs.c | 2 +- hw/ipmi/smbus_ipmi.c| 2 +- hw/isa/i82378.c
[PATCH v2 03/11] target/xtensa: Finalize config in xtensa_register_core()
Make XtensaConfigList::config not const. Only modify
XtensaConfig within xtensa_register_core(), when the
class is registered, not when it is initialized.
Signed-off-by: Philippe Mathieu-Daudé
---
Cc: Max Filippov
---
target/xtensa/cpu.h| 2 +-
target/xtensa/helper.c | 5 +++--
2 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/target/xtensa/cpu.h b/target/xtensa/cpu.h
index 0e6302c5bd3..8d70bfc0cd4 100644
--- a/target/xtensa/cpu.h
+++ b/target/xtensa/cpu.h
@@ -490,7 +490,7 @@ typedef struct XtensaConfig {
} XtensaConfig;
typedef struct XtensaConfigList {
-const XtensaConfig *config;
+XtensaConfig *config;
struct XtensaConfigList *next;
} XtensaConfigList;
diff --git a/target/xtensa/helper.c b/target/xtensa/helper.c
index 2978c471c1f..f64699b116d 100644
--- a/target/xtensa/helper.c
+++ b/target/xtensa/helper.c
@@ -173,9 +173,8 @@ static void xtensa_core_class_init(ObjectClass *oc, void
*data)
{
CPUClass *cc = CPU_CLASS(oc);
XtensaCPUClass *xcc = XTENSA_CPU_CLASS(oc);
-XtensaConfig *config = data;
+const XtensaConfig *config = data;
-xtensa_finalize_config(config);
xcc->config = config;
/*
@@ -195,6 +194,8 @@ void xtensa_register_core(XtensaConfigList *node)
.class_data = (void *)node->config,
};
+xtensa_finalize_config(node->config);
+
node->next = xtensa_cores;
xtensa_cores = node;
type.name = g_strdup_printf(XTENSA_CPU_TYPE_NAME("%s"),
node->config->name);
--
2.47.1
[PATCH v2 04/11] target/riscv: Declare RISCVCPUClass::misa_mxl_max as RISCVMXL
Signed-off-by: Philippe Mathieu-Daudé
---
Cc: qemu-ri...@nongnu.org
---
target/riscv/cpu.h | 2 +-
target/riscv/cpu.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 97713681cbe..fbe5548cf5a 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -529,7 +529,7 @@ struct RISCVCPUClass {
DeviceRealize parent_realize;
ResettablePhases parent_phases;
-uint32_t misa_mxl_max; /* max mxl for this cpu */
+RISCVMXL misa_mxl_max; /* max mxl for this cpu */
};
static inline int riscv_has_ext(CPURISCVState *env, target_ulong ext)
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 3d4bd157d2c..f3ad7f88f0e 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -2955,7 +2955,7 @@ static void riscv_cpu_class_init(ObjectClass *c, void
*data)
{
RISCVCPUClass *mcc = RISCV_CPU_CLASS(c);
-mcc->misa_mxl_max = (uint32_t)(uintptr_t)data;
+mcc->misa_mxl_max = (RISCVMXL)(uintptr_t)data;
riscv_cpu_validate_misa_mxl(mcc);
}
--
2.47.1
[PATCH v2 10/11] qom: Constify TypeInfo::class_data
All callers now correctly expect a const class data.
Signed-off-by: Philippe Mathieu-Daudé
---
include/qom/object.h| 2 +-
hw/arm/armsse.c | 2 +-
hw/block/m25p80.c | 2 +-
hw/isa/vt82c686.c | 4 ++--
hw/net/e1000.c | 2 +-
hw/ppc/spapr_cpu_core.c | 2 +-
hw/scsi/megasas.c | 2 +-
hw/sensor/tmp421.c | 2 +-
hw/virtio/virtio-pci.c | 4 ++--
qom/object.c| 2 +-
target/arm/cpu.c| 2 +-
target/arm/cpu64.c | 2 +-
target/mips/cpu.c | 2 +-
target/s390x/cpu_models.c | 4 ++--
target/sparc/cpu.c | 2 +-
target/xtensa/helper.c | 2 +-
rust/qemu-api/src/qom.rs| 2 +-
scripts/codeconverter/codeconverter/test_regexps.py | 2 +-
18 files changed, 21 insertions(+), 21 deletions(-)
diff --git a/include/qom/object.h b/include/qom/object.h
index 2fb86f00a68..42b75d10a43 100644
--- a/include/qom/object.h
+++ b/include/qom/object.h
@@ -488,7 +488,7 @@ struct TypeInfo
void (*class_init)(ObjectClass *klass, const void *data);
void (*class_base_init)(ObjectClass *klass, const void *data);
-void *class_data;
+const void *class_data;
InterfaceInfo *interfaces;
};
diff --git a/hw/arm/armsse.c b/hw/arm/armsse.c
index d65a46b8d8d..9403b65ddb5 100644
--- a/hw/arm/armsse.c
+++ b/hw/arm/armsse.c
@@ -1730,7 +1730,7 @@ static void armsse_register_types(void)
.name = armsse_variants[i].name,
.parent = TYPE_ARM_SSE,
.class_init = armsse_class_init,
-.class_data = (void *)&armsse_variants[i],
+.class_data = &armsse_variants[i],
};
type_register_static(&ti);
}
diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
index 236fa798c34..eee7bedd6b3 100644
--- a/hw/block/m25p80.c
+++ b/hw/block/m25p80.c
@@ -1891,7 +1891,7 @@ static void m25p80_register_types(void)
.name = known_devices[i].part_name,
.parent = TYPE_M25P80,
.class_init = m25p80_class_init,
-.class_data = (void *)&known_devices[i],
+.class_data = &known_devices[i],
};
type_register_static(&ti);
}
diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
index 80366aaf647..c62afc907b2 100644
--- a/hw/isa/vt82c686.c
+++ b/hw/isa/vt82c686.c
@@ -259,7 +259,7 @@ static const TypeInfo vt82c686b_pm_info = {
.name = TYPE_VT82C686B_PM,
.parent= TYPE_VIA_PM,
.class_init= via_pm_class_init,
-.class_data= (void *)&vt82c686b_pm_init_info,
+.class_data= &vt82c686b_pm_init_info,
};
static const ViaPMInitInfo vt8231_pm_init_info = {
@@ -272,7 +272,7 @@ static const TypeInfo vt8231_pm_info = {
.name = TYPE_VT8231_PM,
.parent= TYPE_VIA_PM,
.class_init= via_pm_class_init,
-.class_data= (void *)&vt8231_pm_init_info,
+.class_data= &vt8231_pm_init_info,
};
diff --git a/hw/net/e1000.c b/hw/net/e1000.c
index d49730f4ad4..13814e84d18 100644
--- a/hw/net/e1000.c
+++ b/hw/net/e1000.c
@@ -1770,7 +1770,7 @@ static void e1000_register_types(void)
type_info.name = info->name;
type_info.parent = TYPE_E1000_BASE;
-type_info.class_data = (void *)info;
+type_info.class_data = info;
type_info.class_init = e1000_class_init;
type_register_static(&type_info);
diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
index c1964d3dc8a..e1929a546a3 100644
--- a/hw/ppc/spapr_cpu_core.c
+++ b/hw/ppc/spapr_cpu_core.c
@@ -380,7 +380,7 @@ static void spapr_cpu_core_class_init(ObjectClass *oc,
const void *data)
#define DEFINE_SPAPR_CPU_CORE_TYPE(cpu_model) \
{ \
.parent = TYPE_SPAPR_CPU_CORE, \
-.class_data = (void *) POWERPC_CPU_TYPE_NAME(cpu_model), \
+.class_data = POWERPC_CPU_TYPE_NAME(cpu_model), \
.class_init = spapr_cpu_core_class_init,\
.name = SPAPR_CPU_CORE_TYPE_NAME(cpu_model),\
}
diff --git a/hw/scsi/megasas.c b/hw/scsi/megasas.c
index cfa5516b96c..6104d4202aa 100644
--- a/hw/scsi/megasas.c
+++ b/hw/scsi/megasas.c
@@ -2573,7 +2573,7 @@ static void megasas_register_types(void)
type_info.name = info->name;
type_info.parent = TYPE_MEGASAS_BASE;
-type_info.class_data = (void *)info;
+type_info.class_data = info;
type_info.class_init = megasas_class_init;
type_info.interfaces = info->interfaces;
diff --git a/hw/sensor/t
[PATCH v2 08/11] qom: Have class_base_init() take a const data argument
Signed-off-by: Philippe Mathieu-Daudé
---
include/qom/object.h | 2 +-
hw/core/machine.c| 2 +-
hw/core/qdev.c | 2 +-
hw/pci/pci.c | 2 +-
qom/object.c | 2 +-
rust/qemu-api/src/qom.rs | 2 +-
6 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/include/qom/object.h b/include/qom/object.h
index 9192265db76..7bb14ca7067 100644
--- a/include/qom/object.h
+++ b/include/qom/object.h
@@ -487,7 +487,7 @@ struct TypeInfo
size_t class_size;
void (*class_init)(ObjectClass *klass, void *data);
-void (*class_base_init)(ObjectClass *klass, void *data);
+void (*class_base_init)(ObjectClass *klass, const void *data);
void *class_data;
InterfaceInfo *interfaces;
diff --git a/hw/core/machine.c b/hw/core/machine.c
index 254cc20c4cb..7bdde9286c2 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -1236,7 +1236,7 @@ static void machine_class_init(ObjectClass *oc, void
*data)
"Memory size configuration");
}
-static void machine_class_base_init(ObjectClass *oc, void *data)
+static void machine_class_base_init(ObjectClass *oc, const void *data)
{
MachineClass *mc = MACHINE_CLASS(oc);
mc->max_cpus = mc->max_cpus ?: 1;
diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 82bbdcb654e..54578299147 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -693,7 +693,7 @@ static void device_finalize(Object *obj)
g_free(dev->id);
}
-static void device_class_base_init(ObjectClass *class, void *data)
+static void device_class_base_init(ObjectClass *class, const void *data)
{
DeviceClass *klass = DEVICE_CLASS(class);
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 2afa423925c..00f50e6f2cc 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -2712,7 +2712,7 @@ static void pci_device_class_init(ObjectClass *klass,
void *data)
"access to indirect DMA memory");
}
-static void pci_device_class_base_init(ObjectClass *klass, void *data)
+static void pci_device_class_base_init(ObjectClass *klass, const void *data)
{
if (!object_class_is_abstract(klass)) {
ObjectClass *conventional =
diff --git a/qom/object.c b/qom/object.c
index ec447f14a78..61ac8cd4842 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -55,7 +55,7 @@ struct TypeImpl
size_t instance_align;
void (*class_init)(ObjectClass *klass, void *data);
-void (*class_base_init)(ObjectClass *klass, void *data);
+void (*class_base_init)(ObjectClass *klass, const void *data);
void *class_data;
diff --git a/rust/qemu-api/src/qom.rs b/rust/qemu-api/src/qom.rs
index f50ee371aac..f3d43e066ef 100644
--- a/rust/qemu-api/src/qom.rs
+++ b/rust/qemu-api/src/qom.rs
@@ -475,7 +475,7 @@ pub trait ObjectImpl: ObjectType +
ClassInitImpl {
/// the effects of copying the contents of the parent's class struct
/// to the descendants.
const CLASS_BASE_INIT: Option<
-unsafe extern "C" fn(klass: *mut ObjectClass, data: *mut c_void),
+unsafe extern "C" fn(klass: *mut ObjectClass, data: *const c_void),
> = None;
const TYPE_INFO: TypeInfo = TypeInfo {
--
2.47.1
Re: [PATCH 04/12] rust: qdev: add clock creation
On Fri, Feb 07, 2025 at 11:16:15AM +0100, Paolo Bonzini wrote: > Date: Fri, 7 Feb 2025 11:16:15 +0100 > From: Paolo Bonzini > Subject: [PATCH 04/12] rust: qdev: add clock creation > X-Mailer: git-send-email 2.48.1 > > Add a Rust version of qdev_init_clock_in, which can be used in > instance_init. There are a couple differences with the C > version: > > - in Rust the object keeps its own reference to the clock (in addition to > the one embedded in the NamedClockList), and the reference is dropped > automatically by instance_finalize(); this is encoded in the signature > of DeviceClassMethods::init_clock_in, which makes the lifetime of the > clock independent of that of the object it holds. This goes unnoticed > in the C version and is due to the existence of aliases. > > - also, anything that happens during instance_init uses the pinned_init > framework to operate on a partially initialized object, and is done > through class methods (i.e. through DeviceClassMethods rather than > DeviceMethods) because the device does not exist yet. Therefore, Rust > code *must* create clocks from instance_init, which is stricter than C. > > Signed-off-by: Paolo Bonzini > --- > rust/hw/char/pl011/src/device.rs | 43 +--- > rust/qemu-api/src/prelude.rs | 2 + > rust/qemu-api/src/qdev.rs| 109 ++- > rust/qemu-api/src/vmstate.rs | 4 +- > 4 files changed, 127 insertions(+), 31 deletions(-) > Reviewed-by: Zhao Liu
Re: [RFC PATCH v2 3/8] migration/multifd: Terminate the TLS connection
On Fri, Feb 07, 2025 at 03:15:48PM -0300, Fabiano Rosas wrote:
> >> +for (i = 0; i < migrate_multifd_channels(); i++) {
> >> +MultiFDSendParams *p = &multifd_send_state->params[i];
> >> +
> >> +/* thread_created implies the TLS handshake has succeeded */
> >> +if (p->tls_thread_created && p->thread_created) {
> >> +Error *local_err = NULL;
> >> +/*
> >> + * The destination expects the TLS session to always be
> >> + * properly terminated. This helps to detect a premature
> >> + * termination in the middle of the stream. Note that
> >> + * older QEMUs always break the connection on the source
> >> + * and the destination always sees
> >> + * GNUTLS_E_PREMATURE_TERMINATION.
> >> + */
> >> +migration_tls_channel_end(p->c, &local_err);
> >> +
> >> +if (local_err) {
> >> +/*
> >> + * The above can fail with broken pipe due to a
> >> + * previous migration error, ignore the error.
> >> + */
> >> +assert(migration_has_failed(migrate_get_current()));
> >
> > Considering this is still src, do we want to be softer on this by
> > error_report?
> >
> > Logically !migration_has_failed() means it succeeded, so we can throw src
> > qemu way now, that shouldn't be a huge deal. More of thinking out loud kind
> > of comment.. Your call.
> >
>
> Maybe even a warning? If at this point migration succeeded, it's probably
> best to let cleanup carry on.
Yep, warning sounds good too.
--
Peter Xu
Re: [PATCH 4/5] hw/arm/smmuv3: Move reset to exit phase
On Mon, 10 Feb 2025 at 14:14, Peter Xu wrote: > > On Fri, Feb 07, 2025 at 06:18:50PM +, Peter Maydell wrote: > > On Fri, 7 Feb 2025 at 17:48, Peter Xu wrote: > > > > > > On Fri, Feb 07, 2025 at 04:58:39PM +, Peter Maydell wrote: > > > > (I wonder if we ought to suggest quiescing outstanding > > > > DMA in the enter phase? But it's probably easier to fix > > > > the iommus like this series does than try to get every > > > > dma-capable pci device to do something different.) > > > > > > I wonder if we should provide some generic helper to register vIOMMU reset > > > callbacks, so that we'll be sure any vIOMMU model impl that will register > > > at exit() phase only, and do nothing during the initial two phases. Then > > > we can put some rich comment on that helper on why. > > > > > > Looks like it means the qemu reset model in the future can be a > > > combination > > > of device tree (which resets depth-first) and the three phases model. We > > > will start to use different approach to solve different problems. > > > > The tree of QOM devices (i.e. the one based on the qbus buses > > and rooted at the sysbus) resets depth-first, but it does so in > > three phases: first we traverse everything doing 'enter'; then > > we traverse everything doing 'hold'; then we traverse everything > > doing 'exit'. There *used* to be an awkward mix of some things > > being three-phase and some not, but we have now got rid of all > > of those so a system reset does a single three-phase reset run > > which resets everything. > > Right. Sorry I wasn't very clear before indeed on what I wanted to > express. > > My understanding is the 3 phases reset, even if existed, was not designed > to order things like vIOMMU and devices that is already described by system > topology. That's, IMHO, exactly what QOM topology wanted to achieve right > now on ordering device resets and the whole depth-first reset method would > make sense with it. > > So from that specific POV, it's a mixture use of both methods on ordering > of devices to reset now (rather than the order of reset process within a > same device, provided into 3 phases). It may not be very intuitive when > someone reads about the two reset mechanisms, as one would naturally take > vIOMMU as a root object of any other PCIe devices under root complex, and > thinking the order should be guaranteed by QOM on reset already. In > reality it's not. So that's the part I wonder if we want to document. Yeah, I see what you mean. The issue here is that the iommu isn't actually a parent of the devices that access through it. This is true both in the "qbus/qdev bus tree" sense (where it is the PCI controller device that owns the PCI bus that the devices are on) and also in the QOM tree sense[*] (where usually the iommu and the PCI controller are both created by the machine or the SoC, I guess?). Instead iommus are separate devices that control data flow but aren't in a parent-child relationship with the devices on either side of that flow. There is a guarantee about reset ordering between bus parent/child (so the PCI controller resets before it resets the PCI bus that resets the PCI devices on the bus), but that doesn't help the iommu. [*] I have a vague idea that ideally we might want reset to be QOM-tree based rather than qbus-tree based. But getting there from here is non-trivial. And maybe what we really want is "objects, especially SoCs, that create children can define what their reset tree is, with the default being to reset all the QOM children". Lots of non-thought-through complexity here ;-) thanks -- PMM
Re: [RFC v4 0/5] Add packed virtqueue to shadow virtqueue
On Mon, Feb 10, 2025 at 11:58 AM Sahil Siddiq wrote: > > Hi, > > On 2/6/25 8:47 PM, Sahil Siddiq wrote: > > On 2/6/25 12:42 PM, Eugenio Perez Martin wrote: > >> On Thu, Feb 6, 2025 at 6:26 AM Sahil Siddiq wrote: > >>> On 2/4/25 11:45 PM, Eugenio Perez Martin wrote: > PS: Please note that you can check packed_vq SVQ implementation > already without CVQ, as these features are totally orthogonal :). > > >>> > >>> Right. Now that I can ping with the ctrl features turned off, I think > >>> this should take precedence. There's another issue specific to the > >>> packed virtqueue case. It causes the kernel to crash. I have been > >>> investigating this and the situation here looks very similar to what's > >>> explained in Jason Wang's mail [2]. My plan of action is to apply his > >>> changes in L2's kernel and check if that resolves the problem. > >>> > >>> The details of the crash can be found in this mail [3]. > >>> > >> > >> If you're testing this series without changes, I think that is caused > >> by not implementing the packed version of vhost_svq_get_buf. > >> > >> https://lists.nongnu.org/archive/html/qemu-devel/2024-12/msg01902.html > >> > > > > Oh, apologies, I think I had misunderstood your response in the linked mail. > > Until now, I thought they were unrelated. In that case, I'll implement the > > packed version of vhost_svq_get_buf. Hopefully that fixes it :). > > > > I noticed one thing while testing some of the changes that I have made. > I haven't finished making the relevant changes to all the functions which > will have to handle split and packed vq differently. L2's kernel crashes > when I launch L0-QEMU with ctrl_vq=on,ctrl_rx=on. Interesting, is a similar crash than this? (NULL ptr deference on virtnet_set_features)? https://issues.redhat.com/browse/RHEL-391 > However, when I start > L0-QEMU with ctrl_vq=off,ctrl_rx=off,ctrl_vlan=off,ctrl_mac_addr=off, L2's > kernel boots successfully. Tracing L2-QEMU also confirms that the packed > feature is enabled. With all the ctrl features disabled, I think pinging > will also be possible once I finish implementing the packed versions of > the other functions. > Good! > There's another thing that I am confused about regarding the current > implementation (in the master branch). > > In hw/virtio/vhost-shadow-virtqueue.c:vhost_svq_vring_write_descs() [1], > svq->free_head saves the descriptor in the specified format using > "le16_to_cpu" (line 171). Good catch, this should be le16_to_cpu actually. But code wise is the same, so we have no visible error. Do you want to send a patch to fix it? > On the other hand, the value of i is stored > in the native endianness using "cpu_to_le16" (line 168). If "i" is to be > stored in the native endianness (little endian in this case), then > should svq->free_head first be converted to little endian before being > assigned to "i" at the start of the function (line 142)? > This part is correct in the code, as it is used by the host, not written to the guest or read from the guest. So no conversion is needed. Thanks!
Re: [PATCH v2 18/18] hw/rtc: Add Ricoh RS5C372 RTC emulation
On 6/2/25 22:58, Bernhard Beschow wrote: Am 6. Februar 2025 17:32:31 UTC schrieb Peter Maydell : On Tue, 4 Feb 2025 at 09:21, Bernhard Beschow wrote: The implementation just allows Linux to determine date and time. Signed-off-by: Bernhard Beschow --- MAINTAINERS | 1 + hw/rtc/rs5c372.c| 227 hw/rtc/Kconfig | 5 + hw/rtc/meson.build | 1 + hw/rtc/trace-events | 4 + 5 files changed, 238 insertions(+) create mode 100644 hw/rtc/rs5c372.c Should there be a patch after this one that adds this device to your board ? As per Kconfig the board selects I2C_DEVICES and this device is "default y if I2C_DEVICES". I've deliberately not hardcoded this device to the board to make it emulate a plain i.MX 8M Plus SoC (see also board documentation). Then maybe add a test to be sure it is not bitroting?
Re: [PATCH v2 2/9] vfio/pci: Replace "iommu_device" by "vIOMMU"
On 30/1/25 14:43, Cédric Le Goater wrote: This is to be consistent with other reported errors related to vIOMMU devices. Signed-off-by: Cédric Le Goater --- hw/vfio/pci.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH v4 17/17] docs/system/arm: Add Description for NPCM8XX SoC
On Thu, 6 Feb 2025 at 22:12, Hao Wu wrote: > > NPCM8XX SoC is the successor of the NPCM7XX. It features quad-core > Cortex-A35 (Armv8, 64-bit) CPUs and some additional peripherals. > > This document describes the NPCM8XX SoC and an evaluation board > (NPCM 845 EVB). > > Signed-off-by: Hao Wu > --- > docs/system/arm/nuvoton.rst | 27 --- > 1 file changed, 20 insertions(+), 7 deletions(-) Reviewed-by: Peter Maydell thanks -- PMM
Re: [PATCH v2 4/9] vfio: Introduce vfio_get_vfio_device()
On 30/1/25 14:43, Cédric Le Goater wrote:
This helper will be useful in the listener handlers to extract the
VFIO device from a memory region using memory_region_owner(). At the
moment, we only care for PCI passthrough devices. If the need arises,
we will add more.
Signed-off-by: Cédric Le Goater
---
include/hw/vfio/vfio-common.h | 1 +
hw/vfio/helpers.c | 10 ++
2 files changed, 11 insertions(+)
diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
index
0c60be5b15c70168f4f94ad7054d9bd750a162d3..ac35136a11051b079cd9d04e6becd344a0e0f7e7
100644
--- a/include/hw/vfio/vfio-common.h
+++ b/include/hw/vfio/vfio-common.h
@@ -252,6 +252,7 @@ bool vfio_device_hiod_realize(VFIODevice *vbasedev, Error
**errp);
bool vfio_attach_device(char *name, VFIODevice *vbasedev,
AddressSpace *as, Error **errp);
void vfio_detach_device(VFIODevice *vbasedev);
+VFIODevice *vfio_get_vfio_device(Object *obj);
int vfio_kvm_device_add_fd(int fd, Error **errp);
int vfio_kvm_device_del_fd(int fd, Error **errp);
diff --git a/hw/vfio/helpers.c b/hw/vfio/helpers.c
index
913796f437f84eece8711cb4b4b654a44040d17c..4b255d4f3a9e81f55df00c68fc71da769fd5bd04
100644
--- a/hw/vfio/helpers.c
+++ b/hw/vfio/helpers.c
@@ -23,6 +23,7 @@
#include
#include "hw/vfio/vfio-common.h"
+#include "hw/vfio/pci.h"
#include "hw/hw.h"
#include "trace.h"
#include "qapi/error.h"
@@ -728,3 +729,12 @@ bool vfio_device_hiod_realize(VFIODevice *vbasedev, Error
**errp)
return HOST_IOMMU_DEVICE_GET_CLASS(hiod)->realize(hiod, vbasedev, errp);
}
+
+VFIODevice *vfio_get_vfio_device(Object *obj)
Can't we take a VFIOPCIDevice argument?
+{
+if (object_dynamic_cast(obj, TYPE_VFIO_PCI)) {
+return &VFIO_PCI(obj)->vbasedev;
+} else {
+return NULL;
+}
+}
Re: [PATCH v2 5/9] vfio: Improve error reporting when MMIO region mapping fails
On 30/1/25 14:43, Cédric Le Goater wrote:
When the IOMMU address space width is smaller than the physical
address width, a MMIO region of a device can fail to map because the
region is outside the supported IOVA ranges of the VM. In this case,
PCI peer-to-peer transactions on BARs are not supported.
This can occur with the 39-bit IOMMU address space width, as can be
the case on some consumer processors or when using a vIOMMU device
with default settings. The current error message is unclear, also
change the error report to a warning because it is a non fatal
condition for the VM.
Signed-off-by: Cédric Le Goater
---
hw/vfio/common.c | 17 -
1 file changed, 16 insertions(+), 1 deletion(-)
diff --git a/hw/vfio/common.c b/hw/vfio/common.c
index
62af1216fc5a9089fc718c2afe3a405d9381db32..5c9d8657d746ce30af5ae8f9122101e086a61ef5
100644
--- a/hw/vfio/common.c
+++ b/hw/vfio/common.c
@@ -555,6 +555,18 @@ static bool vfio_get_section_iova_range(VFIOContainerBase
*bcontainer,
return true;
}
+static void vfio_device_error_append(VFIODevice *vbasedev, Error **errp)
+{
+/*
+ * MMIO region mapping failures are not fatal but in this case PCI
+ * peer-to-peer transactions are broken.
+ */
+if (vbasedev && vbasedev->type == VFIO_DEVICE_TYPE_PCI) {
+error_append_hint(errp, "%s: PCI peer-to-peer transactions "
+ "on BARs are not supported.\n", vbasedev->name);
+}
+}
+
static void vfio_listener_region_add(MemoryListener *listener,
MemoryRegionSection *section)
{
@@ -670,7 +682,10 @@ static void vfio_listener_region_add(MemoryListener
*listener,
strerror(-ret));
if (memory_region_is_ram_device(section->mr)) {
/* Allow unexpected mappings not to be fatal for RAM devices */
-error_report_err(err);
+VFIODevice *vbasedev =
+vfio_get_vfio_device(memory_region_owner(section->mr));
+vfio_device_error_append(vbasedev, &err);
Having vfio_get_vfio_device() returning NULL and
vfio_device_error_append() also checking for NULL is odd.
Maybe just inline everything here?
+warn_report_once_err(err);
return;
}
goto fail;
Re: [PATCH v2 7/9] cpu: Introduce cpu_get_phys_bits()
Hi Cédric,
On 30/1/25 14:43, Cédric Le Goater wrote:
The Intel CPU has a complex history regarding setting of the physical
address space width on KVM. A 'phys_bits' field and a "phys-bits"
property were added by commit af45907a1328 ("target-i386: Allow
physical address bits to be set") to tune this value.
In certain circumstances, it is interesting to know this value to
check that all the conditions are met for optimal operation. For
instance, when the system has a 39-bit IOMMU address space width and a
larger CPU physical address space, we expect issues when mapping the
MMIO regions of passthrough devices and it would good to report to the
user. These hybrid HW configs can be found on some consumer grade
processors or when using a vIOMMU device with default settings.
For this purpose, add an helper routine and a CPUClass callback to
return the physical address space width of a CPU.
Cc: Richard Henderson
Cc: Paolo Bonzini
Cc: Eduardo Habkost
Cc: Marcel Apfelbaum
Cc: "Philippe Mathieu-Daudé"
Cc: Yanan Wang
Cc: Zhao Liu
Signed-off-by: Cédric Le Goater
---
include/hw/core/cpu.h| 9 +
include/hw/core/sysemu-cpu-ops.h | 6 ++
cpu-target.c | 5 +
hw/core/cpu-system.c | 11 +++
target/i386/cpu.c| 6 ++
5 files changed, 37 insertions(+)
diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index
fb397cdfc53d12d40d3e4e7f86251fc31c48b9f6..1b3eead102ce62fcee55ab0ed5e0dff327fa2fc5
100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -748,6 +748,14 @@ int cpu_asidx_from_attrs(CPUState *cpu, MemTxAttrs attrs);
*/
bool cpu_virtio_is_big_endian(CPUState *cpu);
+/**
+ * cpu_get_phys_bits:
+ * @cpu: CPU
+ *
+ * Return the physical address space width of the CPU @cpu.
+ */
+uint32_t cpu_get_phys_bits(const CPUState *cpu);
+
#endif /* CONFIG_USER_ONLY */
/**
@@ -1168,6 +1176,7 @@ void cpu_exec_unrealizefn(CPUState *cpu);
void cpu_exec_reset_hold(CPUState *cpu);
const char *target_name(void);
+uint32_t target_phys_bits(void);
#ifdef COMPILING_PER_TARGET
diff --git a/include/hw/core/sysemu-cpu-ops.h b/include/hw/core/sysemu-cpu-ops.h
index
0df5b058f50073e47d2a6b8286be5204776520d2..210b3ed57985525795b81559e41e0085969210d5
100644
--- a/include/hw/core/sysemu-cpu-ops.h
+++ b/include/hw/core/sysemu-cpu-ops.h
@@ -81,6 +81,12 @@ typedef struct SysemuCPUOps {
*/
bool (*virtio_is_big_endian)(CPUState *cpu);
+/**
+ * @get_phys_bits: Callback to return the physical address space
+ * width of a CPU.
+ */
+uint32_t (*get_phys_bits)(const CPUState *cpu);
Using 32-bit isn't really relevant IMHO, I'd return an unsigned type.
+
/**
* @legacy_vmsd: Legacy state for migration.
* Do not use in new targets, use #DeviceClass::vmsd
instead.
diff --git a/cpu-target.c b/cpu-target.c
index
667688332c929aa53782c94343def34571272d5f..88158272c06cc42424d435b9701e33735f080239
100644
--- a/cpu-target.c
+++ b/cpu-target.c
@@ -472,3 +472,8 @@ const char *target_name(void)
{
return TARGET_NAME;
}
+
+uint32_t target_phys_bits(void)
+{
+return TARGET_PHYS_ADDR_SPACE_BITS;
+}
diff --git a/hw/core/cpu-system.c b/hw/core/cpu-system.c
index
6aae28a349a7a377d010ff9dcab5ebc29e1126ca..05067d84f4258facf4252216f17729e390d38eae
100644
--- a/hw/core/cpu-system.c
+++ b/hw/core/cpu-system.c
@@ -60,6 +60,17 @@ hwaddr cpu_get_phys_page_attrs_debug(CPUState *cpu, vaddr
addr,
return cc->sysemu_ops->get_phys_page_debug(cpu, addr);
}
+uint32_t cpu_get_phys_bits(const CPUState *cpu)
+{
+CPUClass *cc = CPU_GET_CLASS(cpu);
+
+if (cc->sysemu_ops->get_phys_bits) {
+return cc->sysemu_ops->get_phys_bits(cpu);
+}
+
+return target_phys_bits();
With heterogeneous emulation in mind, I'd rather this to be a mandatory
CPUClass handler.
+}
+
hwaddr cpu_get_phys_page_debug(CPUState *cpu, vaddr addr)
{
MemTxAttrs attrs = {};
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index
b5dd60d2812e0c3d13c1743fd485a9068ab29c4f..01cf9a44963710a415c755c17582730f75233143
100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -8393,6 +8393,11 @@ static bool x86_cpu_get_paging_enabled(const CPUState
*cs)
return cpu->env.cr[0] & CR0_PG_MASK;
}
+
+static uint32_t x86_cpu_get_phys_bits(const CPUState *cs)
+{
+return X86_CPU(cs)->phys_bits;
+}
#endif /* !CONFIG_USER_ONLY */
static void x86_cpu_set_pc(CPUState *cs, vaddr value)
@@ -8701,6 +8706,7 @@ static const struct SysemuCPUOps i386_sysemu_ops = {
.get_memory_mapping = x86_cpu_get_memory_mapping,
.get_paging_enabled = x86_cpu_get_paging_enabled,
.get_phys_page_attrs_debug = x86_cpu_get_phys_page_attrs_debug,
+.get_phys_bits = x86_cpu_get_phys_bits,
.asidx_from_attrs = x86_asidx_from_attrs,
.get_crash_info = x86_cpu_get_crash_info,
.write_elf32_note = x86_cpu_write_elf32_note,
[PATCH] 9pfs: fix dead code in qemu_open_flags_tostr()
Coverity scan complained about expression "|LARGEFILE" to be non reachable and the detailed Coverity report claims O_LARGEFILE was zero. I can't reproduce this here, but I assume that means there are at least some system(s) which define O_LARGEFILE as zero. This is not really an issue, but to silence this Coverity warning, add a preprocessor wrapper that checks for O_LARGEFILE being non-zero for this overall expression. The 'defined(O_LARGEFILE)' check is not necessary, but it makes it more clear that we really want to check for the value of O_LARGEFILE, not just whether the macro was defined. Fixes: 9a0dd4b3 Resolves: Coverity CID 1591178 Reported-by: Coverity Scan Signed-off-by: Christian Schoenebeck --- hw/9pfs/9p-util-generic.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/hw/9pfs/9p-util-generic.c b/hw/9pfs/9p-util-generic.c index 4c1e9c887d..02e359f17b 100644 --- a/hw/9pfs/9p-util-generic.c +++ b/hw/9pfs/9p-util-generic.c @@ -19,7 +19,9 @@ char *qemu_open_flags_tostr(int flags) #ifdef O_DIRECT (flags & O_DIRECT) ? "|DIRECT" : "", #endif +#if defined(O_LARGEFILE) && O_LARGEFILE != 0 (flags & O_LARGEFILE) ? "|LARGEFILE" : "", +#endif (flags & O_DIRECTORY) ? "|DIRECTORY" : "", (flags & O_NOFOLLOW) ? "|NOFOLLOW" : "", #ifdef O_NOATIME -- 2.39.5
[PATCH] target/riscv: Respect mseccfg.RLB bit for TOR mode PMP entry
When running in TOR mode (Top of Range) the next PMP entry controls
whether the entry is locked. However simply checking if the PMP_LOCK bit
is set is not sufficient with the Smepmp extension which now provides a
bit (mseccfg.RLB (Rule Lock Bypass)) to disregard the lock bits. In
order to respect this bit use the convenience pmp_is_locked() function
rather than directly checking PMP_LOCK since this function checks
mseccfg.RLB.
Signed-off-by: Rob Bradford
---
target/riscv/pmp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/target/riscv/pmp.c b/target/riscv/pmp.c
index a185c246d6..85ab270dad 100644
--- a/target/riscv/pmp.c
+++ b/target/riscv/pmp.c
@@ -524,7 +524,7 @@ void pmpaddr_csr_write(CPURISCVState *env, uint32_t
addr_index,
uint8_t pmp_cfg = env->pmp_state.pmp[addr_index + 1].cfg_reg;
is_next_cfg_tor = PMP_AMATCH_TOR == pmp_get_a_field(pmp_cfg);
-if (pmp_cfg & PMP_LOCK && is_next_cfg_tor) {
+if (pmp_is_locked(env, addr_index + 1) && is_next_cfg_tor) {
qemu_log_mask(LOG_GUEST_ERROR,
"ignoring pmpaddr write - pmpcfg + 1 locked\n");
return;
--
2.48.1
Re: [PATCH 15/15] arm/cpu: Add generated files
On Fri, Feb 07 2025, Richard Henderson wrote: > On 2/7/25 03:02, Cornelia Huck wrote: >> And switch to using the generated definitions. >> >> Generated against Linux 6.14-rc1. >> >> Signed-off-by: Cornelia Huck >> --- >> target/arm/cpu-sysreg-properties.c | 716 - >> target/arm/cpu-sysregs.h | 116 + >> target/arm/cpu-sysregs.h.inc | 164 +++ >> 3 files changed, 860 insertions(+), 136 deletions(-) >> create mode 100644 target/arm/cpu-sysregs.h.inc > > Why are we committing generated files and not generating them at build-time? We'd either have to carry a copy of Linux' sysregs file, or generate a build dependency on Linux. I think we should handle this similar to the Linux headers update, where we do an explicit update and check for anything unexpected that might have crept in. (Same applies if we switch to any other external source for register definitions.)
