Re: Propose a new application for reading DIMM SPD directly

["Zbigniew, Lukwinski" <zbigniew.lukwinski@linux.intel.com>]

On 2/17/2022 4:59 AM, Michael Shen wrote:
>> I see. So the app will just read SPD non-volatile content and provide it
>> for user, e.g. over DBus, right?
> Yes, the plan is over DBus.
Ok. Thanks.
>> Are you going to access DIMM periodically? It seems that it shall be
>> enough to access DIMMs once per ac cycle/dc cycle. And just return SPD
>> ownership to the CPU for the rest of time.
> I think reading the SPD once per ac/dc cycle is enough like you said.
> But if we want BMC to monitor the DIMM temp instead of CPU, then BMC
> can't return the SPD ownership since the temp needs to be read
> periodically.

Got it!

>
> On Wed, Feb 16, 2022 at 4:39 AM Zbigniew, Lukwinski
>
> <zbigniew.lukwinski@linux.intel.com> wrote:
>> On 2/15/2022 2:50 AM, Michael Shen wrote:
>>>> What about CLTT? Switching MUX to the BMC makes CPU not able to get DIMM
>>>> temperature. Are you assuming here this feature is enabled in BMC FW?
>>> CPU can still monitor the DIMM temp by reading the MR4 register and
>>> trigger the DIMM throttling if needed. So I think the CLTT will not be
>>> affected.
>>> If CPU needs something more than MR4 register provided, BMC can assist
>>> it in another separate daemon like Benjamin mentioned.
>> Got it.
>>
>>> On Tue, Feb 15, 2022 at 6:17 AM Benjamin Fair <benjaminfair@google.com> wrote:
>>>> On Fri, 11 Feb 2022 at 13:21, Zbigniew, Lukwinski
>>>> <zbigniew.lukwinski@linux.intel.com> wrote:
>>>>> On 2/11/2022 1:40 AM, Michael Shen wrote:
>>>>>> On Thu, Feb 10, 2022 at 6:45 AM Ed Tanous <edtanous@google.com> wrote:
>>>>>>> On Wed, Feb 9, 2022 at 1:14 PM Patrick Williams <patrick@stwcx.xyz> wrote:
>>>>>>>> On Wed, Feb 09, 2022 at 12:20:00PM -0800, Ed Tanous wrote:
>>>>>>>>> On Wed, Feb 9, 2022 at 11:56 AM Patrick Williams <patrick@stwcx.xyz> wrote:
>>>>>>>>>> On Tue, Feb 08, 2022 at 04:23:12PM +0800, Michael Shen wrote:
>>>>>>>>>>> On Tue, Feb 8, 2022 at 3:11 PM Patrick Williams <patrick@stwcx.xyz> wrote:
>>>>>>>>>>>> On Tue, Feb 08, 2022 at 01:10:37PM +0800, Michael Shen wrote:
>>>>>>>>>>> BIOS owns the MUX select pin and it can decide who owns the SPD(I2C/I3C) bus.
>>>>>>>>>>>    From my understanding, BIOS only needs to read SPD during the POST stage.
>>>>>>>>>>> For the rest of time, BIOS will hand over the SPD bus to BMC.
>>>>>>>>>> That seems like it might work.  You'll have to deal with the time when the BIOS
>>>>>>>>>> has the mux in the BMC code somehow.  Ideally I'd ask for the mux select to also
>>>>>>>>>> be fed to the BMC as an input GPIO so that you can differentiate between "we
>>>>>>>>>> don't own the mux" and "all the devices are NAKing us".
>>>>>>>>> This seems like a nitty gritty design detail that's best handled in
>>>>>>>>> code when we review it.  I think the important bit here is that there
>>>>>>>>> are paths where this could work without a significant design issue.
>>>>>>>> Just one subtlety.  I wouldn't expect this, necessarily, to be in _our_ design
>>>>>>>> and/or code, except that we'd want to document the GPIO line like we do all
>>>>>>>> others.  I was trying to hint that "if I were involved in this hardware design,
>>>>>>>> I'd ask for...".  If you leave it out, I'm sure it'll work _most_ of the time
>>>>>>>> just fine and it'll be your problem to debug it when it doesn't.
>>>>>>> Understood.
>>>>>> Thanks for all your suggestions. I will keep them in mind during implementation.
>>>>> What about CLTT? Switching MUX to the BMC makes CPU not able to get DIMM
>>>>> temperature. Are you assuming here this feature is enabled in BMC FW?
>>>> BMC could assist with CLTT, but since this is CPU-specific it would
>>>> belong in a separate daemon. That daemon could get the relevant
>>>> temperatures over D-Bus using the standard sensor interface, so I
>>>> don't think it should affect the design for this component.
>>>>
>>>>> Are you going to support DCPMM as well? If so, there is another problem
>>>>> since switching MUX to BMC you brakes NVDIMM related FW/SW running on
>>>>> Host OS.
>>>> There are no plans currently for supporting NVDIMMs, just DDR5 at
>>>> first as Michael mentioned, and possibly other DDR versions in the
>>>> future.
>> I see. So the app will just read SPD non-volatile content and provide it
>> for user, e.g. over DBus, right?
>>
>> Are you going to access DIMM periodically? It seems that it shall be
>> enough to access DIMMs once per ac cycle/dc cycle. And just return SPD
>> ownership to the CPU for the rest of time.
>>
>>>>>>>>>> You should take a look at what is already existing in fru-device (part of
>>>>>>>>>> entity-manager repository).  This is already doing this for IPMI-format EEPROM
>>>>>>>>>> data.  We should be able to replicate/enhance this code, in the same repository,
>>>>>>>>>> to handle SPD format.
>>>>>>>>> I am not sure if it's a good idea to put it into the entity-manager
>>>>>>>>> repo. As you said EM
>>>>>>>>> is designed for IPMI-format EEPROM. Adding another parser into that
>>>>>>>>> repo may violate
>>>>>>>>> EM's design.
>>>>>>>> I'm not sure why it would be an issue.  Hopefully one of the maintainers of that
>>>>>>>> repo can weigh in.  I wouldn't expect "parsing only IPMI-format EEPROMs" is a
>>>>>>>> design but just the current state of implementation.
>>>>>>> So long as it can function properly in its current design, i have no
>>>>>>> problem with FruDevice adding more parsing types.  In fact, there's
>>>>>>> already patchsets out to add Linkedins proprietary fru type to
>>>>>>> FruDevice, so in terms of design, Patricks request seems reasonable.
>>>>>> Got it. Then I will push the code to EM.