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

[Arpit Kumar <arpit1.kumar@samsung.com>]

[-- Attachment #1: Type: text/plain, Size: 2664 bytes --]

On 17/09/25 05:29PM, Jonathan Cameron wrote:
>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
>
Hi Jonathan,
Sure, will look into it as this seems tricky and then send a patch on top of this series.
Also, it would be helpful if you have any suggestions for going about this change.

Thanks,
Arpit
>> +    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;
>> +            }
>> +        }
>> +    }
>>  }
>>

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]