public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Konstantin Aladyshev" <aladyshev22@gmail.com>
To: "Chang, Abner" <Abner.Chang@amd.com>
Cc: "discuss@edk2.groups.io" <discuss@edk2.groups.io>,
	"devel@edk2.groups.io" <devel@edk2.groups.io>
Subject: Re: [edk2-devel] [edk2-discuss] PLDM messages via MCTP over KCS
Date: Thu, 31 Aug 2023 17:56:52 +0300	[thread overview]
Message-ID: <CACSj6VWJ5pb+Q--K=iWeamTdrT4qf0mNKu+uCbCiE20FbYLDQQ@mail.gmail.com> (raw)
In-Reply-To: <MN2PR12MB3966E0D354E8F46170F5FABEEAE5A@MN2PR12MB3966.namprd12.prod.outlook.com>

> (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/ManageabilityTransportMctpLib.h)
MCTP_TRANSPORT_HEADER
(https://github.com/tianocore/edk2/blob/master/MdePkg/Include/IndustryStandard/Mctp.h)
MCTP_MESSAGE_HEADER
(https://github.com/tianocore/edk2/blob/master/MdePkg/Include/IndustryStandard/Mctp.h)
< response data>
PEC
           (Probably we need to define MANAGEABILITY_MCTP_KCS_TRAILER)
```

So in the "KcsTransportSendCommand"
(https://github.com/tianocore/edk2-platforms/blob/14553d31c72afa7289f6a2555b6e91f4f715a05a/Features/ManageabilityPkg/Library/ManageabilityTransportKcsLib/Common/KcsCommon.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 ?
 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?

Best regards,
Konstantin Aladyshev

On Thu, Aug 31, 2023 at 7:59 AM Chang, Abner <Abner.Chang@amd.com> wrote:
>
> [AMD Official Use Only - General]
>
> Hi Aladyshev,
>
> > -----Original Message-----
> > From: Konstantin Aladyshev <aladyshev22@gmail.com>
> > Sent: Wednesday, August 30, 2023 11:09 PM
> > To: Chang, Abner <Abner.Chang@amd.com>
> > Cc: discuss@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.Chang@amd.com>
> > wrote:
> > >
> > > [AMD Official Use Only - General]
> > >
> > > Please see my answers inline.
> > >
> > > > -----Original Message-----
> > > > From: discuss@edk2.groups.io <discuss@edk2.groups.io> On Behalf Of
> > > > Konstantin Aladyshev via groups.io
> > > > Sent: Wednesday, August 23, 2023 1:54 AM
> > > > To: Chang, Abner <Abner.Chang@amd.com>
> > > > Cc: discuss@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.Chang@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@edk2.groups.io <discuss@edk2.groups.io> On Behalf Of
> > > > > > Konstantin Aladyshev via groups.io
> > > > > > Sent: Tuesday, August 22, 2023 7:00 PM
> > > > > > To: discuss <discuss@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 (#108191): https://edk2.groups.io/g/devel/message/108191
Mute This Topic: https://groups.io/mt/100897530/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



  reply	other threads:[~2023-08-31 14:57 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-08-22 11:00 [edk2-devel] PLDM messages via MCTP over KCS Konstantin Aladyshev
2023-08-22 16:26 ` [edk2-devel] [edk2-discuss] " Chang, Abner via groups.io
2023-08-22 17:53   ` Konstantin Aladyshev
2023-08-23  2:18     ` Chang, Abner via groups.io
2023-08-30 15:09       ` Konstantin Aladyshev
2023-08-31  4:59         ` Chang, Abner via groups.io
2023-08-31 14:56           ` Konstantin Aladyshev [this message]
2023-08-31 15:41             ` Chang, Abner via groups.io
2023-08-31 15:52               ` Chang, Abner via groups.io
2023-08-31 16:01                 ` Konstantin Aladyshev
2023-09-01  5:58                   ` Chang, Abner via groups.io
2023-09-08 12:56                     ` Konstantin Aladyshev
2023-09-21  2:31                       ` Chang, Abner via groups.io
2023-09-28 18:17                         ` Konstantin Aladyshev
2023-09-29  6:20                           ` Chang, Abner via groups.io
2023-10-04 11:52                           ` Chang, Abner via groups.io
2023-10-04 15:58                             ` Konstantin Aladyshev
2023-10-04 16:12                               ` Chang, Abner via groups.io
2023-10-04 17:57                                 ` Konstantin Aladyshev
2023-10-05  4:03                                   ` Chang, Abner via groups.io
2023-10-05  9:55                                     ` Konstantin Aladyshev
2023-10-05 12:19                                       ` Konstantin Aladyshev
2023-10-05 12:31                                         ` Konstantin Aladyshev
2023-10-05 15:18                                           ` Konstantin Aladyshev
2023-10-11  5:58                                             ` Chang, Abner via groups.io
2023-10-13 12:15                                               ` Konstantin Aladyshev
2023-10-14  8:06                                                 ` Chang, Abner via groups.io
2023-10-14  8:25                                                   ` Konstantin Aladyshev
2023-10-14  9:10                                                     ` Chang, Abner via groups.io
     [not found]                                                     ` <178DEE4D03C504AF.14388@groups.io>
2023-10-17  3:40                                                       ` Chang, Abner via groups.io
2023-10-17  8:54                                                         ` Konstantin Aladyshev
2023-10-17 10:39                                                           ` Chang, Abner via groups.io
     [not found]                                                             ` <CACSj6VV60bx3heCO+BnePXNxZTx3kD-+re1bm85MNP3+nr5j+A@mail.gmail.com>
     [not found]                                                               ` <CACSj6VUszX76Sn5F_LkDS5KjZurEnJ0YjnGiim+rJqyfKWEs2Q@mail.gmail.com>
     [not found]                                                                 ` <CACSj6VX3Lw71xp8H2=fiecvY0q4-O2SQe7-iky5QSdsj+OrG8Q@mail.gmail.com>
     [not found]                                                                   ` <MN2PR12MB3966B38ADEEA877FCE071F1AEAD5A@MN2PR12MB3966.namprd12.prod.outlook.com>
     [not found]                                                                     ` <MN2PR12MB3966C1AA13CBDABC626D34C8EAD5A@MN2PR12MB3966.namprd12.prod.outlook.com>
2023-10-18 17:14                                                                       ` Konstantin Aladyshev
2023-10-20  8:33                                                                         ` Chang, Abner via groups.io
2023-10-20 13:00                                                                           ` Konstantin Aladyshev
2023-10-22  0:40                                                                             ` Chang, Abner via groups.io

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CACSj6VWJ5pb+Q--K=iWeamTdrT4qf0mNKu+uCbCiE20FbYLDQQ@mail.gmail.com' \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox