<div dir="ltr">+OpenBMC mailing list<div><br></div><div>Hoang,</div><div><br></div><div>Thanks for the overview. I think that we would probably like to see your design for review - it sounds interesting and there have been other complaints regarding FRU parser.  If you want to provide more detail to the entire list I'm sure we will be interested to hear, and also very interested to take a look at your code and consider adoption.</div><div><br></div><div>Thanks!</div><div>Emily</div><div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Fri, Jan 25, 2019 at 1:23 AM Hoang Nguyen <<a href="mailto:hnguyen@amperecomputing.com">hnguyen@amperecomputing.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">





<div lang="EN-US" link="blue" vlink="purple">
<div class="m_-7908660954218574492WordSection1">
<p class="MsoNormal">Hi Emily,<u></u><u></u></p>
<p class="MsoNormal"><u></u> <u></u></p>
<p class="MsoNormal">FYI, now I don’t have too much time to develop new feature of IPMI@OpenBMC, but I can follow the IPMI@OpenBMC activity.<u></u><u></u></p>
<p class="MsoNormal">I’ll push the code for reviewing if you’re interesting in my design.<u></u><u></u></p>
<p class="MsoNormal"><u></u> <u></u></p>
<p class="MsoNormal">Firstly, I’ll overview about my design. And I’ll continue in detail if you agree.<u></u><u></u></p>
<p class="MsoNormal"><u></u> <u></u></p>
<p class="MsoNormal"><b>The package name</b>: FRU manager<u></u><u></u></p>
<p class="MsoNormal"><u></u> <u></u></p>
<p class="MsoNormal"><b>The reason for new package:<u></u><u></u></b></p>
<p class="MsoNormal">+ Current package doesn’t support Multirecord FRU parsing<u></u><u></u></p>
<p class="MsoNormal">+ Sometime, using “IPMI fru write…” command will crash the FRU EEPROM image -> nothing from D-Bus can inform this info<u></u><u></u></p>
<p class="MsoNormal"><u></u> <u></u></p>
<p class="MsoNormal"><b>New package structure:<u></u><u></u></b></p>
<p class="MsoNormal">FRU manager package has 2 parts: <b>Manager</b> and <b>Parser</b><u></u><u></u></p>
<p class="MsoNormal">+ <b>Manager</b>:<u></u><u></u></p>
<p class="MsoNormal">    - Initial the D-Bus interface + all properties of Chassis/Board/Product/Multirecord areas (with
<span style="color:red">default value</span>)<u></u><u></u></p>
<p class="MsoNormal">    - Provide the D-Bus methods for other packages (<b>Parser</b> as an example) to update the real data of FRU to D-Bus<u></u><u></u></p>
<p class="MsoNormal">    - Provide methods to update the FRU device directly (in case of FRU recovering)<u></u><u></u></p>
<p class="MsoNormal"><u></u> <u></u></p>
<p class="MsoNormal">+ <b>Parser</b>:<u></u><u></u></p>
<p class="MsoNormal">    - Access the FRU device directly to parse the FRU data and request to update these date to D-Bus (using
<b>Manager</b> supported methods)<u></u><u></u></p>
<p class="MsoNormal"><u></u> <u></u></p>
<p class="MsoNormal"><b>New services to handle new package:<u></u><u></u></b></p>
<p class="MsoNormal">Add more 2 services:<u></u><u></u></p>
<p class="MsoNormal">+ FRU manager service: start Manager part to handle all FRU request on D-Bus<u></u><u></u></p>
<p class="MsoNormal">+ FRU parser service: start Parser part to parse FRU data and update to D-Bus<u></u><u></u></p>
<p class="MsoNormal"><u></u> <u></u></p>
<p class="MsoNormal"><b>Improvement points:<u></u><u></u></b></p>
<p class="MsoNormal">+ Handle Multirecord data<u></u><u></u></p>
<p class="MsoNormal">+ In case of FRU data is not correct, the FRU parser service will be failed, all data of FRU on D-Bus are “default value”<u></u><u></u></p>
<p class="MsoNormal">   -> We can identify the issue and recovering the FRU image<u></u><u></u></p>
<p class="MsoNormal"><u></u> <u></u></p>
<p class="MsoNormal"><u></u> <u></u></p>
<p class="MsoNormal">Best regards,<u></u><u></u></p>
<p class="MsoNormal">Hoang<u></u><u></u></p>
<p class="MsoNormal"><u></u> <u></u></p>
<div style="border:none;border-top:solid #b5c4df 1.0pt;padding:3.0pt 0cm 0cm 0cm">
<p class="MsoNormal"><b><span style="font-size:12.0pt;color:black">From: </span></b><span style="font-size:12.0pt;color:black">Emily Shaffer <<a href="mailto:emilyshaffer@google.com" target="_blank">emilyshaffer@google.com</a>><br>
<b>Date: </b>Friday, January 25, 2019 at 7:11 AM<br>
<b>To: </b>Hien Pham <<a href="mailto:hpham@amperecomputing.com" target="_blank">hpham@amperecomputing.com</a>><br>
<b>Cc: </b>Hoang Nguyen <<a href="mailto:hnguyen@amperecomputing.com" target="_blank">hnguyen@amperecomputing.com</a>><br>
<b>Subject: </b>Re: ampere-openbmc/phosphor-host-ipmid<u></u><u></u></span></p>
</div></div></div><div lang="EN-US" link="blue" vlink="purple"><div class="m_-7908660954218574492WordSection1">
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<p class="MsoNormal">[NOTICE: This email originated from an external sender. Please be mindful of safe email handling and proprietary information protection practices.] ________________________________________________________________________________________________________________________
<u></u><u></u></p>
<div>
<div>
<p class="MsoNormal">Hien, Hoang, <u></u><u></u></p>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<div>
<p class="MsoNormal">Thanks so much for replying to my cold email :)<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<div>
<p class="MsoNormal">I want to take a look and understand better the approach you guys used. We've added more support to some of the FRU functionality you were missing - specifically multirecord FRU has seen development in the past month or two - but it looks
 like you made a broader change.  If you have the engineer time to do so, I think it'd be valuable for you to push either your code or a design proposal (which maybe you already have written) for the OpenBMC community to take a look at (via gerrit) - I'd like
 to get the rest of the community's feedback too.<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<div>
