openbmc.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
* Implement OEM mechanism to handle xyz.openbmc_project.Condition.HostFirmware interface
@ 2021-09-08 13:19 Thu Nguyen
  2021-09-09  1:54 ` Thang Nguyen
  0 siblings, 1 reply; 7+ messages in thread
From: Thu Nguyen @ 2021-09-08 13:19 UTC (permalink / raw)
  To: openbmc, geissonator

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.








^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Implement OEM mechanism to handle xyz.openbmc_project.Condition.HostFirmware interface
  2021-09-08 13:19 Implement OEM mechanism to handle xyz.openbmc_project.Condition.HostFirmware interface Thu Nguyen
@ 2021-09-09  1:54 ` Thang Nguyen
  2021-09-09 15:42   ` Andrew Geissler
  0 siblings, 1 reply; 7+ messages in thread
From: Thang Nguyen @ 2021-09-09  1:54 UTC (permalink / raw)
  To: openbmc

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@.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?


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.
>
>
>
>
>
>
>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Implement OEM mechanism to handle xyz.openbmc_project.Condition.HostFirmware interface
  2021-09-09  1:54 ` Thang Nguyen
@ 2021-09-09 15:42   ` Andrew Geissler
  2021-09-10 11:34     ` Thu Nguyen
  0 siblings, 1 reply; 7+ messages in thread
From: Andrew Geissler @ 2021-09-09 15:42 UTC (permalink / raw)
  To: Thang Nguyen; +Cc: openbmc



> On Sep 8, 2021, at 8:54 PM, Thang Nguyen <thang@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@.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.

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.

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.
>> 
>> 
>> 
>> 
>> 
>> 
>> 


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Implement OEM mechanism to handle xyz.openbmc_project.Condition.HostFirmware interface
  2021-09-09 15:42   ` Andrew Geissler
@ 2021-09-10 11:34     ` Thu Nguyen
  2021-09-10 21:57       ` Andrew Geissler
  0 siblings, 1 reply; 7+ messages in thread
From: Thu Nguyen @ 2021-09-10 11:34 UTC (permalink / raw)
  To: Andrew Geissler, Thang Nguyen; +Cc: openbmc

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@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@.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.

And the code have to generic enough to be reused in others systems and 
should include compile flag to enable/disable 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.

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.
>>>
>>>
>>>
>>>
>>>
>>>
>>>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Implement OEM mechanism to handle xyz.openbmc_project.Condition.HostFirmware interface
  2021-09-10 11:34     ` Thu Nguyen
@ 2021-09-10 21:57       ` Andrew Geissler
  2021-09-11  3:07         ` Thu Nguyen
  0 siblings, 1 reply; 7+ messages in thread
From: Andrew Geissler @ 2021-09-10 21:57 UTC (permalink / raw)
  To: Thu Nguyen; +Cc: openbmc, Thang Nguyen



> On Sep 10, 2021, at 6:34 AM, Thu Nguyen <thu@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@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@.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.
>>>> 
>>>> 
>>>> 
>>>> 
>>>> 
>>>> 
>>>> 


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Implement OEM mechanism to handle xyz.openbmc_project.Condition.HostFirmware interface
  2021-09-10 21:57       ` Andrew Geissler
@ 2021-09-11  3:07         ` Thu Nguyen
  2021-09-14 21:16           ` Andrew Geissler
  0 siblings, 1 reply; 7+ messages in thread
From: Thu Nguyen @ 2021-09-11  3:07 UTC (permalink / raw)
  To: Andrew Geissler; +Cc: openbmc, Thang Nguyen

Hi Andrew,

Please see my comments.

Thanks.

Thu Nguyen.

On 11/09/2021 04:57, Andrew Geissler wrote:
>
>> On Sep 10, 2021, at 6:34 AM, Thu Nguyen <thu@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@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@.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, …

If we use the dts definition, we will be limited by the number of GPIO 
pins which are used to

identify the host state and also their polarities.

Because the GPIOs polarities are depended on the platform which designed 
by hardware team.

We can't ask them to fix the GPIOs polarities, I think.

>
> 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.
>
Yes. That is why I suggest the GPIOs setting.
>> 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.
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Implement OEM mechanism to handle xyz.openbmc_project.Condition.HostFirmware interface
  2021-09-11  3:07         ` Thu Nguyen
@ 2021-09-14 21:16           ` Andrew Geissler
  0 siblings, 0 replies; 7+ messages in thread
From: Andrew Geissler @ 2021-09-14 21:16 UTC (permalink / raw)
  To: Thu Nguyen; +Cc: openbmc, Thang Nguyen



> On Sep 10, 2021, at 10:07 PM, Thu Nguyen <thu@amperemail.onmicrosoft.com> wrote:
> 
> Hi Andrew,
> 
> Please see my comments.
> 
> Thanks.
> 
> Thu Nguyen.
> 
> On 11/09/2021 04:57, Andrew Geissler wrote:
>> 
>>> On Sep 10, 2021, at 6:34 AM, Thu Nguyen <thu@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@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@.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, …
> 
> If we use the dts definition, we will be limited by the number of GPIO pins which are used to
> identify the host state and also their polarities.

This confuses me a bit. A device tree (dts) is machine specific file in
the kernel which represents your system. You can have as many GPIO’s
defined as you have in your system.

> 
> Because the GPIOs polarities are depended on the platform which designed by hardware team.
> We can't ask them to fix the GPIOs polarities, I think.

Digging a bit further, the dts allows you to define GPIO’s as ACTIVE_HIGH or 
ACTIVE_LOW, I think you can just utilize this to ensure uniformity on reading
them and knowing if the host is running or not.

This way userspace just uses libgpiod to look for the appropriate GPIO
using a standard name, and returns the on/off based on the reading.

> 
>> 
>> 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.
>> 
> Yes. That is why I suggest the GPIOs setting.
>>> 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.


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2021-09-14 21:17 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-08 13:19 Implement OEM mechanism to handle xyz.openbmc_project.Condition.HostFirmware interface Thu Nguyen
2021-09-09  1:54 ` Thang Nguyen
2021-09-09 15:42   ` Andrew Geissler
2021-09-10 11:34     ` Thu Nguyen
2021-09-10 21:57       ` Andrew Geissler
2021-09-11  3:07         ` Thu Nguyen
2021-09-14 21:16           ` Andrew Geissler

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).