[coreboot-gerrit] Change in ...coreboot[master]: soc/intel/skylake: Remove redundent mca_configure() in ramstage

[Pratikkumar V Prajapati (Code Review)]

Pratikkumar V Prajapati has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32553 )

Change subject: soc/intel/skylake: Remove redundent mca_configure() in ramstage
......................................................................

Patch Set 2:
> Patch Set 2:
> > Patch Set 2:
> > > > Patch Set 2:
> > > > > > > Patch Set 2:
> > > > > > >
> > > > > > > > Patch Set 2:
> > > > > > > >
> > > > > > > > > Patch Set 2:
> > > > > > > > >
> > > > > > > > > > Patch Set 2:
> > > > > > > > > >
> > > > > > > > > > (1 comment)
> > > > > > > > >
> > > > > > > > >
> > > > > > > > > On AML SOCs (for Nocturne), we have seen MCEs being
> > >
> > > dumped
> > > > > > after booting to kernel. After debugging i found MCAs were
> > >
> > > not
> > > > > > cleared before booting the kernel so i pushed those 2
> > >
> > > patches and
> > > > > > with that kernel was not reporting MCEs.
> > > > > > > > >
> > > > > > > > > (I am trying to find the crosbug but i am not able to
> > > > >
> > > > > access
> > > > > > them, seems like i have some access issues)
> > > > > > > > >
> > > > > > > > > @Subrata, Have you checked on SKL/KBL/AML platforms
> > >
> > > that
> > > > > MCEs
> > > > > > are not dumped if you dont clear them before booting?
> > > > > > > > >
> > > > > > > > > Clearing from cpu.c might be too early, i think. in my
> > > > > > understanding, Clearing MCEs at that point is needed only if
> > >
> > > we
> > > > > > need to enable SGX, as when we reload microcode for SGX we
> > >
> > > need
> > > > > > have MCA clean.
> > > > > > > >
> > > > > > > > https://partnerissuetracker.corp.google.com/issues/79783714
> > > > >
> > > > > is
> > > > > > the tracker.
> > > > > > >
> > > > > > > I don't see a lot of details on the bug as to why the
> > > > > > mca_configure was required. Also, if MCEs were cleared
> > >
> > > early, are
> > > > > > you saying there were more errors that happened after that?
> > > > > >
> > > > > > there were MCEs before giving control to Depthcharge
> > > > >
> > > > > i can take AR to verify on SKL/KBL/AML
> > > >
> > > > i have tested on KBL (eve) and AML (nocturne) platform and i
> > >
> > > don't see any machine check issue.
> > >
> > > I hope tested the AML sku mentioned in the tracker. MCEs were
> > > reported by Google and after patched Google verified as well and
> > > that's why the patches were +2'd and merged.
> >
> > Hi Furquan, Do you have some time to check this CL on your side AML platform ? as Pratik suggested in past some one from Google has verified the need to mca_configue(NULL) ?
>
> +Nick and +Caveh to help test this.

BTW, how much time are we saving after removing mca_configure from finalize.c? (what could be any other reason to remove from  finalize.c?)
What if we remove mca_config from cpu.c and keep in finalize.c?