Hi Jonathan,
Any update on this?
Thanks,
Anisa
On Mon, Mar 31, 2025 at 12:38 PM Anisa Su <anisa...@samsung.com> wrote:
> 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;
> >
>
