[PATCH] vfio/spapr: Fix L2 crash with PCI device passthrough with L2 guest memory > 128G

[PATCH] vfio/spapr: Fix L2 crash with PCI device passthrough with L2 guest memory > 128G

[Amit Machhiwal]

An L2 KVM guest fails to boot inside a pSeries LPAR when booted with a
memory more than 128 GB and PCI device passthrough. The L2 guest also
crashes when it is booted with a memory greater than 128 GB and a PCI
device is hotplugged later.

The issue arises from a conditional check for `levels > 1` in
`spapr_tce_create_table()` within L1 KVM. This check is meant to prevent
multi-level TCEs, which are not supported by the PowerVM hypervisor. As
a result, when QEMU makes a `VFIO_IOMMU_SPAPR_TCE_CREATE` ioctl call
with `levels > 1`, it triggers the conditional check and returns
`EINVAL`, causing the guest to crash with the following errors:

 2025-03-04T06:36:36.133117Z qemu-system-ppc64: Failed to create a window, ret = -1 (Invalid argument)
 2025-03-04T06:36:36.133176Z qemu-system-ppc64: Failed to create SPAPR window: Invalid argument
 qemu: hardware error: vfio: DMA mapping failed, unable to continue

Fix this by checking the supported DDW "levels" returned by the
VFIO_IOMMU_SPAPR_TCE_GET_INFO ioctl before attempting the TCE create
ioctl in KVM.

The patch has been tested on KVM guests with memory configurations of up
to 390GB, and 450GB on PowerVM and bare-metal environments respectively.

Signed-off-by: Amit Machhiwal <amachhiw@linux.ibm.com>
---
 hw/vfio/spapr.c | 35 ++++++++++++++++++++++++++---------
 1 file changed, 26 insertions(+), 9 deletions(-)

diff --git a/hw/vfio/spapr.c b/hw/vfio/spapr.c
index 1a5d1611f2cd..07498218fea9 100644
--- a/hw/vfio/spapr.c
+++ b/hw/vfio/spapr.c
@@ -26,6 +26,7 @@ typedef struct VFIOSpaprContainer {
     VFIOContainer container;
     MemoryListener prereg_listener;
     QLIST_HEAD(, VFIOHostDMAWindow) hostwin_list;
+    unsigned int levels;
 } VFIOSpaprContainer;
 
 OBJECT_DECLARE_SIMPLE_TYPE(VFIOSpaprContainer, VFIO_IOMMU_SPAPR);
@@ -236,9 +237,11 @@ static int vfio_spapr_create_window(VFIOContainer *container,
 {
     int ret = 0;
     VFIOContainerBase *bcontainer = &container->bcontainer;
+    VFIOSpaprContainer *scontainer = container_of(container, VFIOSpaprContainer,
+                                                  container);
     IOMMUMemoryRegion *iommu_mr = IOMMU_MEMORY_REGION(section->mr);
     uint64_t pagesize = memory_region_iommu_get_min_page_size(iommu_mr), pgmask;
-    unsigned entries, bits_total, bits_per_level, max_levels;
+    unsigned entries, bits_total, bits_per_level, max_levels, ddw_levels;
     struct vfio_iommu_spapr_tce_create create = { .argsz = sizeof(create) };
     long rampagesize = qemu_minrampagesize();
 
@@ -291,16 +294,28 @@ static int vfio_spapr_create_window(VFIOContainer *container,
      */
     bits_per_level = ctz64(qemu_real_host_page_size()) + 8;
     create.levels = bits_total / bits_per_level;
-    if (bits_total % bits_per_level) {
-        ++create.levels;
-    }
-    max_levels = (64 - create.page_shift) / ctz64(qemu_real_host_page_size());
-    for ( ; create.levels <= max_levels; ++create.levels) {
-        ret = ioctl(container->fd, VFIO_IOMMU_SPAPR_TCE_CREATE, &create);
-        if (!ret) {
-            break;
+
+    ddw_levels = scontainer->levels;
+    if (ddw_levels > 1) {
+        if (bits_total % bits_per_level) {
+            ++create.levels;
         }
+        max_levels = (64 - create.page_shift) / ctz64(qemu_real_host_page_size());
+        for ( ; create.levels <= max_levels; ++create.levels) {
+            ret = ioctl(container->fd, VFIO_IOMMU_SPAPR_TCE_CREATE, &create);
+            if (!ret) {
+                break;
+            }
+        }
+    } else { /* ddw_levels == 1 */
+        if (create.levels > ddw_levels) {
+            error_report("Host doesn't support multi-level TCE tables. "
+                         "Use larger IO page size. Supported mask is 0x%lx",
+                         bcontainer->pgsizes);
+        }
+        ret = ioctl(container->fd, VFIO_IOMMU_SPAPR_TCE_CREATE, &create);
     }
+
     if (ret) {
         error_report("Failed to create a window, ret = %d (%m)", ret);
         return -errno;
@@ -502,6 +517,8 @@ static bool vfio_spapr_container_setup(VFIOContainerBase *bcontainer,
         goto listener_unregister_exit;
     }
 
+    scontainer->levels = info.ddw.levels;
+
     if (v2) {
         bcontainer->pgsizes = info.ddw.pgsizes;
         /*

base-commit: 0adf626718bc0ca9c46550249a76047f8e45da15
-- 
2.49.0

Re: [PATCH] vfio/spapr: Fix L2 crash with PCI device passthrough with L2 guest memory > 128G

[Cédric Le Goater]

On 4/4/25 11:17, Amit Machhiwal wrote:
> An L2 KVM guest fails to boot inside a pSeries LPAR when booted with a
> memory more than 128 GB and PCI device passthrough. The L2 guest also
> crashes when it is booted with a memory greater than 128 GB and a PCI
> device is hotplugged later.
> 
> The issue arises from a conditional check for `levels > 1` in
> `spapr_tce_create_table()` within L1 KVM. This check is meant to prevent
> multi-level TCEs, which are not supported by the PowerVM hypervisor. As
> a result, when QEMU makes a `VFIO_IOMMU_SPAPR_TCE_CREATE` ioctl call
> with `levels > 1`, it triggers the conditional check and returns
> `EINVAL`, causing the guest to crash with the following errors:
> 
>   2025-03-04T06:36:36.133117Z qemu-system-ppc64: Failed to create a window, ret = -1 (Invalid argument)
>   2025-03-04T06:36:36.133176Z qemu-system-ppc64: Failed to create SPAPR window: Invalid argument
>   qemu: hardware error: vfio: DMA mapping failed, unable to continue
> 
> Fix this by checking the supported DDW "levels" returned by the
> VFIO_IOMMU_SPAPR_TCE_GET_INFO ioctl before attempting the TCE create
> ioctl in KVM.
> 
> The patch has been tested on KVM guests with memory configurations of up
> to 390GB, and 450GB on PowerVM and bare-metal environments respectively.
> > Signed-off-by: Amit Machhiwal <amachhiw@linux.ibm.com>
> ---
>   hw/vfio/spapr.c | 35 ++++++++++++++++++++++++++---------
>   1 file changed, 26 insertions(+), 9 deletions(-)
> 
> diff --git a/hw/vfio/spapr.c b/hw/vfio/spapr.c
> index 1a5d1611f2cd..07498218fea9 100644
> --- a/hw/vfio/spapr.c
> +++ b/hw/vfio/spapr.c
> @@ -26,6 +26,7 @@ typedef struct VFIOSpaprContainer {
>       VFIOContainer container;
>       MemoryListener prereg_listener;
>       QLIST_HEAD(, VFIOHostDMAWindow) hostwin_list;
> +    unsigned int levels;
>   } VFIOSpaprContainer;
>   
>   OBJECT_DECLARE_SIMPLE_TYPE(VFIOSpaprContainer, VFIO_IOMMU_SPAPR);
> @@ -236,9 +237,11 @@ static int vfio_spapr_create_window(VFIOContainer *container,
>   {
>       int ret = 0;
>       VFIOContainerBase *bcontainer = &container->bcontainer;
> +    VFIOSpaprContainer *scontainer = container_of(container, VFIOSpaprContainer,
> +                                                  container);
>       IOMMUMemoryRegion *iommu_mr = IOMMU_MEMORY_REGION(section->mr);
>       uint64_t pagesize = memory_region_iommu_get_min_page_size(iommu_mr), pgmask;
> -    unsigned entries, bits_total, bits_per_level, max_levels;
> +    unsigned entries, bits_total, bits_per_level, max_levels, ddw_levels;
>       struct vfio_iommu_spapr_tce_create create = { .argsz = sizeof(create) };
>       long rampagesize = qemu_minrampagesize();
>   
> @@ -291,16 +294,28 @@ static int vfio_spapr_create_window(VFIOContainer *container,
>        */
>       bits_per_level = ctz64(qemu_real_host_page_size()) + 8;
>       create.levels = bits_total / bits_per_level;
> -    if (bits_total % bits_per_level) {
> -        ++create.levels;
> -    }
> -    max_levels = (64 - create.page_shift) / ctz64(qemu_real_host_page_size());
> -    for ( ; create.levels <= max_levels; ++create.levels) {
> -        ret = ioctl(container->fd, VFIO_IOMMU_SPAPR_TCE_CREATE, &create);
> -        if (!ret) {
> -            break;
> +
> +    ddw_levels = scontainer->levels;
> +    if (ddw_levels > 1) {
> +        if (bits_total % bits_per_level) {
> +            ++create.levels;
>           }
> +        max_levels = (64 - create.page_shift) / ctz64(qemu_real_host_page_size());
> +        for ( ; create.levels <= max_levels; ++create.levels) {
> +            ret = ioctl(container->fd, VFIO_IOMMU_SPAPR_TCE_CREATE, &create);
> +            if (!ret) {
> +                break;
> +            }
> +        }
> +    } else { /* ddw_levels == 1 */
> +        if (create.levels > ddw_levels) {
> +            error_report("Host doesn't support multi-level TCE tables. "
> +                         "Use larger IO page size. Supported mask is 0x%lx",
> +                         bcontainer->pgsizes);

While at it, please modify vfio_spapr_create_window(), add an 'Error **'
parameter to report errors to the caller with error_setg(errp ...)

Thanks,

C.




> +        }
> +        ret = ioctl(container->fd, VFIO_IOMMU_SPAPR_TCE_CREATE, &create);
>       }
> +
>       if (ret) {
>           error_report("Failed to create a window, ret = %d (%m)", ret);
>           return -errno;
> @@ -502,6 +517,8 @@ static bool vfio_spapr_container_setup(VFIOContainerBase *bcontainer,
>           goto listener_unregister_exit;
>       }
>   
> +    scontainer->levels = info.ddw.levels;
> +
>       if (v2) {
>           bcontainer->pgsizes = info.ddw.pgsizes;
>           /*
> 
> base-commit: 0adf626718bc0ca9c46550249a76047f8e45da15

Re: [PATCH] vfio/spapr: Fix L2 crash with PCI device passthrough with L2 guest memory > 128G

[Amit Machhiwal]

Hi Cédric,

Thanks for looking into this patch. Please find my response inline:

On 2025/04/04 01:29 PM, Cédric Le Goater wrote:
> On 4/4/25 11:17, Amit Machhiwal wrote:
> > An L2 KVM guest fails to boot inside a pSeries LPAR when booted with a
> > memory more than 128 GB and PCI device passthrough. The L2 guest also
> > crashes when it is booted with a memory greater than 128 GB and a PCI
> > device is hotplugged later.
> > 
> > The issue arises from a conditional check for `levels > 1` in
> > `spapr_tce_create_table()` within L1 KVM. This check is meant to prevent
> > multi-level TCEs, which are not supported by the PowerVM hypervisor. As
> > a result, when QEMU makes a `VFIO_IOMMU_SPAPR_TCE_CREATE` ioctl call
> > with `levels > 1`, it triggers the conditional check and returns
> > `EINVAL`, causing the guest to crash with the following errors:
> > 
> >   2025-03-04T06:36:36.133117Z qemu-system-ppc64: Failed to create a window, ret = -1 (Invalid argument)
> >   2025-03-04T06:36:36.133176Z qemu-system-ppc64: Failed to create SPAPR window: Invalid argument
> >   qemu: hardware error: vfio: DMA mapping failed, unable to continue
> > 
> > Fix this by checking the supported DDW "levels" returned by the
> > VFIO_IOMMU_SPAPR_TCE_GET_INFO ioctl before attempting the TCE create
> > ioctl in KVM.
> > 
> > The patch has been tested on KVM guests with memory configurations of up
> > to 390GB, and 450GB on PowerVM and bare-metal environments respectively.
> > > Signed-off-by: Amit Machhiwal <amachhiw@linux.ibm.com>
> > ---
> >   hw/vfio/spapr.c | 35 ++++++++++++++++++++++++++---------
> >   1 file changed, 26 insertions(+), 9 deletions(-)
> > 
> > diff --git a/hw/vfio/spapr.c b/hw/vfio/spapr.c
> > index 1a5d1611f2cd..07498218fea9 100644
> > --- a/hw/vfio/spapr.c
> > +++ b/hw/vfio/spapr.c
> > @@ -26,6 +26,7 @@ typedef struct VFIOSpaprContainer {
> >       VFIOContainer container;
> >       MemoryListener prereg_listener;
> >       QLIST_HEAD(, VFIOHostDMAWindow) hostwin_list;
> > +    unsigned int levels;
> >   } VFIOSpaprContainer;
> >   OBJECT_DECLARE_SIMPLE_TYPE(VFIOSpaprContainer, VFIO_IOMMU_SPAPR);
> > @@ -236,9 +237,11 @@ static int vfio_spapr_create_window(VFIOContainer *container,
> >   {
> >       int ret = 0;
> >       VFIOContainerBase *bcontainer = &container->bcontainer;
> > +    VFIOSpaprContainer *scontainer = container_of(container, VFIOSpaprContainer,
> > +                                                  container);
> >       IOMMUMemoryRegion *iommu_mr = IOMMU_MEMORY_REGION(section->mr);
> >       uint64_t pagesize = memory_region_iommu_get_min_page_size(iommu_mr), pgmask;
> > -    unsigned entries, bits_total, bits_per_level, max_levels;
> > +    unsigned entries, bits_total, bits_per_level, max_levels, ddw_levels;
> >       struct vfio_iommu_spapr_tce_create create = { .argsz = sizeof(create) };
> >       long rampagesize = qemu_minrampagesize();
> > @@ -291,16 +294,28 @@ static int vfio_spapr_create_window(VFIOContainer *container,
> >        */
> >       bits_per_level = ctz64(qemu_real_host_page_size()) + 8;
> >       create.levels = bits_total / bits_per_level;
> > -    if (bits_total % bits_per_level) {
> > -        ++create.levels;
> > -    }
> > -    max_levels = (64 - create.page_shift) / ctz64(qemu_real_host_page_size());
> > -    for ( ; create.levels <= max_levels; ++create.levels) {
> > -        ret = ioctl(container->fd, VFIO_IOMMU_SPAPR_TCE_CREATE, &create);
> > -        if (!ret) {
> > -            break;
> > +
> > +    ddw_levels = scontainer->levels;
> > +    if (ddw_levels > 1) {
> > +        if (bits_total % bits_per_level) {
> > +            ++create.levels;
> >           }
> > +        max_levels = (64 - create.page_shift) / ctz64(qemu_real_host_page_size());
> > +        for ( ; create.levels <= max_levels; ++create.levels) {
> > +            ret = ioctl(container->fd, VFIO_IOMMU_SPAPR_TCE_CREATE, &create);
> > +            if (!ret) {
> > +                break;
> > +            }
> > +        }
> > +    } else { /* ddw_levels == 1 */
> > +        if (create.levels > ddw_levels) {
> > +            error_report("Host doesn't support multi-level TCE tables. "
> > +                         "Use larger IO page size. Supported mask is 0x%lx",
> > +                         bcontainer->pgsizes);
> 
> While at it, please modify vfio_spapr_create_window(), add an 'Error **'
> parameter to report errors to the caller with error_setg(errp ...)

Sure, I'll include the suggested changes and send a v2 soon.

Thanks,
Amit

> 
> Thanks,
> 
> C.
> 
> 
> 
> 
> > +        }
> > +        ret = ioctl(container->fd, VFIO_IOMMU_SPAPR_TCE_CREATE, &create);
> >       }
> > +
> >       if (ret) {
> >           error_report("Failed to create a window, ret = %d (%m)", ret);
> >           return -errno;
> > @@ -502,6 +517,8 @@ static bool vfio_spapr_container_setup(VFIOContainerBase *bcontainer,
> >           goto listener_unregister_exit;
> >       }
> > +    scontainer->levels = info.ddw.levels;
> > +
> >       if (v2) {
> >           bcontainer->pgsizes = info.ddw.pgsizes;
> >           /*
> > 
> > base-commit: 0adf626718bc0ca9c46550249a76047f8e45da15
>