| Message ID | 5767E9DE02000078000F69D3@prv-mh.provo.novell.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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>
>>> 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
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
>>> 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
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
>>> 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
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
--- 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