[edk2-devel] [edk2-discuss] PLDM messages via MCTP over KCS
Konstantin Aladyshev
aladyshev22 at gmail.com
Sat Oct 14 08:25:44 UTC 2023
Sorry I got confused a little bit by your answer.
You were talking about PLDM TIDs (Terminus IDs) and I was asking about
MCTP EIDs. Don't they like different concepts? Or did you mean that we
need to have a mapping of TIDs<-->EIDs in PldmProtocolLib?
> I reviewed these patches just now, that looks good to me. However this way we don’t have the history in the mailing list.
If the code looks good to you, I'll try to properly format all our
patches and send them to the mailing list next week.
> BTW, are you an individual contributor or you are the representative of a firm?
As a part of my job I develop UEFI and BMC firmware. But all the
open-source work that I do I perform on my own free time. So you can
count me as an individual contributor)
Best regards,
Konstantin Aladyshev
On Sat, Oct 14, 2023 at 11:06 AM Chang, Abner <Abner.Chang at amd.com> wrote:
>
> [AMD Official Use Only - General]
>
> > -----Original Message-----
> > From: Konstantin Aladyshev <aladyshev22 at gmail.com>
> > Sent: Friday, October 13, 2023 8:16 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.
> >
> >
> > Hi Chang!
> >
> > Thanks for the response!
> > Is it ok that because of 1) we would need to include MCTP
> > SrcEID/DstEID in each function of the PLDM_SMBIOS_TRANSFER_PROTOCOL?
> >
> > https://github.com/tianocore/edk2-
> > platforms/blob/06f6274d56422084281886fad447ab117fd0e487/Features/
> > ManageabilityPkg/Universal/PldmSmbiosTransferDxe/PldmSmbiosTransferDx
> > e.c#L459
> > ```
> > EDKII_PLDM_SMBIOS_TRANSFER_PROTOCOL_V1_0
> > mPldmSmbiosTransferProtocolV10 = {
> > GetSmbiosStructureTableMetaData,
> > SetSmbiosStructureTableMetaData,
> > GetSmbiosStructureTable,
> > SetSmbiosStructureTable,
> > GetSmbiosStructureByType,
> > GetSmbiosStructureByHandle
> > };
> > ```
> For the PLDM_SMBIOS_TRANSFER_PROTOCOL, we can add a API in EDKII_PLDM_SMBIOS_TRANSFER_PROTOCOL_V1_0 for setting the PLDM terminus. As adding Terminus STID and DTID seems a burden to this protocol.
> So, we can do this,
>
> Define SetPldmTerminus (UINT8 SourceTid, UINT8 DestinationTid) API and put it in the first member of EDKII_PLDM_SMBIOS_TRANSFER_PROTOCOL_V1_0.
> Define PldmSetTerminus in PldmProtocolLib and invoked by SetPldmTerminus API, this way each PLDM application driver (e.g., SMBIOS) has its own PLDM terminus ID in the PldmProtocolLib instance. We still define PCDs for these two PLDM ID which is similar to MCTP protocol. Use PCD instead in case SourceTid and DestinationTid had never been set in PldmProtocolLib.
>
> EDKII_PLDM_SMBIOS_TRANSFER_PROTOCOL_V1_0 =
> mPldmSmbiosTransferProtocolV10 = {
> SetPldmTerminus,
> GetSmbiosStructureTableMetaData,
> SetSmbiosStructureTableMetaData,
> GetSmbiosStructureTable,
> SetSmbiosStructureTable,
> GetSmbiosStructureByType,
> GetSmbiosStructureByHandle
> };
>
> >
> > I've also send you pull requests with my changes to the
> > edk2/edk2-platforms repos:
> > https://github.com/changab/edk2/pull/1
> > https://github.com/changab/edk2-platforms/pull/6
> >
> > If you don't mind I would send patches in this fashion before posting
> > them to the mailing list. So you can review them and provide feedback.
> I reviewed these patches just now, that looks good to me. However this way we don’t have the history in mailing list.
>
> >
> > Also I wanted to ask if you plan to work on the required changes
> > yourself, or am I on my own from here?)
> That would be good if you can help, that is also fine for me to update the code, however it would be later next week as I have to work on some tasks.
>
> BTW, are you an individual contributor or you are the representative of a firm?
>
> Thanks
> Abner
> >
> > Best regards,
> > Konstantin Aladyshev
> >
> > On Wed, Oct 11, 2023 at 8:58 AM Chang, Abner <Abner.Chang at amd.com>
> > wrote:
> > >
> > > [AMD Official Use Only - General]
> > >
> > > Hi Aladyshev,
> > > Here is my response,
> > >
> > > 1. Shouldn't we update the PLDM protocol's 'PldmSubmit' function to
> > receive 'MctpSrcEID'/'MctpDestEID'.
> > > Yes, I see the use case of EFI shell application. I would like to have the input
> > parameters similar with MctpSubmitCommand. Use pointers for the given
> > SEID and DEID, adopt the fix PCD value of the pointer is NULL.
> > >
> > > 2. The 'PldmProtocolCommon' code contains array ofexpected response
> > sizes.
> > > The idea is as an high level PLDM driver, such as SMBIOS, it doesn't have to
> > handle the full size of PLDM message. It should only care about the response
> > data belong to it. That's why I added the array to define the PLDM response
> > size for any PLDM spec.
> > > As the PLDM message and PLDM complete code is in a fixe size. I think we
> > can remove the array and just adjust the response size in PLDM Protocol
> > driver.
> > >
> > > 3. The 'AdditionalTransferError' is designed to accommodate error codes for
> > the specific transport or protocol. We don’t have much implementation of
> > 'AdditionalTransferError' as the entire Manageability needs more practices to
> > make it more mature.
> > > I suggest we can categorize 'AdditionalTransferError' using the most
> > significant byte, for example
> > > 0x00 - Common additional transfer error.
> > > 0x80 - KCS transport error
> > > 0x81 - MCTP
> > > ...
> > > 0xC0 - IPMI protocol error
> > > 0xC1 - PLDM protocol error
> > > ...
> > >
> > > How do you think?
> > > Thanks
> > > Abner
> > >
> > >
> > > > -----Original Message-----
> > > > From: Konstantin Aladyshev <aladyshev22 at gmail.com>
> > > > Sent: Thursday, October 5, 2023 11:18 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.
> > > >
> > > >
> > > > Sorry for many messages today, but I have one more observation.
> > > >
> > > > I think the MCTP code should set specific 'AdditionalTransferError'
> > > > for the caller when it fails on each of the response header checks:
> > > > https://github.com/changab/edk2-
> > > >
> > platforms/blob/5af7ca1555863288bd2887ca465ad07d74b9868e/Features/
> > > >
> > ManageabilityPkg/Universal/MctpProtocol/Common/MctpProtocolCommon.
> > > > c#L464
> > > >
> > > > But I'm not sure how 'AdditionalTransferError' can be utilized for
> > > > this. Right now the errors are generic for the IPMI/MCTP protocols
> > > > https://github.com/changab/edk2-
> > > >
> > platforms/blob/5af7ca1555863288bd2887ca465ad07d74b9868e/Features/
> > > > ManageabilityPkg/Include/Library/ManageabilityTransportLib.h#L63
> > > > Maybe it would be good to reserve some bits for the protocol specific
> > > > errors? What do you think?
> > > >
> > > > Also I don't see anything like 'AdditionalTransferError' in the PLDM
> > > > protocol code. Shouldn't it also support reporting additional error
> > > > information to the user?
> > > >
> > > > I've also created PRs for your edk2/edk2-platforms forks with some
> > > > things that I've found today.
> > > > https://github.com/changab/edk2/pull/1
> > > > https://github.com/changab/edk2-platforms/pull/6
> > > >
> > > > Best regards,
> > > > Konstantin Aladyshev
> > > >
> > > > On Thu, Oct 5, 2023 at 3:31 PM Konstantin Aladyshev
> > > > <aladyshev22 at gmail.com> wrote:
> > > > >
> > > > > I was thinking more about it and I think the right solution would be
> > > > > to move these checks to the
> > EDKII_PLDM_SMBIOS_TRANSFER_PROTOCOL
> > > > code
> > > > > (i.e. https://github.com/changab/edk2-
> > > >
> > platforms/blob/MCTP_OVER_KCS_UPDATE/Features/ManageabilityPkg/Univ
> > > > ersal/PldmSmbiosTransferDxe/PldmSmbiosTransferDxe.c)
> > > > >
> > > > > Because only the PldmSmbios protocol code should know such
> > information
> > > > > as expected response sizes for its commands.
> > > > >
> > > > > Best regards,
> > > > > Konstantin Aladyshev
> > > > >
> > > > > On Thu, Oct 5, 2023 at 3:19 PM Konstantin Aladyshev
> > > > > <aladyshev22 at gmail.com> wrote:
> > > > > >
> > > > > > Also I see that the 'PldmProtocolCommon' code contains array of
> > > > > > expected response sizes for every supported PLDM command:
> > > > > > https://github.com/changab/edk2-
> > > >
> > platforms/blob/5af7ca1555863288bd2887ca465ad07d74b9868e/Features/
> > > >
> > ManageabilityPkg/Universal/PldmProtocol/Common/PldmProtocolCommon.
> > > > c#L24
> > > > > >
> > > > > > I'm not sure that we should have this, since we don't need to know the
> > > > > > MCTP response size apriori.
> > > > > > This only limits allowed PLDM commands to the supported array:
> > > > > > https://github.com/changab/edk2-
> > > >
> > platforms/blob/5af7ca1555863288bd2887ca465ad07d74b9868e/Features/
> > > >
> > ManageabilityPkg/Universal/PldmProtocol/Common/PldmProtocolCommon.
> > > > c#L261
> > > > > >
> > > > > > This means that right now I can't execute my simple 'GetPLDMTypes'
> > > > > > command through the 'PldmSubmit' protocol function.
> > > > > >
> > > > > > Best regards,
> > > > > > Konstantin Aladyshev
> > > > > >
> > > > > > On Thu, Oct 5, 2023 at 12:55 PM Konstantin Aladyshev
> > > > > > <aladyshev22 at gmail.com> wrote:
> > > > > > >
> > > > > > > Shouldn't we update the PLDM protocol's 'PldmSubmit' function to
> > > > > > > receive 'MctpSrcEID'/'MctpDestEID' as incoming parameters? Or
> > maybe
> > > > > > > add some 'PldmInit' function to set those parameters for further
> > PLDM
> > > > > > > communication?
> > > > > > > Because right now the MCTP EIDs are fixed to PCDs with no flexibility
> > > > > > > https://github.com/tianocore/edk2-
> > > >
> > platforms/blob/d6e36a151ff8365cdc55a6914cc5e6138d5788dc/Features/
> > > >
> > ManageabilityPkg/Universal/PldmProtocol/Common/PldmProtocolCommon.
> > > > c#L121
> > > > > > >
> > > > > > > Best regards,
> > > > > > > Konstantin Aladyshev
> > > > > > >
> > > > > > > On Thu, Oct 5, 2023 at 7:03 AM Chang, Abner
> > > > <Abner.Chang at amd.com> wrote:
> > > > > > > >
> > > > > > > > [AMD Official Use Only - General]
> > > > > > > >
> > > > > > > > > -----Original Message-----
> > > > > > > > > From: discuss at edk2.groups.io <discuss at edk2.groups.io> On
> > Behalf
> > > > Of
> > > > > > > > > Konstantin Aladyshev via groups.io
> > > > > > > > > Sent: Thursday, October 5, 2023 1:57 AM
> > > > > > > > > To: Chang, Abner <Abner.Chang at amd.com>
> > > > > > > > > Cc: devel at edk2.groups.io; discuss 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.
> > > > > > > > >
> > > > > > > > >
> > > > > > > > > > That is great, and I'm surprised there are some build errors at
> > your
> > > > end.
> > > > > > > > >
> > > > > > > > > I'm surprised your compiler didn't catch that since it is all basic
> > > > > > > > > syntax errors.
> > > > > > > > > I've used your https://github.com/changab/edk2-
> > > > > > > > > platforms/tree/MCTP_OVER_KCS_UPDATE
> > > > > > > > > directly.
> > > > > > > >
> > > > > > > > Ah I know why, I forget to rebuild the changes of both PEC and
> > MCTP
> > > > EID after I verifying the functionality of IPMI on the new KbcCommonLib.c.
> > > > Yes, I do see the build error now and was fixed at my end. My fault.
> > > > > > > >
> > > > > > > > >
> > > > > > > > > > How do you think we just send it to the mailing list for review
> > and
> > > > keep
> > > > > > > > > working on other problems based on it.?
> > > > > > > > > > Could you please send the patches out, with you as the author
> > and
> > > > I'm the
> > > > > > > > > coauthor? I will review it again on dev mailing list.
> > > > > > > > >
> > > > > > > > > No problem, I can send a patch to the 'edk2-devel' mailing list.
> > > > > > > > > But before that I think I'll write a test app to check if PLDM
> > > > > > > > > protocols work correctly.
> > > > > > > > Ok.
> > > > > > > >
> > > > > > > > >
> > > > > > > > > Also earlier I've pointed to a fact that 'MctpSourceEndpointId' and
> > > > > > > > > 'MctpDestinationEndpointId' aren't actually used in the
> > > > > > > > > 'MctpSubmitMessage' function.
> > > > > > > > > EIDs are always taken from the PCDs:
> > > > > > > > > https://github.com/changab/edk2-
> > > > > > > > >
> > > >
> > platforms/blob/1c8d0d3fa403b47a34667f7f690add7822163111/Features/
> > > > > > > > >
> > > >
> > ManageabilityPkg/Universal/MctpProtocol/Common/MctpProtocolCommon.
> > > > > > > > > c#L178
> > > > > > > > > What can we do about that?
> > > > > > > > Ah yes, we should update the algorithm, it is done here:
> > > > https://github.com/changab/edk2-
> > > > platforms/tree/MCTP_OVER_KCS_UPDATE. You have to update your code
> > > > here:
> > > >
> > https://github.com/Kostr/PLDM/blob/master/PldmMessage/PldmMessage.c
> > > > > > > > And we also need the fix the typo on edk2,
> > > > https://github.com/changab/edk2/tree/MCTP_OVER_KCS_UPDATE
> > > > > > > >
> > > > > > > > >
> > > > > > > > > > Btw, how long do you think I would take to merge your changes
> > on
> > > > > > > > > openBMC?
> > > > > > > > >
> > > > > > > > > So as I've described in the
> > > > https://github.com/Kostr/PLDM/tree/master
> > > > > > > > > there are basically 2 approaches for the MCTP stack in OpenBMC:
> > > > > > > > > (1) userspace approach (legacy, shouldn't be really used now)
> > > > > > > > > (2) kernel approach
> > > > > > > > >
> > > > > > > > > It is hard to tell if OpenBMC patches for the (1) approach will be
> > > > > > > > > merged. Since I've developed the Linux kernel driver (2) nobody
> > really
> > > > > > > > > cares about (1).
> > > > > > > > >
> > > > > > > > > For the (2) there are a couple of OpenBMC patches which I've
> > helped
> > > > to
> > > > > > > > > develop, but I'm just a coathor in them. So it is hard for me to tell
> > > > > > > > > when they would be merged. For me it looks like they are mostly
> > > > ready:
> > > > > > > > > 66591: transport: af-mctp: Add pldm_transport_af_mctp_bind() |
> > > > > > > > > https://gerrit.openbmc.org/c/openbmc/libpldm/+/66591
> > > > > > > > > 63652: pldm: Convert to using libpldm transport APIs |
> > > > > > > > > https://gerrit.openbmc.org/c/openbmc/pldm/+/63652
> > > > > > > > >
> > > > > > > > > For the (2) I also need to push the mctp Linux kernel driver
> > upstream.
> > > > > > > > > Linux kernel development is not what I do every day, so I'm not
> > sure
> > > > > > > > > how long it would take. But I'm pretty determined to finish the
> > work
> > > > > > > > > and push my driver upstream. Currently there are some questions
> > > > > > > > > regarding Linux KCS subsystem, so along with the KCS subsystem
> > > > creator
> > > > > > > > > we have to figure out how to rewrite the subsystem correctly. So
> > this
> > > > > > > > > can take some time.
> > > > > > > > > After the code is pushed to the torvalds/linux, it would be picked
> > up
> > > > > > > > > by the openbmc/linux automatically.
> > > > > > > > Ok, I got it. Thanks for the detailed information.
> > > > > > > >
> > > > > > > > Regards,
> > > > > > > > Abner
> > > > > > > >
> > > > > > > > >
> > > > > > > > > Best regards,
> > > > > > > > > Konstantin Aladyshev
> > > > > > > > >
> > > > > > > > > On Wed, Oct 4, 2023 at 7:12 PM Chang, Abner
> > > > <Abner.Chang at amd.com>
> > > > > > > > > wrote:
> > > > > > > > > >
> > > > > > > > > > [AMD Official Use Only - General]
> > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > That is great, and I'm surprised there are some build errors at
> > your
> > > > end.
> > > > > > > > > > How do you think we just send it to the mailing list for review
> > and
> > > > keep
> > > > > > > > > working on other problems based on it.?
> > > > > > > > > > Could you please send the patches out, with you as the author
> > and
> > > > I'm the
> > > > > > > > > coauthor? I will review it again on dev mailing list.
> > > > > > > > > >
> > > > > > > > > > I will take a look on kernal change. Btw, how long do you think I
> > > > would take
> > > > > > > > > to merge your changes on openBMC?
> > > > > > > > > >
> > > > > > > > > > Thanks
> > > > > > > > > > Abner
> > > > > > > > > >
> > > > > > > > > > Get Outlook for Android
> > > > > > > > > >
> > > > > > > > > > ________________________________
> > > > > > > > > > From: Konstantin Aladyshev <aladyshev22 at gmail.com>
> > > > > > > > > > Sent: Wednesday, October 4, 2023 11:59:16 PM
> > > > > > > > > > To: Chang, Abner <Abner.Chang at amd.com>
> > > > > > > > > > Cc: devel at edk2.groups.io <devel at edk2.groups.io>;
> > > > > > > > > discuss at edk2.groups.io <discuss 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.
> > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > Hi Chang!
> > > > > > > > > >
> > > > > > > > > > Thanks!
> > > > > > > > > > There were a couple of trivial compilation errors, but after I've
> > > > > > > > > > fixed them everything seems to work fine!
> > > > > > > > > > Just in case I've tested the OpenBMC side with the mctp Linux
> > kernel
> > > > > > > > > > driver approach
> > > > > > > > > > (https://github.com/Kostr/PLDM/tree/master/mctp-kernel)
> > > > > > > > > > The latest kernel patches can be found here:
> > > > > > > > > > https://lore.kernel.org/lkml/20231003131505.337-1-
> > > > > > > > > aladyshev22 at gmail.com/
> > > > > > > > > >
> > > > > > > > > > Here is a fix for the build errors that I've found:
> > > > > > > > > > ```
> > > > > > > > > > diff --git
> > > > > > > > >
> > > >
> > a/Features/ManageabilityPkg/Universal/MctpProtocol/Common/MctpProtoc
> > > > > > > > > olCommon.c
> > > > > > > > > >
> > > > > > > > >
> > > >
> > b/Features/ManageabilityPkg/Universal/MctpProtocol/Common/MctpProto
> > > > > > > > > colCommon.c
> > > > > > > > > > index 79501d27aa..345c6da81a 100644
> > > > > > > > > > ---
> > > > > > > > >
> > > >
> > a/Features/ManageabilityPkg/Universal/MctpProtocol/Common/MctpProtoc
> > > > > > > > > olCommon.c
> > > > > > > > > > +++
> > > > > > > > >
> > > >
> > b/Features/ManageabilityPkg/Universal/MctpProtocol/Common/MctpProto
> > > > > > > > > colCommon.c
> > > > > > > > > > @@ -193,7 +193,7 @@ SetupMctpRequestTransportPacket (
> > > > > > > > > >
> > > > > > > > > > //
> > > > > > > > > > // Generate PEC follow SMBUS 2.0 specification.
> > > > > > > > > > - *MctpKcsTrailer->Pec = HelperManageabilityGenerateCrc8
> > > > > > > > > > (MCTP_KCS_PACKET_ERROR_CODE_POLY, 0, ThisPackage,
> > > > > > > > > > MctpKcsHeader->ByteCount);
> > > > > > > > > > + MctpKcsTrailer->Pec = HelperManageabilityGenerateCrc8
> > > > > > > > > > (MCTP_KCS_PACKET_ERROR_CODE_POLY, 0, ThisPackage,
> > > > > > > > > > MctpKcsHeader->ByteCount);
> > > > > > > > > > *PacketBody = (UINT8 *)ThisPackage;
> > > > > > > > > > *PacketBodySize = MctpKcsHeader->ByteCount;
> > > > > > > > > > *PacketTrailer =
> > > > > > > > > (MANAGEABILITY_TRANSPORT_TRAILER)MctpKcsTrailer;
> > > > > > > > > > diff --git
> > > > > > > > >
> > > >
> > a/Features/ManageabilityPkg/Universal/MctpProtocol/Dxe/MctpProtocol.c
> > > > > > > > > >
> > > >
> > b/Features/ManageabilityPkg/Universal/MctpProtocol/Dxe/MctpProtocol.c
> > > > > > > > > > index 863b8d471c..247d032b9b 100644
> > > > > > > > > > ---
> > > > > > > > >
> > > >
> > a/Features/ManageabilityPkg/Universal/MctpProtocol/Dxe/MctpProtocol.c
> > > > > > > > > > +++
> > > > > > > > >
> > > >
> > b/Features/ManageabilityPkg/Universal/MctpProtocol/Dxe/MctpProtocol.c
> > > > > > > > > > @@ -79,17 +79,17 @@ MctpSubmitMessage (
> > > > > > > > > > }
> > > > > > > > > >
> > > > > > > > > > //
> > > > > > > > > > - // Chec source EID and destination EDI.
> > > > > > > > > > + // Check source EID and destination EID
> > > > > > > > > > //
> > > > > > > > > > if ((MctpSourceEndpointId >=
> > > > MCTP_RESERVED_ENDPOINT_START_ID) &&
> > > > > > > > > > - MctpSourceEndpointId <=
> > > > MCTP_RESERVED_ENDPOINT_END_ID)
> > > > > > > > > > + (MctpSourceEndpointId <=
> > > > MCTP_RESERVED_ENDPOINT_END_ID)
> > > > > > > > > > ) {
> > > > > > > > > > DEBUG ((DEBUG_ERROR, "%a: The value of MCTP source EID
> > (%x)
> > > > is
> > > > > > > > > > reserved.\n", func, MctpSourceEndpointId));
> > > > > > > > > > return EFI_INVALID_PARAMETER;
> > > > > > > > > > }
> > > > > > > > > >
> > > > > > > > > > if ((MctpDestinationEndpointId >=
> > > > > > > > > MCTP_RESERVED_ENDPOINT_START_ID) &&
> > > > > > > > > > - MctpDestinationEndpointId <=
> > > > MCTP_RESERVED_ENDPOINT_END_ID)
> > > > > > > > > > + (MctpDestinationEndpointId <=
> > > > MCTP_RESERVED_ENDPOINT_END_ID)
> > > > > > > > > > ) {
> > > > > > > > > > DEBUG ((DEBUG_ERROR, "%a: The value of MCTP destination
> > EID
> > > > (%x)
> > > > > > > > > > is reserved.\n", func, MctpDestinationEndpointId));
> > > > > > > > > > return EFI_INVALID_PARAMETER;
> > > > > > > > > > ```
> > > > > > > > > >
> > > > > > > > > > Best regards,
> > > > > > > > > > Konstantin Aladyshev
> > > > > > > > > >
> > > > > > > > > > On Wed, Oct 4, 2023 at 2:52 PM Chang, Abner
> > > > <Abner.Chang at amd.com>
> > > > > > > > > wrote:
> > > > > > > > > > >
> > > > > > > > > > > [AMD Official Use Only - General]
> > > > > > > > > > >
> > > > > > > > > > > Hi Aladyshev,
> > > > > > > > > > > I have updated the change you made and put those code on
> > below
> > > > link,
> > > > > > > > > > > https://github.com/changab/edk2-
> > > > > > > > >
> > > > platforms/commit/1c8d0d3fa403b47a34667f7f690add7822163111
> > > > > > > > > > >
> > > > > > > > > > > I combined MCTP over KCS changes and IPMI over KCS
> > > > functionality in
> > > > > > > > > KcsCommonLib.c. I also created
> > > > MANAGEABILITY_MCTP_KCS_TRAILER as you
> > > > > > > > > suggested. The source EID and destination EID are checked in
> > > > > > > > > MctpSubmitCommand as well. IPMI/KCS functionality is verified
> > and
> > > > works
> > > > > > > > > fine after this change.
> > > > > > > > > > > As I am no able to use the corresponding change you made on
> > > > OpenBMC
> > > > > > > > > site at my end, could you please help to verify my updates on your
> > > > machine?
> > > > > > > > > Let's see how it works.
> > > > > > > > > > > I also consider to migrate the code that generates MCTP over
> > KCS
> > > > > > > > > header/trailer from MctpProtocolCommon.c to KcsCommonLib.c,
> > > > maybe after
> > > > > > > > > we verifying PLDM->MCTP->KCS route works well on
> > > > ManageabilityPkg.
> > > > > > > > > > >
> > > > > > > > > > > Thank you
> > > > > > > > > > > Abner
> > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > > > > -----Original Message-----
> > > > > > > > > > > > From: Konstantin Aladyshev <aladyshev22 at gmail.com>
> > > > > > > > > > > > Sent: Friday, September 29, 2023 2:18 AM
> > > > > > > > > > > > To: Chang, Abner <Abner.Chang at amd.com>
> > > > > > > > > > > > Cc: devel at edk2.groups.io; discuss 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.
> > > > > > > > > > > >
> > > > > > > > > > > >
> > > > > > > > > > > > Hi, Chang!
> > > > > > > > > > > >
> > > > > > > > > > > > Did you have time to test libmctp MCTP KCS binding solution?
> > > > > > > > > > > >
> > > > > > > > > > > > Here are some updates from my end. As I was saying, I was
> > > > working on
> > > > > > > > > > > > the Linux kernel binding solution.
> > > > > > > > > > > > And now I've finished the initial implementation of the Linux
> > > > kernel
> > > > > > > > > > > > binding driver for the MCTP-over-KCS binding and proposed
> > all
> > > > the
> > > > > > > > > > > > patches upstream
> > > > > > > > > > > > (https://www.spinics.net/lists/kernel/msg4949173.html).
> > > > > > > > > > > > I've also updated instructions in my repo
> > > > > > > > > > > > https://github.com/Kostr/PLDM (the guide for the kernel
> > > > binding
> > > > > > > > > > > > solution and all the necessary Linux kernel patches can be
> > found
> > > > here
> > > > > > > > > > > > https://github.com/Kostr/PLDM/tree/master/mctp-kernel).
> > > > > > > > > > > > So now you can use Linux driver instead of the libmctp utility
> > on
> > > > the BMC
> > > > > > > > > side.
> > > > > > > > > > > >
> > > > > > > > > > > > Couple of things that I've noticed in the development
> > process:
> > > > > > > > > > > > - `MctpSubmitCommand` receives
> > > > > > > > > > > > 'MctpSourceEndpointId'/'MctpDestinationEndpointId' as
> > > > arguments. But
> > > > > > > > > > > > these values aren't actually used. The current code just uses
> > EIDs
> > > > > > > > > > > > that were set via PCDs
> > > > > > > > > > > > (https://github.com/tianocore/edk2-
> > > > > > > > > > > >
> > > > > > > > >
> > > >
> > platforms/blob/d03a60523a6086d200d3eb1e2f25530bf1cb790e/Features/
> > > > > > > > > > > >
> > > > > > > > >
> > > >
> > ManageabilityPkg/Universal/MctpProtocol/Common/MctpProtocolCommon.
> > > > > > > > > > > > c#L178)
> > > > > > > > > > > > - According to the specification DSP0236 (section 8.2) MCTP
> > EID
> > > > 0 to 7
> > > > > > > > > > > > are reserved. It is critical that we do not use them since MCTP
> > > > Linux
> > > > > > > > > > > > kernel subsystem checks that part. So we probably need to
> > add
> > > > some
> > > > > > > > > > > > check to the `MctpSubmitCommand` that would verify that
> > we
> > > > don't use
> > > > > > > > > > > > reserved EIDs.
> > > > > > > > > > > >
> > > > > > > > > > > > Best regards,
> > > > > > > > > > > > Konstantin Aladyshev
> > > > > > > > > > > >
> > > > > > > > > > > > On Thu, Sep 21, 2023 at 5:32 AM Chang, Abner
> > > > > > > > > <Abner.Chang at amd.com>
> > > > > > > > > > > > wrote:
> > > > > > > > > > > > >
> > > > > > > > > > > > > [AMD Official Use Only - General]
> > > > > > > > > > > > >
> > > > > > > > > > > > > Hi Aladyshev,
> > > > > > > > > > > > > Thanks for providing the details, I will take a look at your
> > code
> > > > first,
> > > > > > > > > > > > implement it at my end and then response to your question.
> > > > > > > > > > > > >
> > > > > > > > > > > > > Abner
> > > > > > > > > > > > >
> > > > > > > > > > > > > > -----Original Message-----
> > > > > > > > > > > > > > From: Konstantin Aladyshev <aladyshev22 at gmail.com>
> > > > > > > > > > > > > > Sent: Friday, September 8, 2023 8:57 PM
> > > > > > > > > > > > > > To: Chang, Abner <Abner.Chang at amd.com>
> > > > > > > > > > > > > > Cc: devel at edk2.groups.io; discuss 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.
> > > > > > > > > > > > > >
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > Hi, Chang!
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > I've finished my initial implementation of the MCTP over
> > KCS
> > > > binding.
> > > > > > > > > > > > > > You can find 'edk2-platform' patches and 'openbmc'
> > patches
> > > > along
> > > > > > > > > with
> > > > > > > > > > > > > > all of the instructions in my repository
> > > > > > > > > > > > > > https://github.com/Kostr/PLDM. I hope you'll be able to
> > > > reproduce
> > > > > > > > > > > > > > everything on your hardware configuration. Feel free to
> > ask
> > > > any
> > > > > > > > > > > > > > questions.
> > > > > > > > > > > > > > Also I've sent all the openbmc patches upstream, hope
> > they
> > > > will get
> > > > > > > > > > > > > > accepted soon.
> > > > > > > > > > > > > > As for the 'edk2-platform' patches, right now I don't fully
> > > > understand
> > > > > > > > > > > > > > how to write them correctly to keep IPMI over KCS stack
> > > > working. I
> > > > > > > > > > > > > > think here I would need your help. Right now I've
> > commited
> > > > them to
> > > > > > > > > my
> > > > > > > > > > > > > > `edk2-platforms` fork
> > > > > > > > > > > > > > https://github.com/tianocore/edk2-
> > > > > > > > > > > > > >
> > > > platforms/commit/99a6c98a63b37f955c0d0480149b84fcc3a03f74
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > Couple of questions/notices:
> > > > > > > > > > > > > > 1) You've said that we can differentiate MCTP by the
> > transfer
> > > > token,
> > > > > > > > > > > > > > but it is not passed to the 'KcsTransportSendCommand'
> > > > function
> > > > > > > > > > > > > > https://github.com/tianocore/edk2-
> > > > > > > > > > > > > >
> > > > > > > > > > > >
> > > > > > > > >
> > > >
> > platforms/blob/bb6841e3fd1c60b3f8510b4fc0a380784e05d326/Features/
> > > > > > > > > > > > > >
> > > > > > > > > > > >
> > > > > > > > >
> > > >
> > ManageabilityPkg/Library/ManageabilityTransportKcsLib/Common/KcsComm
> > > > > > > > > > > > > > on.c#L414
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > 2) What function should know about the
> > > > > > > > > > > > > > MANAGEABILITY_MCTP_KCS_HEADER?
> > > > > > > > > > > > > > Keep in mind that this header includes 'ByteCount' for the
> > > > incoming
> > > > > > > > > > > > > > data size that we need to read.
> > > > > > > > > > > > > > - KcsTransportSendCommand or
> > > > CommonMctpSubmitMessage ?
> > > > > > > > > > > > > > (https://github.com/tianocore/edk2-
> > > > > > > > > > > > > >
> > > > > > > > > > > >
> > > > > > > > >
> > > >
> > platforms/blob/master/Features/ManageabilityPkg/Library/ManageabilityTra
> > > > > > > > > > > > > > nsportKcsLib/Common/KcsCommon.c)
> > > > > > > > > > > > > > (https://github.com/tianocore/edk2-
> > > > > > > > > > > > > >
> > > > > > > > > > > >
> > > > > > > > >
> > > >
> > platforms/blob/master/Features/ManageabilityPkg/Universal/MctpProtocol/
> > > > > > > > > > > > > > Common/MctpProtocolCommon.c)?
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > 3) As I've said earlier I think it would be good to add
> > > > > > > > > > > > > > MANAGEABILITY_MCTP_KCS_TRAILER to the Mctp.h
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > 4) Not sure if it is a good idea to pass these parameters to
> > the
> > > > > > > > > > > > > > MctpSubmitCommand protocol function:
> > > > > > > > > > > > > > ```
> > > > > > > > > > > > > > UINT8 MctpType,
> > > > > > > > > > > > > > BOOLEAN RequestDataIntegrityCheck,
> > > > > > > > > > > > > > ```
> > > > > > > > > > > > > > (https://github.com/tianocore/edk2-
> > > > > > > > > > > > > >
> > > > > > > > > > > >
> > > > > > > > >
> > > >
> > platforms/blob/master/Features/ManageabilityPkg/Include/Protocol/MctpPr
> > > > > > > > > > > > > > otocol.h)
> > > > > > > > > > > > > > Shouldn't it be in the `RequestData` directly since it is more
> > of
> > > > a
> > > > > > > > > > > > > > payload than a header for the MCTP? I don't know the
> > > > specification
> > > > > > > > > > > > > > very well, but what if the RequestDataIntegrityCheck
> > would
> > > > be set in
> > > > > > > > > > > > > > the response? Who would need to check the integrity of
> > the
> > > > payload
> > > > > > > > > > > > > > buffer in that case? MCTP library or the code calling the
> > MCTP
> > > > > > > > > > > > > > library?
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > 5) Haven't tested the PldmProtocol, maybe it also needs
> > > > some
> > > > > > > > > corrections.
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > Feel free to ask any questions about my solution.
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > Right now I'll probably focus on the Linux kernel driver for
> > the
> > > > MCTP
> > > > > > > > > > > > > > over KCS binding. So if you want to finish edk2-platforms
> > > > code based
> > > > > > > > > > > > > > on my patches, feel free to do it. If not, I'll try to get back
> > to it
> > > > > > > > > > > > > > after I finish the Linux kernel driver.
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > Best regards,
> > > > > > > > > > > > > > Konstantin Aladyshev
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > On Fri, Sep 1, 2023 at 8:58 AM Chang, Abner
> > > > > > > > > <Abner.Chang at amd.com>
> > > > > > > > > > > > > > wrote:
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > [AMD Official Use Only - General]
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > See my answer below,
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > -----Original Message-----
> > > > > > > > > > > > > > > > From: devel at edk2.groups.io <devel at edk2.groups.io>
> > On
> > > > Behalf
> > > > > > > > > Of
> > > > > > > > > > > > > > > > Konstantin Aladyshev via groups.io
> > > > > > > > > > > > > > > > Sent: Friday, September 1, 2023 12:02 AM
> > > > > > > > > > > > > > > > To: Chang, Abner <Abner.Chang at amd.com>
> > > > > > > > > > > > > > > > Cc: devel at edk2.groups.io; discuss 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.
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > 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/7787906385638850
> > > > > > > > > > > > > > > > 86/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?
> > > > > > > > > > > > > > > If the MCTP KCS packet for host->BMC and BMC->host
> > are
> > > > in the
> > > > > > > > > same
> > > > > > > > > > > > > > structure, then yes, the response data from BMC should
> > > > includes
> > > > > > > > > > > > > > MCTP_TRANSPORT_HEADER/MCTP_MESSAGE_HEADER
> > in
> > > > my
> > > > > > > > > opinion, as
> > > > > > > > > > > > this
> > > > > > > > > > > > > > is defined in MCTP base protocol.
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > So let me explain the implementation for MCTP over KCS
> > > > and what
> > > > > > > > > do we
> > > > > > > > > > > > > > miss now.
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > A. MCTP protocol driver linked with KCS Transport
> > interface
> > > > library
> > > > > > > > > > > > > > > - In MCTP protocol driver, if the transport interface is
> > KCS
> > > > then
> > > > > > > > > > > > > > > 1. MCTP protocol driver builds up the MCTP KCS
> > > > transport
> > > > > > > > > header,
> > > > > > > > > > > > which
> > > > > > > > > > > > > > is DefBody, NetFunc and ByeCount
> > > > > > > > > > > > > > > 2. MCTP protocol driver builds up the MCTP payload
> > > > > > > > > > > > > > > 3. MCTP protocol driver builds up the MCTP KCS
> > > > transport trailer,
> > > > > > > > > > > > which
> > > > > > > > > > > > > > is PEC.
> > > > > > > > > > > > > > > Above three steps are already implemented.
> > > > > > > > > > > > > > > PEC is calculated by MCTP protocol driver and should
> > be
> > > > verified
> > > > > > > > > by
> > > > > > > > > > > > KCS
> > > > > > > > > > > > > > using the same algorithm in my understanding of spec.
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > B. In KCS Transport interface library
> > > > > > > > > > > > > > > 1. KCS Transport interface library sends the transport
> > > > header got
> > > > > > > > > from
> > > > > > > > > > > > > > TransportToken. Same behavior for IPMI protocol, but
> > > > different
> > > > > > > > > content.
> > > > > > > > > > > > KCS
> > > > > > > > > > > > > > Transport interface library doesn't have to understand
> > this.
> > > > > > > > > > > > > > > 2. KCS Transport interface library sends the payload
> > > > > > > > > > > > > > > 3. KCS Transport interface library sends the transport
> > > > trailer got
> > > > > > > > > from
> > > > > > > > > > > > > > TransportToken. Same behavior for IPMI protocol, but
> > > > different
> > > > > > > > > content.
> > > > > > > > > > > > KCS
> > > > > > > > > > > > > > Transport interface library doesn't have to understand
> > this.
> > > > > > > > > > > > > > > Above three steps are already implemented.
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > Then, if Manageability protocol is MCTP, we skip
> > reading
> > > > > > > > > responses
> > > > > > > > > > > > > > header (Not implemented)
> > > > > > > > > > > > > > > For reading response data
> > > > > > > > > > > > > > > 1. If the ResponseData and ResponseSize is valid in
> > the
> > > > given
> > > > > > > > > > > > > > TransportToken, then we read ResponseSize data from
> > KCS.
> > > > (Already
> > > > > > > > > > > > > > implemented)
> > > > > > > > > > > > > > > 2. if Manageability protocol is MCTP, then we skip
> > > > reading
> > > > > > > > > responses
> > > > > > > > > > > > > > header again (Not implemented)
> > > > > > > > > > > > > > > Now the response is returned to MCTP protocol driver
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > C. In MCTP protocol driver, we expect to get the whole
> > > > MCTP over
> > > > > > > > > KCS
> > > > > > > > > > > > > > packet response, that includes DefBody, NetFunc and
> > > > ByeCount,
> > > > > > > > > MCTP
> > > > > > > > > > > > > > message and PEC.
> > > > > > > > > > > > > > > 1. MCTP protocol driver verifies the returned PEC
> > with
> > > > the
> > > > > > > > > payload.
> > > > > > > > > > > > > > > 2. Strip out DefBody, NetFunc, ByeCount and PEC
> > and
> > > > then
> > > > > > > > > returns it
> > > > > > > > > > > > to
> > > > > > > > > > > > > > upper layer (e.g., MCTP transport interface library).
> > Returns
> > > > only
> > > > > > > > > MCTP
> > > > > > > > > > > > > > Transport header and MCTP packet payload as it shows in
> > > > MCTP base
> > > > > > > > > > > > protocol
> > > > > > > > > > > > > > spec.
> > > > > > > > > > > > > > > Above is not implemented
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > D. In MCTP transport interface library, we can strip out
> > > > MCTP
> > > > > > > > > transport
> > > > > > > > > > > > > > header and then return it to upper layer (e.g., PLDM
> > protocol
> > > > driver).
> > > > > > > > > > > > > > > Above is not implemented.
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > E. In PLDM protocol driver,
> > > > > > > > > > > > > > > 1. we verify the Message Integrity Check if the
> > Message
> > > > Type
> > > > > > > > > > > > requests it.
> > > > > > > > > > > > > > > 2. we can remove MCTP message type then return it
> > to
> > > > upper
> > > > > > > > > layer
> > > > > > > > > > > > (e.g.,
> > > > > > > > > > > > > > PLDM SMBIOS transfer)
> > > > > > > > > > > > > > > Above is not implemented.
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > We didn’t implement BMC->Host in step C, D and E as
> > our
> > > > current
> > > > > > > > > > > > demand is
> > > > > > > > > > > > > > to send the SMBIOS table to BMC, which doesn't require
> > the
> > > > response
> > > > > > > > > data
> > > > > > > > > > > > if I
> > > > > > > > > > > > > > am not wrong.
> > > > > > > > > > > > > > > Let me know if it is problematic in the above process.
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > Thanks and regards,
> > > > > > > > > > > > > > > Abner
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > Best regards,
> > > > > > > > > > > > > > > > Konstantin Aladyshev
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > On Thu, Aug 31, 2023 at 6:52 PM Chang, Abner
> > > > > > > > > > > > > > <Abner.Chang at 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 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 (#109616): https://edk2.groups.io/g/devel/message/109616
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]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list