Re: [Qemu-devel] [PATCH 1/2] vfio: warn if host device rom can't be read

[Alex Williamson]

On Tue, 2014-01-14 at 21:45 +0530, Bandan Das wrote:
> If the device rom can't be read, report an error to the
> user. The guest might try to read the rom contents more than
> once, so introduce macros that print a message only once and
> not clutter up the console. This is to alert the user
> that the device has a bad state that is causing rom read
> failure or option rom loading has been disabled from the device
> boot menu (among other reasons).
> 
> Signed-off-by: Bandan Das <address@hidden>
> ---
>  hw/misc/vfio.c              |  7 +++++++
>  include/qemu/error-report.h | 20 ++++++++++++++++++++
>  2 files changed, 27 insertions(+)
> 
> diff --git a/hw/misc/vfio.c b/hw/misc/vfio.c
> index 9aecaa8..e5b2826 100644
> --- a/hw/misc/vfio.c
> +++ b/hw/misc/vfio.c
> @@ -1125,6 +1125,13 @@ static void vfio_pci_load_rom(VFIODevice *vdev)
>      vdev->rom_offset = reg_info.offset;
>  
>      if (!vdev->rom_size) {
> +        error_report_once("vfio-pci: Cannot read device rom at "
> +                    "%04x:%02x:%02x.%x\n",
> +                    vdev->host.domain, vdev->host.bus, vdev->host.slot,
> +                    vdev->host.function);
> +        error_printf_once("Device option ROM contents are probably invalid "
> +                    "(check dmesg).\nSkip option ROM probe with rombar=0, "
> +                    "or load from file with romfile=\n");
>          return;
>      }
>  
> diff --git a/include/qemu/error-report.h b/include/qemu/error-report.h
> index 3b098a9..7d24e4c 100644
> --- a/include/qemu/error-report.h
> +++ b/include/qemu/error-report.h
> @@ -43,4 +43,24 @@ void error_report(const char *fmt, ...) GCC_FMT_ATTR(1, 2);
>  const char *error_get_progname(void);
>  extern bool enable_timestamp_msg;
>  
> +#define error_printf_once(fmt, ...)             \
> +({                                              \
> +        static bool __printf_once;              \
> +                                                \
> +        if (!__printf_once) {                   \
> +            __printf_once = true;               \
> +            error_printf(fmt, ##__VA_ARGS__);   \
> +        }                                       \
> +})                                              \
> +
> +#define error_report_once(fmt, ...)             \
> +({                                              \
> +        static bool __report_once;              \
> +                                                \
> +        if (!__report_once) {                   \
> +            __report_once = true;               \
> +            error_report(fmt, ##__VA_ARGS__);   \
> +        }                                       \
> +})                                              \
> +
>  #endif

Why do we need these if patch 2/2 comes along and only calls
vfio_pci_load_rom() once?  If we do need these, they should be a
separate patch.  Thanks,

Alex