Re: [PATCH V6 03/14] cpus: stop vm in suspended runstate

2024-01-07 Thread Markus Armbruster
Steven Sistare  writes:

[...]

> With these changes, can I add your Acked-by to the commit?

I'm afraid I lost context over the break.  Suggest you post v7, and I
provide my Acked-by there.  Likely easier for me.

Happy new year!

[...]




[PULL 13/17] vfio/iommufd: Remove the use of stat() to check file existence

2024-01-07 Thread Cédric Le Goater
Using stat() before opening a file or a directory can lead to a
time-of-check to time-of-use (TOCTOU) filesystem race, which is
reported by coverity as a Security best practices violations. The
sequence could be replaced by open and fdopendir but it doesn't add
much in this case. Simply use opendir to avoid the race.

Fixes: CID 1531551
Signed-off-by: Cédric Le Goater 
Reviewed-by: Zhenzhong Duan 
---
 hw/vfio/iommufd.c | 6 --
 1 file changed, 6 deletions(-)

diff --git a/hw/vfio/iommufd.c b/hw/vfio/iommufd.c
index 
d4c586e842def8f04d3a914843f5eece2c75ea30..9bfddc1360895413176a9f170e29e89027384a66
 100644
--- a/hw/vfio/iommufd.c
+++ b/hw/vfio/iommufd.c
@@ -121,17 +121,11 @@ static int iommufd_cdev_getfd(const char *sysfs_path, 
Error **errp)
 DIR *dir = NULL;
 struct dirent *dent;
 gchar *contents;
-struct stat st;
 gsize length;
 int major, minor;
 dev_t vfio_devt;
 
 path = g_strdup_printf("%s/vfio-dev", sysfs_path);
