[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [V11 1/3] x86/xsaves: enable xsaves/xrstors/xsavec in xen
On Mon, Nov 23, 2015 at 03:06:01AM -0700, Jan Beulich wrote: > >> numbered to named operands. > > I prefer to use this option. > > > > The original code is below(without xsaves patch) > > asm volatile ( "1: .byte 0x48,0x0f,0xae,0x2f\n" > > ".section .fixup,\"ax\" \n" > > "2: mov %5,%%ecx \n" > > " xor %1,%1 \n" > > " rep stosb \n" > > " lea %2,%0 \n" > > " mov %3,%1 \n" > > " jmp 1b \n" > > ".previous \n" > > _ASM_EXTABLE(1b, 2b) > > : "+&D" (ptr), "+&a" (lmask) > > : "m" (*ptr), "g" (lmask), "d" (hmask), > > "m" (xsave_cntxt_size) > > : "ecx" ); > > > > Then My code to using named operands is below(without xaves patch). > > > > asm volatile ( "1: .byte 0x48,0x0f,0xae,0x2f \n" > > ".section .fixup,\"ax\" \n" > > "2: mov %[SIZE],%%ecx \n" > > " xor %[LMASK_A],%[LMASK_A] \n" > > " rep stosb \n" > > " lea %[PTR_M],%[PTR] \n" > > " mov %[LMASK_G],%[LMASK_A] \n" > > " jmp 1b \n" > > ".previous \n" > > _ASM_EXTABLE(1b, 2b) > > : [PTR] "+&D" (ptr), [LMASK_A] "+&a" (lmask) > > : [PTR_M] "m" (*ptr), [LMASK_G] "g" (lmask), > > [HMASK_D] "d" (hmask), > > [SIZE] "m" (xsave_cntxt_size) > > : "ecx" ); > > > > Is that ok to you ? > > Well, kind of. The variable naming is quite strange: Why upper case? lower case will be used. > And why those strange _A, _G, etc suffixes (I think you instead mean > _out and _in or some such respectively; but see also below - the fewer > dependencies between macro and code surrounding the use sites, the > less important the selection of asm() operand names)? I intend to use _A _G to represent the register it use to make it more readable. But actually it make the code more strange. I intend to add prefix before operand (like below). asm volatile ( "1: .byte 0x48,0x0f,0xae,0x2f \n" ".section .fixup,\"ax\" \n" "2: mov %[size_in],%%ecx \n" " xor %[lmask_out],%[lmask_out] \n" " rep stosb \n" " lea %[ptr_in],%[ptr_out] \n" " mov %[lmask_in],%[lmask_out] \n" " jmp 1b \n" ".previous \n" _ASM_EXTABLE(1b, 2b) : [ptr_out] "+&D" (ptr), [lmask_out] "+&a" (lmask) : [ptr_in] "m" (*ptr), [lmask_in] "g" (lmask), [hmask_in] "d" (hmask), [size_in] "m" (xsave_cntxt_size) : "ecx" ); This seem make the XSAVE_FIXUP macro below depend more on code surrounding the use sites. But the XSAVE_FIXUP macro actually only used in xrstor. So I think it may be proper used like this. Is naming operand in this way ok to you ? > > Also , You ask to splitting things out into a macro ? I do not quite > > unstandand your meaning ? Does it mean define Macro to handle fixup > > code like below ? > > #define XRSTOR_FIXUP > > ".section .fixup,\"ax\" \n" \ > > "2: mov %[SIZE],%%ecx \n" \ > > " xor %[LMASK_A],%[LMASK_A] \n" \ > > " rep stosb \n" \ > > " lea %[PTR_M],%[PTR] \n" \ > > " mov %[LMASK_G],%[LMASK_A] \n" \ > > " jmp 1b \n" \ > > ".previous \n" \ > > _ASM_EXTABLE(1b, 2b) > > > > Then xrstor side can be: > > asm volatile ( "1: .byte 0x48,0x0f,0xae,0x2f \n" > > XRSTOR_FIXUP > > : [PTR] "+&D" (ptr), [LMASK_A] "+&a" (lmask) > > : [PTR_M] "m" (*ptr), [LMASK_G] "g" (lmask), > > [HMASK_D] "d" (hmask), > > [SIZE] "m" (xsave_cntxt_size) > > : "ecx" ); > > Goes in the right direction, but I think all operands related to the fixup > should also get moved to the macro. Of course you'll have to use your > judgment with your actual patches in mind. (To be more precise, the > macro should have as much inside it as possible, so that as little as > possible dependencies exist between it and the code surrounding the > macro invocation. This likely requires adding a parameter or two to > macro.) All the operands realted to fixup moved to macro is ok here. But as I will use alternative asm in restor side(in xsaves patch), alternative will seperate the fixup code and out/in operand definitions like above , and also I intend to reuse XRSTOR_FIXUP in restor side. So in my opintion, seperate like this is ok. Perhaps, there is some point I am not of aware. Please let me if you have much better way to do this ? Thanks for your time. > > _______________________________________________ > Xen-devel mailing list > Xen-devel@xxxxxxxxxxxxx > http://lists.xen.org/xen-devel > _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |