qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH qemu] spapr: tune rtas-size
@ 2021-06-22  7:03 Alexey Kardashevskiy
  2021-06-22  7:33 ` Greg Kurz
  2021-06-24  1:28 ` David Gibson
  0 siblings, 2 replies; 3+ messages in thread
From: Alexey Kardashevskiy @ 2021-06-22  7:03 UTC (permalink / raw)
  To: qemu-ppc; +Cc: Alexey Kardashevskiy, David Gibson, qemu-devel, Greg Kurz

QEMU reserves space for RTAS via /rtas/rtas-size which tells the client
how much space the RTAS requires to work which includes the RTAS binary
blob implementing RTAS runtime. Because pseries supports FWNMI which
requires plenty of space, QEMU reserves more than 2KB which is
enough for the RTAS blob as it is just 20 bytes (under QEMU).

Since FWNMI reset delivery was added, RTAS_SIZE macro is not used anymore.
This replaces RTAS_SIZE with RTAS_MIN_SIZE and uses it in
the /rtas/rtas-size calculation to account for the RTAS blob.

Fixes: 0e236d347790 ("ppc/spapr: Implement FWNMI System Reset delivery")
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---

Alternatively SLOF could add hv_rtas_size to the property itself
but splitting calculations between 2 chunks of code seems an overkill.
The RTAS blob has always been 20 bytes and unlikely to ever change.
---
 include/hw/ppc/spapr.h | 2 +-
 hw/ppc/spapr.c         | 8 ++++++--
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index f05219f75ef6..5697327e4c00 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -770,7 +770,7 @@ void spapr_load_rtas(SpaprMachineState *spapr, void *fdt, hwaddr addr);
 #define SPAPR_IS_PCI_LIOBN(liobn)   (!!((liobn) & 0x80000000))
 #define SPAPR_PCI_DMA_WINDOW_NUM(liobn) ((liobn) & 0xff)
 
-#define RTAS_SIZE               2048
+#define RTAS_MIN_SIZE           20 /* hv_rtas_size in SLOF */
 #define RTAS_ERROR_LOG_MAX      2048
 
 /* Offset from rtas-base where error log is placed */
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 4dd90b75cc52..9e19c570327e 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -919,9 +919,13 @@ static void spapr_dt_rtas(SpaprMachineState *spapr, void *fdt)
      *
      * The extra 8 bytes is required because Linux's FWNMI error log check
      * is off-by-one.
+     *
+     * RTAS_MIN_SIZE is required for the RTAS blob itself.
      */
