On 5/22/24 18:15, Shivaprasad G Bhat wrote:
On 5/13/24 17:53, Cédric Le Goater wrote:
Hello Shivaprasad,
On 5/9/24 21:14, Shivaprasad G Bhat wrote:
The commit 6ad359ec29 "(vfio/spapr: Move prereg_listener into
spapr container)" began to use the newly introduced VFIOSpaprContainer
structure.
After several refactors, today the container_of(container,
VFIOSpaprContainer, ABC) is used when VFIOSpaprContainer is actually
not allocated. On PPC64 systems, this dereference is leading to corruption
showing up as glibc malloc assertion during guest start when using vfio.
Patch adds the missing allocation while also making the structure movement
to vfio common header file.
Fixes: 6ad359ec29 "(vfio/spapr: Move prereg_listener into spapr container)"
Signed-off-by: Shivaprasad G Bhat <sb...@linux.ibm.com>
---
hw/vfio/container.c | 6 ++++--
hw/vfio/spapr.c | 6 ------
include/hw/vfio/vfio-common.h | 6 ++++++
3 files changed, 10 insertions(+), 8 deletions(-)
diff --git a/hw/vfio/container.c b/hw/vfio/container.c
index 77bdec276e..ecaf5786d9 100644
--- a/hw/vfio/container.c
+++ b/hw/vfio/container.c
@@ -539,6 +539,7 @@ static int vfio_connect_container(VFIOGroup *group,
AddressSpace *as,
{
VFIOContainer *container;
VFIOContainerBase *bcontainer;
+ VFIOSpaprContainer *scontainer;
We should do our best to avoid any direct use of ppc related attributes
in the common VFIO code. This comment also applies to VFIO_SPAPR_TCE*
which are still there because the clean up is not finished. So, this
proposal will have to be reworked.
Sure.
The first step is to finish the QOMification of VFIOContainer, so
that the VFIOContainer instance is created in vfio_connect_container()
with :
container = qdev_new(iommu_type_name);
This requires the VFIOContainer to be a DeviceState object.
The existing base class TYPE_VFIO_IOMMU is an InterfaceClass.
I attempted VFIOContainer object declaration with TYPE_VFIO_IOMMU,
like
OBJECT_DECLARE_SIMPLE_TYPE(VFIOContainer, VFIO_IOMMU_LEGACY)
This means reworking this part (and vfio_set_iommu()) :
...
container = g_malloc0(sizeof(*container));
container->fd = fd;
bcontainer = &container->bcontainer;
if (!vfio_set_iommu(container, group->fd, space, errp)) {
goto free_container_exit;
}
...
VFIOSpaprContainer can then implement its own .init_instance() handler
to allocate/initialize attributes required by the pseries machines.
With my above changes,
I see the instance_init() is not supported for the InterfaceClass with the
Yes. We need an Object, hence my remark on "QOMification of VFIOContainer".
VFIOContainerBase needs to be reworked.
Thanks,
C.
checks from below
commit 422ca1432f7b44f2a9f3ad94a65d36927da021fa
Author: Marc-André Lureau <marcandre.lur...@redhat.com>
Date: Wed Sep 12 16:53:03 2018 +0400
qom/object: add some interface asserts
Did you suggest me something else?
Thank you,
Shivaprasad
While doing this, please try to reduce the use of ->iommu_type which
is a design shortcut. I would like to completely remove it at some
point.
Thanks,
C.
int ret, fd;
VFIOAddressSpace *space;
@@ -611,7 +612,8 @@ static int vfio_connect_container(VFIOGroup *group,
AddressSpace *as,
goto close_fd_exit;
}
- container = g_malloc0(sizeof(*container));
+ scontainer = g_malloc0(sizeof(*scontainer));
+ container = &scontainer->container;
container->fd = fd;
bcontainer = &container->bcontainer;
@@ -675,7 +677,7 @@ unregister_container_exit:
vfio_cpr_unregister_container(bcontainer);
free_container_exit:
- g_free(container);
+ g_free(scontainer);
close_fd_exit:
close(fd);
diff --git a/hw/vfio/spapr.c b/hw/vfio/spapr.c
index 0d949bb728..78d218b7e7 100644
--- a/hw/vfio/spapr.c
+++ b/hw/vfio/spapr.c
@@ -24,12 +24,6 @@
#include "qapi/error.h"
#include "trace.h"
-typedef struct VFIOSpaprContainer {
- VFIOContainer container;
- MemoryListener prereg_listener;
- QLIST_HEAD(, VFIOHostDMAWindow) hostwin_list;
-} VFIOSpaprContainer;
-
static bool vfio_prereg_listener_skipped_section(MemoryRegionSection *section)
{
if (memory_region_is_iommu(section->mr)) {
diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
index b9da6c08ef..010fa68ac6 100644
--- a/include/hw/vfio/vfio-common.h
+++ b/include/hw/vfio/vfio-common.h
@@ -82,6 +82,12 @@ typedef struct VFIOContainer {
QLIST_HEAD(, VFIOGroup) group_list;
} VFIOContainer;
+typedef struct VFIOSpaprContainer {
+ VFIOContainer container;
+ MemoryListener prereg_listener;
+ QLIST_HEAD(, VFIOHostDMAWindow) hostwin_list;
+} VFIOSpaprContainer;
+
typedef struct VFIOHostDMAWindow {
hwaddr min_iova;
hwaddr max_iova;
