From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=51865 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Q4EOt-0008EI-4v for qemu-devel@nongnu.org; Mon, 28 Mar 2011 11:30:41 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Q4EOq-0001G7-FZ for qemu-devel@nongnu.org; Mon, 28 Mar 2011 11:30:38 -0400 Received: from mail-vx0-f173.google.com ([209.85.220.173]:55991) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Q4EOq-0001G1-68 for qemu-devel@nongnu.org; Mon, 28 Mar 2011 11:30:36 -0400 Received: by vxb41 with SMTP id 41so2709598vxb.4 for ; Mon, 28 Mar 2011 08:30:35 -0700 (PDT) MIME-Version: 1.0 Sender: anthony.perard@gmail.com In-Reply-To: <7B2F3BE5-FCE1-49E1-A7CC-18E71D88F83D@suse.de> References: <1299004529-31290-1-git-send-email-anthony.perard@citrix.com> <1299004529-31290-4-git-send-email-anthony.perard@citrix.com> <7B2F3BE5-FCE1-49E1-A7CC-18E71D88F83D@suse.de> From: Anthony PERARD Date: Mon, 28 Mar 2011 16:22:09 +0100 Message-ID: Subject: Re: [Xen-devel] Re: [Qemu-devel] [PATCH V11 03/15] xen: Support new libxc calls from xen unstable. Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alexander Graf Cc: Xen Devel , QEMU-devel , Stefano Stabellini On Wed, Mar 23, 2011 at 10:43, Alexander Graf wrote: > > On 01.03.2011, at 19:35, Anthony.Perard@citrix.com wrote: > >> From: Anthony PERARD >> >> This patch updates the libxenctrl calls in Qemu to use the new interface= , >> otherwise Qemu wouldn't be able to build against new versions of the >> library. >> >> We check libxenctrl version in configure, from Xen 3.3.0 to Xen >> unstable. >> >> Signed-off-by: Anthony PERARD >> Signed-off-by: Stefano Stabellini >> Acked-by: Alexander Graf >> --- >> configure =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0| =C2=A0 67 +++++++++= +++++++++++++++++++++++++++++++++++++++- >> hw/xen_backend.c =C2=A0 =C2=A0 | =C2=A0 21 ++++++++------- >> hw/xen_backend.h =C2=A0 =C2=A0 | =C2=A0 =C2=A06 ++-- >> hw/xen_common.h =C2=A0 =C2=A0 =C2=A0| =C2=A0 64 ++++++++++++++++++++++++= +++++++++++++---------- >> hw/xen_disk.c =C2=A0 =C2=A0 =C2=A0 =C2=A0| =C2=A0 =C2=A04 +- >> hw/xen_domainbuild.c | =C2=A0 =C2=A03 +- >> 6 files changed, 133 insertions(+), 32 deletions(-) >> >> diff --git a/configure b/configure >> index 3036faf..a84d974 100755 >> --- a/configure >> +++ b/configure >> @@ -126,6 +126,7 @@ vnc_jpeg=3D"" >> vnc_png=3D"" >> vnc_thread=3D"no" >> xen=3D"" >> +xen_ctrl_version=3D"" >> linux_aio=3D"" >> attr=3D"" >> vhost_net=3D"" >> @@ -1147,20 +1148,81 @@ fi >> >> if test "$xen" !=3D "no" ; then >> =C2=A0 xen_libs=3D"-lxenstore -lxenctrl -lxenguest" >> + >> + =C2=A0# Xen unstable >> =C2=A0 cat > $TMPC <> #include >> #include >> -int main(void) { xs_daemon_open(); xc_interface_open(); return 0; } >> +#include >> +#include >> +#if !defined(HVM_MAX_VCPUS) >> +# error HVM_MAX_VCPUS not defined >> +#endif >> +int main(void) { >> + =C2=A0xc_interface *xc; >> + =C2=A0xs_daemon_open(); >> + =C2=A0xc =3D xc_interface_open(0, 0, 0); >> + =C2=A0xc_hvm_set_mem_type(0, 0, HVMMEM_ram_ro, 0, 0); >> + =C2=A0xc_gnttab_open(NULL, 0); >> + =C2=A0return 0; >> +} >> EOF >> =C2=A0 if compile_prog "" "$xen_libs" ; then >> + =C2=A0 =C2=A0xen_ctrl_version=3D410 >> =C2=A0 =C2=A0 xen=3Dyes >> - =C2=A0 =C2=A0libs_softmmu=3D"$xen_libs $libs_softmmu" >> + >> + =C2=A0# Xen 4.0.0 >> + =C2=A0elif ( >> + =C2=A0 =C2=A0 =C2=A0cat > $TMPC <> +#include >> +#include >> +#include >> +#include >> +#if !defined(HVM_MAX_VCPUS) >> +# error HVM_MAX_VCPUS not defined >> +#endif >> +int main(void) { >> + =C2=A0xs_daemon_open(); >> + =C2=A0xc_interface_open(); >> + =C2=A0xc_gnttab_open(); >> + =C2=A0xc_hvm_set_mem_type(0, 0, HVMMEM_ram_ro, 0, 0); >> + =C2=A0return 0; >> +} >> +EOF >> + =C2=A0 =C2=A0 =C2=A0compile_prog "" "$xen_libs" >> + =C2=A0 =C2=A0) ; then >> + =C2=A0 =C2=A0xen_ctrl_version=3D400 >> + =C2=A0 =C2=A0xen=3Dyes >> + >> + =C2=A0# Xen 3.3.0, 3.4.0 >> + =C2=A0elif ( >> + =C2=A0 =C2=A0 =C2=A0cat > $TMPC <> +#include >> +#include >> +int main(void) { >> + =C2=A0xs_daemon_open(); >> + =C2=A0xc_interface_open(); >> + =C2=A0xc_gnttab_open(); >> + =C2=A0xc_hvm_set_mem_type(0, 0, HVMMEM_ram_ro, 0, 0); >> + =C2=A0return 0; >> +} >> +EOF >> + =C2=A0 =C2=A0 =C2=A0compile_prog "" "$xen_libs" >> + =C2=A0 =C2=A0) ; then >> + =C2=A0 =C2=A0xen_ctrl_version=3D330 >> + =C2=A0 =C2=A0xen=3Dyes >> + >> + =C2=A0# Xen not found or unsupported >> =C2=A0 else >> =C2=A0 =C2=A0 if test "$xen" =3D "yes" ; then >> =C2=A0 =C2=A0 =C2=A0 feature_not_found "xen" >> =C2=A0 =C2=A0 fi >> =C2=A0 =C2=A0 xen=3Dno >> =C2=A0 fi >> + >> + =C2=A0if test "$xen" =3D yes; then >> + =C2=A0 =C2=A0libs_softmmu=3D"$xen_libs $libs_softmmu" >> + =C2=A0fi >> fi >> >> ########################################## >> @@ -2755,6 +2817,7 @@ if test "$bluez" =3D "yes" ; then >> fi >> if test "$xen" =3D "yes" ; then >> =C2=A0 echo "CONFIG_XEN=3Dy" >> $config_host_mak >> + =C2=A0echo "CONFIG_XEN_CTRL_INTERFACE_VERSION=3D$xen_ctrl_version" >> = $config_host_mak >> fi >> if test "$io_thread" =3D "yes" ; then >> =C2=A0 echo "CONFIG_IOTHREAD=3Dy" >> $config_host_mak >> diff --git a/hw/xen_backend.c b/hw/xen_backend.c >> index 9f4ec4b..3907b83 100644 >> --- a/hw/xen_backend.c >> +++ b/hw/xen_backend.c >> @@ -43,7 +43,8 @@ >> /* ------------------------------------------------------------- */ >> >> /* public */ >> -int xen_xc; >> +XenXC xen_xc =3D XC_HANDLER_INITIAL_VALUE; >> +XenGnttab xen_xcg =3D XC_HANDLER_INITIAL_VALUE; >> struct xs_handle *xenstore =3D NULL; >> const char *xen_protocol; >> >> @@ -214,8 +215,8 @@ static struct XenDevice *xen_be_get_xendev(const cha= r *type, int dom, int dev, >> =C2=A0 =C2=A0 xendev->debug =C2=A0 =C2=A0 =C2=A0=3D debug; >> =C2=A0 =C2=A0 xendev->local_port =3D -1; >> >> - =C2=A0 =C2=A0xendev->evtchndev =3D xc_evtchn_open(); >> - =C2=A0 =C2=A0if (xendev->evtchndev < 0) { >> + =C2=A0 =C2=A0xendev->evtchndev =3D xc_evtchn_open(NULL, 0); >> + =C2=A0 =C2=A0if (xendev->evtchndev =3D=3D XC_HANDLER_INITIAL_VALUE) { >> =C2=A0 =C2=A0 =C2=A0 =C2=A0 xen_be_printf(NULL, 0, "can't open evtchn de= vice\n"); >> =C2=A0 =C2=A0 =C2=A0 =C2=A0 qemu_free(xendev); >> =C2=A0 =C2=A0 =C2=A0 =C2=A0 return NULL; >> @@ -223,15 +224,15 @@ static struct XenDevice *xen_be_get_xendev(const c= har *type, int dom, int dev, >> =C2=A0 =C2=A0 fcntl(xc_evtchn_fd(xendev->evtchndev), F_SETFD, FD_CLOEXEC= ); >> >> =C2=A0 =C2=A0 if (ops->flags & DEVOPS_FLAG_NEED_GNTDEV) { >> - =C2=A0 =C2=A0 =C2=A0 =C2=A0xendev->gnttabdev =3D xc_gnttab_open(); >> - =C2=A0 =C2=A0 =C2=A0 =C2=A0if (xendev->gnttabdev < 0) { >> + =C2=A0 =C2=A0 =C2=A0 =C2=A0xendev->gnttabdev =3D xc_gnttab_open(NULL, = 0); >> + =C2=A0 =C2=A0 =C2=A0 =C2=A0if (xendev->gnttabdev =3D=3D XC_HANDLER_INI= TIAL_VALUE) { >> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 xen_be_printf(NULL, 0, "can't = open gnttab device\n"); >> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 xc_evtchn_close(xendev->evtchn= dev); >> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 qemu_free(xendev); >> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 return NULL; >> =C2=A0 =C2=A0 =C2=A0 =C2=A0 } >> =C2=A0 =C2=A0 } else { >> - =C2=A0 =C2=A0 =C2=A0 =C2=A0xendev->gnttabdev =3D -1; >> + =C2=A0 =C2=A0 =C2=A0 =C2=A0xendev->gnttabdev =3D XC_HANDLER_INITIAL_VA= LUE; >> =C2=A0 =C2=A0 } >> >> =C2=A0 =C2=A0 QTAILQ_INSERT_TAIL(&xendevs, xendev, next); >> @@ -277,10 +278,10 @@ static struct XenDevice *xen_be_del_xendev(int dom= , int dev) >> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 qemu_free(xendev->fe); >> =C2=A0 =C2=A0 =C2=A0 =C2=A0 } >> >> - =C2=A0 =C2=A0 =C2=A0 =C2=A0if (xendev->evtchndev >=3D 0) { >> + =C2=A0 =C2=A0 =C2=A0 =C2=A0if (xendev->evtchndev !=3D XC_HANDLER_INITI= AL_VALUE) { >> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 xc_evtchn_close(xendev->evtchn= dev); >> =C2=A0 =C2=A0 =C2=A0 =C2=A0 } >> - =C2=A0 =C2=A0 =C2=A0 =C2=A0if (xendev->gnttabdev >=3D 0) { >> + =C2=A0 =C2=A0 =C2=A0 =C2=A0if (xendev->gnttabdev !=3D XC_HANDLER_INITI= AL_VALUE) { >> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 xc_gnttab_close(xendev->gnttab= dev); >> =C2=A0 =C2=A0 =C2=A0 =C2=A0 } >> >> @@ -664,8 +665,8 @@ int xen_be_init(void) >> =C2=A0 =C2=A0 =C2=A0 =C2=A0 goto err; >> =C2=A0 =C2=A0 } >> >> - =C2=A0 =C2=A0xen_xc =3D xc_interface_open(); >> - =C2=A0 =C2=A0if (xen_xc =3D=3D -1) { >> + =C2=A0 =C2=A0xen_xc =3D xc_interface_open(0, 0, 0); >> + =C2=A0 =C2=A0if (xen_xc =3D=3D XC_HANDLER_INITIAL_VALUE) { >> =C2=A0 =C2=A0 =C2=A0 =C2=A0 xen_be_printf(NULL, 0, "can't open xen inter= face\n"); >> =C2=A0 =C2=A0 =C2=A0 =C2=A0 goto err; >> =C2=A0 =C2=A0 } >> diff --git a/hw/xen_backend.h b/hw/xen_backend.h >> index 1b428e3..6401c85 100644 >> --- a/hw/xen_backend.h >> +++ b/hw/xen_backend.h >> @@ -45,8 +45,8 @@ struct XenDevice { >> =C2=A0 =C2=A0 int =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0remote_port; >> =C2=A0 =C2=A0 int =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0local_port; >> >> - =C2=A0 =C2=A0int =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0evtchndev; >> - =C2=A0 =C2=A0int =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0gnttabdev; >> + =C2=A0 =C2=A0XenEvtchn =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0evtchndev; >> + =C2=A0 =C2=A0XenGnttab =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0gnttabdev; >> >> =C2=A0 =C2=A0 struct XenDevOps =C2=A0 *ops; >> =C2=A0 =C2=A0 QTAILQ_ENTRY(XenDevice) next; >> @@ -55,7 +55,7 @@ struct XenDevice { >> /* ------------------------------------------------------------- */ >> >> /* variables */ >> -extern int xen_xc; >> +extern XenXC xen_xc; >> extern struct xs_handle *xenstore; >> extern const char *xen_protocol; >> >> diff --git a/hw/xen_common.h b/hw/xen_common.h >> index 8a55b44..7e123ec 100644 >> --- a/hw/xen_common.h >> +++ b/hw/xen_common.h >> @@ -1,6 +1,8 @@ >> #ifndef QEMU_HW_XEN_COMMON_H >> #define QEMU_HW_XEN_COMMON_H 1 >> >> +#include "config-host.h" >> + >> #include >> #include >> >> @@ -13,22 +15,56 @@ >> #include "qemu-queue.h" >> >> /* >> - * tweaks needed to build with different xen versions >> - * =C2=A00x00030205 -> 3.1.0 >> - * =C2=A00x00030207 -> 3.2.0 >> - * =C2=A00x00030208 -> unstable >> + * We don't support Xen prior to 3.3.0. >> =C2=A0*/ >> -#include >> -#if __XEN_LATEST_INTERFACE_VERSION__ < 0x00030205 >> -# define evtchn_port_or_error_t int >> -#endif >> -#if __XEN_LATEST_INTERFACE_VERSION__ < 0x00030207 >> -# define xc_map_foreign_pages xc_map_foreign_batch >> + >> +/* Xen before 4.0 */ >> +#if CONFIG_XEN_CTRL_INTERFACE_VERSION < 400 >> +static inline void *xc_map_foreign_bulk(int xc_handle, uint32_t dom, in= t prot, >> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0xen_pf= n_t *arr, int *err, >> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0unsign= ed int num) >> +{ >> + =C2=A0 =C2=A0return xc_map_foreign_batch(xc_handle, dom, prot, arr, nu= m); >> +} >> #endif >> -#if __XEN_LATEST_INTERFACE_VERSION__ < 0x00030208 >> -# define xen_mb() =C2=A0mb() >> -# define xen_rmb() rmb() >> -# define xen_wmb() wmb() >> + >> + >> +/* Xen before 4.1 */ >> +#if CONFIG_XEN_CTRL_INTERFACE_VERSION < 410 >> + >> +typedef int XenXC; >> +typedef int XenEvtchn; >> +typedef int XenGnttab; >> + >> +# =C2=A0define XC_INTERFACE_FMT "%i" >> +# =C2=A0define XC_HANDLER_INITIAL_VALUE =C2=A0 =C2=A0-1 >> + >> +# =C2=A0define xc_interface_open(logger, dombuild_logger, open_flags) \ >> + =C2=A0 =C2=A0xc_interface_open() >> +# =C2=A0define xc_evtchn_open(logger, open_flags) xc_evtchn_open() >> +# =C2=A0define xc_gnttab_open(logger, open_flags) xc_gnttab_open() > > > defines really bite me when abstracting the interface through an indirect= calling table. I had to change the code this way to make it work with xenn= er: > > > diff --git a/hw/xen_common.h b/hw/xen_common.h > index a5fc74b..0c09b37 100644 > --- a/hw/xen_common.h > +++ b/hw/xen_common.h > @@ -39,10 +39,23 @@ typedef int XenGnttab; > =C2=A0# =C2=A0define XC_INTERFACE_FMT "%i" > =C2=A0# =C2=A0define XC_HANDLER_INITIAL_VALUE =C2=A0 =C2=A0-1 > > -# =C2=A0define xc_interface_open(logger, dombuild_logger, open_flags) \ > - =C2=A0 =C2=A0xc_interface_open() > -# =C2=A0define xc_evtchn_open(logger, open_flags) xc_evtchn_open() > -# =C2=A0define xc_gnttab_open(logger, open_flags) xc_gnttab_open() > +static inline XenEvtchn xen_xc_evtchn_open(void *logger, > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 unsigned int open_flags) > +{ > + =C2=A0 =C2=A0return xc_evtchn_open(); > +} > + > +static inline XenGnttab xen_xc_gnttab_open(void *logger, > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 unsigned int open_flags) > +{ > + =C2=A0 =C2=A0return xc_gnttab_open(); > +} > + > +static inline XenXC xen_xc_interface_open(void *logger, void *dombuild_l= ogger, > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0unsigned int open_flags) > +{ > + =C2=A0 =C2=A0return xc_interface_open(); > +} > > =C2=A0static inline int xc_fd(int xen_xc) > =C2=A0{ > @@ -69,6 +82,24 @@ typedef xc_gnttab *XenGnttab; > =C2=A0# =C2=A0define XC_INTERFACE_FMT "%p" > =C2=A0# =C2=A0define XC_HANDLER_INITIAL_VALUE =C2=A0 =C2=A0NULL > > +static inline XenEvtchn xen_xc_evtchn_open(void *logger, > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 unsigned int open_flags) > +{ > + =C2=A0 =C2=A0return xc_evtchn_open(logger, open_flags); > +} > + > +static inline XenGnttab xen_xc_gnttab_open(void *logger, > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 unsigned int open_flags) > +{ > + =C2=A0 =C2=A0return xc_gnttab_open(logger, open_flags); > +} > + > +static inline XenXC xen_xc_interface_open(void *logger, void *dombuild_l= ogger, > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0int open_flags) > +{ > + =C2=A0 =C2=A0return xc_interface_open(logger, dombuild_logger, open_fla= gs); > +} > + > =C2=A0/* FIXME There is now way to have the xen fd */ > =C2=A0static inline int xc_fd(xc_interface *xen_xc) > =C2=A0{ > > > Leave it as is in your patch and I'll change it in my xenner patch set ag= ain, but I wanted to make sure that you're aware of the issues going along = with #defines. I understand the issues with the defines. I just trying to address both comment from Anthony and you. But, I think these static inline instead of the #defines will be ok. So I will put it in my series. --=20 Anthony PERARD