| Message ID | 20190616171024.22799-22-hegdevasant@linux.vnet.ibm.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | MPIPL support | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/apply_patch | success | Successfully applied on branch master (dbf27b6c4af84addb36bd3be34f96580aba9c873) |
| snowpatch_ozlabs/snowpatch_job_snowpatch-skiboot | fail | Test snowpatch/job/snowpatch-skiboot on branch master |
| snowpatch_ozlabs/snowpatch_job_snowpatch-skiboot-dco | success | Signed-off-by present |
Vasant Hegde's on June 17, 2019 3:10 am: > Post dump process, kernel sends FREE_PRESERVE_MEMEORY notification > to OPAL. OPAL will clear metadata section and tags. > > Signed-off-by: Vasant Hegde <hegdevasant@linux.vnet.ibm.com> > --- > core/opal-dump.c | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/core/opal-dump.c b/core/opal-dump.c > index e53991784..555908114 100644 > --- a/core/opal-dump.c > +++ b/core/opal-dump.c > @@ -309,6 +309,16 @@ static int64_t opal_mpipl_update(enum mpipl_ops ops, > prlog(PR_NOTICE, "Payload unregistered for MPIPL\n"); > break; > case OPAL_MPIPL_FREE_PRESERVED_MEMORY: > + /* Clear tags */ > + memset(&mpipl_tags, 0, (sizeof(u64) * MAX_MPIPL_TAGS)); > + max_tags = 0; > + /* Release memory */ > + free(mpipl_opal_data); > + mpipl_opal_data = NULL; > + /* Clear MDRT table */ > + memset((void *)MDRT_TABLE_BASE, 0, MDRT_TABLE_SIZE); > + /* Set MDRT count to max allocated count */ > + ntuple_mdrt->act_cnt = MDRT_TABLE_SIZE / sizeof(struct mdrt_table); Any particular reason you add this here in this patch rather than where the call type was defined? Thanks, Nick
On 06/28/2019 07:17 AM, Nicholas Piggin wrote: > Vasant Hegde's on June 17, 2019 3:10 am: >> Post dump process, kernel sends FREE_PRESERVE_MEMEORY notification >> to OPAL. OPAL will clear metadata section and tags. >> >> Signed-off-by: Vasant Hegde <hegdevasant@linux.vnet.ibm.com> >> --- >> core/opal-dump.c | 10 ++++++++++ >> 1 file changed, 10 insertions(+) >> >> diff --git a/core/opal-dump.c b/core/opal-dump.c >> index e53991784..555908114 100644 >> --- a/core/opal-dump.c >> +++ b/core/opal-dump.c >> @@ -309,6 +309,16 @@ static int64_t opal_mpipl_update(enum mpipl_ops ops, >> prlog(PR_NOTICE, "Payload unregistered for MPIPL\n"); >> break; >> case OPAL_MPIPL_FREE_PRESERVED_MEMORY: >> + /* Clear tags */ >> + memset(&mpipl_tags, 0, (sizeof(u64) * MAX_MPIPL_TAGS)); >> + max_tags = 0; >> + /* Release memory */ >> + free(mpipl_opal_data); >> + mpipl_opal_data = NULL; >> + /* Clear MDRT table */ >> + memset((void *)MDRT_TABLE_BASE, 0, MDRT_TABLE_SIZE); >> + /* Set MDRT count to max allocated count */ >> + ntuple_mdrt->act_cnt = MDRT_TABLE_SIZE / sizeof(struct mdrt_table); > > Any particular reason you add this here in this patch rather than where > the call type was defined? I have added patch based on dump workflow instead of adding everything in one patch. (Prep work -> OPAL dump registration -> kernel API for registration -> handle assert part -> trigger dump -> Post processing -> Invalidate dump). I thought this will make it easy for others to understand. -Vasant
Vasant Hegde's on June 28, 2019 7:48 pm: > On 06/28/2019 07:17 AM, Nicholas Piggin wrote: >> Vasant Hegde's on June 17, 2019 3:10 am: >>> Post dump process, kernel sends FREE_PRESERVE_MEMEORY notification >>> to OPAL. OPAL will clear metadata section and tags. >>> >>> Signed-off-by: Vasant Hegde <hegdevasant@linux.vnet.ibm.com> >>> --- >>> core/opal-dump.c | 10 ++++++++++ >>> 1 file changed, 10 insertions(+) >>> >>> diff --git a/core/opal-dump.c b/core/opal-dump.c >>> index e53991784..555908114 100644 >>> --- a/core/opal-dump.c >>> +++ b/core/opal-dump.c >>> @@ -309,6 +309,16 @@ static int64_t opal_mpipl_update(enum mpipl_ops ops, >>> prlog(PR_NOTICE, "Payload unregistered for MPIPL\n"); >>> break; >>> case OPAL_MPIPL_FREE_PRESERVED_MEMORY: >>> + /* Clear tags */ >>> + memset(&mpipl_tags, 0, (sizeof(u64) * MAX_MPIPL_TAGS)); >>> + max_tags = 0; >>> + /* Release memory */ >>> + free(mpipl_opal_data); >>> + mpipl_opal_data = NULL; >>> + /* Clear MDRT table */ >>> + memset((void *)MDRT_TABLE_BASE, 0, MDRT_TABLE_SIZE); >>> + /* Set MDRT count to max allocated count */ >>> + ntuple_mdrt->act_cnt = MDRT_TABLE_SIZE / sizeof(struct mdrt_table); >> >> Any particular reason you add this here in this patch rather than where >> the call type was defined? > > I have added patch based on dump workflow instead of adding everything in one patch. > (Prep work -> OPAL dump registration -> kernel API for registration -> handle > assert part -> > trigger dump -> Post processing -> Invalidate dump). > > I thought this will make it easy for others to understand. > I don't think it helps. Preferences how to break up patches are diverse and there doesn't seem to be a strong agreement, so I don't pick on it too much. Except that they should bisect and more or less result in a working system each step. So returning success from an API that is not yet implemented doesn't look right. Thanks, Nick
On 06/28/2019 04:46 PM, Nicholas Piggin wrote: > Vasant Hegde's on June 28, 2019 7:48 pm: >> On 06/28/2019 07:17 AM, Nicholas Piggin wrote: >>> Vasant Hegde's on June 17, 2019 3:10 am: >>>> Post dump process, kernel sends FREE_PRESERVE_MEMEORY notification >>>> to OPAL. OPAL will clear metadata section and tags. >>>> >>>> Signed-off-by: Vasant Hegde <hegdevasant@linux.vnet.ibm.com> >>>> --- >>>> core/opal-dump.c | 10 ++++++++++ >>>> 1 file changed, 10 insertions(+) >>>> >>>> diff --git a/core/opal-dump.c b/core/opal-dump.c >>>> index e53991784..555908114 100644 >>>> --- a/core/opal-dump.c >>>> +++ b/core/opal-dump.c >>>> @@ -309,6 +309,16 @@ static int64_t opal_mpipl_update(enum mpipl_ops ops, >>>> prlog(PR_NOTICE, "Payload unregistered for MPIPL\n"); >>>> break; >>>> case OPAL_MPIPL_FREE_PRESERVED_MEMORY: >>>> + /* Clear tags */ >>>> + memset(&mpipl_tags, 0, (sizeof(u64) * MAX_MPIPL_TAGS)); >>>> + max_tags = 0; >>>> + /* Release memory */ >>>> + free(mpipl_opal_data); >>>> + mpipl_opal_data = NULL; >>>> + /* Clear MDRT table */ >>>> + memset((void *)MDRT_TABLE_BASE, 0, MDRT_TABLE_SIZE); >>>> + /* Set MDRT count to max allocated count */ >>>> + ntuple_mdrt->act_cnt = MDRT_TABLE_SIZE / sizeof(struct mdrt_table); >>> >>> Any particular reason you add this here in this patch rather than where >>> the call type was defined? >> >> I have added patch based on dump workflow instead of adding everything in one patch. >> (Prep work -> OPAL dump registration -> kernel API for registration -> handle >> assert part -> >> trigger dump -> Post processing -> Invalidate dump). >> >> I thought this will make it easy for others to understand. >> > > I don't think it helps. Preferences how to break up patches are > diverse and there doesn't seem to be a strong agreement, so I don't Yeah. There is no standard and everyone has their own style. > pick on it too much. Except that they should bisect and more or less > result in a working system each step. So returning success from an > API that is not yet implemented doesn't look right. I have made sure patches are bisect able. But you are right. Previous patch should throw UNSUPPORTED error instead of returning SUCCESS. Will fix it. -Vasant
diff --git a/core/opal-dump.c b/core/opal-dump.c index e53991784..555908114 100644 --- a/core/opal-dump.c +++ b/core/opal-dump.c @@ -309,6 +309,16 @@ static int64_t opal_mpipl_update(enum mpipl_ops ops, prlog(PR_NOTICE, "Payload unregistered for MPIPL\n"); break; case OPAL_MPIPL_FREE_PRESERVED_MEMORY: + /* Clear tags */ + memset(&mpipl_tags, 0, (sizeof(u64) * MAX_MPIPL_TAGS)); + max_tags = 0; + /* Release memory */ + free(mpipl_opal_data); + mpipl_opal_data = NULL; + /* Clear MDRT table */ + memset((void *)MDRT_TABLE_BASE, 0, MDRT_TABLE_SIZE); + /* Set MDRT count to max allocated count */ + ntuple_mdrt->act_cnt = MDRT_TABLE_SIZE / sizeof(struct mdrt_table); rc = OPAL_SUCCESS; break; default:
Post dump process, kernel sends FREE_PRESERVE_MEMEORY notification to OPAL. OPAL will clear metadata section and tags. Signed-off-by: Vasant Hegde <hegdevasant@linux.vnet.ibm.com> --- core/opal-dump.c | 10 ++++++++++ 1 file changed, 10 insertions(+)