Re: [coreboot] Inagua: Synchronize AMD/inagua mainboard.

[Marc Jones]

On Fri, Jan 27, 2012 at 7:43 AM, Wolfgang Kamp - datakamp
 wrote:
> Hello Marc,
>
> in the meantime I debugged my Ubuntu restart problem a little bit further.
>
> The issue seems to be hardware dependant because the Persimmon platform with T56N APU and A55E Hudson works fine and
> a similar platform (Qseven card) with T40E APU and Hudson A50E does not. I can use nearly the same coreboot build.
>
> Things getting strange in the file .../f14/Proc/Mem/NB/ON/mndcton.c function MemNStitchMemoryON
>
> if ((MemNGetBitFieldNb (NBPtr, BFCSBaseAddr0Reg + q) & 7) == 0) {  HERE WE FAIL!!, BFCSBaseAddr0Reg is not zero!!
>
> This leads later to
>
> PutEventLog (AGESA_FATAL, MEM_ERROR_NO_DIMM_FOUND_ON_SYSTEM, 0, 0, 0, 0, &NBPtr->MemPtr->StdHeader); HERE ALL ENDS!!!
>
> And here ends all with no memory.
>
> On Persimmon BFCSBaseAddr0Reg is zero so we are getting in the IF branch and things going on.
>
> BOOLEAN
> MemNStitchMemoryON (
>  IN OUT   MEM_NB_BLOCK *NBPtr
>  )
> {
>  UINT32 NxtCSBase;
>  UINT32 CurCSBase;
>  UINT32 CsSize;
>  UINT32 BiggestBank;
>  UINT8 p;
>  UINT8 q;
>  UINT8 BiggestDimm;
>  DIE_STRUCT *MCTPtr;
>  DCT_STRUCT *DCTPtr;
>  MCTPtr = NBPtr->MCTPtr;
>  DCTPtr = NBPtr->DCTPtr;
>
> DCTPtr->Timings.CsEnabled = 0;
>  NxtCSBase = 0;
>  for (p = 0; p < MAX_CS_PER_CHANNEL_ON; p++) {
>    BiggestBank = 0;
>    BiggestDimm = 0;
>    for (q = 0; q < MAX_CS_PER_CHANNEL_ON; q++) {
>      printk(BIOS_DEBUG, "q = %x, Timings.CsPresent = %x, Timings.CsTestFail = %x\n", q, DCTPtr->Timings.CsPresent, DCTPtr->Timings.CsTestFail); //kamod
>      if (((DCTPtr->Timings.CsPresent & ~DCTPtr->Timings.CsTestFail) & ((UINT16)1 << q)) != 0) {
>                printk(BIOS_DEBUG, "1st IF is true \n"); //kamod
>        if ((MemNGetBitFieldNb (NBPtr, BFCSBaseAddr0Reg + q) & 7) == 0) {  HERE WE FAIL!!, BFCSBaseAddr0Reg is not zero
>          // (CSEnable|Spare==1)bank is not enabled yet
>                printk(BIOS_DEBUG, "2nd IF is true \n"); //kamod
>                CsSize = MemNGetBitFieldNb (NBPtr, BFCSMask0Reg + (q >> 1));
>          printk(BIOS_DEBUG, "CsSize = %x \n", CsSize); //kamod
>          if (CsSize != 0) {
>            CsSize += ((UINT32)1 << 19);
>            CsSize &= 0xFFF80000;
>          }
>          if (CsSize > BiggestBank) {
>            BiggestBank = CsSize;
>            BiggestDimm = q;
>          }
>        }
>      }
>    }
>
> if (BiggestBank != 0) {
>      CurCSBase = NxtCSBase;
>      CurCSBase |= ((UINT32)1 << BFCSEnable);
>      NxtCSBase += BiggestBank;
>      if ((BiggestDimm & 1) != 0) {
>        if ((DCTPtr->Timings.DimmMirrorPresent & (1 << (BiggestDimm >> 1))) != 0) {
>          CurCSBase |= ((UINT32)1 << BFOnDimmMirror);
>        }
>      }
>      MemNSetBitFieldNb (NBPtr, BFCSBaseAddr0Reg + BiggestDimm, CurCSBase);
>      DCTPtr->Timings.CsEnabled |= (1 << BiggestDimm);
>    }
>    if ((DCTPtr->Timings.CsTestFail & ((UINT16)1 << p)) != 0) {
>      MemNSetBitFieldNb (NBPtr, (BFCSBaseAddr0Reg + p), (UINT32)1 << BFTestFail);
>    }
>  }
>
> if (NxtCSBase != 0) {
>    DCTPtr->Timings.DctMemSize = NxtCSBase >> 8;    // Scale base address from [39:8] to [47:16]
>    NBPtr->MCTPtr->NodeMemSize += NBPtr->DCTPtr->Timings.DctMemSize;
>    NBPtr->MCTPtr->NodeSysLimit = NBPtr->MCTPtr->NodeMemSize - 1;
>  } else {
>        printk(BIOS_DEBUG, "NxtCSBase = %x \n", NxtCSBase); //kamod
>        PutEventLog (AGESA_FATAL, MEM_ERROR_NO_DIMM_FOUND_ON_SYSTEM, 0, 0, 0, 0, &NBPtr->MemPtr->StdHeader); HERE ALL ENDS!!!
>    SetMemError (AGESA_FATAL, MCTPtr);
>  }
>
> return (BOOLEAN) (MCTPtr->ErrCode < AGESA_FATAL);
> }
>
> What I have done was to change the way Ubuntu restarts. If I set reboot=acpi restart works.

That is strange. This isn't something I have seen before. I have been
working on making the acpi tables better, but we have a ways to go.
Following that function through, is it failing a PCI register read?
How is Linux doing the reset? Maybe there is some missing southbridge
configuration on that type of reset that leaves the CPU is a funny
state?

Marc
> Thanks
>
> Wolfgang
>
> -----Ursprüngliche Nachricht-----
> Von: coreboot-bounces+wmkamp=[e-mail] [mailto:coreboot-bounces+wmkamp=[e-mail]] Im Auftrag von Wolfgang Kamp - datakamp
> Gesendet: Mittwoch, 25. Januar 2012 17:24
> An: [e-mail]
> Betreff: Re: [coreboot] Change in coreboot[master]: Inagua: Synchronize AMD/inagua mainboard.
>
> Hello Marc,
>
> my debugging shows that SPD reading is ok after Warmstart. This can not be the problem.
> The error check reports 7 errors when system hangs. Is there a problem with cache as RAM?
> Is the buffer of SPD data corrupt?
> I don't know what code here really does.
> Have you an idea what I can do next?
>
> mmflow.c
>
> //----------------------------------------------------------------
>  // Check for errors and return
>  //----------------------------------------------------------------
>  AGESA_TESTPOINT (TpProcMemEnd, &MemPtr->StdHeader);
>  for (Die = 0; Die < DieCount; Die++) {
>    if (NBPtr[Die].MCTPtr->ErrCode > Retval) {
>      Retval = NBPtr[Die].MCTPtr->ErrCode;
>    }
>  }
>  printk(BIOS_DEBUG, "TpProcMemEnd Retval = 0x%x \n", Retval);
>  return Retval;
> }
>
> Regards,
>
> Wolfgang
>
> -----Ursprüngliche Nachricht-----
> Von: Marc Jones [mailto:[e-mail]]
> Gesendet: Dienstag, 24. Januar 2012 19:15
> An: Wolfgang Kamp - datakamp
> Cc: [e-mail]
> Betreff: Re: [coreboot] Change in coreboot[master]: Inagua: Synchronize AMD/inagua mainboard.
>
> On Tue, Jan 24, 2012 at 5:38 AM, Wolfgang Kamp - datakamp  wrote:
> > Hi Marc,
> >
> > DIMM address and i2c address are ok.
> > Please look at the log. I think the SB800 is unaccessable.
> >
> > Regards
> >
> > Wolfgang
>
> The sb800 is accessible, it is fetching rom and initializing devices that it sees:
>
> sb800_enable() PCI: 00:11.0 [1002/4390] enabled
> sb800_enable() PCI: 00:12.0 [1002/4397] ops
> PCI: 00:12.0 [1002/4397] enabled
> sb800_enable() PCI: Static device PCI: 00:12.1 not found, disabling it.
> sb800_enable() PCI: 00:12.2 [1002/4396] ops
> PCI: 00:12.2 [1002/4396] enabled
> sb800_enable() PCI: 00:13.0 [1002/4397] ops
> PCI: 00:13.0 [1002/4397] enabled
> sb800_enable() PCI: Static device PCI: 00:13.1 not found, disabling it.
> sb800_enable() PCI: 00:13.2 [1002/4396] ops
> PCI: 00:13.2 [1002/4396] enabled
> sb800_enable() sm_init().
> .
> PCI: 00:14.0 [1002/4385] enabled
> sb800_enable() PCI: Static device PCI: 00:14.1 not found, disabling it.
> sb800_enable() hda enabled
> PCI: 00:14.2 [1002/4383] ops
> PCI: 00:14.2 [1002/4383] enabled
>
> !4.0 is the smbus device, so that is enabled. I think that you need to see what in the spd read fails. Also, see if you can read it earlier in the init. There could be a different device setting that causes the problem. Check that te SMbus enable is set as expected. Check that the PM registers that set the iobase are accessible.
>
> Marc
> > -----Ursprüngliche Nachricht-----
> > Von: Marc Jones [mailto:[e-mail]]
> > Gesendet: Freitag, 20. Januar 2012 18:46
> > An: Wolfgang Kamp - datakamp
> > Cc: [e-mail]
> > Betreff: Re: [coreboot] Change in coreboot[master]: Inagua: Synchronize AMD/inagua mainboard.
> >
> > On Fri, Jan 20, 2012 at 9:45 AM, Wolfgang Kamp - datakamp
> >  wrote:
> > > Hello Marc,
> > >
> > > I reviewed the code and it looks good.
> > > But real testing shows an issue with soft restart (UBUNTU).
> > > The southbridge seems to hang. Coreboot stops because it could not read the SPI ROM of DIMM Module.
> > > Please see logs.
> > > The cold start log also reports errors but will successful boot Ubuntu.
> > >
> > > Regards
> > >
> > > Wolfgang
> >
> > Woflgang,
> >
> > The ASSERTs in the passing case are non-critical failures for early
> > heap use. These are AGESA bugs and have been reported to AMD, but they
> > are not critical.
> >
> > As you said, The bad failure is this one:
> >
> > EventLog:       EventClass = 7, EventInfo = 4011c00.
> >        Param1 = 0, Param2 = 0.
> >        Param3 = 0, Param4 = 0.
> >
> > Which is the SPD problem...
> > #define MEM_ERROR_NO_DIMM_FOUND_ON_SYSTEM 0x04011C00    ///< No DIMMs
> > have been found
> >
> > Can you check what happens in AmdMemoryReadSPD(), in dimmSpd.c? Does
> > it check the correct dimm address? Is the i2c io address set
> > correctly?
> >
> > Thanks,
> > Marc
> > > -----Ursprüngliche Nachricht-----
> > > Von: gerrit code review [mailto:[e-mail]]
> > > Gesendet: Freitag, 20. Januar 2012 00:52
> > > An: Wolfgang Kamp - datakamp
> > > Cc: Kerry Sheh
> > > Betreff: Change in coreboot[master]: Inagua: Synchronize AMD/inagua mainboard.
> > >
> > > From Marc Jones :
> > >
> > > Hello Wolfgang Kamp,
> > >
> > > I'd like you to do a code review.  Please visit
> > >
> > > http://review.coreboot.org/542
> > >
> > > to review the following change.
> > >
> > > Change subject: Inagua: Synchronize AMD/inagua mainboard.
> > > .....................................................................
> > >
> > > Inagua: Synchronize AMD/inagua mainboard.
> > >
> > > AMD/persimmon mainboard code is derived from AMD/inagua mainbard.
> > > Persimmom update a lot in the last few month, sync these modification to inagua.
> > >
> > > Change-Id: Ia038e5a2b9550fe81bb075f31e30b98354758e9e
> > > Signed-off-by: Kerry Sheh 
> > > Signed-off-by: Kerry Sheh
> > >
> > > M src/mainboard/amd/inagua/BiosCallOuts.c
> > > M src/mainboard/amd/inagua/BiosCallOuts.h
> > > M src/mainboard/amd/inagua/Kconfig
> > > M src/mainboard/amd/inagua/Makefile.inc
> > > M src/mainboard/amd/inagua/OptionsIds.h
> > > M src/mainboard/amd/inagua/PlatformGnbPcie.c
> > > D src/mainboard/amd/inagua/acpi/ssdt2.asl
> > > D src/mainboard/amd/inagua/acpi/ssdt3.asl
> > > D src/mainboard/amd/inagua/acpi/ssdt4.asl
> > > D src/mainboard/amd/inagua/acpi/ssdt5.asl
> > > M src/mainboard/amd/inagua/acpi_tables.c
> > > M src/mainboard/amd/inagua/agesawrapper.c
> > > M src/mainboard/amd/inagua/agesawrapper.h
> > > M src/mainboard/amd/inagua/buildOpts.c
> > > M src/mainboard/amd/inagua/devicetree.cb
> > > M src/mainboard/amd/inagua/dimmSpd.c
> > > M src/mainboard/amd/inagua/dsdt.asl
> > > M src/mainboard/amd/inagua/fadt.c
> > > M src/mainboard/amd/inagua/get_bus_conf.c
> > > M src/mainboard/amd/inagua/irq_tables.c
> > > M src/mainboard/amd/inagua/mainboard.c
> > > M src/mainboard/amd/inagua/mptable.c
> > > M src/mainboard/amd/inagua/platform_cfg.h
> > > M src/mainboard/amd/inagua/romstage.c
> > > 24 files changed, 249 insertions(+), 717 deletions(-)
> > >
> > > git pull ssh://review.coreboot.org:29418/coreboot
> > > refs/changes/42/542/2
> > > --
> > > To view, visit http://review.coreboot.org/542 To unsubscribe, visit
> > > http://review.coreboot.org/settings
> > >
> > > Gerrit-MessageType: newchange
> > > Gerrit-Change-Id: Ia038e5a2b9550fe81bb075f31e30b98354758e9e
> > > Gerrit-PatchSet: 2
> > > Gerrit-Project: coreboot
> > > Gerrit-Branch: master
> > > Gerrit-Owner: Kerry Sheh 
> > > Gerrit-Reviewer: Kerry Sheh 
> > > Gerrit-Reviewer: Wolfgang Kamp 
> > > Gerrit-Reviewer: build bot (Jenkins)