[PATCH] x86: compact supposedly unused entry point code

[PATCH] x86: compact supposedly unused entry point code

[Jan Beulich]

[-- Attachment #1: Type: text/plain, Size: 721 bytes --]

No point in aligning entry points which aren't supposed to be used
anyway.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
TBD: Might consider simply using "andq $-15,%rsp", delivering an
uninitialized error code (which shouldn't matter).

--- a/xen/arch/x86/x86_64/entry.S
+++ b/xen/arch/x86/x86_64/entry.S
@@ -866,11 +866,11 @@ autogen_stubs: /* Automatically generate
 
         vec = 0
         .rept NR_VECTORS
-        ALIGN
 
         /* Common interrupts, heading towards do_IRQ(). */
         .if vec >= FIRST_DYNAMIC_VECTOR && vec != HYPERCALL_VECTOR && vec != LEGACY_SYSCALL_VECTOR
 
+        ALIGN
 1:      pushq $0
         movb  $vec,4(%rsp)
         jmp   common_interrupt




[-- Attachment #2: x86-compact-unknown-entry-points.patch --]
[-- Type: text/plain, Size: 767 bytes --]

x86: compact supposedly unused entry point code 

No point in aligning entry points which aren't supposed to be used
anyway.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
TBD: Might consider simply using "andq $-15,%rsp", delivering an
uninitialized error code (which shouldn't matter).

--- a/xen/arch/x86/x86_64/entry.S
+++ b/xen/arch/x86/x86_64/entry.S
@@ -866,11 +866,11 @@ autogen_stubs: /* Automatically generate
 
         vec = 0
         .rept NR_VECTORS
-        ALIGN
 
         /* Common interrupts, heading towards do_IRQ(). */
         .if vec >= FIRST_DYNAMIC_VECTOR && vec != HYPERCALL_VECTOR && vec != LEGACY_SYSCALL_VECTOR
 
+        ALIGN
 1:      pushq $0
         movb  $vec,4(%rsp)
         jmp   common_interrupt

[-- Attachment #3: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH] x86: compact supposedly unused entry point code

[Andrew Cooper]

[-- Attachment #1.1: Type: text/plain, Size: 224 bytes --]

On 20/06/16 12:04, Jan Beulich wrote:
> No point in aligning entry points which aren't supposed to be used
> anyway.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

[-- Attachment #1.2: Type: text/html, Size: 777 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH] x86: compact supposedly unused entry point code

[Jan Beulich]

>>> On 20.06.16 at 14:15, <andrew.cooper3@citrix.com> wrote:
> On 20/06/16 12:04, Jan Beulich wrote:
>> No point in aligning entry points which aren't supposed to be used
>> anyway.
>>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

Thanks, but - any thoughts on this part:

TBD: Might consider simply using "andq $-15,%rsp", delivering an
uninitialized error code (which shouldn't matter).

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH] x86: compact supposedly unused entry point code

[Andrew Cooper]

On 20/06/16 13:48, Jan Beulich wrote:
>>>> On 20.06.16 at 14:15, <andrew.cooper3@citrix.com> wrote:
>> On 20/06/16 12:04, Jan Beulich wrote:
>>> No point in aligning entry points which aren't supposed to be used
>>> anyway.
>>>
>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Thanks, but - any thoughts on this part:
>
> TBD: Might consider simply using "andq $-15,%rsp", delivering an
> uninitialized error code (which shouldn't matter).

I was still considering that part.

These are entries we never expect to actually take.  At that point, the
small overhead of setting up the error code to 0 is probably better than
leaving it uninitialised.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH] x86: compact supposedly unused entry point code

[Jan Beulich]

>>> On 20.06.16 at 14:54, <andrew.cooper3@citrix.com> wrote:
> On 20/06/16 13:48, Jan Beulich wrote:
>>>>> On 20.06.16 at 14:15, <andrew.cooper3@citrix.com> wrote:
>>> On 20/06/16 12:04, Jan Beulich wrote:
>>>> No point in aligning entry points which aren't supposed to be used
>>>> anyway.
>>>>
>>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
>> Thanks, but - any thoughts on this part:
>>
>> TBD: Might consider simply using "andq $-15,%rsp", delivering an
>> uninitialized error code (which shouldn't matter).
> 
> I was still considering that part.
> 
> These are entries we never expect to actually take.  At that point, the
> small overhead of setting up the error code to 0 is probably better than
> leaving it uninitialised.

I understand - it's really a matter of balancing the overhead on
these paths (which will never have an effect if these entries indeed
are unused, and which is of no interest if they are used by due some
other flaw) with the (likely negligible, but non-zero) overhead they
introduce on _other_ paths (due to cache and TLB consumption). I.e.
my goal was to make these unused entries as small as possible. And

	andq	$-15,%rsp
	movl	$vector,4(%rsp)

(obviously we can't use movb here) is smaller than the current

	testb	$8,%spl
	jz	1f
	pushq	$0
	movb	$vector,4(%rsp)

afaict.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH] x86: compact supposedly unused entry point code

[Andrew Cooper]

On 20/06/16 14:49, Jan Beulich wrote:
>>>> On 20.06.16 at 14:54, <andrew.cooper3@citrix.com> wrote:
>> On 20/06/16 13:48, Jan Beulich wrote:
>>>>>> On 20.06.16 at 14:15, <andrew.cooper3@citrix.com> wrote:
>>>> On 20/06/16 12:04, Jan Beulich wrote:
>>>>> No point in aligning entry points which aren't supposed to be used
>>>>> anyway.
>>>>>
>>>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>>> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
>>> Thanks, but - any thoughts on this part:
>>>
>>> TBD: Might consider simply using "andq $-15,%rsp", delivering an
>>> uninitialized error code (which shouldn't matter).
>> I was still considering that part.
>>
>> These are entries we never expect to actually take.  At that point, the
>> small overhead of setting up the error code to 0 is probably better than
>> leaving it uninitialised.
> I understand - it's really a matter of balancing the overhead on
> these paths (which will never have an effect if these entries indeed
> are unused, and which is of no interest if they are used by due some
> other flaw) with the (likely negligible, but non-zero) overhead they
> introduce on _other_ paths (due to cache and TLB consumption). I.e.
> my goal was to make these unused entries as small as possible. And
>
> 	andq	$-15,%rsp
> 	movl	$vector,4(%rsp)
>
> (obviously we can't use movb here) is smaller than the current
>
> 	testb	$8,%spl
> 	jz	1f
> 	pushq	$0
> 	movb	$vector,4(%rsp)
>
> afaict.

All of them head to do_reserved_trap() and panic().

An alternative would be to drop all this entry code, mark the vectors as
not present in the IDT, and handle #NP[IDT vector] with a slightly
different error message in do_trap().

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH] x86: compact supposedly unused entry point code

[Jan Beulich]

>>> On 20.06.16 at 15:58, <andrew.cooper3@citrix.com> wrote:
> On 20/06/16 14:49, Jan Beulich wrote:
>>>>> On 20.06.16 at 14:54, <andrew.cooper3@citrix.com> wrote:
>>> On 20/06/16 13:48, Jan Beulich wrote:
>>>>>>> On 20.06.16 at 14:15, <andrew.cooper3@citrix.com> wrote:
>>>>> On 20/06/16 12:04, Jan Beulich wrote:
>>>>>> No point in aligning entry points which aren't supposed to be used
>>>>>> anyway.
>>>>>>
>>>>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>>>> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
>>>> Thanks, but - any thoughts on this part:
>>>>
>>>> TBD: Might consider simply using "andq $-15,%rsp", delivering an
>>>> uninitialized error code (which shouldn't matter).
>>> I was still considering that part.
>>>
>>> These are entries we never expect to actually take.  At that point, the
>>> small overhead of setting up the error code to 0 is probably better than
>>> leaving it uninitialised.
>> I understand - it's really a matter of balancing the overhead on
>> these paths (which will never have an effect if these entries indeed
>> are unused, and which is of no interest if they are used by due some
>> other flaw) with the (likely negligible, but non-zero) overhead they
>> introduce on _other_ paths (due to cache and TLB consumption). I.e.
>> my goal was to make these unused entries as small as possible. And
>>
>> 	andq	$-15,%rsp
>> 	movl	$vector,4(%rsp)
>>
>> (obviously we can't use movb here) is smaller than the current
>>
>> 	testb	$8,%spl
>> 	jz	1f
>> 	pushq	$0
>> 	movb	$vector,4(%rsp)
>>
>> afaict.
> 
> All of them head to do_reserved_trap() and panic().

Not sure I'm guessing right what you mean to say with that, so I
can only reiterate that I don't care at all how long it takes for
execution to get through this path. All I care about is that this
code be as small as possible, to limit its impact on surrounding
code. But for the few bytes to save here I guess there was
already way to much talk.

> An alternative would be to drop all this entry code, mark the vectors as
> not present in the IDT, and handle #NP[IDT vector] with a slightly
> different error message in do_trap().

Would likely increase overall code size (i.e. the opposite of what
I'd like to achieve here).

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH] x86: compact supposedly unused entry point code

[Andrew Cooper]

On 20/06/16 15:04, Jan Beulich wrote:
>>>> On 20.06.16 at 15:58, <andrew.cooper3@citrix.com> wrote:
>> On 20/06/16 14:49, Jan Beulich wrote:
>>>>>> On 20.06.16 at 14:54, <andrew.cooper3@citrix.com> wrote:
>>>> On 20/06/16 13:48, Jan Beulich wrote:
>>>>>>>> On 20.06.16 at 14:15, <andrew.cooper3@citrix.com> wrote:
>>>>>> On 20/06/16 12:04, Jan Beulich wrote:
>>>>>>> No point in aligning entry points which aren't supposed to be used
>>>>>>> anyway.
>>>>>>>
>>>>>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>>>>> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
>>>>> Thanks, but - any thoughts on this part:
>>>>>
>>>>> TBD: Might consider simply using "andq $-15,%rsp", delivering an
>>>>> uninitialized error code (which shouldn't matter).
>>>> I was still considering that part.
>>>>
>>>> These are entries we never expect to actually take.  At that point, the
>>>> small overhead of setting up the error code to 0 is probably better than
>>>> leaving it uninitialised.
>>> I understand - it's really a matter of balancing the overhead on
>>> these paths (which will never have an effect if these entries indeed
>>> are unused, and which is of no interest if they are used by due some
>>> other flaw) with the (likely negligible, but non-zero) overhead they
>>> introduce on _other_ paths (due to cache and TLB consumption). I.e.
>>> my goal was to make these unused entries as small as possible. And
>>>
>>> 	andq	$-15,%rsp
>>> 	movl	$vector,4(%rsp)
>>>
>>> (obviously we can't use movb here) is smaller than the current
>>>
>>> 	testb	$8,%spl
>>> 	jz	1f
>>> 	pushq	$0
>>> 	movb	$vector,4(%rsp)
>>>
>>> afaict.
>> All of them head to do_reserved_trap() and panic().
> Not sure I'm guessing right what you mean to say with that, so I
> can only reiterate that I don't care at all how long it takes for
> execution to get through this path. All I care about is that this
> code be as small as possible, to limit its impact on surrounding
> code. But for the few bytes to save here I guess there was
> already way to much talk.
>
>> An alternative would be to drop all this entry code, mark the vectors as
>> not present in the IDT, and handle #NP[IDT vector] with a slightly
>> different error message in do_trap().
> Would likely increase overall code size (i.e. the opposite of what
> I'd like to achieve here).

I find that hard to believe.

I have a number of other improvements pending in this area.  I will add
this to the list.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel