[PATCH v3 0/2] vfio/spapr: Fix L2 crash with PCI device passthrough

[PATCH v3 0/2] vfio/spapr: Fix L2 crash with PCI device passthrough

[Amit Machhiwal]

Hi,

This patch series addresses two aspects in vfio_spapr_create_window() that
includes the enhancement in error handling in this function and also fixes an
issue with KVM guests (L2) inside a pSeries logical partition (LPAR) with larger
memory configurations and PCI device passthrough.

The structure of the patch series is as below:

1. The first patch introduces structured error reporting in
   `vfio_spapr_create_window()` by adding an `Error **` parameter. This allows
   better propagation of failure details to the caller.

2. The second patch fixes a crash observed when booting an L2 KVM guest inside a
   pSeries LPAR with memory more than 128 GB and PCI device passthrough. The
   crash occurs due a check in KVM preventing multi-level TCEs because of not
   being supported by PowerVM hypervisor. This patch ensures that such
   configurations are avoided by first checking the supported number of levels
   returned by the `VFIO_IOMMU_SPAPR_TCE_GET_INFO` ioctl.

The fix has been tested with KVM guests on PowerVM and bare-metal enviroments
with memory sizes up to 390 GB and 450 GB respectively.

Thanks,
Amit

Changes in v3:
  * Change vfio_spapr_create_window() to return bool
  * Replace error_setg() with error_setg_errono() in vfio_spapr_create_window()
  * Pass errp instead of local Error object in vfio_spapr_create_window()
    while calling from vfio_spapr_add_section_window()
  * Modified patch #2 subject

Changes in v2:
  * Link: https://lore.kernel.org/all/20250407143119.1304513-1-amachhiw@linux.ibm.com/
  * Added Error ** parameter in vfio_spapr_create_window() for enhanced error
    handling with error_setg() and friends

v1: https://lore.kernel.org/all/20250404091721.2653539-1-amachhiw@linux.ibm.com/

Amit Machhiwal (2):
  vfio/spapr: Enhance error handling in vfio_spapr_create_window()
  vfio/spapr: Fix L2 crash with PCI device passthrough and memory > 128G

 hw/vfio/spapr.c | 69 ++++++++++++++++++++++++++++++-------------------
 1 file changed, 43 insertions(+), 26 deletions(-)


base-commit: dfaecc04c46d298e9ee81bd0ca96d8754f1c27ed
-- 
2.49.0

[PATCH v3 1/2] vfio/spapr: Enhance error handling in vfio_spapr_create_window()

[Amit Machhiwal]

Introduce an Error ** parameter to vfio_spapr_create_window() to enable
structured error reporting. This allows the function to propagate
detailed errors back to callers.

Suggested-by: Cédric Le Goater <clg@redhat.com>
Signed-off-by: Amit Machhiwal <amachhiw@linux.ibm.com>
---
 hw/vfio/spapr.c | 33 ++++++++++++++++-----------------
 1 file changed, 16 insertions(+), 17 deletions(-)

diff --git a/hw/vfio/spapr.c b/hw/vfio/spapr.c
index 1a5d1611f2cd..dd9207679dbe 100644
--- a/hw/vfio/spapr.c
+++ b/hw/vfio/spapr.c
@@ -230,9 +230,9 @@ static int vfio_spapr_remove_window(VFIOContainer *container,
     return 0;
 }
 
