As I've said there is nothing like "KCS completion code" in the MCTP
over KCS binding specification
(https://www.dmtf.org/sites/default/files/standards/documents/DSP0254_1.0.0.pdf).
The response packet should have the same structure as a request. I.e.
a packet with MANAGEABILITY_MCTP_KCS_HEADER and PEC.

Currently I'm writing "MCTP over KCS" binding for the OpenBMC libmctp
project. So I can send whatever I want, I don't think my output would
be any useful to you. But I've asked this question in the community
and they also confirmed that the response packet has the same
structure. 
(https://discord.com/channels/775381525260664832/778790638563885086/1146782595334549554)

Now I'm a little bit confused about the `KcsTransportSendCommand`
function. It has the following arguments for the function output:
```
OUT UINT8                                       *ResponseData OPTIONAL,
IN  OUT UINT32                               *ResponseDataSize OPTIONAL
```
Should we include MCTP_TRANSPORT_HEADER/MCTP_MESSAGE_HEADER to this
output or not?

Best regards,
Konstantin Aladyshev

On Thu, Aug 31, 2023 at 6:52 PM Chang, Abner <abner.ch...@amd.com> wrote:
>
> [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@edk2.groups.io; aladyshe...@gmail.com
> > Cc: disc...@edk2.groups.io
> > Subject: RE: [edk2-devel] [edk2-discuss] PLDM messages via MCTP over KCS
> >
> >
> >
> > > -----Original Message-----
> > > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
> > > Konstantin Aladyshev via groups.io
> > > Sent: Thursday, August 31, 2023 10:57 PM
> > > To: Chang, Abner <abner.ch...@amd.com>
> > > Cc: disc...@edk2.groups.io; devel@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.ch...@amd.com>
> > > wrote:
> > > >
> > > > [AMD Official Use Only - General]
> > > >
> > > > Hi Aladyshev,
> > > >
> > > > > -----Original Message-----
> > > > > From: Konstantin Aladyshev <aladyshe...@gmail.com>
> > > > > Sent: Wednesday, August 30, 2023 11:09 PM
> > > > > To: Chang, Abner <abner.ch...@amd.com>
> > > > > Cc: disc...@edk2.groups.io; devel@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.ch...@amd.com>
> > > > > wrote:
> > > > > >
> > > > > > [AMD Official Use Only - General]
> > > > > >
> > > > > > Please see my answers inline.
> > > > > >
> > > > > > > -----Original Message-----
> > > > > > > From: disc...@edk2.groups.io <disc...@edk2.groups.io> On Behalf
> > Of
> > > > > > > Konstantin Aladyshev via groups.io
> > > > > > > Sent: Wednesday, August 23, 2023 1:54 AM
> > > > > > > To: Chang, Abner <abner.ch...@amd.com>
> > > > > > > Cc: disc...@edk2.groups.io; devel@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.ch...@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: disc...@edk2.groups.io <disc...@edk2.groups.io> On
> > > Behalf Of
> > > > > > > > > Konstantin Aladyshev via groups.io
> > > > > > > > > Sent: Tuesday, August 22, 2023 7:00 PM
> > > > > > > > > To: discuss <disc...@edk2.groups.io>; devel@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 (#108196): https://edk2.groups.io/g/devel/message/108196
Mute This Topic: https://groups.io/mt/100897530/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-


Reply via email to