[edk2-devel] [edk2-discuss] PLDM messages via MCTP over KCS

Thu Aug 31 15:52:46 UTC 2023

[AMD Official Use Only - General]

But wait, wee my another comment below,

> -----Original Message-----
> From: Chang, Abner
> Sent: Thursday, August 31, 2023 11:42 PM
> To: devel at edk2.groups.io; aladyshev22 at gmail.com
> Cc: discuss at edk2.groups.io
> Subject: RE: [edk2-devel] [edk2-discuss] PLDM messages via MCTP over KCS
>
>
>
> > -----Original Message-----
> > From: devel at edk2.groups.io <devel at edk2.groups.io> On Behalf Of
> > Konstantin Aladyshev via groups.io
> > Sent: Thursday, August 31, 2023 10:57 PM
> > To: Chang, Abner <Abner.Chang at amd.com>
> > Cc: discuss at edk2.groups.io; devel at edk2.groups.io
> > Subject: Re: [edk2-devel] [edk2-discuss] PLDM messages via MCTP over KCS
> >
> > Caution: This message originated from an External Source. Use proper
> caution
> > when opening attachments, clicking links, or responding.
> >
> >
> > > (I don see what is the response header for MCTP KCS in spec though, does
> it
> > mention the KCS response?).
> >
> > The spec doesn't explicitly mention that the format of a send and
> > response packets differ. So I assume it is the same and it is
> > described at the "Figure 1 – MCTP over KCS Packet Format"
> >
> (https://www.dmtf.org/sites/default/files/standards/documents/DSP0254_1
> > .0.0.pdf)
> > Therefore the format of a response would look like this:
> > ```
> > MANAGEABILITY_MCTP_KCS_HEADER
> > (https://github.com/tianocore/edk2-
> >
> platforms/blob/master/Features/ManageabilityPkg/Include/Library/Managea
> > bilityTransportMctpLib.h)
> > MCTP_TRANSPORT_HEADER
> >
> (https://github.com/tianocore/edk2/blob/master/MdePkg/Include/Industry
> > Standard/Mctp.h)
> > MCTP_MESSAGE_HEADER
> >
> (https://github.com/tianocore/edk2/blob/master/MdePkg/Include/Industry
> > Standard/Mctp.h)
> > < response data>
> What do you see the KCS response from BMC? You probably right as the
> header and trailer are labeled in different colors 😊. Could you please enable
> the debug message to capture it?
>
> > PEC
> >            (Probably we need to define MANAGEABILITY_MCTP_KCS_TRAILER)
> > ```
> We have MANAGEABILITY_MCTP_KCS_HEADER defined but no
> MANAGEABILITY_MCTP_KCS_TRAILER as it is hardcoded to one byte.
> If the KCS response is PEC, then yes, we can create
> MANAGEABILITY_MCTP_KCS_TRAILER for KCS transport interface.

In the implementation, PEC is calculated in MCTP protocol and send through KCS as the KCS packet trailer. So, when we send the MCTP request through KCS, KCS shouldn't respond the PEC to upper stack, right? I still think the response should be the KCS completion code. The debug message from your end may help to clarify this as your BMC has the MCTP KCS implementation.

Thanks
Abner

