| Message ID | 1490033952-26735-1-git-send-email-sstabellini@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, 20 Mar 2017 11:19:05 -0700 Stefano Stabellini <sstabellini@kernel.org> wrote: > Do not use the ring.h header installed on the system. Instead, import > the header into the QEMU codebase. This avoids problems when QEMU is > built against a Xen version too old to provide all the ring macros. > > Signed-off-by: Stefano Stabellini <stefano@aporeto.com> > Reviewed-by: Greg Kurz <groug@kaod.org> > CC: anthony.perard@citrix.com > CC: jgross@suse.com > --- > NB: The new macros have not been committed to Xen yet. Do not apply this > patch until they do. > --- Looking at your other series for the kernel part of this feature: https://lkml.org/lkml/2017/3/22/761 I realize that the ring.h header from Xen also exists in the kernel tree... Shouldn't all the code that can be used in both kernel and userspace go to a header file under include/uapi in the kernel tree ? And then we would import it under include/standard-headers/linux in the QEMU tree and we could keep it in sync using scripts/update-linux-headers.sh. Cc'ing Paolo for insights. > --- > hw/block/xen_blkif.h | 2 +- > hw/usb/xen-usb.c | 2 +- > include/hw/xen/io/ring.h | 455 +++++++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 457 insertions(+), 2 deletions(-) > create mode 100644 include/hw/xen/io/ring.h > > diff --git a/hw/block/xen_blkif.h b/hw/block/xen_blkif.h > index 3300b6f..3e6e1ea 100644 > --- a/hw/block/xen_blkif.h > +++ b/hw/block/xen_blkif.h > @@ -1,7 +1,7 @@ > #ifndef XEN_BLKIF_H > #define XEN_BLKIF_H > > -#include <xen/io/ring.h> > +#include "hw/xen/io/ring.h" > #include <xen/io/blkif.h> > #include <xen/io/protocols.h> > > diff --git a/hw/usb/xen-usb.c b/hw/usb/xen-usb.c > index 8e676e6..370b3d9 100644 > --- a/hw/usb/xen-usb.c > +++ b/hw/usb/xen-usb.c > @@ -33,7 +33,7 @@ > #include "qapi/qmp/qint.h" > #include "qapi/qmp/qstring.h" > > -#include <xen/io/ring.h> > +#include "hw/xen/io/ring.h" > #include <xen/io/usbif.h> > > /* > diff --git a/include/hw/xen/io/ring.h b/include/hw/xen/io/ring.h > new file mode 100644 > index 0000000..cf01fc3 > --- /dev/null > +++ b/include/hw/xen/io/ring.h > @@ -0,0 +1,455 @@ > +/****************************************************************************** > + * ring.h > + * > + * Shared producer-consumer ring macros. > + * > + * Permission is hereby granted, free of charge, to any person obtaining a copy > + * of this software and associated documentation files (the "Software"), to > + * deal in the Software without restriction, including without limitation the > + * rights to use, copy, modify, merge, publish, distribute, sublicense, and/or > + * sell copies of the Software, and to permit persons to whom the Software is > + * furnished to do so, subject to the following conditions: > + * > + * The above copyright notice and this permission notice shall be included in > + * all copies or substantial portions of the Software. > + * > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE > + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING > + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER > + * DEALINGS IN THE SOFTWARE. > + * > + * Tim Deegan and Andrew Warfield November 2004. > + */ > + > +#ifndef __XEN_PUBLIC_IO_RING_H__ > +#define __XEN_PUBLIC_IO_RING_H__ > + > +#if __XEN_INTERFACE_VERSION__ < 0x00030208 > +#define xen_mb() mb() > +#define xen_rmb() rmb() > +#define xen_wmb() wmb() > +#endif > + > +typedef unsigned int RING_IDX; > + > +/* Round a 32-bit unsigned constant down to the nearest power of two. */ > +#define __RD2(_x) (((_x) & 0x00000002) ? 0x2 : ((_x) & 0x1)) > +#define __RD4(_x) (((_x) & 0x0000000c) ? __RD2((_x)>>2)<<2 : __RD2(_x)) > +#define __RD8(_x) (((_x) & 0x000000f0) ? __RD4((_x)>>4)<<4 : __RD4(_x)) > +#define __RD16(_x) (((_x) & 0x0000ff00) ? __RD8((_x)>>8)<<8 : __RD8(_x)) > +#define __RD32(_x) (((_x) & 0xffff0000) ? __RD16((_x)>>16)<<16 : __RD16(_x)) > + > +/* > + * Calculate size of a shared ring, given the total available space for the > + * ring and indexes (_sz), and the name tag of the request/response structure. > + * A ring contains as many entries as will fit, rounded down to the nearest > + * power of two (so we can mask with (size-1) to loop around). > + */ > +#define __CONST_RING_SIZE(_s, _sz) \ > + (__RD32(((_sz) - offsetof(struct _s##_sring, ring)) / \ > + sizeof(((struct _s##_sring *)0)->ring[0]))) > +/* > + * The same for passing in an actual pointer instead of a name tag. > + */ > +#define __RING_SIZE(_s, _sz) \ > + (__RD32(((_sz) - (long)(_s)->ring + (long)(_s)) / sizeof((_s)->ring[0]))) > + > +/* > + * Macros to make the correct C datatypes for a new kind of ring. > + * > + * To make a new ring datatype, you need to have two message structures, > + * let's say request_t, and response_t already defined. > + * > + * In a header where you want the ring datatype declared, you then do: > + * > + * DEFINE_RING_TYPES(mytag, request_t, response_t); > + * > + * These expand out to give you a set of types, as you can see below. > + * The most important of these are: > + * > + * mytag_sring_t - The shared ring. > + * mytag_front_ring_t - The 'front' half of the ring. > + * mytag_back_ring_t - The 'back' half of the ring. > + * > + * To initialize a ring in your code you need to know the location and size > + * of the shared memory area (PAGE_SIZE, for instance). To initialise > + * the front half: > + * > + * mytag_front_ring_t front_ring; > + * SHARED_RING_INIT((mytag_sring_t *)shared_page); > + * FRONT_RING_INIT(&front_ring, (mytag_sring_t *)shared_page, PAGE_SIZE); > + * > + * Initializing the back follows similarly (note that only the front > + * initializes the shared ring): > + * > + * mytag_back_ring_t back_ring; > + * BACK_RING_INIT(&back_ring, (mytag_sring_t *)shared_page, PAGE_SIZE); > + */ > + > +#define DEFINE_RING_TYPES(__name, __req_t, __rsp_t) \ > + \ > +/* Shared ring entry */ \ > +union __name##_sring_entry { \ > + __req_t req; \ > + __rsp_t rsp; \ > +}; \ > + \ > +/* Shared ring page */ \ > +struct __name##_sring { \ > + RING_IDX req_prod, req_event; \ > + RING_IDX rsp_prod, rsp_event; \ > + union { \ > + struct { \ > + uint8_t smartpoll_active; \ > + } netif; \ > + struct { \ > + uint8_t msg; \ > + } tapif_user; \ > + uint8_t pvt_pad[4]; \ > + } pvt; \ > + uint8_t __pad[44]; \ > + union __name##_sring_entry ring[1]; /* variable-length */ \ > +}; \ > + \ > +/* "Front" end's private variables */ \ > +struct __name##_front_ring { \ > + RING_IDX req_prod_pvt; \ > + RING_IDX rsp_cons; \ > + unsigned int nr_ents; \ > + struct __name##_sring *sring; \ > +}; \ > + \ > +/* "Back" end's private variables */ \ > +struct __name##_back_ring { \ > + RING_IDX rsp_prod_pvt; \ > + RING_IDX req_cons; \ > + unsigned int nr_ents; \ > + struct __name##_sring *sring; \ > +}; \ > + \ > +/* Syntactic sugar */ \ > +typedef struct __name##_sring __name##_sring_t; \ > +typedef struct __name##_front_ring __name##_front_ring_t; \ > +typedef struct __name##_back_ring __name##_back_ring_t > + > +/* > + * Macros for manipulating rings. > + * > + * FRONT_RING_whatever works on the "front end" of a ring: here > + * requests are pushed on to the ring and responses taken off it. > + * > + * BACK_RING_whatever works on the "back end" of a ring: here > + * requests are taken off the ring and responses put on. > + * > + * N.B. these macros do NO INTERLOCKS OR FLOW CONTROL. > + * This is OK in 1-for-1 request-response situations where the > + * requestor (front end) never has more than RING_SIZE()-1 > + * outstanding requests. > + */ > + > +/* Initialising empty rings */ > +#define SHARED_RING_INIT(_s) do { \ > + (_s)->req_prod = (_s)->rsp_prod = 0; \ > + (_s)->req_event = (_s)->rsp_event = 1; \ > + (void)memset((_s)->pvt.pvt_pad, 0, sizeof((_s)->pvt.pvt_pad)); \ > + (void)memset((_s)->__pad, 0, sizeof((_s)->__pad)); \ > +} while(0) > + > +#define FRONT_RING_INIT(_r, _s, __size) do { \ > + (_r)->req_prod_pvt = 0; \ > + (_r)->rsp_cons = 0; \ > + (_r)->nr_ents = __RING_SIZE(_s, __size); \ > + (_r)->sring = (_s); \ > +} while (0) > + > +#define BACK_RING_INIT(_r, _s, __size) do { \ > + (_r)->rsp_prod_pvt = 0; \ > + (_r)->req_cons = 0; \ > + (_r)->nr_ents = __RING_SIZE(_s, __size); \ > + (_r)->sring = (_s); \ > +} while (0) > + > +/* How big is this ring? */ > +#define RING_SIZE(_r) \ > + ((_r)->nr_ents) > + > +/* Number of free requests (for use on front side only). */ > +#define RING_FREE_REQUESTS(_r) \ > + (RING_SIZE(_r) - ((_r)->req_prod_pvt - (_r)->rsp_cons)) > + > +/* Test if there is an empty slot available on the front ring. > + * (This is only meaningful from the front. ) > + */ > +#define RING_FULL(_r) \ > + (RING_FREE_REQUESTS(_r) == 0) > + > +/* Test if there are outstanding messages to be processed on a ring. */ > +#define RING_HAS_UNCONSUMED_RESPONSES(_r) \ > + ((_r)->sring->rsp_prod - (_r)->rsp_cons) > + > +#ifdef __GNUC__ > +#define RING_HAS_UNCONSUMED_REQUESTS(_r) ({ \ > + unsigned int req = (_r)->sring->req_prod - (_r)->req_cons; \ > + unsigned int rsp = RING_SIZE(_r) - \ > + ((_r)->req_cons - (_r)->rsp_prod_pvt); \ > + req < rsp ? req : rsp; \ > +}) > +#else > +/* Same as above, but without the nice GCC ({ ... }) syntax. */ > +#define RING_HAS_UNCONSUMED_REQUESTS(_r) \ > + ((((_r)->sring->req_prod - (_r)->req_cons) < \ > + (RING_SIZE(_r) - ((_r)->req_cons - (_r)->rsp_prod_pvt))) ? \ > + ((_r)->sring->req_prod - (_r)->req_cons) : \ > + (RING_SIZE(_r) - ((_r)->req_cons - (_r)->rsp_prod_pvt))) > +#endif > + > +/* Direct access to individual ring elements, by index. */ > +#define RING_GET_REQUEST(_r, _idx) \ > + (&((_r)->sring->ring[((_idx) & (RING_SIZE(_r) - 1))].req)) > + > +/* > + * Get a local copy of a request. > + * > + * Use this in preference to RING_GET_REQUEST() so all processing is > + * done on a local copy that cannot be modified by the other end. > + * > + * Note that https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58145 may cause this > + * to be ineffective where _req is a struct which consists of only bitfields. > + */ > +#define RING_COPY_REQUEST(_r, _idx, _req) do { \ > + /* Use volatile to force the copy into _req. */ \ > + *(_req) = *(volatile typeof(_req))RING_GET_REQUEST(_r, _idx); \ > +} while (0) > + > +#define RING_GET_RESPONSE(_r, _idx) \ > + (&((_r)->sring->ring[((_idx) & (RING_SIZE(_r) - 1))].rsp)) > + > +/* Loop termination condition: Would the specified index overflow the ring? */ > +#define RING_REQUEST_CONS_OVERFLOW(_r, _cons) \ > + (((_cons) - (_r)->rsp_prod_pvt) >= RING_SIZE(_r)) > + > +/* Ill-behaved frontend determination: Can there be this many requests? */ > +#define RING_REQUEST_PROD_OVERFLOW(_r, _prod) \ > + (((_prod) - (_r)->rsp_prod_pvt) > RING_SIZE(_r)) > + > +#define RING_PUSH_REQUESTS(_r) do { \ > + xen_wmb(); /* back sees requests /before/ updated producer index */ \ > + (_r)->sring->req_prod = (_r)->req_prod_pvt; \ > +} while (0) > + > +#define RING_PUSH_RESPONSES(_r) do { \ > + xen_wmb(); /* front sees resps /before/ updated producer index */ \ > + (_r)->sring->rsp_prod = (_r)->rsp_prod_pvt; \ > +} while (0) > + > +/* > + * Notification hold-off (req_event and rsp_event): > + * > + * When queueing requests or responses on a shared ring, it may not always be > + * necessary to notify the remote end. For example, if requests are in flight > + * in a backend, the front may be able to queue further requests without > + * notifying the back (if the back checks for new requests when it queues > + * responses). > + * > + * When enqueuing requests or responses: > + * > + * Use RING_PUSH_{REQUESTS,RESPONSES}_AND_CHECK_NOTIFY(). The second argument > + * is a boolean return value. True indicates that the receiver requires an > + * asynchronous notification. > + * > + * After dequeuing requests or responses (before sleeping the connection): > + * > + * Use RING_FINAL_CHECK_FOR_REQUESTS() or RING_FINAL_CHECK_FOR_RESPONSES(). > + * The second argument is a boolean return value. True indicates that there > + * are pending messages on the ring (i.e., the connection should not be put > + * to sleep). > + * > + * These macros will set the req_event/rsp_event field to trigger a > + * notification on the very next message that is enqueued. If you want to > + * create batches of work (i.e., only receive a notification after several > + * messages have been enqueued) then you will need to create a customised > + * version of the FINAL_CHECK macro in your own code, which sets the event > + * field appropriately. > + */ > + > +#define RING_PUSH_REQUESTS_AND_CHECK_NOTIFY(_r, _notify) do { \ > + RING_IDX __old = (_r)->sring->req_prod; \ > + RING_IDX __new = (_r)->req_prod_pvt; \ > + xen_wmb(); /* back sees requests /before/ updated producer index */ \ > + (_r)->sring->req_prod = __new; \ > + xen_mb(); /* back sees new requests /before/ we check req_event */ \ > + (_notify) = ((RING_IDX)(__new - (_r)->sring->req_event) < \ > + (RING_IDX)(__new - __old)); \ > +} while (0) > + > +#define RING_PUSH_RESPONSES_AND_CHECK_NOTIFY(_r, _notify) do { \ > + RING_IDX __old = (_r)->sring->rsp_prod; \ > + RING_IDX __new = (_r)->rsp_prod_pvt; \ > + xen_wmb(); /* front sees resps /before/ updated producer index */ \ > + (_r)->sring->rsp_prod = __new; \ > + xen_mb(); /* front sees new resps /before/ we check rsp_event */ \ > + (_notify) = ((RING_IDX)(__new - (_r)->sring->rsp_event) < \ > + (RING_IDX)(__new - __old)); \ > +} while (0) > + > +#define RING_FINAL_CHECK_FOR_REQUESTS(_r, _work_to_do) do { \ > + (_work_to_do) = RING_HAS_UNCONSUMED_REQUESTS(_r); \ > + if (_work_to_do) break; \ > + (_r)->sring->req_event = (_r)->req_cons + 1; \ > + xen_mb(); \ > + (_work_to_do) = RING_HAS_UNCONSUMED_REQUESTS(_r); \ > +} while (0) > + > +#define RING_FINAL_CHECK_FOR_RESPONSES(_r, _work_to_do) do { \ > + (_work_to_do) = RING_HAS_UNCONSUMED_RESPONSES(_r); \ > + if (_work_to_do) break; \ > + (_r)->sring->rsp_event = (_r)->rsp_cons + 1; \ > + xen_mb(); \ > + (_work_to_do) = RING_HAS_UNCONSUMED_RESPONSES(_r); \ > +} while (0) > + > + > +/* > + * DEFINE_XEN_FLEX_RING_AND_INTF defines two monodirectional rings and > + * functions to check if there is data on the ring, and to read and > + * write to them. > + * > + * DEFINE_XEN_FLEX_RING is similar to DEFINE_XEN_FLEX_RING_AND_INTF, but > + * does not define the indexes page. As different protocols can have > + * extensions to the basic format, this macro allow them to define their > + * own struct. > + * > + * XEN_FLEX_RING_SIZE > + * Convenience macro to calculate the size of one of the two rings > + * from the overall order. > + * > + * $NAME_mask > + * Function to apply the size mask to an index, to reduce the index > + * within the range [0-size]. > + * > + * $NAME_read_packet > + * Function to read data from the ring. The amount of data to read is > + * specified by the "size" argument. > + * > + * $NAME_write_packet > + * Function to write data to the ring. The amount of data to write is > + * specified by the "size" argument. > + * > + * $NAME_get_ring_ptr > + * Convenience function that returns a pointer to read/write to the > + * ring at the right location. > + * > + * $NAME_data_intf > + * Indexes page, shared between frontend and backend. It also > + * contains the array of grant refs. > + * > + * $NAME_queued > + * Function to calculate how many bytes are currently on the ring, > + * ready to be read. It can also be used to calculate how much free > + * space is currently on the ring (ring_size - $NAME_queued()). > + */ > +#define XEN_FLEX_RING_SIZE(order) \ > + (1UL << (order + PAGE_SHIFT - 1)) > + > +#define DEFINE_XEN_FLEX_RING_AND_INTF(name) \ > +struct name##_data_intf { \ > + RING_IDX in_cons, in_prod; \ > + \ > + uint8_t pad1[56]; \ > + \ > + RING_IDX out_cons, out_prod; \ > + \ > + uint8_t pad2[56]; \ > + \ > + RING_IDX ring_order; \ > + grant_ref_t ref[]; \ > +}; \ > +DEFINE_XEN_FLEX_RING(name); > + > +#define DEFINE_XEN_FLEX_RING(name) \ > +static inline RING_IDX name##_mask(RING_IDX idx, RING_IDX ring_size) \ > +{ \ > + return (idx & (ring_size - 1)); \ > +} \ > + \ > +static inline RING_IDX name##_mask_order(RING_IDX idx, RING_IDX ring_order) \ > +{ \ > + return (idx & (XEN_FLEX_RING_SIZE(ring_order) - 1)); \ > +} \ > + \ > +static inline unsigned char* name##_get_ring_ptr(unsigned char *buf, \ > + RING_IDX idx, \ > + RING_IDX ring_order) \ > +{ \ > + return buf + name##_mask_order(idx, ring_order); \ > +} \ > + \ > +static inline void name##_read_packet(const unsigned char *buf, \ > + RING_IDX masked_prod, RING_IDX *masked_cons, \ > + RING_IDX ring_size, void *opaque, size_t size) { \ > + if (*masked_cons < masked_prod || \ > + size <= ring_size - *masked_cons) { \ > + memcpy(opaque, buf + *masked_cons, size); \ > + } else { \ > + memcpy(opaque, buf + *masked_cons, ring_size - *masked_cons); \ > + memcpy((unsigned char *)opaque + ring_size - *masked_cons, buf, \ > + size - (ring_size - *masked_cons)); \ > + } \ > + *masked_cons = name##_mask(*masked_cons + size, ring_size); \ > +} \ > + \ > +static inline void name##_write_packet(unsigned char *buf, \ > + RING_IDX *masked_prod, RING_IDX masked_cons, \ > + RING_IDX ring_size, const void *opaque, size_t size) { \ > + if (*masked_prod < masked_cons || \ > + size <= ring_size - *masked_prod) { \ > + memcpy(buf + *masked_prod, opaque, size); \ > + } else { \ > + memcpy(buf + *masked_prod, opaque, ring_size - *masked_prod); \ > + memcpy(buf, (unsigned char *)opaque + (ring_size - *masked_prod), \ > + size - (ring_size - *masked_prod)); \ > + } \ > + *masked_prod = name##_mask(*masked_prod + size, ring_size); \ > +} \ > + \ > +struct name##_data { \ > + unsigned char *in; /* half of the allocation */ \ > + unsigned char *out; /* half of the allocation */ \ > +}; \ > + \ > + \ > +static inline RING_IDX name##_queued(RING_IDX prod, \ > + RING_IDX cons, RING_IDX ring_size) \ > +{ \ > + RING_IDX size; \ > + \ > + if (prod == cons) \ > + return 0; \ > + \ > + prod = name##_mask(prod, ring_size); \ > + cons = name##_mask(cons, ring_size); \ > + \ > + if (prod == cons) \ > + return ring_size; \ > + \ > + if (prod > cons) \ > + size = prod - cons; \ > + else \ > + size = ring_size - (cons - prod); \ > + return size; \ > +}; > + > +#endif /* __XEN_PUBLIC_IO_RING_H__ */ > + > +/* > + * Local variables: > + * mode: C > + * c-file-style: "BSD" > + * c-basic-offset: 4 > + * tab-width: 4 > + * indent-tabs-mode: nil > + * End: > + */
On 23/03/17 14:00, Greg Kurz wrote: > On Mon, 20 Mar 2017 11:19:05 -0700 > Stefano Stabellini <sstabellini@kernel.org> wrote: > >> Do not use the ring.h header installed on the system. Instead, import >> the header into the QEMU codebase. This avoids problems when QEMU is >> built against a Xen version too old to provide all the ring macros. >> >> Signed-off-by: Stefano Stabellini <stefano@aporeto.com> >> Reviewed-by: Greg Kurz <groug@kaod.org> >> CC: anthony.perard@citrix.com >> CC: jgross@suse.com >> --- >> NB: The new macros have not been committed to Xen yet. Do not apply this >> patch until they do. >> --- > > Looking at your other series for the kernel part of this feature: > > https://lkml.org/lkml/2017/3/22/761 > > I realize that the ring.h header from Xen also exists in the kernel tree... > > Shouldn't all the code that can be used in both kernel and userspace go to a > header file under include/uapi in the kernel tree ? And then we would import > it under include/standard-headers/linux in the QEMU tree and we could keep it > in sync using scripts/update-linux-headers.sh. > > Cc'ing Paolo for insights. As Xen isn't part of the kernel we don't want that. You can use and/or build qemu with xen-9pfs backend support on an old Linux kernel without the related frontend. OTOH I don't see the advantage of not using the headers from Xen. This is working for qdisk and pvusb backends and for all the Xen libraries. Do you expect the 9pfs backend to be used for a qemu version built against a Xen version not supporting that backend? Juergen
On 23/03/2017 14:55, Juergen Gross wrote: > On 23/03/17 14:00, Greg Kurz wrote: >> On Mon, 20 Mar 2017 11:19:05 -0700 >> Stefano Stabellini <sstabellini@kernel.org> wrote: >> >>> Do not use the ring.h header installed on the system. Instead, import >>> the header into the QEMU codebase. This avoids problems when QEMU is >>> built against a Xen version too old to provide all the ring macros. >>> >>> Signed-off-by: Stefano Stabellini <stefano@aporeto.com> >>> Reviewed-by: Greg Kurz <groug@kaod.org> >>> CC: anthony.perard@citrix.com >>> CC: jgross@suse.com >>> --- >>> NB: The new macros have not been committed to Xen yet. Do not apply this >>> patch until they do. >>> --- >> >> Looking at your other series for the kernel part of this feature: >> >> https://lkml.org/lkml/2017/3/22/761 >> >> I realize that the ring.h header from Xen also exists in the kernel tree... >> >> Shouldn't all the code that can be used in both kernel and userspace go to a >> header file under include/uapi in the kernel tree ? And then we would import >> it under include/standard-headers/linux in the QEMU tree and we could keep it >> in sync using scripts/update-linux-headers.sh. >> >> Cc'ing Paolo for insights. > > As Xen isn't part of the kernel we don't want that. You can use and/or > build qemu with xen-9pfs backend support on an old Linux kernel without > the related frontend. As long as the header changes rarely, I guess it's fine not to go through update-linux-headers.sh. Paolo > OTOH I don't see the advantage of not using the headers from Xen. This > is working for qdisk and pvusb backends and for all the Xen libraries. > Do you expect the 9pfs backend to be used for a qemu version built > against a Xen version not supporting that backend? > > > Juergen >
On Thu, 23 Mar 2017 14:55:26 +0100 Juergen Gross <jgross@suse.com> wrote: > On 23/03/17 14:00, Greg Kurz wrote: > > On Mon, 20 Mar 2017 11:19:05 -0700 > > Stefano Stabellini <sstabellini@kernel.org> wrote: > > > >> Do not use the ring.h header installed on the system. Instead, import > >> the header into the QEMU codebase. This avoids problems when QEMU is > >> built against a Xen version too old to provide all the ring macros. > >> > >> Signed-off-by: Stefano Stabellini <stefano@aporeto.com> > >> Reviewed-by: Greg Kurz <groug@kaod.org> > >> CC: anthony.perard@citrix.com > >> CC: jgross@suse.com > >> --- > >> NB: The new macros have not been committed to Xen yet. Do not apply this > >> patch until they do. > >> --- > > > > Looking at your other series for the kernel part of this feature: > > > > https://lkml.org/lkml/2017/3/22/761 > > > > I realize that the ring.h header from Xen also exists in the kernel tree... > > > > Shouldn't all the code that can be used in both kernel and userspace go to a > > header file under include/uapi in the kernel tree ? And then we would import > > it under include/standard-headers/linux in the QEMU tree and we could keep it > > in sync using scripts/update-linux-headers.sh. > > > > Cc'ing Paolo for insights. > > As Xen isn't part of the kernel we don't want that. You can use and/or > build qemu with xen-9pfs backend support on an old Linux kernel without > the related frontend. > Ok. > OTOH I don't see the advantage of not using the headers from Xen. This > is working for qdisk and pvusb backends and for all the Xen libraries. > Do you expect the 9pfs backend to be used for a qemu version built > against a Xen version not supporting that backend? > Not specifically, I was just feeling dubitative about that Xen header being manually copied to the kernel tree and also to the QEMU tree. > > Juergen >
On Thu, 23 Mar 2017 15:19:28 +0100 Paolo Bonzini <pbonzini@redhat.com> wrote: > On 23/03/2017 14:55, Juergen Gross wrote: > > On 23/03/17 14:00, Greg Kurz wrote: > >> On Mon, 20 Mar 2017 11:19:05 -0700 > >> Stefano Stabellini <sstabellini@kernel.org> wrote: > >> > >>> Do not use the ring.h header installed on the system. Instead, import > >>> the header into the QEMU codebase. This avoids problems when QEMU is > >>> built against a Xen version too old to provide all the ring macros. > >>> > >>> Signed-off-by: Stefano Stabellini <stefano@aporeto.com> > >>> Reviewed-by: Greg Kurz <groug@kaod.org> > >>> CC: anthony.perard@citrix.com > >>> CC: jgross@suse.com > >>> --- > >>> NB: The new macros have not been committed to Xen yet. Do not apply this > >>> patch until they do. > >>> --- > >> > >> Looking at your other series for the kernel part of this feature: > >> > >> https://lkml.org/lkml/2017/3/22/761 > >> > >> I realize that the ring.h header from Xen also exists in the kernel tree... > >> > >> Shouldn't all the code that can be used in both kernel and userspace go to a > >> header file under include/uapi in the kernel tree ? And then we would import > >> it under include/standard-headers/linux in the QEMU tree and we could keep it > >> in sync using scripts/update-linux-headers.sh. > >> > >> Cc'ing Paolo for insights. > > > > As Xen isn't part of the kernel we don't want that. You can use and/or > > build qemu with xen-9pfs backend support on an old Linux kernel without > > the related frontend. > > As long as the header changes rarely, I guess it's fine not to go > through update-linux-headers.sh. > > Paolo > If the header (or at least the parts that are relevant to QEMU) are stable then my questioning can be ignored :) Thanks. -- Greg > > OTOH I don't see the advantage of not using the headers from Xen. This > > is working for qdisk and pvusb backends and for all the Xen libraries. > > Do you expect the 9pfs backend to be used for a qemu version built > > against a Xen version not supporting that backend? > > > > > > Juergen > >
On Thu, 23 Mar 2017, Paolo Bonzini wrote: > On 23/03/2017 14:55, Juergen Gross wrote: > > On 23/03/17 14:00, Greg Kurz wrote: > >> On Mon, 20 Mar 2017 11:19:05 -0700 > >> Stefano Stabellini <sstabellini@kernel.org> wrote: > >> > >>> Do not use the ring.h header installed on the system. Instead, import > >>> the header into the QEMU codebase. This avoids problems when QEMU is > >>> built against a Xen version too old to provide all the ring macros. > >>> > >>> Signed-off-by: Stefano Stabellini <stefano@aporeto.com> > >>> Reviewed-by: Greg Kurz <groug@kaod.org> > >>> CC: anthony.perard@citrix.com > >>> CC: jgross@suse.com > >>> --- > >>> NB: The new macros have not been committed to Xen yet. Do not apply this > >>> patch until they do. > >>> --- > >> > >> Looking at your other series for the kernel part of this feature: > >> > >> https://lkml.org/lkml/2017/3/22/761 > >> > >> I realize that the ring.h header from Xen also exists in the kernel tree... > >> > >> Shouldn't all the code that can be used in both kernel and userspace go to a > >> header file under include/uapi in the kernel tree ? And then we would import > >> it under include/standard-headers/linux in the QEMU tree and we could keep it > >> in sync using scripts/update-linux-headers.sh. > >> > >> Cc'ing Paolo for insights. > > > > As Xen isn't part of the kernel we don't want that. You can use and/or > > build qemu with xen-9pfs backend support on an old Linux kernel without > > the related frontend. > > As long as the header changes rarely, I guess it's fine not to go > through update-linux-headers.sh. Very rarely, last time ring.h was changed was 2015, and to introduce a new macro (which we don't necessarily need in QEMU). > > OTOH I don't see the advantage of not using the headers from Xen. This > > is working for qdisk and pvusb backends and for all the Xen libraries. > > Do you expect the 9pfs backend to be used for a qemu version built > > against a Xen version not supporting that backend? Yes, I think that is entirely possible: Xen and QEMU versions can mix and match. Keeping in mind that the 9pfs backend has actually no build dependencies on Xen, except for these new ring.h macros, we have the following options: 1) we build the 9pfs backend only for Xen >= 4.9, because of the new macros in ring.h that we need 2) we import ring.h into QEMU so that ring.h it's not a build dependency anymore, then we can build the 9pfs backend against any Xen versions 3) we don't use the new macros, we write our own in QEMU, so that there is no build dependency and we can build the 9pfs backend against any Xen versions I think they are all acceptable options. I went with 2) because I thought it provides the best trade-off.
On 23/03/17 19:22, Stefano Stabellini wrote: > On Thu, 23 Mar 2017, Paolo Bonzini wrote: >> On 23/03/2017 14:55, Juergen Gross wrote: >>> On 23/03/17 14:00, Greg Kurz wrote: >>>> On Mon, 20 Mar 2017 11:19:05 -0700 >>>> Stefano Stabellini <sstabellini@kernel.org> wrote: >>>> >>>>> Do not use the ring.h header installed on the system. Instead, import >>>>> the header into the QEMU codebase. This avoids problems when QEMU is >>>>> built against a Xen version too old to provide all the ring macros. >>>>> >>>>> Signed-off-by: Stefano Stabellini <stefano@aporeto.com> >>>>> Reviewed-by: Greg Kurz <groug@kaod.org> >>>>> CC: anthony.perard@citrix.com >>>>> CC: jgross@suse.com >>>>> --- >>>>> NB: The new macros have not been committed to Xen yet. Do not apply this >>>>> patch until they do. >>>>> --- >>>> >>>> Looking at your other series for the kernel part of this feature: >>>> >>>> https://lkml.org/lkml/2017/3/22/761 >>>> >>>> I realize that the ring.h header from Xen also exists in the kernel tree... >>>> >>>> Shouldn't all the code that can be used in both kernel and userspace go to a >>>> header file under include/uapi in the kernel tree ? And then we would import >>>> it under include/standard-headers/linux in the QEMU tree and we could keep it >>>> in sync using scripts/update-linux-headers.sh. >>>> >>>> Cc'ing Paolo for insights. >>> >>> As Xen isn't part of the kernel we don't want that. You can use and/or >>> build qemu with xen-9pfs backend support on an old Linux kernel without >>> the related frontend. >> >> As long as the header changes rarely, I guess it's fine not to go >> through update-linux-headers.sh. > > Very rarely, last time ring.h was changed was 2015, and to introduce a > new macro (which we don't necessarily need in QEMU). > > >>> OTOH I don't see the advantage of not using the headers from Xen. This >>> is working for qdisk and pvusb backends and for all the Xen libraries. >>> Do you expect the 9pfs backend to be used for a qemu version built >>> against a Xen version not supporting that backend? > > Yes, I think that is entirely possible: Xen and QEMU versions can mix > and match. > > Keeping in mind that the 9pfs backend has actually no build dependencies > on Xen, except for these new ring.h macros, we have the following > options: > > 1) we build the 9pfs backend only for Xen >= 4.9, because of the new > macros in ring.h that we need Right. You have sent 9pfs support patches for Xen tools. So obviously you need a proper Xen version to use 9pfs. Why not build qemu against it? Do you really expect a new Xen being used with an old qemu while wanting to use new features? That makes no sense for me. Juergen PS: added xen-devel as this should be discussed there, too.
On Fri, 24 Mar 2017, Juergen Gross wrote: > On 23/03/17 19:22, Stefano Stabellini wrote: > > On Thu, 23 Mar 2017, Paolo Bonzini wrote: > >> On 23/03/2017 14:55, Juergen Gross wrote: > >>> On 23/03/17 14:00, Greg Kurz wrote: > >>>> On Mon, 20 Mar 2017 11:19:05 -0700 > >>>> Stefano Stabellini <sstabellini@kernel.org> wrote: > >>>> > >>>>> Do not use the ring.h header installed on the system. Instead, import > >>>>> the header into the QEMU codebase. This avoids problems when QEMU is > >>>>> built against a Xen version too old to provide all the ring macros. > >>>>> > >>>>> Signed-off-by: Stefano Stabellini <stefano@aporeto.com> > >>>>> Reviewed-by: Greg Kurz <groug@kaod.org> > >>>>> CC: anthony.perard@citrix.com > >>>>> CC: jgross@suse.com > >>>>> --- > >>>>> NB: The new macros have not been committed to Xen yet. Do not apply this > >>>>> patch until they do. > >>>>> --- > >>>> > >>>> Looking at your other series for the kernel part of this feature: > >>>> > >>>> https://lkml.org/lkml/2017/3/22/761 > >>>> > >>>> I realize that the ring.h header from Xen also exists in the kernel tree... > >>>> > >>>> Shouldn't all the code that can be used in both kernel and userspace go to a > >>>> header file under include/uapi in the kernel tree ? And then we would import > >>>> it under include/standard-headers/linux in the QEMU tree and we could keep it > >>>> in sync using scripts/update-linux-headers.sh. > >>>> > >>>> Cc'ing Paolo for insights. > >>> > >>> As Xen isn't part of the kernel we don't want that. You can use and/or > >>> build qemu with xen-9pfs backend support on an old Linux kernel without > >>> the related frontend. > >> > >> As long as the header changes rarely, I guess it's fine not to go > >> through update-linux-headers.sh. > > > > Very rarely, last time ring.h was changed was 2015, and to introduce a > > new macro (which we don't necessarily need in QEMU). > > > > > >>> OTOH I don't see the advantage of not using the headers from Xen. This > >>> is working for qdisk and pvusb backends and for all the Xen libraries. > >>> Do you expect the 9pfs backend to be used for a qemu version built > >>> against a Xen version not supporting that backend? > > > > Yes, I think that is entirely possible: Xen and QEMU versions can mix > > and match. > > > > Keeping in mind that the 9pfs backend has actually no build dependencies > > on Xen, except for these new ring.h macros, we have the following > > options: > > > > 1) we build the 9pfs backend only for Xen >= 4.9, because of the new > > macros in ring.h that we need > > Right. You have sent 9pfs support patches for Xen tools. So obviously > you need a proper Xen version to use 9pfs. Why not build qemu against > it? Do you really expect a new Xen being used with an old qemu while > wanting to use new features? That makes no sense for me. Tools support is needed to setup the frontend/backend connection as usual, but that's not a requirement for building the 9pfs backend. In fact, the backend doesn't need any tools support for it to work. The macro themselves are just a convenience - the backend would work just fine without them. Why restrict the QEMU build gratuitously?
On 24/03/17 18:37, Stefano Stabellini wrote: > On Fri, 24 Mar 2017, Juergen Gross wrote: >> On 23/03/17 19:22, Stefano Stabellini wrote: >>> On Thu, 23 Mar 2017, Paolo Bonzini wrote: >>>> On 23/03/2017 14:55, Juergen Gross wrote: >>>>> On 23/03/17 14:00, Greg Kurz wrote: >>>>>> On Mon, 20 Mar 2017 11:19:05 -0700 >>>>>> Stefano Stabellini <sstabellini@kernel.org> wrote: >>>>>> >>>>>>> Do not use the ring.h header installed on the system. Instead, import >>>>>>> the header into the QEMU codebase. This avoids problems when QEMU is >>>>>>> built against a Xen version too old to provide all the ring macros. >>>>>>> >>>>>>> Signed-off-by: Stefano Stabellini <stefano@aporeto.com> >>>>>>> Reviewed-by: Greg Kurz <groug@kaod.org> >>>>>>> CC: anthony.perard@citrix.com >>>>>>> CC: jgross@suse.com >>>>>>> --- >>>>>>> NB: The new macros have not been committed to Xen yet. Do not apply this >>>>>>> patch until they do. >>>>>>> --- >>>>>> >>>>>> Looking at your other series for the kernel part of this feature: >>>>>> >>>>>> https://lkml.org/lkml/2017/3/22/761 >>>>>> >>>>>> I realize that the ring.h header from Xen also exists in the kernel tree... >>>>>> >>>>>> Shouldn't all the code that can be used in both kernel and userspace go to a >>>>>> header file under include/uapi in the kernel tree ? And then we would import >>>>>> it under include/standard-headers/linux in the QEMU tree and we could keep it >>>>>> in sync using scripts/update-linux-headers.sh. >>>>>> >>>>>> Cc'ing Paolo for insights. >>>>> >>>>> As Xen isn't part of the kernel we don't want that. You can use and/or >>>>> build qemu with xen-9pfs backend support on an old Linux kernel without >>>>> the related frontend. >>>> >>>> As long as the header changes rarely, I guess it's fine not to go >>>> through update-linux-headers.sh. >>> >>> Very rarely, last time ring.h was changed was 2015, and to introduce a >>> new macro (which we don't necessarily need in QEMU). >>> >>> >>>>> OTOH I don't see the advantage of not using the headers from Xen. This >>>>> is working for qdisk and pvusb backends and for all the Xen libraries. >>>>> Do you expect the 9pfs backend to be used for a qemu version built >>>>> against a Xen version not supporting that backend? >>> >>> Yes, I think that is entirely possible: Xen and QEMU versions can mix >>> and match. >>> >>> Keeping in mind that the 9pfs backend has actually no build dependencies >>> on Xen, except for these new ring.h macros, we have the following >>> options: >>> >>> 1) we build the 9pfs backend only for Xen >= 4.9, because of the new >>> macros in ring.h that we need >> >> Right. You have sent 9pfs support patches for Xen tools. So obviously >> you need a proper Xen version to use 9pfs. Why not build qemu against >> it? Do you really expect a new Xen being used with an old qemu while >> wanting to use new features? That makes no sense for me. > > Tools support is needed to setup the frontend/backend connection as > usual, but that's not a requirement for building the 9pfs backend. In > fact, the backend doesn't need any tools support for it to work. The > macro themselves are just a convenience - the backend would work just > fine without them. Why restrict the QEMU build gratuitously? You are duplicating a header without any real benefit I can see. This is adding future work for keeping both versions of the header in sync. In which scenario would you want qemu to support xen-9pfs without being built against a Xen version supporting xen-9pfs? I am not completely against copying the header, I just don't see an advantage for any distro or user in doing it. Juergen
On Mon, 27 Mar 2017, Juergen Gross wrote: > On 24/03/17 18:37, Stefano Stabellini wrote: > > On Fri, 24 Mar 2017, Juergen Gross wrote: > >> On 23/03/17 19:22, Stefano Stabellini wrote: > >>> On Thu, 23 Mar 2017, Paolo Bonzini wrote: > >>>> On 23/03/2017 14:55, Juergen Gross wrote: > >>>>> On 23/03/17 14:00, Greg Kurz wrote: > >>>>>> On Mon, 20 Mar 2017 11:19:05 -0700 > >>>>>> Stefano Stabellini <sstabellini@kernel.org> wrote: > >>>>>> > >>>>>>> Do not use the ring.h header installed on the system. Instead, import > >>>>>>> the header into the QEMU codebase. This avoids problems when QEMU is > >>>>>>> built against a Xen version too old to provide all the ring macros. > >>>>>>> > >>>>>>> Signed-off-by: Stefano Stabellini <stefano@aporeto.com> > >>>>>>> Reviewed-by: Greg Kurz <groug@kaod.org> > >>>>>>> CC: anthony.perard@citrix.com > >>>>>>> CC: jgross@suse.com > >>>>>>> --- > >>>>>>> NB: The new macros have not been committed to Xen yet. Do not apply this > >>>>>>> patch until they do. > >>>>>>> --- > >>>>>> > >>>>>> Looking at your other series for the kernel part of this feature: > >>>>>> > >>>>>> https://lkml.org/lkml/2017/3/22/761 > >>>>>> > >>>>>> I realize that the ring.h header from Xen also exists in the kernel tree... > >>>>>> > >>>>>> Shouldn't all the code that can be used in both kernel and userspace go to a > >>>>>> header file under include/uapi in the kernel tree ? And then we would import > >>>>>> it under include/standard-headers/linux in the QEMU tree and we could keep it > >>>>>> in sync using scripts/update-linux-headers.sh. > >>>>>> > >>>>>> Cc'ing Paolo for insights. > >>>>> > >>>>> As Xen isn't part of the kernel we don't want that. You can use and/or > >>>>> build qemu with xen-9pfs backend support on an old Linux kernel without > >>>>> the related frontend. > >>>> > >>>> As long as the header changes rarely, I guess it's fine not to go > >>>> through update-linux-headers.sh. > >>> > >>> Very rarely, last time ring.h was changed was 2015, and to introduce a > >>> new macro (which we don't necessarily need in QEMU). > >>> > >>> > >>>>> OTOH I don't see the advantage of not using the headers from Xen. This > >>>>> is working for qdisk and pvusb backends and for all the Xen libraries. > >>>>> Do you expect the 9pfs backend to be used for a qemu version built > >>>>> against a Xen version not supporting that backend? > >>> > >>> Yes, I think that is entirely possible: Xen and QEMU versions can mix > >>> and match. > >>> > >>> Keeping in mind that the 9pfs backend has actually no build dependencies > >>> on Xen, except for these new ring.h macros, we have the following > >>> options: > >>> > >>> 1) we build the 9pfs backend only for Xen >= 4.9, because of the new > >>> macros in ring.h that we need > >> > >> Right. You have sent 9pfs support patches for Xen tools. So obviously > >> you need a proper Xen version to use 9pfs. Why not build qemu against > >> it? Do you really expect a new Xen being used with an old qemu while > >> wanting to use new features? That makes no sense for me. > > > > Tools support is needed to setup the frontend/backend connection as > > usual, but that's not a requirement for building the 9pfs backend. In > > fact, the backend doesn't need any tools support for it to work. The > > macro themselves are just a convenience - the backend would work just > > fine without them. Why restrict the QEMU build gratuitously? > > You are duplicating a header without any real benefit I can see. This > is adding future work for keeping both versions of the header in sync. > > In which scenario would you want qemu to support xen-9pfs without being > built against a Xen version supporting xen-9pfs? > > I am not completely against copying the header, I just don't see an > advantage for any distro or user in doing it. I understand your point of view, and honestly it wouldn't be a problem doing it the way you suggested either. However, I think that going forward it will be less of a maintenance pain to keep ring.h in sync, compared to maintaining a versioned build dependency between Xen and QEMU for the compilation of one PV backend. We do have version checks in QEMU for Xen compatibility, but not for PV backends or the xenpv machine yet.
On 28/03/17 00:48, Stefano Stabellini wrote: > On Mon, 27 Mar 2017, Juergen Gross wrote: >> On 24/03/17 18:37, Stefano Stabellini wrote: >>> On Fri, 24 Mar 2017, Juergen Gross wrote: >>>> On 23/03/17 19:22, Stefano Stabellini wrote: >>>>> On Thu, 23 Mar 2017, Paolo Bonzini wrote: >>>>>> On 23/03/2017 14:55, Juergen Gross wrote: >>>>>>> On 23/03/17 14:00, Greg Kurz wrote: >>>>>>>> On Mon, 20 Mar 2017 11:19:05 -0700 >>>>>>>> Stefano Stabellini <sstabellini@kernel.org> wrote: >>>>>>>> >>>>>>>>> Do not use the ring.h header installed on the system. Instead, import >>>>>>>>> the header into the QEMU codebase. This avoids problems when QEMU is >>>>>>>>> built against a Xen version too old to provide all the ring macros. >>>>>>>>> >>>>>>>>> Signed-off-by: Stefano Stabellini <stefano@aporeto.com> >>>>>>>>> Reviewed-by: Greg Kurz <groug@kaod.org> >>>>>>>>> CC: anthony.perard@citrix.com >>>>>>>>> CC: jgross@suse.com >>>>>>>>> --- >>>>>>>>> NB: The new macros have not been committed to Xen yet. Do not apply this >>>>>>>>> patch until they do. >>>>>>>>> --- >>>>>>>> >>>>>>>> Looking at your other series for the kernel part of this feature: >>>>>>>> >>>>>>>> https://lkml.org/lkml/2017/3/22/761 >>>>>>>> >>>>>>>> I realize that the ring.h header from Xen also exists in the kernel tree... >>>>>>>> >>>>>>>> Shouldn't all the code that can be used in both kernel and userspace go to a >>>>>>>> header file under include/uapi in the kernel tree ? And then we would import >>>>>>>> it under include/standard-headers/linux in the QEMU tree and we could keep it >>>>>>>> in sync using scripts/update-linux-headers.sh. >>>>>>>> >>>>>>>> Cc'ing Paolo for insights. >>>>>>> >>>>>>> As Xen isn't part of the kernel we don't want that. You can use and/or >>>>>>> build qemu with xen-9pfs backend support on an old Linux kernel without >>>>>>> the related frontend. >>>>>> >>>>>> As long as the header changes rarely, I guess it's fine not to go >>>>>> through update-linux-headers.sh. >>>>> >>>>> Very rarely, last time ring.h was changed was 2015, and to introduce a >>>>> new macro (which we don't necessarily need in QEMU). >>>>> >>>>> >>>>>>> OTOH I don't see the advantage of not using the headers from Xen. This >>>>>>> is working for qdisk and pvusb backends and for all the Xen libraries. >>>>>>> Do you expect the 9pfs backend to be used for a qemu version built >>>>>>> against a Xen version not supporting that backend? >>>>> >>>>> Yes, I think that is entirely possible: Xen and QEMU versions can mix >>>>> and match. >>>>> >>>>> Keeping in mind that the 9pfs backend has actually no build dependencies >>>>> on Xen, except for these new ring.h macros, we have the following >>>>> options: >>>>> >>>>> 1) we build the 9pfs backend only for Xen >= 4.9, because of the new >>>>> macros in ring.h that we need >>>> >>>> Right. You have sent 9pfs support patches for Xen tools. So obviously >>>> you need a proper Xen version to use 9pfs. Why not build qemu against >>>> it? Do you really expect a new Xen being used with an old qemu while >>>> wanting to use new features? That makes no sense for me. >>> >>> Tools support is needed to setup the frontend/backend connection as >>> usual, but that's not a requirement for building the 9pfs backend. In >>> fact, the backend doesn't need any tools support for it to work. The >>> macro themselves are just a convenience - the backend would work just >>> fine without them. Why restrict the QEMU build gratuitously? >> >> You are duplicating a header without any real benefit I can see. This >> is adding future work for keeping both versions of the header in sync. >> >> In which scenario would you want qemu to support xen-9pfs without being >> built against a Xen version supporting xen-9pfs? >> >> I am not completely against copying the header, I just don't see an >> advantage for any distro or user in doing it. > > I understand your point of view, and honestly it wouldn't be a problem > doing it the way you suggested either. However, I think that going > forward it will be less of a maintenance pain to keep ring.h in sync, > compared to maintaining a versioned build dependency between Xen and > QEMU for the compilation of one PV backend. We do have version checks > in QEMU for Xen compatibility, but not for PV backends or the xenpv > machine yet. For the pvUSB backend I just used a mandatory macro from the header for the #ifdef. The backend will signal support when it was defined during build and will refuse initialization otherwise. Xen tools are able to recoginze qemu support of the backend by looking into Xenstore. Juergen
diff --git a/hw/block/xen_blkif.h b/hw/block/xen_blkif.h index 3300b6f..3e6e1ea 100644 --- a/hw/block/xen_blkif.h +++ b/hw/block/xen_blkif.h @@ -1,7 +1,7 @@ #ifndef XEN_BLKIF_H #define XEN_BLKIF_H -#include <xen/io/ring.h> +#include "hw/xen/io/ring.h" #include <xen/io/blkif.h> #include <xen/io/protocols.h> diff --git a/hw/usb/xen-usb.c b/hw/usb/xen-usb.c index 8e676e6..370b3d9 100644 --- a/hw/usb/xen-usb.c +++ b/hw/usb/xen-usb.c @@ -33,7 +33,7 @@ #include "qapi/qmp/qint.h" #include "qapi/qmp/qstring.h" -#include <xen/io/ring.h> +#include "hw/xen/io/ring.h" #include <xen/io/usbif.h> /* diff --git a/include/hw/xen/io/ring.h b/include/hw/xen/io/ring.h new file mode 100644 index 0000000..cf01fc3 --- /dev/null +++ b/include/hw/xen/io/ring.h @@ -0,0 +1,455 @@ +/****************************************************************************** + * ring.h + * + * Shared producer-consumer ring macros. + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to + * deal in the Software without restriction, including without limitation the + * rights to use, copy, modify, merge, publish, distribute, sublicense, and/or + * sell copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER + * DEALINGS IN THE SOFTWARE. + * + * Tim Deegan and Andrew Warfield November 2004. + */ + +#ifndef __XEN_PUBLIC_IO_RING_H__ +#define __XEN_PUBLIC_IO_RING_H__ + +#if __XEN_INTERFACE_VERSION__ < 0x00030208 +#define xen_mb() mb() +#define xen_rmb() rmb() +#define xen_wmb() wmb() +#endif + +typedef unsigned int RING_IDX; + +/* Round a 32-bit unsigned constant down to the nearest power of two. */ +#define __RD2(_x) (((_x) & 0x00000002) ? 0x2 : ((_x) & 0x1)) +#define __RD4(_x) (((_x) & 0x0000000c) ? __RD2((_x)>>2)<<2 : __RD2(_x)) +#define __RD8(_x) (((_x) & 0x000000f0) ? __RD4((_x)>>4)<<4 : __RD4(_x)) +#define __RD16(_x) (((_x) & 0x0000ff00) ? __RD8((_x)>>8)<<8 : __RD8(_x)) +#define __RD32(_x) (((_x) & 0xffff0000) ? __RD16((_x)>>16)<<16 : __RD16(_x)) + +/* + * Calculate size of a shared ring, given the total available space for the + * ring and indexes (_sz), and the name tag of the request/response structure. + * A ring contains as many entries as will fit, rounded down to the nearest + * power of two (so we can mask with (size-1) to loop around). + */ +#define __CONST_RING_SIZE(_s, _sz) \ + (__RD32(((_sz) - offsetof(struct _s##_sring, ring)) / \ + sizeof(((struct _s##_sring *)0)->ring[0]))) +/* + * The same for passing in an actual pointer instead of a name tag. + */ +#define __RING_SIZE(_s, _sz) \ + (__RD32(((_sz) - (long)(_s)->ring + (long)(_s)) / sizeof((_s)->ring[0]))) + +/* + * Macros to make the correct C datatypes for a new kind of ring. + * + * To make a new ring datatype, you need to have two message structures, + * let's say request_t, and response_t already defined. + * + * In a header where you want the ring datatype declared, you then do: + * + * DEFINE_RING_TYPES(mytag, request_t, response_t); + * + * These expand out to give you a set of types, as you can see below. + * The most important of these are: + * + * mytag_sring_t - The shared ring. + * mytag_front_ring_t - The 'front' half of the ring. + * mytag_back_ring_t - The 'back' half of the ring. + * + * To initialize a ring in your code you need to know the location and size + * of the shared memory area (PAGE_SIZE, for instance). To initialise + * the front half: + * + * mytag_front_ring_t front_ring; + * SHARED_RING_INIT((mytag_sring_t *)shared_page); + * FRONT_RING_INIT(&front_ring, (mytag_sring_t *)shared_page, PAGE_SIZE); + * + * Initializing the back follows similarly (note that only the front + * initializes the shared ring): + * + * mytag_back_ring_t back_ring; + * BACK_RING_INIT(&back_ring, (mytag_sring_t *)shared_page, PAGE_SIZE); + */ + +#define DEFINE_RING_TYPES(__name, __req_t, __rsp_t) \ + \ +/* Shared ring entry */ \ +union __name##_sring_entry { \ + __req_t req; \ + __rsp_t rsp; \ +}; \ + \ +/* Shared ring page */ \ +struct __name##_sring { \ + RING_IDX req_prod, req_event; \ + RING_IDX rsp_prod, rsp_event; \ + union { \ + struct { \ + uint8_t smartpoll_active; \ + } netif; \ + struct { \ + uint8_t msg; \ + } tapif_user; \ + uint8_t pvt_pad[4]; \ + } pvt; \ + uint8_t __pad[44]; \ + union __name##_sring_entry ring[1]; /* variable-length */ \ +}; \ + \ +/* "Front" end's private variables */ \ +struct __name##_front_ring { \ + RING_IDX req_prod_pvt; \ + RING_IDX rsp_cons; \ + unsigned int nr_ents; \ + struct __name##_sring *sring; \ +}; \ + \ +/* "Back" end's private variables */ \ +struct __name##_back_ring { \ + RING_IDX rsp_prod_pvt; \ + RING_IDX req_cons; \ + unsigned int nr_ents; \ + struct __name##_sring *sring; \ +}; \ + \ +/* Syntactic sugar */ \ +typedef struct __name##_sring __name##_sring_t; \ +typedef struct __name##_front_ring __name##_front_ring_t; \ +typedef struct __name##_back_ring __name##_back_ring_t + +/* + * Macros for manipulating rings. + * + * FRONT_RING_whatever works on the "front end" of a ring: here + * requests are pushed on to the ring and responses taken off it. + * + * BACK_RING_whatever works on the "back end" of a ring: here + * requests are taken off the ring and responses put on. + * + * N.B. these macros do NO INTERLOCKS OR FLOW CONTROL. + * This is OK in 1-for-1 request-response situations where the + * requestor (front end) never has more than RING_SIZE()-1 + * outstanding requests. + */ + +/* Initialising empty rings */ +#define SHARED_RING_INIT(_s) do { \ + (_s)->req_prod = (_s)->rsp_prod = 0; \ + (_s)->req_event = (_s)->rsp_event = 1; \ + (void)memset((_s)->pvt.pvt_pad, 0, sizeof((_s)->pvt.pvt_pad)); \ + (void)memset((_s)->__pad, 0, sizeof((_s)->__pad)); \ +} while(0) + +#define FRONT_RING_INIT(_r, _s, __size) do { \ + (_r)->req_prod_pvt = 0; \ + (_r)->rsp_cons = 0; \ + (_r)->nr_ents = __RING_SIZE(_s, __size); \ + (_r)->sring = (_s); \ +} while (0) + +#define BACK_RING_INIT(_r, _s, __size) do { \ + (_r)->rsp_prod_pvt = 0; \ + (_r)->req_cons = 0; \ + (_r)->nr_ents = __RING_SIZE(_s, __size); \ + (_r)->sring = (_s); \ +} while (0) + +/* How big is this ring? */ +#define RING_SIZE(_r) \ + ((_r)->nr_ents) + +/* Number of free requests (for use on front side only). */ +#define RING_FREE_REQUESTS(_r) \ + (RING_SIZE(_r) - ((_r)->req_prod_pvt - (_r)->rsp_cons)) + +/* Test if there is an empty slot available on the front ring. + * (This is only meaningful from the front. ) + */ +#define RING_FULL(_r) \ + (RING_FREE_REQUESTS(_r) == 0) + +/* Test if there are outstanding messages to be processed on a ring. */ +#define RING_HAS_UNCONSUMED_RESPONSES(_r) \ + ((_r)->sring->rsp_prod - (_r)->rsp_cons) + +#ifdef __GNUC__ +#define RING_HAS_UNCONSUMED_REQUESTS(_r) ({ \ + unsigned int req = (_r)->sring->req_prod - (_r)->req_cons; \ + unsigned int rsp = RING_SIZE(_r) - \ + ((_r)->req_cons - (_r)->rsp_prod_pvt); \ + req < rsp ? req : rsp; \ +}) +#else +/* Same as above, but without the nice GCC ({ ... }) syntax. */ +#define RING_HAS_UNCONSUMED_REQUESTS(_r) \ + ((((_r)->sring->req_prod - (_r)->req_cons) < \ + (RING_SIZE(_r) - ((_r)->req_cons - (_r)->rsp_prod_pvt))) ? \ + ((_r)->sring->req_prod - (_r)->req_cons) : \ + (RING_SIZE(_r) - ((_r)->req_cons - (_r)->rsp_prod_pvt))) +#endif + +/* Direct access to individual ring elements, by index. */ +#define RING_GET_REQUEST(_r, _idx) \ + (&((_r)->sring->ring[((_idx) & (RING_SIZE(_r) - 1))].req)) + +/* + * Get a local copy of a request. + * + * Use this in preference to RING_GET_REQUEST() so all processing is + * done on a local copy that cannot be modified by the other end. + * + * Note that https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58145 may cause this + * to be ineffective where _req is a struct which consists of only bitfields. + */ +#define RING_COPY_REQUEST(_r, _idx, _req) do { \ + /* Use volatile to force the copy into _req. */ \ + *(_req) = *(volatile typeof(_req))RING_GET_REQUEST(_r, _idx); \ +} while (0) + +#define RING_GET_RESPONSE(_r, _idx) \ + (&((_r)->sring->ring[((_idx) & (RING_SIZE(_r) - 1))].rsp)) + +/* Loop termination condition: Would the specified index overflow the ring? */ +#define RING_REQUEST_CONS_OVERFLOW(_r, _cons) \ + (((_cons) - (_r)->rsp_prod_pvt) >= RING_SIZE(_r)) + +/* Ill-behaved frontend determination: Can there be this many requests? */ +#define RING_REQUEST_PROD_OVERFLOW(_r, _prod) \ + (((_prod) - (_r)->rsp_prod_pvt) > RING_SIZE(_r)) + +#define RING_PUSH_REQUESTS(_r) do { \ + xen_wmb(); /* back sees requests /before/ updated producer index */ \ + (_r)->sring->req_prod = (_r)->req_prod_pvt; \ +} while (0) + +#define RING_PUSH_RESPONSES(_r) do { \ + xen_wmb(); /* front sees resps /before/ updated producer index */ \ + (_r)->sring->rsp_prod = (_r)->rsp_prod_pvt; \ +} while (0) + +/* + * Notification hold-off (req_event and rsp_event): + * + * When queueing requests or responses on a shared ring, it may not always be + * necessary to notify the remote end. For example, if requests are in flight + * in a backend, the front may be able to queue further requests without + * notifying the back (if the back checks for new requests when it queues + * responses). + * + * When enqueuing requests or responses: + * + * Use RING_PUSH_{REQUESTS,RESPONSES}_AND_CHECK_NOTIFY(). The second argument + * is a boolean return value. True indicates that the receiver requires an + * asynchronous notification. + * + * After dequeuing requests or responses (before sleeping the connection): + * + * Use RING_FINAL_CHECK_FOR_REQUESTS() or RING_FINAL_CHECK_FOR_RESPONSES(). + * The second argument is a boolean return value. True indicates that there + * are pending messages on the ring (i.e., the connection should not be put + * to sleep). + * + * These macros will set the req_event/rsp_event field to trigger a + * notification on the very next message that is enqueued. If you want to + * create batches of work (i.e., only receive a notification after several + * messages have been enqueued) then you will need to create a customised + * version of the FINAL_CHECK macro in your own code, which sets the event + * field appropriately. + */ + +#define RING_PUSH_REQUESTS_AND_CHECK_NOTIFY(_r, _notify) do { \ + RING_IDX __old = (_r)->sring->req_prod; \ + RING_IDX __new = (_r)->req_prod_pvt; \ + xen_wmb(); /* back sees requests /before/ updated producer index */ \ + (_r)->sring->req_prod = __new; \ + xen_mb(); /* back sees new requests /before/ we check req_event */ \ + (_notify) = ((RING_IDX)(__new - (_r)->sring->req_event) < \ + (RING_IDX)(__new - __old)); \ +} while (0) + +#define RING_PUSH_RESPONSES_AND_CHECK_NOTIFY(_r, _notify) do { \ + RING_IDX __old = (_r)->sring->rsp_prod; \ + RING_IDX __new = (_r)->rsp_prod_pvt; \ + xen_wmb(); /* front sees resps /before/ updated producer index */ \ + (_r)->sring->rsp_prod = __new; \ + xen_mb(); /* front sees new resps /before/ we check rsp_event */ \ + (_notify) = ((RING_IDX)(__new - (_r)->sring->rsp_event) < \ + (RING_IDX)(__new - __old)); \ +} while (0) + +#define RING_FINAL_CHECK_FOR_REQUESTS(_r, _work_to_do) do { \ + (_work_to_do) = RING_HAS_UNCONSUMED_REQUESTS(_r); \ + if (_work_to_do) break; \ + (_r)->sring->req_event = (_r)->req_cons + 1; \ + xen_mb(); \ + (_work_to_do) = RING_HAS_UNCONSUMED_REQUESTS(_r); \ +} while (0) + +#define RING_FINAL_CHECK_FOR_RESPONSES(_r, _work_to_do) do { \ + (_work_to_do) = RING_HAS_UNCONSUMED_RESPONSES(_r); \ + if (_work_to_do) break; \ + (_r)->sring->rsp_event = (_r)->rsp_cons + 1; \ + xen_mb(); \ + (_work_to_do) = RING_HAS_UNCONSUMED_RESPONSES(_r); \ +} while (0) + + +/* + * DEFINE_XEN_FLEX_RING_AND_INTF defines two monodirectional rings and + * functions to check if there is data on the ring, and to read and + * write to them. + * + * DEFINE_XEN_FLEX_RING is similar to DEFINE_XEN_FLEX_RING_AND_INTF, but + * does not define the indexes page. As different protocols can have + * extensions to the basic format, this macro allow them to define their + * own struct. + * + * XEN_FLEX_RING_SIZE + * Convenience macro to calculate the size of one of the two rings + * from the overall order. + * + * $NAME_mask + * Function to apply the size mask to an index, to reduce the index + * within the range [0-size]. + * + * $NAME_read_packet + * Function to read data from the ring. The amount of data to read is + * specified by the "size" argument. + * + * $NAME_write_packet + * Function to write data to the ring. The amount of data to write is + * specified by the "size" argument. + * + * $NAME_get_ring_ptr + * Convenience function that returns a pointer to read/write to the + * ring at the right location. + * + * $NAME_data_intf + * Indexes page, shared between frontend and backend. It also + * contains the array of grant refs. + * + * $NAME_queued + * Function to calculate how many bytes are currently on the ring, + * ready to be read. It can also be used to calculate how much free + * space is currently on the ring (ring_size - $NAME_queued()). + */ +#define XEN_FLEX_RING_SIZE(order) \ + (1UL << (order + PAGE_SHIFT - 1)) + +#define DEFINE_XEN_FLEX_RING_AND_INTF(name) \ +struct name##_data_intf { \ + RING_IDX in_cons, in_prod; \ + \ + uint8_t pad1[56]; \ + \ + RING_IDX out_cons, out_prod; \ + \ + uint8_t pad2[56]; \ + \ + RING_IDX ring_order; \ + grant_ref_t ref[]; \ +}; \ +DEFINE_XEN_FLEX_RING(name); + +#define DEFINE_XEN_FLEX_RING(name) \ +static inline RING_IDX name##_mask(RING_IDX idx, RING_IDX ring_size) \ +{ \ + return (idx & (ring_size - 1)); \ +} \ + \ +static inline RING_IDX name##_mask_order(RING_IDX idx, RING_IDX ring_order) \ +{ \ + return (idx & (XEN_FLEX_RING_SIZE(ring_order) - 1)); \ +} \ + \ +static inline unsigned char* name##_get_ring_ptr(unsigned char *buf, \ + RING_IDX idx, \ + RING_IDX ring_order) \ +{ \ + return buf + name##_mask_order(idx, ring_order); \ +} \ + \ +static inline void name##_read_packet(const unsigned char *buf, \ + RING_IDX masked_prod, RING_IDX *masked_cons, \ + RING_IDX ring_size, void *opaque, size_t size) { \ + if (*masked_cons < masked_prod || \ + size <= ring_size - *masked_cons) { \ + memcpy(opaque, buf + *masked_cons, size); \ + } else { \ + memcpy(opaque, buf + *masked_cons, ring_size - *masked_cons); \ + memcpy((unsigned char *)opaque + ring_size - *masked_cons, buf, \ + size - (ring_size - *masked_cons)); \ + } \ + *masked_cons = name##_mask(*masked_cons + size, ring_size); \ +} \ + \ +static inline void name##_write_packet(unsigned char *buf, \ + RING_IDX *masked_prod, RING_IDX masked_cons, \ + RING_IDX ring_size, const void *opaque, size_t size) { \ + if (*masked_prod < masked_cons || \ + size <= ring_size - *masked_prod) { \ + memcpy(buf + *masked_prod, opaque, size); \ + } else { \ + memcpy(buf + *masked_prod, opaque, ring_size - *masked_prod); \ + memcpy(buf, (unsigned char *)opaque + (ring_size - *masked_prod), \ + size - (ring_size - *masked_prod)); \ + } \ + *masked_prod = name##_mask(*masked_prod + size, ring_size); \ +} \ + \ +struct name##_data { \ + unsigned char *in; /* half of the allocation */ \ + unsigned char *out; /* half of the allocation */ \ +}; \ + \ + \ +static inline RING_IDX name##_queued(RING_IDX prod, \ + RING_IDX cons, RING_IDX ring_size) \ +{ \ + RING_IDX size; \ + \ + if (prod == cons) \ + return 0; \ + \ + prod = name##_mask(prod, ring_size); \ + cons = name##_mask(cons, ring_size); \ + \ + if (prod == cons) \ + return ring_size; \ + \ + if (prod > cons) \ + size = prod - cons; \ + else \ + size = ring_size - (cons - prod); \ + return size; \ +}; + +#endif /* __XEN_PUBLIC_IO_RING_H__ */ + +/* + * Local variables: + * mode: C + * c-file-style: "BSD" + * c-basic-offset: 4 + * tab-width: 4 + * indent-tabs-mode: nil + * End: + */