On Tue, Mar 18, 2025 at 03:56:24PM +0000, Jonathan Cameron wrote:
> On Mon, 17 Mar 2025 16:31:29 +0000
> anisa.su...@gmail.com wrote:
>
> > From: Anisa Su <anisa...@samsung.com>
> >
> > FM DCD Management command 0x5600 implemented per CXL 3.2 Spec Section
> > 7.6.7.6.1
> >
> > Signed-off-by: Anisa Su <anisa...@samsung.com>
> > --- a/hw/cxl/i2c_mctp_cxl.c
> > +++ b/hw/cxl/i2c_mctp_cxl.c
> > @@ -46,6 +46,9 @@
> > /* Implementation choice - may make this configurable */
> > #define MCTP_CXL_MAILBOX_BYTES 512
> >
> > +/* Supported FMAPI Cmds */
> > +#define FMAPI_CMD_MAX_OPCODE 0x57
> > +
> > typedef struct CXLMCTPMessage {
> > /*
> > * DSP0236 (MCTP Base) Integrity Check + Message Type
> > @@ -200,7 +203,8 @@ static void i2c_mctp_cxl_handle_message(MCTPI2CEndpoint
> > *mctp)
> > if (!(msg->message_type == MCTP_MT_CXL_TYPE3 &&
> > msg->command_set < 0x51) &&
> > !(msg->message_type == MCTP_MT_CXL_FMAPI &&
> > - msg->command_set >= 0x51 && msg->command_set < 0x56)) {
> > + msg->command_set >= 0x51 &&
> > + msg->command_set < FMAPI_CMD_MAX_OPCODE)) {
>
> Hmm. There is a visibility problem here we should address but probably not
> by introducing a new define. Maybe we should move the enum from
> cxl-mailbox-utils.c in a precursor patch.
>
> Jonathan
Thanks for the feedback and review Jonathan.
According to the comment above this condition, "Any command forming part
of the CXL FM-API command set... is valid only with the CXL Fabric
Manager API over MCTP binding (DSP0234)."
>From my understanding, this check is to ensure that any message
sent from the FM API command set (0x51 - 0x59) has the MCTP_MT_CXL_FMAPI
binding and all other commands (opcode < 0x51) are are sent with the
MCTP_MT_CXL_TYPE3 binding.
Although I see from r3.2 Table 8-230 CXL Defined FM API Command Opcodes
that commands from sets 0x57-0x59 are prohibited from being implemented
in the MCTP CCI, would it be more correct to change the condition for
FMAPI commands to msg->command_set < 0x59? Then if/when commands from sets
0x57-0x59 are implemented, if they are implemented according to the spec, they
should not be added to the FM MCTP CCI.
Please correct my understanding if this is incorrect.
Regarding the visibility problem, I intend to move the enum defining all the
opcodes in cxl-mailbox.utils.c to cxl-mailbox.h and including cxl-mailbox.h
in i2c_mctp_cxl.c
Let me know if that is what you intended.
Other than that, I have removed the extraneous TO-DO's from the other
patches and plan to send out v2 with relevant corrections soon.
Hopefully that makes the remaining patches easier for you to review.
Thanks,
Anisa
>
>
> > buf->rc = CXL_MBOX_UNSUPPORTED;
> > st24_le_p(buf->pl_length, len_out);
> > s->len = s->pos;
>
