[Skiboot] [PATCH v8 03/11] Add memcons support for ultravisor

[Oliver O'Halloran]

On Thu, Aug 27, 2020 at 4:40 AM Ryan Grimm <grimm at linux.ibm.com> wrote:
>
> From: Madhavan Srinivasan <maddy at linux.vnet.ibm.com>
>
> The ultravisor console buffer is provided at offset 0x01100000 from the
> skiboot base.
>
> Signed-off-by: Madhavan Srinivasan <maddy at linux.vnet.ibm.com>
> Signed-off-by: Santosh Sivaraj <santosh at fossix.org>
> ---
>  hw/ultravisor.c            | 13 +++++++++++++
>  include/console.h          |  3 +++
>  include/debug_descriptor.h |  1 +
>  include/mem-map.h          | 16 ++++++++++------
>  4 files changed, 27 insertions(+), 6 deletions(-)
>
> diff --git a/hw/ultravisor.c b/hw/ultravisor.c
> index 1be47ec5..467f0ca6 100644
> --- a/hw/ultravisor.c
> +++ b/hw/ultravisor.c
> @@ -8,11 +8,21 @@
>  #include <processor.h>
>  #include <opal-api.h>
>  #include <cpu.h>
> +#include <debug_descriptor.h>
> +#include <console.h>
>
>  static struct dt_node *uv_fw_node;
>  static uint64_t uv_base_addr;
>  bool uv_present;
>
> +struct memcons uv_memcons __section(".data.memcons") = {
> +       .magic          = MEMCONS_MAGIC,
> +       .obuf_phys      = INMEM_UV_CON_START,
> +       .ibuf_phys      = INMEM_UV_CON_START + INMEM_UV_CON_OUT_LEN,
> +       .obuf_size      = INMEM_UV_CON_OUT_LEN,
> +       .ibuf_size      = INMEM_UV_CON_IN_LEN,
> +};
> +
>  static void cpu_start_ultravisor(void *fdt)
>  {
>         uint64_t uv_entry = 0;
> @@ -90,4 +100,7 @@ void init_uv()
>                                         uv_fw_sz, uv_base_addr, uv_dt_src);
>
>         memcpy((void *)uv_base_addr, (void *)uv_dt_src, uv_fw_sz);
> +
> +       dt_add_property_u64(uv_fw_node, "memcons", (u64)&uv_memcons);
> +       debug_descriptor.uv_memcons_phys = (u64)&uv_memcons;
>  }
> diff --git a/include/console.h b/include/console.h
> index 02fc7a4b..47ffe7fd 100644
> --- a/include/console.h
> +++ b/include/console.h
> @@ -28,9 +28,12 @@ struct memcons {
>  };
>
>  extern struct memcons memcons;
> +extern struct memcons uv_memcons;
>
>  #define INMEM_CON_IN_LEN       16
>  #define INMEM_CON_OUT_LEN      (INMEM_CON_LEN - INMEM_CON_IN_LEN)
> +#define INMEM_UV_CON_IN_LEN    16
> +#define INMEM_UV_CON_OUT_LEN   (INMEM_UV_CON_LEN - INMEM_UV_CON_IN_LEN)
>
>  /* Console driver */
>  struct con_ops {
> diff --git a/include/debug_descriptor.h b/include/debug_descriptor.h
> index 3ac487b0..dfcf2686 100644
> --- a/include/debug_descriptor.h
> +++ b/include/debug_descriptor.h
> @@ -20,6 +20,7 @@ struct debug_descriptor {
>
>         /* Memory console */
>         __be64  memcons_phys;
> +       __be64  uv_memcons_phys;

Isn't the debug descriptor part of the ABI? Inserting a field in the
middle seems like it'll break something since it shifts everything
else.

>         __be32  memcons_tce;
>         __be32  memcons_obuf_tce;
>         __be32  memcons_ibuf_tce;
> diff --git a/include/mem-map.h b/include/mem-map.h
> index 15ec09ea..2384684b 100644
> --- a/include/mem-map.h
> +++ b/include/mem-map.h
> @@ -91,16 +91,20 @@
>  #define INMEM_CON_START                (SKIBOOT_BASE + 0x01000000)
>  #define INMEM_CON_LEN                  0x100000
>
> -/* This is the location of HBRT console buffer at base + 17M */
> -#define HBRT_CON_START         (SKIBOOT_BASE + 0x01100000)
> +/* This is the location of our ultravisor console buffer at base + 17M */
> +#define INMEM_UV_CON_START     (SKIBOOT_BASE + 0x01100000)
> +#define INMEM_UV_CON_LEN       0x100000

I'd be happier if we just allocated a buffer using local_alloc() on
systems with UV support rather than hard-coding it and wasting the
memory everywhere else. The fact we have the HBRT console here already
is mildly annoying as-is since it's used on P8 FSP systems and nowhere
else.

> +
> +/* This is the location of HBRT console buffer at base + 18M */
> +#define HBRT_CON_START         (SKIBOOT_BASE + 0x01200000)
>  #define HBRT_CON_LEN           0x100000
>
> -/* Tell FSP to put the init data at base + 20M, allocate 8M */
> -#define SPIRA_HEAP_BASE                (SKIBOOT_BASE + 0x01200000)
> +/* Tell FSP to put the init data at base + 19M, allocate 8M */
> +#define SPIRA_HEAP_BASE                (SKIBOOT_BASE + 0x01300000)
>  #define SPIRA_HEAP_SIZE                0x00800000

That said, if you're going to have a fixed allocation for it then put
it after the SPIRA heap so I don't have to re-write all my scripts
which assume it's at 0x3120_0000.

>  /* This is our PSI TCE table. It's 256K entries on P8 */
> -#define PSI_TCE_TABLE_BASE     (SKIBOOT_BASE + 0x01a00000)
> +#define PSI_TCE_TABLE_BASE     (SKIBOOT_BASE + 0x01c00000)
>  #define PSI_TCE_TABLE_SIZE     0x00200000UL
>
>  /* This is our dump result table after MPIPL. Hostboot will write to this
> @@ -119,7 +123,7 @@
>   *
>   * (Ensure this has at least a 64k alignment)
>   */
> -#define SKIBOOT_SIZE           0x01c10000
> +#define SKIBOOT_SIZE           0x01e00000
>
>  /* We start laying out the CPU stacks from here, indexed by PIR
>   * each stack is STACK_SIZE in size (naturally aligned power of
> --
> 2.18.4
>
> _______________________________________________
> Skiboot mailing list
> Skiboot at lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/skiboot