* 32-on-64: pvfb issue @ 2007-01-18 13:57 Gerd Hoffmann 2007-01-18 14:07 ` Keir Fraser 2007-01-18 15:00 ` Markus Armbruster 0 siblings, 2 replies; 50+ messages in thread From: Gerd Hoffmann @ 2007-01-18 13:57 UTC (permalink / raw) To: Xen devel list [-- Attachment #1: Type: text/plain, Size: 744 bytes --] Hi, There is a problem with the virtual framebuffer: The page directory in the shared page (xenfb_page->pd[]) is unsigned long and thus has different sizes on 32bit and 64bit. The alignment is different too. And on top of that the frontend driver doesn't clear the shared page, which makes it difficult to autodetect the bitsize. I've tried nevertheless, patch (untested!) attached for comments. In the long run this code is supposed to be replaced by grant tables anyway, so it is probably okay to live with the hack for the time being. We could of course also fix the struct if we can afford breaking the interface. As it is quite new and probably (hmm, does fc6 ship it?) not widely used yet that might be an option. cheers, Gerd [-- Attachment #2: bimodal-fb --] [-- Type: text/plain, Size: 2944 bytes --] --- build-32-unstable-13401/tools/xenfb/xenfb.c-13401 2007-01-18 11:35:53.000000000 +0100 +++ build-32-unstable-13401/tools/xenfb/xenfb.c 2007-01-18 12:30:44.000000000 +0100 @@ -329,12 +329,59 @@ struct xenfb_page *page = xenfb->fb.page; int n_fbmfns; int n_fbdirs; + unsigned long pgmfns[2]; unsigned long *fbmfns; + uint32_t *ptr32; + uint64_t *ptr64; + int i,mode; + +/* + * bimodal guess work. + * page->depth is at a 64bit border, thus we get for page->pg[] ... + * + * i386 | depth | 3b pad | pg0 | pg1 | pg2 | pg3 | pg4 | + * x86_64 | depth | 7b pad | pg0 | pg1 | + * + * frontend does *not* clear the page. pg0 and pg1 are initialized. + * at the moment pg0 is used only (800x600x32 needs less than 512 pages, + * which fits into one page directory even with 64bit), pg1 is initialized + * to zero. We also assume the machine has less than 16 TB main memory, + * thus 32bit are enougth for the page frame numbers. By the time this + * assumtion is rendered invalid the code should have been converted to + * grant tables anyway. + */ +#if defined(__i386__) + mode = 32; + ptr32 = (void*)page->pd; + ptr64 = ((void*)ptr32) + 4; + if (ptr32[1] > 0 && + ptr32[2] == 0 && + ptr32[3] == 0 && + ptr32[4] == 0) + mode = 64; +#elif defined(__x86_64__) + mode = 64; + ptr64 = (void*)page->pd; + ptr32 = ((void*)ptr64) - 4; + if (ptr32[0] > 0 && + ptr32[1] == 0) + mode = 32; +#else +# error unknown arch +#endif n_fbmfns = (xenfb->fb_len + (XC_PAGE_SIZE - 1)) / XC_PAGE_SIZE; - n_fbdirs = n_fbmfns * sizeof(unsigned long); + n_fbdirs = n_fbmfns * mode / 4; n_fbdirs = (n_fbdirs + (XC_PAGE_SIZE - 1)) / XC_PAGE_SIZE; + if (32 == mode) { + for (i = 0; i < n_fbdirs; i++) + pgmfns[i] = ptr32[i]; + } else { + for (i = 0; i < n_fbdirs; i++) + pgmfns[i] = ptr64[i]; + } + /* * Bug alert: xc_map_foreign_batch() can fail partly and * return a non-null value. This is a design flaw. When it @@ -342,18 +389,31 @@ * access. */ fbmfns = xc_map_foreign_batch(xenfb->xc, domid, - PROT_READ, page->pd, n_fbdirs); + PROT_READ, pgmfns, n_fbdirs); if (fbmfns == NULL) return -1; + if (32 == mode) { + ptr32 = (void*)fbmfns; + fbmfns = malloc(n_fbmfns * sizeof(*fbmfns)); + for (i = 0; i < n_fbmfns; i++) + fbmfns[i] = ptr32[i]; + munmap(ptr32, n_fbdirs * XC_PAGE_SIZE); + } else { + ptr64 = (void*)fbmfns; + fbmfns = malloc(n_fbmfns * sizeof(*fbmfns)); + for (i = 0; i < n_fbmfns; i++) + fbmfns[i] = ptr64[i]; + munmap(ptr64, n_fbdirs * XC_PAGE_SIZE); + } + xenfb->pub.pixels = xc_map_foreign_batch(xenfb->xc, domid, PROT_READ | PROT_WRITE, fbmfns, n_fbmfns); - if (xenfb->pub.pixels == NULL) { - munmap(fbmfns, n_fbdirs * XC_PAGE_SIZE); + if (xenfb->pub.pixels == NULL) return -1; - } - return munmap(fbmfns, n_fbdirs * XC_PAGE_SIZE); + free(fbmfns); + return 0; } static int xenfb_bind(struct xenfb_device *dev) [-- Attachment #3: Type: text/plain, Size: 138 bytes --] _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: 32-on-64: pvfb issue 2007-01-18 13:57 32-on-64: pvfb issue Gerd Hoffmann @ 2007-01-18 14:07 ` Keir Fraser 2007-01-18 15:00 ` Markus Armbruster 1 sibling, 0 replies; 50+ messages in thread From: Keir Fraser @ 2007-01-18 14:07 UTC (permalink / raw) To: Gerd Hoffmann, Xen devel list On 18/1/07 13:57, "Gerd Hoffmann" <kraxel@suse.de> wrote: > As it is quite new > and probably (hmm, does fc6 ship it?) not widely used yet that might be > an option. This would be preferable but possibly not too popular with Red Hat. :-) Otherwise using aligned uint64_t will obviously save some pain. -- Keir ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: 32-on-64: pvfb issue 2007-01-18 13:57 32-on-64: pvfb issue Gerd Hoffmann 2007-01-18 14:07 ` Keir Fraser @ 2007-01-18 15:00 ` Markus Armbruster 2007-01-18 15:35 ` Gerd Hoffmann 1 sibling, 1 reply; 50+ messages in thread From: Markus Armbruster @ 2007-01-18 15:00 UTC (permalink / raw) To: Gerd Hoffmann; +Cc: Xen devel list Gerd Hoffmann <kraxel@suse.de> writes: > Hi, > > There is a problem with the virtual framebuffer: The page directory in > the shared page (xenfb_page->pd[]) is unsigned long and thus has > different sizes on 32bit and 64bit. The alignment is different too. And > on top of that the frontend driver doesn't clear the shared page, which > makes it difficult to autodetect the bitsize. I've tried nevertheless, > patch (untested!) attached for comments. In the long run this code is > supposed to be replaced by grant tables anyway, so it is probably okay > to live with the hack for the time being. We could of course also fix > the struct if we can afford breaking the interface. As it is quite new > and probably (hmm, does fc6 ship it?) not widely used yet that might be > an option. Breaking the API now is right out of the question, I fear :) You can evolve the API. Let the frontend put something in xenstore[*] that lets the backend detect which page layout to use. Make sure the backend can deal with old and new frontend. I doubt it's worthwhile here. Excuse my ignorance, but why do you have to guess the guest's size? Doesn't dom0 know? [*] I suggested to put version ID right into the page, but that was shot down in favor of xenstore. ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: 32-on-64: pvfb issue 2007-01-18 15:00 ` Markus Armbruster @ 2007-01-18 15:35 ` Gerd Hoffmann 2007-01-18 15:53 ` Daniel P. Berrange 2007-01-18 18:31 ` Keir Fraser 0 siblings, 2 replies; 50+ messages in thread From: Gerd Hoffmann @ 2007-01-18 15:35 UTC (permalink / raw) To: Markus Armbruster; +Cc: Xen devel list Markus Armbruster wrote: > Gerd Hoffmann <kraxel@suse.de> writes: > >> and probably (hmm, does fc6 ship it?) not widely used yet that might be >> an option. > > Breaking the API now is right out of the question, I fear :) Yep, I've seen in the release notes fc6 ships pvfb, so it is used in the wild now and breaking the API is clearly out of question. Damn. Should have reviewed the patches earlier ... > You can evolve the API. Let the frontend put something in xenstore[*] > that lets the backend detect which page layout to use. Make sure the > backend can deal with old and new frontend. I doubt it's worthwhile > here. Well, the problem are the *existing* guests, we already have two different versions *without* indication out there: The 32bit and the 64bit ones. And with the upcoming 32-on-64 support the backend suddenly must be able to deal with both of them. Ideally it should work automatically, without updating the frontends, and without manual configuration. > Excuse my ignorance, but why do you have to guess the guest's size? > Doesn't dom0 know? No, right now there is no hypercall to get that information. Which brings the discussion back onto the table: Should we add one? pvfb isn't the only driver with that kind of problems. And it better shouldn't be a dom0-only hypercall, otherwise driver domains will not be able to use it ... > [*] I suggested to put version ID right into the page, but that was > shot down in favor of xenstore. Too bad. The configuration is in the shared page anyway, so what is the point of *not* placing the version info there as well? cheers, Gerd ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: 32-on-64: pvfb issue 2007-01-18 15:35 ` Gerd Hoffmann @ 2007-01-18 15:53 ` Daniel P. Berrange 2007-01-18 16:34 ` Gerd Hoffmann 2007-01-18 18:31 ` Keir Fraser 1 sibling, 1 reply; 50+ messages in thread From: Daniel P. Berrange @ 2007-01-18 15:53 UTC (permalink / raw) To: Gerd Hoffmann; +Cc: Xen devel list, Markus Armbruster On Thu, Jan 18, 2007 at 04:35:03PM +0100, Gerd Hoffmann wrote: > Markus Armbruster wrote: > > Gerd Hoffmann <kraxel@suse.de> writes: > > > >> and probably (hmm, does fc6 ship it?) not widely used yet that might be > >> an option. > > > > Breaking the API now is right out of the question, I fear :) > > Yep, I've seen in the release notes fc6 ships pvfb, so it is used in the > wild now and breaking the API is clearly out of question. Damn. Should > have reviewed the patches earlier ... Actually FC6 isn't a problem - the ABI already changed between the time we shipped it in FC6, and the time it got merged in xen-devel. For RHEL-5 though we have synced to the ABI currently in xen-devel/xen-3.0.4. We plan to update FC-6 to use this ABI too, to remove the incompatability. The downside is that in FC-6 Xen userspace we now have to maintain some non-upstream back-compat code to let both old &new kernels run on new userspace - that's the price paid for shipping before upstream. Now it is upstream though we really don't want to see incompatible changes again because it will screw over not just Fedora, but RHEL too, and indeed any users of Xen 3.0.4 release. BTW, the way we do the back-compat support is basically to ship two versions of the xen-sdlfb & xen-vncfb daemons - one speaking the old protocol, one speaking the new. We try to launch the new versions & if they fail, we launch the old versions. A nasty hack, but it works. Regards, Dan. -- |=- Red Hat, Engineering, Emerging Technologies, Boston. +1 978 392 2496 -=| |=- Perl modules: http://search.cpan.org/~danberr/ -=| |=- Projects: http://freshmeat.net/~danielpb/ -=| |=- GnuPG: 7D3B9505 F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 -=| ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: 32-on-64: pvfb issue 2007-01-18 15:53 ` Daniel P. Berrange @ 2007-01-18 16:34 ` Gerd Hoffmann 2007-01-18 16:55 ` Gerd Hoffmann 2007-01-18 17:05 ` Daniel P. Berrange 0 siblings, 2 replies; 50+ messages in thread From: Gerd Hoffmann @ 2007-01-18 16:34 UTC (permalink / raw) To: Daniel P. Berrange; +Cc: Xen devel list, Markus Armbruster Daniel P. Berrange wrote: > Actually FC6 isn't a problem - the ABI already changed between the time we > shipped it in FC6, and the time it got merged in xen-devel. Ok. > For RHEL-5 though > we have synced to the ABI currently in xen-devel/xen-3.0.4. Hmm, assuming we'll fix up the ABI now to be binary compatible between 32 and 64 bit and put that one into both 3.0.4 and unstable, could that still make it into RHEL-5 or is that too late now? cheers, Gerd ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: 32-on-64: pvfb issue 2007-01-18 16:34 ` Gerd Hoffmann @ 2007-01-18 16:55 ` Gerd Hoffmann 2007-01-18 17:05 ` Daniel P. Berrange 1 sibling, 0 replies; 50+ messages in thread From: Gerd Hoffmann @ 2007-01-18 16:55 UTC (permalink / raw) To: Gerd Hoffmann; +Cc: Xen devel list, Daniel P. Berrange, Markus Armbruster [-- Attachment #1: Type: text/plain, Size: 303 bytes --] Hi, > Hmm, assuming we'll fix up the ABI now to be binary compatible between > 32 and 64 bit and put that one into both 3.0.4 and unstable, could that > still make it into RHEL-5 or is that too late now? i.e. something along the lines of the attached patch (compile tested only) ... cheers, Gerd [-- Attachment #2: abi-fb --] [-- Type: text/plain, Size: 3713 bytes --] --- build-32-unstable-13401/linux-2.6-xen-sparse/drivers/xen/fbfront/xenfb.c.abi 2007-01-18 17:43:07.000000000 +0100 +++ build-32-unstable-13401/linux-2.6-xen-sparse/drivers/xen/fbfront/xenfb.c 2007-01-18 17:46:29.000000000 +0100 @@ -58,7 +58,7 @@ int irq; struct xenfb_page *page; - unsigned long *mfns; + u64 *mfns; int update_wanted; /* XENFB_TYPE_UPDATE wanted */ struct xenbus_device *xbdev; @@ -474,12 +474,12 @@ if (info->pages == NULL) goto error_nomem; - info->mfns = vmalloc(sizeof(unsigned long) * info->nr_pages); + info->mfns = vmalloc(sizeof(u64) * info->nr_pages); if (!info->mfns) goto error_nomem; /* set up shared page */ - info->page = (void *)__get_free_page(GFP_KERNEL); + info->page = (void *)__get_free_page(GFP_KERNEL | __GFP_ZERO); if (!info->page) goto error_nomem; @@ -600,6 +600,7 @@ for (i = 0; i < info->nr_pages; i++) info->mfns[i] = vmalloc_to_mfn(info->fb + i * PAGE_SIZE); + info->page->protocol = 1; info->page->pd[0] = vmalloc_to_mfn(info->mfns); info->page->pd[1] = 0; info->page->width = XENFB_WIDTH; --- build-32-unstable-13401/tools/xenfb/xenfb.c.abi 2007-01-18 17:48:57.000000000 +0100 +++ build-32-unstable-13401/tools/xenfb/xenfb.c 2007-01-18 17:50:40.000000000 +0100 @@ -329,12 +329,20 @@ struct xenfb_page *page = xenfb->fb.page; int n_fbmfns; int n_fbdirs; + unsigned long pgmfns[2]; unsigned long *fbmfns; + uint64_t *ptr64; + int i; + + ptr64 = page->pd; n_fbmfns = (xenfb->fb_len + (XC_PAGE_SIZE - 1)) / XC_PAGE_SIZE; - n_fbdirs = n_fbmfns * sizeof(unsigned long); + n_fbdirs = n_fbmfns * sizeof(uint64_t); n_fbdirs = (n_fbdirs + (XC_PAGE_SIZE - 1)) / XC_PAGE_SIZE; + for (i = 0; i < n_fbdirs; i++) + pgmfns[i] = ptr64[i]; + /* * Bug alert: xc_map_foreign_batch() can fail partly and * return a non-null value. This is a design flaw. When it @@ -342,18 +350,23 @@ * access. */ fbmfns = xc_map_foreign_batch(xenfb->xc, domid, - PROT_READ, page->pd, n_fbdirs); + PROT_READ, pgmfns, n_fbdirs); if (fbmfns == NULL) return -1; + ptr64 = (void*)fbmfns; + fbmfns = malloc(n_fbmfns * sizeof(*fbmfns)); + for (i = 0; i < n_fbmfns; i++) + fbmfns[i] = ptr64[i]; + munmap(ptr64, n_fbdirs * XC_PAGE_SIZE); + xenfb->pub.pixels = xc_map_foreign_batch(xenfb->xc, domid, PROT_READ | PROT_WRITE, fbmfns, n_fbmfns); - if (xenfb->pub.pixels == NULL) { - munmap(fbmfns, n_fbdirs * XC_PAGE_SIZE); + if (xenfb->pub.pixels == NULL) return -1; - } - return munmap(fbmfns, n_fbdirs * XC_PAGE_SIZE); + free(fbmfns); + return 0; } static int xenfb_bind(struct xenfb_device *dev) --- build-32-unstable-13401/xen/include/public/io/fbif.h.abi 2007-01-18 17:34:49.000000000 +0100 +++ build-32-unstable-13401/xen/include/public/io/fbif.h 2007-01-18 17:38:39.000000000 +0100 @@ -102,6 +102,8 @@ uint32_t line_length; /* the length of a row of pixels (in bytes) */ uint32_t mem_length; /* the length of the framebuffer (in bytes) */ uint8_t depth; /* the depth of a pixel (in bits) */ + uint8_t protocol; /* protocol version, will bump when we switch to grant tables */ + uint8_t pad[6]; /* align next field to 64bit */ /* * Framebuffer page directory @@ -109,10 +111,10 @@ * Each directory page holds PAGE_SIZE / sizeof(*pd) * framebuffer pages, and can thus map up to PAGE_SIZE * * PAGE_SIZE / sizeof(*pd) bytes. With PAGE_SIZE == 4096 and - * sizeof(unsigned long) == 4, that's 4 Megs. Two directory + * sizeof(uint64_t) == 8, that's 2 Megs. Four directory * pages should be enough for a while. */ - unsigned long pd[2]; + uint64_t pd[4]; }; /* [-- Attachment #3: Type: text/plain, Size: 138 bytes --] _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: 32-on-64: pvfb issue 2007-01-18 16:34 ` Gerd Hoffmann 2007-01-18 16:55 ` Gerd Hoffmann @ 2007-01-18 17:05 ` Daniel P. Berrange 1 sibling, 0 replies; 50+ messages in thread From: Daniel P. Berrange @ 2007-01-18 17:05 UTC (permalink / raw) To: Gerd Hoffmann; +Cc: Xen devel list, Markus Armbruster On Thu, Jan 18, 2007 at 05:34:00PM +0100, Gerd Hoffmann wrote: > Daniel P. Berrange wrote: > > Actually FC6 isn't a problem - the ABI already changed between the time we > > shipped it in FC6, and the time it got merged in xen-devel. > > Ok. > > > For RHEL-5 though > > we have synced to the ABI currently in xen-devel/xen-3.0.4. > > Hmm, assuming we'll fix up the ABI now to be binary compatible between > 32 and 64 bit and put that one into both 3.0.4 and unstable, could that > still make it into RHEL-5 or is that too late now? Not any chance whatsoever I'm afraid :-( We're way too far into the release process to make any functional changes now. Regards, Dan. -- |=- Red Hat, Engineering, Emerging Technologies, Boston. +1 978 392 2496 -=| |=- Perl modules: http://search.cpan.org/~danberr/ -=| |=- Projects: http://freshmeat.net/~danielpb/ -=| |=- GnuPG: 7D3B9505 F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 -=| ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: 32-on-64: pvfb issue 2007-01-18 15:35 ` Gerd Hoffmann 2007-01-18 15:53 ` Daniel P. Berrange @ 2007-01-18 18:31 ` Keir Fraser 2007-01-19 9:46 ` Gerd Hoffmann ` (3 more replies) 1 sibling, 4 replies; 50+ messages in thread From: Keir Fraser @ 2007-01-18 18:31 UTC (permalink / raw) To: Gerd Hoffmann, Markus Armbruster; +Cc: Xen devel list On 18/1/07 15:35, "Gerd Hoffmann" <kraxel@suse.de> wrote: >> Excuse my ignorance, but why do you have to guess the guest's size? >> Doesn't dom0 know? > > No, right now there is no hypercall to get that information. Which > brings the discussion back onto the table: Should we add one? pvfb > isn't the only driver with that kind of problems. > > And it better shouldn't be a dom0-only hypercall, otherwise driver > domains will not be able to use it ... Do the same as you did with blkback. Add a protocol version or bitwidth field to xenstore. Update frontend driver to write that field. Make tools write a default value for pv guests for backward compat. This is even better than with blkfront/blkback because there are definitely no PV-on-HVM pvfb driver frontends out in the wild (and it's HVM that makes things harder). Speaking of which, we should add the protocol/bitwidth field to blkfront asap... -- Keir ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: 32-on-64: pvfb issue 2007-01-18 18:31 ` Keir Fraser @ 2007-01-19 9:46 ` Gerd Hoffmann 2007-01-19 9:54 ` Gerd Hoffmann ` (2 subsequent siblings) 3 siblings, 0 replies; 50+ messages in thread From: Gerd Hoffmann @ 2007-01-19 9:46 UTC (permalink / raw) To: Keir Fraser; +Cc: Xen devel list, Markus Armbruster [-- Attachment #1: Type: text/plain, Size: 1217 bytes --] Keir Fraser wrote: > Do the same as you did with blkback. Add a protocol version or bitwidth > field to xenstore. Update frontend driver to write that field. Make tools > write a default value for pv guests for backward compat. This is even better > than with blkfront/blkback because there are definitely no PV-on-HVM pvfb > driver frontends out in the wild (and it's HVM that makes things harder). Ok, point taken, forgot about the pv-on-hvm issue. Why put the protocol into xenstore btw? The framebuffer configuration is in the shared page instead of xenstore anyway. I'd place protocol next to the configuration, it is the natural location IMHO. We can still sneak it in without breaking the struct layout as we have alignment holes in there anyway. > Speaking of which, we should add the protocol/bitwidth field to blkfront > asap... It is sitting in my 32-on-64 patch queue emmanuel is looking at. The blkback bits shouldn't have any dependencies to the other patches though, so I can submit it right away if you are willing to take it now. Patch for unstable attached. I can prepare a minimal one with just the frontend bits for 3.0.4 if you want. cheers, Gerd -- Gerd Hoffmann <kraxel@suse.de> [-- Attachment #2: blkif-bimodal.diff --] [-- Type: text/x-patch, Size: 25463 bytes --] multiprotocol blkback drivers. This is a patch for the block interface, frontend drivers, backend drivers and tools to support multiple ring protocols. Right there are now just two: the 32bit and the 64bit one. If needed it can be extended. Interface changes (io/blkif.h) * Have both request structs there, with "v1" and "v2" added to the name. The old name is aliased to the native protocol of the architecture. * Add helper functions to convert v1/v2 requests to native. Frontend changes: * Create a new node "protocol", add the protocol number it speaks there. Backend changes: * Look at the "protocol" number of the frontend and switch ring handling accordingly. If the protocol node isn't present it assumes native protocol. * As the request struct is copied anyway before being processed (for security reasons) it is converted to native at that point so most backend code doesn't need to know what the frontend speaks. * In case of blktap this is completely transparent to userspace, the kernel/userspace ring is always native no matter what the frontend speaks. Tools changes: * Add one more option to the disk configuration, so one can specify the protocol the frontend speaks in the config file. This is needed for old frontends which don't advertise the protocol they are speaking themself. I'm not that happy with this approach, but it works for now and I'm kida lost in the stack of python classes doing domain and device handling ... --- linux-2.6-xen-sparse/drivers/xen/blkback/blkback.c | 64 ++++++++---- linux-2.6-xen-sparse/drivers/xen/blkback/common.h | 6 - linux-2.6-xen-sparse/drivers/xen/blkback/interface.c | 25 +++- linux-2.6-xen-sparse/drivers/xen/blkback/xenbus.c | 14 ++ linux-2.6-xen-sparse/drivers/xen/blkfront/blkfront.c | 7 + linux-2.6-xen-sparse/drivers/xen/blktap/blktap.c | 63 ++++++++---- linux-2.6-xen-sparse/drivers/xen/blktap/common.h | 6 - linux-2.6-xen-sparse/drivers/xen/blktap/interface.c | 25 +++- linux-2.6-xen-sparse/drivers/xen/blktap/xenbus.c | 14 ++ linux-2.6-xen-sparse/include/xen/blkif.h | 99 +++++++++++++++++++ tools/python/xen/xend/server/blkif.py | 3 tools/python/xen/xm/create.py | 7 + xen/include/public/io/blkif.h | 14 +- 13 files changed, 282 insertions(+), 65 deletions(-) Index: build-32-unstable-13495/linux-2.6-xen-sparse/drivers/xen/blkback/blkback.c =================================================================== --- build-32-unstable-13495.orig/linux-2.6-xen-sparse/drivers/xen/blkback/blkback.c +++ build-32-unstable-13495/linux-2.6-xen-sparse/drivers/xen/blkback/blkback.c @@ -298,17 +298,20 @@ irqreturn_t blkif_be_int(int irq, void * static int do_block_io_op(blkif_t *blkif) { - blkif_back_ring_t *blk_ring = &blkif->blk_ring; + blkif_back_rings_t *blk_rings = &blkif->blk_rings; blkif_request_t req; pending_req_t *pending_req; RING_IDX rc, rp; int more_to_do = 0; - rc = blk_ring->req_cons; - rp = blk_ring->sring->req_prod; + rc = blk_rings->co.req_cons; + rp = blk_rings->co.sring->req_prod; rmb(); /* Ensure we see queued requests up to 'rp'. */ - while ((rc != rp) && !RING_REQUEST_CONS_OVERFLOW(blk_ring, rc)) { + while ((rc != rp)) { + + if (RING_REQUEST_CONS_OVERFLOW(&blk_rings->co, rc)) + break; pending_req = alloc_req(); if (NULL == pending_req) { @@ -317,8 +320,17 @@ static int do_block_io_op(blkif_t *blkif break; } - memcpy(&req, RING_GET_REQUEST(blk_ring, rc), sizeof(req)); - blk_ring->req_cons = ++rc; /* before make_response() */ + switch (blkif->blk_protocol) { + case 1: + blkif_get_v1_req(&req, RING_GET_REQUEST(&blk_rings->v1, rc)); + break; + case 2: + blkif_get_v2_req(&req, RING_GET_REQUEST(&blk_rings->v2, rc)); + break; + default: + BUG(); + } + blk_rings->co.req_cons = ++rc; /* before make_response() */ switch (req.operation) { case BLKIF_OP_READ: @@ -498,34 +510,44 @@ static void dispatch_rw_block_io(blkif_t static void make_response(blkif_t *blkif, unsigned long id, unsigned short op, int st) { - blkif_response_t *resp; + blkif_response_t resp; unsigned long flags; - blkif_back_ring_t *blk_ring = &blkif->blk_ring; + blkif_back_rings_t *blk_rings = &blkif->blk_rings; int more_to_do = 0; int notify; - spin_lock_irqsave(&blkif->blk_ring_lock, flags); - - /* Place on the response ring for the relevant domain. */ - resp = RING_GET_RESPONSE(blk_ring, blk_ring->rsp_prod_pvt); - resp->id = id; - resp->operation = op; - resp->status = st; - blk_ring->rsp_prod_pvt++; - RING_PUSH_RESPONSES_AND_CHECK_NOTIFY(blk_ring, notify); + resp.id = id; + resp.operation = op; + resp.status = st; - if (blk_ring->rsp_prod_pvt == blk_ring->req_cons) { + spin_lock_irqsave(&blkif->blk_ring_lock, flags); + /* Place on the response ring for the relevant domain. */ + switch (blkif->blk_protocol) { + case 1: + memcpy(RING_GET_RESPONSE(&blk_rings->v1, blk_rings->v1.rsp_prod_pvt), + &resp, sizeof(resp)); + break; + case 2: + memcpy(RING_GET_RESPONSE(&blk_rings->v2, blk_rings->v2.rsp_prod_pvt), + &resp, sizeof(resp)); + break; + default: + BUG(); + } + blk_rings->co.rsp_prod_pvt++; + RING_PUSH_RESPONSES_AND_CHECK_NOTIFY(&blk_rings->co, notify); + if (blk_rings->co.rsp_prod_pvt == blk_rings->co.req_cons) { /* * Tail check for pending requests. Allows frontend to avoid * notifications if requests are already in flight (lower * overheads and promotes batching). */ - RING_FINAL_CHECK_FOR_REQUESTS(blk_ring, more_to_do); + RING_FINAL_CHECK_FOR_REQUESTS(&blk_rings->co, more_to_do); - } else if (RING_HAS_UNCONSUMED_REQUESTS(blk_ring)) { + } else if (RING_HAS_UNCONSUMED_REQUESTS(&blk_rings->co)) { more_to_do = 1; - } + spin_unlock_irqrestore(&blkif->blk_ring_lock, flags); if (more_to_do) Index: build-32-unstable-13495/linux-2.6-xen-sparse/drivers/xen/blkback/common.h =================================================================== --- build-32-unstable-13495.orig/linux-2.6-xen-sparse/drivers/xen/blkback/common.h +++ build-32-unstable-13495/linux-2.6-xen-sparse/drivers/xen/blkback/common.h @@ -40,8 +40,7 @@ #include <asm/pgalloc.h> #include <xen/evtchn.h> #include <asm/hypervisor.h> -#include <xen/interface/io/blkif.h> -#include <xen/interface/io/ring.h> +#include <xen/blkif.h> #include <xen/gnttab.h> #include <xen/driver_util.h> #include <xen/xenbus.h> @@ -67,7 +66,8 @@ typedef struct blkif_st { /* Physical parameters of the comms window. */ unsigned int irq; /* Comms information. */ - blkif_back_ring_t blk_ring; + int blk_protocol; + blkif_back_rings_t blk_rings; struct vm_struct *blk_ring_area; /* The VBD attached to this interface. */ struct vbd vbd; Index: build-32-unstable-13495/linux-2.6-xen-sparse/drivers/xen/blkback/interface.c =================================================================== --- build-32-unstable-13495.orig/linux-2.6-xen-sparse/drivers/xen/blkback/interface.c +++ build-32-unstable-13495/linux-2.6-xen-sparse/drivers/xen/blkback/interface.c @@ -95,7 +95,6 @@ static void unmap_frontend_page(blkif_t int blkif_map(blkif_t *blkif, unsigned long shared_page, unsigned int evtchn) { - blkif_sring_t *sring; int err; /* Already connected through? */ @@ -111,8 +110,24 @@ int blkif_map(blkif_t *blkif, unsigned l return err; } - sring = (blkif_sring_t *)blkif->blk_ring_area->addr; - BACK_RING_INIT(&blkif->blk_ring, sring, PAGE_SIZE); + switch (blkif->blk_protocol) { + case 1: + { + blkif_v1_sring_t *sring_v1; + sring_v1 = (blkif_v1_sring_t *)blkif->blk_ring_area->addr; + BACK_RING_INIT(&blkif->blk_rings.v1, sring_v1, PAGE_SIZE); + break; + } + case 2: + { + blkif_v2_sring_t *sring_v2; + sring_v2 = (blkif_v2_sring_t *)blkif->blk_ring_area->addr; + BACK_RING_INIT(&blkif->blk_rings.v2, sring_v2, PAGE_SIZE); + break; + } + default: + BUG(); + } err = bind_interdomain_evtchn_to_irqhandler( blkif->domid, evtchn, blkif_be_int, 0, "blkif-backend", blkif); @@ -143,10 +158,10 @@ void blkif_disconnect(blkif_t *blkif) blkif->irq = 0; } - if (blkif->blk_ring.sring) { + if (blkif->blk_rings.co.sring) { unmap_frontend_page(blkif); free_vm_area(blkif->blk_ring_area); - blkif->blk_ring.sring = NULL; + blkif->blk_rings.co.sring = NULL; } } Index: build-32-unstable-13495/linux-2.6-xen-sparse/drivers/xen/blkback/xenbus.c =================================================================== --- build-32-unstable-13495.orig/linux-2.6-xen-sparse/drivers/xen/blkback/xenbus.c +++ build-32-unstable-13495/linux-2.6-xen-sparse/drivers/xen/blkback/xenbus.c @@ -459,6 +459,7 @@ static int connect_ring(struct backend_i struct xenbus_device *dev = be->dev; unsigned long ring_ref; unsigned int evtchn; + unsigned int protocol; int err; DPRINTK("%s", dev->otherend); @@ -472,6 +473,19 @@ static int connect_ring(struct backend_i return err; } + err = xenbus_gather(XBT_NIL, dev->otherend, "protocol", + "%u", &protocol, NULL); + if (err) + protocol = BLKIF_NATIVE_PROTOCOL; + if (protocol < 1 || protocol > 2) { + xenbus_dev_fatal(dev, err, "unknown fe protocol %d", protocol); + return -1; + } + be->blkif->blk_protocol = protocol; + + printk("blkback: ring-ref %ld, event-channel %d, protocol %d\n", + ring_ref, evtchn, protocol); + /* Map the shared frame, irq etc. */ err = blkif_map(be->blkif, ring_ref, evtchn); if (err) { Index: build-32-unstable-13495/linux-2.6-xen-sparse/drivers/xen/blkfront/blkfront.c =================================================================== --- build-32-unstable-13495.orig/linux-2.6-xen-sparse/drivers/xen/blkfront/blkfront.c +++ build-32-unstable-13495/linux-2.6-xen-sparse/drivers/xen/blkfront/blkfront.c @@ -45,6 +45,7 @@ #include <xen/xenbus.h> #include <xen/interface/grant_table.h> #include <xen/gnttab.h> +#include <xen/blkif.h> #include <asm/hypervisor.h> #include <asm/maddr.h> @@ -180,6 +181,12 @@ again: message = "writing event-channel"; goto abort_transaction; } + err = xenbus_printf(xbt, dev->nodename, + "protocol", "%u", BLKIF_NATIVE_PROTOCOL); + if (err) { + message = "writing protocol"; + goto abort_transaction; + } err = xenbus_transaction_end(xbt, 0); if (err) { Index: build-32-unstable-13495/xen/include/public/io/blkif.h =================================================================== --- build-32-unstable-13495.orig/xen/include/public/io/blkif.h +++ build-32-unstable-13495/xen/include/public/io/blkif.h @@ -71,18 +71,20 @@ */ #define BLKIF_MAX_SEGMENTS_PER_REQUEST 11 +struct blkif_request_segment { + grant_ref_t gref; /* reference to I/O buffer frame */ + /* @first_sect: first sector in frame to transfer (inclusive). */ + /* @last_sect: last sector in frame to transfer (inclusive). */ + uint8_t first_sect, last_sect; +}; + struct blkif_request { uint8_t operation; /* BLKIF_OP_??? */ uint8_t nr_segments; /* number of segments */ blkif_vdev_t handle; /* only for read/write requests */ uint64_t id; /* private guest value, echoed in resp */ blkif_sector_t sector_number;/* start sector idx on disk (r/w only) */ - struct blkif_request_segment { - grant_ref_t gref; /* reference to I/O buffer frame */ - /* @first_sect: first sector in frame to transfer (inclusive). */ - /* @last_sect: last sector in frame to transfer (inclusive). */ - uint8_t first_sect, last_sect; - } seg[BLKIF_MAX_SEGMENTS_PER_REQUEST]; + struct blkif_request_segment seg[BLKIF_MAX_SEGMENTS_PER_REQUEST]; }; typedef struct blkif_request blkif_request_t; Index: build-32-unstable-13495/tools/python/xen/xend/server/blkif.py =================================================================== --- build-32-unstable-13495.orig/tools/python/xen/xend/server/blkif.py +++ build-32-unstable-13495/tools/python/xen/xend/server/blkif.py @@ -38,6 +38,7 @@ class BlkifController(DevController): """@see DevController.getDeviceDetails""" uname = config.get('uname', '') dev = config.get('dev', '') + protocol = config.get('protocol') if 'ioemu:' in dev: (_, dev) = string.split(dev, ':', 1) @@ -85,6 +86,8 @@ class BlkifController(DevController): front = { 'virtual-device' : "%i" % devid, 'device-type' : dev_type } + if protocol: + front.update({ 'protocol' : protocol }); return (devid, back, front) Index: build-32-unstable-13495/tools/python/xen/xm/create.py =================================================================== --- build-32-unstable-13495.orig/tools/python/xen/xm/create.py +++ build-32-unstable-13495/tools/python/xen/xm/create.py @@ -537,7 +537,7 @@ def configure_image(vals): def configure_disks(config_devs, vals): """Create the config for disks (virtual block devices). """ - for (uname, dev, mode, backend) in vals.disk: + for (uname, dev, mode, backend, protocol) in vals.disk: if uname.startswith('tap:'): cls = 'tap' else: @@ -549,6 +549,8 @@ def configure_disks(config_devs, vals): ['mode', mode ] ] if backend: config_vbd.append(['backend', backend]) + if protocol: + config_vbd.append(['protocol', protocol]) config_devs.append(['device', config_vbd]) def configure_pci(config_devs, vals): @@ -803,7 +805,10 @@ def preprocess_disk(vals): n = len(d) if n == 3: d.append(None) + d.append(None) elif n == 4: + d.append(None) + elif n == 5: pass else: err('Invalid disk specifier: ' + v) Index: build-32-unstable-13495/linux-2.6-xen-sparse/drivers/xen/blktap/blktap.c =================================================================== --- build-32-unstable-13495.orig/linux-2.6-xen-sparse/drivers/xen/blktap/blktap.c +++ build-32-unstable-13495/linux-2.6-xen-sparse/drivers/xen/blktap/blktap.c @@ -1094,15 +1094,15 @@ irqreturn_t tap_blkif_be_int(int irq, vo static int print_dbug = 1; static int do_block_io_op(blkif_t *blkif) { - blkif_back_ring_t *blk_ring = &blkif->blk_ring; + blkif_back_rings_t *blk_rings = &blkif->blk_rings; blkif_request_t req; pending_req_t *pending_req; RING_IDX rc, rp; int more_to_do = 0; tap_blkif_t *info; - rc = blk_ring->req_cons; - rp = blk_ring->sring->req_prod; + rc = blk_rings->co.req_cons; + rp = blk_rings->co.sring->req_prod; rmb(); /* Ensure we see queued requests up to 'rp'. */ /*Check blkif has corresponding UE ring*/ @@ -1133,8 +1133,8 @@ static int do_block_io_op(blkif_t *blkif more_to_do = 1; break; } - - if (RING_REQUEST_CONS_OVERFLOW(blk_ring, rc)) { + + if (RING_REQUEST_CONS_OVERFLOW(&blk_rings->co, rc)) { WPRINTK("RING_REQUEST_CONS_OVERFLOW!" " More to do\n"); more_to_do = 1; @@ -1148,8 +1148,17 @@ static int do_block_io_op(blkif_t *blkif break; } - memcpy(&req, RING_GET_REQUEST(blk_ring, rc), sizeof(req)); - blk_ring->req_cons = ++rc; /* before make_response() */ + switch (blkif->blk_protocol) { + case 1: + blkif_get_v1_req(&req, RING_GET_REQUEST(&blk_rings->v1, rc)); + break; + case 2: + blkif_get_v2_req(&req, RING_GET_REQUEST(&blk_rings->v2, rc)); + break; + default: + BUG(); + } + blk_rings->co.req_cons = ++rc; /* before make_response() */ switch (req.operation) { case BLKIF_OP_READ: @@ -1225,7 +1234,7 @@ static void dispatch_rw_block_io(blkif_t WPRINTK("blktap: fe_ring is full, can't add " "IO Request will be dropped. %d %d\n", RING_SIZE(&info->ufe_ring), - RING_SIZE(&blkif->blk_ring)); + RING_SIZE(&blkif->blk_rings.co)); goto fail_response; } @@ -1413,32 +1422,44 @@ static void dispatch_rw_block_io(blkif_t static void make_response(blkif_t *blkif, unsigned long id, unsigned short op, int st) { - blkif_response_t *resp; + blkif_response_t resp; unsigned long flags; - blkif_back_ring_t *blk_ring = &blkif->blk_ring; + blkif_back_rings_t *blk_rings = &blkif->blk_rings; int more_to_do = 0; int notify; + resp.id = id; + resp.operation = op; + resp.status = st; + spin_lock_irqsave(&blkif->blk_ring_lock, flags); - /* Place on the response ring for the relevant domain. */ - resp = RING_GET_RESPONSE(blk_ring, blk_ring->rsp_prod_pvt); - resp->id = id; - resp->operation = op; - resp->status = st; - blk_ring->rsp_prod_pvt++; - RING_PUSH_RESPONSES_AND_CHECK_NOTIFY(blk_ring, notify); + /* Place on the response ring for the relevant domain. */ + switch (blkif->blk_protocol) { + case 1: + memcpy(RING_GET_RESPONSE(&blk_rings->v1, blk_rings->v1.rsp_prod_pvt), + &resp, sizeof(resp)); + break; + case 2: + memcpy(RING_GET_RESPONSE(&blk_rings->v2, blk_rings->v2.rsp_prod_pvt), + &resp, sizeof(resp)); + break; + default: + BUG(); + } + blk_rings->co.rsp_prod_pvt++; + RING_PUSH_RESPONSES_AND_CHECK_NOTIFY(&blk_rings->co, notify); - if (blk_ring->rsp_prod_pvt == blk_ring->req_cons) { + if (blk_rings->co.rsp_prod_pvt == blk_rings->co.req_cons) { /* * Tail check for pending requests. Allows frontend to avoid * notifications if requests are already in flight (lower * overheads and promotes batching). */ - RING_FINAL_CHECK_FOR_REQUESTS(blk_ring, more_to_do); - } else if (RING_HAS_UNCONSUMED_REQUESTS(blk_ring)) { + RING_FINAL_CHECK_FOR_REQUESTS(&blk_rings->co, more_to_do); + } else if (RING_HAS_UNCONSUMED_REQUESTS(&blk_rings->co)) { more_to_do = 1; + } - } spin_unlock_irqrestore(&blkif->blk_ring_lock, flags); if (more_to_do) blkif_notify_work(blkif); Index: build-32-unstable-13495/linux-2.6-xen-sparse/drivers/xen/blktap/common.h =================================================================== --- build-32-unstable-13495.orig/linux-2.6-xen-sparse/drivers/xen/blktap/common.h +++ build-32-unstable-13495/linux-2.6-xen-sparse/drivers/xen/blktap/common.h @@ -39,8 +39,7 @@ #include <asm/pgalloc.h> #include <xen/evtchn.h> #include <asm/hypervisor.h> -#include <xen/interface/io/blkif.h> -#include <xen/interface/io/ring.h> +#include <xen/blkif.h> #include <xen/gnttab.h> #include <xen/driver_util.h> @@ -58,7 +57,8 @@ typedef struct blkif_st { /* Physical parameters of the comms window. */ unsigned int irq; /* Comms information. */ - blkif_back_ring_t blk_ring; + int blk_protocol; + blkif_back_rings_t blk_rings; struct vm_struct *blk_ring_area; /* Back pointer to the backend_info. */ struct backend_info *be; Index: build-32-unstable-13495/linux-2.6-xen-sparse/drivers/xen/blktap/interface.c =================================================================== --- build-32-unstable-13495.orig/linux-2.6-xen-sparse/drivers/xen/blktap/interface.c +++ build-32-unstable-13495/linux-2.6-xen-sparse/drivers/xen/blktap/interface.c @@ -96,7 +96,6 @@ static void unmap_frontend_page(blkif_t int tap_blkif_map(blkif_t *blkif, unsigned long shared_page, unsigned int evtchn) { - blkif_sring_t *sring; int err; /* Already connected through? */ @@ -112,8 +111,24 @@ int tap_blkif_map(blkif_t *blkif, unsign return err; } - sring = (blkif_sring_t *)blkif->blk_ring_area->addr; - BACK_RING_INIT(&blkif->blk_ring, sring, PAGE_SIZE); + switch (blkif->blk_protocol) { + case 1: + { + blkif_v1_sring_t *sring_v1; + sring_v1 = (blkif_v1_sring_t *)blkif->blk_ring_area->addr; + BACK_RING_INIT(&blkif->blk_rings.v1, sring_v1, PAGE_SIZE); + break; + } + case 2: + { + blkif_v2_sring_t *sring_v2; + sring_v2 = (blkif_v2_sring_t *)blkif->blk_ring_area->addr; + BACK_RING_INIT(&blkif->blk_rings.v2, sring_v2, PAGE_SIZE); + break; + } + default: + BUG(); + } err = bind_interdomain_evtchn_to_irqhandler( blkif->domid, evtchn, tap_blkif_be_int, @@ -134,10 +149,10 @@ void tap_blkif_unmap(blkif_t *blkif) unbind_from_irqhandler(blkif->irq, blkif); blkif->irq = 0; } - if (blkif->blk_ring.sring) { + if (blkif->blk_rings.co.sring) { unmap_frontend_page(blkif); free_vm_area(blkif->blk_ring_area); - blkif->blk_ring.sring = NULL; + blkif->blk_rings.co.sring = NULL; } } Index: build-32-unstable-13495/linux-2.6-xen-sparse/drivers/xen/blktap/xenbus.c =================================================================== --- build-32-unstable-13495.orig/linux-2.6-xen-sparse/drivers/xen/blktap/xenbus.c +++ build-32-unstable-13495/linux-2.6-xen-sparse/drivers/xen/blktap/xenbus.c @@ -340,6 +340,7 @@ static int connect_ring(struct backend_i struct xenbus_device *dev = be->dev; unsigned long ring_ref; unsigned int evtchn; + unsigned int protocol; int err; DPRINTK("%s\n", dev->otherend); @@ -353,6 +354,19 @@ static int connect_ring(struct backend_i return err; } + err = xenbus_gather(XBT_NIL, dev->otherend, "protocol", + "%u", &protocol, NULL); + if (err) + protocol = BLKIF_NATIVE_PROTOCOL; + if (protocol < 1 || protocol > 2) { + xenbus_dev_fatal(dev, err, "unknown fe protocol %d", protocol); + return -1; + } + be->blkif->blk_protocol = protocol; + + printk("blktap: ring-ref %ld, event-channel %d, protocol %d\n", + ring_ref, evtchn, protocol); + /* Map the shared frame, irq etc. */ err = tap_blkif_map(be->blkif, ring_ref, evtchn); if (err) { Index: build-32-unstable-13495/linux-2.6-xen-sparse/include/xen/blkif.h =================================================================== --- /dev/null +++ build-32-unstable-13495/linux-2.6-xen-sparse/include/xen/blkif.h @@ -0,0 +1,99 @@ +#ifndef __XEN_BLKIF_H__ +#define __XEN_BLKIF_H__ + +#include <xen/interface/io/ring.h> +#include <xen/interface/io/blkif.h> + +/* Not a real protocol. Used to generate ring structs which contain + * the elements common to all protocols only. This way we get a + * compiler-checkable way to use common struct elements, so we can + * avoid using switch(protocol) in a number of places. */ +struct blkif_co_request { + char dummy; +}; +struct blkif_co_response { + char dummy; +}; + +/* i386 protocol version */ +#pragma pack(push, 4) +struct blkif_v1_request { + uint8_t operation; /* BLKIF_OP_??? */ + uint8_t nr_segments; /* number of segments */ + blkif_vdev_t handle; /* only for read/write requests */ + uint64_t id; /* private guest value, echoed in resp */ + blkif_sector_t sector_number;/* start sector idx on disk (r/w only) */ + struct blkif_request_segment seg[BLKIF_MAX_SEGMENTS_PER_REQUEST]; +}; +struct blkif_v1_response { + uint64_t id; /* copied from request */ + uint8_t operation; /* copied from request */ + int16_t status; /* BLKIF_RSP_??? */ +}; +typedef struct blkif_v1_request blkif_v1_request_t; +typedef struct blkif_v1_response blkif_v1_response_t; +#pragma pack(pop) + +/* x86_64 protocol version */ +struct blkif_v2_request { + uint8_t operation; /* BLKIF_OP_??? */ + uint8_t nr_segments; /* number of segments */ + blkif_vdev_t handle; /* only for read/write requests */ + uint64_t __attribute__((__aligned__(8))) id; + blkif_sector_t sector_number;/* start sector idx on disk (r/w only) */ + struct blkif_request_segment seg[BLKIF_MAX_SEGMENTS_PER_REQUEST]; +}; +struct blkif_v2_response { + uint64_t __attribute__((__aligned__(8))) id; + uint8_t operation; /* copied from request */ + int16_t status; /* BLKIF_RSP_??? */ +}; +typedef struct blkif_v2_request blkif_v2_request_t; +typedef struct blkif_v2_response blkif_v2_response_t; + +DEFINE_RING_TYPES(blkif_co, struct blkif_co_request, struct blkif_co_response); +DEFINE_RING_TYPES(blkif_v1, struct blkif_v1_request, struct blkif_v1_response); +DEFINE_RING_TYPES(blkif_v2, struct blkif_v2_request, struct blkif_v2_response); + +union blkif_back_rings { + blkif_co_back_ring_t co; + blkif_v1_back_ring_t v1; + blkif_v2_back_ring_t v2; +}; +typedef union blkif_back_rings blkif_back_rings_t; + +#if defined(__i386__) +# define BLKIF_NATIVE_PROTOCOL 1 +#elif defined(__x86_64__) || defined(__ia64__) +# define BLKIF_NATIVE_PROTOCOL 2 +#else +# error arch fixup needed here +#endif + +/* translate requests: v1/v2 to native */ +#if 1 == BLKIF_NATIVE_PROTOCOL +static void inline blkif_get_v1_req(blkif_request_t *dst, blkif_v1_request_t *src) +#else +static void inline blkif_get_v2_req(blkif_request_t *dst, blkif_v2_request_t *src) +#endif +{ + memcpy(dst, src, sizeof(*dst)); +} + +#if 1 == BLKIF_NATIVE_PROTOCOL +static void inline blkif_get_v2_req(blkif_request_t *dst, blkif_v2_request_t *src) +#else +static void inline blkif_get_v1_req(blkif_request_t *dst, blkif_v1_request_t *src) +#endif +{ + int i; + dst->operation = src->operation; + dst->nr_segments = src->nr_segments; + dst->handle = src->handle; + dst->id = src->id; + dst->sector_number = src->sector_number; + for (i = 0; i < src->nr_segments; i++) + dst->seg[i] = src->seg[i]; +} + +#endif /* __XEN_BLKIF_H__ */ [-- Attachment #3: Type: text/plain, Size: 138 bytes --] _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: 32-on-64: pvfb issue 2007-01-18 18:31 ` Keir Fraser 2007-01-19 9:46 ` Gerd Hoffmann @ 2007-01-19 9:54 ` Gerd Hoffmann 2007-01-19 10:31 ` Markus Armbruster 2007-01-19 11:10 ` Keir Fraser 2007-01-19 10:43 ` Ian Campbell 2007-01-22 18:32 ` Does vt-x itself have perf. impact on Hypervisor w/o considering HVM? Liang Yang 3 siblings, 2 replies; 50+ messages in thread From: Gerd Hoffmann @ 2007-01-19 9:54 UTC (permalink / raw) To: Keir Fraser; +Cc: Xen devel list, Markus Armbruster [-- Attachment #1: Type: text/plain, Size: 266 bytes --] Keir Fraser wrote: > Make tools > write a default value for pv guests for backward compat. Here is a one-line fix to make the pvfb frontend clear the shared page. Can this go into both 3.0.4 and unstable please? thanks, Gerd -- Gerd Hoffmann <kraxel@suse.de> [-- Attachment #2: fbif-bimodal.diff --] [-- Type: text/x-patch, Size: 689 bytes --] --- linux-2.6-xen-sparse/drivers/xen/fbfront/xenfb.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Index: build-32-unstable-13495/linux-2.6-xen-sparse/drivers/xen/fbfront/xenfb.c =================================================================== --- build-32-unstable-13495.orig/linux-2.6-xen-sparse/drivers/xen/fbfront/xenfb.c +++ build-32-unstable-13495/linux-2.6-xen-sparse/drivers/xen/fbfront/xenfb.c @@ -479,7 +479,7 @@ static int __devinit xenfb_probe(struct goto error_nomem; /* set up shared page */ - info->page = (void *)__get_free_page(GFP_KERNEL); + info->page = (void *)__get_free_page(GFP_KERNEL | __GFP_ZERO); if (!info->page) goto error_nomem; [-- Attachment #3: Type: text/plain, Size: 138 bytes --] _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: 32-on-64: pvfb issue 2007-01-19 9:54 ` Gerd Hoffmann @ 2007-01-19 10:31 ` Markus Armbruster 2007-01-19 10:46 ` Gerd Hoffmann 2007-01-19 11:10 ` Keir Fraser 1 sibling, 1 reply; 50+ messages in thread From: Markus Armbruster @ 2007-01-19 10:31 UTC (permalink / raw) To: Gerd Hoffmann; +Cc: Xen devel list, Keir Fraser Gerd Hoffmann <kraxel@suse.de> writes: [...] > Here is a one-line fix to make the pvfb frontend clear the shared page. > Can this go into both 3.0.4 and unstable please? No objections to clearing the shared page, just keep in mind that any scheme to guess the page layout that relies on the page being cleared is playing a dicey game with 3.0.3 frontends. ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: 32-on-64: pvfb issue 2007-01-19 10:31 ` Markus Armbruster @ 2007-01-19 10:46 ` Gerd Hoffmann 2007-01-19 11:53 ` Markus Armbruster 0 siblings, 1 reply; 50+ messages in thread From: Gerd Hoffmann @ 2007-01-19 10:46 UTC (permalink / raw) To: Markus Armbruster; +Cc: Xen devel list, Keir Fraser Markus Armbruster wrote: > Gerd Hoffmann <kraxel@suse.de> writes: > > [...] >> Here is a one-line fix to make the pvfb frontend clear the shared page. >> Can this go into both 3.0.4 and unstable please? > > No objections to clearing the shared page, just keep in mind that any > scheme to guess the page layout that relies on the page being cleared > is playing a dicey game with 3.0.3 frontends. 3.0.3? Wasn't pvfb merged in the 3.0.4 cycle? Or do you talk about what FC6 ships? As the 3.0.4 frontends use hard-coded 800x600-32 which needs one page directory only I think it is possible to keep them apart even in case they don't carry the protocol info ... cheers, Gerd -- Gerd Hoffmann <kraxel@suse.de> ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: 32-on-64: pvfb issue 2007-01-19 10:46 ` Gerd Hoffmann @ 2007-01-19 11:53 ` Markus Armbruster 0 siblings, 0 replies; 50+ messages in thread From: Markus Armbruster @ 2007-01-19 11:53 UTC (permalink / raw) To: Gerd Hoffmann; +Cc: Xen devel list, Keir Fraser Gerd Hoffmann <kraxel@suse.de> writes: > Markus Armbruster wrote: >> Gerd Hoffmann <kraxel@suse.de> writes: >> >> [...] >>> Here is a one-line fix to make the pvfb frontend clear the shared page. >>> Can this go into both 3.0.4 and unstable please? >> >> No objections to clearing the shared page, just keep in mind that any >> scheme to guess the page layout that relies on the page being cleared >> is playing a dicey game with 3.0.3 frontends. > > 3.0.3? Wasn't pvfb merged in the 3.0.4 cycle? Or do you talk about > what FC6 ships? Sorry, I meant 3.0.4. > As the 3.0.4 frontends use hard-coded 800x600-32 which needs one page > directory only I think it is possible to keep them apart even in case > they don't carry the protocol info ... Okay. If that turns out not to work, consider turning depth into your version number, starting with version 32. Evil, ugly, but no guessing games. New frontends won't work with old backends, but that's okay. ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: 32-on-64: pvfb issue 2007-01-19 9:54 ` Gerd Hoffmann 2007-01-19 10:31 ` Markus Armbruster @ 2007-01-19 11:10 ` Keir Fraser 2007-01-19 11:43 ` Gerd Hoffmann 1 sibling, 1 reply; 50+ messages in thread From: Keir Fraser @ 2007-01-19 11:10 UTC (permalink / raw) To: Gerd Hoffmann, Keir Fraser; +Cc: Xen devel list, Markus Armbruster On 19/1/07 09:54, "Gerd Hoffmann" <kraxel@suse.de> wrote: >> Make tools >> write a default value for pv guests for backward compat. > > Here is a one-line fix to make the pvfb frontend clear the shared page. > Can this go into both 3.0.4 and unstable please? How about a patch to clear the page *and* write a newly-defined protocol field? Or are you still planning to kludge the bitwidth check in the backend? -- Keir ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: 32-on-64: pvfb issue 2007-01-19 11:10 ` Keir Fraser @ 2007-01-19 11:43 ` Gerd Hoffmann 2007-01-19 12:01 ` Keir Fraser 0 siblings, 1 reply; 50+ messages in thread From: Gerd Hoffmann @ 2007-01-19 11:43 UTC (permalink / raw) To: Keir Fraser; +Cc: Xen devel list, Markus Armbruster [-- Attachment #1: Type: text/plain, Size: 686 bytes --] Keir Fraser wrote: > > > On 19/1/07 09:54, "Gerd Hoffmann" <kraxel@suse.de> wrote: > >>> Make tools >>> write a default value for pv guests for backward compat. >> Here is a one-line fix to make the pvfb frontend clear the shared page. >> Can this go into both 3.0.4 and unstable please? > > How about a patch to clear the page *and* write a newly-defined protocol > field? Or are you still planning to kludge the bitwidth check in the > backend? That is better, yes. Minimum patch attached. For unstable I'll brew a nicer version with defines and so on when adjusting the backend code to be able to deal with both protocols. cheers, Gerd -- Gerd Hoffmann <kraxel@suse.de> [-- Attachment #2: fbif-bimodal.diff --] [-- Type: text/x-patch, Size: 1996 bytes --] --- linux-2.6-xen-sparse/drivers/xen/fbfront/xenfb.c | 8 +++++++- xen/include/public/io/fbif.h | 5 +++++ 2 files changed, 12 insertions(+), 1 deletion(-) Index: build-32-unstable-13495/linux-2.6-xen-sparse/drivers/xen/fbfront/xenfb.c =================================================================== --- build-32-unstable-13495.orig/linux-2.6-xen-sparse/drivers/xen/fbfront/xenfb.c +++ build-32-unstable-13495/linux-2.6-xen-sparse/drivers/xen/fbfront/xenfb.c @@ -479,7 +479,7 @@ static int __devinit xenfb_probe(struct goto error_nomem; /* set up shared page */ - info->page = (void *)__get_free_page(GFP_KERNEL); + info->page = (void *)__get_free_page(GFP_KERNEL | __GFP_ZERO); if (!info->page) goto error_nomem; @@ -600,6 +600,12 @@ static void xenfb_init_shared_page(struc for (i = 0; i < info->nr_pages; i++) info->mfns[i] = vmalloc_to_mfn(info->fb + i * PAGE_SIZE); +#if defined(__i386__) + info->page->protocol = 1; +#elif defined(__x86_64__) || defined(__ia64__) + info->page->protocol = 2; +#endif + info->page->pd[0] = vmalloc_to_mfn(info->mfns); info->page->pd[1] = 0; info->page->width = XENFB_WIDTH; Index: build-32-unstable-13495/xen/include/public/io/fbif.h =================================================================== --- build-32-unstable-13495.orig/xen/include/public/io/fbif.h +++ build-32-unstable-13495/xen/include/public/io/fbif.h @@ -102,6 +102,11 @@ struct xenfb_page uint32_t line_length; /* the length of a row of pixels (in bytes) */ uint32_t mem_length; /* the length of the framebuffer (in bytes) */ uint8_t depth; /* the depth of a pixel (in bits) */ + uint8_t protocol; /* protocol version + * 1 -- page directory: i386 + * 2 -- page directory: x86_64, ia64 + * 3 -- grant tables [not yet] + */ /* * Framebuffer page directory [-- Attachment #3: Type: text/plain, Size: 138 bytes --] _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: 32-on-64: pvfb issue 2007-01-19 11:43 ` Gerd Hoffmann @ 2007-01-19 12:01 ` Keir Fraser 2007-01-19 12:59 ` Gerd Hoffmann 0 siblings, 1 reply; 50+ messages in thread From: Keir Fraser @ 2007-01-19 12:01 UTC (permalink / raw) To: Gerd Hoffmann, Keir Fraser; +Cc: Xen devel list, Markus Armbruster On 19/1/07 11:43, "Gerd Hoffmann" <kraxel@suse.de> wrote: >> How about a patch to clear the page *and* write a newly-defined protocol >> field? Or are you still planning to kludge the bitwidth check in the >> backend? > > That is better, yes. Minimum patch attached. For unstable I'll brew a > nicer version with defines and so on when adjusting the backend code to > be able to deal with both protocols. Thinking about this some more -- isn't it a bit skank to have the protocol field embedded in the middle of the structure which it effectively defines? Pvfb does interact with xenbus already, so poking a protocol field in there seems reasonable and makes the scheme the same as blkfront/blkback. Furthermore, as I already pointed out, this allows tools to poke a default value and hence we can support e.g., old 32-bit frontends on new 64-bit backend. Furthermore, if we use a xenstore node then we are not restricted to a protocol number. This is rather nice because it allows us to give more meaningful names to our supported protocols. Right now, since we make no effort to ensure protocol compat across machine architectures (for example we use native endianness) I suggest that we define a per-architecture protocol name: 'x86_32', 'x86_64', 'ia64', 'powerpc64', etc. Yes, some of these protocols are actually identical but I don't think this redundancy in the definition of the protocol field matters at all -- a little bit more information can sometimes be useful, and this definition of the protocol field is imo clearer than a simple enumeration. -- Keir ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: 32-on-64: pvfb issue 2007-01-19 12:01 ` Keir Fraser @ 2007-01-19 12:59 ` Gerd Hoffmann 2007-01-19 13:45 ` Keir Fraser 0 siblings, 1 reply; 50+ messages in thread From: Gerd Hoffmann @ 2007-01-19 12:59 UTC (permalink / raw) To: Keir Fraser; +Cc: Xen devel list, Markus Armbruster Keir Fraser wrote: > On 19/1/07 11:43, "Gerd Hoffmann" <kraxel@suse.de> wrote: > >>> How about a patch to clear the page *and* write a newly-defined protocol >>> field? Or are you still planning to kludge the bitwidth check in the >>> backend? >> That is better, yes. Minimum patch attached. For unstable I'll brew a >> nicer version with defines and so on when adjusting the backend code to >> be able to deal with both protocols. > > Thinking about this some more -- isn't it a bit skank to have the protocol > field embedded in the middle of the structure which it effectively defines? When writing something new I would have placed it at the start of the struct, but with the situation we have now it is more important not shuffle the fields around. All the fields before the new protocol field have a fixed size and no alignment problems, so you can be sure the protocol field has a fixed place everythere. Changing the struct is only possible after the protocol field of course. The only pending change is switching from the page directory to grant tables, so this isn't too bad. > Pvfb does interact with xenbus already, so poking a protocol field in there > seems reasonable and makes the scheme the same as blkfront/blkback. Would be more consistent across drivers, yes. I still think it would be placed better in the struct next to the fb config, but if you insist on having it in xenstore I can do it that way too. > Right now, since we make no > effort to ensure protocol compat across machine architectures (for example > we use native endianness) I suggest that we define a per-architecture > protocol name: 'x86_32', 'x86_64', 'ia64', 'powerpc64', etc. Hmm, not sure I like that idea, especially for pvfb as there certainly will come the protocol switch to grant tables, so using the arch names doesn't sound like a good idea to me. cheers, Gerd -- Gerd Hoffmann <kraxel@suse.de> ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: 32-on-64: pvfb issue 2007-01-19 12:59 ` Gerd Hoffmann @ 2007-01-19 13:45 ` Keir Fraser 2007-01-19 15:08 ` Gerd Hoffmann 0 siblings, 1 reply; 50+ messages in thread From: Keir Fraser @ 2007-01-19 13:45 UTC (permalink / raw) To: Gerd Hoffmann, Keir Fraser; +Cc: Xen devel list, Markus Armbruster On 19/1/07 12:59, "Gerd Hoffmann" <kraxel@suse.de> wrote: >> Right now, since we make no >> effort to ensure protocol compat across machine architectures (for example >> we use native endianness) I suggest that we define a per-architecture >> protocol name: 'x86_32', 'x86_64', 'ia64', 'powerpc64', etc. > > Hmm, not sure I like that idea, especially for pvfb as there certainly > will come the protocol switch to grant tables, so using the arch names > doesn't sound like a good idea to me. Then we would make the protocol name structural: '<arch>-v2' rather than the extending an enumeration to 3 and 4 (in your scheme). Or take advantage of the stringness and call it '<arch>-grant'. Personally I'm of the opinion that the architectural ABI is a fundamental component of our protocol (in fact, the very component that is tripping us up here!). And magic numbers suck compared with intelligible strings for this kind of thing imo. However, I am open to persuasive arguments on this point. I'm not quite as stuck on it as I am regarding use of xenbus for this field: it just occurred to me as a seemingly neat extensible technique. :-) -- Keir ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: 32-on-64: pvfb issue 2007-01-19 13:45 ` Keir Fraser @ 2007-01-19 15:08 ` Gerd Hoffmann 2007-01-19 15:22 ` Keir Fraser 0 siblings, 1 reply; 50+ messages in thread From: Gerd Hoffmann @ 2007-01-19 15:08 UTC (permalink / raw) To: Keir Fraser; +Cc: Xen devel list, Markus Armbruster Keir Fraser wrote: > Then we would make the protocol name structural: '<arch>-v2' rather than the > extending an enumeration to 3 and 4 (in your scheme). Or take advantage of > the stringness and call it '<arch>-grant'. Hmm, well, I think we never ever should need "<arch>-v2". It would suck big time if we manage to layout the structs for some future protocol version in a way which is abi-dependent *again*. > Personally I'm of the opinion > that the architectural ABI is a fundamental component of our protocol (in > fact, the very component that is tripping us up here!). Yep. > And magic numbers > suck compared with intelligible strings for this kind of thing imo. We'll need both then. Strings are certainly nice for human-visible stuff, but you don't want to strcmp() all the time in the backend. Wouldn't be a problem for pvfb, but for blkfront/back where the ring protocol is abi-dependent and thus the backend has to check often. > I'm not quite as stuck on > it as I am regarding use of xenbus for this field: it just occurred to me as > a seemingly neat extensible technique. :-) How about the patch below? It adds both names and enums for the protocols, the enums to be used internally in the backend, the names for xenbus, plus conversion helpers (not used yet, want to collect feedback on the idea first). Location most likely isn't final, not sure where to place that best so all parties involved can see it, probably xen/include/public/io/ comments? Gerd -- Gerd Hoffmann <kraxel@suse.de> ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: 32-on-64: pvfb issue 2007-01-19 15:08 ` Gerd Hoffmann @ 2007-01-19 15:22 ` Keir Fraser 2007-01-19 15:31 ` Gerd Hoffmann 0 siblings, 1 reply; 50+ messages in thread From: Keir Fraser @ 2007-01-19 15:22 UTC (permalink / raw) To: Gerd Hoffmann, Keir Fraser; +Cc: Xen devel list, Markus Armbruster On 19/1/07 15:08, "Gerd Hoffmann" <kraxel@suse.de> wrote: >> And magic numbers >> suck compared with intelligible strings for this kind of thing imo. > > We'll need both then. Strings are certainly nice for human-visible > stuff, but you don't want to strcmp() all the time in the backend. > Wouldn't be a problem for pvfb, but for blkfront/back where the ring > protocol is abi-dependent and thus the backend has to check often. You missed out the patch. I'm sure however that I'll argue you should make the enumeration local to the backend. It will always support his native architecture. Where it supports cross-architecture (i386-on-x64) he can *privately* have a numeric assignment for that situation which it uses on data paths. Then we don't have redundant info in xenstore and we don't get tied to particular magic numbers. But I definitely agree that a private enumeration, or sets of accessor hook functions, makes sense. We'll certainly need one or the other for efficiency. -- Keir ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: 32-on-64: pvfb issue 2007-01-19 15:22 ` Keir Fraser @ 2007-01-19 15:31 ` Gerd Hoffmann 2007-01-19 16:05 ` Keir Fraser 2007-01-19 16:06 ` Markus Armbruster 0 siblings, 2 replies; 50+ messages in thread From: Gerd Hoffmann @ 2007-01-19 15:31 UTC (permalink / raw) To: Keir Fraser; +Cc: Xen devel list, Markus Armbruster [-- Attachment #1: Type: text/plain, Size: 697 bytes --] Keir Fraser wrote: > You missed out the patch. Oops, attached. > I'm sure however that I'll argue you should make > the enumeration local to the backend. It will always support his native > architecture. Where it supports cross-architecture (i386-on-x64) he can > *privately* have a numeric assignment for that situation which it uses on > data paths. Then we don't have redundant info in xenstore and we don't get > tied to particular magic numbers. I don't want to put numbers into xenstore. But there are multiple backends affected (pvfb, blktab, blkback, tpm, maybe more) and thus it would be useful to share the infrastructure IMHO ... cheers, Gerd -- Gerd Hoffmann <kraxel@suse.de> [-- Attachment #2: protocol-bimodal.diff --] [-- Type: text/x-patch, Size: 1924 bytes --] --- linux-2.6-xen-sparse/include/xen/protocols.h | 57 +++++++++++++++++++++++++++ 1 file changed, 57 insertions(+) Index: build-32-unstable-13495/linux-2.6-xen-sparse/include/xen/protocols.h =================================================================== --- /dev/null +++ build-32-unstable-13495/linux-2.6-xen-sparse/include/xen/protocols.h @@ -0,0 +1,57 @@ +#ifndef __XEN_PROTOCOLS_H__ +#define __XEN_PROTOCOLS_H__ + +enum xen_io_proto_id { + /* For all front/backends */ + XEN_IO_PROTO_ABI_X86_32 = 1, + XEN_IO_PROTO_ABI_X86_64 = 2, + XEN_IO_PROTO_ABI_IA64 = 3, + XEN_IO_PROTO_ABI_POWERPC64 = 4, + + /* framebuffer via grant tables on little endian machines */ + XEN_IO_PROTO_FB_LE_GRANT = 5, +}; + +static const char *xen_io_proto_names[] = { + [ XEN_IO_PROTO_ABI_X86_32 ] = "x86_32-abi", + [ XEN_IO_PROTO_ABI_X86_64 ] = "x86_64-abi", + [ XEN_IO_PROTO_ABI_IA64 ] = "ia64-abi", + [ XEN_IO_PROTO_ABI_POWERPC64 ] = "powerpc64-abi", + + [ XEN_IO_PROTO_FB_LE_GRANT ] = "fb-le-grant", +}; +#define XEN_IO_PROTO_COUNT (sizeof(xen_io_proto_names)/sizeof(xen_io_proto_names[0])) + +static enum xen_io_proto_id xen_io_proto_name2id(const char *name) +{ + int id; + + for (id = 0; id < XEN_IO_PROTO_COUNT; id++) { + if (!xen_io_proto_names[id]) + continue; + if (0 != strcmp(name, xen_io_proto_names[id])) + continue; + return i; + } +} + +static const char *xen_io_proto_id2name(enum xen_io_proto_id id) +{ + if (id >= XEN_IO_PROTO_COUNT) + return NULL; + return xen_io_proto_names[id]; +} + +#if defined(__i386__) +# define XEN_IO_PROTO_ABI_NATIVE XEN_IO_PROTO_ABI_X86_32 +#elif defined(__x86_64__) +# define XEN_IO_PROTO_ABI_NATIVE XEN_IO_PROTO_ABI_X86_64 +#elif defined(__ia64__) +# define XEN_IO_PROTO_ABI_NATIVE XEN_IO_PROTO_ABI_IA64 +#elif defined(__powerpc64__) +# define XEN_IO_PROTO_ABI_NATIVE XEN_IO_PROTO_ABI_POWERPC64 +#else +# error arch fixup needed here +#endif + +#endif [-- Attachment #3: Type: text/plain, Size: 138 bytes --] _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: 32-on-64: pvfb issue 2007-01-19 15:31 ` Gerd Hoffmann @ 2007-01-19 16:05 ` Keir Fraser 2007-01-20 0:09 ` [Patch] [VTPM_TOOLS] Add HVM support to vtpm_manager Scarlata, Vincent R 2007-01-22 7:50 ` 32-on-64: pvfb issue Gerd Hoffmann 2007-01-19 16:06 ` Markus Armbruster 1 sibling, 2 replies; 50+ messages in thread From: Keir Fraser @ 2007-01-19 16:05 UTC (permalink / raw) To: Gerd Hoffmann, Keir Fraser; +Cc: Xen devel list, Markus Armbruster On 19/1/07 15:31, "Gerd Hoffmann" <kraxel@suse.de> wrote: >> I'm sure however that I'll argue you should make >> the enumeration local to the backend. It will always support his native >> architecture. Where it supports cross-architecture (i386-on-x64) he can >> *privately* have a numeric assignment for that situation which it uses on >> data paths. Then we don't have redundant info in xenstore and we don't get >> tied to particular magic numbers. > > I don't want to put numbers into xenstore. But there are multiple > backends affected (pvfb, blktab, blkback, tpm, maybe more) and thus it > would be useful to share the infrastructure IMHO ... And we can do so. xenbus_get_native_protocol()? Frontends can write the returned string; backends can strcmp with the returned string (and usually fail on mismatch). The few mismatches we do care about will result in us executing driver-specific code anyway: drivers can declare 'native' ABI to be 0 and have special-case driver-specific non-zero values for the non-native protocols they care about. Would that actually be more code than the potentially-knows-about-every-driver-in-the-world approach of protocols.h? If we can agree on a location for the protocol field (same directory as the xenbus state field?), and a set of names (yours are fine, including the '-abi' suffix), and a time in xenbus state machine to write the protocol (as early as possible I guess!) then let's get the frontend machinery in place first. Then we can continue to thrash out the backend details. -- Keir ^ permalink raw reply [flat|nested] 50+ messages in thread
* [Patch] [VTPM_TOOLS] Add HVM support to vtpm_manager 2007-01-19 16:05 ` Keir Fraser @ 2007-01-20 0:09 ` Scarlata, Vincent R 2007-01-22 7:50 ` 32-on-64: pvfb issue Gerd Hoffmann 1 sibling, 0 replies; 50+ messages in thread From: Scarlata, Vincent R @ 2007-01-20 0:09 UTC (permalink / raw) To: Xen devel list [-- Attachment #1: Type: text/plain, Size: 160 bytes --] VTPM_TOOLS: Added support for QEMU to communicate with vTPM over UNIX socket for HVM guests. Signed-off-by: Vinnie Scarlata <vincent.r.scarlata@intel.com> [-- Attachment #2: vtpm_tools-01182007.patch --] [-- Type: application/octet-stream, Size: 27759 bytes --] # HG changeset patch # User root@TPRL-Linux-D2.jf.intel.com # Node ID 1be602b84f6dba3ea10dabb5da88859410728194 # Parent 003d56dec2ea9ec9c3e2120cabc244d66d56bac6 VTPM_TOOLS: Added support for QEMU to communicate with vTPM over UNIX socket for HVM guests. diff -r 003d56dec2ea -r 1be602b84f6d tools/vtpm_manager/manager/vtpm_manager.h --- a/tools/vtpm_manager/manager/vtpm_manager.h Wed Jan 10 00:29:32 2007 +++ b/tools/vtpm_manager/manager/vtpm_manager.h Fri Jan 19 00:19:24 2007 @@ -70,6 +70,10 @@ #define VTPM_ORD_MIGRATE_OUT (VTPM_PRIV_BASE + 5) // migrate VTPM to dest //************************ Return Codes **************************** +#define VTPM_TYPE_PVM 1 // Paravirtualized Domain +#define VTPM_TYPE_HVM 2 // HVM Domain + +//************************ Return Codes **************************** #define VTPM_SUCCESS 0 #define VTPM_FAIL 1 #define VTPM_UNSUPPORTED 2 @@ -104,8 +108,9 @@ VTPM_Open: Input Parameters: - Domain_type: 1 byte + mig_type: 1 byte startup_mode: 1 byte // Cold Boot = 1, resume = 2, deactive = 3 + domain type: 1 byte instance_id: 4 bytes Output Parameters: None diff -r 003d56dec2ea -r 1be602b84f6d tools/examples/vtpm-impl --- a/tools/examples/vtpm-impl Wed Jan 10 00:29:32 2007 +++ b/tools/examples/vtpm-impl Fri Jan 19 00:19:24 2007 @@ -32,11 +32,14 @@ # OF THE POSSIBILITY OF SUCH DAMAGE. # =================================================================== -# | SRC | TAG | CMD SIZE | ORD | type| mode -TPM_CMD_OPEN=\\x00\\x00\\x00\\x00\\x01\\xc1\\x00\\x00\\x00\\x10\\x01\\x00\\x00\\x01\\x01\\x01 -TPM_CMD_RESM=\\x00\\x00\\x00\\x00\\x01\\xc1\\x00\\x00\\x00\\x10\\x01\\x00\\x00\\x01\\x01\\x02 +# | SRC | TAG | CMD SIZE | ORD |mtype|strt +TPM_CMD_OPEN=\\x00\\x00\\x00\\x00\\x01\\xc1\\x00\\x00\\x00\\x11\\x01\\x00\\x00\\x01\\x01\\x01 +TPM_CMD_RESM=\\x00\\x00\\x00\\x00\\x01\\xc1\\x00\\x00\\x00\\x11\\x01\\x00\\x00\\x01\\x01\\x02 TPM_CMD_CLOS=\\x00\\x00\\x00\\x00\\x01\\xc1\\x00\\x00\\x00\\x0e\\x01\\x00\\x00\\x02 TPM_CMD_DELE=\\x00\\x00\\x00\\x00\\x01\\xc1\\x00\\x00\\x00\\x0e\\x01\\x00\\x00\\x03 + +TPM_TYPE_PVM=\\x01 +TPM_TYPE_HVM=\\x02 TPM_SUCCESS=00000000 @@ -80,11 +83,25 @@ release_lock vtpm_mgr #return whether the command was successful - if [ $resp_hex != $TPM_SUCCESS ]; then + if [ $resp_hex ne $TPM_SUCCESS ]; then vtpm_fatal_error=1 false else true + fi +} + +# Helper to get vm type to pass to vtpm_manager open/resume +function vtpm_get_type() { + local inst=$(xenstore_read $XENBUS_PATH/frontend-id) + local vm=$(xenstore_read /local/domain/$inst/vm) + if [ "$vm" != "" ]; then + local ostype=$(xenstore-read $vm/image/ostype) + if [ "$ostype" == "hvm" ]; then + echo $TPM_TYPE_HVM; + else + echo $TPM_TYPE_PVM; + fi fi } @@ -99,11 +116,13 @@ # Setup vtpm instance for use. function vtpm_start() { - $(vtpm_manager_cmd $TPM_CMD_OPEN $1) + local vmtype=$(vtpm_get_type); + $(vtpm_manager_cmd $TPM_CMD_OPEN$vmtype $1) } function vtpm_resume() { - $(vtpm_manager_cmd $TPM_CMD_RESM $1) + local vmtype=$(vtpm_get_type); + $(vtpm_manager_cmd $TPM_CMD_RESM$vmtype $1) } # Reset the vtpm AKA clear PCRs diff -r 003d56dec2ea -r 1be602b84f6d tools/vtpm_manager/manager/vtpm_manager_handler.c --- a/tools/vtpm_manager/manager/vtpm_manager_handler.c Wed Jan 10 00:29:32 2007 +++ b/tools/vtpm_manager/manager/vtpm_manager_handler.c Fri Jan 19 00:19:24 2007 @@ -40,6 +40,7 @@ #include <stdio.h> #include <unistd.h> #include <string.h> +#include <errno.h> #include "vtpm_manager.h" #include "vtpmpriv.h" @@ -105,7 +106,7 @@ for (i=0; i<size_read; i++) vtpmhandlerloginfomore(VTPM_LOG_VTPM_DEEP, "%x ", cmd_header[i]); } else { - vtpmhandlerlogerror(VTPM_LOG_VTPM, "%s can't read from ipc. Aborting... \n", thread_name); + vtpmhandlerlogerror(VTPM_LOG_VTPM, "%s can't read from ipc. Errono = %d. Aborting... \n", thread_name, errno); goto abort_command; } diff -r 003d56dec2ea -r 1be602b84f6d tools/vtpm/vtpm.patch --- a/tools/vtpm/vtpm.patch Wed Jan 10 00:29:32 2007 +++ b/tools/vtpm/vtpm.patch Fri Jan 19 00:19:24 2007 @@ -1,14 +1,14 @@ diff -uprN tpm_emulator/AUTHORS vtpm/AUTHORS ---- tpm_emulator/AUTHORS 2006-07-24 14:35:35.000000000 -0700 -+++ vtpm/AUTHORS 2006-07-24 14:35:35.000000000 -0700 +--- tpm_emulator/AUTHORS 2006-12-08 12:51:29.000000000 -0800 ++++ vtpm/AUTHORS 2006-12-13 16:38:52.000000000 -0800 @@ -1,3 +1,3 @@ Mario Strasser <mast@gmx.net> Heiko Stamer <stamer@gaos.org> [DAA] -INTEL Corp <> [Dropped to Ring3] +INTEL Corp <> [VTPM Extensions] diff -uprN tpm_emulator/ChangeLog vtpm/ChangeLog ---- tpm_emulator/ChangeLog 2006-07-24 14:35:35.000000000 -0700 -+++ vtpm/ChangeLog 2006-07-24 14:35:35.000000000 -0700 +--- tpm_emulator/ChangeLog 2006-12-08 12:51:29.000000000 -0800 ++++ vtpm/ChangeLog 2006-12-13 16:38:52.000000000 -0800 @@ -1,5 +1,6 @@ ????-??-?? Intel Corp * Moved module out of kernel to run as a ring 3 app @@ -17,9 +17,9 @@ 2006-06-23 Mario Strasser <mast@gmx.net> * tpm_startup.c: behaviour of ST_CLEAR and storage of diff -uprN tpm_emulator/linux_module.h vtpm/linux_module.h ---- tpm_emulator/linux_module.h 2006-07-24 14:35:35.000000000 -0700 -+++ vtpm/linux_module.h 2006-07-24 14:35:35.000000000 -0700 -@@ -44,18 +44,21 @@ +--- tpm_emulator/linux_module.h 2006-12-08 12:51:29.000000000 -0800 ++++ vtpm/linux_module.h 2007-01-09 14:49:06.000000000 -0800 +@@ -44,18 +44,26 @@ #define TPM_DEVICE_NAME "tpm" #define TPM_MODULE_NAME "tpm_emulator" @@ -31,8 +31,13 @@ - __FILE__, __LINE__, ## __VA_ARGS__) +#define debug(fmt, ...) printf("TPMD[%d]: %s:%d: Debug: " fmt "\n", \ + dmi_id, __FILE__, __LINE__, ## __VA_ARGS__) ++#define debug_nostop(fmt, ...) printf("TPMD[%d]: %s:%d: Debug: " fmt, \ ++ dmi_id, __FILE__, __LINE__, ## __VA_ARGS__) ++#define debug_more(fmt, ...) printf( fmt, ## __VA_ARGS__ ) #else #define debug(fmt, ...) ++#define debug_nostop(fmt, ...) ++#define debug_more(fmt, ...) #endif -#define info(fmt, ...) printf("TPMD: %s:%d: Info: " fmt "\n", \ - __FILE__, __LINE__, ## __VA_ARGS__) @@ -50,8 +55,8 @@ /* memory allocation */ diff -uprN tpm_emulator/Makefile vtpm/Makefile ---- tpm_emulator/Makefile 2006-07-24 14:35:35.000000000 -0700 -+++ vtpm/Makefile 2006-07-24 14:35:35.000000000 -0700 +--- tpm_emulator/Makefile 2006-12-08 12:51:29.000000000 -0800 ++++ vtpm/Makefile 2006-12-13 16:38:52.000000000 -0800 @@ -7,7 +7,7 @@ COMPILE_ARCH ?= $(shell uname -m | sed -e s/i.86/x86_32/) @@ -83,9 +88,46 @@ .PHONY: all install clean dist gmp version + +diff -uprN tpm_emulator/tpm/tpm_capability.c vtpm/tpm/tpm_capability.c +--- tpm_emulator/tpm/tpm_capability.c 2006-06-23 03:37:07.000000000 -0700 ++++ vtpm/tpm/tpm_capability.c 2007-01-10 10:00:49.000000000 -0800 +@@ -136,8 +136,18 @@ static TPM_RESULT cap_property(UINT32 su + + case TPM_CAP_PROP_TIS_TIMEOUT: + debug("[TPM_CAP_PROP_TIS_TIMEOUT]"); +- /* TODO: TPM_CAP_PROP_TIS_TIMEOUT */ +- return TPM_FAIL; ++ /* TODO: TPM_CAP_PROP_TIS_TIMEOUT: Measure these values and determine correct ones */ ++ UINT32 len = *respSize = 16; ++ BYTE *ptr = *resp = tpm_malloc(*respSize); ++ if (ptr == NULL || ++ tpm_marshal_UINT32(&ptr, &len, 200000) || ++ tpm_marshal_UINT32(&ptr, &len, 200000) || ++ tpm_marshal_UINT32(&ptr, &len, 200000) || ++ tpm_marshal_UINT32(&ptr, &len, 200000)) { ++ tpm_free(*resp); ++ return TPM_FAIL; ++ } ++ return TPM_SUCCESS; + + case TPM_CAP_PROP_STARTUP_EFFECT: + debug("[TPM_CAP_PROP_STARTUP_EFFECT]"); +@@ -190,7 +200,11 @@ static TPM_RESULT cap_property(UINT32 su + + case TPM_CAP_PROP_DURATION: + debug("[TPM_CAP_PROP_DURATION]"); +- /* TODO: TPM_CAP_PROP_DURATION */ ++ /* TODO: TPM_CAP_PROP_DURATION: Measure these values and return accurate ones */ ++ BYTE dur[]= {0x0,0x0,0x0,0xc,0x0,0x7,0xa1,0x20,0x0,0x1e,0x84,0x80,0x11,0xe1,0xa3,0x0}; ++ *respSize = 16; ++ *resp = tpm_malloc(*respSize); ++ memcpy(*resp,dur,16); + return TPM_FAIL; + + case TPM_CAP_PROP_ACTIVE_COUNTER: diff -uprN tpm_emulator/tpm/tpm_data.c vtpm/tpm/tpm_data.c ---- tpm_emulator/tpm/tpm_data.c 2006-07-24 14:35:35.000000000 -0700 -+++ vtpm/tpm/tpm_data.c 2006-07-24 14:35:35.000000000 -0700 +--- tpm_emulator/tpm/tpm_data.c 2006-12-08 12:51:29.000000000 -0800 ++++ vtpm/tpm/tpm_data.c 2006-12-13 16:38:52.000000000 -0800 @@ -1,6 +1,7 @@ /* Software-Based Trusted Platform Module (TPM) Emulator for Linux * Copyright (C) 2004 Mario Strasser <mast@gmx.net>, @@ -371,10 +413,15 @@ #else diff -uprN tpm_emulator/tpmd.c vtpm/tpmd.c ---- tpm_emulator/tpmd.c 2006-07-24 14:35:35.000000000 -0700 -+++ vtpm/tpmd.c 2006-07-24 14:35:35.000000000 -0700 -@@ -23,13 +23,27 @@ +--- tpm_emulator/tpmd.c 2006-12-08 12:51:29.000000000 -0800 ++++ vtpm/tpmd.c 2007-01-09 14:48:56.000000000 -0800 +@@ -21,12 +21,24 @@ + #include <sys/stat.h> + #include <fcntl.h> #include <sys/time.h> ++#include <sys/socket.h> ++#include <sys/un.h> ++#include <errno.h> #include "tpm_emulator.h" +#include "vtpm_manager.h" @@ -384,61 +431,115 @@ +#ifdef VTPM_MULTI_VM + #define DEV_BE "/dev/vtpm" +#else -+ #define GUEST_RX_FIFO_D "/var/vtpm/fifos/tpm_cmd_to_%d.fifo" -+ #define GUEST_TX_FIFO "/var/vtpm/fifos/tpm_rsp_from_all.fifo" -+#endif - ++ #define PVM_RX_FIFO_D "/var/vtpm/fifos/tpm_cmd_to_%d.fifo" ++ #define PVM_TX_FIFO "/var/vtpm/fifos/tpm_rsp_from_all.fifo" + ++ #define HVM_RX_FIFO_D "/var/vtpm/socks/%d.socket" ++#endif ++ + int dmi_id; + #define BUFFER_SIZE 2048 -+static uint8_t ctrl_msg[] = { 0, 0, 0, 0, // destination -+ 1, 193, // VTPM_TAG -+ 0, 0, 0, 10, // Size -+ 0, 0, 0, 0}; // TPM_SUCCESS -+ -+ static int devurandom=0; -+ - - void get_random_bytes(void *buf, int nbytes) { - -@@ -52,18 +66,26 @@ uint64_t tpm_get_ticks(void) +@@ -38,7 +50,7 @@ void get_random_bytes(void *buf, int nby + } + + if (read(devurandom, buf, nbytes) != nbytes) { +- printf("Can't get random number.\n"); ++ error("Can't get random number.\n"); + exit(-1); + } + } +@@ -52,105 +64,182 @@ uint64_t tpm_get_ticks(void) int main(int argc, char **argv) { - uint8_t in[BUFFER_SIZE], *out; -+ uint8_t in[BUFFER_SIZE], *out, *addressed_out; ++ uint8_t type, in[BUFFER_SIZE], *out, *addressed_out; ++ char *vtpm_rx_file=NULL; uint32_t out_size; int in_size, written; - int i; - struct stat file_info; -- ++ int i, guest_id=-1; + - int tpm_tx_fh=-1, tpm_rx_fh=-1; -+ int i, guest_id=-1; ++#ifndef VTPM_MULTI_VM ++ int sockfd = -1; ++ struct sockaddr_un addr; ++ struct sockaddr_un client_addr; ++ unsigned int client_length; ++ ++#endif + + int vtpm_tx_fh=-1, vtpm_rx_fh=-1; +#ifdef VTPM_MULTI_VM if (argc < 2) { - printf("Usage: tpmd clear|save|deactivated\n" ); -+#else -+ if (argc < 3) { -+ printf("Usage: tpmd clear|save|deactivated vtpmid\n" ); +- printf("Usage: tpmd clear|save|deactivated\n" ); ++ error("Usage: tpmd clear|save|deactivated\n" ); ++#else ++ if (argc < 4) { ++ error("Usage: tpmd clear|save|deactivated pvm|hvm vtpmid\n" ); +#endif return -1; } +#ifndef VTPM_MULTI_VM -+ dmi_id = atoi(argv[2]); ++ /* setup type of vm */ ++ if (!strcmp(argv[2], "pvm")) { ++ type = VTPM_TYPE_PVM; // Get commands from vTPM Manager through fifo ++ } else if (!strcmp(argv[2], "hvm")) { ++ type = VTPM_TYPE_HVM; // Get commands from qemu via socket ++ } else { ++ error("invalid vTPM type '%s'.\n", argv[2]); ++ } ++ ++ dmi_id = atoi(argv[3]); ++ ++ if (type == VTPM_TYPE_PVM) { ++ vtpm_rx_file = malloc(10 + strlen(PVM_RX_FIFO_D)); ++ sprintf(vtpm_rx_file, PVM_RX_FIFO_D, (uint32_t) dmi_id); ++ } else { ++ vtpm_rx_file = malloc(10 + strlen(HVM_RX_FIFO_D)); ++ sprintf(vtpm_rx_file, HVM_RX_FIFO_D, (uint32_t) dmi_id); ++ ++ if ( (sockfd = socket(PF_UNIX,SOCK_STREAM,0)) < 0) { ++ error("Unable to create socket. errno = %d\n", errno); ++ exit (-1); ++ } ++ ++ memset(&addr, 0, sizeof(addr)); ++ addr.sun_family = AF_UNIX; ++ strcpy(addr.sun_path,vtpm_rx_file ); ++ unlink(addr.sun_path); ++ } ++#endif ++ ++#ifdef VTPM_MULTI_VM ++ info("Initializing tpm state: %s\n", argv[1]); ++#else ++ info("Initializing tpm state: %s, type: %s, id: %d\n", argv[1], argv[2], dmi_id); +#endif + /* initialize TPM emulator */ if (!strcmp(argv[1], "clear")) { - printf("Initializing tpm: %s\n", argv[1]); -@@ -80,46 +102,30 @@ int main(int argc, char **argv) +- printf("Initializing tpm: %s\n", argv[1]); + tpm_emulator_init(1); +- } else if (!strcmp(argv[1], "save")) { +- printf("Initializing tpm: %s\n", argv[1]); ++ } else if (!strcmp(argv[1], "save")) { + tpm_emulator_init(2); + } else if (!strcmp(argv[1], "deactivated")) { +- printf("Initializing tpm: %s\n", argv[1]); + tpm_emulator_init(3); + } else { +- printf("invalid startup mode '%s'; must be 'clear', " ++ error("invalid startup mode '%s'; must be 'clear', " + "'save' (default) or 'deactivated", argv[1]); return -1; } - +- - if ( stat(TPM_RX_FNAME, &file_info) == -1) { - if ( mkfifo(TPM_RX_FNAME, S_IWUSR | S_IRUSR ) ) { - printf("Failed to create fifo %s.\n", TPM_RX_FNAME); @@ -453,8 +554,6 @@ - } - } - -+ char *guest_rx_file = malloc(10 + strlen(GUEST_RX_FIFO_D)); -+ sprintf(guest_rx_file, GUEST_RX_FIFO_D, (uint32_t) dmi_id); + while (1) { abort_command: @@ -462,15 +561,33 @@ - tpm_rx_fh = open(TPM_RX_FNAME, O_RDONLY); + if (vtpm_rx_fh < 0) { +#ifdef VTPM_MUTLI_VM -+ vtpm_rx_fh = open(DEV_BE, O_RDWR); -+#else -+ vtpm_rx_fh = open(guest_rx_file, O_RDONLY); ++ vtpm_rx_fh = open(DEV_BE, O_RDWR); ++#else ++ if (type == VTPM_TYPE_PVM) { ++ vtpm_rx_fh = open(vtpm_rx_file, O_RDONLY); ++ } else { ++ if (bind(sockfd, (struct sockaddr *)&addr, sizeof(addr)) < 0) { ++ error("Unable to bind(). errno = %d\n", errno); ++ exit (-1); ++ } ++ ++ if (listen(sockfd, 10) <0) { ++ error("Unable to listen(). errno = %d\n", errno); ++ exit (-1); ++ } ++ ++ memset(&client_addr, 0, sizeof(client_addr)); ++ client_length = sizeof(client_addr); ++ ++ vtpm_rx_fh = vtpm_tx_fh = accept(sockfd, &client_addr, &client_length); ++ } +#endif } - if (tpm_rx_fh < 0) { +- printf("ERROR: failed to open devices to listen to guest.\n"); + if (vtpm_rx_fh < 0) { - printf("ERROR: failed to open devices to listen to guest.\n"); ++ error("Failed to open devices to listen to guest.\n"); return -1; } @@ -486,7 +603,8 @@ - in_size = read(tpm_rx_fh, in, BUFFER_SIZE); + in_size = read(vtpm_rx_fh, in, BUFFER_SIZE); if (in_size < 6) { // Magic size of minium TPM command - printf("Recv[%d] to small: 0x", in_size); +- printf("Recv[%d] to small: 0x", in_size); ++ info("Recv incomplete command of %d bytes.", in_size); if (in_size <= 0) { - close(tpm_rx_fh); - tpm_rx_fh = -1; @@ -495,8 +613,13 @@ goto abort_command; } } else { -@@ -129,28 +135,73 @@ abort_command: - printf("\n"); +- printf("Recv[%d]: 0x", in_size); ++ debug_nostop("Recv[%d]: 0x", in_size); + for (i=0; i< in_size; i++) +- printf("%x ", in[i]); +- printf("\n"); ++ debug_more("%x ", in[i]); ++ debug_more("\n"); } - @@ -504,71 +627,56 @@ - printf("ERROR: Handler Failed.\n"); + if (guest_id == -1) { + guest_id = *((uint32_t *) in); -+ *((uint32_t *) ctrl_msg) = *((uint32_t *) in); + } else { + if (guest_id != *((uint32_t *) in) ) { -+ printf("WARNING: More than one guest attached\n"); ++ error("WARNING: More than one guest attached\n"); + } ++ } ++ ++ if (vtpm_tx_fh < 0) { ++#ifdef VTPM_MUTLI_VM ++ vtpm_tx_fh = open(DEV_BE, O_RDWR); ++ vtpm_rx_fh = vtpm_tx_fh; ++#else ++ if (type == VTPM_TYPE_PVM) { ++ vtpm_tx_fh = open(PVM_TX_FIFO, O_WRONLY); ++ } // No need to open the other direction for HVM ++#endif ++ } ++ ++ if (vtpm_tx_fh < 0) { ++ error("Failed to open devices to respond to guest.\n"); ++ return -1; ++ } ++ ++ // Handle the command, but skip the domain id header ++ if (tpm_handle_command(in + sizeof(uint32_t), in_size - sizeof(uint32_t), &out, &out_size) != 0) { ++ error("Handler Failed.\n"); } - written = write(tpm_tx_fh, out, out_size); -+ if (vtpm_tx_fh < 0) { -+#ifdef VTPM_MUTLI_VM -+ vtpm_tx_fh = open(DEV_BE, O_RDWR); -+ vtpm_rx_fh = vtpm_tx_fh; -+#else -+ vtpm_tx_fh = open(GUEST_TX_FIFO, O_WRONLY); -+#endif -+ } ++ addressed_out = (uint8_t *) tpm_malloc(sizeof(uint32_t) + out_size); ++ *(uint32_t *) addressed_out = *(uint32_t *) in; ++ memcpy(addressed_out + sizeof(uint32_t), out, out_size); ++ ++ written = write(vtpm_tx_fh, addressed_out, out_size + sizeof(uint32_t)); - if (written != out_size ) { - printf("ERROR: Part of response not written %d/%d.\nAttempt: ", written, out_size); -- } else { ++ if (written != out_size + sizeof(uint32_t)) { ++ error("Part of response not written %d/%d.\n", written, out_size); + } else { - printf("Sent[%Zu]: ", out_size); -+ if (vtpm_tx_fh < 0) { -+ printf("ERROR: failed to open devices to respond to guest.\n"); -+ return -1; -+ } -+ -+ // Handle command, but we need to skip the identifier -+ if ( BE16_TO_CPU( ((uint16_t *) in)[2] ) == VTPM_TAG_REQ ) { // Control message from xend -+ // This DM doesn't really care about ctrl messages. Just ACK the message -+ written = write(vtpm_tx_fh, ctrl_msg, sizeof(ctrl_msg)); -+ -+ if (written != sizeof(ctrl_msg)) { -+ printf("ERROR: Part of response not written %d/%Zu.\n", written, sizeof(ctrl_msg)); -+ } else { -+ printf("Send Ctrl Message confermation\n"); -+ } -+ } else { // Message from Guest -+ if (tpm_handle_command(in + sizeof(uint32_t), in_size - sizeof(uint32_t), &out, &out_size) != 0) { -+ printf("ERROR: Handler Failed.\n"); -+ } -+ -+ addressed_out = (uint8_t *) tpm_malloc(sizeof(uint32_t) + out_size); -+ *(uint32_t *) addressed_out = *(uint32_t *) in; -+ memcpy(addressed_out + sizeof(uint32_t), out, out_size); -+ -+ written = write(vtpm_tx_fh, addressed_out, out_size + sizeof(uint32_t)); -+ -+ if (written != out_size + sizeof(uint32_t)) { -+ printf("ERROR: Part of response not written %d/%d.\n", written, out_size); -+ for (i=0; i< out_size+ sizeof(uint32_t); i++) -+ printf("%x ", addressed_out[i]); -+ printf("\n"); -+ } else { -+ printf("Sent[%Zu]: ", out_size + sizeof(uint32_t)); -+ for (i=0; i< out_size+ sizeof(uint32_t); i++) -+ printf("%x ", addressed_out[i]); -+ printf("\n"); -+ } -+ tpm_free(out); -+ tpm_free(addressed_out); ++ debug_nostop("Sent[%Zu]: ", out_size + sizeof(uint32_t)); ++ for (i=0; i< out_size+ sizeof(uint32_t); i++) ++ debug_more("%x ", addressed_out[i]); ++ debug_more("\n"); } - for (i=0; i< out_size; i++) - printf("%x ", out[i]); - printf("\n"); -- tpm_free(out); + tpm_free(out); ++ tpm_free(addressed_out); } // loop @@ -579,19 +687,7 @@ + close(vtpm_tx_fh); +#ifndef VTPM_MUTLI_VM + close(vtpm_rx_fh); -+ free (guest_rx_file); ++ free (vtpm_rx_file); +#endif } -Binary files tpm_emulator/tpm_emulator and vtpm/tpm_emulator differ -diff -uprN tpm_emulator/tpm_version.h vtpm/tpm_version.h ---- tpm_emulator/tpm_version.h 2006-07-24 14:35:41.000000000 -0700 -+++ vtpm/tpm_version.h 2006-07-24 14:35:35.000000000 -0700 -@@ -2,5 +2,5 @@ - #define _TPM_VERSION_H_ - #define VERSION_MAJOR 0 - #define VERSION_MINOR 4 --#define VERSION_BUILD 1153776940 -+#define VERSION_BUILD 1153776935 - #endif /* _TPM_VERSION_H_ */ -Binary files tpm_emulator/vtpmd and vtpm/vtpmd differ diff -r 003d56dec2ea -r 1be602b84f6d tools/vtpm_manager/manager/dmictl.c --- a/tools/vtpm_manager/manager/dmictl.c Wed Jan 10 00:29:32 2007 +++ b/tools/vtpm_manager/manager/dmictl.c Fri Jan 19 00:19:24 2007 @@ -54,7 +54,7 @@ // if dmi_res is non-null, then return a pointer to new object. // Also, this does not fill in the measurements. They should be filled by // design dependent code or saveNVM -TPM_RESULT init_dmi(UINT32 dmi_id, BYTE type, VTPM_DMI_RESOURCE **dmi_res) { +TPM_RESULT init_dmi(UINT32 dmi_id, BYTE dmi_type, VTPM_DMI_RESOURCE **dmi_res) { TPM_RESULT status=TPM_SUCCESS; VTPM_DMI_RESOURCE *new_dmi=NULL; @@ -66,6 +66,7 @@ } memset(new_dmi, 0, sizeof(VTPM_DMI_RESOURCE)); new_dmi->dmi_id = dmi_id; + new_dmi->dmi_type = dmi_type; new_dmi->connected = FALSE; new_dmi->TCSContext = 0; @@ -120,47 +121,46 @@ VTPM_DMI_RESOURCE *new_dmi=NULL; TPM_RESULT status=TPM_FAIL; - BYTE type, startup_mode; + BYTE dmi_type, vm_type, startup_mode; UINT32 dmi_id; if (param_buf == NULL) { // Assume creation of Dom 0 control - type = VTPM_TYPE_NON_MIGRATABLE; + dmi_type = VTPM_TYPE_NON_MIGRATABLE; dmi_id = VTPM_CTL_DM; - } else if (buffer_len(param_buf) != sizeof(BYTE) + sizeof(BYTE) + sizeof(UINT32)) { + } else if (buffer_len(param_buf) != sizeof(BYTE) * 3 + sizeof(UINT32)) { vtpmloginfo(VTPM_LOG_VTPM, "New DMI command wrong length: %d.\n", buffer_len(param_buf)); status = TPM_BAD_PARAMETER; goto abort_egress; } else { vtpm_globals->connected_dmis++; // Put this here so we don't count Dom0 - BSG_UnpackList( param_buf->bytes, 3, - BSG_TYPE_BYTE, &type, + BSG_UnpackList( param_buf->bytes, 4, + BSG_TYPE_BYTE, &dmi_type, BSG_TYPE_BYTE, &startup_mode, + BSG_TYPE_BYTE, &vm_type, BSG_TYPE_UINT32, &dmi_id); + } + + if ((dmi_type != VTPM_TYPE_NON_MIGRATABLE) && (dmi_type != VTPM_TYPE_MIGRATABLE)) { + vtpmlogerror(VTPM_LOG_VTPM, "Creation of VTPM with illegal type.\n"); + status = TPM_BAD_PARAMETER; + goto abort_egress; } new_dmi = (VTPM_DMI_RESOURCE *) hashtable_search(vtpm_globals->dmi_map, &dmi_id); if (new_dmi == NULL) { vtpmloginfo(VTPM_LOG_VTPM, "Creating new DMI instance %d attached.\n", dmi_id ); // Brand New DMI. Initialize the persistent pieces - TPMTRYRETURN(init_dmi(dmi_id, type, &new_dmi) ); + TPMTRYRETURN(init_dmi(dmi_id, dmi_type, &new_dmi) ); } else vtpmloginfo(VTPM_LOG_VTPM, "Re-attaching DMI instance %d.\n", dmi_id); - if (type != VTPM_TYPE_MIGRATED) { - new_dmi->dmi_type = type; - } else { - vtpmlogerror(VTPM_LOG_VTPM, "Creation of VTPM with illegal type.\n"); - status = TPM_BAD_PARAMETER; - goto abort_egress; - } - if (new_dmi->connected) { vtpmlogerror(VTPM_LOG_VTPM, "Attempt to re-attach, currently attached instance %d. Ignoring\n", dmi_id); status = TPM_BAD_PARAMETER; goto abort_egress; } - if (type == VTPM_TYPE_MIGRATED) { + if (new_dmi->dmi_type == VTPM_TYPE_MIGRATED) { vtpmlogerror(VTPM_LOG_VTPM, "Attempt to re-attach previously migrated instance %d without recovering first. Ignoring\n", dmi_id); status = TPM_BAD_PARAMETER; goto abort_egress; @@ -173,7 +173,7 @@ // Design specific new DMI code. // Includes: create IPCs, Measuring DMI, and maybe launching DMI - status = VTPM_New_DMI_Extra(new_dmi, startup_mode); + TPMTRYRETURN(VTPM_New_DMI_Extra(new_dmi, vm_type, startup_mode) ); goto egress; abort_egress: diff -r 003d56dec2ea -r 1be602b84f6d tools/vtpm_manager/manager/vtpmd.c --- a/tools/vtpm_manager/manager/vtpmd.c Wed Jan 10 00:29:32 2007 +++ b/tools/vtpm_manager/manager/vtpmd.c Fri Jan 19 00:19:24 2007 @@ -63,6 +63,9 @@ #define VTPM_TX_HP_FNAME "/var/vtpm/fifos/to_console.fifo" #define VTPM_RX_HP_FNAME "/var/vtpm/fifos/from_console.fifo" +#define VTPM_TYPE_PVM_STRING "pvm" +#define VTPM_TYPE_HVM_STRING "hvm" + struct vtpm_thread_params_s { vtpm_ipc_handle_t *tx_ipc_h; vtpm_ipc_handle_t *rx_ipc_h; @@ -104,12 +107,12 @@ struct sigaction ctl_c_handler; -TPM_RESULT VTPM_New_DMI_Extra(VTPM_DMI_RESOURCE *dmi_res, BYTE startup_mode) { +TPM_RESULT VTPM_New_DMI_Extra(VTPM_DMI_RESOURCE *dmi_res, BYTE vm_type, BYTE startup_mode) { TPM_RESULT status = TPM_SUCCESS; int fh; char dmi_id_str[11]; // UINT32s are up to 10 digits + NULL - char *tx_vtpm_name, *tx_tpm_name; + char *tx_vtpm_name, *tx_tpm_name, *vm_type_string; struct stat file_info; if (dmi_res->dmi_id == VTPM_CTL_DM) { @@ -156,6 +159,10 @@ */ memset(&dmi_res->DMI_measurement, 0xcc, sizeof(TPM_DIGEST)); + if (vm_type == VTPM_TYPE_PVM) + vm_type_string = (BYTE *)&VTPM_TYPE_PVM_STRING; + else + vm_type_string = (BYTE *)&VTPM_TYPE_HVM_STRING; // Launch DMI sprintf(dmi_id_str, "%d", (int) dmi_res->dmi_id); @@ -172,13 +179,13 @@ } else if (pid == 0) { switch (startup_mode) { case TPM_ST_CLEAR: - execl (TPM_EMULATOR_PATH, "vtpmd", "clear", dmi_id_str, NULL); + execl (TPM_EMULATOR_PATH, "vtpmd", "clear", vm_type_string, dmi_id_str, NULL); break; case TPM_ST_STATE: - execl (TPM_EMULATOR_PATH, "vtpmd", "save", dmi_id_str, NULL); + execl (TPM_EMULATOR_PATH, "vtpmd", "save", vm_type_string, dmi_id_str, NULL); break; case TPM_ST_DEACTIVATED: - execl (TPM_EMULATOR_PATH, "vtpmd", "deactivated", dmi_id_str, NULL); + execl (TPM_EMULATOR_PATH, "vtpmd", "deactivated", vm_type_string, dmi_id_str, NULL); break; default: status = TPM_BAD_PARAMETER; diff -r 003d56dec2ea -r 1be602b84f6d tools/vtpm_manager/Rules.mk --- a/tools/vtpm_manager/Rules.mk Wed Jan 10 00:29:32 2007 +++ b/tools/vtpm_manager/Rules.mk Fri Jan 19 00:19:24 2007 @@ -39,7 +39,7 @@ CFLAGS += -D_GNU_SOURCE # Logging Level. See utils/tools.h for usage -CFLAGS += -DLOGGING_MODULES="(BITMASK(VTPM_LOG_TCS)|BITMASK(VTPM_LOG_VTSP)|BITMASK(VTPM_LOG_VTPM)|BITMASK(VTPM_LOG_VTPM_DEEP))" +CFLAGS += -DLOGGING_MODULES="(BITMASK(VTPM_LOG_TCS)|BITMASK(VTPM_LOG_VTSP)|BITMASK(VTPM_LOG_VTPM))" # Silent Mode #CFLAGS += -DLOGGING_MODULES=0x0 diff -r 003d56dec2ea -r 1be602b84f6d tools/vtpm_manager/manager/vtpmpriv.h --- a/tools/vtpm_manager/manager/vtpmpriv.h Wed Jan 10 00:29:32 2007 +++ b/tools/vtpm_manager/manager/vtpmpriv.h Fri Jan 19 00:19:24 2007 @@ -165,7 +165,7 @@ TPM_RESULT VTPM_SaveManagerData(void); TPM_RESULT VTPM_LoadManagerData(void); -TPM_RESULT VTPM_New_DMI_Extra(VTPM_DMI_RESOURCE *dmi_res, BYTE startup_mode); +TPM_RESULT VTPM_New_DMI_Extra(VTPM_DMI_RESOURCE *dmi_res, BYTE vm_type, BYTE startup_mode); TPM_RESULT VTPM_Close_DMI_Extra(VTPM_DMI_RESOURCE *dmi_res); diff -r 003d56dec2ea -r 1be602b84f6d tools/vtpm_manager/manager/Makefile --- a/tools/vtpm_manager/manager/Makefile Wed Jan 10 00:29:32 2007 +++ b/tools/vtpm_manager/manager/Makefile Fri Jan 19 00:19:24 2007 @@ -13,6 +13,9 @@ install: build if [ ! -d "$(DESTDIR)/var/vtpm/fifos" ]; \ then mkdir -p $(DESTDIR)/var/vtpm/fifos; \ + fi + if [ ! -d "$(DESTDIR)/var/vtpm/socks" ]; \ + then mkdir -p $(DESTDIR)/var/vtpm/socks; \ fi $(INSTALL_PROG) $(BIN) $(TOOLS_INSTALL_DIR) [-- Attachment #3: Type: text/plain, Size: 138 bytes --] _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: 32-on-64: pvfb issue 2007-01-19 16:05 ` Keir Fraser 2007-01-20 0:09 ` [Patch] [VTPM_TOOLS] Add HVM support to vtpm_manager Scarlata, Vincent R @ 2007-01-22 7:50 ` Gerd Hoffmann 2007-01-22 14:01 ` Gerd Hoffmann 1 sibling, 1 reply; 50+ messages in thread From: Gerd Hoffmann @ 2007-01-22 7:50 UTC (permalink / raw) To: Keir Fraser; +Cc: Xen devel list, Markus Armbruster Hi, > And we can do so. xenbus_get_native_protocol()? Frontends can write the > returned string; backends can strcmp with the returned string (and usually > fail on mismatch). The few mismatches we do care about will result in us > executing driver-specific code anyway: drivers can declare 'native' ABI to > be 0 and have special-case driver-specific non-zero values for the > non-native protocols they care about. Would that actually be more code than > the potentially-knows-about-every-driver-in-the-world approach of > protocols.h? I'll go code up both front and back bits for block and pvfb to see how it works out in practice, I think I'll have patches later today or tomorrow ... > If we can agree on a location for the protocol field (same directory as the > xenbus state field?), Yes. > and a set of names (yours are fine, including the > '-abi' suffix), Yep, those I'll plan to put into a common header file so they can be shared in any case. Not sure any more how useful sharing the enum for the protocol actually is, I'll see when coding up things. > and a time in xenbus state machine to write the protocol Same transaction event-channel is written. stay tuned, Gerd -- Gerd Hoffmann <kraxel@suse.de> ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: 32-on-64: pvfb issue 2007-01-22 7:50 ` 32-on-64: pvfb issue Gerd Hoffmann @ 2007-01-22 14:01 ` Gerd Hoffmann 2007-01-22 14:48 ` Keir Fraser 2007-01-22 15:22 ` 32-on-64: pvfb issue Markus Armbruster 0 siblings, 2 replies; 50+ messages in thread From: Gerd Hoffmann @ 2007-01-22 14:01 UTC (permalink / raw) To: Gerd Hoffmann; +Cc: Xen devel list, Keir Fraser, Markus Armbruster [-- Attachment #1: Type: text/plain, Size: 574 bytes --] Gerd Hoffmann wrote: > I'll go code up both front and back bits for block and pvfb to see how > it works out in practice, I think I'll have patches later today or > tomorrow ... Here we go. Compile-tested on 32bit, more tests coming, full rebuild still in progress ... Attached are: protocol-bimodal.diff short header file with the protocol names. {blk,fb}front-bimodal.diff small frontend driver patches, just add the protocol name to xenstore. {blk,fb}back-bimodal.diff backend patches (for unstable only). cheers, Gerd -- Gerd Hoffmann <kraxel@suse.de> [-- Attachment #2: protocol-bimodal.diff --] [-- Type: text/x-patch, Size: 987 bytes --] --- xen/include/public/io/protocols.h | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) Index: build-32-unstable-13495/xen/include/public/io/protocols.h =================================================================== --- /dev/null +++ build-32-unstable-13495/xen/include/public/io/protocols.h @@ -0,0 +1,21 @@ +#ifndef __XEN_PROTOCOLS_H__ +#define __XEN_PROTOCOLS_H__ + +#define XEN_IO_PROTO_ABI_X86_32 "x86_32-abi" +#define XEN_IO_PROTO_ABI_X86_64 "x86_64-abi" +#define XEN_IO_PROTO_ABI_IA64 "ia64-abi" +#define XEN_IO_PROTO_ABI_POWERPC64 "powerpc64-abi" + +#if defined(__i386__) +# define XEN_IO_PROTO_ABI_NATIVE XEN_IO_PROTO_ABI_X86_32 +#elif defined(__x86_64__) +# define XEN_IO_PROTO_ABI_NATIVE XEN_IO_PROTO_ABI_X86_64 +#elif defined(__ia64__) +# define XEN_IO_PROTO_ABI_NATIVE XEN_IO_PROTO_ABI_IA64 +#elif defined(__powerpc64__) +# define XEN_IO_PROTO_ABI_NATIVE XEN_IO_PROTO_ABI_POWERPC64 +#else +# error arch fixup needed here +#endif + +#endif [-- Attachment #3: fbfront-bimodal.diff --] [-- Type: text/x-patch, Size: 1260 bytes --] --- linux-2.6-xen-sparse/drivers/xen/fbfront/xenfb.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) Index: build-32-unstable-13495/linux-2.6-xen-sparse/drivers/xen/fbfront/xenfb.c =================================================================== --- build-32-unstable-13495.orig/linux-2.6-xen-sparse/drivers/xen/fbfront/xenfb.c +++ build-32-unstable-13495/linux-2.6-xen-sparse/drivers/xen/fbfront/xenfb.c @@ -27,6 +27,7 @@ #include <asm/hypervisor.h> #include <xen/evtchn.h> #include <xen/interface/io/fbif.h> +#include <xen/interface/io/protocols.h> #include <xen/xenbus.h> #include <linux/kthread.h> @@ -479,7 +480,7 @@ static int __devinit xenfb_probe(struct goto error_nomem; /* set up shared page */ - info->page = (void *)__get_free_page(GFP_KERNEL); + info->page = (void *)__get_free_page(GFP_KERNEL | __GFP_ZERO); if (!info->page) goto error_nomem; @@ -640,6 +641,10 @@ static int xenfb_connect_backend(struct irq_to_evtchn_port(info->irq)); if (ret) goto error_xenbus; + ret = xenbus_printf(xbt, dev->nodename, "protocol", "%s", + XEN_IO_PROTO_ABI_NATIVE); + if (ret) + goto error_xenbus; ret = xenbus_printf(xbt, dev->nodename, "feature-update", "1"); if (ret) goto error_xenbus; [-- Attachment #4: blkfront-bimodal.diff --] [-- Type: text/x-patch, Size: 976 bytes --] --- linux-2.6-xen-sparse/drivers/xen/blkfront/blkfront.c | 7 +++++++ 1 file changed, 7 insertions(+) Index: build-32-unstable-13534/linux-2.6-xen-sparse/drivers/xen/blkfront/blkfront.c =================================================================== --- build-32-unstable-13534.orig/linux-2.6-xen-sparse/drivers/xen/blkfront/blkfront.c +++ build-32-unstable-13534/linux-2.6-xen-sparse/drivers/xen/blkfront/blkfront.c @@ -44,6 +44,7 @@ #include <xen/evtchn.h> #include <xen/xenbus.h> #include <xen/interface/grant_table.h> +#include <xen/interface/io/protocols.h> #include <xen/gnttab.h> #include <asm/hypervisor.h> #include <asm/maddr.h> @@ -180,6 +181,12 @@ again: message = "writing event-channel"; goto abort_transaction; } + err = xenbus_printf(xbt, dev->nodename, "protocol", "%s", + XEN_IO_PROTO_ABI_NATIVE); + if (err) { + message = "writing protocol"; + goto abort_transaction; + } err = xenbus_transaction_end(xbt, 0); if (err) { [-- Attachment #5: fbback-bimodal.diff --] [-- Type: text/x-patch, Size: 4460 bytes --] --- tools/xenfb/xenfb.c | 103 ++++++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 92 insertions(+), 11 deletions(-) Index: build-32-unstable-13495/tools/xenfb/xenfb.c =================================================================== --- build-32-unstable-13495.orig/tools/xenfb/xenfb.c +++ build-32-unstable-13495/tools/xenfb/xenfb.c @@ -7,6 +7,7 @@ #include <xen/io/xenbus.h> #include <xen/io/fbif.h> #include <xen/io/kbdif.h> +#include <xen/io/protocols.h> #include <sys/select.h> #include <stdbool.h> #include <xen/linux/evtchn.h> @@ -40,6 +41,7 @@ struct xenfb_private { struct xs_handle *xsh; /* xs daemon handle */ struct xenfb_device fb, kbd; size_t fb_len; /* size of framebuffer */ + char protocol[64]; /* frontend protocol */ }; static void xenfb_detach_dom(struct xenfb_private *); @@ -324,36 +326,112 @@ static int xenfb_wait_for_frontend_initi return 0; } +static void xenfb_copy_mfns(int mode, int count, unsigned long *dst, void *src) +{ + uint32_t *src32 = src; + uint64_t *src64 = src; + int i; + + if (32 == mode) { + for (i = 0; i < count; i++) + dst[i] = src32[i]; + } else { + for (i = 0; i < count; i++) + dst[i] = src64[i]; + } +} + static int xenfb_map_fb(struct xenfb_private *xenfb, int domid) { struct xenfb_page *page = xenfb->fb.page; int n_fbmfns; int n_fbdirs; - unsigned long *fbmfns; + unsigned long *pgmfns = NULL; + unsigned long *fbmfns = NULL; + void *map, *pd; + int mode, ret = -1; + + /* default to native */ + pd = page->pd; + mode = sizeof(unsigned long) * 8; + + if (0 == strlen(xenfb->protocol)) { + /* + * Undefined protocol, some guesswork needed. + * + * Old frontends which don't set the protocol use + * one page directory only, thus pd[1] must be zero. + * pd[1] of the 32bit struct layout and the lower + * 32 bits of pd[0] of the 64bit struct layout have + * the same location, so we can check that ... + */ + uint32_t *ptr32 = NULL; + uint32_t *ptr64 = NULL; +#if defined(__i386_) + ptr32 = page->pd; + ptr64 = ((void*)page->pd) + 4; +#elif defined(__x86_64__) + ptr32 = ((void*)page->pd) - 4; + ptr64 = page->pd; +#endif + if (ptr32) { + if (0 == ptr32[1]) { + mode = 32; + pd = ptr32; + } else { + mode = 64; + pd = ptr64; + } + } +#if defined(__x86_64__) + } else if (0 == strcmp(xenfb->protocol, XEN_IO_PROTO_ABI_X86_32)) { + /* 64bit dom0, 32bit domU */ + mode = 32; + pd = ((void*)page->pd) - 4; +#elif defined(__i386_) + } else if (0 == strcmp(xenfb->protocol, XEN_IO_PROTO_ABI_X86_64)) { + /* 32bit dom0, 64bit domU */ + mode = 64; + pd = ((void*)page->pd) + 4; +#endif + } n_fbmfns = (xenfb->fb_len + (XC_PAGE_SIZE - 1)) / XC_PAGE_SIZE; - n_fbdirs = n_fbmfns * sizeof(unsigned long); + n_fbdirs = n_fbmfns * mode / 8; n_fbdirs = (n_fbdirs + (XC_PAGE_SIZE - 1)) / XC_PAGE_SIZE; + pgmfns = malloc(sizeof(unsigned long) * n_fbdirs); + fbmfns = malloc(sizeof(unsigned long) * n_fbmfns); + if (!pgmfns || !fbmfns) + goto out; + /* * Bug alert: xc_map_foreign_batch() can fail partly and * return a non-null value. This is a design flaw. When it * happens, we happily continue here, and later crash on * access. */ - fbmfns = xc_map_foreign_batch(xenfb->xc, domid, - PROT_READ, page->pd, n_fbdirs); - if (fbmfns == NULL) - return -1; + xenfb_copy_mfns(mode, n_fbdirs, pgmfns, pd); + map = xc_map_foreign_batch(xenfb->xc, domid, + PROT_READ, pgmfns, n_fbdirs); + if (map == NULL) + goto out; + xenfb_copy_mfns(mode, n_fbmfns, fbmfns, map); + munmap(map, n_fbdirs * XC_PAGE_SIZE); xenfb->pub.pixels = xc_map_foreign_batch(xenfb->xc, domid, PROT_READ | PROT_WRITE, fbmfns, n_fbmfns); - if (xenfb->pub.pixels == NULL) { - munmap(fbmfns, n_fbdirs * XC_PAGE_SIZE); - return -1; - } + if (xenfb->pub.pixels == NULL) + goto out; - return munmap(fbmfns, n_fbdirs * XC_PAGE_SIZE); + ret = 0; /* all is fine */ + + out: + if (pgmfns) + free(pgmfns); + if (fbmfns) + free(fbmfns); + return ret; } static int xenfb_bind(struct xenfb_device *dev) @@ -368,6 +446,9 @@ static int xenfb_bind(struct xenfb_devic if (xenfb_xs_scanf1(xenfb->xsh, dev->otherend, "event-channel", "%u", &evtchn) < 0) return -1; + if (xenfb_xs_scanf1(xenfb->xsh, dev->otherend, "protocol", "%63s", + xenfb->protocol) < 0) + xenfb->protocol[0] = '\0'; dev->port = xc_evtchn_bind_interdomain(xenfb->evt_xch, dev->otherend_id, evtchn); [-- Attachment #6: blkback-bimodal.diff --] [-- Type: text/x-patch, Size: 23028 bytes --] multiprotocol blkback drivers. This is a patch for the block interface, frontend drivers, backend drivers and tools to support multiple ring protocols. Right there are now just two: the 32bit and the 64bit one. If needed it can be extended. Interface changes (io/blkif.h) * Have both request structs there, with "v1" and "v2" added to the name. The old name is aliased to the native protocol of the architecture. * Add helper functions to convert v1/v2 requests to native. Frontend changes: * Create a new node "protocol", add the protocol number it speaks there. Backend changes: * Look at the "protocol" number of the frontend and switch ring handling accordingly. If the protocol node isn't present it assumes native protocol. * As the request struct is copied anyway before being processed (for security reasons) it is converted to native at that point so most backend code doesn't need to know what the frontend speaks. * In case of blktap this is completely transparent to userspace, the kernel/userspace ring is always native no matter what the frontend speaks. Tools changes: * Add one more option to the disk configuration, so one can specify the protocol the frontend speaks in the config file. This is needed for old frontends which don't advertise the protocol they are speaking themself. I'm not that happy with this approach, but it works for now and I'm kida lost in the stack of python classes doing domain and device handling ... --- linux-2.6-xen-sparse/drivers/xen/blkback/blkback.c | 64 ++++++++---- linux-2.6-xen-sparse/drivers/xen/blkback/common.h | 6 - linux-2.6-xen-sparse/drivers/xen/blkback/interface.c | 25 +++- linux-2.6-xen-sparse/drivers/xen/blkback/xenbus.c | 20 +++ linux-2.6-xen-sparse/drivers/xen/blktap/blktap.c | 63 +++++++---- linux-2.6-xen-sparse/drivers/xen/blktap/common.h | 6 - linux-2.6-xen-sparse/drivers/xen/blktap/interface.c | 25 +++- linux-2.6-xen-sparse/drivers/xen/blktap/xenbus.c | 19 +++ linux-2.6-xen-sparse/include/xen/blkif.h | 100 +++++++++++++++++++ xen/include/public/io/blkif.h | 14 +- 10 files changed, 278 insertions(+), 64 deletions(-) Index: build-32-unstable-13534/linux-2.6-xen-sparse/drivers/xen/blkback/blkback.c =================================================================== --- build-32-unstable-13534.orig/linux-2.6-xen-sparse/drivers/xen/blkback/blkback.c +++ build-32-unstable-13534/linux-2.6-xen-sparse/drivers/xen/blkback/blkback.c @@ -298,17 +298,20 @@ irqreturn_t blkif_be_int(int irq, void * static int do_block_io_op(blkif_t *blkif) { - blkif_back_ring_t *blk_ring = &blkif->blk_ring; + blkif_back_rings_t *blk_rings = &blkif->blk_rings; blkif_request_t req; pending_req_t *pending_req; RING_IDX rc, rp; int more_to_do = 0; - rc = blk_ring->req_cons; - rp = blk_ring->sring->req_prod; + rc = blk_rings->co.req_cons; + rp = blk_rings->co.sring->req_prod; rmb(); /* Ensure we see queued requests up to 'rp'. */ - while ((rc != rp) && !RING_REQUEST_CONS_OVERFLOW(blk_ring, rc)) { + while ((rc != rp)) { + + if (RING_REQUEST_CONS_OVERFLOW(&blk_rings->co, rc)) + break; pending_req = alloc_req(); if (NULL == pending_req) { @@ -317,8 +320,17 @@ static int do_block_io_op(blkif_t *blkif break; } - memcpy(&req, RING_GET_REQUEST(blk_ring, rc), sizeof(req)); - blk_ring->req_cons = ++rc; /* before make_response() */ + switch (blkif->blk_protocol) { + case 1: + blkif_get_v1_req(&req, RING_GET_REQUEST(&blk_rings->v1, rc)); + break; + case 2: + blkif_get_v2_req(&req, RING_GET_REQUEST(&blk_rings->v2, rc)); + break; + default: + BUG(); + } + blk_rings->co.req_cons = ++rc; /* before make_response() */ switch (req.operation) { case BLKIF_OP_READ: @@ -498,34 +510,44 @@ static void dispatch_rw_block_io(blkif_t static void make_response(blkif_t *blkif, unsigned long id, unsigned short op, int st) { - blkif_response_t *resp; + blkif_response_t resp; unsigned long flags; - blkif_back_ring_t *blk_ring = &blkif->blk_ring; + blkif_back_rings_t *blk_rings = &blkif->blk_rings; int more_to_do = 0; int notify; - spin_lock_irqsave(&blkif->blk_ring_lock, flags); - - /* Place on the response ring for the relevant domain. */ - resp = RING_GET_RESPONSE(blk_ring, blk_ring->rsp_prod_pvt); - resp->id = id; - resp->operation = op; - resp->status = st; - blk_ring->rsp_prod_pvt++; - RING_PUSH_RESPONSES_AND_CHECK_NOTIFY(blk_ring, notify); + resp.id = id; + resp.operation = op; + resp.status = st; - if (blk_ring->rsp_prod_pvt == blk_ring->req_cons) { + spin_lock_irqsave(&blkif->blk_ring_lock, flags); + /* Place on the response ring for the relevant domain. */ + switch (blkif->blk_protocol) { + case 1: + memcpy(RING_GET_RESPONSE(&blk_rings->v1, blk_rings->v1.rsp_prod_pvt), + &resp, sizeof(resp)); + break; + case 2: + memcpy(RING_GET_RESPONSE(&blk_rings->v2, blk_rings->v2.rsp_prod_pvt), + &resp, sizeof(resp)); + break; + default: + BUG(); + } + blk_rings->co.rsp_prod_pvt++; + RING_PUSH_RESPONSES_AND_CHECK_NOTIFY(&blk_rings->co, notify); + if (blk_rings->co.rsp_prod_pvt == blk_rings->co.req_cons) { /* * Tail check for pending requests. Allows frontend to avoid * notifications if requests are already in flight (lower * overheads and promotes batching). */ - RING_FINAL_CHECK_FOR_REQUESTS(blk_ring, more_to_do); + RING_FINAL_CHECK_FOR_REQUESTS(&blk_rings->co, more_to_do); - } else if (RING_HAS_UNCONSUMED_REQUESTS(blk_ring)) { + } else if (RING_HAS_UNCONSUMED_REQUESTS(&blk_rings->co)) { more_to_do = 1; - } + spin_unlock_irqrestore(&blkif->blk_ring_lock, flags); if (more_to_do) Index: build-32-unstable-13534/linux-2.6-xen-sparse/drivers/xen/blkback/common.h =================================================================== --- build-32-unstable-13534.orig/linux-2.6-xen-sparse/drivers/xen/blkback/common.h +++ build-32-unstable-13534/linux-2.6-xen-sparse/drivers/xen/blkback/common.h @@ -40,8 +40,7 @@ #include <asm/pgalloc.h> #include <xen/evtchn.h> #include <asm/hypervisor.h> -#include <xen/interface/io/blkif.h> -#include <xen/interface/io/ring.h> +#include <xen/blkif.h> #include <xen/gnttab.h> #include <xen/driver_util.h> #include <xen/xenbus.h> @@ -67,7 +66,8 @@ typedef struct blkif_st { /* Physical parameters of the comms window. */ unsigned int irq; /* Comms information. */ - blkif_back_ring_t blk_ring; + int blk_protocol; + blkif_back_rings_t blk_rings; struct vm_struct *blk_ring_area; /* The VBD attached to this interface. */ struct vbd vbd; Index: build-32-unstable-13534/linux-2.6-xen-sparse/drivers/xen/blkback/interface.c =================================================================== --- build-32-unstable-13534.orig/linux-2.6-xen-sparse/drivers/xen/blkback/interface.c +++ build-32-unstable-13534/linux-2.6-xen-sparse/drivers/xen/blkback/interface.c @@ -95,7 +95,6 @@ static void unmap_frontend_page(blkif_t int blkif_map(blkif_t *blkif, unsigned long shared_page, unsigned int evtchn) { - blkif_sring_t *sring; int err; /* Already connected through? */ @@ -111,8 +110,24 @@ int blkif_map(blkif_t *blkif, unsigned l return err; } - sring = (blkif_sring_t *)blkif->blk_ring_area->addr; - BACK_RING_INIT(&blkif->blk_ring, sring, PAGE_SIZE); + switch (blkif->blk_protocol) { + case 1: + { + blkif_v1_sring_t *sring_v1; + sring_v1 = (blkif_v1_sring_t *)blkif->blk_ring_area->addr; + BACK_RING_INIT(&blkif->blk_rings.v1, sring_v1, PAGE_SIZE); + break; + } + case 2: + { + blkif_v2_sring_t *sring_v2; + sring_v2 = (blkif_v2_sring_t *)blkif->blk_ring_area->addr; + BACK_RING_INIT(&blkif->blk_rings.v2, sring_v2, PAGE_SIZE); + break; + } + default: + BUG(); + } err = bind_interdomain_evtchn_to_irqhandler( blkif->domid, evtchn, blkif_be_int, 0, "blkif-backend", blkif); @@ -143,10 +158,10 @@ void blkif_disconnect(blkif_t *blkif) blkif->irq = 0; } - if (blkif->blk_ring.sring) { + if (blkif->blk_rings.co.sring) { unmap_frontend_page(blkif); free_vm_area(blkif->blk_ring_area); - blkif->blk_ring.sring = NULL; + blkif->blk_rings.co.sring = NULL; } } Index: build-32-unstable-13534/linux-2.6-xen-sparse/drivers/xen/blkback/xenbus.c =================================================================== --- build-32-unstable-13534.orig/linux-2.6-xen-sparse/drivers/xen/blkback/xenbus.c +++ build-32-unstable-13534/linux-2.6-xen-sparse/drivers/xen/blkback/xenbus.c @@ -459,6 +459,7 @@ static int connect_ring(struct backend_i struct xenbus_device *dev = be->dev; unsigned long ring_ref; unsigned int evtchn; + char protocol[64] = ""; int err; DPRINTK("%s", dev->otherend); @@ -472,6 +473,25 @@ static int connect_ring(struct backend_i return err; } + be->blkif->blk_protocol = BLKIF_NATIVE_PROTOCOL; + err = xenbus_gather(XBT_NIL, dev->otherend, "protocol", + "%63s", protocol, NULL); + if (err) + strcpy(protocol, "unspecified, assuming native"); + else if (0 == strcmp(protocol, XEN_IO_PROTO_ABI_NATIVE)) + be->blkif->blk_protocol = BLKIF_NATIVE_PROTOCOL; + else if (0 == strcmp(protocol, XEN_IO_PROTO_ABI_X86_32)) + be->blkif->blk_protocol = 1; + else if (0 == strcmp(protocol, XEN_IO_PROTO_ABI_X86_64)) + be->blkif->blk_protocol = 2; + else { + xenbus_dev_fatal(dev, err, "unknown fe protocol %s", protocol); + return -1; + } + + printk("blkback: ring-ref %ld, event-channel %d, protocol %d (%s)\n", + ring_ref, evtchn, be->blkif->blk_protocol, protocol); + /* Map the shared frame, irq etc. */ err = blkif_map(be->blkif, ring_ref, evtchn); if (err) { Index: build-32-unstable-13534/xen/include/public/io/blkif.h =================================================================== --- build-32-unstable-13534.orig/xen/include/public/io/blkif.h +++ build-32-unstable-13534/xen/include/public/io/blkif.h @@ -71,18 +71,20 @@ */ #define BLKIF_MAX_SEGMENTS_PER_REQUEST 11 +struct blkif_request_segment { + grant_ref_t gref; /* reference to I/O buffer frame */ + /* @first_sect: first sector in frame to transfer (inclusive). */ + /* @last_sect: last sector in frame to transfer (inclusive). */ + uint8_t first_sect, last_sect; +}; + struct blkif_request { uint8_t operation; /* BLKIF_OP_??? */ uint8_t nr_segments; /* number of segments */ blkif_vdev_t handle; /* only for read/write requests */ uint64_t id; /* private guest value, echoed in resp */ blkif_sector_t sector_number;/* start sector idx on disk (r/w only) */ - struct blkif_request_segment { - grant_ref_t gref; /* reference to I/O buffer frame */ - /* @first_sect: first sector in frame to transfer (inclusive). */ - /* @last_sect: last sector in frame to transfer (inclusive). */ - uint8_t first_sect, last_sect; - } seg[BLKIF_MAX_SEGMENTS_PER_REQUEST]; + struct blkif_request_segment seg[BLKIF_MAX_SEGMENTS_PER_REQUEST]; }; typedef struct blkif_request blkif_request_t; Index: build-32-unstable-13534/linux-2.6-xen-sparse/drivers/xen/blktap/blktap.c =================================================================== --- build-32-unstable-13534.orig/linux-2.6-xen-sparse/drivers/xen/blktap/blktap.c +++ build-32-unstable-13534/linux-2.6-xen-sparse/drivers/xen/blktap/blktap.c @@ -1091,15 +1091,15 @@ irqreturn_t tap_blkif_be_int(int irq, vo static int print_dbug = 1; static int do_block_io_op(blkif_t *blkif) { - blkif_back_ring_t *blk_ring = &blkif->blk_ring; + blkif_back_rings_t *blk_rings = &blkif->blk_rings; blkif_request_t req; pending_req_t *pending_req; RING_IDX rc, rp; int more_to_do = 0; tap_blkif_t *info; - rc = blk_ring->req_cons; - rp = blk_ring->sring->req_prod; + rc = blk_rings->co.req_cons; + rp = blk_rings->co.sring->req_prod; rmb(); /* Ensure we see queued requests up to 'rp'. */ /*Check blkif has corresponding UE ring*/ @@ -1130,8 +1130,8 @@ static int do_block_io_op(blkif_t *blkif more_to_do = 1; break; } - - if (RING_REQUEST_CONS_OVERFLOW(blk_ring, rc)) { + + if (RING_REQUEST_CONS_OVERFLOW(&blk_rings->co, rc)) { WPRINTK("RING_REQUEST_CONS_OVERFLOW!" " More to do\n"); more_to_do = 1; @@ -1145,8 +1145,17 @@ static int do_block_io_op(blkif_t *blkif break; } - memcpy(&req, RING_GET_REQUEST(blk_ring, rc), sizeof(req)); - blk_ring->req_cons = ++rc; /* before make_response() */ + switch (blkif->blk_protocol) { + case 1: + blkif_get_v1_req(&req, RING_GET_REQUEST(&blk_rings->v1, rc)); + break; + case 2: + blkif_get_v2_req(&req, RING_GET_REQUEST(&blk_rings->v2, rc)); + break; + default: + BUG(); + } + blk_rings->co.req_cons = ++rc; /* before make_response() */ switch (req.operation) { case BLKIF_OP_READ: @@ -1222,7 +1231,7 @@ static void dispatch_rw_block_io(blkif_t WPRINTK("blktap: fe_ring is full, can't add " "IO Request will be dropped. %d %d\n", RING_SIZE(&info->ufe_ring), - RING_SIZE(&blkif->blk_ring)); + RING_SIZE(&blkif->blk_rings.co)); goto fail_response; } @@ -1410,32 +1419,44 @@ static void dispatch_rw_block_io(blkif_t static void make_response(blkif_t *blkif, unsigned long id, unsigned short op, int st) { - blkif_response_t *resp; + blkif_response_t resp; unsigned long flags; - blkif_back_ring_t *blk_ring = &blkif->blk_ring; + blkif_back_rings_t *blk_rings = &blkif->blk_rings; int more_to_do = 0; int notify; + resp.id = id; + resp.operation = op; + resp.status = st; + spin_lock_irqsave(&blkif->blk_ring_lock, flags); - /* Place on the response ring for the relevant domain. */ - resp = RING_GET_RESPONSE(blk_ring, blk_ring->rsp_prod_pvt); - resp->id = id; - resp->operation = op; - resp->status = st; - blk_ring->rsp_prod_pvt++; - RING_PUSH_RESPONSES_AND_CHECK_NOTIFY(blk_ring, notify); + /* Place on the response ring for the relevant domain. */ + switch (blkif->blk_protocol) { + case 1: + memcpy(RING_GET_RESPONSE(&blk_rings->v1, blk_rings->v1.rsp_prod_pvt), + &resp, sizeof(resp)); + break; + case 2: + memcpy(RING_GET_RESPONSE(&blk_rings->v2, blk_rings->v2.rsp_prod_pvt), + &resp, sizeof(resp)); + break; + default: + BUG(); + } + blk_rings->co.rsp_prod_pvt++; + RING_PUSH_RESPONSES_AND_CHECK_NOTIFY(&blk_rings->co, notify); - if (blk_ring->rsp_prod_pvt == blk_ring->req_cons) { + if (blk_rings->co.rsp_prod_pvt == blk_rings->co.req_cons) { /* * Tail check for pending requests. Allows frontend to avoid * notifications if requests are already in flight (lower * overheads and promotes batching). */ - RING_FINAL_CHECK_FOR_REQUESTS(blk_ring, more_to_do); - } else if (RING_HAS_UNCONSUMED_REQUESTS(blk_ring)) { + RING_FINAL_CHECK_FOR_REQUESTS(&blk_rings->co, more_to_do); + } else if (RING_HAS_UNCONSUMED_REQUESTS(&blk_rings->co)) { more_to_do = 1; + } - } spin_unlock_irqrestore(&blkif->blk_ring_lock, flags); if (more_to_do) blkif_notify_work(blkif); Index: build-32-unstable-13534/linux-2.6-xen-sparse/drivers/xen/blktap/common.h =================================================================== --- build-32-unstable-13534.orig/linux-2.6-xen-sparse/drivers/xen/blktap/common.h +++ build-32-unstable-13534/linux-2.6-xen-sparse/drivers/xen/blktap/common.h @@ -39,8 +39,7 @@ #include <asm/pgalloc.h> #include <xen/evtchn.h> #include <asm/hypervisor.h> -#include <xen/interface/io/blkif.h> -#include <xen/interface/io/ring.h> +#include <xen/blkif.h> #include <xen/gnttab.h> #include <xen/driver_util.h> @@ -58,7 +57,8 @@ typedef struct blkif_st { /* Physical parameters of the comms window. */ unsigned int irq; /* Comms information. */ - blkif_back_ring_t blk_ring; + int blk_protocol; + blkif_back_rings_t blk_rings; struct vm_struct *blk_ring_area; /* Back pointer to the backend_info. */ struct backend_info *be; Index: build-32-unstable-13534/linux-2.6-xen-sparse/drivers/xen/blktap/interface.c =================================================================== --- build-32-unstable-13534.orig/linux-2.6-xen-sparse/drivers/xen/blktap/interface.c +++ build-32-unstable-13534/linux-2.6-xen-sparse/drivers/xen/blktap/interface.c @@ -96,7 +96,6 @@ static void unmap_frontend_page(blkif_t int tap_blkif_map(blkif_t *blkif, unsigned long shared_page, unsigned int evtchn) { - blkif_sring_t *sring; int err; /* Already connected through? */ @@ -112,8 +111,24 @@ int tap_blkif_map(blkif_t *blkif, unsign return err; } - sring = (blkif_sring_t *)blkif->blk_ring_area->addr; - BACK_RING_INIT(&blkif->blk_ring, sring, PAGE_SIZE); + switch (blkif->blk_protocol) { + case 1: + { + blkif_v1_sring_t *sring_v1; + sring_v1 = (blkif_v1_sring_t *)blkif->blk_ring_area->addr; + BACK_RING_INIT(&blkif->blk_rings.v1, sring_v1, PAGE_SIZE); + break; + } + case 2: + { + blkif_v2_sring_t *sring_v2; + sring_v2 = (blkif_v2_sring_t *)blkif->blk_ring_area->addr; + BACK_RING_INIT(&blkif->blk_rings.v2, sring_v2, PAGE_SIZE); + break; + } + default: + BUG(); + } err = bind_interdomain_evtchn_to_irqhandler( blkif->domid, evtchn, tap_blkif_be_int, @@ -134,10 +149,10 @@ void tap_blkif_unmap(blkif_t *blkif) unbind_from_irqhandler(blkif->irq, blkif); blkif->irq = 0; } - if (blkif->blk_ring.sring) { + if (blkif->blk_rings.co.sring) { unmap_frontend_page(blkif); free_vm_area(blkif->blk_ring_area); - blkif->blk_ring.sring = NULL; + blkif->blk_rings.co.sring = NULL; } } Index: build-32-unstable-13534/linux-2.6-xen-sparse/drivers/xen/blktap/xenbus.c =================================================================== --- build-32-unstable-13534.orig/linux-2.6-xen-sparse/drivers/xen/blktap/xenbus.c +++ build-32-unstable-13534/linux-2.6-xen-sparse/drivers/xen/blktap/xenbus.c @@ -340,6 +340,7 @@ static int connect_ring(struct backend_i struct xenbus_device *dev = be->dev; unsigned long ring_ref; unsigned int evtchn; + char protocol[64]; int err; DPRINTK("%s\n", dev->otherend); @@ -353,6 +354,24 @@ static int connect_ring(struct backend_i return err; } + be->blkif->blk_protocol = BLKIF_NATIVE_PROTOCOL; + err = xenbus_gather(XBT_NIL, dev->otherend, "protocol", + "%63s", protocol, NULL); + if (err) + strcpy(protocol, "unspecified, assuming native"); + else if (0 == strcmp(protocol, XEN_IO_PROTO_ABI_NATIVE)) + be->blkif->blk_protocol = BLKIF_NATIVE_PROTOCOL; + else if (0 == strcmp(protocol, XEN_IO_PROTO_ABI_X86_32)) + be->blkif->blk_protocol = 1; + else if (0 == strcmp(protocol, XEN_IO_PROTO_ABI_X86_64)) + be->blkif->blk_protocol = 2; + else { + xenbus_dev_fatal(dev, err, "unknown fe protocol %s", protocol); + return -1; + } + printk("blktap: ring-ref %ld, event-channel %d, protocol %d (%s)\n", + ring_ref, evtchn, be->blkif->blk_protocol, protocol); + /* Map the shared frame, irq etc. */ err = tap_blkif_map(be->blkif, ring_ref, evtchn); if (err) { Index: build-32-unstable-13534/linux-2.6-xen-sparse/include/xen/blkif.h =================================================================== --- /dev/null +++ build-32-unstable-13534/linux-2.6-xen-sparse/include/xen/blkif.h @@ -0,0 +1,100 @@ +#ifndef __XEN_BLKIF_H__ +#define __XEN_BLKIF_H__ + +#include <xen/interface/io/ring.h> +#include <xen/interface/io/blkif.h> +#include <xen/interface/io/protocols.h> + +/* Not a real protocol. Used to generate ring structs which contain + * the elements common to all protocols only. This way we get a + * compiler-checkable way to use common struct elements, so we can + * avoid using switch(protocol) in a number of places. */ +struct blkif_co_request { + char dummy; +}; +struct blkif_co_response { + char dummy; +}; + +/* i386 protocol version */ +#pragma pack(push, 4) +struct blkif_v1_request { + uint8_t operation; /* BLKIF_OP_??? */ + uint8_t nr_segments; /* number of segments */ + blkif_vdev_t handle; /* only for read/write requests */ + uint64_t id; /* private guest value, echoed in resp */ + blkif_sector_t sector_number;/* start sector idx on disk (r/w only) */ + struct blkif_request_segment seg[BLKIF_MAX_SEGMENTS_PER_REQUEST]; +}; +struct blkif_v1_response { + uint64_t id; /* copied from request */ + uint8_t operation; /* copied from request */ + int16_t status; /* BLKIF_RSP_??? */ +}; +typedef struct blkif_v1_request blkif_v1_request_t; +typedef struct blkif_v1_response blkif_v1_response_t; +#pragma pack(pop) + +/* x86_64 protocol version */ +struct blkif_v2_request { + uint8_t operation; /* BLKIF_OP_??? */ + uint8_t nr_segments; /* number of segments */ + blkif_vdev_t handle; /* only for read/write requests */ + uint64_t __attribute__((__aligned__(8))) id; + blkif_sector_t sector_number;/* start sector idx on disk (r/w only) */ + struct blkif_request_segment seg[BLKIF_MAX_SEGMENTS_PER_REQUEST]; +}; +struct blkif_v2_response { + uint64_t __attribute__((__aligned__(8))) id; + uint8_t operation; /* copied from request */ + int16_t status; /* BLKIF_RSP_??? */ +}; +typedef struct blkif_v2_request blkif_v2_request_t; +typedef struct blkif_v2_response blkif_v2_response_t; + +DEFINE_RING_TYPES(blkif_co, struct blkif_co_request, struct blkif_co_response); +DEFINE_RING_TYPES(blkif_v1, struct blkif_v1_request, struct blkif_v1_response); +DEFINE_RING_TYPES(blkif_v2, struct blkif_v2_request, struct blkif_v2_response); + +union blkif_back_rings { + blkif_co_back_ring_t co; + blkif_v1_back_ring_t v1; + blkif_v2_back_ring_t v2; +}; +typedef union blkif_back_rings blkif_back_rings_t; + +#if defined(__i386__) +# define BLKIF_NATIVE_PROTOCOL 1 +#elif defined(__x86_64__) || defined(__ia64__) +# define BLKIF_NATIVE_PROTOCOL 2 +#else +# error arch fixup needed here +#endif + +/* translate requests: v1/v2 to native */ +#if 1 == BLKIF_NATIVE_PROTOCOL +static void inline blkif_get_v1_req(blkif_request_t *dst, blkif_v1_request_t *src) +#else +static void inline blkif_get_v2_req(blkif_request_t *dst, blkif_v2_request_t *src) +#endif +{ + memcpy(dst, src, sizeof(*dst)); +} + +#if 1 == BLKIF_NATIVE_PROTOCOL +static void inline blkif_get_v2_req(blkif_request_t *dst, blkif_v2_request_t *src) +#else +static void inline blkif_get_v1_req(blkif_request_t *dst, blkif_v1_request_t *src) +#endif +{ + int i; + dst->operation = src->operation; + dst->nr_segments = src->nr_segments; + dst->handle = src->handle; + dst->id = src->id; + dst->sector_number = src->sector_number; + for (i = 0; i < src->nr_segments; i++) + dst->seg[i] = src->seg[i]; +} + +#endif /* __XEN_BLKIF_H__ */ [-- Attachment #7: blktools-bimodal.diff --] [-- Type: text/x-patch, Size: 2264 bytes --] --- tools/python/xen/xend/server/blkif.py | 3 +++ tools/python/xen/xm/create.py | 7 ++++++- 2 files changed, 9 insertions(+), 1 deletion(-) Index: build-32-unstable-13534/tools/python/xen/xend/server/blkif.py =================================================================== --- build-32-unstable-13534.orig/tools/python/xen/xend/server/blkif.py +++ build-32-unstable-13534/tools/python/xen/xend/server/blkif.py @@ -38,6 +38,7 @@ class BlkifController(DevController): """@see DevController.getDeviceDetails""" uname = config.get('uname', '') dev = config.get('dev', '') + protocol = config.get('protocol') if 'ioemu:' in dev: (_, dev) = string.split(dev, ':', 1) @@ -85,6 +86,8 @@ class BlkifController(DevController): front = { 'virtual-device' : "%i" % devid, 'device-type' : dev_type } + if protocol: + front.update({ 'protocol' : protocol }); return (devid, back, front) Index: build-32-unstable-13534/tools/python/xen/xm/create.py =================================================================== --- build-32-unstable-13534.orig/tools/python/xen/xm/create.py +++ build-32-unstable-13534/tools/python/xen/xm/create.py @@ -537,7 +537,7 @@ def configure_image(vals): def configure_disks(config_devs, vals): """Create the config for disks (virtual block devices). """ - for (uname, dev, mode, backend) in vals.disk: + for (uname, dev, mode, backend, protocol) in vals.disk: if uname.startswith('tap:'): cls = 'tap' else: @@ -549,6 +549,8 @@ def configure_disks(config_devs, vals): ['mode', mode ] ] if backend: config_vbd.append(['backend', backend]) + if protocol: + config_vbd.append(['protocol', protocol]) config_devs.append(['device', config_vbd]) def configure_pci(config_devs, vals): @@ -803,7 +805,10 @@ def preprocess_disk(vals): n = len(d) if n == 3: d.append(None) + d.append(None) elif n == 4: + d.append(None) + elif n == 5: pass else: err('Invalid disk specifier: ' + v) [-- Attachment #8: Type: text/plain, Size: 138 bytes --] _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: 32-on-64: pvfb issue 2007-01-22 14:01 ` Gerd Hoffmann @ 2007-01-22 14:48 ` Keir Fraser 2007-01-23 12:53 ` Gerd Hoffmann 2007-01-22 15:22 ` 32-on-64: pvfb issue Markus Armbruster 1 sibling, 1 reply; 50+ messages in thread From: Keir Fraser @ 2007-01-22 14:48 UTC (permalink / raw) To: Gerd Hoffmann; +Cc: Xen devel list, Keir Fraser, Markus Armbruster On 22/1/07 14:01, "Gerd Hoffmann" <kraxel@suse.de> wrote: > Here we go. Compile-tested on 32bit, more tests coming, full rebuild > still in progress ... Yeah, I like these. They can go in as soon as you're happy with them. One exception is blkback -- I'm not keen on the v1/v2 thing as it is. I think we should continue to include public/io/blkif.h and use the struct definition there when protocol==XEN_IO_PROTO_ABI_NATIVE. The exceptions can be XEN_IO_PROTO_ABI_X86_32 and XEN_IO_PROTO_ABI_X86_64 which can use the v1/v2 definitions (but I suggest renamed to include x86_32/x86_64 in the names). We can generalise the names if they turn out to be useful in future for other architectures (or provide typedef/#define equivalents). So for now we end up with three useful enumerations for blk protocols: native, x86_32, x86_64. -- Keir ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: 32-on-64: pvfb issue 2007-01-22 14:48 ` Keir Fraser @ 2007-01-23 12:53 ` Gerd Hoffmann 2007-01-23 15:07 ` Keir Fraser ` (2 more replies) 0 siblings, 3 replies; 50+ messages in thread From: Gerd Hoffmann @ 2007-01-23 12:53 UTC (permalink / raw) To: Keir Fraser; +Cc: Xen devel list, Markus Armbruster [-- Attachment #1: Type: text/plain, Size: 829 bytes --] Keir Fraser wrote: > On 22/1/07 14:01, "Gerd Hoffmann" <kraxel@suse.de> wrote: > >> Here we go. Compile-tested on 32bit, more tests coming, full rebuild >> still in progress ... > > Yeah, I like these. They can go in as soon as you're happy with them. One > exception is blkback -- I'm not keen on the v1/v2 thing as it is. I think we > should continue to include public/io/blkif.h and use the struct definition > there when protocol==XEN_IO_PROTO_ABI_NATIVE. Fixed. New versions attached, with comments added/updated and signed-off-by. They are partly tested only though, 32-on-64 seems to be broken in current unstable, mixing 32bit and 64bit domains doesn't work. Time to get the domain builder bits merged too, so this can be added to the regression tests ... please apply, Gerd -- Gerd Hoffmann <kraxel@suse.de> [-- Attachment #2: protocol-bimodal.diff --] [-- Type: text/x-patch, Size: 1132 bytes --] bimodal: header file with protocol names. This patch adds a header file with the protocol names. Signed-off-by: Gerd Hoffmann <kraxel@suse.de> --- xen/include/public/io/protocols.h | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) Index: build-32-unstable-13495/xen/include/public/io/protocols.h =================================================================== --- /dev/null +++ build-32-unstable-13495/xen/include/public/io/protocols.h @@ -0,0 +1,21 @@ +#ifndef __XEN_PROTOCOLS_H__ +#define __XEN_PROTOCOLS_H__ + +#define XEN_IO_PROTO_ABI_X86_32 "x86_32-abi" +#define XEN_IO_PROTO_ABI_X86_64 "x86_64-abi" +#define XEN_IO_PROTO_ABI_IA64 "ia64-abi" +#define XEN_IO_PROTO_ABI_POWERPC64 "powerpc64-abi" + +#if defined(__i386__) +# define XEN_IO_PROTO_ABI_NATIVE XEN_IO_PROTO_ABI_X86_32 +#elif defined(__x86_64__) +# define XEN_IO_PROTO_ABI_NATIVE XEN_IO_PROTO_ABI_X86_64 +#elif defined(__ia64__) +# define XEN_IO_PROTO_ABI_NATIVE XEN_IO_PROTO_ABI_IA64 +#elif defined(__powerpc64__) +# define XEN_IO_PROTO_ABI_NATIVE XEN_IO_PROTO_ABI_POWERPC64 +#else +# error arch fixup needed here +#endif + +#endif [-- Attachment #3: blkfront-bimodal.diff --] [-- Type: text/x-patch, Size: 1123 bytes --] bimodal: blkfront Create a new node "protocol" in xenstore, add the protocol name it speaks there. Signed-off-by: Gerd Hoffmann <kraxel@suse.de> --- linux-2.6-xen-sparse/drivers/xen/blkfront/blkfront.c | 7 +++++++ 1 file changed, 7 insertions(+) Index: build-32-unstable-13534/linux-2.6-xen-sparse/drivers/xen/blkfront/blkfront.c =================================================================== --- build-32-unstable-13534.orig/linux-2.6-xen-sparse/drivers/xen/blkfront/blkfront.c +++ build-32-unstable-13534/linux-2.6-xen-sparse/drivers/xen/blkfront/blkfront.c @@ -44,6 +44,7 @@ #include <xen/evtchn.h> #include <xen/xenbus.h> #include <xen/interface/grant_table.h> +#include <xen/interface/io/protocols.h> #include <xen/gnttab.h> #include <asm/hypervisor.h> #include <asm/maddr.h> @@ -180,6 +181,12 @@ again: message = "writing event-channel"; goto abort_transaction; } + err = xenbus_printf(xbt, dev->nodename, "protocol", "%s", + XEN_IO_PROTO_ABI_NATIVE); + if (err) { + message = "writing protocol"; + goto abort_transaction; + } err = xenbus_transaction_end(xbt, 0); if (err) { [-- Attachment #4: fbfront-bimodal.diff --] [-- Type: text/x-patch, Size: 1412 bytes --] bimodal: pvfb frontend Create a new node "protocol" in xenstore, add the protocol name it speaks there. Signed-off-by: Gerd Hoffmann <kraxel@suse.de> --- linux-2.6-xen-sparse/drivers/xen/fbfront/xenfb.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) Index: build-32-unstable-13495/linux-2.6-xen-sparse/drivers/xen/fbfront/xenfb.c =================================================================== --- build-32-unstable-13495.orig/linux-2.6-xen-sparse/drivers/xen/fbfront/xenfb.c +++ build-32-unstable-13495/linux-2.6-xen-sparse/drivers/xen/fbfront/xenfb.c @@ -27,6 +27,7 @@ #include <asm/hypervisor.h> #include <xen/evtchn.h> #include <xen/interface/io/fbif.h> +#include <xen/interface/io/protocols.h> #include <xen/xenbus.h> #include <linux/kthread.h> @@ -479,7 +480,7 @@ static int __devinit xenfb_probe(struct goto error_nomem; /* set up shared page */ - info->page = (void *)__get_free_page(GFP_KERNEL); + info->page = (void *)__get_free_page(GFP_KERNEL | __GFP_ZERO); if (!info->page) goto error_nomem; @@ -640,6 +641,10 @@ static int xenfb_connect_backend(struct irq_to_evtchn_port(info->irq)); if (ret) goto error_xenbus; + ret = xenbus_printf(xbt, dev->nodename, "protocol", "%s", + XEN_IO_PROTO_ABI_NATIVE); + if (ret) + goto error_xenbus; ret = xenbus_printf(xbt, dev->nodename, "feature-update", "1"); if (ret) goto error_xenbus; [-- Attachment #5: blkback-bimodal.diff --] [-- Type: text/x-patch, Size: 23522 bytes --] multiprotocol blkback drivers. This is a patch for the block interface, frontend drivers, backend drivers and tools to support multiple ring protocols. Right there are now just two: the 32bit and the 64bit one. If needed it can be extended. Interface changes (io/blkif.h) * Have both request structs there, with "v1" and "v2" added to the name. The old name is aliased to the native protocol of the architecture. * Add helper functions to convert v1/v2 requests to native. Backend changes: * Look at the "protocol" name of the frontend and switch ring handling accordingly. If the protocol node isn't present it assumes native protocol. * As the request struct is copied anyway before being processed (for security reasons) it is converted to native at that point so most backend code doesn't need to know what the frontend speaks. * In case of blktap this is completely transparent to userspace, the kernel/userspace ring is always native no matter what the frontend speaks. --- linux-2.6-xen-sparse/drivers/xen/blkback/blkback.c | 71 ++++++++++---- linux-2.6-xen-sparse/drivers/xen/blkback/common.h | 6 - linux-2.6-xen-sparse/drivers/xen/blkback/interface.c | 25 ++++- linux-2.6-xen-sparse/drivers/xen/blkback/xenbus.c | 19 +++ linux-2.6-xen-sparse/drivers/xen/blktap/blktap.c | 74 ++++++++++---- linux-2.6-xen-sparse/drivers/xen/blktap/common.h | 6 - linux-2.6-xen-sparse/drivers/xen/blktap/interface.c | 25 ++++- linux-2.6-xen-sparse/drivers/xen/blktap/xenbus.c | 19 +++ linux-2.6-xen-sparse/include/xen/blkif.h | 95 +++++++++++++++++++ xen/include/public/io/blkif.h | 14 +- 10 files changed, 290 insertions(+), 64 deletions(-) Index: build-32-unstable-13534/linux-2.6-xen-sparse/drivers/xen/blkback/blkback.c =================================================================== --- build-32-unstable-13534.orig/linux-2.6-xen-sparse/drivers/xen/blkback/blkback.c +++ build-32-unstable-13534/linux-2.6-xen-sparse/drivers/xen/blkback/blkback.c @@ -298,17 +298,20 @@ irqreturn_t blkif_be_int(int irq, void * static int do_block_io_op(blkif_t *blkif) { - blkif_back_ring_t *blk_ring = &blkif->blk_ring; + blkif_back_rings_t *blk_rings = &blkif->blk_rings; blkif_request_t req; pending_req_t *pending_req; RING_IDX rc, rp; int more_to_do = 0; - rc = blk_ring->req_cons; - rp = blk_ring->sring->req_prod; + rc = blk_rings->common.req_cons; + rp = blk_rings->common.sring->req_prod; rmb(); /* Ensure we see queued requests up to 'rp'. */ - while ((rc != rp) && !RING_REQUEST_CONS_OVERFLOW(blk_ring, rc)) { + while ((rc != rp)) { + + if (RING_REQUEST_CONS_OVERFLOW(&blk_rings->common, rc)) + break; pending_req = alloc_req(); if (NULL == pending_req) { @@ -317,8 +320,20 @@ static int do_block_io_op(blkif_t *blkif break; } - memcpy(&req, RING_GET_REQUEST(blk_ring, rc), sizeof(req)); - blk_ring->req_cons = ++rc; /* before make_response() */ + switch (blkif->blk_protocol) { + case BLKIF_PROTOCOL_NATIVE: + memcpy(&req, RING_GET_REQUEST(&blk_rings->native, rc), sizeof(req)); + break; + case BLKIF_PROTOCOL_X86_32: + blkif_get_x86_32_req(&req, RING_GET_REQUEST(&blk_rings->x86_32, rc)); + break; + case BLKIF_PROTOCOL_X86_64: + blkif_get_x86_64_req(&req, RING_GET_REQUEST(&blk_rings->x86_64, rc)); + break; + default: + BUG(); + } + blk_rings->common.req_cons = ++rc; /* before make_response() */ switch (req.operation) { case BLKIF_OP_READ: @@ -498,34 +513,48 @@ static void dispatch_rw_block_io(blkif_t static void make_response(blkif_t *blkif, unsigned long id, unsigned short op, int st) { - blkif_response_t *resp; + blkif_response_t resp; unsigned long flags; - blkif_back_ring_t *blk_ring = &blkif->blk_ring; + blkif_back_rings_t *blk_rings = &blkif->blk_rings; int more_to_do = 0; int notify; - spin_lock_irqsave(&blkif->blk_ring_lock, flags); - - /* Place on the response ring for the relevant domain. */ - resp = RING_GET_RESPONSE(blk_ring, blk_ring->rsp_prod_pvt); - resp->id = id; - resp->operation = op; - resp->status = st; - blk_ring->rsp_prod_pvt++; - RING_PUSH_RESPONSES_AND_CHECK_NOTIFY(blk_ring, notify); + resp.id = id; + resp.operation = op; + resp.status = st; - if (blk_ring->rsp_prod_pvt == blk_ring->req_cons) { + spin_lock_irqsave(&blkif->blk_ring_lock, flags); + /* Place on the response ring for the relevant domain. */ + switch (blkif->blk_protocol) { + case BLKIF_PROTOCOL_NATIVE: + memcpy(RING_GET_RESPONSE(&blk_rings->native, blk_rings->native.rsp_prod_pvt), + &resp, sizeof(resp)); + break; + case BLKIF_PROTOCOL_X86_32: + memcpy(RING_GET_RESPONSE(&blk_rings->x86_32, blk_rings->x86_32.rsp_prod_pvt), + &resp, sizeof(resp)); + break; + case BLKIF_PROTOCOL_X86_64: + memcpy(RING_GET_RESPONSE(&blk_rings->x86_64, blk_rings->x86_64.rsp_prod_pvt), + &resp, sizeof(resp)); + break; + default: + BUG(); + } + blk_rings->common.rsp_prod_pvt++; + RING_PUSH_RESPONSES_AND_CHECK_NOTIFY(&blk_rings->common, notify); + if (blk_rings->common.rsp_prod_pvt == blk_rings->common.req_cons) { /* * Tail check for pending requests. Allows frontend to avoid * notifications if requests are already in flight (lower * overheads and promotes batching). */ - RING_FINAL_CHECK_FOR_REQUESTS(blk_ring, more_to_do); + RING_FINAL_CHECK_FOR_REQUESTS(&blk_rings->common, more_to_do); - } else if (RING_HAS_UNCONSUMED_REQUESTS(blk_ring)) { + } else if (RING_HAS_UNCONSUMED_REQUESTS(&blk_rings->common)) { more_to_do = 1; - } + spin_unlock_irqrestore(&blkif->blk_ring_lock, flags); if (more_to_do) Index: build-32-unstable-13534/linux-2.6-xen-sparse/drivers/xen/blkback/common.h =================================================================== --- build-32-unstable-13534.orig/linux-2.6-xen-sparse/drivers/xen/blkback/common.h +++ build-32-unstable-13534/linux-2.6-xen-sparse/drivers/xen/blkback/common.h @@ -40,8 +40,7 @@ #include <asm/pgalloc.h> #include <xen/evtchn.h> #include <asm/hypervisor.h> -#include <xen/interface/io/blkif.h> -#include <xen/interface/io/ring.h> +#include <xen/blkif.h> #include <xen/gnttab.h> #include <xen/driver_util.h> #include <xen/xenbus.h> @@ -67,7 +66,8 @@ typedef struct blkif_st { /* Physical parameters of the comms window. */ unsigned int irq; /* Comms information. */ - blkif_back_ring_t blk_ring; + int blk_protocol; + blkif_back_rings_t blk_rings; struct vm_struct *blk_ring_area; /* The VBD attached to this interface. */ struct vbd vbd; Index: build-32-unstable-13534/linux-2.6-xen-sparse/drivers/xen/blkback/interface.c =================================================================== --- build-32-unstable-13534.orig/linux-2.6-xen-sparse/drivers/xen/blkback/interface.c +++ build-32-unstable-13534/linux-2.6-xen-sparse/drivers/xen/blkback/interface.c @@ -95,7 +95,6 @@ static void unmap_frontend_page(blkif_t int blkif_map(blkif_t *blkif, unsigned long shared_page, unsigned int evtchn) { - blkif_sring_t *sring; int err; /* Already connected through? */ @@ -111,8 +110,24 @@ int blkif_map(blkif_t *blkif, unsigned l return err; } - sring = (blkif_sring_t *)blkif->blk_ring_area->addr; - BACK_RING_INIT(&blkif->blk_ring, sring, PAGE_SIZE); + switch (blkif->blk_protocol) { + case 1: + { + blkif_x86_32_sring_t *sring_x86_32; + sring_x86_32 = (blkif_x86_32_sring_t *)blkif->blk_ring_area->addr; + BACK_RING_INIT(&blkif->blk_rings.x86_32, sring_x86_32, PAGE_SIZE); + break; + } + case 2: + { + blkif_x86_64_sring_t *sring_x86_64; + sring_x86_64 = (blkif_x86_64_sring_t *)blkif->blk_ring_area->addr; + BACK_RING_INIT(&blkif->blk_rings.x86_64, sring_x86_64, PAGE_SIZE); + break; + } + default: + BUG(); + } err = bind_interdomain_evtchn_to_irqhandler( blkif->domid, evtchn, blkif_be_int, 0, "blkif-backend", blkif); @@ -143,10 +158,10 @@ void blkif_disconnect(blkif_t *blkif) blkif->irq = 0; } - if (blkif->blk_ring.sring) { + if (blkif->blk_rings.common.sring) { unmap_frontend_page(blkif); free_vm_area(blkif->blk_ring_area); - blkif->blk_ring.sring = NULL; + blkif->blk_rings.common.sring = NULL; } } Index: build-32-unstable-13534/linux-2.6-xen-sparse/drivers/xen/blkback/xenbus.c =================================================================== --- build-32-unstable-13534.orig/linux-2.6-xen-sparse/drivers/xen/blkback/xenbus.c +++ build-32-unstable-13534/linux-2.6-xen-sparse/drivers/xen/blkback/xenbus.c @@ -459,6 +459,7 @@ static int connect_ring(struct backend_i struct xenbus_device *dev = be->dev; unsigned long ring_ref; unsigned int evtchn; + char protocol[64] = ""; int err; DPRINTK("%s", dev->otherend); @@ -472,6 +473,24 @@ static int connect_ring(struct backend_i return err; } + be->blkif->blk_protocol = BLKIF_PROTOCOL_NATIVE; + err = xenbus_gather(XBT_NIL, dev->otherend, "protocol", + "%63s", protocol, NULL); + if (err) + strcpy(protocol, "unspecified, assuming native"); + else if (0 == strcmp(protocol, XEN_IO_PROTO_ABI_NATIVE)) + be->blkif->blk_protocol = BLKIF_PROTOCOL_NATIVE; + else if (0 == strcmp(protocol, XEN_IO_PROTO_ABI_X86_32)) + be->blkif->blk_protocol = BLKIF_PROTOCOL_X86_32; + else if (0 == strcmp(protocol, XEN_IO_PROTO_ABI_X86_64)) + be->blkif->blk_protocol = BLKIF_PROTOCOL_X86_64; + else { + xenbus_dev_fatal(dev, err, "unknown fe protocol %s", protocol); + return -1; + } + printk("blkback: ring-ref %ld, event-channel %d, protocol %d (%s)\n", + ring_ref, evtchn, be->blkif->blk_protocol, protocol); + /* Map the shared frame, irq etc. */ err = blkif_map(be->blkif, ring_ref, evtchn); if (err) { Index: build-32-unstable-13534/xen/include/public/io/blkif.h =================================================================== --- build-32-unstable-13534.orig/xen/include/public/io/blkif.h +++ build-32-unstable-13534/xen/include/public/io/blkif.h @@ -71,18 +71,20 @@ */ #define BLKIF_MAX_SEGMENTS_PER_REQUEST 11 +struct blkif_request_segment { + grant_ref_t gref; /* reference to I/O buffer frame */ + /* @first_sect: first sector in frame to transfer (inclusive). */ + /* @last_sect: last sector in frame to transfer (inclusive). */ + uint8_t first_sect, last_sect; +}; + struct blkif_request { uint8_t operation; /* BLKIF_OP_??? */ uint8_t nr_segments; /* number of segments */ blkif_vdev_t handle; /* only for read/write requests */ uint64_t id; /* private guest value, echoed in resp */ blkif_sector_t sector_number;/* start sector idx on disk (r/w only) */ - struct blkif_request_segment { - grant_ref_t gref; /* reference to I/O buffer frame */ - /* @first_sect: first sector in frame to transfer (inclusive). */ - /* @last_sect: last sector in frame to transfer (inclusive). */ - uint8_t first_sect, last_sect; - } seg[BLKIF_MAX_SEGMENTS_PER_REQUEST]; + struct blkif_request_segment seg[BLKIF_MAX_SEGMENTS_PER_REQUEST]; }; typedef struct blkif_request blkif_request_t; Index: build-32-unstable-13534/linux-2.6-xen-sparse/drivers/xen/blktap/blktap.c =================================================================== --- build-32-unstable-13534.orig/linux-2.6-xen-sparse/drivers/xen/blktap/blktap.c +++ build-32-unstable-13534/linux-2.6-xen-sparse/drivers/xen/blktap/blktap.c @@ -1091,15 +1091,15 @@ irqreturn_t tap_blkif_be_int(int irq, vo static int print_dbug = 1; static int do_block_io_op(blkif_t *blkif) { - blkif_back_ring_t *blk_ring = &blkif->blk_ring; + blkif_back_rings_t *blk_rings = &blkif->blk_rings; blkif_request_t req; pending_req_t *pending_req; RING_IDX rc, rp; int more_to_do = 0; tap_blkif_t *info; - rc = blk_ring->req_cons; - rp = blk_ring->sring->req_prod; + rc = blk_rings->common.req_cons; + rp = blk_rings->common.sring->req_prod; rmb(); /* Ensure we see queued requests up to 'rp'. */ /*Check blkif has corresponding UE ring*/ @@ -1130,8 +1130,8 @@ static int do_block_io_op(blkif_t *blkif more_to_do = 1; break; } - - if (RING_REQUEST_CONS_OVERFLOW(blk_ring, rc)) { + + if (RING_REQUEST_CONS_OVERFLOW(&blk_rings->common, rc)) { WPRINTK("RING_REQUEST_CONS_OVERFLOW!" " More to do\n"); more_to_do = 1; @@ -1145,8 +1145,21 @@ static int do_block_io_op(blkif_t *blkif break; } - memcpy(&req, RING_GET_REQUEST(blk_ring, rc), sizeof(req)); - blk_ring->req_cons = ++rc; /* before make_response() */ + switch (blkif->blk_protocol) { + case BLKIF_PROTOCOL_NATIVE: + memcpy(&req, RING_GET_REQUEST(&blk_rings->native, rc), + sizeof(req)); + break; + case BLKIF_PROTOCOL_X86_32: + blkif_get_x86_32_req(&req, RING_GET_REQUEST(&blk_rings->x86_32, rc)); + break; + case BLKIF_PROTOCOL_X86_64: + blkif_get_x86_64_req(&req, RING_GET_REQUEST(&blk_rings->x86_64, rc)); + break; + default: + BUG(); + } + blk_rings->common.req_cons = ++rc; /* before make_response() */ switch (req.operation) { case BLKIF_OP_READ: @@ -1222,7 +1235,7 @@ static void dispatch_rw_block_io(blkif_t WPRINTK("blktap: fe_ring is full, can't add " "IO Request will be dropped. %d %d\n", RING_SIZE(&info->ufe_ring), - RING_SIZE(&blkif->blk_ring)); + RING_SIZE(&blkif->blk_rings.common)); goto fail_response; } @@ -1410,32 +1423,51 @@ static void dispatch_rw_block_io(blkif_t static void make_response(blkif_t *blkif, unsigned long id, unsigned short op, int st) { - blkif_response_t *resp; + blkif_response_t resp; unsigned long flags; - blkif_back_ring_t *blk_ring = &blkif->blk_ring; + blkif_back_rings_t *blk_rings = &blkif->blk_rings; int more_to_do = 0; int notify; + resp.id = id; + resp.operation = op; + resp.status = st; + spin_lock_irqsave(&blkif->blk_ring_lock, flags); - /* Place on the response ring for the relevant domain. */ - resp = RING_GET_RESPONSE(blk_ring, blk_ring->rsp_prod_pvt); - resp->id = id; - resp->operation = op; - resp->status = st; - blk_ring->rsp_prod_pvt++; - RING_PUSH_RESPONSES_AND_CHECK_NOTIFY(blk_ring, notify); + /* Place on the response ring for the relevant domain. */ + switch (blkif->blk_protocol) { + case BLKIF_PROTOCOL_NATIVE: + memcpy(RING_GET_RESPONSE(&blk_rings->native, + blk_rings->native.rsp_prod_pvt), + &resp, sizeof(resp)); + break; + case BLKIF_PROTOCOL_X86_32: + memcpy(RING_GET_RESPONSE(&blk_rings->x86_32, + blk_rings->x86_32.rsp_prod_pvt), + &resp, sizeof(resp)); + break; + case BLKIF_PROTOCOL_X86_64: + memcpy(RING_GET_RESPONSE(&blk_rings->x86_64, + blk_rings->x86_64.rsp_prod_pvt), + &resp, sizeof(resp)); + break; + default: + BUG(); + } + blk_rings->common.rsp_prod_pvt++; + RING_PUSH_RESPONSES_AND_CHECK_NOTIFY(&blk_rings->common, notify); - if (blk_ring->rsp_prod_pvt == blk_ring->req_cons) { + if (blk_rings->common.rsp_prod_pvt == blk_rings->common.req_cons) { /* * Tail check for pending requests. Allows frontend to avoid * notifications if requests are already in flight (lower * overheads and promotes batching). */ - RING_FINAL_CHECK_FOR_REQUESTS(blk_ring, more_to_do); - } else if (RING_HAS_UNCONSUMED_REQUESTS(blk_ring)) { + RING_FINAL_CHECK_FOR_REQUESTS(&blk_rings->common, more_to_do); + } else if (RING_HAS_UNCONSUMED_REQUESTS(&blk_rings->common)) { more_to_do = 1; + } - } spin_unlock_irqrestore(&blkif->blk_ring_lock, flags); if (more_to_do) blkif_notify_work(blkif); Index: build-32-unstable-13534/linux-2.6-xen-sparse/drivers/xen/blktap/common.h =================================================================== --- build-32-unstable-13534.orig/linux-2.6-xen-sparse/drivers/xen/blktap/common.h +++ build-32-unstable-13534/linux-2.6-xen-sparse/drivers/xen/blktap/common.h @@ -39,8 +39,7 @@ #include <asm/pgalloc.h> #include <xen/evtchn.h> #include <asm/hypervisor.h> -#include <xen/interface/io/blkif.h> -#include <xen/interface/io/ring.h> +#include <xen/blkif.h> #include <xen/gnttab.h> #include <xen/driver_util.h> @@ -58,7 +57,8 @@ typedef struct blkif_st { /* Physical parameters of the comms window. */ unsigned int irq; /* Comms information. */ - blkif_back_ring_t blk_ring; + int blk_protocol; + blkif_back_rings_t blk_rings; struct vm_struct *blk_ring_area; /* Back pointer to the backend_info. */ struct backend_info *be; Index: build-32-unstable-13534/linux-2.6-xen-sparse/drivers/xen/blktap/interface.c =================================================================== --- build-32-unstable-13534.orig/linux-2.6-xen-sparse/drivers/xen/blktap/interface.c +++ build-32-unstable-13534/linux-2.6-xen-sparse/drivers/xen/blktap/interface.c @@ -96,7 +96,6 @@ static void unmap_frontend_page(blkif_t int tap_blkif_map(blkif_t *blkif, unsigned long shared_page, unsigned int evtchn) { - blkif_sring_t *sring; int err; /* Already connected through? */ @@ -112,8 +111,24 @@ int tap_blkif_map(blkif_t *blkif, unsign return err; } - sring = (blkif_sring_t *)blkif->blk_ring_area->addr; - BACK_RING_INIT(&blkif->blk_ring, sring, PAGE_SIZE); + switch (blkif->blk_protocol) { + case 1: + { + blkif_x86_32_sring_t *sring_x86_32; + sring_x86_32 = (blkif_x86_32_sring_t *)blkif->blk_ring_area->addr; + BACK_RING_INIT(&blkif->blk_rings.x86_32, sring_x86_32, PAGE_SIZE); + break; + } + case 2: + { + blkif_x86_64_sring_t *sring_x86_64; + sring_x86_64 = (blkif_x86_64_sring_t *)blkif->blk_ring_area->addr; + BACK_RING_INIT(&blkif->blk_rings.x86_64, sring_x86_64, PAGE_SIZE); + break; + } + default: + BUG(); + } err = bind_interdomain_evtchn_to_irqhandler( blkif->domid, evtchn, tap_blkif_be_int, @@ -134,10 +149,10 @@ void tap_blkif_unmap(blkif_t *blkif) unbind_from_irqhandler(blkif->irq, blkif); blkif->irq = 0; } - if (blkif->blk_ring.sring) { + if (blkif->blk_rings.common.sring) { unmap_frontend_page(blkif); free_vm_area(blkif->blk_ring_area); - blkif->blk_ring.sring = NULL; + blkif->blk_rings.common.sring = NULL; } } Index: build-32-unstable-13534/linux-2.6-xen-sparse/drivers/xen/blktap/xenbus.c =================================================================== --- build-32-unstable-13534.orig/linux-2.6-xen-sparse/drivers/xen/blktap/xenbus.c +++ build-32-unstable-13534/linux-2.6-xen-sparse/drivers/xen/blktap/xenbus.c @@ -340,6 +340,7 @@ static int connect_ring(struct backend_i struct xenbus_device *dev = be->dev; unsigned long ring_ref; unsigned int evtchn; + char protocol[64]; int err; DPRINTK("%s\n", dev->otherend); @@ -353,6 +354,24 @@ static int connect_ring(struct backend_i return err; } + be->blkif->blk_protocol = BLKIF_PROTOCOL_NATIVE; + err = xenbus_gather(XBT_NIL, dev->otherend, "protocol", + "%63s", protocol, NULL); + if (err) + strcpy(protocol, "unspecified, assuming native"); + else if (0 == strcmp(protocol, XEN_IO_PROTO_ABI_NATIVE)) + be->blkif->blk_protocol = BLKIF_PROTOCOL_NATIVE; + else if (0 == strcmp(protocol, XEN_IO_PROTO_ABI_X86_32)) + be->blkif->blk_protocol = BLKIF_PROTOCOL_X86_32; + else if (0 == strcmp(protocol, XEN_IO_PROTO_ABI_X86_64)) + be->blkif->blk_protocol = BLKIF_PROTOCOL_X86_64; + else { + xenbus_dev_fatal(dev, err, "unknown fe protocol %s", protocol); + return -1; + } + printk("blktap: ring-ref %ld, event-channel %d, protocol %d (%s)\n", + ring_ref, evtchn, be->blkif->blk_protocol, protocol); + /* Map the shared frame, irq etc. */ err = tap_blkif_map(be->blkif, ring_ref, evtchn); if (err) { Index: build-32-unstable-13534/linux-2.6-xen-sparse/include/xen/blkif.h =================================================================== --- /dev/null +++ build-32-unstable-13534/linux-2.6-xen-sparse/include/xen/blkif.h @@ -0,0 +1,95 @@ +#ifndef __XEN_BLKIF_H__ +#define __XEN_BLKIF_H__ + +#include <xen/interface/io/ring.h> +#include <xen/interface/io/blkif.h> +#include <xen/interface/io/protocols.h> + +/* Not a real protocol. Used to generate ring structs which contain + * the elements common to all protocols only. This way we get a + * compiler-checkable way to use common struct elements, so we can + * avoid using switch(protocol) in a number of places. */ +struct blkif_common_request { + char dummy; +}; +struct blkif_common_response { + char dummy; +}; + +/* i386 protocol version */ +#pragma pack(push, 4) +struct blkif_x86_32_request { + uint8_t operation; /* BLKIF_OP_??? */ + uint8_t nr_segments; /* number of segments */ + blkif_vdev_t handle; /* only for read/write requests */ + uint64_t id; /* private guest value, echoed in resp */ + blkif_sector_t sector_number;/* start sector idx on disk (r/w only) */ + struct blkif_request_segment seg[BLKIF_MAX_SEGMENTS_PER_REQUEST]; +}; +struct blkif_x86_32_response { + uint64_t id; /* copied from request */ + uint8_t operation; /* copied from request */ + int16_t status; /* BLKIF_RSP_??? */ +}; +typedef struct blkif_x86_32_request blkif_x86_32_request_t; +typedef struct blkif_x86_32_response blkif_x86_32_response_t; +#pragma pack(pop) + +/* x86_64 protocol version */ +struct blkif_x86_64_request { + uint8_t operation; /* BLKIF_OP_??? */ + uint8_t nr_segments; /* number of segments */ + blkif_vdev_t handle; /* only for read/write requests */ + uint64_t __attribute__((__aligned__(8))) id; + blkif_sector_t sector_number;/* start sector idx on disk (r/w only) */ + struct blkif_request_segment seg[BLKIF_MAX_SEGMENTS_PER_REQUEST]; +}; +struct blkif_x86_64_response { + uint64_t __attribute__((__aligned__(8))) id; + uint8_t operation; /* copied from request */ + int16_t status; /* BLKIF_RSP_??? */ +}; +typedef struct blkif_x86_64_request blkif_x86_64_request_t; +typedef struct blkif_x86_64_response blkif_x86_64_response_t; + +DEFINE_RING_TYPES(blkif_common, struct blkif_common_request, struct blkif_common_response); +DEFINE_RING_TYPES(blkif_x86_32, struct blkif_x86_32_request, struct blkif_x86_32_response); +DEFINE_RING_TYPES(blkif_x86_64, struct blkif_x86_64_request, struct blkif_x86_64_response); + +union blkif_back_rings { + blkif_back_ring_t native; + blkif_common_back_ring_t common; + blkif_x86_32_back_ring_t x86_32; + blkif_x86_64_back_ring_t x86_64; +}; +typedef union blkif_back_rings blkif_back_rings_t; + +#define BLKIF_PROTOCOL_NATIVE 1 +#define BLKIF_PROTOCOL_X86_32 2 +#define BLKIF_PROTOCOL_X86_64 3 + +static void inline blkif_get_x86_32_req(blkif_request_t *dst, blkif_x86_32_request_t *src) +{ + int i; + dst->operation = src->operation; + dst->nr_segments = src->nr_segments; + dst->handle = src->handle; + dst->id = src->id; + dst->sector_number = src->sector_number; + for (i = 0; i < src->nr_segments; i++) + dst->seg[i] = src->seg[i]; +} + +static void inline blkif_get_x86_64_req(blkif_request_t *dst, blkif_x86_64_request_t *src) +{ + int i; + dst->operation = src->operation; + dst->nr_segments = src->nr_segments; + dst->handle = src->handle; + dst->id = src->id; + dst->sector_number = src->sector_number; + for (i = 0; i < src->nr_segments; i++) + dst->seg[i] = src->seg[i]; +} + +#endif /* __XEN_BLKIF_H__ */ [-- Attachment #6: fbback-bimodal.diff --] [-- Type: text/x-patch, Size: 4592 bytes --] bimodal: pvfb backend Teach pvfb backend to deal with bith 32 and 64 bit frontends. Signed-off-by: Gerd Hoffmann <kraxel@suse.de> --- tools/xenfb/xenfb.c | 103 ++++++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 92 insertions(+), 11 deletions(-) Index: build-32-unstable-13495/tools/xenfb/xenfb.c =================================================================== --- build-32-unstable-13495.orig/tools/xenfb/xenfb.c +++ build-32-unstable-13495/tools/xenfb/xenfb.c @@ -7,6 +7,7 @@ #include <xen/io/xenbus.h> #include <xen/io/fbif.h> #include <xen/io/kbdif.h> +#include <xen/io/protocols.h> #include <sys/select.h> #include <stdbool.h> #include <xen/linux/evtchn.h> @@ -40,6 +41,7 @@ struct xenfb_private { struct xs_handle *xsh; /* xs daemon handle */ struct xenfb_device fb, kbd; size_t fb_len; /* size of framebuffer */ + char protocol[64]; /* frontend protocol */ }; static void xenfb_detach_dom(struct xenfb_private *); @@ -324,36 +326,112 @@ static int xenfb_wait_for_frontend_initi return 0; } +static void xenfb_copy_mfns(int mode, int count, unsigned long *dst, void *src) +{ + uint32_t *src32 = src; + uint64_t *src64 = src; + int i; + + if (32 == mode) { + for (i = 0; i < count; i++) + dst[i] = src32[i]; + } else { + for (i = 0; i < count; i++) + dst[i] = src64[i]; + } +} + static int xenfb_map_fb(struct xenfb_private *xenfb, int domid) { struct xenfb_page *page = xenfb->fb.page; int n_fbmfns; int n_fbdirs; - unsigned long *fbmfns; + unsigned long *pgmfns = NULL; + unsigned long *fbmfns = NULL; + void *map, *pd; + int mode, ret = -1; + + /* default to native */ + pd = page->pd; + mode = sizeof(unsigned long) * 8; + + if (0 == strlen(xenfb->protocol)) { + /* + * Undefined protocol, some guesswork needed. + * + * Old frontends which don't set the protocol use + * one page directory only, thus pd[1] must be zero. + * pd[1] of the 32bit struct layout and the lower + * 32 bits of pd[0] of the 64bit struct layout have + * the same location, so we can check that ... + */ + uint32_t *ptr32 = NULL; + uint32_t *ptr64 = NULL; +#if defined(__i386_) + ptr32 = page->pd; + ptr64 = ((void*)page->pd) + 4; +#elif defined(__x86_64__) + ptr32 = ((void*)page->pd) - 4; + ptr64 = page->pd; +#endif + if (ptr32) { + if (0 == ptr32[1]) { + mode = 32; + pd = ptr32; + } else { + mode = 64; + pd = ptr64; + } + } +#if defined(__x86_64__) + } else if (0 == strcmp(xenfb->protocol, XEN_IO_PROTO_ABI_X86_32)) { + /* 64bit dom0, 32bit domU */ + mode = 32; + pd = ((void*)page->pd) - 4; +#elif defined(__i386_) + } else if (0 == strcmp(xenfb->protocol, XEN_IO_PROTO_ABI_X86_64)) { + /* 32bit dom0, 64bit domU */ + mode = 64; + pd = ((void*)page->pd) + 4; +#endif + } n_fbmfns = (xenfb->fb_len + (XC_PAGE_SIZE - 1)) / XC_PAGE_SIZE; - n_fbdirs = n_fbmfns * sizeof(unsigned long); + n_fbdirs = n_fbmfns * mode / 8; n_fbdirs = (n_fbdirs + (XC_PAGE_SIZE - 1)) / XC_PAGE_SIZE; + pgmfns = malloc(sizeof(unsigned long) * n_fbdirs); + fbmfns = malloc(sizeof(unsigned long) * n_fbmfns); + if (!pgmfns || !fbmfns) + goto out; + /* * Bug alert: xc_map_foreign_batch() can fail partly and * return a non-null value. This is a design flaw. When it * happens, we happily continue here, and later crash on * access. */ - fbmfns = xc_map_foreign_batch(xenfb->xc, domid, - PROT_READ, page->pd, n_fbdirs); - if (fbmfns == NULL) - return -1; + xenfb_copy_mfns(mode, n_fbdirs, pgmfns, pd); + map = xc_map_foreign_batch(xenfb->xc, domid, + PROT_READ, pgmfns, n_fbdirs); + if (map == NULL) + goto out; + xenfb_copy_mfns(mode, n_fbmfns, fbmfns, map); + munmap(map, n_fbdirs * XC_PAGE_SIZE); xenfb->pub.pixels = xc_map_foreign_batch(xenfb->xc, domid, PROT_READ | PROT_WRITE, fbmfns, n_fbmfns); - if (xenfb->pub.pixels == NULL) { - munmap(fbmfns, n_fbdirs * XC_PAGE_SIZE); - return -1; - } + if (xenfb->pub.pixels == NULL) + goto out; - return munmap(fbmfns, n_fbdirs * XC_PAGE_SIZE); + ret = 0; /* all is fine */ + + out: + if (pgmfns) + free(pgmfns); + if (fbmfns) + free(fbmfns); + return ret; } static int xenfb_bind(struct xenfb_device *dev) @@ -368,6 +446,9 @@ static int xenfb_bind(struct xenfb_devic if (xenfb_xs_scanf1(xenfb->xsh, dev->otherend, "event-channel", "%u", &evtchn) < 0) return -1; + if (xenfb_xs_scanf1(xenfb->xsh, dev->otherend, "protocol", "%63s", + xenfb->protocol) < 0) + xenfb->protocol[0] = '\0'; dev->port = xc_evtchn_bind_interdomain(xenfb->evt_xch, dev->otherend_id, evtchn); [-- Attachment #7: blktools-bimodal.diff --] [-- Type: text/x-patch, Size: 2694 bytes --] bimodal: blk tools Add one more option to the disk configuration, so one can specify the protocol the frontend speaks in the config file. This is needed for old frontends which don't advertise the protocol they are speaking themself. I'm not that happy with this approach, but it works for now and I'm kida lost in the stack of python classes doing domain and device handling ... Signed-off-by: Gerd Hoffmann <kraxel@suse.de> --- tools/python/xen/xend/server/blkif.py | 3 +++ tools/python/xen/xm/create.py | 7 ++++++- 2 files changed, 9 insertions(+), 1 deletion(-) Index: build-32-unstable-13534/tools/python/xen/xend/server/blkif.py =================================================================== --- build-32-unstable-13534.orig/tools/python/xen/xend/server/blkif.py +++ build-32-unstable-13534/tools/python/xen/xend/server/blkif.py @@ -38,6 +38,7 @@ class BlkifController(DevController): """@see DevController.getDeviceDetails""" uname = config.get('uname', '') dev = config.get('dev', '') + protocol = config.get('protocol') if 'ioemu:' in dev: (_, dev) = string.split(dev, ':', 1) @@ -85,6 +86,8 @@ class BlkifController(DevController): front = { 'virtual-device' : "%i" % devid, 'device-type' : dev_type } + if protocol: + front.update({ 'protocol' : protocol }); return (devid, back, front) Index: build-32-unstable-13534/tools/python/xen/xm/create.py =================================================================== --- build-32-unstable-13534.orig/tools/python/xen/xm/create.py +++ build-32-unstable-13534/tools/python/xen/xm/create.py @@ -537,7 +537,7 @@ def configure_image(vals): def configure_disks(config_devs, vals): """Create the config for disks (virtual block devices). """ - for (uname, dev, mode, backend) in vals.disk: + for (uname, dev, mode, backend, protocol) in vals.disk: if uname.startswith('tap:'): cls = 'tap' else: @@ -549,6 +549,8 @@ def configure_disks(config_devs, vals): ['mode', mode ] ] if backend: config_vbd.append(['backend', backend]) + if protocol: + config_vbd.append(['protocol', protocol]) config_devs.append(['device', config_vbd]) def configure_pci(config_devs, vals): @@ -803,7 +805,10 @@ def preprocess_disk(vals): n = len(d) if n == 3: d.append(None) + d.append(None) elif n == 4: + d.append(None) + elif n == 5: pass else: err('Invalid disk specifier: ' + v) [-- Attachment #8: Type: text/plain, Size: 138 bytes --] _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: 32-on-64: pvfb issue 2007-01-23 12:53 ` Gerd Hoffmann @ 2007-01-23 15:07 ` Keir Fraser 2007-01-23 15:56 ` Gerd Hoffmann 2007-01-24 11:23 ` Gerd Hoffmann 2007-01-25 13:16 ` 32-on-64 broken in unstable Gerd Hoffmann 2 siblings, 1 reply; 50+ messages in thread From: Keir Fraser @ 2007-01-23 15:07 UTC (permalink / raw) To: Gerd Hoffmann; +Cc: Xen devel list, Markus Armbruster On 23/1/07 12:53, "Gerd Hoffmann" <kraxel@suse.de> wrote: > Fixed. New versions attached, with comments added/updated and > signed-off-by. They are partly tested only though, 32-on-64 seems to be > broken in current unstable, mixing 32bit and 64bit domains doesn't work. > Time to get the domain builder bits merged too, so this can be added to > the regression tests ... I've checked all in except blkback and blktools: * blktools: I'm not sure if this approach will be necessary. Brendan Cully is adding support for pulling kernel image metadata into xend early during domain creation. We can include target arch as part of that metadata and then plumb that into the BlkifController to set the protocol automatically. Unless there are other reasons to want to manually specify the protocol then we are probably better to wait for the 'proper' solution. * blkback: You didn't sign off, and the changeset comment is stale (still refers to v1/v2 protocols for example). Also in a couple of places (the blkif_mmap functions in both blkback and blktap) you switch on blkif protocol with cases 1 and 2, rather than using the enumeration names. That looks a bit dodgy to me -- did you miss updating them? -- Keir ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: 32-on-64: pvfb issue 2007-01-23 15:07 ` Keir Fraser @ 2007-01-23 15:56 ` Gerd Hoffmann 0 siblings, 0 replies; 50+ messages in thread From: Gerd Hoffmann @ 2007-01-23 15:56 UTC (permalink / raw) To: Keir Fraser; +Cc: Xen devel list, Markus Armbruster [-- Attachment #1: Type: text/plain, Size: 1141 bytes --] Hi, > * blktools: I'm not sure if this approach will be necessary. Brendan Cully > is adding support for pulling kernel image metadata into xend early during > domain creation. We can include target arch as part of that metadata and > then plumb that into the BlkifController to set the protocol automatically. > Unless there are other reasons to want to manually specify the protocol then > we are probably better to wait for the 'proper' solution. I don't mind, as noted in the patch that is just the easiest way I've found and most likely not the best one. Just drop it. > * blkback: You didn't sign off, and the changeset comment is stale (still > refers to v1/v2 protocols for example). Also in a couple of places (the > blkif_mmap functions in both blkback and blktap) you switch on blkif > protocol with cases 1 and 2, rather than using the enumeration names. That > looks a bit dodgy to me -- did you miss updating them? Yep, thanks for spotting these. Fixed version attached. Also changed the blk_protocol to enum, so the compiler will warn on such bugs in the future. cheers, Gerd -- Gerd Hoffmann <kraxel@suse.de> [-- Attachment #2: blkback-bimodal.diff --] [-- Type: text/x-patch, Size: 23981 bytes --] multiprotocol blkback drivers. This is a patch for the block backend drivers to support multiple ring protocols. This is needed for 32-on-64 support. Right there are three protocols: native, x86_32 and x86_64. If needed it can be extended. Interface changes (io/blkif.h) * Define the x86_32 and x86_64 structs additionally to the native version. * Add helper functions to convert them requests to native. Backend changes: * Look at the "protocol" name of the frontend and switch ring handling accordingly. If the protocol node isn't present it assumes native protocol. * As the request struct is copied anyway before being processed (for security reasons) it is converted to native at that point so most backend code doesn't need to know what the frontend speaks. * In case of blktap this is completely transparent to userspace, the kernel/userspace ring is always native no matter what the frontend speaks. Signed-off-by: Gerd Hoffmann <kraxel@suse.de> --- linux-2.6-xen-sparse/drivers/xen/blkback/blkback.c | 71 +++++++++---- linux-2.6-xen-sparse/drivers/xen/blkback/common.h | 6 - linux-2.6-xen-sparse/drivers/xen/blkback/interface.c | 32 +++++- linux-2.6-xen-sparse/drivers/xen/blkback/xenbus.c | 19 +++ linux-2.6-xen-sparse/drivers/xen/blktap/blktap.c | 74 ++++++++++---- linux-2.6-xen-sparse/drivers/xen/blktap/common.h | 6 - linux-2.6-xen-sparse/drivers/xen/blktap/interface.c | 32 +++++- linux-2.6-xen-sparse/drivers/xen/blktap/xenbus.c | 19 +++ linux-2.6-xen-sparse/include/xen/blkif.h | 97 +++++++++++++++++++ xen/include/public/io/blkif.h | 14 +- 10 files changed, 306 insertions(+), 64 deletions(-) Index: build-32-unstable-13553/linux-2.6-xen-sparse/drivers/xen/blkback/blkback.c =================================================================== --- build-32-unstable-13553.orig/linux-2.6-xen-sparse/drivers/xen/blkback/blkback.c +++ build-32-unstable-13553/linux-2.6-xen-sparse/drivers/xen/blkback/blkback.c @@ -298,17 +298,20 @@ irqreturn_t blkif_be_int(int irq, void * static int do_block_io_op(blkif_t *blkif) { - blkif_back_ring_t *blk_ring = &blkif->blk_ring; + blkif_back_rings_t *blk_rings = &blkif->blk_rings; blkif_request_t req; pending_req_t *pending_req; RING_IDX rc, rp; int more_to_do = 0; - rc = blk_ring->req_cons; - rp = blk_ring->sring->req_prod; + rc = blk_rings->common.req_cons; + rp = blk_rings->common.sring->req_prod; rmb(); /* Ensure we see queued requests up to 'rp'. */ - while ((rc != rp) && !RING_REQUEST_CONS_OVERFLOW(blk_ring, rc)) { + while ((rc != rp)) { + + if (RING_REQUEST_CONS_OVERFLOW(&blk_rings->common, rc)) + break; pending_req = alloc_req(); if (NULL == pending_req) { @@ -317,8 +320,20 @@ static int do_block_io_op(blkif_t *blkif break; } - memcpy(&req, RING_GET_REQUEST(blk_ring, rc), sizeof(req)); - blk_ring->req_cons = ++rc; /* before make_response() */ + switch (blkif->blk_protocol) { + case BLKIF_PROTOCOL_NATIVE: + memcpy(&req, RING_GET_REQUEST(&blk_rings->native, rc), sizeof(req)); + break; + case BLKIF_PROTOCOL_X86_32: + blkif_get_x86_32_req(&req, RING_GET_REQUEST(&blk_rings->x86_32, rc)); + break; + case BLKIF_PROTOCOL_X86_64: + blkif_get_x86_64_req(&req, RING_GET_REQUEST(&blk_rings->x86_64, rc)); + break; + default: + BUG(); + } + blk_rings->common.req_cons = ++rc; /* before make_response() */ switch (req.operation) { case BLKIF_OP_READ: @@ -498,34 +513,48 @@ static void dispatch_rw_block_io(blkif_t static void make_response(blkif_t *blkif, unsigned long id, unsigned short op, int st) { - blkif_response_t *resp; + blkif_response_t resp; unsigned long flags; - blkif_back_ring_t *blk_ring = &blkif->blk_ring; + blkif_back_rings_t *blk_rings = &blkif->blk_rings; int more_to_do = 0; int notify; - spin_lock_irqsave(&blkif->blk_ring_lock, flags); - - /* Place on the response ring for the relevant domain. */ - resp = RING_GET_RESPONSE(blk_ring, blk_ring->rsp_prod_pvt); - resp->id = id; - resp->operation = op; - resp->status = st; - blk_ring->rsp_prod_pvt++; - RING_PUSH_RESPONSES_AND_CHECK_NOTIFY(blk_ring, notify); + resp.id = id; + resp.operation = op; + resp.status = st; - if (blk_ring->rsp_prod_pvt == blk_ring->req_cons) { + spin_lock_irqsave(&blkif->blk_ring_lock, flags); + /* Place on the response ring for the relevant domain. */ + switch (blkif->blk_protocol) { + case BLKIF_PROTOCOL_NATIVE: + memcpy(RING_GET_RESPONSE(&blk_rings->native, blk_rings->native.rsp_prod_pvt), + &resp, sizeof(resp)); + break; + case BLKIF_PROTOCOL_X86_32: + memcpy(RING_GET_RESPONSE(&blk_rings->x86_32, blk_rings->x86_32.rsp_prod_pvt), + &resp, sizeof(resp)); + break; + case BLKIF_PROTOCOL_X86_64: + memcpy(RING_GET_RESPONSE(&blk_rings->x86_64, blk_rings->x86_64.rsp_prod_pvt), + &resp, sizeof(resp)); + break; + default: + BUG(); + } + blk_rings->common.rsp_prod_pvt++; + RING_PUSH_RESPONSES_AND_CHECK_NOTIFY(&blk_rings->common, notify); + if (blk_rings->common.rsp_prod_pvt == blk_rings->common.req_cons) { /* * Tail check for pending requests. Allows frontend to avoid * notifications if requests are already in flight (lower * overheads and promotes batching). */ - RING_FINAL_CHECK_FOR_REQUESTS(blk_ring, more_to_do); + RING_FINAL_CHECK_FOR_REQUESTS(&blk_rings->common, more_to_do); - } else if (RING_HAS_UNCONSUMED_REQUESTS(blk_ring)) { + } else if (RING_HAS_UNCONSUMED_REQUESTS(&blk_rings->common)) { more_to_do = 1; - } + spin_unlock_irqrestore(&blkif->blk_ring_lock, flags); if (more_to_do) Index: build-32-unstable-13553/linux-2.6-xen-sparse/drivers/xen/blkback/common.h =================================================================== --- build-32-unstable-13553.orig/linux-2.6-xen-sparse/drivers/xen/blkback/common.h +++ build-32-unstable-13553/linux-2.6-xen-sparse/drivers/xen/blkback/common.h @@ -40,8 +40,7 @@ #include <asm/pgalloc.h> #include <xen/evtchn.h> #include <asm/hypervisor.h> -#include <xen/interface/io/blkif.h> -#include <xen/interface/io/ring.h> +#include <xen/blkif.h> #include <xen/gnttab.h> #include <xen/driver_util.h> #include <xen/xenbus.h> @@ -67,7 +66,8 @@ typedef struct blkif_st { /* Physical parameters of the comms window. */ unsigned int irq; /* Comms information. */ - blkif_back_ring_t blk_ring; + enum blkif_protocol blk_protocol; + blkif_back_rings_t blk_rings; struct vm_struct *blk_ring_area; /* The VBD attached to this interface. */ struct vbd vbd; Index: build-32-unstable-13553/linux-2.6-xen-sparse/drivers/xen/blkback/interface.c =================================================================== --- build-32-unstable-13553.orig/linux-2.6-xen-sparse/drivers/xen/blkback/interface.c +++ build-32-unstable-13553/linux-2.6-xen-sparse/drivers/xen/blkback/interface.c @@ -95,7 +95,6 @@ static void unmap_frontend_page(blkif_t int blkif_map(blkif_t *blkif, unsigned long shared_page, unsigned int evtchn) { - blkif_sring_t *sring; int err; /* Already connected through? */ @@ -111,8 +110,31 @@ int blkif_map(blkif_t *blkif, unsigned l return err; } - sring = (blkif_sring_t *)blkif->blk_ring_area->addr; - BACK_RING_INIT(&blkif->blk_ring, sring, PAGE_SIZE); + switch (blkif->blk_protocol) { + case BLKIF_PROTOCOL_NATIVE: + { + blkif_sring_t *sring; + sring = (blkif_sring_t *)blkif->blk_ring_area->addr; + BACK_RING_INIT(&blkif->blk_rings.native, sring, PAGE_SIZE); + break; + } + case BLKIF_PROTOCOL_X86_32: + { + blkif_x86_32_sring_t *sring_x86_32; + sring_x86_32 = (blkif_x86_32_sring_t *)blkif->blk_ring_area->addr; + BACK_RING_INIT(&blkif->blk_rings.x86_32, sring_x86_32, PAGE_SIZE); + break; + } + case BLKIF_PROTOCOL_X86_64: + { + blkif_x86_64_sring_t *sring_x86_64; + sring_x86_64 = (blkif_x86_64_sring_t *)blkif->blk_ring_area->addr; + BACK_RING_INIT(&blkif->blk_rings.x86_64, sring_x86_64, PAGE_SIZE); + break; + } + default: + BUG(); + } err = bind_interdomain_evtchn_to_irqhandler( blkif->domid, evtchn, blkif_be_int, 0, "blkif-backend", blkif); @@ -143,10 +165,10 @@ void blkif_disconnect(blkif_t *blkif) blkif->irq = 0; } - if (blkif->blk_ring.sring) { + if (blkif->blk_rings.common.sring) { unmap_frontend_page(blkif); free_vm_area(blkif->blk_ring_area); - blkif->blk_ring.sring = NULL; + blkif->blk_rings.common.sring = NULL; } } Index: build-32-unstable-13553/linux-2.6-xen-sparse/drivers/xen/blkback/xenbus.c =================================================================== --- build-32-unstable-13553.orig/linux-2.6-xen-sparse/drivers/xen/blkback/xenbus.c +++ build-32-unstable-13553/linux-2.6-xen-sparse/drivers/xen/blkback/xenbus.c @@ -459,6 +459,7 @@ static int connect_ring(struct backend_i struct xenbus_device *dev = be->dev; unsigned long ring_ref; unsigned int evtchn; + char protocol[64] = ""; int err; DPRINTK("%s", dev->otherend); @@ -472,6 +473,24 @@ static int connect_ring(struct backend_i return err; } + be->blkif->blk_protocol = BLKIF_PROTOCOL_NATIVE; + err = xenbus_gather(XBT_NIL, dev->otherend, "protocol", + "%63s", protocol, NULL); + if (err) + strcpy(protocol, "unspecified, assuming native"); + else if (0 == strcmp(protocol, XEN_IO_PROTO_ABI_NATIVE)) + be->blkif->blk_protocol = BLKIF_PROTOCOL_NATIVE; + else if (0 == strcmp(protocol, XEN_IO_PROTO_ABI_X86_32)) + be->blkif->blk_protocol = BLKIF_PROTOCOL_X86_32; + else if (0 == strcmp(protocol, XEN_IO_PROTO_ABI_X86_64)) + be->blkif->blk_protocol = BLKIF_PROTOCOL_X86_64; + else { + xenbus_dev_fatal(dev, err, "unknown fe protocol %s", protocol); + return -1; + } + printk("blkback: ring-ref %ld, event-channel %d, protocol %d (%s)\n", + ring_ref, evtchn, be->blkif->blk_protocol, protocol); + /* Map the shared frame, irq etc. */ err = blkif_map(be->blkif, ring_ref, evtchn); if (err) { Index: build-32-unstable-13553/xen/include/public/io/blkif.h =================================================================== --- build-32-unstable-13553.orig/xen/include/public/io/blkif.h +++ build-32-unstable-13553/xen/include/public/io/blkif.h @@ -71,18 +71,20 @@ */ #define BLKIF_MAX_SEGMENTS_PER_REQUEST 11 +struct blkif_request_segment { + grant_ref_t gref; /* reference to I/O buffer frame */ + /* @first_sect: first sector in frame to transfer (inclusive). */ + /* @last_sect: last sector in frame to transfer (inclusive). */ + uint8_t first_sect, last_sect; +}; + struct blkif_request { uint8_t operation; /* BLKIF_OP_??? */ uint8_t nr_segments; /* number of segments */ blkif_vdev_t handle; /* only for read/write requests */ uint64_t id; /* private guest value, echoed in resp */ blkif_sector_t sector_number;/* start sector idx on disk (r/w only) */ - struct blkif_request_segment { - grant_ref_t gref; /* reference to I/O buffer frame */ - /* @first_sect: first sector in frame to transfer (inclusive). */ - /* @last_sect: last sector in frame to transfer (inclusive). */ - uint8_t first_sect, last_sect; - } seg[BLKIF_MAX_SEGMENTS_PER_REQUEST]; + struct blkif_request_segment seg[BLKIF_MAX_SEGMENTS_PER_REQUEST]; }; typedef struct blkif_request blkif_request_t; Index: build-32-unstable-13553/linux-2.6-xen-sparse/drivers/xen/blktap/blktap.c =================================================================== --- build-32-unstable-13553.orig/linux-2.6-xen-sparse/drivers/xen/blktap/blktap.c +++ build-32-unstable-13553/linux-2.6-xen-sparse/drivers/xen/blktap/blktap.c @@ -1091,15 +1091,15 @@ irqreturn_t tap_blkif_be_int(int irq, vo static int print_dbug = 1; static int do_block_io_op(blkif_t *blkif) { - blkif_back_ring_t *blk_ring = &blkif->blk_ring; + blkif_back_rings_t *blk_rings = &blkif->blk_rings; blkif_request_t req; pending_req_t *pending_req; RING_IDX rc, rp; int more_to_do = 0; tap_blkif_t *info; - rc = blk_ring->req_cons; - rp = blk_ring->sring->req_prod; + rc = blk_rings->common.req_cons; + rp = blk_rings->common.sring->req_prod; rmb(); /* Ensure we see queued requests up to 'rp'. */ /*Check blkif has corresponding UE ring*/ @@ -1130,8 +1130,8 @@ static int do_block_io_op(blkif_t *blkif more_to_do = 1; break; } - - if (RING_REQUEST_CONS_OVERFLOW(blk_ring, rc)) { + + if (RING_REQUEST_CONS_OVERFLOW(&blk_rings->common, rc)) { WPRINTK("RING_REQUEST_CONS_OVERFLOW!" " More to do\n"); more_to_do = 1; @@ -1145,8 +1145,21 @@ static int do_block_io_op(blkif_t *blkif break; } - memcpy(&req, RING_GET_REQUEST(blk_ring, rc), sizeof(req)); - blk_ring->req_cons = ++rc; /* before make_response() */ + switch (blkif->blk_protocol) { + case BLKIF_PROTOCOL_NATIVE: + memcpy(&req, RING_GET_REQUEST(&blk_rings->native, rc), + sizeof(req)); + break; + case BLKIF_PROTOCOL_X86_32: + blkif_get_x86_32_req(&req, RING_GET_REQUEST(&blk_rings->x86_32, rc)); + break; + case BLKIF_PROTOCOL_X86_64: + blkif_get_x86_64_req(&req, RING_GET_REQUEST(&blk_rings->x86_64, rc)); + break; + default: + BUG(); + } + blk_rings->common.req_cons = ++rc; /* before make_response() */ switch (req.operation) { case BLKIF_OP_READ: @@ -1222,7 +1235,7 @@ static void dispatch_rw_block_io(blkif_t WPRINTK("blktap: fe_ring is full, can't add " "IO Request will be dropped. %d %d\n", RING_SIZE(&info->ufe_ring), - RING_SIZE(&blkif->blk_ring)); + RING_SIZE(&blkif->blk_rings.common)); goto fail_response; } @@ -1410,32 +1423,51 @@ static void dispatch_rw_block_io(blkif_t static void make_response(blkif_t *blkif, unsigned long id, unsigned short op, int st) { - blkif_response_t *resp; + blkif_response_t resp; unsigned long flags; - blkif_back_ring_t *blk_ring = &blkif->blk_ring; + blkif_back_rings_t *blk_rings = &blkif->blk_rings; int more_to_do = 0; int notify; + resp.id = id; + resp.operation = op; + resp.status = st; + spin_lock_irqsave(&blkif->blk_ring_lock, flags); - /* Place on the response ring for the relevant domain. */ - resp = RING_GET_RESPONSE(blk_ring, blk_ring->rsp_prod_pvt); - resp->id = id; - resp->operation = op; - resp->status = st; - blk_ring->rsp_prod_pvt++; - RING_PUSH_RESPONSES_AND_CHECK_NOTIFY(blk_ring, notify); + /* Place on the response ring for the relevant domain. */ + switch (blkif->blk_protocol) { + case BLKIF_PROTOCOL_NATIVE: + memcpy(RING_GET_RESPONSE(&blk_rings->native, + blk_rings->native.rsp_prod_pvt), + &resp, sizeof(resp)); + break; + case BLKIF_PROTOCOL_X86_32: + memcpy(RING_GET_RESPONSE(&blk_rings->x86_32, + blk_rings->x86_32.rsp_prod_pvt), + &resp, sizeof(resp)); + break; + case BLKIF_PROTOCOL_X86_64: + memcpy(RING_GET_RESPONSE(&blk_rings->x86_64, + blk_rings->x86_64.rsp_prod_pvt), + &resp, sizeof(resp)); + break; + default: + BUG(); + } + blk_rings->common.rsp_prod_pvt++; + RING_PUSH_RESPONSES_AND_CHECK_NOTIFY(&blk_rings->common, notify); - if (blk_ring->rsp_prod_pvt == blk_ring->req_cons) { + if (blk_rings->common.rsp_prod_pvt == blk_rings->common.req_cons) { /* * Tail check for pending requests. Allows frontend to avoid * notifications if requests are already in flight (lower * overheads and promotes batching). */ - RING_FINAL_CHECK_FOR_REQUESTS(blk_ring, more_to_do); - } else if (RING_HAS_UNCONSUMED_REQUESTS(blk_ring)) { + RING_FINAL_CHECK_FOR_REQUESTS(&blk_rings->common, more_to_do); + } else if (RING_HAS_UNCONSUMED_REQUESTS(&blk_rings->common)) { more_to_do = 1; + } - } spin_unlock_irqrestore(&blkif->blk_ring_lock, flags); if (more_to_do) blkif_notify_work(blkif); Index: build-32-unstable-13553/linux-2.6-xen-sparse/drivers/xen/blktap/common.h =================================================================== --- build-32-unstable-13553.orig/linux-2.6-xen-sparse/drivers/xen/blktap/common.h +++ build-32-unstable-13553/linux-2.6-xen-sparse/drivers/xen/blktap/common.h @@ -39,8 +39,7 @@ #include <asm/pgalloc.h> #include <xen/evtchn.h> #include <asm/hypervisor.h> -#include <xen/interface/io/blkif.h> -#include <xen/interface/io/ring.h> +#include <xen/blkif.h> #include <xen/gnttab.h> #include <xen/driver_util.h> @@ -58,7 +57,8 @@ typedef struct blkif_st { /* Physical parameters of the comms window. */ unsigned int irq; /* Comms information. */ - blkif_back_ring_t blk_ring; + enum blkif_protocol blk_protocol; + blkif_back_rings_t blk_rings; struct vm_struct *blk_ring_area; /* Back pointer to the backend_info. */ struct backend_info *be; Index: build-32-unstable-13553/linux-2.6-xen-sparse/drivers/xen/blktap/interface.c =================================================================== --- build-32-unstable-13553.orig/linux-2.6-xen-sparse/drivers/xen/blktap/interface.c +++ build-32-unstable-13553/linux-2.6-xen-sparse/drivers/xen/blktap/interface.c @@ -96,7 +96,6 @@ static void unmap_frontend_page(blkif_t int tap_blkif_map(blkif_t *blkif, unsigned long shared_page, unsigned int evtchn) { - blkif_sring_t *sring; int err; /* Already connected through? */ @@ -112,8 +111,31 @@ int tap_blkif_map(blkif_t *blkif, unsign return err; } - sring = (blkif_sring_t *)blkif->blk_ring_area->addr; - BACK_RING_INIT(&blkif->blk_ring, sring, PAGE_SIZE); + switch (blkif->blk_protocol) { + case BLKIF_PROTOCOL_NATIVE: + { + blkif_sring_t *sring; + sring = (blkif_sring_t *)blkif->blk_ring_area->addr; + BACK_RING_INIT(&blkif->blk_rings.native, sring, PAGE_SIZE); + break; + } + case BLKIF_PROTOCOL_X86_32: + { + blkif_x86_32_sring_t *sring_x86_32; + sring_x86_32 = (blkif_x86_32_sring_t *)blkif->blk_ring_area->addr; + BACK_RING_INIT(&blkif->blk_rings.x86_32, sring_x86_32, PAGE_SIZE); + break; + } + case BLKIF_PROTOCOL_X86_64: + { + blkif_x86_64_sring_t *sring_x86_64; + sring_x86_64 = (blkif_x86_64_sring_t *)blkif->blk_ring_area->addr; + BACK_RING_INIT(&blkif->blk_rings.x86_64, sring_x86_64, PAGE_SIZE); + break; + } + default: + BUG(); + } err = bind_interdomain_evtchn_to_irqhandler( blkif->domid, evtchn, tap_blkif_be_int, @@ -134,10 +156,10 @@ void tap_blkif_unmap(blkif_t *blkif) unbind_from_irqhandler(blkif->irq, blkif); blkif->irq = 0; } - if (blkif->blk_ring.sring) { + if (blkif->blk_rings.common.sring) { unmap_frontend_page(blkif); free_vm_area(blkif->blk_ring_area); - blkif->blk_ring.sring = NULL; + blkif->blk_rings.common.sring = NULL; } } Index: build-32-unstable-13553/linux-2.6-xen-sparse/drivers/xen/blktap/xenbus.c =================================================================== --- build-32-unstable-13553.orig/linux-2.6-xen-sparse/drivers/xen/blktap/xenbus.c +++ build-32-unstable-13553/linux-2.6-xen-sparse/drivers/xen/blktap/xenbus.c @@ -340,6 +340,7 @@ static int connect_ring(struct backend_i struct xenbus_device *dev = be->dev; unsigned long ring_ref; unsigned int evtchn; + char protocol[64]; int err; DPRINTK("%s\n", dev->otherend); @@ -353,6 +354,24 @@ static int connect_ring(struct backend_i return err; } + be->blkif->blk_protocol = BLKIF_PROTOCOL_NATIVE; + err = xenbus_gather(XBT_NIL, dev->otherend, "protocol", + "%63s", protocol, NULL); + if (err) + strcpy(protocol, "unspecified, assuming native"); + else if (0 == strcmp(protocol, XEN_IO_PROTO_ABI_NATIVE)) + be->blkif->blk_protocol = BLKIF_PROTOCOL_NATIVE; + else if (0 == strcmp(protocol, XEN_IO_PROTO_ABI_X86_32)) + be->blkif->blk_protocol = BLKIF_PROTOCOL_X86_32; + else if (0 == strcmp(protocol, XEN_IO_PROTO_ABI_X86_64)) + be->blkif->blk_protocol = BLKIF_PROTOCOL_X86_64; + else { + xenbus_dev_fatal(dev, err, "unknown fe protocol %s", protocol); + return -1; + } + printk("blktap: ring-ref %ld, event-channel %d, protocol %d (%s)\n", + ring_ref, evtchn, be->blkif->blk_protocol, protocol); + /* Map the shared frame, irq etc. */ err = tap_blkif_map(be->blkif, ring_ref, evtchn); if (err) { Index: build-32-unstable-13553/linux-2.6-xen-sparse/include/xen/blkif.h =================================================================== --- /dev/null +++ build-32-unstable-13553/linux-2.6-xen-sparse/include/xen/blkif.h @@ -0,0 +1,97 @@ +#ifndef __XEN_BLKIF_H__ +#define __XEN_BLKIF_H__ + +#include <xen/interface/io/ring.h> +#include <xen/interface/io/blkif.h> +#include <xen/interface/io/protocols.h> + +/* Not a real protocol. Used to generate ring structs which contain + * the elements common to all protocols only. This way we get a + * compiler-checkable way to use common struct elements, so we can + * avoid using switch(protocol) in a number of places. */ +struct blkif_common_request { + char dummy; +}; +struct blkif_common_response { + char dummy; +}; + +/* i386 protocol version */ +#pragma pack(push, 4) +struct blkif_x86_32_request { + uint8_t operation; /* BLKIF_OP_??? */ + uint8_t nr_segments; /* number of segments */ + blkif_vdev_t handle; /* only for read/write requests */ + uint64_t id; /* private guest value, echoed in resp */ + blkif_sector_t sector_number;/* start sector idx on disk (r/w only) */ + struct blkif_request_segment seg[BLKIF_MAX_SEGMENTS_PER_REQUEST]; +}; +struct blkif_x86_32_response { + uint64_t id; /* copied from request */ + uint8_t operation; /* copied from request */ + int16_t status; /* BLKIF_RSP_??? */ +}; +typedef struct blkif_x86_32_request blkif_x86_32_request_t; +typedef struct blkif_x86_32_response blkif_x86_32_response_t; +#pragma pack(pop) + +/* x86_64 protocol version */ +struct blkif_x86_64_request { + uint8_t operation; /* BLKIF_OP_??? */ + uint8_t nr_segments; /* number of segments */ + blkif_vdev_t handle; /* only for read/write requests */ + uint64_t __attribute__((__aligned__(8))) id; + blkif_sector_t sector_number;/* start sector idx on disk (r/w only) */ + struct blkif_request_segment seg[BLKIF_MAX_SEGMENTS_PER_REQUEST]; +}; +struct blkif_x86_64_response { + uint64_t __attribute__((__aligned__(8))) id; + uint8_t operation; /* copied from request */ + int16_t status; /* BLKIF_RSP_??? */ +}; +typedef struct blkif_x86_64_request blkif_x86_64_request_t; +typedef struct blkif_x86_64_response blkif_x86_64_response_t; + +DEFINE_RING_TYPES(blkif_common, struct blkif_common_request, struct blkif_common_response); +DEFINE_RING_TYPES(blkif_x86_32, struct blkif_x86_32_request, struct blkif_x86_32_response); +DEFINE_RING_TYPES(blkif_x86_64, struct blkif_x86_64_request, struct blkif_x86_64_response); + +union blkif_back_rings { + blkif_back_ring_t native; + blkif_common_back_ring_t common; + blkif_x86_32_back_ring_t x86_32; + blkif_x86_64_back_ring_t x86_64; +}; +typedef union blkif_back_rings blkif_back_rings_t; + +enum blkif_protocol { + BLKIF_PROTOCOL_NATIVE = 1, + BLKIF_PROTOCOL_X86_32 = 2, + BLKIF_PROTOCOL_X86_64 = 3, +}; + +static void inline blkif_get_x86_32_req(blkif_request_t *dst, blkif_x86_32_request_t *src) +{ + int i; + dst->operation = src->operation; + dst->nr_segments = src->nr_segments; + dst->handle = src->handle; + dst->id = src->id; + dst->sector_number = src->sector_number; + for (i = 0; i < src->nr_segments; i++) + dst->seg[i] = src->seg[i]; +} + +static void inline blkif_get_x86_64_req(blkif_request_t *dst, blkif_x86_64_request_t *src) +{ + int i; + dst->operation = src->operation; + dst->nr_segments = src->nr_segments; + dst->handle = src->handle; + dst->id = src->id; + dst->sector_number = src->sector_number; + for (i = 0; i < src->nr_segments; i++) + dst->seg[i] = src->seg[i]; +} + +#endif /* __XEN_BLKIF_H__ */ [-- Attachment #3: Type: text/plain, Size: 138 bytes --] _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: 32-on-64: pvfb issue 2007-01-23 12:53 ` Gerd Hoffmann 2007-01-23 15:07 ` Keir Fraser @ 2007-01-24 11:23 ` Gerd Hoffmann 2007-01-24 12:02 ` Keir Fraser 2007-01-24 12:24 ` Markus Armbruster 2007-01-25 13:16 ` 32-on-64 broken in unstable Gerd Hoffmann 2 siblings, 2 replies; 50+ messages in thread From: Gerd Hoffmann @ 2007-01-24 11:23 UTC (permalink / raw) To: Gerd Hoffmann; +Cc: Xen devel list, Keir Fraser, Markus Armbruster [-- Attachment #1: Type: text/plain, Size: 1174 bytes --] Gerd Hoffmann wrote: > Keir Fraser wrote: >> On 22/1/07 14:01, "Gerd Hoffmann" <kraxel@suse.de> wrote: >> >>> Here we go. Compile-tested on 32bit, more tests coming, full rebuild >>> still in progress ... >> Yeah, I like these. They can go in as soon as you're happy with them. One >> exception is blkback -- I'm not keen on the v1/v2 thing as it is. I think we >> should continue to include public/io/blkif.h and use the struct definition >> there when protocol==XEN_IO_PROTO_ABI_NATIVE. > > Fixed. New versions attached, with comments added/updated and > signed-off-by. They are partly tested only though, 32-on-64 seems to be > broken in current unstable, mixing 32bit and 64bit domains doesn't work. What is the status of this? It's not in the public tree yet. Any problems? Or just stuck in the patch queue or regression testing? Meanwhile I've tested the block backend patches with 3.0.4 in a mixed environment -- works fine. The pvfb backend has a stupid tyops (missing underscore) which breaks the combination 32bit dom0 and 64bit domU. Fix attached. The patch also adds a cast to fix a warning. please apply, Gerd -- Gerd Hoffmann <kraxel@suse.de> [-- Attachment #2: fbback-bimodal-fix.diff --] [-- Type: text/x-patch, Size: 1322 bytes --] Index: build-32-release304-13133/tools/xenfb/xenfb.c =================================================================== --- build-32-release304-13133.orig/tools/xenfb/xenfb.c +++ build-32-release304-13133/tools/xenfb/xenfb.c @@ -367,12 +367,12 @@ static int xenfb_map_fb(struct xenfb_pri */ uint32_t *ptr32 = NULL; uint32_t *ptr64 = NULL; -#if defined(__i386_) - ptr32 = page->pd; +#if defined(__i386__) + ptr32 = (void*)page->pd; ptr64 = ((void*)page->pd) + 4; #elif defined(__x86_64__) ptr32 = ((void*)page->pd) - 4; - ptr64 = page->pd; + ptr64 = (void*)page->pd; #endif if (ptr32) { if (0 == ptr32[1]) { @@ -388,7 +388,7 @@ static int xenfb_map_fb(struct xenfb_pri /* 64bit dom0, 32bit domU */ mode = 32; pd = ((void*)page->pd) - 4; -#elif defined(__i386_) +#elif defined(__i386__) } else if (0 == strcmp(xenfb->protocol, XEN_IO_PROTO_ABI_X86_64)) { /* 32bit dom0, 64bit domU */ mode = 64; @@ -560,10 +560,10 @@ int xenfb_attach_dom(struct xenfb *xenfb if (xenfb_wait_for_frontend_initialised(&xenfb->kbd) < 0) goto error; - if (xenfb_bind(&xenfb->fb) < 0) - goto error; if (xenfb_bind(&xenfb->kbd) < 0) goto error; + if (xenfb_bind(&xenfb->fb) < 0) + goto error; if (xenfb_xs_scanf1(xsh, xenfb->fb.otherend, "feature-update", "%d", &val) < 0) [-- Attachment #3: Type: text/plain, Size: 138 bytes --] _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: 32-on-64: pvfb issue 2007-01-24 11:23 ` Gerd Hoffmann @ 2007-01-24 12:02 ` Keir Fraser 2007-01-24 12:24 ` Markus Armbruster 1 sibling, 0 replies; 50+ messages in thread From: Keir Fraser @ 2007-01-24 12:02 UTC (permalink / raw) To: Gerd Hoffmann; +Cc: Xen devel list, Markus Armbruster On 24/1/07 11:23, "Gerd Hoffmann" <kraxel@suse.de> wrote: > What is the status of this? It's not in the public tree yet. Any > problems? Or just stuck in the patch queue or regression testing? > Meanwhile I've tested the block backend patches with 3.0.4 in a mixed > environment -- works fine. All the patches are in the staging tree so will be in the public tree after regression tests have run. -- Keir ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: 32-on-64: pvfb issue 2007-01-24 11:23 ` Gerd Hoffmann 2007-01-24 12:02 ` Keir Fraser @ 2007-01-24 12:24 ` Markus Armbruster 2007-01-24 12:38 ` Gerd Hoffmann 1 sibling, 1 reply; 50+ messages in thread From: Markus Armbruster @ 2007-01-24 12:24 UTC (permalink / raw) To: Gerd Hoffmann; +Cc: Xen devel list, Keir Fraser Gerd Hoffmann <kraxel@suse.de> writes: [...] > The pvfb backend has a stupid tyops (missing underscore) which breaks > the combination 32bit dom0 and 64bit domU. Fix attached. The patch > also adds a cast to fix a warning. > > please apply, > > Gerd > > -- > Gerd Hoffmann <kraxel@suse.de> > Index: build-32-release304-13133/tools/xenfb/xenfb.c > =================================================================== > --- build-32-release304-13133.orig/tools/xenfb/xenfb.c > +++ build-32-release304-13133/tools/xenfb/xenfb.c [...] > @@ -560,10 +560,10 @@ int xenfb_attach_dom(struct xenfb *xenfb > if (xenfb_wait_for_frontend_initialised(&xenfb->kbd) < 0) > goto error; > > - if (xenfb_bind(&xenfb->fb) < 0) > - goto error; > if (xenfb_bind(&xenfb->kbd) < 0) > goto error; > + if (xenfb_bind(&xenfb->fb) < 0) > + goto error; > > if (xenfb_xs_scanf1(xsh, xenfb->fb.otherend, "feature-update", > "%d", &val) < 0) Why is this patch hunk necessary? ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: 32-on-64: pvfb issue 2007-01-24 12:24 ` Markus Armbruster @ 2007-01-24 12:38 ` Gerd Hoffmann 2007-01-24 14:24 ` Markus Armbruster 0 siblings, 1 reply; 50+ messages in thread From: Gerd Hoffmann @ 2007-01-24 12:38 UTC (permalink / raw) To: Markus Armbruster; +Cc: Xen devel list, Keir Fraser Hi, >> @@ -560,10 +560,10 @@ int xenfb_attach_dom(struct xenfb *xenfb >> if (xenfb_wait_for_frontend_initialised(&xenfb->kbd) < 0) >> goto error; >> >> - if (xenfb_bind(&xenfb->fb) < 0) >> - goto error; >> if (xenfb_bind(&xenfb->kbd) < 0) >> goto error; >> + if (xenfb_bind(&xenfb->fb) < 0) >> + goto error; >> >> if (xenfb_xs_scanf1(xsh, xenfb->fb.otherend, "feature-update", >> "%d", &val) < 0) > > Why is this patch hunk necessary? Oh, forgot to mention that, sorry. Only vfb has a "protocol" node, vkbd hasn't. So binding fb last makes sure we have the protocol filled correctly in the struct. cheers, Gerd -- Gerd Hoffmann <kraxel@suse.de> ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: 32-on-64: pvfb issue 2007-01-24 12:38 ` Gerd Hoffmann @ 2007-01-24 14:24 ` Markus Armbruster 2007-01-24 15:25 ` Gerd Hoffmann 0 siblings, 1 reply; 50+ messages in thread From: Markus Armbruster @ 2007-01-24 14:24 UTC (permalink / raw) To: Gerd Hoffmann; +Cc: Xen devel list, Keir Fraser Gerd Hoffmann <kraxel@suse.de> writes: > Hi, > >>> @@ -560,10 +560,10 @@ int xenfb_attach_dom(struct xenfb *xenfb >>> if (xenfb_wait_for_frontend_initialised(&xenfb->kbd) < 0) >>> goto error; >>> >>> - if (xenfb_bind(&xenfb->fb) < 0) >>> - goto error; >>> if (xenfb_bind(&xenfb->kbd) < 0) >>> goto error; >>> + if (xenfb_bind(&xenfb->fb) < 0) >>> + goto error; >>> >>> if (xenfb_xs_scanf1(xsh, xenfb->fb.otherend, "feature-update", >>> "%d", &val) < 0) >> >> Why is this patch hunk necessary? > > Oh, forgot to mention that, sorry. Only vfb has a "protocol" node, vkbd > hasn't. So binding fb last makes sure we have the protocol filled > correctly in the struct. > > cheers, > Gerd Unclean! You put protocol[] into xenfb_private, which means it's shared between vfb and vkbd. That's defensible. However, you really shouldn't read it in xenfb_bind() then. That reads it both from vfb (where it is valid) and vkbd (where it is currently undefined), and the one read last wins. Reading it next to reading feature-update would be much cleaner. Alternatively, put protocol[] into xenfb_device. If you insist on keeping it the way it is, you really need a comment. But cleaning it up should be less work than explaining it. ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: 32-on-64: pvfb issue 2007-01-24 14:24 ` Markus Armbruster @ 2007-01-24 15:25 ` Gerd Hoffmann 0 siblings, 0 replies; 50+ messages in thread From: Gerd Hoffmann @ 2007-01-24 15:25 UTC (permalink / raw) To: Markus Armbruster; +Cc: Xen devel list, Keir Fraser [-- Attachment #1: Type: text/plain, Size: 150 bytes --] > Reading it next to reading feature-update would be much cleaner. Done, updated patch attached. cheers, Gerd -- Gerd Hoffmann <kraxel@suse.de> [-- Attachment #2: fbback-bimodal-fix.diff --] [-- Type: text/x-patch, Size: 1766 bytes --] --- tools/xenfb/xenfb.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) Index: build-32-unstable-13568/tools/xenfb/xenfb.c =================================================================== --- build-32-unstable-13568.orig/tools/xenfb/xenfb.c +++ build-32-unstable-13568/tools/xenfb/xenfb.c @@ -362,12 +362,12 @@ static int xenfb_map_fb(struct xenfb_pri */ uint32_t *ptr32 = NULL; uint32_t *ptr64 = NULL; -#if defined(__i386_) - ptr32 = page->pd; +#if defined(__i386__) + ptr32 = (void*)page->pd; ptr64 = ((void*)page->pd) + 4; #elif defined(__x86_64__) ptr32 = ((void*)page->pd) - 4; - ptr64 = page->pd; + ptr64 = (void*)page->pd; #endif if (ptr32) { if (0 == ptr32[1]) { @@ -383,7 +383,7 @@ static int xenfb_map_fb(struct xenfb_pri /* 64bit dom0, 32bit domU */ mode = 32; pd = ((void*)page->pd) - 4; -#elif defined(__i386_) +#elif defined(__i386__) } else if (0 == strcmp(xenfb->protocol, XEN_IO_PROTO_ABI_X86_64)) { /* 32bit dom0, 64bit domU */ mode = 64; @@ -441,9 +441,6 @@ static int xenfb_bind(struct xenfb_devic if (xenfb_xs_scanf1(xenfb->xsh, dev->otherend, "event-channel", "%u", &evtchn) < 0) return -1; - if (xenfb_xs_scanf1(xenfb->xsh, dev->otherend, "protocol", "%63s", - xenfb->protocol) < 0) - xenfb->protocol[0] = '\0'; dev->port = xc_evtchn_bind_interdomain(xenfb->evt_xch, dev->otherend_id, evtchn); @@ -567,6 +564,9 @@ int xenfb_attach_dom(struct xenfb *xenfb errno = ENOTSUP; goto error; } + if (xenfb_xs_scanf1(xsh, xenfb->fb.otherend, "protocol", "%63s", + xenfb->protocol) < 0) + xenfb->protocol[0] = '\0'; xenfb_xs_printf(xsh, xenfb->fb.nodename, "request-update", "1"); /* TODO check for permitted ranges */ [-- Attachment #3: Type: text/plain, Size: 138 bytes --] _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel ^ permalink raw reply [flat|nested] 50+ messages in thread
* 32-on-64 broken in unstable. 2007-01-23 12:53 ` Gerd Hoffmann 2007-01-23 15:07 ` Keir Fraser 2007-01-24 11:23 ` Gerd Hoffmann @ 2007-01-25 13:16 ` Gerd Hoffmann 2007-01-25 13:25 ` Keir Fraser 2 siblings, 1 reply; 50+ messages in thread From: Gerd Hoffmann @ 2007-01-25 13:16 UTC (permalink / raw) To: Gerd Hoffmann; +Cc: Xen devel list, Keir Fraser Hi, > [ bimodal driver patches ] > They are partly tested only though, 32-on-64 seems to be > broken in current unstable, mixing 32bit and 64bit domains doesn't work. bisected that one, changeset 13522 is the culpit which breaks booting 32pae guests on 64bit xen (and dom0). Ideas anyone? cheers, Gerd -- Gerd Hoffmann <kraxel@suse.de> ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: 32-on-64 broken in unstable. 2007-01-25 13:16 ` 32-on-64 broken in unstable Gerd Hoffmann @ 2007-01-25 13:25 ` Keir Fraser 2007-01-25 13:34 ` Gerd Hoffmann 0 siblings, 1 reply; 50+ messages in thread From: Keir Fraser @ 2007-01-25 13:25 UTC (permalink / raw) To: Gerd Hoffmann; +Cc: Xen devel list, Keir Fraser On 25/1/07 13:16, "Gerd Hoffmann" <kraxel@suse.de> wrote: > bisected that one, changeset 13522 is the culpit which breaks booting > 32pae guests on 64bit xen (and dom0). > > Ideas anyone? The hook added to set_info_guest() to call vcpu_reset() is probably the most likely culprit. However this code is churned some more by c/s 13594 which is currently sitting in our staging tree (the Linux upgrade has caused some of our regression tests to start failing). If necessary we'll do a manual push to the public tree later today. -- Keir ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: 32-on-64 broken in unstable. 2007-01-25 13:25 ` Keir Fraser @ 2007-01-25 13:34 ` Gerd Hoffmann 0 siblings, 0 replies; 50+ messages in thread From: Gerd Hoffmann @ 2007-01-25 13:34 UTC (permalink / raw) To: Keir Fraser; +Cc: Xen devel list Keir Fraser wrote: > On 25/1/07 13:16, "Gerd Hoffmann" <kraxel@suse.de> wrote: > >> bisected that one, changeset 13522 is the culpit which breaks booting >> 32pae guests on 64bit xen (and dom0). >> >> Ideas anyone? > > The hook added to set_info_guest() to call vcpu_reset() is probably the most > likely culprit. It is. ifdef'ing out these three lines makes it work again. thanks, Gerd -- Gerd Hoffmann <kraxel@suse.de> ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: 32-on-64: pvfb issue 2007-01-22 14:01 ` Gerd Hoffmann 2007-01-22 14:48 ` Keir Fraser @ 2007-01-22 15:22 ` Markus Armbruster 2007-01-22 15:33 ` Gerd Hoffmann 1 sibling, 1 reply; 50+ messages in thread From: Markus Armbruster @ 2007-01-22 15:22 UTC (permalink / raw) To: Gerd Hoffmann; +Cc: Xen devel list, Keir Fraser Gerd Hoffmann <kraxel@suse.de> writes: > Gerd Hoffmann wrote: > >> I'll go code up both front and back bits for block and pvfb to see how >> it works out in practice, I think I'll have patches later today or >> tomorrow ... > > Here we go. Compile-tested on 32bit, more tests coming, full rebuild > still in progress ... > > Attached are: > > protocol-bimodal.diff > short header file with the protocol names. > > {blk,fb}front-bimodal.diff > small frontend driver patches, just add the protocol name to xenstore. > > {blk,fb}back-bimodal.diff > backend patches (for unstable only). > > cheers, > Gerd > > -- > Gerd Hoffmann <kraxel@suse.de> > --- > xen/include/public/io/protocols.h | 21 +++++++++++++++++++++ > 1 file changed, 21 insertions(+) > > Index: build-32-unstable-13495/xen/include/public/io/protocols.h > =================================================================== > --- /dev/null > +++ build-32-unstable-13495/xen/include/public/io/protocols.h > @@ -0,0 +1,21 @@ > +#ifndef __XEN_PROTOCOLS_H__ > +#define __XEN_PROTOCOLS_H__ > + > +#define XEN_IO_PROTO_ABI_X86_32 "x86_32-abi" > +#define XEN_IO_PROTO_ABI_X86_64 "x86_64-abi" > +#define XEN_IO_PROTO_ABI_IA64 "ia64-abi" > +#define XEN_IO_PROTO_ABI_POWERPC64 "powerpc64-abi" > + > +#if defined(__i386__) > +# define XEN_IO_PROTO_ABI_NATIVE XEN_IO_PROTO_ABI_X86_32 > +#elif defined(__x86_64__) > +# define XEN_IO_PROTO_ABI_NATIVE XEN_IO_PROTO_ABI_X86_64 > +#elif defined(__ia64__) > +# define XEN_IO_PROTO_ABI_NATIVE XEN_IO_PROTO_ABI_IA64 > +#elif defined(__powerpc64__) > +# define XEN_IO_PROTO_ABI_NATIVE XEN_IO_PROTO_ABI_POWERPC64 > +#else > +# error arch fixup needed here > +#endif > + > +#endif > --- > linux-2.6-xen-sparse/drivers/xen/fbfront/xenfb.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > Index: build-32-unstable-13495/linux-2.6-xen-sparse/drivers/xen/fbfront/xenfb.c > =================================================================== > --- build-32-unstable-13495.orig/linux-2.6-xen-sparse/drivers/xen/fbfront/xenfb.c > +++ build-32-unstable-13495/linux-2.6-xen-sparse/drivers/xen/fbfront/xenfb.c > @@ -27,6 +27,7 @@ > #include <asm/hypervisor.h> > #include <xen/evtchn.h> > #include <xen/interface/io/fbif.h> > +#include <xen/interface/io/protocols.h> > #include <xen/xenbus.h> > #include <linux/kthread.h> > > @@ -479,7 +480,7 @@ static int __devinit xenfb_probe(struct > goto error_nomem; > > /* set up shared page */ > - info->page = (void *)__get_free_page(GFP_KERNEL); > + info->page = (void *)__get_free_page(GFP_KERNEL | __GFP_ZERO); > if (!info->page) > goto error_nomem; > > @@ -640,6 +641,10 @@ static int xenfb_connect_backend(struct > irq_to_evtchn_port(info->irq)); > if (ret) > goto error_xenbus; > + ret = xenbus_printf(xbt, dev->nodename, "protocol", "%s", > + XEN_IO_PROTO_ABI_NATIVE); > + if (ret) > + goto error_xenbus; This identifies the ABI, but how does it provide versioning? > ret = xenbus_printf(xbt, dev->nodename, "feature-update", "1"); > if (ret) > goto error_xenbus; > --- > linux-2.6-xen-sparse/drivers/xen/blkfront/blkfront.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > Index: build-32-unstable-13534/linux-2.6-xen-sparse/drivers/xen/blkfront/blkfront.c > =================================================================== > --- build-32-unstable-13534.orig/linux-2.6-xen-sparse/drivers/xen/blkfront/blkfront.c > +++ build-32-unstable-13534/linux-2.6-xen-sparse/drivers/xen/blkfront/blkfront.c > @@ -44,6 +44,7 @@ > #include <xen/evtchn.h> > #include <xen/xenbus.h> > #include <xen/interface/grant_table.h> > +#include <xen/interface/io/protocols.h> > #include <xen/gnttab.h> > #include <asm/hypervisor.h> > #include <asm/maddr.h> > @@ -180,6 +181,12 @@ again: > message = "writing event-channel"; > goto abort_transaction; > } > + err = xenbus_printf(xbt, dev->nodename, "protocol", "%s", > + XEN_IO_PROTO_ABI_NATIVE); > + if (err) { > + message = "writing protocol"; > + goto abort_transaction; > + } > > err = xenbus_transaction_end(xbt, 0); > if (err) { > --- > tools/xenfb/xenfb.c | 103 ++++++++++++++++++++++++++++++++++++++++++++++------ > 1 file changed, 92 insertions(+), 11 deletions(-) > > Index: build-32-unstable-13495/tools/xenfb/xenfb.c > =================================================================== > --- build-32-unstable-13495.orig/tools/xenfb/xenfb.c > +++ build-32-unstable-13495/tools/xenfb/xenfb.c > @@ -7,6 +7,7 @@ > #include <xen/io/xenbus.h> > #include <xen/io/fbif.h> > #include <xen/io/kbdif.h> > +#include <xen/io/protocols.h> > #include <sys/select.h> > #include <stdbool.h> > #include <xen/linux/evtchn.h> > @@ -40,6 +41,7 @@ struct xenfb_private { > struct xs_handle *xsh; /* xs daemon handle */ > struct xenfb_device fb, kbd; > size_t fb_len; /* size of framebuffer */ > + char protocol[64]; /* frontend protocol */ > }; > > static void xenfb_detach_dom(struct xenfb_private *); > @@ -324,36 +326,112 @@ static int xenfb_wait_for_frontend_initi > return 0; > } > > +static void xenfb_copy_mfns(int mode, int count, unsigned long *dst, void *src) Nitpick: the argument order dst, src, count got burned into my synapses; other orders tend to confuse me. > +{ > + uint32_t *src32 = src; > + uint64_t *src64 = src; > + int i; > + > + if (32 == mode) { > + for (i = 0; i < count; i++) > + dst[i] = src32[i]; > + } else { > + for (i = 0; i < count; i++) > + dst[i] = src64[i]; > + } > +} > + > static int xenfb_map_fb(struct xenfb_private *xenfb, int domid) > { > struct xenfb_page *page = xenfb->fb.page; > int n_fbmfns; > int n_fbdirs; > - unsigned long *fbmfns; > + unsigned long *pgmfns = NULL; > + unsigned long *fbmfns = NULL; > + void *map, *pd; > + int mode, ret = -1; > + > + /* default to native */ > + pd = page->pd; > + mode = sizeof(unsigned long) * 8; Nitick: encoding modes as 4, 8 and so forth rather than 32, 64, ... would be slightly simpler. > + > + if (0 == strlen(xenfb->protocol)) { > + /* > + * Undefined protocol, some guesswork needed. Guesswork is only required if we want to support old domU with different width than dom0. Do we? > + * > + * Old frontends which don't set the protocol use > + * one page directory only, thus pd[1] must be zero. > + * pd[1] of the 32bit struct layout and the lower > + * 32 bits of pd[0] of the 64bit struct layout have > + * the same location, so we can check that ... > + */ > + uint32_t *ptr32 = NULL; > + uint32_t *ptr64 = NULL; > +#if defined(__i386_) > + ptr32 = page->pd; > + ptr64 = ((void*)page->pd) + 4; > +#elif defined(__x86_64__) > + ptr32 = ((void*)page->pd) - 4; > + ptr64 = page->pd; > +#endif > + if (ptr32) { > + if (0 == ptr32[1]) { > + mode = 32; > + pd = ptr32; > + } else { > + mode = 64; > + pd = ptr64; > + } This clever hack tests those 32 pg bits that are guaranteed to be initialized for both modes. In other words, it uses all the information there is. In 32-bit mode, ptr32[1] is pd[1], and that is certainly zero for old frontends. In 64-bit mode, ptr32[1] is pd[0] & 0xffffffff. Non-zero unless we get very unlucky with the address of mfns. Why can't that happen? > + } > +#if defined(__x86_64__) > + } else if (0 == strcmp(xenfb->protocol, XEN_IO_PROTO_ABI_X86_32)) { > + /* 64bit dom0, 32bit domU */ > + mode = 32; > + pd = ((void*)page->pd) - 4; > +#elif defined(__i386_) > + } else if (0 == strcmp(xenfb->protocol, XEN_IO_PROTO_ABI_X86_64)) { > + /* 32bit dom0, 64bit domU */ > + mode = 64; > + pd = ((void*)page->pd) + 4; > +#endif Hackery so that we can keep the old page layout. Could perhaps be done in a cleaner way, but I don't care. > + } > > n_fbmfns = (xenfb->fb_len + (XC_PAGE_SIZE - 1)) / XC_PAGE_SIZE; > - n_fbdirs = n_fbmfns * sizeof(unsigned long); > + n_fbdirs = n_fbmfns * mode / 8; > n_fbdirs = (n_fbdirs + (XC_PAGE_SIZE - 1)) / XC_PAGE_SIZE; > > + pgmfns = malloc(sizeof(unsigned long) * n_fbdirs); > + fbmfns = malloc(sizeof(unsigned long) * n_fbmfns); > + if (!pgmfns || !fbmfns) > + goto out; > + > /* > * Bug alert: xc_map_foreign_batch() can fail partly and > * return a non-null value. This is a design flaw. When it > * happens, we happily continue here, and later crash on > * access. > */ > - fbmfns = xc_map_foreign_batch(xenfb->xc, domid, > - PROT_READ, page->pd, n_fbdirs); > - if (fbmfns == NULL) > - return -1; > + xenfb_copy_mfns(mode, n_fbdirs, pgmfns, pd); > + map = xc_map_foreign_batch(xenfb->xc, domid, > + PROT_READ, pgmfns, n_fbdirs); > + if (map == NULL) > + goto out; > + xenfb_copy_mfns(mode, n_fbmfns, fbmfns, map); > + munmap(map, n_fbdirs * XC_PAGE_SIZE); > > xenfb->pub.pixels = xc_map_foreign_batch(xenfb->xc, domid, > PROT_READ | PROT_WRITE, fbmfns, n_fbmfns); > - if (xenfb->pub.pixels == NULL) { > - munmap(fbmfns, n_fbdirs * XC_PAGE_SIZE); > - return -1; > - } > + if (xenfb->pub.pixels == NULL) > + goto out; > > - return munmap(fbmfns, n_fbdirs * XC_PAGE_SIZE); > + ret = 0; /* all is fine */ > + > + out: > + if (pgmfns) > + free(pgmfns); > + if (fbmfns) > + free(fbmfns); > + return ret; > } > > static int xenfb_bind(struct xenfb_device *dev) > @@ -368,6 +446,9 @@ static int xenfb_bind(struct xenfb_devic > if (xenfb_xs_scanf1(xenfb->xsh, dev->otherend, "event-channel", "%u", > &evtchn) < 0) > return -1; > + if (xenfb_xs_scanf1(xenfb->xsh, dev->otherend, "protocol", "%63s", > + xenfb->protocol) < 0) > + xenfb->protocol[0] = '\0'; > > dev->port = xc_evtchn_bind_interdomain(xenfb->evt_xch, > dev->otherend_id, evtchn); [...] ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: 32-on-64: pvfb issue 2007-01-22 15:22 ` 32-on-64: pvfb issue Markus Armbruster @ 2007-01-22 15:33 ` Gerd Hoffmann 2007-01-22 15:40 ` Keir Fraser 0 siblings, 1 reply; 50+ messages in thread From: Gerd Hoffmann @ 2007-01-22 15:33 UTC (permalink / raw) To: Markus Armbruster; +Cc: Xen devel list, Keir Fraser Markus Armbruster wrote: > Gerd Hoffmann <kraxel@suse.de> writes: > >> + ret = xenbus_printf(xbt, dev->nodename, "protocol", "%s", >> + XEN_IO_PROTO_ABI_NATIVE); >> + if (ret) >> + goto error_xenbus; > > This identifies the ABI, but how does it provide versioning? #define XEN_IO_PROTO_FB_GRANT_TABLES "fb-grant-tables" ? Just create a descriptive name you like. Assuming we don't create abi-dependent structs *again* when we change the protocol. We can of course also just use XEN_IO_PROTO_ABI_NATIVE "-v2". With the protocol identifier being a string there lots of ways ... cheers, Gerd -- Gerd Hoffmann <kraxel@suse.de> ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: 32-on-64: pvfb issue 2007-01-22 15:33 ` Gerd Hoffmann @ 2007-01-22 15:40 ` Keir Fraser 0 siblings, 0 replies; 50+ messages in thread From: Keir Fraser @ 2007-01-22 15:40 UTC (permalink / raw) To: Gerd Hoffmann, Markus Armbruster; +Cc: Xen devel list On 22/1/07 15:33, "Gerd Hoffmann" <kraxel@suse.de> wrote: >> >> This identifies the ABI, but how does it provide versioning? > > #define XEN_IO_PROTO_FB_GRANT_TABLES "fb-grant-tables" ? > > Just create a descriptive name you like. Assuming we don't create > abi-dependent structs *again* when we change the protocol. > > We can of course also just use XEN_IO_PROTO_ABI_NATIVE "-v2". With the > protocol identifier being a string there lots of ways ... I'd suggest we add a suffix after the ABI part of the protocol name unless we're really after full portability across any arbitrary architectures (and so deal with e.g. endianness issues too). Whatever, clearly this approach has the flexibility to be extended in future without adding all that much code. -- Keir ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: 32-on-64: pvfb issue 2007-01-19 15:31 ` Gerd Hoffmann 2007-01-19 16:05 ` Keir Fraser @ 2007-01-19 16:06 ` Markus Armbruster 2007-01-22 7:56 ` Gerd Hoffmann 1 sibling, 1 reply; 50+ messages in thread From: Markus Armbruster @ 2007-01-19 16:06 UTC (permalink / raw) To: Gerd Hoffmann; +Cc: Xen devel list, Keir Fraser I really don't understand why we need this level of generality and complexity, in particular when a simple hypercall to query a domain's width would do. Or a simple, stupid version number in the shared page. We'll hardly end up with an unmanageable number of versions. ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: 32-on-64: pvfb issue 2007-01-19 16:06 ` Markus Armbruster @ 2007-01-22 7:56 ` Gerd Hoffmann 0 siblings, 0 replies; 50+ messages in thread From: Gerd Hoffmann @ 2007-01-22 7:56 UTC (permalink / raw) To: Markus Armbruster; +Cc: Xen devel list, Keir Fraser Markus Armbruster wrote: > I really don't understand why we need this level of generality and > complexity, in particular when a simple hypercall to query a domain's > width would do. Or a simple, stupid version number in the shared > page. We'll hardly end up with an unmanageable number of versions. A simple hypercall will *not* do for paravirtualized drivers in fully virtualized domains. cheers, Gerd -- Gerd Hoffmann <kraxel@suse.de> ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: 32-on-64: pvfb issue 2007-01-18 18:31 ` Keir Fraser 2007-01-19 9:46 ` Gerd Hoffmann 2007-01-19 9:54 ` Gerd Hoffmann @ 2007-01-19 10:43 ` Ian Campbell 2007-01-19 12:03 ` Markus Armbruster 2007-01-22 18:32 ` Does vt-x itself have perf. impact on Hypervisor w/o considering HVM? Liang Yang 3 siblings, 1 reply; 50+ messages in thread From: Ian Campbell @ 2007-01-19 10:43 UTC (permalink / raw) To: Keir Fraser; +Cc: Gerd Hoffmann, Xen devel list, Markus Armbruster On Thu, 2007-01-18 at 18:31 +0000, Keir Fraser wrote: > This is even better than with blkfront/blkback because there are definitely > no PV-on-HVM pvfb driver frontends out in the wild (and it's HVM that makes > things harder). PV on HVM for fbfront is pretty hard since the kernel.org tree does not export zap_page_range to modules, neither do many of the older distro kernels I've looked at. I tried doing a compat version like with other such functions but it ended up pulling half of mm/memory.c into the compat layer which I don't like. Do I vaguely recall plans to get rid of the use of zap_page_range or is my memory playing tricks? Ian. ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: 32-on-64: pvfb issue 2007-01-19 10:43 ` Ian Campbell @ 2007-01-19 12:03 ` Markus Armbruster 0 siblings, 0 replies; 50+ messages in thread From: Markus Armbruster @ 2007-01-19 12:03 UTC (permalink / raw) To: Ian Campbell; +Cc: Gerd Hoffmann, Xen devel list, Keir Fraser Ian Campbell <Ian.Campbell@xensource.com> writes: > On Thu, 2007-01-18 at 18:31 +0000, Keir Fraser wrote: >> This is even better than with blkfront/blkback because there are definitely >> no PV-on-HVM pvfb driver frontends out in the wild (and it's HVM that makes >> things harder). > > PV on HVM for fbfront is pretty hard since the kernel.org tree does not > export zap_page_range to modules, neither do many of the older distro > kernels I've looked at. > > I tried doing a compat version like with other such functions but it > ended up pulling half of mm/memory.c into the compat layer which I don't > like. > > Do I vaguely recall plans to get rid of the use of zap_page_range or is > my memory playing tricks? > > Ian. We'd like to replace zap_page_range() with unmap_mapping_range(), for several good reasons: * it is already fully exported, * it deals with locking automatically via the address_space i_mmap_lock spinlock, * it automatically iterates over all the vmas on the address_space without us having to loop over them ourselves. ^ permalink raw reply [flat|nested] 50+ messages in thread
* Does vt-x itself have perf. impact on Hypervisor w/o considering HVM? 2007-01-18 18:31 ` Keir Fraser ` (2 preceding siblings ...) 2007-01-19 10:43 ` Ian Campbell @ 2007-01-22 18:32 ` Liang Yang 2007-01-23 10:05 ` [Xen-users] " Petersson, Mats 3 siblings, 1 reply; 50+ messages in thread From: Liang Yang @ 2007-01-22 18:32 UTC (permalink / raw) To: Xen devel list, xen-users Hello, Suppose I have two different kinds of CPUs which have exactly the same configuration except one supports VT-X while the other does not. If I want to test the I/O performance (or other perf. testing which is not particularly related to I/O) of the both domain0 and Para-Virtualized Guest Domain (HVM domain is not considered), shall I expect to get the same performance results on these two CPUs? Thanks, Liang ^ permalink raw reply [flat|nested] 50+ messages in thread
* RE: [Xen-users] Does vt-x itself have perf. impact on Hypervisor w/o considering HVM? 2007-01-22 18:32 ` Does vt-x itself have perf. impact on Hypervisor w/o considering HVM? Liang Yang @ 2007-01-23 10:05 ` Petersson, Mats 2007-01-23 16:15 ` Liang Yang 0 siblings, 1 reply; 50+ messages in thread From: Petersson, Mats @ 2007-01-23 10:05 UTC (permalink / raw) To: Liang Yang, Xen devel list, xen-users > -----Original Message----- > From: xen-users-bounces@lists.xensource.com > [mailto:xen-users-bounces@lists.xensource.com] On Behalf Of Liang Yang > Sent: 22 January 2007 18:33 > To: Xen devel list; xen-users@lists.xensource.com > Subject: [Xen-users] Does vt-x itself have perf. impact on > Hypervisor w/o considering HVM? > > Hello, > > Suppose I have two different kinds of CPUs which have exactly > the same > configuration except one supports VT-X while the other does > not. If I want > to test the I/O performance (or other perf. testing which is not > particularly related to I/O) of the both domain0 and > Para-Virtualized Guest > Domain (HVM domain is not considered), shall I expect to get the same > performance results on these two CPUs? Assuming ALL other aspects are the same, when you're not using HVM, there should be absolutely zero impact from it (aside from it using up a few kilobytes of memory, to be precise, HVM (including both VMX and SVM) takes up 129459 bytes when not used - more memory is allocated dynamically when it's being used for obvious reasons. For modern systems, that's so small that it doesn't matter). -- Mats > > Thanks, > > Liang > > > _______________________________________________ > Xen-users mailing list > Xen-users@lists.xensource.com > http://lists.xensource.com/xen-users > > > ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [Xen-users] Does vt-x itself have perf. impact on Hypervisor w/o considering HVM? 2007-01-23 10:05 ` [Xen-users] " Petersson, Mats @ 2007-01-23 16:15 ` Liang Yang 2007-01-23 16:33 ` Petersson, Mats 0 siblings, 1 reply; 50+ messages in thread From: Liang Yang @ 2007-01-23 16:15 UTC (permalink / raw) To: Petersson, Mats, Xen devel list, xen-users Without VT-x support, binary translation has to be used to make those non-virtualizable instructions throw exception. With VT-x support, no binary translation is needed. So you mean, binary translation could be implemented as efficient as they are done in hardware? Thanks, Liang ----- Original Message ----- From: "Petersson, Mats" <Mats.Petersson@amd.com> To: "Liang Yang" <multisyncfe991@hotmail.com>; "Xen devel list" <xen-devel@lists.xensource.com>; <xen-users@lists.xensource.com> Sent: Tuesday, January 23, 2007 3:05 AM Subject: RE: [Xen-users] Does vt-x itself have perf. impact on Hypervisor w/o considering HVM? > -----Original Message----- > From: xen-users-bounces@lists.xensource.com > [mailto:xen-users-bounces@lists.xensource.com] On Behalf Of Liang Yang > Sent: 22 January 2007 18:33 > To: Xen devel list; xen-users@lists.xensource.com > Subject: [Xen-users] Does vt-x itself have perf. impact on > Hypervisor w/o considering HVM? > > Hello, > > Suppose I have two different kinds of CPUs which have exactly > the same > configuration except one supports VT-X while the other does > not. If I want > to test the I/O performance (or other perf. testing which is not > particularly related to I/O) of the both domain0 and > Para-Virtualized Guest > Domain (HVM domain is not considered), shall I expect to get the same > performance results on these two CPUs? Assuming ALL other aspects are the same, when you're not using HVM, there should be absolutely zero impact from it (aside from it using up a few kilobytes of memory, to be precise, HVM (including both VMX and SVM) takes up 129459 bytes when not used - more memory is allocated dynamically when it's being used for obvious reasons. For modern systems, that's so small that it doesn't matter). -- Mats > > Thanks, > > Liang > > > _______________________________________________ > Xen-users mailing list > Xen-users@lists.xensource.com > http://lists.xensource.com/xen-users > > > ^ permalink raw reply [flat|nested] 50+ messages in thread
* RE: [Xen-users] Does vt-x itself have perf. impact on Hypervisor w/o considering HVM? 2007-01-23 16:15 ` Liang Yang @ 2007-01-23 16:33 ` Petersson, Mats 0 siblings, 0 replies; 50+ messages in thread From: Petersson, Mats @ 2007-01-23 16:33 UTC (permalink / raw) To: Liang Yang, Xen devel list, xen-users > -----Original Message----- > From: Liang Yang [mailto:multisyncfe991@hotmail.com] > Sent: 23 January 2007 16:15 > To: Petersson, Mats; Xen devel list; xen-users@lists.xensource.com > Subject: Re: [Xen-users] Does vt-x itself have perf. impact > on Hypervisor w/o considering HVM? > > Without VT-x support, binary translation has to be used to make those > non-virtualizable instructions throw exception. With VT-x > support, no binary > translation is needed. So you mean, binary translation could > be implemented > as efficient as they are done in hardware? Not at all - I'm saying that if you're not using VT-x, then VT-x has no performance impact to your tests, which is what you originally asked. Or at least, that's how I interpreted: "both domain0 and Para-Virtualized Guest Domain (HVM domain is not considered)" in your original post. Xen doesn't use binary translation, it uses para-virtualization, which is another way to say "source-code of the operating systme being modified to support virtualization". Binary translation or hardware support is needed if you're using an operating system where it is unpractical to do para-virtualization (for example, source-code is not publicly available (say Windows), or the user-base isn't big enough to sustain a para-virtualization effort (say Linux kernel 2.4.x)). Whether binary translation or hardware is more efficient will depend on: 1. Implementation of the binary translation and hardware. 2. What application and OS is being run on the system. It is by no means sure that Binary translation is either faster or slower than hardware implementations of virtualization. There are too many other factors that affect the situation to say for sure - one case may be faster for binary translation (because the type of operations suit binary translation), whilst another is slower. For most common cases, Para-virtualization should be able to be overall faster than both, because there are possibilities to bunch together several operations that cause a hypervisor interaction, and those can be "pacakged together" rahter than each event needing it's own separate hypervisor interaction. Of course, REALLLY clever binary translation may be able to detect a loop writing to the page-table and translate that into a block-call of "translate these 100 page-writes", rather than "for(...) do { translate one page write }" as the obvious solution would be. In hardware, there is no choice but to intercept each and every operation on it's own, so it's most likely the slowest operation, unless there's some way to avoid the hypervisor being called overall (there are some such cases too, and again, it depends on the exact circumstances of the application + OS that is run as a guest). There's also more possibilities to add things to the hardware to support future enhancements of the hardware, which just plain isn't going to happen in the software solution. One such feature is nested paging, which allows the guest-page-table operations to avoid being intercepted (instead of an intercept and a whole bunch of code to perform the page-table write, the processor does a second page-table access of the "host-page-table" which translates the guest physical address into machine physical address). -- Mats > > Thanks, > > Liang > > ----- Original Message ----- > From: "Petersson, Mats" <Mats.Petersson@amd.com> > To: "Liang Yang" <multisyncfe991@hotmail.com>; "Xen devel list" > <xen-devel@lists.xensource.com>; <xen-users@lists.xensource.com> > Sent: Tuesday, January 23, 2007 3:05 AM > Subject: RE: [Xen-users] Does vt-x itself have perf. impact > on Hypervisor > w/o considering HVM? > > > > -----Original Message----- > > From: xen-users-bounces@lists.xensource.com > > [mailto:xen-users-bounces@lists.xensource.com] On Behalf Of > Liang Yang > > Sent: 22 January 2007 18:33 > > To: Xen devel list; xen-users@lists.xensource.com > > Subject: [Xen-users] Does vt-x itself have perf. impact on > > Hypervisor w/o considering HVM? > > > > Hello, > > > > Suppose I have two different kinds of CPUs which have exactly > > the same > > configuration except one supports VT-X while the other does > > not. If I want > > to test the I/O performance (or other perf. testing which is not > > particularly related to I/O) of the both domain0 and > > Para-Virtualized Guest > > Domain (HVM domain is not considered), shall I expect to > get the same > > performance results on these two CPUs? > > Assuming ALL other aspects are the same, when you're not using HVM, > there should be absolutely zero impact from it (aside from it > using up a > few kilobytes of memory, to be precise, HVM (including both > VMX and SVM) > takes up 129459 bytes when not used - more memory is allocated > dynamically when it's being used for obvious reasons. For modern > systems, that's so small that it doesn't matter). > > -- > Mats > > > > Thanks, > > > > Liang > > > > > > _______________________________________________ > > Xen-users mailing list > > Xen-users@lists.xensource.com > > http://lists.xensource.com/xen-users > > > > > > > > > > > > ^ permalink raw reply [flat|nested] 50+ messages in thread
end of thread, other threads:[~2007-01-25 13:34 UTC | newest] Thread overview: 50+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2007-01-18 13:57 32-on-64: pvfb issue Gerd Hoffmann 2007-01-18 14:07 ` Keir Fraser 2007-01-18 15:00 ` Markus Armbruster 2007-01-18 15:35 ` Gerd Hoffmann 2007-01-18 15:53 ` Daniel P. Berrange 2007-01-18 16:34 ` Gerd Hoffmann 2007-01-18 16:55 ` Gerd Hoffmann 2007-01-18 17:05 ` Daniel P. Berrange 2007-01-18 18:31 ` Keir Fraser 2007-01-19 9:46 ` Gerd Hoffmann 2007-01-19 9:54 ` Gerd Hoffmann 2007-01-19 10:31 ` Markus Armbruster 2007-01-19 10:46 ` Gerd Hoffmann 2007-01-19 11:53 ` Markus Armbruster 2007-01-19 11:10 ` Keir Fraser 2007-01-19 11:43 ` Gerd Hoffmann 2007-01-19 12:01 ` Keir Fraser 2007-01-19 12:59 ` Gerd Hoffmann 2007-01-19 13:45 ` Keir Fraser 2007-01-19 15:08 ` Gerd Hoffmann 2007-01-19 15:22 ` Keir Fraser 2007-01-19 15:31 ` Gerd Hoffmann 2007-01-19 16:05 ` Keir Fraser 2007-01-20 0:09 ` [Patch] [VTPM_TOOLS] Add HVM support to vtpm_manager Scarlata, Vincent R 2007-01-22 7:50 ` 32-on-64: pvfb issue Gerd Hoffmann 2007-01-22 14:01 ` Gerd Hoffmann 2007-01-22 14:48 ` Keir Fraser 2007-01-23 12:53 ` Gerd Hoffmann 2007-01-23 15:07 ` Keir Fraser 2007-01-23 15:56 ` Gerd Hoffmann 2007-01-24 11:23 ` Gerd Hoffmann 2007-01-24 12:02 ` Keir Fraser 2007-01-24 12:24 ` Markus Armbruster 2007-01-24 12:38 ` Gerd Hoffmann 2007-01-24 14:24 ` Markus Armbruster 2007-01-24 15:25 ` Gerd Hoffmann 2007-01-25 13:16 ` 32-on-64 broken in unstable Gerd Hoffmann 2007-01-25 13:25 ` Keir Fraser 2007-01-25 13:34 ` Gerd Hoffmann 2007-01-22 15:22 ` 32-on-64: pvfb issue Markus Armbruster 2007-01-22 15:33 ` Gerd Hoffmann 2007-01-22 15:40 ` Keir Fraser 2007-01-19 16:06 ` Markus Armbruster 2007-01-22 7:56 ` Gerd Hoffmann 2007-01-19 10:43 ` Ian Campbell 2007-01-19 12:03 ` Markus Armbruster 2007-01-22 18:32 ` Does vt-x itself have perf. impact on Hypervisor w/o considering HVM? Liang Yang 2007-01-23 10:05 ` [Xen-users] " Petersson, Mats 2007-01-23 16:15 ` Liang Yang 2007-01-23 16:33 ` Petersson, Mats
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.