Re: [QEMU][PATCH v2 05/11] include/hw/xen/xen_common: return error from xen_create_ioreq_server

From: Stefano Stabellini <sstabellini@kernel.org>
To: "Philippe Mathieu-Daudé" <philmd@linaro.org>
Cc: "Vikram Garhwal" <vikram.garhwal@amd.com>,
	qemu-dev@xilinx.com, "Paolo Bonzini" <pbonzini@redhat.com>,
	stefano.stabellini@amd.com, alex.bennee@linaro.org,
	xen-devel@lists.xenproject.org,
	"Stefano Stabellini" <sstabellini@kernel.org>,
	"Anthony Perard" <anthony.perard@citrix.com>,
	"Paul Durrant" <paul@xen.org>,
	"open list:All patches CC here" <qemu-devel@nongnu.org>,
	"Daniel P. Berrangé" <berrange@redhat.com>,
	"Thomas Huth" <thuth@redhat.com>
Subject: Re: [QEMU][PATCH v2 05/11] include/hw/xen/xen_common: return error from xen_create_ioreq_server
Date: Fri, 2 Dec 2022 13:17:09 -0800 (PST)	[thread overview]
Message-ID: <alpine.DEB.2.22.394.2212021316530.4039@ubuntu-linux-20-04-desktop> (raw)
In-Reply-To: <4b143736-d5a4-ac75-3671-9566a00707af@linaro.org>

[-- Attachment #1: Type: text/plain, Size: 3506 bytes --]

On Fri, 1 Dec 2022, Philippe Mathieu-Daudé wrote:
> On 2/12/22 08:19, Philippe Mathieu-Daudé wrote:
> > Hi Stefano and Vikram,
> > 
> > On 2/12/22 03:59, Vikram Garhwal wrote:
> > > From: Stefano Stabellini <stefano.stabellini@amd.com>
> > > 
> > > This is done to prepare for enabling xenpv support for ARM architecture.
> > > On ARM it is possible to have a functioning xenpv machine with only the
> > > PV backends and no IOREQ server. If the IOREQ server creation fails,
> > > continue to the PV backends initialization.
> > > 
> > > Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
> > > Signed-off-by: Vikram Garhwal <vikram.garhwal@amd.com>
> > > ---
> > >   include/hw/xen/xen_common.h | 13 ++++++++-----
> > >   1 file changed, 8 insertions(+), 5 deletions(-)
> > > 
> > > diff --git a/include/hw/xen/xen_common.h b/include/hw/xen/xen_common.h
> > > index 77ce17d8a4..6510ac15e0 100644
> > > --- a/include/hw/xen/xen_common.h
> > > +++ b/include/hw/xen/xen_common.h
> > > @@ -467,9 +467,10 @@ static inline void xen_unmap_pcidev(domid_t dom,
> > >   {
> > >   }
> > > -static inline void xen_create_ioreq_server(domid_t dom,
> > > -                                           ioservid_t *ioservid)
> > 
> > How long are we supposed to maintain this code? Per [*]:
> > 
> >    In general XenProject.org supports stable branches for 18 months full
> >    support plus 18 months security fixes. When a new X.Y.0 release is
> >    made there is usually one more release on the to-be-retired stable
> >    branch to mop up any loose patches sitting in the repository at which
> >    point the branch is retired.
> > 
> > 4.17 was just released. 4.5 was 7 years ago. IIUC EOL'ed 4 years ago.

Hi Philippe,

So far we have not removed any of the old compatibility code in the xen
headers like xen_common.h. However, you have a point and I think we
could do so going forward. Like you wrote, 4.5 was 7 years ago, I would
be happy to remove the old compatibility code to support ancient
releases and that would simplify the code in the QEMU xen headers quite
a bit.

That said, the change in this patch is orthogonal. This is needed anyway
because we can have very modern Xen builds without IOREQ server
capabilities (it is a kconfig option). So we would still need this patch.

Cheers,

Stefano

> > [*]
> > https://wiki.xenproject.org/wiki/Xen_Project_Maintenance_Releases#Stable_Maintenance_Branches
> 
> +Paolo for commit 14efd8d3b5 ("meson, configure: move Xen detection to
> meson"):
> 
>     xen_libs = {
>       '4.11.0': [ 'xenstore', 'xenctrl', 'xendevicemodel', 'xenforeignmemory',
> 'xengnttab', 'xenevtchn', 'xentoolcore' ],
>       '4.10.0': [ 'xenstore', 'xenctrl', 'xendevicemodel', 'xenforeignmemory',
> 'xengnttab', 'xenevtchn', 'xentoolcore' ],
>       '4.9.0': [ 'xenstore', 'xenctrl', 'xendevicemodel', 'xenforeignmemory',
> 'xengnttab', 'xenevtchn' ],
>       '4.8.0': [ 'xenstore', 'xenctrl', 'xenforeignmemory', 'xengnttab',
> 'xenevtchn' ],
>       '4.7.1': [ 'xenstore', 'xenctrl', 'xenforeignmemory', 'xengnttab',
> 'xenevtchn' ],
>       '4.6.0': [ 'xenstore', 'xenctrl' ],
>       '4.5.0': [ 'xenstore', 'xenctrl' ],
>       '4.2.0': [ 'xenstore', 'xenctrl' ],
>     }
> 
> According to repology for the 'xen' package:
> 
>    FreeBSD (ports):    4.16
>    Debian 11:          4.14.5
>    Fedora 35:          4.16.2
>    Ubuntu 20.04:       4.11.3
>    OpenSUSE Leap 15.3: 4.14.1
>    RHEL 8:             ?
> 