Re: [PATCH 2/3] xen/ppc: Add .text.exceptions section for exception vectors

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

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@xxxxxxxxxxxxxxxxxxxxx>
> ---
>  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