-static int vfio_spapr_create_window(VFIOContainer *container,
+static bool vfio_spapr_create_window(VFIOContainer *container,
                                     MemoryRegionSection *section,
-                                    hwaddr *pgsize)
+                                    hwaddr *pgsize, Error **errp)
 {
     int ret = 0;
     VFIOContainerBase *bcontainer = &container->bcontainer;
@@ -252,11 +252,11 @@ static int vfio_spapr_create_window(VFIOContainer *container,
     pgmask = bcontainer->pgsizes & (pagesize | (pagesize - 1));
     pagesize = pgmask ? (1ULL << (63 - clz64(pgmask))) : 0;
     if (!pagesize) {
-        error_report("Host doesn't support page size 0x%"PRIx64
-                     ", the supported mask is 0x%lx",
-                     memory_region_iommu_get_min_page_size(iommu_mr),
-                     bcontainer->pgsizes);
-        return -EINVAL;
+        error_setg_errno(errp, EINVAL, "Host doesn't support page size 0x%"PRIx64
+                         ", the supported mask is 0x%lx",
+                         memory_region_iommu_get_min_page_size(iommu_mr),
+                         bcontainer->pgsizes);
+        return false;
     }
 
     /*
@@ -302,17 +302,17 @@ static int vfio_spapr_create_window(VFIOContainer *container,
         }
     }
     if (ret) {
-        error_report("Failed to create a window, ret = %d (%m)", ret);
-        return -errno;
+        error_setg_errno(errp, errno, "Failed to create a window, ret = %d", ret);
+        return false;
     }
 
     if (create.start_addr != section->offset_within_address_space) {
         vfio_spapr_remove_window(container, create.start_addr);
 
-        error_report("Host doesn't support DMA window at %"HWADDR_PRIx", must be %"PRIx64,
-                     section->offset_within_address_space,
-                     (uint64_t)create.start_addr);
-        return -EINVAL;
+        error_setg_errno(errp, EINVAL, "Host doesn't support DMA window at %"HWADDR_PRIx
+                         ", must be %"PRIx64, section->offset_within_address_space,
+                         (uint64_t)create.start_addr);
+        return false;
     }
     trace_vfio_spapr_create_window(create.page_shift,
                                    create.levels,
@@ -320,7 +320,7 @@ static int vfio_spapr_create_window(VFIOContainer *container,
                                    create.start_addr);
     *pgsize = pagesize;
 
-    return 0;
+    return true;
 }
 
 static bool
@@ -377,9 +377,8 @@ vfio_spapr_container_add_section_window(VFIOContainerBase *bcontainer,
         }
     }
 
-    ret = vfio_spapr_create_window(container, section, &pgsize);
-    if (ret) {
-        error_setg_errno(errp, -ret, "Failed to create SPAPR window");
+    ret = vfio_spapr_create_window(container, section, &pgsize, errp);
+    if (!ret) {
         return false;
     }
 
-- 
2.49.0

[PATCH v3 2/2] vfio/spapr: Fix L2 crash with PCI device passthrough and 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 | 36 +++++++++++++++++++++++++++---------
 1 file changed, 27 insertions(+), 9 deletions(-)

diff --git a/hw/vfio/spapr.c b/hw/vfio/spapr.c
index dd9207679dbe..32611096fa29 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 bool 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,29 @@ static bool 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_setg_errno(errp, EINVAL, "Host doesn't support multi-level TCE tables"
+                             ". Use larger IO page size. Supported mask is 0x%lx",
+                             bcontainer->pgsizes);
+            return false;
+        }
+        ret = ioctl(container->fd, VFIO_IOMMU_SPAPR_TCE_CREATE, &create);
     }
+
     if (ret) {
         error_setg_errno(errp, errno, "Failed to create a window, ret = %d", ret);
         return false;
@@ -501,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;
         /*
-- 
2.49.0

Re: [PATCH v3 1/2] vfio/spapr: Enhance error handling in vfio_spapr_create_window()

[Cédric Le Goater]

On 4/8/25 14:40, Amit Machhiwal wrote:
> Introduce an Error ** parameter to vfio_spapr_create_window() to enable
> structured error reporting. This allows the function to propagate
> detailed errors back to callers.
> 
> Suggested-by: Cédric Le Goater <clg@redhat.com>
> Signed-off-by: Amit Machhiwal <amachhiw@linux.ibm.com>
> --->   hw/vfio/spapr.c | 33 ++++++++++++++++-----------------
>   1 file changed, 16 insertions(+), 17 deletions(-)
> 
> diff --git a/hw/vfio/spapr.c b/hw/vfio/spapr.c
> index 1a5d1611f2cd..dd9207679dbe 100644
> --- a/hw/vfio/spapr.c
> +++ b/hw/vfio/spapr.c
> @@ -230,9 +230,9 @@ static int vfio_spapr_remove_window(VFIOContainer *container,
>       return 0;
>   }
>   
> -static int vfio_spapr_create_window(VFIOContainer *container,
> +static bool vfio_spapr_create_window(VFIOContainer *container,
>                                       MemoryRegionSection *section,
> -                                    hwaddr *pgsize)
> +                                    hwaddr *pgsize, Error **errp)
>   {
>       int ret = 0;
>       VFIOContainerBase *bcontainer = &container->bcontainer;
> @@ -252,11 +252,11 @@ static int vfio_spapr_create_window(VFIOContainer *container,
>       pgmask = bcontainer->pgsizes & (pagesize | (pagesize - 1));
>       pagesize = pgmask ? (1ULL << (63 - clz64(pgmask))) : 0;
>       if (!pagesize) {
> -        error_report("Host doesn't support page size 0x%"PRIx64
> -                     ", the supported mask is 0x%lx",
> -                     memory_region_iommu_get_min_page_size(iommu_mr),
> -                     bcontainer->pgsizes);
> -        return -EINVAL;
> +        error_setg_errno(errp, EINVAL, "Host doesn't support page size 0x%"PRIx64
> +                         ", the supported mask is 0x%lx",
> +                         memory_region_iommu_get_min_page_size(iommu_mr),
> +                         bcontainer->pgsizes);
> +        return false;
>       }
>   
>       /*
> @@ -302,17 +302,17 @@ static int vfio_spapr_create_window(VFIOContainer *container,
>           }
>       }
>       if (ret) {
> -        error_report("Failed to create a window, ret = %d (%m)", ret);
> -        return -errno;
> +        error_setg_errno(errp, errno, "Failed to create a window, ret = %d", ret);
> +        return false;
>       }
>   
>       if (create.start_addr != section->offset_within_address_space) {
>           vfio_spapr_remove_window(container, create.start_addr);
>   
> -        error_report("Host doesn't support DMA window at %"HWADDR_PRIx", must be %"PRIx64,
> -                     section->offset_within_address_space,
> -                     (uint64_t)create.start_addr);
> -        return -EINVAL;
> +        error_setg_errno(errp, EINVAL, "Host doesn't support DMA window at %"HWADDR_PRIx
> +                         ", must be %"PRIx64, section->offset_within_address_space,
> +                         (uint64_t)create.start_addr);
> +        return false;
>       }
>       trace_vfio_spapr_create_window(create.page_shift,
>                                      create.levels,
> @@ -320,7 +320,7 @@ static int vfio_spapr_create_window(VFIOContainer *container,
>                                      create.start_addr);
>       *pgsize = pagesize;
>   
> -    return 0;
> +    return true;
>   }
>   
>   static bool
> @@ -377,9 +377,8 @@ vfio_spapr_container_add_section_window(VFIOContainerBase *bcontainer,
>           }
>       }
>   
> -    ret = vfio_spapr_create_window(container, section, &pgsize);
> -    if (ret) {
> -        error_setg_errno(errp, -ret, "Failed to create SPAPR window");
> +    ret = vfio_spapr_create_window(container, section, &pgsize, errp);
> +    if (!ret) {
>           return false;
>       }
>   

ret is not needed. Minor.


Reviewed-by: Cédric Le Goater <clg@redhat.com>

Thanks,

C.

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

[Cédric Le Goater]

On 4/8/25 14:40, 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>


Reviewed-by: Cédric Le Goater <clg@redhat.com>

Thanks,

C.


> ---
>   hw/vfio/spapr.c | 36 +++++++++++++++++++++++++++---------
>   1 file changed, 27 insertions(+), 9 deletions(-)
> 
> diff --git a/hw/vfio/spapr.c b/hw/vfio/spapr.c
> index dd9207679dbe..32611096fa29 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 bool 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,29 @@ static bool 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_setg_errno(errp, EINVAL, "Host doesn't support multi-level TCE tables"
> +                             ". Use larger IO page size. Supported mask is 0x%lx",
> +                             bcontainer->pgsizes);
> +            return false;
> +        }
> +        ret = ioctl(container->fd, VFIO_IOMMU_SPAPR_TCE_CREATE, &create);
>       }
> +
>       if (ret) {
>           error_setg_errno(errp, errno, "Failed to create a window, ret = %d", ret);
>           return false;
> @@ -501,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;
>           /*

Re: [PATCH v3 0/2] vfio/spapr: Fix L2 crash with PCI device passthrough

[Cédric Le Goater]

On 4/8/25 14:40, Amit Machhiwal wrote:
> Hi,
> 
> This patch series addresses two aspects in vfio_spapr_create_window() that
> includes the enhancement in error handling in this function and also fixes an
> issue with KVM guests (L2) inside a pSeries logical partition (LPAR) with larger
> memory configurations and PCI device passthrough.
> 
> The structure of the patch series is as below:
> 
> 1. The first patch introduces structured error reporting in
>     `vfio_spapr_create_window()` by adding an `Error **` parameter. This allows
>     better propagation of failure details to the caller.
> 
> 2. The second patch fixes a crash observed when booting an L2 KVM guest inside a
>     pSeries LPAR with memory more than 128 GB and PCI device passthrough. The
>     crash occurs due a check in KVM preventing multi-level TCEs because of not
>     being supported by PowerVM hypervisor. This patch ensures that such
>     configurations are avoided by first checking the supported number of levels
>     returned by the `VFIO_IOMMU_SPAPR_TCE_GET_INFO` ioctl.
> 
> The fix has been tested with KVM guests on PowerVM and bare-metal enviroments
> with memory sizes up to 390 GB and 450 GB respectively.
> 
> Thanks,
> Amit
> 
> Changes in v3:
>    * Change vfio_spapr_create_window() to return bool
>    * Replace error_setg() with error_setg_errono() in vfio_spapr_create_window()
>    * Pass errp instead of local Error object in vfio_spapr_create_window()
>      while calling from vfio_spapr_add_section_window()
>    * Modified patch #2 subject
> 
> Changes in v2:
>    * Link: https://lore.kernel.org/all/20250407143119.1304513-1-amachhiw@linux.ibm.com/
>    * Added Error ** parameter in vfio_spapr_create_window() for enhanced error
>      handling with error_setg() and friends
> 
> v1: https://lore.kernel.org/all/20250404091721.2653539-1-amachhiw@linux.ibm.com/
> 
> Amit Machhiwal (2):
>    vfio/spapr: Enhance error handling in vfio_spapr_create_window()
>    vfio/spapr: Fix L2 crash with PCI device passthrough and memory > 128G
> 
>   hw/vfio/spapr.c | 69 ++++++++++++++++++++++++++++++-------------------
>   1 file changed, 43 insertions(+), 26 deletions(-)
> 
> 
> base-commit: dfaecc04c46d298e9ee81bd0ca96d8754f1c27ed


Applied to vfio-next.

Thanks,

C.