On 17/09/25 05:05PM, Jonathan Cameron wrote:
On Tue, 16 Sep 2025 13:37:36 +0530
Arpit Kumar <arpit1.ku...@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.ku...@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)
