* [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* 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
* [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