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

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

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 <clg@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;