Re: [PATCH v2 5/9] vfio: Improve error reporting when MMIO region mapping fails

[Philippe Mathieu-Daudé]

On 30/1/25 14:43, Cédric Le Goater wrote:

When the IOMMU address space width is smaller than the physical
address width, a MMIO region of a device can fail to map because the
region is outside the supported IOVA ranges of the VM. In this case,
PCI peer-to-peer transactions on BARs are not supported.

This can occur with the 39-bit IOMMU address space width, as can be
the case on some consumer processors or when using a vIOMMU device
with default settings. The current error message is unclear, also
change the error report to a warning because it is a non fatal
condition for the VM.

Signed-off-by: Cédric Le Goater <c...@redhat.com>
---
  hw/vfio/common.c | 17 ++++++++++++++++-
  1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/hw/vfio/common.c b/hw/vfio/common.c
index 
62af1216fc5a9089fc718c2afe3a405d9381db32..5c9d8657d746ce30af5ae8f9122101e086a61ef5
 100644
--- a/hw/vfio/common.c
+++ b/hw/vfio/common.c
@@ -555,6 +555,18 @@ static bool vfio_get_section_iova_range(VFIOContainerBase 
*bcontainer,
      return true;
  }

+static void vfio_device_error_append(VFIODevice *vbasedev, Error **errp)

+{
+    /*
+     * MMIO region mapping failures are not fatal but in this case PCI
+     * peer-to-peer transactions are broken.
+     */
+    if (vbasedev && vbasedev->type == VFIO_DEVICE_TYPE_PCI) {
+        error_append_hint(errp, "%s: PCI peer-to-peer transactions "
+                          "on BARs are not supported.\n", vbasedev->name);
+    }
+}
+
  static void vfio_listener_region_add(MemoryListener *listener,
                                       MemoryRegionSection *section)
  {
@@ -670,7 +682,10 @@ static void vfio_listener_region_add(MemoryListener 
*listener,
                     strerror(-ret));
          if (memory_region_is_ram_device(section->mr)) {
              /* Allow unexpected mappings not to be fatal for RAM devices */
-            error_report_err(err);
+            VFIODevice *vbasedev =
+                vfio_get_vfio_device(memory_region_owner(section->mr));
+            vfio_device_error_append(vbasedev, &err);

Having vfio_get_vfio_device() returning NULL and
vfio_device_error_append() also checking for NULL is odd.

Maybe just inline everything here?

+            warn_report_once_err(err);
              return;
          }
          goto fail;