Re: [PULL 25/53] hw/cxl/mailbox: interface to add CCI commands to an existing CCI

[Jonathan Cameron via <qemu-devel@nongnu.org>]

On Tue, 4 Jun 2024 09:15:02 -0400
"Michael S. Tsirkin" <mst@redhat.com> wrote:

> On Tue, Jun 04, 2024 at 09:13:26AM -0400, Michael S. Tsirkin wrote:
> > From: Gregory Price <gourry.memverge@gmail.com>
> > 
> > This enables wrapper devices to customize the base device's CCI
> > (for example, with custom commands outside the specification)
> > without the need to change the base device.
> > 
> > The also enabled the base device to dispatch those commands without
> > requiring additional driver support.
> > 
> > Signed-off-by: Gregory Price <gregory.price@memverge.com>
> > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > Signed-off-by: Fan Ni <fan.ni@samsung.com>
> > 
> > --
> > Heavily edited by Jonathan Cameron to increase code reuse
> > Message-Id: <20240523174651.1089554-3-nifan.cxl@gmail.com>
> > Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > ---
> >  include/hw/cxl/cxl_device.h |  2 ++
> >  hw/cxl/cxl-mailbox-utils.c  | 19 +++++++++++++++++--
> >  2 files changed, 19 insertions(+), 2 deletions(-)
> > 
> > diff --git a/include/hw/cxl/cxl_device.h b/include/hw/cxl/cxl_device.h
> > index ccc4611875..a5f8e25020 100644
> > --- a/include/hw/cxl/cxl_device.h
> > +++ b/include/hw/cxl/cxl_device.h
> > @@ -301,6 +301,8 @@ void cxl_initialize_mailbox_t3(CXLCCI *cci, DeviceState *d, size_t payload_max);
> >  void cxl_initialize_mailbox_swcci(CXLCCI *cci, DeviceState *intf,
> >                                    DeviceState *d, size_t payload_max);
> >  void cxl_init_cci(CXLCCI *cci, size_t payload_max);
> > +void cxl_add_cci_commands(CXLCCI *cci, const struct cxl_cmd (*cxl_cmd_set)[256],
> > +                          size_t payload_max);
> >  int cxl_process_cci_message(CXLCCI *cci, uint8_t set, uint8_t cmd,
> >                              size_t len_in, uint8_t *pl_in,
> >                              size_t *len_out, uint8_t *pl_out,
> > diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c
> > index 2c9f50f0f9..4bcd727f4c 100644
> > --- a/hw/cxl/cxl-mailbox-utils.c
> > +++ b/hw/cxl/cxl-mailbox-utils.c
> > @@ -1424,9 +1424,9 @@ static void bg_timercb(void *opaque)
> >      }
> >  }
> >  
> > -void cxl_init_cci(CXLCCI *cci, size_t payload_max)
> > +static void cxl_rebuild_cel(CXLCCI *cci)
> >  {
> > -    cci->payload_max = payload_max;
> > +    cci->cel_size = 0; /* Reset for a fresh build */
> >      for (int set = 0; set < 256; set++) {
> >          for (int cmd = 0; cmd < 256; cmd++) {
> >              if (cci->cxl_cmd_set[set][cmd].handler) {
> > @@ -1440,6 +1440,13 @@ void cxl_init_cci(CXLCCI *cci, size_t payload_max)
> >              }
> >          }
> >      }
> > +}
> > +
> > +void cxl_init_cci(CXLCCI *cci, size_t payload_max)
> > +{
> > +    cci->payload_max = payload_max;
> > +    cxl_rebuild_cel(cci);
> > +
> >      cci->bg.complete_pct = 0;
> >      cci->bg.starttime = 0;
> >      cci->bg.runtime = 0;
> > @@ -1458,6 +1465,14 @@ static void cxl_copy_cci_commands(CXLCCI *cci, const struct cxl_cmd (*cxl_cmds)[
> >      }
> >  }
> >  
> > +void cxl_add_cci_commands(CXLCCI *cci, const struct cxl_cmd (*cxl_cmd_set)[256],
> > +                                 size_t payload_max)
> > +{
> > +    cci->payload_max = payload_max > cci->payload_max ? payload_max : cci->payload_max;  
> 
> Line too long.

Hi Michael,

Your resolution with MAX() on your tree looks good to me.

Thanks for fixing up and sorry we've been such a pain!

Jonathan