Jan Beulich
2010-Jan-11 14:50 UTC
[Xen-devel] [PATCH, RFC 3/4] qemu: use new (replacement) mmap-batch ioctl
--- 2010-01-06.orig/qemu/hw/xen_common.h 2009-01-27 08:32:14.000000000 +0100
+++ 2010-01-06/qemu/hw/xen_common.h 2010-01-08 17:19:07.000000000 +0100
@@ -30,5 +30,10 @@
# define xen_rmb() rmb()
# define xen_wmb() wmb()
#endif
+#if __XEN_LATEST_INTERFACE_VERSION__ < 0x0003020a
+void * __attribute__((__weak__))
+xc_map_foreign_bulk(int xc_handle, uint32_t dom, int prot,
+ const xen_pfn_t *, int *err, unsigned int num);
+#endif
#endif /* QEMU_HW_XEN_COMMON_H */
--- 2010-01-06.orig/qemu/hw/xen_machine_fv.c 2009-05-19 16:24:10.000000000 +0200
+++ 2010-01-06/qemu/hw/xen_machine_fv.c 2010-01-08 17:24:10.000000000 +0100
@@ -109,6 +109,34 @@ static void qemu_remap_bucket(struct map
for (i = 0; i < MCACHE_BUCKET_SIZE >> XC_PAGE_SHIFT; i++)
pfns[i] = (address_index << (MCACHE_BUCKET_SHIFT-XC_PAGE_SHIFT))
+ i;
+ if (xc_map_foreign_bulk != NULL) {
+ int err[MCACHE_BUCKET_SIZE >> XC_PAGE_SHIFT];
+
+ vaddr_base = xc_map_foreign_bulk(xc_handle, domid,
+ PROT_READ|PROT_WRITE, pfns, err,
+ MCACHE_BUCKET_SIZE >>
XC_PAGE_SHIFT);
+ if (vaddr_base == NULL) {
+ fprintf(logfile, "xc_map_foreign_bulk error %d\n",
errno);
+ exit(-1);
+ }
+
+ entry->vaddr_base = vaddr_base;
+ entry->paddr_index = address_index;
+
+ for (i = 0; i < MCACHE_BUCKET_SIZE >> XC_PAGE_SHIFT;
+ i += BITS_PER_LONG) {
+ unsigned long word = 0;
+ j = ((i + BITS_PER_LONG) > (MCACHE_BUCKET_SIZE >>
XC_PAGE_SHIFT)) ?
+ (MCACHE_BUCKET_SIZE >> XC_PAGE_SHIFT) % BITS_PER_LONG :
+ BITS_PER_LONG;
+ while (j > 0)
+ word = (word << 1) | !err[i + --j];
+ entry->valid_mapping[i / BITS_PER_LONG] = word;
+ }
+
+ return;
+ }
+
vaddr_base = xc_map_foreign_batch(xc_handle, domid, PROT_READ|PROT_WRITE,
pfns, MCACHE_BUCKET_SIZE >>
XC_PAGE_SHIFT);
if (vaddr_base == NULL) {
@@ -347,11 +375,11 @@ static void xen_init_fv(ram_addr_t ram_s
(STORE_PAGE_START >> XC_PAGE_SHIFT);
}
- phys_ram_base = xc_map_foreign_batch(xc_handle, domid,
+ phys_ram_base = xc_map_foreign_pages(xc_handle, domid,
PROT_READ|PROT_WRITE,
page_array, nr_pages);
if (phys_ram_base == 0) {
- fprintf(logfile, "xc_map_foreign_batch returned error %d\n",
errno);
+ fprintf(logfile, "xc_map_foreign_pages returned error %d\n",
errno);
exit(-1);
}
free(page_array);
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Ian Jackson
2010-Jan-12 17:25 UTC
Re: [Xen-devel] [PATCH, RFC 3/4] qemu: use new (replacement) mmap-batch ioctl
I don''t have any objections to this in principle, but:> +#if __XEN_LATEST_INTERFACE_VERSION__ < 0x0003020a > +void * __attribute__((__weak__)) > +xc_map_foreign_bulk(int xc_handle, uint32_t dom, int prot, > + const xen_pfn_t *, int *err, unsigned int num); > +#endifThis is pretty horrible. This should be done by using the #ifdef arround the call to xc_map_foreign_bulk, not by having a weak symbol compared to NULL. You might consider whether the ability to use the old call is intended to last forever. Probably not, soi you should mark it deprecated. Jan Beulich writes ("[Xen-devel] [PATCH, RFC 3/4] qemu: use new (replacement) mmap-batch ioctl"):> - phys_ram_base = xc_map_foreign_batch(xc_handle, domid, > + phys_ram_base = xc_map_foreign_pages(xc_handle, domid, > PROT_READ|PROT_WRITE, > page_array, nr_pages);I''m not sure I understand this bug I haven''t looked at it in any kind of detail so it may be correct. Ian. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Jan Beulich
2010-Jan-13 07:50 UTC
Re: [Xen-devel] [PATCH, RFC 3/4] qemu: use new (replacement) mmap-batch ioctl
>>> Ian Jackson <Ian.Jackson@eu.citrix.com> 12.01.10 18:25 >>> >> +#if __XEN_LATEST_INTERFACE_VERSION__ < 0x0003020a >> +void * __attribute__((__weak__)) >> +xc_map_foreign_bulk(int xc_handle, uint32_t dom, int prot, >> + const xen_pfn_t *, int *err, unsigned int num); >> +#endif > >This is pretty horrible. > >This should be done by using the #ifdef arround the call to >xc_map_foreign_bulk, not by having a weak symbol compared to NULL.While indeed I wasn''t sure about how (or if at all) to put in backward compatibility, this seemed to be the consolidated place. Using and #ifdef in the source file doesn''t seem nice though, as it would tie a qemu built against older headers to using the old interface. But since I''m not sure about the compatibility needs in the first place, I will listen to whatever you say you deem appropriate.>You might consider whether the ability to use the old call is intended >to last forever. Probably not, soi you should mark it deprecated.It is being marked deprecated in a comment; using the respective __attribute_((__deprecated__)) didn''t seem a proper thing in a header that ought to be consumable by non-GNU compilers.>> - phys_ram_base = xc_map_foreign_batch(xc_handle, domid, >> + phys_ram_base = xc_map_foreign_pages(xc_handle, domid, >> PROT_READ|PROT_WRITE, >> page_array, nr_pages); > >I''m not sure I understand this bug I haven''t looked at it in any kind >of detail so it may be correct.This just follows the libxc changes: Any call to xc_map_foreign_batch() where the array wasn''t looked at for per-page errors just wasn''t correct, and now gets converted to use xc_map_foreign_pages() instead (which does the error checking on behalf of the caller). Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Keir Fraser
2010-Jan-13 07:59 UTC
Re: [Xen-devel] [PATCH, RFC 3/4] qemu: use new (replacement) mmap-batch ioctl
On 13/01/2010 07:50, "Jan Beulich" <JBeulich@novell.com> wrote:>> This should be done by using the #ifdef arround the call to >> xc_map_foreign_bulk, not by having a weak symbol compared to NULL. > > While indeed I wasn''t sure about how (or if at all) to put in > backward compatibility, this seemed to be the consolidated place. > > Using and #ifdef in the source file doesn''t seem nice though, as it > would tie a qemu built against older headers to using the old > interface. But since I''m not sure about the compatibility needs in > the first place, I will listen to whatever you say you deem > appropriate.Qemu is deemed part of the tollstack matched set, so compatibility with older libxc is not an issue. The primary concern is to provide continued compatibility with older dom0 kernels, which I believe you entirely handle within libxc. -- Keir _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Jan Beulich
2010-Jan-13 08:03 UTC
Re: [Xen-devel] [PATCH, RFC 3/4] qemu: use new (replacement) mmap-batch ioctl
>>> Keir Fraser <keir.fraser@eu.citrix.com> 13.01.10 08:59 >>> >Qemu is deemed part of the tollstack matched set, so compatibility with >older libxc is not an issue. The primary concern is to provide continued >compatibility with older dom0 kernels, which I believe you entirely handle >within libxc.Yes, that''s correct. Should I re-spin the patch with the compatibility handling entirely removed then? Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Keir Fraser
2010-Jan-13 08:28 UTC
Re: [Xen-devel] [PATCH, RFC 3/4] qemu: use new (replacement) mmap-batch ioctl
On 13/01/2010 08:03, "Jan Beulich" <JBeulich@novell.com> wrote:>>>> Keir Fraser <keir.fraser@eu.citrix.com> 13.01.10 08:59 >>> >> Qemu is deemed part of the tollstack matched set, so compatibility with >> older libxc is not an issue. The primary concern is to provide continued >> compatibility with older dom0 kernels, which I believe you entirely handle >> within libxc. > > Yes, that''s correct. Should I re-spin the patch with the compatibility > handling entirely removed then?Yes please. I''ve committed the other patches by the way (not yet pushed though). Re-spin the qemu one and we''ll get that in for -rc2 as well. Thanks, Keir _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Jan Beulich
2010-Jan-13 08:56 UTC
Re: [Xen-devel] [PATCH, RFC 3/4] qemu: use new (replacement) mmap-batch ioctl
>>> Keir Fraser <keir.fraser@eu.citrix.com> 13.01.10 09:28 >>> >On 13/01/2010 08:03, "Jan Beulich" <JBeulich@novell.com> wrote: >> Yes, that''s correct. Should I re-spin the patch with the compatibility >> handling entirely removed then? > >Yes please. I''ve committed the other patches by the way (not yet pushed >though). Re-spin the qemu one and we''ll get that in for -rc2 as well.Here we go. Thanks! Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel