| Message ID | 20180524013720.1589-2-joel@jms.id.au |
|---|---|
| State | Superseded |
| Headers | show |
| Series | BMC dumping | expand |
On 05/24/2018 07:07 AM, Joel Stanley wrote: > Signed-off-by: Joel Stanley <joel@jms.id.au> > --- > We could bump the version instead, but I don't think there's any call > for that. > > Another option would be to re-use some of the memcons fields that are > currently only used by FSP. > > A third option would be to use this field to link to yet another debug > structure to allow for future flexibility. > --- > include/skiboot.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/include/skiboot.h b/include/skiboot.h > index b4bdf37795dd..7f3740965412 100644 > --- a/include/skiboot.h > +++ b/include/skiboot.h > @@ -75,7 +75,7 @@ struct debug_descriptor { > * low 4 bits driver (e.g. uart). */ > u8 state_flags; /* various state flags - OPAL_BOOT_COMPLETE etc */ > u16 reserved2; > - u32 reserved[2]; > + u64 log_buf_phys; /* Pointer to kernel log buffer */ May be increment DEBUG_DESC_VERSION as well? So that user can identify whether this field is supported or not? -Vasant
On 25 May 2018 at 15:24, Vasant Hegde <hegdevasant@linux.vnet.ibm.com> wrote: > On 05/24/2018 07:07 AM, Joel Stanley wrote: >> >> Signed-off-by: Joel Stanley <joel@jms.id.au> >> --- >> We could bump the version instead, but I don't think there's any call >> for that. >> >> Another option would be to re-use some of the memcons fields that are >> currently only used by FSP. >> >> A third option would be to use this field to link to yet another debug >> structure to allow for future flexibility. >> --- >> include/skiboot.h | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/include/skiboot.h b/include/skiboot.h >> index b4bdf37795dd..7f3740965412 100644 >> --- a/include/skiboot.h >> +++ b/include/skiboot.h >> @@ -75,7 +75,7 @@ struct debug_descriptor { >> * low 4 bits driver (e.g. uart). >> */ >> u8 state_flags; /* various state flags - OPAL_BOOT_COMPLETE >> etc */ >> u16 reserved2; >> - u32 reserved[2]; >> + u64 log_buf_phys; /* Pointer to kernel log buffer */ > > > May be increment DEBUG_DESC_VERSION as well? So that user can identify > whether this field > is supported or not? I considered this. Users running skiboot before this patch will see zeros for the reserved[2] field. Users running skiboot after this patch will either see zeroes, or see the field populated with the memory address. Given that there's no change in behaviour, I don't think there's any value in bumping the field. That said, there's no reason bumping the value will hurt, if someone thinks we need to. I'll leave it as-is for now. Cheers, Joel
On 05/28/2018 09:10 AM, Joel Stanley wrote: > On 25 May 2018 at 15:24, Vasant Hegde <hegdevasant@linux.vnet.ibm.com> wrote: >> On 05/24/2018 07:07 AM, Joel Stanley wrote: >>> >>> Signed-off-by: Joel Stanley <joel@jms.id.au> >>> --- >>> We could bump the version instead, but I don't think there's any call >>> for that. >>> >>> Another option would be to re-use some of the memcons fields that are >>> currently only used by FSP. >>> >>> A third option would be to use this field to link to yet another debug >>> structure to allow for future flexibility. >>> --- >>> include/skiboot.h | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/include/skiboot.h b/include/skiboot.h >>> index b4bdf37795dd..7f3740965412 100644 >>> --- a/include/skiboot.h >>> +++ b/include/skiboot.h >>> @@ -75,7 +75,7 @@ struct debug_descriptor { >>> * low 4 bits driver (e.g. uart). >>> */ >>> u8 state_flags; /* various state flags - OPAL_BOOT_COMPLETE >>> etc */ >>> u16 reserved2; >>> - u32 reserved[2]; >>> + u64 log_buf_phys; /* Pointer to kernel log buffer */ >> >> >> May be increment DEBUG_DESC_VERSION as well? So that user can identify >> whether this field >> is supported or not? > > I considered this. Users running skiboot before this patch will see > zeros for the reserved[2] field. Users running skiboot after this > patch will either see zeroes, or see the field populated with the > memory address. Given that there's no change in behaviour, I don't > think there's any value in bumping the field. That's correct. Only issue is its difficult to differentiate between old OPAL vs kernel failed to register.. Not a big deal.. upto you. -Vasant
diff --git a/include/skiboot.h b/include/skiboot.h index b4bdf37795dd..7f3740965412 100644 --- a/include/skiboot.h +++ b/include/skiboot.h @@ -75,7 +75,7 @@ struct debug_descriptor { * low 4 bits driver (e.g. uart). */ u8 state_flags; /* various state flags - OPAL_BOOT_COMPLETE etc */ u16 reserved2; - u32 reserved[2]; + u64 log_buf_phys; /* Pointer to kernel log buffer */ /* Memory console */ u64 memcons_phys;
Signed-off-by: Joel Stanley <joel@jms.id.au> --- We could bump the version instead, but I don't think there's any call for that. Another option would be to re-use some of the memcons fields that are currently only used by FSP. A third option would be to use this field to link to yet another debug structure to allow for future flexibility. --- include/skiboot.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)