* [PATCH 0/4] hw/pci: Convert rom_bar into OnOffAuto
@ 2024-07-06 9:29 Akihiko Odaki
2024-07-06 9:29 ` [PATCH 1/4] qapi: Add visit_type_str_preserving() Akihiko Odaki
` (3 more replies)
0 siblings, 4 replies; 6+ messages in thread
From: Akihiko Odaki @ 2024-07-06 9:29 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, Michael S. Tsirkin, Marcel Apfelbaum,
Alex Williamson, Cédric Le Goater, Paolo Bonzini,
Daniel P. Berrangé, Eduardo Habkost, Sriram Yagnaraman,
Jason Wang, Keith Busch, Klaus Jensen, Markus Armbruster
Cc: qemu-devel, qemu-block, Akihiko Odaki
rom_bar is tristate but was defined as uint32_t so convert it into
OnOffAuto to clarify that. For compatibility, a uint32 value set via
QOM will be converted into OnOffAuto.
Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
Akihiko Odaki (4):
qapi: Add visit_type_str_preserving()
qapi: Do not consume a value when visit_type_enum() fails
hw/pci: Convert rom_bar into OnOffAuto
hw/qdev: Remove opts member
docs/igd-assign.txt | 2 +-
include/hw/pci/pci_device.h | 2 +-
include/hw/qdev-core.h | 4 ---
include/qapi/visitor-impl.h | 3 ++-
include/qapi/visitor.h | 25 +++++++++++++----
hw/core/qdev.c | 1 -
hw/pci/pci.c | 57 +++++++++++++++++++++++++++++++++++++--
hw/vfio/pci-quirks.c | 2 +-
hw/vfio/pci.c | 11 ++++----
hw/xen/xen_pt_load_rom.c | 4 +--
qapi/opts-visitor.c | 12 ++++-----
qapi/qapi-clone-visitor.c | 2 +-
qapi/qapi-dealloc-visitor.c | 4 +--
qapi/qapi-forward-visitor.c | 4 ++-
qapi/qapi-visit-core.c | 21 ++++++++++++---
qapi/qobject-input-visitor.c | 23 ++++++++--------
qapi/qobject-output-visitor.c | 2 +-
qapi/string-input-visitor.c | 2 +-
qapi/string-output-visitor.c | 2 +-
system/qdev-monitor.c | 12 +++++----
tests/qtest/virtio-net-failover.c | 32 +++++++++++-----------
21 files changed, 154 insertions(+), 73 deletions(-)
---
base-commit: f2cb4026fccfe073f84a4b440e41d3ed0c3134f6
change-id: 20240704-rombar-1a4ba2890d6c
Best regards,
--
Akihiko Odaki <akihiko.odaki@daynix.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/4] qapi: Add visit_type_str_preserving()
2024-07-06 9:29 [PATCH 0/4] hw/pci: Convert rom_bar into OnOffAuto Akihiko Odaki
@ 2024-07-06 9:29 ` Akihiko Odaki
2024-07-06 9:29 ` [PATCH 2/4] qapi: Do not consume a value when visit_type_enum() fails Akihiko Odaki
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Akihiko Odaki @ 2024-07-06 9:29 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, Michael S. Tsirkin, Marcel Apfelbaum,
Alex Williamson, Cédric Le Goater, Paolo Bonzini,
Daniel P. Berrangé, Eduardo Habkost, Sriram Yagnaraman,
Jason Wang, Keith Busch, Klaus Jensen, Markus Armbruster
Cc: qemu-devel, qemu-block, Akihiko Odaki
visit_type_str_preserving() is mostly indentical with visit_type_str()
but leaves the value intact. This is useful when the caller may
interpret the value with a different type.
Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
include/qapi/visitor-impl.h | 3 ++-
include/qapi/visitor.h | 20 ++++++++++++++++++++
qapi/opts-visitor.c | 7 +++++--
qapi/qapi-clone-visitor.c | 2 +-
qapi/qapi-dealloc-visitor.c | 4 ++--
qapi/qapi-forward-visitor.c | 4 +++-
qapi/qapi-visit-core.c | 17 +++++++++++++++--
qapi/qobject-input-visitor.c | 23 ++++++++++++-----------
qapi/qobject-output-visitor.c | 2 +-
qapi/string-input-visitor.c | 2 +-
qapi/string-output-visitor.c | 2 +-
11 files changed, 63 insertions(+), 23 deletions(-)
diff --git a/include/qapi/visitor-impl.h b/include/qapi/visitor-impl.h
index 2badec5ba460..7b61f55995b5 100644
--- a/include/qapi/visitor-impl.h
+++ b/include/qapi/visitor-impl.h
@@ -95,7 +95,8 @@ struct Visitor
bool (*type_bool)(Visitor *v, const char *name, bool *obj, Error **errp);
/* Must be set */
- bool (*type_str)(Visitor *v, const char *name, char **obj, Error **errp);
+ bool (*type_str)(Visitor *v, const char *name, char **obj, bool consume,
+ Error **errp);
/* Must be set to visit numbers */
bool (*type_number)(Visitor *v, const char *name, double *obj,
diff --git a/include/qapi/visitor.h b/include/qapi/visitor.h
index 27b85d4700f2..b3ae3188edfb 100644
--- a/include/qapi/visitor.h
+++ b/include/qapi/visitor.h
@@ -654,6 +654,26 @@ bool visit_type_bool(Visitor *v, const char *name, bool *obj, Error **errp);
*/
bool visit_type_str(Visitor *v, const char *name, char **obj, Error **errp);
+/*
+ * Visit a string value but do not consume it.
+ *
+ * @name expresses the relationship of this string to its parent
+ * container; see the general description of @name above.
+ *
+ * @obj must be non-NULL. Input and clone visitors set *@obj to the
+ * value (always using "" rather than NULL for an empty string).
+ * Other visitors leave *@obj unchanged, and commonly treat NULL like
+ * "".
+ *
+ * This function must be called only with an input visitor.
+ *
+ * This is mostly identical with visit_type_str() but leaves the value intact.
+ * This is useful when the caller may interpret the value with a different
+ * type.
+ */
+bool visit_type_str_preserving(Visitor *v, const char *name, char **obj,
+ Error **errp);
+
/*
* Visit a number (i.e. double) value.
*
diff --git a/qapi/opts-visitor.c b/qapi/opts-visitor.c
index 3d1a28b41918..e9fad756e189 100644
--- a/qapi/opts-visitor.c
+++ b/qapi/opts-visitor.c
@@ -347,7 +347,8 @@ processed(OptsVisitor *ov, const char *name)
static bool
-opts_type_str(Visitor *v, const char *name, char **obj, Error **errp)
+opts_type_str(Visitor *v, const char *name, char **obj, bool consume,
+ Error **errp)
{
OptsVisitor *ov = to_ov(v);
const QemuOpt *opt;
@@ -363,7 +364,9 @@ opts_type_str(Visitor *v, const char *name, char **obj, Error **errp)
* valid enum value; this is harmless because tracking what gets
* consumed only matters to visit_end_struct() as the final error
* check if there were no other failures during the visit. */
- processed(ov, name);
+ if (consume) {
+ processed(ov, name);
+ }
return true;
}
diff --git a/qapi/qapi-clone-visitor.c b/qapi/qapi-clone-visitor.c
index bbf953698f38..90e6641835a7 100644
--- a/qapi/qapi-clone-visitor.c
+++ b/qapi/qapi-clone-visitor.c
@@ -108,7 +108,7 @@ static bool qapi_clone_type_bool(Visitor *v, const char *name, bool *obj,
}
static bool qapi_clone_type_str(Visitor *v, const char *name, char **obj,
- Error **errp)
+ bool consume, Error **errp)
{
QapiCloneVisitor *qcv = to_qcv(v);
diff --git a/qapi/qapi-dealloc-visitor.c b/qapi/qapi-dealloc-visitor.c
index ef283f296601..6b0f957047d5 100644
--- a/qapi/qapi-dealloc-visitor.c
+++ b/qapi/qapi-dealloc-visitor.c
@@ -62,9 +62,9 @@ static void qapi_dealloc_end_list(Visitor *v, void **obj)
}
static bool qapi_dealloc_type_str(Visitor *v, const char *name, char **obj,
- Error **errp)
+ bool consume, Error **errp)
{
- if (obj) {
+ if (obj && consume) {
g_free(*obj);
}
return true;
diff --git a/qapi/qapi-forward-visitor.c b/qapi/qapi-forward-visitor.c
index e36d9bc9ba7e..891282e0c3b3 100644
--- a/qapi/qapi-forward-visitor.c
+++ b/qapi/qapi-forward-visitor.c
@@ -180,6 +180,7 @@ static bool forward_field_type_bool(Visitor *v, const char *name, bool *obj,
}
static bool forward_field_type_str(Visitor *v, const char *name, char **obj,
+ bool consume,
Error **errp)
{
ForwardFieldVisitor *ffv = to_ffv(v);
@@ -187,7 +188,8 @@ static bool forward_field_type_str(Visitor *v, const char *name, char **obj,
if (!forward_field_translate_name(ffv, &name, errp)) {
return false;
}
- return visit_type_str(ffv->target, name, obj, errp);
+ return (consume ? visit_type_str : visit_type_str_preserving)(
+ ffv->target, name, obj, errp);
}
static bool forward_field_type_size(Visitor *v, const char *name, uint64_t *obj,
diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c
index 6c13510a2bc7..89b52fc99202 100644
--- a/qapi/qapi-visit-core.c
+++ b/qapi/qapi-visit-core.c
@@ -336,7 +336,8 @@ bool visit_type_bool(Visitor *v, const char *name, bool *obj, Error **errp)
return v->type_bool(v, name, obj, errp);
}
-bool visit_type_str(Visitor *v, const char *name, char **obj, Error **errp)
+static bool visit_type_str_common(Visitor *v, const char *name, char **obj,
+ bool consume, Error **errp)
{
bool ok;
@@ -346,13 +347,25 @@ bool visit_type_str(Visitor *v, const char *name, char **obj, Error **errp)
assert(!(v->type & VISITOR_OUTPUT) || *obj);
*/
trace_visit_type_str(v, name, obj);
- ok = v->type_str(v, name, obj, errp);
+ ok = v->type_str(v, name, obj, consume, errp);
if (v->type & VISITOR_INPUT) {
assert(ok != !*obj);
}
return ok;
}
+bool visit_type_str(Visitor *v, const char *name, char **obj, Error **errp)
+{
+ return visit_type_str_common(v, name, obj, true, errp);
+}
+
+bool visit_type_str_preserving(Visitor *v, const char *name, char **obj,
+ Error **errp)
+{
+ assert(v->type & VISITOR_INPUT);
+ return visit_type_str_common(v, name, obj, false, errp);
+}
+
bool visit_type_number(Visitor *v, const char *name, double *obj,
Error **errp)
{
diff --git a/qapi/qobject-input-visitor.c b/qapi/qobject-input-visitor.c
index f110a804b2ae..facbbf01bd27 100644
--- a/qapi/qobject-input-visitor.c
+++ b/qapi/qobject-input-visitor.c
@@ -174,13 +174,13 @@ static QObject *qobject_input_get_object(QObjectInputVisitor *qiv,
}
static const char *qobject_input_get_keyval(QObjectInputVisitor *qiv,
- const char *name,
+ const char *name, bool consume,
Error **errp)
{
QObject *qobj;
QString *qstr;
- qobj = qobject_input_get_object(qiv, name, true, errp);
+ qobj = qobject_input_get_object(qiv, name, consume, errp);
if (!qobj) {
return NULL;
}
@@ -416,7 +416,7 @@ static bool qobject_input_type_int64_keyval(Visitor *v, const char *name,
int64_t *obj, Error **errp)
{
QObjectInputVisitor *qiv = to_qiv(v);
- const char *str = qobject_input_get_keyval(qiv, name, errp);
+ const char *str = qobject_input_get_keyval(qiv, name, true, errp);
if (!str) {
return false;
@@ -467,7 +467,7 @@ static bool qobject_input_type_uint64_keyval(Visitor *v, const char *name,
uint64_t *obj, Error **errp)
{
QObjectInputVisitor *qiv = to_qiv(v);
- const char *str = qobject_input_get_keyval(qiv, name, errp);
+ const char *str = qobject_input_get_keyval(qiv, name, true, errp);
if (!str) {
return false;
@@ -507,7 +507,7 @@ static bool qobject_input_type_bool_keyval(Visitor *v, const char *name,
bool *obj, Error **errp)
{
QObjectInputVisitor *qiv = to_qiv(v);
- const char *str = qobject_input_get_keyval(qiv, name, errp);
+ const char *str = qobject_input_get_keyval(qiv, name, true, errp);
if (!str) {
return false;
@@ -522,10 +522,10 @@ static bool qobject_input_type_bool_keyval(Visitor *v, const char *name,
}
static bool qobject_input_type_str(Visitor *v, const char *name, char **obj,
- Error **errp)
+ bool consume, Error **errp)
{
QObjectInputVisitor *qiv = to_qiv(v);
- QObject *qobj = qobject_input_get_object(qiv, name, true, errp);
+ QObject *qobj = qobject_input_get_object(qiv, name, consume, errp);
QString *qstr;
*obj = NULL;
@@ -544,10 +544,11 @@ static bool qobject_input_type_str(Visitor *v, const char *name, char **obj,
}
static bool qobject_input_type_str_keyval(Visitor *v, const char *name,
- char **obj, Error **errp)
+ char **obj, bool consume,
+ Error **errp)
{
QObjectInputVisitor *qiv = to_qiv(v);
- const char *str = qobject_input_get_keyval(qiv, name, errp);
+ const char *str = qobject_input_get_keyval(qiv, name, consume, errp);
*obj = g_strdup(str);
return !!str;
@@ -578,7 +579,7 @@ static bool qobject_input_type_number_keyval(Visitor *v, const char *name,
double *obj, Error **errp)
{
QObjectInputVisitor *qiv = to_qiv(v);
- const char *str = qobject_input_get_keyval(qiv, name, errp);
+ const char *str = qobject_input_get_keyval(qiv, name, true, errp);
double val;
if (!str) {
@@ -635,7 +636,7 @@ static bool qobject_input_type_size_keyval(Visitor *v, const char *name,
uint64_t *obj, Error **errp)
{
QObjectInputVisitor *qiv = to_qiv(v);
- const char *str = qobject_input_get_keyval(qiv, name, errp);
+ const char *str = qobject_input_get_keyval(qiv, name, true, errp);
if (!str) {
return false;
diff --git a/qapi/qobject-output-visitor.c b/qapi/qobject-output-visitor.c
index 74770edd73c6..36a9fc245b79 100644
--- a/qapi/qobject-output-visitor.c
+++ b/qapi/qobject-output-visitor.c
@@ -173,7 +173,7 @@ static bool qobject_output_type_bool(Visitor *v, const char *name, bool *obj,
}
static bool qobject_output_type_str(Visitor *v, const char *name, char **obj,
- Error **errp)
+ bool consume, Error **errp)
{
QObjectOutputVisitor *qov = to_qov(v);
if (*obj) {
diff --git a/qapi/string-input-visitor.c b/qapi/string-input-visitor.c
index 3f1b9e9b4137..6562bbe3cfd4 100644
--- a/qapi/string-input-visitor.c
+++ b/qapi/string-input-visitor.c
@@ -336,7 +336,7 @@ static bool parse_type_bool(Visitor *v, const char *name, bool *obj,
}
static bool parse_type_str(Visitor *v, const char *name, char **obj,
- Error **errp)
+ bool consume, Error **errp)
{
StringInputVisitor *siv = to_siv(v);
diff --git a/qapi/string-output-visitor.c b/qapi/string-output-visitor.c
index 5115536b1539..9a8cca0d5c68 100644
--- a/qapi/string-output-visitor.c
+++ b/qapi/string-output-visitor.c
@@ -269,7 +269,7 @@ static bool print_type_bool(Visitor *v, const char *name, bool *obj,
}
static bool print_type_str(Visitor *v, const char *name, char **obj,
- Error **errp)
+ bool consume, Error **errp)
{
StringOutputVisitor *sov = to_sov(v);
char *out;
--
2.45.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/4] qapi: Do not consume a value when visit_type_enum() fails
2024-07-06 9:29 [PATCH 0/4] hw/pci: Convert rom_bar into OnOffAuto Akihiko Odaki
2024-07-06 9:29 ` [PATCH 1/4] qapi: Add visit_type_str_preserving() Akihiko Odaki
@ 2024-07-06 9:29 ` Akihiko Odaki
2024-07-06 9:29 ` [PATCH 3/4] hw/pci: Convert rom_bar into OnOffAuto Akihiko Odaki
2024-07-06 9:29 ` [PATCH 4/4] hw/qdev: Remove opts member Akihiko Odaki
3 siblings, 0 replies; 6+ messages in thread
From: Akihiko Odaki @ 2024-07-06 9:29 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, Michael S. Tsirkin, Marcel Apfelbaum,
Alex Williamson, Cédric Le Goater, Paolo Bonzini,
Daniel P. Berrangé, Eduardo Habkost, Sriram Yagnaraman,
Jason Wang, Keith Busch, Klaus Jensen, Markus Armbruster
Cc: qemu-devel, qemu-block, Akihiko Odaki
Consuming a value when visit_type_enum() fails makes it impossible to
reinterpret the value with a different type.
Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
include/qapi/visitor.h | 5 -----
qapi/opts-visitor.c | 5 -----
qapi/qapi-visit-core.c | 4 +++-
3 files changed, 3 insertions(+), 11 deletions(-)
diff --git a/include/qapi/visitor.h b/include/qapi/visitor.h
index b3ae3188edfb..8e841b26428b 100644
--- a/include/qapi/visitor.h
+++ b/include/qapi/visitor.h
@@ -513,11 +513,6 @@ void visit_set_policy(Visitor *v, CompatPolicy *policy);
* is an input visitor.
*
* Return true on success, false on failure.
- *
- * May call visit_type_str() under the hood, and the enum visit may
- * fail even if the corresponding string visit succeeded; this implies
- * that an input visitor's visit_type_str() must have no unwelcome
- * side effects.
*/
bool visit_type_enum(Visitor *v, const char *name, int *obj,
const QEnumLookup *lookup, Error **errp);
diff --git a/qapi/opts-visitor.c b/qapi/opts-visitor.c
index e9fad756e189..d83434b95a56 100644
--- a/qapi/opts-visitor.c
+++ b/qapi/opts-visitor.c
@@ -359,11 +359,6 @@ opts_type_str(Visitor *v, const char *name, char **obj, bool consume,
return false;
}
*obj = g_strdup(opt->str ? opt->str : "");
- /* Note that we consume a string even if this is called as part of
- * an enum visit that later fails because the string is not a
- * valid enum value; this is harmless because tracking what gets
- * consumed only matters to visit_end_struct() as the final error
- * check if there were no other failures during the visit. */
if (consume) {
processed(ov, name);
}
diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c
index 89b52fc99202..1137d472290b 100644
--- a/qapi/qapi-visit-core.c
+++ b/qapi/qapi-visit-core.c
@@ -411,7 +411,7 @@ static bool input_type_enum(Visitor *v, const char *name, int *obj,
int64_t value;
g_autofree char *enum_str = NULL;
- if (!visit_type_str(v, name, &enum_str, errp)) {
+ if (!visit_type_str_preserving(v, name, &enum_str, errp)) {
return false;
}
@@ -430,6 +430,8 @@ static bool input_type_enum(Visitor *v, const char *name, int *obj,
return false;
}
+ enum_str = NULL;
+ visit_type_str(v, name, &enum_str, &error_abort);
*obj = value;
return true;
}
--
2.45.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 3/4] hw/pci: Convert rom_bar into OnOffAuto
2024-07-06 9:29 [PATCH 0/4] hw/pci: Convert rom_bar into OnOffAuto Akihiko Odaki
2024-07-06 9:29 ` [PATCH 1/4] qapi: Add visit_type_str_preserving() Akihiko Odaki
2024-07-06 9:29 ` [PATCH 2/4] qapi: Do not consume a value when visit_type_enum() fails Akihiko Odaki
@ 2024-07-06 9:29 ` Akihiko Odaki
2024-07-08 8:00 ` Daniel P. Berrangé
2024-07-06 9:29 ` [PATCH 4/4] hw/qdev: Remove opts member Akihiko Odaki
3 siblings, 1 reply; 6+ messages in thread
From: Akihiko Odaki @ 2024-07-06 9:29 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, Michael S. Tsirkin, Marcel Apfelbaum,
Alex Williamson, Cédric Le Goater, Paolo Bonzini,
Daniel P. Berrangé, Eduardo Habkost, Sriram Yagnaraman,
Jason Wang, Keith Busch, Klaus Jensen, Markus Armbruster
Cc: qemu-devel, qemu-block, Akihiko Odaki
rom_bar is tristate but was defined as uint32_t so convert it into
OnOffAuto to clarify that. For compatibility, a uint32 value set via
QOM will be converted into OnOffAuto.
Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
docs/igd-assign.txt | 2 +-
include/hw/pci/pci_device.h | 2 +-
hw/pci/pci.c | 57 +++++++++++++++++++++++++++++++++++++--
hw/vfio/pci-quirks.c | 2 +-
hw/vfio/pci.c | 11 ++++----
hw/xen/xen_pt_load_rom.c | 4 +--
tests/qtest/virtio-net-failover.c | 32 +++++++++++-----------
7 files changed, 81 insertions(+), 29 deletions(-)
diff --git a/docs/igd-assign.txt b/docs/igd-assign.txt
index e17bb50789ad..35c6c8e28493 100644
--- a/docs/igd-assign.txt
+++ b/docs/igd-assign.txt
@@ -35,7 +35,7 @@ IGD has two different modes for assignment using vfio-pci:
ISA/LPC bridge device (vfio-pci-igd-lpc-bridge) on the root bus at
PCI address 1f.0.
* The IGD device must have a VGA ROM, either provided via the romfile
- option or loaded automatically through vfio (standard). rombar=0
+ option or loaded automatically through vfio (standard). rombar=off
will disable legacy mode support.
* Hotplug of the IGD device is not supported.
* The IGD device must be a SandyBridge or newer model device.
diff --git a/include/hw/pci/pci_device.h b/include/hw/pci/pci_device.h
index ca151325085d..49b341ce2e27 100644
--- a/include/hw/pci/pci_device.h
+++ b/include/hw/pci/pci_device.h
@@ -147,7 +147,7 @@ struct PCIDevice {
uint32_t romsize;
bool has_rom;
MemoryRegion rom;
- uint32_t rom_bar;
+ OnOffAuto rom_bar;
/* INTx routing notifier */
PCIINTxRoutingNotifier intx_routing_notifier;
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 4c7be5295110..ca8fb5383765 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -67,11 +67,64 @@ static char *pcibus_get_fw_dev_path(DeviceState *dev);
static void pcibus_reset_hold(Object *obj, ResetType type);
static bool pcie_has_upstream_port(PCIDevice *dev);
+static void rom_bar_get(Object *obj, Visitor *v, const char *name, void *opaque,
+ Error **errp)
+{
+ Property *prop = opaque;
+ int *ptr = object_field_prop_ptr(obj, prop);
+
+ visit_type_enum(v, name, ptr, prop->info->enum_table, errp);
+}
+
+static void rom_bar_set(Object *obj, Visitor *v, const char *name, void *opaque,
+ Error **errp)
+{
+ Property *prop = opaque;
+ Error *local_err = NULL;
+ int *ptr = object_field_prop_ptr(obj, prop);
+ uint32_t value;
+
+ visit_type_enum(v, name, ptr, prop->info->enum_table, &local_err);
+ if (!local_err) {
+ return;
+ }
+
+ if (visit_type_uint32(v, name, &value, NULL)) {
+ if (value) {
+ *ptr = ON_OFF_AUTO_ON;
+ warn_report("Specifying a number for rombar is deprecated; replace a non-zero value with 'on'");
+ } else {
+ *ptr = ON_OFF_AUTO_OFF;
+ warn_report("Specifying a number for rombar is deprecated; replace 0 with 'off'");
+ }
+
+ return;
+ }
+
+ error_propagate(errp, local_err);
+}
+
+static void rom_bar_set_default_value(ObjectProperty *op, const Property *prop)
+{
+ object_property_set_default_str(op,
+ qapi_enum_lookup(prop->info->enum_table, prop->defval.i));
+}
+
+static const PropertyInfo qdev_prop_rom_bar = {
+ .name = "OnOffAuto",
+ .description = "on/off/auto",
+ .enum_table = &OnOffAuto_lookup,
+ .get = rom_bar_get,
+ .set = rom_bar_set,
+ .set_default_value = rom_bar_set_default_value,
+};
+
static Property pci_props[] = {
DEFINE_PROP_PCI_DEVFN("addr", PCIDevice, devfn, -1),
DEFINE_PROP_STRING("romfile", PCIDevice, romfile),
DEFINE_PROP_UINT32("romsize", PCIDevice, romsize, UINT32_MAX),
- DEFINE_PROP_UINT32("rombar", PCIDevice, rom_bar, 1),
+ DEFINE_PROP_SIGNED("rombar", PCIDevice, rom_bar, ON_OFF_AUTO_AUTO,
+ qdev_prop_rom_bar, OnOffAuto),
DEFINE_PROP_BIT("multifunction", PCIDevice, cap_present,
QEMU_PCI_CAP_MULTIFUNCTION_BITNR, false),
DEFINE_PROP_BIT("x-pcie-lnksta-dllla", PCIDevice, cap_present,
@@ -2334,7 +2387,7 @@ static void pci_add_option_rom(PCIDevice *pdev, bool is_default_rom,
return;
}
- if (!pdev->rom_bar) {
+ if (pdev->rom_bar == ON_OFF_AUTO_OFF) {
/*
* Load rom via fw_cfg instead of creating a rom bar,
* for 0.11 compatibility.
diff --git a/hw/vfio/pci-quirks.c b/hw/vfio/pci-quirks.c
index 39dae72497e0..0e920ed0691a 100644
--- a/hw/vfio/pci-quirks.c
+++ b/hw/vfio/pci-quirks.c
@@ -33,7 +33,7 @@
* execution as noticed with the BCM 57810 card for lack of a
* more better way to handle such issues.
* The user can still override by specifying a romfile or
- * rombar=1.
+ * rombar=on.
* Please see https://bugs.launchpad.net/qemu/+bug/1284874
* for an analysis of the 57810 card hang. When adding
* a new vendor id/device id combination below, please also add
diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index e03d9f3ba546..502ee2ed0489 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -902,7 +902,7 @@ static void vfio_pci_load_rom(VFIOPCIDevice *vdev)
error_report("vfio-pci: Cannot read device rom at "
"%s", vdev->vbasedev.name);
error_printf("Device option ROM contents are probably invalid "
- "(check dmesg).\nSkip option ROM probe with rombar=0, "
+ "(check dmesg).\nSkip option ROM probe with rombar=off, "
"or load from file with romfile=\n");
return;
}
@@ -1012,11 +1012,10 @@ static void vfio_pci_size_rom(VFIOPCIDevice *vdev)
{
uint32_t orig, size = cpu_to_le32((uint32_t)PCI_ROM_ADDRESS_MASK);
off_t offset = vdev->config_offset + PCI_ROM_ADDRESS;
- DeviceState *dev = DEVICE(vdev);
char *name;
int fd = vdev->vbasedev.fd;
- if (vdev->pdev.romfile || !vdev->pdev.rom_bar) {
+ if (vdev->pdev.romfile || vdev->pdev.rom_bar == ON_OFF_AUTO_OFF) {
/* Since pci handles romfile, just print a message and return */
if (vfio_opt_rom_in_denylist(vdev) && vdev->pdev.romfile) {
warn_report("Device at %s is known to cause system instability"
@@ -1046,17 +1045,17 @@ static void vfio_pci_size_rom(VFIOPCIDevice *vdev)
}
if (vfio_opt_rom_in_denylist(vdev)) {
- if (dev->opts && qdict_haskey(dev->opts, "rombar")) {
+ if (vdev->pdev.rom_bar == ON_OFF_AUTO_ON) {
warn_report("Device at %s is known to cause system instability"
" issues during option rom execution",
vdev->vbasedev.name);
error_printf("Proceeding anyway since user specified"
- " non zero value for rombar\n");
+ " on for rombar\n");
} else {
warn_report("Rom loading for device at %s has been disabled"
" due to system instability issues",
vdev->vbasedev.name);
- error_printf("Specify rombar=1 or romfile to force\n");
+ error_printf("Specify rombar=on or romfile to force\n");
return;
}
}
diff --git a/hw/xen/xen_pt_load_rom.c b/hw/xen/xen_pt_load_rom.c
index 6bc64acd3352..025a6b25a916 100644
--- a/hw/xen/xen_pt_load_rom.c
+++ b/hw/xen/xen_pt_load_rom.c
@@ -26,7 +26,7 @@ void *pci_assign_dev_load_option_rom(PCIDevice *dev,
Object *owner = OBJECT(dev);
/* If loading ROM from file, pci handles it */
- if (dev->romfile || !dev->rom_bar) {
+ if (dev->romfile || dev->rom_bar == ON_OFF_AUTO_OFF) {
return NULL;
}
@@ -71,7 +71,7 @@ void *pci_assign_dev_load_option_rom(PCIDevice *dev,
if (!fread(ptr, 1, st.st_size, fp)) {
error_report("pci-assign: Cannot read from host %s", rom_file);
error_printf("Device option ROM contents are probably invalid "
- "(check dmesg).\nSkip option ROM probe with rombar=0, "
+ "(check dmesg).\nSkip option ROM probe with rombar=off, "
"or load from file with romfile=\n");
goto close_rom;
}
diff --git a/tests/qtest/virtio-net-failover.c b/tests/qtest/virtio-net-failover.c
index 73dfabc2728b..f65b97683fb6 100644
--- a/tests/qtest/virtio-net-failover.c
+++ b/tests/qtest/virtio-net-failover.c
@@ -568,7 +568,7 @@ static void test_hotplug_2_reverse(void)
"{'bus': 'root0',"
"'failover': true,"
"'netdev': 'hs0',"
- "'rombar': 0,"
+ "'rombar': 'off',"
"'romfile': '',"
"'mac': '"MAC_STANDBY0"'}");
@@ -655,7 +655,7 @@ static void test_migrate_out(gconstpointer opaque)
"{'bus': 'root1',"
"'failover_pair_id': 'standby0',"
"'netdev': 'hs1',"
- "'rombar': 0,"
+ "'rombar': 'off',"
"'romfile': '',"
"'mac': '"MAC_PRIMARY0"'}");
@@ -765,7 +765,7 @@ static void test_migrate_in(gconstpointer opaque)
"{'bus': 'root1',"
"'failover_pair_id': 'standby0',"
"'netdev': 'hs1',"
- "'rombar': 0,"
+ "'rombar': 'off',"
"'romfile': '',"
"'mac': '"MAC_PRIMARY0"'}");
@@ -819,7 +819,7 @@ static void test_off_migrate_out(gconstpointer opaque)
"{'bus': 'root1',"
"'failover_pair_id': 'standby0',"
"'netdev': 'hs1',"
- "'rombar': 0,"
+ "'rombar': 'off',"
"'romfile': '',"
"'mac': '"MAC_PRIMARY0"'}");
@@ -887,7 +887,7 @@ static void test_off_migrate_in(gconstpointer opaque)
"{'bus': 'root1',"
"'failover_pair_id': 'standby0',"
"'netdev': 'hs1',"
- "'rombar': 0,"
+ "'rombar': 'off',"
"'romfile': '',"
"'mac': '"MAC_PRIMARY0"'}");
@@ -938,7 +938,7 @@ static void test_guest_off_migrate_out(gconstpointer opaque)
"{'bus': 'root1',"
"'failover_pair_id': 'standby0',"
"'netdev': 'hs1',"
- "'rombar': 0,"
+ "'rombar': 'off',"
"'romfile': '',"
"'mac': '"MAC_PRIMARY0"'}");
@@ -1014,7 +1014,7 @@ static void test_guest_off_migrate_in(gconstpointer opaque)
"{'bus': 'root1',"
"'failover_pair_id': 'standby0',"
"'netdev': 'hs1',"
- "'rombar': 0,"
+ "'rombar': 'off',"
"'romfile': '',"
"'mac': '"MAC_PRIMARY0"'}");
@@ -1065,7 +1065,7 @@ static void test_migrate_guest_off_abort(gconstpointer opaque)
"{'bus': 'root1',"
"'failover_pair_id': 'standby0',"
"'netdev': 'hs1',"
- "'rombar': 0,"
+ "'rombar': 'off',"
"'romfile': '',"
"'mac': '"MAC_PRIMARY0"'}");
@@ -1170,7 +1170,7 @@ static void test_migrate_abort_wait_unplug(gconstpointer opaque)
"{'bus': 'root1',"
"'failover_pair_id': 'standby0',"
"'netdev': 'hs1',"
- "'rombar': 0,"
+ "'rombar': 'off',"
"'romfile': '',"
"'mac': '"MAC_PRIMARY0"'}");
@@ -1259,7 +1259,7 @@ static void test_migrate_abort_active(gconstpointer opaque)
"{'bus': 'root1',"
"'failover_pair_id': 'standby0',"
"'netdev': 'hs1',"
- "'rombar': 0,"
+ "'rombar': 'off',"
"'romfile': '',"
"'mac': '"MAC_PRIMARY0"'}");
@@ -1358,7 +1358,7 @@ static void test_migrate_off_abort(gconstpointer opaque)
"{'bus': 'root1',"
"'failover_pair_id': 'standby0',"
"'netdev': 'hs1',"
- "'rombar': 0,"
+ "'rombar': 'off',"
"'romfile': '',"
"'mac': '"MAC_PRIMARY0"'}");
@@ -1450,7 +1450,7 @@ static void test_migrate_abort_timeout(gconstpointer opaque)
"{'bus': 'root1',"
"'failover_pair_id': 'standby0',"
"'netdev': 'hs1',"
- "'rombar': 0,"
+ "'rombar': 'off',"
"'romfile': '',"
"'mac': '"MAC_PRIMARY0"'}");
@@ -1543,7 +1543,7 @@ static void test_multi_out(gconstpointer opaque)
"{'bus': 'root1',"
"'failover_pair_id': 'standby0',"
"'netdev': 'hs1',"
- "'rombar': 0,"
+ "'rombar': 'off',"
"'romfile': '',"
"'mac': '"MAC_PRIMARY0"'}");
@@ -1574,7 +1574,7 @@ static void test_multi_out(gconstpointer opaque)
"{'bus': 'root3',"
"'failover_pair_id': 'standby1',"
"'netdev': 'hs3',"
- "'rombar': 0,"
+ "'rombar': 'off',"
"'romfile': '',"
"'mac': '"MAC_PRIMARY1"'}");
@@ -1713,7 +1713,7 @@ static void test_multi_in(gconstpointer opaque)
"{'bus': 'root1',"
"'failover_pair_id': 'standby0',"
"'netdev': 'hs1',"
- "'rombar': 0,"
+ "'rombar': 'off',"
"'romfile': '',"
"'mac': '"MAC_PRIMARY0"'}");
@@ -1737,7 +1737,7 @@ static void test_multi_in(gconstpointer opaque)
"{'bus': 'root3',"
"'failover_pair_id': 'standby1',"
"'netdev': 'hs3',"
- "'rombar': 0,"
+ "'rombar': 'off',"
"'romfile': '',"
"'mac': '"MAC_PRIMARY1"'}");
--
2.45.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 4/4] hw/qdev: Remove opts member
2024-07-06 9:29 [PATCH 0/4] hw/pci: Convert rom_bar into OnOffAuto Akihiko Odaki
` (2 preceding siblings ...)
2024-07-06 9:29 ` [PATCH 3/4] hw/pci: Convert rom_bar into OnOffAuto Akihiko Odaki
@ 2024-07-06 9:29 ` Akihiko Odaki
3 siblings, 0 replies; 6+ messages in thread
From: Akihiko Odaki @ 2024-07-06 9:29 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, Michael S. Tsirkin, Marcel Apfelbaum,
Alex Williamson, Cédric Le Goater, Paolo Bonzini,
Daniel P. Berrangé, Eduardo Habkost, Sriram Yagnaraman,
Jason Wang, Keith Busch, Klaus Jensen, Markus Armbruster
Cc: qemu-devel, qemu-block, Akihiko Odaki
It is no longer used.
Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
---
include/hw/qdev-core.h | 4 ----
hw/core/qdev.c | 1 -
system/qdev-monitor.c | 12 +++++++-----
3 files changed, 7 insertions(+), 10 deletions(-)
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index 5336728a23f6..40f2d185f17c 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -237,10 +237,6 @@ struct DeviceState {
* @pending_deleted_expires_ms: optional timeout for deletion events
*/
int64_t pending_deleted_expires_ms;
- /**
- * @opts: QDict of options for the device
- */
- QDict *opts;
/**
* @hotplugged: was device added after PHASE_MACHINE_READY?
*/
diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index f3a996f57dee..2fc84699d432 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -706,7 +706,6 @@ static void device_finalize(Object *obj)
dev->canonical_path = NULL;
}
- qobject_unref(dev->opts);
g_free(dev->id);
}
diff --git a/system/qdev-monitor.c b/system/qdev-monitor.c
index 6af6ef7d667f..3551989d5153 100644
--- a/system/qdev-monitor.c
+++ b/system/qdev-monitor.c
@@ -624,6 +624,7 @@ DeviceState *qdev_device_add_from_qdict(const QDict *opts,
char *id;
DeviceState *dev = NULL;
BusState *bus = NULL;
+ QDict *properties;
driver = qdict_get_try_str(opts, "driver");
if (!driver) {
@@ -705,13 +706,14 @@ DeviceState *qdev_device_add_from_qdict(const QDict *opts,
}
/* set properties */
- dev->opts = qdict_clone_shallow(opts);
- qdict_del(dev->opts, "driver");
- qdict_del(dev->opts, "bus");
- qdict_del(dev->opts, "id");
+ properties = qdict_clone_shallow(opts);
+ qdict_del(properties, "driver");
+ qdict_del(properties, "bus");
+ qdict_del(properties, "id");
- object_set_properties_from_keyval(&dev->parent_obj, dev->opts, from_json,
+ object_set_properties_from_keyval(&dev->parent_obj, properties, from_json,
errp);
+ qobject_unref(properties);
if (*errp) {
goto err_del_dev;
}
--
2.45.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 3/4] hw/pci: Convert rom_bar into OnOffAuto
2024-07-06 9:29 ` [PATCH 3/4] hw/pci: Convert rom_bar into OnOffAuto Akihiko Odaki
@ 2024-07-08 8:00 ` Daniel P. Berrangé
0 siblings, 0 replies; 6+ messages in thread
From: Daniel P. Berrangé @ 2024-07-08 8:00 UTC (permalink / raw)
To: Akihiko Odaki
Cc: Philippe Mathieu-Daudé, Michael S. Tsirkin, Marcel Apfelbaum,
Alex Williamson, Cédric Le Goater, Paolo Bonzini,
Eduardo Habkost, Sriram Yagnaraman, Jason Wang, Keith Busch,
Klaus Jensen, Markus Armbruster, qemu-devel, qemu-block
On Sat, Jul 06, 2024 at 06:29:23PM +0900, Akihiko Odaki wrote:
> rom_bar is tristate but was defined as uint32_t so convert it into
> OnOffAuto to clarify that. For compatibility, a uint32 value set via
> QOM will be converted into OnOffAuto.
>
> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
> ---
> docs/igd-assign.txt | 2 +-
> include/hw/pci/pci_device.h | 2 +-
> hw/pci/pci.c | 57 +++++++++++++++++++++++++++++++++++++--
> hw/vfio/pci-quirks.c | 2 +-
> hw/vfio/pci.c | 11 ++++----
> hw/xen/xen_pt_load_rom.c | 4 +--
> tests/qtest/virtio-net-failover.c | 32 +++++++++++-----------
> 7 files changed, 81 insertions(+), 29 deletions(-)
> +static void rom_bar_set(Object *obj, Visitor *v, const char *name, void *opaque,
> + Error **errp)
> +{
> + Property *prop = opaque;
> + Error *local_err = NULL;
> + int *ptr = object_field_prop_ptr(obj, prop);
> + uint32_t value;
> +
> + visit_type_enum(v, name, ptr, prop->info->enum_table, &local_err);
> + if (!local_err) {
> + return;
> + }
> +
> + if (visit_type_uint32(v, name, &value, NULL)) {
> + if (value) {
> + *ptr = ON_OFF_AUTO_ON;
> + warn_report("Specifying a number for rombar is deprecated; replace a non-zero value with 'on'");
> + } else {
> + *ptr = ON_OFF_AUTO_OFF;
> + warn_report("Specifying a number for rombar is deprecated; replace 0 with 'off'");
> + }
If you're going to say something is deprecated, you need to update
the deprecated.rst docs to make this visible to users.
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 :|
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-07-08 8:02 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-07-06 9:29 [PATCH 0/4] hw/pci: Convert rom_bar into OnOffAuto Akihiko Odaki
2024-07-06 9:29 ` [PATCH 1/4] qapi: Add visit_type_str_preserving() Akihiko Odaki
2024-07-06 9:29 ` [PATCH 2/4] qapi: Do not consume a value when visit_type_enum() fails Akihiko Odaki
2024-07-06 9:29 ` [PATCH 3/4] hw/pci: Convert rom_bar into OnOffAuto Akihiko Odaki
2024-07-08 8:00 ` Daniel P. Berrangé
2024-07-06 9:29 ` [PATCH 4/4] hw/qdev: Remove opts member Akihiko Odaki
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.