<p class="MsoNormal">It sounds like you aren't able to dedicate a lot of resources right now so I definitely understand if you don't want to put up a review; if you won't have the time, could you let me know? I'll ask the other IPMI maintainers to take a look
 at your approach too and we can decide if it's something we want to bring over ourselves.  (But it's more cohesive for the community if you're able to push the review from your end :) )<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<div>
<p class="MsoNormal">Thanks,<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">Emily<u></u><u></u></p>
</div>
</div>
<p class="MsoNormal"><u></u> <u></u></p>
<div>
<div>
<p class="MsoNormal">On Wed, Jan 23, 2019 at 11:53 PM Hien Pham <<a href="mailto:hpham@amperecomputing.com" target="_blank">hpham@amperecomputing.com</a>> wrote:<u></u><u></u></p>
</div>
<blockquote style="border:none;border-left:solid #cccccc 1.0pt;padding:0cm 0cm 0cm 6.0pt;margin-left:4.8pt;margin-right:0cm">
<div>
<div id="m_-7908660954218574492m_3349455740235281957divtagdefaultwrapper">
<p style="margin:0cm;margin-bottom:.0001pt"><span style="font-size:12.0pt;color:black">Dear Emily,<u></u><u></u></span></p>
<p style="margin:0cm;margin-bottom:.0001pt"><span style="font-size:12.0pt;color:black"><u></u> <u></u></span></p>
<p style="margin:0cm;margin-bottom:.0001pt"><span style="font-size:12.0pt;color:black">I'm Hien Plam (Ampere OpenBMC CLA manager).  Nice to hear that you care the change we did.<u></u><u></u></span></p>
<p style="margin:0cm;margin-bottom:.0001pt"><span style="font-size:12.0pt;color:black"><u></u> <u></u></span></p>
<p style="margin:0cm;margin-bottom:.0001pt"><span style="font-size:12.0pt;color:black">No worry, we committed to contribute back and willing to submit the change to the upstream anyway (if you see the change are applicable).  Please let us know if you are interesting
 in our FRU refactor (Hoang's explanation below).  If yes, we can process  next step to upstream the changes.<u></u><u></u></span></p>
<p style="margin:0cm;margin-bottom:.0001pt"><span style="font-size:12.0pt;color:black"><u></u> <u></u></span></p>
<p style="margin:0cm;margin-bottom:.0001pt"><span style="font-size:12.0pt;color:black">Thank and best regards,<u></u><u></u></span></p>
</div>
<div class="MsoNormal" align="center" style="text-align:center">
<hr size="0" width="100%" align="center">
</div>
<div id="m_-7908660954218574492m_3349455740235281957divRplyFwdMsg">
<p class="MsoNormal"><b><span style="color:black">From:</span></b><span style="color:black"> Hoang Nguyen<br>
<b>Sent:</b> Thursday, January 24, 2019 2:02:23 PM<br>
<b>To:</b> Emily Shaffer<br>
<b>Cc:</b> Hien Pham<br>
<b>Subject:</b> Re: ampere-openbmc/phosphor-host-ipmid</span> <u></u><u></u></p>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
</div>
</div>
<div>
<div>
<div>
<p class="m_-7908660954218574492m3349455740235281957xmsonormal">Hi Emily,<u></u><u></u></p>
<p class="m_-7908660954218574492m3349455740235281957xmsonormal"> <u></u><u></u></p>
<p class="m_-7908660954218574492m3349455740235281957xmsonormal">First at all, I’d like to inform that now we’re suspending the OpenBMC activity due to the business strategy.<u></u><u></u></p>
<p class="m_-7908660954218574492m3349455740235281957xmsonormal">But we’ll come back with OpenBMC soon (I’m
<b>sure</b> about it).<u></u><u></u></p>
<p class="m_-7908660954218574492m3349455740235281957xmsonormal"> <u></u><u></u></p>
<p class="m_-7908660954218574492m3349455740235281957xmsonormal">Regarding to your interesting of IPMI implementations (FRU interaction in phosphor-host-ipmid), I’ve not push these code because I’m using another design for the communication between IPMI implementation (phosphor-host-ipmid)
 and FRU access handler (ipmi-fru-parser).<u></u><u></u></p>
<p class="m_-7908660954218574492m3349455740235281957xmsonormal"> <u></u><u></u></p>
<p class="m_-7908660954218574492m3349455740235281957xmsonormal">At current design, I found some limitation, such as:<u></u><u></u></p>
<ul type="disc">
<li class="m_-7908660954218574492m3349455740235281957xmsolistparagraph">
ipmi-fru-parser cannot parse the multi-record area<u></u><u></u></li><li class="m_-7908660954218574492m3349455740235281957xmsolistparagraph">
ipmi-fru-parser takes many responsibilities: <u></u><u></u></li></ul>
<ul type="disc">
<ul type="circle">
<li class="m_-7908660954218574492m3349455740235281957xmsolistparagraph">
Handle FRU device<u></u><u></u></li><li class="m_-7908660954218574492m3349455740235281957xmsolistparagraph">
Implement IPMI FRU commands<u></u><u></u></li></ul>
</ul>
<ul type="disc">
<li class="m_-7908660954218574492m3349455740235281957xmsolistparagraph">
many modules can access the FRU EEPROM device directly<u></u><u></u></li></ul>
<p class="m_-7908660954218574492m3349455740235281957xmsonormal"> <u></u><u></u></p>
<p class="m_-7908660954218574492m3349455740235281957xmsonormal">So I changed the design, include:<u></u><u></u></p>
<ol start="1" type="1">
<li class="m_-7908660954218574492m3349455740235281957xmsolistparagraph">
Disable the ipmi-fru-parser<u></u><u></u></li><li class="m_-7908660954218574492m3349455740235281957xmsolistparagraph">
Create new module (like FRU manager package) to handle the FRU device.<u></u><u></u></li></ol>
<p class="m_-7908660954218574492m3349455740235281957xmsolistparagraph">Overview about this package, it can:<u></u><u></u></p>
<ul type="disc">
<li class="m_-7908660954218574492m3349455740235281957xmsolistparagraph">
Access (read/write) the FRU device directly.<u></u><u></u></li><li class="m_-7908660954218574492m3349455740235281957xmsolistparagraph">
Parse the multi-record area and public all info to D-Bus interface<u></u><u></u></li><li class="m_-7908660954218574492m3349455740235281957xmsolistparagraph">
Provide some methods (using D-Bus interface) to support the read/write FRU device request from other packages.<u></u><u></u></li></ul>
<ol start="3" type="1">
<li class="m_-7908660954218574492m3349455740235281957xmsolistparagraph">
Update all relate packages (phosphor-host-ipmid is example):<u></u><u></u></li></ol>
<ul type="disc">
<li class="m_-7908660954218574492m3349455740235281957xmsolistparagraph">
Stop accessing FRU device directly and start using FRU manager package by calling supported methods<u></u><u></u></li><li class="m_-7908660954218574492m3349455740235281957xmsolistparagraph">
Move IPMI FRU commands to phosphor-host-ipmid<u></u><u></u></li></ul>
<p class="m_-7908660954218574492m3349455740235281957xmsonormal"> <u></u><u></u></p>
<p class="m_-7908660954218574492m3349455740235281957xmsonormal">With this design, each package takes only one responsibility; prevent the issue of FRU device accessing from many packages.<u></u><u></u></p>
<p class="m_-7908660954218574492m3349455740235281957xmsonormal"> <u></u><u></u></p>
<p class="m_-7908660954218574492m3349455740235281957xmsonormal">Finally, because this solution is specific for our system, so I did not pushed it.<u></u><u></u></p>
<p class="m_-7908660954218574492m3349455740235281957xmsonormal">(but it always follow the common specification)<u></u><u></u></p>
<p class="m_-7908660954218574492m3349455740235281957xmsonormal"> <u></u><u></u></p>
<p class="m_-7908660954218574492m3349455740235281957xmsonormal">Best regards,<u></u><u></u></p>
<p class="m_-7908660954218574492m3349455740235281957xmsonormal">Hoang<u></u><u></u></p>
<p class="m_-7908660954218574492m3349455740235281957xmsonormal"> <u></u><u></u></p>
<div style="border:none;border-top:solid #b5c4df 1.0pt;padding:3.0pt 0cm 0cm 0cm">
<p class="m_-7908660954218574492m3349455740235281957xmsonormal"><b><span style="font-size:12.0pt;color:black">From:
</span></b><span style="font-size:12.0pt;color:black">Emily Shaffer <<a href="mailto:emilyshaffer@google.com" target="_blank">emilyshaffer@google.com</a>><br>
<b>Date: </b>Thursday, January 24, 2019 at 5:26 AM<br>
<b>To: </b>Hoang Nguyen <<a href="mailto:hnguyen@amperecomputing.com" target="_blank">hnguyen@amperecomputing.com</a>><br>
<b>Subject: </b>ampere-openbmc/phosphor-host-ipmid</span><u></u><u></u></p>
</div>
<div>
<p class="m_-7908660954218574492m3349455740235281957xmsonormal"> <u></u><u></u></p>
</div>
<p class="m_-7908660954218574492m3349455740235281957xmsonormal">[NOTICE: This email originated from an external sender. Please be mindful of safe email handling and proprietary information protection practices.] ________________________________________________________________________________________________________________________
<u></u><u></u></p>
<div>
<div>
<p class="m_-7908660954218574492m3349455740235281957xmsonormal">Hi Hoang, <u></u><u></u></p>
<div>
<p class="m_-7908660954218574492m3349455740235281957xmsonormal"> <u></u><u></u></p>
</div>
<div>
<p class="m_-7908660954218574492m3349455740235281957xmsonormal">I'm a maintainer of the IPMI stack for OpenBMC (<a href="http://github.com/openbmc" target="_blank">github.com/openbmc</a>) and came across your forked project today.<u></u><u></u></p>
</div>
<div>
<p class="m_-7908660954218574492m3349455740235281957xmsonormal"> <u></u><u></u></p>
</div>
<div>
<p class="m_-7908660954218574492m3349455740235281957xmsonormal">I noticed that Ampere executed a CLA with OpenBMC in July 2018, but I don't see that you're contributing your IPMI implementations back to OpenBMC. It actually looks like in general Ampere has written some code that
 we want, especially regarding FRU interaction in phosphor-host-ipmid, so I'm curious why it is that you haven't pushed any code back up to us.<u></u><u></u></p>
</div>
<div>
<p class="m_-7908660954218574492m3349455740235281957xmsonormal"> <u></u><u></u></p>
</div>
<div>
<p class="m_-7908660954218574492m3349455740235281957xmsonormal">Can I facilitate getting Ampere to contribute back to the OpenBMC community?<u></u><u></u></p>
</div>
<div>
<p class="m_-7908660954218574492m3349455740235281957xmsonormal"> <u></u><u></u></p>
</div>
<div>
<p class="m_-7908660954218574492m3349455740235281957xmsonormal">Thanks,<u></u><u></u></p>
</div>
<div>
<p class="m_-7908660954218574492m3349455740235281957xmsonormal">Emily Shaffer<br>
(Github/Freenode: nasamuffin)<u></u><u></u></p>
</div>
</div>
</div>
</div>
</div>
</div>
</blockquote>
</div>
</div>
</div></div></blockquote></div></div></div>