[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCHv9 0/9] Xen: extend kexec hypercall for use with pv-ops kernels
On Thu, Oct 10, 2013 at 05:35:39PM +0100, David Vrabel wrote: > On 10/10/13 16:45, Daniel Kiper wrote: > > On Wed, Oct 09, 2013 at 05:03:22PM +0100, David Vrabel wrote: > >> On 09/10/13 16:26, Daniel Kiper wrote: > >>> > >>> What about setting GPRs to known value (e.g. 0 like in Linux Kernel) > >>> before jumping into purgatory? > >> > >> I have (repeatedly) explained why and you have not provided a sensible > > > > What do you mean by that? I have not seen any real explanation. > > You were saying only that I am defining an ABI. I do not buy it. > > Even you did not reply to my last question: Could you tell me > > where do you see an ABI here? > > I'm going to comment on your points one final time. I am not going to > debate with you any further on any of this. Well, we are here to discuss technical stuff and we are doing that. Sometimes it is tough but it does not mean that we should break that discussion. Especially if adversaries obey the rules. I think this is the case here. Additionally, we agreed some things so this is not counterproductive. So still I count on cooperation with you. > The register state on executing the image is undefined (this is the > specified ABI), so there is no need to set the registers to any > particular value. So let's look into the docs. I have found at least two interesting source files for us. linux/arch/x86/kernel/relocate_kernel_64.S (let's focus on 64-bit; 32-bit is quite similar) contains something like: /* * set all of the registers to known values * leave %rsp alone */ testq %r11, %r11 jnz 1f xorl %eax, %eax xorl %ebx, %ebx xorl %ecx, %ecx xorl %edx, %edx xorl %esi, %esi xorl %edi, %edi xorl %ebp, %ebp xorl %r8d, %r8d xorl %r9d, %r9d xorl %r10d, %r10d xorl %r11d, %r11d xorl %r12d, %r12d xorl %r13d, %r13d xorl %r14d, %r14d xorl %r15d, %r15d ret Comment clearly states: set all of the registers to known values; leave %rsp alone. So registers states, just before jump into purgatory, are clearly defined. Now look into kexec-tools/purgatory/arch/x86_64/setup-x86_64.S. There is no any single word about registers states. Even any instruction assumes their states. Excluding %rsp which should point to jump_back_entry. In our case %rsp should point to 0UL stored at the stack (we have missed that and it should be fixed by us). We call purgatory. Purgatory is shared code used at least by Linux and Xen. It was created for Linux by Linux guys so we are the guest here. There is no other (correct me if I missed something) docs saying anything about registers states just before jump into purgatory. So we should obey the rules described in linux/arch/x86/kernel/relocate_kernel_64.S. If we do not like them we should ask authors of original kexec/kdump about this stuff and act accordingly to their reply. There is no other way here. > If the implementation did zero the registers then an image could rely on > this. It would then be impossible to change the implementation to do This is bogus. Any sane developer or maintainer do not assume any register state if it is not clearly described by existing docs. And it is not. Such brain dead code would not be accepted at least at kexec list. Guys doing that are crazy and I do not care about them. They are just asking for problems. Additionally, there are more reliable ways to get 0 for our needs. > anything other than zero the registers as that would break existing > users. Zeroing the register is thus an implicit or defacto ABI (even > though we specified the register values as undefined). > > If the registers are not zeroed then it is highly unlikely that an image > could make use of their values and thus if we wish to change the > specification to set some register values we can safely do so without > breaking existing images. So how are you going differentiate between old Xen kexec implementation (current proposal) and new Xen kexec implementation (current proposal + some changes) in purgatory (if it be needed) using just only registers if existing proposal does not enforce registers values (they are simply random)? > > However, why we do not care about compatiblity with existing implementation? > > Xen does diverge from ABI provided by Linux where it makes sense. i.e., ABI should be compatible with exiting implementation because we are using existing code. Please look above. > where doing so makes for a better ABI or a better implementation. For However, I do not object against better implementation... > example, 64-bit images are exec'd with page tables that only cover the > image segments (unlike on Linux were the page tables cover all of RAM > which has problems as noted by Jan with cachable mapping overlapping > with uncachable regions). ...like in that case. > Compatibility with existing Linux tools is a nice bonus but should not > and does not constrain the Xen ABI or implementation. Ditto. > > Are we going to write special purgatory > > code for Xen if one day original purgatory require 0 or another known value > > in one or more registers? > > If that happens we can always revist the Xen implementation and consider > changing or (or just fix purgatory). Why we are assuming that we need fix our implementation just before inclusion in Xen unstable? Why we cannot fix it now? Why we could not assume that our implementation should run as long as possible without any changes? > >>> By the way, you do not need to save and restore %rdi, %rsi and %rbx > >>> in relocate_pages() in xen/arch/x86/x86_64/kexec_reloc.S. > >> > >> This is done so relocate_pages() behaves like a proper function with the > >> standard calling convention. > > > > If you would like to be inline with GCC (and a few others) calling convetion > > then you should save %rbx in relocate_pages() only. %rcx, %rdi and %rsi > > should > > be saved by caller if needed. > > Yes, I got that wrong, but you're really into trivial nit-picking here > which is quite frankly neither helpful nor productive. We are here to discuss nitpicks too. It is not counterproductive. > > Anyway, I do not care about saving registers not > > used later in relocate_pages() or around it. > > This is stupid -- relocate_pages() is called like a function so it > should behave like one. Anything else is going to trip up someone else > looking at or modifying the code in the future. I do not agree with you but respect your opinion. If you insist on making relocate_pages() as a real function do that. However, please be inline with GCC calling convention. Daniel _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |