Re: [PULL 0/4] hw/nvme updates
On Jan 10 08:17, Klaus Jensen wrote: > From: Klaus Jensen > > Hi, > > The following changes since commit 528d9f33cad5245c1099d77084c78bb2244d5143: > > Merge tag 'pull-tcg-20230106' of https://gitlab.com/rth7680/qemu into > staging (2023-01-08 11:23:17 +) > > are available in the Git repository at: > > git://git.infradead.org/qemu-nvme.git tags/nvme-next-pull-request > > for you to fetch changes up to fa5db2aa168bdc0f15c269b6212ef47632fab8ba: > > hw/nvme: fix missing cq eventidx update (2023-01-09 08:48:46 +0100) > > > hw/nvme updates > > > > Klaus Jensen (4): > hw/nvme: use QOM accessors > hw/nvme: rename shadow doorbell related trace events > hw/nvme: fix missing endian conversions for doorbell buffers > hw/nvme: fix missing cq eventidx update > > hw/nvme/ctrl.c | 121 ++- > hw/nvme/trace-events | 8 +-- > 2 files changed, 78 insertions(+), 51 deletions(-) > > -- > 2.39.0 > Argh. I forgot to update the pull url to something https://. Do I need to send a new pull? signature.asc Description: PGP signature
[PULL 0/4] hw/nvme updates
From: Klaus Jensen Hi, The following changes since commit 528d9f33cad5245c1099d77084c78bb2244d5143: Merge tag 'pull-tcg-20230106' of https://gitlab.com/rth7680/qemu into staging (2023-01-08 11:23:17 +) are available in the Git repository at: git://git.infradead.org/qemu-nvme.git tags/nvme-next-pull-request for you to fetch changes up to fa5db2aa168bdc0f15c269b6212ef47632fab8ba: hw/nvme: fix missing cq eventidx update (2023-01-09 08:48:46 +0100) hw/nvme updates Klaus Jensen (4): hw/nvme: use QOM accessors hw/nvme: rename shadow doorbell related trace events hw/nvme: fix missing endian conversions for doorbell buffers hw/nvme: fix missing cq eventidx update hw/nvme/ctrl.c | 121 ++- hw/nvme/trace-events | 8 +-- 2 files changed, 78 insertions(+), 51 deletions(-) -- 2.39.0
[PULL 2/4] hw/nvme: rename shadow doorbell related trace events
From: Klaus Jensen
Rename the trace events related to writing the event index and reading
the doorbell value to make it more clear that the event is associated
with an actual update (write or read respectively).
Reviewed-by: Philippe Mathieu-Daudé
Reviewed-by: Keith Busch
Signed-off-by: Klaus Jensen
---
hw/nvme/ctrl.c | 11 +++
hw/nvme/trace-events | 8
2 files changed, 11 insertions(+), 8 deletions(-)
diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
index 78c2f4e39d0a..cfe16476f0a4 100644
--- a/hw/nvme/ctrl.c
+++ b/hw/nvme/ctrl.c
@@ -1337,8 +1337,9 @@ static inline void nvme_blk_write(BlockBackend *blk,
int64_t offset,
static void nvme_update_cq_head(NvmeCQueue *cq)
{
pci_dma_read(PCI_DEVICE(cq->ctrl), cq->db_addr, >head,
-sizeof(cq->head));
-trace_pci_nvme_shadow_doorbell_cq(cq->cqid, cq->head);
+ sizeof(cq->head));
+
+trace_pci_nvme_update_cq_head(cq->cqid, cq->head);
}
static void nvme_post_cqes(void *opaque)
@@ -6147,16 +6148,18 @@ static uint16_t nvme_admin_cmd(NvmeCtrl *n, NvmeRequest
*req)
static void nvme_update_sq_eventidx(const NvmeSQueue *sq)
{
+trace_pci_nvme_update_sq_eventidx(sq->sqid, sq->tail);
+
pci_dma_write(PCI_DEVICE(sq->ctrl), sq->ei_addr, >tail,
sizeof(sq->tail));
-trace_pci_nvme_eventidx_sq(sq->sqid, sq->tail);
}
static void nvme_update_sq_tail(NvmeSQueue *sq)
{
pci_dma_read(PCI_DEVICE(sq->ctrl), sq->db_addr, >tail,
sizeof(sq->tail));
-trace_pci_nvme_shadow_doorbell_sq(sq->sqid, sq->tail);
+
+trace_pci_nvme_update_sq_tail(sq->sqid, sq->tail);
}
static void nvme_process_sq(void *opaque)
diff --git a/hw/nvme/trace-events b/hw/nvme/trace-events
index fccb79f48973..b16f2260b4fd 100644
--- a/hw/nvme/trace-events
+++ b/hw/nvme/trace-events
@@ -84,8 +84,8 @@ pci_nvme_enqueue_event_noqueue(int queued) "queued %d"
pci_nvme_enqueue_event_masked(uint8_t typ) "type 0x%"PRIx8""
pci_nvme_no_outstanding_aers(void) "ignoring event; no outstanding AERs"
pci_nvme_enqueue_req_completion(uint16_t cid, uint16_t cqid, uint32_t dw0,
uint32_t dw1, uint16_t status) "cid %"PRIu16" cqid %"PRIu16" dw0 0x%"PRIx32"
dw1 0x%"PRIx32" status 0x%"PRIx16""
-pci_nvme_eventidx_cq(uint16_t cqid, uint16_t new_eventidx) "cqid %"PRIu16"
new_eventidx %"PRIu16""
-pci_nvme_eventidx_sq(uint16_t sqid, uint16_t new_eventidx) "sqid %"PRIu16"
new_eventidx %"PRIu16""
+pci_nvme_update_cq_eventidx(uint16_t cqid, uint16_t new_eventidx) "cqid
%"PRIu16" new_eventidx %"PRIu16""
+pci_nvme_update_sq_eventidx(uint16_t sqid, uint16_t new_eventidx) "sqid
%"PRIu16" new_eventidx %"PRIu16""
pci_nvme_mmio_read(uint64_t addr, unsigned size) "addr 0x%"PRIx64" size %d"
pci_nvme_mmio_write(uint64_t addr, uint64_t data, unsigned size) "addr
0x%"PRIx64" data 0x%"PRIx64" size %d"
pci_nvme_mmio_doorbell_cq(uint16_t cqid, uint16_t new_head) "cqid %"PRIu16"
new_head %"PRIu16""
@@ -102,8 +102,8 @@ pci_nvme_mmio_start_success(void) "setting controller
enable bit succeeded"
pci_nvme_mmio_stopped(void) "cleared controller enable bit"
pci_nvme_mmio_shutdown_set(void) "shutdown bit set"
pci_nvme_mmio_shutdown_cleared(void) "shutdown bit cleared"
-pci_nvme_shadow_doorbell_cq(uint16_t cqid, uint16_t new_shadow_doorbell) "cqid
%"PRIu16" new_shadow_doorbell %"PRIu16""
-pci_nvme_shadow_doorbell_sq(uint16_t sqid, uint16_t new_shadow_doorbell) "sqid
%"PRIu16" new_shadow_doorbell %"PRIu16""
+pci_nvme_update_cq_head(uint16_t cqid, uint16_t new_head) "cqid %"PRIu16"
new_head %"PRIu16""
+pci_nvme_update_sq_tail(uint16_t sqid, uint16_t new_tail) "sqid %"PRIu16"
new_tail %"PRIu16""
pci_nvme_open_zone(uint64_t slba, uint32_t zone_idx, int all) "open zone,
slba=%"PRIu64", idx=%"PRIu32", all=%"PRIi32""
pci_nvme_close_zone(uint64_t slba, uint32_t zone_idx, int all) "close zone,
slba=%"PRIu64", idx=%"PRIu32", all=%"PRIi32""
pci_nvme_finish_zone(uint64_t slba, uint32_t zone_idx, int all) "finish zone,
slba=%"PRIu64", idx=%"PRIu32", all=%"PRIi32""
--
2.39.0
[PULL 3/4] hw/nvme: fix missing endian conversions for doorbell buffers
From: Klaus Jensen
The eventidx and doorbell value are not handling endianness correctly.
Fix this.
Fixes: 3f7fe8de3d49 ("hw/nvme: Implement shadow doorbell buffer support")
Cc: qemu-sta...@nongnu.org
Reported-by: Guenter Roeck
Reviewed-by: Keith Busch
Signed-off-by: Klaus Jensen
---
hw/nvme/ctrl.c | 19 +--
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
index cfe16476f0a4..28e02ec7baa6 100644
--- a/hw/nvme/ctrl.c
+++ b/hw/nvme/ctrl.c
@@ -1336,8 +1336,11 @@ static inline void nvme_blk_write(BlockBackend *blk,
int64_t offset,
static void nvme_update_cq_head(NvmeCQueue *cq)
{
-pci_dma_read(PCI_DEVICE(cq->ctrl), cq->db_addr, >head,
- sizeof(cq->head));
+uint32_t v;
+
+pci_dma_read(PCI_DEVICE(cq->ctrl), cq->db_addr, , sizeof(v));
+
+cq->head = le32_to_cpu(v);
trace_pci_nvme_update_cq_head(cq->cqid, cq->head);
}
@@ -6148,16 +6151,20 @@ static uint16_t nvme_admin_cmd(NvmeCtrl *n, NvmeRequest
*req)
static void nvme_update_sq_eventidx(const NvmeSQueue *sq)
{
+uint32_t v = cpu_to_le32(sq->tail);
+
trace_pci_nvme_update_sq_eventidx(sq->sqid, sq->tail);
-pci_dma_write(PCI_DEVICE(sq->ctrl), sq->ei_addr, >tail,
- sizeof(sq->tail));
+pci_dma_write(PCI_DEVICE(sq->ctrl), sq->ei_addr, , sizeof(v));
}
static void nvme_update_sq_tail(NvmeSQueue *sq)
{
-pci_dma_read(PCI_DEVICE(sq->ctrl), sq->db_addr, >tail,
- sizeof(sq->tail));
+uint32_t v;
+
+pci_dma_read(PCI_DEVICE(sq->ctrl), sq->db_addr, , sizeof(v));
+
+sq->tail = le32_to_cpu(v);
trace_pci_nvme_update_sq_tail(sq->sqid, sq->tail);
}
--
2.39.0
[PULL 4/4] hw/nvme: fix missing cq eventidx update
From: Klaus Jensen
Prior to reading the shadow doorbell cq head, we have to update the
eventidx. Otherwise, we risk that the driver will skip an mmio doorbell
write. This happens on riscv64, as reported by Guenter.
Adding the missing update to the cq eventidx fixes the issue.
Fixes: 3f7fe8de3d49 ("hw/nvme: Implement shadow doorbell buffer support")
Cc: qemu-sta...@nongnu.org
Cc: qemu-ri...@nongnu.org
Reported-by: Guenter Roeck
Reviewed-by: Keith Busch
Signed-off-by: Klaus Jensen
---
hw/nvme/ctrl.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
index 28e02ec7baa6..226480033771 100644
--- a/hw/nvme/ctrl.c
+++ b/hw/nvme/ctrl.c
@@ -1334,6 +1334,15 @@ static inline void nvme_blk_write(BlockBackend *blk,
int64_t offset,
}
}
+static void nvme_update_cq_eventidx(const NvmeCQueue *cq)
+{
+uint32_t v = cpu_to_le32(cq->head);
+
+trace_pci_nvme_update_cq_eventidx(cq->cqid, cq->head);
+
+pci_dma_write(PCI_DEVICE(cq->ctrl), cq->ei_addr, , sizeof(v));
+}
+
static void nvme_update_cq_head(NvmeCQueue *cq)
{
uint32_t v;
@@ -1358,6 +1367,7 @@ static void nvme_post_cqes(void *opaque)
hwaddr addr;
if (n->dbbuf_enabled) {
+nvme_update_cq_eventidx(cq);
nvme_update_cq_head(cq);
}
--
2.39.0
[PULL 1/4] hw/nvme: use QOM accessors
From: Klaus Jensen
Replace various ->parent_obj use with the equivalent QOM accessors.
Reviewed-by: Philippe Mathieu-Daudé
Signed-off-by: Klaus Jensen
---
hw/nvme/ctrl.c | 89 +++---
1 file changed, 48 insertions(+), 41 deletions(-)
diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
index 4a0c51a9477e..78c2f4e39d0a 100644
--- a/hw/nvme/ctrl.c
+++ b/hw/nvme/ctrl.c
@@ -449,7 +449,7 @@ static int nvme_addr_read(NvmeCtrl *n, hwaddr addr, void
*buf, int size)
return 0;
}
-return pci_dma_read(>parent_obj, addr, buf, size);
+return pci_dma_read(PCI_DEVICE(n), addr, buf, size);
}
static int nvme_addr_write(NvmeCtrl *n, hwaddr addr, const void *buf, int size)
@@ -469,7 +469,7 @@ static int nvme_addr_write(NvmeCtrl *n, hwaddr addr, const
void *buf, int size)
return 0;
}
-return pci_dma_write(>parent_obj, addr, buf, size);
+return pci_dma_write(PCI_DEVICE(n), addr, buf, size);
}
static bool nvme_nsid_valid(NvmeCtrl *n, uint32_t nsid)
@@ -514,24 +514,27 @@ static uint8_t nvme_sq_empty(NvmeSQueue *sq)
static void nvme_irq_check(NvmeCtrl *n)
{
+PCIDevice *pci = PCI_DEVICE(n);
uint32_t intms = ldl_le_p(>bar.intms);
-if (msix_enabled(&(n->parent_obj))) {
+if (msix_enabled(pci)) {
return;
}
if (~intms & n->irq_status) {
-pci_irq_assert(>parent_obj);
+pci_irq_assert(pci);
} else {
-pci_irq_deassert(>parent_obj);
+pci_irq_deassert(pci);
}
}
static void nvme_irq_assert(NvmeCtrl *n, NvmeCQueue *cq)
{
+PCIDevice *pci = PCI_DEVICE(n);
+
if (cq->irq_enabled) {
-if (msix_enabled(&(n->parent_obj))) {
+if (msix_enabled(pci)) {
trace_pci_nvme_irq_msix(cq->vector);
-msix_notify(&(n->parent_obj), cq->vector);
+msix_notify(pci, cq->vector);
} else {
trace_pci_nvme_irq_pin();
assert(cq->vector < 32);
@@ -546,7 +549,7 @@ static void nvme_irq_assert(NvmeCtrl *n, NvmeCQueue *cq)
static void nvme_irq_deassert(NvmeCtrl *n, NvmeCQueue *cq)
{
if (cq->irq_enabled) {
-if (msix_enabled(&(n->parent_obj))) {
+if (msix_enabled(PCI_DEVICE(n))) {
return;
} else {
assert(cq->vector < 32);
@@ -570,7 +573,7 @@ static void nvme_req_clear(NvmeRequest *req)
static inline void nvme_sg_init(NvmeCtrl *n, NvmeSg *sg, bool dma)
{
if (dma) {
-pci_dma_sglist_init(>qsg, >parent_obj, 0);
+pci_dma_sglist_init(>qsg, PCI_DEVICE(n), 0);
sg->flags = NVME_SG_DMA;
} else {
qemu_iovec_init(>iov, 0);
@@ -1333,7 +1336,7 @@ static inline void nvme_blk_write(BlockBackend *blk,
int64_t offset,
static void nvme_update_cq_head(NvmeCQueue *cq)
{
-pci_dma_read(>ctrl->parent_obj, cq->db_addr, >head,
+pci_dma_read(PCI_DEVICE(cq->ctrl), cq->db_addr, >head,
sizeof(cq->head));
trace_pci_nvme_shadow_doorbell_cq(cq->cqid, cq->head);
}
@@ -1363,7 +1366,7 @@ static void nvme_post_cqes(void *opaque)
req->cqe.sq_id = cpu_to_le16(sq->sqid);
req->cqe.sq_head = cpu_to_le16(sq->head);
addr = cq->dma_addr + cq->tail * n->cqe_size;
-ret = pci_dma_write(>parent_obj, addr, (void *)>cqe,
+ret = pci_dma_write(PCI_DEVICE(n), addr, (void *)>cqe,
sizeof(req->cqe));
if (ret) {
trace_pci_nvme_err_addr_write(addr);
@@ -4615,6 +4618,7 @@ static uint16_t nvme_get_log(NvmeCtrl *n, NvmeRequest
*req)
static void nvme_free_cq(NvmeCQueue *cq, NvmeCtrl *n)
{
+PCIDevice *pci = PCI_DEVICE(n);
uint16_t offset = (cq->cqid << 3) + (1 << 2);
n->cq[cq->cqid] = NULL;
@@ -4625,8 +4629,8 @@ static void nvme_free_cq(NvmeCQueue *cq, NvmeCtrl *n)
event_notifier_set_handler(>notifier, NULL);
event_notifier_cleanup(>notifier);
}
-if (msix_enabled(>parent_obj)) {
-msix_vector_unuse(>parent_obj, cq->vector);
+if (msix_enabled(pci)) {
+msix_vector_unuse(pci, cq->vector);
}
if (cq->cqid) {
g_free(cq);
@@ -4664,8 +4668,10 @@ static void nvme_init_cq(NvmeCQueue *cq, NvmeCtrl *n,
uint64_t dma_addr,
uint16_t cqid, uint16_t vector, uint16_t size,
uint16_t irq_enabled)
{
-if (msix_enabled(>parent_obj)) {
-msix_vector_use(>parent_obj, vector);
+PCIDevice *pci = PCI_DEVICE(n);
+
+if (msix_enabled(pci)) {
+msix_vector_use(pci, vector);
}
cq->ctrl = n;
cq->cqid = cqid;
@@ -4716,7 +4722,7 @@ static uint16_t nvme_create_cq(NvmeCtrl *n, NvmeRequest
*req)
trace_pci_nvme_err_invalid_create_cq_addr(prp1);
return NVME_INVALID_PRP_OFFSET | NVME_DNR;
}
-if (unlikely(!msix_enabled(>parent_obj) && vector)) {
+if (unlikely(!msix_enabled(PCI_DEVICE(n)) && vector)) {
trace_pci_nvme_err_invalid_create_cq_vector(vector);
[RFC PATCH 4/4] qom: Warn when deprecated class property can be removed
Per docs/system/deprecated.rst, a deprecated feature can be
removed after 2 releases. Since we commit when a class property
is deprecated, we can warn when the deprecation period is over.
See also commit ef1f5b0a96 ("docs: clarify deprecation schedule").
Signed-off-by: Philippe Mathieu-Daudé
---
qom/object.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/qom/object.c b/qom/object.c
index 05b97cd424..cb829f1e44 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -17,6 +17,7 @@
#include "qom/object_interfaces.h"
#include "qemu/cutils.h"
#include "qemu/memalign.h"
+#include "qemu/qemu-version.h"
#include "qapi/visitor.h"
#include "qapi/string-input-visitor.h"
#include "qapi/string-output-visitor.h"
@@ -1300,6 +1301,12 @@ void object_class_property_deprecate(ObjectClass *klass,
ObjectProperty *prop = object_class_property_find(klass, name);
assert(prop);
+if (qemu_version_delta_current(version_major, version_minor) <= 2) {
+warn_report_once("Property '%s.%s' has been deprecated in release"
+ " v%u.%u and can be removed.",
+ object_class_get_name(klass), name,
+ version_major, version_minor);
+}
prop->deprecation_reason = reason;
}
--
2.38.1
[RFC PATCH 1/4] qom: Introduce object_class_property_deprecate()
Introduce object_class_property_deprecate() to register
a QOM property as deprecated. When this property's getter /
setter is called, a deprecation warning is displayed on the
monitor.
Inspired-by: Daniel P. Berrange
Signed-off-by: Philippe Mathieu-Daudé
---
include/qom/object.h | 17 +
qom/object.c | 23 +++
2 files changed, 40 insertions(+)
diff --git a/include/qom/object.h b/include/qom/object.h
index ef7258a5e1..b76724292c 100644
--- a/include/qom/object.h
+++ b/include/qom/object.h
@@ -97,6 +97,7 @@ struct ObjectProperty
ObjectPropertyInit *init;
void *opaque;
QObject *defval;
+const char *deprecation_reason;
};
/**
@@ -1075,6 +1076,22 @@ ObjectProperty *object_class_property_add(ObjectClass
*klass, const char *name,
ObjectPropertyRelease *release,
void *opaque);
+/**
+ * object_class_property_deprecate:
+ * @klass: the class to add a property to
+ * @name: the name of the property. This can contain any character except for
+ * a forward slash. In general, you should use hyphens '-' instead of
+ * underscores '_' when naming properties.
+ * @reason: the deprecation reason.
+ * @version_major: the major version since this property is deprecated.
+ * @version_minor: the minor version since this property is deprecated.
+ *
+ * Deprecate a class property.
+ */
+void object_class_property_deprecate(ObjectClass *klass,
+ const char *name, const char *reason,
+ int version_major, int version_minor);
+
/**
* object_property_set_default_bool:
* @prop: the property to set
diff --git a/qom/object.c b/qom/object.c
index e25f1e96db..05b97cd424 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -1293,6 +1293,16 @@ object_class_property_add(ObjectClass *klass,
return prop;
}
+void object_class_property_deprecate(ObjectClass *klass,
+ const char *name, const char *reason,
+ int version_major, int version_minor)
+{
+ObjectProperty *prop = object_class_property_find(klass, name);
+
+assert(prop);
+prop->deprecation_reason = reason;
+}
+
ObjectProperty *object_property_find(Object *obj, const char *name)
{
ObjectProperty *prop;
@@ -1382,6 +1392,17 @@ void object_property_del(Object *obj, const char *name)
g_hash_table_remove(obj->properties, name);
}
+static void object_property_check_deprecation(const Object *obj,
+ const char *name,
+ const ObjectProperty *prop)
+{
+if (!prop->deprecation_reason) {
+return;
+}
+warn_report("Property '%s.%s' is deprecated (%s).",
+object_get_typename(obj), name, prop->deprecation_reason);
+}
+
bool object_property_get(Object *obj, const char *name, Visitor *v,
Error **errp)
{
@@ -1392,6 +1413,7 @@ bool object_property_get(Object *obj, const char *name,
Visitor *v,
return false;
}
+object_property_check_deprecation(obj, name, prop);
if (!prop->get) {
error_setg(errp, "Property '%s.%s' is not readable",
object_get_typename(obj), name);
@@ -1412,6 +1434,7 @@ bool object_property_set(Object *obj, const char *name,
Visitor *v,
return false;
}
+object_property_check_deprecation(obj, name, prop);
if (!prop->set) {
error_setg(errp, "Property '%s.%s' is not writable",
object_get_typename(obj), name);
--
2.38.1
[RFC PATCH 3/4] util: Introduce helpers to compare QEMU versions
Add qemu_version_delta() to compare 2 QEMU versions,
and qemu_version_delta_current() to compare with the
current QEMU version.
Signed-off-by: Philippe Mathieu-Daudé
---
include/qemu/qemu-version.h | 36
util/meson.build| 1 +
util/qemu-version.c | 37 +
3 files changed, 74 insertions(+)
create mode 100644 include/qemu/qemu-version.h
create mode 100644 util/qemu-version.c
diff --git a/include/qemu/qemu-version.h b/include/qemu/qemu-version.h
new file mode 100644
index 00..c9274bfaf0
--- /dev/null
+++ b/include/qemu/qemu-version.h
@@ -0,0 +1,36 @@
+/*
+ * Utility function around QEMU release version
+ *
+ * Copyright (c) 2023 Linaro Ltd
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#ifndef QEMU_UTIL_VERSION_H
+#define QEMU_UTIL_VERSION_H
+
+/**
+ * qemu_version_delta - Return delta between two release versions ('A' and
'B').
+ * @version_major_a: Version 'A' major number
+ * @version_minor_a: Version 'A' minor number
+ * @version_major_b: Version 'B' major number
+ * @version_minor_b: Version 'B' minor number
+ *
+ * Returns a negative number is returned if 'A' is older than 'B', or positive
+ * if 'A' is newer than 'B'. The number represents the number of minor
versions.
+ */
+int qemu_version_delta(unsigned version_major_a, unsigned version_minor_a,
+ unsigned version_major_b, unsigned version_minor_b);
+
+/**
+ * qemu_version_delta_current - Return delta with current QEMU release version.
+ * @version_major: The major version
+ * @version_minor: The minor version
+ *
+ * Returns the number of minor versions between the current released
+ * version and the requested $major.$minor. A negative number is returned
+ * for older versions and positive for newer.
+ */
+int qemu_version_delta_current(unsigned version_major, unsigned version_minor);
+
+#endif
diff --git a/util/meson.build b/util/meson.build
index d8d109ff84..655debeec1 100644
--- a/util/meson.build
+++ b/util/meson.build
@@ -58,6 +58,7 @@ util_ss.add(files('yank.c'))
util_ss.add(files('int128.c'))
util_ss.add(files('memalign.c'))
util_ss.add(files('interval-tree.c'))
+util_ss.add(files('qemu-version.c'))
if have_user
util_ss.add(files('selfmap.c'))
diff --git a/util/qemu-version.c b/util/qemu-version.c
new file mode 100644
index 00..d409a6e574
--- /dev/null
+++ b/util/qemu-version.c
@@ -0,0 +1,37 @@
+/*
+ * Utility function around QEMU release version
+ *
+ * Copyright (c) 2023 Linaro Ltd
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/qemu-version.h"
+#include "config-host.h"
+
+#define QEMU_FIRST_MAJOR_VERSION_SUPPORTED 4
+#define QEMU_MINOR_VERSIONS_PER_MAJOR 3
+
+int qemu_version_delta(unsigned version_major_a, unsigned version_minor_a,
+ unsigned version_major_b, unsigned version_minor_b)
+{
+int delta;
+
+assert(version_major_a >= QEMU_FIRST_MAJOR_VERSION_SUPPORTED);
+assert(version_major_b >= QEMU_FIRST_MAJOR_VERSION_SUPPORTED);
+assert(version_minor_a < QEMU_MINOR_VERSIONS_PER_MAJOR);
+assert(version_minor_b < QEMU_MINOR_VERSIONS_PER_MAJOR);
+
+delta = version_major_b - version_major_a;
+delta *= QEMU_MINOR_VERSIONS_PER_MAJOR;
+delta += version_minor_b - version_minor_a;
+
+return delta;
+}
+
+int qemu_version_delta_current(unsigned version_major, unsigned version_minor)
+{
+return qemu_version_delta(QEMU_VERSION_MAJOR, QEMU_VERSION_MINOR,
+ version_major, version_minor);
+}
--
2.38.1
[RFC PATCH 2/4] hw/block: Rename TYPE_PFLASH_CFI02 'width' property as 'device-width'
Use the same property name than the TYPE_PFLASH_CFI01 model.
Deprecate the current 'width' property and add the 'device-width'
property pointing to the same field in PFlashCFI02.
Signed-off-by: Philippe Mathieu-Daudé
---
hw/block/pflash_cfi02.c | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/hw/block/pflash_cfi02.c b/hw/block/pflash_cfi02.c
index 2a99b286b0..bbf78ad1e4 100644
--- a/hw/block/pflash_cfi02.c
+++ b/hw/block/pflash_cfi02.c
@@ -950,6 +950,7 @@ static Property pflash_cfi02_properties[] = {
DEFINE_PROP_UINT32("num-blocks3", PFlashCFI02, nb_blocs[3], 0),
DEFINE_PROP_UINT32("sector-length3", PFlashCFI02, sector_len[3], 0),
DEFINE_PROP_UINT8("width", PFlashCFI02, width, 0),
+DEFINE_PROP_UINT8("device-width", PFlashCFI02, width, 0),
DEFINE_PROP_UINT8("mappings", PFlashCFI02, mappings, 0),
DEFINE_PROP_UINT8("big-endian", PFlashCFI02, be, 0),
DEFINE_PROP_UINT16("id0", PFlashCFI02, ident0, 0),
@@ -978,6 +979,11 @@ static void pflash_cfi02_class_init(ObjectClass *klass,
void *data)
dc->unrealize = pflash_cfi02_unrealize;
device_class_set_props(dc, pflash_cfi02_properties);
set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
+
+object_class_property_deprecate(klass, "width",
+"renamed as '"
+TYPE_PFLASH_CFI02 ".device-width'",
+8, 0);
}
static const TypeInfo pflash_cfi02_info = {
@@ -1014,7 +1020,7 @@ PFlashCFI02 *pflash_cfi02_register(hwaddr base,
assert(QEMU_IS_ALIGNED(size, sector_len));
qdev_prop_set_uint32(dev, "num-blocks", size / sector_len);
qdev_prop_set_uint32(dev, "sector-length", sector_len);
-qdev_prop_set_uint8(dev, "width", width);
+qdev_prop_set_uint8(dev, "device-width", width);
qdev_prop_set_uint8(dev, "mappings", nb_mappings);
qdev_prop_set_uint8(dev, "big-endian", !!be);
qdev_prop_set_uint16(dev, "id0", id0);
--
2.38.1
[RFC PATCH 0/4] qom: Introduce object_class_property_deprecate()
Hi, There will always be a need to deprecate things. Here I'm tackling the QOM (class) properties, since they can be set from some CLI options (-object -device -global ...). As an experiment, we add object_class_property_deprecate() to register a class property as deprecated (since some version), then we deprecate the TYPE_PFLASH_CFI02 'width' property, and finally as a bonus we emit a warning when the deprecation period is over, as a reminder. (For that we introduce few 'versions' helpers). Output example: $ qemu-system-arm -M musicpal,accel=qtest -S \ -drive if=pflash,driver=null-co,read-zeroes=on,size=8M \ -global driver=cfi.pflash02,property=width,value=2 qemu-system-arm: warning: Property 'cfi.pflash02.width' is deprecated (renamed as 'cfi.pflash02.device-width'). $ qemu-system-arm -M musicpal,accel=qtest -S \ -drive if=pflash,driver=null-co,read-zeroes=on,size=8M \ -global driver=cfi.pflash02,property=device-width,value=2 qemu-system-arm: warning: Property 'cfi.pflash02.width' has been deprecated in release v8.0 and can be removed. Thought? Regards, Phil. [earlier inspiration: https://lore.kernel.org/qemu-devel/Y7wlnqwU+/aue...@redhat.com/] Philippe Mathieu-Daudé (4): qom: Introduce object_class_property_deprecate() hw/block: Rename TYPE_PFLASH_CFI02 'width' property as 'device-width' util: Introduce helpers to compare QEMU versions qom: Warn when deprecated class property can be removed hw/block/pflash_cfi02.c | 8 +++- include/qemu/qemu-version.h | 36 include/qom/object.h| 17 + qom/object.c| 30 ++ util/meson.build| 1 + util/qemu-version.c | 37 + 6 files changed, 128 insertions(+), 1 deletion(-) create mode 100644 include/qemu/qemu-version.h create mode 100644 util/qemu-version.c -- 2.38.1
Re: [PATCH 0/1] hw/ide: share bmdma read and write functions
On Tue, Sep 6, 2022 at 10:27 AM Bernhard Beschow wrote: > > Am 19. Februar 2022 08:08:17 UTC schrieb Liav Albani : > >This is a preparation before I send v3 of ich6-ide controller emulation > >patch. > >I figured that it's more trivial to split the changes this way, by extracting > >the bmdma functions from via.c and piix.c and sharing them together. Then, > >I could easily put these into use when I send v3 of the ich6-ide patch by > >just > >using the already separated functions. This was suggested by BALATON Zoltan > >when > >he submitted a code review on my ich6-ide controller emulation patch. > > Ping. Any news? *cough*. Has this been folded into subsequent series, or does this still need attention? > > >Liav Albani (1): > > hw/ide: share bmdma read and write functions between piix.c and via.c > > > > hw/ide/pci.c | 47 > > hw/ide/piix.c| 50 ++- > > hw/ide/via.c | 51 ++-- > > include/hw/ide/pci.h | 4 > > 4 files changed, 55 insertions(+), 97 deletions(-) > > >
Re: [PATCH 1/2] hw/ide/core.c (cmd_read_native_max): Avoid limited device parameters
On Mon, Oct 10, 2022 at 4:52 AM Lev Kujawski wrote:
>
> Always use the native CHS device parameters for the ATA commands READ
> NATIVE MAX ADDRESS and READ NATIVE MAX ADDRESS EXT, not those limited
> by the ATA command INITIALIZE_DEVICE_PARAMETERS (introduced in patch
> 176e4961, hw/ide/core.c: Implement ATA INITIALIZE_DEVICE_PARAMETERS
> command, 2022-07-07.)
>
> As stated by the ATA/ATAPI specification, "[t]he native maximum is the
> highest address accepted by the device in the factory default
> condition." Therefore this patch substitutes the native values in
> drive_heads and drive_sectors before calling ide_set_sector().
>
> One consequence of the prior behavior was that setting zero sectors
> per track could lead to an FPE within ide_set_sector(). Thanks to
> Alexander Bulekov for reporting this issue.
>
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1243
> Signed-off-by: Lev Kujawski
Does this need attention?
--js
> ---
> hw/ide/core.c | 21 +
> 1 file changed, 17 insertions(+), 4 deletions(-)
>
> diff --git a/hw/ide/core.c b/hw/ide/core.c
> index 39afdc0006..ee836401bc 100644
> --- a/hw/ide/core.c
> +++ b/hw/ide/core.c
> @@ -1608,11 +1608,24 @@ static bool cmd_read_native_max(IDEState *s, uint8_t
> cmd)
> /* Refuse if no sectors are addressable (e.g. medium not inserted) */
> if (s->nb_sectors == 0) {
> ide_abort_command(s);
> -return true;
> -}
> +} else {
> +/*
> + * Save the active drive parameters, which may have been
> + * limited from their native counterparts by, e.g., INITIALIZE
> + * DEVICE PARAMETERS or SET MAX ADDRESS.
> + */
> +const int aheads = s->heads;
> +const int asectors = s->sectors;
>
> -ide_cmd_lba48_transform(s, lba48);
> -ide_set_sector(s, s->nb_sectors - 1);
> +s->heads = s->drive_heads;
> +s->sectors = s->drive_sectors;
> +
> +ide_cmd_lba48_transform(s, lba48);
> +ide_set_sector(s, s->nb_sectors - 1);
> +
> +s->heads = aheads;
> +s->sectors = asectors;
> +}
>
> return true;
> }
> --
> 2.34.1
>
Re: [PATCH 2/2] tests/qtest/ide-test: Verify READ NATIVE MAX ADDRESS is not limited
On Mon, Oct 10, 2022 at 4:52 AM Lev Kujawski wrote:
>
> Verify that the ATA command READ NATIVE MAX ADDRESS returns the last
> valid CHS tuple for the native device rather than any limit
> established by INITIALIZE DEVICE PARAMETERS.
>
> Signed-off-by: Lev Kujawski
Does this still need to be staged or merged?
--js
> ---
> tests/qtest/ide-test.c | 47 +-
> 1 file changed, 46 insertions(+), 1 deletion(-)
>
> diff --git a/tests/qtest/ide-test.c b/tests/qtest/ide-test.c
> index dbe1563b23..c406e6752a 100644
> --- a/tests/qtest/ide-test.c
> +++ b/tests/qtest/ide-test.c
> @@ -37,7 +37,8 @@
> /* TODO actually test the results and get rid of this */
> #define qmp_discard_response(q, ...) qobject_unref(qtest_qmp(q, __VA_ARGS__))
>
> -#define TEST_IMAGE_SIZE 64 * 1024 * 1024
> +/* Specified by ATA (physical) CHS geometry for ~64 MiB device. */
> +#define TEST_IMAGE_SIZE ((130 * 16 * 63) * 512)
>
> #define IDE_PCI_DEV 1
> #define IDE_PCI_FUNC1
> @@ -91,11 +92,13 @@ enum {
> enum {
> CMD_DSM = 0x06,
> CMD_DIAGNOSE= 0x90,
> +CMD_INIT_DP = 0x91, /* INITIALIZE DEVICE PARAMETERS */
> CMD_READ_DMA= 0xc8,
> CMD_WRITE_DMA = 0xca,
> CMD_FLUSH_CACHE = 0xe7,
> CMD_IDENTIFY= 0xec,
> CMD_PACKET = 0xa0,
> +CMD_READ_NATIVE = 0xf8, /* READ NATIVE MAX ADDRESS */
>
> CMDF_ABORT = 0x100,
> CMDF_NO_BM = 0x200,
> @@ -562,6 +565,46 @@ static void string_cpu_to_be16(uint16_t *s, size_t bytes)
> }
> }
>
> +static void test_specify(void)
> +{
> +QTestState *qts;
> +QPCIDevice *dev;
> +QPCIBar bmdma_bar, ide_bar;
> +uint16_t cyls;
> +uint8_t heads, spt;
> +
> +qts = ide_test_start(
> +"-blockdev driver=file,node-name=hda,filename=%s "
> +"-device ide-hd,drive=hda,bus=ide.0,unit=0 ",
> +tmp_path[0]);
> +
> +dev = get_pci_device(qts, _bar, _bar);
> +
> +/* Initialize drive with zero sectors per track and one head. */
> +qpci_io_writeb(dev, ide_bar, reg_nsectors, 0);
> +qpci_io_writeb(dev, ide_bar, reg_device, 0);
> +qpci_io_writeb(dev, ide_bar, reg_command, CMD_INIT_DP);
> +
> +/* READ NATIVE MAX ADDRESS (CHS mode). */
> +qpci_io_writeb(dev, ide_bar, reg_device, 0xa0);
> +qpci_io_writeb(dev, ide_bar, reg_command, CMD_READ_NATIVE);
> +
> +heads = qpci_io_readb(dev, ide_bar, reg_device) & 0xf;
> +++heads;
> +g_assert_cmpint(heads, ==, 16);
> +
> +cyls = qpci_io_readb(dev, ide_bar, reg_lba_high) << 8;
> +cyls |= qpci_io_readb(dev, ide_bar, reg_lba_middle);
> +++cyls;
> +g_assert_cmpint(cyls, ==, 130);
> +
> +spt = qpci_io_readb(dev, ide_bar, reg_lba_low);
> +g_assert_cmpint(spt, ==, 63);
> +
> +ide_test_quit(qts);
> +free_pci_device(dev);
> +}
> +
> static void test_identify(void)
> {
> QTestState *qts;
> @@ -1079,6 +1122,8 @@ int main(int argc, char **argv)
> /* Run the tests */
> g_test_init(, , NULL);
>
> +qtest_add_func("/ide/read_native", test_specify);
> +
> qtest_add_func("/ide/identify", test_identify);
>
> qtest_add_func("/ide/diagnostic", test_diagnostic);
> --
> 2.34.1
>
Re: [PATCH v5 10/14] vfio/migration: Implement VFIO migration protocol v2
On Mon, Jan 09, 2023 at 06:27:21PM +0100, Cédric Le Goater wrote:
> also, in vfio_migration_query_flags() :
>
> +static int vfio_migration_query_flags(VFIODevice *vbasedev, uint64_t
> *mig_flags)
> +{
> +uint64_t buf[DIV_ROUND_UP(sizeof(struct vfio_device_feature) +
> + sizeof(struct
> vfio_device_feature_migration),
> + sizeof(uint64_t))] = {};
> +struct vfio_device_feature *feature = (struct vfio_device_feature
> *)buf;
> +struct vfio_device_feature_migration *mig =
> +(struct vfio_device_feature_migration *)feature->data;
> +
> +feature->argsz = sizeof(buf);
> +feature->flags = VFIO_DEVICE_FEATURE_GET |
> VFIO_DEVICE_FEATURE_MIGRATION;
> +if (ioctl(vbasedev->fd, VFIO_DEVICE_FEATURE, feature)) {
> +return -EOPNOTSUPP;
> +}
> +
> +*mig_flags = mig->flags;
> +
> +return 0;
> +}
>
>
> The code is using any possible error returned by the VFIO_DEVICE_FEATURE
> ioctl to distinguish protocol v1 from v2.
I'm comfortable with that from a kernel perspective.
There is no such thing as this API failing in the kernel but userspace
should continue on, no matter what the error code. So always failing
here is correct.
About the only thing you might want to do is convert anything other
than ENOTTY into a hard qemu failure similar to failing to open
/dev/vfio or something - it means something has gone really
wrong.. But that is pretty obscure stuff
Jason
[PATCH v6 28/33] hw/isa/piix3: Merge hw/isa/piix4.c
Now that the PIIX3 and PIIX4 device models are sufficiently consolidated,
their implementations can be merged into one file for further
consolidation.
Signed-off-by: Bernhard Beschow
Reviewed-by: Michael S. Tsirkin
Message-Id: <20221022150508.26830-37-shen...@gmail.com>
---
hw/isa/{piix3.c => piix.c} | 158
hw/isa/piix4.c | 285 -
MAINTAINERS| 6 +-
hw/i386/Kconfig| 2 +-
hw/isa/Kconfig | 12 +-
hw/isa/meson.build | 3 +-
hw/mips/Kconfig| 2 +-
7 files changed, 165 insertions(+), 303 deletions(-)
rename hw/isa/{piix3.c => piix.c} (75%)
delete mode 100644 hw/isa/piix4.c
diff --git a/hw/isa/piix3.c b/hw/isa/piix.c
similarity index 75%
rename from hw/isa/piix3.c
rename to hw/isa/piix.c
index 4ce1653406..81cb4e64ab 100644
--- a/hw/isa/piix3.c
+++ b/hw/isa/piix.c
@@ -2,6 +2,7 @@
* QEMU PIIX PCI ISA Bridge Emulation
*
* Copyright (c) 2006 Fabrice Bellard
+ * Copyright (c) 2018 Hervé Poussineau
*
* Permission is hereby granted, free of charge, to any person obtaining a copy
* of this software and associated documentation files (the "Software"), to
deal
@@ -27,6 +28,7 @@
#include "qapi/error.h"
#include "hw/dma/i8257.h"
#include "hw/southbridge/piix.h"
+#include "hw/timer/i8254.h"
#include "hw/irq.h"
#include "hw/qdev-properties.h"
#include "hw/ide/piix.h"
@@ -81,6 +83,27 @@ static void piix3_set_irq(void *opaque, int pirq, int level)
piix3_set_irq_level(piix3, pirq, level);
}
+static void piix4_set_irq(void *opaque, int irq_num, int level)
+{
+int i, pic_irq, pic_level;
+PIIXState *s = opaque;
+PCIBus *bus = pci_get_bus(>dev);
+
+/* now we change the pic irq level according to the piix irq mappings */
+/* XXX: optimize */
+pic_irq = s->dev.config[PIIX_PIRQCA + irq_num];
+if (pic_irq < ISA_NUM_IRQS) {
+/* The pic level is the logical OR of all the PCI irqs mapped to it. */
+pic_level = 0;
+for (i = 0; i < PIIX_NUM_PIRQS; i++) {
+if (pic_irq == s->dev.config[PIIX_PIRQCA + i]) {
+pic_level |= pci_bus_get_irq_level(bus, i);
+}
+}
+qemu_set_irq(s->pic.in_irqs[pic_irq], pic_level);
+}
+}
+
static PCIINTxRoute piix3_route_intx_pin_to_irq(void *opaque, int pin)
{
PIIXState *piix3 = opaque;
@@ -208,6 +231,17 @@ static int piix3_post_load(void *opaque, int version_id)
return 0;
}
+static int piix4_post_load(void *opaque, int version_id)
+{
+PIIXState *s = opaque;
+
+if (version_id == 2) {
+s->rcr = 0;
+}
+
+return 0;
+}
+
static int piix3_pre_save(void *opaque)
{
int i;
@@ -257,6 +291,17 @@ static const VMStateDescription vmstate_piix3 = {
}
};
+static const VMStateDescription vmstate_piix4 = {
+.name = "PIIX4",
+.version_id = 3,
+.minimum_version_id = 2,
+.post_load = piix4_post_load,
+.fields = (VMStateField[]) {
+VMSTATE_PCI_DEVICE(dev, PIIXState),
+VMSTATE_UINT8_V(rcr, PIIXState, 3),
+VMSTATE_END_OF_LIST()
+}
+};
static void rcr_write(void *opaque, hwaddr addr, uint64_t val, unsigned len)
{
@@ -489,11 +534,124 @@ static const TypeInfo piix3_xen_info = {
.class_init= piix3_xen_class_init,
};
+static void piix4_realize(PCIDevice *dev, Error **errp)
+{
+PIIXState *s = PIIX_PCI_DEVICE(dev);
+PCIBus *pci_bus = pci_get_bus(dev);
+ISABus *isa_bus;
+
+isa_bus = isa_bus_new(DEVICE(dev), pci_address_space(dev),
+ pci_address_space_io(dev), errp);
+if (!isa_bus) {
+return;
+}
+
+memory_region_init_io(>rcr_mem, OBJECT(dev), _ops, s,
+ "reset-control", 1);
+memory_region_add_subregion_overlap(pci_address_space_io(dev),
+PIIX_RCR_IOPORT, >rcr_mem, 1);
+
+/* initialize i8259 pic */
+if (!qdev_realize(DEVICE(>pic), NULL, errp)) {
+return;
+}
+
+/* initialize ISA irqs */
+isa_bus_irqs(isa_bus, s->pic.in_irqs);
+
+/* initialize pit */
+i8254_pit_init(isa_bus, 0x40, 0, NULL);
+
+/* DMA */
+i8257_dma_init(isa_bus, 0);
+
+/* RTC */
+qdev_prop_set_int32(DEVICE(>rtc), "base_year", 2000);
+if (!qdev_realize(DEVICE(>rtc), BUS(isa_bus), errp)) {
+return;
+}
+s->rtc.irq = qdev_get_gpio_in(DEVICE(>pic), s->rtc.isairq);
+
+/* IDE */
+qdev_prop_set_int32(DEVICE(>ide), "addr", dev->devfn + 1);
+if (!qdev_realize(DEVICE(>ide), BUS(pci_bus), errp)) {
+return;
+}
+
+/* USB */
+if (s->has_usb) {
+object_initialize_child(OBJECT(dev), "uhci", >uhci,
+TYPE_PIIX4_USB_UHCI);
+qdev_prop_set_int32(DEVICE(>uhci), "addr", dev->devfn + 2);
+if (!qdev_realize(DEVICE(>uhci), BUS(pci_bus), errp)) {
+return;
+}
+}
+
+/* ACPI controller */
+if (s->has_acpi)
[PATCH v6 31/33] hw/isa/piix: Rename functions to be shared for interrupt triggering
PIIX4 will get the same optimizations which are already implemented for
PIIX3.
Signed-off-by: Bernhard Beschow
Reviewed-by: Michael S. Tsirkin
Message-Id: <20221022150508.26830-40-shen...@gmail.com>
---
hw/isa/piix.c | 56 +--
1 file changed, 28 insertions(+), 28 deletions(-)
diff --git a/hw/isa/piix.c b/hw/isa/piix.c
index 0132f6e70a..33ea5275ec 100644
--- a/hw/isa/piix.c
+++ b/hw/isa/piix.c
@@ -40,47 +40,47 @@
#define XEN_PIIX_NUM_PIRQS 128ULL
-static void piix3_set_irq_pic(PIIXState *piix3, int pic_irq)
+static void piix_set_irq_pic(PIIXState *piix, int pic_irq)
{
-qemu_set_irq(piix3->pic.in_irqs[pic_irq],
- !!(piix3->pic_levels &
+qemu_set_irq(piix->pic.in_irqs[pic_irq],
+ !!(piix->pic_levels &
(((1ULL << PIIX_NUM_PIRQS) - 1) <<
(pic_irq * PIIX_NUM_PIRQS;
}
-static void piix3_set_irq_level_internal(PIIXState *piix3, int pirq, int level)
+static void piix_set_irq_level_internal(PIIXState *piix, int pirq, int level)
{
int pic_irq;
uint64_t mask;
-pic_irq = piix3->dev.config[PIIX_PIRQCA + pirq];
+pic_irq = piix->dev.config[PIIX_PIRQCA + pirq];
if (pic_irq >= ISA_NUM_IRQS) {
return;
}
mask = 1ULL << ((pic_irq * PIIX_NUM_PIRQS) + pirq);
-piix3->pic_levels &= ~mask;
-piix3->pic_levels |= mask * !!level;
+piix->pic_levels &= ~mask;
+piix->pic_levels |= mask * !!level;
}
-static void piix3_set_irq_level(PIIXState *piix3, int pirq, int level)
+static void piix_set_irq_level(PIIXState *piix, int pirq, int level)
{
int pic_irq;
-pic_irq = piix3->dev.config[PIIX_PIRQCA + pirq];
+pic_irq = piix->dev.config[PIIX_PIRQCA + pirq];
if (pic_irq >= ISA_NUM_IRQS) {
return;
}
-piix3_set_irq_level_internal(piix3, pirq, level);
+piix_set_irq_level_internal(piix, pirq, level);
-piix3_set_irq_pic(piix3, pic_irq);
+piix_set_irq_pic(piix, pic_irq);
}
-static void piix3_set_irq(void *opaque, int pirq, int level)
+static void piix_set_irq(void *opaque, int pirq, int level)
{
-PIIXState *piix3 = opaque;
-piix3_set_irq_level(piix3, pirq, level);
+PIIXState *piix = opaque;
+piix_set_irq_level(piix, pirq, level);
}
static void piix4_set_irq(void *opaque, int irq_num, int level)
@@ -121,29 +121,29 @@ static PCIINTxRoute piix3_route_intx_pin_to_irq(void
*opaque, int pin)
}
/* irq routing is changed. so rebuild bitmap */
-static void piix3_update_irq_levels(PIIXState *piix3)
+static void piix_update_irq_levels(PIIXState *piix)
{
-PCIBus *bus = pci_get_bus(>dev);
+PCIBus *bus = pci_get_bus(>dev);
int pirq;
-piix3->pic_levels = 0;
+piix->pic_levels = 0;
for (pirq = 0; pirq < PIIX_NUM_PIRQS; pirq++) {
-piix3_set_irq_level(piix3, pirq, pci_bus_get_irq_level(bus, pirq));
+piix_set_irq_level(piix, pirq, pci_bus_get_irq_level(bus, pirq));
}
}
-static void piix3_write_config(PCIDevice *dev,
- uint32_t address, uint32_t val, int len)
+static void piix_write_config(PCIDevice *dev, uint32_t address, uint32_t val,
+ int len)
{
pci_default_write_config(dev, address, val, len);
if (ranges_overlap(address, len, PIIX_PIRQCA, 4)) {
-PIIXState *piix3 = PIIX_PCI_DEVICE(dev);
+PIIXState *piix = PIIX_PCI_DEVICE(dev);
int pic_irq;
-pci_bus_fire_intx_routing_notifier(pci_get_bus(>dev));
-piix3_update_irq_levels(piix3);
+pci_bus_fire_intx_routing_notifier(pci_get_bus(>dev));
+piix_update_irq_levels(piix);
for (pic_irq = 0; pic_irq < ISA_NUM_IRQS; pic_irq++) {
-piix3_set_irq_pic(piix3, pic_irq);
+piix_set_irq_pic(piix, pic_irq);
}
}
}
@@ -165,7 +165,7 @@ static void piix3_write_config_xen(PCIDevice *dev,
}
}
-piix3_write_config(dev, address, val, len);
+piix_write_config(dev, address, val, len);
}
static void piix_reset(DeviceState *dev)
@@ -225,7 +225,7 @@ static int piix3_post_load(void *opaque, int version_id)
*/
piix3->pic_levels = 0;
for (pirq = 0; pirq < PIIX_NUM_PIRQS; pirq++) {
-piix3_set_irq_level_internal(piix3, pirq,
+piix_set_irq_level_internal(piix3, pirq,
pci_bus_get_irq_level(pci_get_bus(>dev), pirq));
}
return 0;
@@ -482,7 +482,7 @@ static void piix3_realize(PCIDevice *dev, Error **errp)
return;
}
-pci_bus_irqs(pci_bus, piix3_set_irq, piix3, PIIX_NUM_PIRQS);
+pci_bus_irqs(pci_bus, piix_set_irq, piix3, PIIX_NUM_PIRQS);
pci_bus_set_route_irq_fn(pci_bus, piix3_route_intx_pin_to_irq);
}
@@ -490,7 +490,7 @@ static void piix3_class_init(ObjectClass *klass, void *data)
{
PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
-k->config_write = piix3_write_config;
+k->config_write = piix_write_config;
[PATCH v6 23/33] hw/isa/piix4: Make PIIX4's ACPI and USB functions optional
This aligns PIIX4 with PIIX3.
Signed-off-by: Bernhard Beschow
Reviewed-by: Michael S. Tsirkin
Message-Id: <20221022150508.26830-30-shen...@gmail.com>
---
hw/isa/piix4.c | 44
hw/mips/malta.c | 6 --
2 files changed, 36 insertions(+), 14 deletions(-)
diff --git a/hw/isa/piix4.c b/hw/isa/piix4.c
index de60ceef73..de4133f573 100644
--- a/hw/isa/piix4.c
+++ b/hw/isa/piix4.c
@@ -51,9 +51,16 @@ struct PIIX4State {
PCIIDEState ide;
UHCIState uhci;
PIIX4PMState pm;
+
+uint32_t smb_io_base;
+
/* Reset Control Register */
MemoryRegion rcr_mem;
uint8_t rcr;
+
+bool has_acpi;
+bool has_usb;
+bool smm_enabled;
};
OBJECT_DECLARE_SIMPLE_TYPE(PIIX4State, PIIX4_PCI_DEVICE)
@@ -234,17 +241,26 @@ static void piix4_realize(PCIDevice *dev, Error **errp)
}
/* USB */
-qdev_prop_set_int32(DEVICE(>uhci), "addr", dev->devfn + 2);
-if (!qdev_realize(DEVICE(>uhci), BUS(pci_bus), errp)) {
-return;
+if (s->has_usb) {
+object_initialize_child(OBJECT(dev), "uhci", >uhci,
+TYPE_PIIX4_USB_UHCI);
+qdev_prop_set_int32(DEVICE(>uhci), "addr", dev->devfn + 2);
+if (!qdev_realize(DEVICE(>uhci), BUS(pci_bus), errp)) {
+return;
+}
}
/* ACPI controller */
-qdev_prop_set_int32(DEVICE(>pm), "addr", dev->devfn + 3);
-if (!qdev_realize(DEVICE(>pm), BUS(pci_bus), errp)) {
-return;
+if (s->has_acpi) {
+object_initialize_child(OBJECT(s), "pm", >pm, TYPE_PIIX4_PM);
+qdev_prop_set_int32(DEVICE(>pm), "addr", dev->devfn + 3);
+qdev_prop_set_uint32(DEVICE(>pm), "smb_io_base", s->smb_io_base);
+qdev_prop_set_bit(DEVICE(>pm), "smm-enabled", s->smm_enabled);
+if (!qdev_realize(DEVICE(>pm), BUS(pci_bus), errp)) {
+return;
+}
+qdev_connect_gpio_out(DEVICE(>pm), 0, s->isa[9]);
}
-qdev_connect_gpio_out(DEVICE(>pm), 0, s->isa[9]);
pci_bus_irqs(pci_bus, piix4_set_irq, s, PIIX_NUM_PIRQS);
}
@@ -255,13 +271,16 @@ static void piix4_init(Object *obj)
object_initialize_child(obj, "rtc", >rtc, TYPE_MC146818_RTC);
object_initialize_child(obj, "ide", >ide, TYPE_PIIX4_IDE);
-object_initialize_child(obj, "uhci", >uhci, TYPE_PIIX4_USB_UHCI);
-
-object_initialize_child(obj, "pm", >pm, TYPE_PIIX4_PM);
-qdev_prop_set_uint32(DEVICE(>pm), "smb_io_base", 0x1100);
-qdev_prop_set_bit(DEVICE(>pm), "smm-enabled", 0);
}
+static Property piix4_props[] = {
+DEFINE_PROP_UINT32("smb_io_base", PIIX4State, smb_io_base, 0),
+DEFINE_PROP_BOOL("has-acpi", PIIX4State, has_acpi, true),
+DEFINE_PROP_BOOL("has-usb", PIIX4State, has_usb, true),
+DEFINE_PROP_BOOL("smm-enabled", PIIX4State, smm_enabled, false),
+DEFINE_PROP_END_OF_LIST(),
+};
+
static void piix4_class_init(ObjectClass *klass, void *data)
{
DeviceClass *dc = DEVICE_CLASS(klass);
@@ -280,6 +299,7 @@ static void piix4_class_init(ObjectClass *klass, void *data)
*/
dc->user_creatable = false;
dc->hotpluggable = false;
+device_class_set_props(dc, piix4_props);
}
static const TypeInfo piix4_info = {
diff --git a/hw/mips/malta.c b/hw/mips/malta.c
index 94d4c49bf5..4a2e0edd98 100644
--- a/hw/mips/malta.c
+++ b/hw/mips/malta.c
@@ -1449,8 +1449,10 @@ void mips_malta_init(MachineState *machine)
empty_slot_init("GT64120", 0, 0x2000);
/* Southbridge */
-piix4 = pci_create_simple_multifunction(pci_bus, PIIX4_PCI_DEVFN, true,
-TYPE_PIIX4_PCI_DEVICE);
+piix4 = pci_new_multifunction(PIIX4_PCI_DEVFN, true,
+ TYPE_PIIX4_PCI_DEVICE);
+qdev_prop_set_uint32(DEVICE(piix4), "smb_io_base", 0x1100);
+pci_realize_and_unref(piix4, pci_bus, _fatal);
isa_bus = ISA_BUS(qdev_get_child_bus(DEVICE(piix4), "isa.0"));
dev = DEVICE(object_resolve_path_component(OBJECT(piix4), "ide"));
--
2.39.0
Re: [PATCH v5 10/14] vfio/migration: Implement VFIO migration protocol v2
On 1/9/23 16:12, Avihai Horon wrote:
On 09/01/2023 12:20, Cédric Le Goater wrote:
External email: Use caution opening links or attachments
Hello Avihai,
On 12/29/22 12:03, Avihai Horon wrote:
+static int vfio_save_setup(QEMUFile *f, void *opaque)
+{
+ VFIODevice *vbasedev = opaque;
+ VFIOMigration *migration = vbasedev->migration;
+ uint64_t stop_copy_size;
+
+ qemu_put_be64(f, VFIO_MIG_FLAG_DEV_SETUP_STATE);
+
+ if (vfio_query_stop_copy_size(vbasedev, _copy_size)) {
+ stop_copy_size = VFIO_MIG_DEFAULT_DATA_BUFFER_SIZE;
+ }
+ migration->data_buffer_size = MIN(VFIO_MIG_DEFAULT_DATA_BUFFER_SIZE,
+ stop_copy_size);
+ migration->data_buffer = g_try_malloc0(migration->data_buffer_size);
+ if (!migration->data_buffer) {
+ error_report("%s: Failed to allocate migration data buffer",
+ vbasedev->name);
+ return -ENOMEM;
+ }
+
+ trace_vfio_save_setup(vbasedev->name, migration->data_buffer_size);
+
+ qemu_put_be64(f, VFIO_MIG_FLAG_END_OF_STATE);
+
+ return qemu_file_get_error(f);
+}
+
This fails to compile with :
gcc version 12.2.1 20221121 (Red Hat 12.2.1-4) (GCC) complains with :
../include/qemu/osdep.h:315:22: error: ‘stop_copy_size’ may be used
uninitialized [-Werror=maybe-uninitialized]
315 | _a < _b ? _a : _b; \
| ^
../hw/vfio/migration.c:262:14: note: ‘stop_copy_size’ was declared here
262 | uint64_t stop_copy_size;
| ^~
cc1: all warnings being treated as errors
May be rework the code slightly to avoid the breakage :
+++ qemu.git/hw/vfio/migration.c
@@ -259,13 +259,11 @@ static int vfio_save_setup(QEMUFile *f,
{
VFIODevice *vbasedev = opaque;
VFIOMigration *migration = vbasedev->migration;
- uint64_t stop_copy_size;
+ uint64_t stop_copy_size = VFIO_MIG_DEFAULT_DATA_BUFFER_SIZE;
qemu_put_be64(f, VFIO_MIG_FLAG_DEV_SETUP_STATE);
- if (vfio_query_stop_copy_size(vbasedev, _copy_size)) {
- stop_copy_size = VFIO_MIG_DEFAULT_DATA_BUFFER_SIZE;
- }
+ vfio_query_stop_copy_size(vbasedev, _copy_size);
migration->data_buffer_size = MIN(VFIO_MIG_DEFAULT_DATA_BUFFER_SIZE,
stop_copy_size);
migration->data_buffer = g_try_malloc0(migration->data_buffer_size);
and report the error in vfio_query_stop_copy_size()
Thanks, Cedric.
There is another similar case in vfio_save_pending().
I will fix both of them.
also, in vfio_migration_query_flags() :
+static int vfio_migration_query_flags(VFIODevice *vbasedev, uint64_t
*mig_flags)
+{
+uint64_t buf[DIV_ROUND_UP(sizeof(struct vfio_device_feature) +
+ sizeof(struct
vfio_device_feature_migration),
+ sizeof(uint64_t))] = {};
+struct vfio_device_feature *feature = (struct vfio_device_feature *)buf;
+struct vfio_device_feature_migration *mig =
+(struct vfio_device_feature_migration *)feature->data;
+
+feature->argsz = sizeof(buf);
+feature->flags = VFIO_DEVICE_FEATURE_GET | VFIO_DEVICE_FEATURE_MIGRATION;
+if (ioctl(vbasedev->fd, VFIO_DEVICE_FEATURE, feature)) {
+return -EOPNOTSUPP;
+}
+
+*mig_flags = mig->flags;
+
+return 0;
+}
The code is using any possible error returned by the VFIO_DEVICE_FEATURE
ioctl to distinguish protocol v1 from v2.
Couldn't we use ENOTTY ? I think a pre-v6.0 kernel would return this errno.
Some error report would be good to have.
Thanks,
C.
[PATCH v6 32/33] hw/isa/piix: Consolidate IRQ triggering
Speeds up PIIX4 which resolves an old TODO.
Signed-off-by: Bernhard Beschow
Reviewed-by: Michael S. Tsirkin
Message-Id: <20221022150508.26830-41-shen...@gmail.com>
---
hw/isa/piix.c | 26 +++---
1 file changed, 3 insertions(+), 23 deletions(-)
diff --git a/hw/isa/piix.c b/hw/isa/piix.c
index 33ea5275ec..f125a6175f 100644
--- a/hw/isa/piix.c
+++ b/hw/isa/piix.c
@@ -83,27 +83,6 @@ static void piix_set_irq(void *opaque, int pirq, int level)
piix_set_irq_level(piix, pirq, level);
}
-static void piix4_set_irq(void *opaque, int irq_num, int level)
-{
-int i, pic_irq, pic_level;
-PIIXState *s = opaque;
-PCIBus *bus = pci_get_bus(>dev);
-
-/* now we change the pic irq level according to the piix irq mappings */
-/* XXX: optimize */
-pic_irq = s->dev.config[PIIX_PIRQCA + irq_num];
-if (pic_irq < ISA_NUM_IRQS) {
-/* The pic level is the logical OR of all the PCI irqs mapped to it. */
-pic_level = 0;
-for (i = 0; i < PIIX_NUM_PIRQS; i++) {
-if (pic_irq == s->dev.config[PIIX_PIRQCA + i]) {
-pic_level |= pci_bus_get_irq_level(bus, i);
-}
-}
-qemu_set_irq(s->pic.in_irqs[pic_irq], pic_level);
-}
-}
-
static PCIINTxRoute piix3_route_intx_pin_to_irq(void *opaque, int pin)
{
PIIXState *piix3 = opaque;
@@ -239,7 +218,7 @@ static int piix4_post_load(void *opaque, int version_id)
s->rcr = 0;
}
-return 0;
+return piix3_post_load(opaque, version_id);
}
static int piix3_pre_save(void *opaque)
@@ -554,7 +533,7 @@ static void piix4_realize(PCIDevice *dev, Error **errp)
/* RTC */
s->rtc.irq = qdev_get_gpio_in(DEVICE(>pic), s->rtc.isairq);
-pci_bus_irqs(pci_bus, piix4_set_irq, s, PIIX_NUM_PIRQS);
+pci_bus_irqs(pci_bus, piix_set_irq, s, PIIX_NUM_PIRQS);
}
static void piix4_init(Object *obj)
@@ -571,6 +550,7 @@ static void piix4_class_init(ObjectClass *klass, void *data)
DeviceClass *dc = DEVICE_CLASS(klass);
PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
+k->config_write = piix_write_config;
k->realize = piix4_realize;
k->vendor_id = PCI_VENDOR_ID_INTEL;
k->device_id = PCI_DEVICE_ID_INTEL_82371AB_0;
--
2.39.0
[PATCH v6 15/33] hw/isa/piix3: Create power management controller in host device
The power management controller is an integral part of PIIX3 (function
3). So create it as part of the south bridge.
Note that the ACPI function is optional in QEMU. This is why it gets
object_initialize_child()'ed in realize rather than in instance_init.
Signed-off-by: Bernhard Beschow
Reviewed-by: Michael S. Tsirkin
Message-Id: <20221022150508.26830-14-shen...@gmail.com>
---
include/hw/southbridge/piix.h | 6 ++
hw/i386/pc_piix.c | 24 ++--
hw/isa/piix3.c| 14 ++
hw/isa/Kconfig| 1 +
4 files changed, 35 insertions(+), 10 deletions(-)
diff --git a/include/hw/southbridge/piix.h b/include/hw/southbridge/piix.h
index 762709f2fd..b1eaab1d95 100644
--- a/include/hw/southbridge/piix.h
+++ b/include/hw/southbridge/piix.h
@@ -14,6 +14,7 @@
#include "hw/pci/pci_device.h"
#include "qom/object.h"
+#include "hw/acpi/piix4.h"
#include "hw/rtc/mc146818rtc.h"
#include "hw/usb/hcd-uhci.h"
@@ -56,6 +57,9 @@ struct PIIXState {
RTCState rtc;
UHCIState uhci;
+PIIX4PMState pm;
+
+uint32_t smb_io_base;
/* Reset Control Register contents */
uint8_t rcr;
@@ -63,7 +67,9 @@ struct PIIXState {
/* IO memory region for Reset Control Register (PIIX_RCR_IOPORT) */
MemoryRegion rcr_mem;
+bool has_acpi;
bool has_usb;
+bool smm_enabled;
};
typedef struct PIIXState PIIX3State;
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 61d8152078..5ea8d4a585 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -46,12 +46,12 @@
#include "sysemu/kvm.h"
#include "hw/kvm/clock.h"
#include "hw/sysbus.h"
+#include "hw/i2c/i2c.h"
#include "hw/i2c/smbus_eeprom.h"
#include "hw/xen/xen-x86.h"
#include "hw/xen/xen.h"
#include "exec/memory.h"
#include "hw/acpi/acpi.h"
-#include "hw/acpi/piix4.h"
#include "qapi/error.h"
#include "qemu/error-report.h"
#include "sysemu/xen.h"
@@ -97,6 +97,7 @@ static void pc_init1(MachineState *machine,
MemoryRegion *system_io = get_system_io();
PCIBus *pci_bus;
ISABus *isa_bus;
+Object *piix4_pm;
int piix3_devfn = -1;
qemu_irq smi_irq;
GSIState *gsi_state;
@@ -237,15 +238,25 @@ static void pc_init1(MachineState *machine,
pci_dev = pci_new_multifunction(-1, true, type);
object_property_set_bool(OBJECT(pci_dev), "has-usb",
machine_usb(machine), _abort);
+object_property_set_bool(OBJECT(pci_dev), "has-acpi",
+ x86_machine_is_acpi_enabled(x86ms),
+ _abort);
+qdev_prop_set_uint32(DEVICE(pci_dev), "smb_io_base", 0xb100);
+object_property_set_bool(OBJECT(pci_dev), "smm-enabled",
+ x86_machine_is_smm_enabled(x86ms),
+ _abort);
pci_realize_and_unref(pci_dev, pci_bus, _fatal);
+
piix3 = PIIX3_PCI_DEVICE(pci_dev);
piix3->pic = x86ms->gsi;
piix3_devfn = piix3->dev.devfn;
isa_bus = ISA_BUS(qdev_get_child_bus(DEVICE(piix3), "isa.0"));
rtc_state = ISA_DEVICE(object_resolve_path_component(OBJECT(pci_dev),
"rtc"));
+piix4_pm = object_resolve_path_component(OBJECT(pci_dev), "pm");
} else {
pci_bus = NULL;
+piix4_pm = NULL;
isa_bus = isa_bus_new(NULL, get_system_memory(), system_io,
_abort);
@@ -315,15 +326,8 @@ static void pc_init1(MachineState *machine,
}
#endif
-if (pcmc->pci_enabled && x86_machine_is_acpi_enabled(X86_MACHINE(pcms))) {
-PCIDevice *piix4_pm;
-
+if (piix4_pm) {
smi_irq = qemu_allocate_irq(pc_acpi_smi_interrupt, first_cpu, 0);
-piix4_pm = pci_new(piix3_devfn + 3, TYPE_PIIX4_PM);
-qdev_prop_set_uint32(DEVICE(piix4_pm), "smb_io_base", 0xb100);
-qdev_prop_set_bit(DEVICE(piix4_pm), "smm-enabled",
- x86_machine_is_smm_enabled(x86ms));
-pci_realize_and_unref(piix4_pm, pci_bus, _fatal);
qdev_connect_gpio_out(DEVICE(piix4_pm), 0, x86ms->gsi[9]);
qdev_connect_gpio_out_named(DEVICE(piix4_pm), "smi-irq", 0, smi_irq);
@@ -337,7 +341,7 @@ static void pc_init1(MachineState *machine,
object_property_allow_set_link,
OBJ_PROP_LINK_STRONG);
object_property_set_link(OBJECT(machine), PC_MACHINE_ACPI_DEVICE_PROP,
- OBJECT(piix4_pm), _abort);
+ piix4_pm, _abort);
}
if (machine->nvdimms_state->is_enabled) {
diff --git a/hw/isa/piix3.c b/hw/isa/piix3.c
index 45c20dea17..ed7d58bc98 100644
--- a/hw/isa/piix3.c
+++ b/hw/isa/piix3.c
@@ -319,6 +319,17 @@ static void pci_piix3_realize(PCIDevice *dev, Error **errp)
return;
}
}
+
+/* Power Management */
+if (d->has_acpi) {
+
Re: [PATCH v5 00/31] Consolidate PIIX south bridges
Am 8. Januar 2023 18:28:28 UTC schrieb "Philippe Mathieu-Daudé" : >On 8/1/23 16:12, Bernhard Beschow wrote: >> Am 7. Januar 2023 23:57:32 UTC schrieb Mark Cave-Ayland >> : >>> On 05/01/2023 14:31, Bernhard Beschow wrote: > Bernhard Beschow (28): hw/mips/Kconfig: Track Malta's PIIX dependencies via Kconfig hw/usb/hcd-uhci: Introduce TYPE_ defines for device models hw/i386/pc_piix: Associate pci_map_irq_fn as soon as PCI bus is created hw/i386/pc_piix: Allow for setting properties before realizing PIIX3 south bridge hw/i386/pc: Create RTC controllers in south bridges hw/i386/pc: No need for rtc_state to be an out-parameter hw/isa/piix3: Create USB controller in host device hw/isa/piix3: Create power management controller in host device hw/intc/i8259: Make using the isa_pic singleton more type-safe hw/intc/i8259: Introduce i8259 proxy "isa-pic" hw/isa/piix3: Create ISA PIC in host device hw/isa/piix3: Create IDE controller in host device hw/isa/piix3: Wire up ACPI interrupt internally hw/isa/piix3: Resolve redundant PIIX_NUM_PIC_IRQS hw/isa/piix3: Rename pci_piix3_props for sharing with PIIX4 hw/isa/piix3: Rename piix3_reset() for sharing with PIIX4 hw/isa/piix3: Drop the "3" from PIIX base class hw/isa/piix4: Make PIIX4's ACPI and USB functions optional hw/isa/piix4: Remove unused inbound ISA interrupt lines hw/isa/piix4: Use ISA PIC device hw/isa/piix4: Reuse struct PIIXState from PIIX3 hw/isa/piix4: Rename reset control operations to match PIIX3 hw/isa/piix3: Merge hw/isa/piix4.c hw/isa/piix: Harmonize names of reset control memory regions hw/isa/piix: Reuse PIIX3 base class' realize method in PIIX4 hw/isa/piix: Rename functions to be shared for interrupt triggering hw/isa/piix: Consolidate IRQ triggering hw/isa/piix: Share PIIX3's base class with PIIX4 > >>> Phil - over to you! > >Thanks for the review Mark! > >> Shall I respin? I could integrate my PCI series into this one in order to >> avoid the outdated MIPS patches while still delivering a working series. >> Yes/No? > >If you don't mind, that is certainly easier for me :) v6 is out! I've also rebased onto latest master which resolves some merge conflicts (PCI, building) for you. I hope you don't mind some minor cleanups that I've made to the PCI INTx series which aligns board code with my latest fuloong2e cleanup series. Best regards, Bernhard >
[PATCH v6 29/33] hw/isa/piix: Harmonize names of reset control memory regions
There is no need for having different names here. Having the same name further allows code to be shared between PIIX3 and PIIX4. Signed-off-by: Bernhard Beschow Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Michael S. Tsirkin Message-Id: <20221022150508.26830-38-shen...@gmail.com> --- hw/isa/piix.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hw/isa/piix.c b/hw/isa/piix.c index 81cb4e64ab..d8cd80e859 100644 --- a/hw/isa/piix.c +++ b/hw/isa/piix.c @@ -351,7 +351,7 @@ static void pci_piix3_realize(PCIDevice *dev, Error **errp) isa_bus_irqs(isa_bus, d->pic.in_irqs); memory_region_init_io(>rcr_mem, OBJECT(dev), _ops, d, - "piix3-reset-control", 1); + "piix-reset-control", 1); memory_region_add_subregion_overlap(pci_address_space_io(dev), PIIX_RCR_IOPORT, >rcr_mem, 1); @@ -547,7 +547,7 @@ static void piix4_realize(PCIDevice *dev, Error **errp) } memory_region_init_io(>rcr_mem, OBJECT(dev), _ops, s, - "reset-control", 1); + "piix-reset-control", 1); memory_region_add_subregion_overlap(pci_address_space_io(dev), PIIX_RCR_IOPORT, >rcr_mem, 1); -- 2.39.0
[PATCH v6 24/33] hw/isa/piix4: Remove unused inbound ISA interrupt lines
The Malta board, which is the only user of PIIX4, doesn't connect to the
exported interrupt lines. PIIX3 doesn't expose such interrupt lines
either, so remove them for PIIX4 for simplicity and consistency.
Signed-off-by: Bernhard Beschow
Reviewed-by: Michael S. Tsirkin
Message-Id: <20221022150508.26830-32-shen...@gmail.com>
---
hw/isa/piix4.c | 8
1 file changed, 8 deletions(-)
diff --git a/hw/isa/piix4.c b/hw/isa/piix4.c
index de4133f573..9edaa5de3e 100644
--- a/hw/isa/piix4.c
+++ b/hw/isa/piix4.c
@@ -155,12 +155,6 @@ static void piix4_request_i8259_irq(void *opaque, int irq,
int level)
qemu_set_irq(s->cpu_intr, level);
}
-static void piix4_set_i8259_irq(void *opaque, int irq, int level)
-{
-PIIX4State *s = opaque;
-qemu_set_irq(s->isa[irq], level);
-}
-
static void piix4_rcr_write(void *opaque, hwaddr addr, uint64_t val,
unsigned int len)
{
@@ -204,8 +198,6 @@ static void piix4_realize(PCIDevice *dev, Error **errp)
return;
}
-qdev_init_gpio_in_named(DEVICE(dev), piix4_set_i8259_irq,
-"isa", ISA_NUM_IRQS);
qdev_init_gpio_out_named(DEVICE(dev), >cpu_intr,
"intr", 1);
--
2.39.0
[PATCH v6 08/33] hw/usb/hcd-uhci: Introduce TYPE_ defines for device models
Suggested-by: Mark Cave-Ayland
Signed-off-by: Bernhard Beschow
Reviewed-by: Philippe Mathieu-Daudé
Reviewed-by: Michael S. Tsirkin
Message-Id: <20221022150508.26830-10-shen...@gmail.com>
---
hw/usb/hcd-uhci.h | 4
hw/i386/pc_piix.c | 3 ++-
hw/i386/pc_q35.c | 13 +++--
hw/isa/piix4.c| 2 +-
hw/usb/hcd-uhci.c | 16
5 files changed, 22 insertions(+), 16 deletions(-)
diff --git a/hw/usb/hcd-uhci.h b/hw/usb/hcd-uhci.h
index 5843af504a..e0fdb98ef1 100644
--- a/hw/usb/hcd-uhci.h
+++ b/hw/usb/hcd-uhci.h
@@ -91,4 +91,8 @@ typedef struct UHCIInfo {
void uhci_data_class_init(ObjectClass *klass, void *data);
void usb_uhci_common_realize(PCIDevice *dev, Error **errp);
+#define TYPE_PIIX3_USB_UHCI "piix3-usb-uhci"
+#define TYPE_PIIX4_USB_UHCI "piix4-usb-uhci"
+#define TYPE_ICH9_USB_UHCI(fn) "ich9-usb-uhci" #fn
+
#endif
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index bb3b10557f..df64dd8dcc 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -51,6 +51,7 @@
#include "exec/memory.h"
#include "hw/acpi/acpi.h"
#include "hw/acpi/piix4.h"
+#include "hw/usb/hcd-uhci.h"
#include "qapi/error.h"
#include "qemu/error-report.h"
#include "sysemu/xen.h"
@@ -305,7 +306,7 @@ static void pc_init1(MachineState *machine,
#endif
if (pcmc->pci_enabled && machine_usb(machine)) {
-pci_create_simple(pci_bus, piix3_devfn + 2, "piix3-usb-uhci");
+pci_create_simple(pci_bus, piix3_devfn + 2, TYPE_PIIX3_USB_UHCI);
}
if (pcmc->pci_enabled && x86_machine_is_acpi_enabled(X86_MACHINE(pcms))) {
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 65ea226211..83c57c6eb1 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -48,6 +48,7 @@
#include "hw/ide/pci.h"
#include "hw/ide/ahci.h"
#include "hw/usb.h"
+#include "hw/usb/hcd-uhci.h"
#include "qapi/error.h"
#include "qemu/error-report.h"
#include "sysemu/numa.h"
@@ -65,15 +66,15 @@ struct ehci_companions {
};
static const struct ehci_companions ich9_1d[] = {
-{ .name = "ich9-usb-uhci1", .func = 0, .port = 0 },
-{ .name = "ich9-usb-uhci2", .func = 1, .port = 2 },
-{ .name = "ich9-usb-uhci3", .func = 2, .port = 4 },
+{ .name = TYPE_ICH9_USB_UHCI(1), .func = 0, .port = 0 },
+{ .name = TYPE_ICH9_USB_UHCI(2), .func = 1, .port = 2 },
+{ .name = TYPE_ICH9_USB_UHCI(3), .func = 2, .port = 4 },
};
static const struct ehci_companions ich9_1a[] = {
-{ .name = "ich9-usb-uhci4", .func = 0, .port = 0 },
-{ .name = "ich9-usb-uhci5", .func = 1, .port = 2 },
-{ .name = "ich9-usb-uhci6", .func = 2, .port = 4 },
+{ .name = TYPE_ICH9_USB_UHCI(4), .func = 0, .port = 0 },
+{ .name = TYPE_ICH9_USB_UHCI(5), .func = 1, .port = 2 },
+{ .name = TYPE_ICH9_USB_UHCI(6), .func = 2, .port = 4 },
};
static int ehci_create_ich9_with_companions(PCIBus *bus, int slot)
diff --git a/hw/isa/piix4.c b/hw/isa/piix4.c
index 6e9434129d..de60ceef73 100644
--- a/hw/isa/piix4.c
+++ b/hw/isa/piix4.c
@@ -255,7 +255,7 @@ static void piix4_init(Object *obj)
object_initialize_child(obj, "rtc", >rtc, TYPE_MC146818_RTC);
object_initialize_child(obj, "ide", >ide, TYPE_PIIX4_IDE);
-object_initialize_child(obj, "uhci", >uhci, "piix4-usb-uhci");
+object_initialize_child(obj, "uhci", >uhci, TYPE_PIIX4_USB_UHCI);
object_initialize_child(obj, "pm", >pm, TYPE_PIIX4_PM);
qdev_prop_set_uint32(DEVICE(>pm), "smb_io_base", 0x1100);
diff --git a/hw/usb/hcd-uhci.c b/hw/usb/hcd-uhci.c
index d1b5657d72..30ae0104bb 100644
--- a/hw/usb/hcd-uhci.c
+++ b/hw/usb/hcd-uhci.c
@@ -1292,56 +1292,56 @@ void uhci_data_class_init(ObjectClass *klass, void
*data)
static UHCIInfo uhci_info[] = {
{
-.name = "piix3-usb-uhci",
+.name = TYPE_PIIX3_USB_UHCI,
.vendor_id = PCI_VENDOR_ID_INTEL,
.device_id = PCI_DEVICE_ID_INTEL_82371SB_2,
.revision = 0x01,
.irq_pin = 3,
.unplug= true,
},{
-.name = "piix4-usb-uhci",
+.name = TYPE_PIIX4_USB_UHCI,
.vendor_id = PCI_VENDOR_ID_INTEL,
.device_id = PCI_DEVICE_ID_INTEL_82371AB_2,
.revision = 0x01,
.irq_pin = 3,
.unplug= true,
},{
-.name = "ich9-usb-uhci1", /* 00:1d.0 */
+.name = TYPE_ICH9_USB_UHCI(1), /* 00:1d.0 */
.vendor_id = PCI_VENDOR_ID_INTEL,
.device_id = PCI_DEVICE_ID_INTEL_82801I_UHCI1,
.revision = 0x03,
.irq_pin = 0,
.unplug= false,
},{
-.name = "ich9-usb-uhci2", /* 00:1d.1 */
+.name = TYPE_ICH9_USB_UHCI(2), /* 00:1d.1 */
.vendor_id = PCI_VENDOR_ID_INTEL,
.device_id = PCI_DEVICE_ID_INTEL_82801I_UHCI2,
.revision = 0x03,
.irq_pin = 1,
.unplug= false,
},{
-.name = "ich9-usb-uhci3", /* 00:1d.2 */
+.name = TYPE_ICH9_USB_UHCI(3), /* 00:1d.2 */
.vendor_id = PCI_VENDOR_ID_INTEL,
[PATCH v6 22/33] hw/isa/piix3: Drop the "3" from PIIX base class
This commit marks the finalization of the PIIX3 preparations
to be merged with PIIX4. In particular, PIIXState is prepared
to be reused in piix4.c.
Signed-off-by: Bernhard Beschow
Reviewed-by: Michael S. Tsirkin
Message-Id: <20221022150508.26830-25-shen...@gmail.com>
---
include/hw/southbridge/piix.h | 6 ++--
hw/isa/piix3.c| 60 +--
2 files changed, 32 insertions(+), 34 deletions(-)
diff --git a/include/hw/southbridge/piix.h b/include/hw/southbridge/piix.h
index f48cfd7936..907c3568b6 100644
--- a/include/hw/southbridge/piix.h
+++ b/include/hw/southbridge/piix.h
@@ -72,11 +72,9 @@ struct PIIXState {
bool has_usb;
bool smm_enabled;
};
-typedef struct PIIXState PIIX3State;
-#define TYPE_PIIX3_PCI_DEVICE "pci-piix3"
-DECLARE_INSTANCE_CHECKER(PIIX3State, PIIX3_PCI_DEVICE,
- TYPE_PIIX3_PCI_DEVICE)
+#define TYPE_PIIX_PCI_DEVICE "pci-piix"
+OBJECT_DECLARE_SIMPLE_TYPE(PIIXState, PIIX_PCI_DEVICE)
#define TYPE_PIIX3_DEVICE "PIIX3"
#define TYPE_PIIX3_XEN_DEVICE "PIIX3-xen"
diff --git a/hw/isa/piix3.c b/hw/isa/piix3.c
index d674130fc9..4ce1653406 100644
--- a/hw/isa/piix3.c
+++ b/hw/isa/piix3.c
@@ -38,7 +38,7 @@
#define XEN_PIIX_NUM_PIRQS 128ULL
-static void piix3_set_irq_pic(PIIX3State *piix3, int pic_irq)
+static void piix3_set_irq_pic(PIIXState *piix3, int pic_irq)
{
qemu_set_irq(piix3->pic.in_irqs[pic_irq],
!!(piix3->pic_levels &
@@ -46,7 +46,7 @@ static void piix3_set_irq_pic(PIIX3State *piix3, int pic_irq)
(pic_irq * PIIX_NUM_PIRQS;
}
-static void piix3_set_irq_level_internal(PIIX3State *piix3, int pirq, int
level)
+static void piix3_set_irq_level_internal(PIIXState *piix3, int pirq, int level)
{
int pic_irq;
uint64_t mask;
@@ -61,7 +61,7 @@ static void piix3_set_irq_level_internal(PIIX3State *piix3,
int pirq, int level)
piix3->pic_levels |= mask * !!level;
}
-static void piix3_set_irq_level(PIIX3State *piix3, int pirq, int level)
+static void piix3_set_irq_level(PIIXState *piix3, int pirq, int level)
{
int pic_irq;
@@ -77,13 +77,13 @@ static void piix3_set_irq_level(PIIX3State *piix3, int
pirq, int level)
static void piix3_set_irq(void *opaque, int pirq, int level)
{
-PIIX3State *piix3 = opaque;
+PIIXState *piix3 = opaque;
piix3_set_irq_level(piix3, pirq, level);
}
static PCIINTxRoute piix3_route_intx_pin_to_irq(void *opaque, int pin)
{
-PIIX3State *piix3 = opaque;
+PIIXState *piix3 = opaque;
int irq = piix3->dev.config[PIIX_PIRQCA + pin];
PCIINTxRoute route;
@@ -98,7 +98,7 @@ static PCIINTxRoute piix3_route_intx_pin_to_irq(void *opaque,
int pin)
}
/* irq routing is changed. so rebuild bitmap */
-static void piix3_update_irq_levels(PIIX3State *piix3)
+static void piix3_update_irq_levels(PIIXState *piix3)
{
PCIBus *bus = pci_get_bus(>dev);
int pirq;
@@ -114,7 +114,7 @@ static void piix3_write_config(PCIDevice *dev,
{
pci_default_write_config(dev, address, val, len);
if (ranges_overlap(address, len, PIIX_PIRQCA, 4)) {
-PIIX3State *piix3 = PIIX3_PCI_DEVICE(dev);
+PIIXState *piix3 = PIIX_PCI_DEVICE(dev);
int pic_irq;
pci_bus_fire_intx_routing_notifier(pci_get_bus(>dev));
@@ -147,7 +147,7 @@ static void piix3_write_config_xen(PCIDevice *dev,
static void piix_reset(DeviceState *dev)
{
-PIIX3State *d = PIIX3_PCI_DEVICE(dev);
+PIIXState *d = PIIX_PCI_DEVICE(dev);
uint8_t *pci_conf = d->dev.config;
pci_conf[0x04] = 0x07; /* master, memory and I/O */
@@ -188,7 +188,7 @@ static void piix_reset(DeviceState *dev)
static int piix3_post_load(void *opaque, int version_id)
{
-PIIX3State *piix3 = opaque;
+PIIXState *piix3 = opaque;
int pirq;
/*
@@ -211,7 +211,7 @@ static int piix3_post_load(void *opaque, int version_id)
static int piix3_pre_save(void *opaque)
{
int i;
-PIIX3State *piix3 = opaque;
+PIIXState *piix3 = opaque;
for (i = 0; i < ARRAY_SIZE(piix3->pci_irq_levels_vmstate); i++) {
piix3->pci_irq_levels_vmstate[i] =
@@ -223,7 +223,7 @@ static int piix3_pre_save(void *opaque)
static bool piix3_rcr_needed(void *opaque)
{
-PIIX3State *piix3 = opaque;
+PIIXState *piix3 = opaque;
return (piix3->rcr != 0);
}
@@ -234,7 +234,7 @@ static const VMStateDescription vmstate_piix3_rcr = {
.minimum_version_id = 1,
.needed = piix3_rcr_needed,
.fields = (VMStateField[]) {
-VMSTATE_UINT8(rcr, PIIX3State),
+VMSTATE_UINT8(rcr, PIIXState),
VMSTATE_END_OF_LIST()
}
};
@@ -246,8 +246,8 @@ static const VMStateDescription vmstate_piix3 = {
.post_load = piix3_post_load,
.pre_save = piix3_pre_save,
.fields = (VMStateField[]) {
-VMSTATE_PCI_DEVICE(dev, PIIX3State),
-VMSTATE_INT32_ARRAY_V(pci_irq_levels_vmstate, PIIX3State,
+VMSTATE_PCI_DEVICE(dev, PIIXState),
+
[PATCH v6 04/33] hw/pci/pci: Factor out pci_bus_map_irqs() from pci_bus_irqs()
pci_bus_irqs() coupled together the assignment of pci_set_irq_fn and
pci_map_irq_fn to a PCI bus. This coupling gets in the way when the
pci_map_irq_fn is board-specific while the pci_set_irq_fn is device-
specific.
For example, both of QEMU's PIIX south bridge models have different
pci_map_irq_fn implementations which are board-specific rather than
device-specific. These implementations should therefore reside in board
code. The pci_set_irq_fn's, however, should stay in the device models
because they access memory internal to the model.
Factoring out pci_bus_map_irqs() from pci_bus_irqs() allows the
assignments to be decoupled, resolving the problem described above.
Note also how pci_vpb_realize() which gets touched in this commit
assigns different pci_map_irq_fn's depending on the board.
Signed-off-by: Bernhard Beschow
Reviewed-by: Michael S. Tsirkin
Reviewed-by: Philippe Mathieu-Daudé
---
include/hw/pci/pci.h| 3 ++-
hw/i386/pc_q35.c| 4 ++--
hw/isa/piix3.c | 8
hw/isa/piix4.c | 3 ++-
hw/pci-host/raven.c | 3 ++-
hw/pci-host/versatile.c | 3 ++-
hw/pci/pci.c| 12 +---
hw/remote/machine.c | 3 ++-
8 files changed, 25 insertions(+), 14 deletions(-)
diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
index 7048a373d1..85ee458cd2 100644
--- a/include/hw/pci/pci.h
+++ b/include/hw/pci/pci.h
@@ -282,8 +282,9 @@ PCIBus *pci_root_bus_new(DeviceState *parent, const char
*name,
MemoryRegion *address_space_io,
uint8_t devfn_min, const char *typename);
void pci_root_bus_cleanup(PCIBus *bus);
-void pci_bus_irqs(PCIBus *bus, pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
+void pci_bus_irqs(PCIBus *bus, pci_set_irq_fn set_irq,
void *irq_opaque, int nirq);
+void pci_bus_map_irqs(PCIBus *bus, pci_map_irq_fn map_irq);
void pci_bus_irqs_cleanup(PCIBus *bus);
int pci_bus_get_irq_level(PCIBus *bus, int irq_num);
/* 0 <= pin <= 3 0 = INTA, 1 = INTB, 2 = INTC, 3 = INTD */
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 67ceb04bcc..65ea226211 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -268,8 +268,8 @@ static void pc_q35_init(MachineState *machine)
for (i = 0; i < GSI_NUM_PINS; i++) {
qdev_connect_gpio_out_named(lpc_dev, ICH9_GPIO_GSI, i, x86ms->gsi[i]);
}
-pci_bus_irqs(host_bus, ich9_lpc_set_irq, ich9_lpc_map_irq, ich9_lpc,
- ICH9_LPC_NB_PIRQS);
+pci_bus_irqs(host_bus, ich9_lpc_set_irq, ich9_lpc, ICH9_LPC_NB_PIRQS);
+pci_bus_map_irqs(host_bus, ich9_lpc_map_irq);
pci_bus_set_route_irq_fn(host_bus, ich9_route_intx_pin_to_irq);
isa_bus = ich9_lpc->isa_bus;
diff --git a/hw/isa/piix3.c b/hw/isa/piix3.c
index eabad7ba58..666e794f77 100644
--- a/hw/isa/piix3.c
+++ b/hw/isa/piix3.c
@@ -384,8 +384,8 @@ static void piix3_realize(PCIDevice *dev, Error **errp)
return;
}
-pci_bus_irqs(pci_bus, piix3_set_irq, pci_slot_get_pirq,
- piix3, PIIX_NUM_PIRQS);
+pci_bus_irqs(pci_bus, piix3_set_irq, piix3, PIIX_NUM_PIRQS);
+pci_bus_map_irqs(pci_bus, pci_slot_get_pirq);
pci_bus_set_route_irq_fn(pci_bus, piix3_route_intx_pin_to_irq);
}
@@ -420,8 +420,8 @@ static void piix3_xen_realize(PCIDevice *dev, Error **errp)
* connected to the IOAPIC directly.
* These additional routes can be discovered through ACPI.
*/
-pci_bus_irqs(pci_bus, xen_piix3_set_irq, xen_pci_slot_get_pirq,
- piix3, XEN_PIIX_NUM_PIRQS);
+pci_bus_irqs(pci_bus, xen_piix3_set_irq, piix3, XEN_PIIX_NUM_PIRQS);
+pci_bus_map_irqs(pci_bus, xen_pci_slot_get_pirq);
}
static void piix3_xen_class_init(ObjectClass *klass, void *data)
diff --git a/hw/isa/piix4.c b/hw/isa/piix4.c
index 0d23e11a39..9c79c9677b 100644
--- a/hw/isa/piix4.c
+++ b/hw/isa/piix4.c
@@ -271,7 +271,8 @@ static void piix4_realize(PCIDevice *dev, Error **errp)
}
qdev_connect_gpio_out(DEVICE(>pm), 0, s->isa[9]);
-pci_bus_irqs(pci_bus, piix4_set_irq, pci_slot_get_pirq, s, PIIX_NUM_PIRQS);
+pci_bus_irqs(pci_bus, piix4_set_irq, s, PIIX_NUM_PIRQS);
+pci_bus_map_irqs(pci_bus, pci_slot_get_pirq);
}
static void piix4_init(Object *obj)
diff --git a/hw/pci-host/raven.c b/hw/pci-host/raven.c
index 2c96ddf8fe..5b00b4e462 100644
--- a/hw/pci-host/raven.c
+++ b/hw/pci-host/raven.c
@@ -258,7 +258,8 @@ static void raven_pcihost_realizefn(DeviceState *d, Error
**errp)
qdev_init_gpio_in(d, raven_change_gpio, 1);
-pci_bus_irqs(>pci_bus, raven_set_irq, raven_map_irq, s, PCI_NUM_PINS);
+pci_bus_irqs(>pci_bus, raven_set_irq, s, PCI_NUM_PINS);
+pci_bus_map_irqs(>pci_bus, raven_map_irq);
memory_region_init_io(>conf_mem, OBJECT(h), _host_conf_le_ops, s,
"pci-conf-idx", 4);
diff --git a/hw/pci-host/versatile.c b/hw/pci-host/versatile.c
index 0d50ea4cc0..60d4e7cd92 100644
--- a/hw/pci-host/versatile.c
+++ b/hw/pci-host/versatile.c
[PATCH v6 07/33] hw/mips/Kconfig: Track Malta's PIIX dependencies via Kconfig
Tracking dependencies via Kconfig seems much cleaner. Note that PIIX4 already depends on ACPI_PIIX4. Signed-off-by: Bernhard Beschow Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Michael S. Tsirkin --- configs/devices/mips-softmmu/common.mak | 2 -- hw/mips/Kconfig | 1 + 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/configs/devices/mips-softmmu/common.mak b/configs/devices/mips-softmmu/common.mak index 88aff94625..8ed6b62ae7 100644 --- a/configs/devices/mips-softmmu/common.mak +++ b/configs/devices/mips-softmmu/common.mak @@ -17,9 +17,7 @@ CONFIG_I8254=y CONFIG_PCSPK=y CONFIG_PCKBD=y CONFIG_FDC=y -CONFIG_ACPI_PIIX4=y CONFIG_I8257=y -CONFIG_PIIX4=y CONFIG_IDE_ISA=y CONFIG_PFLASH_CFI01=y CONFIG_I8259=y diff --git a/hw/mips/Kconfig b/hw/mips/Kconfig index 725525358d..4e7042f03d 100644 --- a/hw/mips/Kconfig +++ b/hw/mips/Kconfig @@ -1,6 +1,7 @@ config MALTA bool select ISA_SUPERIO +select PIIX4 config MIPSSIM bool -- 2.39.0
[PATCH v6 30/33] hw/isa/piix: Reuse PIIX3 base class' realize method in PIIX4
Resolves duplicate code.
Signed-off-by: Bernhard Beschow
Reviewed-by: Michael S. Tsirkin
Message-Id: <20221022150508.26830-39-shen...@gmail.com>
---
hw/isa/piix.c | 65 +++
1 file changed, 9 insertions(+), 56 deletions(-)
diff --git a/hw/isa/piix.c b/hw/isa/piix.c
index d8cd80e859..0132f6e70a 100644
--- a/hw/isa/piix.c
+++ b/hw/isa/piix.c
@@ -331,7 +331,8 @@ static const MemoryRegionOps rcr_ops = {
},
};
-static void pci_piix3_realize(PCIDevice *dev, Error **errp)
+static void pci_piix_realize(PCIDevice *dev, const char *uhci_type,
+ Error **errp)
{
PIIXState *d = PIIX_PCI_DEVICE(dev);
PCIBus *pci_bus = pci_get_bus(dev);
@@ -371,8 +372,7 @@ static void pci_piix3_realize(PCIDevice *dev, Error **errp)
/* USB */
if (d->has_usb) {
-object_initialize_child(OBJECT(dev), "uhci", >uhci,
-TYPE_PIIX3_USB_UHCI);
+object_initialize_child(OBJECT(dev), "uhci", >uhci, uhci_type);
qdev_prop_set_int32(DEVICE(>uhci), "addr", dev->devfn + 2);
if (!qdev_realize(DEVICE(>uhci), BUS(pci_bus), errp)) {
return;
@@ -477,7 +477,7 @@ static void piix3_realize(PCIDevice *dev, Error **errp)
PIIXState *piix3 = PIIX_PCI_DEVICE(dev);
PCIBus *pci_bus = pci_get_bus(dev);
-pci_piix3_realize(dev, errp);
+pci_piix_realize(dev, TYPE_PIIX3_USB_UHCI, errp);
if (*errp) {
return;
}
@@ -506,7 +506,7 @@ static void piix3_xen_realize(PCIDevice *dev, Error **errp)
PIIXState *piix3 = PIIX_PCI_DEVICE(dev);
PCIBus *pci_bus = pci_get_bus(dev);
-pci_piix3_realize(dev, errp);
+pci_piix_realize(dev, TYPE_PIIX3_USB_UHCI, errp);
if (*errp) {
return;
}
@@ -536,71 +536,24 @@ static const TypeInfo piix3_xen_info = {
static void piix4_realize(PCIDevice *dev, Error **errp)
{
+ERRP_GUARD();
PIIXState *s = PIIX_PCI_DEVICE(dev);
PCIBus *pci_bus = pci_get_bus(dev);
ISABus *isa_bus;
-isa_bus = isa_bus_new(DEVICE(dev), pci_address_space(dev),
- pci_address_space_io(dev), errp);
-if (!isa_bus) {
-return;
-}
-
-memory_region_init_io(>rcr_mem, OBJECT(dev), _ops, s,
- "piix-reset-control", 1);
-memory_region_add_subregion_overlap(pci_address_space_io(dev),
-PIIX_RCR_IOPORT, >rcr_mem, 1);
-
-/* initialize i8259 pic */
-if (!qdev_realize(DEVICE(>pic), NULL, errp)) {
+pci_piix_realize(dev, TYPE_PIIX4_USB_UHCI, errp);
+if (*errp) {
return;
}
-/* initialize ISA irqs */
-isa_bus_irqs(isa_bus, s->pic.in_irqs);
+isa_bus = ISA_BUS(qdev_get_child_bus(DEVICE(dev), "isa.0"));
/* initialize pit */
i8254_pit_init(isa_bus, 0x40, 0, NULL);
-/* DMA */
-i8257_dma_init(isa_bus, 0);
-
/* RTC */
-qdev_prop_set_int32(DEVICE(>rtc), "base_year", 2000);
-if (!qdev_realize(DEVICE(>rtc), BUS(isa_bus), errp)) {
-return;
-}
s->rtc.irq = qdev_get_gpio_in(DEVICE(>pic), s->rtc.isairq);
-/* IDE */
-qdev_prop_set_int32(DEVICE(>ide), "addr", dev->devfn + 1);
-if (!qdev_realize(DEVICE(>ide), BUS(pci_bus), errp)) {
-return;
-}
-
-/* USB */
-if (s->has_usb) {
-object_initialize_child(OBJECT(dev), "uhci", >uhci,
-TYPE_PIIX4_USB_UHCI);
-qdev_prop_set_int32(DEVICE(>uhci), "addr", dev->devfn + 2);
-if (!qdev_realize(DEVICE(>uhci), BUS(pci_bus), errp)) {
-return;
-}
-}
-
-/* ACPI controller */
-if (s->has_acpi) {
-object_initialize_child(OBJECT(s), "pm", >pm, TYPE_PIIX4_PM);
-qdev_prop_set_int32(DEVICE(>pm), "addr", dev->devfn + 3);
-qdev_prop_set_uint32(DEVICE(>pm), "smb_io_base", s->smb_io_base);
-qdev_prop_set_bit(DEVICE(>pm), "smm-enabled", s->smm_enabled);
-if (!qdev_realize(DEVICE(>pm), BUS(pci_bus), errp)) {
-return;
-}
-qdev_connect_gpio_out(DEVICE(>pm), 0,
- qdev_get_gpio_in(DEVICE(>pic), 9));
-}
-
pci_bus_irqs(pci_bus, piix4_set_irq, s, PIIX_NUM_PIRQS);
}
--
2.39.0
[PATCH v6 03/33] hw/isa/piix4: Correct IRQRC[A:D] reset values
From: Philippe Mathieu-Daudé IRQRC[A:D] registers reset value is 0x80. We were forcing the MIPS Malta machine routing to be able to boot a Linux kernel without any bootloader. We now have these registers initialized in the Malta machine write_bootloader(), so we can use the correct reset values. Signed-off-by: Philippe Mathieu-Daudé Message-Id: <20221027204720.33611-4-phi...@linaro.org> --- hw/isa/piix4.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/hw/isa/piix4.c b/hw/isa/piix4.c index 8fc1db6dc9..0d23e11a39 100644 --- a/hw/isa/piix4.c +++ b/hw/isa/piix4.c @@ -116,10 +116,10 @@ static void piix4_isa_reset(DeviceState *dev) pci_conf[0x4c] = 0x4d; pci_conf[0x4e] = 0x03; pci_conf[0x4f] = 0x00; -pci_conf[0x60] = 0x0a; // PCI A -> IRQ 10 -pci_conf[0x61] = 0x0a; // PCI B -> IRQ 10 -pci_conf[0x62] = 0x0b; // PCI C -> IRQ 11 -pci_conf[0x63] = 0x0b; // PCI D -> IRQ 11 +pci_conf[0x60] = 0x80; +pci_conf[0x61] = 0x80; +pci_conf[0x62] = 0x80; +pci_conf[0x63] = 0x80; pci_conf[0x69] = 0x02; pci_conf[0x70] = 0x80; pci_conf[0x76] = 0x0c; -- 2.39.0
[PATCH v6 06/33] hw/isa/piix4: Decouple INTx-to-LNKx routing which is board-specific
pci_map_irq_fn's in general seem to be board-specific, and PIIX4's
pci_slot_get_pirq() in particular seems very Malta-specific. So move the
latter to malta.c to 1/ keep the board logic in one place and 2/ avoid
PIIX4 to make assumptions about its board.
Signed-off-by: Bernhard Beschow
Reviewed-by: Michael S. Tsirkin
Reviewed-by: Philippe Mathieu-Daudé
---
hw/isa/piix4.c | 26 --
hw/mips/malta.c | 27 +++
2 files changed, 27 insertions(+), 26 deletions(-)
diff --git a/hw/isa/piix4.c b/hw/isa/piix4.c
index 9c79c9677b..6e9434129d 100644
--- a/hw/isa/piix4.c
+++ b/hw/isa/piix4.c
@@ -79,31 +79,6 @@ static void piix4_set_irq(void *opaque, int irq_num, int
level)
}
}
-static int pci_slot_get_pirq(PCIDevice *pci_dev, int irq_num)
-{
-int slot;
-
-slot = PCI_SLOT(pci_dev->devfn);
-
-switch (slot) {
-/* PIIX4 USB */
-case 10:
-return 3;
-/* AMD 79C973 Ethernet */
-case 11:
-return 1;
-/* Crystal 4281 Sound */
-case 12:
-return 2;
-/* PCI slot 1 to 4 */
-case 18 ... 21:
-return ((slot - 18) + irq_num) & 0x03;
-/* Unknown device, don't do any translation */
-default:
-return irq_num;
-}
-}
-
static void piix4_isa_reset(DeviceState *dev)
{
PIIX4State *d = PIIX4_PCI_DEVICE(dev);
@@ -272,7 +247,6 @@ static void piix4_realize(PCIDevice *dev, Error **errp)
qdev_connect_gpio_out(DEVICE(>pm), 0, s->isa[9]);
pci_bus_irqs(pci_bus, piix4_set_irq, s, PIIX_NUM_PIRQS);
-pci_bus_map_irqs(pci_bus, pci_slot_get_pirq);
}
static void piix4_init(Object *obj)
diff --git a/hw/mips/malta.c b/hw/mips/malta.c
index c3dcd43f37..94d4c49bf5 100644
--- a/hw/mips/malta.c
+++ b/hw/mips/malta.c
@@ -39,6 +39,7 @@
#include "hw/mips/bootloader.h"
#include "hw/mips/cpudevs.h"
#include "hw/pci/pci.h"
+#include "hw/pci/pci_bus.h"
#include "qemu/log.h"
#include "hw/mips/bios.h"
#include "hw/ide/pci.h"
@@ -1161,6 +1162,31 @@ static void malta_mips_config(MIPSCPU *cpu)
}
}
+static int malta_pci_slot_get_pirq(PCIDevice *pci_dev, int irq_num)
+{
+int slot;
+
+slot = PCI_SLOT(pci_dev->devfn);
+
+switch (slot) {
+/* PIIX4 USB */
+case 10:
+return 3;
+/* AMD 79C973 Ethernet */
+case 11:
+return 1;
+/* Crystal 4281 Sound */
+case 12:
+return 2;
+/* PCI slot 1 to 4 */
+case 18 ... 21:
+return ((slot - 18) + irq_num) & 0x03;
+/* Unknown device, don't do any translation */
+default:
+return irq_num;
+}
+}
+
static void main_cpu_reset(void *opaque)
{
MIPSCPU *cpu = opaque;
@@ -1414,6 +1440,7 @@ void mips_malta_init(MachineState *machine)
/* Northbridge */
dev = sysbus_create_simple("gt64120", -1, NULL);
pci_bus = PCI_BUS(qdev_get_child_bus(dev, "pci"));
+pci_bus_map_irqs(pci_bus, malta_pci_slot_get_pirq);
/*
* The whole address space decoded by the GT-64120A doesn't generate
* exception when accessing invalid memory. Create an empty slot to
--
2.39.0
[PATCH v6 09/33] hw/intc/i8259: Make using the isa_pic singleton more type-safe
This even spares some casts in hot code paths along the way.
Signed-off-by: Bernhard Beschow
Reviewed-by: Michael S. Tsirkin
Reviewed-by: Mark Cave-Ayland
---
Note: The next patch will introduce a class "isa-pic", which is
shall not be confused with the isa_pic singleton.
---
include/hw/intc/i8259.h | 6 +++---
include/qemu/typedefs.h | 1 +
hw/intc/i8259.c | 11 ---
3 files changed, 8 insertions(+), 10 deletions(-)
diff --git a/include/hw/intc/i8259.h b/include/hw/intc/i8259.h
index e2b1e8c59a..a0e34dd990 100644
--- a/include/hw/intc/i8259.h
+++ b/include/hw/intc/i8259.h
@@ -3,10 +3,10 @@
/* i8259.c */
-extern DeviceState *isa_pic;
+extern PICCommonState *isa_pic;
qemu_irq *i8259_init(ISABus *bus, qemu_irq parent_irq);
qemu_irq *kvm_i8259_init(ISABus *bus);
-int pic_get_output(DeviceState *d);
-int pic_read_irq(DeviceState *d);
+int pic_get_output(PICCommonState *s);
+int pic_read_irq(PICCommonState *s);
#endif
diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h
index 073abab998..fba04875c2 100644
--- a/include/qemu/typedefs.h
+++ b/include/qemu/typedefs.h
@@ -99,6 +99,7 @@ typedef struct PCIExpressDevice PCIExpressDevice;
typedef struct PCIExpressHost PCIExpressHost;
typedef struct PCIHostDeviceAddress PCIHostDeviceAddress;
typedef struct PCIHostState PCIHostState;
+typedef struct PICCommonState PICCommonState;
typedef struct PostcopyDiscardState PostcopyDiscardState;
typedef struct Property Property;
typedef struct PropertyInfo PropertyInfo;
diff --git a/hw/intc/i8259.c b/hw/intc/i8259.c
index cc4e21ffec..0261f087b2 100644
--- a/hw/intc/i8259.c
+++ b/hw/intc/i8259.c
@@ -55,7 +55,7 @@ struct PICClass {
#ifdef DEBUG_IRQ_LATENCY
static int64_t irq_time[16];
#endif
-DeviceState *isa_pic;
+PICCommonState *isa_pic;
static PICCommonState *slave_pic;
/* return the highest priority found in mask (highest = smallest
@@ -173,9 +173,8 @@ static void pic_intack(PICCommonState *s, int irq)
pic_update_irq(s);
}
-int pic_read_irq(DeviceState *d)
+int pic_read_irq(PICCommonState *s)
{
-PICCommonState *s = PIC_COMMON(d);
int irq, intno;
irq = pic_get_irq(s);
@@ -354,10 +353,8 @@ static uint64_t pic_ioport_read(void *opaque, hwaddr addr,
return ret;
}
-int pic_get_output(DeviceState *d)
+int pic_get_output(PICCommonState *s)
{
-PICCommonState *s = PIC_COMMON(d);
-
return (pic_get_irq(s) >= 0);
}
@@ -426,7 +423,7 @@ qemu_irq *i8259_init(ISABus *bus, qemu_irq parent_irq)
irq_set[i] = qdev_get_gpio_in(dev, i);
}
-isa_pic = dev;
+isa_pic = PIC_COMMON(dev);
isadev = i8259_init_chip(TYPE_I8259, bus, false);
dev = DEVICE(isadev);
--
2.39.0
[PATCH v6 18/33] hw/isa/piix3: Wire up ACPI interrupt internally
Now that PIIX3 has the PIC integrated, the ACPI controller can be wired
up internally.
Signed-off-by: Bernhard Beschow
Reviewed-by: Michael S. Tsirkin
Message-Id: <20221022150508.26830-18-shen...@gmail.com>
---
hw/i386/pc_piix.c | 1 -
hw/isa/piix3.c| 2 ++
2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 4675981021..92d9dd1ae2 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -329,7 +329,6 @@ static void pc_init1(MachineState *machine,
if (piix4_pm) {
smi_irq = qemu_allocate_irq(pc_acpi_smi_interrupt, first_cpu, 0);
-qdev_connect_gpio_out(DEVICE(piix4_pm), 0, x86ms->gsi[9]);
qdev_connect_gpio_out_named(DEVICE(piix4_pm), "smi-irq", 0, smi_irq);
pcms->smbus = I2C_BUS(qdev_get_child_bus(DEVICE(piix4_pm), "i2c"));
/* TODO: Populate SPD eeprom data. */
diff --git a/hw/isa/piix3.c b/hw/isa/piix3.c
index a549b1a8a5..6d2ffd449c 100644
--- a/hw/isa/piix3.c
+++ b/hw/isa/piix3.c
@@ -343,6 +343,8 @@ static void pci_piix3_realize(PCIDevice *dev, Error **errp)
if (!qdev_realize(DEVICE(>pm), BUS(pci_bus), errp)) {
return;
}
+qdev_connect_gpio_out(DEVICE(>pm), 0,
+ qdev_get_gpio_in(DEVICE(>pic), 9));
}
}
--
2.39.0
[PATCH v6 01/33] hw/mips/malta: Introduce PIIX4_PCI_DEVFN definition
From: Philippe Mathieu-Daudé
The PIIX4 PCI-ISA bridge function is always located at 10:0.
Since we want to re-use its address, add the PIIX4_PCI_DEVFN
definition.
Signed-off-by: Philippe Mathieu-Daudé
Message-Id: <20221027204720.33611-2-phi...@linaro.org>
---
hw/mips/malta.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/hw/mips/malta.c b/hw/mips/malta.c
index c0a2e0ab04..9bffa1b128 100644
--- a/hw/mips/malta.c
+++ b/hw/mips/malta.c
@@ -71,6 +71,8 @@
#define FLASH_SIZE 0x40
+#define PIIX4_PCI_DEVFN PCI_DEVFN(10, 0)
+
typedef struct {
MemoryRegion iomem;
MemoryRegion iomem_lo; /* 0 - 0x900 */
@@ -1401,7 +1403,7 @@ void mips_malta_init(MachineState *machine)
empty_slot_init("GT64120", 0, 0x2000);
/* Southbridge */
-piix4 = pci_create_simple_multifunction(pci_bus, PCI_DEVFN(10, 0), true,
+piix4 = pci_create_simple_multifunction(pci_bus, PIIX4_PCI_DEVFN, true,
TYPE_PIIX4_PCI_DEVICE);
isa_bus = ISA_BUS(qdev_get_child_bus(DEVICE(piix4), "isa.0"));
--
2.39.0
[PATCH v6 21/33] hw/isa/piix3: Rename piix3_reset() for sharing with PIIX4
Signed-off-by: Bernhard Beschow
Reviewed-by: Philippe Mathieu-Daudé
Reviewed-by: Michael S. Tsirkin
Message-Id: <20221022150508.26830-23-shen...@gmail.com>
---
hw/isa/piix3.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/isa/piix3.c b/hw/isa/piix3.c
index c6659bbfda..d674130fc9 100644
--- a/hw/isa/piix3.c
+++ b/hw/isa/piix3.c
@@ -145,7 +145,7 @@ static void piix3_write_config_xen(PCIDevice *dev,
piix3_write_config(dev, address, val, len);
}
-static void piix3_reset(DeviceState *dev)
+static void piix_reset(DeviceState *dev)
{
PIIX3State *d = PIIX3_PCI_DEVICE(dev);
uint8_t *pci_conf = d->dev.config;
@@ -395,7 +395,7 @@ static void pci_piix3_class_init(ObjectClass *klass, void
*data)
PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
AcpiDevAmlIfClass *adevc = ACPI_DEV_AML_IF_CLASS(klass);
-dc->reset = piix3_reset;
+dc->reset = piix_reset;
dc->desc= "ISA bridge";
dc->vmsd= _piix3;
dc->hotpluggable = false;
--
2.39.0
[PATCH v6 02/33] hw/mips/malta: Set PIIX4 IRQ routes in embedded bootloader
From: Philippe Mathieu-Daudé
Linux kernel expects the northbridge & southbridge chipsets
configured by the BIOS firmware. We emulate that by writing
a tiny bootloader code in write_bootloader().
Upon introduction in commit 5c2b87e34d ("PIIX4 support"),
the PIIX4 configuration space included values specific to
the Malta board.
Set the Malta-specific IRQ routing values in the embedded
bootloader, so the next commit can remove the Malta specific
bits from the PIIX4 PCI-ISA bridge and make it generic
(matching the real hardware).
Signed-off-by: Philippe Mathieu-Daudé
Message-Id: <20221027204720.33611-3-phi...@linaro.org>
---
hw/mips/malta.c | 19 +++
1 file changed, 19 insertions(+)
diff --git a/hw/mips/malta.c b/hw/mips/malta.c
index 9bffa1b128..c3dcd43f37 100644
--- a/hw/mips/malta.c
+++ b/hw/mips/malta.c
@@ -803,6 +803,8 @@ static void write_bootloader_nanomips(uint8_t *base,
uint64_t run_addr,
stw_p(p++, 0x8422); stw_p(p++, 0x9088);
/* sw t0, 0x88(t1) */
+/* TODO set PIIX IRQC[A:D] routing values! */
+
stw_p(p++, 0xe320 | NM_HI1(kernel_entry));
stw_p(p++, NM_HI2(kernel_entry));
@@ -840,6 +842,9 @@ static void write_bootloader_nanomips(uint8_t *base,
uint64_t run_addr,
static void write_bootloader(uint8_t *base, uint64_t run_addr,
uint64_t kernel_entry)
{
+const char pci_pins_cfg[PCI_NUM_PINS] = {
+10, 10, 11, 11 /* PIIX IRQRC[A:D] */
+};
uint32_t *p;
/* Small bootloader */
@@ -914,6 +919,20 @@ static void write_bootloader(uint8_t *base, uint64_t
run_addr,
#undef cpu_to_gt32
+/*
+ * The PIIX ISA bridge is on PCI bus 0 dev 10 func 0.
+ * Load the PIIX IRQC[A:D] routing config address, then
+ * write routing configuration to the config data register.
+ */
+bl_gen_write_u32(, /* GT_PCI0_CFGADDR */
+ cpu_mips_phys_to_kseg1(NULL, 0x1be0 + 0xcf8),
+ tswap32((1 << 31) /* ConfigEn */
+ | PCI_BUILD_BDF(0, PIIX4_PCI_DEVFN) << 8
+ | PIIX_PIRQCA));
+bl_gen_write_u32(, /* GT_PCI0_CFGDATA */
+ cpu_mips_phys_to_kseg1(NULL, 0x1be0 + 0xcfc),
+ tswap32(ldl_be_p(pci_pins_cfg)));
+
bl_gen_jump_kernel(,
true, ENVP_VADDR - 64,
/*
--
2.39.0
[PATCH v6 19/33] hw/isa/piix3: Resolve redundant PIIX_NUM_PIC_IRQS
PIIX_NUM_PIC_IRQS is assumed to be the same as ISA_NUM_IRQS, otherwise
inconsistencies can occur.
Signed-off-by: Bernhard Beschow
Reviewed-by: Michael S. Tsirkin
Message-Id: <20221022150508.26830-21-shen...@gmail.com>
---
include/hw/southbridge/piix.h | 5 ++---
hw/isa/piix3.c| 8
2 files changed, 6 insertions(+), 7 deletions(-)
diff --git a/include/hw/southbridge/piix.h b/include/hw/southbridge/piix.h
index e3c35ca16f..f48cfd7936 100644
--- a/include/hw/southbridge/piix.h
+++ b/include/hw/southbridge/piix.h
@@ -32,7 +32,6 @@
*/
#define PIIX_RCR_IOPORT 0xcf9
-#define PIIX_NUM_PIC_IRQS 16 /* i8259 * 2 */
#define PIIX_NUM_PIRQS 4ULL/* PIRQ[A-D] */
struct PIIXState {
@@ -44,10 +43,10 @@ struct PIIXState {
* So one PIC level is tracked by PIIX_NUM_PIRQS bits.
*
* PIRQ is mapped to PIC pins, we track it by
- * PIIX_NUM_PIRQS * PIIX_NUM_PIC_IRQS = 64 bits with
+ * PIIX_NUM_PIRQS * ISA_NUM_IRQS = 64 bits with
* pic_irq * PIIX_NUM_PIRQS + pirq
*/
-#if PIIX_NUM_PIC_IRQS * PIIX_NUM_PIRQS > 64
+#if ISA_NUM_IRQS * PIIX_NUM_PIRQS > 64
#error "unable to encode pic state in 64bit in pic_levels."
#endif
uint64_t pic_levels;
diff --git a/hw/isa/piix3.c b/hw/isa/piix3.c
index 6d2ffd449c..e813e20639 100644
--- a/hw/isa/piix3.c
+++ b/hw/isa/piix3.c
@@ -52,7 +52,7 @@ static void piix3_set_irq_level_internal(PIIX3State *piix3,
int pirq, int level)
uint64_t mask;
pic_irq = piix3->dev.config[PIIX_PIRQCA + pirq];
-if (pic_irq >= PIIX_NUM_PIC_IRQS) {
+if (pic_irq >= ISA_NUM_IRQS) {
return;
}
@@ -66,7 +66,7 @@ static void piix3_set_irq_level(PIIX3State *piix3, int pirq,
int level)
int pic_irq;
pic_irq = piix3->dev.config[PIIX_PIRQCA + pirq];
-if (pic_irq >= PIIX_NUM_PIC_IRQS) {
+if (pic_irq >= ISA_NUM_IRQS) {
return;
}
@@ -87,7 +87,7 @@ static PCIINTxRoute piix3_route_intx_pin_to_irq(void *opaque,
int pin)
int irq = piix3->dev.config[PIIX_PIRQCA + pin];
PCIINTxRoute route;
-if (irq < PIIX_NUM_PIC_IRQS) {
+if (irq < ISA_NUM_IRQS) {
route.mode = PCI_INTX_ENABLED;
route.irq = irq;
} else {
@@ -119,7 +119,7 @@ static void piix3_write_config(PCIDevice *dev,
pci_bus_fire_intx_routing_notifier(pci_get_bus(>dev));
piix3_update_irq_levels(piix3);
-for (pic_irq = 0; pic_irq < PIIX_NUM_PIC_IRQS; pic_irq++) {
+for (pic_irq = 0; pic_irq < ISA_NUM_IRQS; pic_irq++) {
piix3_set_irq_pic(piix3, pic_irq);
}
}
--
2.39.0
[PATCH v6 27/33] hw/isa/piix4: Rename reset control operations to match PIIX3
Both implementations are the same and will be shared upon merging.
Signed-off-by: Bernhard Beschow
Reviewed-by: Philippe Mathieu-Daudé
Reviewed-by: Michael S. Tsirkin
Message-Id: <20221022150508.26830-35-shen...@gmail.com>
---
hw/isa/piix4.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/hw/isa/piix4.c b/hw/isa/piix4.c
index ce88377630..de7cde192f 100644
--- a/hw/isa/piix4.c
+++ b/hw/isa/piix4.c
@@ -127,7 +127,7 @@ static const VMStateDescription vmstate_piix4 = {
}
};
-static void piix4_rcr_write(void *opaque, hwaddr addr, uint64_t val,
+static void rcr_write(void *opaque, hwaddr addr, uint64_t val,
unsigned int len)
{
PIIXState *s = opaque;
@@ -140,16 +140,16 @@ static void piix4_rcr_write(void *opaque, hwaddr addr,
uint64_t val,
s->rcr = val & 2; /* keep System Reset type only */
}
-static uint64_t piix4_rcr_read(void *opaque, hwaddr addr, unsigned int len)
+static uint64_t rcr_read(void *opaque, hwaddr addr, unsigned int len)
{
PIIXState *s = opaque;
return s->rcr;
}
-static const MemoryRegionOps piix4_rcr_ops = {
-.read = piix4_rcr_read,
-.write = piix4_rcr_write,
+static const MemoryRegionOps rcr_ops = {
+.read = rcr_read,
+.write = rcr_write,
.endianness = DEVICE_LITTLE_ENDIAN,
.impl = {
.min_access_size = 1,
@@ -169,7 +169,7 @@ static void piix4_realize(PCIDevice *dev, Error **errp)
return;
}
-memory_region_init_io(>rcr_mem, OBJECT(dev), _rcr_ops, s,
+memory_region_init_io(>rcr_mem, OBJECT(dev), _ops, s,
"reset-control", 1);
memory_region_add_subregion_overlap(pci_address_space_io(dev),
PIIX_RCR_IOPORT, >rcr_mem, 1);
--
2.39.0
[PATCH v6 05/33] hw/isa/piix3: Decouple INTx-to-LNKx routing which is board-specific
pci_map_irq_fn's in general seem to be board-specific. So move PIIX3's
pci_slot_get_pirq() to board code to not have PIIX3 make assuptions
about its board.
Signed-off-by: Bernhard Beschow
Reviewed-by: Michael S. Tsirkin
Reviewed-by: Philippe Mathieu-Daudé
---
hw/i386/pc_piix.c | 15 +++
hw/isa/piix3.c| 13 -
2 files changed, 15 insertions(+), 13 deletions(-)
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index b48047f50c..bb3b10557f 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -47,6 +47,7 @@
#include "hw/sysbus.h"
#include "hw/i2c/smbus_eeprom.h"
#include "hw/xen/xen-x86.h"
+#include "hw/xen/xen.h"
#include "exec/memory.h"
#include "hw/acpi/acpi.h"
#include "hw/acpi/piix4.h"
@@ -73,6 +74,17 @@ static const int ide_iobase2[MAX_IDE_BUS] = { 0x3f6, 0x376 };
static const int ide_irq[MAX_IDE_BUS] = { 14, 15 };
#endif
+/*
+ * Return the global irq number corresponding to a given device irq
+ * pin. We could also use the bus number to have a more precise mapping.
+ */
+static int pc_pci_slot_get_pirq(PCIDevice *pci_dev, int pci_intx)
+{
+int slot_addend;
+slot_addend = PCI_SLOT(pci_dev->devfn) - 1;
+return (pci_intx + slot_addend) & 3;
+}
+
/* PC hardware initialisation */
static void pc_init1(MachineState *machine,
const char *host_type, const char *pci_type)
@@ -216,6 +228,9 @@ static void pc_init1(MachineState *machine,
x86ms->below_4g_mem_size,
x86ms->above_4g_mem_size,
pci_memory, ram_memory);
+pci_bus_map_irqs(pci_bus,
+ xen_enabled() ? xen_pci_slot_get_pirq
+ : pc_pci_slot_get_pirq);
pcms->bus = pci_bus;
pci_dev = pci_create_simple_multifunction(pci_bus, -1, true, type);
diff --git a/hw/isa/piix3.c b/hw/isa/piix3.c
index 666e794f77..283b971ec4 100644
--- a/hw/isa/piix3.c
+++ b/hw/isa/piix3.c
@@ -79,17 +79,6 @@ static void piix3_set_irq(void *opaque, int pirq, int level)
piix3_set_irq_level(piix3, pirq, level);
}
-/*
- * Return the global irq number corresponding to a given device irq
- * pin. We could also use the bus number to have a more precise mapping.
- */
-static int pci_slot_get_pirq(PCIDevice *pci_dev, int pci_intx)
-{
-int slot_addend;
-slot_addend = PCI_SLOT(pci_dev->devfn) - 1;
-return (pci_intx + slot_addend) & 3;
-}
-
static PCIINTxRoute piix3_route_intx_pin_to_irq(void *opaque, int pin)
{
PIIX3State *piix3 = opaque;
@@ -385,7 +374,6 @@ static void piix3_realize(PCIDevice *dev, Error **errp)
}
pci_bus_irqs(pci_bus, piix3_set_irq, piix3, PIIX_NUM_PIRQS);
-pci_bus_map_irqs(pci_bus, pci_slot_get_pirq);
pci_bus_set_route_irq_fn(pci_bus, piix3_route_intx_pin_to_irq);
}
@@ -421,7 +409,6 @@ static void piix3_xen_realize(PCIDevice *dev, Error **errp)
* These additional routes can be discovered through ACPI.
*/
pci_bus_irqs(pci_bus, xen_piix3_set_irq, piix3, XEN_PIIX_NUM_PIRQS);
-pci_bus_map_irqs(pci_bus, xen_pci_slot_get_pirq);
}
static void piix3_xen_class_init(ObjectClass *klass, void *data)
--
2.39.0
[PATCH v6 10/33] hw/intc/i8259: Introduce i8259 proxy TYPE_ISA_PIC
Having an i8259 proxy allows for ISA PICs to be created and wired up in
southbridges. This is especially interesting for PIIX3 for two reasons:
First, the southbridge doesn't need to care about the virtualization
technology used (KVM, TCG, Xen) due to in-IRQs (where devices get
attached) and out-IRQs (which will trigger the IRQs of the respective
virtualization technology) are separated. Second, since the in-IRQs are
populated with fully initialized qemu_irq's, they can already be wired
up inside PIIX3.
Cc: Mark Cave-Ayland
Signed-off-by: Bernhard Beschow
Reviewed-by: Michael S. Tsirkin
Reviewed-by: Mark Cave-Ayland
---
include/hw/intc/i8259.h | 18 ++
hw/intc/i8259.c | 27 +++
2 files changed, 45 insertions(+)
diff --git a/include/hw/intc/i8259.h b/include/hw/intc/i8259.h
index a0e34dd990..7fb403971c 100644
--- a/include/hw/intc/i8259.h
+++ b/include/hw/intc/i8259.h
@@ -1,6 +1,24 @@
#ifndef HW_I8259_H
#define HW_I8259_H
+#include "qom/object.h"
+#include "hw/isa/isa.h"
+#include "qemu/typedefs.h"
+
+#define TYPE_ISA_PIC "isa-pic"
+OBJECT_DECLARE_SIMPLE_TYPE(ISAPICState, ISA_PIC)
+
+/*
+ * TYPE_ISA_PIC is currently a PIC proxy which allows for interrupt wiring in
+ * a virtualization technology agnostic way.
+ */
+struct ISAPICState {
+DeviceState parent_obj;
+
+qemu_irq in_irqs[ISA_NUM_IRQS];
+qemu_irq out_irqs[ISA_NUM_IRQS];
+};
+
/* i8259.c */
extern PICCommonState *isa_pic;
diff --git a/hw/intc/i8259.c b/hw/intc/i8259.c
index 0261f087b2..e99d02136d 100644
--- a/hw/intc/i8259.c
+++ b/hw/intc/i8259.c
@@ -455,9 +455,36 @@ static const TypeInfo i8259_info = {
.class_size = sizeof(PICClass),
};
+static void isapic_set_irq(void *opaque, int irq, int level)
+{
+ISAPICState *s = opaque;
+
+qemu_set_irq(s->out_irqs[irq], level);
+}
+
+static void isapic_init(Object *obj)
+{
+ISAPICState *s = ISA_PIC(obj);
+
+qdev_init_gpio_in(DEVICE(s), isapic_set_irq, ISA_NUM_IRQS);
+qdev_init_gpio_out(DEVICE(s), s->out_irqs, ISA_NUM_IRQS);
+
+for (int i = 0; i < ISA_NUM_IRQS; ++i) {
+s->in_irqs[i] = qdev_get_gpio_in(DEVICE(s), i);
+}
+}
+
+static const TypeInfo isapic_info = {
+.name = TYPE_ISA_PIC,
+.parent= TYPE_DEVICE,
+.instance_size = sizeof(ISAPICState),
+.instance_init = isapic_init,
+};
+
static void pic_register_types(void)
{
type_register_static(_info);
+type_register_static(_info);
}
type_init(pic_register_types)
--
2.39.0
[PATCH v6 33/33] hw/isa/piix: Share PIIX3's base class with PIIX4
Having a common base class will allow for substituting PIIX3 with PIIX4
and vice versa. Moreover, it makes PIIX4 implement the
acpi-dev-aml-interface.
Signed-off-by: Bernhard Beschow
Reviewed-by: Michael S. Tsirkin
Message-Id: <20221022150508.26830-42-shen...@gmail.com>
---
hw/isa/piix.c | 49 ++---
1 file changed, 22 insertions(+), 27 deletions(-)
diff --git a/hw/isa/piix.c b/hw/isa/piix.c
index f125a6175f..54a1246a9d 100644
--- a/hw/isa/piix.c
+++ b/hw/isa/piix.c
@@ -396,13 +396,12 @@ static void build_pci_isa_aml(AcpiDevAmlIf *adev, Aml
*scope)
}
}
-static void pci_piix3_init(Object *obj)
+static void pci_piix_init(Object *obj)
{
PIIXState *d = PIIX_PCI_DEVICE(obj);
object_initialize_child(obj, "pic", >pic, TYPE_ISA_PIC);
object_initialize_child(obj, "rtc", >rtc, TYPE_MC146818_RTC);
-object_initialize_child(obj, "ide", >ide, TYPE_PIIX3_IDE);
}
static Property pci_piix_props[] = {
@@ -413,7 +412,7 @@ static Property pci_piix_props[] = {
DEFINE_PROP_END_OF_LIST(),
};
-static void pci_piix3_class_init(ObjectClass *klass, void *data)
+static void pci_piix_class_init(ObjectClass *klass, void *data)
{
DeviceClass *dc = DEVICE_CLASS(klass);
PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
@@ -421,11 +420,8 @@ static void pci_piix3_class_init(ObjectClass *klass, void
*data)
dc->reset = piix_reset;
dc->desc= "ISA bridge";
-dc->vmsd= _piix3;
dc->hotpluggable = false;
k->vendor_id= PCI_VENDOR_ID_INTEL;
-/* 82371SB PIIX3 PCI-to-ISA bridge (Step A1) */
-k->device_id= PCI_DEVICE_ID_INTEL_82371SB_0;
k->class_id = PCI_CLASS_BRIDGE_ISA;
/*
* Reason: part of PIIX3 southbridge, needs to be wired up by
@@ -440,9 +436,9 @@ static const TypeInfo piix_pci_type_info = {
.name = TYPE_PIIX_PCI_DEVICE,
.parent = TYPE_PCI_DEVICE,
.instance_size = sizeof(PIIXState),
-.instance_init = pci_piix3_init,
+.instance_init = pci_piix_init,
.abstract = true,
-.class_init = pci_piix3_class_init,
+.class_init = pci_piix_class_init,
.interfaces = (InterfaceInfo[]) {
{ INTERFACE_CONVENTIONAL_PCI_DEVICE },
{ TYPE_ACPI_DEV_AML_IF },
@@ -465,17 +461,29 @@ static void piix3_realize(PCIDevice *dev, Error **errp)
pci_bus_set_route_irq_fn(pci_bus, piix3_route_intx_pin_to_irq);
}
+static void piix3_init(Object *obj)
+{
+PIIXState *d = PIIX_PCI_DEVICE(obj);
+
+object_initialize_child(obj, "ide", >ide, TYPE_PIIX3_IDE);
+}
+
static void piix3_class_init(ObjectClass *klass, void *data)
{
+DeviceClass *dc = DEVICE_CLASS(klass);
PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
k->config_write = piix_write_config;
k->realize = piix3_realize;
+/* 82371SB PIIX3 PCI-to-ISA bridge (Step A1) */
+k->device_id = PCI_DEVICE_ID_INTEL_82371SB_0;
+dc->vmsd = _piix3;
}
static const TypeInfo piix3_info = {
.name = TYPE_PIIX3_DEVICE,
.parent= TYPE_PIIX_PCI_DEVICE,
+.instance_init = piix3_init,
.class_init= piix3_class_init,
};
@@ -501,15 +509,20 @@ static void piix3_xen_realize(PCIDevice *dev, Error
**errp)
static void piix3_xen_class_init(ObjectClass *klass, void *data)
{
+DeviceClass *dc = DEVICE_CLASS(klass);
PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
k->config_write = piix3_write_config_xen;
k->realize = piix3_xen_realize;
+/* 82371SB PIIX3 PCI-to-ISA bridge (Step A1) */
+k->device_id = PCI_DEVICE_ID_INTEL_82371SB_0;
+dc->vmsd = _piix3;
}
static const TypeInfo piix3_xen_info = {
.name = TYPE_PIIX3_XEN_DEVICE,
.parent= TYPE_PIIX_PCI_DEVICE,
+.instance_init = piix3_init,
.class_init= piix3_xen_class_init,
};
@@ -540,8 +553,6 @@ static void piix4_init(Object *obj)
{
PIIXState *s = PIIX_PCI_DEVICE(obj);
-object_initialize_child(obj, "pic", >pic, TYPE_ISA_PIC);
-object_initialize_child(obj, "rtc", >rtc, TYPE_MC146818_RTC);
object_initialize_child(obj, "ide", >ide, TYPE_PIIX4_IDE);
}
@@ -552,31 +563,15 @@ static void piix4_class_init(ObjectClass *klass, void
*data)
k->config_write = piix_write_config;
k->realize = piix4_realize;
-k->vendor_id = PCI_VENDOR_ID_INTEL;
k->device_id = PCI_DEVICE_ID_INTEL_82371AB_0;
-k->class_id = PCI_CLASS_BRIDGE_ISA;
-dc->reset = piix_reset;
-dc->desc = "ISA bridge";
dc->vmsd = _piix4;
-/*
- * Reason: part of PIIX4 southbridge, needs to be wired up,
- * e.g. by mips_malta_init()
- */
-dc->user_creatable = false;
-dc->hotpluggable = false;
-device_class_set_props(dc, pci_piix_props);
}
static const TypeInfo piix4_info = {
.name = TYPE_PIIX4_PCI_DEVICE,
-.parent= TYPE_PCI_DEVICE,
-.instance_size = sizeof(PIIXState),
+.parent= TYPE_PIIX_PCI_DEVICE,
.instance_init = piix4_init,
[PATCH v6 11/33] hw/i386/pc: Create RTC controllers in south bridges
Just like in the real hardware (and in PIIX4), create the RTC
controllers in the south bridges.
Signed-off-by: Bernhard Beschow
Reviewed-by: Michael S. Tsirkin
Reviewed-by: Thomas Huth
Message-Id: <20221022150508.26830-11-shen...@gmail.com>
---
include/hw/i386/ich9.h| 2 ++
include/hw/southbridge/piix.h | 4
hw/i386/pc.c | 12 +++-
hw/i386/pc_piix.c | 8
hw/i386/pc_q35.c | 1 +
hw/isa/lpc_ich9.c | 8
hw/isa/piix3.c| 15 +++
hw/isa/Kconfig| 2 ++
8 files changed, 51 insertions(+), 1 deletion(-)
diff --git a/include/hw/i386/ich9.h b/include/hw/i386/ich9.h
index 222781e8b9..dab309d5e3 100644
--- a/include/hw/i386/ich9.h
+++ b/include/hw/i386/ich9.h
@@ -7,6 +7,7 @@
#include "hw/isa/apm.h"
#include "hw/acpi/acpi.h"
#include "hw/acpi/ich9.h"
+#include "hw/rtc/mc146818rtc.h"
#include "qom/object.h"
void ich9_lpc_set_irq(void *opaque, int irq_num, int level);
@@ -35,6 +36,7 @@ struct ICH9LPCState {
*/
uint8_t irr[PCI_SLOT_MAX][PCI_NUM_PINS];
+RTCState rtc;
APMState apm;
ICH9LPCPMRegs pm;
uint32_t sci_level; /* track sci level */
diff --git a/include/hw/southbridge/piix.h b/include/hw/southbridge/piix.h
index 0bf48e936d..b06d26fa11 100644
--- a/include/hw/southbridge/piix.h
+++ b/include/hw/southbridge/piix.h
@@ -13,6 +13,8 @@
#define HW_SOUTHBRIDGE_PIIX_H
#include "hw/pci/pci_device.h"
+#include "qom/object.h"
+#include "hw/rtc/mc146818rtc.h"
/* PIRQRC[A:D]: PIRQx Route Control Registers */
#define PIIX_PIRQCA 0x60
@@ -51,6 +53,8 @@ struct PIIXState {
/* This member isn't used. Just for save/load compatibility */
int32_t pci_irq_levels_vmstate[PIIX_NUM_PIRQS];
+RTCState rtc;
+
/* Reset Control Register contents */
uint8_t rcr;
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index d489ecc0d1..448557333b 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1304,7 +1304,17 @@ void pc_basic_device_init(struct PCMachineState *pcms,
pit_alt_irq = qdev_get_gpio_in(hpet, HPET_LEGACY_PIT_INT);
rtc_irq = qdev_get_gpio_in(hpet, HPET_LEGACY_RTC_INT);
}
-*rtc_state = mc146818_rtc_init(isa_bus, 2000, rtc_irq);
+
+if (rtc_irq) {
+qdev_connect_gpio_out(DEVICE(*rtc_state), 0, rtc_irq);
+} else {
+uint32_t irq = object_property_get_uint(OBJECT(*rtc_state),
+"irq",
+_fatal);
+isa_connect_gpio_out(*rtc_state, 0, irq);
+}
+object_property_add_alias(OBJECT(pcms), "rtc-time", OBJECT(*rtc_state),
+ "date");
qemu_register_boot_set(pc_boot_set, *rtc_state);
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index df64dd8dcc..8f894714e5 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -32,6 +32,7 @@
#include "hw/i386/pc.h"
#include "hw/i386/apic.h"
#include "hw/pci-host/i440fx.h"
+#include "hw/rtc/mc146818rtc.h"
#include "hw/southbridge/piix.h"
#include "hw/display/ramfb.h"
#include "hw/firmware/smbios.h"
@@ -239,10 +240,17 @@ static void pc_init1(MachineState *machine,
piix3->pic = x86ms->gsi;
piix3_devfn = piix3->dev.devfn;
isa_bus = ISA_BUS(qdev_get_child_bus(DEVICE(piix3), "isa.0"));
+rtc_state = ISA_DEVICE(object_resolve_path_component(OBJECT(pci_dev),
+ "rtc"));
} else {
pci_bus = NULL;
isa_bus = isa_bus_new(NULL, get_system_memory(), system_io,
_abort);
+
+rtc_state = isa_new(TYPE_MC146818_RTC);
+qdev_prop_set_int32(DEVICE(rtc_state), "base_year", 2000);
+isa_realize_and_unref(rtc_state, isa_bus, _fatal);
+
i8257_dma_init(isa_bus, 0);
pcms->hpet_enabled = false;
}
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 83c57c6eb1..da97df69f7 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -239,6 +239,7 @@ static void pc_q35_init(MachineState *machine)
lpc = pci_create_simple_multifunction(host_bus, PCI_DEVFN(ICH9_LPC_DEV,
ICH9_LPC_FUNC), true,
TYPE_ICH9_LPC_DEVICE);
+rtc_state = ISA_DEVICE(object_resolve_path_component(OBJECT(lpc), "rtc"));
object_property_add_link(OBJECT(machine), PC_MACHINE_ACPI_DEVICE_PROP,
TYPE_HOTPLUG_HANDLER,
diff --git a/hw/isa/lpc_ich9.c b/hw/isa/lpc_ich9.c
index 8d541e2b54..498175c1cc 100644
--- a/hw/isa/lpc_ich9.c
+++ b/hw/isa/lpc_ich9.c
@@ -663,6 +663,8 @@ static void ich9_lpc_initfn(Object *obj)
static const uint8_t acpi_enable_cmd = ICH9_APM_ACPI_ENABLE;
static const uint8_t acpi_disable_cmd = ICH9_APM_ACPI_DISABLE;
+object_initialize_child(obj, "rtc", >rtc, TYPE_MC146818_RTC);
+
object_property_add_uint8_ptr(obj,
[PATCH v6 00/33] Consolidate PIIX south bridges
This series consolidates the implementations of the PIIX3 and PIIX4 south bridges and is an extended version of [1]. The motivation is to share as much code as possible and to bring both device models to feature parity such that perhaps PIIX4 can become a drop-in-replacement for PIIX3 in the pc machine. This could resolve the "Frankenstein" PIIX4-PM problem in PIIX3 discussed on this list before. The series is structured as follows: These patches are included for compatibility, please ignore: * hw/mips/malta: Introduce PIIX4_PCI_DEVFN definition * hw/mips/malta: Set PIIX4 IRQ routes in embedded bootloader * hw/isa/piix4: Correct IRQRC[A:D] reset values 'Decouple INTx-to-LNKx routing from south bridges', see [2]: * hw/pci/pci: Factor out pci_bus_map_irqs() from pci_bus_irqs() * hw/isa/piix3: Decouple INTx-to-LNKx routing which is board-specific * hw/isa/piix4: Decouple INTx-to-LNKx routing which is board-specific * hw/mips/Kconfig: Track Malta's PIIX dependencies via Kconfig * hw/usb/hcd-uhci: Introduce TYPE_ defines for device models Allow for making both PIIX south bridges agnostic about the virtualization technology used by allowing to shift the virtualization policies into board code: * hw/intc/i8259: Make using the isa_pic singleton more type-safe * hw/intc/i8259: Introduce i8259 proxy TYPE_ISA_PIC Move sub devices into the PIIX3 south bridge, like PIIX4 does already: * hw/i386/pc: Create RTC controllers in south bridges * hw/i386/pc: No need for rtc_state to be an out-parameter * hw/i386/pc_piix: Allow for setting properties before realizing PIIX3 south bridge * hw/isa/piix3: Create USB controller in host device * hw/isa/piix3: Create power management controller in host device * hw/isa/piix3: Create TYPE_ISA_PIC in host device * hw/isa/piix3: Create IDE controller in host device * hw/isa/piix3: Wire up ACPI interrupt internally Make PIIX3 and PIIX4 south bridges more similar: * hw/isa/piix3: Resolve redundant PIIX_NUM_PIC_IRQS * hw/isa/piix3: Rename pci_piix3_props for sharing with PIIX4 * hw/isa/piix3: Rename piix3_reset() for sharing with PIIX4 * hw/isa/piix3: Drop the "3" from PIIX base class * hw/isa/piix4: Make PIIX4's ACPI and USB functions optional * hw/isa/piix4: Remove unused inbound ISA interrupt lines * hw/isa/piix4: Use TYPE_ISA_PIC device * hw/isa/piix4: Reuse struct PIIXState from PIIX3 * hw/isa/piix4: Rename reset control operations to match PIIX3 This patch achieves the main goal of the series: * hw/isa/piix3: Merge hw/isa/piix4.c Perform some further consolidations which were easier to do after the merge: * hw/isa/piix: Harmonize names of reset control memory regions * hw/isa/piix: Reuse PIIX3 base class' realize method in PIIX4 * hw/isa/piix: Rename functions to be shared for interrupt triggering * hw/isa/piix: Consolidate IRQ triggering * hw/isa/piix: Share PIIX3's base class with PIIX4 One particular challenge in this series was that the PIC of PIIX3 used to be instantiated outside of the south bridge while some sub functions require a PIC with populated qemu_irqs. This has been solved by introducing a proxy PIC which furthermore allows PIIX3 to be agnostic towards the virtualization technology used (KVM, TCG, Xen). Due to consolidation PIIX4 gained the proxy PIC as well. Another challenge was dealing with optional devices where Peter already gave advice in [1] which this series implements. Last but not least there might be some opportunity to consolidate VM state handling, probably by reusing the one from PIIX3. Since I'm not very familiar with the requirements I didn't touch it so far. v6: - Fix some comments about TYPE_ISA_PIC (Mark) ... and use it consistently within the patch series. - Incorporate series "[PATCH v2 0/3] Decouple INTx-to-LNKx routing from south bridges" [2] for maintainer convenience. - Merge v5's 'hw/i386/pc_piix: Associate pci_map_irq_fn as soon as PCI bus is created' into https://lists.nongnu.org/archive/html/qemu-devel/2022-11/msg03312.html . Do similar for Malta. - Rebase onto latest master (d6271b657286 "Merge tag 'for_upstream' of https://git.kernel.org/pub/scm/virt/kvm/mst/qemu into staging") Testing done: * make check * Boot live CD: * `qemu-system-x86_64 -M pc -m 2G -accel kvm -cpu host -cdrom manjaro-kde-21.3.2-220704-linux515.iso` * `qemu-system-x86_64 -M q35 -m 2G -accel kvm -cpu host -cdrom manjaro-kde-21.3.2-220704-linux515.iso` * 'qemu-system-mips64el -M malta -kernel vmlinux-3.2.0-4-5kc-malta -hda debian_wheezy_mipsel_standard.qcow2 -append "root=/dev/sda1 console=ttyS0"` v5: - Pick up Reviewed-by tags from https://lists.nongnu.org/archive/html/qemu-devel/2023-01/msg00116.html - Add patch to make usage of the isa_pic global more type-safe - Re-introduce isa-pic as PIC specific proxy (Mark) v4: - Rebase onto "[PATCH v2 0/3] Decouple INTx-to-LNKx routing from south bridges" since it is already queued via mips-next. This eliminates patches 'hw/isa/piix3: Prefix pci_slot_get_pirq() with "piix3_"' and
[PATCH v6 26/33] hw/isa/piix4: Reuse struct PIIXState from PIIX3
Now that PIIX4 also uses TYPE_ISA_PIC, both implementations
can share the same struct.
Signed-off-by: Bernhard Beschow
Reviewed-by: Michael S. Tsirkin
Message-Id: <20221022150508.26830-34-shen...@gmail.com>
---
hw/isa/piix4.c | 51 +++---
1 file changed, 15 insertions(+), 36 deletions(-)
diff --git a/hw/isa/piix4.c b/hw/isa/piix4.c
index eae4db0182..ce88377630 100644
--- a/hw/isa/piix4.c
+++ b/hw/isa/piix4.c
@@ -42,32 +42,10 @@
#include "sysemu/runstate.h"
#include "qom/object.h"
-struct PIIX4State {
-PCIDevice dev;
-
-ISAPICState pic;
-RTCState rtc;
-PCIIDEState ide;
-UHCIState uhci;
-PIIX4PMState pm;
-
-uint32_t smb_io_base;
-
-/* Reset Control Register */
-MemoryRegion rcr_mem;
-uint8_t rcr;
-
-bool has_acpi;
-bool has_usb;
-bool smm_enabled;
-};
-
-OBJECT_DECLARE_SIMPLE_TYPE(PIIX4State, PIIX4_PCI_DEVICE)
-
static void piix4_set_irq(void *opaque, int irq_num, int level)
{
int i, pic_irq, pic_level;
-PIIX4State *s = opaque;
+PIIXState *s = opaque;
PCIBus *bus = pci_get_bus(>dev);
/* now we change the pic irq level according to the piix irq mappings */
@@ -87,7 +65,7 @@ static void piix4_set_irq(void *opaque, int irq_num, int
level)
static void piix4_isa_reset(DeviceState *dev)
{
-PIIX4State *d = PIIX4_PCI_DEVICE(dev);
+PIIXState *d = PIIX_PCI_DEVICE(dev);
uint8_t *pci_conf = d->dev.config;
pci_conf[0x04] = 0x07; // master, memory and I/O
@@ -122,12 +100,13 @@ static void piix4_isa_reset(DeviceState *dev)
pci_conf[0xac] = 0x00;
pci_conf[0xae] = 0x00;
+d->pic_levels = 0; /* not used in PIIX4 */
d->rcr = 0;
}
static int piix4_post_load(void *opaque, int version_id)
{
-PIIX4State *s = opaque;
+PIIXState *s = opaque;
if (version_id == 2) {
s->rcr = 0;
@@ -142,8 +121,8 @@ static const VMStateDescription vmstate_piix4 = {
.minimum_version_id = 2,
.post_load = piix4_post_load,
.fields = (VMStateField[]) {
-VMSTATE_PCI_DEVICE(dev, PIIX4State),
-VMSTATE_UINT8_V(rcr, PIIX4State, 3),
+VMSTATE_PCI_DEVICE(dev, PIIXState),
+VMSTATE_UINT8_V(rcr, PIIXState, 3),
VMSTATE_END_OF_LIST()
}
};
@@ -151,7 +130,7 @@ static const VMStateDescription vmstate_piix4 = {
static void piix4_rcr_write(void *opaque, hwaddr addr, uint64_t val,
unsigned int len)
{
-PIIX4State *s = opaque;
+PIIXState *s = opaque;
if (val & 4) {
qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
@@ -163,7 +142,7 @@ static void piix4_rcr_write(void *opaque, hwaddr addr,
uint64_t val,
static uint64_t piix4_rcr_read(void *opaque, hwaddr addr, unsigned int len)
{
-PIIX4State *s = opaque;
+PIIXState *s = opaque;
return s->rcr;
}
@@ -180,7 +159,7 @@ static const MemoryRegionOps piix4_rcr_ops = {
static void piix4_realize(PCIDevice *dev, Error **errp)
{
-PIIX4State *s = PIIX4_PCI_DEVICE(dev);
+PIIXState *s = PIIX_PCI_DEVICE(dev);
PCIBus *pci_bus = pci_get_bus(dev);
ISABus *isa_bus;
@@ -250,7 +229,7 @@ static void piix4_realize(PCIDevice *dev, Error **errp)
static void piix4_init(Object *obj)
{
-PIIX4State *s = PIIX4_PCI_DEVICE(obj);
+PIIXState *s = PIIX_PCI_DEVICE(obj);
object_initialize_child(obj, "pic", >pic, TYPE_ISA_PIC);
object_initialize_child(obj, "rtc", >rtc, TYPE_MC146818_RTC);
@@ -258,10 +237,10 @@ static void piix4_init(Object *obj)
}
static Property piix4_props[] = {
-DEFINE_PROP_UINT32("smb_io_base", PIIX4State, smb_io_base, 0),
-DEFINE_PROP_BOOL("has-acpi", PIIX4State, has_acpi, true),
-DEFINE_PROP_BOOL("has-usb", PIIX4State, has_usb, true),
-DEFINE_PROP_BOOL("smm-enabled", PIIX4State, smm_enabled, false),
+DEFINE_PROP_UINT32("smb_io_base", PIIXState, smb_io_base, 0),
+DEFINE_PROP_BOOL("has-acpi", PIIXState, has_acpi, true),
+DEFINE_PROP_BOOL("has-usb", PIIXState, has_usb, true),
+DEFINE_PROP_BOOL("smm-enabled", PIIXState, smm_enabled, false),
DEFINE_PROP_END_OF_LIST(),
};
@@ -289,7 +268,7 @@ static void piix4_class_init(ObjectClass *klass, void *data)
static const TypeInfo piix4_info = {
.name = TYPE_PIIX4_PCI_DEVICE,
.parent= TYPE_PCI_DEVICE,
-.instance_size = sizeof(PIIX4State),
+.instance_size = sizeof(PIIXState),
.instance_init = piix4_init,
.class_init= piix4_class_init,
.interfaces = (InterfaceInfo[]) {
--
2.39.0
[PATCH v6 14/33] hw/isa/piix3: Create USB controller in host device
The USB controller is an integral part of PIIX3 (function 2). So create
it as part of the south bridge.
Note that the USB function is optional in QEMU. This is why it gets
object_initialize_child()'ed in realize rather than in instance_init.
Signed-off-by: Bernhard Beschow
Reviewed-by: Michael S. Tsirkin
Message-Id: <20221022150508.26830-13-shen...@gmail.com>
---
include/hw/southbridge/piix.h | 4
hw/i386/pc_piix.c | 7 ++-
hw/isa/piix3.c| 17 +
hw/isa/Kconfig| 1 +
4 files changed, 24 insertions(+), 5 deletions(-)
diff --git a/include/hw/southbridge/piix.h b/include/hw/southbridge/piix.h
index b06d26fa11..762709f2fd 100644
--- a/include/hw/southbridge/piix.h
+++ b/include/hw/southbridge/piix.h
@@ -15,6 +15,7 @@
#include "hw/pci/pci_device.h"
#include "qom/object.h"
#include "hw/rtc/mc146818rtc.h"
+#include "hw/usb/hcd-uhci.h"
/* PIRQRC[A:D]: PIRQx Route Control Registers */
#define PIIX_PIRQCA 0x60
@@ -54,12 +55,15 @@ struct PIIXState {
int32_t pci_irq_levels_vmstate[PIIX_NUM_PIRQS];
RTCState rtc;
+UHCIState uhci;
/* Reset Control Register contents */
uint8_t rcr;
/* IO memory region for Reset Control Register (PIIX_RCR_IOPORT) */
MemoryRegion rcr_mem;
+
+bool has_usb;
};
typedef struct PIIXState PIIX3State;
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 37afc01d30..61d8152078 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -52,7 +52,6 @@
#include "exec/memory.h"
#include "hw/acpi/acpi.h"
#include "hw/acpi/piix4.h"
-#include "hw/usb/hcd-uhci.h"
#include "qapi/error.h"
#include "qemu/error-report.h"
#include "sysemu/xen.h"
@@ -236,6 +235,8 @@ static void pc_init1(MachineState *machine,
pcms->bus = pci_bus;
pci_dev = pci_new_multifunction(-1, true, type);
+object_property_set_bool(OBJECT(pci_dev), "has-usb",
+ machine_usb(machine), _abort);
pci_realize_and_unref(pci_dev, pci_bus, _fatal);
piix3 = PIIX3_PCI_DEVICE(pci_dev);
piix3->pic = x86ms->gsi;
@@ -314,10 +315,6 @@ static void pc_init1(MachineState *machine,
}
#endif
-if (pcmc->pci_enabled && machine_usb(machine)) {
-pci_create_simple(pci_bus, piix3_devfn + 2, TYPE_PIIX3_USB_UHCI);
-}
-
if (pcmc->pci_enabled && x86_machine_is_acpi_enabled(X86_MACHINE(pcms))) {
PCIDevice *piix4_pm;
diff --git a/hw/isa/piix3.c b/hw/isa/piix3.c
index e8ddb6a602..45c20dea17 100644
--- a/hw/isa/piix3.c
+++ b/hw/isa/piix3.c
@@ -288,6 +288,7 @@ static const MemoryRegionOps rcr_ops = {
static void pci_piix3_realize(PCIDevice *dev, Error **errp)
{
PIIX3State *d = PIIX3_PCI_DEVICE(dev);
+PCIBus *pci_bus = pci_get_bus(dev);
ISABus *isa_bus;
isa_bus = isa_bus_new(DEVICE(d), pci_address_space(dev),
@@ -308,6 +309,16 @@ static void pci_piix3_realize(PCIDevice *dev, Error **errp)
if (!qdev_realize(DEVICE(>rtc), BUS(isa_bus), errp)) {
return;
}
+
+/* USB */
+if (d->has_usb) {
+object_initialize_child(OBJECT(dev), "uhci", >uhci,
+TYPE_PIIX3_USB_UHCI);
+qdev_prop_set_int32(DEVICE(>uhci), "addr", dev->devfn + 2);
+if (!qdev_realize(DEVICE(>uhci), BUS(pci_bus), errp)) {
+return;
+}
+}
}
static void build_pci_isa_aml(AcpiDevAmlIf *adev, Aml *scope)
@@ -341,6 +352,11 @@ static void pci_piix3_init(Object *obj)
object_initialize_child(obj, "rtc", >rtc, TYPE_MC146818_RTC);
}
+static Property pci_piix3_props[] = {
+DEFINE_PROP_BOOL("has-usb", PIIX3State, has_usb, true),
+DEFINE_PROP_END_OF_LIST(),
+};
+
static void pci_piix3_class_init(ObjectClass *klass, void *data)
{
DeviceClass *dc = DEVICE_CLASS(klass);
@@ -360,6 +376,7 @@ static void pci_piix3_class_init(ObjectClass *klass, void
*data)
* pc_piix.c's pc_init1()
*/
dc->user_creatable = false;
+device_class_set_props(dc, pci_piix3_props);
adevc->build_dev_aml = build_pci_isa_aml;
}
diff --git a/hw/isa/Kconfig b/hw/isa/Kconfig
index c10cbc5fc1..f01bc0dff3 100644
--- a/hw/isa/Kconfig
+++ b/hw/isa/Kconfig
@@ -36,6 +36,7 @@ config PIIX3
select I8257
select ISA_BUS
select MC146818RTC
+select USB_UHCI
config PIIX4
bool
--
2.39.0
[PATCH v6 25/33] hw/isa/piix4: Use TYPE_ISA_PIC device
Aligns the code with PIIX3 such that PIIXState can be used in PIIX4,
too.
Signed-off-by: Bernhard Beschow
Reviewed-by: Michael S. Tsirkin
Message-Id: <20221022150508.26830-33-shen...@gmail.com>
---
hw/isa/piix4.c | 28 ++--
hw/mips/malta.c | 11 +--
hw/mips/Kconfig | 1 +
3 files changed, 20 insertions(+), 20 deletions(-)
diff --git a/hw/isa/piix4.c b/hw/isa/piix4.c
index 9edaa5de3e..eae4db0182 100644
--- a/hw/isa/piix4.c
+++ b/hw/isa/piix4.c
@@ -44,9 +44,8 @@
struct PIIX4State {
PCIDevice dev;
-qemu_irq cpu_intr;
-qemu_irq *isa;
+ISAPICState pic;
RTCState rtc;
PCIIDEState ide;
UHCIState uhci;
@@ -82,7 +81,7 @@ static void piix4_set_irq(void *opaque, int irq_num, int
level)
pic_level |= pci_bus_get_irq_level(bus, i);
}
}
-qemu_set_irq(s->isa[pic_irq], pic_level);
+qemu_set_irq(s->pic.in_irqs[pic_irq], pic_level);
}
}
@@ -149,12 +148,6 @@ static const VMStateDescription vmstate_piix4 = {
}
};
-static void piix4_request_i8259_irq(void *opaque, int irq, int level)
-{
-PIIX4State *s = opaque;
-qemu_set_irq(s->cpu_intr, level);
-}
-
static void piix4_rcr_write(void *opaque, hwaddr addr, uint64_t val,
unsigned int len)
{
@@ -190,7 +183,6 @@ static void piix4_realize(PCIDevice *dev, Error **errp)
PIIX4State *s = PIIX4_PCI_DEVICE(dev);
PCIBus *pci_bus = pci_get_bus(dev);
ISABus *isa_bus;
-qemu_irq *i8259_out_irq;
isa_bus = isa_bus_new(DEVICE(dev), pci_address_space(dev),
pci_address_space_io(dev), errp);
@@ -198,20 +190,18 @@ static void piix4_realize(PCIDevice *dev, Error **errp)
return;
}
-qdev_init_gpio_out_named(DEVICE(dev), >cpu_intr,
- "intr", 1);
-
memory_region_init_io(>rcr_mem, OBJECT(dev), _rcr_ops, s,
"reset-control", 1);
memory_region_add_subregion_overlap(pci_address_space_io(dev),
PIIX_RCR_IOPORT, >rcr_mem, 1);
/* initialize i8259 pic */
-i8259_out_irq = qemu_allocate_irqs(piix4_request_i8259_irq, s, 1);
-s->isa = i8259_init(isa_bus, *i8259_out_irq);
+if (!qdev_realize(DEVICE(>pic), NULL, errp)) {
+return;
+}
/* initialize ISA irqs */
-isa_bus_irqs(isa_bus, s->isa);
+isa_bus_irqs(isa_bus, s->pic.in_irqs);
/* initialize pit */
i8254_pit_init(isa_bus, 0x40, 0, NULL);
@@ -224,7 +214,7 @@ static void piix4_realize(PCIDevice *dev, Error **errp)
if (!qdev_realize(DEVICE(>rtc), BUS(isa_bus), errp)) {
return;
}
-s->rtc.irq = isa_get_irq(ISA_DEVICE(>rtc), s->rtc.isairq);
+s->rtc.irq = qdev_get_gpio_in(DEVICE(>pic), s->rtc.isairq);
/* IDE */
qdev_prop_set_int32(DEVICE(>ide), "addr", dev->devfn + 1);
@@ -251,7 +241,8 @@ static void piix4_realize(PCIDevice *dev, Error **errp)
if (!qdev_realize(DEVICE(>pm), BUS(pci_bus), errp)) {
return;
}
-qdev_connect_gpio_out(DEVICE(>pm), 0, s->isa[9]);
+qdev_connect_gpio_out(DEVICE(>pm), 0,
+ qdev_get_gpio_in(DEVICE(>pic), 9));
}
pci_bus_irqs(pci_bus, piix4_set_irq, s, PIIX_NUM_PIRQS);
@@ -261,6 +252,7 @@ static void piix4_init(Object *obj)
{
PIIX4State *s = PIIX4_PCI_DEVICE(obj);
+object_initialize_child(obj, "pic", >pic, TYPE_ISA_PIC);
object_initialize_child(obj, "rtc", >rtc, TYPE_MC146818_RTC);
object_initialize_child(obj, "ide", >ide, TYPE_PIIX4_IDE);
}
diff --git a/hw/mips/malta.c b/hw/mips/malta.c
index 4a2e0edd98..55f3e0c54a 100644
--- a/hw/mips/malta.c
+++ b/hw/mips/malta.c
@@ -29,6 +29,7 @@
#include "qemu/guest-random.h"
#include "hw/clock.h"
#include "hw/southbridge/piix.h"
+#include "hw/intc/i8259.h"
#include "hw/isa/superio.h"
#include "hw/char/serial.h"
#include "net/net.h"
@@ -1280,10 +1281,11 @@ void mips_malta_init(MachineState *machine)
PCIBus *pci_bus;
ISABus *isa_bus;
qemu_irq cbus_irq, i8259_irq;
+qemu_irq *i8259;
I2CBus *smbus;
DriveInfo *dinfo;
int fl_idx = 0;
-int be;
+int be, i;
MaltaState *s;
PCIDevice *piix4;
DeviceState *dev;
@@ -1459,7 +1461,12 @@ void mips_malta_init(MachineState *machine)
pci_ide_create_devs(PCI_DEVICE(dev));
/* Interrupt controller */
-qdev_connect_gpio_out_named(DEVICE(piix4), "intr", 0, i8259_irq);
+dev = DEVICE(object_resolve_path_component(OBJECT(piix4), "pic"));
+i8259 = i8259_init(isa_bus, i8259_irq);
+for (i = 0; i < ISA_NUM_IRQS; i++) {
+qdev_connect_gpio_out(dev, i, i8259[i]);
+}
+g_free(i8259);
/* generate SPD EEPROM data */
dev = DEVICE(object_resolve_path_component(OBJECT(piix4), "pm"));
diff --git a/hw/mips/Kconfig b/hw/mips/Kconfig
index 4e7042f03d..d156de812c 100644
--- a/hw/mips/Kconfig
+++ b/hw/mips/Kconfig
@@ -1,5 +1,6 @@
config
[PATCH v6 20/33] hw/isa/piix3: Rename pci_piix3_props for sharing with PIIX4
Signed-off-by: Bernhard Beschow
Reviewed-by: Philippe Mathieu-Daudé
Reviewed-by: Michael S. Tsirkin
Message-Id: <20221022150508.26830-22-shen...@gmail.com>
---
hw/isa/piix3.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/isa/piix3.c b/hw/isa/piix3.c
index e813e20639..c6659bbfda 100644
--- a/hw/isa/piix3.c
+++ b/hw/isa/piix3.c
@@ -381,7 +381,7 @@ static void pci_piix3_init(Object *obj)
object_initialize_child(obj, "ide", >ide, TYPE_PIIX3_IDE);
}
-static Property pci_piix3_props[] = {
+static Property pci_piix_props[] = {
DEFINE_PROP_UINT32("smb_io_base", PIIX3State, smb_io_base, 0),
DEFINE_PROP_BOOL("has-acpi", PIIX3State, has_acpi, true),
DEFINE_PROP_BOOL("has-usb", PIIX3State, has_usb, true),
@@ -408,7 +408,7 @@ static void pci_piix3_class_init(ObjectClass *klass, void
*data)
* pc_piix.c's pc_init1()
*/
dc->user_creatable = false;
-device_class_set_props(dc, pci_piix3_props);
+device_class_set_props(dc, pci_piix_props);
adevc->build_dev_aml = build_pci_isa_aml;
}
--
2.39.0
[PATCH v6 17/33] hw/isa/piix3: Create IDE controller in host device
Now that PIIX3 contains the new TYPE_ISA_PIC, it is possible to
instantiate PIIX3 IDE in the PIIX3 southbridge. PIIX3 IDE wires up its
interrupts to the ISA bus in its realize method which requires the
interrupt controller to provide fully populated qemu_irqs. This is the
case for TYPE_ISA_PIC even though the virtualization technology isn't
known yet.
Signed-off-by: Bernhard Beschow
Reviewed-by: Michael S. Tsirkin
Message-Id: <20221022150508.26830-17-shen...@gmail.com>
---
include/hw/southbridge/piix.h | 2 ++
hw/i386/pc_piix.c | 15 ++-
hw/isa/piix3.c| 8
hw/i386/Kconfig | 1 -
hw/isa/Kconfig| 1 +
5 files changed, 17 insertions(+), 10 deletions(-)
diff --git a/include/hw/southbridge/piix.h b/include/hw/southbridge/piix.h
index 514ccdb7fd..e3c35ca16f 100644
--- a/include/hw/southbridge/piix.h
+++ b/include/hw/southbridge/piix.h
@@ -15,6 +15,7 @@
#include "hw/pci/pci_device.h"
#include "qom/object.h"
#include "hw/acpi/piix4.h"
+#include "hw/ide/pci.h"
#include "hw/intc/i8259.h"
#include "hw/rtc/mc146818rtc.h"
#include "hw/usb/hcd-uhci.h"
@@ -56,6 +57,7 @@ struct PIIXState {
ISAPICState pic;
RTCState rtc;
+PCIIDEState ide;
UHCIState uhci;
PIIX4PMState pm;
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index df8e2e389b..4675981021 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -41,7 +41,6 @@
#include "hw/usb.h"
#include "net/net.h"
#include "hw/ide/pci.h"
-#include "hw/ide/piix.h"
#include "hw/irq.h"
#include "sysemu/kvm.h"
#include "hw/kvm/clock.h"
@@ -98,7 +97,6 @@ static void pc_init1(MachineState *machine,
PCIBus *pci_bus;
ISABus *isa_bus;
Object *piix4_pm;
-int piix3_devfn = -1;
qemu_irq smi_irq;
GSIState *gsi_state;
BusState *idebus[MAX_IDE_BUS];
@@ -252,11 +250,14 @@ static void pc_init1(MachineState *machine,
for (i = 0; i < ISA_NUM_IRQS; i++) {
qdev_connect_gpio_out(dev, i, x86ms->gsi[i]);
}
-piix3_devfn = pci_dev->devfn;
isa_bus = ISA_BUS(qdev_get_child_bus(DEVICE(pci_dev), "isa.0"));
rtc_state = ISA_DEVICE(object_resolve_path_component(OBJECT(pci_dev),
"rtc"));
piix4_pm = object_resolve_path_component(OBJECT(pci_dev), "pm");
+dev = DEVICE(object_resolve_path_component(OBJECT(pci_dev), "ide"));
+pci_ide_create_devs(PCI_DEVICE(dev));
+idebus[0] = qdev_get_child_bus(dev, "ide.0");
+idebus[1] = qdev_get_child_bus(dev, "ide.1");
} else {
pci_bus = NULL;
piix4_pm = NULL;
@@ -270,6 +271,8 @@ static void pc_init1(MachineState *machine,
i8257_dma_init(isa_bus, 0);
pcms->hpet_enabled = false;
+idebus[0] = NULL;
+idebus[1] = NULL;
}
if (x86ms->pic == ON_OFF_AUTO_ON || x86ms->pic == ON_OFF_AUTO_AUTO) {
@@ -298,12 +301,6 @@ static void pc_init1(MachineState *machine,
pc_nic_init(pcmc, isa_bus, pci_bus);
if (pcmc->pci_enabled) {
-PCIDevice *dev;
-
-dev = pci_create_simple(pci_bus, piix3_devfn + 1, TYPE_PIIX3_IDE);
-pci_ide_create_devs(dev);
-idebus[0] = qdev_get_child_bus(>qdev, "ide.0");
-idebus[1] = qdev_get_child_bus(>qdev, "ide.1");
pc_cmos_init(pcms, idebus[0], idebus[1], rtc_state);
}
#ifdef CONFIG_IDE_ISA
diff --git a/hw/isa/piix3.c b/hw/isa/piix3.c
index 88a6bf28ea..a549b1a8a5 100644
--- a/hw/isa/piix3.c
+++ b/hw/isa/piix3.c
@@ -29,6 +29,7 @@
#include "hw/southbridge/piix.h"
#include "hw/irq.h"
#include "hw/qdev-properties.h"
+#include "hw/ide/piix.h"
#include "hw/isa/isa.h"
#include "hw/xen/xen.h"
#include "sysemu/runstate.h"
@@ -317,6 +318,12 @@ static void pci_piix3_realize(PCIDevice *dev, Error **errp)
return;
}
+/* IDE */
+qdev_prop_set_int32(DEVICE(>ide), "addr", dev->devfn + 1);
+if (!qdev_realize(DEVICE(>ide), BUS(pci_bus), errp)) {
+return;
+}
+
/* USB */
if (d->has_usb) {
object_initialize_child(OBJECT(dev), "uhci", >uhci,
@@ -369,6 +376,7 @@ static void pci_piix3_init(Object *obj)
object_initialize_child(obj, "pic", >pic, TYPE_ISA_PIC);
object_initialize_child(obj, "rtc", >rtc, TYPE_MC146818_RTC);
+object_initialize_child(obj, "ide", >ide, TYPE_PIIX3_IDE);
}
static Property pci_piix3_props[] = {
diff --git a/hw/i386/Kconfig b/hw/i386/Kconfig
index 1291f59896..7e53dc8f82 100644
--- a/hw/i386/Kconfig
+++ b/hw/i386/Kconfig
@@ -74,7 +74,6 @@ config I440FX
select I8259
select PCI_I440FX
select PIIX3
-select IDE_PIIX
select DIMM
select SMBIOS
select FW_CFG_DMA
diff --git a/hw/isa/Kconfig b/hw/isa/Kconfig
index fccf095d07..b911306909 100644
--- a/hw/isa/Kconfig
+++ b/hw/isa/Kconfig
@@ -36,6 +36,7 @@ config PIIX3
select ACPI_PIIX4
select I8257
select I8259
+select IDE_PIIX
select ISA_BUS
[PATCH v6 16/33] hw/isa/piix3: Create TYPE_ISA_PIC in host device
The newly introduced TYPE_ISA_PIC allows for wiring up devices
in the southbridge where the virtualization technology used
(KVM, TCG, Xen) is not yet known.
Signed-off-by: Bernhard Beschow
Reviewed-by: Michael S. Tsirkin
Message-Id: <20221022150508.26830-16-shen...@gmail.com>
---
include/hw/southbridge/piix.h | 4 ++--
hw/i386/pc_piix.c | 15 +--
hw/isa/piix3.c| 10 +-
hw/i386/Kconfig | 1 +
hw/isa/Kconfig| 1 +
5 files changed, 22 insertions(+), 9 deletions(-)
diff --git a/include/hw/southbridge/piix.h b/include/hw/southbridge/piix.h
index b1eaab1d95..514ccdb7fd 100644
--- a/include/hw/southbridge/piix.h
+++ b/include/hw/southbridge/piix.h
@@ -15,6 +15,7 @@
#include "hw/pci/pci_device.h"
#include "qom/object.h"
#include "hw/acpi/piix4.h"
+#include "hw/intc/i8259.h"
#include "hw/rtc/mc146818rtc.h"
#include "hw/usb/hcd-uhci.h"
@@ -50,11 +51,10 @@ struct PIIXState {
#endif
uint64_t pic_levels;
-qemu_irq *pic;
-
/* This member isn't used. Just for save/load compatibility */
int32_t pci_irq_levels_vmstate[PIIX_NUM_PIRQS];
+ISAPICState pic;
RTCState rtc;
UHCIState uhci;
PIIX4PMState pm;
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 5ea8d4a585..df8e2e389b 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -219,10 +219,11 @@ static void pc_init1(MachineState *machine,
gsi_state = pc_gsi_create(>gsi, pcmc->pci_enabled);
if (pcmc->pci_enabled) {
-PIIX3State *piix3;
+DeviceState *dev;
PCIDevice *pci_dev;
const char *type = xen_enabled() ? TYPE_PIIX3_XEN_DEVICE
: TYPE_PIIX3_DEVICE;
+int i;
pci_bus = i440fx_init(pci_type,
i440fx_host,
@@ -247,10 +248,12 @@ static void pc_init1(MachineState *machine,
_abort);
pci_realize_and_unref(pci_dev, pci_bus, _fatal);
-piix3 = PIIX3_PCI_DEVICE(pci_dev);
-piix3->pic = x86ms->gsi;
-piix3_devfn = piix3->dev.devfn;
-isa_bus = ISA_BUS(qdev_get_child_bus(DEVICE(piix3), "isa.0"));
+dev = DEVICE(object_resolve_path_component(OBJECT(pci_dev), "pic"));
+for (i = 0; i < ISA_NUM_IRQS; i++) {
+qdev_connect_gpio_out(dev, i, x86ms->gsi[i]);
+}
+piix3_devfn = pci_dev->devfn;
+isa_bus = ISA_BUS(qdev_get_child_bus(DEVICE(pci_dev), "isa.0"));
rtc_state = ISA_DEVICE(object_resolve_path_component(OBJECT(pci_dev),
"rtc"));
piix4_pm = object_resolve_path_component(OBJECT(pci_dev), "pm");
@@ -259,6 +262,7 @@ static void pc_init1(MachineState *machine,
piix4_pm = NULL;
isa_bus = isa_bus_new(NULL, get_system_memory(), system_io,
_abort);
+isa_bus_irqs(isa_bus, x86ms->gsi);
rtc_state = isa_new(TYPE_MC146818_RTC);
qdev_prop_set_int32(DEVICE(rtc_state), "base_year", 2000);
@@ -267,7 +271,6 @@ static void pc_init1(MachineState *machine,
i8257_dma_init(isa_bus, 0);
pcms->hpet_enabled = false;
}
-isa_bus_irqs(isa_bus, x86ms->gsi);
if (x86ms->pic == ON_OFF_AUTO_ON || x86ms->pic == ON_OFF_AUTO_AUTO) {
pc_i8259_create(isa_bus, gsi_state->i8259_irq);
diff --git a/hw/isa/piix3.c b/hw/isa/piix3.c
index ed7d58bc98..88a6bf28ea 100644
--- a/hw/isa/piix3.c
+++ b/hw/isa/piix3.c
@@ -39,7 +39,7 @@
static void piix3_set_irq_pic(PIIX3State *piix3, int pic_irq)
{
-qemu_set_irq(piix3->pic[pic_irq],
+qemu_set_irq(piix3->pic.in_irqs[pic_irq],
!!(piix3->pic_levels &
(((1ULL << PIIX_NUM_PIRQS) - 1) <<
(pic_irq * PIIX_NUM_PIRQS;
@@ -297,6 +297,13 @@ static void pci_piix3_realize(PCIDevice *dev, Error **errp)
return;
}
+/* PIC */
+if (!qdev_realize(DEVICE(>pic), NULL, errp)) {
+return;
+}
+
+isa_bus_irqs(isa_bus, d->pic.in_irqs);
+
memory_region_init_io(>rcr_mem, OBJECT(dev), _ops, d,
"piix3-reset-control", 1);
memory_region_add_subregion_overlap(pci_address_space_io(dev),
@@ -360,6 +367,7 @@ static void pci_piix3_init(Object *obj)
{
PIIX3State *d = PIIX3_PCI_DEVICE(obj);
+object_initialize_child(obj, "pic", >pic, TYPE_ISA_PIC);
object_initialize_child(obj, "rtc", >rtc, TYPE_MC146818_RTC);
}
diff --git a/hw/i386/Kconfig b/hw/i386/Kconfig
index c4fb5b49bd..1291f59896 100644
--- a/hw/i386/Kconfig
+++ b/hw/i386/Kconfig
@@ -71,6 +71,7 @@ config I440FX
select ACPI_PIIX4
select PC_PCI
select PC_ACPI
+select I8259
select PCI_I440FX
select PIIX3
select IDE_PIIX
diff --git a/hw/isa/Kconfig b/hw/isa/Kconfig
index cf79580384..fccf095d07 100644
--- a/hw/isa/Kconfig
+++ b/hw/isa/Kconfig
@@ -35,6 +35,7 @@ config PIIX3
bool
[PATCH v6 12/33] hw/i386/pc: No need for rtc_state to be an out-parameter
Now that the RTC is created as part of the southbridges it doesn't need
to be an out-parameter any longer.
Signed-off-by: Bernhard Beschow
Reviewed-by: Peter Maydell
Reviewed-by: Michael S. Tsirkin
Reviewed-by: Thomas Huth
Reviewed-by: Philippe Mathieu-Daudé
Message-Id: <20221022150508.26830-12-shen...@gmail.com>
---
include/hw/i386/pc.h | 2 +-
hw/i386/pc.c | 12 ++--
hw/i386/pc_piix.c| 2 +-
hw/i386/pc_q35.c | 2 +-
4 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 991f905f5d..dd059e8667 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -169,7 +169,7 @@ uint64_t pc_pci_hole64_start(void);
DeviceState *pc_vga_init(ISABus *isa_bus, PCIBus *pci_bus);
void pc_basic_device_init(struct PCMachineState *pcms,
ISABus *isa_bus, qemu_irq *gsi,
- ISADevice **rtc_state,
+ ISADevice *rtc_state,
bool create_fdctrl,
uint32_t hpet_irqs);
void pc_cmos_init(PCMachineState *pcms,
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 448557333b..53a5443e09 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1251,7 +1251,7 @@ static void pc_superio_init(ISABus *isa_bus, bool
create_fdctrl,
void pc_basic_device_init(struct PCMachineState *pcms,
ISABus *isa_bus, qemu_irq *gsi,
- ISADevice **rtc_state,
+ ISADevice *rtc_state,
bool create_fdctrl,
uint32_t hpet_irqs)
{
@@ -1306,17 +1306,17 @@ void pc_basic_device_init(struct PCMachineState *pcms,
}
if (rtc_irq) {
-qdev_connect_gpio_out(DEVICE(*rtc_state), 0, rtc_irq);
+qdev_connect_gpio_out(DEVICE(rtc_state), 0, rtc_irq);
} else {
-uint32_t irq = object_property_get_uint(OBJECT(*rtc_state),
+uint32_t irq = object_property_get_uint(OBJECT(rtc_state),
"irq",
_fatal);
-isa_connect_gpio_out(*rtc_state, 0, irq);
+isa_connect_gpio_out(rtc_state, 0, irq);
}
-object_property_add_alias(OBJECT(pcms), "rtc-time", OBJECT(*rtc_state),
+object_property_add_alias(OBJECT(pcms), "rtc-time", OBJECT(rtc_state),
"date");
-qemu_register_boot_set(pc_boot_set, *rtc_state);
+qemu_register_boot_set(pc_boot_set, rtc_state);
if (!xen_enabled() &&
(x86ms->pit == ON_OFF_AUTO_AUTO || x86ms->pit == ON_OFF_AUTO_ON)) {
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 8f894714e5..a577ea2f4e 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -276,7 +276,7 @@ static void pc_init1(MachineState *machine,
}
/* init basic PC hardware */
-pc_basic_device_init(pcms, isa_bus, x86ms->gsi, _state, true,
+pc_basic_device_init(pcms, isa_bus, x86ms->gsi, rtc_state, true,
0x4);
pc_nic_init(pcmc, isa_bus, pci_bus);
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index da97df69f7..58c51fbd9e 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -293,7 +293,7 @@ static void pc_q35_init(MachineState *machine)
}
/* init basic PC hardware */
-pc_basic_device_init(pcms, isa_bus, x86ms->gsi, _state, !mc->no_floppy,
+pc_basic_device_init(pcms, isa_bus, x86ms->gsi, rtc_state, !mc->no_floppy,
0xff0104);
/* connect pm stuff to lpc */
--
2.39.0
[PATCH v6 13/33] hw/i386/pc_piix: Allow for setting properties before realizing PIIX3 south bridge
The next patches will need to take advantage of it. Signed-off-by: Bernhard Beschow Reviewed-by: Peter Maydell Reviewed-by: Michael S. Tsirkin Message-Id: <20221022150508.26830-3-shen...@gmail.com> --- hw/i386/pc_piix.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c index a577ea2f4e..37afc01d30 100644 --- a/hw/i386/pc_piix.c +++ b/hw/i386/pc_piix.c @@ -235,7 +235,8 @@ static void pc_init1(MachineState *machine, : pc_pci_slot_get_pirq); pcms->bus = pci_bus; -pci_dev = pci_create_simple_multifunction(pci_bus, -1, true, type); +pci_dev = pci_new_multifunction(-1, true, type); +pci_realize_and_unref(pci_dev, pci_bus, _fatal); piix3 = PIIX3_PCI_DEVICE(pci_dev); piix3->pic = x86ms->gsi; piix3_devfn = piix3->dev.devfn; -- 2.39.0
Re: [PATCH v5 06/14] migration/qemu-file: Add qemu_file_get_to_fd()
On 09/01/2023 13:20, Cédric Le Goater wrote:
External email: Use caution opening links or attachments
On 12/29/22 12:03, Avihai Horon wrote:
Add new function qemu_file_get_to_fd() that allows reading data from
QEMUFile and writing it straight into a given fd.
This will be used later in VFIO migration code.
Signed-off-by: Avihai Horon
Reviewed-by: Vladimir Sementsov-Ogievskiy
---
migration/qemu-file.h | 1 +
migration/qemu-file.c | 34 ++
2 files changed, 35 insertions(+)
diff --git a/migration/qemu-file.h b/migration/qemu-file.h
index fa13d04d78..9d0155a2a1 100644
--- a/migration/qemu-file.h
+++ b/migration/qemu-file.h
@@ -148,6 +148,7 @@ int qemu_file_shutdown(QEMUFile *f);
QEMUFile *qemu_file_get_return_path(QEMUFile *f);
void qemu_fflush(QEMUFile *f);
void qemu_file_set_blocking(QEMUFile *f, bool block);
+int qemu_file_get_to_fd(QEMUFile *f, int fd, size_t size);
void ram_control_before_iterate(QEMUFile *f, uint64_t flags);
void ram_control_after_iterate(QEMUFile *f, uint64_t flags);
diff --git a/migration/qemu-file.c b/migration/qemu-file.c
index 2d5f74ffc2..79303c9d34 100644
--- a/migration/qemu-file.c
+++ b/migration/qemu-file.c
@@ -940,3 +940,37 @@ QIOChannel *qemu_file_get_ioc(QEMUFile *file)
{
return file->ioc;
}
+
+/*
+ * Read size bytes from QEMUFile f and write them to fd.
+ */
+int qemu_file_get_to_fd(QEMUFile *f, int fd, size_t size)
+{
+ while (size) {
+ size_t pending = f->buf_size - f->buf_index;
+ ssize_t rc;
+
+ if (!pending) {
+ rc = qemu_fill_buffer(f);
+ if (rc < 0) {
+ return rc;
+ }
+ if (rc == 0) {
+ return -1;
Given the call stack, -1 would be interpreted as EPERM. May be EIO
instead ?
Sure.
I will also change write (down below) to return -errno instead of rc on
error.
C.
+ }
+ continue;
+ }
+
+ rc = write(fd, f->buf + f->buf_index, MIN(pending, size));
+ if (rc < 0) {
+ return rc;
+ }
+ if (rc == 0) {
+ return -1;
+ }
+ f->buf_index += rc;
+ size -= rc;
+ }
+
+ return 0;
+}
Re: [PATCH v5 10/14] vfio/migration: Implement VFIO migration protocol v2
On 09/01/2023 12:20, Cédric Le Goater wrote:
External email: Use caution opening links or attachments
Hello Avihai,
On 12/29/22 12:03, Avihai Horon wrote:
+static int vfio_save_setup(QEMUFile *f, void *opaque)
+{
+ VFIODevice *vbasedev = opaque;
+ VFIOMigration *migration = vbasedev->migration;
+ uint64_t stop_copy_size;
+
+ qemu_put_be64(f, VFIO_MIG_FLAG_DEV_SETUP_STATE);
+
+ if (vfio_query_stop_copy_size(vbasedev, _copy_size)) {
+ stop_copy_size = VFIO_MIG_DEFAULT_DATA_BUFFER_SIZE;
+ }
+ migration->data_buffer_size =
MIN(VFIO_MIG_DEFAULT_DATA_BUFFER_SIZE,
+ stop_copy_size);
+ migration->data_buffer =
g_try_malloc0(migration->data_buffer_size);
+ if (!migration->data_buffer) {
+ error_report("%s: Failed to allocate migration data buffer",
+ vbasedev->name);
+ return -ENOMEM;
+ }
+
+ trace_vfio_save_setup(vbasedev->name, migration->data_buffer_size);
+
+ qemu_put_be64(f, VFIO_MIG_FLAG_END_OF_STATE);
+
+ return qemu_file_get_error(f);
+}
+
This fails to compile with :
gcc version 12.2.1 20221121 (Red Hat 12.2.1-4) (GCC) complains with :
../include/qemu/osdep.h:315:22: error: ‘stop_copy_size’ may be used
uninitialized [-Werror=maybe-uninitialized]
315 | _a < _b ? _a : _b; \
| ^
../hw/vfio/migration.c:262:14: note: ‘stop_copy_size’ was declared here
262 | uint64_t stop_copy_size;
| ^~
cc1: all warnings being treated as errors
May be rework the code slightly to avoid the breakage :
+++ qemu.git/hw/vfio/migration.c
@@ -259,13 +259,11 @@ static int vfio_save_setup(QEMUFile *f,
{
VFIODevice *vbasedev = opaque;
VFIOMigration *migration = vbasedev->migration;
- uint64_t stop_copy_size;
+ uint64_t stop_copy_size = VFIO_MIG_DEFAULT_DATA_BUFFER_SIZE;
qemu_put_be64(f, VFIO_MIG_FLAG_DEV_SETUP_STATE);
- if (vfio_query_stop_copy_size(vbasedev, _copy_size)) {
- stop_copy_size = VFIO_MIG_DEFAULT_DATA_BUFFER_SIZE;
- }
+ vfio_query_stop_copy_size(vbasedev, _copy_size);
migration->data_buffer_size = MIN(VFIO_MIG_DEFAULT_DATA_BUFFER_SIZE,
stop_copy_size);
migration->data_buffer = g_try_malloc0(migration->data_buffer_size);
and report the error in vfio_query_stop_copy_size()
Thanks, Cedric.
There is another similar case in vfio_save_pending().
I will fix both of them.
Thanks,
C.
Re: Questions about how block devices use snapshots
Am 09.01.2023 um 13:45 hat Zhiyong Ye geschrieben: > Qemu provides powerful snapshot capabilities for different file > formats. But this is limited to the block backend being a file, and > support is not good enough when it is a block device. When creating > snapshots based on files, there is no need to specify the size of the > snapshot image, which can grow dynamically as the virtual machine is > used. But block devices are fixed in size at creation and cannot be > dynamically grown at a later time. > > So is there any way to support snapshots when the block backend is a > block device? In order to have snapshots, you need to have an image format like qcow2. A qcow2 file can have a raw block device as its backing file, so even if you store the overlay image on a filesystem, you have technically snapshotted a block device. This may or may not be enough for your use case. It is also possible to store qcow2 files on block devices, though depending on your requirements, it can get very tricky because then you're responsible for making sure that there is always enough free space on the block device. So a second, still very simple, approach could be taking a second block device that is a little bit larger than the virtual disk (for the qcow2 metadata) and use that as the external snapshot. Obviously, you require a lot of disk space this way, because each snapshots needs to be able to store the full image. You could also use internal snapshots. In this case, you just need to make sure that the block device is a lot larger than the virtual disk, so that there is enough space left for storing the snapshots. At some point it will be full. And finally, for example if your block devices are actually LVs, you could start resizing the block device dynmically as needed. This becomes very complex quickly and you're on your own, but it is possible and has been done by oVirt. Kevin
Re: [PATCH v4 0/2] hw/nvme: Support for Namespaces Management from guest OS
Am 28.12.2022 um 20:41 hat Jonathan Derrick geschrieben: > Here is the approach: > The nvme device will get new parameter: > - auto-ns-path, which specifies the path to the storage area where back-end >image and necessary config files located stored. > > The virtual devices representing namespaces will be created dynamically during > the Qemu running session following issuance of nvme create-ns and delete-ns > commands from the guest OS. QOM classes and instances will be created > utilizing > existing configuration scheme used during Qemu's start-up. Back-end image > files > will be neither created nor deleted during Qemu's startup or running session. > Instead a set of back-end image files and relevant config will be created by > qemu-img tool with createns sub-command prior to Qemu's session. > Required parameters are: -S serial number which must match serial parameter of > qemu-system-xx -device nvme command line specification, -C total capacity, and > optional -N that will set a maximal limit on number of allowed > namespaces (default 256) which will be followed by path name pointing to > storage location corresponding to auto-ns-path of qemu-system-xx -device nvme > parameter. > > Those created back-end image files will be pre-loaded during Qemu's start-up > and then during running Qemu's session will be associated or disassociated > with > QOM namespaces virtual instances, as a result of issuing nvme create-ns or > delete-ns commands. The associated back-end image file for relevant namespace > will be re-sized as follows: delete-ns command will truncate image file to the > size of 0, whereas create-ns command will re-size the image file to the size > provided by nvme create-ns command parameters. Truncating/re-sizing is a > result > of blk_truncate() API which utilizes co-routines and should not block Qemu > main > thread while scheduling AIO operations. It is assured that all settings will > retain over Qemu's start-ups and shutdowns. The implementation makes it > possible to combine the existing "Additional Namespace" implementation with > the > new "Managed Namespaces". Those will coexist with obvious restrictions, like > both will share the same NsIds space, "static" namespaces cannot be deleted or > if its NsId specified at Qemu's command line will conflicts with previously > created one by nvme create-ns (and retained), this will lead to an abort of > Qemu at its start up. This looks like a valid approach for a proof of concept, but from a backend perspective, I'm concerned that this approach might be too limiting and we won't have a good path forward. For example, how can we integrate this with snapshots? You expect a specific filename for the image, but taking an external snapshot means creating an overlay image with a different name. How do we migrate storage like this? If the management tool (probably libvirt) knows about all the namespace images and the config file (!), it can possibly migrate them individually, but note that while a mirror job is active, images can't be resized any more. What if we don't want to use a directory on the local filesystem to store the images, but some network protocol? It seems to me that we should define proper block layer APIs for handling namespaces, and then we can have your implementation as one possible image driver that supports these APIs, for which we can accept these limitations for now. At least this would already avoid having backend logic in the device implementation, and allow us to replace it with something better later without having to change the design of the device emulation code. Eventually, I think, if we want to have dynamic namespaces properly supported, they need to be a feature on the image format level, so that you could keep all namespaces in a single qcow2 file. Kevin
Re: [PATCH v5 09/14] vfio/migration: Rename functions/structs related to v1 protocol
On 12/29/22 12:03, Avihai Horon wrote:
To avoid name collisions, rename functions and structs related to VFIO
migration protocol v1. This will allow the two protocols to co-exist
when v2 protocol is added, until v1 is removed. No functional changes
intended.
Signed-off-by: Avihai Horon
Reviewed-by: Cédric Le Goater
Thanks,
C.
---
include/hw/vfio/vfio-common.h | 2 +-
hw/vfio/common.c | 6 +-
hw/vfio/migration.c | 106 +-
hw/vfio/trace-events | 12 ++--
4 files changed, 63 insertions(+), 63 deletions(-)
diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
index e573f5a9f1..bbaf72ba00 100644
--- a/include/hw/vfio/vfio-common.h
+++ b/include/hw/vfio/vfio-common.h
@@ -62,7 +62,7 @@ typedef struct VFIOMigration {
struct VFIODevice *vbasedev;
VMChangeStateEntry *vm_state;
VFIORegion region;
-uint32_t device_state;
+uint32_t device_state_v1;
int vm_running;
Notifier migration_state;
uint64_t pending_bytes;
diff --git a/hw/vfio/common.c b/hw/vfio/common.c
index 7a35edb0e9..6f0157c848 100644
--- a/hw/vfio/common.c
+++ b/hw/vfio/common.c
@@ -355,8 +355,8 @@ static bool vfio_devices_all_dirty_tracking(VFIOContainer
*container)
return false;
}
-if ((vbasedev->pre_copy_dirty_page_tracking == ON_OFF_AUTO_OFF)
-&& (migration->device_state & VFIO_DEVICE_STATE_V1_RUNNING)) {
+if ((vbasedev->pre_copy_dirty_page_tracking == ON_OFF_AUTO_OFF) &&
+(migration->device_state_v1 & VFIO_DEVICE_STATE_V1_RUNNING)) {
return false;
}
}
@@ -385,7 +385,7 @@ static bool
vfio_devices_all_running_and_mig_active(VFIOContainer *container)
return false;
}
-if (migration->device_state & VFIO_DEVICE_STATE_V1_RUNNING) {
+if (migration->device_state_v1 & VFIO_DEVICE_STATE_V1_RUNNING) {
continue;
} else {
return false;
diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
index 977da64411..9df859f4d3 100644
--- a/hw/vfio/migration.c
+++ b/hw/vfio/migration.c
@@ -107,8 +107,8 @@ static int vfio_mig_rw(VFIODevice *vbasedev, __u8 *buf,
size_t count,
* an error is returned.
*/
-static int vfio_migration_set_state(VFIODevice *vbasedev, uint32_t mask,
-uint32_t value)
+static int vfio_migration_v1_set_state(VFIODevice *vbasedev, uint32_t mask,
+ uint32_t value)
{
VFIOMigration *migration = vbasedev->migration;
VFIORegion *region = >region;
@@ -145,8 +145,8 @@ static int vfio_migration_set_state(VFIODevice *vbasedev,
uint32_t mask,
return ret;
}
-migration->device_state = device_state;
-trace_vfio_migration_set_state(vbasedev->name, device_state);
+migration->device_state_v1 = device_state;
+trace_vfio_migration_v1_set_state(vbasedev->name, device_state);
return 0;
}
@@ -260,8 +260,8 @@ static int vfio_save_buffer(QEMUFile *f, VFIODevice *vbasedev, uint64_t *size)
return ret;
}
-static int vfio_load_buffer(QEMUFile *f, VFIODevice *vbasedev,
-uint64_t data_size)
+static int vfio_v1_load_buffer(QEMUFile *f, VFIODevice *vbasedev,
+ uint64_t data_size)
{
VFIORegion *region = >migration->region;
uint64_t data_offset = 0, size, report_size;
@@ -288,7 +288,7 @@ static int vfio_load_buffer(QEMUFile *f, VFIODevice
*vbasedev,
data_size = 0;
}
-trace_vfio_load_state_device_data(vbasedev->name, data_offset, size);
+trace_vfio_v1_load_state_device_data(vbasedev->name, data_offset,
size);
while (size) {
void *buf;
@@ -394,7 +394,7 @@ static int vfio_load_device_config_state(QEMUFile *f, void
*opaque)
return qemu_file_get_error(f);
}
-static void vfio_migration_cleanup(VFIODevice *vbasedev)
+static void vfio_migration_v1_cleanup(VFIODevice *vbasedev)
{
VFIOMigration *migration = vbasedev->migration;
@@ -405,13 +405,13 @@ static void vfio_migration_cleanup(VFIODevice *vbasedev)
/* -- */
-static int vfio_save_setup(QEMUFile *f, void *opaque)
+static int vfio_v1_save_setup(QEMUFile *f, void *opaque)
{
VFIODevice *vbasedev = opaque;
VFIOMigration *migration = vbasedev->migration;
int ret;
-trace_vfio_save_setup(vbasedev->name);
+trace_vfio_v1_save_setup(vbasedev->name);
qemu_put_be64(f, VFIO_MIG_FLAG_DEV_SETUP_STATE);
@@ -431,8 +431,8 @@ static int vfio_save_setup(QEMUFile *f, void *opaque)
}
}
-ret = vfio_migration_set_state(vbasedev, VFIO_DEVICE_STATE_MASK,
-
Re: [PATCH v5 08/14] vfio/migration: Move migration v1 logic to vfio_migration_init()
On 12/29/22 12:03, Avihai Horon wrote:
Move vfio_dev_get_region_info() logic from vfio_migration_probe() to
vfio_migration_init(). This logic is specific to v1 protocol and moving
it will make it easier to add the v2 protocol implementation later.
No functional changes intended.
Signed-off-by: Avihai Horon
Reviewed-by: Cédric Le Goater
Thanks,
C.
---
hw/vfio/migration.c | 30 +++---
hw/vfio/trace-events | 2 +-
2 files changed, 16 insertions(+), 16 deletions(-)
diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
index 552c2313b2..977da64411 100644
--- a/hw/vfio/migration.c
+++ b/hw/vfio/migration.c
@@ -788,14 +788,14 @@ static void vfio_migration_exit(VFIODevice *vbasedev)
vbasedev->migration = NULL;
}
-static int vfio_migration_init(VFIODevice *vbasedev,
- struct vfio_region_info *info)
+static int vfio_migration_init(VFIODevice *vbasedev)
{
int ret;
Object *obj;
VFIOMigration *migration;
char id[256] = "";
g_autofree char *path = NULL, *oid = NULL;
+struct vfio_region_info *info;
if (!vbasedev->ops->vfio_get_object) {
return -EINVAL;
@@ -806,6 +806,14 @@ static int vfio_migration_init(VFIODevice *vbasedev,
return -EINVAL;
}
+ret = vfio_get_dev_region_info(vbasedev,
+ VFIO_REGION_TYPE_MIGRATION_DEPRECATED,
+ VFIO_REGION_SUBTYPE_MIGRATION_DEPRECATED,
+ );
+if (ret) {
+return ret;
+}
+
vbasedev->migration = g_new0(VFIOMigration, 1);
vbasedev->migration->device_state = VFIO_DEVICE_STATE_V1_RUNNING;
vbasedev->migration->vm_running = runstate_is_running();
@@ -825,6 +833,8 @@ static int vfio_migration_init(VFIODevice *vbasedev,
goto err;
}
+g_free(info);
+
migration = vbasedev->migration;
migration->vbasedev = vbasedev;
@@ -847,6 +857,7 @@ static int vfio_migration_init(VFIODevice *vbasedev,
return 0;
err:
+g_free(info);
vfio_migration_exit(vbasedev);
return ret;
}
@@ -860,34 +871,23 @@ int64_t vfio_mig_bytes_transferred(void)
int vfio_migration_probe(VFIODevice *vbasedev, Error **errp)
{
-struct vfio_region_info *info = NULL;
int ret = -ENOTSUP;
if (!vbasedev->enable_migration) {
goto add_blocker;
}
-ret = vfio_get_dev_region_info(vbasedev,
- VFIO_REGION_TYPE_MIGRATION_DEPRECATED,
- VFIO_REGION_SUBTYPE_MIGRATION_DEPRECATED,
- );
-if (ret) {
-goto add_blocker;
-}
-
-ret = vfio_migration_init(vbasedev, info);
+ret = vfio_migration_init(vbasedev);
if (ret) {
goto add_blocker;
}
-trace_vfio_migration_probe(vbasedev->name, info->index);
-g_free(info);
+trace_vfio_migration_probe(vbasedev->name);
return 0;
add_blocker:
error_setg(>migration_blocker,
"VFIO device doesn't support migration");
-g_free(info);
ret = migrate_add_blocker(vbasedev->migration_blocker, errp);
if (ret < 0) {
diff --git a/hw/vfio/trace-events b/hw/vfio/trace-events
index 73dffe9e00..b259dcc644 100644
--- a/hw/vfio/trace-events
+++ b/hw/vfio/trace-events
@@ -148,7 +148,7 @@ vfio_display_edid_update(uint32_t prefx, uint32_t prefy)
"%ux%u"
vfio_display_edid_write_error(void) ""
# migration.c
-vfio_migration_probe(const char *name, uint32_t index) " (%s) Region %d"
+vfio_migration_probe(const char *name) " (%s)"
vfio_migration_set_state(const char *name, uint32_t state) " (%s) state %d"
vfio_vmstate_change(const char *name, int running, const char *reason, uint32_t
dev_state) " (%s) running %d reason %s device state %d"
vfio_migration_state_notifier(const char *name, const char *state) " (%s) state
%s"
Questions about how block devices use snapshots
Dear all, Qemu provides powerful snapshot capabilities for different file formats. But this is limited to the block backend being a file, and support is not good enough when it is a block device. When creating snapshots based on files, there is no need to specify the size of the snapshot image, which can grow dynamically as the virtual machine is used. But block devices are fixed in size at creation and cannot be dynamically grown at a later time. So is there any way to support snapshots when the block backend is a block device? Regards Zhiyong
Re: [PATCH 5/6] tests: add G_GNUC_PRINTF for various functions
On Thu, Dec 29, 2022 at 10:34:55AM +0100, Thomas Huth wrote:
> On 19/12/2022 14.02, Daniel P. Berrangé wrote:
> > Signed-off-by: Daniel P. Berrangé
> > ---
> > tests/qtest/ahci-test.c | 3 +++
> > tests/qtest/arm-cpu-features.c| 1 +
> > tests/qtest/erst-test.c | 2 +-
> > tests/qtest/ide-test.c| 3 ++-
> > tests/qtest/ivshmem-test.c| 4 ++--
> > tests/qtest/libqmp.c | 2 +-
> > tests/qtest/libqos/libqos-pc.h| 6 --
> > tests/qtest/libqos/libqos-spapr.h | 6 --
> > tests/qtest/libqos/libqos.h | 6 --
> > tests/qtest/libqos/virtio-9p.c| 1 +
> > tests/qtest/migration-helpers.h | 1 +
> > tests/qtest/rtas-test.c | 2 +-
> > tests/qtest/usb-hcd-uhci-test.c | 4 ++--
> > tests/unit/test-qmp-cmds.c| 13 +
> > 14 files changed, 36 insertions(+), 18 deletions(-)
> ...
> > diff --git a/tests/unit/test-qmp-cmds.c b/tests/unit/test-qmp-cmds.c
> > index 2373cd64cb..6d52b4e5d8 100644
> > --- a/tests/unit/test-qmp-cmds.c
> > +++ b/tests/unit/test-qmp-cmds.c
> > @@ -138,6 +138,7 @@ void qmp___org_qemu_x_command(__org_qemu_x_EnumList *a,
> > }
> > +G_GNUC_PRINTF(2, 3)
> > static QObject *do_qmp_dispatch(bool allow_oob, const char *template, ...)
> > {
> > va_list ap;
> > @@ -160,6 +161,7 @@ static QObject *do_qmp_dispatch(bool allow_oob, const
> > char *template, ...)
> > return ret;
> > }
> > +G_GNUC_PRINTF(3, 4)
> > static void do_qmp_dispatch_error(bool allow_oob, ErrorClass cls,
> > const char *template, ...)
> > {
> > @@ -269,7 +271,7 @@ static void test_dispatch_cmd_io(void)
> > static void test_dispatch_cmd_deprecated(void)
> > {
> > -const char *cmd = "{ 'execute': 'test-command-features1' }";
> > +#define cmd "{ 'execute': 'test-command-features1' }"
> > QDict *ret;
>
> That looks weird, why is this required?
This means that the compiler can see we're passing a constant string
into the API call a few lines lower. Without it, the compiler can't
see that 'cmd' doesn't contain any printf format specifiers, and thus
it would force us to use func('%s', cmd)... except that's not actually
possible since our crazy json printf formatter doesn't allow arbitrary
'%s', the '%s' can only occur at well defined points in the JSON doc.
Using a constant string via #define was the easiest way to give the
compiler visibility of the safety.
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 v5 06/14] migration/qemu-file: Add qemu_file_get_to_fd()
On 12/29/22 12:03, Avihai Horon wrote:
Add new function qemu_file_get_to_fd() that allows reading data from
QEMUFile and writing it straight into a given fd.
This will be used later in VFIO migration code.
Signed-off-by: Avihai Horon
Reviewed-by: Vladimir Sementsov-Ogievskiy
---
migration/qemu-file.h | 1 +
migration/qemu-file.c | 34 ++
2 files changed, 35 insertions(+)
diff --git a/migration/qemu-file.h b/migration/qemu-file.h
index fa13d04d78..9d0155a2a1 100644
--- a/migration/qemu-file.h
+++ b/migration/qemu-file.h
@@ -148,6 +148,7 @@ int qemu_file_shutdown(QEMUFile *f);
QEMUFile *qemu_file_get_return_path(QEMUFile *f);
void qemu_fflush(QEMUFile *f);
void qemu_file_set_blocking(QEMUFile *f, bool block);
+int qemu_file_get_to_fd(QEMUFile *f, int fd, size_t size);
void ram_control_before_iterate(QEMUFile *f, uint64_t flags);
void ram_control_after_iterate(QEMUFile *f, uint64_t flags);
diff --git a/migration/qemu-file.c b/migration/qemu-file.c
index 2d5f74ffc2..79303c9d34 100644
--- a/migration/qemu-file.c
+++ b/migration/qemu-file.c
@@ -940,3 +940,37 @@ QIOChannel *qemu_file_get_ioc(QEMUFile *file)
{
return file->ioc;
}
+
+/*
+ * Read size bytes from QEMUFile f and write them to fd.
+ */
+int qemu_file_get_to_fd(QEMUFile *f, int fd, size_t size)
+{
+while (size) {
+size_t pending = f->buf_size - f->buf_index;
+ssize_t rc;
+
+if (!pending) {
+rc = qemu_fill_buffer(f);
+if (rc < 0) {
+return rc;
+}
+if (rc == 0) {
+return -1;
Given the call stack, -1 would be interpreted as EPERM. May be EIO instead ?
C.
+}
+continue;
+}
+
+rc = write(fd, f->buf + f->buf_index, MIN(pending, size));
+if (rc < 0) {
+return rc;
+}
+if (rc == 0) {
+return -1;
+}
+f->buf_index += rc;
+size -= rc;
+}
+
+return 0;
+}
Re: [PATCH 00/20] hw: Remove implicit sysbus_mmio_map() from pflash APIs
On 6/1/23 18:51, Peter Maydell wrote: On Wed, 4 Jan 2023 at 22:04, Philippe Mathieu-Daudé wrote: Paving the road toward heterogeneous QEMU, the limitations of having a single machine sysbus become more apparent. The sysbus_mmio_map() API forces the caller to map a sysbus device to an address on the system bus (system bus here is the root MemoryRegion returned by get_system_memory() ). This is not practical when each core has its own address space and group of cores have access to a part of the peripherals. Experimenting with the PFLASH devices. Here the fix is quite easy, we split the pflash_cfi_register() -- which does the implicit sysbus mapping -- into an explicit qdev pflash_cfi_create() followed by the sysbus_mmio_map() call. pflash_cfi_register() is a legacy convenience function. If you don't like the sysbus_mmio_map() it does then you can create, configure, realize and map the device directly. This is what hw/arm/virt.c does, for instance (it wants to map the flash devices into either secure or non secure RAM). Good point, thanks!
Re: [PATCH v5 03/14] migration: Simplify migration_iteration_run()
On 12/29/22 12:03, Avihai Horon wrote:
From: Juan Quintela
Signed-off-by: Juan Quintela
Signed-off-by: Avihai Horon
Reviewed-by: Cédric Le Goater
Thanks,
C.
---
migration/migration.c | 25 +
1 file changed, 13 insertions(+), 12 deletions(-)
diff --git a/migration/migration.c b/migration/migration.c
index 9795d0ec5c..61b9ce0fe8 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -3758,23 +3758,24 @@ static MigIterateState
migration_iteration_run(MigrationState *s)
trace_migrate_pending(pending_size, s->threshold_size,
pend_pre, pend_compat, pend_post);
-if (pending_size && pending_size >= s->threshold_size) {
-/* Still a significant amount to transfer */
-if (!in_postcopy && pend_pre <= s->threshold_size &&
-qatomic_read(>start_postcopy)) {
-if (postcopy_start(s)) {
-error_report("%s: postcopy failed to start", __func__);
-}
-return MIG_ITERATE_SKIP;
-}
-/* Just another iteration step */
-qemu_savevm_state_iterate(s->to_dst_file, in_postcopy);
-} else {
+
+if (!pending_size || pending_size < s->threshold_size) {
trace_migration_thread_low_pending(pending_size);
migration_completion(s);
return MIG_ITERATE_BREAK;
}
+/* Still a significant amount to transfer */
+if (!in_postcopy && pend_pre <= s->threshold_size &&
+qatomic_read(>start_postcopy)) {
+if (postcopy_start(s)) {
+error_report("%s: postcopy failed to start", __func__);
+}
+return MIG_ITERATE_SKIP;
+}
+
+/* Just another iteration step */
+qemu_savevm_state_iterate(s->to_dst_file, in_postcopy);
return MIG_ITERATE_RESUME;
}
Re: [PATCH v5 10/14] vfio/migration: Implement VFIO migration protocol v2
Hello Avihai,
On 12/29/22 12:03, Avihai Horon wrote:
+static int vfio_save_setup(QEMUFile *f, void *opaque)
+{
+VFIODevice *vbasedev = opaque;
+VFIOMigration *migration = vbasedev->migration;
+uint64_t stop_copy_size;
+
+qemu_put_be64(f, VFIO_MIG_FLAG_DEV_SETUP_STATE);
+
+if (vfio_query_stop_copy_size(vbasedev, _copy_size)) {
+stop_copy_size = VFIO_MIG_DEFAULT_DATA_BUFFER_SIZE;
+}
+migration->data_buffer_size = MIN(VFIO_MIG_DEFAULT_DATA_BUFFER_SIZE,
+ stop_copy_size);
+migration->data_buffer = g_try_malloc0(migration->data_buffer_size);
+if (!migration->data_buffer) {
+error_report("%s: Failed to allocate migration data buffer",
+ vbasedev->name);
+return -ENOMEM;
+}
+
+trace_vfio_save_setup(vbasedev->name, migration->data_buffer_size);
+
+qemu_put_be64(f, VFIO_MIG_FLAG_END_OF_STATE);
+
+return qemu_file_get_error(f);
+}
+
This fails to compile with :
gcc version 12.2.1 20221121 (Red Hat 12.2.1-4) (GCC) complains with :
../include/qemu/osdep.h:315:22: error: ‘stop_copy_size’ may be used
uninitialized [-Werror=maybe-uninitialized]
315 | _a < _b ? _a : _b; \
| ^
../hw/vfio/migration.c:262:14: note: ‘stop_copy_size’ was declared here
262 | uint64_t stop_copy_size;
| ^~
cc1: all warnings being treated as errors
May be rework the code slightly to avoid the breakage :
+++ qemu.git/hw/vfio/migration.c
@@ -259,13 +259,11 @@ static int vfio_save_setup(QEMUFile *f,
{
VFIODevice *vbasedev = opaque;
VFIOMigration *migration = vbasedev->migration;
-uint64_t stop_copy_size;
+uint64_t stop_copy_size = VFIO_MIG_DEFAULT_DATA_BUFFER_SIZE;
qemu_put_be64(f, VFIO_MIG_FLAG_DEV_SETUP_STATE);
-if (vfio_query_stop_copy_size(vbasedev, _copy_size)) {
-stop_copy_size = VFIO_MIG_DEFAULT_DATA_BUFFER_SIZE;
-}
+vfio_query_stop_copy_size(vbasedev, _copy_size);
migration->data_buffer_size = MIN(VFIO_MIG_DEFAULT_DATA_BUFFER_SIZE,
stop_copy_size);
migration->data_buffer = g_try_malloc0(migration->data_buffer_size);
and report the error in vfio_query_stop_copy_size()
Thanks,
C.
