* [PATCH v3 0/2] FM-API Physical Switch Command Set Support
[not found] <CGME20250904131926epcas5p2a363cf0604a4801038d32e7da5397da1@epcas5p2.samsung.com>
@ 2025-09-04 13:19 ` Arpit Kumar
[not found] ` <CGME20250904131933epcas5p2ab29fa060d8a7df32a222aad740fedc6@epcas5p2.samsung.com>
` (2 more replies)
0 siblings, 3 replies; 11+ messages in thread
From: Arpit Kumar @ 2025-09-04 13:19 UTC (permalink / raw)
To: qemu-devel
Cc: gost.dev, linux-cxl, dave, Jonathan.Cameron, vishak.g,
krish.reddy, a.manzanares, alok.rathore, cpgs, Arpit Kumar
This patch series refactor existing support for Identify Switch Device
and Get Physical Port State by utilizing physical ports (USP & DSP)
information stored during enumeration.
Additionally, it introduces new support for Physical Port Control
of FM-API based physical switch command set as per CXL spec r3.2
Table 8-230:Physical Switch. It primarily constitutes two logic:
-Assert-Deassert PERST: Assert PERST involves physical port to be in
hold reset phase for minimum 100ms. No other physical port control
request are entertained until Deassert PERST command for the given
port is issued.
-Reset PPB: cold reset of physical port (completing enter->hold->exit phases).
Tested using libcxl-mi interface[1]:
All active ports and all opcodes per active port is tested. Also, tested
against possible edge cases manually since the interface currently dosen't
support run time input.
Example topology (1 USP + 3 DSP's->switch with 2 CXLType3 devices connected
to 2 DSP's):
FM="-object memory-backend-file,id=cxl-mem1,mem-path=$TMP_DIR/t3_cxl1.raw,size=256M \
-object memory-backend-file,id=cxl-lsa1,mem-path=$TMP_DIR/t3_lsa1.raw,size=1M \
-object memory-backend-file,id=cxl-mem2,mem-path=$TMP_DIR/t3_cxl2.raw,size=512M \
-object memory-backend-file,id=cxl-lsa2,mem-path=$TMP_DIR/t3_lsa2.raw,size=512M \
-device pxb-cxl,bus_nr=12,bus=pcie.0,id=cxl.1,hdm_for_passthrough=true \
-device cxl-rp,port=0,bus=cxl.1,id=cxl_rp_port0,chassis=0,slot=2 \
-device cxl-upstream,port=2,sn=1234,bus=cxl_rp_port0,id=us0,addr=0.0,multifunction=on, \
-device cxl-switch-mailbox-cci,bus=cxl_rp_port0,addr=0.1,target=us0 \
-device cxl-downstream,port=0,bus=us0,id=swport0,chassis=0,slot=4 \
-device cxl-downstream,port=1,bus=us0,id=swport1,chassis=0,slot=5 \
-device cxl-downstream,port=3,bus=us0,id=swport2,chassis=0,slot=6 \
-device cxl-type3,bus=swport0,memdev=cxl-mem1,id=cxl-pmem1,lsa=cxl-lsa1,sn=3 \
-device cxl-type3,bus=swport2,memdev=cxl-mem2,id=cxl-pmem2,lsa=cxl-lsa2,sn=4 \
-machine cxl-fmw.0.targets.0=cxl.1,cxl-fmw.0.size=4G,cxl-fmw.0.interleave-granularity=1k \
-device i2c_mctp_cxl,bus=aspeed.i2c.bus.0,address=4,target=us0 \
-device i2c_mctp_cxl,bus=aspeed.i2c.bus.0,address=5,target=cxl-pmem1 \
-device i2c_mctp_cxl,bus=aspeed.i2c.bus.0,address=6,target=cxl-pmem2 \
-device virtio-rng-pci,bus=swport1"
Multiple Qemu Topologies tested:
-without any devices connected to downstream ports.
-with virtio-rng-pci devices connected to downstream ports.
-with CXLType3 devices connected to downstream ports.
-with different unique values of ports (both upstream and downstream).
Changes from v2->v3:
-cxl_set_port_type(): optimized storing of strucutre members.
-namespace defines instead of enum.
-Calculating size for active_port_bitmask than hardcoding to 0x20.
-Defined struct phy_port directly inside struct CXLUpstreamPort as pports.
-Renamed struct pperst to struct CXLPhyPortPerst.
-Optimized perst member initializations for ports inside
cxl_initialize_usp_mctpcci() using active_port_bitmask.
[1] https://github.com/computexpresslink/libcxlmi/commit/35fe68bd9a31469f832a87694d7b18d2d50be5b8
The patches are generated against the Johnathan's tree
https://gitlab.com/jic23/qemu.git and branch cxl-2025-07-03.
Signed-off-by: Arpit Kumar <arpit1.kumar@samsung.com>
Arpit Kumar (2):
hw/cxl: Refactored Identify Switch Device & Get Physical Port State
hw/cxl: Add Physical Port Control (Opcode 5102h)
hw/cxl/cxl-mailbox-utils.c | 368 +++++++++++++++-------
include/hw/cxl/cxl_device.h | 76 +++++
include/hw/cxl/cxl_mailbox.h | 1 +
include/hw/pci-bridge/cxl_upstream_port.h | 9 +
4 files changed, 347 insertions(+), 107 deletions(-)
--
2.34.1
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v3 1/2] hw/cxl: Refactored Identify Switch Device & Get Physical Port State
[not found] ` <CGME20250904131933epcas5p2ab29fa060d8a7df32a222aad740fedc6@epcas5p2.samsung.com>
@ 2025-09-04 13:19 ` Arpit Kumar
2025-09-05 15:59 ` Jonathan Cameron via
0 siblings, 1 reply; 11+ messages in thread
From: Arpit Kumar @ 2025-09-04 13:19 UTC (permalink / raw)
To: qemu-devel
Cc: gost.dev, linux-cxl, dave, Jonathan.Cameron, vishak.g,
krish.reddy, a.manzanares, alok.rathore, cpgs, Arpit Kumar
-Storing physical ports info during enumeration.
-Refactored changes using physical ports info for
Identify Switch Device (Opcode 5100h) & Get Physical Port State
(Opcode 5101h) physical switch FM-API command set.
Signed-off-by: Arpit Kumar <arpit1.kumar@samsung.com>
---
hw/cxl/cxl-mailbox-utils.c | 230 ++++++++++++----------
include/hw/cxl/cxl_device.h | 67 +++++++
include/hw/pci-bridge/cxl_upstream_port.h | 8 +
3 files changed, 198 insertions(+), 107 deletions(-)
diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c
index c5177dfd92..cb36880f0b 100644
--- a/hw/cxl/cxl-mailbox-utils.c
+++ b/hw/cxl/cxl-mailbox-utils.c
@@ -435,16 +435,6 @@ static CXLRetCode cmd_set_response_msg_limit(const struct cxl_cmd *cmd,
return CXL_MBOX_SUCCESS;
}
-static void cxl_set_dsp_active_bm(PCIBus *b, PCIDevice *d,
- void *private)
-{
- uint8_t *bm = private;
- if (object_dynamic_cast(OBJECT(d), TYPE_CXL_DSP)) {
- uint8_t port = PCIE_PORT(d)->port;
- bm[port / 8] |= 1 << (port % 8);
- }
-}
-
/* CXL r3.1 Section 7.6.7.1.1: Identify Switch Device (Opcode 5100h) */
static CXLRetCode cmd_identify_switch_device(const struct cxl_cmd *cmd,
uint8_t *payload_in,
@@ -453,9 +443,8 @@ static CXLRetCode cmd_identify_switch_device(const struct cxl_cmd *cmd,
size_t *len_out,
CXLCCI *cci)
{
- PCIEPort *usp = PCIE_PORT(cci->d);
- PCIBus *bus = &PCI_BRIDGE(cci->d)->sec_bus;
- int num_phys_ports = pcie_count_ds_ports(bus);
+ CXLUpstreamPort *pp = CXL_USP(cci->d);
+ uint8_t num_phys_ports = pp->pports.num_ports;
struct cxl_fmapi_ident_switch_dev_resp_pl {
uint8_t ingress_port_id;
@@ -472,11 +461,11 @@ static CXLRetCode cmd_identify_switch_device(const struct cxl_cmd *cmd,
out = (struct cxl_fmapi_ident_switch_dev_resp_pl *)payload_out;
*out = (struct cxl_fmapi_ident_switch_dev_resp_pl) {
- .num_physical_ports = num_phys_ports + 1, /* 1 USP */
+ .num_physical_ports = num_phys_ports,
.num_vcss = 1, /* Not yet support multiple VCS - potentially tricky */
.active_vcs_bitmask[0] = 0x1,
- .total_vppbs = num_phys_ports + 1,
- .bound_vppbs = num_phys_ports + 1,
+ .total_vppbs = num_phys_ports,
+ .bound_vppbs = num_phys_ports,
.num_hdm_decoders_per_usp = 4,
};
@@ -488,16 +477,14 @@ static CXLRetCode cmd_identify_switch_device(const struct cxl_cmd *cmd,
out->ingress_port_id = 0;
}
- pci_for_each_device_under_bus(bus, cxl_set_dsp_active_bm,
- out->active_port_bitmask);
- out->active_port_bitmask[usp->port / 8] |= (1 << usp->port % 8);
-
+ memcpy(out->active_port_bitmask, pp->pports.active_port_bitmask,
+ sizeof(pp->pports.active_port_bitmask));
*len_out = sizeof(*out);
return CXL_MBOX_SUCCESS;
}
-/* CXL r3.1 Section 7.6.7.1.2: Get Physical Port State (Opcode 5101h) */
+/* CXL r3.2 Section 7.6.7.1.2: Get Physical Port State (Opcode 5101h) */
static CXLRetCode cmd_get_physical_port_state(const struct cxl_cmd *cmd,
uint8_t *payload_in,
size_t len_in,
@@ -505,44 +492,22 @@ static CXLRetCode cmd_get_physical_port_state(const struct cxl_cmd *cmd,
size_t *len_out,
CXLCCI *cci)
{
- /* CXL r3.1 Table 7-17: Get Physical Port State Request Payload */
+ CXLUpstreamPort *pp = CXL_USP(cci->d);
+ size_t pl_size;
+ int i;
+
+ /* CXL r3.2 Table 7-17: Get Physical Port State Request Payload */
struct cxl_fmapi_get_phys_port_state_req_pl {
uint8_t num_ports;
uint8_t ports[];
} QEMU_PACKED *in;
- /*
- * CXL r3.1 Table 7-19: Get Physical Port State Port Information Block
- * Format
- */
- struct cxl_fmapi_port_state_info_block {
- uint8_t port_id;
- uint8_t config_state;
- uint8_t connected_device_cxl_version;
- uint8_t rsv1;
- uint8_t connected_device_type;
- uint8_t port_cxl_version_bitmask;
- uint8_t max_link_width;
- uint8_t negotiated_link_width;
- uint8_t supported_link_speeds_vector;
- uint8_t max_link_speed;
- uint8_t current_link_speed;
- uint8_t ltssm_state;
- uint8_t first_lane_num;
- uint16_t link_state;
- uint8_t supported_ld_count;
- } QEMU_PACKED;
-
- /* CXL r3.1 Table 7-18: Get Physical Port State Response Payload */
+ /* CXL r3.2 Table 7-18: Get Physical Port State Response Payload */
struct cxl_fmapi_get_phys_port_state_resp_pl {
uint8_t num_ports;
uint8_t rsv1[3];
- struct cxl_fmapi_port_state_info_block ports[];
+ CXLPhyPortInfo ports[];
} QEMU_PACKED *out;
- PCIBus *bus = &PCI_BRIDGE(cci->d)->sec_bus;
- PCIEPort *usp = PCIE_PORT(cci->d);
- size_t pl_size;
- int i;
in = (struct cxl_fmapi_get_phys_port_state_req_pl *)payload_in;
out = (struct cxl_fmapi_get_phys_port_state_resp_pl *)payload_out;
@@ -555,69 +520,20 @@ static CXLRetCode cmd_get_physical_port_state(const struct cxl_cmd *cmd,
return CXL_MBOX_INVALID_INPUT;
}
- /* For success there should be a match for each requested */
- out->num_ports = in->num_ports;
+ if (in->num_ports > pp->pports.num_ports) {
+ return CXL_MBOX_INVALID_INPUT;
+ }
+ out->num_ports = in->num_ports;
for (i = 0; i < in->num_ports; i++) {
- struct cxl_fmapi_port_state_info_block *port;
- /* First try to match on downstream port */
- PCIDevice *port_dev;
- uint16_t lnkcap, lnkcap2, lnksta;
+ int pn = in->ports[i];
- port = &out->ports[i];
-
- port_dev = pcie_find_port_by_pn(bus, in->ports[i]);
- if (port_dev) { /* DSP */
- PCIDevice *ds_dev = pci_bridge_get_sec_bus(PCI_BRIDGE(port_dev))
- ->devices[0];
- port->config_state = 3;
- if (ds_dev) {
- if (object_dynamic_cast(OBJECT(ds_dev), TYPE_CXL_TYPE3)) {
- port->connected_device_type = 5; /* Assume MLD for now */
- } else {
- port->connected_device_type = 1;
- }
- } else {
- port->connected_device_type = 0;
- }
- port->supported_ld_count = 3;
- } else if (usp->port == in->ports[i]) { /* USP */
- port_dev = PCI_DEVICE(usp);
- port->config_state = 4;
- port->connected_device_type = 0;
- } else {
+ if (pp->pports.pport_info[pn].port_id != pn) {
return CXL_MBOX_INVALID_INPUT;
}
-
- port->port_id = in->ports[i];
- /* Information on status of this port in lnksta, lnkcap */
- if (!port_dev->exp.exp_cap) {
- return CXL_MBOX_INTERNAL_ERROR;
- }
- lnksta = port_dev->config_read(port_dev,
- port_dev->exp.exp_cap + PCI_EXP_LNKSTA,
- sizeof(lnksta));
- lnkcap = port_dev->config_read(port_dev,
- port_dev->exp.exp_cap + PCI_EXP_LNKCAP,
- sizeof(lnkcap));
- lnkcap2 = port_dev->config_read(port_dev,
- port_dev->exp.exp_cap + PCI_EXP_LNKCAP2,
- sizeof(lnkcap2));
-
- port->max_link_width = (lnkcap & PCI_EXP_LNKCAP_MLW) >> 4;
- port->negotiated_link_width = (lnksta & PCI_EXP_LNKSTA_NLW) >> 4;
- /* No definition for SLS field in linux/pci_regs.h */
- port->supported_link_speeds_vector = (lnkcap2 & 0xFE) >> 1;
- port->max_link_speed = lnkcap & PCI_EXP_LNKCAP_SLS;
- port->current_link_speed = lnksta & PCI_EXP_LNKSTA_CLS;
- /* TODO: Track down if we can get the rest of the info */
- port->ltssm_state = 0x7;
- port->first_lane_num = 0;
- port->link_state = 0;
- port->port_cxl_version_bitmask = 0x2;
- port->connected_device_cxl_version = 0x2;
+ memcpy(&out->ports[i], &(pp->pports.pport_info[pn]),
+ sizeof(CXLPhyPortInfo));
}
-
pl_size = sizeof(*out) + sizeof(*out->ports) * in->num_ports;
*len_out = pl_size;
@@ -4684,6 +4600,104 @@ void cxl_add_cci_commands(CXLCCI *cci, const struct cxl_cmd (*cxl_cmd_set)[256],
cxl_rebuild_cel(cci);
}
+static CXLRetCode cxl_set_port_type(CXLUpstreamPort *ports, int pnum,
+ CXLCCI *cci)
+{
+ uint8_t current_port_config_state;
+ uint8_t connected_device_type;
+ uint8_t supported_ld_count;
+ uint16_t lnkcap, lnkcap2, lnksta;
+ PCIBus *bus;
+ PCIDevice *port_dev;
+ PCIEPort *usp = PCIE_PORT(cci->d);
+
+ if (usp->port == pnum) {
+ port_dev = PCI_DEVICE(usp);
+ current_port_config_state = CXL_PORT_CONFIG_STATE_USP;
+ connected_device_type = NO_DEVICE_DETECTED;
+ supported_ld_count = 0;
+ } else {
+ bus = &PCI_BRIDGE(cci->d)->sec_bus;
+ port_dev = pcie_find_port_by_pn(bus, pnum);
+ if (port_dev) { /* DSP */
+ PCIDevice *ds_dev = pci_bridge_get_sec_bus(PCI_BRIDGE(port_dev))
+ ->devices[0];
+ current_port_config_state = CXL_PORT_CONFIG_STATE_DSP;
+ if (ds_dev) {
+ if (object_dynamic_cast(OBJECT(ds_dev), TYPE_CXL_TYPE3)) {
+ /* To-do: controllable */
+ connected_device_type = CXL_TYPE_3_SLD;
+ } else {
+ connected_device_type = PCIE_DEVICE;
+ }
+ } else {
+ connected_device_type = NO_DEVICE_DETECTED;
+ }
+ supported_ld_count = 3;
+ } else {
+ return CXL_MBOX_INVALID_INPUT;
+ }
+ }
+
+ if (!port_dev->exp.exp_cap) {
+ return CXL_MBOX_INTERNAL_ERROR;
+ }
+ lnksta = port_dev->config_read(port_dev,
+ port_dev->exp.exp_cap + PCI_EXP_LNKSTA,
+ sizeof(lnksta));
+ lnkcap = port_dev->config_read(port_dev,
+ port_dev->exp.exp_cap + PCI_EXP_LNKCAP,
+ sizeof(lnkcap));
+ lnkcap2 = port_dev->config_read(port_dev,
+ port_dev->exp.exp_cap + PCI_EXP_LNKCAP2,
+ sizeof(lnkcap2));
+
+ ports->pports.pport_info[pnum] = (CXLPhyPortInfo) {
+ .port_id = pnum,
+ .current_port_config_state = current_port_config_state,
+ .connected_device_mode = STANDARD_256B_FLIT_MODE,
+ .connected_device_type = connected_device_type,
+ .supported_cxl_modes = CXL_256B_FLIT_CAPABLE,
+ .max_link_width = (lnkcap & PCI_EXP_LNKCAP_MLW) >> 4,
+ .negotiated_link_width = (lnksta & PCI_EXP_LNKSTA_NLW) >> 4,
+ .supported_link_speeds_vector = (lnkcap2 & 0xFE) >> 1,
+ .max_link_speed = lnkcap & PCI_EXP_LNKCAP_SLS,
+ .current_link_speed = lnksta & PCI_EXP_LNKSTA_CLS,
+ .ltssm_state = CXL_LTSSM_L2,
+ .first_negotiated_lane_num = 0,
+ .link_state_flags = 0,
+ .supported_ld_count = supported_ld_count,
+ };
+ ports->pports.active_port_bitmask[pnum / 8] |= (1 << pnum % 8);
+
+ return CXL_MBOX_SUCCESS;
+}
+
+static void cxl_set_dsp_port(PCIBus *bus, PCIDevice *dev, void *opaque)
+{
+ CXLCCI *cci = (CXLCCI *)opaque;
+ CXLUpstreamPort *pp = CXL_USP(cci->d);
+
+ if (object_dynamic_cast(OBJECT(dev), TYPE_CXL_DSP)) {
+ cxl_set_port_type(pp, PCIE_PORT(dev)->port, cci);
+ }
+}
+
+static CXLRetCode cxl_set_phy_port_info(CXLCCI *cci)
+{
+ PCIEPort *usp = PCIE_PORT(cci->d);
+ PCIBus *bus = &PCI_BRIDGE(cci->d)->sec_bus;
+ CXLUpstreamPort *pp = CXL_USP(cci->d);
+ int num_phys_ports = pcie_count_ds_ports(bus) + 1;
+ pp->pports.num_ports = num_phys_ports;
+ uint8_t phy_port_num = usp->port;
+
+ cxl_set_port_type(pp, phy_port_num, cci); /* USP */
+ pci_for_each_device_under_bus(bus, cxl_set_dsp_port, cci); /* DSP */
+
+ return CXL_MBOX_SUCCESS;
+}
+
void cxl_initialize_mailbox_swcci(CXLCCI *cci, DeviceState *intf,
DeviceState *d, size_t payload_max)
{
@@ -4691,6 +4705,7 @@ void cxl_initialize_mailbox_swcci(CXLCCI *cci, DeviceState *intf,
cci->d = d;
cci->intf = intf;
cxl_init_cci(cci, payload_max);
+ cxl_set_phy_port_info(cci);
}
void cxl_initialize_mailbox_t3(CXLCCI *cci, DeviceState *d, size_t payload_max)
@@ -4777,4 +4792,5 @@ void cxl_initialize_usp_mctpcci(CXLCCI *cci, DeviceState *d, DeviceState *intf,
cci->d = d;
cci->intf = intf;
cxl_init_cci(cci, payload_max);
+ cxl_set_phy_port_info(cci);
}
diff --git a/include/hw/cxl/cxl_device.h b/include/hw/cxl/cxl_device.h
index 068c20d61e..9fc720ec10 100644
--- a/include/hw/cxl/cxl_device.h
+++ b/include/hw/cxl/cxl_device.h
@@ -129,6 +129,73 @@
CXL_NUM_CPMU_INSTANCES * (1 << 16), \
(1 << 16))
+#define CXL_MAX_PHY_PORTS 256
+
+/* physical port control info - CXL r3.2 table 7-19 */
+#define CXL_PORT_CONFIG_STATE_DISABLED 0x0
+#define CXL_PORT_CONFIG_STATE_BIND_IN_PROGRESS 0x1
+#define CXL_PORT_CONFIG_STATE_UNBIND_IN_PROGRESS 0x2
+#define CXL_PORT_CONFIG_STATE_DSP 0x3
+#define CXL_PORT_CONFIG_STATE_USP 0x4
+#define CXL_PORT_CONFIG_STATE_FABRIC_PORT 0x5
+#define CXL_PORT_CONFIG_STATE_INVALID_PORT_ID 0xF
+
+#define NOT_CXL_OR_DISCONNECTED 0x00
+#define RCD_MODE 0x01
+#define CXL_68B_FLIT_AND_VH_MODE 0x02
+#define STANDARD_256B_FLIT_MODE 0x03
+#define CXL_LATENCY_OPTIMIZED_256B_FLIT_MODE 0x04
+#define PBR_MODE 0x05
+
+#define NO_DEVICE_DETECTED 0
+#define PCIE_DEVICE 1
+#define CXL_TYPE_1_DEVICE 2
+#define CXL_TYPE_2_DEVICE_OR_HBR_SWITCH 3
+#define CXL_TYPE_3_SLD 4
+#define CXL_TYPE_3_MLD 5
+#define PBR_COMPONENT 6
+
+#define CXL_RCD_MODE 0x00
+#define CXL_68B_FLIT_AND_VH_CAPABLE 0x01
+#define CXL_256B_FLIT_CAPABLE 0x02
+#define CXL_LATENCY_OPTIMIZED_256B_FLIT 0x03
+#define CXL_PBR_CAPABLE 0x04
+
+#define CXL_LTSSM_DETECT 0x00
+#define CXL_LTSSM_POLLING 0x01
+#define CXL_LTSSM_CONFIGURATION 0x02
+#define CXL_LTSSM_RECOVERY 0x03
+#define CXL_LTSSM_L0 0x04
+#define CXL_LTSSM_L0S 0x05
+#define CXL_LTSSM_L1 0x06
+#define CXL_LTSSM_L2 0x07
+#define CXL_LTSSM_DISABLED 0x08
+#define CXL_LTSSM_LOOPBACK 0x09
+#define CXL_LTSSM_HOT_RESET 0x0A
+
+#define LINK_STATE_FLAG_LANE_REVERSED BIT(0)
+#define LINK_STATE_FLAG_PERST_ASSERTED BIT(1)
+#define LINK_STATE_FLAG_PRSNT BIT(2)
+#define LINK_STATE_FLAG_POWER_OFF BIT(3)
+
+typedef struct CXLPhyPortInfo {
+ uint8_t port_id;
+ uint8_t current_port_config_state;
+ uint8_t connected_device_mode;
+ uint8_t rsv1;
+ uint8_t connected_device_type;
+ uint8_t supported_cxl_modes;
+ uint8_t max_link_width;
+ uint8_t negotiated_link_width;
+ uint8_t supported_link_speeds_vector;
+ uint8_t max_link_speed;
+ uint8_t current_link_speed;
+ uint8_t ltssm_state;
+ uint8_t first_negotiated_lane_num;
+ uint16_t link_state_flags;
+ uint8_t supported_ld_count;
+} QEMU_PACKED CXLPhyPortInfo;
+
/* CXL r3.1 Table 8-34: Command Return Codes */
typedef enum {
CXL_MBOX_SUCCESS = 0x0,
diff --git a/include/hw/pci-bridge/cxl_upstream_port.h b/include/hw/pci-bridge/cxl_upstream_port.h
index db1dfb6afd..3b7e72bfe0 100644
--- a/include/hw/pci-bridge/cxl_upstream_port.h
+++ b/include/hw/pci-bridge/cxl_upstream_port.h
@@ -4,6 +4,7 @@
#include "hw/pci/pcie.h"
#include "hw/pci/pcie_port.h"
#include "hw/cxl/cxl.h"
+#include "include/hw/cxl/cxl_device.h"
typedef struct CXLUpstreamPort {
/*< private >*/
@@ -23,6 +24,13 @@ typedef struct CXLUpstreamPort {
DOECap doe_cdat;
uint64_t sn;
+
+ /*< physical ports information >*/
+ struct {
+ uint8_t num_ports;
+ uint8_t active_port_bitmask[CXL_MAX_PHY_PORTS / BITS_PER_BYTE];
+ CXLPhyPortInfo pport_info[CXL_MAX_PHY_PORTS];
+ } pports;
} CXLUpstreamPort;
#endif /* CXL_SUP_H */
--
2.34.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v3 2/2] hw/cxl: Add Physical Port Control (Opcode 5102h)
[not found] ` <CGME20250904131944epcas5p351c0e073a975b1347c4a61aa0dd511f3@epcas5p3.samsung.com>
@ 2025-09-04 13:19 ` Arpit Kumar
2025-09-05 8:48 ` ALOK TIWARI
0 siblings, 1 reply; 11+ messages in thread
From: Arpit Kumar @ 2025-09-04 13:19 UTC (permalink / raw)
To: qemu-devel
Cc: gost.dev, linux-cxl, dave, Jonathan.Cameron, vishak.g,
krish.reddy, a.manzanares, alok.rathore, cpgs, Arpit Kumar
-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 | 138 ++++++++++++++++++++++
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, 149 insertions(+)
diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c
index cb36880f0b..a0b76946a2 100644
--- a/hw/cxl/cxl-mailbox-utils.c
+++ b/hw/cxl/cxl-mailbox-utils.c
@@ -540,6 +540,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 &=
+ ~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 |=
+ 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,
@@ -4781,6 +4895,8 @@ static const struct cxl_cmd cxl_cmd_set_usp_mctp[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 },
};
@@ -4791,6 +4907,28 @@ void cxl_initialize_usp_mctpcci(CXLCCI *cci, DeviceState *d, DeviceState *intf,
cxl_copy_cci_commands(cci, cxl_cmd_set_usp_mctp);
cci->d = d;
cci->intf = intf;
+ CXLUpstreamPort *pp;
+ int pn = 0;
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
+ * physcial port control requests are entertained
+ * until Deassert PERST command.
+ */
+ pp->pports.perst[pn].asrt_time = ASSERT_WAIT_TIME_MS;
+ }
+ }
+ }
}
diff --git a/include/hw/cxl/cxl_device.h b/include/hw/cxl/cxl_device.h
index 9fc720ec10..033d9bf11a 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 */
/* physical port control info - CXL r3.2 table 7-19 */
#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..5c31023590 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 3b7e72bfe0..4b9da87d77 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.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v3 2/2] hw/cxl: Add Physical Port Control (Opcode 5102h)
2025-09-04 13:19 ` [PATCH v3 2/2] hw/cxl: Add Physical Port Control (Opcode 5102h) Arpit Kumar
@ 2025-09-05 8:48 ` ALOK TIWARI
2025-09-08 13:03 ` Arpit Kumar
0 siblings, 1 reply; 11+ messages in thread
From: ALOK TIWARI @ 2025-09-05 8:48 UTC (permalink / raw)
To: Arpit Kumar, qemu-devel
Cc: gost.dev, linux-cxl, dave, Jonathan.Cameron, vishak.g,
krish.reddy, a.manzanares, alok.rathore, cpgs
> @@ -4791,6 +4907,28 @@ void cxl_initialize_usp_mctpcci(CXLCCI *cci, DeviceState *d, DeviceState *intf,
> cxl_copy_cci_commands(cci, cxl_cmd_set_usp_mctp);
> cci->d = d;
> cci->intf = intf;
> + CXLUpstreamPort *pp;
> + int pn = 0;
> 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
> + * physcial port control requests are entertained
typo physcial -> physical
> + * until Deassert PERST command.
> + */
> + pp->pports.perst[pn].asrt_time = ASSERT_WAIT_TIME_MS;
> + }
> + }
> + }
> }
> diff --git a/include/hw/cxl/cxl_device.h b/include/hw/cxl/cxl_device.h
> index 9fc720ec10..033d9bf11a 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 */
>
> /* physical port control info - CXL r3.2 table 7-19 */
> #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..5c31023590 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
use 0X2 -> 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 3b7e72bfe0..4b9da87d77 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.1
>
Thanks,
Alok
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 1/2] hw/cxl: Refactored Identify Switch Device & Get Physical Port State
2025-09-04 13:19 ` [PATCH v3 1/2] hw/cxl: Refactored Identify Switch Device & Get Physical Port State Arpit Kumar
@ 2025-09-05 15:59 ` Jonathan Cameron via
2025-09-08 13:48 ` Arpit Kumar
0 siblings, 1 reply; 11+ messages in thread
From: Jonathan Cameron via @ 2025-09-05 15:59 UTC (permalink / raw)
To: Arpit Kumar
Cc: qemu-devel, gost.dev, linux-cxl, dave, vishak.g, krish.reddy,
a.manzanares, alok.rathore, cpgs
On Thu, 4 Sep 2025 18:49:03 +0530
Arpit Kumar <arpit1.kumar@samsung.com> wrote:
> -Storing physical ports info during enumeration.
> -Refactored changes using physical ports info for
> Identify Switch Device (Opcode 5100h) & Get Physical Port State
> (Opcode 5101h) physical switch FM-API command set.
>
> Signed-off-by: Arpit Kumar <arpit1.kumar@samsung.com>
Hi Arpit,
A few minor things inline. Biggest one is making sure
to namespace the defines which is quite challenging to do
here without a really long name.
Jonathan
> ---
> hw/cxl/cxl-mailbox-utils.c | 230 ++++++++++++----------
> include/hw/cxl/cxl_device.h | 67 +++++++
> include/hw/pci-bridge/cxl_upstream_port.h | 8 +
> 3 files changed, 198 insertions(+), 107 deletions(-)
>
> diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c
> index c5177dfd92..cb36880f0b 100644
> --- a/hw/cxl/cxl-mailbox-utils.c
> +++ b/hw/cxl/cxl-mailbox-utils.c
> @@ -435,16 +435,6 @@ static CXLRetCode cmd_set_response_msg_limit(const struct cxl_cmd *cmd,
> return CXL_MBOX_SUCCESS;
> }
> @@ -555,69 +520,20 @@ static CXLRetCode cmd_get_physical_port_state(const struct cxl_cmd *cmd,
> return CXL_MBOX_INVALID_INPUT;
> }
>
> - /* For success there should be a match for each requested */
> - out->num_ports = in->num_ports;
> + if (in->num_ports > pp->pports.num_ports) {
> + return CXL_MBOX_INVALID_INPUT;
> + }
>
> + out->num_ports = in->num_ports;
> for (i = 0; i < in->num_ports; i++) {
> - struct cxl_fmapi_port_state_info_block *port;
> - /* First try to match on downstream port */
> - PCIDevice *port_dev;
> - uint16_t lnkcap, lnkcap2, lnksta;
> + int pn = in->ports[i];
>
> - port = &out->ports[i];
> -
> - port_dev = pcie_find_port_by_pn(bus, in->ports[i]);
> - if (port_dev) { /* DSP */
> - PCIDevice *ds_dev = pci_bridge_get_sec_bus(PCI_BRIDGE(port_dev))
> - ->devices[0];
> - port->config_state = 3;
> - if (ds_dev) {
> - if (object_dynamic_cast(OBJECT(ds_dev), TYPE_CXL_TYPE3)) {
> - port->connected_device_type = 5; /* Assume MLD for now */
> - } else {
> - port->connected_device_type = 1;
> - }
> - } else {
> - port->connected_device_type = 0;
> - }
> - port->supported_ld_count = 3;
> - } else if (usp->port == in->ports[i]) { /* USP */
> - port_dev = PCI_DEVICE(usp);
> - port->config_state = 4;
> - port->connected_device_type = 0;
> - } else {
> + if (pp->pports.pport_info[pn].port_id != pn) {
> return CXL_MBOX_INVALID_INPUT;
> }
> -
> - port->port_id = in->ports[i];
> - /* Information on status of this port in lnksta, lnkcap */
> - if (!port_dev->exp.exp_cap) {
> - return CXL_MBOX_INTERNAL_ERROR;
> - }
> - lnksta = port_dev->config_read(port_dev,
> - port_dev->exp.exp_cap + PCI_EXP_LNKSTA,
> - sizeof(lnksta));
> - lnkcap = port_dev->config_read(port_dev,
> - port_dev->exp.exp_cap + PCI_EXP_LNKCAP,
> - sizeof(lnkcap));
> - lnkcap2 = port_dev->config_read(port_dev,
> - port_dev->exp.exp_cap + PCI_EXP_LNKCAP2,
> - sizeof(lnkcap2));
> -
> - port->max_link_width = (lnkcap & PCI_EXP_LNKCAP_MLW) >> 4;
> - port->negotiated_link_width = (lnksta & PCI_EXP_LNKSTA_NLW) >> 4;
> - /* No definition for SLS field in linux/pci_regs.h */
> - port->supported_link_speeds_vector = (lnkcap2 & 0xFE) >> 1;
> - port->max_link_speed = lnkcap & PCI_EXP_LNKCAP_SLS;
> - port->current_link_speed = lnksta & PCI_EXP_LNKSTA_CLS;
> - /* TODO: Track down if we can get the rest of the info */
> - port->ltssm_state = 0x7;
> - port->first_lane_num = 0;
> - port->link_state = 0;
> - port->port_cxl_version_bitmask = 0x2;
> - port->connected_device_cxl_version = 0x2;
> + memcpy(&out->ports[i], &(pp->pports.pport_info[pn]),
> + sizeof(CXLPhyPortInfo));
> }
> -
That's a stray change that shouldn't be here.
> pl_size = sizeof(*out) + sizeof(*out->ports) * in->num_ports;
> *len_out = pl_size;
>
> +static void cxl_set_dsp_port(PCIBus *bus, PCIDevice *dev, void *opaque)
> +{
> + CXLCCI *cci = (CXLCCI *)opaque;
That cast isn't needed. You can always implicitly cast from a void *
to another pointer type (and the other way).
> + CXLUpstreamPort *pp = CXL_USP(cci->d);
> +
> + if (object_dynamic_cast(OBJECT(dev), TYPE_CXL_DSP)) {
> + cxl_set_port_type(pp, PCIE_PORT(dev)->port, cci);
> + }
> +}
> +
> +static CXLRetCode cxl_set_phy_port_info(CXLCCI *cci)
> +{
> + PCIEPort *usp = PCIE_PORT(cci->d);
> + PCIBus *bus = &PCI_BRIDGE(cci->d)->sec_bus;
> + CXLUpstreamPort *pp = CXL_USP(cci->d);
> + int num_phys_ports = pcie_count_ds_ports(bus) + 1;
> + pp->pports.num_ports = num_phys_ports;
Don't mix and match declarations and non declarations.
> + uint8_t phy_port_num = usp->port;
> +
> + cxl_set_port_type(pp, phy_port_num, cci); /* USP */
> + pci_for_each_device_under_bus(bus, cxl_set_dsp_port, cci); /* DSP */
> +
> + return CXL_MBOX_SUCCESS;
> +}
> diff --git a/include/hw/cxl/cxl_device.h b/include/hw/cxl/cxl_device.h
> index 068c20d61e..9fc720ec10 100644
> --- a/include/hw/cxl/cxl_device.h
> +++ b/include/hw/cxl/cxl_device.h
> @@ -129,6 +129,73 @@
> CXL_NUM_CPMU_INSTANCES * (1 << 16), \
> (1 << 16))
>
> +#define CXL_MAX_PHY_PORTS 256
> +
> +/* physical port control info - CXL r3.2 table 7-19 */
Stick to local style. That makes this
/* CXL r3.2 Table 7-19: Get Physical Port State Port Information Block Format */
> +#define CXL_PORT_CONFIG_STATE_DISABLED 0x0
> +#define CXL_PORT_CONFIG_STATE_BIND_IN_PROGRESS 0x1
> +#define CXL_PORT_CONFIG_STATE_UNBIND_IN_PROGRESS 0x2
> +#define CXL_PORT_CONFIG_STATE_DSP 0x3
> +#define CXL_PORT_CONFIG_STATE_USP 0x4
> +#define CXL_PORT_CONFIG_STATE_FABRIC_PORT 0x5
> +#define CXL_PORT_CONFIG_STATE_INVALID_PORT_ID 0xF
> +
> +#define NOT_CXL_OR_DISCONNECTED 0x00
These need a clean prefix like the you have for CONFIG_STATE
Some of these names are tricky to put concisely.
#define CXL_PORT_CONNECTED_DEV_MODE_NOT_CXL_OR_DISCONN 0x0
#define CXL_PORT_CONNECTED_DEV_MODE_RCD 0x1
#define CXL_PORT_CONNECTED_DEV_MODE_68B_VH 0x2
#define CXL_PORT_CONNECTED_DEV_MODE_256B 0x3
#define CXL_PORT_CONNECTED_DEV_MODE_LO_256B 0x4
#define CXL_PORT_CONNECTED_DEV_MODE_PBR 0x5
Is about the shortest that covers what these are.
Kind of need to retain these are about the device down the
other end of the wires. Only one bibblee so can use just 1 character.
> +#define RCD_MODE 0x01
> +#define CXL_68B_FLIT_AND_VH_MODE 0x02
> +#define STANDARD_256B_FLIT_MODE 0x03
> +#define CXL_LATENCY_OPTIMIZED_256B_FLIT_MODE 0x04
> +#define PBR_MODE 0x05
> +
> +#define NO_DEVICE_DETECTED 0
Similar here - also keep to hex to match spec and 2 digits as this one is byte.
#define CXL_PORT_CONNECTED_DEV_TYPE_NONE 0x00
#define CXL_PORT_CONNECTED_DEV_TYPE_PCIE 0x01
etc
> +#define PCIE_DEVICE 1
> +#define CXL_TYPE_1_DEVICE 2
> +#define CXL_TYPE_2_DEVICE_OR_HBR_SWITCH 3
> +#define CXL_TYPE_3_SLD 4
> +#define CXL_TYPE_3_MLD 5
> +#define PBR_COMPONENT 6
> +
> +#define CXL_RCD_MODE 0x00
> +#define CXL_68B_FLIT_AND_VH_CAPABLE 0x01
> +#define CXL_256B_FLIT_CAPABLE 0x02
> +#define CXL_LATENCY_OPTIMIZED_256B_FLIT 0x03
> +#define CXL_PBR_CAPABLE 0x04
This lot are confusing as they are a bits in a bitmap. I'd express them as
#define CXL_PORT_SUPPORTS_RCD BIT(0)
#define CXL_PORT_SUPPORTS_68B_VH BIT(1)
etc, matching names with above suggestion on short forms.
> +
> +#define CXL_LTSSM_DETECT 0x00
I'd call out the PORT bit for these as they aren't generic
and that this is the current state.
CXL_PORT_LTSSM_STATE_DETECT etc
> +#define CXL_LTSSM_POLLING 0x01
> +#define CXL_LTSSM_CONFIGURATION 0x02
> +#define CXL_LTSSM_RECOVERY 0x03
> +#define CXL_LTSSM_L0 0x04
> +#define CXL_LTSSM_L0S 0x05
> +#define CXL_LTSSM_L1 0x06
> +#define CXL_LTSSM_L2 0x07
> +#define CXL_LTSSM_DISABLED 0x08
> +#define CXL_LTSSM_LOOPBACK 0x09
> +#define CXL_LTSSM_HOT_RESET 0x0A
> +
> +#define LINK_STATE_FLAG_LANE_REVERSED BIT(0)
Probably stick CXL_PORT_ as prefix for these as well.
> +#define LINK_STATE_FLAG_PERST_ASSERTED BIT(1)
> +#define LINK_STATE_FLAG_PRSNT BIT(2)
> +#define LINK_STATE_FLAG_POWER_OFF BIT(3)
> +
> +typedef struct CXLPhyPortInfo {
> + uint8_t port_id;
> + uint8_t current_port_config_state;
> + uint8_t connected_device_mode;
> + uint8_t rsv1;
> + uint8_t connected_device_type;
> + uint8_t supported_cxl_modes;
> + uint8_t max_link_width;
> + uint8_t negotiated_link_width;
> + uint8_t supported_link_speeds_vector;
> + uint8_t max_link_speed;
> + uint8_t current_link_speed;
> + uint8_t ltssm_state;
> + uint8_t first_negotiated_lane_num;
> + uint16_t link_state_flags;
> + uint8_t supported_ld_count;
> +} QEMU_PACKED CXLPhyPortInfo;
> +
> /* CXL r3.1 Table 8-34: Command Return Codes */
> typedef enum {
> CXL_MBOX_SUCCESS = 0x0,
> diff --git a/include/hw/pci-bridge/cxl_upstream_port.h b/include/hw/pci-bridge/cxl_upstream_port.h
> index db1dfb6afd..3b7e72bfe0 100644
> --- a/include/hw/pci-bridge/cxl_upstream_port.h
> +++ b/include/hw/pci-bridge/cxl_upstream_port.h
> @@ -4,6 +4,7 @@
> #include "hw/pci/pcie.h"
> #include "hw/pci/pcie_port.h"
> #include "hw/cxl/cxl.h"
> +#include "include/hw/cxl/cxl_device.h"
>
> typedef struct CXLUpstreamPort {
> /*< private >*/
> @@ -23,6 +24,13 @@ typedef struct CXLUpstreamPort {
>
> DOECap doe_cdat;
> uint64_t sn;
> +
> + /*< physical ports information >*/
I believe the magic /*< private >*/ syntax with the <> is special for public/private.
There are a few other comments marked like that in qemu, but very very few.
So this probably just wants to be
/* Physical ports information */
> + struct {
> + uint8_t num_ports;
> + uint8_t active_port_bitmask[CXL_MAX_PHY_PORTS / BITS_PER_BYTE];
> + CXLPhyPortInfo pport_info[CXL_MAX_PHY_PORTS];
> + } pports;
> } CXLUpstreamPort;
>
> #endif /* CXL_SUP_H */
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 0/2] FM-API Physical Switch Command Set Support
2025-09-04 13:19 ` [PATCH v3 0/2] FM-API Physical Switch Command Set Support Arpit Kumar
[not found] ` <CGME20250904131933epcas5p2ab29fa060d8a7df32a222aad740fedc6@epcas5p2.samsung.com>
[not found] ` <CGME20250904131944epcas5p351c0e073a975b1347c4a61aa0dd511f3@epcas5p3.samsung.com>
@ 2025-09-05 16:12 ` Jonathan Cameron via
2025-09-08 13:22 ` Arpit Kumar
2 siblings, 1 reply; 11+ messages in thread
From: Jonathan Cameron via @ 2025-09-05 16:12 UTC (permalink / raw)
To: Arpit Kumar
Cc: qemu-devel, gost.dev, linux-cxl, dave, vishak.g, krish.reddy,
a.manzanares, alok.rathore, cpgs
On Thu, 4 Sep 2025 18:49:02 +0530
Arpit Kumar <arpit1.kumar@samsung.com> wrote:
> This patch series refactor existing support for Identify Switch Device
> and Get Physical Port State by utilizing physical ports (USP & DSP)
> information stored during enumeration.
>
> Additionally, it introduces new support for Physical Port Control
> of FM-API based physical switch command set as per CXL spec r3.2
> Table 8-230:Physical Switch. It primarily constitutes two logic:
> -Assert-Deassert PERST: Assert PERST involves physical port to be in
> hold reset phase for minimum 100ms. No other physical port control
> request are entertained until Deassert PERST command for the given
> port is issued.
> -Reset PPB: cold reset of physical port (completing enter->hold->exit phases).
>
> Tested using libcxl-mi interface[1]:
> All active ports and all opcodes per active port is tested. Also, tested
> against possible edge cases manually since the interface currently dosen't
> support run time input.
>
> Example topology (1 USP + 3 DSP's->switch with 2 CXLType3 devices connected
> to 2 DSP's):
> FM="-object memory-backend-file,id=cxl-mem1,mem-path=$TMP_DIR/t3_cxl1.raw,size=256M \
> -object memory-backend-file,id=cxl-lsa1,mem-path=$TMP_DIR/t3_lsa1.raw,size=1M \
> -object memory-backend-file,id=cxl-mem2,mem-path=$TMP_DIR/t3_cxl2.raw,size=512M \
> -object memory-backend-file,id=cxl-lsa2,mem-path=$TMP_DIR/t3_lsa2.raw,size=512M \
> -device pxb-cxl,bus_nr=12,bus=pcie.0,id=cxl.1,hdm_for_passthrough=true \
> -device cxl-rp,port=0,bus=cxl.1,id=cxl_rp_port0,chassis=0,slot=2 \
> -device cxl-upstream,port=2,sn=1234,bus=cxl_rp_port0,id=us0,addr=0.0,multifunction=on, \
> -device cxl-switch-mailbox-cci,bus=cxl_rp_port0,addr=0.1,target=us0 \
> -device cxl-downstream,port=0,bus=us0,id=swport0,chassis=0,slot=4 \
> -device cxl-downstream,port=1,bus=us0,id=swport1,chassis=0,slot=5 \
> -device cxl-downstream,port=3,bus=us0,id=swport2,chassis=0,slot=6 \
> -device cxl-type3,bus=swport0,memdev=cxl-mem1,id=cxl-pmem1,lsa=cxl-lsa1,sn=3 \
> -device cxl-type3,bus=swport2,memdev=cxl-mem2,id=cxl-pmem2,lsa=cxl-lsa2,sn=4 \
> -machine cxl-fmw.0.targets.0=cxl.1,cxl-fmw.0.size=4G,cxl-fmw.0.interleave-granularity=1k \
> -device i2c_mctp_cxl,bus=aspeed.i2c.bus.0,address=4,target=us0 \
> -device i2c_mctp_cxl,bus=aspeed.i2c.bus.0,address=5,target=cxl-pmem1 \
> -device i2c_mctp_cxl,bus=aspeed.i2c.bus.0,address=6,target=cxl-pmem2 \
> -device virtio-rng-pci,bus=swport1"
>
> Multiple Qemu Topologies tested:
> -without any devices connected to downstream ports.
> -with virtio-rng-pci devices connected to downstream ports.
> -with CXLType3 devices connected to downstream ports.
> -with different unique values of ports (both upstream and downstream).
>
> Changes from v2->v3:
> -cxl_set_port_type(): optimized storing of strucutre members.
> -namespace defines instead of enum.
> -Calculating size for active_port_bitmask than hardcoding to 0x20.
> -Defined struct phy_port directly inside struct CXLUpstreamPort as pports.
> -Renamed struct pperst to struct CXLPhyPortPerst.
> -Optimized perst member initializations for ports inside
> cxl_initialize_usp_mctpcci() using active_port_bitmask.
>
> [1] https://github.com/computexpresslink/libcxlmi/commit/35fe68bd9a31469f832a87694d7b18d2d50be5b8
>
> The patches are generated against the Johnathan's tree
> https://gitlab.com/jic23/qemu.git and branch cxl-2025-07-03.
>
> Signed-off-by: Arpit Kumar <arpit1.kumar@samsung.com>
Hi Arpit,
I'll have a go (probably next week) at rebasing this rather earlier in my tree as I'd
like to get this upstream without it having a dependency on the MCTP support.
That means bring it up with the switch-cci / pcie mailbox CCI and squashing
the MCTP bit into the patch that brings that support up later in my tree.
I do plan to fix up the remaining 'feature' gap on the FMAPI/MCTP/USB
emulation which is that it's ignoring the MTU to the host and so not
breaking messages up as it should. Linux doesn't care but maybe some other
OS will. Not entirely sure when I'll get to that though and I'd like to
move your work forward before that.
Jonathan
>
> Arpit Kumar (2):
> hw/cxl: Refactored Identify Switch Device & Get Physical Port State
> hw/cxl: Add Physical Port Control (Opcode 5102h)
>
> hw/cxl/cxl-mailbox-utils.c | 368 +++++++++++++++-------
> include/hw/cxl/cxl_device.h | 76 +++++
> include/hw/cxl/cxl_mailbox.h | 1 +
> include/hw/pci-bridge/cxl_upstream_port.h | 9 +
> 4 files changed, 347 insertions(+), 107 deletions(-)
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 2/2] hw/cxl: Add Physical Port Control (Opcode 5102h)
2025-09-05 8:48 ` ALOK TIWARI
@ 2025-09-08 13:03 ` Arpit Kumar
0 siblings, 0 replies; 11+ messages in thread
From: Arpit Kumar @ 2025-09-08 13:03 UTC (permalink / raw)
To: ALOK TIWARI
Cc: qemu-devel, gost.dev, linux-cxl, dave, Jonathan.Cameron, vishak.g,
krish.reddy, a.manzanares, alok.rathore, cpgs
[-- Attachment #1: Type: text/plain, Size: 3487 bytes --]
On 05/09/25 02:18PM, ALOK TIWARI wrote:
>
>>@@ -4791,6 +4907,28 @@ void cxl_initialize_usp_mctpcci(CXLCCI *cci, DeviceState *d, DeviceState *intf,
>> cxl_copy_cci_commands(cci, cxl_cmd_set_usp_mctp);
>> cci->d = d;
>> cci->intf = intf;
>>+ CXLUpstreamPort *pp;
>>+ int pn = 0;
>> 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
>>+ * physcial port control requests are entertained
>
>typo physcial -> physical
>
Thanks for pointing out!
>>+ * until Deassert PERST command.
>>+ */
>>+ pp->pports.perst[pn].asrt_time = ASSERT_WAIT_TIME_MS;
>>+ }
>>+ }
>>+ }
>> }
>>diff --git a/include/hw/cxl/cxl_device.h b/include/hw/cxl/cxl_device.h
>>index 9fc720ec10..033d9bf11a 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 */
>> /* physical port control info - CXL r3.2 table 7-19 */
>> #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..5c31023590 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
>
>use 0X2 -> 0x2
>
Thanks for pointing out the typo's, will update the same in v4.
>> 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 3b7e72bfe0..4b9da87d77 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.1
>>
>
>Thanks,
>Alok
>
[-- Attachment #2: Type: text/plain, Size: 0 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 0/2] FM-API Physical Switch Command Set Support
2025-09-05 16:12 ` [PATCH v3 0/2] FM-API Physical Switch Command Set Support Jonathan Cameron via
@ 2025-09-08 13:22 ` Arpit Kumar
2025-09-09 15:03 ` Jonathan Cameron via
0 siblings, 1 reply; 11+ messages in thread
From: Arpit Kumar @ 2025-09-08 13:22 UTC (permalink / raw)
To: Jonathan Cameron
Cc: qemu-devel, gost.dev, linux-cxl, dave, vishak.g, krish.reddy,
a.manzanares, alok.rathore, cpgs
[-- Attachment #1: Type: text/plain, Size: 5118 bytes --]
On 05/09/25 05:12PM, Jonathan Cameron wrote:
>On Thu, 4 Sep 2025 18:49:02 +0530
>Arpit Kumar <arpit1.kumar@samsung.com> wrote:
>
>> This patch series refactor existing support for Identify Switch Device
>> and Get Physical Port State by utilizing physical ports (USP & DSP)
>> information stored during enumeration.
>>
>> Additionally, it introduces new support for Physical Port Control
>> of FM-API based physical switch command set as per CXL spec r3.2
>> Table 8-230:Physical Switch. It primarily constitutes two logic:
>> -Assert-Deassert PERST: Assert PERST involves physical port to be in
>> hold reset phase for minimum 100ms. No other physical port control
>> request are entertained until Deassert PERST command for the given
>> port is issued.
>> -Reset PPB: cold reset of physical port (completing enter->hold->exit phases).
>>
>> Tested using libcxl-mi interface[1]:
>> All active ports and all opcodes per active port is tested. Also, tested
>> against possible edge cases manually since the interface currently dosen't
>> support run time input.
>>
>> Example topology (1 USP + 3 DSP's->switch with 2 CXLType3 devices connected
>> to 2 DSP's):
>> FM="-object memory-backend-file,id=cxl-mem1,mem-path=$TMP_DIR/t3_cxl1.raw,size=256M \
>> -object memory-backend-file,id=cxl-lsa1,mem-path=$TMP_DIR/t3_lsa1.raw,size=1M \
>> -object memory-backend-file,id=cxl-mem2,mem-path=$TMP_DIR/t3_cxl2.raw,size=512M \
>> -object memory-backend-file,id=cxl-lsa2,mem-path=$TMP_DIR/t3_lsa2.raw,size=512M \
>> -device pxb-cxl,bus_nr=12,bus=pcie.0,id=cxl.1,hdm_for_passthrough=true \
>> -device cxl-rp,port=0,bus=cxl.1,id=cxl_rp_port0,chassis=0,slot=2 \
>> -device cxl-upstream,port=2,sn=1234,bus=cxl_rp_port0,id=us0,addr=0.0,multifunction=on, \
>> -device cxl-switch-mailbox-cci,bus=cxl_rp_port0,addr=0.1,target=us0 \
>> -device cxl-downstream,port=0,bus=us0,id=swport0,chassis=0,slot=4 \
>> -device cxl-downstream,port=1,bus=us0,id=swport1,chassis=0,slot=5 \
>> -device cxl-downstream,port=3,bus=us0,id=swport2,chassis=0,slot=6 \
>> -device cxl-type3,bus=swport0,memdev=cxl-mem1,id=cxl-pmem1,lsa=cxl-lsa1,sn=3 \
>> -device cxl-type3,bus=swport2,memdev=cxl-mem2,id=cxl-pmem2,lsa=cxl-lsa2,sn=4 \
>> -machine cxl-fmw.0.targets.0=cxl.1,cxl-fmw.0.size=4G,cxl-fmw.0.interleave-granularity=1k \
>> -device i2c_mctp_cxl,bus=aspeed.i2c.bus.0,address=4,target=us0 \
>> -device i2c_mctp_cxl,bus=aspeed.i2c.bus.0,address=5,target=cxl-pmem1 \
>> -device i2c_mctp_cxl,bus=aspeed.i2c.bus.0,address=6,target=cxl-pmem2 \
>> -device virtio-rng-pci,bus=swport1"
>>
>> Multiple Qemu Topologies tested:
>> -without any devices connected to downstream ports.
>> -with virtio-rng-pci devices connected to downstream ports.
>> -with CXLType3 devices connected to downstream ports.
>> -with different unique values of ports (both upstream and downstream).
>>
>> Changes from v2->v3:
>> -cxl_set_port_type(): optimized storing of strucutre members.
>> -namespace defines instead of enum.
>> -Calculating size for active_port_bitmask than hardcoding to 0x20.
>> -Defined struct phy_port directly inside struct CXLUpstreamPort as pports.
>> -Renamed struct pperst to struct CXLPhyPortPerst.
>> -Optimized perst member initializations for ports inside
>> cxl_initialize_usp_mctpcci() using active_port_bitmask.
>>
>> [1] https://github.com/computexpresslink/libcxlmi/commit/35fe68bd9a31469f832a87694d7b18d2d50be5b8
>>
>> The patches are generated against the Johnathan's tree
>> https://gitlab.com/jic23/qemu.git and branch cxl-2025-07-03.
>>
>> Signed-off-by: Arpit Kumar <arpit1.kumar@samsung.com>
>
>Hi Arpit,
>
>I'll have a go (probably next week) at rebasing this rather earlier in my tree as I'd
>like to get this upstream without it having a dependency on the MCTP support.
>
>That means bring it up with the switch-cci / pcie mailbox CCI and squashing
>the MCTP bit into the patch that brings that support up later in my tree.
>
>I do plan to fix up the remaining 'feature' gap on the FMAPI/MCTP/USB
>emulation which is that it's ignoring the MTU to the host and so not
>breaking messages up as it should. Linux doesn't care but maybe some other
>OS will. Not entirely sure when I'll get to that though and I'd like to
>move your work forward before that.
>
>Jonathan
>
>
Hi Jonathan,
Thanks for the review comments!
As per my understanding from CXL spec r3.2 Table 8-215: Physical Port Control
request is allowed only for switch FM interface and is prohibited for
switch-cci/pcie mailbox CCI. However, if possible, should I be using
cxl_initialize_mailbox_swcci() to initialize my perst members?
Thanks,
Arpit.
>>
>> Arpit Kumar (2):
>> hw/cxl: Refactored Identify Switch Device & Get Physical Port State
>> hw/cxl: Add Physical Port Control (Opcode 5102h)
>>
>> hw/cxl/cxl-mailbox-utils.c | 368 +++++++++++++++-------
>> include/hw/cxl/cxl_device.h | 76 +++++
>> include/hw/cxl/cxl_mailbox.h | 1 +
>> include/hw/pci-bridge/cxl_upstream_port.h | 9 +
>> 4 files changed, 347 insertions(+), 107 deletions(-)
>>
>
[-- Attachment #2: Type: text/plain, Size: 0 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 1/2] hw/cxl: Refactored Identify Switch Device & Get Physical Port State
2025-09-05 15:59 ` Jonathan Cameron via
@ 2025-09-08 13:48 ` Arpit Kumar
2025-09-09 15:07 ` Jonathan Cameron via
0 siblings, 1 reply; 11+ messages in thread
From: Arpit Kumar @ 2025-09-08 13:48 UTC (permalink / raw)
To: Jonathan Cameron
Cc: qemu-devel, gost.dev, linux-cxl, dave, vishak.g, krish.reddy,
a.manzanares, alok.rathore, cpgs
[-- Attachment #1: Type: text/plain, Size: 12371 bytes --]
On 05/09/25 04:59PM, Jonathan Cameron wrote:
>On Thu, 4 Sep 2025 18:49:03 +0530
>Arpit Kumar <arpit1.kumar@samsung.com> wrote:
>
>> -Storing physical ports info during enumeration.
>> -Refactored changes using physical ports info for
>> Identify Switch Device (Opcode 5100h) & Get Physical Port State
>> (Opcode 5101h) physical switch FM-API command set.
>>
>> Signed-off-by: Arpit Kumar <arpit1.kumar@samsung.com>
>Hi Arpit,
>
>A few minor things inline. Biggest one is making sure
>to namespace the defines which is quite challenging to do
>here without a really long name.
>
>Jonathan
>
Hi Jonathan,
Thanks for the review! Will update the changes in next
iteration v4 of the patch-set.
>> ---
>> hw/cxl/cxl-mailbox-utils.c | 230 ++++++++++++----------
>> include/hw/cxl/cxl_device.h | 67 +++++++
>> include/hw/pci-bridge/cxl_upstream_port.h | 8 +
>> 3 files changed, 198 insertions(+), 107 deletions(-)
>>
>> diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c
>> index c5177dfd92..cb36880f0b 100644
>> --- a/hw/cxl/cxl-mailbox-utils.c
>> +++ b/hw/cxl/cxl-mailbox-utils.c
>> @@ -435,16 +435,6 @@ static CXLRetCode cmd_set_response_msg_limit(const struct cxl_cmd *cmd,
>> return CXL_MBOX_SUCCESS;
>> }
>
>> @@ -555,69 +520,20 @@ static CXLRetCode cmd_get_physical_port_state(const struct cxl_cmd *cmd,
>> return CXL_MBOX_INVALID_INPUT;
>> }
>>
>> - /* For success there should be a match for each requested */
>> - out->num_ports = in->num_ports;
>> + if (in->num_ports > pp->pports.num_ports) {
>> + return CXL_MBOX_INVALID_INPUT;
>> + }
>>
>> + out->num_ports = in->num_ports;
>> for (i = 0; i < in->num_ports; i++) {
>> - struct cxl_fmapi_port_state_info_block *port;
>> - /* First try to match on downstream port */
>> - PCIDevice *port_dev;
>> - uint16_t lnkcap, lnkcap2, lnksta;
>> + int pn = in->ports[i];
>>
>> - port = &out->ports[i];
>> -
>> - port_dev = pcie_find_port_by_pn(bus, in->ports[i]);
>> - if (port_dev) { /* DSP */
>> - PCIDevice *ds_dev = pci_bridge_get_sec_bus(PCI_BRIDGE(port_dev))
>> - ->devices[0];
>> - port->config_state = 3;
>> - if (ds_dev) {
>> - if (object_dynamic_cast(OBJECT(ds_dev), TYPE_CXL_TYPE3)) {
>> - port->connected_device_type = 5; /* Assume MLD for now */
>> - } else {
>> - port->connected_device_type = 1;
>> - }
>> - } else {
>> - port->connected_device_type = 0;
>> - }
>> - port->supported_ld_count = 3;
>> - } else if (usp->port == in->ports[i]) { /* USP */
>> - port_dev = PCI_DEVICE(usp);
>> - port->config_state = 4;
>> - port->connected_device_type = 0;
>> - } else {
>> + if (pp->pports.pport_info[pn].port_id != pn) {
>> return CXL_MBOX_INVALID_INPUT;
>> }
>> -
>> - port->port_id = in->ports[i];
>> - /* Information on status of this port in lnksta, lnkcap */
>> - if (!port_dev->exp.exp_cap) {
>> - return CXL_MBOX_INTERNAL_ERROR;
>> - }
>> - lnksta = port_dev->config_read(port_dev,
>> - port_dev->exp.exp_cap + PCI_EXP_LNKSTA,
>> - sizeof(lnksta));
>> - lnkcap = port_dev->config_read(port_dev,
>> - port_dev->exp.exp_cap + PCI_EXP_LNKCAP,
>> - sizeof(lnkcap));
>> - lnkcap2 = port_dev->config_read(port_dev,
>> - port_dev->exp.exp_cap + PCI_EXP_LNKCAP2,
>> - sizeof(lnkcap2));
>> -
>> - port->max_link_width = (lnkcap & PCI_EXP_LNKCAP_MLW) >> 4;
>> - port->negotiated_link_width = (lnksta & PCI_EXP_LNKSTA_NLW) >> 4;
>> - /* No definition for SLS field in linux/pci_regs.h */
>> - port->supported_link_speeds_vector = (lnkcap2 & 0xFE) >> 1;
>> - port->max_link_speed = lnkcap & PCI_EXP_LNKCAP_SLS;
>> - port->current_link_speed = lnksta & PCI_EXP_LNKSTA_CLS;
>> - /* TODO: Track down if we can get the rest of the info */
>> - port->ltssm_state = 0x7;
>> - port->first_lane_num = 0;
>> - port->link_state = 0;
>> - port->port_cxl_version_bitmask = 0x2;
>> - port->connected_device_cxl_version = 0x2;
>> + memcpy(&out->ports[i], &(pp->pports.pport_info[pn]),
>> + sizeof(CXLPhyPortInfo));
>> }
>> -
>
>That's a stray change that shouldn't be here.
>
Correct me if I am wrong but the current initializations for the ports are
moved to cxl_set_port_type(), hence this might appear stray in this case due
to eliminations but this is with respect to the response payload of
cmd_get_physical_port_state() so seems apt.
However, one change required would be to move: out->num_ports = in->num_ports; after
the below for loop as it validates the port_id's:
for (i = 0; i < in->num_ports; i++) {
int pn = in->ports[i];
if (pp->pports.pport_info[pn].port_id != pn) {
return CXL_MBOX_INVALID_INPUT;
}
memcpy(&out->ports[i], &(pp->pports.pport_info[pn]),
sizeof(CXLPhyPortInfo));
}
>> pl_size = sizeof(*out) + sizeof(*out->ports) * in->num_ports;
>> *len_out = pl_size;
>>
>
>> +static void cxl_set_dsp_port(PCIBus *bus, PCIDevice *dev, void *opaque)
>> +{
>> + CXLCCI *cci = (CXLCCI *)opaque;
>
>That cast isn't needed. You can always implicitly cast from a void *
>to another pointer type (and the other way).
>
okay, thanks!
>> + CXLUpstreamPort *pp = CXL_USP(cci->d);
>> +
>> + if (object_dynamic_cast(OBJECT(dev), TYPE_CXL_DSP)) {
>> + cxl_set_port_type(pp, PCIE_PORT(dev)->port, cci);
>> + }
>> +}
>> +
>> +static CXLRetCode cxl_set_phy_port_info(CXLCCI *cci)
>> +{
>> + PCIEPort *usp = PCIE_PORT(cci->d);
>> + PCIBus *bus = &PCI_BRIDGE(cci->d)->sec_bus;
>> + CXLUpstreamPort *pp = CXL_USP(cci->d);
>> + int num_phys_ports = pcie_count_ds_ports(bus) + 1;
>> + pp->pports.num_ports = num_phys_ports;
>
>Don't mix and match declarations and non declarations.
>
Sure, I'll keep note of it.
>> + uint8_t phy_port_num = usp->port;
>> +
>> + cxl_set_port_type(pp, phy_port_num, cci); /* USP */
>> + pci_for_each_device_under_bus(bus, cxl_set_dsp_port, cci); /* DSP */
>> +
>> + return CXL_MBOX_SUCCESS;
>> +}
>
>> diff --git a/include/hw/cxl/cxl_device.h b/include/hw/cxl/cxl_device.h
>> index 068c20d61e..9fc720ec10 100644
>> --- a/include/hw/cxl/cxl_device.h
>> +++ b/include/hw/cxl/cxl_device.h
>> @@ -129,6 +129,73 @@
>> CXL_NUM_CPMU_INSTANCES * (1 << 16), \
>> (1 << 16))
>>
>> +#define CXL_MAX_PHY_PORTS 256
>> +
>> +/* physical port control info - CXL r3.2 table 7-19 */
>Stick to local style. That makes this
>/* CXL r3.2 Table 7-19: Get Physical Port State Port Information Block Format */
>
>
thanks for pointing out!
>> +#define CXL_PORT_CONFIG_STATE_DISABLED 0x0
>> +#define CXL_PORT_CONFIG_STATE_BIND_IN_PROGRESS 0x1
>> +#define CXL_PORT_CONFIG_STATE_UNBIND_IN_PROGRESS 0x2
>> +#define CXL_PORT_CONFIG_STATE_DSP 0x3
>> +#define CXL_PORT_CONFIG_STATE_USP 0x4
>> +#define CXL_PORT_CONFIG_STATE_FABRIC_PORT 0x5
>> +#define CXL_PORT_CONFIG_STATE_INVALID_PORT_ID 0xF
>> +
>> +#define NOT_CXL_OR_DISCONNECTED 0x00
>These need a clean prefix like the you have for CONFIG_STATE
>Some of these names are tricky to put concisely.
>
>#define CXL_PORT_CONNECTED_DEV_MODE_NOT_CXL_OR_DISCONN 0x0
>#define CXL_PORT_CONNECTED_DEV_MODE_RCD 0x1
>#define CXL_PORT_CONNECTED_DEV_MODE_68B_VH 0x2
>#define CXL_PORT_CONNECTED_DEV_MODE_256B 0x3
>#define CXL_PORT_CONNECTED_DEV_MODE_LO_256B 0x4
>#define CXL_PORT_CONNECTED_DEV_MODE_PBR 0x5
>
>Is about the shortest that covers what these are.
>Kind of need to retain these are about the device down the
>other end of the wires. Only one bibblee so can use just 1 character.
>
okay
>> +#define RCD_MODE 0x01
>> +#define CXL_68B_FLIT_AND_VH_MODE 0x02
>> +#define STANDARD_256B_FLIT_MODE 0x03
>> +#define CXL_LATENCY_OPTIMIZED_256B_FLIT_MODE 0x04
>> +#define PBR_MODE 0x05
>> +
>> +#define NO_DEVICE_DETECTED 0
>Similar here - also keep to hex to match spec and 2 digits as this one is byte.
>#define CXL_PORT_CONNECTED_DEV_TYPE_NONE 0x00
>#define CXL_PORT_CONNECTED_DEV_TYPE_PCIE 0x01
>etc
>
got it
>> +#define PCIE_DEVICE 1
>> +#define CXL_TYPE_1_DEVICE 2
>> +#define CXL_TYPE_2_DEVICE_OR_HBR_SWITCH 3
>> +#define CXL_TYPE_3_SLD 4
>> +#define CXL_TYPE_3_MLD 5
>> +#define PBR_COMPONENT 6
>> +
>> +#define CXL_RCD_MODE 0x00
>> +#define CXL_68B_FLIT_AND_VH_CAPABLE 0x01
>> +#define CXL_256B_FLIT_CAPABLE 0x02
>> +#define CXL_LATENCY_OPTIMIZED_256B_FLIT 0x03
>> +#define CXL_PBR_CAPABLE 0x04
>This lot are confusing as they are a bits in a bitmap. I'd express them as
>#define CXL_PORT_SUPPORTS_RCD BIT(0)
>#define CXL_PORT_SUPPORTS_68B_VH BIT(1)
>etc, matching names with above suggestion on short forms.
>
okay
>> +
>> +#define CXL_LTSSM_DETECT 0x00
>I'd call out the PORT bit for these as they aren't generic
>and that this is the current state.
>
>CXL_PORT_LTSSM_STATE_DETECT etc
>
>
okay
>> +#define CXL_LTSSM_POLLING 0x01
>> +#define CXL_LTSSM_CONFIGURATION 0x02
>> +#define CXL_LTSSM_RECOVERY 0x03
>> +#define CXL_LTSSM_L0 0x04
>> +#define CXL_LTSSM_L0S 0x05
>> +#define CXL_LTSSM_L1 0x06
>> +#define CXL_LTSSM_L2 0x07
>> +#define CXL_LTSSM_DISABLED 0x08
>> +#define CXL_LTSSM_LOOPBACK 0x09
>> +#define CXL_LTSSM_HOT_RESET 0x0A
>> +
>> +#define LINK_STATE_FLAG_LANE_REVERSED BIT(0)
>
>Probably stick CXL_PORT_ as prefix for these as well.
>
okay, will update these namespace defines in v4
>> +#define LINK_STATE_FLAG_PERST_ASSERTED BIT(1)
>> +#define LINK_STATE_FLAG_PRSNT BIT(2)
>> +#define LINK_STATE_FLAG_POWER_OFF BIT(3)
>> +
>> +typedef struct CXLPhyPortInfo {
>> + uint8_t port_id;
>> + uint8_t current_port_config_state;
>> + uint8_t connected_device_mode;
>> + uint8_t rsv1;
>> + uint8_t connected_device_type;
>> + uint8_t supported_cxl_modes;
>> + uint8_t max_link_width;
>> + uint8_t negotiated_link_width;
>> + uint8_t supported_link_speeds_vector;
>> + uint8_t max_link_speed;
>> + uint8_t current_link_speed;
>> + uint8_t ltssm_state;
>> + uint8_t first_negotiated_lane_num;
>> + uint16_t link_state_flags;
>> + uint8_t supported_ld_count;
>> +} QEMU_PACKED CXLPhyPortInfo;
>> +
>> /* CXL r3.1 Table 8-34: Command Return Codes */
>> typedef enum {
>> CXL_MBOX_SUCCESS = 0x0,
>> diff --git a/include/hw/pci-bridge/cxl_upstream_port.h b/include/hw/pci-bridge/cxl_upstream_port.h
>> index db1dfb6afd..3b7e72bfe0 100644
>> --- a/include/hw/pci-bridge/cxl_upstream_port.h
>> +++ b/include/hw/pci-bridge/cxl_upstream_port.h
>> @@ -4,6 +4,7 @@
>> #include "hw/pci/pcie.h"
>> #include "hw/pci/pcie_port.h"
>> #include "hw/cxl/cxl.h"
>> +#include "include/hw/cxl/cxl_device.h"
>>
>> typedef struct CXLUpstreamPort {
>> /*< private >*/
>> @@ -23,6 +24,13 @@ typedef struct CXLUpstreamPort {
>>
>> DOECap doe_cdat;
>> uint64_t sn;
>> +
>> + /*< physical ports information >*/
>
>I believe the magic /*< private >*/ syntax with the <> is special for public/private.
>There are a few other comments marked like that in qemu, but very very few.
>So this probably just wants to be
>
> /* Physical ports information */
>
got it, thanks for the info!
>> + struct {
>> + uint8_t num_ports;
>> + uint8_t active_port_bitmask[CXL_MAX_PHY_PORTS / BITS_PER_BYTE];
>> + CXLPhyPortInfo pport_info[CXL_MAX_PHY_PORTS];
>> + } pports;
>> } CXLUpstreamPort;
>>
>> #endif /* CXL_SUP_H */
>
[-- Attachment #2: Type: text/plain, Size: 0 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 0/2] FM-API Physical Switch Command Set Support
2025-09-08 13:22 ` Arpit Kumar
@ 2025-09-09 15:03 ` Jonathan Cameron via
0 siblings, 0 replies; 11+ messages in thread
From: Jonathan Cameron via @ 2025-09-09 15:03 UTC (permalink / raw)
To: Arpit Kumar
Cc: qemu-devel, gost.dev, linux-cxl, dave, vishak.g, krish.reddy,
a.manzanares, alok.rathore, cpgs
On Mon, 8 Sep 2025 18:52:11 +0530
Arpit Kumar <arpit1.kumar@samsung.com> wrote:
> On 05/09/25 05:12PM, Jonathan Cameron wrote:
> >On Thu, 4 Sep 2025 18:49:02 +0530
> >Arpit Kumar <arpit1.kumar@samsung.com> wrote:
> >
> >> This patch series refactor existing support for Identify Switch Device
> >> and Get Physical Port State by utilizing physical ports (USP & DSP)
> >> information stored during enumeration.
> >>
> >> Additionally, it introduces new support for Physical Port Control
> >> of FM-API based physical switch command set as per CXL spec r3.2
> >> Table 8-230:Physical Switch. It primarily constitutes two logic:
> >> -Assert-Deassert PERST: Assert PERST involves physical port to be in
> >> hold reset phase for minimum 100ms. No other physical port control
> >> request are entertained until Deassert PERST command for the given
> >> port is issued.
> >> -Reset PPB: cold reset of physical port (completing enter->hold->exit phases).
> >>
> >> Tested using libcxl-mi interface[1]:
> >> All active ports and all opcodes per active port is tested. Also, tested
> >> against possible edge cases manually since the interface currently dosen't
> >> support run time input.
> >>
> >> Example topology (1 USP + 3 DSP's->switch with 2 CXLType3 devices connected
> >> to 2 DSP's):
> >> FM="-object memory-backend-file,id=cxl-mem1,mem-path=$TMP_DIR/t3_cxl1.raw,size=256M \
> >> -object memory-backend-file,id=cxl-lsa1,mem-path=$TMP_DIR/t3_lsa1.raw,size=1M \
> >> -object memory-backend-file,id=cxl-mem2,mem-path=$TMP_DIR/t3_cxl2.raw,size=512M \
> >> -object memory-backend-file,id=cxl-lsa2,mem-path=$TMP_DIR/t3_lsa2.raw,size=512M \
> >> -device pxb-cxl,bus_nr=12,bus=pcie.0,id=cxl.1,hdm_for_passthrough=true \
> >> -device cxl-rp,port=0,bus=cxl.1,id=cxl_rp_port0,chassis=0,slot=2 \
> >> -device cxl-upstream,port=2,sn=1234,bus=cxl_rp_port0,id=us0,addr=0.0,multifunction=on, \
> >> -device cxl-switch-mailbox-cci,bus=cxl_rp_port0,addr=0.1,target=us0 \
> >> -device cxl-downstream,port=0,bus=us0,id=swport0,chassis=0,slot=4 \
> >> -device cxl-downstream,port=1,bus=us0,id=swport1,chassis=0,slot=5 \
> >> -device cxl-downstream,port=3,bus=us0,id=swport2,chassis=0,slot=6 \
> >> -device cxl-type3,bus=swport0,memdev=cxl-mem1,id=cxl-pmem1,lsa=cxl-lsa1,sn=3 \
> >> -device cxl-type3,bus=swport2,memdev=cxl-mem2,id=cxl-pmem2,lsa=cxl-lsa2,sn=4 \
> >> -machine cxl-fmw.0.targets.0=cxl.1,cxl-fmw.0.size=4G,cxl-fmw.0.interleave-granularity=1k \
> >> -device i2c_mctp_cxl,bus=aspeed.i2c.bus.0,address=4,target=us0 \
> >> -device i2c_mctp_cxl,bus=aspeed.i2c.bus.0,address=5,target=cxl-pmem1 \
> >> -device i2c_mctp_cxl,bus=aspeed.i2c.bus.0,address=6,target=cxl-pmem2 \
> >> -device virtio-rng-pci,bus=swport1"
> >>
> >> Multiple Qemu Topologies tested:
> >> -without any devices connected to downstream ports.
> >> -with virtio-rng-pci devices connected to downstream ports.
> >> -with CXLType3 devices connected to downstream ports.
> >> -with different unique values of ports (both upstream and downstream).
> >>
> >> Changes from v2->v3:
> >> -cxl_set_port_type(): optimized storing of strucutre members.
> >> -namespace defines instead of enum.
> >> -Calculating size for active_port_bitmask than hardcoding to 0x20.
> >> -Defined struct phy_port directly inside struct CXLUpstreamPort as pports.
> >> -Renamed struct pperst to struct CXLPhyPortPerst.
> >> -Optimized perst member initializations for ports inside
> >> cxl_initialize_usp_mctpcci() using active_port_bitmask.
> >>
> >> [1] https://github.com/computexpresslink/libcxlmi/commit/35fe68bd9a31469f832a87694d7b18d2d50be5b8
> >>
> >> The patches are generated against the Johnathan's tree
> >> https://gitlab.com/jic23/qemu.git and branch cxl-2025-07-03.
> >>
> >> Signed-off-by: Arpit Kumar <arpit1.kumar@samsung.com>
> >
> >Hi Arpit,
> >
> >I'll have a go (probably next week) at rebasing this rather earlier in my tree as I'd
> >like to get this upstream without it having a dependency on the MCTP support.
> >
> >That means bring it up with the switch-cci / pcie mailbox CCI and squashing
> >the MCTP bit into the patch that brings that support up later in my tree.
> >
> >I do plan to fix up the remaining 'feature' gap on the FMAPI/MCTP/USB
> >emulation which is that it's ignoring the MTU to the host and so not
> >breaking messages up as it should. Linux doesn't care but maybe some other
> >OS will. Not entirely sure when I'll get to that though and I'd like to
> >move your work forward before that.
> >
> >Jonathan
> >
> >
> Hi Jonathan,
> Thanks for the review comments!
>
> As per my understanding from CXL spec r3.2 Table 8-215: Physical Port Control
I guess table 8-230?
> request is allowed only for switch FM interface and is prohibited for
> switch-cci/pcie mailbox CCI. However, if possible, should I be using
> cxl_initialize_mailbox_swcci() to initialize my perst members?
I always find that table really hard to understand. I'm not sure
what intent is wrt to "host mailbox" in that table. I can't immediately think
why the FM API CCI (thing that was switch cci in earlier specs) wouldn't support
this particular command. You are supposed to be able to control anything via
that which is controllable for MCTP. Only exceptions should be the few
things to do with the MCTP interface itself.
Just to add to the confusion, the entry for Get Physical Port State in the
host mailbox column is "MSW = mandatory on MCTP-based CCIs for all switches that support
the FM API MCTP message type" which is odd given that mailbox isn't mctp based.
Gut feeling is that it can be implemented on the 'switch CCI'. I'd like to do
that for now just because the MCTP stuff is going to take longer to upstream
and I'd rather your series was not queued up behind it.
Jonathan
>
> Thanks,
> Arpit.
> >>
> >> Arpit Kumar (2):
> >> hw/cxl: Refactored Identify Switch Device & Get Physical Port State
> >> hw/cxl: Add Physical Port Control (Opcode 5102h)
> >>
> >> hw/cxl/cxl-mailbox-utils.c | 368 +++++++++++++++-------
> >> include/hw/cxl/cxl_device.h | 76 +++++
> >> include/hw/cxl/cxl_mailbox.h | 1 +
> >> include/hw/pci-bridge/cxl_upstream_port.h | 9 +
> >> 4 files changed, 347 insertions(+), 107 deletions(-)
> >>
> >
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 1/2] hw/cxl: Refactored Identify Switch Device & Get Physical Port State
2025-09-08 13:48 ` Arpit Kumar
@ 2025-09-09 15:07 ` Jonathan Cameron via
0 siblings, 0 replies; 11+ messages in thread
From: Jonathan Cameron via @ 2025-09-09 15:07 UTC (permalink / raw)
To: Arpit Kumar
Cc: qemu-devel, gost.dev, linux-cxl, dave, vishak.g, krish.reddy,
a.manzanares, alok.rathore, cpgs
On Mon, 8 Sep 2025 19:18:56 +0530
Arpit Kumar <arpit1.kumar@samsung.com> wrote:
> On 05/09/25 04:59PM, Jonathan Cameron wrote:
> >On Thu, 4 Sep 2025 18:49:03 +0530
> >Arpit Kumar <arpit1.kumar@samsung.com> wrote:
> >
> >> -Storing physical ports info during enumeration.
> >> -Refactored changes using physical ports info for
> >> Identify Switch Device (Opcode 5100h) & Get Physical Port State
> >> (Opcode 5101h) physical switch FM-API command set.
> >>
> >> Signed-off-by: Arpit Kumar <arpit1.kumar@samsung.com>
> >Hi Arpit,
> >
> >A few minor things inline. Biggest one is making sure
> >to namespace the defines which is quite challenging to do
> >here without a really long name.
> >
> >Jonathan
> >
> Hi Jonathan,
> Thanks for the review! Will update the changes in next
> iteration v4 of the patch-set.
> >> - port->connected_device_cxl_version = 0x2;
> >> + memcpy(&out->ports[i], &(pp->pports.pport_info[pn]),
> >> + sizeof(CXLPhyPortInfo));
> >> }
> >> -
> >
> >That's a stray change that shouldn't be here.
I wasn't clear enough. I just meant that final line removal, not the rest
of the code! Sorry for confusion.
> >
> Correct me if I am wrong but the current initializations for the ports are
> moved to cxl_set_port_type(), hence this might appear stray in this case due
> to eliminations but this is with respect to the response payload of
> cmd_get_physical_port_state() so seems apt.
> However, one change required would be to move: out->num_ports = in->num_ports; after
> the below for loop as it validates the port_id's:
> for (i = 0; i < in->num_ports; i++) {
> int pn = in->ports[i];
>
> if (pp->pports.pport_info[pn].port_id != pn) {
> return CXL_MBOX_INVALID_INPUT;
> }
> memcpy(&out->ports[i], &(pp->pports.pport_info[pn]),
> sizeof(CXLPhyPortInfo));
> }
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2025-09-09 15:09 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <CGME20250904131926epcas5p2a363cf0604a4801038d32e7da5397da1@epcas5p2.samsung.com>
2025-09-04 13:19 ` [PATCH v3 0/2] FM-API Physical Switch Command Set Support Arpit Kumar
[not found] ` <CGME20250904131933epcas5p2ab29fa060d8a7df32a222aad740fedc6@epcas5p2.samsung.com>
2025-09-04 13:19 ` [PATCH v3 1/2] hw/cxl: Refactored Identify Switch Device & Get Physical Port State Arpit Kumar
2025-09-05 15:59 ` Jonathan Cameron via
2025-09-08 13:48 ` Arpit Kumar
2025-09-09 15:07 ` Jonathan Cameron via
[not found] ` <CGME20250904131944epcas5p351c0e073a975b1347c4a61aa0dd511f3@epcas5p3.samsung.com>
2025-09-04 13:19 ` [PATCH v3 2/2] hw/cxl: Add Physical Port Control (Opcode 5102h) Arpit Kumar
2025-09-05 8:48 ` ALOK TIWARI
2025-09-08 13:03 ` Arpit Kumar
2025-09-05 16:12 ` [PATCH v3 0/2] FM-API Physical Switch Command Set Support Jonathan Cameron via
2025-09-08 13:22 ` Arpit Kumar
2025-09-09 15:03 ` Jonathan Cameron via
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).