-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>
---
hw/cxl/cxl-mailbox-utils.c | 139 ++++++++++++++++++++++
include/hw/cxl/cxl_device.h | 9 ++
include/hw/cxl/cxl_mailbox.h | 1 +
include/hw/pci-bridge/cxl_upstream_port.h | 1 +
4 files changed, 150 insertions(+)
diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c
index 2a104dd337..8cccb2c0ed 100644
--- a/hw/cxl/cxl-mailbox-utils.c
+++ b/hw/cxl/cxl-mailbox-utils.c
@@ -541,6 +541,120 @@ static CXLRetCode cmd_get_physical_port_state(const struct cxl_cmd *cmd,
return CXL_MBOX_SUCCESS;
}
+static void *bg_assertcb(void *opaque)
+{
+ CXLPhyPortPerst *perst = opaque;
+
+ /* holding reset phase for 100ms */
+ while (perst->asrt_time--) {
+ usleep(1000);
+ }
+ perst->issued_assert_perst = true;
+ return NULL;
+}
+
+static CXLRetCode deassert_perst(Object *obj, uint8_t pn, CXLUpstreamPort *pp)
+{
+ if (!pp->pports.perst[pn].issued_assert_perst) {
+ return CXL_MBOX_INTERNAL_ERROR;
+ }
+
+ QEMU_LOCK_GUARD(&pp->pports.perst[pn].lock);
+ resettable_release_reset(obj, RESET_TYPE_COLD);
+ pp->pports.perst[pn].issued_assert_perst = false;
+ pp->pports.pport_info[pn].link_state_flags &=
+ ~CXL_PORT_LINK_STATE_FLAG_PERST_ASSERTED;
+ pp->pports.perst[pn].asrt_time = ASSERT_WAIT_TIME_MS;
+
+ return CXL_MBOX_SUCCESS;
+}
+
+static CXLRetCode assert_perst(Object *obj, uint8_t pn, CXLUpstreamPort *pp)
+{
+ if (pp->pports.perst[pn].issued_assert_perst ||
+ pp->pports.perst[pn].asrt_time < ASSERT_WAIT_TIME_MS) {
+ return CXL_MBOX_INTERNAL_ERROR;
+ }
+
+ QEMU_LOCK_GUARD(&pp->pports.perst[pn].lock);
+ pp->pports.pport_info[pn].link_state_flags |=
+ CXL_PORT_LINK_STATE_FLAG_PERST_ASSERTED;
+ resettable_assert_reset(obj, RESET_TYPE_COLD);
+ qemu_thread_create(&pp->pports.perst[pn].asrt_thread, "assert_thread",
+ bg_assertcb, &pp->pports.perst[pn], QEMU_THREAD_DETACHED);
+
+ return CXL_MBOX_SUCCESS;
+}
+
+static struct PCIDevice *cxl_find_port_dev(uint8_t pn, CXLCCI *cci)
+{
+ CXLUpstreamPort *pp = CXL_USP(cci->d);
+ PCIBus *bus = &PCI_BRIDGE(cci->d)->sec_bus;
+
+ if (pp->pports.pport_info[pn].current_port_config_state ==
+ CXL_PORT_CONFIG_STATE_USP) {
+ return pci_bridge_get_device(bus);
+ }
+
+ if (pp->pports.pport_info[pn].current_port_config_state ==
+ CXL_PORT_CONFIG_STATE_DSP) {
+ return pcie_find_port_by_pn(bus, pn);
+ }
+ return NULL;
+}
+
+/* CXL r3.2 Section 7.6.7.1.3: Get Physical Port Control (Opcode 5102h) */
+static CXLRetCode cmd_physical_port_control(const struct cxl_cmd *cmd,
+ uint8_t *payload_in,
+ size_t len_in,
+ uint8_t *payload_out,
+ size_t *len_out,
+ CXLCCI *cci)
+{
+ CXLUpstreamPort *pp = CXL_USP(cci->d);
+ PCIDevice *dev;
+ uint8_t pn;
+ uint8_t ret = CXL_MBOX_SUCCESS;
+
+ struct cxl_fmapi_get_physical_port_control_req_pl {
+ uint8_t ppb_id;
+ uint8_t ports_op;
+ } QEMU_PACKED *in = (void *)payload_in;
+
+ if (len_in < sizeof(*in)) {
+ return CXL_MBOX_INVALID_PAYLOAD_LENGTH;
+ }
+
+ pn = in->ppb_id;
+ if (pp->pports.pport_info[pn].port_id != pn) {
+ return CXL_MBOX_INTERNAL_ERROR;
+ }
+
+ dev = cxl_find_port_dev(pn, cci);
+ if (!dev) {
+ return CXL_MBOX_INTERNAL_ERROR;
+ }
+
+ switch (in->ports_op) {
+ case 0:
+ ret = assert_perst(OBJECT(&dev->qdev), pn, pp);
+ break;
+ case 1:
+ ret = deassert_perst(OBJECT(&dev->qdev), pn, pp);
+ break;
+ case 2:
+ if (pp->pports.perst[pn].issued_assert_perst ||
+ pp->pports.perst[pn].asrt_time < ASSERT_WAIT_TIME_MS) {
+ return CXL_MBOX_INTERNAL_ERROR;
+ }
+ device_cold_reset(&dev->qdev);
+ break;
+ default:
+ return CXL_MBOX_INVALID_INPUT;
+ }
+ return ret;
+}
+
/* CXL r3.1 Section 8.2.9.1.2: Background Operation Status (Opcode 0002h) */
static CXLRetCode cmd_infostat_bg_op_sts(const struct cxl_cmd *cmd,
uint8_t *payload_in,
@@ -4347,6 +4461,8 @@ static const struct cxl_cmd cxl_cmd_set_sw[256][256] = {
cmd_identify_switch_device, 0, 0 },
[PHYSICAL_SWITCH][GET_PHYSICAL_PORT_STATE] = { "SWITCH_PHYSICAL_PORT_STATS",
cmd_get_physical_port_state, ~0, 0 },
+ [PHYSICAL_SWITCH][PHYSICAL_PORT_CONTROL] = { "SWITCH_PHYSICAL_PORT_CONTROL",
+ cmd_physical_port_control, 2, 0 },
[TUNNEL][MANAGEMENT_COMMAND] = { "TUNNEL_MANAGEMENT_COMMAND",
cmd_tunnel_management_cmd, ~0, 0 },
};
@@ -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);
+ 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;
+ }
+ }
+ }
}
void cxl_initialize_mailbox_t3(CXLCCI *cci, DeviceState *d, size_t payload_max)
diff --git a/include/hw/cxl/cxl_device.h b/include/hw/cxl/cxl_device.h
index 536d465f42..4d00e76983 100644
--- a/include/hw/cxl/cxl_device.h
+++ b/include/hw/cxl/cxl_device.h
@@ -130,6 +130,7 @@
(1 << 16))
#define CXL_MAX_PHY_PORTS 256
+#define ASSERT_WAIT_TIME_MS 100 /* Assert - Deassert PERST */
/* CXL r3.2 Table 7-19: Get Physical Port State Port Information Block Format */
#define CXL_PORT_CONFIG_STATE_DISABLED 0x0
@@ -196,6 +197,14 @@ typedef struct CXLPhyPortInfo {
uint8_t supported_ld_count;
} QEMU_PACKED CXLPhyPortInfo;
+/* Assert - Deassert PERST */
+typedef struct CXLPhyPortPerst {
+ bool issued_assert_perst;
+ QemuMutex lock; /* protecting assert-deassert reset request */
+ uint64_t asrt_time;
+ QemuThread asrt_thread; /* thread for 100ms delay */
+} CXLPhyPortPerst;
+
/* CXL r3.1 Table 8-34: Command Return Codes */
typedef enum {
CXL_MBOX_SUCCESS = 0x0,
diff --git a/include/hw/cxl/cxl_mailbox.h b/include/hw/cxl/cxl_mailbox.h
index 5c918c53a9..2c71dab670 100644
--- a/include/hw/cxl/cxl_mailbox.h
+++ b/include/hw/cxl/cxl_mailbox.h
@@ -88,6 +88,7 @@ enum {
PHYSICAL_SWITCH = 0x51,
#define IDENTIFY_SWITCH_DEVICE 0x0
#define GET_PHYSICAL_PORT_STATE 0x1
+ #define PHYSICAL_PORT_CONTROL 0x2
TUNNEL = 0x53,
#define MANAGEMENT_COMMAND 0x0
MHD = 0x55,
diff --git a/include/hw/pci-bridge/cxl_upstream_port.h b/include/hw/pci-bridge/cxl_upstream_port.h
index c6218100a2..e8f5e43faf 100644
--- a/include/hw/pci-bridge/cxl_upstream_port.h
+++ b/include/hw/pci-bridge/cxl_upstream_port.h
@@ -30,6 +30,7 @@ typedef struct CXLUpstreamPort {
uint8_t num_ports;
uint8_t active_port_bitmask[CXL_MAX_PHY_PORTS / BITS_PER_BYTE];
CXLPhyPortInfo pport_info[CXL_MAX_PHY_PORTS];
+ CXLPhyPortPerst perst[CXL_MAX_PHY_PORTS];
} pports;
} CXLUpstreamPort;
--
2.34.1On 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;
> + }
> + }
> + }
> }
>
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;
>> + }
>> + }
>> + }
>> }
>>
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>
See below for why but I've picked this up with this and a define move that
was needed to build it where I've queued it up.
Thanks!
diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c
index 6f7291e6ad..1d16d033c7 100644
--- a/hw/cxl/cxl-mailbox-utils.c
+++ b/hw/cxl/cxl-mailbox-utils.c
@@ -607,7 +607,7 @@ static void *bg_assertcb(void *opaque)
return NULL;
}
-static CXLRetCode deassert_perst(Object *obj, uint8_t pn, CXLUpstreamPort *pp)
+static CXLRetCode cxl_deassert_perst(Object *obj, uint8_t pn, CXLUpstreamPort *pp)
{
if (!pp->pports.perst[pn].issued_assert_perst) {
return CXL_MBOX_INTERNAL_ERROR;
@@ -623,7 +623,7 @@ static CXLRetCode deassert_perst(Object *obj, uint8_t pn, CXLUpstreamPort *pp)
return CXL_MBOX_SUCCESS;
}
-static CXLRetCode assert_perst(Object *obj, uint8_t pn, CXLUpstreamPort *pp)
+static CXLRetCode cxl_assert_perst(Object *obj, uint8_t pn, CXLUpstreamPort *pp)
{
if (pp->pports.perst[pn].issued_assert_perst ||
pp->pports.perst[pn].asrt_time < ASSERT_WAIT_TIME_MS) {
@@ -668,7 +668,6 @@ static CXLRetCode cmd_physical_port_control(const struct cxl_cmd *cmd,
CXLUpstreamPort *pp = CXL_USP(cci->d);
PCIDevice *dev;
uint8_t pn;
- uint8_t ret = CXL_MBOX_SUCCESS;
struct cxl_fmapi_get_physical_port_control_req_pl {
uint8_t ppb_id;
@@ -691,22 +690,19 @@ static CXLRetCode cmd_physical_port_control(const struct cxl_cmd *cmd,
switch (in->ports_op) {
case 0:
- ret = assert_perst(OBJECT(&dev->qdev), pn, pp);
- break;
+ return cxl_assert_perst(OBJECT(&dev->qdev), pn, pp);
case 1:
- ret = deassert_perst(OBJECT(&dev->qdev), pn, pp);
- break;
+ return cxl_deassert_perst(OBJECT(&dev->qdev), pn, pp);
case 2:
if (pp->pports.perst[pn].issued_assert_perst ||
pp->pports.perst[pn].asrt_time < ASSERT_WAIT_TIME_MS) {
return CXL_MBOX_INTERNAL_ERROR;
}
device_cold_reset(&dev->qdev);
- break;
+ return CXL_MBOX_SUCCESS;
default:
return CXL_MBOX_INVALID_INPUT;
}
- return ret;
}
> ---
> hw/cxl/cxl-mailbox-utils.c | 139 ++++++++++++++++++++++
> include/hw/cxl/cxl_device.h | 9 ++
> include/hw/cxl/cxl_mailbox.h | 1 +
Had to move the define for now that went in here because I'm carrying this
further up my tree than Anisa's patch that moved this as a precursor
to the MCTP support.
> include/hw/pci-bridge/cxl_upstream_port.h | 1 +
> 4 files changed, 150 insertions(+)
>
> diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c
> index 2a104dd337..8cccb2c0ed 100644
> --- a/hw/cxl/cxl-mailbox-utils.c
> +++ b/hw/cxl/cxl-mailbox-utils.c
> @@ -541,6 +541,120 @@ static CXLRetCode cmd_get_physical_port_state(const struct cxl_cmd *cmd,
> return CXL_MBOX_SUCCESS;
> }
>
> +static void *bg_assertcb(void *opaque)
> +{
> + CXLPhyPortPerst *perst = opaque;
> +
> + /* holding reset phase for 100ms */
> + while (perst->asrt_time--) {
> + usleep(1000);
> + }
> + perst->issued_assert_perst = true;
> + return NULL;
> +}
> +
> +static CXLRetCode deassert_perst(Object *obj, uint8_t pn, CXLUpstreamPort *pp)
I think we want to namespace these to cxl given these are not
a PCI standard thing. So when I pick these up I'll prefix with cxl_
> +{
> + if (!pp->pports.perst[pn].issued_assert_perst) {
> + return CXL_MBOX_INTERNAL_ERROR;
> + }
> +
> + QEMU_LOCK_GUARD(&pp->pports.perst[pn].lock);
> + resettable_release_reset(obj, RESET_TYPE_COLD);
> + pp->pports.perst[pn].issued_assert_perst = false;
> + pp->pports.pport_info[pn].link_state_flags &=
> + ~CXL_PORT_LINK_STATE_FLAG_PERST_ASSERTED;
> + pp->pports.perst[pn].asrt_time = ASSERT_WAIT_TIME_MS;
> +
> + return CXL_MBOX_SUCCESS;
> +}
> +
> +static CXLRetCode assert_perst(Object *obj, uint8_t pn, CXLUpstreamPort *pp)
> +{
> + if (pp->pports.perst[pn].issued_assert_perst ||
> + pp->pports.perst[pn].asrt_time < ASSERT_WAIT_TIME_MS) {
> + return CXL_MBOX_INTERNAL_ERROR;
> + }
> +
> + QEMU_LOCK_GUARD(&pp->pports.perst[pn].lock);
> + pp->pports.pport_info[pn].link_state_flags |=
> + CXL_PORT_LINK_STATE_FLAG_PERST_ASSERTED;
> + resettable_assert_reset(obj, RESET_TYPE_COLD);
> + qemu_thread_create(&pp->pports.perst[pn].asrt_thread, "assert_thread",
> + bg_assertcb, &pp->pports.perst[pn], QEMU_THREAD_DETACHED);
> +
> + return CXL_MBOX_SUCCESS;
> +}
> +
> +static struct PCIDevice *cxl_find_port_dev(uint8_t pn, CXLCCI *cci)
> +{
> + CXLUpstreamPort *pp = CXL_USP(cci->d);
> + PCIBus *bus = &PCI_BRIDGE(cci->d)->sec_bus;
> +
> + if (pp->pports.pport_info[pn].current_port_config_state ==
> + CXL_PORT_CONFIG_STATE_USP) {
> + return pci_bridge_get_device(bus);
> + }
> +
> + if (pp->pports.pport_info[pn].current_port_config_state ==
> + CXL_PORT_CONFIG_STATE_DSP) {
> + return pcie_find_port_by_pn(bus, pn);
> + }
> + return NULL;
> +}
> +
> +/* CXL r3.2 Section 7.6.7.1.3: Get Physical Port Control (Opcode 5102h) */
> +static CXLRetCode cmd_physical_port_control(const struct cxl_cmd *cmd,
> + uint8_t *payload_in,
> + size_t len_in,
> + uint8_t *payload_out,
> + size_t *len_out,
> + CXLCCI *cci)
> +{
> + CXLUpstreamPort *pp = CXL_USP(cci->d);
> + PCIDevice *dev;
> + uint8_t pn;
> + uint8_t ret = CXL_MBOX_SUCCESS;
> +
> + struct cxl_fmapi_get_physical_port_control_req_pl {
> + uint8_t ppb_id;
> + uint8_t ports_op;
> + } QEMU_PACKED *in = (void *)payload_in;
> +
> + if (len_in < sizeof(*in)) {
> + return CXL_MBOX_INVALID_PAYLOAD_LENGTH;
> + }
> +
> + pn = in->ppb_id;
> + if (pp->pports.pport_info[pn].port_id != pn) {
> + return CXL_MBOX_INTERNAL_ERROR;
> + }
> +
> + dev = cxl_find_port_dev(pn, cci);
> + if (!dev) {
> + return CXL_MBOX_INTERNAL_ERROR;
> + }
> +
> + switch (in->ports_op) {
> + case 0:
> + ret = assert_perst(OBJECT(&dev->qdev), pn, pp);
To me, direct returns are clearer here.
return cxl_assert_perst();
I tweaked this and the other case statements in here. That lets
me drop the local variable ret and the return code hidden up top.
> + break;
> + case 1:
> + ret = deassert_perst(OBJECT(&dev->qdev), pn, pp);
> + break;
> + case 2:
> + if (pp->pports.perst[pn].issued_assert_perst ||
> + pp->pports.perst[pn].asrt_time < ASSERT_WAIT_TIME_MS) {
> + return CXL_MBOX_INTERNAL_ERROR;
> + }
> + device_cold_reset(&dev->qdev);
> + break;
> + default:
> + return CXL_MBOX_INVALID_INPUT;
> + }
> + return ret;
> +}
> +
> /* CXL r3.1 Section 8.2.9.1.2: Background Operation Status (Opcode 0002h) */
> static CXLRetCode cmd_infostat_bg_op_sts(const struct cxl_cmd *cmd,
> uint8_t *payload_in,
> @@ -4347,6 +4461,8 @@ static const struct cxl_cmd cxl_cmd_set_sw[256][256] = {
> cmd_identify_switch_device, 0, 0 },
> [PHYSICAL_SWITCH][GET_PHYSICAL_PORT_STATE] = { "SWITCH_PHYSICAL_PORT_STATS",
> cmd_get_physical_port_state, ~0, 0 },
> + [PHYSICAL_SWITCH][PHYSICAL_PORT_CONTROL] = { "SWITCH_PHYSICAL_PORT_CONTROL",
> + cmd_physical_port_control, 2, 0 },
> [TUNNEL][MANAGEMENT_COMMAND] = { "TUNNEL_MANAGEMENT_COMMAND",
> cmd_tunnel_management_cmd, ~0, 0 },
> };
> @@ -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);
> + 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;
> + }
> + }
> + }
> }
>
> void cxl_initialize_mailbox_t3(CXLCCI *cci, DeviceState *d, size_t payload_max)
On 17/09/25 05:05PM, 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>
>See below for why but I've picked this up with this and a define move that
>was needed to build it where I've queued it up.
>
>Thanks!
>
Understood, the changes look good to me.
Thanks for handling it.
>diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c
>index 6f7291e6ad..1d16d033c7 100644
>--- a/hw/cxl/cxl-mailbox-utils.c
>+++ b/hw/cxl/cxl-mailbox-utils.c
>@@ -607,7 +607,7 @@ static void *bg_assertcb(void *opaque)
> return NULL;
> }
>
>-static CXLRetCode deassert_perst(Object *obj, uint8_t pn, CXLUpstreamPort *pp)
>+static CXLRetCode cxl_deassert_perst(Object *obj, uint8_t pn, CXLUpstreamPort *pp)
> {
> if (!pp->pports.perst[pn].issued_assert_perst) {
> return CXL_MBOX_INTERNAL_ERROR;
>@@ -623,7 +623,7 @@ static CXLRetCode deassert_perst(Object *obj, uint8_t pn, CXLUpstreamPort *pp)
> return CXL_MBOX_SUCCESS;
> }
>
>-static CXLRetCode assert_perst(Object *obj, uint8_t pn, CXLUpstreamPort *pp)
>+static CXLRetCode cxl_assert_perst(Object *obj, uint8_t pn, CXLUpstreamPort *pp)
> {
> if (pp->pports.perst[pn].issued_assert_perst ||
> pp->pports.perst[pn].asrt_time < ASSERT_WAIT_TIME_MS) {
>@@ -668,7 +668,6 @@ static CXLRetCode cmd_physical_port_control(const struct cxl_cmd *cmd,
> CXLUpstreamPort *pp = CXL_USP(cci->d);
> PCIDevice *dev;
> uint8_t pn;
>- uint8_t ret = CXL_MBOX_SUCCESS;
>
> struct cxl_fmapi_get_physical_port_control_req_pl {
> uint8_t ppb_id;
>@@ -691,22 +690,19 @@ static CXLRetCode cmd_physical_port_control(const struct cxl_cmd *cmd,
>
> switch (in->ports_op) {
> case 0:
>- ret = assert_perst(OBJECT(&dev->qdev), pn, pp);
>- break;
>+ return cxl_assert_perst(OBJECT(&dev->qdev), pn, pp);
> case 1:
>- ret = deassert_perst(OBJECT(&dev->qdev), pn, pp);
>- break;
>+ return cxl_deassert_perst(OBJECT(&dev->qdev), pn, pp);
> case 2:
> if (pp->pports.perst[pn].issued_assert_perst ||
> pp->pports.perst[pn].asrt_time < ASSERT_WAIT_TIME_MS) {
> return CXL_MBOX_INTERNAL_ERROR;
> }
> device_cold_reset(&dev->qdev);
>- break;
>+ return CXL_MBOX_SUCCESS;
> default:
> return CXL_MBOX_INVALID_INPUT;
> }
>- return ret;
> }
>
>> ---
>> hw/cxl/cxl-mailbox-utils.c | 139 ++++++++++++++++++++++
>> include/hw/cxl/cxl_device.h | 9 ++
>> include/hw/cxl/cxl_mailbox.h | 1 +
>
>Had to move the define for now that went in here because I'm carrying this
>further up my tree than Anisa's patch that moved this as a precursor
>to the MCTP support.
>
got it
>
>> include/hw/pci-bridge/cxl_upstream_port.h | 1 +
>> 4 files changed, 150 insertions(+)
>>
>> diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c
>> index 2a104dd337..8cccb2c0ed 100644
>> --- a/hw/cxl/cxl-mailbox-utils.c
>> +++ b/hw/cxl/cxl-mailbox-utils.c
>> @@ -541,6 +541,120 @@ static CXLRetCode cmd_get_physical_port_state(const struct cxl_cmd *cmd,
>> return CXL_MBOX_SUCCESS;
>> }
>>
>> +static void *bg_assertcb(void *opaque)
>> +{
>> + CXLPhyPortPerst *perst = opaque;
>> +
>> + /* holding reset phase for 100ms */
>> + while (perst->asrt_time--) {
>> + usleep(1000);
>> + }
>> + perst->issued_assert_perst = true;
>> + return NULL;
>> +}
>> +
>> +static CXLRetCode deassert_perst(Object *obj, uint8_t pn, CXLUpstreamPort *pp)
>
>I think we want to namespace these to cxl given these are not
>a PCI standard thing. So when I pick these up I'll prefix with cxl_
>
got it
>> +{
>> + if (!pp->pports.perst[pn].issued_assert_perst) {
>> + return CXL_MBOX_INTERNAL_ERROR;
>> + }
>> +
>> + QEMU_LOCK_GUARD(&pp->pports.perst[pn].lock);
>> + resettable_release_reset(obj, RESET_TYPE_COLD);
>> + pp->pports.perst[pn].issued_assert_perst = false;
>> + pp->pports.pport_info[pn].link_state_flags &=
>> + ~CXL_PORT_LINK_STATE_FLAG_PERST_ASSERTED;
>> + pp->pports.perst[pn].asrt_time = ASSERT_WAIT_TIME_MS;
>> +
>> + return CXL_MBOX_SUCCESS;
>> +}
>> +
>> +static CXLRetCode assert_perst(Object *obj, uint8_t pn, CXLUpstreamPort *pp)
>> +{
>> + if (pp->pports.perst[pn].issued_assert_perst ||
>> + pp->pports.perst[pn].asrt_time < ASSERT_WAIT_TIME_MS) {
>> + return CXL_MBOX_INTERNAL_ERROR;
>> + }
>> +
>> + QEMU_LOCK_GUARD(&pp->pports.perst[pn].lock);
>> + pp->pports.pport_info[pn].link_state_flags |=
>> + CXL_PORT_LINK_STATE_FLAG_PERST_ASSERTED;
>> + resettable_assert_reset(obj, RESET_TYPE_COLD);
>> + qemu_thread_create(&pp->pports.perst[pn].asrt_thread, "assert_thread",
>> + bg_assertcb, &pp->pports.perst[pn], QEMU_THREAD_DETACHED);
>> +
>> + return CXL_MBOX_SUCCESS;
>> +}
>> +
>> +static struct PCIDevice *cxl_find_port_dev(uint8_t pn, CXLCCI *cci)
>> +{
>> + CXLUpstreamPort *pp = CXL_USP(cci->d);
>> + PCIBus *bus = &PCI_BRIDGE(cci->d)->sec_bus;
>> +
>> + if (pp->pports.pport_info[pn].current_port_config_state ==
>> + CXL_PORT_CONFIG_STATE_USP) {
>> + return pci_bridge_get_device(bus);
>> + }
>> +
>> + if (pp->pports.pport_info[pn].current_port_config_state ==
>> + CXL_PORT_CONFIG_STATE_DSP) {
>> + return pcie_find_port_by_pn(bus, pn);
>> + }
>> + return NULL;
>> +}
>> +
>> +/* CXL r3.2 Section 7.6.7.1.3: Get Physical Port Control (Opcode 5102h) */
>> +static CXLRetCode cmd_physical_port_control(const struct cxl_cmd *cmd,
>> + uint8_t *payload_in,
>> + size_t len_in,
>> + uint8_t *payload_out,
>> + size_t *len_out,
>> + CXLCCI *cci)
>> +{
>> + CXLUpstreamPort *pp = CXL_USP(cci->d);
>> + PCIDevice *dev;
>> + uint8_t pn;
>> + uint8_t ret = CXL_MBOX_SUCCESS;
>> +
>> + struct cxl_fmapi_get_physical_port_control_req_pl {
>> + uint8_t ppb_id;
>> + uint8_t ports_op;
>> + } QEMU_PACKED *in = (void *)payload_in;
>> +
>> + if (len_in < sizeof(*in)) {
>> + return CXL_MBOX_INVALID_PAYLOAD_LENGTH;
>> + }
>> +
>> + pn = in->ppb_id;
>> + if (pp->pports.pport_info[pn].port_id != pn) {
>> + return CXL_MBOX_INTERNAL_ERROR;
>> + }
>> +
>> + dev = cxl_find_port_dev(pn, cci);
>> + if (!dev) {
>> + return CXL_MBOX_INTERNAL_ERROR;
>> + }
>> +
>> + switch (in->ports_op) {
>> + case 0:
>> + ret = assert_perst(OBJECT(&dev->qdev), pn, pp);
>To me, direct returns are clearer here.
> return cxl_assert_perst();
>
>I tweaked this and the other case statements in here. That lets
>me drop the local variable ret and the return code hidden up top.
>
makes sense, thanks!
>> + break;
>> + case 1:
>> + ret = deassert_perst(OBJECT(&dev->qdev), pn, pp);
>> + break;
>> + case 2:
>> + if (pp->pports.perst[pn].issued_assert_perst ||
>> + pp->pports.perst[pn].asrt_time < ASSERT_WAIT_TIME_MS) {
>> + return CXL_MBOX_INTERNAL_ERROR;
>> + }
>> + device_cold_reset(&dev->qdev);
>> + break;
>> + default:
>> + return CXL_MBOX_INVALID_INPUT;
>> + }
>> + return ret;
>> +}
>> +
>> /* CXL r3.1 Section 8.2.9.1.2: Background Operation Status (Opcode 0002h) */
>> static CXLRetCode cmd_infostat_bg_op_sts(const struct cxl_cmd *cmd,
>> uint8_t *payload_in,
>> @@ -4347,6 +4461,8 @@ static const struct cxl_cmd cxl_cmd_set_sw[256][256] = {
>> cmd_identify_switch_device, 0, 0 },
>> [PHYSICAL_SWITCH][GET_PHYSICAL_PORT_STATE] = { "SWITCH_PHYSICAL_PORT_STATS",
>> cmd_get_physical_port_state, ~0, 0 },
>> + [PHYSICAL_SWITCH][PHYSICAL_PORT_CONTROL] = { "SWITCH_PHYSICAL_PORT_CONTROL",
>> + cmd_physical_port_control, 2, 0 },
>> [TUNNEL][MANAGEMENT_COMMAND] = { "TUNNEL_MANAGEMENT_COMMAND",
>> cmd_tunnel_management_cmd, ~0, 0 },
>> };
>> @@ -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);
>> + 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;
>> + }
>> + }
>> + }
>> }
>>
>> void cxl_initialize_mailbox_t3(CXLCCI *cci, DeviceState *d, size_t payload_max)
© 2016 - 2025 Red Hat, Inc.