[Skiboot] [PATCH V2 08/10] libmctp: Import libmctp library handling MCTP protocol

[Christophe Lombard]

Le 31/03/2023 à 16:03, Frederic Barrat a écrit :
>
> On 15/03/2022 13:49, Christophe Lombard wrote:
>> The Management Component Transport Protocol (MCTP) is a protocol 
>> defined by
>> the DMTF Platform Management Component Intercommunications sub-team 
>> of the
>> DMTF Pre-OS Workgroup. MCTP is designed to support communications 
>> between
>> different intelligent hardware components that make up a platform
>> management subsystem that is provides monitoring and control functions
>> inside a managed system. DMTF standard "DSP2016"
>>
>> This library is intended to be a portable implementation of the 
>> Management
>> Component Transport Protocol (MCTP), as defined by DMTF standard 
>> "DSP0236",
>> plus transport binding specifications.
>> MCTP has been designed to carry multiple types of manageability-related
>> traffic across a common medium. The base MCTP specifications define
>> message types for supporting the initialization and configuration of 
>> MCTP
>> itself, and to support vendor-specific messages over MCTP.
>> Other message types, such as message types to support a Platform Level
>> Data Model (PLDM).
>>
>> The source is located here: https://github.com/openbmc/libmctp and 
>> use as
>> is, without any update.
>>
>> The libmctp code is integrated into the folder ./libmctp as a set of
>> sources, compilated if the compiler flag CONFIG_PLDM is set.
>
>
> Typo "compilated" => "compiled" (also in the next 2 patches)
>
> It may be worth re-synchronizing with latest upstream, since this is 
> now one year old?
>

We should, yes.

Thanks

>
> > diff --git a/Makefile.main b/Makefile.main
> > index 2a346a6c..738e0857 100644
> > --- a/Makefile.main
> > +++ b/Makefile.main
> > @@ -75,7 +75,7 @@ DBG=-g
> >
> >   CPPFLAGS := -I$(SRC)/include -Iinclude -MMD -include 
> $(SRC)/include/config.h
> >   CPPFLAGS += -I$(SRC)/libfdt -I$(SRC)/libflash -I$(SRC)/libxz 
> -I$(SRC)/libc/include -I$(SRC)
> > -CPPFLAGS += -I$(SRC)/libpore
> > +CPPFLAGS += -I$(SRC)/libpore -I$(SRC)/libmctp
> >   CPPFLAGS += -D__SKIBOOT__ -nostdinc
> >   CPPFLAGS += -isystem $(shell $(CC) -print-file-name=include)
> >   CPPFLAGS += -DBITS_PER_LONG=64
> > @@ -91,6 +91,13 @@ else
> >   CPPFLAGS += -DHAVE_BIG_ENDIAN
> >   endif
> >
> > +# Also add the asm/byte-order.h style ones used by libmctp
> > +ifeq ($(LITTLE_ENDIAN),1)
> > +CPPFLAGS += -D__LITTLE_ENDIAN_BITFIELD
> > +else
> > +CPPFLAGS += -D__BIG_ENDIAN_BITFIELD
> > +endif
> > +
>
>
> It would be simpler to merge with the previous "ifeq 
> ($(LITTLE_ENDIAN),1)" just above, since they are almost similar.
>
>
>> diff --git a/libmctp/Makefile.inc b/libmctp/Makefile.inc
>> new file mode 100644
>> index 00000000..24d6fcf5
>> --- /dev/null
>> +++ b/libmctp/Makefile.inc
>> @@ -0,0 +1,22 @@
>> +# SPDX-License-Identifier: Apache-2.0 OR GPL-2.0-or-later
>> +# Copyright 2022 IBM Corp.
>> +
>> +LIBMCTP_DIR ?= libmctp
>> +SUBDIRS += $(LIBMCTP_DIR)
>> +
>> +LIBMCTP_OBJS = crc32.o core.o alloc.o log.o
>> +LIBMCTP_BINDINGS ?= astlpc
>> +
>> +LIBMCTP_OBJS += $(LIBMCTP_BINDINGS:%=%.o)
>> +
>> +CFLAGS_$(LIBMCTP_DIR)/ = -I$(SRC)/ccan/endian/ \
>
>
> The "-I$(SRC)/ccan/endian/" is useless, as you now have a 
> include/endian.h file, which is in the default path and will also 
> include the file found in ccan/
>
>
>> diff --git a/libmctp/config.h b/libmctp/config.h
>
> You may want to add a note in the commit message that even though the 
> config.h file is typically automatically generated by 'configure', 
> it's not the case here and it has been carefully edited to match the 
> environment provided in the skiboot repo.
>
>   Fred
>