-if (stat(path, ) < 0) {
-error_setg_errno(errp, errno, "no such host device");
-goto out_free_path;
-}
-
 dir = opendir(path);
 if (!dir) {
 error_setg_errno(errp, errno, "couldn't open directory %s", path);
-- 
2.43.0




[PULL 08/17] vfio/iommufd: Introduce a VFIOIOMMU iommufd QOM interface

2024-01-07 Thread Cédric Le Goater
As previously done for the sPAPR and legacy IOMMU backends, convert
the VFIOIOMMUOps struct to a QOM interface. The set of of operations
for this backend can be referenced with a literal typename instead of
a C struct.

Reviewed-by: Zhenzhong Duan 
Tested-by: Eric Farman 
Signed-off-by: Cédric Le Goater 
---
 include/hw/vfio/vfio-common.h |  1 -
 include/hw/vfio/vfio-container-base.h |  2 +-
 hw/vfio/common.c  |  2 +-
 hw/vfio/iommufd.c | 35 ---
 4 files changed, 28 insertions(+), 12 deletions(-)

diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
index 
14c497b6b0a79466e8f567aceed384ec2c75ea90..9b7ef7d02b5a0ad5266bcc4d06cd6874178978e4
 100644
--- a/include/hw/vfio/vfio-common.h
+++ b/include/hw/vfio/vfio-common.h
@@ -210,7 +210,6 @@ typedef QLIST_HEAD(VFIOGroupList, VFIOGroup) VFIOGroupList;
 typedef QLIST_HEAD(VFIODeviceList, VFIODevice) VFIODeviceList;
 extern VFIOGroupList vfio_group_list;
 extern VFIODeviceList vfio_device_list;
-extern const VFIOIOMMUOps vfio_iommufd_ops;
 extern const MemoryListener vfio_memory_listener;
 extern int vfio_kvm_device_fd;
 
diff --git a/include/hw/vfio/vfio-container-base.h 
b/include/hw/vfio/vfio-container-base.h
index 
9e21d7811f3810ca2c63d9f28bdcc9aa6f75f9ad..b2813b0c117985425c842d91f011bb895955d738
 100644
--- a/include/hw/vfio/vfio-container-base.h
+++ b/include/hw/vfio/vfio-container-base.h
@@ -17,7 +17,6 @@
 
 typedef struct VFIODevice VFIODevice;
 typedef struct VFIOIOMMUClass VFIOIOMMUClass;
-#define VFIOIOMMUOps VFIOIOMMUClass /* To remove */
 
 typedef struct {
 unsigned long *bitmap;
@@ -96,6 +95,7 @@ void vfio_container_destroy(VFIOContainerBase *bcontainer);
 #define TYPE_VFIO_IOMMU "vfio-iommu"
 #define TYPE_VFIO_IOMMU_LEGACY TYPE_VFIO_IOMMU "-legacy"
 #define TYPE_VFIO_IOMMU_SPAPR TYPE_VFIO_IOMMU "-spapr"
+#define TYPE_VFIO_IOMMU_IOMMUFD TYPE_VFIO_IOMMU "-iommufd"
 
 /*
  * VFIOContainerBase is not an abstract QOM object because it felt
diff --git a/hw/vfio/common.c b/hw/vfio/common.c
index 
2329d0efc8c1d617f0bfee5283e82b295d2d477d..89ff1c7aeda14d20b2e24f8bc251db0a71d4527c
 100644
--- a/hw/vfio/common.c
+++ b/hw/vfio/common.c
@@ -1508,7 +1508,7 @@ int vfio_attach_device(char *name, VFIODevice *vbasedev,
 
 #ifdef CONFIG_IOMMUFD
 if (vbasedev->iommufd) {
-ops = _iommufd_ops;
+ops = VFIO_IOMMU_CLASS(object_class_by_name(TYPE_VFIO_IOMMU_IOMMUFD));
 }
 #endif
 
diff --git a/hw/vfio/iommufd.c b/hw/vfio/iommufd.c
index 
87a561c54580adc6d7b2711331a00940ff13bd43..d4c586e842def8f04d3a914843f5eece2c75ea30
 100644
--- a/hw/vfio/iommufd.c
+++ b/hw/vfio/iommufd.c
@@ -319,6 +319,8 @@ static int iommufd_cdev_attach(const char *name, VFIODevice 
*vbasedev,
 int ret, devfd;
 uint32_t ioas_id;
 Error *err = NULL;
+const VFIOIOMMUClass *iommufd_vioc =
+VFIO_IOMMU_CLASS(object_class_by_name(TYPE_VFIO_IOMMU_IOMMUFD));
 
 if (vbasedev->fd < 0) {
 devfd = iommufd_cdev_getfd(vbasedev->sysfsdev, errp);
@@ -340,7 +342,7 @@ static int iommufd_cdev_attach(const char *name, VFIODevice 
*vbasedev,
 /* try to attach to an existing container in this space */
 QLIST_FOREACH(bcontainer, >containers, next) {
 container = container_of(bcontainer, VFIOIOMMUFDContainer, bcontainer);
-if (bcontainer->ops != _iommufd_ops ||
+if (bcontainer->ops != iommufd_vioc ||
 vbasedev->iommufd != container->be) {
 continue;
 }
@@ -374,7 +376,7 @@ static int iommufd_cdev_attach(const char *name, VFIODevice 
*vbasedev,
 container->ioas_id = ioas_id;
 
 bcontainer = >bcontainer;
-vfio_container_init(bcontainer, space, _iommufd_ops);
+vfio_container_init(bcontainer, space, iommufd_vioc);
 QLIST_INSERT_HEAD(>containers, bcontainer, next);
 
 ret = iommufd_cdev_attach_container(vbasedev, container, errp);
@@ -476,9 +478,11 @@ static void iommufd_cdev_detach(VFIODevice *vbasedev)
 static VFIODevice *iommufd_cdev_pci_find_by_devid(__u32 devid)
 {
 VFIODevice *vbasedev_iter;
+const VFIOIOMMUClass *iommufd_vioc =
+VFIO_IOMMU_CLASS(object_class_by_name(TYPE_VFIO_IOMMU_IOMMUFD));
 
 QLIST_FOREACH(vbasedev_iter, _device_list, global_next) {
-if (vbasedev_iter->bcontainer->ops != _iommufd_ops) {
+if (vbasedev_iter->bcontainer->ops != iommufd_vioc) {
 continue;
 }
 if (devid == vbasedev_iter->devid) {
@@ -621,10 +625,23 @@ out_single:
 return ret;
 }
 
-const VFIOIOMMUOps vfio_iommufd_ops = {
-.dma_map = iommufd_cdev_map,
-.dma_unmap = iommufd_cdev_unmap,
-.attach_device = iommufd_cdev_attach,
-.detach_device = iommufd_cdev_detach,
-.pci_hot_reset = iommufd_cdev_pci_hot_reset,
+static void vfio_iommu_iommufd_class_init(ObjectClass *klass, void *data)
+{
+VFIOIOMMUClass *vioc = VFIO_IOMMU_CLASS(klass);
+
+vioc->dma_map = iommufd_cdev_map;
+vioc->dma_unmap = iommufd_cdev_unmap;

[PULL 01/17] vfio/spapr: Extend VFIOIOMMUOps with a release handler

2024-01-07 Thread Cédric Le Goater
This allows to abstract a bit more the sPAPR IOMMU support in the
legacy IOMMU backend.

Reviewed-by: Zhenzhong Duan 
Tested-by: Eric Farman 
Signed-off-by: Cédric Le Goater 
---
 include/hw/vfio/vfio-container-base.h |  1 +
 hw/vfio/container.c   | 10 +++-
 hw/vfio/spapr.c   | 35 +++
 3 files changed, 24 insertions(+), 22 deletions(-)

diff --git a/include/hw/vfio/vfio-container-base.h 
b/include/hw/vfio/vfio-container-base.h
index 
2ae297ccda93fd97986c852a8329b390fa1ab91f..5c9594b6c77681e5593236e711e7e391e5f2bdff
 100644
--- a/include/hw/vfio/vfio-container-base.h
+++ b/include/hw/vfio/vfio-container-base.h
@@ -117,5 +117,6 @@ struct VFIOIOMMUOps {
   Error **errp);
 void (*del_window)(VFIOContainerBase *bcontainer,
MemoryRegionSection *section);
+void (*release)(VFIOContainerBase *bcontainer);
 };
 #endif /* HW_VFIO_VFIO_CONTAINER_BASE_H */
diff --git a/hw/vfio/container.c b/hw/vfio/container.c
index 
b22feb8ded0a0d9ed98d6e206b78c0c6e2554d5c..1e77a2929e90ed1d2ee84062549c477ae651c5a8
 100644
--- a/hw/vfio/container.c
+++ b/hw/vfio/container.c
@@ -632,9 +632,8 @@ listener_release_exit:
 QLIST_REMOVE(bcontainer, next);
 vfio_kvm_device_del_group(group);
 memory_listener_unregister(>listener);
-if (container->iommu_type == VFIO_SPAPR_TCE_v2_IOMMU ||
-container->iommu_type == VFIO_SPAPR_TCE_IOMMU) {
-vfio_spapr_container_deinit(container);
+if (bcontainer->ops->release) {
+bcontainer->ops->release(bcontainer);
 }
 
 enable_discards_exit:
@@ -667,9 +666,8 @@ static void vfio_disconnect_container(VFIOGroup *group)
  */
 if (QLIST_EMPTY(>group_list)) {
 memory_listener_unregister(>listener);
-if (container->iommu_type == VFIO_SPAPR_TCE_v2_IOMMU ||
-container->iommu_type == VFIO_SPAPR_TCE_IOMMU) {
-vfio_spapr_container_deinit(container);
+if (bcontainer->ops->release) {
+bcontainer->ops->release(bcontainer);
 }
 }
 
diff --git a/hw/vfio/spapr.c b/hw/vfio/spapr.c
index 
5c6426e6973bec606667ebcaca5b0585b184a214..44617dfc6b5f1a2a3a1c37436b76042aebda8b63
 100644
--- a/hw/vfio/spapr.c
+++ b/hw/vfio/spapr.c
@@ -440,6 +440,24 @@ vfio_spapr_container_del_section_window(VFIOContainerBase 
*bcontainer,
 }
 }
 
+static void vfio_spapr_container_release(VFIOContainerBase *bcontainer)
+{
+VFIOContainer *container = container_of(bcontainer, VFIOContainer,
+bcontainer);
+VFIOSpaprContainer *scontainer = container_of(container, 
VFIOSpaprContainer,
+  container);
+VFIOHostDMAWindow *hostwin, *next;
+
+if (container->iommu_type == VFIO_SPAPR_TCE_v2_IOMMU) {
+memory_listener_unregister(>prereg_listener);
+}
+QLIST_FOREACH_SAFE(hostwin, >hostwin_list, hostwin_next,
+   next) {
+QLIST_REMOVE(hostwin, hostwin_next);
+g_free(hostwin);
+}
+}
+
 static VFIOIOMMUOps vfio_iommu_spapr_ops;
 
 static void setup_spapr_ops(VFIOContainerBase *bcontainer)
@@ -447,6 +465,7 @@ static void setup_spapr_ops(VFIOContainerBase *bcontainer)
 vfio_iommu_spapr_ops = *bcontainer->ops;
 vfio_iommu_spapr_ops.add_window = vfio_spapr_container_add_section_window;
 vfio_iommu_spapr_ops.del_window = vfio_spapr_container_del_section_window;
+vfio_iommu_spapr_ops.release = vfio_spapr_container_release;
 bcontainer->ops = _iommu_spapr_ops;
 }
 
@@ -527,19 +546,3 @@ listener_unregister_exit:
 }
 return ret;
 }
-
-void vfio_spapr_container_deinit(VFIOContainer *container)
-{
-VFIOSpaprContainer *scontainer = container_of(container, 
VFIOSpaprContainer,
-  container);
-VFIOHostDMAWindow *hostwin, *next;
-
-if (container->iommu_type == VFIO_SPAPR_TCE_v2_IOMMU) {
-memory_listener_unregister(>prereg_listener);
-}
-QLIST_FOREACH_SAFE(hostwin, >hostwin_list, hostwin_next,
-   next) {
-QLIST_REMOVE(hostwin, hostwin_next);
-g_free(hostwin);
-}
-}
-- 
2.43.0




[PULL 04/17] vfio/container: Introduce a VFIOIOMMU QOM interface

2024-01-07 Thread Cédric Le Goater
VFIOContainerBase was not introduced as an abstract QOM object because
it felt unnecessary to expose all the IOMMU backends to the QEMU
machine and human interface. However, we can still abstract the IOMMU
backend handlers using a QOM interface class. This provides more
flexibility when referencing the various implementations.

Simply transform the VFIOIOMMUOps struct in an InterfaceClass and do
some initial name replacements. Next changes will start converting
VFIOIOMMUOps.

Reviewed-by: Zhenzhong Duan 
Tested-by: Eric Farman 
Signed-off-by: Cédric Le Goater 
---
 include/hw/vfio/vfio-container-base.h | 23 +++
 hw/vfio/common.c  |  2 +-
 hw/vfio/container-base.c  | 12 +++-
 hw/vfio/pci.c |  2 +-
 4 files changed, 32 insertions(+), 7 deletions(-)

diff --git a/include/hw/vfio/vfio-container-base.h 
b/include/hw/vfio/vfio-container-base.h
index 
5c9594b6c77681e5593236e711e7e391e5f2bdff..d6147b4aeef26b6075c88579108e566720f58ebb
 100644
--- a/include/hw/vfio/vfio-container-base.h
+++ b/include/hw/vfio/vfio-container-base.h
@@ -16,7 +16,8 @@
 #include "exec/memory.h"
 
 typedef struct VFIODevice VFIODevice;
-typedef struct VFIOIOMMUOps VFIOIOMMUOps;
+typedef struct VFIOIOMMUClass VFIOIOMMUClass;
+#define VFIOIOMMUOps VFIOIOMMUClass /* To remove */
 
 typedef struct {
 unsigned long *bitmap;
@@ -34,7 +35,7 @@ typedef struct VFIOAddressSpace {
  * This is the base object for vfio container backends
  */
 typedef struct VFIOContainerBase {
-const VFIOIOMMUOps *ops;
+const VFIOIOMMUClass *ops;
 VFIOAddressSpace *space;
 MemoryListener listener;
 Error *error;
@@ -88,10 +89,24 @@ int vfio_container_query_dirty_bitmap(const 
VFIOContainerBase *bcontainer,
 
 void vfio_container_init(VFIOContainerBase *bcontainer,
  VFIOAddressSpace *space,
- const VFIOIOMMUOps *ops);
+ const VFIOIOMMUClass *ops);
 void vfio_container_destroy(VFIOContainerBase *bcontainer);
 
-struct VFIOIOMMUOps {
+
+#define TYPE_VFIO_IOMMU "vfio-iommu"
+
+/*
+ * VFIOContainerBase is not an abstract QOM object because it felt
+ * unnecessary to expose all the IOMMU backends to the QEMU machine
+ * and human interface. However, we can still abstract the IOMMU
+ * backend handlers using a QOM interface class. This provides more
+ * flexibility when referencing the various implementations.
+ */
+DECLARE_CLASS_CHECKERS(VFIOIOMMUClass, VFIO_IOMMU, TYPE_VFIO_IOMMU)
+
+struct VFIOIOMMUClass {
+InterfaceClass parent_class;
+
 /* basic feature */
 int (*dma_map)(const VFIOContainerBase *bcontainer,
hwaddr iova, ram_addr_t size,
diff --git a/hw/vfio/common.c b/hw/vfio/common.c
index 
08a3e576725b1fc9f2f7e425375df3b827c4fe56..49dab41566f07ba7be1100fed1973e028d34467c
 100644
--- a/hw/vfio/common.c
+++ b/hw/vfio/common.c
@@ -1503,7 +1503,7 @@ retry:
 int vfio_attach_device(char *name, VFIODevice *vbasedev,
AddressSpace *as, Error **errp)
 {
-const VFIOIOMMUOps *ops = _legacy_ops;
+const VFIOIOMMUClass *ops = _legacy_ops;
 
 #ifdef CONFIG_IOMMUFD
 if (vbasedev->iommufd) {
diff --git a/hw/vfio/container-base.c b/hw/vfio/container-base.c
index 
1ffd25bbfa8bd3d404e43b96357273b95f5a0031..913ae49077c4f09b7b27517c1231cfbe4befb7fb
 100644
--- a/hw/vfio/container-base.c
+++ b/hw/vfio/container-base.c
@@ -72,7 +72,7 @@ int vfio_container_query_dirty_bitmap(const VFIOContainerBase 
*bcontainer,
 }
 
 void vfio_container_init(VFIOContainerBase *bcontainer, VFIOAddressSpace 
*space,
- const VFIOIOMMUOps *ops)
+ const VFIOIOMMUClass *ops)
 {
 bcontainer->ops = ops;
 bcontainer->space = space;
@@ -99,3 +99,13 @@ void vfio_container_destroy(VFIOContainerBase *bcontainer)
 
 g_list_free_full(bcontainer->iova_ranges, g_free);
 }
+
+static const TypeInfo types[] = {
+{
+.name = TYPE_VFIO_IOMMU,
+.parent = TYPE_INTERFACE,
+.class_size = sizeof(VFIOIOMMUClass),
+},
+};
+
+DEFINE_TYPES(types)
diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index 
9f838978bea11cacf95b0907c76ac0879a6313bf..d7fe06715c4b9cde66a68c31aaf405315921b0d6
 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -2488,7 +2488,7 @@ int vfio_pci_get_pci_hot_reset_info(VFIOPCIDevice *vdev,
 static int vfio_pci_hot_reset(VFIOPCIDevice *vdev, bool single)
 {
 VFIODevice *vbasedev = >vbasedev;
-const VFIOIOMMUOps *ops = vbasedev->bcontainer->ops;
+const VFIOIOMMUClass *ops = vbasedev->bcontainer->ops;
 
 return ops->pci_hot_reset(vbasedev, single);
 }
-- 
2.43.0




[PULL 11/17] vfio/container: Replace basename with g_path_get_basename

2024-01-07 Thread Cédric Le Goater
g_path_get_basename() is a portable utility function that has the
advantage of not modifing the string argument. It also fixes a compile
breakage with the Musl C library reported in [1].

[1] https://lore.kernel.org/all/20231212010228.2701544-1-raj.k...@gmail.com/

Reported-by: Khem Raj 
Reviewed-by: Eric Auger 
Reviewed-by: Zhao Liu 
Reviewed-by: Zhenzhong Duan 
Signed-off-by: Cédric Le Goater 
---
 hw/vfio/container.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/hw/vfio/container.c b/hw/vfio/container.c
index 
688cf23bab88f85246378bc5a7da3c51ea6b79d9..8d334f52f2438d05f632502e07ffd4dc2ec76cb5
 100644
--- a/hw/vfio/container.c
+++ b/hw/vfio/container.c
@@ -869,7 +869,8 @@ static void vfio_put_base_device(VFIODevice *vbasedev)
 
 static int vfio_device_groupid(VFIODevice *vbasedev, Error **errp)
 {
-char *tmp, group_path[PATH_MAX], *group_name;
+char *tmp, group_path[PATH_MAX];
+g_autofree char *group_name = NULL;
 int ret, groupid;
 ssize_t len;
 
@@ -885,7 +886,7 @@ static int vfio_device_groupid(VFIODevice *vbasedev, Error 
**errp)
 
 group_path[len] = 0;
 
-group_name = basename(group_path);
+group_name = g_path_get_basename(group_path);
 if (sscanf(group_name, "%d", ) != 1) {
 error_setg_errno(errp, errno, "failed to read %s", group_path);
 return -errno;
-- 
2.43.0




[PULL 06/17] vfio/container: Intoduce a new VFIOIOMMUClass::setup handler

2024-01-07 Thread Cédric Le Goater
This will help in converting the sPAPR IOMMU backend to a QOM interface.

Reviewed-by: Zhenzhong Duan 
Tested-by: Eric Farman 
Signed-off-by: Cédric Le Goater 
---
 include/hw/vfio/vfio-container-base.h | 1 +
 hw/vfio/container.c   | 1 +
 2 files changed, 2 insertions(+)

diff --git a/include/hw/vfio/vfio-container-base.h 
b/include/hw/vfio/vfio-container-base.h
index 
c60370fc5ebe65474816dbf2b065aa0912de1a3c..ce8b1fba88c145135adc20e96591bafd6050d5f1
 100644
--- a/include/hw/vfio/vfio-container-base.h
+++ b/include/hw/vfio/vfio-container-base.h
@@ -109,6 +109,7 @@ struct VFIOIOMMUClass {
 InterfaceClass parent_class;
 
 /* basic feature */
+int (*setup)(VFIOContainerBase *bcontainer, Error **errp);
 int (*dma_map)(const VFIOContainerBase *bcontainer,
hwaddr iova, ram_addr_t size,
void *vaddr, bool readonly);
diff --git a/hw/vfio/container.c b/hw/vfio/container.c
index 
220e838a917f9a135af1e040a450cb52064428cf..c22bdd321677026e52c7cdffce853523ef679cd0
 100644
--- a/hw/vfio/container.c
+++ b/hw/vfio/container.c
@@ -1129,6 +1129,7 @@ static void vfio_iommu_legacy_class_init(ObjectClass 
*klass, void *data)
 {
 VFIOIOMMUClass *vioc = VFIO_IOMMU_CLASS(klass);
 
+vioc->setup = vfio_legacy_setup;
 vioc->dma_map = vfio_legacy_dma_map;
 vioc->dma_unmap = vfio_legacy_dma_unmap;
 vioc->attach_device = vfio_legacy_attach_device;
-- 
2.43.0




[PULL 15/17] vfio/migration: Add helper function to set state or reset device

2024-01-07 Thread Cédric Le Goater
From: Avihai Horon 

There are several places where failure in setting the device state leads
to a device reset, which is done by setting ERROR as the recover state.

Add a helper function that sets the device state and resets the device
in case of failure. This will make the code cleaner and remove duplicate
comments.

Signed-off-by: Avihai Horon 
Reviewed-by: Cédric Le Goater 
Reviewed-by: Philippe Mathieu-Daudé 
---
 hw/vfio/migration.c | 41 +
 1 file changed, 17 insertions(+), 24 deletions(-)

diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
index 
28d422b39f9f70e94a2f396b0fb064c5de17dc28..70e6b1a709f9b67e4c9eb41033d76347275cac42
 100644
--- a/hw/vfio/migration.c
+++ b/hw/vfio/migration.c
@@ -163,6 +163,19 @@ reset_device:
 return ret;
 }
 
+/*
+ * Some device state transitions require resetting the device if they fail.
+ * This function sets the device in new_state and resets the device if that
+ * fails. Reset is done by using ERROR as the recover state.
+ */
+static int
+vfio_migration_set_state_or_reset(VFIODevice *vbasedev,
+  enum vfio_device_mig_state new_state)
+{
+return vfio_migration_set_state(vbasedev, new_state,
+VFIO_DEVICE_STATE_ERROR);
+}
+
 static int vfio_load_buffer(QEMUFile *f, VFIODevice *vbasedev,
 uint64_t data_size)
 {
@@ -422,12 +435,7 @@ static void vfio_save_cleanup(void *opaque)
  * after migration has completed, so it won't increase downtime.
  */
 if (migration->device_state == VFIO_DEVICE_STATE_STOP_COPY) {
-/*
- * If setting the device in STOP state fails, the device should be
- * reset. To do so, use ERROR state as a recover state.
- */
-vfio_migration_set_state(vbasedev, VFIO_DEVICE_STATE_STOP,
- VFIO_DEVICE_STATE_ERROR);
+vfio_migration_set_state_or_reset(vbasedev, VFIO_DEVICE_STATE_STOP);
 }
 
 g_free(migration->data_buffer);
@@ -699,12 +707,7 @@ static void vfio_vmstate_change_prepare(void *opaque, bool 
running,
 VFIO_DEVICE_STATE_PRE_COPY_P2P :
 VFIO_DEVICE_STATE_RUNNING_P2P;
 
-/*
- * If setting the device in new_state fails, the device should be reset.
- * To do so, use ERROR state as a recover state.
- */
-ret = vfio_migration_set_state(vbasedev, new_state,
-   VFIO_DEVICE_STATE_ERROR);
+ret = vfio_migration_set_state_or_reset(vbasedev, new_state);
 if (ret) {
 /*
  * Migration should be aborted in this case, but vm_state_notify()
@@ -736,12 +739,7 @@ static void vfio_vmstate_change(void *opaque, bool 
running, RunState state)
 VFIO_DEVICE_STATE_STOP;
 }
 
-/*
- * If setting the device in new_state fails, the device should be reset.
- * To do so, use ERROR state as a recover state.
- */
-ret = vfio_migration_set_state(vbasedev, new_state,
-   VFIO_DEVICE_STATE_ERROR);
+ret = vfio_migration_set_state_or_reset(vbasedev, new_state);
 if (ret) {
 /*
  * Migration should be aborted in this case, but vm_state_notify()
@@ -770,12 +768,7 @@ static void vfio_migration_state_notifier(Notifier 
*notifier, void *data)
 case MIGRATION_STATUS_CANCELLING:
 case MIGRATION_STATUS_CANCELLED:
 case MIGRATION_STATUS_FAILED:
-/*
- * If setting the device in RUNNING state fails, the device should
- * be reset. To do so, use ERROR state as a recover state.
- */
-vfio_migration_set_state(vbasedev, VFIO_DEVICE_STATE_RUNNING,
- VFIO_DEVICE_STATE_ERROR);
+vfio_migration_set_state_or_reset(vbasedev, VFIO_DEVICE_STATE_RUNNING);
 }
 }
 
-- 
2.43.0




[PULL 16/17] backends/iommufd: Remove check on number of backend users

2024-01-07 Thread Cédric Le Goater
QOM already has a ref count on objects and it will assert much
earlier, when INT_MAX is reached.

Reviewed-by: Eric Auger 
Reviewed-by: Zhenzhong Duan 
Signed-off-by: Cédric Le Goater 
---
 backends/iommufd.c | 5 -
 1 file changed, 5 deletions(-)

diff --git a/backends/iommufd.c b/backends/iommufd.c
index 
ba58a0eb0d0ba9aae625c987eb728547543dba66..393c0d9a3719e3de1a6b51a8ff2e75e184badc82
 100644
--- a/backends/iommufd.c
+++ b/backends/iommufd.c
@@ -80,11 +80,6 @@ int iommufd_backend_connect(IOMMUFDBackend *be, Error **errp)
 int fd, ret = 0;
 
 qemu_mutex_lock(>lock);
-if (be->users == UINT32_MAX) {
-error_setg(errp, "too many connections");
-ret = -E2BIG;
-goto out;
-}
 if (be->owned && !be->users) {
 fd = qemu_open_old("/dev/iommu", O_RDWR);
 if (fd < 0) {
-- 
2.43.0




[PULL 17/17] backends/iommufd: Remove mutex

2024-01-07 Thread Cédric Le Goater
Coverity reports a concurrent data access violation because be->users
is being accessed in iommufd_backend_can_be_deleted() without holding
the mutex.

However, these routines are called from the QEMU main thread when a
device is created. In this case, the code paths should be protected by
the BQL lock and it should be safe to drop the IOMMUFD backend mutex.
Simply remove it.

Fixes: CID 1531550
Fixes: CID 1531549
Reviewed-by: Eric Auger 
Reviewed-by: Zhenzhong Duan 
Signed-off-by: Cédric Le Goater 
---
 include/sysemu/iommufd.h | 2 --
 backends/iommufd.c   | 7 ---
 2 files changed, 9 deletions(-)

diff --git a/include/sysemu/iommufd.h b/include/sysemu/iommufd.h
index 
9c5524b0ed15ef5f81be159415bc216572a283d8..9af27ebd6ccb78ca8e16aa3c62629aab9f7f31e4
 100644
--- a/include/sysemu/iommufd.h
+++ b/include/sysemu/iommufd.h
@@ -2,7 +2,6 @@
 #define SYSEMU_IOMMUFD_H
 
 #include "qom/object.h"
-#include "qemu/thread.h"
 #include "exec/hwaddr.h"
 #include "exec/cpu-common.h"
 
@@ -19,7 +18,6 @@ struct IOMMUFDBackend {
 /*< protected >*/
 int fd;/* /dev/iommu file descriptor */
 bool owned;/* is the /dev/iommu opened internally */
-QemuMutex lock;
 uint32_t users;
 
 /*< public >*/
diff --git a/backends/iommufd.c b/backends/iommufd.c
index 
393c0d9a3719e3de1a6b51a8ff2e75e184badc82..1ef683c7b080e688af46c5b98e61eafa73e39895
 100644
--- a/backends/iommufd.c
+++ b/backends/iommufd.c
@@ -29,7 +29,6 @@ static void iommufd_backend_init(Object *obj)
 be->fd = -1;
 be->users = 0;
 be->owned = true;
-qemu_mutex_init(>lock);
 }
 
 static void iommufd_backend_finalize(Object *obj)
@@ -52,10 +51,8 @@ static void iommufd_backend_set_fd(Object *obj, const char 
*str, Error **errp)
 error_prepend(errp, "Could not parse remote object fd %s:", str);
 return;
 }
-qemu_mutex_lock(>lock);
 be->fd = fd;
 be->owned = false;
-qemu_mutex_unlock(>lock);
 trace_iommu_backend_set_fd(be->fd);
 }
 
@@ -79,7 +76,6 @@ int iommufd_backend_connect(IOMMUFDBackend *be, Error **errp)
 {
 int fd, ret = 0;
 
-qemu_mutex_lock(>lock);
 if (be->owned && !be->users) {
 fd = qemu_open_old("/dev/iommu", O_RDWR);
 if (fd < 0) {
@@ -93,13 +89,11 @@ int iommufd_backend_connect(IOMMUFDBackend *be, Error 
**errp)
 out:
 trace_iommufd_backend_connect(be->fd, be->owned,
   be->users, ret);
-qemu_mutex_unlock(>lock);
 return ret;
 }
 
 void iommufd_backend_disconnect(IOMMUFDBackend *be)
 {
-qemu_mutex_lock(>lock);
 if (!be->users) {
 goto out;
 }
@@ -110,7 +104,6 @@ void iommufd_backend_disconnect(IOMMUFDBackend *be)
 }
 out:
 trace_iommufd_backend_disconnect(be->fd, be->users);
-qemu_mutex_unlock(>lock);
 }
 
 int iommufd_backend_alloc_ioas(IOMMUFDBackend *be, uint32_t *ioas_id,
-- 
2.43.0




[PULL 05/17] vfio/container: Introduce a VFIOIOMMU legacy QOM interface

2024-01-07 Thread Cédric Le Goater
Convert the legacy VFIOIOMMUOps struct to the new VFIOIOMMU QOM
interface. The set of of operations for this backend can be referenced
with a literal typename instead of a C struct. This will simplify
support of multiple backends.

Reviewed-by: Zhenzhong Duan 
Tested-by: Eric Farman 
Signed-off-by: Cédric Le Goater 
---
 include/hw/vfio/vfio-common.h |  1 -
 include/hw/vfio/vfio-container-base.h |  1 +
 hw/vfio/common.c  |  6 ++-
 hw/vfio/container.c   | 58 ++-
 4 files changed, 55 insertions(+), 11 deletions(-)

diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
index 
b8aa8a549532442a31c8e85ce385c992d84f6bd5..14c497b6b0a79466e8f567aceed384ec2c75ea90
 100644
--- a/include/hw/vfio/vfio-common.h
+++ b/include/hw/vfio/vfio-common.h
@@ -210,7 +210,6 @@ typedef QLIST_HEAD(VFIOGroupList, VFIOGroup) VFIOGroupList;
 typedef QLIST_HEAD(VFIODeviceList, VFIODevice) VFIODeviceList;
 extern VFIOGroupList vfio_group_list;
 extern VFIODeviceList vfio_device_list;
-extern const VFIOIOMMUOps vfio_legacy_ops;
 extern const VFIOIOMMUOps vfio_iommufd_ops;
 extern const MemoryListener vfio_memory_listener;
 extern int vfio_kvm_device_fd;
diff --git a/include/hw/vfio/vfio-container-base.h 
b/include/hw/vfio/vfio-container-base.h
index 
d6147b4aeef26b6075c88579108e566720f58ebb..c60370fc5ebe65474816dbf2b065aa0912de1a3c
 100644
--- a/include/hw/vfio/vfio-container-base.h
+++ b/include/hw/vfio/vfio-container-base.h
@@ -94,6 +94,7 @@ void vfio_container_destroy(VFIOContainerBase *bcontainer);
 
 
 #define TYPE_VFIO_IOMMU "vfio-iommu"
+#define TYPE_VFIO_IOMMU_LEGACY TYPE_VFIO_IOMMU "-legacy"
 
 /*
  * VFIOContainerBase is not an abstract QOM object because it felt
diff --git a/hw/vfio/common.c b/hw/vfio/common.c
index 
49dab41566f07ba7be1100fed1973e028d34467c..2329d0efc8c1d617f0bfee5283e82b295d2d477d
 100644
--- a/hw/vfio/common.c
+++ b/hw/vfio/common.c
@@ -1503,13 +1503,17 @@ retry:
 int vfio_attach_device(char *name, VFIODevice *vbasedev,
AddressSpace *as, Error **errp)
 {
-const VFIOIOMMUClass *ops = _legacy_ops;
+const VFIOIOMMUClass *ops =
+VFIO_IOMMU_CLASS(object_class_by_name(TYPE_VFIO_IOMMU_LEGACY));
 
 #ifdef CONFIG_IOMMUFD
 if (vbasedev->iommufd) {
 ops = _iommufd_ops;
 }
 #endif
+
+assert(ops);
+
 return ops->attach_device(name, vbasedev, as, errp);
 }
 
diff --git a/hw/vfio/container.c b/hw/vfio/container.c
index 
f4a0434a5239bfb6a17b91c8879cb98e686afccc..220e838a917f9a135af1e040a450cb52064428cf
 100644
--- a/hw/vfio/container.c
+++ b/hw/vfio/container.c
@@ -369,10 +369,30 @@ static int vfio_get_iommu_type(VFIOContainer *container,
 return -EINVAL;
 }
 
+/*
+ * vfio_get_iommu_ops - get a VFIOIOMMUClass associated with a type
+ */
+static const VFIOIOMMUClass *vfio_get_iommu_class(int iommu_type, Error **errp)
+{
+ObjectClass *klass = NULL;
+
+switch (iommu_type) {
+case VFIO_TYPE1v2_IOMMU:
+case VFIO_TYPE1_IOMMU:
+klass = object_class_by_name(TYPE_VFIO_IOMMU_LEGACY);
+break;
+default:
+g_assert_not_reached();
+};
+
+return VFIO_IOMMU_CLASS(klass);
+}
+
 static int vfio_init_container(VFIOContainer *container, int group_fd,
VFIOAddressSpace *space, Error **errp)
 {
 int iommu_type, ret;
+const VFIOIOMMUClass *vioc;
 
 iommu_type = vfio_get_iommu_type(container, errp);
 if (iommu_type < 0) {
@@ -401,7 +421,14 @@ static int vfio_init_container(VFIOContainer *container, 
int group_fd,
 }
 
 container->iommu_type = iommu_type;
-vfio_container_init(>bcontainer, space, _legacy_ops);
+
+vioc = vfio_get_iommu_class(iommu_type, errp);
+if (!vioc) {
+error_setg(errp, "No available IOMMU models");
+return -EINVAL;
+}
+
+vfio_container_init(>bcontainer, space, vioc);
 return 0;
 }
 
@@ -1098,12 +1125,25 @@ out_single:
 return ret;
 }
 
-const VFIOIOMMUOps vfio_legacy_ops = {
-.dma_map = vfio_legacy_dma_map,
-.dma_unmap = vfio_legacy_dma_unmap,
-.attach_device = vfio_legacy_attach_device,
-.detach_device = vfio_legacy_detach_device,
-.set_dirty_page_tracking = vfio_legacy_set_dirty_page_tracking,
-.query_dirty_bitmap = vfio_legacy_query_dirty_bitmap,
-.pci_hot_reset = vfio_legacy_pci_hot_reset,
+static void vfio_iommu_legacy_class_init(ObjectClass *klass, void *data)
+{
+VFIOIOMMUClass *vioc = VFIO_IOMMU_CLASS(klass);
+
+vioc->dma_map = vfio_legacy_dma_map;
+vioc->dma_unmap = vfio_legacy_dma_unmap;
+vioc->attach_device = vfio_legacy_attach_device;
+vioc->detach_device = vfio_legacy_detach_device;
+vioc->set_dirty_page_tracking = vfio_legacy_set_dirty_page_tracking;
+vioc->query_dirty_bitmap = vfio_legacy_query_dirty_bitmap;
+vioc->pci_hot_reset = vfio_legacy_pci_hot_reset;
 };
+
+static const TypeInfo types[] = {
+{
+.name = TYPE_VFIO_IOMMU_LEGACY,
+

[PULL 09/17] vfio/spapr: Only compile sPAPR IOMMU support when needed

2024-01-07 Thread Cédric Le Goater
sPAPR IOMMU support is only needed for pseries machines. Compile out
support when CONFIG_PSERIES is not set. This saves ~7K of text.

Reviewed-by: Zhenzhong Duan 
Tested-by: Eric Farman 
Signed-off-by: Cédric Le Goater 
---
 hw/vfio/meson.build | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/vfio/meson.build b/hw/vfio/meson.build
index 
e5d98b6adc223061f6b0c3e1a7db3ba93d4eef16..bb98493b53e858c53181e224f9cb46892838a8be
 100644
--- a/hw/vfio/meson.build
+++ b/hw/vfio/meson.build
@@ -4,9 +4,9 @@ vfio_ss.add(files(
   'common.c',
   'container-base.c',
   'container.c',
-  'spapr.c',
   'migration.c',
 ))
+vfio_ss.add(when: 'CONFIG_PSERIES', if_true: files('spapr.c'))
 vfio_ss.add(when: 'CONFIG_IOMMUFD', if_true: files(
   'iommufd.c',
 ))
-- 
2.43.0




[PULL 12/17] hw/vfio: fix iteration over global VFIODevice list

2024-01-07 Thread Cédric Le Goater
From: Volker Rümelin 

Commit 3d779abafe ("vfio/common: Introduce a global VFIODevice list")
introduced a global VFIODevice list, but forgot to update the list
element field name when iterating over the new list. Change the code
to use the correct list element field.

Fixes: 3d779abafe ("vfio/common: Introduce a global VFIODevice list")
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2061
Signed-off-by: Volker Rümelin 
Reviewed-by: Zhenzhong Duan 
Reviewed-by: Cédric Le Goater 
Reviewed-by: Eric Auger 
---
 hw/vfio/common.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/hw/vfio/common.c b/hw/vfio/common.c
index 
0d4d8b8416c6a4770677e1ebe5e1fc7dbaaef004..0b3352f2a9d278f252a460e339732f1ccac0a96d
 100644
--- a/hw/vfio/common.c
+++ b/hw/vfio/common.c
@@ -73,7 +73,7 @@ bool vfio_mig_active(void)
 return false;
 }
 
-QLIST_FOREACH(vbasedev, _device_list, next) {
+QLIST_FOREACH(vbasedev, _device_list, global_next) {
 if (vbasedev->migration_blocker) {
 return false;
 }
@@ -94,7 +94,7 @@ static bool vfio_multiple_devices_migration_is_supported(void)
 unsigned int device_num = 0;
 bool all_support_p2p = true;
 
-QLIST_FOREACH(vbasedev, _device_list, next) {
+QLIST_FOREACH(vbasedev, _device_list, global_next) {
 if (vbasedev->migration) {
 device_num++;
 
@@ -1366,13 +1366,13 @@ void vfio_reset_handler(void *opaque)
 {
 VFIODevice *vbasedev;
 
-QLIST_FOREACH(vbasedev, _device_list, next) {
+QLIST_FOREACH(vbasedev, _device_list, global_next) {
 if (vbasedev->dev->realized) {
 vbasedev->ops->vfio_compute_needs_reset(vbasedev);
 }
 }
 
-QLIST_FOREACH(vbasedev, _device_list, next) {
+QLIST_FOREACH(vbasedev, _device_list, global_next) {
 if (vbasedev->dev->realized && vbasedev->needs_reset) {
 vbasedev->ops->vfio_hot_reset_multi(vbasedev);
 }
-- 
2.43.0




[PULL 03/17] vfio/container: Initialize VFIOIOMMUOps under vfio_init_container()

2024-01-07 Thread Cédric Le Goater
vfio_init_container() already defines the IOMMU type of the container.
Do the same for the VFIOIOMMUOps struct. This prepares ground for the
following patches that will deduce the associated VFIOIOMMUOps struct
from the IOMMU type.

Reviewed-by: Zhenzhong Duan 
Tested-by: Eric Farman 
Signed-off-by: Cédric Le Goater 
---
 hw/vfio/container.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/vfio/container.c b/hw/vfio/container.c
index 
afcfe8048805c58291d1104ff0ef20bdc457f99c..f4a0434a5239bfb6a17b91c8879cb98e686afccc
 100644
--- a/hw/vfio/container.c
+++ b/hw/vfio/container.c
@@ -370,7 +370,7 @@ static int vfio_get_iommu_type(VFIOContainer *container,
 }
 
 static int vfio_init_container(VFIOContainer *container, int group_fd,
-   Error **errp)
+   VFIOAddressSpace *space, Error **errp)
 {
 int iommu_type, ret;
 
@@ -401,6 +401,7 @@ static int vfio_init_container(VFIOContainer *container, 
int group_fd,
 }
 
 container->iommu_type = iommu_type;
+vfio_container_init(>bcontainer, space, _legacy_ops);
 return 0;
 }
 
@@ -583,9 +584,8 @@ static int vfio_connect_container(VFIOGroup *group, 
AddressSpace *as,
 container = g_malloc0(sizeof(*container));
 container->fd = fd;
 bcontainer = >bcontainer;
-vfio_container_init(bcontainer, space, _legacy_ops);
 
-ret = vfio_init_container(container, group->fd, errp);
+ret = vfio_init_container(container, group->fd, space, errp);
 if (ret) {
 goto free_container_exit;
 }
-- 
2.43.0




[PULL 00/17] vfio queue

2024-01-07 Thread Cédric Le Goater
The following changes since commit 0c1eccd368af8805ec0fb11e6cf25d0684d37328:

  Merge tag 'hw-cpus-20240105' of https://github.com/philmd/qemu into staging 
(2024-01-05 16:08:58 +)

are available in the Git repository at:

  https://github.com/legoater/qemu/ tags/pull-vfio-20240107

for you to fetch changes up to 19368b1905b4b917e915526fcbd5bfa3f7439451:

  backends/iommufd: Remove mutex (2024-01-05 21:25:20 +0100)


vfio queue:

* Minor cleanups
* Fix for a regression in device reset introduced in 8.2
* Coverity fixes, including the removal of the iommufd backend mutex
* Introduced VFIOIOMMUClass, to avoid compiling spapr when !CONFIG_PSERIES


Avihai Horon (1):
  vfio/migration: Add helper function to set state or reset device

Cédric Le Goater (14):
  vfio/spapr: Extend VFIOIOMMUOps with a release handler
  vfio/container: Introduce vfio_legacy_setup() for further cleanups
  vfio/container: Initialize VFIOIOMMUOps under vfio_init_container()
  vfio/container: Introduce a VFIOIOMMU QOM interface
  vfio/container: Introduce a VFIOIOMMU legacy QOM interface
  vfio/container: Intoduce a new VFIOIOMMUClass::setup handler
  vfio/spapr: Introduce a sPAPR VFIOIOMMU QOM interface
  vfio/iommufd: Introduce a VFIOIOMMU iommufd QOM interface
  vfio/spapr: Only compile sPAPR IOMMU support when needed
  vfio/iommufd: Remove CONFIG_IOMMUFD usage
  vfio/container: Replace basename with g_path_get_basename
  vfio/iommufd: Remove the use of stat() to check file existence
  backends/iommufd: Remove check on number of backend users
  backends/iommufd: Remove mutex

Volker Rümelin (1):
  hw/vfio: fix iteration over global VFIODevice list

Zhenzhong Duan (1):
  vfio/container: Rename vfio_init_container to vfio_set_iommu

 include/hw/vfio/vfio-common.h |   2 -
 include/hw/vfio/vfio-container-base.h |  27 +-
 include/sysemu/iommufd.h  |   2 -
 backends/iommufd.c|  12 ---
 hw/vfio/common.c  |  19 +++--
 hw/vfio/container-base.c  |  12 ++-
 hw/vfio/container.c   | 153 +-
 hw/vfio/iommufd.c |  41 +
 hw/vfio/migration.c   |  41 -
 hw/vfio/pci.c |   2 +-
 hw/vfio/spapr.c   |  60 +++--
 hw/vfio/meson.build   |   2 +-
 12 files changed, 222 insertions(+), 151 deletions(-)



[PULL 14/17] vfio/container: Rename vfio_init_container to vfio_set_iommu

2024-01-07 Thread Cédric Le Goater
From: Zhenzhong Duan 

vfio_container_init() and vfio_init_container() names are confusing
especially when we see vfio_init_container() calls vfio_container_init().

vfio_container_init() operates on base container which is consistent
with all routines handling 'VFIOContainerBase *' ops.

vfio_init_container() operates on legacy container and setup IOMMU
context with ioctl(VFIO_SET_IOMMU).

So choose to rename vfio_init_container to vfio_set_iommu to avoid
the confusion.

No functional change intended.

Suggested-by: Cédric Le Goater 
Signed-off-by: Zhenzhong Duan 
Reviewed-by: Cédric Le Goater 
---
 hw/vfio/container.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/vfio/container.c b/hw/vfio/container.c
index 
8d334f52f2438d05f632502e07ffd4dc2ec76cb5..bd25b9fbad2e717e63c2ab0e331186e5f63cef49
 100644
--- a/hw/vfio/container.c
+++ b/hw/vfio/container.c
@@ -392,8 +392,8 @@ static const VFIOIOMMUClass *vfio_get_iommu_class(int 
iommu_type, Error **errp)
 return VFIO_IOMMU_CLASS(klass);
 }
 
-static int vfio_init_container(VFIOContainer *container, int group_fd,
-   VFIOAddressSpace *space, Error **errp)
+static int vfio_set_iommu(VFIOContainer *container, int group_fd,
+  VFIOAddressSpace *space, Error **errp)
 {
 int iommu_type, ret;
 const VFIOIOMMUClass *vioc;
@@ -616,7 +616,7 @@ static int vfio_connect_container(VFIOGroup *group, 
AddressSpace *as,
 container->fd = fd;
 bcontainer = >bcontainer;
 
-ret = vfio_init_container(container, group->fd, space, errp);
+ret = vfio_set_iommu(container, group->fd, space, errp);
 if (ret) {
 goto free_container_exit;
 }
-- 
2.43.0




[PULL 02/17] vfio/container: Introduce vfio_legacy_setup() for further cleanups

2024-01-07 Thread Cédric Le Goater
This will help subsequent patches to unify the initialization of type1
and sPAPR IOMMU backends.

Reviewed-by: Zhenzhong Duan 
Tested-by: Eric Farman 
Signed-off-by: Cédric Le Goater 
---
 hw/vfio/container.c | 63 +
 1 file changed, 35 insertions(+), 28 deletions(-)

diff --git a/hw/vfio/container.c b/hw/vfio/container.c
index 
1e77a2929e90ed1d2ee84062549c477ae651c5a8..afcfe8048805c58291d1104ff0ef20bdc457f99c
 100644
--- a/hw/vfio/container.c
+++ b/hw/vfio/container.c
@@ -474,6 +474,35 @@ static void vfio_get_iommu_info_migration(VFIOContainer 
*container,
 }
 }
 
+static int vfio_legacy_setup(VFIOContainerBase *bcontainer, Error **errp)
+{
+VFIOContainer *container = container_of(bcontainer, VFIOContainer,
+bcontainer);
+g_autofree struct vfio_iommu_type1_info *info = NULL;
+int ret;
+
+ret = vfio_get_iommu_info(container, );
+if (ret) {
+error_setg_errno(errp, -ret, "Failed to get VFIO IOMMU info");
+return ret;
+}
+
+if (info->flags & VFIO_IOMMU_INFO_PGSIZES) {
+bcontainer->pgsizes = info->iova_pgsizes;
+} else {
+bcontainer->pgsizes = qemu_real_host_page_size();
+}
+
+if (!vfio_get_info_dma_avail(info, >dma_max_mappings)) {
+bcontainer->dma_max_mappings = 65535;
+}
+
+vfio_get_info_iova_range(info, bcontainer);
+
+vfio_get_iommu_info_migration(container, info);
+return 0;
+}
+
 static int vfio_connect_container(VFIOGroup *group, AddressSpace *as,
   Error **errp)
 {
@@ -570,40 +599,18 @@ static int vfio_connect_container(VFIOGroup *group, 
AddressSpace *as,
 switch (container->iommu_type) {
 case VFIO_TYPE1v2_IOMMU:
 case VFIO_TYPE1_IOMMU:
-{
-struct vfio_iommu_type1_info *info;
-
-ret = vfio_get_iommu_info(container, );
-if (ret) {
-error_setg_errno(errp, -ret, "Failed to get VFIO IOMMU info");
-goto enable_discards_exit;
-}
-
-if (info->flags & VFIO_IOMMU_INFO_PGSIZES) {
-bcontainer->pgsizes = info->iova_pgsizes;
-} else {
-bcontainer->pgsizes = qemu_real_host_page_size();
-}
-
-if (!vfio_get_info_dma_avail(info, >dma_max_mappings)) {
-bcontainer->dma_max_mappings = 65535;
-}
-
-vfio_get_info_iova_range(info, bcontainer);
-
-vfio_get_iommu_info_migration(container, info);
-g_free(info);
+ret = vfio_legacy_setup(bcontainer, errp);
 break;
-}
 case VFIO_SPAPR_TCE_v2_IOMMU:
 case VFIO_SPAPR_TCE_IOMMU:
-{
 ret = vfio_spapr_container_init(container, errp);
-if (ret) {
-goto enable_discards_exit;
-}
 break;
+default:
+g_assert_not_reached();
 }
+
+if (ret) {
+goto enable_discards_exit;
 }
 
 vfio_kvm_device_add_group(group);
-- 
2.43.0




[PULL 07/17] vfio/spapr: Introduce a sPAPR VFIOIOMMU QOM interface

2024-01-07 Thread Cédric Le Goater
Move vfio_spapr_container_setup() to a VFIOIOMMUClass::setup handler
and convert the sPAPR VFIOIOMMUOps struct to a QOM interface. The
sPAPR QOM interface inherits from the legacy QOM interface because
because both have the same basic needs. The sPAPR interface is then
extended with the handlers specific to the sPAPR IOMMU.

This allows reuse and provides better abstraction of the backends. It
will be useful to avoid compiling the sPAPR IOMMU backend on targets
not supporting it.

Reviewed-by: Zhenzhong Duan 
Tested-by: Eric Farman 
Signed-off-by: Cédric Le Goater 
---
 include/hw/vfio/vfio-container-base.h |  1 +
 hw/vfio/container.c   | 18 +
 hw/vfio/spapr.c   | 39 ---
 3 files changed, 31 insertions(+), 27 deletions(-)

diff --git a/include/hw/vfio/vfio-container-base.h 
b/include/hw/vfio/vfio-container-base.h
index 
ce8b1fba88c145135adc20e96591bafd6050d5f1..9e21d7811f3810ca2c63d9f28bdcc9aa6f75f9ad
 100644
--- a/include/hw/vfio/vfio-container-base.h
+++ b/include/hw/vfio/vfio-container-base.h
@@ -95,6 +95,7 @@ void vfio_container_destroy(VFIOContainerBase *bcontainer);
 
 #define TYPE_VFIO_IOMMU "vfio-iommu"
 #define TYPE_VFIO_IOMMU_LEGACY TYPE_VFIO_IOMMU "-legacy"
+#define TYPE_VFIO_IOMMU_SPAPR TYPE_VFIO_IOMMU "-spapr"
 
 /*
  * VFIOContainerBase is not an abstract QOM object because it felt
diff --git a/hw/vfio/container.c b/hw/vfio/container.c
index 
c22bdd321677026e52c7cdffce853523ef679cd0..688cf23bab88f85246378bc5a7da3c51ea6b79d9
 100644
--- a/hw/vfio/container.c
+++ b/hw/vfio/container.c
@@ -381,6 +381,10 @@ static const VFIOIOMMUClass *vfio_get_iommu_class(int 
iommu_type, Error **errp)
 case VFIO_TYPE1_IOMMU:
 klass = object_class_by_name(TYPE_VFIO_IOMMU_LEGACY);
 break;
+case VFIO_SPAPR_TCE_v2_IOMMU:
+case VFIO_SPAPR_TCE_IOMMU:
+klass = object_class_by_name(TYPE_VFIO_IOMMU_SPAPR);
+break;
 default:
 g_assert_not_reached();
 };
@@ -623,19 +627,9 @@ static int vfio_connect_container(VFIOGroup *group, 
AddressSpace *as,
 goto free_container_exit;
 }
 
-switch (container->iommu_type) {
-case VFIO_TYPE1v2_IOMMU:
-case VFIO_TYPE1_IOMMU:
-ret = vfio_legacy_setup(bcontainer, errp);
-break;
-case VFIO_SPAPR_TCE_v2_IOMMU:
-case VFIO_SPAPR_TCE_IOMMU:
-ret = vfio_spapr_container_init(container, errp);
-break;
-default:
-g_assert_not_reached();
-}
+assert(bcontainer->ops->setup);
 
+ret = bcontainer->ops->setup(bcontainer, errp);
 if (ret) {
 goto enable_discards_exit;
 }
diff --git a/hw/vfio/spapr.c b/hw/vfio/spapr.c
index 
44617dfc6b5f1a2a3a1c37436b76042aebda8b63..0d949bb728212534a7e2296e491aa8d95f45945d
 100644
--- a/hw/vfio/spapr.c
+++ b/hw/vfio/spapr.c
@@ -458,20 +458,11 @@ static void 
vfio_spapr_container_release(VFIOContainerBase *bcontainer)
 }
 }
 
-static VFIOIOMMUOps vfio_iommu_spapr_ops;
-
-static void setup_spapr_ops(VFIOContainerBase *bcontainer)
-{
-vfio_iommu_spapr_ops = *bcontainer->ops;
-vfio_iommu_spapr_ops.add_window = vfio_spapr_container_add_section_window;
-vfio_iommu_spapr_ops.del_window = vfio_spapr_container_del_section_window;
-vfio_iommu_spapr_ops.release = vfio_spapr_container_release;
-bcontainer->ops = _iommu_spapr_ops;
-}
-
-int vfio_spapr_container_init(VFIOContainer *container, Error **errp)
+static int vfio_spapr_container_setup(VFIOContainerBase *bcontainer,
+  Error **errp)
 {
-VFIOContainerBase *bcontainer = >bcontainer;
+VFIOContainer *container = container_of(bcontainer, VFIOContainer,
+bcontainer);
 VFIOSpaprContainer *scontainer = container_of(container, 
VFIOSpaprContainer,
   container);
 struct vfio_iommu_spapr_tce_info info;
@@ -536,8 +527,6 @@ int vfio_spapr_container_init(VFIOContainer *container, 
Error **errp)
   0x1000);
 }
 
-setup_spapr_ops(bcontainer);
-
 return 0;
 
 listener_unregister_exit:
@@ -546,3 +535,23 @@ listener_unregister_exit:
 }
 return ret;
 }
+
+static void vfio_iommu_spapr_class_init(ObjectClass *klass, void *data)
+{
+VFIOIOMMUClass *vioc = VFIO_IOMMU_CLASS(klass);
+
+vioc->add_window = vfio_spapr_container_add_section_window;
+vioc->del_window = vfio_spapr_container_del_section_window;
+vioc->release = vfio_spapr_container_release;
+vioc->setup = vfio_spapr_container_setup;
+};
+
+static const TypeInfo types[] = {
+{
+.name = TYPE_VFIO_IOMMU_SPAPR,
+.parent = TYPE_VFIO_IOMMU_LEGACY,
+.class_init = vfio_iommu_spapr_class_init,
+},
+};
+
+DEFINE_TYPES(types)
-- 
2.43.0




[PULL 10/17] vfio/iommufd: Remove CONFIG_IOMMUFD usage

2024-01-07 Thread Cédric Le Goater
Availability of the IOMMUFD backend can now be fully determined at
runtime and the ifdef check was a build time protection (for PPC not
supporting it mostly).

Reviewed-by: Zhenzhong Duan 
Tested-by: Eric Farman 
Signed-off-by: Cédric Le Goater 
---
 hw/vfio/common.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/hw/vfio/common.c b/hw/vfio/common.c
index 
89ff1c7aeda14d20b2e24f8bc251db0a71d4527c..0d4d8b8416c6a4770677e1ebe5e1fc7dbaaef004
 100644
--- a/hw/vfio/common.c
+++ b/hw/vfio/common.c
@@ -19,7 +19,6 @@
  */
 
 #include "qemu/osdep.h"
-#include CONFIG_DEVICES /* CONFIG_IOMMUFD */
 #include 
 #ifdef CONFIG_KVM
 #include 
@@ -1506,11 +1505,9 @@ int vfio_attach_device(char *name, VFIODevice *vbasedev,
 const VFIOIOMMUClass *ops =
 VFIO_IOMMU_CLASS(object_class_by_name(TYPE_VFIO_IOMMU_LEGACY));
 
-#ifdef CONFIG_IOMMUFD
 if (vbasedev->iommufd) {
 ops = VFIO_IOMMU_CLASS(object_class_by_name(TYPE_VFIO_IOMMU_IOMMUFD));
 }
-#endif
 
 assert(ops);
 
-- 
2.43.0




Re: [PATCH] tcg/tci: Fix TCI on hppa host and update TCI test matrix

2024-01-07 Thread Akihiko Odaki

On 2024/01/07 19:54, Philippe Mathieu-Daudé wrote:

Cc'ing Akihiko for commit a1eaa6281f.

On 7/1/24 08:19, Helge Deller wrote:

Update the TCI interpreter test matrix for big-endian hosts with
big- (hppa, hppa64) and little-endian (x86,x96-64) targets.
I used native ppc64 and hppa hosts for those tests.

Starting TCI on a hppa host crashed immediately, because hppa is
the only archive left where the stack grows upwards.
Write-protecting the stack guard page at the top of the stack
fixes the crash.



Fixes: a1eaa6281f ("util: Delete checks for old host definitions")


The change is intentional. If you need HP-PA host support, please 
explain why you need to run latest QEMU on HP-PA. You may also look at 
commit b1cef6d02f ("Drop remaining bits of ia64 host support"), which is 
mentioned in commit a1eaa6281f ("util: Delete checks for old host 
definitions"), if you are going to restore HP-PA host support.


Regards,
Akihiko Odaki



Re: [PATCH 3/3] tests/qtest: Re-enable multifd cancel test

2024-01-07 Thread Peter Xu
On Wed, Jun 07, 2023 at 10:27:15AM +0200, Juan Quintela wrote:
> Fabiano Rosas  wrote:
> > We've found the source of flakiness in this test, so re-enable it.
> >
> > Signed-off-by: Fabiano Rosas 
> > ---
> >  tests/qtest/migration-test.c | 10 ++
> >  1 file changed, 2 insertions(+), 8 deletions(-)
> >
> > diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
> > index b0c355bbd9..800ad23b75 100644
> > --- a/tests/qtest/migration-test.c
> > +++ b/tests/qtest/migration-test.c
> > @@ -2778,14 +2778,8 @@ int main(int argc, char **argv)
> >  }
> >  qtest_add_func("/migration/multifd/tcp/plain/none",
> > test_multifd_tcp_none);
> > -/*
> > - * This test is flaky and sometimes fails in CI and otherwise:
> > - * don't run unless user opts in via environment variable.
> > - */
> > -if (getenv("QEMU_TEST_FLAKY_TESTS")) {
> > -qtest_add_func("/migration/multifd/tcp/plain/cancel",
> > -   test_multifd_tcp_cancel);
> > -}
> > +qtest_add_func("/migration/multifd/tcp/plain/cancel",
> > +   test_multifd_tcp_cancel);
> >  qtest_add_func("/migration/multifd/tcp/plain/zlib",
> > test_multifd_tcp_zlib);
> >  #ifdef CONFIG_ZSTD
> 
> Reviewed-by: Juan Quintela 
> 
> 
> There was another failure with migration test that I will post during
> the rest of the day.  It needs both to get it right.

This one didn't yet land upstream.  I'm not sure, but maybe Juan was saying
about this change:

commit d2026ee117147893f8d80f060cede6d872ecbd7f
Author: Juan Quintela 
Date:   Wed Apr 26 12:20:36 2023 +0200

multifd: Fix the number of channels ready

Fabiano, did you try to reproduce multifd-cancel with current master?  I'm
wondering whether this test has already been completely fixed, then maybe
we can pick up this patch now.

-- 
Peter Xu




RE: [External] Re: [PATCH 3/5] migration: Introduce unimplemented 'qatzip' compression method

2024-01-07 Thread Liu, Yuan1
> -Original Message-
> From: Hao Xiang 
> Sent: Saturday, January 6, 2024 7:53 AM
> To: Fabiano Rosas 
> Cc: Bryan Zhang ; qemu-devel@nongnu.org;
> marcandre.lur...@redhat.com; pet...@redhat.com; quint...@redhat.com;
> peter.mayd...@linaro.org; Liu, Yuan1 ;
> berra...@redhat.com
> Subject: Re: [External] Re: [PATCH 3/5] migration: Introduce unimplemented
> 'qatzip' compression method
> 
> On Fri, Jan 5, 2024 at 12:07 PM Fabiano Rosas  wrote:
> >
> > Bryan Zhang  writes:
> >
> > +cc Yuan Liu, Daniel Berrangé
> >
> > > Adds support for 'qatzip' as an option for the multifd compression
> > > method parameter, but copy-pastes the no-op logic to leave the
> > > actual methods effectively unimplemented. This is in preparation of
> > > a subsequent commit that will implement actually using QAT for
> > > compression and decompression.
> > >
> > > Signed-off-by: Bryan Zhang 
> > > Signed-off-by: Hao Xiang 
> > > ---
> > >  hw/core/qdev-properties-system.c |  6 ++-
> > >  migration/meson.build|  1 +
> > >  migration/multifd-qatzip.c   | 81
> 
> > >  migration/multifd.h  |  1 +
> > >  qapi/migration.json  |  5 +-
> > >  5 files changed, 92 insertions(+), 2 deletions(-)  create mode
> > > 100644 migration/multifd-qatzip.c
> > >
> > > diff --git a/hw/core/qdev-properties-system.c
> > > b/hw/core/qdev-properties-system.c
> > > index 1a396521d5..d8e48dcb0e 100644
> > > --- a/hw/core/qdev-properties-system.c
> > > +++ b/hw/core/qdev-properties-system.c
> > > @@ -658,7 +658,11 @@ const PropertyInfo qdev_prop_fdc_drive_type = {
> > > const PropertyInfo qdev_prop_multifd_compression = {
> > >  .name = "MultiFDCompression",
> > >  .description = "multifd_compression values, "
> > > -   "none/zlib/zstd",
> > > +   "none/zlib/zstd"
> > > +#ifdef CONFIG_QATZIP
> > > +   "/qatzip"
> > > +#endif
> > > +   ,
> > >  .enum_table = _lookup,
> > >  .get = qdev_propinfo_get_enum,
> > >  .set = qdev_propinfo_set_enum,
> > > diff --git a/migration/meson.build b/migration/meson.build index
> > > 92b1cc4297..e20f318379 100644
> > > --- a/migration/meson.build
> > > +++ b/migration/meson.build
> > > @@ -40,6 +40,7 @@ if get_option('live_block_migration').allowed()
> > >system_ss.add(files('block.c'))
> > >  endif
> > >  system_ss.add(when: zstd, if_true: files('multifd-zstd.c'))
> > > +system_ss.add(when: qatzip, if_true: files('multifd-qatzip.c'))
> > >
> > >  specific_ss.add(when: 'CONFIG_SYSTEM_ONLY',
> > >  if_true: files('ram.c', diff --git
> > > a/migration/multifd-qatzip.c b/migration/multifd-qatzip.c new file
> > > mode 100644 index 00..1733bbddb7
> > > --- /dev/null
> > > +++ b/migration/multifd-qatzip.c
> > > @@ -0,0 +1,81 @@
> > > +/*
> > > + * Multifd QATzip compression implementation
> > > + *
> > > + * Copyright (c) Bytedance
> > > + *
> > > + * Authors:
> > > + *  Bryan Zhang 
> > > + *  Hao Xiang   
> > > + *
> > > + * This work is licensed under the terms of the GNU GPL, version 2 or
> later.
> > > + * See the COPYING file in the top-level directory.
> > > + */
> > > +
> > > +#include "qemu/osdep.h"
> > > +#include "exec/ramblock.h"
> > > +#include "exec/target_page.h"
> > > +#include "qapi/error.h"
> > > +#include "migration.h"
> > > +#include "options.h"
> > > +#include "multifd.h"
> > > +
> > > +static int qatzip_send_setup(MultiFDSendParams *p, Error **errp) {
> > > +return 0;
> > > +}
> > > +
> > > +static void qatzip_send_cleanup(MultiFDSendParams *p, Error **errp)
> > > +{};
> > > +
> > > +static int qatzip_send_prepare(MultiFDSendParams *p, Error **errp)
> > > +{
> > > +MultiFDPages_t *pages = p->pages;
> > > +
> > > +for (int i = 0; i < p->normal_num; i++) {
> > > +p->iov[p->iovs_num].iov_base = pages->block->host + p-
> >normal[i];
> > > +p->iov[p->iovs_num].iov_len = p->page_size;
> > > +p->iovs_num++;
> > > +}
> > > +
> > > +p->next_packet_size = p->normal_num * p->page_size;
> > > +p->flags |= MULTIFD_FLAG_NOCOMP;
> > > +return 0;
> > > +}
> > > +
> > > +static int qatzip_recv_setup(MultiFDRecvParams *p, Error **errp) {
> > > +return 0;
> > > +}
> > > +
> > > +static void qatzip_recv_cleanup(MultiFDRecvParams *p) {};
> > > +
> > > +static int qatzip_recv_pages(MultiFDRecvParams *p, Error **errp) {
> > > +uint32_t flags = p->flags & MULTIFD_FLAG_COMPRESSION_MASK;
> > > +
> > > +if (flags != MULTIFD_FLAG_NOCOMP) {
> > > +error_setg(errp, "multifd %u: flags received %x flags
> expected %x",
> > > +   p->id, flags, MULTIFD_FLAG_NOCOMP);
> > > +return -1;
> > > +}
> > > +for (int i = 0; i < p->normal_num; i++) {
> > > +p->iov[i].iov_base = p->host + p->normal[i];
> > > +p->iov[i].iov_len = p->page_size;
> > > +}
> > > +return qio_channel_readv_all(p->c, p->iov, p->normal_num,
> > > +errp); }
> > > +
> 

RE: [PATCH 3/5] migration: Introduce unimplemented 'qatzip' compression method

2024-01-07 Thread Liu, Yuan1
> -Original Message-
> From: Fabiano Rosas 
> Sent: Saturday, January 6, 2024 4:07 AM
> To: Bryan Zhang ; qemu-devel@nongnu.org;
> marcandre.lur...@redhat.com; pet...@redhat.com; quint...@redhat.com;
> peter.mayd...@linaro.org; hao.xi...@bytedance.com
> Cc: bryan.zh...@bytedance.com; Liu, Yuan1 ;
> berra...@redhat.com
> Subject: Re: [PATCH 3/5] migration: Introduce unimplemented 'qatzip'
> compression method
> 
> Bryan Zhang  writes:
> 
> +cc Yuan Liu, Daniel Berrangé
> 
> > Adds support for 'qatzip' as an option for the multifd compression
> > method parameter, but copy-pastes the no-op logic to leave the actual
> > methods effectively unimplemented. This is in preparation of a
> > subsequent commit that will implement actually using QAT for
> > compression and decompression.
> >
> > Signed-off-by: Bryan Zhang 
> > Signed-off-by: Hao Xiang 
> > ---
> >  hw/core/qdev-properties-system.c |  6 ++-
> >  migration/meson.build|  1 +
> >  migration/multifd-qatzip.c   | 81 
> >  migration/multifd.h  |  1 +
> >  qapi/migration.json  |  5 +-
> >  5 files changed, 92 insertions(+), 2 deletions(-)  create mode 100644
> > migration/multifd-qatzip.c
> >
> > diff --git a/hw/core/qdev-properties-system.c
> > b/hw/core/qdev-properties-system.c
> > index 1a396521d5..d8e48dcb0e 100644
> > --- a/hw/core/qdev-properties-system.c
> > +++ b/hw/core/qdev-properties-system.c
> > @@ -658,7 +658,11 @@ const PropertyInfo qdev_prop_fdc_drive_type = {
> > const PropertyInfo qdev_prop_multifd_compression = {
> >  .name = "MultiFDCompression",
> >  .description = "multifd_compression values, "
> > -   "none/zlib/zstd",
> > +   "none/zlib/zstd"
> > +#ifdef CONFIG_QATZIP
> > +   "/qatzip"
> > +#endif
> > +   ,
> >  .enum_table = _lookup,
> >  .get = qdev_propinfo_get_enum,
> >  .set = qdev_propinfo_set_enum,
> > diff --git a/migration/meson.build b/migration/meson.build index
> > 92b1cc4297..e20f318379 100644
> > --- a/migration/meson.build
> > +++ b/migration/meson.build
> > @@ -40,6 +40,7 @@ if get_option('live_block_migration').allowed()
> >system_ss.add(files('block.c'))
> >  endif
> >  system_ss.add(when: zstd, if_true: files('multifd-zstd.c'))
> > +system_ss.add(when: qatzip, if_true: files('multifd-qatzip.c'))
> >
> >  specific_ss.add(when: 'CONFIG_SYSTEM_ONLY',
> >  if_true: files('ram.c', diff --git
> > a/migration/multifd-qatzip.c b/migration/multifd-qatzip.c new file
> > mode 100644 index 00..1733bbddb7
> > --- /dev/null
> > +++ b/migration/multifd-qatzip.c
> > @@ -0,0 +1,81 @@
> > +/*
> > + * Multifd QATzip compression implementation
> > + *
> > + * Copyright (c) Bytedance
> > + *
> > + * Authors:
> > + *  Bryan Zhang 
> > + *  Hao Xiang   
> > + *
> > + * This work is licensed under the terms of the GNU GPL, version 2 or
> later.
> > + * See the COPYING file in the top-level directory.
> > + */
> > +
> > +#include "qemu/osdep.h"
> > +#include "exec/ramblock.h"
> > +#include "exec/target_page.h"
> > +#include "qapi/error.h"
> > +#include "migration.h"
> > +#include "options.h"
> > +#include "multifd.h"
> > +
> > +static int qatzip_send_setup(MultiFDSendParams *p, Error **errp) {
> > +return 0;
> > +}
> > +
> > +static void qatzip_send_cleanup(MultiFDSendParams *p, Error **errp)
> > +{};
> > +
> > +static int qatzip_send_prepare(MultiFDSendParams *p, Error **errp) {
> > +MultiFDPages_t *pages = p->pages;
> > +
> > +for (int i = 0; i < p->normal_num; i++) {
> > +p->iov[p->iovs_num].iov_base = pages->block->host + p-
> >normal[i];
> > +p->iov[p->iovs_num].iov_len = p->page_size;
> > +p->iovs_num++;
> > +}
> > +
> > +p->next_packet_size = p->normal_num * p->page_size;
> > +p->flags |= MULTIFD_FLAG_NOCOMP;
> > +return 0;
> > +}
> > +
> > +static int qatzip_recv_setup(MultiFDRecvParams *p, Error **errp) {
> > +return 0;
> > +}
> > +
> > +static void qatzip_recv_cleanup(MultiFDRecvParams *p) {};
> > +
> > +static int qatzip_recv_pages(MultiFDRecvParams *p, Error **errp) {
> > +uint32_t flags = p->flags & MULTIFD_FLAG_COMPRESSION_MASK;
> > +
> > +if (flags != MULTIFD_FLAG_NOCOMP) {
> > +error_setg(errp, "multifd %u: flags received %x flags
> expected %x",
> > +   p->id, flags, MULTIFD_FLAG_NOCOMP);
> > +return -1;
> > +}
> > +for (int i = 0; i < p->normal_num; i++) {
> > +p->iov[i].iov_base = p->host + p->normal[i];
> > +p->iov[i].iov_len = p->page_size;
> > +}
> > +return qio_channel_readv_all(p->c, p->iov, p->normal_num, errp);
> > +}
> > +
> > +static MultiFDMethods multifd_qatzip_ops = {
> > +.send_setup = qatzip_send_setup,
> > +.send_cleanup = qatzip_send_cleanup,
> > +.send_prepare = qatzip_send_prepare,
> > +.recv_setup = qatzip_recv_setup,
> > +.recv_cleanup = qatzip_recv_cleanup,
> > +

Re: [PULL 00/26] Migration 20240104 patches

2024-01-07 Thread Peter Xu
On Mon, Jan 08, 2024 at 10:10:24AM +0800, Peter Xu wrote:
> On Sun, Jan 07, 2024 at 11:28:25AM -0500, Stefan Hajnoczi wrote:
> > On Sun, 7 Jan 2024 at 10:23, Peter Maydell  wrote:
> > >
> > > On Sun, 7 Jan 2024 at 12:41, Stefan Hajnoczi  wrote:
> > > >
> > > > On Sun, 7 Jan 2024 at 07:34, Peter Xu  wrote:
> > > > >
> > > > > On Fri, Jan 05, 2024 at 04:08:40PM +, Peter Maydell wrote:
> > > > > > I notice that your gpg key doesn't seem to be signed by anybody
> > > > > > else; you might look at whether it's easy to get it signed
> > > > > > by somebody else (eg some of your redhat colleagues).
> > > > >
> > > > > Hmm, I think I have signed with at least Juan and Stefan.  Which is 
> > > > > the key
> > > > > server we normally use?  Maybe I missed some steps there?
> > > >
> > > > Yes, Peter's key is signed by me:
> > > >
> > > > $ gpg --list-signatures 3B5FCCCDF3ABD706
> > > > pub   ed25519/0x3B5FCCCDF3ABD706 2023-10-03 [SC]
> > > >   Key fingerprint = B918 4DC2 0CC4 57DA CF7D  D1A9 3B5F CCCD F3AB 
> > > > D706
> > > > uid   [  full  ] Peter Xu 
> > > > sig 30x3B5FCCCDF3ABD706 2023-10-03  [self-signature]
> > > > sig  0x9CA4ABB381AB73C8 2023-10-10  Stefan Hajnoczi
> > > > 
> > > > uid   [  full  ] Peter Xu 
> > > > sig 30x3B5FCCCDF3ABD706 2023-10-03  [self-signature]
> > > > sig  0x9CA4ABB381AB73C8 2023-10-10  Stefan Hajnoczi
> > > > 
> > > > sub   cv25519/0xD5261EB1CB0C6E45 2023-10-03 [E]
> > > > sig  0x3B5FCCCDF3ABD706 2023-10-03  [self-signature]
> > > >
> > > > I have pushed to the keyservers again in case I forget.
> > >
> > > Thanks. Which keyservers did you use? I think these days the
> > > keyserver infrastructure is unfortunately fragmented; I
> > > probably didn't try refreshing from the right keyserver.
> > 
> > I ran gpg --send-key again and it said hkps://keyserver.ubuntu.com.
> 
> Thanks Stefan.  Indeed I can only see Stefan's sig there on the key server:
> 
> https://keyserver.ubuntu.com/pks/lookup?search=3b5fcccdf3abd706=on=index
> 
> I am guessing Juan forgot to do a "gpg --send-keys 3B5FCCCDF3ABD706". I'll
> also try to ask maybe one or two more people to exchange keys.  Maybe
> that'll also help.

Besides that, just now I also tried to do a remote --recv-keys on my own
key and I found that indeed the signature from Stefan was not attached.

Then I found this:

https://daniel-lange.com/archives/178-Getting-gpg-to-import-signatures-again.html

So it seems the default behavior of gpg command changed recently that it'll
stop to receive signatures besides the self signature to avoid DoS to the
keyservers.

https://dev.gnupg.org/rG23c978640812d123eaffd4108744bdfcf48f7c93

In short, now we seem to need:

  $ gpg --recv-keys --keyserver-option no-self-sigs-only $KEY_ID

To recover the old behavior to receive signs from others.

Thanks,

-- 
Peter Xu




Re: [PULL 00/26] Migration 20240104 patches

2024-01-07 Thread Peter Xu
On Sun, Jan 07, 2024 at 11:28:25AM -0500, Stefan Hajnoczi wrote:
> On Sun, 7 Jan 2024 at 10:23, Peter Maydell  wrote:
> >
> > On Sun, 7 Jan 2024 at 12:41, Stefan Hajnoczi  wrote:
> > >
> > > On Sun, 7 Jan 2024 at 07:34, Peter Xu  wrote:
> > > >
> > > > On Fri, Jan 05, 2024 at 04:08:40PM +, Peter Maydell wrote:
> > > > > I notice that your gpg key doesn't seem to be signed by anybody
> > > > > else; you might look at whether it's easy to get it signed
> > > > > by somebody else (eg some of your redhat colleagues).
> > > >
> > > > Hmm, I think I have signed with at least Juan and Stefan.  Which is the 
> > > > key
> > > > server we normally use?  Maybe I missed some steps there?
> > >
> > > Yes, Peter's key is signed by me:
> > >
> > > $ gpg --list-signatures 3B5FCCCDF3ABD706
> > > pub   ed25519/0x3B5FCCCDF3ABD706 2023-10-03 [SC]
> > >   Key fingerprint = B918 4DC2 0CC4 57DA CF7D  D1A9 3B5F CCCD F3AB D706
> > > uid   [  full  ] Peter Xu 
> > > sig 30x3B5FCCCDF3ABD706 2023-10-03  [self-signature]
> > > sig  0x9CA4ABB381AB73C8 2023-10-10  Stefan Hajnoczi
> > > 
> > > uid   [  full  ] Peter Xu 
> > > sig 30x3B5FCCCDF3ABD706 2023-10-03  [self-signature]
> > > sig  0x9CA4ABB381AB73C8 2023-10-10  Stefan Hajnoczi
> > > 
> > > sub   cv25519/0xD5261EB1CB0C6E45 2023-10-03 [E]
> > > sig  0x3B5FCCCDF3ABD706 2023-10-03  [self-signature]
> > >
> > > I have pushed to the keyservers again in case I forget.
> >
> > Thanks. Which keyservers did you use? I think these days the
> > keyserver infrastructure is unfortunately fragmented; I
> > probably didn't try refreshing from the right keyserver.
> 
> I ran gpg --send-key again and it said hkps://keyserver.ubuntu.com.

Thanks Stefan.  Indeed I can only see Stefan's sig there on the key server:

https://keyserver.ubuntu.com/pks/lookup?search=3b5fcccdf3abd706=on=index

I am guessing Juan forgot to do a "gpg --send-keys 3B5FCCCDF3ABD706". I'll
also try to ask maybe one or two more people to exchange keys.  Maybe
that'll also help.

Thanks,

-- 
Peter Xu




!qemu_in_coroutine() assert on ARM64 XEN

2024-01-07 Thread Peng Fan
Hi All,

When enabling virtio disk and virtio net on Xen, I could see qemu blk assert
and being killed sometimes,  This is not 100% reproducible. I am using
qemu master branch

7425b6277f12e82952cede1f531bfc689bf77fb1 (HEAD -> dummy, origin/staging, 
origin/master, origin/HEAD, master) Merge tag 'tracing-pull-request' 
of https://gitlab.com/stefanha/qemu into staging

The qemu built option is the one in xen tool/Makefile, I just
change to qemu-system-aarch64.

Anyone has suggestions?

The coredump stack:

Symbols already loaded for /usr/lib/libc.so.6
(gdb) bt
#0  __pthread_kill_implementation (threadid=,
signo=signo@entry=6, no_tid=no_tid@entry=0) at pthread_kill.c:44
#1  0x9e100568 in __pthread_kill_internal (signo=6,
threadid=) at pthread_kill.c:78
#2  0x9e0bacd0 in __GI_raise (sig=sig@entry=6)
at /usr/src/debug/glibc/2.38+git-r0/sysdeps/posix/raise.c:26
#3  0x9e0a6ef0 in __GI_abort () at abort.c:79
#4  0x9e0b43f8 in __assert_fail_base (
fmt=0x9e1ca8a8 "%s%s%s:%u: %s%sAssertion `%s' failed.\n%n",
assertion=assertion@entry=0xe0d25740 "!qemu_in_coroutine()",
file=file@entry=0xe0d301a8 "../qemu-xen-dir-remote/block/graph-lock.c", 
line=line@entry=260,
function=function@entry=0xe0e522c0 <__PRETTY_FUNCTION__.3> 
"bdrv_graph_rdlock_main_loop") at assert.c:92
#5  0x9e0b4470 in __assert_fail (
assertion=assertion@entry=0xe0d25740 "!qemu_in_coroutine()",
file=file@entry=0xe0d301a8 "../qemu-xen-dir-remote/block/graph-lock.c", 
line=line@entry=260,
function=function@entry=0xe0e522c0 <__PRETTY_FUNCTION__.3> 
"bdrv_graph_rdlock_main_loop") at assert.c:101
#6  0xe0a66a60 in bdrv_graph_rdlock_main_loop ()
at ../qemu-xen-dir-remote/block/graph-lock.c:260
#7  0xe0a6d9e0 in graph_lockable_auto_lock_mainloop (x=)
--Type  for more, q to quit, c to continue without paging--
at 
/home/Freenix/work/sw-stash/xen/upstream/tools/qemu-xen-dir-remote/include/block/graph-lock.h:259
#8  bdrv_unregister_buf (bs=bs@entry=0xf619d5a0,
host=host@entry=0x742c8000, size=size@entry=2097152)
at ../qemu-xen-dir-remote/block/io.c:3362
#9  0xe0a5ddd4 in blk_unregister_buf (blk=,
host=0x742c8000, size=2097152)
at ../qemu-xen-dir-remote/block/block-backend.c:2859
#10 0xe060aab4 in ram_block_removed (n=,
host=, size=, max_size=)
at ../qemu-xen-dir-remote/block/block-ram-registrar.c:33
#11 0xe0399318 in ram_block_notify_remove (host=0x742c8000,
size=2097152, max_size=2097152)
at ../qemu-xen-dir-remote/hw/core/numa.c:883
#12 0xe097cf84 in xen_invalidate_map_cache_entry_unlocked (
buffer=buffer@entry=0x743c5000 "")
at ../qemu-xen-dir-remote/hw/xen/xen-mapcache.c:475
#13 0xe097dad0 in xen_invalidate_map_cache_entry (
buffer=buffer@entry=0x743c5000 "")
at ../qemu-xen-dir-remote/hw/xen/xen-mapcache.c:487
#14 0xe0993e18 in address_space_unmap (
as=as@entry=0xe1ca3ae8 , buffer=0x743c5000,
len=, is_write=is_write@entry=true,
--Type  for more, q to quit, c to continue without paging--
access_len=access_len@entry=32768)
at ../qemu-xen-dir-remote/system/physmem.c:3199
#15 0xe095cc9c in dma_memory_unmap (access_len=32768,
dir=DMA_DIRECTION_FROM_DEVICE, len=,
buffer=, as=0xe1ca3ae8 )

at 
/home/Freenix/work/sw-stash/xen/upstream/tools/qemu-xen-dir-remote/include/sysemu/dma.h:236
#16 virtqueue_unmap_sg (vq=vq@entry=0x965cc010,
elem=elem@entry=0xf620aa30, len=len@entry=32769)

at ../qemu-xen-dir-remote/hw/virtio/virtio.c:758
#17 0xe095efa4 in virtqueue_fill (vq=vq@entry=0x965cc010,
elem=elem@entry=0xf620aa30, len=len@entry=32769, idx=idx@entry=0)
at ../qemu-xen-dir-remote/hw/virtio/virtio.c:919
#18 0xe095f0b8 in virtqueue_push (vq=0x965cc010,

elem=elem@entry=0xf620aa30, len=32769)
at ../qemu-xen-dir-remote/hw/virtio/virtio.c:994
#19 0xe091a608 in virtio_blk_req_complete (
req=req@entry=0xf620aa30, status=status@entry=0 '\000')

at ../qemu-xen-dir-remote/hw/block/virtio-blk.c:67
#20 0xe091bdc8 in virtio_blk_rw_complete (opaque=,
ret=0) at ../qemu-xen-dir-remote/hw/block/virtio-blk.c:136
#21 0xe0a5a938 in blk_aio_complete (acb=acb@entry=0x880015f0)

at ../qemu-xen-dir-remote/block/block-backend.c:1559
--Type  for more, q to quit, c to continue without paging--
#22 0xe0a5b58c in blk_aio_read_entry (opaque=0x880015f0)
at ../qemu-xen-dir-remote/block/block-backend.c:1614

#23 0xe0b96c2c in coroutine_trampoline (i0=,
i1=) at ../qemu-xen-dir-remote/util/coroutine-ucontext.c:177
#24 0x9e0bfb40 in ?? ()
at ../sysdeps/unix/sysv/linux/aarch64/setcontext.S:123

   from /usr/lib/libc.so.6

(gdb) thread apply all bt

Thread 10 (Thread 0x951348c0 (LWP 5460)):
#0  0x9e15d8c4 in __GI___libc_read (nbytes=16, 

Re: [PATCH v9 0/9] Unified CPU type check

2024-01-07 Thread Gavin Shan

Hi Phil,

On 1/6/24 08:12, Philippe Mathieu-Daudé wrote:

On 13/12/23 11:54, Gavin Shan wrote:

On 12/13/23 20:08, Philippe Mathieu-Daudé wrote:

On 12/12/23 05:55, Gavin Shan wrote:

On 12/4/23 10:47, Gavin Shan wrote:

This series bases on Phil's repository because the prepatory commits
have been queued to the branch.

   https://gitlab.com/philmd/qemu.git (branch: cpus-next)

There are two places where the user specified CPU type is checked to see
if it's supported or allowed by the board: machine_run_board_init() and
mc->init(). We don't have to maintain two duplicate sets of logic. This
series intends to move the check to machine_run_board_init() so that we
have unified CPU type check.

This series can be checked out from:

   g...@github.com:gwshan/qemu.git (branch: kvm/cpu-type)

PATCH[1-4] refactors and improves the logic to validate CPU type in
    machine_run_board_init()
PATCH[5-9] validates the CPU type in machine_run_board_init() for the
    individual boards

v6: https://lists.nongnu.org/archive/html/qemu-arm/2023-11/msg00768.html
v7: https://lists.nongnu.org/archive/html/qemu-arm/2023-11/msg01045.html
v8: https://lists.nongnu.org/archive/html/qemu-arm/2023-11/msg01168.html



Ping to see if there is a chance to queue it up before the Chrismas? :)


Series queued. "Before" Christmas will depend on the final release tag.

Thanks for the various iterations,



Phil, thank you for you continuous reviews and valuable comments.

Yes, the final merge to master branch depends on the release plan.
'queue' meant to merge the series to your 'cpus-next' branch ;-)


I had to fix 3 different issues caught by our CI. Next time please
run your series on GitLab CI, you just have to push your branch and
wait for the result :)

Now merged as 445946f4dd..cd75cc6337.

Happy new year!



Happy new year. I just came back from holiday.

Thanks for the reminder for GitLab CI, which I don't know how to run yet.
I will learn how to run it from gitlab :)

Thanks,
Gavin




Re: [PATCH v9 3/9] machine: Improve is_cpu_type_supported()

2024-01-07 Thread Gavin Shan

On 1/6/24 08:09, Philippe Mathieu-Daudé wrote:

On 4/12/23 01:47, Gavin Shan wrote:

It's no sense to check the CPU type when mc->valid_cpu_types[0] is
NULL, which is a program error. Raise an assert on this.

A precise hint for the error message is given when mc->valid_cpu_types[0]
is the only valid entry. Besides, enumeration on mc->valid_cpu_types[0]
when we have mutiple valid entries there is avoided to increase the code
readability, as suggested by Philippe Mathieu-Daudé.

Besides, @cc comes from machine->cpu_type or mc->default_cpu_type. For
the later case, it can be NULL and it's also a program error. We should
use assert() in this case.

Signed-off-by: Gavin Shan 
v9: assert(mc->valid_cpu_types[0] != NULL)   (Phil)
 assert(cc != NULL);  (Phil)
---
  hw/core/machine.c | 20 ++--
  1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/hw/core/machine.c b/hw/core/machine.c
index 1797e002f9..4ae9aaee8e 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -1400,6 +1400,7 @@ static bool is_cpu_type_supported(const MachineState 
*machine, Error **errp)
   * type is provided through '-cpu' option.
   */
  if (mc->valid_cpu_types && machine->cpu_type) {
+    assert(mc->valid_cpu_types[0] != NULL);
  for (i = 0; mc->valid_cpu_types[i]; i++) {
  if (object_class_dynamic_cast(oc, mc->valid_cpu_types[i])) {
  break;
@@ -1409,20 +1410,27 @@ static bool is_cpu_type_supported(const MachineState 
*machine, Error **errp)
  /* The user specified CPU type isn't valid */
  if (!mc->valid_cpu_types[i]) {
  error_setg(errp, "Invalid CPU type: %s", machine->cpu_type);
-    error_append_hint(errp, "The valid types are: %s",
-  mc->valid_cpu_types[0]);
-    for (i = 1; mc->valid_cpu_types[i]; i++) {
-    error_append_hint(errp, ", %s", mc->valid_cpu_types[i]);
+    if (!mc->valid_cpu_types[1]) {
+    error_append_hint(errp, "The only valid type is: %s\n",
+  mc->valid_cpu_types[0]);
+    } else {
+    error_append_hint(errp, "The valid types are: ");
+    for (i = 0; mc->valid_cpu_types[i]; i++) {
+    error_append_hint(errp, "%s%s",
+  mc->valid_cpu_types[i],
+  mc->valid_cpu_types[i + 1] ? ", " : "");
+    }
+    error_append_hint(errp, "\n");
  }
-    error_append_hint(errp, "\n");
  return false;
  }
  }
  /* Check if CPU type is deprecated and warn if so */
  cc = CPU_CLASS(oc);
-    if (cc && cc->deprecation_note) {
+    assert(cc != NULL);
+    if (cc->deprecation_note) {
  warn_report("CPU model %s is deprecated -- %s",
  machine->cpu_type, cc->deprecation_note);
  }


Since we were getting:

$ qemu-system-s390x -M none
QEMU 8.2.50 monitor - type 'help' for more information
qemu-system-s390x: ../../hw/core/machine.c:1444: _Bool 
is_cpu_type_supported(const MachineState *, Error **): Assertion `cc != NULL' 
failed.
Aborted (core dumped)

I added a check on mc->valid_cpu_types before calling
is_cpu_type_supported() in the previous patch. See commit acbadc5a29.



Phil, thanks for fixing it up and it looks good to me.

Thanks,
Gavin




Re: [PATCH v9 1/9] machine: Use error handling when CPU type is checked

2024-01-07 Thread Gavin Shan

On 1/5/24 21:00, Philippe Mathieu-Daudé wrote:

On 4/12/23 01:47, Gavin Shan wrote:

Functions that use an Error **errp parameter to return errors should
not also report them to the user, because reporting is the caller's
job. The principle is violated by machine_run_board_init() because
it calls error_report(), error_printf(), and exit(1) when the machine
doesn't support the requested CPU type.

Clean this up by using error_setg() and error_append_hint() instead.
No functional change, as the only caller passes _fatal.

Suggested-by: Igor Mammedov 
Signed-off-by: Gavin Shan 
Reviewed-by: Markus Armbruster 
---
v9: Improved change log  (Markus)
---
  hw/core/machine.c | 13 +++--
  1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/hw/core/machine.c b/hw/core/machine.c
index 0c17398141..bde7f4af6d 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -1466,15 +1466,16 @@ void machine_run_board_init(MachineState *machine, 
const char *mem_path, Error *
  if (!machine_class->valid_cpu_types[i]) {
  /* The user specified CPU is not valid */
-    error_report("Invalid CPU type: %s", machine->cpu_type);
-    error_printf("The valid types are: %s",
- machine_class->valid_cpu_types[0]);
+    error_setg(errp, "Invalid CPU type: %s", machine->cpu_type);
+    error_append_hint(errp, "The valid types are: %s",
+  machine_class->valid_cpu_types[0]);
  for (i = 1; machine_class->valid_cpu_types[i]; i++) {
-    error_printf(", %s", machine_class->valid_cpu_types[i]);
+    error_append_hint(errp, ", %s",
+  machine_class->valid_cpu_types[i]);
  }
-    error_printf("\n");
-    exit(1);
+    error_append_hint(, "\n");


This doesn't build:

hw/core/machine.c:1488:31: error: incompatible pointer types passing 'Error ***' 
(aka 'struct Error ***') to parameter of type 'Error *const *' (aka 'struct Error 
*const *'); remove & [-Werror,-Wincompatible-pointer-types]
     error_append_hint(, "\n");
   ^



Yes,  should have been errp. The problematic code was carried from
previous revisions and has been corrected by PATCH[2/9]. It's how I missed
the building error. Thanks for fixing it up!

Thanks,
Gavin


+    return;
  }
  }


Squashing:
-- >8 --
diff --git a/hw/core/machine.c b/hw/core/machine.c
index 021044aaaf..1898d1d1d7 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -1487,3 +1487,3 @@ void machine_run_board_init(MachineState *machine, const 
char *mem_path, Error *

-    error_append_hint(, "\n");
+    error_append_hint(errp, "\n");
  return;
---






Re: [PATCH v2 1/2] target/riscv: FIX xATP_MODE validation

2024-01-07 Thread Alistair Francis
On Tue, Dec 12, 2023 at 9:04 PM Irina Ryapolova
 wrote:
>
> [Changes since v1]
> used satp_mode.map instead of satp_mode.supported

The changelog needs to go

>
> [Original cover]
> The SATP register is an SXLEN-bit read/write WARL register. It means that CSR 
> fields are only defined
> for a subset of bit encodings, but allow any value to be written while 
> guaranteeing to return a legal
> value whenever read (See riscv-privileged-20211203, SATP CSR).
>
> For example on rv64 we are trying to write to SATP CSR val = 
> 0x1000 (SATP_MODE = 1 - Reserved for standard use)
> and after that we are trying to read SATP_CSR. We read from the SATP CSR 
> value = 0x1000, which is not a correct
> operation (return illegal value).
>
> Signed-off-by: Irina Ryapolova 
> ---

Below this line.

Otherwise it will be included in the git history, which we don't want

Alistair

>  target/riscv/csr.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index fde7ce1a53..735fb27be7 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -1278,8 +1278,8 @@ static RISCVException read_mstatus(CPURISCVState *env, 
> int csrno,
>
>  static bool validate_vm(CPURISCVState *env, target_ulong vm)
>  {
> -return (vm & 0xf) <=
> -   satp_mode_max_from_map(riscv_cpu_cfg(env)->satp_mode.map);
> +uint64_t mode_supported = riscv_cpu_cfg(env)->satp_mode.map;
> +return get_field(mode_supported, (1 << vm));
>  }
>
>  static target_ulong legalize_mpp(CPURISCVState *env, target_ulong old_mpp,
> --
> 2.25.1
>
>



Re: [PATCH 1/1] target/riscv: pmp: Ignore writes when RW=01 and MML=0

2024-01-07 Thread Alistair Francis
On Thu, Dec 21, 2023 at 1:32 AM Ivan Klokov  wrote:
>
> This patch changes behavior on writing RW=01 to pmpcfg with MML=0.
> RWX filed is form of collective WARL with the combination of
> pmpcfg.RW=01 remains reserved for future standard use.
>
> According to definition of WARL writing the CSR has no other side
> effect. But current implementation change architectural state and
> change system behavior. After writing we will get unreadable-unwriteble
> region regardless on the previous state.
>
> On the other side WARL said that we should read legal value and nothing
> says about what we should write. Current behavior change system state
> regardless of whether we read this register or not.
>
> Fixes: ac66f2f0 ("target/riscv: pmp: Ignore writes when RW=01")
>
> Signed-off-by: Ivan Klokov 

Thanks!

Applied to riscv-to-apply.next

Alistair

> ---
>  target/riscv/pmp.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/target/riscv/pmp.c b/target/riscv/pmp.c
> index 162e88a90a..c0b814699e 100644
> --- a/target/riscv/pmp.c
> +++ b/target/riscv/pmp.c
> @@ -126,7 +126,7 @@ static bool pmp_write_cfg(CPURISCVState *env, uint32_t 
> pmp_index, uint8_t val)
>  /* If !mseccfg.MML then ignore writes with encoding RW=01 */
>  if ((val & PMP_WRITE) && !(val & PMP_READ) &&
>  !MSECCFG_MML_ISSET(env)) {
> -val &= ~(PMP_WRITE | PMP_READ);
> +return false;
>  }
>  env->pmp_state.pmp[pmp_index].cfg_reg = val;
>  pmp_update_rule_addr(env, pmp_index);
> --
> 2.34.1
>
>



Re: [PATCH 1/1] target/riscv: pmp: Ignore writes when RW=01 and MML=0

2024-01-07 Thread Alistair Francis
On Thu, Dec 21, 2023 at 1:32 AM Ivan Klokov  wrote:
>
> This patch changes behavior on writing RW=01 to pmpcfg with MML=0.
> RWX filed is form of collective WARL with the combination of
> pmpcfg.RW=01 remains reserved for future standard use.
>
> According to definition of WARL writing the CSR has no other side
> effect. But current implementation change architectural state and
> change system behavior. After writing we will get unreadable-unwriteble
> region regardless on the previous state.
>
> On the other side WARL said that we should read legal value and nothing
> says about what we should write. Current behavior change system state
> regardless of whether we read this register or not.
>
> Fixes: ac66f2f0 ("target/riscv: pmp: Ignore writes when RW=01")
>
> Signed-off-by: Ivan Klokov 

Reviewed-by: Alistair Francis 

Alistair

> ---
>  target/riscv/pmp.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/target/riscv/pmp.c b/target/riscv/pmp.c
> index 162e88a90a..c0b814699e 100644
> --- a/target/riscv/pmp.c
> +++ b/target/riscv/pmp.c
> @@ -126,7 +126,7 @@ static bool pmp_write_cfg(CPURISCVState *env, uint32_t 
> pmp_index, uint8_t val)
>  /* If !mseccfg.MML then ignore writes with encoding RW=01 */
>  if ((val & PMP_WRITE) && !(val & PMP_READ) &&
>  !MSECCFG_MML_ISSET(env)) {
> -val &= ~(PMP_WRITE | PMP_READ);
> +return false;
>  }
>  env->pmp_state.pmp[pmp_index].cfg_reg = val;
>  pmp_update_rule_addr(env, pmp_index);
> --
> 2.34.1
>
>



Re: [PATCH] roms/opensbi: Upgrade from v1.3.1 to v1.4

2024-01-07 Thread Alistair Francis
On Wed, Jan 3, 2024 at 1:13 AM Bin Meng  wrote:
>
> Upgrade OpenSBI from v1.3.1 to v1.4 and the pre-built bios images.
>
> The v1.4 release includes the following commits:
>
> 1a398d9 lib: sbi: Add Zicntr as a HART ISA extension
> 669089c lib: sbi: Add Zihpm as a HART ISA extension
> 72b9c8f lib: sbi: Alphabetically sort HART ISA extensions
> 5359fc6 lib: sbi: Rename hart_pmu_get_allowed_bits() function
> 976895c lib: sbi: Fix Priv spec version for [m|s]counteren and mcountinhibit 
> CSRs
> 6053917 lib: sbi: Fix how print gets flags
> 35ef182 lib: sbi: print not fill '0' when left-aligned
> 40dac06 lib: sbi: Add '+' flags for print
> 458fa74 lib: sbi: Add ' ' '\'' flags for print
> 05cbb6e lib: sbi: implifying the parameters of printi
> fe08281 lib: sbi: print add 'o' type
> c6ee5ae lib: sbi: Fix printi
> 3b6fcdd lib: sbi: Simplify prints
> cc89fa7 lib: sbi: Fix printc
> ff43168 lib: sbi: Fix timing of clearing tbuf
> a73982d lib: sbi: Fix missing '\0' when buffer szie equal 1
> ea6533a lib: utils/gpio: Fix RV32 compile error for designware GPIO driver
> c3b98c6 include: sbi: Add macro definitions for mseccfg CSR
> 1c099c4 lib: sbi: Add functions to manipulate PMP entries
> 6c202c5 include: sbi: Add Smepmp specific access flags for PMP entries
> cbcfc7b lib: sbi: Add smepmp in hart extensions
> d72f5f1 lib: utils: Add detection of Smepmp from ISA string in FDT
> 4a42a23 lib: sbi: Grant SU R/W/X permissions to whole memory
> f3fdd04 lib: sbi: Change the order of PMP initialization
> 5dd8db5 lib: sbi: Add support for Smepmp
> 6e44ef6 lib: sbi: Add functions to map/unmap shared memory
> 0ad8660 lib: sbi: Map/Unmap debug console shared memory buffers
> 057eb10 lib: utils/gpio: Fix RV32 compile error for designware GPIO driver
> 0e2111e libfdt: fix SPDX license identifiers
> e05a9cf lib: sbi: Update system suspend to spec
> 5e20d25 include: sbi: fix CSR define of mseccfg
> 44c5151 include: sbi_utils: Remove driver pointer from struct i2c_adapter
> 14a35b0 lib: utils/regmap: Add generic regmap access library
> 8e97275 lib: utils/regmap: Add simple FDT based regmap framework
> f21d8f7 lib: utils/regmap: Add simple FDT based syscon regmap driver
> 4a344a9 lib: utils/reset: Add syscon based reboot and poweroff
> c2e6027 lib: utils/reset: Remove SiFive Test reset driver
> f536e0b gitignore: allow gitignore to ignore most dot file
> c744ed7 lib: sbi_pmu: Enable noncontigous hpm event and counters
> 6259b2e lib: utils/fdt: Fix fdt_parse_isa_extensions() implementation
> f46a564 lib: sbi: Fix typo for finding fixed event counter
> 94197a8 fw_base.S: Fix assembler error with clang 16+
> c104c60 lib: sbi: Add support for smcntrpmf
> 7aabeee Makefile: Fix grep warning
> e7e73aa platform: generic: allwinner: correct mhpmevent count
> ee1f83c lib: sbi_pmu: remove mhpm_count field in hart feature
> a9cffd6 firmware: payload: test: Change to SBI v2.0 DBCN ecalls
> b20bd47 lib: sbi: improve the definition of SBI_IPI_EVENT_MAX
> 664692f lib: sbi_pmu: ensure update hpm counter before starting counting
> c9a296d platform: generic: allwinner: fix OF process for T-HEAD c9xx pmu
> 901d3d7 lib: sbi_pmu: keep overflow interrupt of stopped hpm counter disabled
> cacfba3 platform: Allow platforms to specify the size of tlb fifo
> 5bd9694 lib: sbi: alloc tlb fifo by sbi_malloc
> 130e65d lib: sbi: Implement SET_FS_DIRTY() to make sure the mstatus FS dirty 
> is set
> d1e4dff lib: sbi: Introduce HART index in sbi_scratch
> e6125c3 lib: sbi: Remove sbi_platform_hart_index/invalid() functions
> 296e70d lib: sbi: Extend sbi_hartmask to support both hartid and hartindex
> e632cd7 lib: sbi: Use sbi_scratch_last_hartindex() in remote TLB managment
> 78c667b lib: sbi: Prefer hartindex over hartid in IPI framework
> 22d6ff8 lib: sbi: Remove sbi_scratch_last_hartid() macro
> 112daa2 lib: sbi: Maximize the use of HART index in sbi_domain
> 9560fb3 include: sbi: Remove sbi_hartmask_for_each_hart() macro
> b8fb96e include: sbi_domain: Fix permission test macros
> bff27c1 lib: sbi: Factor-out Smepmp configuration as separate function
> 5240d31 lib: sbi: Don't clear mseccfg.MML bit in sbi_hart_smepmp_configure()
> 2b51a9d lib: sbi: Fix pmp_flags for Smepmp read-only shared region
> 73aea28 lib: sbi: Populate M-only Smepmp entries before setting mseccfg.MML
> e8bc162 lib: utils/serial: Add shared regions for serial drivers
> b7e9d34 lib: utils/regmap: Mark syscon region as shared read-write
> 3669153 platform: generic: thead: fix stale TLB entries for th1520/sg2042
> de525ac firmware: Remove ALIGN in .rela.dyn in linker script
> 2a6d725 firmware: Remove handling of R_RISCV_{32,64}
> 6ed125a Makefile: Add --exclude-libs ALL to avoid .dynsym
> e21901d doc: Fix fw_payload.md
> a125423 lib: utils/serial: Ensure proper allocation of PMP entries for 
> uart8250
> d36709f lib: utils: timer/ipi: Update memregion flags for PLMT and PLICSW
> 8197c2f lib: sbi: fix sbi_domain_get_assigned_hartmask()
> 9da30f6 lib: utils/fdt: simplify dt_parse_isa_extensions
> 

Re: [PATCH v3 6/6] target/riscv: Enable updates for pointer masking variables and thus enable pointer masking extension

2024-01-07 Thread Alistair Francis
On Fri, Jan 5, 2024 at 5:23 PM Alexey Baturo  wrote:
>
> I might be wrong here, but right now J in MISA is unused.

I suspect you are right, it would be worth confirming though.

> I think the J-letter extension is still a thing, but current extensions like 
> Zjpm and Zjid follow the Z ext scheme.
> Do you think it should be removed?

It sounds like it should be removed then

Alistair

>
>
> пт, 5 янв. 2024 г. в 08:28, Alistair Francis :
>>
>> On Thu, Jan 4, 2024 at 4:58 AM Alexey Baturo  wrote:
>> >
>> > From: Alexey Baturo 
>> >
>> > Signed-off-by: Alexey Baturo 
>> > ---
>> >  target/riscv/cpu.c | 8 
>> >  1 file changed, 8 insertions(+)
>> >
>> > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
>> > index 1e6571ce99..13389ddc55 100644
>> > --- a/target/riscv/cpu.c
>> > +++ b/target/riscv/cpu.c
>> > @@ -153,6 +153,9 @@ const RISCVIsaExtData isa_edata_arr[] = {
>> >  ISA_EXT_DATA_ENTRY(svinval, PRIV_VERSION_1_12_0, ext_svinval),
>> >  ISA_EXT_DATA_ENTRY(svnapot, PRIV_VERSION_1_12_0, ext_svnapot),
>> >  ISA_EXT_DATA_ENTRY(svpbmt, PRIV_VERSION_1_12_0, ext_svpbmt),
>> > +ISA_EXT_DATA_ENTRY(ssnpm, PRIV_VERSION_1_12_0, ext_ssnpm),
>> > +ISA_EXT_DATA_ENTRY(smnpm, PRIV_VERSION_1_12_0, ext_smnpm),
>> > +ISA_EXT_DATA_ENTRY(smmpm, PRIV_VERSION_1_12_0, ext_smmpm),
>> >  ISA_EXT_DATA_ENTRY(xtheadba, PRIV_VERSION_1_11_0, ext_xtheadba),
>> >  ISA_EXT_DATA_ENTRY(xtheadbb, PRIV_VERSION_1_11_0, ext_xtheadbb),
>> >  ISA_EXT_DATA_ENTRY(xtheadbs, PRIV_VERSION_1_11_0, ext_xtheadbs),
>> > @@ -1337,6 +1340,11 @@ const RISCVCPUMultiExtConfig riscv_cpu_extensions[] 
>> > = {
>> >
>> >  MULTI_EXT_CFG_BOOL("zmmul", ext_zmmul, false),
>> >
>> > +/* Zjpm v0.8 extensions */
>> > +MULTI_EXT_CFG_BOOL("ssnpm", ext_ssnpm, false),
>> > +MULTI_EXT_CFG_BOOL("smnpm", ext_smnpm, false),
>> > +MULTI_EXT_CFG_BOOL("smmpm", ext_smmpm, false),
>>
>> What happens to the existing J property?
>>
>>
>> Alistair
>>
>> > +
>> >  MULTI_EXT_CFG_BOOL("zca", ext_zca, false),
>> >  MULTI_EXT_CFG_BOOL("zcb", ext_zcb, false),
>> >  MULTI_EXT_CFG_BOOL("zcd", ext_zcd, false),
>> > --
>> > 2.34.1
>> >
>> >



Re: [PATCH] roms/opensbi: Upgrade from v1.3.1 to v1.4

2024-01-07 Thread Alistair Francis
On Wed, Jan 3, 2024 at 1:13 AM Bin Meng  wrote:
>
> Upgrade OpenSBI from v1.3.1 to v1.4 and the pre-built bios images.
>
> The v1.4 release includes the following commits:
>
> 1a398d9 lib: sbi: Add Zicntr as a HART ISA extension
> 669089c lib: sbi: Add Zihpm as a HART ISA extension
> 72b9c8f lib: sbi: Alphabetically sort HART ISA extensions
> 5359fc6 lib: sbi: Rename hart_pmu_get_allowed_bits() function
> 976895c lib: sbi: Fix Priv spec version for [m|s]counteren and mcountinhibit 
> CSRs
> 6053917 lib: sbi: Fix how print gets flags
> 35ef182 lib: sbi: print not fill '0' when left-aligned
> 40dac06 lib: sbi: Add '+' flags for print
> 458fa74 lib: sbi: Add ' ' '\'' flags for print
> 05cbb6e lib: sbi: implifying the parameters of printi
> fe08281 lib: sbi: print add 'o' type
> c6ee5ae lib: sbi: Fix printi
> 3b6fcdd lib: sbi: Simplify prints
> cc89fa7 lib: sbi: Fix printc
> ff43168 lib: sbi: Fix timing of clearing tbuf
> a73982d lib: sbi: Fix missing '\0' when buffer szie equal 1
> ea6533a lib: utils/gpio: Fix RV32 compile error for designware GPIO driver
> c3b98c6 include: sbi: Add macro definitions for mseccfg CSR
> 1c099c4 lib: sbi: Add functions to manipulate PMP entries
> 6c202c5 include: sbi: Add Smepmp specific access flags for PMP entries
> cbcfc7b lib: sbi: Add smepmp in hart extensions
> d72f5f1 lib: utils: Add detection of Smepmp from ISA string in FDT
> 4a42a23 lib: sbi: Grant SU R/W/X permissions to whole memory
> f3fdd04 lib: sbi: Change the order of PMP initialization
> 5dd8db5 lib: sbi: Add support for Smepmp
> 6e44ef6 lib: sbi: Add functions to map/unmap shared memory
> 0ad8660 lib: sbi: Map/Unmap debug console shared memory buffers
> 057eb10 lib: utils/gpio: Fix RV32 compile error for designware GPIO driver
> 0e2111e libfdt: fix SPDX license identifiers
> e05a9cf lib: sbi: Update system suspend to spec
> 5e20d25 include: sbi: fix CSR define of mseccfg
> 44c5151 include: sbi_utils: Remove driver pointer from struct i2c_adapter
> 14a35b0 lib: utils/regmap: Add generic regmap access library
> 8e97275 lib: utils/regmap: Add simple FDT based regmap framework
> f21d8f7 lib: utils/regmap: Add simple FDT based syscon regmap driver
> 4a344a9 lib: utils/reset: Add syscon based reboot and poweroff
> c2e6027 lib: utils/reset: Remove SiFive Test reset driver
> f536e0b gitignore: allow gitignore to ignore most dot file
> c744ed7 lib: sbi_pmu: Enable noncontigous hpm event and counters
> 6259b2e lib: utils/fdt: Fix fdt_parse_isa_extensions() implementation
> f46a564 lib: sbi: Fix typo for finding fixed event counter
> 94197a8 fw_base.S: Fix assembler error with clang 16+
> c104c60 lib: sbi: Add support for smcntrpmf
> 7aabeee Makefile: Fix grep warning
> e7e73aa platform: generic: allwinner: correct mhpmevent count
> ee1f83c lib: sbi_pmu: remove mhpm_count field in hart feature
> a9cffd6 firmware: payload: test: Change to SBI v2.0 DBCN ecalls
> b20bd47 lib: sbi: improve the definition of SBI_IPI_EVENT_MAX
> 664692f lib: sbi_pmu: ensure update hpm counter before starting counting
> c9a296d platform: generic: allwinner: fix OF process for T-HEAD c9xx pmu
> 901d3d7 lib: sbi_pmu: keep overflow interrupt of stopped hpm counter disabled
> cacfba3 platform: Allow platforms to specify the size of tlb fifo
> 5bd9694 lib: sbi: alloc tlb fifo by sbi_malloc
> 130e65d lib: sbi: Implement SET_FS_DIRTY() to make sure the mstatus FS dirty 
> is set
> d1e4dff lib: sbi: Introduce HART index in sbi_scratch
> e6125c3 lib: sbi: Remove sbi_platform_hart_index/invalid() functions
> 296e70d lib: sbi: Extend sbi_hartmask to support both hartid and hartindex
> e632cd7 lib: sbi: Use sbi_scratch_last_hartindex() in remote TLB managment
> 78c667b lib: sbi: Prefer hartindex over hartid in IPI framework
> 22d6ff8 lib: sbi: Remove sbi_scratch_last_hartid() macro
> 112daa2 lib: sbi: Maximize the use of HART index in sbi_domain
> 9560fb3 include: sbi: Remove sbi_hartmask_for_each_hart() macro
> b8fb96e include: sbi_domain: Fix permission test macros
> bff27c1 lib: sbi: Factor-out Smepmp configuration as separate function
> 5240d31 lib: sbi: Don't clear mseccfg.MML bit in sbi_hart_smepmp_configure()
> 2b51a9d lib: sbi: Fix pmp_flags for Smepmp read-only shared region
> 73aea28 lib: sbi: Populate M-only Smepmp entries before setting mseccfg.MML
> e8bc162 lib: utils/serial: Add shared regions for serial drivers
> b7e9d34 lib: utils/regmap: Mark syscon region as shared read-write
> 3669153 platform: generic: thead: fix stale TLB entries for th1520/sg2042
> de525ac firmware: Remove ALIGN in .rela.dyn in linker script
> 2a6d725 firmware: Remove handling of R_RISCV_{32,64}
> 6ed125a Makefile: Add --exclude-libs ALL to avoid .dynsym
> e21901d doc: Fix fw_payload.md
> a125423 lib: utils/serial: Ensure proper allocation of PMP entries for 
> uart8250
> d36709f lib: utils: timer/ipi: Update memregion flags for PLMT and PLICSW
> 8197c2f lib: sbi: fix sbi_domain_get_assigned_hartmask()
> 9da30f6 lib: utils/fdt: simplify dt_parse_isa_extensions
> 

[PATCH 2/3] target/riscv: Don't adjust vscause for exceptions

2024-01-07 Thread Alistair Francis
We have been incorrectly adjusting both the interrupt and exception
cause when using the hypervisor extension and trapping to VS-mode. This
patch changes the conditional to ensure we only adjust the cause for
interrupts and not exceptions.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1708
Signed-off-by: Alistair Francis 
---
 target/riscv/cpu_helper.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index e7e23b34f4..886a558a42 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -1749,8 +1749,8 @@ void riscv_cpu_do_interrupt(CPUState *cs)
  * See if we need to adjust cause. Yes if its VS mode interrupt
  * no if hypervisor has delegated one of hs mode's interrupt
  */
-if (cause == IRQ_VS_TIMER || cause == IRQ_VS_SOFT ||
-cause == IRQ_VS_EXT) {
+if (async && (cause == IRQ_VS_TIMER || cause == IRQ_VS_SOFT ||
+  cause == IRQ_VS_EXT)) {
 cause = cause - 1;
 }
 write_gva = false;
-- 
2.43.0




[PATCH 3/3] target/riscv: Ensure mideleg is set correctly on reset

2024-01-07 Thread Alistair Francis
Bits 10, 6, 2 and 12 of mideleg are read only 1 when the Hypervisor is
enabled. We currently only set them on accesses to mideleg, but they
aren't correctly set on reset. Let's ensure they are always the correct
value.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1617
Signed-off-by: Alistair Francis 
---
 target/riscv/cpu.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index b07a76ef6b..e20ff46c23 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -891,6 +891,14 @@ static void riscv_cpu_reset_hold(Object *obj)
 /* mmte is supposed to have pm.current hardwired to 1 */
 env->mmte |= (EXT_STATUS_INITIAL | MMTE_M_PM_CURRENT);
 
+/*
+ * Bits 10, 6, 2 and 12 of mideleg are read only 1 when the Hypervisor
+ * extension is enabled.
+ */
+if (riscv_has_ext(env, RVH)) {
+env->mideleg |= HS_MODE_INTERRUPTS;
+}
+
 /*
  * Clear mseccfg and unlock all the PMP entries upon reset.
  * This is allowed as per the priv and smepmp specifications
-- 
2.43.0




[PATCH 1/3] target/riscv: Assert that the CSR numbers will be correct

2024-01-07 Thread Alistair Francis
The CSRs will always be between either CSR_MHPMCOUNTER3 and
CSR_MHPMCOUNTER31 or CSR_MHPMCOUNTER3H and CSR_MHPMCOUNTER31H.

So although ctr_index can't be negative, Coverity doesn't know this and
it isn't obvious to human readers either. Let's add an assert to ensure
that Coverity knows the values will be within range.

To simplify the code let's also change the RV32 adjustment.

Fixes: Coverity CID 1523910
Signed-off-by: Alistair Francis 
---
 target/riscv/csr.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index fde7ce1a53..336ec7eda7 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -195,8 +195,11 @@ static RISCVException mctr(CPURISCVState *env, int csrno)
 
 if ((riscv_cpu_mxl(env) == MXL_RV32) && csrno >= CSR_MCYCLEH) {
 /* Offset for RV32 mhpmcounternh counters */
-base_csrno += 0x80;
+csrno -= 0x80;
 }
+
+g_assert(csrno >= CSR_MHPMCOUNTER3 && csrno <= CSR_MHPMCOUNTER31);
+
 ctr_index = csrno - base_csrno;
 if ((BIT(ctr_index) & pmu_avail_ctrs >> 3) == 0) {
 /* The PMU is not enabled or counter is out of range */
-- 
2.43.0




[PATCH 0/3] target/riscv: A few bug fixes and Coverity fix

2024-01-07 Thread Alistair Francis
A few bug fixes for some Gitlab issues and a Coverity fix

Alistair Francis (3):
  target/riscv: Assert that the CSR numbers will be correct
  target/riscv: Don't adjust vscause for exceptions
  target/riscv: Ensure mideleg is set correctly on reset

 target/riscv/cpu.c| 8 
 target/riscv/cpu_helper.c | 4 ++--
 target/riscv/csr.c| 5 -
 3 files changed, 14 insertions(+), 3 deletions(-)

-- 
2.43.0




Re: [PATCH] docs/system/riscv: sifive_u: Update S-mode U-Boot image build instructions

2024-01-07 Thread Alistair Francis
On Thu, Jan 4, 2024 at 5:18 PM Bin Meng  wrote:
>
> Currently, the documentation outlines the process for building the
> S-mode U-Boot image using `make menuconfig` and manual actions within
> the menuconfig UI. However, this approach is fragile due to Kconfig
> options potentially changing across different releases. For example,
> CONFIG_OF_PRIOR_STAGE has been replaced by CONFIG_BOARD since v2022.01
> release, and CONFIG_TEXT_BASE has been moved to the 'General setup'
> menu from the 'Boot options' menu in v2024.01 release.
>
> This update aims to make the S-mode U-Boot image build instructions
> future-proof. It leverages the 'config' script provided in the U-Boot
> source tree to edit the .config file, followed by a `make olddefconfig`.
>
> Validated with U-Boot v2024.01 release.
>
> Signed-off-by: Bin Meng 

Thanks!

Applied to riscv-to-apply.next

Alistair

>
> ---
>
>  docs/system/riscv/sifive_u.rst | 33 -
>  1 file changed, 12 insertions(+), 21 deletions(-)
>
> diff --git a/docs/system/riscv/sifive_u.rst b/docs/system/riscv/sifive_u.rst
> index 7b166567f9..8f55ae8e31 100644
> --- a/docs/system/riscv/sifive_u.rst
> +++ b/docs/system/riscv/sifive_u.rst
> @@ -210,7 +210,7 @@ command line options with ``qemu-system-riscv32``.
>  Running U-Boot
>  --
>
> -U-Boot mainline v2021.07 release is tested at the time of writing. To build a
> +U-Boot mainline v2024.01 release is tested at the time of writing. To build a
>  U-Boot mainline bootloader that can be booted by the ``sifive_u`` machine, 
> use
>  the sifive_unleashed_defconfig with similar commands as described above for
>  Linux:
> @@ -325,15 +325,10 @@ configuration of U-Boot:
>
>$ export CROSS_COMPILE=riscv64-linux-
>$ make sifive_unleashed_defconfig
> -  $ make menuconfig
> -
> -then manually select the following configuration:
> -
> -  * Device Tree Control ---> Provider of DTB for DT Control ---> Prior Stage 
> bootloader DTB
> -
> -and unselect the following configuration:
> -
> -  * Library routines ---> Allow access to binman information in the device 
> tree
> +  $ ./scripts/config --enable OF_BOARD
> +  $ ./scripts/config --disable BINMAN_FDT
> +  $ ./scripts/config --disable SPL
> +  $ make olddefconfig
>
>  This changes U-Boot to use the QEMU generated device tree blob, and bypass
>  running the U-Boot SPL stage.
> @@ -352,17 +347,13 @@ It's possible to create a 32-bit U-Boot S-mode image as 
> well.
>
>$ export CROSS_COMPILE=riscv64-linux-
>$ make sifive_unleashed_defconfig
> -  $ make menuconfig
> -
> -then manually update the following configuration in U-Boot:
> -
> -  * Device Tree Control ---> Provider of DTB for DT Control ---> Prior Stage 
> bootloader DTB
> -  * RISC-V architecture ---> Base ISA ---> RV32I
> -  * Boot options ---> Boot images ---> Text Base ---> 0x8040
> -
> -and unselect the following configuration:
> -
> -  * Library routines ---> Allow access to binman information in the device 
> tree
> +  $ ./scripts/config --disable ARCH_RV64I
> +  $ ./scripts/config --enable ARCH_RV32I
> +  $ ./scripts/config --set-val TEXT_BASE 0x8040
> +  $ ./scripts/config --enable OF_BOARD
> +  $ ./scripts/config --disable BINMAN_FDT
> +  $ ./scripts/config --disable SPL
> +  $ make olddefconfig
>
>  Use the same command line options to boot the 32-bit U-Boot S-mode image:
>
> --
> 2.34.1
>
>



[PATCH] hw/i386/pc_piix: Make piix_intx_routing_notifier_xen() more device independent

2024-01-07 Thread Bernhard Beschow
This is a follow-up on commit 89965db43cce "hw/isa/piix3: Avoid Xen-specific
variant of piix3_write_config()" which introduced
piix_intx_routing_notifier_xen(). This function is implemented in board code but
accesses the PCI configuration space of the PIIX ISA function to determine the
PCI interrupt routes. Avoid this by reusing pci_device_route_intx_to_irq() which
makes piix_intx_routing_notifier_xen() more device-agnostic.

One remaining improvement would be making piix_intx_routing_notifier_xen()
agnostic towards the number of PCI interrupt routes and move it to xen-hvm.
This might be useful for possible Q35 Xen efforts but remains a future exercise
for now.

Signed-off-by: Bernhard Beschow 
---
 hw/i386/pc_piix.c | 9 +++--
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 042c13cdbc..abfcfe4d2b 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -92,13 +92,10 @@ static void piix_intx_routing_notifier_xen(PCIDevice *dev)
 {
 int i;
 
-/* Scan for updates to PCI link routes (0x60-0x63). */
+/* Scan for updates to PCI link routes. */
 for (i = 0; i < PIIX_NUM_PIRQS; i++) {
-uint8_t v = dev->config_read(dev, PIIX_PIRQCA + i, 1);
-if (v & 0x80) {
-v = 0;
-}
-v &= 0xf;
+const PCIINTxRoute route = pci_device_route_intx_to_irq(dev, i);
+const uint8_t v = route.mode == PCI_INTX_ENABLED ? route.irq : 0;
 xen_set_pci_link_route(i, v);
 }
 }
-- 
2.43.0




Re: [PATCH] net/vmnet: Pad short Ethernet frames

2024-01-07 Thread Bin Meng
On Sun, Jan 7, 2024 at 7:19 AM William Hooper  wrote:
>
> At least on macOS 12.7.2, vmnet doesn't pad Ethernet frames, such as the
> host's ARP replies, to the minimum size (60 bytes before the frame check
> sequence) defined in IEEE Std 802.3-2022, so guests' Ethernet device
> drivers may drop them with "frame too short" errors.
>
> This patch calls eth_pad_short_frame() to add padding, as in net/tap.c
> and net/slirp.c.
>
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2058
> Signed-off-by: William Hooper 
> ---
>  net/vmnet-common.m | 20 +---
>  1 file changed, 17 insertions(+), 3 deletions(-)
>

Reviewed-by: Bin Meng 



Re: [PATCH] tcg/tci: Fix TCI on hppa host and update TCI test matrix

2024-01-07 Thread Helge Deller

On 1/7/24 16:22, Peter Maydell wrote:

On Sun, 7 Jan 2024 at 07:20, Helge Deller  wrote:


Update the TCI interpreter test matrix for big-endian hosts with
big- (hppa, hppa64) and little-endian (x86,x96-64) targets.
I used native ppc64 and hppa hosts for those tests.

Starting TCI on a hppa host crashed immediately, because hppa is
the only archive left where the stack grows upwards.
Write-protecting the stack guard page at the top of the stack
fixes the crash.


We deliberately dropped support for HPPA hosts, under
commit a1eaa6281f8b and commit b1cef6d02f84bd8.
Do we really care enough about trying to run on these
ancient host CPUs to want to bring it back?

My personal rule of thumb is that if a host CPU is supported
only by TCI then we are better off saying it is entirely
unsupported -- in practice the performance will be so
terrible as to not be something anybody will want to use,
especially for older architectures which are slow to
start with.


I can see your point (and the performance is really horrible).
It's not my intention to make hppa a supported TCI platform,
but for me it's a good candidate to at least test TCI on
a big-endian machine, mostly because I have access to some of
such machines.
And, this patch is all what's needed and it's pretty trivial, so
it would be great if it could be accepted.

Helge



[PATCH 1/2] nubus-device: round Declaration ROM memory region address to qemu_target_page_size()

2024-01-07 Thread Mark Cave-Ayland
Declaration ROM binary images can be any arbitrary size, however if a host ROM
memory region is not aligned to qemu_target_page_size() then we fail the
"assert(!(iotlb & ~TARGET_PAGE_MASK))" check in tlb_set_page_full().

Ensure that the host ROM memory region is aligned to qemu_target_page_size()
and adjust the offset at which the Declaration ROM image is loaded so that the
image is still aligned to the end of the Nubus slot.

Signed-off-by: Mark Cave-Ayland 
---
 hw/nubus/nubus-device.c | 16 
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/hw/nubus/nubus-device.c b/hw/nubus/nubus-device.c
index 49008e4938..e4f824d58b 100644
--- a/hw/nubus/nubus-device.c
+++ b/hw/nubus/nubus-device.c
@@ -10,6 +10,7 @@
 
 #include "qemu/osdep.h"
 #include "qemu/datadir.h"
+#include "exec/target_page.h"
 #include "hw/irq.h"
 #include "hw/loader.h"
 #include "hw/nubus/nubus.h"
@@ -30,7 +31,7 @@ static void nubus_device_realize(DeviceState *dev, Error 
**errp)
 NubusDevice *nd = NUBUS_DEVICE(dev);
 char *name, *path;
 hwaddr slot_offset;
-int64_t size;
+int64_t size, align_size;
 int ret;
 
 /* Super */
@@ -76,16 +77,23 @@ static void nubus_device_realize(DeviceState *dev, Error 
**errp)
 }
 
 name = g_strdup_printf("nubus-slot-%x-declaration-rom", nd->slot);
-memory_region_init_rom(>decl_rom, OBJECT(dev), name, size,
+
+/*
+ * Ensure ROM memory region is aligned to target page size regardless
+ * of the size of the Declaration ROM image
+ */
+align_size = ROUND_UP(size, qemu_target_page_size());
+memory_region_init_rom(>decl_rom, OBJECT(dev), name, align_size,
_abort);
-ret = load_image_mr(path, >decl_rom);
+ret = load_image_size(path, memory_region_get_ram_ptr(>decl_rom) +
+(uintptr_t)align_size - size, size);
 g_free(path);
 g_free(name);
 if (ret < 0) {
 error_setg(errp, "could not load romfile \"%s\"", nd->romfile);
 return;
 }
-memory_region_add_subregion(>slot_mem, NUBUS_SLOT_SIZE - size,
+memory_region_add_subregion(>slot_mem, NUBUS_SLOT_SIZE - 
align_size,
 >decl_rom);
 }
 }
-- 
2.39.2




[PATCH 2/2] nubus: add nubus-virtio-mmio device

2024-01-07 Thread Mark Cave-Ayland
The nubus-virtio-mmio device is a Nubus card that contains a set of 32 
virtio-mmio
devices and a goldfish PIC similar to the m68k virt machine that can be plugged
into the m68k q800 machine.

There are currently a number of drivers under development that can be used in
conjunction with this device to provide accelerated and/or additional hypervisor
services to 68k Classic MacOS.

Signed-off-by: Mark Cave-Ayland 
---
 hw/nubus/meson.build |   1 +
 hw/nubus/nubus-virtio-mmio.c | 102 +++
 include/hw/nubus/nubus-virtio-mmio.h |  36 ++
 3 files changed, 139 insertions(+)
 create mode 100644 hw/nubus/nubus-virtio-mmio.c
 create mode 100644 include/hw/nubus/nubus-virtio-mmio.h

diff --git a/hw/nubus/meson.build b/hw/nubus/meson.build
index e7ebda8993..9a7a12ea68 100644
--- a/hw/nubus/meson.build
+++ b/hw/nubus/meson.build
@@ -2,6 +2,7 @@ nubus_ss = ss.source_set()
 nubus_ss.add(files('nubus-device.c'))
 nubus_ss.add(files('nubus-bus.c'))
 nubus_ss.add(files('nubus-bridge.c'))
+nubus_ss.add(files('nubus-virtio-mmio.c'))
 nubus_ss.add(when: 'CONFIG_Q800', if_true: files('mac-nubus-bridge.c'))
 
 system_ss.add_all(when: 'CONFIG_NUBUS', if_true: nubus_ss)
diff --git a/hw/nubus/nubus-virtio-mmio.c b/hw/nubus/nubus-virtio-mmio.c
new file mode 100644
index 00..58a63c84d0
--- /dev/null
+++ b/hw/nubus/nubus-virtio-mmio.c
@@ -0,0 +1,102 @@
+/*
+ * QEMU Macintosh Nubus Virtio MMIO card
+ *
+ * Copyright (c) 2024 Mark Cave-Ayland 
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "qemu/osdep.h"
+#include "hw/nubus/nubus-virtio-mmio.h"
+
+
+#define NUBUS_VIRTIO_MMIO_PIC_OFFSET   0
+#define NUBUS_VIRTIO_MMIO_DEV_OFFSET   0x200
+
+
+static void nubus_virtio_mmio_set_input_irq(void *opaque, int n, int level)
+{
+NubusDevice *nd = NUBUS_DEVICE(opaque);
+
+nubus_set_irq(nd, level);
+}
+
+static void nubus_virtio_mmio_realize(DeviceState *dev, Error **errp)
+{
+NubusVirtioMMIODeviceClass *nvmdc = NUBUS_VIRTIO_MMIO_GET_CLASS(dev);
+NubusVirtioMMIO *s = NUBUS_VIRTIO_MMIO(dev);
+NubusDevice *nd = NUBUS_DEVICE(dev);
+SysBusDevice *sbd;
+int i, offset;
+
+nvmdc->parent_realize(dev, errp);
+if (*errp) {
+return;
+}
+
+/* Goldfish PIC */
+sbd = SYS_BUS_DEVICE(>pic);
+if (!sysbus_realize(sbd, errp)) {
+return;
+}
+memory_region_add_subregion(>slot_mem, NUBUS_VIRTIO_MMIO_PIC_OFFSET,
+sysbus_mmio_get_region(sbd, 0));
+sysbus_connect_irq(sbd, 0,
+   qdev_get_gpio_in_named(dev, "pic-input-irq", 0));
+
+/* virtio-mmio devices */
+offset = NUBUS_VIRTIO_MMIO_DEV_OFFSET;
+for (i = 0; i < NUBUS_VIRTIO_MMIO_NUM_DEVICES; i++) {
+sbd = SYS_BUS_DEVICE(>virtio_mmio[i]);
+qdev_prop_set_bit(DEVICE(sbd), "force-legacy", false);
+if (!sysbus_realize_and_unref(sbd, errp)) {
+return;
+}
+
+memory_region_add_subregion(>slot_mem, offset,
+sysbus_mmio_get_region(sbd, 0));
+offset += 0x200;
+
+sysbus_connect_irq(sbd, 0, qdev_get_gpio_in(DEVICE(>pic), i));
+}
+}
+
+static void nubus_virtio_mmio_init(Object *obj)
+{
+NubusVirtioMMIO *s = NUBUS_VIRTIO_MMIO(obj);
+int i;
+
+object_initialize_child(obj, "pic", >pic, TYPE_GOLDFISH_PIC);
+for (i = 0; i < NUBUS_VIRTIO_MMIO_NUM_DEVICES; i++) {
+char *name = g_strdup_printf("virtio-mmio[%d]", i);
+object_initialize_child(obj, name, >virtio_mmio[i],
+TYPE_VIRTIO_MMIO);
+g_free(name);
+}
+
+/* Input from goldfish PIC */
+qdev_init_gpio_in_named(DEVICE(obj), nubus_virtio_mmio_set_input_irq,
+"pic-input-irq", 1);
+}
+
+static void nubus_virtio_mmio_class_init(ObjectClass *oc, void *data)
+{
+DeviceClass *dc = DEVICE_CLASS(oc);
+NubusVirtioMMIODeviceClass *nvmdc = NUBUS_VIRTIO_MMIO_CLASS(oc);
+
+device_class_set_parent_realize(dc, nubus_virtio_mmio_realize,
+>parent_realize);
+}
+
+static const TypeInfo nubus_virtio_mmio_types[] = {
+{
+.name = TYPE_NUBUS_VIRTIO_MMIO,
+.parent = TYPE_NUBUS_DEVICE,
+.instance_init = nubus_virtio_mmio_init,
+.instance_size = sizeof(NubusVirtioMMIO),
+.class_init = nubus_virtio_mmio_class_init,
+.class_size = sizeof(NubusVirtioMMIODeviceClass),
+},
+};
+
+DEFINE_TYPES(nubus_virtio_mmio_types)
diff --git a/include/hw/nubus/nubus-virtio-mmio.h 
b/include/hw/nubus/nubus-virtio-mmio.h
new file mode 100644
index 00..de497b7f76
--- /dev/null
+++ b/include/hw/nubus/nubus-virtio-mmio.h
@@ -0,0 +1,36 @@
+/*
+ * QEMU Macintosh Nubus Virtio MMIO card
+ *
+ * Copyright (c) 2023 Mark Cave-Ayland 
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#ifndef HW_NUBUS_VIRTIO_MMIO_H
+#define HW_NUBUS_VIRTIO_MMIO_H
+
+#include "hw/nubus/nubus.h"
+#include 

[PATCH 0/2] nubus: add nubus-virtio-mmio device

2024-01-07 Thread Mark Cave-Ayland
This series introduces a new nubus-virtio-mmio device which can be plugged into
the q800 machine to enable a 68k Classic MacOS guest to access virtio devices
such as virtio-9p-device (host filesharing), virtio-gpu (extended framebuffer
support) and virtio-tablet-device (absolute positioning).

Once the nubus-virtio-mmio device has been plugged into the q800 machine, virtio
devices can be accessed by a Classic MacOS guest using the drivers from the
classicvirtio project at https://github.com/elliotnunn/classicvirtio.

The nubus-virtio-mmio device is purposefully designed to be similar to the
virtio-mmio interface used by the existing 68k virt machine, making use of a
similar memory layout and the goldfish PIC for simple interrupt management. The
main difference is that only a single goldfish PIC is used, however that still
allows up to 32 virtio devices to be connected using a single nubus card.

Patch 1 fixes an alignment bug in the existing nubus-device Declaration ROM code
whereby some ROM images could trigger an assert() in QEMU, whilst patch 2 adds
the nubus-virtio-mmio device itself.

Signed-off-by: Mark Cave-Ayland 


Mark Cave-Ayland (2):
  nubus-device: round Declaration ROM memory region address to
qemu_target_page_size()
  nubus: add nubus-virtio-mmio device

 hw/nubus/meson.build |   1 +
 hw/nubus/nubus-device.c  |  16 +++--
 hw/nubus/nubus-virtio-mmio.c | 102 +++
 include/hw/nubus/nubus-virtio-mmio.h |  36 ++
 4 files changed, 151 insertions(+), 4 deletions(-)
 create mode 100644 hw/nubus/nubus-virtio-mmio.c
 create mode 100644 include/hw/nubus/nubus-virtio-mmio.h

-- 
2.39.2




Re: [PATCH] target/ppc: Fix crash on machine check caused by ifetch

2024-01-07 Thread BALATON Zoltan

On Mon, 8 Jan 2024, Nicholas Piggin wrote:

is_prefix_insn_excp() loads the first word of the instruction address
which caused an exception, to determine whether or not it was prefixed
so the prefix bit can be set in [H]SRR1.

In case it was the instruction fetch itself that caused the exception,
the [H]SRR1 prefix bit is not required to be set, because it is not the
instruction itself that causes the interrupt. If the load is attempted,
t could cause a recursive exception.


Missing 'i' in "it" above. Is the long stack trace really needed to make a 
point here? I don't much care about what's in the commit message, I've 
seen full scientific articles posted there sometimes but this one seems a 
bit extreme without adding much info on the issue.


Regards,
BALATON Zoltan


Instruction storage interrupts, HDSIs caused by ifetch are excluded from
the prefix check. Machine checks caused by ifetch are not, and these
can cause bugs. For example fetching from an unmapped physical address
can result in:

 ERROR:../system/cpus.c:504:qemu_mutex_lock_iothread_impl:
 assertion failed: (!qemu_mutex_iothread_locked())
 #0  __pthread_kill_implementation
 (threadid=, signo=signo@entry=6, no_tid=no_tid@entry=0)
 at ./nptl/pthread_kill.c:44
 #1  0x7705a15f in __pthread_kill_internal
 (signo=6, threadid=) at ./nptl/pthread_kill.c:78
 #2  0x7700c472 in __GI_raise (sig=sig@entry=6)
 at ../sysdeps/posix/raise.c:26
 #3  0x76ff64b2 in __GI_abort () at ./stdlib/abort.c:79
 #4  0x773def08 in  () at /lib/x86_64-linux-gnu/libglib-2.0.so.0
 #5  0x77445e4e in g_assertion_message_expr ()
 at /lib/x86_64-linux-gnu/libglib-2.0.so.0
 #6  0x55a833f1 in qemu_mutex_lock_iothread_impl
 (file=0x55efda6e "../accel/tcg/cputlb.c", line=2033)
 at ../system/cpus.c:504
 #7  qemu_mutex_lock_iothread_impl
 (file=file@entry=0x55efda6e "../accel/tcg/cputlb.c", 
line=line@entry=2033) at ../system/cpus.c:500
 #8  0x55cbf786 in do_ld_mmio_beN
 (cpu=cpu@entry=0x56b72010, full=0x7fff5408e010, ret_be=ret_be@entry=0, 
addr=2310065133864353792, size=size@entry=4, mmu_idx=7, type=MMU_INST_FETCH, 
ra=0) at ../accel/tcg/cputlb.c:2033
 #9  0x55cc2ec6 in do_ld_4
 (ra=0, memop=MO_BEUL, type=MMU_INST_FETCH, mmu_idx=, 
p=0x7fff67dfc660, cpu=0x56b72010) at ../accel/tcg/cputlb.c:2336
 #10 do_ld4_mmu
 (cpu=cpu@entry=0x56b72010, addr=, oi=, 
ra=ra@entry=0, access_type=access_type@entry=MMU_INST_FETCH)
 at ../accel/tcg/cputlb.c:2418
 #11 0x55ccbaf6 in cpu_ldl_code
 (env=env@entry=0x56b747d0, addr=)
 at ../accel/tcg/cputlb.c:2975
 #12 0x55b7a47c in ppc_ldl_code
 (addr=, env=0x56b747d0)
 at ../target/ppc/excp_helper.c:147
 #13 is_prefix_insn_excp (excp=1, cpu=0x56b72010)
 at ../target/ppc/excp_helper.c:1350
 #14 powerpc_excp_books (excp=1, cpu=0x56b72010)
 at ../target/ppc/excp_helper.c:1415
 #15 powerpc_excp (cpu=0x56b72010, excp=)
 at ../target/ppc/excp_helper.c:1733
 #16 0x55cb1c74 in cpu_handle_exception
 (ret=, cpu=)

Fix this by excluding machine checks caused by ifetch from the prefix
check.

Fixes: 55a7fa34f89 ("target/ppc: Machine check on invalid real address access on 
POWER9/10")
Fixes: 5a5d3b23cb2 ("target/ppc: Add SRR1 prefix indication to interrupt 
handlers")
Signed-off-by: Nicholas Piggin 
---
target/ppc/excp_helper.c | 32 +---
1 file changed, 21 insertions(+), 11 deletions(-)

diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index a42743a3e0..34c307b572 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -1322,6 +1322,15 @@ static bool is_prefix_insn_excp(PowerPCCPU *cpu, int 
excp)
}

switch (excp) {
+case POWERPC_EXCP_MCHECK:
+if (!(env->error_code & PPC_BIT(42))) {
+/*
+ * Fetch attempt caused a machine check, so attempting to fetch
+ * again would cause a recursive machine check.
+ */
+return false;
+}
+break;
case POWERPC_EXCP_HDSI:
/* HDSI PRTABLE_FAULT has the originating access type in error_code */
if ((env->spr[SPR_HDSISR] & DSISR_PRTABLE_FAULT) &&
@@ -1332,10 +1341,10 @@ static bool is_prefix_insn_excp(PowerPCCPU *cpu, int 
excp)
 * instruction at NIP would cause recursive faults with the same
 * translation).
 */
-break;
+return false;
}
-/* fall through */
-case POWERPC_EXCP_MCHECK:
+break;
+
case POWERPC_EXCP_DSI:
case POWERPC_EXCP_DSEG:
case POWERPC_EXCP_ALIGN:
@@ -1346,17 +1355,14 @@ static bool is_prefix_insn_excp(PowerPCCPU *cpu, int 
excp)
case POWERPC_EXCP_VPU:
case POWERPC_EXCP_VSXU:
case POWERPC_EXCP_FU:
-case POWERPC_EXCP_HV_FU: {
-uint32_t insn = ppc_ldl_code(env, env->nip);
-if (is_prefix_insn(env, insn)) {
-   

[PATCH] target/ppc: Fix crash on machine check caused by ifetch

2024-01-07 Thread Nicholas Piggin
is_prefix_insn_excp() loads the first word of the instruction address
which caused an exception, to determine whether or not it was prefixed
so the prefix bit can be set in [H]SRR1.

In case it was the instruction fetch itself that caused the exception,
the [H]SRR1 prefix bit is not required to be set, because it is not the
instruction itself that causes the interrupt. If the load is attempted,
t could cause a recursive exception.

Instruction storage interrupts, HDSIs caused by ifetch are excluded from
the prefix check. Machine checks caused by ifetch are not, and these
can cause bugs. For example fetching from an unmapped physical address
can result in:

  ERROR:../system/cpus.c:504:qemu_mutex_lock_iothread_impl:
  assertion failed: (!qemu_mutex_iothread_locked())
  #0  __pthread_kill_implementation
  (threadid=, signo=signo@entry=6, no_tid=no_tid@entry=0)
  at ./nptl/pthread_kill.c:44
  #1  0x7705a15f in __pthread_kill_internal
  (signo=6, threadid=) at ./nptl/pthread_kill.c:78
  #2  0x7700c472 in __GI_raise (sig=sig@entry=6)
  at ../sysdeps/posix/raise.c:26
  #3  0x76ff64b2 in __GI_abort () at ./stdlib/abort.c:79
  #4  0x773def08 in  () at /lib/x86_64-linux-gnu/libglib-2.0.so.0
  #5  0x77445e4e in g_assertion_message_expr ()
  at /lib/x86_64-linux-gnu/libglib-2.0.so.0
  #6  0x55a833f1 in qemu_mutex_lock_iothread_impl
  (file=0x55efda6e "../accel/tcg/cputlb.c", line=2033)
  at ../system/cpus.c:504
  #7  qemu_mutex_lock_iothread_impl
  (file=file@entry=0x55efda6e "../accel/tcg/cputlb.c", 
line=line@entry=2033) at ../system/cpus.c:500
  #8  0x55cbf786 in do_ld_mmio_beN
  (cpu=cpu@entry=0x56b72010, full=0x7fff5408e010, 
ret_be=ret_be@entry=0, addr=2310065133864353792, size=size@entry=4, mmu_idx=7, 
type=MMU_INST_FETCH, ra=0) at ../accel/tcg/cputlb.c:2033
  #9  0x55cc2ec6 in do_ld_4
  (ra=0, memop=MO_BEUL, type=MMU_INST_FETCH, mmu_idx=, 
p=0x7fff67dfc660, cpu=0x56b72010) at ../accel/tcg/cputlb.c:2336
  #10 do_ld4_mmu
  (cpu=cpu@entry=0x56b72010, addr=, oi=, 
ra=ra@entry=0, access_type=access_type@entry=MMU_INST_FETCH)
  at ../accel/tcg/cputlb.c:2418
  #11 0x55ccbaf6 in cpu_ldl_code
  (env=env@entry=0x56b747d0, addr=)
  at ../accel/tcg/cputlb.c:2975
  #12 0x55b7a47c in ppc_ldl_code
  (addr=, env=0x56b747d0)
  at ../target/ppc/excp_helper.c:147
  #13 is_prefix_insn_excp (excp=1, cpu=0x56b72010)
  at ../target/ppc/excp_helper.c:1350
  #14 powerpc_excp_books (excp=1, cpu=0x56b72010)
  at ../target/ppc/excp_helper.c:1415
  #15 powerpc_excp (cpu=0x56b72010, excp=)
  at ../target/ppc/excp_helper.c:1733
  #16 0x55cb1c74 in cpu_handle_exception
  (ret=, cpu=)

Fix this by excluding machine checks caused by ifetch from the prefix
check.

Fixes: 55a7fa34f89 ("target/ppc: Machine check on invalid real address access 
on POWER9/10")
Fixes: 5a5d3b23cb2 ("target/ppc: Add SRR1 prefix indication to interrupt 
handlers")
Signed-off-by: Nicholas Piggin 
---
 target/ppc/excp_helper.c | 32 +---
 1 file changed, 21 insertions(+), 11 deletions(-)

diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index a42743a3e0..34c307b572 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -1322,6 +1322,15 @@ static bool is_prefix_insn_excp(PowerPCCPU *cpu, int 
excp)
 }
 
 switch (excp) {
+case POWERPC_EXCP_MCHECK:
+if (!(env->error_code & PPC_BIT(42))) {
+/*
+ * Fetch attempt caused a machine check, so attempting to fetch
+ * again would cause a recursive machine check.
+ */
+return false;
+}
+break;
 case POWERPC_EXCP_HDSI:
 /* HDSI PRTABLE_FAULT has the originating access type in error_code */
 if ((env->spr[SPR_HDSISR] & DSISR_PRTABLE_FAULT) &&
@@ -1332,10 +1341,10 @@ static bool is_prefix_insn_excp(PowerPCCPU *cpu, int 
excp)
  * instruction at NIP would cause recursive faults with the same
  * translation).
  */
-break;
+return false;
 }
-/* fall through */
-case POWERPC_EXCP_MCHECK:
+break;
+
 case POWERPC_EXCP_DSI:
 case POWERPC_EXCP_DSEG:
 case POWERPC_EXCP_ALIGN:
@@ -1346,17 +1355,14 @@ static bool is_prefix_insn_excp(PowerPCCPU *cpu, int 
excp)
 case POWERPC_EXCP_VPU:
 case POWERPC_EXCP_VSXU:
 case POWERPC_EXCP_FU:
-case POWERPC_EXCP_HV_FU: {
-uint32_t insn = ppc_ldl_code(env, env->nip);
-if (is_prefix_insn(env, insn)) {
-return true;
-}
+case POWERPC_EXCP_HV_FU:
 break;
-}
 default:
-break;
+return false;
 }
-return false;
+
+
+return is_prefix_insn(env, ppc_ldl_code(env, env->nip));
 }
 #else
 static bool is_prefix_insn_excp(PowerPCCPU *cpu, int 

[PATCH 9/9] tests/avocado: Add FreeBSD distro boot tests for ppc

2024-01-07 Thread Nicholas Piggin
FreeBSD project provides qcow2 images that work well for testing QEMU.
Add pseries tests for HPT and Radix, KVM and TCG. This uses a short
term VM image, because FreeBSD has not set up long term builds for
ppc64 at present.

Other architectures could be added so this does not get a ppc_ prefix
but is instead named similarly to boot_linux.

Reviewed-by: Warner Losh 
Signed-off-by: Nicholas Piggin 
---
Unfortunately the latest stable (14.0) x86-64 VM image does not seem to
output to console by default and I've not been able to find a reliable
way to edit the filesystem to change the boot loader options, or use
console input in the test case to change it on the fly.

Thanks,
Nick
---
 tests/avocado/boot_freebsd.py | 106 ++
 1 file changed, 106 insertions(+)
 create mode 100644 tests/avocado/boot_freebsd.py

diff --git a/tests/avocado/boot_freebsd.py b/tests/avocado/boot_freebsd.py
new file mode 100644
index 00..79c68b149a
--- /dev/null
+++ b/tests/avocado/boot_freebsd.py
@@ -0,0 +1,106 @@
+# Functional tests that boot FreeBSD in various configurations
+#
+# Copyright (c) 2023 IBM Corporation
+#
+# This work is licensed under the terms of the GNU GPL, version 2 or
+# later. See the COPYING file in the top-level directory.
+
+import os
+
+from avocado import skipUnless
+from avocado_qemu import QemuSystemTest
+from avocado_qemu import wait_for_console_pattern
+from avocado_qemu import exec_command
+from avocado.utils import archive
+from avocado.utils import process
+from avocado.utils.path import find_command
+
+@skipUnless(os.getenv('AVOCADO_ALLOW_LARGE_STORAGE'), 'storage limited')
+@skipUnless(os.getenv('AVOCADO_ALLOW_LONG_RUNTIME'), 'runtime limited')
+class BootFreeBSDPPC64(QemuSystemTest):
+"""
+:avocado: tags=arch:ppc64
+"""
+
+timeout = 360
+
+def setUp(self):
+super().setUp()
+
+# We need zstd for all the tests
+# See https://github.com/avocado-framework/avocado/issues/5609
+zstd = find_command('zstd', False)
+if zstd is False:
+self.cancel('Could not find "zstd", which is required to '
+'decompress rootfs')
+drive_url = 
('https://artifact.ci.freebsd.org/snapshot/15.0-CURRENT/8a735ffdf04936c6785ac4fa31486639262dd416/powerpc/powerpc64le/disk.qcow2.zst')
+drive_hash = '95d863dbbc4b60f4899d1ef21d6489fca05bf03d'
+drive_path_zstd = self.fetch_asset(drive_url, asset_hash=drive_hash)
+drive_path = os.path.join(self.workdir, 'disk.qcow2')
+
+cmd = f"{zstd} -d {drive_path_zstd} -o {drive_path}"
+process.run(cmd)
+
+self.drive_opt = f"file={drive_path},format=qcow2,if=virtio"
+
+def run_pseries_test(self, force_HPT=False):
+if force_HPT:
+self.vm.add_args('-m', '4g')
+else:
+self.vm.add_args('-m', '1g')
+self.vm.add_args('-smp', '4')
+self.vm.add_args('-drive', self.drive_opt)
+self.vm.add_args('-net', 'nic,model=virtio')
+self.vm.set_console()
+self.vm.launch()
+
+wait_for_console_pattern(self, 'Hit [Enter] to boot immediately, or 
any other key for command prompt.')
+if force_HPT:
+exec_command(self, 'x')
+wait_for_console_pattern(self, 'OK')
+exec_command(self, 'set radix_mmu=0')
+exec_command(self, 'boot')
+wait_for_console_pattern(self, 'cas: selected hash MMU', 'panic:')
+else:
+exec_command(self, '')
+wait_for_console_pattern(self, 'cas: selected radix MMU', 'panic:')
+
+wait_for_console_pattern(self, 'FreeBSD 15.0-CURRENT', 'panic:')
+wait_for_console_pattern(self, 'FreeBSD/SMP: Multiprocessor System 
Detected: 4 CPUs', 'panic:')
+wait_for_console_pattern(self, 'FreeBSD/powerpc (Amnesiac) (ttyu0)', 
'panic:')
+
+def test_pseries_tcg(self):
+"""
+:avocado: tags=arch:ppc64
+:avocado: tags=machine:pseries
+:avocado: tags=accel:tcg
+"""
+self.require_accelerator("tcg")
+self.run_pseries_test()
+
+def test_pseries_hpt_tcg(self):
+"""
+:avocado: tags=arch:ppc64
+:avocado: tags=machine:pseries
+:avocado: tags=accel:tcg
+"""
+self.require_accelerator("tcg")
+self.run_pseries_test(force_HPT=True)
+
+def test_pseries_kvm(self):
+"""
+:avocado: tags=arch:ppc64
+:avocado: tags=machine:pseries
+:avocado: tags=accel:kvm
+"""
+self.require_accelerator("kvm")
+self.run_pseries_test()
+
+def test_pseries_hpt_kvm(self):
+"""
+:avocado: tags=arch:ppc64
+:avocado: tags=machine:pseries
+:avocado: tags=accel:kvm
+"""
+self.require_accelerator("kvm")
+self.run_pseries_test(force_HPT=True)
-- 
2.42.0




[PATCH 7/9] tests/avocado: Add pseries KVM boot_linux test

2024-01-07 Thread Nicholas Piggin
ppc has no avocado tests for the KVM backend. Add a KVM boot_linux.py
test for pseries.

Signed-off-by: Nicholas Piggin 
---
 tests/avocado/boot_linux.py | 8 
 1 file changed, 8 insertions(+)

diff --git a/tests/avocado/boot_linux.py b/tests/avocado/boot_linux.py
index a4a78122ac..b20b2fc620 100644
--- a/tests/avocado/boot_linux.py
+++ b/tests/avocado/boot_linux.py
@@ -102,6 +102,14 @@ def test_pseries_tcg(self):
 self.vm.add_args("-accel", "tcg")
 self.launch_and_wait(set_up_ssh_connection=False)
 
+def test_pseries_kvm(self):
+"""
+:avocado: tags=machine:pseries
+:avocado: tags=accel:kvm
+"""
+self.require_accelerator("kvm")
+self.vm.add_args("-accel", "kvm")
+self.launch_and_wait(set_up_ssh_connection=False)
 
 class BootLinuxS390X(LinuxTest):
 """
-- 
2.42.0




[PATCH 6/9] tests/avocado: Add ppc pseries and powernv hash MMU tests

2024-01-07 Thread Nicholas Piggin
POWER CPUs support hash and radix MMU modes. Linux supports running in
either mode, but defaults to radix. To keep up testing of QEMU's hash
MMU implementation, add some Linux hash boot tests.

Signed-off-by: Nicholas Piggin 
---
 tests/avocado/ppc_powernv.py | 23 +++
 tests/avocado/ppc_pseries.py | 20 +---
 2 files changed, 36 insertions(+), 7 deletions(-)

diff --git a/tests/avocado/ppc_powernv.py b/tests/avocado/ppc_powernv.py
index d0e5c07bde..4342941d5d 100644
--- a/tests/avocado/ppc_powernv.py
+++ b/tests/avocado/ppc_powernv.py
@@ -12,11 +12,11 @@
 class powernvMachine(QemuSystemTest):
 
 timeout = 90
-KERNEL_COMMON_COMMAND_LINE = 'printk.time=0 '
+KERNEL_COMMON_COMMAND_LINE = 'printk.time=0 console=hvc0 '
 panic_message = 'Kernel panic - not syncing'
 good_message = 'VFS: Cannot open root device'
 
-def do_test_linux_boot(self):
+def do_test_linux_boot(self, command_line = KERNEL_COMMON_COMMAND_LINE):
 self.require_accelerator("tcg")
 kernel_url = ('https://archives.fedoraproject.org/pub/archive'
   '/fedora-secondary/releases/29/Everything/ppc64le/os'
@@ -25,9 +25,8 @@ def do_test_linux_boot(self):
 kernel_path = self.fetch_asset(kernel_url, asset_hash=kernel_hash)
 
 self.vm.set_console()
-kernel_command_line = self.KERNEL_COMMON_COMMAND_LINE + 'console=hvc0'
 self.vm.add_args('-kernel', kernel_path,
- '-append', kernel_command_line)
+ '-append', command_line)
 self.vm.launch()
 
 def test_linux_boot(self):
@@ -54,6 +53,22 @@ def test_linux_smp_boot(self):
 wait_for_console_pattern(self, console_pattern, self.panic_message)
 wait_for_console_pattern(self, self.good_message, self.panic_message)
 
+def test_linux_smp_hpt_boot(self):
+"""
+:avocado: tags=arch:ppc64
+:avocado: tags=machine:powernv
+:avocado: tags=accel:tcg
+"""
+
+self.vm.add_args('-smp', '4')
+self.do_test_linux_boot(self.KERNEL_COMMON_COMMAND_LINE +
+'disable_radix')
+console_pattern = 'smp: Brought up 1 node, 4 CPUs'
+wait_for_console_pattern(self, 'hash-mmu: Initializing hash mmu',
+ self.panic_message)
+wait_for_console_pattern(self, console_pattern, self.panic_message)
+wait_for_console_pattern(self, self.good_message, self.panic_message)
+
 def test_linux_smt_boot(self):
 """
 :avocado: tags=arch:ppc64
diff --git a/tests/avocado/ppc_pseries.py b/tests/avocado/ppc_pseries.py
index a8311e6555..74aaa4ac4a 100644
--- a/tests/avocado/ppc_pseries.py
+++ b/tests/avocado/ppc_pseries.py
@@ -12,11 +12,11 @@
 class pseriesMachine(QemuSystemTest):
 
 timeout = 90
-KERNEL_COMMON_COMMAND_LINE = 'printk.time=0 '
+KERNEL_COMMON_COMMAND_LINE = 'printk.time=0 console=hvc0 '
 panic_message = 'Kernel panic - not syncing'
 good_message = 'VFS: Cannot open root device'
 
-def do_test_ppc64_linux_boot(self):
+def do_test_ppc64_linux_boot(self, kernel_command_line = 
KERNEL_COMMON_COMMAND_LINE):
 kernel_url = ('https://archives.fedoraproject.org/pub/archive'
   '/fedora-secondary/releases/29/Everything/ppc64le/os'
   '/ppc/ppc64/vmlinuz')
@@ -24,7 +24,6 @@ def do_test_ppc64_linux_boot(self):
 kernel_path = self.fetch_asset(kernel_url, asset_hash=kernel_hash)
 
 self.vm.set_console()
-kernel_command_line = self.KERNEL_COMMON_COMMAND_LINE + 'console=hvc0'
 self.vm.add_args('-kernel', kernel_path,
  '-append', kernel_command_line)
 self.vm.launch()
@@ -62,6 +61,21 @@ def test_ppc64_linux_smp_boot(self):
 wait_for_console_pattern(self, console_pattern, self.panic_message)
 wait_for_console_pattern(self, self.good_message, self.panic_message)
 
+def test_ppc64_linux_hpt_smp_boot(self):
+"""
+:avocado: tags=arch:ppc64
+:avocado: tags=machine:pseries
+"""
+
+self.vm.add_args('-smp', '4')
+self.do_test_ppc64_linux_boot(self.KERNEL_COMMON_COMMAND_LINE +
+  'disable_radix')
+console_pattern = 'smp: Brought up 1 node, 4 CPUs'
+wait_for_console_pattern(self, 'hash-mmu: Initializing hash mmu',
+ self.panic_message)
+wait_for_console_pattern(self, console_pattern, self.panic_message)
+wait_for_console_pattern(self, self.good_message, self.panic_message)
+
 def test_ppc64_linux_smt_boot(self):
 """
 :avocado: tags=arch:ppc64
-- 
2.42.0




[PATCH 2/9] tests/avocado: mark boot_linux.py long runtime instead of flaky

2024-01-07 Thread Nicholas Piggin
The ppc64 and s390x tests were first marked skipIf GITLAB_CI by commit
c0c8687ef0f ("tests/avocado: disable BootLinuxPPC64 test in CI"), and
commit 0f26d94ec9e ("tests/acceptance: skip s390x_ccw_vrtio_tcg on
GitLab") due to being very heavy-weight for gitlab CI.

Commit 9b45cc99318 ("docs/devel: rationalise unstable gitlab tests under
FLAKY_TESTS") changed this to being flaky but it isn't really, it just
had a long runtime.

So introduce a new AVOCADO_ALLOW_LONG_RUNTIME variable and make these
tests require it. Re-testing the s390x and ppc64 tests on gitlab shows
about 100-150s runtime each, which is similar to the x86-64 tests.
Since these are among the longest running avocado tests, make x86-64
require long runtime as well.

Signed-off-by: Nicholas Piggin 
---
 docs/devel/testing.rst  | 8 
 tests/avocado/boot_linux.py | 8 ++--
 2 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/docs/devel/testing.rst b/docs/devel/testing.rst
index bd132306c1..3a9c1327be 100644
--- a/docs/devel/testing.rst
+++ b/docs/devel/testing.rst
@@ -1346,6 +1346,14 @@ the environment.
 The definition of *large* is a bit arbitrary here, but it usually means an
 asset which occupies at least 1GB of size on disk when uncompressed.
 
+AVOCADO_ALLOW_LONG_RUNTIME
+^^
+Tests which have a long runtime will not be run unless that
+``AVOCADO_ALLOW_LONG_RUNTIME=1`` is exported on the environment.
+
+The definition of *long* is a bit arbitrary here, but it usually means a
+test which takes more than 100 seconds to complete.
+
 AVOCADO_ALLOW_UNTRUSTED_CODE
 
 There are tests which will boot a kernel image or firmware that can be
diff --git a/tests/avocado/boot_linux.py b/tests/avocado/boot_linux.py
index 7c4769904e..6df0fc0489 100644
--- a/tests/avocado/boot_linux.py
+++ b/tests/avocado/boot_linux.py
@@ -93,13 +93,11 @@ class BootLinuxPPC64(LinuxTest):
 
 timeout = 360
 
-@skipUnless(os.getenv('QEMU_TEST_FLAKY_TESTS'), 'Test is unstable on 
GitLab')
-
+@skipUnless(os.getenv('AVOCADO_ALLOW_LONG_RUNTIME'), 'runtime limited')
 def test_pseries_tcg(self):
 """
 :avocado: tags=machine:pseries
 :avocado: tags=accel:tcg
-:avocado: tags=flaky
 """
 self.require_accelerator("tcg")
 self.vm.add_args("-accel", "tcg")
@@ -113,13 +111,11 @@ class BootLinuxS390X(LinuxTest):
 
 timeout = 240
 
-@skipUnless(os.getenv('QEMU_TEST_FLAKY_TESTS'), 'Test is unstable on 
GitLab')
-
+@skipUnless(os.getenv('AVOCADO_ALLOW_LONG_RUNTIME'), 'runtime limited')
 def test_s390_ccw_virtio_tcg(self):
 """
 :avocado: tags=machine:s390-ccw-virtio
 :avocado: tags=accel:tcg
-:avocado: tags=flaky
 """
 self.require_accelerator("tcg")
 self.vm.add_args("-accel", "tcg")
-- 
2.42.0




[PATCH 8/9] tests/avocado: ppc add hypervisor tests

2024-01-07 Thread Nicholas Piggin
The powernv and pseries machines both provide hypervisor facilities
that are supported by KVM. This is a large and complicated set of
features that don't get much system-level testing in ppc tests.

Add a new test case for these which runs QEMU KVM inside the target.
This downloads an Alpine VM image, boots it and downloads and installs
the qemu package, then boots a virtual machine under it, re-using the
original Alpine VM image.

Signed-off-by: Nicholas Piggin 
---
 MAINTAINERS   |   1 +
 tests/avocado/ppc_hv_tests.py | 201 ++
 2 files changed, 202 insertions(+)
 create mode 100644 tests/avocado/ppc_hv_tests.py

diff --git a/MAINTAINERS b/MAINTAINERS
index 395f26ba86..ffe91a2bc5 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1509,6 +1509,7 @@ F: tests/qtest/libqos/*spapr*
 F: tests/qtest/rtas*
 F: tests/qtest/libqos/rtas*
 F: tests/avocado/ppc_pseries.py
+F: tests/avocado/ppc_hv_tests.py
 
 PowerNV (Non-Virtualized)
 M: Cédric Le Goater 
diff --git a/tests/avocado/ppc_hv_tests.py b/tests/avocado/ppc_hv_tests.py
new file mode 100644
index 00..2162d6bd68
--- /dev/null
+++ b/tests/avocado/ppc_hv_tests.py
@@ -0,0 +1,201 @@
+# Tests that specifically try to exercise hypervisor features of the
+# target machines. powernv supports the Power hypervisor ISA, and
+# pseries supports the nested-HV hypervisor spec.
+#
+# Copyright (c) 2023 IBM Corporation
+#
+# This work is licensed under the terms of the GNU GPL, version 2 or
+# later.  See the COPYING file in the top-level directory.
+
+from avocado import skipIf, skipUnless
+from avocado.utils import archive
+from avocado_qemu import QemuSystemTest
+from avocado_qemu import wait_for_console_pattern, exec_command
+import os
+import time
+import subprocess
+
+deps = ["xorriso"] # dependent tools needed in the test setup/box.
+
+def which(tool):
+""" looks up the full path for @tool, returns None if not found
+or if @tool does not have executable permissions.
+"""
+paths=os.getenv('PATH')
+for p in paths.split(os.path.pathsep):
+p = os.path.join(p, tool)
+if os.path.exists(p) and os.access(p, os.X_OK):
+return p
+return None
+
+def missing_deps():
+""" returns True if any of the test dependent tools are absent.
+"""
+for dep in deps:
+if which(dep) is None:
+return True
+return False
+
+# Alpine is a light weight distro that supports QEMU. These tests boot
+# that on the machine then run a QEMU guest inside it in KVM mode,
+# that runs the same Alpine distro image.
+# QEMU packages are downloaded and installed on each test. That's not a
+# large download, but it may be more polite to create qcow2 image with
+# QEMU already installed and use that.
+@skipUnless(os.getenv('AVOCADO_ALLOW_LARGE_STORAGE'), 'storage limited')
+@skipUnless(os.getenv('AVOCADO_ALLOW_LONG_RUNTIME'), 'runtime limited')
+@skipIf(missing_deps(), 'dependencies (%s) not installed' % ','.join(deps))
+class HypervisorTest(QemuSystemTest):
+
+timeout = 1000
+KERNEL_COMMON_COMMAND_LINE = 'printk.time=0 console=hvc0 '
+panic_message = 'Kernel panic - not syncing'
+good_message = 'VFS: Cannot open root device'
+
+def extract_from_iso(self, iso, path):
+"""
+Extracts a file from an iso file into the test workdir
+
+:param iso: path to the iso file
+:param path: path within the iso file of the file to be extracted
+:returns: path of the extracted file
+"""
+filename = os.path.basename(path)
+
+cwd = os.getcwd()
+os.chdir(self.workdir)
+
+with open(filename, "w") as outfile:
+cmd = "xorriso -osirrox on -indev %s -cpx %s %s" %
+  (iso, path, filename)
+subprocess.run(cmd.split(),
+   stdout=subprocess.DEVNULL, 
stderr=subprocess.DEVNULL)
+
+os.chdir(cwd)
+
+# Return complete path to extracted file.  Because callers to
+# extract_from_iso() specify 'path' with a leading slash, it is
+# necessary to use os.path.relpath() as otherwise os.path.join()
+# interprets it as an absolute path and drops the self.workdir part.
+return os.path.normpath(os.path.join(self.workdir, filename))
+
+def setUp(self):
+super().setUp()
+
+iso_url = 
('https://dl-cdn.alpinelinux.org/alpine/v3.18/releases/ppc64le/alpine-standard-3.18.4-ppc64le.iso')
+
+# Alpine use sha256 so I recalculated this myself
+iso_sha256 = 
'c26b8d3e17c2f3f0fed02b4b1296589c2390e6d5548610099af75300edd7b3ff'
+iso_path = self.fetch_asset(iso_url, asset_hash=iso_sha256,
+algorithm = "sha256")
+
+self.iso_path = iso_path
+self.vmlinuz = self.extract_from_iso(iso_path, '/boot/vmlinuz-lts')
+self.initramfs = self.extract_from_iso(iso_path, '/boot/initramfs-lts')
+
+def do_start_alpine(self):
+

[PATCH 4/9] tests/avocado: Enable replay_linux.py on ppc64 pseries

2024-01-07 Thread Nicholas Piggin
Add a ppc64 pseries test. This tends to hang in the replay phase near
the end of the trace due to a missing event, so it is marked flaky.

spapr-vscsi IO is extremely slow when running in record-replay modes,
particularly when driven by SLOF. This causes tests to time-out even
after an hour, so this uses guestfish to extract the kernel and initrd
and boot them directly.

Signed-off-by: Nicholas Piggin 
---
 tests/avocado/replay_linux.py | 76 ++-
 1 file changed, 75 insertions(+), 1 deletion(-)

diff --git a/tests/avocado/replay_linux.py b/tests/avocado/replay_linux.py
index 270ccc1eae..82daba9f3f 100644
--- a/tests/avocado/replay_linux.py
+++ b/tests/avocado/replay_linux.py
@@ -11,8 +11,9 @@
 import os
 import logging
 import time
+import subprocess
 
-from avocado import skipUnless
+from avocado import skipIf, skipUnless
 from avocado_qemu import BUILD_DIR
 from avocado.utils import cloudinit
 from avocado.utils import network
@@ -191,3 +192,76 @@ def test_virt_gicv3(self):
 self.run_rr(shift=3,
 args=(*self.get_common_args(),
   "-machine", "virt,gic-version=3"))
+
+# ppc64 pseries test.
+#
+# This machine tends to fail replay and hang very close to the end of the
+# trace, with missing events, which is still an open issue.
+#
+# spapr-scsi IO driven by SLOF/grub is extremely slow in record/replay mode,
+# so jump through some hoops to boot the kernel directly. With this, the test
+# runs in about 5 minutes (modulo hang), which suggests other machines may
+# have similar issues and could benefit from bypassing bootloaders.
+#
+ppc_deps = ["guestfish"] # dependent tools needed in the test setup/box.
+
+def which(tool):
+""" looks up the full path for @tool, returns None if not found
+or if @tool does not have executable permissions.
+"""
+paths=os.getenv('PATH')
+for p in paths.split(os.path.pathsep):
+p = os.path.join(p, tool)
+if os.path.exists(p) and os.access(p, os.X_OK):
+return p
+return None
+
+def ppc_missing_deps():
+""" returns True if any of the test dependent tools are absent.
+"""
+for dep in ppc_deps:
+if which(dep) is None:
+return True
+return False
+
+@skipIf(ppc_missing_deps(), 'dependencies (%s) not installed' % 
','.join(ppc_deps))
+@skipUnless(os.getenv('QEMU_TEST_FLAKY_TESTS'), 'known failure in trace 
replay')
+class ReplayLinuxPPC64(ReplayLinux):
+"""
+:avocado: tags=arch:ppc64
+:avocado: tags=accel:tcg
+"""
+
+hdd = 'scsi-hd'
+cd = 'scsi-cd'
+bus = None
+
+def setUp(self):
+super().setUp()
+
+# kernel, initramfs, and kernel cmdline are all taken by hand from
+# the Fedora image.
+self.kernel="vmlinuz-5.3.7-301.fc31.ppc64le"
+self.initramfs="initramfs-5.3.7-301.fc31.ppc64le.img"
+cmd = "guestfish --ro -a %s run "
+  ": mount /dev/sda2 / "
+  ": copy-out /boot/%s %s "
+  ": copy-out /boot/%s %s "
+  % (self.boot_path, self.kernel, self.workdir,
+ self.initramfs, self.workdir)
+subprocess.run(cmd.split())
+
+def test_pseries(self):
+"""
+:avocado: tags=machine:pseries
+"""
+kernel=os.path.normpath(os.path.join(self.workdir, self.kernel))
+initramfs=os.path.normpath(os.path.join(self.workdir, self.initramfs))
+cmdline="root=UUID=8a409ee6-3cb3-4b06-a266-39e2dae3e5fa ro "
+"no_timer_check net.ifnames=0 console=tty1 "
+"console=ttyS0,115200n8"
+self.run_rr(shift=1, args=("-device", "spapr-vscsi",
+   "-machine", "x-vof=on",
+   "-kernel", kernel,
+   "-initrd", initramfs,
+   "-append", cmdline))
-- 
2.42.0




[PATCH 1/9] gitlab: fix s390x tag for avocado-system-centos

2024-01-07 Thread Nicholas Piggin
The 390x tag should be s390x.

Signed-off-by: Nicholas Piggin 
---
 .gitlab-ci.d/buildtest.yml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/.gitlab-ci.d/buildtest.yml b/.gitlab-ci.d/buildtest.yml
index 91663946de..cfe737aca2 100644
--- a/.gitlab-ci.d/buildtest.yml
+++ b/.gitlab-ci.d/buildtest.yml
@@ -184,7 +184,7 @@ avocado-system-centos:
   variables:
 IMAGE: centos8
 MAKE_CHECK_ARGS: check-avocado
-AVOCADO_TAGS: arch:ppc64 arch:or1k arch:390x arch:x86_64 arch:rx
+AVOCADO_TAGS: arch:ppc64 arch:or1k arch:s390x arch:x86_64 arch:rx
   arch:sh4 arch:nios2
 
 build-system-opensuse:
-- 
2.42.0




[PATCH 5/9] tests/avocado: ppc add powernv10 boot_linux_console test

2024-01-07 Thread Nicholas Piggin
Add test for POWER10.

Signed-off-by: Nicholas Piggin 
---
 tests/avocado/boot_linux_console.py | 8 
 1 file changed, 8 insertions(+)

diff --git a/tests/avocado/boot_linux_console.py 
b/tests/avocado/boot_linux_console.py
index 3f0180e1f8..4f05bb7441 100644
--- a/tests/avocado/boot_linux_console.py
+++ b/tests/avocado/boot_linux_console.py
@@ -1386,6 +1386,14 @@ def test_ppc_powernv9(self):
 """
 self.do_test_ppc64_powernv('P9')
 
+def test_ppc_powernv10(self):
+"""
+:avocado: tags=arch:ppc64
+:avocado: tags=machine:powernv10
+:avocado: tags=accel:tcg
+"""
+self.do_test_ppc64_powernv('P10')
+
 def test_ppc_g3beige(self):
 """
 :avocado: tags=arch:ppc
-- 
2.42.0




[PATCH 0/9] tests/avocado: ppc additions and other fixes

2024-01-07 Thread Nicholas Piggin
This is a set of ppc avocado tests that I have posted before, but
it has also grown a few misc changes, and patch 2 creates a
dependency with the added ppc tests that use the
AVOCADO_ALLOW_LONG_RUNTIME it introduces. I could take patches 2-3
via the ppc tree if they get acked, but happy to rebase ppc tests
if they'd better go via testing tree.

Thanks,
Nick

Nicholas Piggin (9):
  gitlab: fix s390x tag for avocado-system-centos
  tests/avocado: mark boot_linux.py long runtime instead of flaky
  tests/avocado: Mark x86-64 boot_linux.py TCG tests as long runtime
  tests/avocado: Enable replay_linux.py on ppc64 pseries
  tests/avocado: ppc add powernv10 boot_linux_console test
  tests/avocado: Add ppc pseries and powernv hash MMU tests
  tests/avocado: Add pseries KVM boot_linux test
  tests/avocado: ppc add hypervisor tests
  tests/avocado: Add FreeBSD distro boot tests for ppc

 MAINTAINERS |   1 +
 docs/devel/testing.rst  |   8 ++
 .gitlab-ci.d/buildtest.yml  |   2 +-
 tests/avocado/boot_freebsd.py   | 106 +++
 tests/avocado/boot_linux.py |  32 ++---
 tests/avocado/boot_linux_console.py |   8 ++
 tests/avocado/ppc_hv_tests.py   | 201 
 tests/avocado/ppc_powernv.py|  23 +++-
 tests/avocado/ppc_pseries.py|  20 ++-
 tests/avocado/replay_linux.py   |  76 ++-
 10 files changed, 453 insertions(+), 24 deletions(-)
 create mode 100644 tests/avocado/boot_freebsd.py
 create mode 100644 tests/avocado/ppc_hv_tests.py

-- 
2.42.0




[PATCH 3/9] tests/avocado: Mark x86-64 boot_linux.py TCG tests as long runtime

2024-01-07 Thread Nicholas Piggin
Re-testing gitlab CI shows the x86-64 TCG tests take ~100s each, are
the longest-running tests. They are close to the ~150s taken by the
disabled ppc64 and s390x tests. From avocado-system-centos:

  boot_linux.py:BootLinuxX8664.test_pc_i440fx_tcg:  PASS (112.34 s)
  boot_linux.py:BootLinuxX8664.test_pc_q35_tcg:  PASS (97.05 s)
  boot_linux.py:BootLinuxPPC64.test_pseries_tcg:  PASS (148.86 s)
  boot_linux.py:BootLinuxS390X.test_s390_ccw_virtio_tcg:  PASS (149.83 s)

So disable the x86-64 tests as well.

Signed-off-by: Nicholas Piggin 
---

The other way we could go is enabling them all since ppc64 and s390s are
now much faster than when they were originally disabled; or to only
enable q35, giving at least one boot_linux.py test.

[Test time results from here https://gitlab.com/npiggin/qemu/-/jobs/5842257510]

Thanks,
Nick
---
 tests/avocado/boot_linux.py | 16 +++-
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/tests/avocado/boot_linux.py b/tests/avocado/boot_linux.py
index 6df0fc0489..a4a78122ac 100644
--- a/tests/avocado/boot_linux.py
+++ b/tests/avocado/boot_linux.py
@@ -14,6 +14,9 @@
 
 from avocado import skipUnless
 
+# We don't run TCG tests in CI, as booting the current Fedora OS in TCG tests
+# is very heavyweight (~100s per test). There are lighter weight distros which
+# we use in the machine_aarch64_virt.py, tux_baseline.py, etc.
 
 class BootLinuxX8664(LinuxTest):
 """
@@ -21,6 +24,7 @@ class BootLinuxX8664(LinuxTest):
 """
 timeout = 480
 
+@skipUnless(os.getenv('AVOCADO_ALLOW_LONG_RUNTIME'), 'runtime limited')
 def test_pc_i440fx_tcg(self):
 """
 :avocado: tags=machine:pc
@@ -39,6 +43,7 @@ def test_pc_i440fx_kvm(self):
 self.vm.add_args("-accel", "kvm")
 self.launch_and_wait(set_up_ssh_connection=False)
 
+@skipUnless(os.getenv('AVOCADO_ALLOW_LONG_RUNTIME'), 'runtime limited')
 def test_pc_q35_tcg(self):
 """
 :avocado: tags=machine:q35
@@ -58,9 +63,6 @@ def test_pc_q35_kvm(self):
 self.launch_and_wait(set_up_ssh_connection=False)
 
 
-# For Aarch64 we only boot KVM tests in CI as booting the current
-# Fedora OS in TCG tests is very heavyweight. There are lighter weight
-# distros which we use in the machine_aarch64_virt.py tests.
 class BootLinuxAarch64(LinuxTest):
 """
 :avocado: tags=arch:aarch64
@@ -84,14 +86,11 @@ def test_virt_kvm(self):
 self.launch_and_wait(set_up_ssh_connection=False)
 
 
-# See the tux_baseline.py tests for almost the same coverage in a lot
-# less time.
 class BootLinuxPPC64(LinuxTest):
 """
 :avocado: tags=arch:ppc64
 """
-
-timeout = 360
+timeout = 480
 
 @skipUnless(os.getenv('AVOCADO_ALLOW_LONG_RUNTIME'), 'runtime limited')
 def test_pseries_tcg(self):
@@ -108,8 +107,7 @@ class BootLinuxS390X(LinuxTest):
 """
 :avocado: tags=arch:s390x
 """
-
-timeout = 240
+timeout = 480
 
 @skipUnless(os.getenv('AVOCADO_ALLOW_LONG_RUNTIME'), 'runtime limited')
 def test_s390_ccw_virtio_tcg(self):
-- 
2.42.0




[PATCH] hw/arm: add PCIe to Freescale i.MX6

2024-01-07 Thread Nikita Ostrenkov
Signed-off-by: Nikita Ostrenkov 
---
 hw/arm/Kconfig|  3 +++
 hw/arm/fsl-imx6.c | 25 ++
 include/hw/arm/fsl-imx6.h | 44 ---
 3 files changed, 51 insertions(+), 21 deletions(-)

diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
index 660f49db49..575bb68c76 100644
--- a/hw/arm/Kconfig
+++ b/hw/arm/Kconfig
@@ -530,13 +530,16 @@ config FSL_IMX31
 
 config FSL_IMX6
 bool
+imply PCI_DEVICES
 imply I2C_DEVICES
 select A9MPCORE
+select PCI
 select IMX
 select IMX_FEC
 select IMX_I2C
 select IMX_USBPHY
 select WDT_IMX2
+select PCI_EXPRESS_DESIGNWARE
 select SDHCI
 
 config ASPEED_SOC
diff --git a/hw/arm/fsl-imx6.c b/hw/arm/fsl-imx6.c
index b2153022c0..27702b6d6d 100644
--- a/hw/arm/fsl-imx6.c
+++ b/hw/arm/fsl-imx6.c
@@ -22,6 +22,7 @@
 #include "qemu/osdep.h"
 #include "qapi/error.h"
 #include "hw/arm/fsl-imx6.h"
+#include "hw/misc/unimp.h"
 #include "hw/usb/imx-usb-phy.h"
 #include "hw/boards.h"
 #include "hw/qdev-properties.h"
@@ -102,6 +103,8 @@ static void fsl_imx6_init(Object *obj)
 
 
 object_initialize_child(obj, "eth", >eth, TYPE_IMX_ENET);
+
+object_initialize_child(obj, "pcie", >pcie, TYPE_DESIGNWARE_PCIE_HOST);
 }
 
 static void fsl_imx6_realize(DeviceState *dev, Error **errp)
@@ -109,6 +112,7 @@ static void fsl_imx6_realize(DeviceState *dev, Error **errp)
 MachineState *ms = MACHINE(qdev_get_machine());
 FslIMX6State *s = FSL_IMX6(dev);
 uint16_t i;
+qemu_irq irq;
 unsigned int smp_cpus = ms->smp.cpus;
 
 if (smp_cpus > FSL_IMX6_NUM_CPUS) {
@@ -421,6 +425,27 @@ static void fsl_imx6_realize(DeviceState *dev, Error 
**errp)
 FSL_IMX6_WDOGn_IRQ[i]));
 }
 
+/*
+ * PCIe
+ */
+sysbus_realize(SYS_BUS_DEVICE(>pcie), _abort);
+sysbus_mmio_map(SYS_BUS_DEVICE(>pcie), 0, FSL_IMX6_PCIe_REG_ADDR);
+
+irq = qdev_get_gpio_in(DEVICE(>a9mpcore), FSL_IMX6_PCIE1_IRQ);
+sysbus_connect_irq(SYS_BUS_DEVICE(>pcie), 0, irq);
+irq = qdev_get_gpio_in(DEVICE(>a9mpcore), FSL_IMX6_PCIE2_IRQ);
+sysbus_connect_irq(SYS_BUS_DEVICE(>pcie), 1, irq);
+irq = qdev_get_gpio_in(DEVICE(>a9mpcore), FSL_IMX6_PCIE3_IRQ);
+sysbus_connect_irq(SYS_BUS_DEVICE(>pcie), 2, irq);
+irq = qdev_get_gpio_in(DEVICE(>a9mpcore), FSL_IMX6_PCIE4_IRQ);
+sysbus_connect_irq(SYS_BUS_DEVICE(>pcie), 3, irq);
+
+/*
+ * PCIe PHY
+ */
+create_unimplemented_device("pcie-phy", FSL_IMX6_PCIe_ADDR,
+FSL_IMX6_PCIe_SIZE);
+
 /* ROM memory */
 if (!memory_region_init_rom(>rom, OBJECT(dev), "imx6.rom",
 FSL_IMX6_ROM_SIZE, errp)) {
diff --git a/include/hw/arm/fsl-imx6.h b/include/hw/arm/fsl-imx6.h
index 519b871014..61c593ffd2 100644
--- a/include/hw/arm/fsl-imx6.h
+++ b/include/hw/arm/fsl-imx6.h
@@ -32,6 +32,7 @@
 #include "hw/net/imx_fec.h"
 #include "hw/usb/chipidea.h"
 #include "hw/usb/imx-usb-phy.h"
+#include "hw/pci-host/designware.h"
 #include "exec/memory.h"
 #include "cpu.h"
 #include "qom/object.h"
@@ -55,27 +56,28 @@ struct FslIMX6State {
 DeviceState parent_obj;
 
 /*< public >*/
-ARMCPU cpu[FSL_IMX6_NUM_CPUS];
-A9MPPrivState  a9mpcore;
-IMX6CCMState   ccm;
-IMX6SRCState   src;
-IMX7SNVSState  snvs;
-IMXSerialState uart[FSL_IMX6_NUM_UARTS];
-IMXGPTStategpt;
-IMXEPITState   epit[FSL_IMX6_NUM_EPITS];
-IMXI2CStatei2c[FSL_IMX6_NUM_I2CS];
-IMXGPIOState   gpio[FSL_IMX6_NUM_GPIOS];
-SDHCIState esdhc[FSL_IMX6_NUM_ESDHCS];
-IMXSPIStatespi[FSL_IMX6_NUM_ECSPIS];
-IMX2WdtState   wdt[FSL_IMX6_NUM_WDTS];
-IMXUSBPHYState usbphy[FSL_IMX6_NUM_USB_PHYS];
-ChipideaState  usb[FSL_IMX6_NUM_USBS];
-IMXFECStateeth;
-MemoryRegion   rom;
-MemoryRegion   caam;
-MemoryRegion   ocram;
-MemoryRegion   ocram_alias;
-uint32_t   phy_num;
+ARMCPU cpu[FSL_IMX6_NUM_CPUS];
+A9MPPrivState  a9mpcore;
+IMX6CCMState   ccm;
+IMX6SRCState   src;
+IMX7SNVSState  snvs;
+IMXSerialState uart[FSL_IMX6_NUM_UARTS];
+IMXGPTStategpt;
+IMXEPITState   epit[FSL_IMX6_NUM_EPITS];
+IMXI2CStatei2c[FSL_IMX6_NUM_I2CS];
+IMXGPIOState   gpio[FSL_IMX6_NUM_GPIOS];
+SDHCIState esdhc[FSL_IMX6_NUM_ESDHCS];
+IMXSPIStatespi[FSL_IMX6_NUM_ECSPIS];
+IMX2WdtState   wdt[FSL_IMX6_NUM_WDTS];
+IMXUSBPHYState usbphy[FSL_IMX6_NUM_USB_PHYS];
+ChipideaState  usb[FSL_IMX6_NUM_USBS];
+IMXFECStateeth;
+DesignwarePCIEHost pcie;
+MemoryRegion   rom;
+MemoryRegion   caam;
+MemoryRegion   ocram;
+MemoryRegion   ocram_alias;
+uint32_t   phy_num;
 };
 
 
-- 
2.34.1




Re: [PULL 00/26] Migration 20240104 patches

2024-01-07 Thread Stefan Hajnoczi
On Sun, 7 Jan 2024 at 10:23, Peter Maydell  wrote:
>
> On Sun, 7 Jan 2024 at 12:41, Stefan Hajnoczi  wrote:
> >
> > On Sun, 7 Jan 2024 at 07:34, Peter Xu  wrote:
> > >
> > > On Fri, Jan 05, 2024 at 04:08:40PM +, Peter Maydell wrote:
> > > > I notice that your gpg key doesn't seem to be signed by anybody
> > > > else; you might look at whether it's easy to get it signed
> > > > by somebody else (eg some of your redhat colleagues).
> > >
> > > Hmm, I think I have signed with at least Juan and Stefan.  Which is the 
> > > key
> > > server we normally use?  Maybe I missed some steps there?
> >
> > Yes, Peter's key is signed by me:
> >
> > $ gpg --list-signatures 3B5FCCCDF3ABD706
> > pub   ed25519/0x3B5FCCCDF3ABD706 2023-10-03 [SC]
> >   Key fingerprint = B918 4DC2 0CC4 57DA CF7D  D1A9 3B5F CCCD F3AB D706
> > uid   [  full  ] Peter Xu 
> > sig 30x3B5FCCCDF3ABD706 2023-10-03  [self-signature]
> > sig  0x9CA4ABB381AB73C8 2023-10-10  Stefan Hajnoczi
> > 
> > uid   [  full  ] Peter Xu 
> > sig 30x3B5FCCCDF3ABD706 2023-10-03  [self-signature]
> > sig  0x9CA4ABB381AB73C8 2023-10-10  Stefan Hajnoczi
> > 
> > sub   cv25519/0xD5261EB1CB0C6E45 2023-10-03 [E]
> > sig  0x3B5FCCCDF3ABD706 2023-10-03  [self-signature]
> >
> > I have pushed to the keyservers again in case I forget.
>
> Thanks. Which keyservers did you use? I think these days the
> keyserver infrastructure is unfortunately fragmented; I
> probably didn't try refreshing from the right keyserver.

I ran gpg --send-key again and it said hkps://keyserver.ubuntu.com.

Stefan



Re: acpiBitsTest.test_acpi_smbios_bits test intermittently times out

2024-01-07 Thread Ani Sinha
On Sat, Jan 6, 2024 at 5:39 PM Peter Maydell  wrote:
>
> On Sat, 6 Jan 2024 at 05:41, Ani Sinha  wrote:
> >
> > On Sat, Jan 6, 2024 at 10:05 AM Ani Sinha  wrote:
> > >
> > > On Sat, Jan 6, 2024 at 12:11 AM Peter Maydell  
> > > wrote:
> > > >
> > > > The avocado test acpiBitsTest.test_acpi_smbios_bits seems to be
> > > > flaky in CI -- sometimes it appears to time out.
> > > >
> > > > https://gitlab.com/qemu-project/qemu/-/issues/2077
> > > > has the details (including links to jobs etc).
> > >
> > > Do you have more data points in terms of the jobs that failed?
> >
> > I just noticed that you attached three examples of failed tests. In
> > all of them the test seems to be stuck at the ami latency test.
>
> OK, if you think that subtest is suspicious, could you send a
> patch that disables just that subpart, and we'll see if it helps?

I pushed the patches here:
https://gitlab.com/anisinha/qemu/-/commits/disable-smilatency
and ran the pipeline. It passed
https://gitlab.com/anisinha/qemu/-/jobs/5878585312

and the smilatency test was not run:
https://anisinha.gitlab.io/-/qemu/-/jobs/5878585312/artifacts/build/tests/results/latest/test-results/01-tests_avocado_acpi-bits.py_AcpiBitsTest.test_acpi_smbios_bits/debug.log

Not sure what we can do to have some confidence that disabling the
test got rid of the flakiness. I can send out those two patches in the
list.




Re: [PULL 00/26] Migration 20240104 patches

2024-01-07 Thread Peter Maydell
On Sun, 7 Jan 2024 at 12:41, Stefan Hajnoczi  wrote:
>
> On Sun, 7 Jan 2024 at 07:34, Peter Xu  wrote:
> >
> > On Fri, Jan 05, 2024 at 04:08:40PM +, Peter Maydell wrote:
> > > I notice that your gpg key doesn't seem to be signed by anybody
> > > else; you might look at whether it's easy to get it signed
> > > by somebody else (eg some of your redhat colleagues).
> >
> > Hmm, I think I have signed with at least Juan and Stefan.  Which is the key
> > server we normally use?  Maybe I missed some steps there?
>
> Yes, Peter's key is signed by me:
>
> $ gpg --list-signatures 3B5FCCCDF3ABD706
> pub   ed25519/0x3B5FCCCDF3ABD706 2023-10-03 [SC]
>   Key fingerprint = B918 4DC2 0CC4 57DA CF7D  D1A9 3B5F CCCD F3AB D706
> uid   [  full  ] Peter Xu 
> sig 30x3B5FCCCDF3ABD706 2023-10-03  [self-signature]
> sig  0x9CA4ABB381AB73C8 2023-10-10  Stefan Hajnoczi
> 
> uid   [  full  ] Peter Xu 
> sig 30x3B5FCCCDF3ABD706 2023-10-03  [self-signature]
> sig  0x9CA4ABB381AB73C8 2023-10-10  Stefan Hajnoczi
> 
> sub   cv25519/0xD5261EB1CB0C6E45 2023-10-03 [E]
> sig  0x3B5FCCCDF3ABD706 2023-10-03  [self-signature]
>
> I have pushed to the keyservers again in case I forget.

Thanks. Which keyservers did you use? I think these days the
keyserver infrastructure is unfortunately fragmented; I
probably didn't try refreshing from the right keyserver.

-- PMM



Re: [PATCH] tcg/tci: Fix TCI on hppa host and update TCI test matrix

2024-01-07 Thread Peter Maydell
On Sun, 7 Jan 2024 at 07:20, Helge Deller  wrote:
>
> Update the TCI interpreter test matrix for big-endian hosts with
> big- (hppa, hppa64) and little-endian (x86,x96-64) targets.
> I used native ppc64 and hppa hosts for those tests.
>
> Starting TCI on a hppa host crashed immediately, because hppa is
> the only archive left where the stack grows upwards.
> Write-protecting the stack guard page at the top of the stack
> fixes the crash.

We deliberately dropped support for HPPA hosts, under
commit a1eaa6281f8b and commit b1cef6d02f84bd8.
Do we really care enough about trying to run on these
ancient host CPUs to want to bring it back?

My personal rule of thumb is that if a host CPU is supported
only by TCI then we are better off saying it is entirely
unsupported -- in practice the performance will be so
terrible as to not be something anybody will want to use,
especially for older architectures which are slow to
start with.

thanks
-- PMM



Re: [PATCH v4 00/11] hw/isa/vt82c686: Implement relocation and toggling of SuperI/O functions

2024-01-07 Thread Mark Cave-Ayland

On 06/01/2024 21:05, Bernhard Beschow wrote:


This series implements relocation of the SuperI/O functions of the VIA south
bridges which resolves some FIXME's. It is part of my via-apollo-pro-133t
branch [1] which is an extension of bringing the VIA south bridges to the PC
machine [2]. This branch is able to run some real-world X86 BIOSes in the hope
that it allows us to form a better understanding of the real vt82c686b devices.
Implementing relocation and toggling of the SuperI/O functions is one step to
make these BIOSes run without error messages, so here we go.

The series is structured as follows: Patches 1-3 prepare the TYPE_ISA_FDC,
TYPE_ISA_PARALLEL and TYPE_ISA_SERIAL to relocate and toggle (enable/disable)
themselves without breaking encapsulation of their respective device states.
This is achieved by moving the MemoryRegions and PortioLists from the device
states into the encapsulating ISA devices since they will be relocated and
toggled.

Inspired by the memory API patches 4-6 add two convenience functions to the
portio_list API to toggle and relocate portio lists. Patch 5 is a preparation
for that which removes some redundancies which otherwise had to be dealt with
during relocation.

Patches 7-9 implement toggling and relocation for types TYPE_ISA_FDC,
TYPE_ISA_PARALLEL and TYPE_ISA_SERIAL. Patch 10 prepares the pegasos2 machine
which would end up with all SuperI/O functions disabled if no -bios argument is
given. Patch 11 finally implements the main feature which now relies on
firmware to configure the SuperI/O functions accordingly (except for pegasos2).

v4:
* Drop incomplete SuperI/O vmstate handling (Zoltan)

v3:
* Rework various commit messages (Zoltan)
* Drop patch "hw/char/serial: Free struct SerialState from MemoryRegion"
   (Zoltan)
* Generalize wording in migration.rst to include portio_list API (Zoltan)

v2:
* Improve commit messages (Zoltan)
* Split pegasos2 from vt82c686 patch (Zoltan)
* Avoid poking into device internals (Zoltan)

Testing done:
* `make check`
* `make check-avocado`
* Run MorphOS on pegasos2 with and without pegasos2.rom
* Run Linux on amigaone
* Run real-world BIOSes on via-apollo-pro-133t branch
* Start rescue-yl on fuloong2e

[1] https://github.com/shentok/qemu/tree/via-apollo-pro-133t
[2] https://github.com/shentok/qemu/tree/pc-via

Bernhard Beschow (11):
   hw/block/fdc-isa: Move portio_list from FDCtrl to FDCtrlISABus
   hw/block/fdc-sysbus: Move iomem from FDCtrl to FDCtrlSysBus
   hw/char/parallel: Move portio_list from ParallelState to
 ISAParallelState
   exec/ioport: Resolve redundant .base attribute in struct
 MemoryRegionPortio
   exec/ioport: Add portio_list_set_address()
   exec/ioport: Add portio_list_set_enabled()
   hw/block/fdc-isa: Implement relocation and enabling/disabling for
 TYPE_ISA_FDC
   hw/char/serial-isa: Implement relocation and enabling/disabling for
 TYPE_ISA_SERIAL
   hw/char/parallel-isa: Implement relocation and enabling/disabling for
 TYPE_ISA_PARALLEL
   hw/ppc/pegasos2: Let pegasos2 machine configure SuperI/O functions
   hw/isa/vt82c686: Implement relocation and toggling of SuperI/O
 functions

  docs/devel/migration.rst   |  6 ++--
  hw/block/fdc-internal.h|  4 ---
  include/exec/ioport.h  |  4 ++-
  include/hw/block/fdc.h |  3 ++
  include/hw/char/parallel-isa.h |  5 +++
  include/hw/char/parallel.h |  2 --
  include/hw/char/serial.h   |  2 ++
  hw/block/fdc-isa.c | 18 +-
  hw/block/fdc-sysbus.c  |  6 ++--
  hw/char/parallel-isa.c | 14 
  hw/char/parallel.c |  2 +-
  hw/char/serial-isa.c   | 14 
  hw/isa/vt82c686.c  | 66 --
  hw/ppc/pegasos2.c  | 15 
  system/ioport.c| 41 +
  15 files changed, 172 insertions(+), 30 deletions(-)


I think this series generally looks good: the only thing I think it's worth checking 
is whether portio lists are considered exclusive to ISA devices or not? (Paolo?).


The portio_list_set_enabled() API looks interesting, and could be considered for use 
by my PCI IDE mode-switching changes too.


Apologies I don't have a huge amount of time for review right now, but I wanted to 
feed back that generally these patches look good, and if people are happy with the 
portio list changes then this series should be considered for merge.



ATB,

Mark.




[PATCH v4 3/4] tests/qtest/pvpanic: use centralized definition of supported events

2024-01-07 Thread Thomas Weißschuh
Avoid the necessity to update all tests when new events are added
to the device.

Acked-by: Thomas Huth 
Signed-off-by: Thomas Weißschuh 
---
 tests/qtest/pvpanic-pci-test.c | 5 +++--
 tests/qtest/pvpanic-test.c | 5 +++--
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/tests/qtest/pvpanic-pci-test.c b/tests/qtest/pvpanic-pci-test.c
index 2c05b376ba72..b372caf41dc0 100644
--- a/tests/qtest/pvpanic-pci-test.c
+++ b/tests/qtest/pvpanic-pci-test.c
@@ -16,6 +16,7 @@
 #include "qapi/qmp/qdict.h"
 #include "libqos/pci.h"
 #include "libqos/pci-pc.h"
+#include "hw/misc/pvpanic.h"
 #include "hw/pci/pci_regs.h"
 
 static void test_panic_nopause(void)
@@ -34,7 +35,7 @@ static void test_panic_nopause(void)
 bar = qpci_iomap(dev, 0, NULL);
 
 qpci_memread(dev, bar, 0, , sizeof(val));
-g_assert_cmpuint(val, ==, 3);
+g_assert_cmpuint(val, ==, PVPANIC_EVENTS);
 
 val = 1;
 qpci_memwrite(dev, bar, 0, , sizeof(val));
@@ -67,7 +68,7 @@ static void test_panic(void)
 bar = qpci_iomap(dev, 0, NULL);
 
 qpci_memread(dev, bar, 0, , sizeof(val));
-g_assert_cmpuint(val, ==, 3);
+g_assert_cmpuint(val, ==, PVPANIC_EVENTS);
 
 val = 1;
 qpci_memwrite(dev, bar, 0, , sizeof(val));
diff --git a/tests/qtest/pvpanic-test.c b/tests/qtest/pvpanic-test.c
index 78f1cf8186b0..ccc603472f5d 100644
--- a/tests/qtest/pvpanic-test.c
+++ b/tests/qtest/pvpanic-test.c
@@ -10,6 +10,7 @@
 #include "qemu/osdep.h"
 #include "libqtest.h"
 #include "qapi/qmp/qdict.h"
+#include "hw/misc/pvpanic.h"
 
 static void test_panic_nopause(void)
 {
@@ -20,7 +21,7 @@ static void test_panic_nopause(void)
 qts = qtest_init("-device pvpanic -action panic=none");
 
 val = qtest_inb(qts, 0x505);
-g_assert_cmpuint(val, ==, 3);
+g_assert_cmpuint(val, ==, PVPANIC_EVENTS);
 
 qtest_outb(qts, 0x505, 0x1);
 
@@ -43,7 +44,7 @@ static void test_panic(void)
 qts = qtest_init("-device pvpanic -action panic=pause");
 
 val = qtest_inb(qts, 0x505);
-g_assert_cmpuint(val, ==, 3);
+g_assert_cmpuint(val, ==, PVPANIC_EVENTS);
 
 qtest_outb(qts, 0x505, 0x1);
 

-- 
2.43.0




[PATCH v4 4/4] hw/misc/pvpanic: add support for normal shutdowns

2024-01-07 Thread Thomas Weißschuh
Shutdown requests are normally hardware dependent.
By extending pvpanic to also handle shutdown requests, guests can
submit such requests with an easily implementable and cross-platform
mechanism.

Signed-off-by: Thomas Weißschuh 
---
 docs/specs/pvpanic.rst| 2 ++
 hw/misc/pvpanic.c | 5 +
 include/hw/misc/pvpanic.h | 3 ++-
 3 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/docs/specs/pvpanic.rst b/docs/specs/pvpanic.rst
index f894bc19555f..796cc0348a38 100644
--- a/docs/specs/pvpanic.rst
+++ b/docs/specs/pvpanic.rst
@@ -29,6 +29,8 @@ bit 1
   a guest panic has happened and will be handled by the guest;
   the host should record it or report it, but should not affect
   the execution of the guest.
+bit 2
+  a guest shutdown has happened and should be processed by the host
 
 PCI Interface
 -
diff --git a/hw/misc/pvpanic.c b/hw/misc/pvpanic.c
index a4982cc5928e..246f9ae4e992 100644
--- a/hw/misc/pvpanic.c
+++ b/hw/misc/pvpanic.c
@@ -40,6 +40,11 @@ static void handle_event(int event)
 qemu_system_guest_crashloaded(NULL);
 return;
 }
+
+if (event & PVPANIC_SHUTDOWN) {
+qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN);
+return;
+}
 }
 
 /* return supported events on read */
diff --git a/include/hw/misc/pvpanic.h b/include/hw/misc/pvpanic.h
index 48f2ec4c86a1..9e36a02d5a4f 100644
--- a/include/hw/misc/pvpanic.h
+++ b/include/hw/misc/pvpanic.h
@@ -20,7 +20,8 @@
 
 #define PVPANIC_PANICKED   (1 << 0)
 #define PVPANIC_CRASH_LOADED   (1 << 1)
-#define PVPANIC_EVENTS (PVPANIC_PANICKED | PVPANIC_CRASH_LOADED)
+#define PVPANIC_SHUTDOWN   (1 << 2)
+#define PVPANIC_EVENTS (PVPANIC_PANICKED | PVPANIC_CRASH_LOADED | 
PVPANIC_SHUTDOWN)
 
 #define TYPE_PVPANIC_ISA_DEVICE "pvpanic"
 #define TYPE_PVPANIC_PCI_DEVICE "pvpanic-pci"

-- 
2.43.0




[PATCH v4 2/4] hw/misc/pvpanic: centralize definition of supported events

2024-01-07 Thread Thomas Weißschuh
The different components of pvpanic duplicate the list of supported
events. Move it to the shared header file to minimize changes when new
events are added.

Signed-off-by: Thomas Weißschuh 
---
 hw/misc/pvpanic-isa.c | 2 +-
 hw/misc/pvpanic-pci.c | 2 +-
 hw/misc/pvpanic.c | 2 +-
 include/hw/misc/pvpanic.h | 1 +
 4 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/hw/misc/pvpanic-isa.c b/hw/misc/pvpanic-isa.c
index ef438a31fbe9..9a923b786907 100644
--- a/hw/misc/pvpanic-isa.c
+++ b/hw/misc/pvpanic-isa.c
@@ -101,7 +101,7 @@ static void build_pvpanic_isa_aml(AcpiDevAmlIf *adev, Aml 
*scope)
 static Property pvpanic_isa_properties[] = {
 DEFINE_PROP_UINT16(PVPANIC_IOPORT_PROP, PVPanicISAState, ioport, 0x505),
 DEFINE_PROP_UINT8("events", PVPanicISAState, pvpanic.events,
-  PVPANIC_PANICKED | PVPANIC_CRASH_LOADED),
+  PVPANIC_EVENTS),
 DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/hw/misc/pvpanic-pci.c b/hw/misc/pvpanic-pci.c
index 01e269b55284..be4063121e1d 100644
--- a/hw/misc/pvpanic-pci.c
+++ b/hw/misc/pvpanic-pci.c
@@ -54,7 +54,7 @@ static void pvpanic_pci_realizefn(PCIDevice *dev, Error 
**errp)
 
 static Property pvpanic_pci_properties[] = {
 DEFINE_PROP_UINT8("events", PVPanicPCIState, pvpanic.events,
-  PVPANIC_PANICKED | PVPANIC_CRASH_LOADED),
+  PVPANIC_EVENTS),
 DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/hw/misc/pvpanic.c b/hw/misc/pvpanic.c
index 4915ef256e74..a4982cc5928e 100644
--- a/hw/misc/pvpanic.c
+++ b/hw/misc/pvpanic.c
@@ -26,7 +26,7 @@ static void handle_event(int event)
 {
 static bool logged;
 
-if (event & ~(PVPANIC_PANICKED | PVPANIC_CRASH_LOADED) && !logged) {
+if (event & ~PVPANIC_EVENTS && !logged) {
 qemu_log_mask(LOG_GUEST_ERROR, "pvpanic: unknown event %#x.\n", event);
 logged = true;
 }
diff --git a/include/hw/misc/pvpanic.h b/include/hw/misc/pvpanic.h
index dffca827f77a..48f2ec4c86a1 100644
--- a/include/hw/misc/pvpanic.h
+++ b/include/hw/misc/pvpanic.h
@@ -20,6 +20,7 @@
 
 #define PVPANIC_PANICKED   (1 << 0)
 #define PVPANIC_CRASH_LOADED   (1 << 1)
+#define PVPANIC_EVENTS (PVPANIC_PANICKED | PVPANIC_CRASH_LOADED)
 
 #define TYPE_PVPANIC_ISA_DEVICE "pvpanic"
 #define TYPE_PVPANIC_PCI_DEVICE "pvpanic-pci"

-- 
2.43.0




[PATCH v4 1/4] linux-headers: drop pvpanic.h

2024-01-07 Thread Thomas Weißschuh
misc/pvpanic.h from the Linux UAPI does not define a Linux UAPI but a
qemu device API.

This leads to a weird process when updates to the interface are needed:
1) Change to the specification in the qemu tree
2) Change to the header in the Linux tree
3) Re-import of the header into Qemu.

The kernel prefers to drop the header anyways.

Prepare for the removal from the Linux UAPI headers by moving the
contents to the existing pvpanic.h header.

Link: https://lore.kernel.org/lkml/2023110431-pacemaker-pruning-0e4c@gregkh/
Signed-off-by: Thomas Weißschuh 
---
 hw/misc/pvpanic-isa.c| 1 -
 hw/misc/pvpanic-pci.c| 1 -
 hw/misc/pvpanic.c| 1 -
 include/hw/misc/pvpanic.h| 3 +++
 include/standard-headers/linux/pvpanic.h | 9 -
 scripts/update-linux-headers.sh  | 3 +--
 6 files changed, 4 insertions(+), 14 deletions(-)

diff --git a/hw/misc/pvpanic-isa.c b/hw/misc/pvpanic-isa.c
index ccec50f61bbd..ef438a31fbe9 100644
--- a/hw/misc/pvpanic-isa.c
+++ b/hw/misc/pvpanic-isa.c
@@ -21,7 +21,6 @@
 #include "hw/misc/pvpanic.h"
 #include "qom/object.h"
 #include "hw/isa/isa.h"
-#include "standard-headers/linux/pvpanic.h"
 #include "hw/acpi/acpi_aml_interface.h"
 
 OBJECT_DECLARE_SIMPLE_TYPE(PVPanicISAState, PVPANIC_ISA_DEVICE)
diff --git a/hw/misc/pvpanic-pci.c b/hw/misc/pvpanic-pci.c
index c01e4ce8646a..01e269b55284 100644
--- a/hw/misc/pvpanic-pci.c
+++ b/hw/misc/pvpanic-pci.c
@@ -21,7 +21,6 @@
 #include "hw/misc/pvpanic.h"
 #include "qom/object.h"
 #include "hw/pci/pci_device.h"
-#include "standard-headers/linux/pvpanic.h"
 
 OBJECT_DECLARE_SIMPLE_TYPE(PVPanicPCIState, PVPANIC_PCI_DEVICE)
 
diff --git a/hw/misc/pvpanic.c b/hw/misc/pvpanic.c
index 1540e9091a45..4915ef256e74 100644
--- a/hw/misc/pvpanic.c
+++ b/hw/misc/pvpanic.c
@@ -21,7 +21,6 @@
 #include "hw/qdev-properties.h"
 #include "hw/misc/pvpanic.h"
 #include "qom/object.h"
-#include "standard-headers/linux/pvpanic.h"
 
 static void handle_event(int event)
 {
diff --git a/include/hw/misc/pvpanic.h b/include/hw/misc/pvpanic.h
index fab94165d03d..dffca827f77a 100644
--- a/include/hw/misc/pvpanic.h
+++ b/include/hw/misc/pvpanic.h
@@ -18,6 +18,9 @@
 #include "exec/memory.h"
 #include "qom/object.h"
 
+#define PVPANIC_PANICKED   (1 << 0)
+#define PVPANIC_CRASH_LOADED   (1 << 1)
+
 #define TYPE_PVPANIC_ISA_DEVICE "pvpanic"
 #define TYPE_PVPANIC_PCI_DEVICE "pvpanic-pci"
 
diff --git a/include/standard-headers/linux/pvpanic.h 
b/include/standard-headers/linux/pvpanic.h
deleted file mode 100644
index 54b7485390d3..
--- a/include/standard-headers/linux/pvpanic.h
+++ /dev/null
@@ -1,9 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
-
-#ifndef __PVPANIC_H__
-#define __PVPANIC_H__
-
-#define PVPANIC_PANICKED   (1 << 0)
-#define PVPANIC_CRASH_LOADED   (1 << 1)
-
-#endif /* __PVPANIC_H__ */
diff --git a/scripts/update-linux-headers.sh b/scripts/update-linux-headers.sh
index 34295c0fe55b..555bdc8af2eb 100755
--- a/scripts/update-linux-headers.sh
+++ b/scripts/update-linux-headers.sh
@@ -215,8 +215,7 @@ for i in "$tmpdir"/include/linux/*virtio*.h \
  "$tmpdir/include/linux/const.h" \
  "$tmpdir/include/linux/kernel.h" \
  "$tmpdir/include/linux/vhost_types.h" \
- "$tmpdir/include/linux/sysinfo.h" \
- "$tmpdir/include/misc/pvpanic.h"; do
+ "$tmpdir/include/linux/sysinfo.h"; do
 cp_portable "$i" "$output/include/standard-headers/linux"
 done
 mkdir -p "$output/include/standard-headers/drm"

-- 
2.43.0




Re: [PATCH v5 2/3] tests/qtest: Add STM32L4x5 EXTI QTest testcase

2024-01-07 Thread Mark Cave-Ayland

On 05/01/2024 10:13, Philippe Mathieu-Daudé wrote:


(+Mark & Eduardo)

On 4/1/24 14:37, inesvarhol wrote:


Le jeudi 4 janvier 2024 à 14:05, Philippe Mathieu-Daudé  a 
écrit :

Hello,


+static void test_edge_selector(void)
+{
+ enable_nvic_irq(EXTI0_IRQ);
+
+ / Configure EXTI line 0 irq on rising edge */
+ qtest_set_irq_in(global_qtest, "/machine/unattached/device[0]/exti",



Markus, this qtest use seems to expect some stability in QOM path...

Inès, Arnaud, having the SoC unattached is dubious, it belongs to
the machine.


Noted, we will fix that.
Should we be concerned about the "stability in QOM path" ?


Don't worry about this Inès, I wanted to raise Markus attention on this.

You showed a legit use of stable QOM path, and Markus told me recently
there is no contract for QOM paths (it shouldn't be considered as a
stable API). IIRC Markus explanation, "/unattached" container was
added as a temporary hack to allow migrating QDev objects to QOM (see
around commit da57febfed "qdev: give all devices a canonical path",
11 years ago).

I agree anything under "/unattached" can be expected to be stable
(but we need a community consensus). Then the big question remaining
is "can any qom-path out of /unattached be considered stable?"


For the moment I would definitely say no, and that is mainly because if we were to 
assume that QOM paths were stable today then I can see it being a barrier to updating 
older code to meet our current guidelines.


These days I think more about QOM paths being related to the lifecycle of the objects 
e.g. a machine object has child devices, which may also consist of a number of other 
children in the case of a multi-function device. For me this means that using 
object_resolve_path_component() to look up a child object seems reasonable, in 
contrast with expecting the entire path to be stable.


One thing I think about often is whether the use of device[n] is suitable within QOM 
tree. For example, if I have a command line like:


  -device foo,myprop=prop0,id=fooid0 -device foo,myprop=prop1,id=fooid1

currently they would appear in "info qom-tree" as:

  /machine
/unattached
  /device[0] (foo)
  /device[1] (foo)

whereas it feels this could be done better as:

  /machine
/unattached
  /foo[0] (fooid0)
  /foo[1] (fooid1)

This would automatically place devices of the same type within a QOM array to allow 
them to be accessed separately by type, or even directly via the "id" if we assume 
they are unique. In particular if you have a machine with 2 foo in-built devices you 
could then potentially configure them separately using -global foo[0].myprop=newprop0 
and/or -global foo[1].myprop=newprop1 which is something that currently isn't possible.



ATB,

Mark.




[PATCH v4 0/4] hw/misc/pvpanic: add support for normal shutdowns

2024-01-07 Thread Thomas Weißschuh
Shutdown requests are normally hardware dependent.
By extending pvpanic to also handle shutdown requests, guests can
submit such requests with an easily implementable and cross-platform
mechanism.

The background is the usage of minimal Linux kernels with different
architectures for testing purposes.
Poweroff support varies highly per architecture and requires a bunch of
code to be compiled to work.
pvpanic on the other hand is very small and uniform.

I sent an RFC[0] for this before to qemu-devel and lkml which didn't
generate feedback, so let's discuss the concrete proposal.

Patch 1 and 2 are general cleanups, that seems useful even without this
proposal being implemented.

A corresponding patch has been submitted for Linux [1].
This is also where the request was voiced to drop move away from a
pvpanic uapi header in Linux.

[0] https://lore.kernel.org/all/984794aa-4af0-4c68-a74e-7420ec315...@t-8ch.de/
[1] 
https://lore.kernel.org/lkml/20231104-pvpanic-shutdown-v1-1-5ee7c9b3e...@weissschuh.net/

Signed-off-by: Thomas Weißschuh 
---
Changes in v4:
- Rebase on 8.2 master
- Resend after tree reopened and holidays
- Link to v3: 
https://lore.kernel.org/r/20231129-pvpanic-shutdown-v3-0-c9a2892fc...@t-8ch.de

Changes in v3:
- Drop from Linux imported pvpanic header as discussed with Cornelia and
  requested by Greg
- Link to v2: 
https://lore.kernel.org/r/20231128-pvpanic-shutdown-v2-0-830393b45...@t-8ch.de

Changes in v2:
- Remove RFC status
- Add Ack from Thomas to 2nd patch
- Fix typo in title of 2nd patch
- Link to v1: 
https://lore.kernel.org/r/20231104-pvpanic-shutdown-v1-0-023531578...@t-8ch.de

---
Thomas Weißschuh (4):
  linux-headers: drop pvpanic.h
  hw/misc/pvpanic: centralize definition of supported events
  tests/qtest/pvpanic: use centralized definition of supported events
  hw/misc/pvpanic: add support for normal shutdowns

 docs/specs/pvpanic.rst   | 2 ++
 hw/misc/pvpanic-isa.c| 3 +--
 hw/misc/pvpanic-pci.c| 3 +--
 hw/misc/pvpanic.c| 8 ++--
 include/hw/misc/pvpanic.h| 5 +
 include/standard-headers/linux/pvpanic.h | 9 -
 scripts/update-linux-headers.sh  | 3 +--
 tests/qtest/pvpanic-pci-test.c   | 5 +++--
 tests/qtest/pvpanic-test.c   | 5 +++--
 9 files changed, 22 insertions(+), 21 deletions(-)
---
base-commit: 0c1eccd368af8805ec0fb11e6cf25d0684d37328
change-id: 20231104-pvpanic-shutdown-02e4b4cb4949

Best regards,
-- 
Thomas Weißschuh 




[PATCH v2 0/2] linux-user: two fixes to coredump generation

2024-01-07 Thread Thomas Weißschuh
Signed-off-by: Thomas Weißschuh 
---
Changes in v2:
- Rebase on 8.2 master
- Resend after closed tree and holidays
- Link to v1: 
https://lore.kernel.org/r/20231115-qemu-user-dumpable-v1-0-edbe7f0fb...@t-8ch.de

---
Thomas Weißschuh (2):
  linux-user/elfload: test return value of getrlimit
  linux-user/elfload: check PR_GET_DUMPABLE before creating coredump

 linux-user/elfload.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)
---
base-commit: 0c1eccd368af8805ec0fb11e6cf25d0684d37328
change-id: 20231115-qemu-user-dumpable-d499c0396103

Best regards,
-- 
Thomas Weißschuh 




[PATCH v2 2/2] linux-user/elfload: check PR_GET_DUMPABLE before creating coredump

2024-01-07 Thread Thomas Weißschuh
A process can opt-out of coredump creation by calling
prctl(PR_SET_DUMPABLE, 0).
linux-user passes this call from the guest through to the
operating system.
>From there it can be read back again to avoid creating coredumps from
qemu-user itself if the guest chose so.

Signed-off-by: Thomas Weißschuh 
---
 linux-user/elfload.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index 74c9ecda1806..956cb3ae2da5 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -2,6 +2,7 @@
 #include "qemu/osdep.h"
 #include 
 
+#include 
 #include 
 #include 
 
@@ -4667,6 +4668,10 @@ static int elf_core_dump(int signr, const CPUArchState 
*env)
 init_note_info();
 
 errno = 0;
+
+if (prctl(PR_GET_DUMPABLE) == 0)
+return 0;
+
 if (getrlimit(RLIMIT_CORE, ) == 0 && dumpsize.rlim_cur == 0)
 return 0;
 

-- 
2.43.0




[PATCH v2 1/2] linux-user/elfload: test return value of getrlimit

2024-01-07 Thread Thomas Weißschuh
Should getrlimit() fail the value of dumpsize.rlimit_cur may not be
initialized. Avoid reading garbage data by checking the return value of
getrlimit.

Signed-off-by: Thomas Weißschuh 
---
 linux-user/elfload.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index cf9e74468b11..74c9ecda1806 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -4667,8 +4667,7 @@ static int elf_core_dump(int signr, const CPUArchState 
*env)
 init_note_info();
 
 errno = 0;
-getrlimit(RLIMIT_CORE, );
-if (dumpsize.rlim_cur == 0)
+if (getrlimit(RLIMIT_CORE, ) == 0 && dumpsize.rlim_cur == 0)
 return 0;
 
 corefile = core_dump_filename(ts);

-- 
2.43.0




Re: [PATCH v4 11/11] hw/isa/vt82c686: Implement relocation and toggling of SuperI/O functions

2024-01-07 Thread BALATON Zoltan

On Sat, 6 Jan 2024, Bernhard Beschow wrote:

The VIA south bridges are able to relocate and toggle (enable or disable) their
SuperI/O functions. So far this is hardcoded such that all functions are always
enabled and are located at fixed addresses.

Some PC BIOSes seem to probe for I/O occupancy before activating such a function
and issue an error in case of a conflict. Since the functions are currently
enabled on reset, conflicts are always detected. Prevent that by implementing
relocation and toggling of the SuperI/O functions.

Note that all SuperI/O functions are now deactivated upon reset (except for
VT82C686B's serial ports where Fuloong 2e's rescue-yl seems to expect them to be
enabled by default). Rely on firmware to configure the functions accordingly.

Signed-off-by: Bernhard Beschow 
---
hw/isa/vt82c686.c | 66 ---
1 file changed, 56 insertions(+), 10 deletions(-)

diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
index d3e0f6d01f..9f62fb5964 100644
--- a/hw/isa/vt82c686.c
+++ b/hw/isa/vt82c686.c
@@ -15,6 +15,9 @@

#include "qemu/osdep.h"
#include "hw/isa/vt82c686.h"
+#include "hw/block/fdc.h"
+#include "hw/char/parallel-isa.h"
+#include "hw/char/serial.h"
#include "hw/pci/pci.h"
#include "hw/qdev-properties.h"
#include "hw/ide/pci.h"
@@ -323,6 +326,18 @@ static uint64_t via_superio_cfg_read(void *opaque, hwaddr 
addr, unsigned size)
return val;
}

+static void via_superio_devices_enable(ViaSuperIOState *s, uint8_t data)
+{
+ISASuperIOClass *ic = ISA_SUPERIO_GET_CLASS(s);
+size_t i;


The expected value for i is 0 or 1 (maybe up to 3 sometimes it there are 
more serial ports in a chip). so why use such big type? This should just 
be int. Newly it's also allowed to declare it within the for so if you 
want that you could do so but I have no preference on that and declaring 
it here is also OK. Otherwise:


Reviewed-by: BALATON Zoltan 


+
+isa_parallel_set_enabled(s->superio.parallel[0], (data & 0x3) != 3);
+for (i = 0; i < ic->serial.count; i++) {
+isa_serial_set_enabled(s->superio.serial[i], data & BIT(i + 2));
+}
+isa_fdc_set_enabled(s->superio.floppy, data & BIT(4));
+}
+
static void via_superio_class_init(ObjectClass *klass, void *data)
{
DeviceClass *dc = DEVICE_CLASS(klass);
@@ -368,7 +383,25 @@ static void vt82c686b_superio_cfg_write(void *opaque, 
hwaddr addr,
case 0xfd ... 0xff:
/* ignore write to read only registers */
return;
-/* case 0xe6 ... 0xe8: Should set base port of parallel and serial */
+case 0xe2:
+data &= 0x1f;
+via_superio_devices_enable(sc, data);
+break;
+case 0xe3:
+data &= 0xfc;
+isa_fdc_set_iobase(sc->superio.floppy, data << 2);
+break;
+case 0xe6:
+isa_parallel_set_iobase(sc->superio.parallel[0], data << 2);
+break;
+case 0xe7:
+data &= 0xfe;
+isa_serial_set_iobase(sc->superio.serial[0], data << 2);
+break;
+case 0xe8:
+data &= 0xfe;
+isa_serial_set_iobase(sc->superio.serial[1], data << 2);
+break;
default:
qemu_log_mask(LOG_UNIMP,
  "via_superio_cfg: unimplemented register 0x%x\n", idx);
@@ -395,9 +428,14 @@ static void vt82c686b_superio_reset(DeviceState *dev)
/* Device ID */
vt82c686b_superio_cfg_write(s, 0, 0xe0, 1);
vt82c686b_superio_cfg_write(s, 1, 0x3c, 1);
-/* Function select - all disabled */
+/*
+ * Function select - only serial enabled
+ * Fuloong 2e's rescue-yl prints to the serial console w/o enabling it. 
This
+ * suggests that the serial ports are enabled by default, so override the
+ * datasheet.
+ */
vt82c686b_superio_cfg_write(s, 0, 0xe2, 1);
-vt82c686b_superio_cfg_write(s, 1, 0x03, 1);
+vt82c686b_superio_cfg_write(s, 1, 0x0f, 1);
/* Floppy ctrl base addr 0x3f0-7 */
vt82c686b_superio_cfg_write(s, 0, 0xe3, 1);
vt82c686b_superio_cfg_write(s, 1, 0xfc, 1);
@@ -465,6 +503,21 @@ static void vt8231_superio_cfg_write(void *opaque, hwaddr 
addr,
case 0xfd:
/* ignore write to read only registers */
return;
+case 0xf2:
+data &= 0x17;
+via_superio_devices_enable(sc, data);
+break;
+case 0xf4:
+data &= 0xfe;
+isa_serial_set_iobase(sc->superio.serial[0], data << 2);
+break;
+case 0xf6:
+isa_parallel_set_iobase(sc->superio.parallel[0], data << 2);
+break;
+case 0xf7:
+data &= 0xfc;
+isa_fdc_set_iobase(sc->superio.floppy, data << 2);
+break;
default:
qemu_log_mask(LOG_UNIMP,
  "via_superio_cfg: unimplemented register 0x%x\n", idx);
@@ -513,12 +566,6 @@ static void vt8231_superio_init(Object *obj)
VIA_SUPERIO(obj)->io_ops = _superio_cfg_ops;
}

-static uint16_t vt8231_superio_serial_iobase(ISASuperIODevice *sio,
- uint8_t index)
-{
-  

Re: [PATCH v4 10/11] hw/ppc/pegasos2: Let pegasos2 machine configure SuperI/O functions

2024-01-07 Thread BALATON Zoltan

On Sat, 6 Jan 2024, Bernhard Beschow wrote:

This is a preparation for implementing relocation and toggling of SuperI/O
functions in the VT8231 device model. Upon reset, all SuperI/O functions will be
deactivated, so in case if no -bios is given, let the machine configure those
functions the same way Pegasos II firmware would do.

Signed-off-by: Bernhard Beschow 
---
hw/ppc/pegasos2.c | 15 +++
1 file changed, 15 insertions(+)

diff --git a/hw/ppc/pegasos2.c b/hw/ppc/pegasos2.c
index 3203a4a728..0a40ebd542 100644
--- a/hw/ppc/pegasos2.c
+++ b/hw/ppc/pegasos2.c
@@ -285,6 +285,15 @@ static void pegasos2_pci_config_write(Pegasos2MachineState 
*pm, int bus,
pegasos2_mv_reg_write(pm, pcicfg + 4, len, val);
}

+static void pegasos2_superio_write(Pegasos2MachineState *pm, uint32_t addr,
+   uint32_t val)
+{
+AddressSpace *as = CPU(pm->cpu)->as;


I think this function should not need Pegasos2MachineState *pm and can 
just use cpu_physical_memory_write() instead. Otherwise


Reviewed-by: BALATON Zoltan 


+
+stb_phys(as, PCI1_IO_BASE + 0x3f0, addr);
+stb_phys(as, PCI1_IO_BASE + 0x3f1, val);
+}
+
static void pegasos2_machine_reset(MachineState *machine, ShutdownCause reason)
{
Pegasos2MachineState *pm = PEGASOS2_MACHINE(machine);
@@ -310,6 +319,12 @@ static void pegasos2_machine_reset(MachineState *machine, 
ShutdownCause reason)

pegasos2_pci_config_write(pm, 1, (PCI_DEVFN(12, 0) << 8) |
  PCI_INTERRUPT_LINE, 2, 0x9);
+pegasos2_pci_config_write(pm, 1, (PCI_DEVFN(12, 0) << 8) |
+  0x50, 1, 0x6);
+pegasos2_superio_write(pm, 0xf4, 0xbe);
+pegasos2_superio_write(pm, 0xf6, 0xef);
+pegasos2_superio_write(pm, 0xf7, 0xfc);
+pegasos2_superio_write(pm, 0xf2, 0x14);
pegasos2_pci_config_write(pm, 1, (PCI_DEVFN(12, 0) << 8) |
  0x50, 1, 0x2);
pegasos2_pci_config_write(pm, 1, (PCI_DEVFN(12, 0) << 8) |





Re: [PATCH v2 0/9] target/hppa qemu v8.2 regression fixes

2024-01-07 Thread Bruno Haible
> The whole series can be pulled from the "hppa-fixes-8.2" branch from:
> https://github.com/hdeller/qemu-hppa.githppa-fixes-8.2

Tested-by: Bruno Haible 






Re: [PATCH v11 0/7] Support x2APIC mode with TCG accelerator

2024-01-07 Thread Michael S. Tsirkin
On Sat, Jan 06, 2024 at 11:33:29PM +0700, Bui Quang Minh wrote:
> On 12/28/23 22:44, Bui Quang Minh wrote:
> > On 12/26/23 16:21, Michael S. Tsirkin wrote:
> > > On Mon, Dec 25, 2023 at 11:40:54PM +0700, Bui Quang Minh wrote:
> > > > Hi everyone,
> > > > 
> > > > This series implements x2APIC mode in userspace local APIC and the
> > > > RDMSR/WRMSR helper to access x2APIC registers in x2APIC mode.
> > > > Intel iommu
> > > > and AMD iommu are adjusted to support x2APIC interrupt
> > > > remapping. With this
> > > > series, we can now boot Linux kernel into x2APIC mode with TCG
> > > > accelerator
> > > > using either Intel or AMD iommu.
> > > > 
> > > > Testing to boot my own built Linux 6.3.0-rc2, the kernel
> > > > successfully boot
> > > > with enabled x2APIC and can enumerate CPU with APIC ID 257
> > > > 
> > > > Using Intel IOMMU
> > > > 
> > > > qemu/build/qemu-system-x86_64 \
> > > >    -smp 2,maxcpus=260 \
> > > >    -cpu qemu64,x2apic=on \
> > > >    -machine q35 \
> > > >    -device intel-iommu,intremap=on,eim=on \
> > > >    -device
> > > > qemu64-x86_64-cpu,x2apic=on,core-id=257,socket-id=0,thread-id=0
> > > > \
> > > >    -m 2G \
> > > >    -kernel $KERNEL_DIR \
> > > >    -append "nokaslr console=ttyS0 root=/dev/sda
> > > > earlyprintk=serial net.ifnames=0" \
> > > >    -drive file=$IMAGE_DIR,format=raw \
> > > >    -nographic \
> > > >    -s
> > > > 
> > > > Using AMD IOMMU
> > > > 
> > > > qemu/build/qemu-system-x86_64 \
> > > >    -smp 2,maxcpus=260 \
> > > >    -cpu qemu64,x2apic=on \
> > > >    -machine q35 \
> > > >    -device amd-iommu,intremap=on,xtsup=on \
> > > >    -device
> > > > qemu64-x86_64-cpu,x2apic=on,core-id=257,socket-id=0,thread-id=0
> > > > \
> > > >    -m 2G \
> > > >    -kernel $KERNEL_DIR \
> > > >    -append "nokaslr console=ttyS0 root=/dev/sda
> > > > earlyprintk=serial net.ifnames=0" \
> > > >    -drive file=$IMAGE_DIR,format=raw \
> > > >    -nographic \
> > > >    -s
> > > > 
> > > > Testing the emulated userspace APIC with kvm-unit-tests, disable test
> > > > device with this patch
> > > 
> > > Seems to break build for windows/amd64
> > > https://gitlab.com/mstredhat/qemu/-/pipelines/1118886361/failures
> > 
> > I saw the CI test "pages" failed too. On my CI, most of the time, it
> > failed with
> > 
> > $ htags -anT --tree-view=filetree -m qemu_init -t "Welcome to the QEMU
> > sourcecode"
> > 00:24
> > htags: Negative exec line limit = -8099
> > 
> > It only succeeded once. I could not reproduce locally. Do you have any
> > ideas what the problem is?
> 
> I think I briefly understand why pages test fails. Internally, htags call
> global to parse the output of gtags. When executing command, it expects the
> size of argv and env to 20*1024 
> (https://github.com/harai/gnu-global/blob/f86ba74d867385353815f8656c4a6cf4029c1f0b/libutil/xargs.c#L92-L105).
> The failed test case only happens when the last commit is patch 7 (test:
> bios-tables-test: add IVRS changed binary) which has a very long commit
> message (around 9000 bytes). By default, Gitlab appends some environment
> variables to the runner and one of them is CI_COMMIT_MESSAGE which contains
> the long commit message. So it exceeds the limit of 20*1024 bytes and fails.


Thanks for debugging this!
So I think we should clear CI_COMMIT_MESSAGE when invoking htags, right?


> In my opinion, this failed test is not so critical and seems unrelated to
> the series so I skip this failed test.

Yes ok to ignore. But even better if we also add a workaround.

> I will post the new version to fix
> the windows/amd64 build soon.
> 
> Thanks,
> Quang Minh.




[PATCH v2 2/9] hw/hppa/machine: Disable default devices with --nodefaults option

2024-01-07 Thread deller
From: Helge Deller 

Add support for the qemu --nodefaults option, which will disable the
following default devices:
- lsi53c895a SCSI controller,
- artist graphics card,
- LASI 82596 NIC,
- tulip PCI NIC,
- second serial PCI card,
- USB OHCI controller.

Adding this option is very useful to allow manual testing and
debugging of the other possible devices on the command line.

Signed-off-by: Helge Deller 
---
 hw/hppa/machine.c | 15 +--
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/hw/hppa/machine.c b/hw/hppa/machine.c
index b11907617e..8017002a2a 100644
--- a/hw/hppa/machine.c
+++ b/hw/hppa/machine.c
@@ -346,11 +346,14 @@ static void machine_HP_common_init_tail(MachineState 
*machine, PCIBus *pci_bus,
 SysBusDevice *s;
 
 /* SCSI disk setup. */
-dev = DEVICE(pci_create_simple(pci_bus, -1, "lsi53c895a"));
-lsi53c8xx_handle_legacy_cmdline(dev);
+if (defaults_enabled()) {
+dev = DEVICE(pci_create_simple(pci_bus, -1, "lsi53c895a"));
+lsi53c8xx_handle_legacy_cmdline(dev);
+}
 
 /* Graphics setup. */
-if (machine->enable_graphics && vga_interface_type != VGA_NONE) {
+if (defaults_enabled() && machine->enable_graphics &&
+vga_interface_type != VGA_NONE) {
 vga_interface_created = true;
 dev = qdev_new("artist");
 s = SYS_BUS_DEVICE(dev);
@@ -360,7 +363,7 @@ static void machine_HP_common_init_tail(MachineState 
*machine, PCIBus *pci_bus,
 }
 
 /* Network setup. */
-if (enable_lasi_lan()) {
+if (defaults_enabled() && enable_lasi_lan()) {
 lasi_82596_init(addr_space, translate(NULL, LASI_LAN_HPA),
 qdev_get_gpio_in(lasi_dev, LASI_IRQ_LAN_HPA));
 }
@@ -385,7 +388,7 @@ static void machine_HP_common_init_tail(MachineState 
*machine, PCIBus *pci_bus,
 pci_set_word(_dev->config[PCI_SUBSYSTEM_ID], 0x1227); /* Powerbar */
 
 /* create a second serial PCI card when running Astro */
-if (!lasi_dev) {
+if (defaults_enabled() && !lasi_dev) {
 pci_dev = pci_new(-1, "pci-serial-4x");
 qdev_prop_set_chr(DEVICE(pci_dev), "chardev1", serial_hd(1));
 qdev_prop_set_chr(DEVICE(pci_dev), "chardev2", serial_hd(2));
@@ -395,7 +398,7 @@ static void machine_HP_common_init_tail(MachineState 
*machine, PCIBus *pci_bus,
 }
 
 /* create USB OHCI controller for USB keyboard & mouse on Astro machines */
-if (!lasi_dev && machine->enable_graphics) {
+if (defaults_enabled() && !lasi_dev && machine->enable_graphics) {
 pci_create_simple(pci_bus, -1, "pci-ohci");
 usb_create_simple(usb_bus_find(-1), "usb-kbd");
 usb_create_simple(usb_bus_find(-1), "usb-mouse");
-- 
2.43.0




[PATCH v2 3/9] hw/pci-host/astro: Add missing astro & elroy registers for NetBSD

2024-01-07 Thread deller
From: Helge Deller 

NetBSD accesses some astro and elroy registers which aren't accessed
by Linux yet. Add emulation for those registers to allow NetBSD to
boot further.
Please note that this patch is not sufficient to completely boot up
NetBSD on the 64-bit C3700 machine yet.

Signed-off-by: Helge Deller 
---
 hw/pci-host/astro.c | 26 +++---
 1 file changed, 23 insertions(+), 3 deletions(-)

diff --git a/hw/pci-host/astro.c b/hw/pci-host/astro.c
index 7d68ccee7e..cb2c8a828d 100644
--- a/hw/pci-host/astro.c
+++ b/hw/pci-host/astro.c
@@ -166,6 +166,8 @@ static MemTxResult elroy_chip_write_with_attrs(void 
*opaque, hwaddr addr,
 trace_elroy_write(addr, size, val);
 
 switch ((addr >> 3) << 3) {
+case 0x000: /* PCI_ID & PCI_COMMAND_STATUS_REG */
+break;
 case 0x080:
 put_val_in_int64(>arb_mask, addr, size, val);
 break;
@@ -175,6 +177,9 @@ static MemTxResult elroy_chip_write_with_attrs(void 
*opaque, hwaddr addr,
 case 0x200 ... 0x250 - 1:   /* LMMIO, GMMIO, WLMMIO, WGMMIO, ... */
 put_val_in_arrary(s->mmio_base, 0x200, addr, size, val);
 break;
+case 0x300: /* ibase */
+case 0x308: /* imask */
+break;
 case 0x0680:
 put_val_in_int64(>error_config, addr, size, val);
 break;
@@ -538,6 +543,9 @@ static MemTxResult astro_chip_read_with_attrs(void *opaque, 
hwaddr addr,
 case 0x0030:/* HP-UX 10.20 and 11.11 reads it. No idea. */
 val = -1;
 break;
+case 0x0078:/* NetBSD reads 0x78 ? */
+val = -1;
+break;
 case 0x0300 ... 0x03d8: /* LMMIO_DIRECT0_BASE... */
 index = (addr - 0x300) / 8;
 val = s->ioc_ranges[index];
@@ -624,31 +632,43 @@ static MemTxResult astro_chip_write_with_attrs(void 
*opaque, hwaddr addr,
 case 0x10220:
 case 0x10230:/* HP-UX 11.11 reads it. No idea. */
 break;
-case 0x22108:/* IOC STATUS_CONTROL */
-put_val_in_int64(>ioc_status_ctrl, addr, size, val);
-break;
 case 0x20200 ... 0x20240 - 1: /* IOC Rope0_Control ... */
 put_val_in_arrary(s->ioc_rope_control, 0x20200, addr, size, val);
 break;
 case 0x20040:/* IOC Rope config */
+case 0x22040:
 put_val_in_int64(>ioc_rope_config, addr, size, val);
 break;
 case 0x20300:
+case 0x22300:
 put_val_in_int64(>tlb_ibase, addr, size, val);
 break;
 case 0x20308:
+case 0x22308:
 put_val_in_int64(>tlb_imask, addr, size, val);
 break;
 case 0x20310:
+case 0x22310:
 put_val_in_int64(>tlb_pcom, addr, size, val);
 /* TODO: flush iommu */
 break;
 case 0x20318:
+case 0x22318:
 put_val_in_int64(>tlb_tcnfg, addr, size, val);
 break;
 case 0x20320:
+case 0x22320:
 put_val_in_int64(>tlb_pdir_base, addr, size, val);
 break;
+case 0x22000:   /* func_id */
+break;
+case 0x22008:   /* func_class */
+break;
+case 0x22050:   /* rope_debug */
+break;
+case 0x22108:/* IOC STATUS_CONTROL */
+put_val_in_int64(>ioc_status_ctrl, addr, size, val);
+break;
 /*
  * empty placeholders for non-existent elroys, e.g.
  * func_class, pci config & data
-- 
2.43.0




[PATCH v2 1/9] hw/hppa/machine: Allow up to 3840 MB total memory

2024-01-07 Thread deller
From: Helge Deller 

The physical hardware allows DIMMs of 4 MB size and above, allowing up
to 3840 MB of memory, but is restricted by setup code to 3 GB.
Increase the limit to allow up to the maximum amount of memory.

Btw. the memory area from 0xf000. to 0x. is reserved by
the architecture for firmware and I/O memory and can not be used for
standard memory.

An upcoming 64-bit SeaBIOS-hppa firmware will allow more than 3.75GB
on 64-bit HPPA64. In this case the ram_max for the pa20 case will change.

Signed-off-by: Helge Deller 
Noticed-by: Nelson H. F. Beebe 
Fixes: b7746b1194c8 ("hw/hppa/machine: Restrict the total memory size to 3GB")
---
 hw/hppa/machine.c | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/hw/hppa/machine.c b/hw/hppa/machine.c
index c8da7c18d5..b11907617e 100644
--- a/hw/hppa/machine.c
+++ b/hw/hppa/machine.c
@@ -276,6 +276,7 @@ static TranslateFn 
*machine_HP_common_init_cpus(MachineState *machine)
 unsigned int smp_cpus = machine->smp.cpus;
 TranslateFn *translate;
 MemoryRegion *cpu_region;
+uint64_t ram_max;
 
 /* Create CPUs.  */
 for (unsigned int i = 0; i < smp_cpus; i++) {
@@ -288,8 +289,10 @@ static TranslateFn 
*machine_HP_common_init_cpus(MachineState *machine)
  */
 if (hppa_is_pa20([0]->env)) {
 translate = translate_pa20;
+ram_max = 0xf000;  /* 3.75 GB (limited by 32-bit firmware) */
 } else {
 translate = translate_pa10;
+ram_max = 0xf000;  /* 3.75 GB (32-bit CPU) */
 }
 
 for (unsigned int i = 0; i < smp_cpus; i++) {
@@ -311,9 +314,9 @@ static TranslateFn 
*machine_HP_common_init_cpus(MachineState *machine)
 cpu_region);
 
 /* Main memory region. */
-if (machine->ram_size > 3 * GiB) {
-error_report("RAM size is currently restricted to 3GB");
-exit(EXIT_FAILURE);
+if (machine->ram_size > ram_max) {
+info_report("Max RAM size limited to %" PRIu64 " MB", ram_max / MiB);
+machine->ram_size = ram_max;
 }
 memory_region_add_subregion_overlap(addr_space, 0, machine->ram, -1);
 
-- 
2.43.0




[PATCH v2 8/9] target/hppa: Avoid accessing %gr0 when raising exception

2024-01-07 Thread deller
From: Helge Deller 

The value of unwind_breg may reference register %r0, but we need to avoid
accessing gr0 directly and use the value 0 instead.

At runtime I've seen unwind_breg being zero with the Linux kernel when
rfi is used to jump to smp_callin().

Signed-off-by: Helge Deller 
---
 target/hppa/mem_helper.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/hppa/mem_helper.c b/target/hppa/mem_helper.c
index 011b192406..42bd0063c0 100644
--- a/target/hppa/mem_helper.c
+++ b/target/hppa/mem_helper.c
@@ -335,7 +335,7 @@ raise_exception_with_ior(CPUHPPAState *env, int excp, 
uintptr_t retaddr,
 
 cpu_restore_state(cs, retaddr);
 
-b = env->gr[env->unwind_breg];
+b = env->unwind_breg ? env->gr[env->unwind_breg] : 0;
 b >>= (env->psw & PSW_W ? 62 : 30);
 env->cr[CR_IOR] |= b << 62;
 
-- 
2.43.0




[PATCH v2 5/9] target/hppa: Strip upper 32-bits of IOR on error in probe

2024-01-07 Thread deller
From: Helge Deller 

Limit IOR to the lower 32-bits on failure.
Keep patch short for easier backporting.

Signed-off-by: Helge Deller 
---
 target/hppa/op_helper.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/hppa/op_helper.c b/target/hppa/op_helper.c
index 7f607c3afd..60c9014242 100644
--- a/target/hppa/op_helper.c
+++ b/target/hppa/op_helper.c
@@ -353,7 +353,7 @@ target_ulong HELPER(probe)(CPUHPPAState *env, target_ulong 
addr,
 if (excp >= 0) {
 if (env->psw & PSW_Q) {
 /* ??? Needs tweaking for hppa64.  */
-env->cr[CR_IOR] = addr;
+env->cr[CR_IOR] = (uint32_t)addr;
 env->cr[CR_ISR] = addr >> 32;
 }
 if (excp == EXCP_DTLB_MISS) {
-- 
2.43.0




[PATCH v2 7/9] hw/hppa: Move software power button address back into PDC

2024-01-07 Thread deller
From: Helge Deller 

The various operating systems (e.g. Linux, NetBSD) have issues
mapping the power button when it's stored in page zero.
NetBSD even crashes, because it fails to map that page and then
accesses unmapped memory.

Since we now have a consistent memory mapping of PDC in 32-bit
and 64-bit address space (the lower 32-bits of the address are in
sync) the power button can be moved back to PDC space.

This patch fixes the power button on Linux, NetBSD and HP-UX.

Signed-off-by: Helge Deller 
---
 hw/hppa/machine.c | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/hw/hppa/machine.c b/hw/hppa/machine.c
index 8017002a2a..9bf2116934 100644
--- a/hw/hppa/machine.c
+++ b/hw/hppa/machine.c
@@ -36,8 +36,8 @@
 
 #define MIN_SEABIOS_HPPA_VERSION 12 /* require at least this fw version */
 
-/* Power button address at >pad[4] */
-#define HPA_POWER_BUTTON (0x40 + 4 * sizeof(uint32_t))
+#define HPA_POWER_BUTTON(FIRMWARE_END - 0x10)
+static hwaddr soft_power_reg;
 
 #define enable_lasi_lan()   0
 
@@ -45,7 +45,6 @@ static DeviceState *lasi_dev;
 
 static void hppa_powerdown_req(Notifier *n, void *opaque)
 {
-hwaddr soft_power_reg = HPA_POWER_BUTTON;
 uint32_t val;
 
 val = ldl_be_phys(_space_memory, soft_power_reg);
@@ -221,7 +220,7 @@ static FWCfgState *create_fw_cfg(MachineState *ms, PCIBus 
*pci_bus,
 fw_cfg_add_file(fw_cfg, "/etc/hppa/machine",
 g_memdup(mc->name, len), len);
 
-val = cpu_to_le64(HPA_POWER_BUTTON);
+val = cpu_to_le64(soft_power_reg);
 fw_cfg_add_file(fw_cfg, "/etc/hppa/power-button-addr",
 g_memdup(, sizeof(val)), sizeof(val));
 
@@ -295,6 +294,8 @@ static TranslateFn 
*machine_HP_common_init_cpus(MachineState *machine)
 ram_max = 0xf000;  /* 3.75 GB (32-bit CPU) */
 }
 
+soft_power_reg = translate(NULL, HPA_POWER_BUTTON);
+
 for (unsigned int i = 0; i < smp_cpus; i++) {
 g_autofree char *name = g_strdup_printf("cpu%u-io-eir", i);
 
-- 
2.43.0




[PATCH v2 4/9] target/hppa: Fix PDC address translation on PA2.0 with PSW.W=0

2024-01-07 Thread deller
From: Helge Deller 

Fix the address translation for PDC space on PA2.0 if PSW.W=0.
Basically, for any address in the 32-bit PDC range from 0xf000 to
0xf100 keep the lower 32-bits and just set the upper 32-bits to
0xfff0.

This mapping fixes the emulated power button in PDC space for 32- and
64-bit machines and is how the physical C3700 machine seems to map
PDC.

Signed-off-by: Helge Deller 
---
 target/hppa/mem_helper.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/hppa/mem_helper.c b/target/hppa/mem_helper.c
index 08abd1a9f9..011b192406 100644
--- a/target/hppa/mem_helper.c
+++ b/target/hppa/mem_helper.c
@@ -56,7 +56,7 @@ hwaddr hppa_abs_to_phys_pa2_w0(vaddr addr)
 addr = (int32_t)addr;
 } else {
 /* PDC address space */
-addr &= MAKE_64BIT_MASK(0, 24);
+addr = (uint32_t)addr;
 addr |= -1ull << (TARGET_PHYS_ADDR_SPACE_BITS - 4);
 }
 return addr;
-- 
2.43.0




[PATCH v2 0/9] target/hppa qemu v8.2 regression fixes

2024-01-07 Thread deller
From: Helge Deller 

There were some regressions introduced with Qemu v8.2 on the hppa/hppa64
target, e.g.:

- 32-bit HP-UX crashes on B160L (32-bit) machine
- NetBSD boot failure due to power button in page zero
- NetBSD FPU detection failure
- OpenBSD 7.4 boot failure

This small patch series fixes those known regressions and
additionally:

- allows usage of the max. 3840MB of memory (instead of 3GB),
- adds support for the qemu --nodefaults option (to debug other devices)

I tried to keep the patches small to make backporting easier.

This patch set will not fix those known (non-regression) bugs:
- HP-UX and NetBSD still fail to boot on the new 64-bit C3700 machine
- Linux kernel will still fail to boot on C3700 as long as kernel modules are 
used.

The whole series can be pulled from the "hppa-fixes-8.2" branch from:
https://github.com/hdeller/qemu-hppa.githppa-fixes-8.2

Changes v1->v2:
- fix OpenBSD boot with SeaBIOS v15 instead of v14
- commit message enhancements suggested by BALATON Zoltan
- use uint64_t for ram_max in patch #1

Helge Deller (9):
  hw/hppa/machine: Allow up to 3840 MB total memory
  hw/hppa/machine: Disable default devices with --nodefaults option
  hw/pci-host/astro: Add missing astro & elroy registers for NetBSD
  target/hppa: Fix PDC address translation on PA2.0 with PSW.W=0
  target/hppa: Strip upper 32-bits of IOR on error in probe
  target/hppa: Strip upper 32-bits of IOR on unaligned access error
  hw/hppa: Move software power button address back into PDC
  target/hppa: Avoid accessing %gr0 when raising exception
  target/hppa: Update SeaBIOS-hppa to version 15

 hw/hppa/machine.c |  33 -
 hw/pci-host/astro.c   |  26 +++---
 pc-bios/hppa-firmware.img | Bin 681388 -> 163324 bytes
 roms/seabios-hppa |   2 +-
 target/hppa/cpu.c |   2 +-
 target/hppa/mem_helper.c  |   4 ++--
 target/hppa/op_helper.c   |   2 +-
 7 files changed, 48 insertions(+), 21 deletions(-)

-- 
2.43.0




[PATCH v2 6/9] target/hppa: Strip upper 32-bits of IOR on unaligned access error

2024-01-07 Thread deller
From: Helge Deller 

Limit IOR to the lower 32-bits on failure.
Keep patch short for easier backporting.

Signed-off-by: Helge Deller 
---
 target/hppa/cpu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/hppa/cpu.c b/target/hppa/cpu.c
index 04de1689d7..3c384d5855 100644
--- a/target/hppa/cpu.c
+++ b/target/hppa/cpu.c
@@ -112,7 +112,7 @@ void hppa_cpu_do_unaligned_access(CPUState *cs, vaddr addr,
 cs->exception_index = EXCP_UNALIGN;
 if (env->psw & PSW_Q) {
 /* ??? Needs tweaking for hppa64.  */
-env->cr[CR_IOR] = addr;
+env->cr[CR_IOR] = (uint32_t)addr;
 env->cr[CR_ISR] = addr >> 32;
 }
 
-- 
2.43.0




Re: [PULL 00/26] Migration 20240104 patches

2024-01-07 Thread Stefan Hajnoczi
On Sun, 7 Jan 2024 at 07:34, Peter Xu  wrote:
>
> On Fri, Jan 05, 2024 at 04:08:40PM +, Peter Maydell wrote:
> > I notice that your gpg key doesn't seem to be signed by anybody
> > else; you might look at whether it's easy to get it signed
> > by somebody else (eg some of your redhat colleagues).
>
> Hmm, I think I have signed with at least Juan and Stefan.  Which is the key
> server we normally use?  Maybe I missed some steps there?

Yes, Peter's key is signed by me:

$ gpg --list-signatures 3B5FCCCDF3ABD706
pub   ed25519/0x3B5FCCCDF3ABD706 2023-10-03 [SC]
  Key fingerprint = B918 4DC2 0CC4 57DA CF7D  D1A9 3B5F CCCD F3AB D706
uid   [  full  ] Peter Xu 
sig 30x3B5FCCCDF3ABD706 2023-10-03  [self-signature]
sig  0x9CA4ABB381AB73C8 2023-10-10  Stefan Hajnoczi

uid   [  full  ] Peter Xu 
sig 30x3B5FCCCDF3ABD706 2023-10-03  [self-signature]
sig  0x9CA4ABB381AB73C8 2023-10-10  Stefan Hajnoczi

sub   cv25519/0xD5261EB1CB0C6E45 2023-10-03 [E]
sig  0x3B5FCCCDF3ABD706 2023-10-03  [self-signature]

I have pushed to the keyservers again in case I forget.

Stefan



Re: [PULL 00/26] Migration 20240104 patches

2024-01-07 Thread Peter Xu
On Fri, Jan 05, 2024 at 04:08:40PM +, Peter Maydell wrote:
> I notice that your gpg key doesn't seem to be signed by anybody
> else; you might look at whether it's easy to get it signed
> by somebody else (eg some of your redhat colleagues).

Hmm, I think I have signed with at least Juan and Stefan.  Which is the key
server we normally use?  Maybe I missed some steps there?

Thanks,

-- 
Peter Xu




Re: [PATCH RESEND v3 04/10] crypto: Introduce creation option and structure for detached LUKS header

2024-01-07 Thread Yong Huang
On Thu, Jan 4, 2024 at 10:51 PM Daniel P. Berrangé 
wrote:

> On Mon, Dec 25, 2023 at 01:45:06PM +0800, Hyman Huang wrote:
> > Introduce 'header' field in BlockdevCreateOptionsLUKS to support
> > detached LUKS header creation. Meanwhile, introduce header-related
> > field in QCryptoBlock.
> >
> > Signed-off-by: Hyman Huang 
> > ---
> >  crypto/blockpriv.h   | 3 +++
> >  qapi/block-core.json | 3 +++
> >  qapi/crypto.json | 5 -
> >  3 files changed, 10 insertions(+), 1 deletion(-)
> >
> > diff --git a/crypto/blockpriv.h b/crypto/blockpriv.h
> > index 3c7ccea504..6289aea961 100644
> > --- a/crypto/blockpriv.h
> > +++ b/crypto/blockpriv.h
> > @@ -42,6 +42,9 @@ struct QCryptoBlock {
> >  size_t niv;
> >  uint64_t payload_offset; /* In bytes */
> >  uint64_t sector_size; /* In bytes */
> > +
> > +bool detached_header; /* True if disk has a detached LUKS header */
> > +uint64_t detached_header_size; /* LUKS header size plus key slot
> size */
>
> This field can be replaced by a local variable I believe.
>
> >  };
> >
> >  struct QCryptoBlockDriver {
> > diff --git a/qapi/block-core.json b/qapi/block-core.json
> > index 9ac256c489..8aec179926 100644
> > --- a/qapi/block-core.json
> > +++ b/qapi/block-core.json
> > @@ -4948,6 +4948,8 @@
> >  # @file: Node to create the image format on, mandatory except when
> >  #'preallocation' is not requested
> >  #
> > +# @header: Detached LUKS header node to format. (since 9.0)
> > +#
> >  # @size: Size of the virtual disk in bytes
> >  #
> >  # @preallocation: Preallocation mode for the new image (since: 4.2)
> > @@ -4958,6 +4960,7 @@
> >  { 'struct': 'BlockdevCreateOptionsLUKS',
> >'base': 'QCryptoBlockCreateOptionsLUKS',
> >'data': { '*file':'BlockdevRef',
> > +'*header':  'BlockdevRef',
> >  'size': 'size',
> >  '*preallocation':   'PreallocMode' } }
> >
> > diff --git a/qapi/crypto.json b/qapi/crypto.json
> > index fd3d46ebd1..6b4e86cb81 100644
> > --- a/qapi/crypto.json
> > +++ b/qapi/crypto.json
> > @@ -195,10 +195,13 @@
> >  # decryption key.  Mandatory except when probing image for
> >  # metadata only.
> >  #
> > +# @detached-header: if true, disk has detached LUKS header.
> > +#
> >  # Since: 2.6
> >  ##
> >  { 'struct': 'QCryptoBlockOptionsLUKS',
> > -  'data': { '*key-secret': 'str' }}
> > +  'data': { '*key-secret': 'str',
> > +'*detached-header': 'bool' }}
>
> I don't think we need this change if we pass this info as an enum flag
>

Agree.


> >
> >  ##
> >  # @QCryptoBlockCreateOptionsLUKS:
> > --
> > 2.39.1
> >
>
> With regards,
> Daniel
> --
> |: https://berrange.com  -o-
> https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org -o-
> https://fstop138.berrange.com :|
> |: https://entangle-photo.org-o-
> https://www.instagram.com/dberrange :|
>
>

-- 
Best regards


Re: [PATCH RESEND v3 02/10] crypto: Support generic LUKS encryption

2024-01-07 Thread Yong Huang
On Thu, Jan 4, 2024 at 10:40 PM Daniel P. Berrangé 
wrote:

> On Mon, Dec 25, 2023 at 01:45:04PM +0800, Hyman Huang wrote:
> > By enhancing the LUKS driver, it is possible to enable
> > the detachable LUKS header and, as a result, achieve
> > general encryption for any disk format that QEMU has
> > supported.
> >
> > Take the qcow2 as an example, the usage of the generic
> > LUKS encryption as follows:
> >
> > 1. add a protocol blockdev node of data disk
> > $ virsh qemu-monitor-command vm '{"execute":"blockdev-add",
> > > "arguments":{"node-name":"libvirt-1-storage", "driver":"file",
> > > "filename":"/path/to/test_disk.qcow2"}}'
> >
> > 2. add a protocol blockdev node of LUKS header as above.
> > $ virsh qemu-monitor-command vm '{"execute":"blockdev-add",
> > > "arguments":{"node-name":"libvirt-2-storage", "driver":"file",
> > > "filename": "/path/to/cipher.gluks" }}'
> >
> > 3. add the secret for decrypting the cipher stored in LUKS
> >header above
> > $ virsh qemu-monitor-command vm '{"execute":"object-add",
> > > "arguments":{"qom-type":"secret", "id":
> > > "libvirt-2-storage-secret0", "data":"abc123"}}'
> >
> > 4. add the qcow2-drived blockdev format node
> > $ virsh qemu-monitor-command vm '{"execute":"blockdev-add",
> > > "arguments":{"node-name":"libvirt-1-format", "driver":"qcow2",
> > > "file":"libvirt-1-storage"}}'
> >
> > 5. add the luks-drived blockdev to link the qcow2 disk with
> >LUKS header by specifying the field "header"
> > $ virsh qemu-monitor-command vm '{"execute":"blockdev-add",
> > > "arguments":{"node-name":"libvirt-2-format", "driver":"luks",
> > > "file":"libvirt-1-format", "header":"libvirt-2-storage",
> > > "key-secret":"libvirt-2-format-secret0"}}'
> >
> > 6. add the virtio-blk device finally
> > $ virsh qemu-monitor-command vm '{"execute":"device_add",
> > > "arguments": {"num-queues":"1", "driver":"virtio-blk-pci",
> > > "drive": "libvirt-2-format", "id":"virtio-disk2"}}'
> >
> > The generic LUKS encryption method of starting a virtual
> > machine (VM) is somewhat similar to hot-plug in that both
> > maintaining the same json command while the starting VM
> > changes the "blockdev-add/device_add" parameters to
> > "blockdev/device".
> >
> > Signed-off-by: Hyman Huang 
> > Message-Id: <
> 910801f303da1601051479d3b7e5c2c6b4e01eb7.1701879996.git.yong.hu...@smartx.com
> >
> > ---
> >  block/crypto.c | 14 +-
> >  1 file changed, 13 insertions(+), 1 deletion(-)
> >
> > diff --git a/block/crypto.c b/block/crypto.c
> > index f82b13d32b..6063879bac 100644
> > --- a/block/crypto.c
> > +++ b/block/crypto.c
> > @@ -64,12 +64,14 @@ static int block_crypto_read_func(QCryptoBlock
> *block,
> >Error **errp)
> >  {
> >  BlockDriverState *bs = opaque;
> > +BlockCrypto *crypto = bs->opaque;
> >  ssize_t ret;
> >
> >  GLOBAL_STATE_CODE();
> >  GRAPH_RDLOCK_GUARD_MAINLOOP();
> >
> > -ret = bdrv_pread(bs->file, offset, buflen, buf, 0);
> > +ret = bdrv_pread(crypto->header ? crypto->header : bs->file,
> > + offset, buflen, buf, 0);
> >  if (ret < 0) {
> >  error_setg_errno(errp, -ret, "Could not read encryption
> header");
> >  return ret;
> > @@ -269,6 +271,7 @@ static int
> block_crypto_open_generic(QCryptoBlockFormat format,
> >  QCryptoBlockOpenOptions *open_opts = NULL;
> >  unsigned int cflags = 0;
> >  QDict *cryptoopts = NULL;
> > +const char *hdr_bdref = qdict_get_try_str(options, "header");
>
> This is an invalid check to make, because it is assuming the user is
> referencing a separate blockdev node name and doesn't work for an
> inline definition. eg
>
>   qemu-img info
> 'json:{"driver":"luks","file":{"filename":"test-payload.img"},"header":{"filename":"test-header.img"}}'
>
>
> >
> >  GLOBAL_STATE_CODE();
> >
> > @@ -277,6 +280,15 @@ static int
> block_crypto_open_generic(QCryptoBlockFormat format,
> >  return ret;
> >  }
> >
> > +if (hdr_bdref) {
>
> Get rid of this 'if' clause and unconditionally call the next line:
>
> > +crypto->header = bdrv_open_child(NULL, options, "header", bs,
> > + _of_bds,
> BDRV_CHILD_METADATA,
> > + false, errp);
>
> but pass 'true' instead of 'false' here to allow the child to be absent,
> and thus let it return NULL.
>
> > +if (!crypto->header) {
>
> You'll need to then check  '*errp != NULL' instead
>
> You'll also need "ERRP_GUARD" at the start of the method
>
> > +return -EINVAL;
> > +}
> > +}
> > +
> >  GRAPH_RDLOCK_GUARD_MAINLOOP();
> >
> >  bs->supported_write_flags = BDRV_REQ_FUA &
>
> This patch should be combined with the previous patch that adds the new
> QAPI schema element as splitting them doesn't add value.
>
>
> Testing this patch with the changes I suggest above, however, still does
> not work:
>
>   $ dd if=/dev/zero of=test-header.img bs=1M 

[PATCH v2 1/2] i386/sev: Sort the error message

2024-01-07 Thread Hyman Huang
Prior to giving the caller the return number(in the next commit),
sorting the error message:
1. report the error number on the ram_block_discard_disable
   failure path
2. report the error number on the syscall "open" failure path
3. report EINVAL when a prerequisite check fails or the command
   line is invalid

Signed-off-by: Hyman Huang 
---
 target/i386/sev.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/target/i386/sev.c b/target/i386/sev.c
index 9a71246682..96eff73001 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -923,7 +923,7 @@ int sev_kvm_init(ConfidentialGuestSupport *cgs, Error 
**errp)
 ret = ram_block_discard_disable(true);
 if (ret) {
 error_report("%s: cannot disable RAM discard", __func__);
-return -1;
+return ret;
 }
 
 sev_guest = sev;
@@ -940,6 +940,7 @@ int sev_kvm_init(ConfidentialGuestSupport *cgs, Error 
**errp)
 if (host_cbitpos != sev->cbitpos) {
 error_setg(errp, "%s: cbitpos check failed, host '%d' requested '%d'",
__func__, host_cbitpos, sev->cbitpos);
+ret = -EINVAL;
 goto err;
 }
 
@@ -952,11 +953,12 @@ int sev_kvm_init(ConfidentialGuestSupport *cgs, Error 
**errp)
 error_setg(errp, "%s: reduced_phys_bits check failed,"
" it should be in the range of 1 to 63, requested '%d'",
__func__, sev->reduced_phys_bits);
+ret = -EINVAL;
 goto err;
 }
 
 devname = object_property_get_str(OBJECT(sev), "sev-device", NULL);
-sev->sev_fd = open(devname, O_RDWR);
+ret = sev->sev_fd = open(devname, O_RDWR);
 if (sev->sev_fd < 0) {
 error_setg(errp, "%s: Failed to open %s '%s'", __func__,
devname, strerror(errno));
@@ -981,6 +983,7 @@ int sev_kvm_init(ConfidentialGuestSupport *cgs, Error 
**errp)
 if (!kvm_kernel_irqchip_allowed()) {
 error_report("%s: SEV-ES guests require in-kernel irqchip support",
  __func__);
+ret = -EINVAL;
 goto err;
 }
 
@@ -988,6 +991,7 @@ int sev_kvm_init(ConfidentialGuestSupport *cgs, Error 
**errp)
 error_report("%s: guest policy requires SEV-ES, but "
  "host SEV-ES support unavailable",
  __func__);
+ret = -EINVAL;
 goto err;
 }
 cmd = KVM_SEV_ES_INIT;
-- 
2.39.1




[PATCH v2 0/2] Nitpick at the error message's output

2024-01-07 Thread Hyman Huang
v2:
- rebase on master
- add a commit to sort the error message so that an explanation
  error number can be returned on all failure paths

Hyman Huang (2):
  i386/sev: Sort the error message
  i386/sev: Nitpick at the error message's output

 target/i386/sev.c | 10 +++---
 1 file changed, 7 insertions(+), 3 deletions(-)

-- 
2.39.1




[PATCH v2 2/2] i386/sev: Nitpick at the error message's output

2024-01-07 Thread Hyman Huang
The incorrect error message was produced as a result of
the return number being disregarded on the sev_kvm_init
failure path.

For instance, when a user's failure to launch a SEV guest
is caused by an incorrect IOCTL, the following message is
reported:

kvm: sev_kvm_init: failed to initialize ret=-25 fw_error=0
kvm: failed to initialize kvm: Operation not permitted

While the error message's accurate output should be:

kvm: sev_kvm_init: failed to initialize ret=-25 fw_error=0
kvm: failed to initialize kvm: Inappropriate ioctl for device

Fix this by returning the return number directly on the
failure path.

Signed-off-by: Hyman Huang 
Reviewed-by: Daniel P. Berrangé 
Message-Id: 

---
 target/i386/sev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/i386/sev.c b/target/i386/sev.c
index 96eff73001..3fef8cf163 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -1023,7 +1023,7 @@ int sev_kvm_init(ConfidentialGuestSupport *cgs, Error 
**errp)
 err:
 sev_guest = NULL;
 ram_block_discard_disable(false);
-return -1;
+return ret;
 }
 
 int
-- 
2.39.1




Re: [PATCH] i386/sev: Nitpick at the error message's output

2024-01-07 Thread Yong Huang
On Sat, Jan 6, 2024 at 12:43 AM Alex Bennée  wrote:

> Hyman Huang  writes:
>
> > The incorrect error message was produced as a result of
> > the return number being disregarded on the sev_kvm_init
> > failure path.
> >
> > For instance, when a user's failure to launch a SEV guest
> > is caused by an incorrect IOCTL, the following message is
> > reported:
> >
> > kvm: sev_kvm_init: failed to initialize ret=-25 fw_error=0
> > kvm: failed to initialize kvm: Operation not permitted
> >
> > While the error message's accurate output should be:
> >
> > kvm: sev_kvm_init: failed to initialize ret=-25 fw_error=0
> > kvm: failed to initialize kvm: Inappropriate ioctl for device
> >
> > Fix this by returning the return number directly on the
> > failure path.
> >
> > Signed-off-by: Hyman Huang 
> > ---
> >  target/i386/sev.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/target/i386/sev.c b/target/i386/sev.c
> > index 9a71246682..4a69ca457c 100644
> > --- a/target/i386/sev.c
> > +++ b/target/i386/sev.c
> > @@ -1019,7 +1019,7 @@ int sev_kvm_init(ConfidentialGuestSupport *cgs,
> Error **errp)
> >  err:
> >  sev_guest = NULL;
> >  ram_block_discard_disable(false);
> > -return -1;
> > +return ret;
>
> I don't think this will be correct in all cases because there are other
> legs (e.g. if (host_cbitpos != sev->cbitpos)) where ret may be the
> successful setting of ram_block_discard_disable(true).
>

Indeed, the other failed paths are overlooked by me. I'll try a
commit aiming to sort the error message on all failure paths in
the next version.


> You might want to explore if the function can be re-written with
> explicit return's and utilise autofree to do the clean-up of dynamic
> objects.
>
> I think this entails setting sev_guest at the end of the function just
> before the return 0.
>
> I'm not sure if there is a clean way to handle
> ram_block_discard_disable(false); cleanly for all the failure legs
> though. Maybe someone with more familiarity with the code has some ideas?
>
>
> >  }
> >
> >  int
>
> --
> Alex Bennée
> Virtualisation Tech Lead @ Linaro
>


-- 
Best regards


[PATCH trivial] colo: examples: remove mentions of script= and (wrong) downscript=

2024-01-07 Thread Michael Tokarev
There's no need to repeat script=/etc/qemu-ifup in examples,
as it is already in there.  More, all examples uses incorrect
"down script=" (which should be "downscript=").
---
I'm not sure we need so many identical examples, and why it
uses vnet=off, - it looks like vnet= should also be dropped.

 docs/colo-proxy.txt | 6 +++---
 qemu-options.hx | 8 
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/docs/colo-proxy.txt b/docs/colo-proxy.txt
index 1fc38aed1b..e712c883db 100644
--- a/docs/colo-proxy.txt
+++ b/docs/colo-proxy.txt
@@ -162,7 +162,7 @@ Here is an example using demonstration IP and port 
addresses to more
 clearly describe the usage.
 
 Primary(ip:3.3.3.3):
--netdev tap,id=hn0,vhost=off,script=/etc/qemu-ifup,downscript=/etc/qemu-ifdown
+-netdev tap,id=hn0,vhost=off
 -device e1000,id=e0,netdev=hn0,mac=52:a4:00:12:78:66
 -chardev socket,id=mirror0,host=3.3.3.3,port=9003,server=on,wait=off
 -chardev socket,id=compare1,host=3.3.3.3,port=9004,server=on,wait=off
@@ -177,7 +177,7 @@ Primary(ip:3.3.3.3):
 -object 
colo-compare,id=comp0,primary_in=compare0-0,secondary_in=compare1,outdev=compare_out0,iothread=iothread1
 
 Secondary(ip:3.3.3.8):
--netdev tap,id=hn0,vhost=off,script=/etc/qemu-ifup,down script=/etc/qemu-ifdown
+-netdev tap,id=hn0,vhost=off
 -device e1000,netdev=hn0,mac=52:a4:00:12:78:66
 -chardev socket,id=red0,host=3.3.3.3,port=9003
 -chardev socket,id=red1,host=3.3.3.3,port=9004
@@ -202,7 +202,7 @@ Primary(ip:3.3.3.3):
 -object 
colo-compare,id=comp0,primary_in=compare0-0,secondary_in=compare1,outdev=compare_out0,vnet_hdr_support
 
 Secondary(ip:3.3.3.8):
--netdev tap,id=hn0,vhost=off,script=/etc/qemu-ifup,down script=/etc/qemu-ifdown
+-netdev tap,id=hn0,vhost=off
 -device e1000,netdev=hn0,mac=52:a4:00:12:78:66
 -chardev socket,id=red0,host=3.3.3.3,port=9003
 -chardev socket,id=red1,host=3.3.3.3,port=9004
diff --git a/qemu-options.hx b/qemu-options.hx
index b66570ae00..d667bfa0c2 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -5500,7 +5500,7 @@ SRST
 KVM COLO
 
 primary:
--netdev 
tap,id=hn0,vhost=off,script=/etc/qemu-ifup,downscript=/etc/qemu-ifdown
+-netdev tap,id=hn0,vhost=off
 -device e1000,id=e0,netdev=hn0,mac=52:a4:00:12:78:66
 -chardev 
socket,id=mirror0,host=3.3.3.3,port=9003,server=on,wait=off
 -chardev 
socket,id=compare1,host=3.3.3.3,port=9004,server=on,wait=off
@@ -5515,7 +5515,7 @@ SRST
 -object 
colo-compare,id=comp0,primary_in=compare0-0,secondary_in=compare1,outdev=compare_out0,iothread=iothread1
 
 secondary:
--netdev tap,id=hn0,vhost=off,script=/etc/qemu-ifup,down 
script=/etc/qemu-ifdown
+-netdev tap,id=hn0,vhost=off
 -device e1000,netdev=hn0,mac=52:a4:00:12:78:66
 -chardev socket,id=red0,host=3.3.3.3,port=9003
 -chardev socket,id=red1,host=3.3.3.3,port=9004
@@ -5526,7 +5526,7 @@ SRST
 Xen COLO
 
 primary:
--netdev 
tap,id=hn0,vhost=off,script=/etc/qemu-ifup,downscript=/etc/qemu-ifdown
+-netdev tap,id=hn0,vhost=off
 -device e1000,id=e0,netdev=hn0,mac=52:a4:00:12:78:66
 -chardev 
socket,id=mirror0,host=3.3.3.3,port=9003,server=on,wait=off
 -chardev 
socket,id=compare1,host=3.3.3.3,port=9004,server=on,wait=off
@@ -5542,7 +5542,7 @@ SRST
 -object 
colo-compare,id=comp0,primary_in=compare0-0,secondary_in=compare1,outdev=compare_out0,notify_dev=nofity_way,iothread=iothread1
 
 secondary:
--netdev tap,id=hn0,vhost=off,script=/etc/qemu-ifup,down 
script=/etc/qemu-ifdown
+-netdev tap,id=hn0,vhost=off
 -device e1000,netdev=hn0,mac=52:a4:00:12:78:66
 -chardev socket,id=red0,host=3.3.3.3,port=9003
 -chardev socket,id=red1,host=3.3.3.3,port=9004
-- 
2.39.2




Re: [PATCH] tcg/tci: Fix TCI on hppa host and update TCI test matrix

2024-01-07 Thread Philippe Mathieu-Daudé

Cc'ing Akihiko for commit a1eaa6281f.

On 7/1/24 08:19, Helge Deller wrote:

Update the TCI interpreter test matrix for big-endian hosts with
big- (hppa, hppa64) and little-endian (x86,x96-64) targets.
I used native ppc64 and hppa hosts for those tests.

Starting TCI on a hppa host crashed immediately, because hppa is
the only archive left where the stack grows upwards.
Write-protecting the stack guard page at the top of the stack
fixes the crash.



Fixes: a1eaa6281f ("util: Delete checks for old host definitions")


Signed-off-by: Helge Deller 

diff --git a/tcg/tci/README b/tcg/tci/README
index 4a8b5b5401..0c1e50779e 100644
--- a/tcg/tci/README
+++ b/tcg/tci/README
@@ -72,16 +72,16 @@ host and target with same or different endianness.
  | host (le) host (be)
  | 32 64 32 64
  +
-target (le) | s0, u0 s1, u1 s?, u? s?, u?
+target (le) | s0, u0 s1, u1 s2, u? s2, u?
  32 bit  |
  |
-target (le) | sc, uc s1, u1 s?, u? s?, u?
+target (le) | sc, uc s1, u1 s2, u? s2, u?
  64 bit  |
  |
-target (be) | sc, u0 sc, uc s?, u? s?, u?
+target (be) | sc, u0 sc, uc s2, u? s2, u?
  32 bit  |
  |
-target (be) | sc, uc sc, uc s?, u? s?, u?
+target (be) | sc, uc sc, uc s?, u? s2, u?
  64 bit  |
  |
  
@@ -110,6 +115,10 @@ u1 = linux-user-test works

A cross compiled QEMU for ppc host works at least partially:
i386-linux-user/qemu-i386 can run a simple hello-world program
(tested in a ppc emulation).
+  The big-endian tests were run on native hppa (big-endian, 32-bit) and
+  ppc64 (big-endian, 64-bit) machines. Tested target machines were
+  x86 and x86-64 (little-endian, debian install ISO) and 32- and 64-bit
+  big-endian hppa (NetBSD and Debian install ISOs).
  
  * Some TCG opcodes are either missing in the code generator and/or

in the interpreter. These opcodes raise a runtime exception, so it is
diff --git a/util/oslib-posix.c b/util/oslib-posix.c
index e86fd64e09..e378b71641 100644
--- a/util/oslib-posix.c
+++ b/util/oslib-posix.c
@@ -585,11 +585,8 @@ char *qemu_get_pid_name(pid_t pid)
  
  void *qemu_alloc_stack(size_t *sz)

  {
-void *ptr;
+void *ptr, *ptr2;
  int flags;
-#ifdef CONFIG_DEBUG_STACK_USAGE
-void *ptr2;
-#endif
  size_t pagesz = qemu_real_host_page_size();
  #ifdef _SC_THREAD_STACK_MIN
  /* avoid stacks smaller than _SC_THREAD_STACK_MIN */
@@ -619,7 +616,12 @@ void *qemu_alloc_stack(size_t *sz)
  }
  
  /* Stack grows down -- guard page at the bottom. */

-if (mprotect(ptr, pagesz, PROT_NONE) != 0) {
+ptr2 = ptr;
+#if defined(__hppa__)


Is it worth make this generic by declaring some TARGET_STACK_GROWS_UP
definition in target/foo/cpu-param.h?


+/* but on hppa the stack grows up, so guard the top page instead */
+ptr2 = ptr + *sz - pagesz;
+#endif
+if (mprotect(ptr2, pagesz, PROT_NONE) != 0) {
  perror("failed to set up stack guard page");
  abort();
  }