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 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
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 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
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. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
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 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
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 -=| _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
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 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
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 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
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 -=| _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
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 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
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> _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
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> _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
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. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
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. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
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> _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
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 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
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> _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
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. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
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 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
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. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
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> _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
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 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
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> _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
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 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
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> _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
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 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
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. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Scarlata, Vincent R
2007-Jan-20 00:09 UTC
[Xen-devel] [Patch] [VTPM_TOOLS] Add HVM support to vtpm_manager
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> _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
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 protocolSame transaction event-channel is written. stay tuned, Gerd -- Gerd Hoffmann <kraxel@suse.de> _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
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> _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
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-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
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 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
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; > +#endifHackery 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);[...] _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
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> _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
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 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Liang Yang
2007-Jan-22 18:32 UTC
[Xen-devel] 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? Thanks, Liang _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Petersson, Mats
2007-Jan-23 10:05 UTC
[Xen-devel] 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 > > >_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
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> _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
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 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
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> _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Liang Yang
2007-Jan-23 16:15 UTC
[Xen-devel] 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? 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 > > >_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Petersson, Mats
2007-Jan-23 16:33 UTC
[Xen-devel] RE: [Xen-users] Does vt-x itself have perf. impact on Hypervisor w/o considering HVM?
> -----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 > > > > > > > > > > > >_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
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> _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
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 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
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? _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
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> _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
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, > GerdUnclean! 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. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
> Reading it next to reading feature-update would be much cleaner.Done, updated patch attached. cheers, Gerd -- Gerd Hoffmann <kraxel@suse.de> _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
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> _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
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 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
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> _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel