[v3,2/3] debug_descriptor: Claim reserved field for host kernel log buffer

Message ID 20180531042943.15804-3-joel@jms.id.au
State New
Headers show
Series
  • BMC dumping
Related show

Commit Message

Joel Stanley May 31, 2018, 4:29 a.m.
This will be used by the dump region opal call.

This bumps the version of the debug descriptor to indicate this field is
now in use. However, any users of the descriptor should remain
compatible as the non-reserved fields behave in the same way as
previously.

Signed-off-by: Joel Stanley <joel@jms.id.au>
---
 include/skiboot.h | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

Comments

Benjamin Herrenschmidt May 31, 2018, 4:46 a.m. | #1
On Thu, 2018-05-31 at 13:59 +0930, Joel Stanley wrote:
> This will be used by the dump region opal call.
> 
> This bumps the version of the debug descriptor to indicate this field is
> now in use. However, any users of the descriptor should remain
> compatible as the non-reserved fields behave in the same way as
> previously.
> 
> Signed-off-by: Joel Stanley <joel@jms.id.au>
> ---
>  include/skiboot.h | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/include/skiboot.h b/include/skiboot.h
> index b4bdf37795dd..9a49bfca74a5 100644
> --- a/include/skiboot.h
> +++ b/include/skiboot.h
> @@ -69,13 +69,13 @@ static inline bool is_rodata(const void *p)
>   */
>  struct debug_descriptor {
>         u8      eye_catcher[8]; /* "OPALdbug" */
> -#define DEBUG_DESC_VERSION     1
> +#define DEBUG_DESC_VERSION     2
>         u32     version;
>         u8      console_log_levels;     /* high 4 bits in memory,
>                                          * low 4 bits driver (e.g. uart). */
>         u8      state_flags; /* various state flags - OPAL_BOOT_COMPLETE etc */
> -       u16     reserved2;
> -       u32     reserved[2];
> +       u16     reserved;
> +       u64     log_buf_phys;           /* Pointer to kernel log buffer */

What about passing the size of the buffer ?

Ben.
>         /* Memory console */
>         u64     memcons_phys;
> @@ -90,7 +90,7 @@ struct debug_descriptor {
>         u64     trace_phys[DEBUG_DESC_MAX_TRACES];
>         u32     trace_size[DEBUG_DESC_MAX_TRACES];
>         u32     trace_tce[DEBUG_DESC_MAX_TRACES];
> -};
> +} __packed;
>  extern struct debug_descriptor debug_descriptor;
>  
>  static inline bool opal_booting(void)
Joel Stanley May 31, 2018, 4:58 a.m. | #2
On 31 May 2018 at 14:16, Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:
> On Thu, 2018-05-31 at 13:59 +0930, Joel Stanley wrote:
>> This will be used by the dump region opal call.
>>
>> This bumps the version of the debug descriptor to indicate this field is
>> now in use. However, any users of the descriptor should remain
>> compatible as the non-reserved fields behave in the same way as
>> previously.
>>
>> Signed-off-by: Joel Stanley <joel@jms.id.au>
>> ---
>>  include/skiboot.h | 8 ++++----
>>  1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/include/skiboot.h b/include/skiboot.h
>> index b4bdf37795dd..9a49bfca74a5 100644
>> --- a/include/skiboot.h
>> +++ b/include/skiboot.h
>> @@ -69,13 +69,13 @@ static inline bool is_rodata(const void *p)
>>   */
>>  struct debug_descriptor {
>>         u8      eye_catcher[8]; /* "OPALdbug" */
>> -#define DEBUG_DESC_VERSION     1
>> +#define DEBUG_DESC_VERSION     2
>>         u32     version;
>>         u8      console_log_levels;     /* high 4 bits in memory,
>>                                          * low 4 bits driver (e.g. uart). */
>>         u8      state_flags; /* various state flags - OPAL_BOOT_COMPLETE etc */
>> -       u16     reserved2;
>> -       u32     reserved[2];
>> +       u16     reserved;
>> +       u64     log_buf_phys;           /* Pointer to kernel log buffer */
>
> What about passing the size of the buffer ?

I guess something could use it to dump the entire region to be
processed later. We'd need to move things around in the descriptor to
do this, as the size is u32 and we only have a u16 left over.

If you're processing it online then we read one entry at a time, and
keep going until we find a zero length entry in the buffer to know
that it's the end.

Joel
Benjamin Herrenschmidt May 31, 2018, 6:11 a.m. | #3
On Thu, 2018-05-31 at 14:28 +0930, Joel Stanley wrote:
> I guess something could use it to dump the entire region to be
> processed later. We'd need to move things around in the descriptor to
> do this, as the size is u32 and we only have a u16 left over.
> 
> If you're processing it online then we read one entry at a time, and
> keep going until we find a zero length entry in the buffer to know
> that it's the end.

It's a ring buffer, how can you wrap if you don't know its size ?

Cheers,
Ben.

Patch

diff --git a/include/skiboot.h b/include/skiboot.h
index b4bdf37795dd..9a49bfca74a5 100644
--- a/include/skiboot.h
+++ b/include/skiboot.h
@@ -69,13 +69,13 @@  static inline bool is_rodata(const void *p)
  */
 struct debug_descriptor {
 	u8	eye_catcher[8];	/* "OPALdbug" */
-#define DEBUG_DESC_VERSION	1
+#define DEBUG_DESC_VERSION	2
 	u32	version;
 	u8	console_log_levels;	/* high 4 bits in memory,
 					 * low 4 bits driver (e.g. uart). */
 	u8	state_flags; /* various state flags - OPAL_BOOT_COMPLETE etc */
-	u16	reserved2;
-	u32	reserved[2];
+	u16	reserved;
+	u64	log_buf_phys;		/* Pointer to kernel log buffer */
 
 	/* Memory console */
 	u64	memcons_phys;
@@ -90,7 +90,7 @@  struct debug_descriptor {
 	u64	trace_phys[DEBUG_DESC_MAX_TRACES];
 	u32	trace_size[DEBUG_DESC_MAX_TRACES];
 	u32	trace_tce[DEBUG_DESC_MAX_TRACES];
-};
+} __packed;
 extern struct debug_descriptor debug_descriptor;
 
 static inline bool opal_booting(void)