| Message ID | de5b99d79671a7fe11c5720797aaa6e3207661d1.1695942864.git.sanastasio@raptorengineering.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Early exception handlers on Power | expand |
On 29/09/2023 12:19 am, Shawn Anastasio wrote: > On Power, the exception vectors must lie at a fixed address, depending > on the state of the Alternate Interrupt Location (AIL) field of the > Logical Partition Control Register (LPCR). Create a .text.exceptions > section in the linker script at an address suitable for AIL=3 plus an > accompanying assertion to pave the way for implementing exception > support. Thanks - this is a perfect level of detail as far as I'm concerned as a PPC novice. > > Signed-off-by: Shawn Anastasio <sanastasio@raptorengineering.com> > --- > xen/arch/ppc/include/asm/config.h | 3 +++ > xen/arch/ppc/xen.lds.S | 7 +++++++ > 2 files changed, 10 insertions(+) > > diff --git a/xen/arch/ppc/include/asm/config.h b/xen/arch/ppc/include/asm/config.h > index a11a09c570..e012b75beb 100644 > --- a/xen/arch/ppc/include/asm/config.h > +++ b/xen/arch/ppc/include/asm/config.h > @@ -42,6 +42,9 @@ > > #define XEN_VIRT_START _AC(0xc000000000000000, UL) > > +/* Fixed address for start of the section containing exception vectors */ > +#define EXCEPTION_VECTORS_START _AC(0xc000000000000100, UL) The patch looks fine, but a PPC question. Does AIL=3 really mean a hard coded address at 0xc000000000000100 ? Or is it +0x100 from something else that happens to be programmed to XEN_VIRT_START ? > + > #define VMAP_VIRT_START (XEN_VIRT_START + GB(1)) > #define VMAP_VIRT_SIZE GB(1) > > diff --git a/xen/arch/ppc/xen.lds.S b/xen/arch/ppc/xen.lds.S > index 9e46035155..9e888d7383 100644 > --- a/xen/arch/ppc/xen.lds.S > +++ b/xen/arch/ppc/xen.lds.S > @@ -24,6 +24,10 @@ SECTIONS > _stext = .; /* Text section */ > *(.text.header) > > + . = ALIGN(256); > + _stext_exceptions = .; If this is really only used for the linker assertion, then it wants to be HIDDEN(_stext_exceptions = .); otherwise the debugging symbols will have _stext_exceptions typically hiding exc_sysreset in the disassembly and symbol table. > + *(.text.exceptions) > + > *(.text.cold) > *(.text.unlikely .text.*_unlikely .text.unlikely.*) > > @@ -184,3 +188,6 @@ ASSERT(IS_ALIGNED(__bss_end, POINTER_ALIGN), "__bss_end is misaligned") > > ASSERT(!SIZEOF(.got), ".got non-empty") > ASSERT(!SIZEOF(.got.plt), ".got.plt non-empty") > + > +ASSERT(_stext_exceptions == EXCEPTION_VECTORS_START, \ > + ".text.exceptions not at expected location -- .text.header too big?"); No need for ; at the end, and no need for \ either. As I said for patch 1, we're now at 4.18-rc1. Does this need to be included now, or wait for 4.19? There's something to be said for having a basic exception handler, but it is technically a new feature... ~Andrew
On 9/29/23 4:28 AM, Andrew Cooper wrote: > On 29/09/2023 12:19 am, Shawn Anastasio wrote: >> On Power, the exception vectors must lie at a fixed address, depending >> on the state of the Alternate Interrupt Location (AIL) field of the >> Logical Partition Control Register (LPCR). Create a .text.exceptions >> section in the linker script at an address suitable for AIL=3 plus an >> accompanying assertion to pave the way for implementing exception >> support. > > Thanks - this is a perfect level of detail as far as I'm concerned as a > PPC novice. > >> >> Signed-off-by: Shawn Anastasio <sanastasio@raptorengineering.com> >> --- >> xen/arch/ppc/include/asm/config.h | 3 +++ >> xen/arch/ppc/xen.lds.S | 7 +++++++ >> 2 files changed, 10 insertions(+) >> >> diff --git a/xen/arch/ppc/include/asm/config.h b/xen/arch/ppc/include/asm/config.h >> index a11a09c570..e012b75beb 100644 >> --- a/xen/arch/ppc/include/asm/config.h >> +++ b/xen/arch/ppc/include/asm/config.h >> @@ -42,6 +42,9 @@ >> >> #define XEN_VIRT_START _AC(0xc000000000000000, UL) >> >> +/* Fixed address for start of the section containing exception vectors */ >> +#define EXCEPTION_VECTORS_START _AC(0xc000000000000100, UL) > > The patch looks fine, but a PPC question. Does AIL=3 really mean a hard > coded address at 0xc000000000000100 ? > > Or is it +0x100 from something else that happens to be programmed to > XEN_VIRT_START ? > AIL=3 means a hardcoded address at 0xc000000000004000, actually, but by placing the section earlier we can take advantage of some of the space in between ...0100 and ...4000 for things like the {h_,}exception_common routines introduced in the next patch instead of just having the linker pad it with nops. (Now that I look closely though, I see that I erroneously placed those routines in .text rather than .text.exceptions in the next patch -- that's something I'll fix for v2). >> + >> #define VMAP_VIRT_START (XEN_VIRT_START + GB(1)) >> #define VMAP_VIRT_SIZE GB(1) >> >> diff --git a/xen/arch/ppc/xen.lds.S b/xen/arch/ppc/xen.lds.S >> index 9e46035155..9e888d7383 100644 >> --- a/xen/arch/ppc/xen.lds.S >> +++ b/xen/arch/ppc/xen.lds.S >> @@ -24,6 +24,10 @@ SECTIONS >> _stext = .; /* Text section */ >> *(.text.header) >> >> + . = ALIGN(256); >> + _stext_exceptions = .; > > If this is really only used for the linker assertion, then it wants to be > > HIDDEN(_stext_exceptions = .); > > otherwise the debugging symbols will have _stext_exceptions typically > hiding exc_sysreset in the disassembly and symbol table. > It is indeed only used for the assertion -- I'll make this change in v2. >> + *(.text.exceptions) >> + >> *(.text.cold) >> *(.text.unlikely .text.*_unlikely .text.unlikely.*) >> >> @@ -184,3 +188,6 @@ ASSERT(IS_ALIGNED(__bss_end, POINTER_ALIGN), "__bss_end is misaligned") >> >> ASSERT(!SIZEOF(.got), ".got non-empty") >> ASSERT(!SIZEOF(.got.plt), ".got.plt non-empty") >> + >> +ASSERT(_stext_exceptions == EXCEPTION_VECTORS_START, \ >> + ".text.exceptions not at expected location -- .text.header too big?"); > > No need for ; at the end, and no need for \ either. > Will fix. > As I said for patch 1, we're now at 4.18-rc1. Does this need to be > included now, or wait for 4.19? There's something to be said for having > a basic exception handler, but it is technically a new feature... > I don't think there's any pressing need to bring this into 4.18, unless the burden of doing so is trivial. > ~Andrew Thanks, Shawn
On 29/09/2023 7:15 pm, Shawn Anastasio wrote: > On 9/29/23 4:28 AM, Andrew Cooper wrote: >> As I said for patch 1, we're now at 4.18-rc1. Does this need to be >> included now, or wait for 4.19? There's something to be said for having >> a basic exception handler, but it is technically a new feature... >> > I don't think there's any pressing need to bring this into 4.18, unless > the burden of doing so is trivial. Ok, in which case lets not complicate 4.18 with this. Shortly I'll start up my usual xen-next branch to hold okay'd content for 4.19, but the patches can sit on mailing list in the short term. Thanks, ~Andrew
diff --git a/xen/arch/ppc/include/asm/config.h b/xen/arch/ppc/include/asm/config.h index a11a09c570..e012b75beb 100644 --- a/xen/arch/ppc/include/asm/config.h +++ b/xen/arch/ppc/include/asm/config.h @@ -42,6 +42,9 @@ #define XEN_VIRT_START _AC(0xc000000000000000, UL) +/* Fixed address for start of the section containing exception vectors */ +#define EXCEPTION_VECTORS_START _AC(0xc000000000000100, UL) + #define VMAP_VIRT_START (XEN_VIRT_START + GB(1)) #define VMAP_VIRT_SIZE GB(1) diff --git a/xen/arch/ppc/xen.lds.S b/xen/arch/ppc/xen.lds.S index 9e46035155..9e888d7383 100644 --- a/xen/arch/ppc/xen.lds.S +++ b/xen/arch/ppc/xen.lds.S @@ -24,6 +24,10 @@ SECTIONS _stext = .; /* Text section */ *(.text.header) + . = ALIGN(256); + _stext_exceptions = .; + *(.text.exceptions) + *(.text.cold) *(.text.unlikely .text.*_unlikely .text.unlikely.*) @@ -184,3 +188,6 @@ ASSERT(IS_ALIGNED(__bss_end, POINTER_ALIGN), "__bss_end is misaligned") ASSERT(!SIZEOF(.got), ".got non-empty") ASSERT(!SIZEOF(.got.plt), ".got.plt non-empty") + +ASSERT(_stext_exceptions == EXCEPTION_VECTORS_START, \ + ".text.exceptions not at expected location -- .text.header too big?");
On Power, the exception vectors must lie at a fixed address, depending on the state of the Alternate Interrupt Location (AIL) field of the Logical Partition Control Register (LPCR). Create a .text.exceptions section in the linker script at an address suitable for AIL=3 plus an accompanying assertion to pave the way for implementing exception support. Signed-off-by: Shawn Anastasio <sanastasio@raptorengineering.com> --- xen/arch/ppc/include/asm/config.h | 3 +++ xen/arch/ppc/xen.lds.S | 7 +++++++ 2 files changed, 10 insertions(+)