-    _FDT(fdt_setprop_cell(fdt, rtas, "rtas-size", RTAS_ERROR_LOG_MAX +
-			  ms->smp.max_cpus * sizeof(uint64_t)*2 + sizeof(uint64_t)));
+    _FDT(fdt_setprop_cell(fdt, rtas, "rtas-size", RTAS_MIN_SIZE +
+                          RTAS_ERROR_LOG_MAX +
+                          ms->smp.max_cpus * sizeof(uint64_t) * 2 +
+                          sizeof(uint64_t)));
     _FDT(fdt_setprop_cell(fdt, rtas, "rtas-error-log-max",
                           RTAS_ERROR_LOG_MAX));
     _FDT(fdt_setprop_cell(fdt, rtas, "rtas-event-scan-rate",
-- 
2.30.2



^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH qemu] spapr: tune rtas-size
  2021-06-22  7:03 [PATCH qemu] spapr: tune rtas-size Alexey Kardashevskiy
@ 2021-06-22  7:33 ` Greg Kurz
  2021-06-24  1:28 ` David Gibson
  1 sibling, 0 replies; 3+ messages in thread
From: Greg Kurz @ 2021-06-22  7:33 UTC (permalink / raw)
  To: Alexey Kardashevskiy; +Cc: qemu-ppc, qemu-devel, David Gibson

On Tue, 22 Jun 2021 17:03:36 +1000
Alexey Kardashevskiy <aik@ozlabs.ru> wrote:

> QEMU reserves space for RTAS via /rtas/rtas-size which tells the client
> how much space the RTAS requires to work which includes the RTAS binary
> blob implementing RTAS runtime. Because pseries supports FWNMI which
> requires plenty of space, QEMU reserves more than 2KB which is
> enough for the RTAS blob as it is just 20 bytes (under QEMU).
> 
> Since FWNMI reset delivery was added, RTAS_SIZE macro is not used anymore.
> This replaces RTAS_SIZE with RTAS_MIN_SIZE and uses it in
> the /rtas/rtas-size calculation to account for the RTAS blob.
> 
> Fixes: 0e236d347790 ("ppc/spapr: Implement FWNMI System Reset delivery")
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> ---
> 
> Alternatively SLOF could add hv_rtas_size to the property itself
> but splitting calculations between 2 chunks of code seems an overkill.
> The RTAS blob has always been 20 bytes and unlikely to ever change.

I guess this is acceptable.

Reviewed-by: Greg Kurz <groug@kaod.org>

> ---
>  include/hw/ppc/spapr.h | 2 +-
>  hw/ppc/spapr.c         | 8 ++++++--
>  2 files changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> index f05219f75ef6..5697327e4c00 100644
> --- a/include/hw/ppc/spapr.h
> +++ b/include/hw/ppc/spapr.h
> @@ -770,7 +770,7 @@ void spapr_load_rtas(SpaprMachineState *spapr, void *fdt, hwaddr addr);
>  #define SPAPR_IS_PCI_LIOBN(liobn)   (!!((liobn) & 0x80000000))
>  #define SPAPR_PCI_DMA_WINDOW_NUM(liobn) ((liobn) & 0xff)
>  
> -#define RTAS_SIZE               2048
> +#define RTAS_MIN_SIZE           20 /* hv_rtas_size in SLOF */
>  #define RTAS_ERROR_LOG_MAX      2048
>  
>  /* Offset from rtas-base where error log is placed */
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 4dd90b75cc52..9e19c570327e 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -919,9 +919,13 @@ static void spapr_dt_rtas(SpaprMachineState *spapr, void *fdt)
>       *
>       * The extra 8 bytes is required because Linux's FWNMI error log check
>       * is off-by-one.
> +     *
> +     * RTAS_MIN_SIZE is required for the RTAS blob itself.
>       */
> -    _FDT(fdt_setprop_cell(fdt, rtas, "rtas-size", RTAS_ERROR_LOG_MAX +
> -			  ms->smp.max_cpus * sizeof(uint64_t)*2 + sizeof(uint64_t)));
> +    _FDT(fdt_setprop_cell(fdt, rtas, "rtas-size", RTAS_MIN_SIZE +
> +                          RTAS_ERROR_LOG_MAX +
> +                          ms->smp.max_cpus * sizeof(uint64_t) * 2 +
> +                          sizeof(uint64_t)));
>      _FDT(fdt_setprop_cell(fdt, rtas, "rtas-error-log-max",
>                            RTAS_ERROR_LOG_MAX));
>      _FDT(fdt_setprop_cell(fdt, rtas, "rtas-event-scan-rate",



^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH qemu] spapr: tune rtas-size
  2021-06-22  7:03 [PATCH qemu] spapr: tune rtas-size Alexey Kardashevskiy
  2021-06-22  7:33 ` Greg Kurz
@ 2021-06-24  1:28 ` David Gibson
  1 sibling, 0 replies; 3+ messages in thread
From: David Gibson @ 2021-06-24  1:28 UTC (permalink / raw)
  To: Alexey Kardashevskiy; +Cc: qemu-ppc, qemu-devel, Greg Kurz

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

On Tue, Jun 22, 2021 at 05:03:36PM +1000, Alexey Kardashevskiy wrote:
> QEMU reserves space for RTAS via /rtas/rtas-size which tells the client
> how much space the RTAS requires to work which includes the RTAS binary
> blob implementing RTAS runtime. Because pseries supports FWNMI which
> requires plenty of space, QEMU reserves more than 2KB which is
> enough for the RTAS blob as it is just 20 bytes (under QEMU).
> 
> Since FWNMI reset delivery was added, RTAS_SIZE macro is not used anymore.
> This replaces RTAS_SIZE with RTAS_MIN_SIZE and uses it in
> the /rtas/rtas-size calculation to account for the RTAS blob.
> 
> Fixes: 0e236d347790 ("ppc/spapr: Implement FWNMI System Reset delivery")
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>

Applied to ppc-for-6.1, thanks.

> ---
> 
> Alternatively SLOF could add hv_rtas_size to the property itself
> but splitting calculations between 2 chunks of code seems an overkill.
> The RTAS blob has always been 20 bytes and unlikely to ever change.
> ---
>  include/hw/ppc/spapr.h | 2 +-
>  hw/ppc/spapr.c         | 8 ++++++--
>  2 files changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> index f05219f75ef6..5697327e4c00 100644
> --- a/include/hw/ppc/spapr.h
> +++ b/include/hw/ppc/spapr.h
> @@ -770,7 +770,7 @@ void spapr_load_rtas(SpaprMachineState *spapr, void *fdt, hwaddr addr);
>  #define SPAPR_IS_PCI_LIOBN(liobn)   (!!((liobn) & 0x80000000))
>  #define SPAPR_PCI_DMA_WINDOW_NUM(liobn) ((liobn) & 0xff)
>  
> -#define RTAS_SIZE               2048
> +#define RTAS_MIN_SIZE           20 /* hv_rtas_size in SLOF */
>  #define RTAS_ERROR_LOG_MAX      2048
>  
>  /* Offset from rtas-base where error log is placed */
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 4dd90b75cc52..9e19c570327e 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -919,9 +919,13 @@ static void spapr_dt_rtas(SpaprMachineState *spapr, void *fdt)
>       *
>       * The extra 8 bytes is required because Linux's FWNMI error log check
>       * is off-by-one.
> +     *
> +     * RTAS_MIN_SIZE is required for the RTAS blob itself.
>       */
> -    _FDT(fdt_setprop_cell(fdt, rtas, "rtas-size", RTAS_ERROR_LOG_MAX +
> -			  ms->smp.max_cpus * sizeof(uint64_t)*2 + sizeof(uint64_t)));
> +    _FDT(fdt_setprop_cell(fdt, rtas, "rtas-size", RTAS_MIN_SIZE +
> +                          RTAS_ERROR_LOG_MAX +
> +                          ms->smp.max_cpus * sizeof(uint64_t) * 2 +
> +                          sizeof(uint64_t)));
>      _FDT(fdt_setprop_cell(fdt, rtas, "rtas-error-log-max",
>                            RTAS_ERROR_LOG_MAX));
>      _FDT(fdt_setprop_cell(fdt, rtas, "rtas-event-scan-rate",

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2021-06-24  3:20 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-22  7:03 [PATCH qemu] spapr: tune rtas-size Alexey Kardashevskiy
2021-06-22  7:33 ` Greg Kurz
2021-06-24  1:28 ` David Gibson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).