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

[Vikram Garhwal <vikram.garhwal@amd.com>]

Hi Anthony,
On Thu, Oct 05, 2023 at 11:40:57AM +0100, Anthony PERARD wrote:
> 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?
I missed it. Initial version didn't have this file change and i missed updating
my cc list.
> 
> 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
Thanks for spotting this.
I will create a fix and gitlab-ci for the patch.
> 
> > +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?
Changed return to -ENOSYS for older version.
> 
> > 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)?
This is something Oleksandr can answer better as it was written by him. But
I think we can print an error "virtio init failed" and exit the
machine init. Does that aligns with your thinking?
> 
> Cheers,
> 
> -- 
> Anthony PERARD