[PATCH 3/3] tests/qtest/readconfig: Test the docs/config/q35-*.cfg files
Test that we can successfully parse the docs/config/q35-emulated.cfg,
docs/config/q35-virtio-graphical.cfg and docs/config/q35-virtio-serial.cfg
config files (the "...-serial.cfg" file is a subset of the graphical
config file, so we skip that in quick mode).
These config files use two hard-coded image names which we have to
replace with unique temporary files to avoid race conditions in case
the tests are run in parallel. So after creating the temporary image
files, we also have to create a copy of the config file where we
replaced the hard-coded image names.
If KVM is not available, we also have to disable the "accel" lines.
Once everything is in place, we can start QEMU with the modified
config file and check that everything is available in QEMU.
Signed-off-by: Thomas Huth
---
tests/qtest/readconfig-test.c | 196 ++
1 file changed, 196 insertions(+)
diff --git a/tests/qtest/readconfig-test.c b/tests/qtest/readconfig-test.c
index 74526f3af2..760f974e63 100644
--- a/tests/qtest/readconfig-test.c
+++ b/tests/qtest/readconfig-test.c
@@ -197,6 +197,189 @@ static void test_docs_config_ich9(void)
qtest_quit(qts);
}
+#if defined(CONFIG_POSIX) && defined(CONFIG_SLIRP)
+
+static char *make_temp_img(const char *template, const char *format, int size)
+{
+GError *error = NULL;
+char *temp_name;
+int fd;
+
+/* Create a temporary image names */
+fd = g_file_open_tmp(template, &temp_name, &error);
+if (fd == -1) {
+fprintf(stderr, "unable to create file: %s\n", error->message);
+g_error_free(error);
+return NULL;
+}
+close(fd);
+
+if (!mkimg(temp_name, format, size)) {
+fprintf(stderr, "qemu-img failed to create %s\n", temp_name);
+g_free(temp_name);
+return NULL;
+}
+
+return temp_name;
+}
+
+struct device {
+const char *name;
+const char *type;
+};
+
+static void test_docs_q35(const char *input_file, struct device *devices)
+{
+QTestState *qts;
+QDict *resp;
+QObject *qobj;
+int ret, i;
+g_autofree char *cfg_file = NULL, *sedcmd = NULL;
+g_autofree char *hd_file = NULL, *cd_file = NULL;
+
+/* Check that all the devices are available in the QEMU binary */
+for (i = 0; devices[i].name; i++) {
+if (!qtest_has_device(devices[i].type)) {
+g_test_skip("one of the required devices is not available");
+return;
+}
+}
+
+hd_file = make_temp_img("qtest_disk_XX.qcow2", "qcow2", 1);
+cd_file = make_temp_img("qtest_cdrom_XX.iso", "raw", 1);
+if (!hd_file || !cd_file) {
+g_test_skip("could not create disk images");
+goto cleanup;
+}
+
+/* Create a temporary config file where we replace the disk image names */
+ret = g_file_open_tmp("q35-emulated-XX.cfg", &cfg_file, NULL);
+if (ret == -1) {
+g_test_skip("could not create temporary config file");
+goto cleanup;
+}
+close(ret);
+
+sedcmd = g_strdup_printf("sed -e 's,guest.qcow2,%s,' -e
's,install.iso,%s,'"
+ " %s %s > '%s'",
+ hd_file, cd_file,
+ !qtest_has_accel("kvm") ? "-e '/accel/d'" : "",
+ input_file, cfg_file);
+ret = system(sedcmd);
+if (ret) {
+g_test_skip("could not modify temporary config file");
+goto cleanup;
+}
+
+qts = qtest_initf("-machine none -nodefaults -readconfig %s", cfg_file);
+
+/* Check memory size */
+resp = qtest_qmp(qts, "{ 'execute': 'query-memdev' }");
+test_x86_memdev_resp(qdict_get(resp, "return"), "pc.ram", 1024);
+qobject_unref(resp);
+
+resp = qtest_qmp(qts, "{ 'execute': 'qom-list',"
+ " 'arguments': {'path': '/machine/peripheral' }}");
+qobj = qdict_get(resp, "return");
+
+/* Check that all the devices have been created */
+for (i = 0; devices[i].name; i++) {
+test_object_available(qobj, devices[i].name, devices[i].type);
+}
+
+qobject_unref(resp);
+
+qtest_quit(qts);
+
+cleanup:
+if (hd_file) {
+unlink(hd_file);
+}
+if (cd_file) {
+unlink(cd_file);
+}
+if (cfg_file) {
+unlink(cfg_file);
+}
+}
+
+static void test_docs_q35_emulated(void)
+{
+struct device devices[] = {
+{ "ich9-pcie-port-1", "ioh3420" },
+{ "ich9-pcie-port-2", "ioh3420" },
+{ "ich9-pcie-port-3", "ioh3420" },
+{ "ich9-pcie-port-4", "ioh3420" },
+{ "ich9-pci-bridge", "i82801b11-bridge" },
+{ "ich9-ehci-1", "ich9-usb-ehci1" },
+{ "ich9-ehci-2", "ich9-usb-ehci2" },
+{ "ich9-uhci-1", "ich9-usb-uhci1" },
+{ "ich9-uhci-2", "ich9-usb-uhci2" },
+{ "ich9-uhci-3", "ich9-usb-uhci3" },
+{ "ich9-uhci-4", "ich9-usb-uhci4" },
+{ "ich9-uhci-5", "ich9-usb-uhci5" },
+{ "ich9-uhci-6", "ich9-usb-uhci6" },
+{ "sata-disk", "i
[PATCH 0/3] Test the docs/config/q35-*.cfg config files
With some tweaking (e.g. by creating temporary image files), we can check whether the docs/config/q35-*.cfg files can be loaded by QEMU successfully, so we can avoid that these files bitrot and avoid that our config file parser gets regressions. Thomas Huth (3): tests/qtest/readconfig-test: Allow testing for arbitrary memory sizes tests/qtest: Move mkimg() and have_qemu_img() from libqos to libqtest tests/qtest/readconfig: Test the docs/config/q35-*.cfg files tests/qtest/libqos/libqos.h | 2 - tests/qtest/libqtest.h| 20 tests/qtest/libqos/libqos.c | 49 +--- tests/qtest/libqtest.c| 52 + tests/qtest/readconfig-test.c | 204 +- 5 files changed, 273 insertions(+), 54 deletions(-) -- 2.39.3
[PATCH 2/3] tests/qtest: Move mkimg() and have_qemu_img() from libqos to libqtest
These two functions can be useful for other qtests beside the
qos-test, too, so move them to libqtest instead.
Signed-off-by: Thomas Huth
---
tests/qtest/libqos/libqos.h | 2 --
tests/qtest/libqtest.h | 20 ++
tests/qtest/libqos/libqos.c | 49 +-
tests/qtest/libqtest.c | 52 +
4 files changed, 73 insertions(+), 50 deletions(-)
diff --git a/tests/qtest/libqos/libqos.h b/tests/qtest/libqos/libqos.h
index 12d05b2365..c04950e2b1 100644
--- a/tests/qtest/libqos/libqos.h
+++ b/tests/qtest/libqos/libqos.h
@@ -27,8 +27,6 @@ QOSState *qtest_boot(QOSOps *ops, const char *cmdline_fmt,
...)
G_GNUC_PRINTF(2, 3);
void qtest_common_shutdown(QOSState *qs);
void qtest_shutdown(QOSState *qs);
-bool have_qemu_img(void);
-void mkimg(const char *file, const char *fmt, unsigned size_mb);
void mkqcow2(const char *file, unsigned size_mb);
void migrate(QOSState *from, QOSState *to, const char *uri);
void prepare_blkdebug_script(const char *debug_fn, const char *event);
diff --git a/tests/qtest/libqtest.h b/tests/qtest/libqtest.h
index 913acc3d5c..3a71bc45fc 100644
--- a/tests/qtest/libqtest.h
+++ b/tests/qtest/libqtest.h
@@ -994,4 +994,24 @@ bool qtest_qom_get_bool(QTestState *s, const char *path,
const char *property);
*/
pid_t qtest_pid(QTestState *s);
+/**
+ * have_qemu_img:
+ *
+ * Returns: true if "qemu-img" is available.
+ */
+bool have_qemu_img(void);
+
+/**
+ * mkimg:
+ * @file: File name of the image that should be created
+ * @fmt: Format, e.g. "qcow2" or "raw"
+ * @size_mb: Size of the image in megabytes
+ *
+ * Create a disk image with qemu-img. Note that the QTEST_QEMU_IMG
+ * environment variable must point to the qemu-img file.
+ *
+ * Returns: true if the image has been created successfully.
+ */
+bool mkimg(const char *file, const char *fmt, unsigned size_mb);
+
#endif
diff --git a/tests/qtest/libqos/libqos.c b/tests/qtest/libqos/libqos.c
index 5ffda080ec..5c0fa1f7c5 100644
--- a/tests/qtest/libqos/libqos.c
+++ b/tests/qtest/libqos/libqos.c
@@ -137,56 +137,9 @@ void migrate(QOSState *from, QOSState *to, const char *uri)
migrate_allocator(&from->alloc, &to->alloc);
}
-bool have_qemu_img(void)
-{
-char *rpath;
-const char *path = getenv("QTEST_QEMU_IMG");
-if (!path) {
-return false;
-}
-
-rpath = realpath(path, NULL);
-if (!rpath) {
-return false;
-} else {
-free(rpath);
-return true;
-}
-}
-
-void mkimg(const char *file, const char *fmt, unsigned size_mb)
-{
-gchar *cli;
-bool ret;
-int rc;
-GError *err = NULL;
-char *qemu_img_path;
-gchar *out, *out2;
-char *qemu_img_abs_path;
-
-qemu_img_path = getenv("QTEST_QEMU_IMG");
-g_assert(qemu_img_path);
-qemu_img_abs_path = realpath(qemu_img_path, NULL);
-g_assert(qemu_img_abs_path);
-
-cli = g_strdup_printf("%s create -f %s %s %uM", qemu_img_abs_path,
- fmt, file, size_mb);
-ret = g_spawn_command_line_sync(cli, &out, &out2, &rc, &err);
-if (err || !g_spawn_check_exit_status(rc, &err)) {
-fprintf(stderr, "%s\n", err->message);
-g_error_free(err);
-}
-g_assert(ret && !err);
-
-g_free(out);
-g_free(out2);
-g_free(cli);
-free(qemu_img_abs_path);
-}
-
void mkqcow2(const char *file, unsigned size_mb)
{
-return mkimg(file, "qcow2", size_mb);
+g_assert_true(mkimg(file, "qcow2", size_mb));
}
void prepare_blkdebug_script(const char *debug_fn, const char *event)
diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
index 79152f0ec3..c22dfc30d3 100644
--- a/tests/qtest/libqtest.c
+++ b/tests/qtest/libqtest.c
@@ -1742,3 +1742,55 @@ bool qtest_qom_get_bool(QTestState *s, const char *path,
const char *property)
return b;
}
+
+bool have_qemu_img(void)
+{
+char *rpath;
+const char *path = getenv("QTEST_QEMU_IMG");
+if (!path) {
+return false;
+}
+
+rpath = realpath(path, NULL);
+if (!rpath) {
+return false;
+} else {
+free(rpath);
+return true;
+}
+}
+
+bool mkimg(const char *file, const char *fmt, unsigned size_mb)
+{
+gchar *cli;
+bool ret;
+int rc;
+GError *err = NULL;
+char *qemu_img_path;
+gchar *out, *out2;
+char *qemu_img_abs_path;
+
+qemu_img_path = getenv("QTEST_QEMU_IMG");
+if (!qemu_img_path) {
+return false;
+}
+qemu_img_abs_path = realpath(qemu_img_path, NULL);
+if (!qemu_img_abs_path) {
+return false;
+}
+
+cli = g_strdup_printf("%s create -f %s %s %uM", qemu_img_abs_path,
+ fmt, file, size_mb);
+ret = g_spawn_command_line_sync(cli, &out, &out2, &rc, &err);
+if (err || !g_spawn_check_exit_status(rc, &err)) {
+fprintf(stderr, "%s\n", err->message);
+g_error_free(err);
+}
+
+g_free(out);
+g_free(out2);
+g_free(cli);
+free(qemu_img_abs_path);
+
[PATCH 1/3] tests/qtest/readconfig-test: Allow testing for arbitrary memory sizes
Make test_x86_memdev_resp() more flexible by allowing arbitrary
memory sizes as parameter here.
Signed-off-by: Thomas Huth
---
tests/qtest/readconfig-test.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/tests/qtest/readconfig-test.c b/tests/qtest/readconfig-test.c
index ac7242451b..74526f3af2 100644
--- a/tests/qtest/readconfig-test.c
+++ b/tests/qtest/readconfig-test.c
@@ -48,7 +48,7 @@ static QTestState *qtest_init_with_config(const char *cfgdata)
return qts;
}
-static void test_x86_memdev_resp(QObject *res)
+static void test_x86_memdev_resp(QObject *res, const char *mem_id, int size)
{
Visitor *v;
g_autoptr(MemdevList) memdevs = NULL;
@@ -63,8 +63,8 @@ static void test_x86_memdev_resp(QObject *res)
g_assert(!memdevs->next);
memdev = memdevs->value;
-g_assert_cmpstr(memdev->id, ==, "ram");
-g_assert_cmpint(memdev->size, ==, 200 * MiB);
+g_assert_cmpstr(memdev->id, ==, mem_id);
+g_assert_cmpint(memdev->size, ==, size * MiB);
visit_free(v);
}
@@ -80,7 +80,7 @@ static void test_x86_memdev(void)
qts = qtest_init_with_config(cfgdata);
/* Test valid command */
resp = qtest_qmp(qts, "{ 'execute': 'query-memdev' }");
-test_x86_memdev_resp(qdict_get(resp, "return"));
+test_x86_memdev_resp(qdict_get(resp, "return"), "ram", 200);
qobject_unref(resp);
qtest_quit(qts);
--
2.39.3
[PATCH v2 1/1] vhost-vdpa: mute unaligned memory error report
With TPM CRM device, vhost-vdpa reports an error when it tries
to register a listener for a non aligned memory region:
qemu-system-x86_64: vhost_vdpa_listener_region_add received unaligned region
qemu-system-x86_64: vhost_vdpa_listener_region_del received unaligned region
This error can be confusing for the user whereas we only need to skip
the region (as it's already done after the error_report())
Rather than introducing a special case for TPM CRB memory section
to not display the message in this case, simply replace the
error_report() by a trace function (with more information, like the
memory region name).
Signed-off-by: Laurent Vivier
---
hw/virtio/trace-events | 2 ++
hw/virtio/vhost-vdpa.c | 8 ++--
2 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events
index 8f8d05cf9b01..9b0d643b9475 100644
--- a/hw/virtio/trace-events
+++ b/hw/virtio/trace-events
@@ -34,7 +34,9 @@ vhost_vdpa_dma_map(void *vdpa, int fd, uint32_t msg_type,
uint32_t asid, uint64_
vhost_vdpa_dma_unmap(void *vdpa, int fd, uint32_t msg_type, uint32_t asid,
uint64_t iova, uint64_t size, uint8_t type) "vdpa:%p fd: %d msg_type: %"PRIu32"
asid: %"PRIu32" iova: 0x%"PRIx64" size: 0x%"PRIx64" type: %"PRIu8
vhost_vdpa_listener_begin_batch(void *v, int fd, uint32_t msg_type, uint8_t
type) "vdpa:%p fd: %d msg_type: %"PRIu32" type: %"PRIu8
vhost_vdpa_listener_commit(void *v, int fd, uint32_t msg_type, uint8_t type)
"vdpa:%p fd: %d msg_type: %"PRIu32" type: %"PRIu8
+vhost_vdpa_listener_region_add_unaligned(void *v, const char *name, uint64_t
offset_as, uint64_t offset_page) "vdpa: %p region %s
offset_within_address_space %"PRIu64" offset_within_region %"PRIu64
vhost_vdpa_listener_region_add(void *vdpa, uint64_t iova, uint64_t llend, void
*vaddr, bool readonly) "vdpa: %p iova 0x%"PRIx64" llend 0x%"PRIx64" vaddr: %p
read-only: %d"
+vhost_vdpa_listener_region_del_unaligned(void *v, const char *name, uint64_t
offset_as, uint64_t offset_page) "vdpa: %p region %s
offset_within_address_space %"PRIu64" offset_within_region %"PRIu64
vhost_vdpa_listener_region_del(void *vdpa, uint64_t iova, uint64_t llend)
"vdpa: %p iova 0x%"PRIx64" llend 0x%"PRIx64
vhost_vdpa_add_status(void *dev, uint8_t status) "dev: %p status: 0x%"PRIx8
vhost_vdpa_init(void *dev, void *vdpa) "dev: %p vdpa: %p"
diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
index 3c575a9a6e9e..24d32f0d3728 100644
--- a/hw/virtio/vhost-vdpa.c
+++ b/hw/virtio/vhost-vdpa.c
@@ -323,7 +323,9 @@ static void vhost_vdpa_listener_region_add(MemoryListener
*listener,
if (unlikely((section->offset_within_address_space & ~TARGET_PAGE_MASK) !=
(section->offset_within_region & ~TARGET_PAGE_MASK))) {
-error_report("%s received unaligned region", __func__);
+trace_vhost_vdpa_listener_region_add_unaligned(v, section->mr->name,
+ section->offset_within_address_space &
~TARGET_PAGE_MASK,
+ section->offset_within_region & ~TARGET_PAGE_MASK);
return;
}
@@ -405,7 +407,9 @@ static void vhost_vdpa_listener_region_del(MemoryListener
*listener,
if (unlikely((section->offset_within_address_space & ~TARGET_PAGE_MASK) !=
(section->offset_within_region & ~TARGET_PAGE_MASK))) {
-error_report("%s received unaligned region", __func__);
+trace_vhost_vdpa_listener_region_del_unaligned(v, section->mr->name,
+ section->offset_within_address_space &
~TARGET_PAGE_MASK,
+ section->offset_within_region & ~TARGET_PAGE_MASK);
return;
}
--
2.41.0
[PATCH v2 0/1] vhost-vdpa: skip TPM CRB memory section
An error is reported for vhost-vdpa case: qemu-kvm: vhost_vdpa_listener_region_add received unaligned region Marc-André has proposed a fix to this problem by skipping the memory region owned by the TPM CRB but it seems more generic to skip not aligned memory. v1 of this series proposed to set the RAM_PROTECTED flag for the TPM CRB memory region. v2: - do not introduce special case for TPM CRB - do not set RAM_PROTECTED flag for TPM CRB - remove error_report() and replace it with a trace For the previous discussions, see https://lists.nongnu.org/archive/html/qemu-devel/2022-11/msg03670.html and from Eric for VFIO: https://lore.kernel.org/all/20220506132510.1847942-1-eric.au...@redhat.com/ https://lore.kernel.org/all/20220524091405.416256-1-eric.au...@redhat.com/ Bug: https://bugzilla.redhat.com/show_bug.cgi?id=2141965 Thanks, Laurent Laurent Vivier (1): vhost-vdpa: mute unaligned memory error report hw/virtio/trace-events | 2 ++ hw/virtio/vhost-vdpa.c | 8 ++-- 2 files changed, 8 insertions(+), 2 deletions(-) -- 2.41.0
Re: [PATCH v2 1/1] vhost-vdpa: mute unaligned memory error report
On 04.07.23 09:19, Laurent Vivier wrote:
With TPM CRM device, vhost-vdpa reports an error when it tries
to register a listener for a non aligned memory region:
qemu-system-x86_64: vhost_vdpa_listener_region_add received unaligned region
qemu-system-x86_64: vhost_vdpa_listener_region_del received unaligned region
This error can be confusing for the user whereas we only need to skip
the region (as it's already done after the error_report())
Rather than introducing a special case for TPM CRB memory section
to not display the message in this case, simply replace the
error_report() by a trace function (with more information, like the
memory region name).
Signed-off-by: Laurent Vivier
---
hw/virtio/trace-events | 2 ++
hw/virtio/vhost-vdpa.c | 8 ++--
2 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events
index 8f8d05cf9b01..9b0d643b9475 100644
--- a/hw/virtio/trace-events
+++ b/hw/virtio/trace-events
@@ -34,7 +34,9 @@ vhost_vdpa_dma_map(void *vdpa, int fd, uint32_t msg_type,
uint32_t asid, uint64_
vhost_vdpa_dma_unmap(void *vdpa, int fd, uint32_t msg_type, uint32_t asid, uint64_t iova, uint64_t size, uint8_t type)
"vdpa:%p fd: %d msg_type: %"PRIu32" asid: %"PRIu32" iova: 0x%"PRIx64" size:
0x%"PRIx64" type: %"PRIu8
vhost_vdpa_listener_begin_batch(void *v, int fd, uint32_t msg_type, uint8_t type) "vdpa:%p
fd: %d msg_type: %"PRIu32" type: %"PRIu8
vhost_vdpa_listener_commit(void *v, int fd, uint32_t msg_type, uint8_t type) "vdpa:%p fd:
%d msg_type: %"PRIu32" type: %"PRIu8
+vhost_vdpa_listener_region_add_unaligned(void *v, const char *name, uint64_t offset_as, uint64_t
offset_page) "vdpa: %p region %s offset_within_address_space %"PRIu64"
offset_within_region %"PRIu64
vhost_vdpa_listener_region_add(void *vdpa, uint64_t iova, uint64_t llend, void *vaddr, bool readonly)
"vdpa: %p iova 0x%"PRIx64" llend 0x%"PRIx64" vaddr: %p read-only: %d"
+vhost_vdpa_listener_region_del_unaligned(void *v, const char *name, uint64_t offset_as, uint64_t
offset_page) "vdpa: %p region %s offset_within_address_space %"PRIu64"
offset_within_region %"PRIu64
vhost_vdpa_listener_region_del(void *vdpa, uint64_t iova, uint64_t llend) "vdpa: %p iova
0x%"PRIx64" llend 0x%"PRIx64
vhost_vdpa_add_status(void *dev, uint8_t status) "dev: %p status: 0x%"PRIx8
vhost_vdpa_init(void *dev, void *vdpa) "dev: %p vdpa: %p"
diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
index 3c575a9a6e9e..24d32f0d3728 100644
--- a/hw/virtio/vhost-vdpa.c
+++ b/hw/virtio/vhost-vdpa.c
@@ -323,7 +323,9 @@ static void vhost_vdpa_listener_region_add(MemoryListener
*listener,
if (unlikely((section->offset_within_address_space & ~TARGET_PAGE_MASK) !=
(section->offset_within_region & ~TARGET_PAGE_MASK))) {
-error_report("%s received unaligned region", __func__);
+trace_vhost_vdpa_listener_region_add_unaligned(v, section->mr->name,
+ section->offset_within_address_space &
~TARGET_PAGE_MASK,
+ section->offset_within_region & ~TARGET_PAGE_MASK);
return;
}
@@ -405,7 +407,9 @@ static void vhost_vdpa_listener_region_del(MemoryListener *listener,
if (unlikely((section->offset_within_address_space & ~TARGET_PAGE_MASK) !=
(section->offset_within_region & ~TARGET_PAGE_MASK))) {
-error_report("%s received unaligned region", __func__);
+trace_vhost_vdpa_listener_region_del_unaligned(v, section->mr->name,
+ section->offset_within_address_space &
~TARGET_PAGE_MASK,
+ section->offset_within_region & ~TARGET_PAGE_MASK);
return;
}
Reviewed-by: David Hildenbrand
Do we also want to touch the vfio side in vfio_listener_valid_section(),
or why is that one unaffected?
--
Cheers,
David / dhildenb
Re: [PATCH 01/12] linux-user: elfload: Add more initial s390x PSW bits
On 03.07.23 17:50, Ilya Leoshkevich wrote:
Make the PSW look more similar to the real s390x userspace PSW.
Except for being there, the newly added bits should not affect the
userspace code execution.
What's the purpose of this then? Required for follow-up patches?
Signed-off-by: Ilya Leoshkevich
---
linux-user/elfload.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index 6900974c373..7935110bff4 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -1635,7 +1635,9 @@ const char *elf_hwcap_str(uint32_t bit)
static inline void init_thread(struct target_pt_regs *regs, struct image_info
*infop)
{
regs->psw.addr = infop->entry;
-regs->psw.mask = PSW_MASK_64 | PSW_MASK_32;
+regs->psw.mask = PSW_MASK_DAT | PSW_MASK_IO | PSW_MASK_EXT | \
+ PSW_MASK_MCHECK | PSW_MASK_PSTATE | PSW_MASK_64 | \
+ PSW_MASK_32;
regs->gprs[15] = infop->start_stack;
}
--
Cheers,
David / dhildenb
Re: [PATCH 02/12] target/s390x: Fix EPSW CC reporting
On 03.07.23 17:50, Ilya Leoshkevich wrote:
EPSW should explicitly calculate and insert CC, like IPM does.
Fixes: e30a9d3fea58 ("target-s390: Implement EPSW")
Cc: qemu-sta...@nongnu.org
Signed-off-by: Ilya Leoshkevich
---
Reviewed-by: David Hildenbrand
--
Cheers,
David / dhildenb
Re: [PATCH 03/12] target/s390x: Fix MDEB and MDEBR
On 03.07.23 17:50, Ilya Leoshkevich wrote:
These instructions multiply 32 bits by 32 bits, not 32 bits by 64 bits.
Fixes: 83b00736f3d8 ("target-s390: Convert FP MULTIPLY")
Cc: qemu-sta...@nongnu.org
Signed-off-by: Ilya Leoshkevich
---
Reviewed-by: David Hildenbrand
--
Cheers,
David / dhildenb
Re: [PATCH 01/12] linux-user: elfload: Add more initial s390x PSW bits
On Tue, 2023-07-04 at 09:32 +0200, David Hildenbrand wrote: > On 03.07.23 17:50, Ilya Leoshkevich wrote: > > Make the PSW look more similar to the real s390x userspace PSW. > > Except for being there, the newly added bits should not affect the > > userspace code execution. > > What's the purpose of this then? Required for follow-up patches? That's required for the EPSW test. I could, of course, mask out the bits that are not emulated in the test, but I thought it was better to make the emulation closer to reality, if only for cosmetic purposes. [...]
Re: [PATCH 04/12] target/s390x: Fix MVCRL with a large value in R0
On 03.07.23 17:50, Ilya Leoshkevich wrote:
Using a large R0 causes an assertion error:
qemu-s390x: target/s390x/tcg/mem_helper.c:183: access_prepare_nf: Assertion `size > 0
&& size <= 4096' failed.
Even though PoP explicitly advises against using more than 8 bits for the
size, an emulator crash is never a good thing.
Fix by truncating the size to 8 bits.
Fixes: ea0a1053e276 ("s390x/tcg: Implement Miscellaneous-Instruction-Extensions
Facility 3 for the s390x")
Cc: qemu-sta...@nongnu.org
Signed-off-by: Ilya Leoshkevich
Reviewed-by: David Hildenbrand
--
Cheers,
David / dhildenb
Re: [PATCH 05/12] target/s390x: Fix LRA overwriting the top 32 bits on DAT error
On 03.07.23 17:50, Ilya Leoshkevich wrote:
When a DAT error occurs, LRA is supposed to write the error information
to the bottom 32 bits of R1, and leave the top 32 bits of R1 alone.
Fix by passing the original value of R1 into helper and copying the
top 32 bits to the return value.
Fixes: d8fe4a9c284f ("target-s390: Convert LRA")
Cc: qemu-sta...@nongnu.org
Signed-off-by: Ilya Leoshkevich
---
target/s390x/helper.h | 2 +-
target/s390x/tcg/mem_helper.c | 4 ++--
target/s390x/tcg/translate.c | 2 +-
3 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/target/s390x/helper.h b/target/s390x/helper.h
index 6bc01df73d7..05102578fc9 100644
--- a/target/s390x/helper.h
+++ b/target/s390x/helper.h
@@ -355,7 +355,7 @@ DEF_HELPER_FLAGS_4(idte, TCG_CALL_NO_RWG, void, env, i64,
i64, i32)
DEF_HELPER_FLAGS_4(ipte, TCG_CALL_NO_RWG, void, env, i64, i64, i32)
DEF_HELPER_FLAGS_1(ptlb, TCG_CALL_NO_RWG, void, env)
DEF_HELPER_FLAGS_1(purge, TCG_CALL_NO_RWG, void, env)
-DEF_HELPER_2(lra, i64, env, i64)
+DEF_HELPER_3(lra, i64, env, i64, i64)
DEF_HELPER_1(per_check_exception, void, env)
DEF_HELPER_FLAGS_3(per_branch, TCG_CALL_NO_RWG, void, env, i64, i64)
DEF_HELPER_FLAGS_2(per_ifetch, TCG_CALL_NO_RWG, void, env, i64)
diff --git a/target/s390x/tcg/mem_helper.c b/target/s390x/tcg/mem_helper.c
index 84ad85212c9..94d93d7ea78 100644
--- a/target/s390x/tcg/mem_helper.c
+++ b/target/s390x/tcg/mem_helper.c
@@ -2356,7 +2356,7 @@ void HELPER(purge)(CPUS390XState *env)
}
/* load real address */
-uint64_t HELPER(lra)(CPUS390XState *env, uint64_t addr)
+uint64_t HELPER(lra)(CPUS390XState *env, uint64_t r1, uint64_t addr)
{
uint64_t asc = env->psw.mask & PSW_MASK_ASC;
uint64_t ret, tec;
@@ -2370,7 +2370,7 @@ uint64_t HELPER(lra)(CPUS390XState *env, uint64_t addr)
exc = mmu_translate(env, addr, MMU_S390_LRA, asc, &ret, &flags, &tec);
if (exc) {
cc = 3;
-ret = exc | 0x8000;
+ret = (r1 & 0x) | exc | 0x8000;
ull missing for large constant?
} else {
cc = 0;
ret |= addr & ~TARGET_PAGE_MASK;
diff --git a/target/s390x/tcg/translate.c b/target/s390x/tcg/translate.c
index 0cef6efbef4..a6079ab7b4f 100644
--- a/target/s390x/tcg/translate.c
+++ b/target/s390x/tcg/translate.c
@@ -2932,7 +2932,7 @@ static DisasJumpType op_lctlg(DisasContext *s, DisasOps
*o)
static DisasJumpType op_lra(DisasContext *s, DisasOps *o)
{
-gen_helper_lra(o->out, cpu_env, o->in2);
+gen_helper_lra(o->out, cpu_env, o->out, o->in2);
set_cc_static(s);
return DISAS_NEXT;
}
Can't we use something like in1_r1 + wout_r1_32 instead ? *maybe* cleaner :)
--
Cheers,
David / dhildenb
Re: [PATCH 01/12] linux-user: elfload: Add more initial s390x PSW bits
On 04.07.23 09:40, Ilya Leoshkevich wrote: On Tue, 2023-07-04 at 09:32 +0200, David Hildenbrand wrote: On 03.07.23 17:50, Ilya Leoshkevich wrote: Make the PSW look more similar to the real s390x userspace PSW. Except for being there, the newly added bits should not affect the userspace code execution. What's the purpose of this then? Required for follow-up patches? That's required for the EPSW test. I could, of course, mask out the bits that are not emulated in the test, but I thought it was better to make the emulation closer to reality, if only for cosmetic purposes. Thanks Reviewed-by: David Hildenbrand -- Cheers, David / dhildenb
Re: [PATCH v4] target: ppc: Use MSR_HVB bit to get the target endianness for memory dump
Thanks for fixing this Narayana,
Narayana Murty N writes:
> Currently on PPC64 qemu always dumps the guest memory in
> Big Endian (BE) format even though the guest running in Little Endian
> (LE) mode. So crash tool fails to load the dump as illustrated below:
>
> Log :
> $ virsh dump DOMAIN --memory-only dump.file
>
> Domain 'DOMAIN' dumped to dump.file
>
> $ crash vmlinux dump.file
>
>
> crash 8.0.2-1.el9
>
> WARNING: endian mismatch:
> crash utility: little-endian
> dump.file: big-endian
>
> WARNING: machine type mismatch:
> crash utility: PPC64
> dump.file: (unknown)
>
> crash: dump.file: not a supported file format
>
>
> This happens because cpu_get_dump_info() passes cpu->env->has_hv_mode
> to function ppc_interrupts_little_endian(), the cpu->env->has_hv_mode
> always set for powerNV even though the guest is not running in hv mode.
> The hv mode should be taken from msr_mask MSR_HVB bit
> (cpu->env.msr_mask & MSR_HVB). This patch fixes the issue by passing
> MSR_HVB value to ppc_interrupts_little_endian() in order to determine
> the guest endianness.
>
> The crash tool also expects guest kernel endianness should match the
> endianness of the dump.
>
> The patch was tested on POWER9 box booted with Linux as host in
> following cases:
>
> Host-Endianess Qemu-Target-MachineQemu-Generated-Guest
> Memory-Dump-Format
> BE powernv(OPAL/PowerNV) LE
> BE powernv(OPAL/PowerNV) BE
> LE powernv(OPAL/PowerNV) LE
> LE powernv(OPAL/PowerNV) BE
> LE pseries(OPAL/PowerNV/pSeries) KVMHV LE
> LE pseries TCG LE
>
> Fixes: 5609400a4228 ("target/ppc: Set the correct endianness for powernv
> memory
> dumps")
> Signed-off-by: Narayana Murty N
Reviewed-by: Vaibhav Jain
--
Cheers
~ Vaibhav
Re: [PATCH v2 1/1] vhost-vdpa: mute unaligned memory error report
On 7/4/23 09:25, David Hildenbrand wrote:
On 04.07.23 09:19, Laurent Vivier wrote:
With TPM CRM device, vhost-vdpa reports an error when it tries
to register a listener for a non aligned memory region:
qemu-system-x86_64: vhost_vdpa_listener_region_add received unaligned region
qemu-system-x86_64: vhost_vdpa_listener_region_del received unaligned region
This error can be confusing for the user whereas we only need to skip
the region (as it's already done after the error_report())
Rather than introducing a special case for TPM CRB memory section
to not display the message in this case, simply replace the
error_report() by a trace function (with more information, like the
memory region name).
Signed-off-by: Laurent Vivier
---
hw/virtio/trace-events | 2 ++
hw/virtio/vhost-vdpa.c | 8 ++--
2 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events
index 8f8d05cf9b01..9b0d643b9475 100644
--- a/hw/virtio/trace-events
+++ b/hw/virtio/trace-events
@@ -34,7 +34,9 @@ vhost_vdpa_dma_map(void *vdpa, int fd, uint32_t msg_type, uint32_t
asid, uint64_
vhost_vdpa_dma_unmap(void *vdpa, int fd, uint32_t msg_type, uint32_t asid, uint64_t
iova, uint64_t size, uint8_t type) "vdpa:%p fd: %d msg_type: %"PRIu32" asid: %"PRIu32"
iova: 0x%"PRIx64" size: 0x%"PRIx64" type: %"PRIu8
vhost_vdpa_listener_begin_batch(void *v, int fd, uint32_t msg_type, uint8_t type)
"vdpa:%p fd: %d msg_type: %"PRIu32" type: %"PRIu8
vhost_vdpa_listener_commit(void *v, int fd, uint32_t msg_type, uint8_t type) "vdpa:%p
fd: %d msg_type: %"PRIu32" type: %"PRIu8
+vhost_vdpa_listener_region_add_unaligned(void *v, const char *name, uint64_t offset_as,
uint64_t offset_page) "vdpa: %p region %s offset_within_address_space %"PRIu64"
offset_within_region %"PRIu64
vhost_vdpa_listener_region_add(void *vdpa, uint64_t iova, uint64_t llend, void *vaddr,
bool readonly) "vdpa: %p iova 0x%"PRIx64" llend 0x%"PRIx64" vaddr: %p read-only: %d"
+vhost_vdpa_listener_region_del_unaligned(void *v, const char *name, uint64_t offset_as,
uint64_t offset_page) "vdpa: %p region %s offset_within_address_space %"PRIu64"
offset_within_region %"PRIu64
vhost_vdpa_listener_region_del(void *vdpa, uint64_t iova, uint64_t llend) "vdpa: %p
iova 0x%"PRIx64" llend 0x%"PRIx64
vhost_vdpa_add_status(void *dev, uint8_t status) "dev: %p status: 0x%"PRIx8
vhost_vdpa_init(void *dev, void *vdpa) "dev: %p vdpa: %p"
diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
index 3c575a9a6e9e..24d32f0d3728 100644
--- a/hw/virtio/vhost-vdpa.c
+++ b/hw/virtio/vhost-vdpa.c
@@ -323,7 +323,9 @@ static void vhost_vdpa_listener_region_add(MemoryListener
*listener,
if (unlikely((section->offset_within_address_space & ~TARGET_PAGE_MASK) !=
(section->offset_within_region & ~TARGET_PAGE_MASK))) {
- error_report("%s received unaligned region", __func__);
+ trace_vhost_vdpa_listener_region_add_unaligned(v, section->mr->name,
+ section->offset_within_address_space &
~TARGET_PAGE_MASK,
+ section->offset_within_region & ~TARGET_PAGE_MASK);
return;
}
@@ -405,7 +407,9 @@ static void vhost_vdpa_listener_region_del(MemoryListener
*listener,
if (unlikely((section->offset_within_address_space & ~TARGET_PAGE_MASK) !=
(section->offset_within_region & ~TARGET_PAGE_MASK))) {
- error_report("%s received unaligned region", __func__);
+ trace_vhost_vdpa_listener_region_del_unaligned(v, section->mr->name,
+ section->offset_within_address_space &
~TARGET_PAGE_MASK,
+ section->offset_within_region & ~TARGET_PAGE_MASK);
return;
}
Reviewed-by: David Hildenbrand
Do we also want to touch the vfio side in vfio_listener_valid_section(), or why is that
one unaffected?
I don't know if we can apply the same solution for VFIO.
I don't know if the error message is relevant or if we can keep only the trace and remove
the error_report() for all the cases (in this case vfio_known_safe_misalignment() becomes
useless).
Thanks,
Laurent
Re: [PATCH 06/12] target/s390x: Fix LRA when DAT is off
On 03.07.23 17:50, Ilya Leoshkevich wrote:
LRA should perform DAT regardless of whether it's on or off.
Disable DAT check for MMU_S390_LRA.
Fixes: defb0e3157af ("s390x: Implement opcode helpers")
Cc: qemu-sta...@nongnu.org
Signed-off-by: Ilya Leoshkevich
---
target/s390x/mmu_helper.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/target/s390x/mmu_helper.c b/target/s390x/mmu_helper.c
index b04b57c2356..fbb2f1b4d48 100644
--- a/target/s390x/mmu_helper.c
+++ b/target/s390x/mmu_helper.c
@@ -417,7 +417,7 @@ int mmu_translate(CPUS390XState *env, target_ulong vaddr,
int rw, uint64_t asc,
vaddr &= TARGET_PAGE_MASK;
-if (!(env->psw.mask & PSW_MASK_DAT)) {
+if (rw != MMU_S390_LRA && !(env->psw.mask & PSW_MASK_DAT)) {
*raddr = vaddr;
goto nodat;
}
Interesting
Reviewed-by: David Hildenbrand
--
Cheers,
David / dhildenb
Re: [PATCH 07/12] target/s390x: Fix relative long instructions with large offsets
On 03.07.23 17:50, Ilya Leoshkevich wrote:
The expression "imm * 2" in gen_ri2() can wrap around if imm is large
enough.
Fix by casting imm to int64_t, like it's done in disas_jdest().
Fixes: e8ecdfeb30f0 ("Fix EXECUTE of relative branches")
Signed-off-by: Ilya Leoshkevich
---
target/s390x/tcg/translate.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/target/s390x/tcg/translate.c b/target/s390x/tcg/translate.c
index a6079ab7b4f..6661b27efa4 100644
--- a/target/s390x/tcg/translate.c
+++ b/target/s390x/tcg/translate.c
@@ -5794,7 +5794,7 @@ static TCGv gen_ri2(DisasContext *s)
disas_jdest(s, i2, is_imm, imm, ri2);
if (is_imm) {
-ri2 = tcg_constant_i64(s->base.pc_next + imm * 2);
+ri2 = tcg_constant_i64(s->base.pc_next + (int64_t)imm * 2);
}
return ri2;
Reviewed-by: David Hildenbrand
--
Cheers,
David / dhildenb
Re: [PATCH 08/12] tests/tcg/s390x: Test EPSW
On 03.07.23 17:50, Ilya Leoshkevich wrote:
Add a small test to prevent regressions.
Signed-off-by: Ilya Leoshkevich
---
tests/tcg/s390x/Makefile.target | 1 +
tests/tcg/s390x/epsw.c | 23 +++
2 files changed, 24 insertions(+)
create mode 100644 tests/tcg/s390x/epsw.c
diff --git a/tests/tcg/s390x/Makefile.target b/tests/tcg/s390x/Makefile.target
index 85abfbb98c0..2ef22c88d95 100644
--- a/tests/tcg/s390x/Makefile.target
+++ b/tests/tcg/s390x/Makefile.target
@@ -36,6 +36,7 @@ TESTS+=rxsbg
TESTS+=ex-relative-long
TESTS+=ex-branch
TESTS+=mxdb
+TESTS+=epsw
cdsg: CFLAGS+=-pthread
cdsg: LDFLAGS+=-pthread
diff --git a/tests/tcg/s390x/epsw.c b/tests/tcg/s390x/epsw.c
new file mode 100644
index 000..affb1a5e3a1
--- /dev/null
+++ b/tests/tcg/s390x/epsw.c
@@ -0,0 +1,23 @@
+/*
+ * Test the EPSW instruction.
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+#include
+#include
+
+int main(void)
+{
+unsigned long r1 = 0x1234567887654321UL, r2 = 0x8765432112345678UL;
+
+asm("cr %[r1],%[r2]\n" /* cc = 1 */
+"epsw %[r1],%[r2]"
+: [r1] "+r" (r1), [r2] "+r" (r2) : : "cc");
+
+/* Do not check the R and RI bits. */
+r1 &= ~0x4008UL;
+assert(r1 == 0x1234567807051001UL);
+assert(r2 == 0x876543218000UL);
+
+return EXIT_SUCCESS;
+}
Reviewed-by: David Hildenbrand
--
Cheers,
David / dhildenb
Re: [PATCH 09/12] tests/tcg/s390x: Test LARL with a large offset
On 03.07.23 17:50, Ilya Leoshkevich wrote:
Add a small test to prevent regressions.
Signed-off-by: Ilya Leoshkevich
---
tests/tcg/s390x/Makefile.target | 1 +
tests/tcg/s390x/larl.c | 17 +
2 files changed, 18 insertions(+)
create mode 100644 tests/tcg/s390x/larl.c
diff --git a/tests/tcg/s390x/Makefile.target b/tests/tcg/s390x/Makefile.target
index 2ef22c88d95..dbf64c991e9 100644
--- a/tests/tcg/s390x/Makefile.target
+++ b/tests/tcg/s390x/Makefile.target
@@ -37,6 +37,7 @@ TESTS+=ex-relative-long
TESTS+=ex-branch
TESTS+=mxdb
TESTS+=epsw
+TESTS+=larl
cdsg: CFLAGS+=-pthread
cdsg: LDFLAGS+=-pthread
diff --git a/tests/tcg/s390x/larl.c b/tests/tcg/s390x/larl.c
new file mode 100644
index 000..b9ced99a023
--- /dev/null
+++ b/tests/tcg/s390x/larl.c
@@ -0,0 +1,17 @@
+/*
+ * Test the LARL instruction.
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+#include
+
+int main(void)
+{
+long algfi = (long)main;
+long larl;
+
+asm("algfi %[r],0xd000" : [r] "+r" (algfi) : : "cc");
+asm("larl %[r],main+0xd000" : [r] "=r" (larl));
Not sure if worth combining both statements.
+
+return algfi == larl ? EXIT_SUCCESS : EXIT_FAILURE;
+}
Acked-by: David Hildenbrand
--
Cheers,
David / dhildenb
Re: [PATCH 11/12] tests/tcg/s390x: Test MDEB and MDEBR
On 03.07.23 17:50, Ilya Leoshkevich wrote:
Add a small test to prevent regressions.
Signed-off-by: Ilya Leoshkevich
---
tests/tcg/s390x/Makefile.target | 1 +
tests/tcg/s390x/mdeb.c | 30 ++
2 files changed, 31 insertions(+)
create mode 100644 tests/tcg/s390x/mdeb.c
diff --git a/tests/tcg/s390x/Makefile.target b/tests/tcg/s390x/Makefile.target
index dbf64c991e9..19fbbc6e531 100644
--- a/tests/tcg/s390x/Makefile.target
+++ b/tests/tcg/s390x/Makefile.target
@@ -38,6 +38,7 @@ TESTS+=ex-branch
TESTS+=mxdb
TESTS+=epsw
TESTS+=larl
+TESTS+=mdeb
cdsg: CFLAGS+=-pthread
cdsg: LDFLAGS+=-pthread
diff --git a/tests/tcg/s390x/mdeb.c b/tests/tcg/s390x/mdeb.c
new file mode 100644
index 000..4897d28069f
--- /dev/null
+++ b/tests/tcg/s390x/mdeb.c
@@ -0,0 +1,30 @@
+/*
+ * Test the MDEB and MDEBR instructions.
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+#include
+#include
+
+int main(void)
+{
+union {
+float f[2];
+double d;
+} a;
+float b;
+
+a.f[0] = 1.2345;
+a.f[1] = 999;
+b = 6.789;
+asm("mdeb %[a],%[b]" : [a] "+f" (a.d) : [b] "R" (b));
+assert(a.d > 8.38 && a.d < 8.39);
+
+a.f[0] = 1.2345;
+a.f[1] = 999;
+b = 6.789;
+asm("mdebr %[a],%[b]" : [a] "+f" (a.d) : [b] "f" (b));
+assert(a.d > 8.38 && a.d < 8.39);
+
+return EXIT_SUCCESS;
+}
Acked-by: David Hildenbrand
--
Cheers,
David / dhildenb
Re: [PATCH 09/12] tests/tcg/s390x: Test LARL with a large offset
On Tue, 2023-07-04 at 09:56 +0200, David Hildenbrand wrote:
> On 03.07.23 17:50, Ilya Leoshkevich wrote:
> > Add a small test to prevent regressions.
> >
> > Signed-off-by: Ilya Leoshkevich
> > ---
> > tests/tcg/s390x/Makefile.target | 1 +
> > tests/tcg/s390x/larl.c | 17 +
> > 2 files changed, 18 insertions(+)
> > create mode 100644 tests/tcg/s390x/larl.c
> >
> > diff --git a/tests/tcg/s390x/Makefile.target
> > b/tests/tcg/s390x/Makefile.target
> > index 2ef22c88d95..dbf64c991e9 100644
> > --- a/tests/tcg/s390x/Makefile.target
> > +++ b/tests/tcg/s390x/Makefile.target
> > @@ -37,6 +37,7 @@ TESTS+=ex-relative-long
> > TESTS+=ex-branch
> > TESTS+=mxdb
> > TESTS+=epsw
> > +TESTS+=larl
> >
> > cdsg: CFLAGS+=-pthread
> > cdsg: LDFLAGS+=-pthread
> > diff --git a/tests/tcg/s390x/larl.c b/tests/tcg/s390x/larl.c
> > new file mode 100644
> > index 000..b9ced99a023
> > --- /dev/null
> > +++ b/tests/tcg/s390x/larl.c
> > @@ -0,0 +1,17 @@
> > +/*
> > + * Test the LARL instruction.
> > + *
> > + * SPDX-License-Identifier: GPL-2.0-or-later
> > + */
> > +#include
> > +
> > +int main(void)
> > +{
> > + long algfi = (long)main;
> > + long larl;
> > +
> > + asm("algfi %[r],0xd000" : [r] "+r" (algfi) : : "cc");
> > + asm("larl %[r],main+0xd000" : [r] "=r" (larl));
>
> Not sure if worth combining both statements.
I thought it would be easier on the eyes; this way one immediately sees
that they are independent.
And maybe I should've added a comment about this, but the reason I used
algfi instead of C addition was that I feared that the compiler might
generate larl, making the test useless.
>
> > +
> > + return algfi == larl ? EXIT_SUCCESS : EXIT_FAILURE;
> > +}
>
> Acked-by: David Hildenbrand
>
Re: [PATCH 12/12] tests/tcg/s390x: Test MVCRL with a large value in R0
On 03.07.23 17:50, Ilya Leoshkevich wrote:
Add a small test to prevent regressions.
Signed-off-by: Ilya Leoshkevich
---
tests/tcg/s390x/mie3-mvcrl.c | 46
1 file changed, 36 insertions(+), 10 deletions(-)
diff --git a/tests/tcg/s390x/mie3-mvcrl.c b/tests/tcg/s390x/mie3-mvcrl.c
index 93c7b0a2903..ec78dd1d493 100644
--- a/tests/tcg/s390x/mie3-mvcrl.c
+++ b/tests/tcg/s390x/mie3-mvcrl.c
@@ -1,29 +1,55 @@
+#include
#include
+#include
#include
-
-static inline void mvcrl_8(const char *dst, const char *src)
+static void mvcrl(const char *dst, const char *src, size_t len)
{
+register long r0 asm("r0") = len;
+
asm volatile (
-"llill %%r0, 8\n"
".insn sse, 0xE50A, 0(%[dst]), 0(%[src])"
-: : [dst] "d" (dst), [src] "d" (src)
-: "r0", "memory");
+: : [dst] "d" (dst), [src] "d" (src), "r" (r0)
+: "memory");
}
-
-int main(int argc, char *argv[])
+static bool test(void)
{
const char *alpha = "abcdefghijklmnop";
/* array missing 'i' */
-char tstr[17] = "abcdefghjklmnop\0" ;
+char tstr[17] = "abcdefghjklmnop\0";
/* mvcrl reference use: 'open a hole in an array' */
-mvcrl_8(tstr + 9, tstr + 8);
+mvcrl(tstr + 9, tstr + 8, 8);
/* place missing 'i' */
tstr[8] = 'i';
-return strncmp(alpha, tstr, 16ul);
+return strncmp(alpha, tstr, 16ul) == 0;
+}
+
+static bool test_bad_r0(void)
+{
+char src[256];
+
+/*
+ * PoP says: Bits 32-55 of general register 0 should contain zeros;
+ * otherwise, the program may not operate compatibly in the future.
+ *
+ * Try it anyway in order to check whether this would crash QEMU itself.
+ */
+mvcrl(src, src, (size_t)-1);
+
+return true;
+}
+
+int main(void)
+{
+bool ok = true;
+
+ok &= test();
+ok &= test_bad_r0();
+
+return ok ? EXIT_SUCCESS : EXIT_FAILURE;
}
Reviewed-by: David Hildenbrand
--
Cheers,
David / dhildenb
Re: [PATCH 05/12] target/s390x: Fix LRA overwriting the top 32 bits on DAT error
On Tue, 2023-07-04 at 09:47 +0200, David Hildenbrand wrote:
> On 03.07.23 17:50, Ilya Leoshkevich wrote:
> > When a DAT error occurs, LRA is supposed to write the error
> > information
> > to the bottom 32 bits of R1, and leave the top 32 bits of R1 alone.
> >
> > Fix by passing the original value of R1 into helper and copying the
> > top 32 bits to the return value.
> >
> > Fixes: d8fe4a9c284f ("target-s390: Convert LRA")
> > Cc: qemu-sta...@nongnu.org
> > Signed-off-by: Ilya Leoshkevich
> > ---
> > target/s390x/helper.h | 2 +-
> > target/s390x/tcg/mem_helper.c | 4 ++--
> > target/s390x/tcg/translate.c | 2 +-
> > 3 files changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/target/s390x/helper.h b/target/s390x/helper.h
> > index 6bc01df73d7..05102578fc9 100644
> > --- a/target/s390x/helper.h
> > +++ b/target/s390x/helper.h
> > @@ -355,7 +355,7 @@ DEF_HELPER_FLAGS_4(idte, TCG_CALL_NO_RWG, void,
> > env, i64, i64, i32)
> > DEF_HELPER_FLAGS_4(ipte, TCG_CALL_NO_RWG, void, env, i64, i64,
> > i32)
> > DEF_HELPER_FLAGS_1(ptlb, TCG_CALL_NO_RWG, void, env)
> > DEF_HELPER_FLAGS_1(purge, TCG_CALL_NO_RWG, void, env)
> > -DEF_HELPER_2(lra, i64, env, i64)
> > +DEF_HELPER_3(lra, i64, env, i64, i64)
> > DEF_HELPER_1(per_check_exception, void, env)
> > DEF_HELPER_FLAGS_3(per_branch, TCG_CALL_NO_RWG, void, env, i64,
> > i64)
> > DEF_HELPER_FLAGS_2(per_ifetch, TCG_CALL_NO_RWG, void, env, i64)
> > diff --git a/target/s390x/tcg/mem_helper.c
> > b/target/s390x/tcg/mem_helper.c
> > index 84ad85212c9..94d93d7ea78 100644
> > --- a/target/s390x/tcg/mem_helper.c
> > +++ b/target/s390x/tcg/mem_helper.c
> > @@ -2356,7 +2356,7 @@ void HELPER(purge)(CPUS390XState *env)
> > }
> >
> > /* load real address */
> > -uint64_t HELPER(lra)(CPUS390XState *env, uint64_t addr)
> > +uint64_t HELPER(lra)(CPUS390XState *env, uint64_t r1, uint64_t
> > addr)
> > {
> > uint64_t asc = env->psw.mask & PSW_MASK_ASC;
> > uint64_t ret, tec;
> > @@ -2370,7 +2370,7 @@ uint64_t HELPER(lra)(CPUS390XState *env,
> > uint64_t addr)
> > exc = mmu_translate(env, addr, MMU_S390_LRA, asc, &ret,
> > &flags, &tec);
> > if (exc) {
> > cc = 3;
> > - ret = exc | 0x8000;
> > + ret = (r1 & 0x) | exc | 0x8000;
>
> ull missing for large constant?
Will do.
Just for my understanding, why is this necessary?
The current code base tends towards using ULL, but it's a little bit
inconsistent:
$ git grep -i 0xf | wc -l
2338
$ git grep -i 0xf | grep -i -v ul | wc -l
95
>
> > } else {
> > cc = 0;
> > ret |= addr & ~TARGET_PAGE_MASK;
> > diff --git a/target/s390x/tcg/translate.c
> > b/target/s390x/tcg/translate.c
> > index 0cef6efbef4..a6079ab7b4f 100644
> > --- a/target/s390x/tcg/translate.c
> > +++ b/target/s390x/tcg/translate.c
> > @@ -2932,7 +2932,7 @@ static DisasJumpType op_lctlg(DisasContext
> > *s, DisasOps *o)
> >
> > static DisasJumpType op_lra(DisasContext *s, DisasOps *o)
> > {
> > - gen_helper_lra(o->out, cpu_env, o->in2);
> > + gen_helper_lra(o->out, cpu_env, o->out, o->in2);
> > set_cc_static(s);
> > return DISAS_NEXT;
> > }
>
> Can't we use something like in1_r1 + wout_r1_32 instead ? *maybe*
> cleaner :)
>
The problem is that we want all 64 bits for the non-error case.
Re: [PATCH 05/12] target/s390x: Fix LRA overwriting the top 32 bits on DAT error
On 7/4/23 10:05, Ilya Leoshkevich wrote: + ret = (r1 & 0x) | exc | 0x8000; ull missing for large constant? Will do. Just for my understanding, why is this necessary? 32-bit host; you'll get a warning for the large constant. r~
[PATCH 1/3] softmmu: Support concurrent bounce buffers
It is not uncommon for device models to request mapping of several DMA
regions at the same time. An example is igb (and probably other net
devices as well) when a packet is spread across multiple descriptors.
In order to support this when indirect DMA is used, as is the case when
running the device model in a vfio-server process without mmap()-ed DMA,
this change allocates DMA bounce buffers dynamically instead of
supporting only a single buffer.
Signed-off-by: Mattias Nissler
---
softmmu/physmem.c | 74 ++-
1 file changed, 35 insertions(+), 39 deletions(-)
diff --git a/softmmu/physmem.c b/softmmu/physmem.c
index bda475a719..56130b5a1d 100644
--- a/softmmu/physmem.c
+++ b/softmmu/physmem.c
@@ -2904,13 +2904,11 @@ void cpu_flush_icache_range(hwaddr start, hwaddr len)
typedef struct {
MemoryRegion *mr;
-void *buffer;
hwaddr addr;
-hwaddr len;
-bool in_use;
+uint8_t buffer[];
} BounceBuffer;
-static BounceBuffer bounce;
+static size_t bounce_buffers_in_use;
typedef struct MapClient {
QEMUBH *bh;
@@ -2947,7 +2945,7 @@ void cpu_register_map_client(QEMUBH *bh)
QLIST_INSERT_HEAD(&map_client_list, client, link);
/* Write map_client_list before reading in_use. */
smp_mb();
-if (!qatomic_read(&bounce.in_use)) {
+if (qatomic_read(&bounce_buffers_in_use)) {
cpu_notify_map_clients_locked();
}
qemu_mutex_unlock(&map_client_list_lock);
@@ -3076,31 +3074,24 @@ void *address_space_map(AddressSpace *as,
RCU_READ_LOCK_GUARD();
fv = address_space_to_flatview(as);
mr = flatview_translate(fv, addr, &xlat, &l, is_write, attrs);
+memory_region_ref(mr);
if (!memory_access_is_direct(mr, is_write)) {
-if (qatomic_xchg(&bounce.in_use, true)) {
-*plen = 0;
-return NULL;
-}
-/* Avoid unbounded allocations */
-l = MIN(l, TARGET_PAGE_SIZE);
-bounce.buffer = qemu_memalign(TARGET_PAGE_SIZE, l);
-bounce.addr = addr;
-bounce.len = l;
-
-memory_region_ref(mr);
-bounce.mr = mr;
+qatomic_inc_fetch(&bounce_buffers_in_use);
+
+BounceBuffer *bounce = g_malloc(l + sizeof(BounceBuffer));
+bounce->addr = addr;
+bounce->mr = mr;
+
if (!is_write) {
flatview_read(fv, addr, MEMTXATTRS_UNSPECIFIED,
- bounce.buffer, l);
+ bounce->buffer, l);
}
*plen = l;
-return bounce.buffer;
+return bounce->buffer;
}
-
-memory_region_ref(mr);
*plen = flatview_extend_translation(fv, addr, len, mr, xlat,
l, is_write, attrs);
fuzz_dma_read_cb(addr, *plen, mr);
@@ -3114,31 +3105,36 @@ void *address_space_map(AddressSpace *as,
void address_space_unmap(AddressSpace *as, void *buffer, hwaddr len,
bool is_write, hwaddr access_len)
{
-if (buffer != bounce.buffer) {
-MemoryRegion *mr;
-ram_addr_t addr1;
+MemoryRegion *mr;
+ram_addr_t addr1;
+
+mr = memory_region_from_host(buffer, &addr1);
+if (mr == NULL) {
+/*
+ * Must be a bounce buffer (unless the caller passed a pointer which
+ * wasn't returned by address_space_map, which is illegal).
+ */
+BounceBuffer *bounce = container_of(buffer, BounceBuffer, buffer);
-mr = memory_region_from_host(buffer, &addr1);
-assert(mr != NULL);
if (is_write) {
-invalidate_and_set_dirty(mr, addr1, access_len);
+address_space_write(as, bounce->addr, MEMTXATTRS_UNSPECIFIED,
+bounce->buffer, access_len);
}
-if (xen_enabled()) {
-xen_invalidate_map_cache_entry(buffer);
+memory_region_unref(bounce->mr);
+g_free(bounce);
+
+if (qatomic_dec_fetch(&bounce_buffers_in_use) == 1) {
+cpu_notify_map_clients();
}
-memory_region_unref(mr);
return;
}
+
+if (xen_enabled()) {
+xen_invalidate_map_cache_entry(buffer);
+}
if (is_write) {
-address_space_write(as, bounce.addr, MEMTXATTRS_UNSPECIFIED,
-bounce.buffer, access_len);
-}
-qemu_vfree(bounce.buffer);
-bounce.buffer = NULL;
-memory_region_unref(bounce.mr);
-/* Clear in_use before reading map_client_list. */
-qatomic_set_mb(&bounce.in_use, false);
-cpu_notify_map_clients();
+invalidate_and_set_dirty(mr, addr1, access_len);
+}
}
void *cpu_physical_memory_map(hwaddr addr,
--
2.34.1
[PATCH 3/3] vfio-user: Message-based DMA support
Wire up support for DMA for the case where the vfio-user client does not
provide mmap()-able file descriptors, but DMA requests must be performed
via the VFIO-user protocol. This installs an indirect memory region,
which already works for pci_dma_{read,write}, and pci_dma_map works
thanks to the existing DMA bounce buffering support.
Note that while simple scenarios work with this patch, there's a known
race condition in libvfio-user that will mess up the communication
channel: https://github.com/nutanix/libvfio-user/issues/279 I intend to
contribute a fix for this problem, see discussion on the github issue
for more details.
Signed-off-by: Mattias Nissler
---
hw/remote/vfio-user-obj.c | 62 ++-
1 file changed, 55 insertions(+), 7 deletions(-)
diff --git a/hw/remote/vfio-user-obj.c b/hw/remote/vfio-user-obj.c
index 8b10c32a3c..9799580c77 100644
--- a/hw/remote/vfio-user-obj.c
+++ b/hw/remote/vfio-user-obj.c
@@ -300,6 +300,53 @@ static ssize_t vfu_object_cfg_access(vfu_ctx_t *vfu_ctx,
char * const buf,
return count;
}
+static MemTxResult vfu_dma_read(void *opaque, hwaddr addr, uint64_t *val,
+unsigned size, MemTxAttrs attrs)
+{
+MemoryRegion *region = opaque;
+VfuObject *o = VFU_OBJECT(region->owner);
+
+dma_sg_t *sg = alloca(dma_sg_size());
+vfu_dma_addr_t vfu_addr = (vfu_dma_addr_t)(region->addr + addr);
+if (vfu_addr_to_sgl(o->vfu_ctx, vfu_addr, size, sg, 1, PROT_READ) < 0 ||
+vfu_sgl_read(o->vfu_ctx, sg, 1, val) != 0) {
+return MEMTX_ERROR;
+}
+
+return MEMTX_OK;
+}
+
+static MemTxResult vfu_dma_write(void *opaque, hwaddr addr, uint64_t val,
+ unsigned size, MemTxAttrs attrs)
+{
+MemoryRegion *region = opaque;
+VfuObject *o = VFU_OBJECT(region->owner);
+
+dma_sg_t *sg = alloca(dma_sg_size());
+vfu_dma_addr_t vfu_addr = (vfu_dma_addr_t)(region->addr + addr);
+if (vfu_addr_to_sgl(o->vfu_ctx, vfu_addr, size, sg, 1, PROT_WRITE) < 0 ||
+vfu_sgl_write(o->vfu_ctx, sg, 1, &val) != 0) {
+return MEMTX_ERROR;
+}
+
+return MEMTX_OK;
+}
+
+static const MemoryRegionOps vfu_dma_ops = {
+.read_with_attrs = vfu_dma_read,
+.write_with_attrs = vfu_dma_write,
+.endianness = DEVICE_NATIVE_ENDIAN,
+.valid = {
+.min_access_size = 1,
+.max_access_size = 8,
+.unaligned = true,
+},
+.impl = {
+.min_access_size = 1,
+.max_access_size = 8,
+},
+};
+
static void dma_register(vfu_ctx_t *vfu_ctx, vfu_dma_info_t *info)
{
VfuObject *o = vfu_get_private(vfu_ctx);
@@ -308,17 +355,18 @@ static void dma_register(vfu_ctx_t *vfu_ctx,
vfu_dma_info_t *info)
g_autofree char *name = NULL;
struct iovec *iov = &info->iova;
-if (!info->vaddr) {
-return;
-}
-
name = g_strdup_printf("mem-%s-%"PRIx64"", o->device,
- (uint64_t)info->vaddr);
+ (uint64_t)iov->iov_base);
subregion = g_new0(MemoryRegion, 1);
-memory_region_init_ram_ptr(subregion, NULL, name,
- iov->iov_len, info->vaddr);
+if (info->vaddr) {
+memory_region_init_ram_ptr(subregion, OBJECT(o), name,
+ iov->iov_len, info->vaddr);
+} else {
+memory_region_init_io(subregion, OBJECT(o), &vfu_dma_ops, subregion,
+ name, iov->iov_len);
+}
dma_as = pci_device_iommu_address_space(o->pci_dev);
--
2.34.1
[PATCH 0/3] Support message-based DMA in vfio-user server
This series adds basic support for message-based DMA in qemu's vfio-user server. This is useful for cases where the client does not provide file descriptors for accessing system memory via memory mappings. My motivating use case is to hook up device models as PCIe endpoints to a hardware design. This works by bridging the PCIe transaction layer to vfio-user, and the endpoint does not access memory directly, but sends memory requests TLPs to the hardware design in order to perform DMA. Note that in addition to the 3 commits included, we also need a subprojects/libvfio-user roll to bring in this bugfix: https://github.com/nutanix/libvfio-user/commit/bb308a2e8ee9486a4c8b53d8d773f7c8faaeba08 Stefan, can I ask you to kindly update the https://gitlab.com/qemu-project/libvfio-user mirror? I'll be happy to include an update to subprojects/libvfio-user.wrap in this series. Finally, there is some more work required on top of this series to get message-based DMA to really work well: * libvfio-user has a long-standing issue where socket communication gets messed up when messages are sent from both ends at the same time. See https://github.com/nutanix/libvfio-user/issues/279 for more details. I've been engaging there and plan to contribute a fix. * qemu currently breaks down DMA accesses into chunks of size 8 bytes at maximum, each of which will be handled in a separate vfio-user DMA request message. This is quite terrible for large DMA acceses, such as when nvme reads and writes page-sized blocks for example. Thus, I would like to improve qemu to be able to perform larger accesses, at least for indirect memory regions. I have something working locally, but since this will likely result in more involved surgery and discussion, I am leaving this to be addressed in a separate patch. Mattias Nissler (3): softmmu: Support concurrent bounce buffers softmmu: Remove DMA unmap notification callback vfio-user: Message-based DMA support hw/remote/vfio-user-obj.c | 62 -- softmmu/dma-helpers.c | 28 softmmu/physmem.c | 131 -- 3 files changed, 83 insertions(+), 138 deletions(-) -- 2.34.1
[PATCH 2/3] softmmu: Remove DMA unmap notification callback
According to old commit messages, this was introduced to retry a DMA
operation at a later point in case the single bounce buffer is found to
be busy. This was never used widely - only the dma-helpers code made use
of it, but there are other device models that use multiple DMA mappings
(concurrently) and just failed.
After the improvement to support multiple concurrent bounce buffers,
the condition the notification callback allowed to work around no
longer exists, so we can just remove the logic and simplify the code.
Signed-off-by: Mattias Nissler
---
softmmu/dma-helpers.c | 28 -
softmmu/physmem.c | 71 ---
2 files changed, 99 deletions(-)
diff --git a/softmmu/dma-helpers.c b/softmmu/dma-helpers.c
index 2463964805..d05d226f11 100644
--- a/softmmu/dma-helpers.c
+++ b/softmmu/dma-helpers.c
@@ -68,23 +68,10 @@ typedef struct {
int sg_cur_index;
dma_addr_t sg_cur_byte;
QEMUIOVector iov;
-QEMUBH *bh;
DMAIOFunc *io_func;
void *io_func_opaque;
} DMAAIOCB;
-static void dma_blk_cb(void *opaque, int ret);
-
-static void reschedule_dma(void *opaque)
-{
-DMAAIOCB *dbs = (DMAAIOCB *)opaque;
-
-assert(!dbs->acb && dbs->bh);
-qemu_bh_delete(dbs->bh);
-dbs->bh = NULL;
-dma_blk_cb(dbs, 0);
-}
-
static void dma_blk_unmap(DMAAIOCB *dbs)
{
int i;
@@ -101,7 +88,6 @@ static void dma_complete(DMAAIOCB *dbs, int ret)
{
trace_dma_complete(dbs, ret, dbs->common.cb);
-assert(!dbs->acb && !dbs->bh);
dma_blk_unmap(dbs);
if (dbs->common.cb) {
dbs->common.cb(dbs->common.opaque, ret);
@@ -164,13 +150,6 @@ static void dma_blk_cb(void *opaque, int ret)
}
}
-if (dbs->iov.size == 0) {
-trace_dma_map_wait(dbs);
-dbs->bh = aio_bh_new(ctx, reschedule_dma, dbs);
-cpu_register_map_client(dbs->bh);
-goto out;
-}
-
if (!QEMU_IS_ALIGNED(dbs->iov.size, dbs->align)) {
qemu_iovec_discard_back(&dbs->iov,
QEMU_ALIGN_DOWN(dbs->iov.size, dbs->align));
@@ -189,18 +168,12 @@ static void dma_aio_cancel(BlockAIOCB *acb)
trace_dma_aio_cancel(dbs);
-assert(!(dbs->acb && dbs->bh));
if (dbs->acb) {
/* This will invoke dma_blk_cb. */
blk_aio_cancel_async(dbs->acb);
return;
}
-if (dbs->bh) {
-cpu_unregister_map_client(dbs->bh);
-qemu_bh_delete(dbs->bh);
-dbs->bh = NULL;
-}
if (dbs->common.cb) {
dbs->common.cb(dbs->common.opaque, -ECANCELED);
}
@@ -239,7 +212,6 @@ BlockAIOCB *dma_blk_io(AioContext *ctx,
dbs->dir = dir;
dbs->io_func = io_func;
dbs->io_func_opaque = io_func_opaque;
-dbs->bh = NULL;
qemu_iovec_init(&dbs->iov, sg->nsg);
dma_blk_cb(dbs, 0);
return &dbs->common;
diff --git a/softmmu/physmem.c b/softmmu/physmem.c
index 56130b5a1d..2b4123c127 100644
--- a/softmmu/physmem.c
+++ b/softmmu/physmem.c
@@ -2908,49 +2908,6 @@ typedef struct {
uint8_t buffer[];
} BounceBuffer;
-static size_t bounce_buffers_in_use;
-
-typedef struct MapClient {
-QEMUBH *bh;
-QLIST_ENTRY(MapClient) link;
-} MapClient;
-
-QemuMutex map_client_list_lock;
-static QLIST_HEAD(, MapClient) map_client_list
-= QLIST_HEAD_INITIALIZER(map_client_list);
-
-static void cpu_unregister_map_client_do(MapClient *client)
-{
-QLIST_REMOVE(client, link);
-g_free(client);
-}
-
-static void cpu_notify_map_clients_locked(void)
-{
-MapClient *client;
-
-while (!QLIST_EMPTY(&map_client_list)) {
-client = QLIST_FIRST(&map_client_list);
-qemu_bh_schedule(client->bh);
-cpu_unregister_map_client_do(client);
-}
-}
-
-void cpu_register_map_client(QEMUBH *bh)
-{
-MapClient *client = g_malloc(sizeof(*client));
-
-qemu_mutex_lock(&map_client_list_lock);
-client->bh = bh;
-QLIST_INSERT_HEAD(&map_client_list, client, link);
-/* Write map_client_list before reading in_use. */
-smp_mb();
-if (qatomic_read(&bounce_buffers_in_use)) {
-cpu_notify_map_clients_locked();
-}
-qemu_mutex_unlock(&map_client_list_lock);
-}
-
void cpu_exec_init_all(void)
{
qemu_mutex_init(&ram_list.mutex);
@@ -2964,28 +2921,6 @@ void cpu_exec_init_all(void)
finalize_target_page_bits();
io_mem_init();
memory_map_init();
-qemu_mutex_init(&map_client_list_lock);
-}
-
-void cpu_unregister_map_client(QEMUBH *bh)
-{
-MapClient *client;
-
-qemu_mutex_lock(&map_client_list_lock);
-QLIST_FOREACH(client, &map_client_list, link) {
-if (client->bh == bh) {
-cpu_unregister_map_client_do(client);
-break;
-}
-}
-qemu_mutex_unlock(&map_client_list_lock);
-}
-
-static void cpu_notify_map_clients(void)
-{
-qemu_mutex_lock(&map_client_list_lock);
-cpu_notify_map_clients_locked();
-qemu_mutex_unlock(&map_client_list_lock);
}
static bool flatview_access_valid(FlatView *fv,
Re: [PATCH 05/12] target/s390x: Fix LRA overwriting the top 32 bits on DAT error
} else {
cc = 0;
ret |= addr & ~TARGET_PAGE_MASK;
diff --git a/target/s390x/tcg/translate.c
b/target/s390x/tcg/translate.c
index 0cef6efbef4..a6079ab7b4f 100644
--- a/target/s390x/tcg/translate.c
+++ b/target/s390x/tcg/translate.c
@@ -2932,7 +2932,7 @@ static DisasJumpType op_lctlg(DisasContext
*s, DisasOps *o)
static DisasJumpType op_lra(DisasContext *s, DisasOps *o)
{
- gen_helper_lra(o->out, cpu_env, o->in2);
+ gen_helper_lra(o->out, cpu_env, o->out, o->in2);
set_cc_static(s);
return DISAS_NEXT;
}
Can't we use something like in1_r1 + wout_r1_32 instead ? *maybe*
cleaner :)
The problem is that we want all 64 bits for the non-error case.
Ah, I missed that detail, thanks.
--
Cheers,
David / dhildenb
[PATCH v2 00/12] target/s390x: Miscellaneous TCG fixes
v1: https://lists.gnu.org/archive/html/qemu-devel/2023-07/msg00454.html v1 -> v2: Add ULL for a large constant (David). Add a comment explaining the usage of ALGFI in the LARL test. Hi, Randomized testing found a number of issues in the s390x emulation. This series fixes 6 of them (patches 2-7) and adds tests (patches 8-12); patch 1 is a cosmetic improvement needed for the EPSW test. There are more issues, but I thought it would be better to send this batch now. Best regards, Ilya Ilya Leoshkevich (12): linux-user: elfload: Add more initial s390x PSW bits target/s390x: Fix EPSW CC reporting target/s390x: Fix MDEB and MDEBR target/s390x: Fix MVCRL with a large value in R0 target/s390x: Fix LRA overwriting the top 32 bits on DAT error target/s390x: Fix LRA when DAT is off target/s390x: Fix relative long instructions with large offsets tests/tcg/s390x: Test EPSW tests/tcg/s390x: Test LARL with a large offset tests/tcg/s390x: Test LRA tests/tcg/s390x: Test MDEB and MDEBR tests/tcg/s390x: Test MVCRL with a large value in R0 linux-user/elfload.c| 4 ++- target/s390x/helper.h | 2 +- target/s390x/mmu_helper.c | 2 +- target/s390x/tcg/fpu_helper.c | 3 +- target/s390x/tcg/insn-data.h.inc| 4 +-- target/s390x/tcg/mem_helper.c | 5 +-- target/s390x/tcg/translate.c| 8 +++-- tests/tcg/s390x/Makefile.softmmu-target | 1 + tests/tcg/s390x/Makefile.target | 3 ++ tests/tcg/s390x/epsw.c | 23 + tests/tcg/s390x/larl.c | 21 +++ tests/tcg/s390x/lra.S | 19 ++ tests/tcg/s390x/mdeb.c | 30 tests/tcg/s390x/mie3-mvcrl.c| 46 +++-- 14 files changed, 151 insertions(+), 20 deletions(-) create mode 100644 tests/tcg/s390x/epsw.c create mode 100644 tests/tcg/s390x/larl.c create mode 100644 tests/tcg/s390x/lra.S create mode 100644 tests/tcg/s390x/mdeb.c -- 2.41.0
[PATCH v2 03/12] target/s390x: Fix MDEB and MDEBR
These instructions multiply 32 bits by 32 bits, not 32 bits by 64 bits.
Fixes: 83b00736f3d8 ("target-s390: Convert FP MULTIPLY")
Cc: qemu-sta...@nongnu.org
Reviewed-by: David Hildenbrand
Signed-off-by: Ilya Leoshkevich
---
target/s390x/tcg/fpu_helper.c| 3 ++-
target/s390x/tcg/insn-data.h.inc | 4 ++--
2 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/target/s390x/tcg/fpu_helper.c b/target/s390x/tcg/fpu_helper.c
index 57e58292833..4b7fa58af3e 100644
--- a/target/s390x/tcg/fpu_helper.c
+++ b/target/s390x/tcg/fpu_helper.c
@@ -306,8 +306,9 @@ uint64_t HELPER(mdb)(CPUS390XState *env, uint64_t f1,
uint64_t f2)
/* 64/32-bit FP multiplication */
uint64_t HELPER(mdeb)(CPUS390XState *env, uint64_t f1, uint64_t f2)
{
+float64 f1_64 = float32_to_float64(f1, &env->fpu_status);
float64 ret = float32_to_float64(f2, &env->fpu_status);
-ret = float64_mul(f1, ret, &env->fpu_status);
+ret = float64_mul(f1_64, ret, &env->fpu_status);
handle_exceptions(env, false, GETPC());
return ret;
}
diff --git a/target/s390x/tcg/insn-data.h.inc b/target/s390x/tcg/insn-data.h.inc
index 0a45dbbcda8..457ed25d2fa 100644
--- a/target/s390x/tcg/insn-data.h.inc
+++ b/target/s390x/tcg/insn-data.h.inc
@@ -667,11 +667,11 @@
F(0xb317, MEEBR, RRE, Z, e1, e2, new, e1, meeb, 0, IF_BFP)
F(0xb31c, MDBR,RRE, Z, f1, f2, new, f1, mdb, 0, IF_BFP)
F(0xb34c, MXBR,RRE, Z, x1, x2, new_x, x1, mxb, 0, IF_BFP)
-F(0xb30c, MDEBR, RRE, Z, f1, e2, new, f1, mdeb, 0, IF_BFP)
+F(0xb30c, MDEBR, RRE, Z, e1, e2, new, f1, mdeb, 0, IF_BFP)
F(0xb307, MXDBR, RRE, Z, f1, f2, new_x, x1, mxdb, 0, IF_BFP)
F(0xed17, MEEB,RXE, Z, e1, m2_32u, new, e1, meeb, 0, IF_BFP)
F(0xed1c, MDB, RXE, Z, f1, m2_64, new, f1, mdb, 0, IF_BFP)
-F(0xed0c, MDEB,RXE, Z, f1, m2_32u, new, f1, mdeb, 0, IF_BFP)
+F(0xed0c, MDEB,RXE, Z, e1, m2_32u, new, f1, mdeb, 0, IF_BFP)
F(0xed07, MXDB,RXE, Z, f1, m2_64, new_x, x1, mxdb, 0, IF_BFP)
/* MULTIPLY HALFWORD */
C(0x4c00, MH, RX_a, Z, r1_o, m2_16s, new, r1_32, mul, 0)
--
2.41.0
[PATCH v2 09/12] tests/tcg/s390x: Test LARL with a large offset
Add a small test to prevent regressions.
Acked-by: David Hildenbrand
Signed-off-by: Ilya Leoshkevich
---
tests/tcg/s390x/Makefile.target | 1 +
tests/tcg/s390x/larl.c | 21 +
2 files changed, 22 insertions(+)
create mode 100644 tests/tcg/s390x/larl.c
diff --git a/tests/tcg/s390x/Makefile.target b/tests/tcg/s390x/Makefile.target
index 2ef22c88d95..dbf64c991e9 100644
--- a/tests/tcg/s390x/Makefile.target
+++ b/tests/tcg/s390x/Makefile.target
@@ -37,6 +37,7 @@ TESTS+=ex-relative-long
TESTS+=ex-branch
TESTS+=mxdb
TESTS+=epsw
+TESTS+=larl
cdsg: CFLAGS+=-pthread
cdsg: LDFLAGS+=-pthread
diff --git a/tests/tcg/s390x/larl.c b/tests/tcg/s390x/larl.c
new file mode 100644
index 000..7c95f89be73
--- /dev/null
+++ b/tests/tcg/s390x/larl.c
@@ -0,0 +1,21 @@
+/*
+ * Test the LARL instruction.
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+#include
+
+int main(void)
+{
+long algfi = (long)main;
+long larl;
+
+/*
+ * The compiler may emit larl for the C addition, so compute the expected
+ * value using algfi.
+ */
+asm("algfi %[r],0xd000" : [r] "+r" (algfi) : : "cc");
+asm("larl %[r],main+0xd000" : [r] "=r" (larl));
+
+return algfi == larl ? EXIT_SUCCESS : EXIT_FAILURE;
+}
--
2.41.0
[PATCH v2 07/12] target/s390x: Fix relative long instructions with large offsets
The expression "imm * 2" in gen_ri2() can wrap around if imm is large
enough.
Fix by casting imm to int64_t, like it's done in disas_jdest().
Fixes: e8ecdfeb30f0 ("Fix EXECUTE of relative branches")
Reviewed-by: David Hildenbrand
Signed-off-by: Ilya Leoshkevich
---
target/s390x/tcg/translate.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/target/s390x/tcg/translate.c b/target/s390x/tcg/translate.c
index a6079ab7b4f..6661b27efa4 100644
--- a/target/s390x/tcg/translate.c
+++ b/target/s390x/tcg/translate.c
@@ -5794,7 +5794,7 @@ static TCGv gen_ri2(DisasContext *s)
disas_jdest(s, i2, is_imm, imm, ri2);
if (is_imm) {
-ri2 = tcg_constant_i64(s->base.pc_next + imm * 2);
+ri2 = tcg_constant_i64(s->base.pc_next + (int64_t)imm * 2);
}
return ri2;
--
2.41.0
[PATCH v2 01/12] linux-user: elfload: Add more initial s390x PSW bits
Make the PSW look more similar to the real s390x userspace PSW.
Except for being there, the newly added bits should not affect the
userspace code execution.
Reviewed-by: David Hildenbrand
Signed-off-by: Ilya Leoshkevich
---
linux-user/elfload.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index 6900974c373..7935110bff4 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -1635,7 +1635,9 @@ const char *elf_hwcap_str(uint32_t bit)
static inline void init_thread(struct target_pt_regs *regs, struct image_info
*infop)
{
regs->psw.addr = infop->entry;
-regs->psw.mask = PSW_MASK_64 | PSW_MASK_32;
+regs->psw.mask = PSW_MASK_DAT | PSW_MASK_IO | PSW_MASK_EXT | \
+ PSW_MASK_MCHECK | PSW_MASK_PSTATE | PSW_MASK_64 | \
+ PSW_MASK_32;
regs->gprs[15] = infop->start_stack;
}
--
2.41.0
Re: [PATCH v2 05/12] target/s390x: Fix LRA overwriting the top 32 bits on DAT error
On 04.07.23 10:12, Ilya Leoshkevich wrote:
When a DAT error occurs, LRA is supposed to write the error information
to the bottom 32 bits of R1, and leave the top 32 bits of R1 alone.
Fix by passing the original value of R1 into helper and copying the
top 32 bits to the return value.
Fixes: d8fe4a9c284f ("target-s390: Convert LRA")
Cc: qemu-sta...@nongnu.org
Signed-off-by: Ilya Leoshkevich
---
target/s390x/helper.h | 2 +-
target/s390x/tcg/mem_helper.c | 4 ++--
target/s390x/tcg/translate.c | 2 +-
3 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/target/s390x/helper.h b/target/s390x/helper.h
index 6bc01df73d7..05102578fc9 100644
--- a/target/s390x/helper.h
+++ b/target/s390x/helper.h
@@ -355,7 +355,7 @@ DEF_HELPER_FLAGS_4(idte, TCG_CALL_NO_RWG, void, env, i64,
i64, i32)
DEF_HELPER_FLAGS_4(ipte, TCG_CALL_NO_RWG, void, env, i64, i64, i32)
DEF_HELPER_FLAGS_1(ptlb, TCG_CALL_NO_RWG, void, env)
DEF_HELPER_FLAGS_1(purge, TCG_CALL_NO_RWG, void, env)
-DEF_HELPER_2(lra, i64, env, i64)
+DEF_HELPER_3(lra, i64, env, i64, i64)
DEF_HELPER_1(per_check_exception, void, env)
DEF_HELPER_FLAGS_3(per_branch, TCG_CALL_NO_RWG, void, env, i64, i64)
DEF_HELPER_FLAGS_2(per_ifetch, TCG_CALL_NO_RWG, void, env, i64)
diff --git a/target/s390x/tcg/mem_helper.c b/target/s390x/tcg/mem_helper.c
index 84ad85212c9..f417fb1183c 100644
--- a/target/s390x/tcg/mem_helper.c
+++ b/target/s390x/tcg/mem_helper.c
@@ -2356,7 +2356,7 @@ void HELPER(purge)(CPUS390XState *env)
}
/* load real address */
-uint64_t HELPER(lra)(CPUS390XState *env, uint64_t addr)
+uint64_t HELPER(lra)(CPUS390XState *env, uint64_t r1, uint64_t addr)
{
uint64_t asc = env->psw.mask & PSW_MASK_ASC;
uint64_t ret, tec;
@@ -2370,7 +2370,7 @@ uint64_t HELPER(lra)(CPUS390XState *env, uint64_t addr)
exc = mmu_translate(env, addr, MMU_S390_LRA, asc, &ret, &flags, &tec);
if (exc) {
cc = 3;
-ret = exc | 0x8000;
+ret = (r1 & 0xULL) | exc | 0x8000;
} else {
cc = 0;
ret |= addr & ~TARGET_PAGE_MASK;
diff --git a/target/s390x/tcg/translate.c b/target/s390x/tcg/translate.c
index 0cef6efbef4..a6079ab7b4f 100644
--- a/target/s390x/tcg/translate.c
+++ b/target/s390x/tcg/translate.c
@@ -2932,7 +2932,7 @@ static DisasJumpType op_lctlg(DisasContext *s, DisasOps
*o)
static DisasJumpType op_lra(DisasContext *s, DisasOps *o)
{
-gen_helper_lra(o->out, cpu_env, o->in2);
+gen_helper_lra(o->out, cpu_env, o->out, o->in2);
set_cc_static(s);
return DISAS_NEXT;
}
Reviewed-by: David Hildenbrand
--
Cheers,
David / dhildenb
[PATCH v2 11/12] tests/tcg/s390x: Test MDEB and MDEBR
Add a small test to prevent regressions.
Acked-by: David Hildenbrand
Signed-off-by: Ilya Leoshkevich
---
tests/tcg/s390x/Makefile.target | 1 +
tests/tcg/s390x/mdeb.c | 30 ++
2 files changed, 31 insertions(+)
create mode 100644 tests/tcg/s390x/mdeb.c
diff --git a/tests/tcg/s390x/Makefile.target b/tests/tcg/s390x/Makefile.target
index dbf64c991e9..19fbbc6e531 100644
--- a/tests/tcg/s390x/Makefile.target
+++ b/tests/tcg/s390x/Makefile.target
@@ -38,6 +38,7 @@ TESTS+=ex-branch
TESTS+=mxdb
TESTS+=epsw
TESTS+=larl
+TESTS+=mdeb
cdsg: CFLAGS+=-pthread
cdsg: LDFLAGS+=-pthread
diff --git a/tests/tcg/s390x/mdeb.c b/tests/tcg/s390x/mdeb.c
new file mode 100644
index 000..4897d28069f
--- /dev/null
+++ b/tests/tcg/s390x/mdeb.c
@@ -0,0 +1,30 @@
+/*
+ * Test the MDEB and MDEBR instructions.
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+#include
+#include
+
+int main(void)
+{
+union {
+float f[2];
+double d;
+} a;
+float b;
+
+a.f[0] = 1.2345;
+a.f[1] = 999;
+b = 6.789;
+asm("mdeb %[a],%[b]" : [a] "+f" (a.d) : [b] "R" (b));
+assert(a.d > 8.38 && a.d < 8.39);
+
+a.f[0] = 1.2345;
+a.f[1] = 999;
+b = 6.789;
+asm("mdebr %[a],%[b]" : [a] "+f" (a.d) : [b] "f" (b));
+assert(a.d > 8.38 && a.d < 8.39);
+
+return EXIT_SUCCESS;
+}
--
2.41.0
[PATCH v2 10/12] tests/tcg/s390x: Test LRA
Add a small test to prevent regressions. Signed-off-by: Ilya Leoshkevich --- tests/tcg/s390x/Makefile.softmmu-target | 1 + tests/tcg/s390x/lra.S | 19 +++ 2 files changed, 20 insertions(+) create mode 100644 tests/tcg/s390x/lra.S diff --git a/tests/tcg/s390x/Makefile.softmmu-target b/tests/tcg/s390x/Makefile.softmmu-target index 44dfd716291..242c7b0f83c 100644 --- a/tests/tcg/s390x/Makefile.softmmu-target +++ b/tests/tcg/s390x/Makefile.softmmu-target @@ -20,6 +20,7 @@ ASM_TESTS = \ sam \ lpsw \ lpswe-early \ +lra \ ssm-early \ stosm-early \ unaligned-lowcore diff --git a/tests/tcg/s390x/lra.S b/tests/tcg/s390x/lra.S new file mode 100644 index 000..79ab86f36bb --- /dev/null +++ b/tests/tcg/s390x/lra.S @@ -0,0 +1,19 @@ +.org 0x200 /* lowcore padding */ +.globl _start +_start: +lgrl %r1,initial_r1 +lra %r1,0(%r1) +cgrl %r1,expected_r1 +jne 1f +lpswe success_psw +1: +lpswe failure_psw +.align 8 +initial_r1: +.quad 0x8765432112345678 +expected_r1: +.quad 0x876543218038 /* ASCE type exception */ +success_psw: +.quad 0x2,0xfff/* see is_special_wait_psw() */ +failure_psw: +.quad 0x2,0/* disabled wait */ -- 2.41.0
[PATCH v2 02/12] target/s390x: Fix EPSW CC reporting
EPSW should explicitly calculate and insert CC, like IPM does.
Fixes: e30a9d3fea58 ("target-s390: Implement EPSW")
Cc: qemu-sta...@nongnu.org
Reviewed-by: David Hildenbrand
Signed-off-by: Ilya Leoshkevich
---
target/s390x/tcg/translate.c | 4
1 file changed, 4 insertions(+)
diff --git a/target/s390x/tcg/translate.c b/target/s390x/tcg/translate.c
index a6ee2d44234..0cef6efbef4 100644
--- a/target/s390x/tcg/translate.c
+++ b/target/s390x/tcg/translate.c
@@ -2383,10 +2383,14 @@ static DisasJumpType op_epsw(DisasContext *s, DisasOps
*o)
int r1 = get_field(s, r1);
int r2 = get_field(s, r2);
TCGv_i64 t = tcg_temp_new_i64();
+TCGv_i64 t_cc = tcg_temp_new_i64();
/* Note the "subsequently" in the PoO, which implies a defined result
if r1 == r2. Thus we cannot defer these writes to an output hook. */
+gen_op_calc_cc(s);
+tcg_gen_extu_i32_i64(t_cc, cc_op);
tcg_gen_shri_i64(t, psw_mask, 32);
+tcg_gen_deposit_i64(t, t, t_cc, 12, 2);
store_reg32_i64(r1, t);
if (r2 != 0) {
store_reg32_i64(r2, psw_mask);
--
2.41.0
[PATCH v2 05/12] target/s390x: Fix LRA overwriting the top 32 bits on DAT error
When a DAT error occurs, LRA is supposed to write the error information
to the bottom 32 bits of R1, and leave the top 32 bits of R1 alone.
Fix by passing the original value of R1 into helper and copying the
top 32 bits to the return value.
Fixes: d8fe4a9c284f ("target-s390: Convert LRA")
Cc: qemu-sta...@nongnu.org
Signed-off-by: Ilya Leoshkevich
---
target/s390x/helper.h | 2 +-
target/s390x/tcg/mem_helper.c | 4 ++--
target/s390x/tcg/translate.c | 2 +-
3 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/target/s390x/helper.h b/target/s390x/helper.h
index 6bc01df73d7..05102578fc9 100644
--- a/target/s390x/helper.h
+++ b/target/s390x/helper.h
@@ -355,7 +355,7 @@ DEF_HELPER_FLAGS_4(idte, TCG_CALL_NO_RWG, void, env, i64,
i64, i32)
DEF_HELPER_FLAGS_4(ipte, TCG_CALL_NO_RWG, void, env, i64, i64, i32)
DEF_HELPER_FLAGS_1(ptlb, TCG_CALL_NO_RWG, void, env)
DEF_HELPER_FLAGS_1(purge, TCG_CALL_NO_RWG, void, env)
-DEF_HELPER_2(lra, i64, env, i64)
+DEF_HELPER_3(lra, i64, env, i64, i64)
DEF_HELPER_1(per_check_exception, void, env)
DEF_HELPER_FLAGS_3(per_branch, TCG_CALL_NO_RWG, void, env, i64, i64)
DEF_HELPER_FLAGS_2(per_ifetch, TCG_CALL_NO_RWG, void, env, i64)
diff --git a/target/s390x/tcg/mem_helper.c b/target/s390x/tcg/mem_helper.c
index 84ad85212c9..f417fb1183c 100644
--- a/target/s390x/tcg/mem_helper.c
+++ b/target/s390x/tcg/mem_helper.c
@@ -2356,7 +2356,7 @@ void HELPER(purge)(CPUS390XState *env)
}
/* load real address */
-uint64_t HELPER(lra)(CPUS390XState *env, uint64_t addr)
+uint64_t HELPER(lra)(CPUS390XState *env, uint64_t r1, uint64_t addr)
{
uint64_t asc = env->psw.mask & PSW_MASK_ASC;
uint64_t ret, tec;
@@ -2370,7 +2370,7 @@ uint64_t HELPER(lra)(CPUS390XState *env, uint64_t addr)
exc = mmu_translate(env, addr, MMU_S390_LRA, asc, &ret, &flags, &tec);
if (exc) {
cc = 3;
-ret = exc | 0x8000;
+ret = (r1 & 0xULL) | exc | 0x8000;
} else {
cc = 0;
ret |= addr & ~TARGET_PAGE_MASK;
diff --git a/target/s390x/tcg/translate.c b/target/s390x/tcg/translate.c
index 0cef6efbef4..a6079ab7b4f 100644
--- a/target/s390x/tcg/translate.c
+++ b/target/s390x/tcg/translate.c
@@ -2932,7 +2932,7 @@ static DisasJumpType op_lctlg(DisasContext *s, DisasOps
*o)
static DisasJumpType op_lra(DisasContext *s, DisasOps *o)
{
-gen_helper_lra(o->out, cpu_env, o->in2);
+gen_helper_lra(o->out, cpu_env, o->out, o->in2);
set_cc_static(s);
return DISAS_NEXT;
}
--
2.41.0
[PATCH v2 06/12] target/s390x: Fix LRA when DAT is off
LRA should perform DAT regardless of whether it's on or off.
Disable DAT check for MMU_S390_LRA.
Fixes: defb0e3157af ("s390x: Implement opcode helpers")
Cc: qemu-sta...@nongnu.org
Reviewed-by: David Hildenbrand
Signed-off-by: Ilya Leoshkevich
---
target/s390x/mmu_helper.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/target/s390x/mmu_helper.c b/target/s390x/mmu_helper.c
index b04b57c2356..fbb2f1b4d48 100644
--- a/target/s390x/mmu_helper.c
+++ b/target/s390x/mmu_helper.c
@@ -417,7 +417,7 @@ int mmu_translate(CPUS390XState *env, target_ulong vaddr,
int rw, uint64_t asc,
vaddr &= TARGET_PAGE_MASK;
-if (!(env->psw.mask & PSW_MASK_DAT)) {
+if (rw != MMU_S390_LRA && !(env->psw.mask & PSW_MASK_DAT)) {
*raddr = vaddr;
goto nodat;
}
--
2.41.0
[PATCH v2 12/12] tests/tcg/s390x: Test MVCRL with a large value in R0
Add a small test to prevent regressions.
Reviewed-by: David Hildenbrand
Signed-off-by: Ilya Leoshkevich
---
tests/tcg/s390x/mie3-mvcrl.c | 46
1 file changed, 36 insertions(+), 10 deletions(-)
diff --git a/tests/tcg/s390x/mie3-mvcrl.c b/tests/tcg/s390x/mie3-mvcrl.c
index 93c7b0a2903..ec78dd1d493 100644
--- a/tests/tcg/s390x/mie3-mvcrl.c
+++ b/tests/tcg/s390x/mie3-mvcrl.c
@@ -1,29 +1,55 @@
+#include
#include
+#include
#include
-
-static inline void mvcrl_8(const char *dst, const char *src)
+static void mvcrl(const char *dst, const char *src, size_t len)
{
+register long r0 asm("r0") = len;
+
asm volatile (
-"llill %%r0, 8\n"
".insn sse, 0xE50A, 0(%[dst]), 0(%[src])"
-: : [dst] "d" (dst), [src] "d" (src)
-: "r0", "memory");
+: : [dst] "d" (dst), [src] "d" (src), "r" (r0)
+: "memory");
}
-
-int main(int argc, char *argv[])
+static bool test(void)
{
const char *alpha = "abcdefghijklmnop";
/* array missing 'i' */
-char tstr[17] = "abcdefghjklmnop\0" ;
+char tstr[17] = "abcdefghjklmnop\0";
/* mvcrl reference use: 'open a hole in an array' */
-mvcrl_8(tstr + 9, tstr + 8);
+mvcrl(tstr + 9, tstr + 8, 8);
/* place missing 'i' */
tstr[8] = 'i';
-return strncmp(alpha, tstr, 16ul);
+return strncmp(alpha, tstr, 16ul) == 0;
+}
+
+static bool test_bad_r0(void)
+{
+char src[256];
+
+/*
+ * PoP says: Bits 32-55 of general register 0 should contain zeros;
+ * otherwise, the program may not operate compatibly in the future.
+ *
+ * Try it anyway in order to check whether this would crash QEMU itself.
+ */
+mvcrl(src, src, (size_t)-1);
+
+return true;
+}
+
+int main(void)
+{
+bool ok = true;
+
+ok &= test();
+ok &= test_bad_r0();
+
+return ok ? EXIT_SUCCESS : EXIT_FAILURE;
}
--
2.41.0
[PATCH v2 08/12] tests/tcg/s390x: Test EPSW
Add a small test to prevent regressions.
Reviewed-by: David Hildenbrand
Signed-off-by: Ilya Leoshkevich
---
tests/tcg/s390x/Makefile.target | 1 +
tests/tcg/s390x/epsw.c | 23 +++
2 files changed, 24 insertions(+)
create mode 100644 tests/tcg/s390x/epsw.c
diff --git a/tests/tcg/s390x/Makefile.target b/tests/tcg/s390x/Makefile.target
index 85abfbb98c0..2ef22c88d95 100644
--- a/tests/tcg/s390x/Makefile.target
+++ b/tests/tcg/s390x/Makefile.target
@@ -36,6 +36,7 @@ TESTS+=rxsbg
TESTS+=ex-relative-long
TESTS+=ex-branch
TESTS+=mxdb
+TESTS+=epsw
cdsg: CFLAGS+=-pthread
cdsg: LDFLAGS+=-pthread
diff --git a/tests/tcg/s390x/epsw.c b/tests/tcg/s390x/epsw.c
new file mode 100644
index 000..affb1a5e3a1
--- /dev/null
+++ b/tests/tcg/s390x/epsw.c
@@ -0,0 +1,23 @@
+/*
+ * Test the EPSW instruction.
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+#include
+#include
+
+int main(void)
+{
+unsigned long r1 = 0x1234567887654321UL, r2 = 0x8765432112345678UL;
+
+asm("cr %[r1],%[r2]\n" /* cc = 1 */
+"epsw %[r1],%[r2]"
+: [r1] "+r" (r1), [r2] "+r" (r2) : : "cc");
+
+/* Do not check the R and RI bits. */
+r1 &= ~0x4008UL;
+assert(r1 == 0x1234567807051001UL);
+assert(r2 == 0x876543218000UL);
+
+return EXIT_SUCCESS;
+}
--
2.41.0
Re: [PULL 00/38] maintainer updates for 8.1: testing, fuzz, plugins, docs, gdbstub
On 7/3/23 15:43, Alex Bennée wrote: The following changes since commit d145c0da22cde391d8c6672d33146ce306e8bf75: Merge tag 'pull-tcg-20230701' ofhttps://gitlab.com/rth7680/qemu into staging (2023-07-01 08:55:37 +0200) are available in the Git repository at: https://gitlab.com/stsquad/qemu.git tags/pull-maintainer-ominbus-030723-1 for you to fetch changes up to a6341482695e1d15f11915f12dba98724efb0697: tests/tcg: Add a test for info proc mappings (2023-07-03 12:52:38 +0100) maintainer updates: testing, fuzz, plugins, docs, gdbstub - clean up gitlab artefact handling - ensure gitlab publishes artefacts with coverage data - reduce testing scope for coverage job - mention CI pipeline in developer docs - add ability to add plugin args to check-tcg - fix some memory leaks and UB in tests - suppress xcb leaks from fuzzing output - add a test-fuzz to mirror the CI run - allow lci-refresh to be run in $SRC - update lcitool to latest version - add qemu-minimal package set with gcc-native - convert riscv64-cross to lcitool - update sbsa-ref tests - don't include arm_casq_ptw emulation unless TCG - convert plugins to use g_memdup2 - ensure plugins instrument SVE helper mem access - improve documentation of QOM/QDEV - make gdbstub send stop responses when it should - report user-mode pid in gdbstub - add support for info proc mappings in gdbstub Applied, thanks. Please update https://wiki.qemu.org/ChangeLog/8.1 as appropriate. r~
Re: [PATCH 0/3] Support message-based DMA in vfio-user server
On 04.07.23 10:06, Mattias Nissler wrote: This series adds basic support for message-based DMA in qemu's vfio-user server. This is useful for cases where the client does not provide file descriptors for accessing system memory via memory mappings. My motivating use case is to hook up device models as PCIe endpoints to a hardware design. This works by bridging the PCIe transaction layer to vfio-user, and the endpoint does not access memory directly, but sends memory requests TLPs to the hardware design in order to perform DMA. Note that in addition to the 3 commits included, we also need a subprojects/libvfio-user roll to bring in this bugfix: https://github.com/nutanix/libvfio-user/commit/bb308a2e8ee9486a4c8b53d8d773f7c8faaeba08 Stefan, can I ask you to kindly update the https://gitlab.com/qemu-project/libvfio-user mirror? I'll be happy to include an update to subprojects/libvfio-user.wrap in this series. Finally, there is some more work required on top of this series to get message-based DMA to really work well: * libvfio-user has a long-standing issue where socket communication gets messed up when messages are sent from both ends at the same time. See https://github.com/nutanix/libvfio-user/issues/279 for more details. I've been engaging there and plan to contribute a fix. * qemu currently breaks down DMA accesses into chunks of size 8 bytes at maximum, each of which will be handled in a separate vfio-user DMA request message. This is quite terrible for large DMA acceses, such as when nvme reads and writes page-sized blocks for example. Thus, I would like to improve qemu to be able to perform larger accesses, at least for indirect memory regions. I have something working locally, but since this will likely result in more involved surgery and discussion, I am leaving this to be addressed in a separate patch. I remember asking Stefan in the past if there wouldn't be a way to avoid that mmap dance (and also handle uffd etc. easier) for vhost-user (especially, virtiofsd) by only making QEMU access guest memory. That could make memory-backend-ram support something like vhost-user, avoiding shared memory and everything that comes with that (e.g., no KSM, no shared zeropage). So this series tackles vfio-user, does anybody know what it would take to get something similar running for vhost-user? -- Cheers, David / dhildenb
[PATCH v2 04/12] target/s390x: Fix MVCRL with a large value in R0
Using a large R0 causes an assertion error:
qemu-s390x: target/s390x/tcg/mem_helper.c:183: access_prepare_nf: Assertion
`size > 0 && size <= 4096' failed.
Even though PoP explicitly advises against using more than 8 bits for the
size, an emulator crash is never a good thing.
Fix by truncating the size to 8 bits.
Fixes: ea0a1053e276 ("s390x/tcg: Implement Miscellaneous-Instruction-Extensions
Facility 3 for the s390x")
Cc: qemu-sta...@nongnu.org
Reviewed-by: David Hildenbrand
Signed-off-by: Ilya Leoshkevich
---
target/s390x/tcg/mem_helper.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/target/s390x/tcg/mem_helper.c b/target/s390x/tcg/mem_helper.c
index d02ec861d8b..84ad85212c9 100644
--- a/target/s390x/tcg/mem_helper.c
+++ b/target/s390x/tcg/mem_helper.c
@@ -514,6 +514,7 @@ void HELPER(mvcrl)(CPUS390XState *env, uint64_t l, uint64_t
dest, uint64_t src)
int32_t i;
/* MVCRL always copies one more byte than specified - maximum is 256 */
+l &= 0xff;
l++;
access_prepare(&srca, env, src, l, MMU_DATA_LOAD, mmu_idx, ra);
--
2.41.0
[PATCH] target/riscv: Add Zihintntl extension ISA string to DTS
RVA23 Profiles states:
The RVA23 profiles are intended to be used for 64-bit application
processors that will run rich OS stacks from standard binary OS
distributions and with a substantial number of third-party binary user
applications that will be supported over a considerable length of time
in the field.
The chapter 4 of the unprivileged spec introduces the Zihintntl extension
and Zihintntl is a mandatory extension presented in RVA23 Profiles, whose
purpose is to enable application and operating system portability across
different implementations. Thus the DTS should contain the Zihintntl ISA
string in order to pass to software.
The unprivileged spec states:
Like any HINTs, these instructions may be freely ignored. Hence, although
they are described in terms of cache-based memory hierarchies, they do not
mandate the provision of caches.
These instructions are encoded with used opcode, e.g. ADD x0, x0, x2, which
QEMU already supports, and QEMU does not emulate cache. Therefore these
instructions can be considered as a no-op, and we only need to add a new
property for the Zihintntl extension.
Signed-off-by: Jason Chien
---
target/riscv/cpu.c | 2 ++
target/riscv/cpu_cfg.h | 1 +
2 files changed, 3 insertions(+)
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 881bddf393..6fd21466a4 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -81,6 +81,7 @@ static const struct isa_ext_data isa_edata_arr[] = {
ISA_EXT_DATA_ENTRY(zicond, PRIV_VERSION_1_12_0, ext_zicond),
ISA_EXT_DATA_ENTRY(zicsr, PRIV_VERSION_1_10_0, ext_icsr),
ISA_EXT_DATA_ENTRY(zifencei, PRIV_VERSION_1_10_0, ext_ifencei),
+ISA_EXT_DATA_ENTRY(zihintntl, PRIV_VERSION_1_10_0, ext_zihintntl),
ISA_EXT_DATA_ENTRY(zihintpause, PRIV_VERSION_1_10_0, ext_zihintpause),
ISA_EXT_DATA_ENTRY(zawrs, PRIV_VERSION_1_12_0, ext_zawrs),
ISA_EXT_DATA_ENTRY(zfh, PRIV_VERSION_1_11_0, ext_zfh),
@@ -1598,6 +1599,7 @@ static Property riscv_cpu_extensions[] = {
DEFINE_PROP_BOOL("sscofpmf", RISCVCPU, cfg.ext_sscofpmf, false),
DEFINE_PROP_BOOL("Zifencei", RISCVCPU, cfg.ext_ifencei, true),
DEFINE_PROP_BOOL("Zicsr", RISCVCPU, cfg.ext_icsr, true),
+DEFINE_PROP_BOOL("Zihintntl", RISCVCPU, cfg.ext_zihintntl, true),
DEFINE_PROP_BOOL("Zihintpause", RISCVCPU, cfg.ext_zihintpause, true),
DEFINE_PROP_BOOL("Zawrs", RISCVCPU, cfg.ext_zawrs, true),
DEFINE_PROP_BOOL("Zfh", RISCVCPU, cfg.ext_zfh, false),
diff --git a/target/riscv/cpu_cfg.h b/target/riscv/cpu_cfg.h
index c4a627d335..c7da2facef 100644
--- a/target/riscv/cpu_cfg.h
+++ b/target/riscv/cpu_cfg.h
@@ -66,6 +66,7 @@ struct RISCVCPUConfig {
bool ext_icbom;
bool ext_icboz;
bool ext_zicond;
+bool ext_zihintntl;
bool ext_zihintpause;
bool ext_smstateen;
bool ext_sstc;
--
2.17.1
[PATCH] ui/vnc-clipboard: fix infinite loop in inflate_buffer (CVE-2023-3255)
A wrong exit condition may lead to an infinite loop when inflating a
valid zlib buffer containing some extra bytes in the `inflate_buffer`
function. The bug only occurs post-authentication. Return the buffer
immediately if the end of the compressed data has been reached
(Z_STREAM_END).
Fixes: CVE-2023-3255
Fixes: 0bf41cab ("ui/vnc: clipboard support")
Reported-by: Kevin Denis
Signed-off-by: Mauro Matteo Cascella
---
ui/vnc-clipboard.c | 10 --
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/ui/vnc-clipboard.c b/ui/vnc-clipboard.c
index 8aeadfaa21..c759be3438 100644
--- a/ui/vnc-clipboard.c
+++ b/ui/vnc-clipboard.c
@@ -50,8 +50,11 @@ static uint8_t *inflate_buffer(uint8_t *in, uint32_t in_len,
uint32_t *size)
ret = inflate(&stream, Z_FINISH);
switch (ret) {
case Z_OK:
-case Z_STREAM_END:
break;
+case Z_STREAM_END:
+*size = stream.total_out;
+inflateEnd(&stream);
+return out;
case Z_BUF_ERROR:
out_len <<= 1;
if (out_len > (1 << 20)) {
@@ -66,11 +69,6 @@ static uint8_t *inflate_buffer(uint8_t *in, uint32_t in_len,
uint32_t *size)
}
}
-*size = stream.total_out;
-inflateEnd(&stream);
-
-return out;
-
err_end:
inflateEnd(&stream);
err:
--
2.41.0
Re: [PATCH 5/9] accel: Move CPUTLB to CPUState and assert offset
On 6/30/23 16:16, Richard Henderson wrote:
On 6/30/23 14:25, Anton Johansson wrote:
@@ -448,6 +448,13 @@ struct CPUState {
/* track IOMMUs whose translations we've cached in the TCG
TLB */
GArray *iommu_notifiers;
+
+ /*
+ * The following fields needs to be within
CPU_MAX_NEGATIVE_ENV_OFFSET of
+ * CPUArchState. As CPUArchState is assumed to follow CPUState
in the
+ * ArchCPU struct these are placed last. This is checked
statically.
+ */
+ CPUTLB tlb;
};
This is what we had before CPUNegativeOffsetState, comment and all,
and over the course of time the comment was ignored and the CPUTLB
crept toward the middle of the structure.
I recall you talking about this earlier. Is there any reason the
drifting of CPUTLB/IcountDecr from env wouldn't be caught by the static
asserts we've added?
I really don't see how this merge helps. There's nothing
target-specific about CPUTLBDescFast or CPUNegativeOffsetState, and
keeping them separate enforces their importance.
There isn't anything target specific about CPUTLBDescFast but its offset
in ArchCPU does depend on the target. This is due to the
TARGET_PAGE_ENTRY_EXTRA macro in CPUTLBEntryFull which is replaced
with a union in the first patch of this patchset.
On second thought, I think you're right and keeping CPUTLB and IcountDecr
together to emphasize their importance makes sense. There would still be
an implicit assumption on the target to keep CPUArchState and CPUState
close together, at least the intent is signaled better by
CPUNegativeOffsetState.
Even so, statically asserting the offset to CPUTLB and IcountDecr would be a
good idea.
The main concern of this patchset is being able to access the tlb and
icount_decr fields in a target-independent way only having access to
CPUState. Merging CPUNegativeOffsetState simply made the accessing part
simpler, but let's have a cpu_tlb(CPUState *) function instead.
I'll pull out the patch for making CPUTLB target independent and include
it in the patchset replacing CPUArchState with CPUState in cputlb.c and
user-exec.c. The static assert patches will be resubmitted separately.
Thanks,
--
Anton Johansson,
rev.ng Labs Srl.
Re: [PATCH 02/13] ppc440: Add cpu link property to PCIe controller model
On 4/7/23 00:02, BALATON Zoltan wrote: The PCIe controller model uses PPC DCRs but cannot be modeled with TYPE_PPC4xx_DCR_DEVICE as it derives from TYPE_PCIE_HOST_BRIDGE. Add a cpu link property to it similar to other DCR devices to allow registering DCRs from the device model. Signed-off-by: BALATON Zoltan --- hw/ppc/ppc440_uc.c | 114 - 1 file changed, 62 insertions(+), 52 deletions(-) Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH 01/13] ppc440: Change ppc460ex_pcie_init() parameter type
On 4/7/23 00:02, BALATON Zoltan wrote: Change parameter of ppc460ex_pcie_init() from env to cpu to allow further refactoring. Signed-off-by: BALATON Zoltan --- hw/ppc/ppc440.h| 2 +- hw/ppc/ppc440_uc.c | 7 --- hw/ppc/sam460ex.c | 2 +- 3 files changed, 6 insertions(+), 5 deletions(-) FWIW I'd rather move ppc460ex_pcie_register_dcrs() in this patch. Regardless: Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH 4/9] include/hw: introduce CPU_MAX_NEGATIVE_ENV_OFFSET
On 6/30/23 16:19, Richard Henderson wrote: On 6/30/23 14:25, Anton Johansson wrote: For reasons related to code-generation quality, the offset of CPUTLBDescFast and IcountDecr from CPUArchState needs to fit within 11 bits of displacement (arm[32|64] and riscv addressing modes). This commit introduces a new constant to store the maximum allowed negative offset, so it can be statically asserted to hold later on. Signed-off-by: Anton Johansson --- include/hw/core/cpu.h | 11 +++ 1 file changed, 11 insertions(+) diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h index c226d7263c..0377f74d48 100644 --- a/include/hw/core/cpu.h +++ b/include/hw/core/cpu.h @@ -259,6 +259,17 @@ struct qemu_work_item; #define CPU_UNSET_NUMA_NODE_ID -1 +/* + * For reasons related to code-generation quality the fast path + * CPUTLBDescFast array and IcountDecr fields need to be located within a + * small negative offset of CPUArchState. This requirement comes from + * host-specific addressing modes of arm[32|64] and riscv which uses 12- + * and 11 bits of displacement respectively. + */ +#define CPU_MIN_DISPLACEMENT_BITS 11 +#define CPU_MAX_NEGATIVE_ENV_OFFSET \ + (-(1 << CPU_MIN_DISPLACEMENT_BITS)) You'd want 6 bits, for AArch64 LDP (7-bit signed immediate). Ah right, but it's scaled so it would be -512 for AArch64 and -256 for arm32, that would also agree with the smallest MIN_TLB_MASK_TABLE_OFS in the backends. -- Anton Johansson, rev.ng Labs Srl.
Re: [PATCH 03/13] ppc440: Add a macro to shorten PCIe controller DCR registration
On 4/7/23 00:02, BALATON Zoltan wrote: It is more readable to wrap the complex call to ppc_dcr_register in a macro when needed repeatedly. Signed-off-by: BALATON Zoltan --- hw/ppc/ppc440_uc.c | 76 +- 1 file changed, 28 insertions(+), 48 deletions(-) +#define PPC440_PCIE_DCR(s, dcrn) \ +ppc_dcr_register(&(s)->cpu->env, (s)->dcrn_base + (dcrn), s, \ '(s), \' + &dcr_read_pcie, &dcr_write_pcie) + + Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH 04/13] ppc440: Rename local variable in dcr_read_pcie()
On 4/7/23 00:02, BALATON Zoltan wrote: Rename local variable storing state struct in dcr_read_pcie() for brevity and consistency with other functions. Signed-off-by: BALATON Zoltan --- hw/ppc/ppc440_uc.c | 50 +++--- 1 file changed, 25 insertions(+), 25 deletions(-) Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH 05/13] ppc440: Stop using system io region for PCIe buses
On 4/7/23 00:02, BALATON Zoltan wrote:
Add separate memory regions for the mem and io spaces of the PCIe bus
to avoid different buses using the same system io region.
"Reduce the I/O space to 64K."
Signed-off-by: BALATON Zoltan
---
hw/ppc/ppc440_uc.c | 9 ++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/hw/ppc/ppc440_uc.c b/hw/ppc/ppc440_uc.c
index 38ee27f437..0c5d999878 100644
--- a/hw/ppc/ppc440_uc.c
+++ b/hw/ppc/ppc440_uc.c
@@ -776,6 +776,7 @@ OBJECT_DECLARE_SIMPLE_TYPE(PPC460EXPCIEState,
PPC460EX_PCIE_HOST)
struct PPC460EXPCIEState {
PCIExpressHost host;
+MemoryRegion busmem;
MemoryRegion iomem;
qemu_irq irq[4];
int32_t dcrn_base;
@@ -1056,15 +1057,17 @@ static void ppc460ex_pcie_realize(DeviceState *dev,
Error **errp)
error_setg(errp, "invalid PCIe DCRN base");
return;
}
+snprintf(buf, sizeof(buf), "pcie%d-mem", id);
+memory_region_init(&s->busmem, OBJECT(s), buf, UINT64_MAX);
snprintf(buf, sizeof(buf), "pcie%d-io", id);
-memory_region_init(&s->iomem, OBJECT(s), buf, UINT64_MAX);
+memory_region_init(&s->iomem, OBJECT(s), buf, 0x1);
64 * KiB
for (i = 0; i < 4; i++) {
sysbus_init_irq(SYS_BUS_DEVICE(dev), &s->irq[i]);
}
snprintf(buf, sizeof(buf), "pcie.%d", id);
pci->bus = pci_register_root_bus(DEVICE(s), buf, ppc460ex_set_irq,
-pci_swizzle_map_irq_fn, s, &s->iomem,
-get_system_io(), 0, 4, TYPE_PCIE_BUS);
+pci_swizzle_map_irq_fn, s, &s->busmem,
+&s->iomem, 0, 4, TYPE_PCIE_BUS);
ppc460ex_pcie_register_dcrs(s);
}
With the changes addressed:
Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH 07/13] ppc440: Add busnum property to PCIe controller model
On 4/7/23 00:02, BALATON Zoltan wrote: Instead of guessing controller number from dcrn_base add a property so the device does not need knowledge about where it is used. Signed-off-by: BALATON Zoltan --- hw/ppc/ppc440_uc.c | 25 +++-- 1 file changed, 11 insertions(+), 14 deletions(-) Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH 08/13] ppc440: Remove ppc460ex_pcie_init legacy init function
On 4/7/23 00:02, BALATON Zoltan wrote: After previous changes we can now remove the legacy init function and move the device creation to board code. Signed-off-by: BALATON Zoltan --- hw/ppc/ppc440.h | 1 - hw/ppc/ppc440_uc.c | 21 - hw/ppc/sam460ex.c | 17 - include/hw/ppc/ppc4xx.h | 1 + 4 files changed, 17 insertions(+), 23 deletions(-) Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH 0/9] Collapse CPUNegativeOffsetState into CPUState
On 7/1/23 11:21, Paolo Bonzini wrote: On 6/30/23 14:25, Anton Johansson via wrote: CPUNegativeOffsetState is a struct placed immediately before CPUArchState in the ArchCPU struct. Its purpose is to ensure that certain fields (CPUTLBDescFast, IcountDecr) lay within a small negative offset of CPUArchState in memory. This is desired for better code-generation on arm[32|64] and riscv hosts which has addressing modes with 12- and 11 bits of displacement respectively. The purpose is also to ensure that general purpose registers stay close to the beginning of the CPUArchState and thus can also be accessed with a small displacement. Can you check if this becomes worse for any architecture? On some 64-bit targets, 8 bytes * 32 registers is 512 bytes and it's a substantial part of the 11 bits that are available. Paolo I'm dropping the CPUNegativeOffsetState changes, but the fields would still have had a negative displacement from the beginning of env, so the positive offset of the gprs from env wouldn't be affected. -- Anton Johansson, rev.ng Labs Srl.
Re: [PATCH 09/13] ppc4xx_pci: Rename QOM type name define
On 4/7/23 00:02, BALATON Zoltan wrote:
Rename the TYPE_PPC4xx_PCI_HOST_BRIDGE define and its string value to
match each other and other similar types and to avoid confusion with
"ppc4xx-host-bridge" type defined in same file.
Signed-off-by: BALATON Zoltan
---
hw/ppc/ppc440_bamboo.c | 3 +--
hw/ppc/ppc4xx_pci.c | 6 +++---
include/hw/ppc/ppc4xx.h | 2 +-
3 files changed, 5 insertions(+), 6 deletions(-)
diff --git a/hw/ppc/ppc440_bamboo.c b/hw/ppc/ppc440_bamboo.c
index f061b8cf3b..45f409c838 100644
--- a/hw/ppc/ppc440_bamboo.c
+++ b/hw/ppc/ppc440_bamboo.c
@@ -205,8 +205,7 @@ static void bamboo_init(MachineState *machine)
ppc4xx_sdram_ddr_enable(PPC4xx_SDRAM_DDR(dev));
/* PCI */
-dev = sysbus_create_varargs(TYPE_PPC4xx_PCI_HOST_BRIDGE,
-PPC440EP_PCI_CONFIG,
+dev = sysbus_create_varargs(TYPE_PPC4xx_PCI_HOST, PPC440EP_PCI_CONFIG,
qdev_get_gpio_in(uicdev, pci_irq_nrs[0]),
qdev_get_gpio_in(uicdev, pci_irq_nrs[1]),
qdev_get_gpio_in(uicdev, pci_irq_nrs[2]),
diff --git a/hw/ppc/ppc4xx_pci.c b/hw/ppc/ppc4xx_pci.c
index 1d4a50fa7c..fbdf8266d8 100644
--- a/hw/ppc/ppc4xx_pci.c
+++ b/hw/ppc/ppc4xx_pci.c
@@ -46,7 +46,7 @@ struct PCITargetMap {
uint32_t la;
};
-OBJECT_DECLARE_SIMPLE_TYPE(PPC4xxPCIState, PPC4xx_PCI_HOST_BRIDGE)
+OBJECT_DECLARE_SIMPLE_TYPE(PPC4xxPCIState, PPC4xx_PCI_HOST)
#define PPC4xx_PCI_NR_PMMS 3
#define PPC4xx_PCI_NR_PTMS 2
@@ -321,7 +321,7 @@ static void ppc4xx_pcihost_realize(DeviceState *dev, Error
**errp)
int i;
h = PCI_HOST_BRIDGE(dev);
-s = PPC4xx_PCI_HOST_BRIDGE(dev);
+s = PPC4xx_PCI_HOST(dev);
for (i = 0; i < ARRAY_SIZE(s->irq); i++) {
sysbus_init_irq(sbd, &s->irq[i]);
@@ -386,7 +386,7 @@ static void ppc4xx_pcihost_class_init(ObjectClass *klass,
void *data)
}
static const TypeInfo ppc4xx_pcihost_info = {
-.name = TYPE_PPC4xx_PCI_HOST_BRIDGE,
+.name = TYPE_PPC4xx_PCI_HOST,
.parent= TYPE_PCI_HOST_BRIDGE,
.instance_size = sizeof(PPC4xxPCIState),
.class_init= ppc4xx_pcihost_class_init,
diff --git a/include/hw/ppc/ppc4xx.h b/include/hw/ppc/ppc4xx.h
index 39ca602442..e053b9751b 100644
--- a/include/hw/ppc/ppc4xx.h
+++ b/include/hw/ppc/ppc4xx.h
@@ -29,7 +29,7 @@
#include "exec/memory.h"
#include "hw/sysbus.h"
-#define TYPE_PPC4xx_PCI_HOST_BRIDGE "ppc4xx-pcihost"
+#define TYPE_PPC4xx_PCI_HOST "ppc4xx-pci-host"
This is the host bridge however... Maybe we want to rename:
#define TYPE_PPC4xx_PCI_HOST_BRIDGE "ppc4xx-pci-hostbridge"
Re: [PATCH 10/13] ppc4xx_pci: Add define for ppc4xx-host-bridge type name
On 4/7/23 00:02, BALATON Zoltan wrote:
Add a QOM type name define for ppc4xx-host-bridge in the common header
and replace direct use of the string name with the constant.
Signed-off-by: BALATON Zoltan
---
hw/ppc/ppc440_pcix.c| 3 ++-
hw/ppc/ppc4xx_pci.c | 4 ++--
include/hw/ppc/ppc4xx.h | 1 +
3 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/hw/ppc/ppc440_pcix.c b/hw/ppc/ppc440_pcix.c
index f10f93c533..dfec25ac83 100644
--- a/hw/ppc/ppc440_pcix.c
+++ b/hw/ppc/ppc440_pcix.c
@@ -495,7 +495,8 @@ static void ppc440_pcix_realize(DeviceState *dev, Error
**errp)
ppc440_pcix_map_irq, &s->irq, &s->busmem,
get_system_io(), PCI_DEVFN(0, 0), 1, TYPE_PCI_BUS);
-s->dev = pci_create_simple(h->bus, PCI_DEVFN(0, 0), "ppc4xx-host-bridge");
+s->dev = pci_create_simple(h->bus, PCI_DEVFN(0, 0),
+ TYPE_PPC4xx_HOST_BRIDGE);
memory_region_init(&s->bm, OBJECT(s), "bm-ppc440-pcix", UINT64_MAX);
memory_region_add_subregion(&s->bm, 0x0, &s->busmem);
diff --git a/hw/ppc/ppc4xx_pci.c b/hw/ppc/ppc4xx_pci.c
index fbdf8266d8..6652119008 100644
--- a/hw/ppc/ppc4xx_pci.c
+++ b/hw/ppc/ppc4xx_pci.c
@@ -333,7 +333,7 @@ static void ppc4xx_pcihost_realize(DeviceState *dev, Error
**errp)
TYPE_PCI_BUS);
h->bus = b;
-pci_create_simple(b, 0, "ppc4xx-host-bridge");
+pci_create_simple(b, 0, TYPE_PPC4xx_HOST_BRIDGE);
/* XXX split into 2 memory regions, one for config space, one for regs */
memory_region_init(&s->container, OBJECT(s), "pci-container",
PCI_ALL_SIZE);
@@ -367,7 +367,7 @@ static void ppc4xx_host_bridge_class_init(ObjectClass
*klass, void *data)
}
static const TypeInfo ppc4xx_host_bridge_info = {
-.name = "ppc4xx-host-bridge",
+.name = TYPE_PPC4xx_HOST_BRIDGE,
.parent= TYPE_PCI_DEVICE,
.instance_size = sizeof(PCIDevice),
.class_init= ppc4xx_host_bridge_class_init,
diff --git a/include/hw/ppc/ppc4xx.h b/include/hw/ppc/ppc4xx.h
index e053b9751b..766d575e86 100644
--- a/include/hw/ppc/ppc4xx.h
+++ b/include/hw/ppc/ppc4xx.h
@@ -29,6 +29,7 @@
#include "exec/memory.h"
#include "hw/sysbus.h"
+#define TYPE_PPC4xx_HOST_BRIDGE "ppc4xx-host-bridge"
This is the function #0 of the host bridge, maybe:
#define TYPE_PPC4xx_HOST_BRIDGE_FN0 "ppc4xx-pci-host-bridge-fn0"
#define TYPE_PPC4xx_PCI_HOST "ppc4xx-pci-host"
#define TYPE_PPC460EX_PCIE_HOST "ppc460ex-pcie-host"
Re: [PATCH 11/13] ppc440_pcix: Rename QOM type define abd move it to common header
On 4/7/23 00:02, BALATON Zoltan wrote:
Rename TYPE_PPC440_PCIX_HOST_BRIDGE to better match its string value,
move it to common header and use it also in sam460ex to replace hard
coded type name.
Signed-off-by: BALATON Zoltan
---
hw/ppc/ppc440_pcix.c| 9 -
hw/ppc/sam460ex.c | 2 +-
include/hw/ppc/ppc4xx.h | 1 +
3 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/hw/ppc/ppc440_pcix.c b/hw/ppc/ppc440_pcix.c
index dfec25ac83..adfecf1e76 100644
--- a/hw/ppc/ppc440_pcix.c
+++ b/hw/ppc/ppc440_pcix.c
@@ -44,8 +44,7 @@ struct PLBInMap {
MemoryRegion mr;
};
-#define TYPE_PPC440_PCIX_HOST_BRIDGE "ppc440-pcix-host"
-OBJECT_DECLARE_SIMPLE_TYPE(PPC440PCIXState, PPC440_PCIX_HOST_BRIDGE)
+OBJECT_DECLARE_SIMPLE_TYPE(PPC440PCIXState, PPC440_PCIX_HOST)
#define PPC440_PCIX_NR_POMS 3
#define PPC440_PCIX_NR_PIMS 3
@@ -397,7 +396,7 @@ static const MemoryRegionOps pci_reg_ops = {
static void ppc440_pcix_reset(DeviceState *dev)
{
-struct PPC440PCIXState *s = PPC440_PCIX_HOST_BRIDGE(dev);
+struct PPC440PCIXState *s = PPC440_PCIX_HOST(dev);
int i;
for (i = 0; i < PPC440_PCIX_NR_POMS; i++) {
@@ -487,7 +486,7 @@ static void ppc440_pcix_realize(DeviceState *dev, Error
**errp)
PCIHostState *h;
h = PCI_HOST_BRIDGE(dev);
-s = PPC440_PCIX_HOST_BRIDGE(dev);
+s = PPC440_PCIX_HOST(dev);
sysbus_init_irq(sbd, &s->irq);
memory_region_init(&s->busmem, OBJECT(dev), "pci bus memory", UINT64_MAX);
@@ -525,7 +524,7 @@ static void ppc440_pcix_class_init(ObjectClass *klass, void
*data)
}
static const TypeInfo ppc440_pcix_info = {
-.name = TYPE_PPC440_PCIX_HOST_BRIDGE,
+.name = TYPE_PPC440_PCIX_HOST,
.parent= TYPE_PCI_HOST_BRIDGE,
.instance_size = sizeof(PPC440PCIXState),
.class_init= ppc440_pcix_class_init,
diff --git a/hw/ppc/sam460ex.c b/hw/ppc/sam460ex.c
index d446cfc37b..8d0e551d14 100644
--- a/hw/ppc/sam460ex.c
+++ b/hw/ppc/sam460ex.c
@@ -439,7 +439,7 @@ static void sam460ex_init(MachineState *machine)
/* PCI bus */
/* All PCI irqs are connected to the same UIC pin (cf. UBoot source) */
-dev = sysbus_create_simple("ppc440-pcix-host", 0xc0ec0,
+dev = sysbus_create_simple(TYPE_PPC440_PCIX_HOST, 0xc0ec0,
qdev_get_gpio_in(uic[1], 0));
pci_bus = PCI_BUS(qdev_get_child_bus(dev, "pci.0"));
diff --git a/include/hw/ppc/ppc4xx.h b/include/hw/ppc/ppc4xx.h
index 766d575e86..ea7740239b 100644
--- a/include/hw/ppc/ppc4xx.h
+++ b/include/hw/ppc/ppc4xx.h
@@ -31,6 +31,7 @@
#define TYPE_PPC4xx_HOST_BRIDGE "ppc4xx-host-bridge"
#define TYPE_PPC4xx_PCI_HOST "ppc4xx-pci-host"
+#define TYPE_PPC440_PCIX_HOST "ppc440-pcix-host"
#define TYPE_PPC460EX_PCIE_HOST "ppc460ex-pcie-host"
Again, this is a host bridge, why not name it HOST_BRIDGE?
Re: [PATCH 12/13] ppc440_pcix: Don't use iomem for regs
On 4/7/23 00:02, BALATON Zoltan wrote: The iomem memory region is better used for the PCI IO space but currently used for registers. Stop using it for that to allow this to be cleaned up in the next patch. Signed-off-by: BALATON Zoltan --- hw/ppc/ppc440_pcix.c | 7 --- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/hw/ppc/ppc440_pcix.c b/hw/ppc/ppc440_pcix.c index adfecf1e76..ee2dc44f67 100644 --- a/hw/ppc/ppc440_pcix.c +++ b/hw/ppc/ppc440_pcix.c @@ -484,6 +484,7 @@ static void ppc440_pcix_realize(DeviceState *dev, Error **errp) SysBusDevice *sbd = SYS_BUS_DEVICE(dev); PPC440PCIXState *s; PCIHostState *h; +MemoryRegion *regs = g_new(MemoryRegion, 1); Why not hold it within PPC440PCIXState? h = PCI_HOST_BRIDGE(dev); s = PPC440_PCIX_HOST(dev); @@ -507,11 +508,11 @@ static void ppc440_pcix_realize(DeviceState *dev, Error **errp) h, "pci-conf-idx", 4); memory_region_init_io(&h->data_mem, OBJECT(s), &pci_host_data_le_ops, h, "pci-conf-data", 4); -memory_region_init_io(&s->iomem, OBJECT(s), &pci_reg_ops, s, - "pci.reg", PPC440_REG_SIZE); +memory_region_init_io(regs, OBJECT(s), &pci_reg_ops, s, "pci-reg", + PPC440_REG_SIZE); memory_region_add_subregion(&s->container, PCIC0_CFGADDR, &h->conf_mem); memory_region_add_subregion(&s->container, PCIC0_CFGDATA, &h->data_mem); -memory_region_add_subregion(&s->container, PPC440_REG_BASE, &s->iomem); +memory_region_add_subregion(&s->container, PPC440_REG_BASE, regs); sysbus_init_mmio(sbd, &s->container); }
Re: [PATCH 13/13] ppc440_pcix: Stop using system io region for PCI bus
On 4/7/23 00:02, BALATON Zoltan wrote: Use the iomem region for the PCI io space and map it directly from the board without an intermediate alias that is not really needed. "Reduce the I/O region to 64K." Signed-off-by: BALATON Zoltan --- hw/ppc/ppc440_pcix.c | 8 +--- hw/ppc/sam460ex.c| 6 +- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/hw/ppc/ppc440_pcix.c b/hw/ppc/ppc440_pcix.c index ee2dc44f67..cca8a72c72 100644 --- a/hw/ppc/ppc440_pcix.c +++ b/hw/ppc/ppc440_pcix.c @@ -490,10 +490,11 @@ static void ppc440_pcix_realize(DeviceState *dev, Error **errp) s = PPC440_PCIX_HOST(dev); sysbus_init_irq(sbd, &s->irq); -memory_region_init(&s->busmem, OBJECT(dev), "pci bus memory", UINT64_MAX); +memory_region_init(&s->busmem, OBJECT(dev), "pci-mem", UINT64_MAX); +memory_region_init(&s->iomem, OBJECT(dev), "pci-io", 0x1); 64 * KiB h->bus = pci_register_root_bus(dev, NULL, ppc440_pcix_set_irq, - ppc440_pcix_map_irq, &s->irq, &s->busmem, - get_system_io(), PCI_DEVFN(0, 0), 1, TYPE_PCI_BUS); + ppc440_pcix_map_irq, &s->irq, &s->busmem, &s->iomem, + PCI_DEVFN(0, 0), 1, TYPE_PCI_BUS); s->dev = pci_create_simple(h->bus, PCI_DEVFN(0, 0), TYPE_PPC4xx_HOST_BRIDGE); @@ -514,6 +515,7 @@ static void ppc440_pcix_realize(DeviceState *dev, Error **errp) memory_region_add_subregion(&s->container, PCIC0_CFGDATA, &h->data_mem); memory_region_add_subregion(&s->container, PPC440_REG_BASE, regs); sysbus_init_mmio(sbd, &s->container); +sysbus_init_mmio(sbd, &s->iomem); } With the changes requested: Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH] ui/vnc-clipboard: fix infinite loop in inflate_buffer (CVE-2023-3255)
On Tue, Jul 4, 2023 at 10:42 AM Mauro Matteo Cascella
wrote:
> A wrong exit condition may lead to an infinite loop when inflating a
> valid zlib buffer containing some extra bytes in the `inflate_buffer`
> function. The bug only occurs post-authentication. Return the buffer
> immediately if the end of the compressed data has been reached
> (Z_STREAM_END).
>
> Fixes: CVE-2023-3255
> Fixes: 0bf41cab ("ui/vnc: clipboard support")
> Reported-by: Kevin Denis
> Signed-off-by: Mauro Matteo Cascella
>
Tested-by: Marc-André Lureau
Reviewed-by: Marc-André Lureau
Note: we may want to disconnect the client when there are extra bytes in
the message, or print some warnings.
> ---
> ui/vnc-clipboard.c | 10 --
> 1 file changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/ui/vnc-clipboard.c b/ui/vnc-clipboard.c
> index 8aeadfaa21..c759be3438 100644
> --- a/ui/vnc-clipboard.c
> +++ b/ui/vnc-clipboard.c
> @@ -50,8 +50,11 @@ static uint8_t *inflate_buffer(uint8_t *in, uint32_t
> in_len, uint32_t *size)
> ret = inflate(&stream, Z_FINISH);
> switch (ret) {
> case Z_OK:
> -case Z_STREAM_END:
> break;
> +case Z_STREAM_END:
> +*size = stream.total_out;
> +inflateEnd(&stream);
> +return out;
> case Z_BUF_ERROR:
> out_len <<= 1;
> if (out_len > (1 << 20)) {
> @@ -66,11 +69,6 @@ static uint8_t *inflate_buffer(uint8_t *in, uint32_t
> in_len, uint32_t *size)
> }
> }
>
> -*size = stream.total_out;
> -inflateEnd(&stream);
> -
> -return out;
> -
> err_end:
> inflateEnd(&stream);
> err:
> --
> 2.41.0
>
>
>
--
Marc-André Lureau
Re: [PATCH v1 2/5] linux-headers: Import arm-smccc.h from Linux v6.4-rc7
On Mon, Jun 26 2023, Shaoqin Huang wrote: > Copy in the SMCCC definitions from the kernel, which will be used to > implement SMCCC handling in userspace. > > Signed-off-by: Shaoqin Huang > --- > linux-headers/linux/arm-smccc.h | 240 > 1 file changed, 240 insertions(+) > create mode 100644 linux-headers/linux/arm-smccc.h I think you need to add this to the headers update script, so that changes get propagated in the future as well. (I'd just add it to the script and do the update in one go.)
Re: [PATCH v1 4/5] arm/kvm: add skeleton implementation for userspace SMCCC call handling
On Mon, Jun 26 2023, Shaoqin Huang wrote:
> The SMCCC call filtering provide the ability to forward the SMCCC call
> to userspace, so we provide a new option `user-smccc` to enable handling
> SMCCC call in userspace, the default value is off.
>
> And add the skeleton implementation for userspace SMCCC call
> initialization and handling.
>
> Signed-off-by: Shaoqin Huang
> ---
> docs/system/arm/virt.rst | 4 +++
> hw/arm/virt.c| 21
> include/hw/arm/virt.h| 1 +
> target/arm/kvm.c | 54
> 4 files changed, 80 insertions(+)
>
> diff --git a/docs/system/arm/virt.rst b/docs/system/arm/virt.rst
> index 1cab33f02e..ff43d52f04 100644
> --- a/docs/system/arm/virt.rst
> +++ b/docs/system/arm/virt.rst
> @@ -155,6 +155,10 @@ dtb-randomness
>DTB to be non-deterministic. It would be the responsibility of
>the firmware to come up with a seed and pass it on if it wants to.
>
> +user-smccc
> + Set ``on``/``off`` to enable/disable handling smccc call in userspace
> + instead of kernel.
> +
> dtb-kaslr-seed
>A deprecated synonym for dtb-randomness.
>
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 9b9f7d9c68..767720321c 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -42,6 +42,7 @@
> #include "hw/vfio/vfio-amd-xgbe.h"
> #include "hw/display/ramfb.h"
> #include "net/net.h"
> +#include "qom/object.h"
> #include "sysemu/device_tree.h"
> #include "sysemu/numa.h"
> #include "sysemu/runstate.h"
> @@ -2511,6 +2512,19 @@ static void virt_set_oem_table_id(Object *obj, const
> char *value,
> strncpy(vms->oem_table_id, value, 8);
> }
>
> +static bool virt_get_user_smccc(Object *obj, Error **errp)
> +{
> +VirtMachineState *vms = VIRT_MACHINE(obj);
> +
> +return vms->user_smccc;
> +}
> +
> +static void virt_set_user_smccc(Object *obj, bool value, Error **errp)
> +{
> +VirtMachineState *vms = VIRT_MACHINE(obj);
> +
> +vms->user_smccc = value;
> +}
>
> bool virt_is_acpi_enabled(VirtMachineState *vms)
> {
> @@ -3155,6 +3169,13 @@ static void virt_machine_class_init(ObjectClass *oc,
> void *data)
>"in ACPI table header."
>"The string may be up to 8 bytes
> in size");
>
> +object_class_property_add_bool(oc, "user-smccc",
> + virt_get_user_smccc,
> + virt_set_user_smccc);
> +object_class_property_set_description(oc, "user-smccc",
> + "Set on/off to enable/disable "
> + "handling smccc call in
> userspace");
> +
> }
>
> static void virt_instance_init(Object *obj)
This knob pretty much only makes sense for KVM guests, and we'll ignore
it with tcg -- would it make sense to check that we are actually using
KVM before we proceed (like we do for the tcg-only props?)
[PATCH] virtio-gpu: fix potential divide-by-zero regression
From: Marc-André Lureau
Commit 9462ff4695aa0 ("virtio-gpu/win32: allocate shareable 2d
resources/images") introduces a division, which can lead to crashes when
"height" is 0.
Fixes: https://gitlab.com/qemu-project/qemu/-/issues/1744
Signed-off-by: Marc-André Lureau
---
hw/display/virtio-gpu.c | 4 ++--
tests/lcitool/libvirt-ci | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index 347e17d490..7371a5cbf0 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -324,7 +324,7 @@ static void virtio_gpu_resource_create_2d(VirtIOGPU *g,
res->image = pixman_image_create_bits(pformat,
c2d.width,
c2d.height,
- bits, res->hostmem / c2d.height);
+ bits, c2d.height ? res->hostmem
/ c2d.height : 0);
#ifdef WIN32
if (res->image) {
pixman_image_set_destroy_function(res->image,
win32_pixman_image_destroy, res->handle);
@@ -1292,7 +1292,7 @@ static int virtio_gpu_load(QEMUFile *f, void *opaque,
size_t size,
#endif
res->image = pixman_image_create_bits(pformat,
res->width, res->height,
- bits, res->hostmem /
res->height);
+ bits, res->height ? res->hostmem
/ res->height : 0);
if (!res->image) {
g_free(res);
return -EINVAL;
diff --git a/tests/lcitool/libvirt-ci b/tests/lcitool/libvirt-ci
index b0f44f929a..c8971e90ac 16
--- a/tests/lcitool/libvirt-ci
+++ b/tests/lcitool/libvirt-ci
@@ -1 +1 @@
-Subproject commit b0f44f929a81c0a604fb7fbf8afc34d37ab0eae9
+Subproject commit c8971e90ac169ee2b539c747f74d96c876debdf9
--
2.41.0
Re: [PATCH] virtio-gpu: fix potential divide-by-zero regression
On 04/07/2023 11.19, marcandre.lur...@redhat.com wrote:
From: Marc-André Lureau
Commit 9462ff4695aa0 ("virtio-gpu/win32: allocate shareable 2d
resources/images") introduces a division, which can lead to crashes when
"height" is 0.
Fixes: https://gitlab.com/qemu-project/qemu/-/issues/1744
Signed-off-by: Marc-André Lureau
---
...
diff --git a/tests/lcitool/libvirt-ci b/tests/lcitool/libvirt-ci
index b0f44f929a..c8971e90ac 16
--- a/tests/lcitool/libvirt-ci
+++ b/tests/lcitool/libvirt-ci
@@ -1 +1 @@
-Subproject commit b0f44f929a81c0a604fb7fbf8afc34d37ab0eae9
+Subproject commit c8971e90ac169ee2b539c747f74d96c876debdf9
That submodule update looks like an accident?
Thomas
Re: [PATCH] virtio-gpu: fix potential divide-by-zero regression
On 230704 1119, marcandre.lur...@redhat.com wrote:
> From: Marc-André Lureau
>
> Commit 9462ff4695aa0 ("virtio-gpu/win32: allocate shareable 2d
> resources/images") introduces a division, which can lead to crashes when
> "height" is 0.
>
> Fixes: https://gitlab.com/qemu-project/qemu/-/issues/1744
> Signed-off-by: Marc-André Lureau
Reviewed-by: Alexander Bulekov
Re: [PATCH] virtio-gpu: fix potential divide-by-zero regression
On Tue, Jul 4, 2023 at 11:24 AM Thomas Huth wrote:
> On 04/07/2023 11.19, marcandre.lur...@redhat.com wrote:
> > From: Marc-André Lureau
> >
> > Commit 9462ff4695aa0 ("virtio-gpu/win32: allocate shareable 2d
> > resources/images") introduces a division, which can lead to crashes when
> > "height" is 0.
> >
> > Fixes: https://gitlab.com/qemu-project/qemu/-/issues/1744
> > Signed-off-by: Marc-André Lureau
> > ---
> ...
> > diff --git a/tests/lcitool/libvirt-ci b/tests/lcitool/libvirt-ci
> > index b0f44f929a..c8971e90ac 16
> > --- a/tests/lcitool/libvirt-ci
> > +++ b/tests/lcitool/libvirt-ci
> > @@ -1 +1 @@
> > -Subproject commit b0f44f929a81c0a604fb7fbf8afc34d37ab0eae9
> > +Subproject commit c8971e90ac169ee2b539c747f74d96c876debdf9
>
> That submodule update looks like an accident?
>
>
Oops.. thanks for noticing
--
Marc-André Lureau
Re: [PATCH 1/1] A new virtio pci device named virtio-vcpu-stall-watchdog-pci
> On Thu, Jun 15, 2023 at 02:13:02PM +0800, zhanghao1 wrote:
> > Each vcpu creates a corresponding timer task. The watchdog
> > is driven by a timer according to a certain period. Each time
> > the timer expires, the counter is decremented. When the counter
> > is "0", the watchdog considers the vcpu to be stalling and resets
> > the VM. To avoid watchdog expiration, the guest kernel driver
> > needs to periodically send a pet event to update the counter.
>
> I'm wondering how this is different/better from the existing
> watchdogs we have in QEMU.
>
> IIUC, the interesting bit is that this watchdog is monitoring
> liveliness of every individual guest VCPU, whereas the existing
> watchdogs are all just a single timer for the whole VM.
>
> IOW, with this watchdog we're proving that *every* vCPU is
> alive, while with other watchdogs we're merely providing that
> at least 1 vCPU is still alive.
>
> Have you got a Linux guest impl for this watchdog that you're
> submitting too ?
Upstream there is a DTB-based driver with the following address:
https://lore.kernel.org/lkml/20220707154226.1478674-1-sebastian...@google.com/T/#m6ffdbdd43e19f608eaa1580d829a2dad8a6cf2f7
Considering that this driver is not compatible with qemu's virtio device,
I re-implemented a virtio driver. I will submit to the upstream
communitiy as soon as possible.
> > Signed-off-by: zhanghao1
> > ---
> > hw/virtio/Kconfig | 5 +
> > hw/virtio/meson.build | 2 +
> > hw/virtio/virtio-vcpu-stall-watchdog-pci.c| 89 +++
> > hw/virtio/virtio-vcpu-stall-watchdog.c| 240 ++
> > .../hw/virtio/virtio-vcpu-stall-watchdog.h| 45
> > 5 files changed, 381 insertions(+)
> > create mode 100644 hw/virtio/virtio-vcpu-stall-watchdog-pci.c
> > create mode 100644 hw/virtio/virtio-vcpu-stall-watchdog.c
> > create mode 100644 include/hw/virtio/virtio-vcpu-stall-watchdog.h
>
> > diff --git a/hw/virtio/virtio-vcpu-stall-watchdog-pci.c
> > b/hw/virtio/virtio-vcpu-stall-watchdog-pci.c
> > new file mode 100644
> > index 00..fce735abc7
> > --- /dev/null
> > +++ b/hw/virtio/virtio-vcpu-stall-watchdog-pci.c
> > @@ -0,0 +1,89 @@
> > +/*
> > + * Virtio cpu stall watchdog PCI Bindings
> > + *
> > + * Copyright 2023 Kylin, Inc.
> > + * Copyright 2023 Hao Zhang
> > + *
> > + * This work is licensed under the terms of the GNU GPL, version 2 or
> > + * (at your option) any later version. See the COPYING file in the
> > + * top-level directory.
> > + */
> > +
> > +#include "qemu/osdep.h"
> > +
> > +#include "hw/virtio/virtio-pci.h"
> > +#include "hw/virtio/virtio-vcpu-stall-watchdog.h"
> > +#include "qapi/error.h"
> > +#include "qemu/module.h"
> > +
> > +typedef struct VirtIOCpuStallWatchdogPCI VirtIOCpuStallWatchdogPCI;
> > +
> > +/*
> > + * virtio-cpu-stall-watchdog-pci: This extends VirtioPCIProxy.
> > + */
> > +#define TYPE_VIRTIO_CPU_STALL_WATCHDOG_PCI
> > "virtio-vcpu-stall-watchdog-pci-base"
> > +#define VIRTIO_CPU_STALL_WATCHDOG_PCI(obj) \
> > +OBJECT_CHECK(VirtIOCpuStallWatchdogPCI, (obj),
> > TYPE_VIRTIO_CPU_STALL_WATCHDOG_PCI)
> > +
> > +struct VirtIOCpuStallWatchdogPCI {
> > +VirtIOPCIProxy parent_obj;
> > +VirtIOCPUSTALLWATCHDOG vdev;
> > +};
> > +
> > +static Property vcpu_stall_watchdog_properties[] = {
> > +DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors,
> > + DEV_NVECTORS_UNSPECIFIED),
> > +DEFINE_PROP_END_OF_LIST(),
> > +};
> > +
> > +static void virtio_vcpu_stall_watchdog_pci_realize(VirtIOPCIProxy
> > *vpci_dev, Error **errp)
> > +{
> > +VirtIOCpuStallWatchdogPCI *dev =
> > VIRTIO_CPU_STALL_WATCHDOG_PCI(vpci_dev);
> > +DeviceState *vdev = DEVICE(&dev->vdev);
> > +
> > +if (vpci_dev->nvectors == DEV_NVECTORS_UNSPECIFIED) {
> > +vpci_dev->nvectors = 1;
> > +}
> > +
> > +if (!qdev_realize(vdev, BUS(&vpci_dev->bus), errp)) {
> > +return;
> > +}
> > +}
> > +
> > +static void virtio_vcpu_stall_watchdog_pci_class_init(ObjectClass *klass,
> > void *data)
> > +{
> > +DeviceClass *dc = DEVICE_CLASS(klass);
> > +VirtioPCIClass *k = VIRTIO_PCI_CLASS(klass);
> > +PCIDeviceClass *pcidev_k = PCI_DEVICE_CLASS(klass);
> > +
> > +k->realize = virtio_vcpu_stall_watchdog_pci_realize;
> > +set_bit(DEVICE_CATEGORY_MISC, dc->categories);
> > +device_class_set_props(dc, vcpu_stall_watchdog_properties);
> > +pcidev_k->revision = VIRTIO_PCI_ABI_VERSION;
> > +pcidev_k->class_id = PCI_CLASS_OTHERS;
> > +}
> > +
> > +static void virtio_vcpu_stall_watchdog_init(Object *obj)
> > +{
> > +VirtIOCpuStallWatchdogPCI *dev = VIRTIO_CPU_STALL_WATCHDOG_PCI(obj);
> > +
> > +virtio_instance_init_common(obj, &dev->vdev, sizeof(dev->vdev),
> > +TYPE_VIRTIO_CPU_STALL_WATCHDOG);
> > +}
> > +
> > +static const VirtioPCIDeviceTypeInfo virtio_vcpu_stall_watchdog_pci_info =
> > {
> > +.base_name = TYPE_VI
[PATCH] kconfig: Add PCIe devices to s390xx machines
It is useful to extend the number of available PCI devices to KVM guests for passthrough scenarios and also to expose these models to a different (big endian) architecture. Signed-off-by: Cédric Le Goater --- hw/s390x/Kconfig | 4 1 file changed, 4 insertions(+) diff --git a/hw/s390x/Kconfig b/hw/s390x/Kconfig index 5e7d8a2bae8b..373f38adcd6b 100644 --- a/hw/s390x/Kconfig +++ b/hw/s390x/Kconfig @@ -10,3 +10,7 @@ config S390_CCW_VIRTIO select SCLPCONSOLE select VIRTIO_CCW select MSI_NONBROKEN +select PCI_EXPRESS +select E1000E_PCI_EXPRESS +select IGB_PCI_EXPRESS +select USB_XHCI_PCI -- 2.41.0
Re: [PATCH 03/13] ppc440: Add a macro to shorten PCIe controller DCR registration
On Tue, 4 Jul 2023, Philippe Mathieu-Daudé wrote: On 4/7/23 00:02, BALATON Zoltan wrote: It is more readable to wrap the complex call to ppc_dcr_register in a macro when needed repeatedly. Signed-off-by: BALATON Zoltan --- hw/ppc/ppc440_uc.c | 76 +- 1 file changed, 28 insertions(+), 48 deletions(-) +#define PPC440_PCIE_DCR(s, dcrn) \ +ppc_dcr_register(&(s)->cpu->env, (s)->dcrn_base + (dcrn), s, \ '(s), \' The parenthesis here would be superfluous as it stands alone in a function parameter between commas so no matter what you substitue here should not have an unwanted side effect (unless it has a comma but that's an error anyway) so maybe this is not needed. + &dcr_read_pcie, &dcr_write_pcie) + + Reviewed-by: Philippe Mathieu-Daudé Thanks for the quick review, I'll post a v2 in a few days to wait a bit if anobody else has any other request. Regards, BALATON Zoltan
Re: [PATCH 10/13] ppc4xx_pci: Add define for ppc4xx-host-bridge type name
On Tue, 4 Jul 2023, Philippe Mathieu-Daudé wrote:
On 4/7/23 00:02, BALATON Zoltan wrote:
Add a QOM type name define for ppc4xx-host-bridge in the common header
and replace direct use of the string name with the constant.
Signed-off-by: BALATON Zoltan
---
hw/ppc/ppc440_pcix.c| 3 ++-
hw/ppc/ppc4xx_pci.c | 4 ++--
include/hw/ppc/ppc4xx.h | 1 +
3 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/hw/ppc/ppc440_pcix.c b/hw/ppc/ppc440_pcix.c
index f10f93c533..dfec25ac83 100644
--- a/hw/ppc/ppc440_pcix.c
+++ b/hw/ppc/ppc440_pcix.c
@@ -495,7 +495,8 @@ static void ppc440_pcix_realize(DeviceState *dev, Error
**errp)
ppc440_pcix_map_irq, &s->irq, &s->busmem,
get_system_io(), PCI_DEVFN(0, 0), 1,
TYPE_PCI_BUS);
-s->dev = pci_create_simple(h->bus, PCI_DEVFN(0, 0),
"ppc4xx-host-bridge");
+s->dev = pci_create_simple(h->bus, PCI_DEVFN(0, 0),
+ TYPE_PPC4xx_HOST_BRIDGE);
memory_region_init(&s->bm, OBJECT(s), "bm-ppc440-pcix",
UINT64_MAX);
memory_region_add_subregion(&s->bm, 0x0, &s->busmem);
diff --git a/hw/ppc/ppc4xx_pci.c b/hw/ppc/ppc4xx_pci.c
index fbdf8266d8..6652119008 100644
--- a/hw/ppc/ppc4xx_pci.c
+++ b/hw/ppc/ppc4xx_pci.c
@@ -333,7 +333,7 @@ static void ppc4xx_pcihost_realize(DeviceState *dev,
Error **errp)
TYPE_PCI_BUS);
h->bus = b;
-pci_create_simple(b, 0, "ppc4xx-host-bridge");
+pci_create_simple(b, 0, TYPE_PPC4xx_HOST_BRIDGE);
/* XXX split into 2 memory regions, one for config space, one for
regs */
memory_region_init(&s->container, OBJECT(s), "pci-container",
PCI_ALL_SIZE);
@@ -367,7 +367,7 @@ static void ppc4xx_host_bridge_class_init(ObjectClass
*klass, void *data)
}
static const TypeInfo ppc4xx_host_bridge_info = {
-.name = "ppc4xx-host-bridge",
+.name = TYPE_PPC4xx_HOST_BRIDGE,
.parent= TYPE_PCI_DEVICE,
.instance_size = sizeof(PCIDevice),
.class_init= ppc4xx_host_bridge_class_init,
diff --git a/include/hw/ppc/ppc4xx.h b/include/hw/ppc/ppc4xx.h
index e053b9751b..766d575e86 100644
--- a/include/hw/ppc/ppc4xx.h
+++ b/include/hw/ppc/ppc4xx.h
@@ -29,6 +29,7 @@
#include "exec/memory.h"
#include "hw/sysbus.h"
+#define TYPE_PPC4xx_HOST_BRIDGE "ppc4xx-host-bridge"
This is the function #0 of the host bridge, maybe:
#define TYPE_PPC4xx_HOST_BRIDGE_FN0 "ppc4xx-pci-host-bridge-fn0"
That's way too long so I'd drop bridge from all of these and maybe name
this ppc4xx-pci-host-fn0 or ppc4xx-pci-host-func (there are no other
functions of the bridge so this shows this is the PCI side of it). I'd
still go for shorted defines and not changing the string types too much.
Would that be acceptable?
Regards,
BALATON Zoltan
#define TYPE_PPC4xx_PCI_HOST "ppc4xx-pci-host"
#define TYPE_PPC460EX_PCIE_HOST "ppc460ex-pcie-host"
Re: QEMU assert (was: [xen-unstable test] 181558: regressions - FAIL)
On Wed, Jun 28, 2023 at 02:31:39PM +0200, Roger Pau Monné wrote: > On Fri, Jun 23, 2023 at 03:04:21PM +, osstest service owner wrote: > > flight 181558 xen-unstable real [real] > > http://logs.test-lab.xenproject.org/osstest/logs/181558/ > > > > Regressions :-( > > > > Tests which did not succeed and are blocking, > > including tests which could not be run: > > test-amd64-amd64-xl-qcow2 21 guest-start/debian.repeat fail REGR. vs. > > 181545 > > The test failing here is hitting the assert in qemu_cond_signal() as > called by worker_thread(): > > #0 __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50 > #1 0x7740b535 in __GI_abort () at abort.c:79 > #2 0x7740b40f in __assert_fail_base (fmt=0x7756cef0 "%s%s%s:%u: > %s%sAssertion `%s' failed.\n%n", assertion=0x5614abcb "cond->initialized", > file=0x5614ab88 "../qemu-xen-dir-remote/util/qemu-thread-posix.c", > line=198, function=) at assert.c:92 > #3 0x774191a2 in __GI___assert_fail (assertion=0x5614abcb > "cond->initialized", file=0x5614ab88 > "../qemu-xen-dir-remote/util/qemu-thread-posix.c", line=198, > function=0x5614ad80 <__PRETTY_FUNCTION__.17104> "qemu_cond_signal") > at assert.c:101 > #4 0x55f1c8d2 in qemu_cond_signal (cond=0x7fffb800db30) at > ../qemu-xen-dir-remote/util/qemu-thread-posix.c:198 > #5 0x55f36973 in worker_thread (opaque=0x7fffb800dab0) at > ../qemu-xen-dir-remote/util/thread-pool.c:129 > #6 0x55f1d1d2 in qemu_thread_start (args=0x7fffb8000b20) at > ../qemu-xen-dir-remote/util/qemu-thread-posix.c:505 > #7 0x775b0fa3 in start_thread (arg=) at > pthread_create.c:486 > #8 0x774e206f in clone () at > ../sysdeps/unix/sysv/linux/x86_64/clone.S:95 > > I've been trying to figure out how it can get in such state, but so > far I had no luck. I'm not a QEMU expert, so it's probably better if > someone else could handle this. > > In the failures I've seen, and the reproduction I have, the assert > triggers in the QEMU dom0 instance responsible for locally-attaching > the disk to dom0 in order to run pygrub. > > This is also with QEMU 7.2, as testing with upstream QEMU is blocked > ATM, so there's a chance it has already been fixed upstream. > > Thanks, Roger. So, I've run a test with the latest QEMU and I can still reproduce the issue. The test also fails with QEMU 7.1.0. But, QEMU 7.0 seems to pass the test, even with a start-stop loop of 200 iteration. So I'll try to find out if something change in that range. Or try to find out why would the thread pool be not initialised properly. Cheers, -- Anthony PERARD
Re: [PATCH 12/13] ppc440_pcix: Don't use iomem for regs
On Tue, 4 Jul 2023, Philippe Mathieu-Daudé wrote: On 4/7/23 00:02, BALATON Zoltan wrote: The iomem memory region is better used for the PCI IO space but currently used for registers. Stop using it for that to allow this to be cleaned up in the next patch. Signed-off-by: BALATON Zoltan --- hw/ppc/ppc440_pcix.c | 7 --- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/hw/ppc/ppc440_pcix.c b/hw/ppc/ppc440_pcix.c index adfecf1e76..ee2dc44f67 100644 --- a/hw/ppc/ppc440_pcix.c +++ b/hw/ppc/ppc440_pcix.c @@ -484,6 +484,7 @@ static void ppc440_pcix_realize(DeviceState *dev, Error **errp) SysBusDevice *sbd = SYS_BUS_DEVICE(dev); PPC440PCIXState *s; PCIHostState *h; +MemoryRegion *regs = g_new(MemoryRegion, 1); Why not hold it within PPC440PCIXState? Because it's never needed after this function. Regards, BALATON Zoltan h = PCI_HOST_BRIDGE(dev); s = PPC440_PCIX_HOST(dev); @@ -507,11 +508,11 @@ static void ppc440_pcix_realize(DeviceState *dev, Error **errp) h, "pci-conf-idx", 4); memory_region_init_io(&h->data_mem, OBJECT(s), &pci_host_data_le_ops, h, "pci-conf-data", 4); -memory_region_init_io(&s->iomem, OBJECT(s), &pci_reg_ops, s, - "pci.reg", PPC440_REG_SIZE); +memory_region_init_io(regs, OBJECT(s), &pci_reg_ops, s, "pci-reg", + PPC440_REG_SIZE); memory_region_add_subregion(&s->container, PCIC0_CFGADDR, &h->conf_mem); memory_region_add_subregion(&s->container, PCIC0_CFGDATA, &h->data_mem); -memory_region_add_subregion(&s->container, PPC440_REG_BASE, &s->iomem); +memory_region_add_subregion(&s->container, PPC440_REG_BASE, regs); sysbus_init_mmio(sbd, &s->container); }
Re: [PATCH] ui/vnc-clipboard: fix infinite loop in inflate_buffer (CVE-2023-3255)
On Tue, Jul 4, 2023 at 11:03 AM Marc-André Lureau
wrote:
>
>
>
> On Tue, Jul 4, 2023 at 10:42 AM Mauro Matteo Cascella
> wrote:
>>
>> A wrong exit condition may lead to an infinite loop when inflating a
>> valid zlib buffer containing some extra bytes in the `inflate_buffer`
>> function. The bug only occurs post-authentication. Return the buffer
>> immediately if the end of the compressed data has been reached
>> (Z_STREAM_END).
>>
>> Fixes: CVE-2023-3255
>> Fixes: 0bf41cab ("ui/vnc: clipboard support")
>> Reported-by: Kevin Denis
>> Signed-off-by: Mauro Matteo Cascella
>
>
> Tested-by: Marc-André Lureau
> Reviewed-by: Marc-André Lureau
>
> Note: we may want to disconnect the client when there are extra bytes in the
> message, or print some warnings.
Sure, I guess we can call vnc_disconnect_finish or vnc_client_error
for disconnecting, not sure how to properly print warnings. Feel free
to add that yourself when applying the patch. Or I can try to send v2
if you prefer.
Thanks,
>>
>> ---
>> ui/vnc-clipboard.c | 10 --
>> 1 file changed, 4 insertions(+), 6 deletions(-)
>>
>> diff --git a/ui/vnc-clipboard.c b/ui/vnc-clipboard.c
>> index 8aeadfaa21..c759be3438 100644
>> --- a/ui/vnc-clipboard.c
>> +++ b/ui/vnc-clipboard.c
>> @@ -50,8 +50,11 @@ static uint8_t *inflate_buffer(uint8_t *in, uint32_t
>> in_len, uint32_t *size)
>> ret = inflate(&stream, Z_FINISH);
>> switch (ret) {
>> case Z_OK:
>> -case Z_STREAM_END:
>> break;
>> +case Z_STREAM_END:
>> +*size = stream.total_out;
>> +inflateEnd(&stream);
>> +return out;
>> case Z_BUF_ERROR:
>> out_len <<= 1;
>> if (out_len > (1 << 20)) {
>> @@ -66,11 +69,6 @@ static uint8_t *inflate_buffer(uint8_t *in, uint32_t
>> in_len, uint32_t *size)
>> }
>> }
>>
>> -*size = stream.total_out;
>> -inflateEnd(&stream);
>> -
>> -return out;
>> -
>> err_end:
>> inflateEnd(&stream);
>> err:
>> --
>> 2.41.0
>>
>>
>
>
> --
> Marc-André Lureau
--
Mauro Matteo Cascella
Red Hat Product Security
PGP-Key ID: BB3410B0
Re: [PATCH 05/13] ppc440: Stop using system io region for PCIe buses
On Tue, 4 Jul 2023, Philippe Mathieu-Daudé wrote:
On 4/7/23 00:02, BALATON Zoltan wrote:
Add separate memory regions for the mem and io spaces of the PCIe bus
to avoid different buses using the same system io region.
"Reduce the I/O space to 64K."
Unlike the other similar patch this does not reduce the IO space size
because get_system_io() was that size already. I've changed the size below
to use KiB.
Regards,
BALATON Zoltan
Signed-off-by: BALATON Zoltan
---
hw/ppc/ppc440_uc.c | 9 ++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/hw/ppc/ppc440_uc.c b/hw/ppc/ppc440_uc.c
index 38ee27f437..0c5d999878 100644
--- a/hw/ppc/ppc440_uc.c
+++ b/hw/ppc/ppc440_uc.c
@@ -776,6 +776,7 @@ OBJECT_DECLARE_SIMPLE_TYPE(PPC460EXPCIEState,
PPC460EX_PCIE_HOST)
struct PPC460EXPCIEState {
PCIExpressHost host;
+MemoryRegion busmem;
MemoryRegion iomem;
qemu_irq irq[4];
int32_t dcrn_base;
@@ -1056,15 +1057,17 @@ static void ppc460ex_pcie_realize(DeviceState *dev,
Error **errp)
error_setg(errp, "invalid PCIe DCRN base");
return;
}
+snprintf(buf, sizeof(buf), "pcie%d-mem", id);
+memory_region_init(&s->busmem, OBJECT(s), buf, UINT64_MAX);
snprintf(buf, sizeof(buf), "pcie%d-io", id);
-memory_region_init(&s->iomem, OBJECT(s), buf, UINT64_MAX);
+memory_region_init(&s->iomem, OBJECT(s), buf, 0x1);
64 * KiB
for (i = 0; i < 4; i++) {
sysbus_init_irq(SYS_BUS_DEVICE(dev), &s->irq[i]);
}
snprintf(buf, sizeof(buf), "pcie.%d", id);
pci->bus = pci_register_root_bus(DEVICE(s), buf, ppc460ex_set_irq,
-pci_swizzle_map_irq_fn, s, &s->iomem,
-get_system_io(), 0, 4, TYPE_PCIE_BUS);
+pci_swizzle_map_irq_fn, s, &s->busmem,
+&s->iomem, 0, 4, TYPE_PCIE_BUS);
ppc460ex_pcie_register_dcrs(s);
}
With the changes addressed:
Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH 03/13] ppc440: Add a macro to shorten PCIe controller DCR registration
On 4/7/23 11:33, BALATON Zoltan wrote: On Tue, 4 Jul 2023, Philippe Mathieu-Daudé wrote: On 4/7/23 00:02, BALATON Zoltan wrote: It is more readable to wrap the complex call to ppc_dcr_register in a macro when needed repeatedly. Signed-off-by: BALATON Zoltan --- hw/ppc/ppc440_uc.c | 76 +- 1 file changed, 28 insertions(+), 48 deletions(-) +#define PPC440_PCIE_DCR(s, dcrn) \ + ppc_dcr_register(&(s)->cpu->env, (s)->dcrn_base + (dcrn), s, \ '(s), \' The parenthesis here would be superfluous as it stands alone in a function parameter between commas so no matter what you substitue here should not have an unwanted side effect (unless it has a comma but that's an error anyway) so maybe this is not needed. Well I noticed because you used it for the 2 other cases, so I'm just trying to be consistent here. Besides, not using parenthesis for macro arguments is a bad practice. Problems happen when others copy code. + &dcr_read_pcie, &dcr_write_pcie) + + Reviewed-by: Philippe Mathieu-Daudé Thanks for the quick review, I'll post a v2 in a few days to wait a bit if anobody else has any other request. Regards, BALATON Zoltan
Re: QEMU assert (was: [xen-unstable test] 181558: regressions - FAIL)
On Tue, Jul 04, 2023 at 10:37:38AM +0100, Anthony PERARD wrote: > On Wed, Jun 28, 2023 at 02:31:39PM +0200, Roger Pau Monné wrote: > > On Fri, Jun 23, 2023 at 03:04:21PM +, osstest service owner wrote: > > > flight 181558 xen-unstable real [real] > > > http://logs.test-lab.xenproject.org/osstest/logs/181558/ > > > > > > Regressions :-( > > > > > > Tests which did not succeed and are blocking, > > > including tests which could not be run: > > > test-amd64-amd64-xl-qcow2 21 guest-start/debian.repeat fail REGR. vs. > > > 181545 > > > > The test failing here is hitting the assert in qemu_cond_signal() as > > called by worker_thread(): > > > > #0 __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50 > > #1 0x7740b535 in __GI_abort () at abort.c:79 > > #2 0x7740b40f in __assert_fail_base (fmt=0x7756cef0 > > "%s%s%s:%u: %s%sAssertion `%s' failed.\n%n", assertion=0x5614abcb > > "cond->initialized", > > file=0x5614ab88 "../qemu-xen-dir-remote/util/qemu-thread-posix.c", > > line=198, function=) at assert.c:92 > > #3 0x774191a2 in __GI___assert_fail (assertion=0x5614abcb > > "cond->initialized", file=0x5614ab88 > > "../qemu-xen-dir-remote/util/qemu-thread-posix.c", line=198, > > function=0x5614ad80 <__PRETTY_FUNCTION__.17104> "qemu_cond_signal") > > at assert.c:101 > > #4 0x55f1c8d2 in qemu_cond_signal (cond=0x7fffb800db30) at > > ../qemu-xen-dir-remote/util/qemu-thread-posix.c:198 > > #5 0x55f36973 in worker_thread (opaque=0x7fffb800dab0) at > > ../qemu-xen-dir-remote/util/thread-pool.c:129 > > #6 0x55f1d1d2 in qemu_thread_start (args=0x7fffb8000b20) at > > ../qemu-xen-dir-remote/util/qemu-thread-posix.c:505 > > #7 0x775b0fa3 in start_thread (arg=) at > > pthread_create.c:486 > > #8 0x774e206f in clone () at > > ../sysdeps/unix/sysv/linux/x86_64/clone.S:95 > > > > I've been trying to figure out how it can get in such state, but so > > far I had no luck. I'm not a QEMU expert, so it's probably better if > > someone else could handle this. > > > > In the failures I've seen, and the reproduction I have, the assert > > triggers in the QEMU dom0 instance responsible for locally-attaching > > the disk to dom0 in order to run pygrub. > > > > This is also with QEMU 7.2, as testing with upstream QEMU is blocked > > ATM, so there's a chance it has already been fixed upstream. > > > > Thanks, Roger. > > So, I've run a test with the latest QEMU and I can still reproduce the > issue. The test also fails with QEMU 7.1.0. > > But, QEMU 7.0 seems to pass the test, even with a start-stop loop of 200 > iteration. So I'll try to find out if something change in that range. > Or try to find out why would the thread pool be not initialised > properly. Thanks for looking into this. There are a set of changes from Paolo Bonzini: 232e9255478f thread-pool: remove stopping variable 900fa208f506 thread-pool: replace semaphore with condition variable 3c7b72ddca9c thread-pool: optimize scheduling of completion bottom half That landed in 7.1 that seem like possible candidates. Roger.
Re: [PATCH 12/13] ppc440_pcix: Don't use iomem for regs
On 4/7/23 11:37, BALATON Zoltan wrote: On Tue, 4 Jul 2023, Philippe Mathieu-Daudé wrote: On 4/7/23 00:02, BALATON Zoltan wrote: The iomem memory region is better used for the PCI IO space but currently used for registers. Stop using it for that to allow this to be cleaned up in the next patch. Signed-off-by: BALATON Zoltan --- hw/ppc/ppc440_pcix.c | 7 --- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/hw/ppc/ppc440_pcix.c b/hw/ppc/ppc440_pcix.c index adfecf1e76..ee2dc44f67 100644 --- a/hw/ppc/ppc440_pcix.c +++ b/hw/ppc/ppc440_pcix.c @@ -484,6 +484,7 @@ static void ppc440_pcix_realize(DeviceState *dev, Error **errp) SysBusDevice *sbd = SYS_BUS_DEVICE(dev); PPC440PCIXState *s; PCIHostState *h; + MemoryRegion *regs = g_new(MemoryRegion, 1); Why not hold it within PPC440PCIXState? Because it's never needed after this function. But we can't free() it because it has to stay valid as long as PPC440PCIXState is in use. So it seems to belong there. Regards, BALATON Zoltan h = PCI_HOST_BRIDGE(dev); s = PPC440_PCIX_HOST(dev); @@ -507,11 +508,11 @@ static void ppc440_pcix_realize(DeviceState *dev, Error **errp) h, "pci-conf-idx", 4); memory_region_init_io(&h->data_mem, OBJECT(s), &pci_host_data_le_ops, h, "pci-conf-data", 4); - memory_region_init_io(&s->iomem, OBJECT(s), &pci_reg_ops, s, - "pci.reg", PPC440_REG_SIZE); + memory_region_init_io(regs, OBJECT(s), &pci_reg_ops, s, "pci-reg", + PPC440_REG_SIZE); memory_region_add_subregion(&s->container, PCIC0_CFGADDR, &h->conf_mem); memory_region_add_subregion(&s->container, PCIC0_CFGDATA, &h->data_mem); - memory_region_add_subregion(&s->container, PPC440_REG_BASE, &s->iomem); + memory_region_add_subregion(&s->container, PPC440_REG_BASE, regs); sysbus_init_mmio(sbd, &s->container); }
RE: [PATCH v1 0/5] target/arm: Handle psci calls in userspace
Hi Shaoqin, Just saw this. Apologies. I missed to reply this earlier as I was bit disconnected for last few days. > From: Shaoqin Huang > Sent: Tuesday, June 27, 2023 3:35 AM > Hi Salil, > > On 6/26/23 21:42, Salil Mehta wrote: > >> From: Shaoqin Huang > >> Sent: Monday, June 26, 2023 7:49 AM > >> To: qemu-devel@nongnu.org; qemu-...@nongnu.org > >> Cc: oliver.up...@linux.dev; Salil Mehta ; > >> james.mo...@arm.com; gs...@redhat.com; Shaoqin Huang ; > >> Cornelia Huck ; k...@vger.kernel.org; Michael S. Tsirkin > >> ; Paolo Bonzini ; Peter Maydell > >> > >> Subject: [PATCH v1 0/5] target/arm: Handle psci calls in userspace > >> > >> The userspace SMCCC call filtering[1] provides the ability to forward the > >> SMCCC > >> calls to the userspace. The vCPU hotplug[2] would be the first legitimate > >> use > >> case to handle the psci calls in userspace, thus the vCPU hotplug can deny > >> the > >> PSCI_ON call if the vCPU is not present now. > >> > >> This series try to enable the userspace SMCCC call filtering, thus can > >> handle > >> the SMCCC call in userspace. The first enabled SMCCC call is psci call, by > >> using > >> the new added option 'user-smccc', we can enable handle psci calls in > >> userspace. > >> > >> qemu-system-aarch64 -machine virt,user-smccc=on > >> > >> This series reuse the qemu implementation of the psci handling, thus the > >> handling process is very simple. But when handling psci in userspace when > >> using > >> kvm, the reset vcpu process need to be taking care, the detail is included > >> in > >> the patch05. > > > > This change is intended for VCPU Hotplug and are duplicating the code > > we are working on. Unless this change is also intended for any other > > feature I would request you to defer this. > > Thanks for sharing me the information. I'm not intended for merging this > series, but discuss something about the VCPU Hotplug, since I'm also > following the work of vCPU Hotplug. Sure. I am not against this work in any way but there was bit of an overlap and was trying to avoid that. > > Just curious, what is your plan to update a new version of VCPU Hotplug > which is based on the userspace SMCCC filtering? We have already incorporated this. We have not tested it properly though and there are some issues related to the migration we are fixing. I did mention about this in the KVMForum2023 presentation as well. Latest Qemu Prototype (Pre RFC V2) (Not in the final shape of the patches) https://github.com/salil-mehta/qemu.git virt-cpuhp-armv8/rfc-v1-port11052023.dev-1 should work against below kernel changes as confirmed by James, Latest Kernel Prototype (Pre RFC V2 = RFC V1 + Fixes) https://git.gitlab.arm.com/linux-arm/linux-jm.git virtual_cpu_hotplug/rfc/v2 We have not added the support of user-configurability which your patch-set does. Many thanks Salil.
Re: [PATCH v2 1/5] ppc/pnv: quad xscom callbacks are P9 specific
On 04/07/2023 07:42, Joel Stanley wrote:
Rename the functions to include P9 in the name in preparation for adding
P10 versions.
Correct the unimp read message while we're changing the function.
Reviewed-by: Cédric Le Goater
Signed-off-by: Joel Stanley
---
Reviewed-by: Frederic Barrat
v2: Fix unimp print, and grammar in the commit message
---
hw/ppc/pnv_core.c | 19 ++-
1 file changed, 10 insertions(+), 9 deletions(-)
diff --git a/hw/ppc/pnv_core.c b/hw/ppc/pnv_core.c
index 0bc3ad41c81c..0f451b3b6e1f 100644
--- a/hw/ppc/pnv_core.c
+++ b/hw/ppc/pnv_core.c
@@ -360,8 +360,8 @@ DEFINE_TYPES(pnv_core_infos)
#define P9X_EX_NCU_SPEC_BAR 0x11010
-static uint64_t pnv_quad_xscom_read(void *opaque, hwaddr addr,
-unsigned int width)
+static uint64_t pnv_quad_power9_xscom_read(void *opaque, hwaddr addr,
+ unsigned int width)
{
uint32_t offset = addr >> 3;
uint64_t val = -1;
@@ -372,15 +372,15 @@ static uint64_t pnv_quad_xscom_read(void *opaque, hwaddr
addr,
val = 0;
break;
default:
-qemu_log_mask(LOG_UNIMP, "%s: writing @0x%08x\n", __func__,
+qemu_log_mask(LOG_UNIMP, "%s: reading @0x%08x\n", __func__,
offset);
}
return val;
}
-static void pnv_quad_xscom_write(void *opaque, hwaddr addr, uint64_t val,
- unsigned int width)
+static void pnv_quad_power9_xscom_write(void *opaque, hwaddr addr, uint64_t
val,
+unsigned int width)
{
uint32_t offset = addr >> 3;
@@ -394,9 +394,9 @@ static void pnv_quad_xscom_write(void *opaque, hwaddr addr, uint64_t val,
}
}
-static const MemoryRegionOps pnv_quad_xscom_ops = {
-.read = pnv_quad_xscom_read,
-.write = pnv_quad_xscom_write,
+static const MemoryRegionOps pnv_quad_power9_xscom_ops = {
+.read = pnv_quad_power9_xscom_read,
+.write = pnv_quad_power9_xscom_write,
.valid.min_access_size = 8,
.valid.max_access_size = 8,
.impl.min_access_size = 8,
@@ -410,7 +410,8 @@ static void pnv_quad_realize(DeviceState *dev, Error **errp)
char name[32];
snprintf(name, sizeof(name), "xscom-quad.%d", eq->quad_id);
-pnv_xscom_region_init(&eq->xscom_regs, OBJECT(dev), &pnv_quad_xscom_ops,
+pnv_xscom_region_init(&eq->xscom_regs, OBJECT(dev),
+ &pnv_quad_power9_xscom_ops,
eq, name, PNV9_XSCOM_EQ_SIZE);
}
Re: [PATCH v2 2/5] ppc/pnv: Subclass quad xscom callbacks
On 04/07/2023 07:42, Joel Stanley wrote:
Make the existing pnv_quad_xscom_read/write be P9 specific, in
preparation for a different P10 callback.
Reviewed-by: Cédric Le Goater
Signed-off-by: Joel Stanley
---
Reviewed-by: Frederic Barrat
Fred
v2: Add scom region size to class
---
include/hw/ppc/pnv_core.h | 13 -
hw/ppc/pnv.c | 11 +++
hw/ppc/pnv_core.c | 40 ++-
3 files changed, 46 insertions(+), 18 deletions(-)
diff --git a/include/hw/ppc/pnv_core.h b/include/hw/ppc/pnv_core.h
index 3d75706e95da..77ef00f47a72 100644
--- a/include/hw/ppc/pnv_core.h
+++ b/include/hw/ppc/pnv_core.h
@@ -60,8 +60,19 @@ static inline PnvCPUState *pnv_cpu_state(PowerPCCPU *cpu)
return (PnvCPUState *)cpu->machine_data;
}
+struct PnvQuadClass {
+DeviceClass parent_class;
+
+const MemoryRegionOps *xscom_ops;
+uint64_t xscom_size;
+};
+
#define TYPE_PNV_QUAD "powernv-cpu-quad"
-OBJECT_DECLARE_SIMPLE_TYPE(PnvQuad, PNV_QUAD)
+
+#define PNV_QUAD_TYPE_SUFFIX "-" TYPE_PNV_QUAD
+#define PNV_QUAD_TYPE_NAME(cpu_model) cpu_model PNV_QUAD_TYPE_SUFFIX
+
+OBJECT_DECLARE_TYPE(PnvQuad, PnvQuadClass, PNV_QUAD)
struct PnvQuad {
DeviceState parent_obj;
diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index fc083173f346..c77fdb6747a4 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -1429,14 +1429,15 @@ static void pnv_chip_power9_instance_init(Object *obj)
}
static void pnv_chip_quad_realize_one(PnvChip *chip, PnvQuad *eq,
- PnvCore *pnv_core)
+ PnvCore *pnv_core,
+ const char *type)
{
char eq_name[32];
int core_id = CPU_CORE(pnv_core)->core_id;
snprintf(eq_name, sizeof(eq_name), "eq[%d]", core_id);
object_initialize_child_with_props(OBJECT(chip), eq_name, eq,
- sizeof(*eq), TYPE_PNV_QUAD,
+ sizeof(*eq), type,
&error_fatal, NULL);
object_property_set_int(OBJECT(eq), "quad-id", core_id, &error_fatal);
@@ -1454,7 +1455,8 @@ static void pnv_chip_quad_realize(Pnv9Chip *chip9, Error
**errp)
for (i = 0; i < chip9->nr_quads; i++) {
PnvQuad *eq = &chip9->quads[i];
-pnv_chip_quad_realize_one(chip, eq, chip->cores[i * 4]);
+pnv_chip_quad_realize_one(chip, eq, chip->cores[i * 4],
+ PNV_QUAD_TYPE_NAME("power9"));
pnv_xscom_add_subregion(chip, PNV9_XSCOM_EQ_BASE(eq->quad_id),
&eq->xscom_regs);
@@ -1666,7 +1668,8 @@ static void pnv_chip_power10_quad_realize(Pnv10Chip
*chip10, Error **errp)
for (i = 0; i < chip10->nr_quads; i++) {
PnvQuad *eq = &chip10->quads[i];
-pnv_chip_quad_realize_one(chip, eq, chip->cores[i * 4]);
+pnv_chip_quad_realize_one(chip, eq, chip->cores[i * 4],
+ PNV_QUAD_TYPE_NAME("power9"));
pnv_xscom_add_subregion(chip, PNV10_XSCOM_EQ_BASE(eq->quad_id),
&eq->xscom_regs);
diff --git a/hw/ppc/pnv_core.c b/hw/ppc/pnv_core.c
index 0f451b3b6e1f..73d25409c937 100644
--- a/hw/ppc/pnv_core.c
+++ b/hw/ppc/pnv_core.c
@@ -407,12 +407,14 @@ static const MemoryRegionOps pnv_quad_power9_xscom_ops = {
static void pnv_quad_realize(DeviceState *dev, Error **errp)
{
PnvQuad *eq = PNV_QUAD(dev);
+PnvQuadClass *pqc = PNV_QUAD_GET_CLASS(eq);
char name[32];
snprintf(name, sizeof(name), "xscom-quad.%d", eq->quad_id);
pnv_xscom_region_init(&eq->xscom_regs, OBJECT(dev),
- &pnv_quad_power9_xscom_ops,
- eq, name, PNV9_XSCOM_EQ_SIZE);
+ pqc->xscom_ops,
+ eq, name,
+ pqc->xscom_size);
}
static Property pnv_quad_properties[] = {
@@ -420,6 +422,14 @@ static Property pnv_quad_properties[] = {
DEFINE_PROP_END_OF_LIST(),
};
+static void pnv_quad_power9_class_init(ObjectClass *oc, void *data)
+{
+PnvQuadClass *pqc = PNV_QUAD_CLASS(oc);
+
+pqc->xscom_ops = &pnv_quad_power9_xscom_ops;
+pqc->xscom_size = PNV9_XSCOM_EQ_SIZE;
+}
+
static void pnv_quad_class_init(ObjectClass *oc, void *data)
{
DeviceClass *dc = DEVICE_CLASS(oc);
@@ -429,16 +439,20 @@ static void pnv_quad_class_init(ObjectClass *oc, void
*data)
dc->user_creatable = false;
}
-static const TypeInfo pnv_quad_info = {
-.name = TYPE_PNV_QUAD,
-.parent= TYPE_DEVICE,
-.instance_size = sizeof(PnvQuad),
-.class_init= pnv_quad_class_init,
+static const TypeInfo pnv_quad_infos[] = {
+{
+.name = TYPE_PNV_QUAD,
+.parent= TYPE_DEVICE,
+.instance_size = sizeof(PnvQuad),
+.class_size= sizeof(PnvQuadClass
Re: [PATCH 07/15] hw/timer/arm_timer: Extract arm_timer_reset()
On 8/6/23 16:46, Peter Maydell wrote:
On Wed, 31 May 2023 at 21:36, Philippe Mathieu-Daudé wrote:
Extract arm_timer_reset() before converting this model to QOM/QDev
in few commits. This will become our DeviceReset handler.
Signed-off-by: Philippe Mathieu-Daudé
---
hw/timer/arm_timer.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
+static void arm_timer_reset(ArmTimerState *s)
+{
+s->control = TIMER_CTRL_IE;
+}
Reset also should zero s->limit and s->int_level.
(in arm_timer_init() this was implicit in the g_new0()).)
Oops I missed that, thanks!
Re: [PATCH] kconfig: Add PCIe devices to s390xx machines
On 04/07/2023 11.32, Cédric Le Goater wrote: It is useful to extend the number of available PCI devices to KVM guests for passthrough scenarios and also to expose these models to a different (big endian) architecture. Maybe mention that these devices can work on s390x since they support MSI-X ? (While most of the other devices don't work on s390x since they only support legacy interrupts) Signed-off-by: Cédric Le Goater --- hw/s390x/Kconfig | 4 1 file changed, 4 insertions(+) diff --git a/hw/s390x/Kconfig b/hw/s390x/Kconfig index 5e7d8a2bae8b..373f38adcd6b 100644 --- a/hw/s390x/Kconfig +++ b/hw/s390x/Kconfig @@ -10,3 +10,7 @@ config S390_CCW_VIRTIO select SCLPCONSOLE select VIRTIO_CCW select MSI_NONBROKEN +select PCI_EXPRESS +select E1000E_PCI_EXPRESS +select IGB_PCI_EXPRESS +select USB_XHCI_PCI Please don't use "select" here - you still want these devices to be disabled in case you run configure with "--without-default-devices". You can use "imply" instead of "select" instead. Thomas
Re: [PATCH v2 3/5] ppc/pnv: Add P10 quad xscom model
On 04/07/2023 07:42, Joel Stanley wrote:
Add a PnvQuad class for the P10 powernv machine. No xscoms are
implemented yet, but this allows them to be added.
The size is reduced to avoid the quad region from overlapping with the
core region.
address-space: xscom-0
-0003 (prio 0, i/o): xscom-0
0001-0001000f (prio 0, i/o): xscom-quad.0
000100108000-000100907fff (prio 0, i/o): xscom-core.3
00010011-00010090 (prio 0, i/o): xscom-core.2
00010012-00010091 (prio 0, i/o): xscom-core.1
00010014-00010093 (prio 0, i/o): xscom-core.0
Signed-off-by: Joel Stanley
---
Reviewed-by: Frederic Barrat
Fred
v2: Fix unimp read message
Wrap lines at 80 col
Set size
---
include/hw/ppc/pnv_xscom.h | 2 +-
hw/ppc/pnv.c | 2 +-
hw/ppc/pnv_core.c | 54 ++
3 files changed, 56 insertions(+), 2 deletions(-)
diff --git a/include/hw/ppc/pnv_xscom.h b/include/hw/ppc/pnv_xscom.h
index cbe848d27ba0..f7da9a1dc617 100644
--- a/include/hw/ppc/pnv_xscom.h
+++ b/include/hw/ppc/pnv_xscom.h
@@ -129,7 +129,7 @@ struct PnvXScomInterfaceClass {
#define PNV10_XSCOM_EQ_BASE(core) \
((uint64_t) PNV10_XSCOM_EQ(PNV10_XSCOM_EQ_CHIPLET(core)))
-#define PNV10_XSCOM_EQ_SIZE0x10
+#define PNV10_XSCOM_EQ_SIZE0x2
#define PNV10_XSCOM_EC_BASE(core) \
((uint64_t) PNV10_XSCOM_EQ_BASE(core) | PNV10_XSCOM_EC(core & 0x3))
diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index c77fdb6747a4..5f25fe985ab2 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -1669,7 +1669,7 @@ static void pnv_chip_power10_quad_realize(Pnv10Chip
*chip10, Error **errp)
PnvQuad *eq = &chip10->quads[i];
pnv_chip_quad_realize_one(chip, eq, chip->cores[i * 4],
- PNV_QUAD_TYPE_NAME("power9"));
+ PNV_QUAD_TYPE_NAME("power10"));
pnv_xscom_add_subregion(chip, PNV10_XSCOM_EQ_BASE(eq->quad_id),
&eq->xscom_regs);
diff --git a/hw/ppc/pnv_core.c b/hw/ppc/pnv_core.c
index 73d25409c937..e4df435b15e9 100644
--- a/hw/ppc/pnv_core.c
+++ b/hw/ppc/pnv_core.c
@@ -404,6 +404,47 @@ static const MemoryRegionOps pnv_quad_power9_xscom_ops = {
.endianness = DEVICE_BIG_ENDIAN,
};
+/*
+ * POWER10 Quads
+ */
+
+static uint64_t pnv_quad_power10_xscom_read(void *opaque, hwaddr addr,
+unsigned int width)
+{
+uint32_t offset = addr >> 3;
+uint64_t val = -1;
+
+switch (offset) {
+default:
+qemu_log_mask(LOG_UNIMP, "%s: reading @0x%08x\n", __func__,
+ offset);
+}
+
+return val;
+}
+
+static void pnv_quad_power10_xscom_write(void *opaque, hwaddr addr,
+ uint64_t val, unsigned int width)
+{
+uint32_t offset = addr >> 3;
+
+switch (offset) {
+default:
+qemu_log_mask(LOG_UNIMP, "%s: writing @0x%08x\n", __func__,
+ offset);
+}
+}
+
+static const MemoryRegionOps pnv_quad_power10_xscom_ops = {
+.read = pnv_quad_power10_xscom_read,
+.write = pnv_quad_power10_xscom_write,
+.valid.min_access_size = 8,
+.valid.max_access_size = 8,
+.impl.min_access_size = 8,
+.impl.max_access_size = 8,
+.endianness = DEVICE_BIG_ENDIAN,
+};
+
static void pnv_quad_realize(DeviceState *dev, Error **errp)
{
PnvQuad *eq = PNV_QUAD(dev);
@@ -430,6 +471,14 @@ static void pnv_quad_power9_class_init(ObjectClass *oc,
void *data)
pqc->xscom_size = PNV9_XSCOM_EQ_SIZE;
}
+static void pnv_quad_power10_class_init(ObjectClass *oc, void *data)
+{
+PnvQuadClass *pqc = PNV_QUAD_CLASS(oc);
+
+pqc->xscom_ops = &pnv_quad_power10_xscom_ops;
+pqc->xscom_size = PNV10_XSCOM_EQ_SIZE;
+}
+
static void pnv_quad_class_init(ObjectClass *oc, void *data)
{
DeviceClass *dc = DEVICE_CLASS(oc);
@@ -453,6 +502,11 @@ static const TypeInfo pnv_quad_infos[] = {
.name = PNV_QUAD_TYPE_NAME("power9"),
.class_init = pnv_quad_power9_class_init,
},
+{
+.parent = TYPE_PNV_QUAD,
+.name = PNV_QUAD_TYPE_NAME("power10"),
+.class_init = pnv_quad_power10_class_init,
+},
};
DEFINE_TYPES(pnv_quad_infos);
Re: [PATCH v2 5/5] ppc/pnv: Return zero for core thread state xscom
On 04/07/2023 07:42, Joel Stanley wrote:
Firmware now warns if booting in LPAR per core mode (PPC bit 62). So
this warning doesn't trigger, report the core thread state is 0.
Reviewed-by: Cédric Le Goater
Signed-off-by: Joel Stanley
---
Reviewed-by: Frederic Barrat
Fred
hw/ppc/pnv_core.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/hw/ppc/pnv_core.c b/hw/ppc/pnv_core.c
index 1eec28c88c41..b7223bb44597 100644
--- a/hw/ppc/pnv_core.c
+++ b/hw/ppc/pnv_core.c
@@ -116,6 +116,8 @@ static const MemoryRegionOps pnv_core_power8_xscom_ops = {
#define PNV9_XSCOM_EC_PPM_SPECIAL_WKUP_HYP 0xf010d
#define PNV9_XSCOM_EC_PPM_SPECIAL_WKUP_OTR 0xf010a
+#define PNV9_XSCOM_EC_CORE_THREAD_STATE0x10ab3
+
static uint64_t pnv_core_power9_xscom_read(void *opaque, hwaddr addr,
unsigned int width)
{
@@ -134,6 +136,9 @@ static uint64_t pnv_core_power9_xscom_read(void *opaque,
hwaddr addr,
case PNV9_XSCOM_EC_PPM_SPECIAL_WKUP_OTR:
val = 0x0;
break;
+case PNV9_XSCOM_EC_CORE_THREAD_STATE:
+val = 0;
+break;
default:
qemu_log_mask(LOG_UNIMP, "Warning: reading reg=0x%" HWADDR_PRIx "\n",
addr);
@@ -171,6 +176,8 @@ static const MemoryRegionOps pnv_core_power9_xscom_ops = {
* POWER10 core controls
*/
+#define PNV10_XSCOM_EC_CORE_THREAD_STATE0x412
+
static uint64_t pnv_core_power10_xscom_read(void *opaque, hwaddr addr,
unsigned int width)
{
@@ -178,6 +185,9 @@ static uint64_t pnv_core_power10_xscom_read(void *opaque,
hwaddr addr,
uint64_t val = 0;
switch (offset) {
+case PNV10_XSCOM_EC_CORE_THREAD_STATE:
+val = 0;
+break;
default:
qemu_log_mask(LOG_UNIMP, "Warning: reading reg=0x%" HWADDR_PRIx "\n",
addr);
Re: [PATCH v2 4/5] ppc/pnv: Add P10 core xscom model
On 04/07/2023 07:42, Joel Stanley wrote:
Like the quad xscoms, add a core model for P10 to allow future
differentiation from P9.
Signed-off-by: Joel Stanley
---
Reviewed-by: Frederic Barrat
Fred
hw/ppc/pnv_core.c | 44 ++--
1 file changed, 42 insertions(+), 2 deletions(-)
diff --git a/hw/ppc/pnv_core.c b/hw/ppc/pnv_core.c
index e4df435b15e9..1eec28c88c41 100644
--- a/hw/ppc/pnv_core.c
+++ b/hw/ppc/pnv_core.c
@@ -167,6 +167,47 @@ static const MemoryRegionOps pnv_core_power9_xscom_ops = {
.endianness = DEVICE_BIG_ENDIAN,
};
+/*
+ * POWER10 core controls
+ */
+
+static uint64_t pnv_core_power10_xscom_read(void *opaque, hwaddr addr,
+ unsigned int width)
+{
+uint32_t offset = addr >> 3;
+uint64_t val = 0;
+
+switch (offset) {
+default:
+qemu_log_mask(LOG_UNIMP, "Warning: reading reg=0x%" HWADDR_PRIx "\n",
+ addr);
+}
+
+return val;
+}
+
+static void pnv_core_power10_xscom_write(void *opaque, hwaddr addr,
+ uint64_t val, unsigned int width)
+{
+uint32_t offset = addr >> 3;
+
+switch (offset) {
+default:
+qemu_log_mask(LOG_UNIMP, "Warning: writing to reg=0x%" HWADDR_PRIx
"\n",
+ addr);
+}
+}
+
+static const MemoryRegionOps pnv_core_power10_xscom_ops = {
+.read = pnv_core_power10_xscom_read,
+.write = pnv_core_power10_xscom_write,
+.valid.min_access_size = 8,
+.valid.max_access_size = 8,
+.impl.min_access_size = 8,
+.impl.max_access_size = 8,
+.endianness = DEVICE_BIG_ENDIAN,
+};
+
static void pnv_core_cpu_realize(PnvCore *pc, PowerPCCPU *cpu, Error **errp)
{
CPUPPCState *env = &cpu->env;
@@ -315,8 +356,7 @@ static void pnv_core_power10_class_init(ObjectClass *oc,
void *data)
{
PnvCoreClass *pcc = PNV_CORE_CLASS(oc);
-/* TODO: Use the P9 XSCOMs for now on P10 */
-pcc->xscom_ops = &pnv_core_power9_xscom_ops;
+pcc->xscom_ops = &pnv_core_power10_xscom_ops;
}
static void pnv_core_class_init(ObjectClass *oc, void *data)
Re: [PATCH 12/13] ppc440_pcix: Don't use iomem for regs
On Tue, 4 Jul 2023, Philippe Mathieu-Daudé wrote: On 4/7/23 11:37, BALATON Zoltan wrote: On Tue, 4 Jul 2023, Philippe Mathieu-Daudé wrote: On 4/7/23 00:02, BALATON Zoltan wrote: The iomem memory region is better used for the PCI IO space but currently used for registers. Stop using it for that to allow this to be cleaned up in the next patch. Signed-off-by: BALATON Zoltan --- hw/ppc/ppc440_pcix.c | 7 --- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/hw/ppc/ppc440_pcix.c b/hw/ppc/ppc440_pcix.c index adfecf1e76..ee2dc44f67 100644 --- a/hw/ppc/ppc440_pcix.c +++ b/hw/ppc/ppc440_pcix.c @@ -484,6 +484,7 @@ static void ppc440_pcix_realize(DeviceState *dev, Error **errp) SysBusDevice *sbd = SYS_BUS_DEVICE(dev); PPC440PCIXState *s; PCIHostState *h; + MemoryRegion *regs = g_new(MemoryRegion, 1); Why not hold it within PPC440PCIXState? Because it's never needed after this function. But we can't free() it because it has to stay valid as long as PPC440PCIXState is in use. So it seems to belong there. OK, moved it to PPC440PCIXState. Regards, BALATON Zoltan h = PCI_HOST_BRIDGE(dev); s = PPC440_PCIX_HOST(dev); @@ -507,11 +508,11 @@ static void ppc440_pcix_realize(DeviceState *dev, Error **errp) h, "pci-conf-idx", 4); memory_region_init_io(&h->data_mem, OBJECT(s), &pci_host_data_le_ops, h, "pci-conf-data", 4); - memory_region_init_io(&s->iomem, OBJECT(s), &pci_reg_ops, s, - "pci.reg", PPC440_REG_SIZE); + memory_region_init_io(regs, OBJECT(s), &pci_reg_ops, s, "pci-reg", + PPC440_REG_SIZE); memory_region_add_subregion(&s->container, PCIC0_CFGADDR, &h->conf_mem); memory_region_add_subregion(&s->container, PCIC0_CFGDATA, &h->data_mem); - memory_region_add_subregion(&s->container, PPC440_REG_BASE, &s->iomem); + memory_region_add_subregion(&s->container, PPC440_REG_BASE, regs); sysbus_init_mmio(sbd, &s->container); }
Re: [PATCH qemu v3] aspeed add montblanc bmc reference from fuji
On 7/3/23 15:31, Cédric Le Goater wrote:
On 7/3/23 15:06, ~ssinprem wrote:
From: Sittisak Sinprem
- I2C list follow I2C Tree v1.6 20230320
- fru eeprom data use FB FRU format version 4
Signed-off-by: Sittisak Sinprem
Super !
Reviewed-by: Cédric Le Goater
Taking that back.
I missed a few things. See below.
---
docs/system/arm/aspeed.rst | 1 +
hw/arm/aspeed.c | 63 ++
hw/arm/aspeed_eeprom.c | 50 ++
hw/arm/aspeed_eeprom.h | 7 +
4 files changed, 121 insertions(+)
diff --git a/docs/system/arm/aspeed.rst b/docs/system/arm/aspeed.rst
index 80538422a1..5e0824f48b 100644
--- a/docs/system/arm/aspeed.rst
+++ b/docs/system/arm/aspeed.rst
@@ -33,6 +33,7 @@ AST2600 SoC based machines :
- ``tacoma-bmc`` OpenPOWER Witherspoon POWER9 AST2600 BMC
- ``rainier-bmc`` IBM Rainier POWER10 BMC
- ``fuji-bmc`` Facebook Fuji BMC
+- ``montblanc-bmc`` Facebook Montblanc BMC
- ``bletchley-bmc`` Facebook Bletchley BMC
- ``fby35-bmc`` Facebook fby35 BMC
- ``qcom-dc-scm-v1-bmc`` Qualcomm DC-SCM V1 BMC
diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index 9fca644d92..91bd4e5637 100644
--- a/hw/arm/aspeed.c
+++ b/hw/arm/aspeed.c
@@ -189,6 +189,10 @@ struct AspeedMachineState {
#define FUJI_BMC_HW_STRAP1 0x
#define FUJI_BMC_HW_STRAP2 0x
+/* Montblanc hardware value */
+#define MONTBLANC_BMC_HW_STRAP1 0x
+#define MONTBLANC_BMC_HW_STRAP2 0x
+
/* Bletchley hardware value */
/* TODO: Leave same as EVB for now. */
#define BLETCHLEY_BMC_HW_STRAP1 AST2600_EVB_HW_STRAP1
@@ -925,6 +929,39 @@ static void fuji_bmc_i2c_init(AspeedMachineState *bmc)
}
}
+static void montblanc_bmc_i2c_init(AspeedMachineState *bmc)
+{
+ AspeedSoCState *soc = &bmc->soc;
+ I2CBus *i2c[16] = {};
+
+ for (int i = 0; i < 16; i++) {
+ i2c[i] = aspeed_i2c_get_bus(&soc->i2c, i);
+ }
+
+ /* Ref from Minipack3_I2C_Tree_V1.6 20230320 */
+ at24c_eeprom_init_rom(i2c[3], 0x56, 8192, montblanc_scm_fruid, true);
+ at24c_eeprom_init_rom(i2c[6], 0x53, 8192, montblanc_fcm_fruid, true);
+
+ /* CPLD and FPGA */
+ at24c_eeprom_init(i2c[1], 0x35, 256); /* SCM CPLD */
+ at24c_eeprom_init(i2c[5], 0x35, 256); /* COMe CPLD TODO: need to update */
+ at24c_eeprom_init(i2c[12], 0x60, 256); /* MCB PWR CPLD */
+ at24c_eeprom_init(i2c[13], 0x35, 256); /* IOB FPGA */
+
+ /* on BMC board */
+ at24c_eeprom_init_rom(i2c[8], 0x51, 8192, montblanc_bmc_fruid, true);
+ /* BMC EEPROM */
+ i2c_slave_create_simple(i2c[8], TYPE_LM75, 0x48); /* Thermal Sensor */
+
+ /* COMe Sensor/EEPROM */
+ at24c_eeprom_init(i2c[0], 0x56, 16384); /* FRU EEPROM */
+ i2c_slave_create_simple(i2c[0], TYPE_LM75, 0x48); /* INLET Sensor */
+ i2c_slave_create_simple(i2c[0], TYPE_LM75, 0x4A); /* OUTLET Sensor */
+
+ /* It expects a pca9555 but a pca9552 is compatible */
+ create_pca9552(soc, 4, 0x27);
+}
+
#define TYPE_TMP421 "tmp421"
static void bletchley_bmc_i2c_init(AspeedMachineState *bmc)
@@ -1452,6 +1489,28 @@ static void aspeed_machine_fuji_class_init(ObjectClass
*oc, void *data)
aspeed_soc_num_cpus(amc->soc_name);
};
+#define MONTBLANC_BMC_RAM_SIZE ASPEED_RAM_SIZE(2 * GiB)
+
+static void aspeed_machine_montblanc_class_init(ObjectClass *oc, void *data)
+{
+ MachineClass *mc = MACHINE_CLASS(oc);
+ AspeedMachineClass *amc = ASPEED_MACHINE_CLASS(oc);
+
+ mc->desc = "Facebook Montblanc BMC (Cortex-A7)";
+ amc->soc_name = "ast2600-a3";
+ amc->hw_strap1 = MONTBLANC_BMC_HW_STRAP1;
+ amc->hw_strap2 = MONTBLANC_BMC_HW_STRAP2;
+ amc->fmc_model = "mx66l1g45g";
+ amc->spi_model = "mx66l1g45g";
+ amc->num_cs = 2;
+ amc->macs_mask = ASPEED_MAC3_ON;
+ amc->i2c_init = montblanc_bmc_i2c_init;
+ amc->uart_default = ASPEED_DEV_UART1;
+ mc->default_ram_size = MONTBLANC_BMC_RAM_SIZE;
+ mc->default_cpus = mc->min_cpus = mc->max_cpus =
+ aspeed_soc_num_cpus(amc->soc_name);
+};
+
#define BLETCHLEY_BMC_RAM_SIZE ASPEED_RAM_SIZE(2 * GiB)
static void aspeed_machine_bletchley_class_init(ObjectClass *oc, void *data)
@@ -1703,6 +1762,10 @@ static const TypeInfo aspeed_machine_types[] = {
.name = MACHINE_TYPE_NAME("fuji-bmc"),
.parent = TYPE_ASPEED_MACHINE,
.class_init = aspeed_machine_fuji_class_init,
+ }, {
+ .name = MACHINE_TYPE_NAME("montblanc-bmc"),
+ .parent = TYPE_ASPEED_MACHINE,
+ .class_init = aspeed_machine_montblanc_class_init,
}, {
.name = MACHINE_TYPE_NAME("bletchley-bmc"),
.parent = TYPE_ASPEED_MACHINE,
diff --git a/hw/arm/aspeed_eeprom.c b/hw/arm/aspeed_eeprom.c
index ace5266cec..6b3ffba0f8 100644
--- a/hw/arm/aspeed_eeprom.c
+++ b/hw/arm/aspeed_eeprom.c
@@ -161,6 +161,53
Re: [PATCH v21 02/20] s390x/cpu topology: add topology entries on CPU hotplug
On 30/06/2023 11.17, Pierre Morel wrote:
The topology information are attributes of the CPU and are
specified during the CPU device creation.
On hot plug we:
- calculate the default values for the topology for drawers,
books and sockets in the case they are not specified.
- verify the CPU attributes
- check that we have still room on the desired socket
The possibility to insert a CPU in a mask is dependent on the
number of cores allowed in a socket, a book or a drawer, the
checking is done during the hot plug of the CPU to have an
immediate answer.
If the complete topology is not specified, the core is added
in the physical topology based on its core ID and it gets
defaults values for the modifier attributes.
This way, starting QEMU without specifying the topology can
still get some advantage of the CPU topology.
Signed-off-by: Pierre Morel
---
...
diff --git a/include/hw/s390x/cpu-topology.h b/include/hw/s390x/cpu-topology.h
new file mode 100644
index 00..9164ac00a7
--- /dev/null
+++ b/include/hw/s390x/cpu-topology.h
@@ -0,0 +1,54 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * CPU Topology
+ *
+ * Copyright IBM Corp. 2022,2023
Nit: Add a space after the comma ?
+ * Author(s): Pierre Morel
+ *
+ */
+#ifndef HW_S390X_CPU_TOPOLOGY_H
+#define HW_S390X_CPU_TOPOLOGY_H
+
+#ifndef CONFIG_USER_ONLY
+
+#include "qemu/queue.h"
+#include "hw/boards.h"
+#include "qapi/qapi-types-machine-target.h"
+
+typedef struct S390Topology {
+uint8_t *cores_per_socket;
+} S390Topology;
So S390Topology has only one entry, "cores_per_socket" here...
...
diff --git a/hw/s390x/cpu-topology.c b/hw/s390x/cpu-topology.c
new file mode 100644
index 00..b163c17f8f
--- /dev/null
+++ b/hw/s390x/cpu-topology.c
@@ -0,0 +1,264 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * CPU Topology
+ *
+ * Copyright IBM Corp. 2022,2023
+ * Author(s): Pierre Morel
+ *
+ * S390 topology handling can be divided in two parts:
+ *
+ * - The first part in this file is taking care of all common functions
+ * used by KVM and TCG to create and modify the topology.
+ *
+ * - The second part, building the topology information data for the
+ * guest with CPU and KVM specificity will be implemented inside
+ * the target/s390/kvm sub tree.
+ */
+
+#include "qemu/osdep.h"
+#include "qapi/error.h"
+#include "qemu/error-report.h"
+#include "hw/qdev-properties.h"
+#include "hw/boards.h"
+#include "target/s390x/cpu.h"
+#include "hw/s390x/s390-virtio-ccw.h"
+#include "hw/s390x/cpu-topology.h"
+
+/*
+ * s390_topology is used to keep the topology information.
+ * .cores_per_socket: tracks information on the count of cores
+ *per socket.
+ * .smp: keeps track of the machine topology.
+ *
... but the description talks about ".smp" here, too, which never seems to
be added. Leftover from a previous iteration?
(also: please remove the empty line at the end of the comment)
+ */
+S390Topology s390_topology = {
+/* will be initialized after the cpu model is realized */
+.cores_per_socket = NULL,
+};
+
+/**
+ * s390_socket_nb:
+ * @cpu: s390x CPU
+ *
+ * Returns the socket number used inside the cores_per_socket array
+ * for a topology tree entry
+ */
+static int __s390_socket_nb(int drawer_id, int book_id, int socket_id)
Please don't use function names starting with double underscores. This
namespace is reserved by the C standard.
Maybe "s390_socket_nb_from_ids" ?
+{
+return (drawer_id * current_machine->smp.books + book_id) *
+ current_machine->smp.sockets + socket_id;
+}
+
+/**
+ * s390_socket_nb:
+ * @cpu: s390x CPU
+ *
+ * Returns the socket number used inside the cores_per_socket array
+ * for a cpu.
+ */
+static int s390_socket_nb(S390CPU *cpu)
+{
+return __s390_socket_nb(cpu->env.drawer_id, cpu->env.book_id,
+cpu->env.socket_id);
+}
+
+/**
+ * s390_has_topology:
+ *
+ * Return value: if the topology is supported by the machine.
"Return: true if the topology is supported by the machine"
(QEMU uses kerneldoc style, so it's just "Return:" and not "Return value:",
see e.g. https://www.kernel.org/doc/html/latest/doc-guide/kernel-doc.html )
+ */
+bool s390_has_topology(void)
+{
+return false;
+}
+
+/**
+ * s390_topology_init:
+ * @ms: the machine state where the machine topology is defined
+ *
+ * Keep track of the machine topology.
+ *
+ * Allocate an array to keep the count of cores per socket.
+ * The index of the array starts at socket 0 from book 0 and
+ * drawer 0 up to the maximum allowed by the machine topology.
+ */
+static void s390_topology_init(MachineState *ms)
+{
+CpuTopology *smp = &ms->smp;
+
+s390_topology.cores_per_socket = g_new0(uint8_t, smp->sockets *
+smp->books * smp->drawers);
+}
+
+/**
+ * s390_topology_cpu_default:
+ * @cpu: pointer to a S390CPU
+ * @errp: Error pointer
+ *
+ * Setup the default topology if no attributes are already set.
+
Re: [PATCH v6 5/5] hw/pci: ensure PCIE devices are plugged into only slot 0 of PCIE port
> On 04-Jul-2023, at 11:09 AM, Ani Sinha wrote: > > > >> On 04-Jul-2023, at 10:31 AM, Akihiko Odaki wrote: >> >> On 2023/07/03 15:08, Ani Sinha wrote: On 02-Jul-2023, at 10:29 AM, Michael S. Tsirkin wrote: On Sat, Jul 01, 2023 at 04:09:31PM +0900, Akihiko Odaki wrote: > Yes, I want the slot number restriction to be enforced. If it worries you > too much for regressions, you may implement it as a warning first and then > turn it a hard error when the next development phase starts. That's not a bad idea. >>> If we had not enforced the check strongly, the tests that we fixed would >>> not get noticed. >> >> Perhaps so, but we don't have much time before feature freeze. I rather want >> to see the check implemented as warning in 8.1 instead of delaying the >> initial implementation of the check after 8.1 (though I worry if it's >> already too late for 8.1.) > > The feature hard freeze window starts from 12th of next week. So I am still > debating whether to keep the hard check or just have a warning. If the hard > check causes regressions, we can always revert it to a warning later. mst?
Re: [PATCH v6 5/5] hw/pci: ensure PCIE devices are plugged into only slot 0 of PCIE port
On Tue, Jul 04, 2023 at 04:03:54PM +0530, Ani Sinha wrote: > > > > On 04-Jul-2023, at 11:09 AM, Ani Sinha wrote: > > > > > > > >> On 04-Jul-2023, at 10:31 AM, Akihiko Odaki > >> wrote: > >> > >> On 2023/07/03 15:08, Ani Sinha wrote: > On 02-Jul-2023, at 10:29 AM, Michael S. Tsirkin wrote: > > On Sat, Jul 01, 2023 at 04:09:31PM +0900, Akihiko Odaki wrote: > > Yes, I want the slot number restriction to be enforced. If it worries > > you > > too much for regressions, you may implement it as a warning first and > > then > > turn it a hard error when the next development phase starts. > > That's not a bad idea. > >>> If we had not enforced the check strongly, the tests that we fixed would > >>> not get noticed. > >> > >> Perhaps so, but we don't have much time before feature freeze. I rather > >> want to see the check implemented as warning in 8.1 instead of delaying > >> the initial implementation of the check after 8.1 (though I worry if it's > >> already too late for 8.1.) > > > > The feature hard freeze window starts from 12th of next week. So I am still > > debating whether to keep the hard check or just have a warning. If the hard > > check causes regressions, we can always revert it to a warning later. > > mst? I'd go for a warning now. Let's see what triggers for users without actually breaking things too badly for them. -- MST
Re: [PATCH] vfio: Fix null pointer dereference bug in vfio_bars_finalize()
On 03/07/2023 19:56, Philippe Mathieu-Daudé wrote:
External email: Use caution opening links or attachments
On 3/7/23 18:39, Avihai Horon wrote:
vfio_realize() has the following flow:
1. vfio_bars_prepare() -- sets VFIOBAR->size.
2. msix_early_setup().
3. vfio_bars_register() -- allocates VFIOBAR->mr.
After vfio_bars_prepare() is called msix_early_setup() can fail. If it
does fail, vfio_bars_register() is never called and VFIOBAR->mr is not
allocated.
In this case, vfio_bars_finalize() is called as part of the error flow
to free the bars' resources. However, vfio_bars_finalize() calls
object_unparent() for VFIOBAR->mr unconditionally and thus we get a null
pointer dereference.
Fix it by checking VFIOBAR->mr in vfio_bars_finalize().
Fixes: 89d5202edc50 ("vfio/pci: Allow relocating MSI-X MMIO")
Signed-off-by: Avihai Horon
---
hw/vfio/pci.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index ab6645ba60..95e077082b 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -1752,7 +1752,7 @@ static void vfio_bars_finalize(VFIOPCIDevice
*vdev)
vfio_bar_quirk_finalize(vdev, i);
vfio_region_finalize(&bar->region);
- if (bar->size) {
+ if (bar->size && bar->mr) {
object_unparent(OBJECT(bar->mr));
g_free(bar->mr);
}
What about:
if (bar->mr) {
assert(bar->size);
object_unparent(OBJECT(bar->mr));
g_free(bar->mr);
bar->mr = NULL;
}
?
Nice touch, it's indeed more accurate.
Will change and post v2.
Thanks!
Re: [RISC-V] ERROR:../accel/tcg/cpu-exec.c:1028:cpu_exec_setjmp: assertion failed: (cpu == current_cpu)
I think the issue is that the value returned from brk(0) is no longer
page aligned.
$ ./qemu-riscv64 -strace ../exe1
18329 brk(NULL) = 0x00303000
18329 faccessat(AT_FDCWD,"/etc/ld.so.preload",R_OK,0x3010d0) = -1 errno=2 (No
such file or directory)
18329 openat(AT_FDCWD,"/etc/ld.so.cache",O_RDONLY|O_CLOEXEC) = 3
18329 newfstatat(3,"",0x0040007fe900,0x1000) = 0
18329 mmap(NULL,8799,PROT_READ,MAP_PRIVATE,3,0) = 0x004000824000
18329 close(3) = 0
18329 openat(AT_FDCWD,"/lib64/lp64d/libc.so.6",O_RDONLY|O_CLOEXEC) = 3
18329 read(3,0x7fea70,832) = 832
18329 newfstatat(3,"",0x0040007fe8f0,0x1000) = 0
18329 mmap(NULL,1405128,PROT_EXEC|PROT_READ,MAP_PRIVATE|MAP_DENYWRITE,3,0) =
0x004000827000
18329
mmap(0x00400096d000,20480,PROT_READ|PROT_WRITE,MAP_PRIVATE|MAP_DENYWRITE|MAP_FIXED,3,0x146000)
= 0x00400096d000
18329
mmap(0x004000972000,49352,PROT_READ|PROT_WRITE,MAP_PRIVATE|MAP_ANONYMOUS|MAP_FIXED,-1,0)
= 0x004000972000
18329 close(3) = 0
18329 mmap(NULL,8192,PROT_READ|PROT_WRITE,MAP_PRIVATE|MAP_ANONYMOUS,-1,0) =
0x00400097f000
18329 set_tid_address(0x400097f710) = 18329
18329 set_robust_list(0x400097f720,24) = -1 errno=38 (Function not implemented)
18329 mprotect(0x00400096d000,12288,PROT_READ) = 0
18329 mprotect(0x00400082,4096,PROT_READ) = 0
18329 prlimit64(0,RLIMIT_STACK,NULL,0x0040007ff4f8) = 0
({rlim_cur=8388608,rlim_max=-1})
18329 munmap(0x004000824000,8799) = 0
18329 newfstatat(1,"",0x0040007ff658,0x1000) = 0
18329 getrandom(0x4000976a40,8,1) = 8
18329 brk(NULL) = 0x00303000
18329 brk(0x00324000) = 0x00324000
18329 write(1,0x3032a0,12)Hello world
= 12
18329 exit_group(0)
$ qemu-riscv64 -strace ../exe1
18369 brk(NULL) = 0x003022e8
18369 faccessat(AT_FDCWD,"/etc/ld.so.preload",R_OK,0x3010d0) = -1 errno=2 (No
such file or directory)
18369 openat(AT_FDCWD,"/etc/ld.so.cache",O_RDONLY|O_CLOEXEC) = 3
18369 newfstatat(3,"",0x0040007fe8f0,0x1000) = 0
18369 mmap(NULL,8799,PROT_READ,MAP_PRIVATE,3,0) = 0x004000824000
18369 close(3) = 0
18369 openat(AT_FDCWD,"/lib64/lp64d/libc.so.6",O_RDONLY|O_CLOEXEC) = 3
18369 read(3,0x7fea60,832) = 832
18369 newfstatat(3,"",0x0040007fe8e0,0x1000) = 0
18369 mmap(NULL,1405128,PROT_EXEC|PROT_READ,MAP_PRIVATE|MAP_DENYWRITE,3,0) =
0x004000827000
18369
mmap(0x00400096d000,20480,PROT_READ|PROT_WRITE,MAP_PRIVATE|MAP_DENYWRITE|MAP_FIXED,3,0x146000)
= 0x00400096d000
18369
mmap(0x004000972000,49352,PROT_READ|PROT_WRITE,MAP_PRIVATE|MAP_ANONYMOUS|MAP_FIXED,-1,0)
= 0x004000972000
18369 close(3) = 0
18369 mmap(NULL,8192,PROT_READ|PROT_WRITE,MAP_PRIVATE|MAP_ANONYMOUS,-1,0) =
0x00400097f000
18369 set_tid_address(0x400097f710) = 18369
18369 set_robust_list(0x400097f720,24) = -1 errno=38 (Function not implemented)
18369 mprotect(0x00400096d000,12288,PROT_READ) = 0
18369 mprotect(0x00400082,4096,PROT_READ) = 0
18369 prlimit64(0,RLIMIT_STACK,NULL,0x0040007ff4e8) = 0
({rlim_cur=8388608,rlim_max=-1})
18369 munmap(0x004000824000,8799) = 0
18369 newfstatat(1,"",0x0040007ff648,0x1000) = 0
18369 getrandom(0x4000976a40,8,1) = 8
18369 brk(NULL) = 0x003022e8
18369 brk(0x003232e8)**
ERROR:../accel/tcg/cpu-exec.c:1028:cpu_exec_setjmp: assertion failed: (cpu ==
current_cpu)
Bail out! ERROR:../accel/tcg/cpu-exec.c:1028:cpu_exec_setjmp: assertion failed:
(cpu == current_cpu)
**
ERROR:../accel/tcg/cpu-exec.c:1028:cpu_exec_setjmp: assertion failed: (cpu ==
current_cpu)
Bail out! ERROR:../accel/tcg/cpu-exec.c:1028:cpu_exec_setjmp: assertion failed:
(cpu == current_cpu)
--
Andreas Schwab, SUSE Labs, sch...@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."
[PATCH qemu v4] aspeed add montblanc bmc reference from fuji
From: Sittisak Sinprem
- I2C list follow I2C Tree v1.6 20230320
- fru eeprom data use FB FRU format version 4
Signed-off-by: Sittisak Sinprem
---
docs/system/arm/aspeed.rst | 1 +
hw/arm/aspeed.c| 63 ++
hw/arm/aspeed_eeprom.c | 50 ++
hw/arm/aspeed_eeprom.h | 7 +
4 files changed, 121 insertions(+)
diff --git a/docs/system/arm/aspeed.rst b/docs/system/arm/aspeed.rst
index 80538422a1..5e0824f48b 100644
--- a/docs/system/arm/aspeed.rst
+++ b/docs/system/arm/aspeed.rst
@@ -33,6 +33,7 @@ AST2600 SoC based machines :
- ``tacoma-bmc`` OpenPOWER Witherspoon POWER9 AST2600 BMC
- ``rainier-bmc`` IBM Rainier POWER10 BMC
- ``fuji-bmc`` Facebook Fuji BMC
+- ``montblanc-bmc``Facebook Montblanc BMC
- ``bletchley-bmc``Facebook Bletchley BMC
- ``fby35-bmc``Facebook fby35 BMC
- ``qcom-dc-scm-v1-bmc`` Qualcomm DC-SCM V1 BMC
diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index 9fca644d92..91bd4e5637 100644
--- a/hw/arm/aspeed.c
+++ b/hw/arm/aspeed.c
@@ -189,6 +189,10 @@ struct AspeedMachineState {
#define FUJI_BMC_HW_STRAP10x
#define FUJI_BMC_HW_STRAP20x
+/* Montblanc hardware value */
+#define MONTBLANC_BMC_HW_STRAP10x
+#define MONTBLANC_BMC_HW_STRAP20x
+
/* Bletchley hardware value */
/* TODO: Leave same as EVB for now. */
#define BLETCHLEY_BMC_HW_STRAP1 AST2600_EVB_HW_STRAP1
@@ -925,6 +929,39 @@ static void fuji_bmc_i2c_init(AspeedMachineState *bmc)
}
}
+static void montblanc_bmc_i2c_init(AspeedMachineState *bmc)
+{
+AspeedSoCState *soc = &bmc->soc;
+I2CBus *i2c[16] = {};
+
+for (int i = 0; i < 16; i++) {
+i2c[i] = aspeed_i2c_get_bus(&soc->i2c, i);
+}
+
+/* Ref from Minipack3_I2C_Tree_V1.6 20230320 */
+at24c_eeprom_init_rom(i2c[3], 0x56, 8192, montblanc_scm_fruid, true);
+at24c_eeprom_init_rom(i2c[6], 0x53, 8192, montblanc_fcm_fruid, true);
+
+/* CPLD and FPGA */
+at24c_eeprom_init(i2c[1], 0x35, 256); /* SCM CPLD */
+at24c_eeprom_init(i2c[5], 0x35, 256); /* COMe CPLD TODO: need to update */
+at24c_eeprom_init(i2c[12], 0x60, 256); /* MCB PWR CPLD */
+at24c_eeprom_init(i2c[13], 0x35, 256); /* IOB FPGA */
+
+/* on BMC board */
+at24c_eeprom_init_rom(i2c[8], 0x51, 8192, montblanc_bmc_fruid, true);
+ /* BMC EEPROM */
+i2c_slave_create_simple(i2c[8], TYPE_LM75, 0x48); /* Thermal Sensor */
+
+/* COMe Sensor/EEPROM */
+at24c_eeprom_init(i2c[0], 0x56, 16384); /* FRU EEPROM */
+i2c_slave_create_simple(i2c[0], TYPE_LM75, 0x48); /* INLET Sensor */
+i2c_slave_create_simple(i2c[0], TYPE_LM75, 0x4A); /* OUTLET Sensor */
+
+/* It expects a pca9555 but a pca9552 is compatible */
+create_pca9552(soc, 4, 0x27);
+}
+
#define TYPE_TMP421 "tmp421"
static void bletchley_bmc_i2c_init(AspeedMachineState *bmc)
@@ -1452,6 +1489,28 @@ static void aspeed_machine_fuji_class_init(ObjectClass
*oc, void *data)
aspeed_soc_num_cpus(amc->soc_name);
};
+#define MONTBLANC_BMC_RAM_SIZE ASPEED_RAM_SIZE(2 * GiB)
+
+static void aspeed_machine_montblanc_class_init(ObjectClass *oc, void *data)
+{
+MachineClass *mc = MACHINE_CLASS(oc);
+AspeedMachineClass *amc = ASPEED_MACHINE_CLASS(oc);
+
+mc->desc = "Facebook Montblanc BMC (Cortex-A7)";
+amc->soc_name = "ast2600-a3";
+amc->hw_strap1 = MONTBLANC_BMC_HW_STRAP1;
+amc->hw_strap2 = MONTBLANC_BMC_HW_STRAP2;
+amc->fmc_model = "mx66l1g45g";
+amc->spi_model = "mx66l1g45g";
+amc->num_cs = 2;
+amc->macs_mask = ASPEED_MAC3_ON;
+amc->i2c_init = montblanc_bmc_i2c_init;
+amc->uart_default = ASPEED_DEV_UART1;
+mc->default_ram_size = MONTBLANC_BMC_RAM_SIZE;
+mc->default_cpus = mc->min_cpus = mc->max_cpus =
+aspeed_soc_num_cpus(amc->soc_name);
+};
+
#define BLETCHLEY_BMC_RAM_SIZE ASPEED_RAM_SIZE(2 * GiB)
static void aspeed_machine_bletchley_class_init(ObjectClass *oc, void *data)
@@ -1703,6 +1762,10 @@ static const TypeInfo aspeed_machine_types[] = {
.name = MACHINE_TYPE_NAME("fuji-bmc"),
.parent= TYPE_ASPEED_MACHINE,
.class_init= aspeed_machine_fuji_class_init,
+}, {
+.name = MACHINE_TYPE_NAME("montblanc-bmc"),
+.parent= TYPE_ASPEED_MACHINE,
+.class_init= aspeed_machine_montblanc_class_init,
}, {
.name = MACHINE_TYPE_NAME("bletchley-bmc"),
.parent= TYPE_ASPEED_MACHINE,
diff --git a/hw/arm/aspeed_eeprom.c b/hw/arm/aspeed_eeprom.c
index ace5266cec..8cc73f83dc 100644
--- a/hw/arm/aspeed_eeprom.c
+++ b/hw/arm/aspeed_eeprom.c
@@ -161,6 +161,53 @@ const uint8_t rainier_bmc_fruid[] = {
0x31, 0x50, 0x46, 0x04, 0x00, 0x00, 0x00, 0x00, 0x00,
};
+/* Montblanc BMC FRU */
+const uint8_t montblanc_scm_fruid[] = {
+0xfb, 0xfb, 0x04, 0xff, 0
Re: [PATCH qemu v4] aspeed add montblanc bmc reference from fuji
Hi Cédric,
Please stop this patch,
after the test, the eeprom content is incorrect,
root@bmc:~# weutil -l
bmc_eeprom/sys/bus/i2c/devices/i2c-8/8-0051/eeprom
chassis_eeprom/sys/bus/i2c/devices/i2c-6/6-0053/eeprom
dummy_eeprom/etc/weutil/meta_eeprom_v4_sample.bin
scm_eeprom/sys/bus/i2c/devices/i2c-3/3-0056/eeprom
root@bmc:~# hexdump -C /sys/bus/i2c/devices/i2c-6/6-0053/eeprom
fb 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
||
0010 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
||
*
2000
root@bmc:~# hexdump -C /sys/bus/i2c/devices/i2c-8/8-0051/eeprom
fb 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
||
0010 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
||
*
2000
root@bmc:~#
I will send the next version after fixed
On Tue, Jul 4, 2023 at 5:52 PM ~ssinprem wrote:
> From: Sittisak Sinprem
>
> - I2C list follow I2C Tree v1.6 20230320
> - fru eeprom data use FB FRU format version 4
>
> Signed-off-by: Sittisak Sinprem
> ---
> docs/system/arm/aspeed.rst | 1 +
> hw/arm/aspeed.c| 63 ++
> hw/arm/aspeed_eeprom.c | 50 ++
> hw/arm/aspeed_eeprom.h | 7 +
> 4 files changed, 121 insertions(+)
>
> diff --git a/docs/system/arm/aspeed.rst b/docs/system/arm/aspeed.rst
> index 80538422a1..5e0824f48b 100644
> --- a/docs/system/arm/aspeed.rst
> +++ b/docs/system/arm/aspeed.rst
> @@ -33,6 +33,7 @@ AST2600 SoC based machines :
> - ``tacoma-bmc`` OpenPOWER Witherspoon POWER9 AST2600 BMC
> - ``rainier-bmc`` IBM Rainier POWER10 BMC
> - ``fuji-bmc`` Facebook Fuji BMC
> +- ``montblanc-bmc``Facebook Montblanc BMC
> - ``bletchley-bmc``Facebook Bletchley BMC
> - ``fby35-bmc``Facebook fby35 BMC
> - ``qcom-dc-scm-v1-bmc`` Qualcomm DC-SCM V1 BMC
> diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
> index 9fca644d92..91bd4e5637 100644
> --- a/hw/arm/aspeed.c
> +++ b/hw/arm/aspeed.c
> @@ -189,6 +189,10 @@ struct AspeedMachineState {
> #define FUJI_BMC_HW_STRAP10x
> #define FUJI_BMC_HW_STRAP20x
>
> +/* Montblanc hardware value */
> +#define MONTBLANC_BMC_HW_STRAP10x
> +#define MONTBLANC_BMC_HW_STRAP20x
> +
> /* Bletchley hardware value */
> /* TODO: Leave same as EVB for now. */
> #define BLETCHLEY_BMC_HW_STRAP1 AST2600_EVB_HW_STRAP1
> @@ -925,6 +929,39 @@ static void fuji_bmc_i2c_init(AspeedMachineState *bmc)
> }
> }
>
> +static void montblanc_bmc_i2c_init(AspeedMachineState *bmc)
> +{
> +AspeedSoCState *soc = &bmc->soc;
> +I2CBus *i2c[16] = {};
> +
> +for (int i = 0; i < 16; i++) {
> +i2c[i] = aspeed_i2c_get_bus(&soc->i2c, i);
> +}
> +
> +/* Ref from Minipack3_I2C_Tree_V1.6 20230320 */
> +at24c_eeprom_init_rom(i2c[3], 0x56, 8192, montblanc_scm_fruid, true);
> +at24c_eeprom_init_rom(i2c[6], 0x53, 8192, montblanc_fcm_fruid, true);
> +
> +/* CPLD and FPGA */
> +at24c_eeprom_init(i2c[1], 0x35, 256); /* SCM CPLD */
> +at24c_eeprom_init(i2c[5], 0x35, 256); /* COMe CPLD TODO: need to
> update */
> +at24c_eeprom_init(i2c[12], 0x60, 256); /* MCB PWR CPLD */
> +at24c_eeprom_init(i2c[13], 0x35, 256); /* IOB FPGA */
> +
> +/* on BMC board */
> +at24c_eeprom_init_rom(i2c[8], 0x51, 8192, montblanc_bmc_fruid, true);
> + /* BMC EEPROM */
> +i2c_slave_create_simple(i2c[8], TYPE_LM75, 0x48); /* Thermal Sensor */
> +
> +/* COMe Sensor/EEPROM */
> +at24c_eeprom_init(i2c[0], 0x56, 16384); /* FRU EEPROM */
> +i2c_slave_create_simple(i2c[0], TYPE_LM75, 0x48); /* INLET Sensor */
> +i2c_slave_create_simple(i2c[0], TYPE_LM75, 0x4A); /* OUTLET Sensor */
> +
> +/* It expects a pca9555 but a pca9552 is compatible */
> +create_pca9552(soc, 4, 0x27);
> +}
> +
> #define TYPE_TMP421 "tmp421"
>
> static void bletchley_bmc_i2c_init(AspeedMachineState *bmc)
> @@ -1452,6 +1489,28 @@ static void
> aspeed_machine_fuji_class_init(ObjectClass *oc, void *data)
> aspeed_soc_num_cpus(amc->soc_name);
> };
>
> +#define MONTBLANC_BMC_RAM_SIZE ASPEED_RAM_SIZE(2 * GiB)
> +
> +static void aspeed_machine_montblanc_class_init(ObjectClass *oc, void
> *data)
> +{
> +MachineClass *mc = MACHINE_CLASS(oc);
> +AspeedMachineClass *amc = ASPEED_MACHINE_CLASS(oc);
> +
> +mc->desc = "Facebook Montblanc BMC (Cortex-A7)";
> +amc->soc_name = "ast2600-a3";
> +amc->hw_strap1 = MONTBLANC_BMC_HW_STRAP1;
> +amc->hw_strap2 = MONTBLANC_BMC_HW_STRAP2;
> +amc->fmc_model = "mx66l1g45g";
> +amc->spi_model = "mx66l1g45g";
> +amc->num_cs = 2;
> +amc->macs_mask = ASPEED_MAC3_ON;
> +amc->i2c_init = montblanc_bmc_i2c_init;
> +amc->uart_default = ASPEED_DEV_UART1;
> +mc->default_ram_size = MONTBLANC_BMC_RAM_SIZE;
> +mc->default_cpus = mc->min_cpus = mc->max_cpu
Re: [PATCH v6 5/5] hw/pci: ensure PCIE devices are plugged into only slot 0 of PCIE port
> On 04-Jul-2023, at 4:06 PM, Michael S. Tsirkin wrote: > > On Tue, Jul 04, 2023 at 04:03:54PM +0530, Ani Sinha wrote: >> >> >>> On 04-Jul-2023, at 11:09 AM, Ani Sinha wrote: >>> >>> >>> On 04-Jul-2023, at 10:31 AM, Akihiko Odaki wrote: On 2023/07/03 15:08, Ani Sinha wrote: >> On 02-Jul-2023, at 10:29 AM, Michael S. Tsirkin wrote: >> >> On Sat, Jul 01, 2023 at 04:09:31PM +0900, Akihiko Odaki wrote: >>> Yes, I want the slot number restriction to be enforced. If it worries >>> you >>> too much for regressions, you may implement it as a warning first and >>> then >>> turn it a hard error when the next development phase starts. >> >> That's not a bad idea. > If we had not enforced the check strongly, the tests that we fixed would > not get noticed. Perhaps so, but we don't have much time before feature freeze. I rather want to see the check implemented as warning in 8.1 instead of delaying the initial implementation of the check after 8.1 (though I worry if it's already too late for 8.1.) >>> >>> The feature hard freeze window starts from 12th of next week. So I am still >>> debating whether to keep the hard check or just have a warning. If the hard >>> check causes regressions, we can always revert it to a warning later. >> >> mst? > > I'd go for a warning now. Let's see what triggers for users without > actually breaking things too badly for them. 🤦🏻
[PATCH qemu v5] aspeed add montblanc bmc reference from fuji
From: Sittisak Sinprem
- I2C list follow I2C Tree v1.6 20230320
- fru eeprom data use FB FRU format version 4
Signed-off-by: Sittisak Sinprem
---
docs/system/arm/aspeed.rst | 1 +
hw/arm/aspeed.c| 65 ++
hw/arm/aspeed_eeprom.c | 50 +
hw/arm/aspeed_eeprom.h | 7
4 files changed, 123 insertions(+)
diff --git a/docs/system/arm/aspeed.rst b/docs/system/arm/aspeed.rst
index 80538422a1..5e0824f48b 100644
--- a/docs/system/arm/aspeed.rst
+++ b/docs/system/arm/aspeed.rst
@@ -33,6 +33,7 @@ AST2600 SoC based machines :
- ``tacoma-bmc`` OpenPOWER Witherspoon POWER9 AST2600 BMC
- ``rainier-bmc`` IBM Rainier POWER10 BMC
- ``fuji-bmc`` Facebook Fuji BMC
+- ``montblanc-bmc``Facebook Montblanc BMC
- ``bletchley-bmc``Facebook Bletchley BMC
- ``fby35-bmc``Facebook fby35 BMC
- ``qcom-dc-scm-v1-bmc`` Qualcomm DC-SCM V1 BMC
diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index 9fca644d92..bbb7a3392c 100644
--- a/hw/arm/aspeed.c
+++ b/hw/arm/aspeed.c
@@ -189,6 +189,10 @@ struct AspeedMachineState {
#define FUJI_BMC_HW_STRAP10x
#define FUJI_BMC_HW_STRAP20x
+/* Montblanc hardware value */
+#define MONTBLANC_BMC_HW_STRAP10x
+#define MONTBLANC_BMC_HW_STRAP20x
+
/* Bletchley hardware value */
/* TODO: Leave same as EVB for now. */
#define BLETCHLEY_BMC_HW_STRAP1 AST2600_EVB_HW_STRAP1
@@ -925,6 +929,41 @@ static void fuji_bmc_i2c_init(AspeedMachineState *bmc)
}
}
+static void montblanc_bmc_i2c_init(AspeedMachineState *bmc)
+{
+AspeedSoCState *soc = &bmc->soc;
+I2CBus *i2c[16] = {};
+
+for (int i = 0; i < 16; i++) {
+i2c[i] = aspeed_i2c_get_bus(&soc->i2c, i);
+}
+
+/* Ref from Minipack3_I2C_Tree_V1.6 20230320 */
+at24c_eeprom_init_rom(i2c[3], 0x56, 8192, montblanc_scm_fruid,
+ montblanc_scm_fruid_len);
+at24c_eeprom_init_rom(i2c[6], 0x53, 8192, montblanc_fcm_fruid,
+ montblanc_fcm_fruid_len);
+
+/* CPLD and FPGA */
+at24c_eeprom_init(i2c[1], 0x35, 256); /* SCM CPLD */
+at24c_eeprom_init(i2c[5], 0x35, 256); /* COMe CPLD TODO: need to update */
+at24c_eeprom_init(i2c[12], 0x60, 256); /* MCB PWR CPLD */
+at24c_eeprom_init(i2c[13], 0x35, 256); /* IOB FPGA */
+
+/* on BMC board */
+at24c_eeprom_init_rom(i2c[8], 0x51, 8192, montblanc_bmc_fruid,
+ montblanc_bmc_fruid_len); /* BMC EEPROM */
+i2c_slave_create_simple(i2c[8], TYPE_LM75, 0x48); /* Thermal Sensor */
+
+/* COMe Sensor/EEPROM */
+at24c_eeprom_init(i2c[0], 0x56, 16384); /* FRU EEPROM */
+i2c_slave_create_simple(i2c[0], TYPE_LM75, 0x48); /* INLET Sensor */
+i2c_slave_create_simple(i2c[0], TYPE_LM75, 0x4A); /* OUTLET Sensor */
+
+/* It expects a pca9555 but a pca9552 is compatible */
+create_pca9552(soc, 4, 0x27);
+}
+
#define TYPE_TMP421 "tmp421"
static void bletchley_bmc_i2c_init(AspeedMachineState *bmc)
@@ -1452,6 +1491,28 @@ static void aspeed_machine_fuji_class_init(ObjectClass
*oc, void *data)
aspeed_soc_num_cpus(amc->soc_name);
};
+#define MONTBLANC_BMC_RAM_SIZE ASPEED_RAM_SIZE(2 * GiB)
+
+static void aspeed_machine_montblanc_class_init(ObjectClass *oc, void *data)
+{
+MachineClass *mc = MACHINE_CLASS(oc);
+AspeedMachineClass *amc = ASPEED_MACHINE_CLASS(oc);
+
+mc->desc = "Facebook Montblanc BMC (Cortex-A7)";
+amc->soc_name = "ast2600-a3";
+amc->hw_strap1 = MONTBLANC_BMC_HW_STRAP1;
+amc->hw_strap2 = MONTBLANC_BMC_HW_STRAP2;
+amc->fmc_model = "mx66l1g45g";
+amc->spi_model = "mx66l1g45g";
+amc->num_cs = 2;
+amc->macs_mask = ASPEED_MAC3_ON;
+amc->i2c_init = montblanc_bmc_i2c_init;
+amc->uart_default = ASPEED_DEV_UART1;
+mc->default_ram_size = MONTBLANC_BMC_RAM_SIZE;
+mc->default_cpus = mc->min_cpus = mc->max_cpus =
+aspeed_soc_num_cpus(amc->soc_name);
+};
+
#define BLETCHLEY_BMC_RAM_SIZE ASPEED_RAM_SIZE(2 * GiB)
static void aspeed_machine_bletchley_class_init(ObjectClass *oc, void *data)
@@ -1703,6 +1764,10 @@ static const TypeInfo aspeed_machine_types[] = {
.name = MACHINE_TYPE_NAME("fuji-bmc"),
.parent= TYPE_ASPEED_MACHINE,
.class_init= aspeed_machine_fuji_class_init,
+}, {
+.name = MACHINE_TYPE_NAME("montblanc-bmc"),
+.parent= TYPE_ASPEED_MACHINE,
+.class_init= aspeed_machine_montblanc_class_init,
}, {
.name = MACHINE_TYPE_NAME("bletchley-bmc"),
.parent= TYPE_ASPEED_MACHINE,
diff --git a/hw/arm/aspeed_eeprom.c b/hw/arm/aspeed_eeprom.c
index ace5266cec..8cc73f83dc 100644
--- a/hw/arm/aspeed_eeprom.c
+++ b/hw/arm/aspeed_eeprom.c
@@ -161,6 +161,53 @@ const uint8_t rainier_bmc_fruid[] = {
0x31, 0x50, 0x46, 0x04, 0x00, 0x00, 0x00, 0x00, 0x00,
};
+/* Montblanc B