>
> >
> > So in the "KcsTransportSendCommand"
> > (https://github.com/tianocore/edk2-
> > platforms/blob/14553d31c72afa7289f6a2555b6e91f4f715a05a/Features/
> >
> ManageabilityPkg/Library/ManageabilityTransportKcsLib/Common/KcsComm
> > on.c#L414)
> > we can check if we transfer is MCTP (based on
> > "TransportToken->ManagebilityProtocolSpecification == MCTP" like
> > you've suggested) and handle response accordingly.
> >
> > But which headers should we check in this function? Only
> >
> MANAGEABILITY_MCTP_KCS_HEADER/MANAGEABILITY_MCTP_KCS_TRAILER
> > ?
> Yes, only check header and trailer for transport interface.
>
> >  What about MCTP_TRANSPORT_HEADER/MCTP_MESSAGE_HEADER? Do
> we
> > need to
> > check them here as well? Or do we need to check them somewhere upper
> > the call stack?
> We should leave this to MCTP protocol driver as this is belong to protocol
> layer, the upper layer stack.
>
> Thanks
> Abner
>
> >
> > Best regards,
> > Konstantin Aladyshev
> >
> > On Thu, Aug 31, 2023 at 7:59 AM Chang, Abner <Abner.Chang at amd.com>
> > wrote:
> > >
> > > [AMD Official Use Only - General]
> > >
> > > Hi Aladyshev,
> > >
> > > > -----Original Message-----
> > > > From: Konstantin Aladyshev <aladyshev22 at gmail.com>
> > > > Sent: Wednesday, August 30, 2023 11:09 PM
> > > > To: Chang, Abner <Abner.Chang at amd.com>
> > > > Cc: discuss at edk2.groups.io; devel at edk2.groups.io
> > > > Subject: Re: [edk2-discuss] PLDM messages via MCTP over KCS
> > > >
> > > > Caution: This message originated from an External Source. Use proper
> > caution
> > > > when opening attachments, clicking links, or responding.
> > > >
> > > >
> > > > Hi!
> > > >
> > > > I've started to implement MCTP over KCS binding for the libmctp
> > > > (https://github.com/openbmc/libmctp) and test it with the current code
> > > > in the ManageabilityPkg.
> > > >
> > > > I was able successfully send the MCTP packet to the BMC, but right now
> > > > I'm having some troubles with receiving the answer back.
> > > >
> > > > I think I've found some bug in the `KcsTransportSendCommand` code.
> > > >
> > > > After it sends data over KCS in expects a responce starting with a
> > > > 'IPMI_KCS_RESPONSE_HEADER'
> > > > https://github.com/tianocore/edk2-
> > > >
> > platforms/blob/14553d31c72afa7289f6a2555b6e91f4f715a05a/Features/
> > > >
> >
> ManageabilityPkg/Library/ManageabilityTransportKcsLib/Common/KcsComm
> > > > on.c#L476
> > > >
> > > > Isn't it wrong, assuming that the right header in case of MCTP should
> > > > be 'MANAGEABILITY_MCTP_KCS_HEADER' ?
> > > >
> > > > I guess the 'IpmiHelperCheckCompletionCode' check after the data
> > > > receive is also not relevant for the MCTP.
> > >
> > >
> > > This is something I don’t really sure as I can't verify the response payload
> > because our BMC doesn't have the code to handle MCTP over KCS
> command.
> > However it is appreciated if community can help to verify this. As I can
> > remember, I can see the return KCS status is 0xC1, the invalid command.
> Thus I
> > think if we do a MCTP over KCS, the first response is still KCS response
> header.
> > > This is not what do you see on the BCM it does support MCTP over KCS? If
> > so, then I would like to have your help to correct this code.
> > >
> > > >
> > > > Since 'ManageabilityTransportKcsLib' can be used both for IPMI and
> > > > MCTP, how should we deal with this?
> > > If KcsCommon.c, we can have different code path for the given protocol
> > GUID. e.g., if (TransportToken->ManagebilityProtocolSpecification == MCTP).
> > > Then skip reading the KCS_REPOSNSE_HEADER or to read the
> > MCTP_RESPONSE_HEADER (I don see what is the response header for MCTP
> > KCS in spec though, does it mention the KCS response?).
> > >
> > > Thanks
> > > Abner
> > >
> > > >
> > > > Best regards,
> > > > Konstantin Aladyshev
> > > >
> > > > On Wed, Aug 23, 2023 at 5:18 AM Chang, Abner
> > <Abner.Chang at amd.com>
> > > > wrote:
> > > > >
> > > > > [AMD Official Use Only - General]
> > > > >
> > > > > Please see my answers inline.
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: discuss at edk2.groups.io <discuss at edk2.groups.io> On Behalf
> Of
> > > > > > Konstantin Aladyshev via groups.io
> > > > > > Sent: Wednesday, August 23, 2023 1:54 AM
> > > > > > To: Chang, Abner <Abner.Chang at amd.com>
> > > > > > Cc: discuss at edk2.groups.io; devel at edk2.groups.io
> > > > > > Subject: Re: [edk2-discuss] PLDM messages via MCTP over KCS
> > > > > >
> > > > > > Caution: This message originated from an External Source. Use proper
> > > > caution
> > > > > > when opening attachments, clicking links, or responding.
> > > > > >
> > > > > >
> > > > > > Thanks for the answer!
> > > > > >
> > > > > > I was a little bit confused about the part, that in the same package I
> > > > > > actually need to provide different library implementations for the
> > > > > > same 'ManageabilityTransportLib', thanks for the clarification!
> > > > > > I think your DSC example should go into the package documentation.
> > > > > Yes, this is a good idea.  I will update it.
> > > > >
> > > > > >
> > > > > > As for me, I'm working with the OpenBMC distribution
> > > > > > (https://github.com/openbmc/openbmc) and my goal is to transfer
> > data
> > > > > > from the BIOS to the BMC via MCTP/PLDM.
> > > > > > Currently there is no solution for the MCTP over KCS binding in Linux,
> > > > > > so I need to add this support:
> > > > > > - either to the MCTP userspace library
> > > > > > (https://github.com/openbmc/libmctp) [old OpenBMC way, but
> > probably
> > > > > > easier]
> > > > > > - or to the MCTP kernel binding
> > > > > > (https://github.com/torvalds/linux/tree/master/drivers/net/mctp)
> > > > > > [modern mctp Linux driver approach]
> > > > > >
> > > > > > Both don't sound like an easy task, so can I ask, what MC (i.e.
> > > > > > management controller) device and firmware do you use on the other
> > > > > > side of the MCTP KCS transmissions?
> > > > >
> > > > > We use OpenBMC as well, but as you mention there are some missing
> > pieces
> > > > to fully support manageability between host and BMC.
> > > > > We don’t have code to handle MCTP IPMI either, the edk2
> > ManageabilityPkg
> > > > provides the framework while MCTP/PLDM/KCS implementation
> provides
> > a
> > > > sample other than IPMI/KCS to prove the flexibility of ManageabilityPkg.
> > > > > Actually, MCTP over KCS is not supported in our BMC firmware yet,
> thus
> > > > BMC just returns the invalid command. However, the transport
> framework
> > > > has been verified to make sure the implementation works fine as expect.
> > > > > We need help from community to provide more manageability
> protocols
> > and
> > > > transport interface libraries to this package.
> > > > >
> > > > > >
> > > > > > You've also mentioned PLDM SMBIOS, isn't it covered by the
> > > > > > https://github.com/tianocore/edk2-
> > > > > >
> > > >
> >
> platforms/blob/master/Features/ManageabilityPkg/Universal/PldmSmbiosTr
> > > > > > ansferDxe/PldmSmbiosTransferDxe.c
> > > > > > ?
> > > > > Ah hah, yes I forget I upstream it.
> > > > >
> > > > > Please just feel free to send patch to make more functionalities to this
> > > > package.
> > > > > Thanks
> > > > > Abner
> > > > >
> > > > > >
> > > > > >
> > > > > > Best regards,
> > > > > > Konstantin Aladyshev
> > > > > >
> > > > > > On Tue, Aug 22, 2023 at 7:26 PM Chang, Abner
> > > > <Abner.Chang at amd.com>
> > > > > > wrote:
> > > > > > >
> > > > > > > [AMD Official Use Only - General]
> > > > > > >
> > > > > > > Hi Aladyshev,
> > > > > > > We use library class to specify the desire transport interface for the
> > > > > > management protocol, such as MCTP, PLDM and IPMI. This way we
> can
> > > > > > flexibly support any transport interface for the management protocol.
> > > > > > >
> > > > > > > Here is the example of using ManageabilityPkg, which is PLDM over
> > MCTP
> > > > > > over KCS.
> > > > > > >
> ManageabilityPkg/Universal/IpmiProtocol/Dxe/IpmiProtocolDxe.inf
> > {
> > > > > > >     <LibraryClasses>
> > > > > > >
> > > > > >
> > > >
> >
> ManageabilityTransportLib|ManageabilityPkg/Library/ManageabilityTranspor
> > > > > > tKcsLib/Dxe/DxeManageabilityTransportKcs.inf
> > > > > > >   }
> > > > > > >
> > ManageabilityPkg/Universal/MctpProtocol/Dxe/MctpProtocolDxe.inf {
> > > > > > >     <LibraryClasses>
> > > > > > >
> > > > > >
> > > >
> >
> ManageabilityTransportLib|ManageabilityPkg/Library/ManageabilityTranspor
> > > > > > tKcsLib/Dxe/DxeManageabilityTransportKcs.inf
> > > > > > >   }
> > > > > > >
> > ManageabilityPkg/Universal/PldmProtocol/Dxe/PldmProtocolDxe.inf {
> > > > > > >     <LibraryClasses>
> > > > > > >
> > > > > >
> > > >
> >
> ManageabilityTransportLib|ManageabilityPkg/Library/ManageabilityTranspor
> > > > > > tMctpLib/Dxe/DxeManageabilityTransportMctp.inf
> > > > > > >   }
> > > > > > >
> > > > > > > So you can implement ManageabilityTransport library for either
> > industry
> > > > > > standard or proprietary implementation for the specific management
> > > > > > protocol.
> > > > > > >
> > > > > > > BTW, We do have PLDM SMBIOS over MCTP implementation but
> not
> > > > > > upstream yet.
> > > > > > >
> > > > > > > Hope this information helps.
> > > > > > > Thanks
> > > > > > > Abner
> > > > > > >
> > > > > > >
> > > > > > > > -----Original Message-----
> > > > > > > > From: discuss at edk2.groups.io <discuss at edk2.groups.io> On
> > Behalf Of
> > > > > > > > Konstantin Aladyshev via groups.io
> > > > > > > > Sent: Tuesday, August 22, 2023 7:00 PM
> > > > > > > > To: discuss <discuss at edk2.groups.io>; devel at edk2.groups.io
> > > > > > > > Subject: [edk2-discuss] PLDM messages via MCTP over KCS
> > > > > > > >
> > > > > > > > Caution: This message originated from an External Source. Use
> > proper
> > > > > > caution
> > > > > > > > when opening attachments, clicking links, or responding.
> > > > > > > >
> > > > > > > >
> > > > > > > > Hi!
> > > > > > > >
> > > > > > > > I'm trying to build `ManageabilityPkg` from the edk2-platforms
> > > > > > > >  repo to issue PLDM messages via MCTP over KCS. Is it possible
> with
> > > > > > > > the current code? I see all the building blocks, but have trouble
> > > > > > > > putting it all together.
> > > > > > > >
> > > > > > > > The main question that bothers me is what implementation should
> I
> > set
> > > > > > > > for the `ManageabilityTransportLib`?
> > > > > > > > By default it is set to dummy `BaseManageabilityTransportNull.inf`
> > > > > > > > (https://github.com/tianocore/edk2-
> > > > > > > >
> > > > > >
> > > >
> > platforms/blob/master/Features/ManageabilityPkg/ManageabilityPkg.dsc).
> > > > > > > >
> > > > > > > > On one case to get PLDM via MCTP it looks that I need to set it to
> > > > > > > > `DxeManageabilityTransportMctp.inf`
> > > > > > > > ManageabilityTransportLib|
> > <...>/DxeManageabilityTransportMctp.inf
> > > > > > > > (https://github.com/tianocore/edk2-
> > > > > > > >
> > > > > >
> > > >
> >
> platforms/blob/master/Features/ManageabilityPkg/Library/ManageabilityTra
> > > > > > > > nsportMctpLib/Dxe/DxeManageabilityTransportMctp.inf)
> > > > > > > >
> > > > > > > > But on the other case if I want MCTP over KCS I need to set it to
> > > > > > > > `DxeManageabilityTransportKcs.inf`
> > > > > > > > ManageabilityTransportLib|
> <...>/DxeManageabilityTransportKcs.inf
> > > > > > > > (https://github.com/tianocore/edk2-
> > > > > > > >
> > > > > >
> > > >
> >
> platforms/blob/master/Features/ManageabilityPkg/Library/ManageabilityTra
> > > > > > > > nsportKcsLib/Dxe/DxeManageabilityTransportKcs.inf)
> > > > > > > >
> > > > > > > > What is the right way to resolve this?
> > > > > > > >
> > > > > > > > There are no platforms in the repo that actually implement
> > PLDM/MCTP
> > > > > > > > functionality, so there is no example that I can use as a reference.
> > > > > > > >
> > > > > > > >
> > > > > > > > Best regards,
> > > > > > > > Konstantin Aladyshev
> > > > > > > >
> > > > > > > >
> > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > >
> >
> >
> > 
> >

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#108194): https://edk2.groups.io/g/devel/message/108194
Mute This Topic: https://groups.io/mt/100897530/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-