Re: [PATCH v4 2/2] hw/cxl: Add Physical Port Control (Opcode 5102h)

[Jonathan Cameron <Jonathan.Cameron@huawei.com>]

On Tue, 16 Sep 2025 13:37:36 +0530
Arpit Kumar <arpit1.kumar@samsung.com> wrote:

> -added assert-deassert PERST implementation
>  for physical ports (both USP and DSP's).
> -assert PERST involves bg operation for holding 100ms.
> -reset PPB implementation for physical ports.
> 
> Signed-off-by: Arpit Kumar <arpit1.kumar@samsung.com>

> @@ -4702,11 +4818,34 @@ static CXLRetCode cxl_set_phy_port_info(CXLCCI *cci)
>  void cxl_initialize_mailbox_swcci(CXLCCI *cci, DeviceState *intf,
>                                    DeviceState *d, size_t payload_max)
>  {
> +    CXLUpstreamPort *pp;
> +    uint8_t pn = 0;
> +
>      cxl_copy_cci_commands(cci, cxl_cmd_set_sw);
>      cci->d = d;
>      cci->intf = intf;
>      cxl_init_cci(cci, payload_max);
>      cxl_set_phy_port_info(cci);
> +    /* physical port control */
> +    pp = CXL_USP(cci->d);
This bit feels like it is wrongly located.  I ran into this whilst
trying to add back the mctp variant as part of shuffling my cxl staging tree.

Whilst this only gets used for the CCI commands, it is a USP thing not
a mailbox thing as we only want this called once per USP, not once per CCI on the
USP.

Could we move this to a call from cxl_usp_realize?

If something like that works would you mind sending me a patch on top of this
series to do so? I'm not yet set up to test this series so better you do it.

We don't need that upstream until the first MCTP support on USP so this doesn't
block us on that front.

 
Thanks,
Jonathan

> +    for (int byte_index = 0; byte_index < (CXL_MAX_PHY_PORTS / BITS_PER_BYTE);
> +         byte_index++) {
> +        unsigned char byte = pp->pports.active_port_bitmask[byte_index];
> +
> +        for (int bit_index = 0; bit_index < 8; bit_index++, pn++) {
> +            if (((byte) & (1 << bit_index)) != 0) {
> +                qemu_mutex_init(&pp->pports.perst[pn].lock);
> +                pp->pports.perst[pn].issued_assert_perst = false;
> +                /*
> +                 * Assert PERST involves physical port to be in
> +                 * hold reset phase for minimum 100ms. No other
> +                 * physical port control requests are entertained
> +                 * until Deassert PERST command.
> +                 */
> +                pp->pports.perst[pn].asrt_time = ASSERT_WAIT_TIME_MS;
> +            }
> +        }
> +    }
>  }
>