Re: [QEMU][PATCH v4 1/2] xen_arm: Create virtio-mmio devices during initialization

[Anthony PERARD <anthony.perard@citrix.com>]

Hi Vikram,

This patch prevent QEMU from been build with Xen 4.15. See comments.

Also, why didn't you CC all the maintainers of
include/hw/xen/xen_native.h?

On Tue, Aug 29, 2023 at 09:35:17PM -0700, Vikram Garhwal wrote:
> diff --git a/include/hw/xen/xen_native.h b/include/hw/xen/xen_native.h
> index 4dce905fde..a4b1aa9e5d 100644
> --- a/include/hw/xen/xen_native.h
> +++ b/include/hw/xen/xen_native.h
> @@ -523,4 +523,20 @@ static inline int xen_set_ioreq_server_state(domid_t dom,
>                                                   enable);
>  }
>  
> +#if CONFIG_XEN_CTRL_INTERFACE_VERSION <= 41500

xendevicemodel_set_irq_level() was introduced in Xen 4.15, so this
should say '<' and not '<=', otherwise, we have:
    include/hw/xen/xen_native.h:527:19: error: static declaration of ‘xendevicemodel_set_irq_level’ follows non-static declaration

> +static inline int xendevicemodel_set_irq_level(xendevicemodel_handle *dmod,
> +                                               domid_t domid, uint32_t irq,
> +                                               unsigned int level)
> +{
> +    return 0;

Shouldn't this return something like -ENOSYS, instead of returning a
success?

> diff --git a/hw/arm/xen_arm.c b/hw/arm/xen_arm.c
> index 1d3e6d481a..7393b37355 100644
> --- a/hw/arm/xen_arm.c
> +++ b/hw/arm/xen_arm.c
> +
> +static void xen_set_irq(void *opaque, int irq, int level)
> +{
> +    xendevicemodel_set_irq_level(xen_dmod, xen_domid, irq, level);

So, you just ignore the return value here. Shouldn't there be some kind
of error check?

And is it OK to create a virtio-mmio device without an error, even when
we could find out that it never going to work (e.g. on Xen 4.14)?

Cheers,

-- 
Anthony PERARD