Implement OEM mechanism to handle xyz.openbmc_project.Condition.HostFirmware interface

[Andrew Geissler]

> On Sep 10, 2021, at 6:34 AM, Thu Nguyen <thu at amperemail.onmicrosoft.com> wrote:
> 
> Hi Andrew,
> 
> 
> Please see my comments:
> 
> 
> Thanks.
> 
> Thu Nguyen.
> 
> On 09/09/2021 22:42, Andrew Geissler wrote:
>> 
>>> On Sep 8, 2021, at 8:54 PM, Thang Nguyen <thang at amperemail.onmicrosoft.com> wrote:
>>> 
>>> Hi,
>>> 
>>> Let me explain more detail about our cases:
>>> 
>>> - Our system uses a GPIO called FW_BOOT_OK to detect if the Host is currently ON or OFF. The Host firmware set this GPIO when the first core initialized.
>>> 
>>> - We have no problem in Host State with power control. But the problem is in the case of BMC rebooted while the Host is ON.
>>> 
>>> - Before the commit https://github.com/openbmc/phosphor-state-manager/commit/0d1c3f1f9329c853677f0581287afef83eeea0f0, phosphor-reset-host-check at .service  is used to check and update Host State in case of BMC rebooted. But after that commit, the service file was removed. This makes no target service to update the Host State and the host check is fail at https://github.com/openbmc/phosphor-state-manager/blob/0a675215d6a6d2eb13e030ba0f618a4691de58d4/host_check.cpp#L109.
>>> 
>>> We would like to ask for your idea on how should we implement for the Host check when BMC is rebooted?
>> Hi Thang. Yeah, the reason for moving the logic directly into phosphor-host-state
>> is we had a window where the host state would say off (default) even when the
>> host was actually on. The other service would run and update it to the correct
>> value but there was a window where external clients would see an incorrect
>> state. So since we don’t want to report an invalid state, I needed the logic
>> within the app itself on startup.
>> 
>> I think you’re on the right path here. The design is to implement the
>> xyz.openbmc_project.Condition.HostFirmware object and support the read
>> of the CurrentFirmwareCondition property. Based on your GPIO state, you’d
>> respond accordingly to the read. That way the state-manager code will just
>> work as-is.
>> 
>> On where to put this code… So far we’ve put it in the area that is doing the logic,
>> like PLDM and IPMI. Since this is really just a GPIO read, I’m not sure the best
>> place. I’d be interested if anyone on the list has some thoughts. Could host it
>> outside of openbmc and just pull in via a recipe.
>> 
>> I’d entertain a subdirectory in phosphor-state-manager with this small
>> app (to host the interface you’ll want a c++ app) and service to run it.
>> We could just enable it via a meson/compile flag. It seems like it could
>> be fairly generic and something that other system owners could utilize.
> 
> So you mean we can add the code in subdirectory in phosphor-state-manager code.

Yes

> And the code have to generic enough to be reused in others systems and should include compile flag to enable/disable it.

Yes. We’ll just treat it as a sub-package within the state-manager bitbake recipe
and users can pull it in if they want it.

> 
> This code will response the host state base on the GPIO pins status.
> 
>> 
>> Please take a look at https://github.com/openbmc/docs/blob/master/designs/device-tree-gpio-naming.md
>> We’d want the GPIO utilized here to have a standard name so others
>> could potentially make use of this logic.
> 
> In the specs, I don't see any sections mention about the host GPIOs.

I was hoping you could name the GPIO’s in the dts so they could be generic
and then others who want to make use of your function could just use the
same names in their dts.

So you’d make a proposed change to that document. A new “Host Status”
section. Something like host0-status, host1-status, …

Then ideally we could avoid the need for the json file below and the code
just looks for the GPIO’s using libgpiod. Although if they really can have
different polarities, that may be an issue.

> 
> So I think I will use the GPIO configuration file host_gpios.json with below format.
> 
> {
>   "host_state":{
>     "host_0":[
>       {
>         "KEY": 48,
>         "Polarity": "High"
>       },
>       {
>         "KEY": 49,
>         "Polarity": "Low"
>       }
>     ],
>     "host_1":[
>       {
>         "KEY": 202,
>         "Polarity": "Low"
>       },
>       {
>         "KEY": 203,
>         "Polarity": "High"
>       }
>     ]
>   }
> }
> 
> The host_state fields will contain the GPIO settings to verify the running state of the hosts.
> 
> I will support multi-host setting. For each host, I will also support identify the host state thru one or some GPIO pin status.
> 
>> 
>> Andrew
>> 
>>> 
>>> Thanks,
>>> 
>>> Thang Q. Nguyen
>>> 
>>> On 08/09/2021 20:19, Thu Nguyen wrote:
>>>> Dear Geissonator,
>>>> 
>>>> 
>>>> After commit https://github.com/openbmc/phosphor-state-manager/commit/0d1c3f1f9329c853677f0581287afef83eeea0f0
>>>> 
>>>> when BMC boots up, phosphor-host-state directly checks  the host state thru interface xyz.openbmc_project.Condition.HostFirmware.
>>>> 
>>>> It does not check the existing of /run/openbmc/host@%d-on as before.
>>>> 
>>>> 
>>>> I plan to implement "oem mechanism" to handle the interface xyz.openbmc_project.Condition.HostFirmware.
>>>> 
>>>> Which will use the GPIO interface to update the host state. I researched the code handle this interface in phosphor-host-ipmi and pldm.
>>>> 
>>>> I wonder which repo should I upstream the code? Currently, we don't have any OEM repo in github to upstream the code.
>>>> 
>>>> Do you have any idea to handle interface in bash scripts?
>>>> 
>>>> 
>>>> Regards.
>>>> 
>>>> Thu Nguyen.
>>>> 
>>>> 
>>>> 
>>>> 
>>>> 
>>>> 
>>>>