Linux CXL
 help / color / mirror / Atom feed
* [PATCH v4 0/2] FM-API Physical Switch Command Set Support
       [not found] <CGME20250916080757epcas5p2db403c9648f7818b22413d90acb4fdd4@epcas5p2.samsung.com>
@ 2025-09-16  8:07 ` Arpit Kumar
  2025-09-16  8:07   ` [PATCH v4 1/2] hw/cxl: Refactored Identify Switch Device & Get Physical Port State Arpit Kumar
                     ` (2 more replies)
  0 siblings, 3 replies; 13+ messages in thread
From: Arpit Kumar @ 2025-09-16  8:07 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 Physical Switch Command Set as per CXL spec r3.2 Section 7.6.7.1.3.
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.

Typical Qemu topology
(1 USP + 3 DSP's in a switch with 2 CXLType3 devices connected to the 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"

Tested multiple Qemu topologies:
        -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 v3 (https://lore.kernel.org/qemu-devel/20250909160316.00000190@huawei.com/T/):
        -Namespaced the defines with cleaner prefix for Get Physical Port State 
         Port Information Block members.
        -switch CCI implementation instead of switch FM interface as per
         Jonathan's review comments, hence moved perst members initializations
         from: cxl_initialize_usp_mctpcci() -> cxl_initialize_mailbox_swcci().

[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, 348 insertions(+), 106 deletions(-)

-- 
2.34.1


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v4 1/2] hw/cxl: Refactored Identify Switch Device & Get Physical Port State
  2025-09-16  8:07 ` [PATCH v4 0/2] FM-API Physical Switch Command Set Support Arpit Kumar
@ 2025-09-16  8:07   ` Arpit Kumar
  2025-09-17 15:55     ` Jonathan Cameron
  2025-09-16  8:07   ` [PATCH v4 2/2] hw/cxl: Add Physical Port Control (Opcode 5102h) Arpit Kumar
  2026-01-27 15:23   ` [PATCH v4 0/2] FM-API Physical Switch Command Set Support Jonathan Cameron
  2 siblings, 1 reply; 13+ messages in thread
From: Arpit Kumar @ 2025-09-16  8:07 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                | 229 ++++++++++++----------
 include/hw/cxl/cxl_device.h               |  67 +++++++
 include/hw/pci-bridge/cxl_upstream_port.h |   8 +
 3 files changed, 198 insertions(+), 106 deletions(-)

diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c
index c5177dfd92..2a104dd337 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,21 @@ 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;
+    }
 
     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;
-
-        port = &out->ports[i];
+        int pn = in->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));
     }
 
+    out->num_ports = in->num_ports;
     pl_size = sizeof(*out) + sizeof(*out->ports) * in->num_ports;
     *len_out = pl_size;
 
@@ -4684,6 +4601,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 = CXL_PORT_CONNECTED_DEV_TYPE_NONE;
+        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_PORT_CONNECTED_DEV_TYPE_3_SLD;
+                } else {
+                    connected_device_type = CXL_PORT_CONNECTED_DEV_TYPE_PCIE;
+                }
+            } else {
+                connected_device_type = CXL_PORT_CONNECTED_DEV_TYPE_NONE;
+            }
+            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 = CXL_PORT_CONNECTED_DEV_MODE_256B,
+        .connected_device_type = connected_device_type,
+        .supported_cxl_modes = CXL_PORT_SUPPORTS_256B,
+        .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_PORT_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 = 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);
+    uint8_t num_phys_ports = pcie_count_ds_ports(bus) + 1;
+    uint8_t phy_port_num = usp->port;
+
+    pp->pports.num_ports = num_phys_ports;
+    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 +4706,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 +4793,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..536d465f42 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
+
+/* 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 CXL_PORT_CONNECTED_DEV_MODE_NOT_CXL_OR_DISCONN 0x00
+#define CXL_PORT_CONNECTED_DEV_MODE_RCD                0x01
+#define CXL_PORT_CONNECTED_DEV_MODE_68B_VH             0x02
+#define CXL_PORT_CONNECTED_DEV_MODE_256B               0x03
+#define CXL_PORT_CONNECTED_DEV_MODE_LO_256B            0x04
+#define CXL_PORT_CONNECTED_DEV_MODE_PBR                0x05
+
+#define CXL_PORT_CONNECTED_DEV_TYPE_NONE            0x00
+#define CXL_PORT_CONNECTED_DEV_TYPE_PCIE            0x01
+#define CXL_PORT_CONNECTED_DEV_TYPE_1               0x02
+#define CXL_PORT_CONNECTED_DEV_TYPE_2_OR_HBR_SWITCH 0x03
+#define CXL_PORT_CONNECTED_DEV_TYPE_3_SLD           0x04
+#define CXL_PORT_CONNECTED_DEV_TYPE_3_MLD           0x05
+#define CXL_PORT_CONNECTED_DEV_PBR_COMPONENT        0x06
+
+#define CXL_PORT_SUPPORTS_RCD        BIT(0)
+#define CXL_PORT_SUPPORTS_68B_VH     BIT(1)
+#define CXL_PORT_SUPPORTS_256B       BIT(2)
+#define CXL_PORT_SUPPORTS_LO_256B    BIT(3)
+#define CXL_PORT_SUPPORTS_PBR        BIT(4)
+
+#define CXL_PORT_LTSSM_DETECT        0x00
+#define CXL_PORT_LTSSM_POLLING       0x01
+#define CXL_PORT_LTSSM_CONFIGURATION 0x02
+#define CXL_PORT_LTSSM_RECOVERY      0x03
+#define CXL_PORT_LTSSM_L0            0x04
+#define CXL_PORT_LTSSM_L0S           0x05
+#define CXL_PORT_LTSSM_L1            0x06
+#define CXL_PORT_LTSSM_L2            0x07
+#define CXL_PORT_LTSSM_DISABLED      0x08
+#define CXL_PORT_LTSSM_LOOPBACK      0x09
+#define CXL_PORT_LTSSM_HOT_RESET     0x0A
+
+#define CXL_PORT_LINK_STATE_FLAG_LANE_REVERSED    BIT(0)
+#define CXL_PORT_LINK_STATE_FLAG_PERST_ASSERTED   BIT(1)
+#define CXL_PORT_LINK_STATE_FLAG_PRSNT            BIT(2)
+#define CXL_PORT_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..c6218100a2 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] 13+ messages in thread

* [PATCH v4 2/2] hw/cxl: Add Physical Port Control (Opcode 5102h)
  2025-09-16  8:07 ` [PATCH v4 0/2] FM-API Physical Switch Command Set Support Arpit Kumar
  2025-09-16  8:07   ` [PATCH v4 1/2] hw/cxl: Refactored Identify Switch Device & Get Physical Port State Arpit Kumar
@ 2025-09-16  8:07   ` Arpit Kumar
  2025-09-17 16:05     ` Jonathan Cameron
  2025-09-17 16:29     ` Jonathan Cameron
  2026-01-27 15:23   ` [PATCH v4 0/2] FM-API Physical Switch Command Set Support Jonathan Cameron
  2 siblings, 2 replies; 13+ messages in thread
From: Arpit Kumar @ 2025-09-16  8:07 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                | 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.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH v4 1/2] hw/cxl: Refactored Identify Switch Device & Get Physical Port State
  2025-09-16  8:07   ` [PATCH v4 1/2] hw/cxl: Refactored Identify Switch Device & Get Physical Port State Arpit Kumar
@ 2025-09-17 15:55     ` Jonathan Cameron
  2025-09-19 11:03       ` Arpit Kumar
  0 siblings, 1 reply; 13+ messages in thread
From: Jonathan Cameron @ 2025-09-17 15:55 UTC (permalink / raw)
  To: Arpit Kumar
  Cc: qemu-devel, gost.dev, linux-cxl, dave, vishak.g, krish.reddy,
	a.manzanares, alok.rathore, cpgs

On Tue, 16 Sep 2025 13:37:35 +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.  One question inline, and one comment on code I've moved
around whilst queue this up.  I'll push out a tree to gitlab
(probably tomorrow) and when I do please check I didn't mess that up!

Jonathan


> +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 = CXL_PORT_CONNECTED_DEV_TYPE_NONE;
> +        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 */

In what sense controllable?  It should always match what the downstream device
is presenting as.  Do you ultimately mean if we mess with the alternate modes
and reset the port to have it come up as a PCI only device?
This will need to be more complex as we add different CXL type 3 device support
of course, but I'd still expect to auto detect it rather that control it directly.

> +                    connected_device_type = CXL_PORT_CONNECTED_DEV_TYPE_3_SLD;
> +                } else {
> +                    connected_device_type = CXL_PORT_CONNECTED_DEV_TYPE_PCIE;
> +                }
> +            } else {
> +                connected_device_type = CXL_PORT_CONNECTED_DEV_TYPE_NONE;
> +            }
> +            supported_ld_count = 3;
> +        } else {
> +            return CXL_MBOX_INVALID_INPUT;
> +        }
> +    }

>  void cxl_initialize_mailbox_swcci(CXLCCI *cci, DeviceState *intf,
>                                    DeviceState *d, size_t payload_max)
>  {
> @@ -4691,6 +4706,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 +4793,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);

I'll shift this to a later patch whilst picking this up for my staging tree.
I want this ahead of where we introduce cxl_initialize_usp_mctpcci.
 
>  }

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v4 2/2] hw/cxl: Add Physical Port Control (Opcode 5102h)
  2025-09-16  8:07   ` [PATCH v4 2/2] hw/cxl: Add Physical Port Control (Opcode 5102h) Arpit Kumar
@ 2025-09-17 16:05     ` Jonathan Cameron
  2025-09-19 11:16       ` Arpit Kumar
  2025-09-17 16:29     ` Jonathan Cameron
  1 sibling, 1 reply; 13+ messages in thread
From: Jonathan Cameron @ 2025-09-17 16:05 UTC (permalink / raw)
  To: Arpit Kumar
  Cc: qemu-devel, gost.dev, linux-cxl, dave, vishak.g, krish.reddy,
	a.manzanares, alok.rathore, cpgs

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)

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH v4 2/2] hw/cxl: Add Physical Port Control (Opcode 5102h)
  2025-09-16  8:07   ` [PATCH v4 2/2] hw/cxl: Add Physical Port Control (Opcode 5102h) Arpit Kumar
  2025-09-17 16:05     ` Jonathan Cameron
@ 2025-09-17 16:29     ` Jonathan Cameron
  2025-09-19 11:21       ` Arpit Kumar
  1 sibling, 1 reply; 13+ messages in thread
From: Jonathan Cameron @ 2025-09-17 16:29 UTC (permalink / raw)
  To: Arpit Kumar
  Cc: qemu-devel, gost.dev, linux-cxl, dave, vishak.g, krish.reddy,
	a.manzanares, alok.rathore, cpgs

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

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

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

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

Could we move this to a call from cxl_usp_realize?

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

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

 
Thanks,
Jonathan

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v4 1/2] hw/cxl: Refactored Identify Switch Device & Get Physical Port State
  2025-09-17 15:55     ` Jonathan Cameron
@ 2025-09-19 11:03       ` Arpit Kumar
  2025-09-30 14:26         ` Jonathan Cameron
  0 siblings, 1 reply; 13+ messages in thread
From: Arpit Kumar @ 2025-09-19 11:03 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: qemu-devel, gost.dev, linux-cxl, dave, vishak.g, krish.reddy,
	a.manzanares, alok.rathore, arpit.sysdev, cpgs

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

On 17/09/25 04:55PM, Jonathan Cameron wrote:
>On Tue, 16 Sep 2025 13:37:35 +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.  One question inline, and one comment on code I've moved
>around whilst queue this up.  I'll push out a tree to gitlab
>(probably tomorrow) and when I do please check I didn't mess that up!
>
>Jonathan
>

Hi Jonathan,
Thank you for the swift response and review comments.
Sure, will look into gitlab tree once up.

Thanks,
Arpit
>
>> +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 = CXL_PORT_CONNECTED_DEV_TYPE_NONE;
>> +        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 */
>
>In what sense controllable?  It should always match what the downstream device
>is presenting as.  Do you ultimately mean if we mess with the alternate modes
>and reset the port to have it come up as a PCI only device?
>This will need to be more complex as we add different CXL type 3 device support
>of course, but I'd still expect to auto detect it rather that control it directly.
>

This is with respect to your review comment from v1 patch:
https://lore.kernel.org/qemu-devel/20250602135942.2773823-1-arpit1.kumar@samsung.com/T/
As per my understanding, controllable was identification of the specific type of 
CXL type 3 device and accordingly initializing connected_device_type. Since you 
mention auto-detect, does it mean using object_get_typename() to identify the type
of device and initiliaze it directly to connected_device_type rather than specifying
it explicitly. If yes, then this anyways rules out controllable part, hence the comment
can be removed.

>> +                    connected_device_type = CXL_PORT_CONNECTED_DEV_TYPE_3_SLD;
>> +                } else {
>> +                    connected_device_type = CXL_PORT_CONNECTED_DEV_TYPE_PCIE;
>> +                }
>> +            } else {
>> +                connected_device_type = CXL_PORT_CONNECTED_DEV_TYPE_NONE;
>> +            }
>> +            supported_ld_count = 3;
>> +        } else {
>> +            return CXL_MBOX_INVALID_INPUT;
>> +        }
>> +    }
>
>>  void cxl_initialize_mailbox_swcci(CXLCCI *cci, DeviceState *intf,
>>                                    DeviceState *d, size_t payload_max)
>>  {
>> @@ -4691,6 +4706,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 +4793,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);
>
>I'll shift this to a later patch whilst picking this up for my staging tree.
>I want this ahead of where we introduce cxl_initialize_usp_mctpcci.
>
Okay
>>  }

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



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v4 2/2] hw/cxl: Add Physical Port Control (Opcode 5102h)
  2025-09-17 16:05     ` Jonathan Cameron
@ 2025-09-19 11:16       ` Arpit Kumar
  0 siblings, 0 replies; 13+ messages in thread
From: Arpit Kumar @ 2025-09-19 11:16 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: qemu-devel, gost.dev, linux-cxl, dave, vishak.g, krish.reddy,
	a.manzanares, alok.rathore, arpit.sysdev, cpgs

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

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)

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



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v4 2/2] hw/cxl: Add Physical Port Control (Opcode 5102h)
  2025-09-17 16:29     ` Jonathan Cameron
@ 2025-09-19 11:21       ` Arpit Kumar
  0 siblings, 0 replies; 13+ messages in thread
From: Arpit Kumar @ 2025-09-19 11:21 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: qemu-devel, gost.dev, linux-cxl, dave, vishak.g, krish.reddy,
	a.manzanares, alok.rathore, arpit.sysdev, cpgs

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

On 17/09/25 05:29PM, Jonathan Cameron wrote:
>On Tue, 16 Sep 2025 13:37:36 +0530
>Arpit Kumar <arpit1.kumar@samsung.com> wrote:
>
>> -added assert-deassert PERST implementation
>>  for physical ports (both USP and DSP's).
>> -assert PERST involves bg operation for holding 100ms.
>> -reset PPB implementation for physical ports.
>>
>> Signed-off-by: Arpit Kumar <arpit1.kumar@samsung.com>
>
>> @@ -4702,11 +4818,34 @@ static CXLRetCode cxl_set_phy_port_info(CXLCCI *cci)
>>  void cxl_initialize_mailbox_swcci(CXLCCI *cci, DeviceState *intf,
>>                                    DeviceState *d, size_t payload_max)
>>  {
>> +    CXLUpstreamPort *pp;
>> +    uint8_t pn = 0;
>> +
>>      cxl_copy_cci_commands(cci, cxl_cmd_set_sw);
>>      cci->d = d;
>>      cci->intf = intf;
>>      cxl_init_cci(cci, payload_max);
>>      cxl_set_phy_port_info(cci);
>> +    /* physical port control */
>> +    pp = CXL_USP(cci->d);
>This bit feels like it is wrongly located.  I ran into this whilst
>trying to add back the mctp variant as part of shuffling my cxl staging tree.
>
>Whilst this only gets used for the CCI commands, it is a USP thing not
>a mailbox thing as we only want this called once per USP, not once per CCI on the
>USP.
>
>Could we move this to a call from cxl_usp_realize?
>
>If something like that works would you mind sending me a patch on top of this
>series to do so? I'm not yet set up to test this series so better you do it.
>
>We don't need that upstream until the first MCTP support on USP so this doesn't
>block us on that front.
>
>
>Thanks,
>Jonathan
>
Hi Jonathan,
Sure, will look into it as this seems tricky and then send a patch on top of this series.
Also, it would be helpful if you have any suggestions for going about this change.

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

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



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v4 1/2] hw/cxl: Refactored Identify Switch Device & Get Physical Port State
  2025-09-19 11:03       ` Arpit Kumar
@ 2025-09-30 14:26         ` Jonathan Cameron
  0 siblings, 0 replies; 13+ messages in thread
From: Jonathan Cameron @ 2025-09-30 14:26 UTC (permalink / raw)
  To: Arpit Kumar
  Cc: qemu-devel, gost.dev, linux-cxl, dave, vishak.g, krish.reddy,
	a.manzanares, alok.rathore, arpit.sysdev, cpgs

On Fri, 19 Sep 2025 16:33:12 +0530
Arpit Kumar <arpit1.kumar@samsung.com> wrote:

> On 17/09/25 04:55PM, Jonathan Cameron wrote:
> >On Tue, 16 Sep 2025 13:37:35 +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.  One question inline, and one comment on code I've moved
> >around whilst queue this up.  I'll push out a tree to gitlab
> >(probably tomorrow) and when I do please check I didn't mess that up!
> >
> >Jonathan
> >  
> 
> Hi Jonathan,
> Thank you for the swift response and review comments.
> Sure, will look into gitlab tree once up.

Oops. Got rather delayed on that - getting back to tree mangling this week.

> 
> Thanks,
> Arpit
> >  
> >> +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 = CXL_PORT_CONNECTED_DEV_TYPE_NONE;
> >> +        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 */  
> >
> >In what sense controllable?  It should always match what the downstream device
> >is presenting as.  Do you ultimately mean if we mess with the alternate modes
> >and reset the port to have it come up as a PCI only device?
> >This will need to be more complex as we add different CXL type 3 device support
> >of course, but I'd still expect to auto detect it rather that control it directly.
> >  
> 
> This is with respect to your review comment from v1 patch:
> https://lore.kernel.org/qemu-devel/20250602135942.2773823-1-arpit1.kumar@samsung.com/T/
> As per my understanding, controllable was identification of the specific type of 
> CXL type 3 device and accordingly initializing connected_device_type. Since you 
> mention auto-detect, does it mean using object_get_typename() to identify the type
> of device and initiliaze it directly to connected_device_type rather than specifying
> it explicitly. If yes, then this anyways rules out controllable part, hence the comment
> can be removed.

Hmm. I thought I'd replied here, but seems not!

I meant indeed that we can query the downstream device to find out what it is and
assume that we have trained up to match that.

So agreed just dropping the comment is the easy way forward.

J


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v4 0/2] FM-API Physical Switch Command Set Support
  2025-09-16  8:07 ` [PATCH v4 0/2] FM-API Physical Switch Command Set Support Arpit Kumar
  2025-09-16  8:07   ` [PATCH v4 1/2] hw/cxl: Refactored Identify Switch Device & Get Physical Port State Arpit Kumar
  2025-09-16  8:07   ` [PATCH v4 2/2] hw/cxl: Add Physical Port Control (Opcode 5102h) Arpit Kumar
@ 2026-01-27 15:23   ` Jonathan Cameron
  2026-01-30 12:03     ` Arpit Kumar
  2 siblings, 1 reply; 13+ messages in thread
From: Jonathan Cameron @ 2026-01-27 15:23 UTC (permalink / raw)
  To: Arpit Kumar
  Cc: qemu-devel, gost.dev, linux-cxl, dave, vishak.g, krish.reddy,
	a.manzanares, alok.rathore, cpgs

On Tue, 16 Sep 2025 13:37:34 +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.

Hi Arpit.

So I came back to this series to try and tweak it into being in a state
for upstreaming.  I made some fairly heavy changes (to push the
data down to the individual ports where to me it more logically sits)
but it became apparent to me during this that there is a fundamental
problem.  This stuff isn't static because it needs to reflect port
retraining and hotplug events etc.  If we want to store cached
data we are going to have to deal with updating it from a significant
number of different places. The perst one you have covered in patch 2
is an example but there are others.

Now we definitely can do the generation in a neater way, but I've
come to the conclusion that caching it really doesn't make sense.

Maybe we can do that for the USP, but definitely not DSPs.

Given I did a bunch of refactoring I might as well share it even though
I'm currently thinking it might not be worth doing:

From 40a8a36f22a6eb053c965231a698c8054d417049 Mon Sep 17 00:00:00 2001
From: Arpit Kumar <arpit1.kumar@samsung.com>
Date: Tue, 16 Sep 2025 13:37:35 +0530
Subject: [PATCH] hw/cxl: Refactored Identify Switch Device & Get Physical Port
 State

Store the physical port info for the USP on reset but refresh
it on demand for the DSPs as they are hotplug capable.

Enables common data to be used for both of:
* 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>
Co-developed: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

---
Added a Co-dev as I have modified this a lot from Arpit's v4.
v5:
- Create a new header to avoid importing all of cxl_device.h into
the upstream port emulation.
- Move the call that caches state to the cxl_upstream_port reset
  to ensure downstream ports are in place before it is called.
  Also will make it available from whatever CCI.  If we ever support
  multiple VCS switches this will need to move an appropriate structure
  representing whole switch information. With only one USP that is
  a reasonable place to put full switch info.
- Downstream ports have more dynamic state (as devices can be
  hotplugged below them).  As such, only fill in the data at time
  of request.
---
 include/hw/cxl/cxl_port.h                   |  87 ++++++++
 include/hw/pci-bridge/cxl_downstream_port.h |  11 +
 include/hw/pci-bridge/cxl_upstream_port.h   |   5 +
 hw/cxl/cxl-mailbox-utils.c                  | 210 ++++++++++----------
 hw/pci-bridge/cxl_downstream.c              |  27 +++
 hw/pci-bridge/cxl_upstream.c                |   5 +
 6 files changed, 245 insertions(+), 100 deletions(-)
 create mode 100644 include/hw/cxl/cxl_port.h
 create mode 100644 include/hw/pci-bridge/cxl_downstream_port.h

diff --git a/include/hw/cxl/cxl_port.h b/include/hw/cxl/cxl_port.h
new file mode 100644
index 000000000000..15960f8b5778
--- /dev/null
+++ b/include/hw/cxl/cxl_port.h
@@ -0,0 +1,87 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+
+#ifndef CXL_PORT_H
+#define CXL_PORT_H
+
+#include "hw/pci/pci.h"
+
+/* Port related commands */
+#define CXL_MAX_PHY_PORTS 256
+
+/* 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 CXL_PORT_CONNECTED_DEV_MODE_NOT_CXL_OR_DISCONN 0x00
+#define CXL_PORT_CONNECTED_DEV_MODE_RCD                0x01
+#define CXL_PORT_CONNECTED_DEV_MODE_68B_VH             0x02
+#define CXL_PORT_CONNECTED_DEV_MODE_256B               0x03
+#define CXL_PORT_CONNECTED_DEV_MODE_LO_256B            0x04
+#define CXL_PORT_CONNECTED_DEV_MODE_PBR                0x05
+
+#define CXL_PORT_CONNECTED_DEV_TYPE_NONE            0x00
+#define CXL_PORT_CONNECTED_DEV_TYPE_PCIE            0x01
+#define CXL_PORT_CONNECTED_DEV_TYPE_1               0x02
+#define CXL_PORT_CONNECTED_DEV_TYPE_2_OR_HBR_SWITCH 0x03
+#define CXL_PORT_CONNECTED_DEV_TYPE_3_SLD           0x04
+#define CXL_PORT_CONNECTED_DEV_TYPE_3_MLD           0x05
+#define CXL_PORT_CONNECTED_DEV_PBR_COMPONENT        0x06
+
+#define CXL_PORT_SUPPORTS_RCD        BIT(0)
+#define CXL_PORT_SUPPORTS_68B_VH     BIT(1)
+#define CXL_PORT_SUPPORTS_256B       BIT(2)
+#define CXL_PORT_SUPPORTS_LO_256B    BIT(3)
+#define CXL_PORT_SUPPORTS_PBR        BIT(4)
+
+#define CXL_PORT_LTSSM_DETECT        0x00
+#define CXL_PORT_LTSSM_POLLING       0x01
+#define CXL_PORT_LTSSM_CONFIGURATION 0x02
+#define CXL_PORT_LTSSM_RECOVERY      0x03
+#define CXL_PORT_LTSSM_L0            0x04
+#define CXL_PORT_LTSSM_L0S           0x05
+#define CXL_PORT_LTSSM_L1            0x06
+#define CXL_PORT_LTSSM_L2            0x07
+#define CXL_PORT_LTSSM_DISABLED      0x08
+#define CXL_PORT_LTSSM_LOOPBACK      0x09
+#define CXL_PORT_LTSSM_HOT_RESET     0x0A
+
+#define CXL_PORT_LINK_STATE_FLAG_LANE_REVERSED    BIT(0)
+#define CXL_PORT_LINK_STATE_FLAG_PERST_ASSERTED   BIT(1)
+#define CXL_PORT_LINK_STATE_FLAG_PRSNT            BIT(2)
+#define CXL_PORT_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;
+
+void cxl_init_physical_port_info(PCIDevice *port_dev, CXLPhyPortInfo *info,
+                                 uint8_t pn,
+                                 uint8_t current_port_config_state,
+                                 uint8_t connected_device_type,
+                                 uint8_t supported_ld_count);
+
+/* Common data for CXL USP and DSP */
+typedef struct CXLSwitchPortData {
+    CXLPhyPortInfo info;
+} CXLSwitchPortData;
+
+#endif /* CXL_PORT_H */
diff --git a/include/hw/pci-bridge/cxl_downstream_port.h b/include/hw/pci-bridge/cxl_downstream_port.h
new file mode 100644
index 000000000000..c3f058365283
--- /dev/null
+++ b/include/hw/pci-bridge/cxl_downstream_port.h
@@ -0,0 +1,11 @@
+#ifndef CXL_DOWNSTREAM_PORT_H
+#define CXL_DOWNSTREAM_PORT_H
+#include "hw/pci/pcie.h"
+#include "hw/pci/pcie_port.h"
+#include "hw/cxl/cxl.h"
+#include "include/hw/cxl/cxl_port.h"
+
+typedef struct CXLDownstreamPort CXLDownstreamPort;
+CXLPhyPortInfo *cxl_dsp_get_physical_port_info(CXLDownstreamPort *dsp);
+
+#endif
diff --git a/include/hw/pci-bridge/cxl_upstream_port.h b/include/hw/pci-bridge/cxl_upstream_port.h
index e3d6a27acc86..fe367fb66f0c 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_port.h"
 
 typedef struct CXLUpstreamPort {
     /*< private >*/
@@ -19,6 +20,10 @@ typedef struct CXLUpstreamPort {
 
     DOECap doe_cdat;
     uint64_t sn;
+
+    CXLSwitchPortData swport_data;
 } CXLUpstreamPort;
 
+void cxl_set_physical_port_info(CXLUpstreamPort *cxl_usp);
+
 #endif /* CXL_SUP_H */
diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c
index 2f449980cdc0..f7aa56b0d385 100644
--- a/hw/cxl/cxl-mailbox-utils.c
+++ b/hw/cxl/cxl-mailbox-utils.c
@@ -13,9 +13,11 @@
 #include "hw/pci/msi.h"
 #include "hw/pci/msix.h"
 #include "hw/cxl/cxl.h"
+#include "hw/cxl/cxl_port.h"
 #include "hw/cxl/cxl_events.h"
 #include "hw/cxl/cxl_mailbox.h"
 #include "hw/pci/pci.h"
+#include "hw/pci-bridge/cxl_downstream_port.h"
 #include "hw/pci-bridge/cxl_upstream_port.h"
 #include "qemu/cutils.h"
 #include "qemu/host-utils.h"
@@ -488,13 +490,21 @@ 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)
+static uint8_t cxl_num_switch_ports(CXLUpstreamPort *usp)
 {
-    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);
+    PCIBus *bus = &PCI_BRIDGE(usp)->sec_bus;
+
+    return pcie_count_ds_ports(bus) + 1;
+}
+
+static void cxl_mark_dsp_active(PCIBus *bus, PCIDevice *dev, void *opaque)
+{
+    uint8_t *bitmask = opaque;
+
+    if (object_dynamic_cast(OBJECT(dev), TYPE_CXL_DSP)) {
+        uint8_t pn = PCIE_PORT(dev)->port;
+
+        bitmask[pn / 8] |= (1 << (pn % 8));
     }
 }
 
@@ -506,9 +516,9 @@ 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 = cxl_num_switch_ports(pp);
+    uint8_t pn;
 
     struct cxl_fmapi_ident_switch_dev_resp_pl {
         uint8_t ingress_port_id;
@@ -525,11 +535,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,
     };
 
@@ -541,16 +551,46 @@ 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,
+    pn = PCIE_PORT(pp)->port;
+    out->active_port_bitmask[pn / 8] |= (1 << pn % 8);
+    pci_for_each_device_under_bus(&PCI_BRIDGE(pp)->sec_bus,
+                                  cxl_mark_dsp_active,
                                   out->active_port_bitmask);
-    out->active_port_bitmask[usp->port / 8] |= (1 << usp->port % 8);
 
     *len_out = sizeof(*out);
 
     return CXL_MBOX_SUCCESS;
 }
 
-/* CXL r3.1 Section 7.6.7.1.2: Get Physical Port State (Opcode 5101h) */
+static CXLDownstreamPort *cxl_find_dsp_on_bus(PCIBus *bus, uint8_t pn)
+{
+
+    PCIDevice *port_dev = pcie_find_port_by_pn(bus, pn);
+
+    if (object_dynamic_cast(OBJECT(port_dev), TYPE_CXL_DSP)) {
+        return CXL_DSP(port_dev);
+    }
+
+    return NULL;
+}
+
+static CXLPhyPortInfo *cxl_get_phyport_info(CXLUpstreamPort *usp, uint8_t pn)
+{
+    CXLDownstreamPort *dsp;
+
+    if (usp->swport_data.info.port_id == pn) {
+        return &usp->swport_data.info;
+    }
+
+    dsp = cxl_find_dsp_on_bus(&PCI_BRIDGE(usp)->sec_bus, pn);
+    if (dsp) {
+        return cxl_dsp_get_physical_port_info(dsp);
+    }
+
+    return NULL;
+}
+
+/* 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,
@@ -558,44 +598,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;
@@ -608,69 +626,22 @@ 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 > cxl_num_switch_ports(pp)) {
+        return CXL_MBOX_INVALID_INPUT;
+    }
 
     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;
-
-        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 {
+        int pn = in->ports[i];
+        CXLPhyPortInfo *info = cxl_get_phyport_info(pp, pn);
+
+        if (!info) {
             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], info, sizeof(*info));
     }
 
+    out->num_ports = in->num_ports;
     pl_size = sizeof(*out) + sizeof(*out->ports) * in->num_ports;
     *len_out = pl_size;
 
@@ -4633,6 +4604,45 @@ void cxl_add_cci_commands(CXLCCI *cci, const struct cxl_cmd (*cxl_cmd_set)[256],
     cxl_rebuild_cel(cci);
 }
 
+void cxl_init_physical_port_info(PCIDevice *port_dev, CXLPhyPortInfo *info,
+                                 uint8_t pn,
+                                 uint8_t current_port_config_state,
+                                 uint8_t connected_device_type,
+                                 uint8_t supported_ld_count)
+{
+    uint16_t lnkcap, lnkcap2, lnksta;
+
+    if (!port_dev->exp.exp_cap) {
+        return;
+    }
+    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));
+
+    *info = (CXLPhyPortInfo) {
+        .port_id = pn,
+        .current_port_config_state = current_port_config_state,
+        .connected_device_mode = CXL_PORT_CONNECTED_DEV_MODE_256B,
+        .connected_device_type = connected_device_type,
+        .supported_cxl_modes = CXL_PORT_SUPPORTS_256B,
+        .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_PORT_LTSSM_L2,
+        .first_negotiated_lane_num = 0,
+        .link_state_flags = 0,
+        .supported_ld_count = supported_ld_count,
+    };
+}
+
 void cxl_initialize_mailbox_swcci(CXLCCI *cci, DeviceState *intf,
                                   DeviceState *d, size_t payload_max)
 {
diff --git a/hw/pci-bridge/cxl_downstream.c b/hw/pci-bridge/cxl_downstream.c
index 320818a8f1ce..8611f54ebd0c 100644
--- a/hw/pci-bridge/cxl_downstream.c
+++ b/hw/pci-bridge/cxl_downstream.c
@@ -13,6 +13,7 @@
 #include "hw/pci/msi.h"
 #include "hw/pci/pcie.h"
 #include "hw/pci/pcie_port.h"
+#include "hw/pci-bridge/cxl_downstream_port.h"
 #include "hw/core/qdev-properties.h"
 #include "hw/core/qdev-properties-system.h"
 #include "hw/cxl/cxl.h"
@@ -24,6 +25,7 @@ typedef struct CXLDownstreamPort {
 
     /*< public >*/
     CXLComponentState cxl_cstate;
+    CXLSwitchPortData swport_data;
 } CXLDownstreamPort;
 
 #define CXL_DOWNSTREAM_PORT_MSI_OFFSET 0x70
@@ -81,6 +83,31 @@ static void cxl_dsp_config_write(PCIDevice *d, uint32_t address,
     cxl_dsp_dvsec_write_config(d, address, val, len);
 }
 
+CXLPhyPortInfo *cxl_dsp_get_physical_port_info(CXLDownstreamPort *dsp)
+{
+    CXLPhyPortInfo *info = &dsp->swport_data.info;
+    uint8_t connected_device_type;
+    PCIDevice *port_dev = PCI_DEVICE(dsp);
+    PCIDevice *ds_dev = pci_bridge_get_sec_bus(PCI_BRIDGE(dsp))
+        ->devices[0];
+
+    if (ds_dev) {
+        if (object_dynamic_cast(OBJECT(ds_dev), TYPE_CXL_TYPE3)) {
+            connected_device_type = CXL_PORT_CONNECTED_DEV_TYPE_3_SLD;
+        } else {
+            connected_device_type = CXL_PORT_CONNECTED_DEV_TYPE_PCIE;
+        }
+    } else {
+        connected_device_type = CXL_PORT_CONNECTED_DEV_TYPE_NONE;
+    }
+
+    cxl_init_physical_port_info(port_dev, info, PCIE_PORT(dsp)->port,
+                                CXL_PORT_CONFIG_STATE_DSP,
+                                connected_device_type, 3);
+
+    return info;
+}
+
 static void cxl_dsp_reset(DeviceState *qdev)
 {
     PCIDevice *d = PCI_DEVICE(qdev);
diff --git a/hw/pci-bridge/cxl_upstream.c b/hw/pci-bridge/cxl_upstream.c
index fb8d19539c9f..64f31110ff2c 100644
--- a/hw/pci-bridge/cxl_upstream.c
+++ b/hw/pci-bridge/cxl_upstream.c
@@ -103,6 +103,11 @@ static void cxl_usp_reset(DeviceState *qdev)
     pcie_cap_deverr_reset(d);
     pcie_cap_fill_link_ep_usp(d, usp->width, usp->speed, usp->flitmode);
     latch_registers(usp);
+
+    /* Assume that the info about the upstream link only changes on reset. */
+    cxl_init_physical_port_info(PCI_DEVICE(usp), &usp->swport_data.info,
+                                PCIE_PORT(usp)->port, CXL_PORT_CONFIG_STATE_USP,
+                                CXL_PORT_CONNECTED_DEV_TYPE_NONE, 0);
 }
 
 static void build_dvsecs(CXLUpstreamPort *usp)
-- 
2.48.1



> 
> Additionally, it introduces new support for Physical Port Control
> of Physical Switch Command Set as per CXL spec r3.2 Section 7.6.7.1.3.
> 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.
> 
> Typical Qemu topology
> (1 USP + 3 DSP's in a switch with 2 CXLType3 devices connected to the 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"
> 
> Tested multiple Qemu topologies:
>         -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 v3 (https://lore.kernel.org/qemu-devel/20250909160316.00000190@huawei.com/T/):
>         -Namespaced the defines with cleaner prefix for Get Physical Port State 
>          Port Information Block members.
>         -switch CCI implementation instead of switch FM interface as per
>          Jonathan's review comments, hence moved perst members initializations
>          from: cxl_initialize_usp_mctpcci() -> cxl_initialize_mailbox_swcci().
> 
> [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, 348 insertions(+), 106 deletions(-)
> 


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH v4 0/2] FM-API Physical Switch Command Set Support
  2026-01-27 15:23   ` [PATCH v4 0/2] FM-API Physical Switch Command Set Support Jonathan Cameron
@ 2026-01-30 12:03     ` Arpit Kumar
  2026-01-30 12:31       ` Jonathan Cameron
  0 siblings, 1 reply; 13+ messages in thread
From: Arpit Kumar @ 2026-01-30 12:03 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: qemu-devel, gost.dev, linux-cxl, dave, vishak.g, krish.reddy,
	a.manzanares, alok.rathore, arpit.sysdev, cpgs

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

On 27/01/26 03:23PM, Jonathan Cameron wrote:
>On Tue, 16 Sep 2025 13:37:34 +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.
>
>Hi Arpit.
>
>So I came back to this series to try and tweak it into being in a state
>for upstreaming.  I made some fairly heavy changes (to push the
>data down to the individual ports where to me it more logically sits)
>but it became apparent to me during this that there is a fundamental
>problem.  This stuff isn't static because it needs to reflect port
>retraining and hotplug events etc.  If we want to store cached
>data we are going to have to deal with updating it from a significant
>number of different places. The perst one you have covered in patch 2
>is an example but there are others.
>
>Now we definitely can do the generation in a neater way, but I've
>come to the conclusion that caching it really doesn't make sense.
>
>Maybe we can do that for the USP, but definitely not DSPs.
>
>Given I did a bunch of refactoring I might as well share it even though
>I'm currently thinking it might not be worth doing:
>
>From 40a8a36f22a6eb053c965231a698c8054d417049 Mon Sep 17 00:00:00 2001
>From: Arpit Kumar <arpit1.kumar@samsung.com>
>Date: Tue, 16 Sep 2025 13:37:35 +0530
>Subject: [PATCH] hw/cxl: Refactored Identify Switch Device & Get Physical Port
> State
>
>Store the physical port info for the USP on reset but refresh
>it on demand for the DSPs as they are hotplug capable.
>
>Enables common data to be used for both of:
>* 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>
>Co-developed: Jonathan Cameron <Jonathan.Cameron@huawei.com>
>Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
>
>---
>Added a Co-dev as I have modified this a lot from Arpit's v4.
>v5:
>- Create a new header to avoid importing all of cxl_device.h into
>the upstream port emulation.
>- Move the call that caches state to the cxl_upstream_port reset
>  to ensure downstream ports are in place before it is called.
>  Also will make it available from whatever CCI.  If we ever support
>  multiple VCS switches this will need to move an appropriate structure
>  representing whole switch information. With only one USP that is
>  a reasonable place to put full switch info.
>- Downstream ports have more dynamic state (as devices can be
>  hotplugged below them).  As such, only fill in the data at time
>  of request.

Hi Jonathan,
Thanks for the update! The changes look good to me. I do concede that
it requires more dynamic approach. This would help in future command 
support like multiple VCS, MLD etc. I intially refrained from making changes
to individual ports but it does logically fits there and would
optimize the command support.

>---
> include/hw/cxl/cxl_port.h                   |  87 ++++++++
> include/hw/pci-bridge/cxl_downstream_port.h |  11 +
> include/hw/pci-bridge/cxl_upstream_port.h   |   5 +
> hw/cxl/cxl-mailbox-utils.c                  | 210 ++++++++++----------
> hw/pci-bridge/cxl_downstream.c              |  27 +++
> hw/pci-bridge/cxl_upstream.c                |   5 +
> 6 files changed, 245 insertions(+), 100 deletions(-)
> create mode 100644 include/hw/cxl/cxl_port.h
> create mode 100644 include/hw/pci-bridge/cxl_downstream_port.h
>
>diff --git a/include/hw/cxl/cxl_port.h b/include/hw/cxl/cxl_port.h
>new file mode 100644
>index 000000000000..15960f8b5778
>--- /dev/null
>+++ b/include/hw/cxl/cxl_port.h
>@@ -0,0 +1,87 @@
>+/* SPDX-License-Identifier: GPL-2.0-or-later */
>+
>+#ifndef CXL_PORT_H
>+#define CXL_PORT_H
>+
>+#include "hw/pci/pci.h"
>+
>+/* Port related commands */
>+#define CXL_MAX_PHY_PORTS 256
>+
>+/* 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 CXL_PORT_CONNECTED_DEV_MODE_NOT_CXL_OR_DISCONN 0x00
>+#define CXL_PORT_CONNECTED_DEV_MODE_RCD                0x01
>+#define CXL_PORT_CONNECTED_DEV_MODE_68B_VH             0x02
>+#define CXL_PORT_CONNECTED_DEV_MODE_256B               0x03
>+#define CXL_PORT_CONNECTED_DEV_MODE_LO_256B            0x04
>+#define CXL_PORT_CONNECTED_DEV_MODE_PBR                0x05
>+
>+#define CXL_PORT_CONNECTED_DEV_TYPE_NONE            0x00
>+#define CXL_PORT_CONNECTED_DEV_TYPE_PCIE            0x01
>+#define CXL_PORT_CONNECTED_DEV_TYPE_1               0x02
>+#define CXL_PORT_CONNECTED_DEV_TYPE_2_OR_HBR_SWITCH 0x03
>+#define CXL_PORT_CONNECTED_DEV_TYPE_3_SLD           0x04
>+#define CXL_PORT_CONNECTED_DEV_TYPE_3_MLD           0x05
>+#define CXL_PORT_CONNECTED_DEV_PBR_COMPONENT        0x06
>+
>+#define CXL_PORT_SUPPORTS_RCD        BIT(0)
>+#define CXL_PORT_SUPPORTS_68B_VH     BIT(1)
>+#define CXL_PORT_SUPPORTS_256B       BIT(2)
>+#define CXL_PORT_SUPPORTS_LO_256B    BIT(3)
>+#define CXL_PORT_SUPPORTS_PBR        BIT(4)
>+
>+#define CXL_PORT_LTSSM_DETECT        0x00
>+#define CXL_PORT_LTSSM_POLLING       0x01
>+#define CXL_PORT_LTSSM_CONFIGURATION 0x02
>+#define CXL_PORT_LTSSM_RECOVERY      0x03
>+#define CXL_PORT_LTSSM_L0            0x04
>+#define CXL_PORT_LTSSM_L0S           0x05
>+#define CXL_PORT_LTSSM_L1            0x06
>+#define CXL_PORT_LTSSM_L2            0x07
>+#define CXL_PORT_LTSSM_DISABLED      0x08
>+#define CXL_PORT_LTSSM_LOOPBACK      0x09
>+#define CXL_PORT_LTSSM_HOT_RESET     0x0A
>+
>+#define CXL_PORT_LINK_STATE_FLAG_LANE_REVERSED    BIT(0)
>+#define CXL_PORT_LINK_STATE_FLAG_PERST_ASSERTED   BIT(1)
>+#define CXL_PORT_LINK_STATE_FLAG_PRSNT            BIT(2)
>+#define CXL_PORT_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;
>+
>+void cxl_init_physical_port_info(PCIDevice *port_dev, CXLPhyPortInfo *info,
>+                                 uint8_t pn,
>+                                 uint8_t current_port_config_state,
>+                                 uint8_t connected_device_type,
>+                                 uint8_t supported_ld_count);
>+
>+/* Common data for CXL USP and DSP */
>+typedef struct CXLSwitchPortData {
>+    CXLPhyPortInfo info;
>+} CXLSwitchPortData;
>+
>+#endif /* CXL_PORT_H */
>diff --git a/include/hw/pci-bridge/cxl_downstream_port.h b/include/hw/pci-bridge/cxl_downstream_port.h
>new file mode 100644
>index 000000000000..c3f058365283
>--- /dev/null
>+++ b/include/hw/pci-bridge/cxl_downstream_port.h
>@@ -0,0 +1,11 @@
>+#ifndef CXL_DOWNSTREAM_PORT_H
>+#define CXL_DOWNSTREAM_PORT_H
>+#include "hw/pci/pcie.h"
>+#include "hw/pci/pcie_port.h"
>+#include "hw/cxl/cxl.h"
>+#include "include/hw/cxl/cxl_port.h"
>+
>+typedef struct CXLDownstreamPort CXLDownstreamPort;
>+CXLPhyPortInfo *cxl_dsp_get_physical_port_info(CXLDownstreamPort *dsp);
>+
>+#endif
>diff --git a/include/hw/pci-bridge/cxl_upstream_port.h b/include/hw/pci-bridge/cxl_upstream_port.h
>index e3d6a27acc86..fe367fb66f0c 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_port.h"
>
> typedef struct CXLUpstreamPort {
>     /*< private >*/
>@@ -19,6 +20,10 @@ typedef struct CXLUpstreamPort {
>
>     DOECap doe_cdat;
>     uint64_t sn;
>+
>+    CXLSwitchPortData swport_data;
> } CXLUpstreamPort;
>
>+void cxl_set_physical_port_info(CXLUpstreamPort *cxl_usp);
>+
> #endif /* CXL_SUP_H */
>diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c
>index 2f449980cdc0..f7aa56b0d385 100644
>--- a/hw/cxl/cxl-mailbox-utils.c
>+++ b/hw/cxl/cxl-mailbox-utils.c
>@@ -13,9 +13,11 @@
> #include "hw/pci/msi.h"
> #include "hw/pci/msix.h"
> #include "hw/cxl/cxl.h"
>+#include "hw/cxl/cxl_port.h"
> #include "hw/cxl/cxl_events.h"
> #include "hw/cxl/cxl_mailbox.h"
> #include "hw/pci/pci.h"
>+#include "hw/pci-bridge/cxl_downstream_port.h"
> #include "hw/pci-bridge/cxl_upstream_port.h"
> #include "qemu/cutils.h"
> #include "qemu/host-utils.h"
>@@ -488,13 +490,21 @@ 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)
>+static uint8_t cxl_num_switch_ports(CXLUpstreamPort *usp)
> {
>-    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);
>+    PCIBus *bus = &PCI_BRIDGE(usp)->sec_bus;
>+
>+    return pcie_count_ds_ports(bus) + 1;
>+}
>+
>+static void cxl_mark_dsp_active(PCIBus *bus, PCIDevice *dev, void *opaque)
>+{
>+    uint8_t *bitmask = opaque;
>+
>+    if (object_dynamic_cast(OBJECT(dev), TYPE_CXL_DSP)) {
>+        uint8_t pn = PCIE_PORT(dev)->port;
>+
>+        bitmask[pn / 8] |= (1 << (pn % 8));
>     }
> }
>
>@@ -506,9 +516,9 @@ 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 = cxl_num_switch_ports(pp);
>+    uint8_t pn;
>
>     struct cxl_fmapi_ident_switch_dev_resp_pl {
>         uint8_t ingress_port_id;
>@@ -525,11 +535,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,
>     };
>
>@@ -541,16 +551,46 @@ 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,
>+    pn = PCIE_PORT(pp)->port;
>+    out->active_port_bitmask[pn / 8] |= (1 << pn % 8);
>+    pci_for_each_device_under_bus(&PCI_BRIDGE(pp)->sec_bus,
>+                                  cxl_mark_dsp_active,
>                                   out->active_port_bitmask);
>-    out->active_port_bitmask[usp->port / 8] |= (1 << usp->port % 8);
>
>     *len_out = sizeof(*out);
>
>     return CXL_MBOX_SUCCESS;
> }
>
>-/* CXL r3.1 Section 7.6.7.1.2: Get Physical Port State (Opcode 5101h) */
>+static CXLDownstreamPort *cxl_find_dsp_on_bus(PCIBus *bus, uint8_t pn)
>+{
>+
>+    PCIDevice *port_dev = pcie_find_port_by_pn(bus, pn);
>+
>+    if (object_dynamic_cast(OBJECT(port_dev), TYPE_CXL_DSP)) {
>+        return CXL_DSP(port_dev);
>+    }
>+
>+    return NULL;
>+}
>+
>+static CXLPhyPortInfo *cxl_get_phyport_info(CXLUpstreamPort *usp, uint8_t pn)
>+{
>+    CXLDownstreamPort *dsp;
>+
>+    if (usp->swport_data.info.port_id == pn) {
>+        return &usp->swport_data.info;
>+    }
>+
>+    dsp = cxl_find_dsp_on_bus(&PCI_BRIDGE(usp)->sec_bus, pn);
>+    if (dsp) {
>+        return cxl_dsp_get_physical_port_info(dsp);
>+    }
>+
>+    return NULL;
>+}
>+
>+/* 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,
>@@ -558,44 +598,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;
>@@ -608,69 +626,22 @@ 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 > cxl_num_switch_ports(pp)) {
>+        return CXL_MBOX_INVALID_INPUT;
>+    }
>
>     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;
>-
>-        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 {
>+        int pn = in->ports[i];
>+        CXLPhyPortInfo *info = cxl_get_phyport_info(pp, pn);
>+
>+        if (!info) {
>             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], info, sizeof(*info));
>     }
>
>+    out->num_ports = in->num_ports;
>     pl_size = sizeof(*out) + sizeof(*out->ports) * in->num_ports;
>     *len_out = pl_size;
>
>@@ -4633,6 +4604,45 @@ void cxl_add_cci_commands(CXLCCI *cci, const struct cxl_cmd (*cxl_cmd_set)[256],
>     cxl_rebuild_cel(cci);
> }
>
>+void cxl_init_physical_port_info(PCIDevice *port_dev, CXLPhyPortInfo *info,
>+                                 uint8_t pn,
>+                                 uint8_t current_port_config_state,
>+                                 uint8_t connected_device_type,
>+                                 uint8_t supported_ld_count)
>+{
>+    uint16_t lnkcap, lnkcap2, lnksta;
>+
>+    if (!port_dev->exp.exp_cap) {
>+        return;
>+    }
>+    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));
>+
>+    *info = (CXLPhyPortInfo) {
>+        .port_id = pn,
>+        .current_port_config_state = current_port_config_state,
>+        .connected_device_mode = CXL_PORT_CONNECTED_DEV_MODE_256B,
>+        .connected_device_type = connected_device_type,
>+        .supported_cxl_modes = CXL_PORT_SUPPORTS_256B,
>+        .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_PORT_LTSSM_L2,
>+        .first_negotiated_lane_num = 0,
>+        .link_state_flags = 0,
>+        .supported_ld_count = supported_ld_count,
>+    };
>+}
>+
> void cxl_initialize_mailbox_swcci(CXLCCI *cci, DeviceState *intf,
>                                   DeviceState *d, size_t payload_max)
> {
>diff --git a/hw/pci-bridge/cxl_downstream.c b/hw/pci-bridge/cxl_downstream.c
>index 320818a8f1ce..8611f54ebd0c 100644
>--- a/hw/pci-bridge/cxl_downstream.c
>+++ b/hw/pci-bridge/cxl_downstream.c
>@@ -13,6 +13,7 @@
> #include "hw/pci/msi.h"
> #include "hw/pci/pcie.h"
> #include "hw/pci/pcie_port.h"
>+#include "hw/pci-bridge/cxl_downstream_port.h"
> #include "hw/core/qdev-properties.h"
> #include "hw/core/qdev-properties-system.h"
> #include "hw/cxl/cxl.h"
>@@ -24,6 +25,7 @@ typedef struct CXLDownstreamPort {
>
>     /*< public >*/
>     CXLComponentState cxl_cstate;
>+    CXLSwitchPortData swport_data;
> } CXLDownstreamPort;
>
> #define CXL_DOWNSTREAM_PORT_MSI_OFFSET 0x70
>@@ -81,6 +83,31 @@ static void cxl_dsp_config_write(PCIDevice *d, uint32_t address,
>     cxl_dsp_dvsec_write_config(d, address, val, len);
> }
>
>+CXLPhyPortInfo *cxl_dsp_get_physical_port_info(CXLDownstreamPort *dsp)
>+{
>+    CXLPhyPortInfo *info = &dsp->swport_data.info;
>+    uint8_t connected_device_type;
>+    PCIDevice *port_dev = PCI_DEVICE(dsp);
>+    PCIDevice *ds_dev = pci_bridge_get_sec_bus(PCI_BRIDGE(dsp))
>+        ->devices[0];
>+
>+    if (ds_dev) {
>+        if (object_dynamic_cast(OBJECT(ds_dev), TYPE_CXL_TYPE3)) {
>+            connected_device_type = CXL_PORT_CONNECTED_DEV_TYPE_3_SLD;
>+        } else {
>+            connected_device_type = CXL_PORT_CONNECTED_DEV_TYPE_PCIE;
>+        }
>+    } else {
>+        connected_device_type = CXL_PORT_CONNECTED_DEV_TYPE_NONE;
>+    }
>+
>+    cxl_init_physical_port_info(port_dev, info, PCIE_PORT(dsp)->port,
>+                                CXL_PORT_CONFIG_STATE_DSP,
>+                                connected_device_type, 3);
>+
>+    return info;
>+}
>+
> static void cxl_dsp_reset(DeviceState *qdev)
> {
>     PCIDevice *d = PCI_DEVICE(qdev);
>diff --git a/hw/pci-bridge/cxl_upstream.c b/hw/pci-bridge/cxl_upstream.c
>index fb8d19539c9f..64f31110ff2c 100644
>--- a/hw/pci-bridge/cxl_upstream.c
>+++ b/hw/pci-bridge/cxl_upstream.c
>@@ -103,6 +103,11 @@ static void cxl_usp_reset(DeviceState *qdev)
>     pcie_cap_deverr_reset(d);
>     pcie_cap_fill_link_ep_usp(d, usp->width, usp->speed, usp->flitmode);
>     latch_registers(usp);
>+
>+    /* Assume that the info about the upstream link only changes on reset. */
>+    cxl_init_physical_port_info(PCI_DEVICE(usp), &usp->swport_data.info,
>+                                PCIE_PORT(usp)->port, CXL_PORT_CONFIG_STATE_USP,
>+                                CXL_PORT_CONNECTED_DEV_TYPE_NONE, 0);
> }
>
> static void build_dvsecs(CXLUpstreamPort *usp)
>-- 
>2.48.1
>
>
>
>>
>> Additionally, it introduces new support for Physical Port Control
>> of Physical Switch Command Set as per CXL spec r3.2 Section 7.6.7.1.3.
>> 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.
>>
>> Typical Qemu topology
>> (1 USP + 3 DSP's in a switch with 2 CXLType3 devices connected to the 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"
>>
>> Tested multiple Qemu topologies:
>>         -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 v3 (https://lore.kernel.org/qemu-devel/20250909160316.00000190@huawei.com/T/):
>>         -Namespaced the defines with cleaner prefix for Get Physical Port State
>>          Port Information Block members.
>>         -switch CCI implementation instead of switch FM interface as per
>>          Jonathan's review comments, hence moved perst members initializations
>>          from: cxl_initialize_usp_mctpcci() -> cxl_initialize_mailbox_swcci().
>>
>> [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, 348 insertions(+), 106 deletions(-)
>>
>

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



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v4 0/2] FM-API Physical Switch Command Set Support
  2026-01-30 12:03     ` Arpit Kumar
@ 2026-01-30 12:31       ` Jonathan Cameron
  0 siblings, 0 replies; 13+ messages in thread
From: Jonathan Cameron @ 2026-01-30 12:31 UTC (permalink / raw)
  To: Arpit Kumar
  Cc: qemu-devel, gost.dev, linux-cxl, dave, vishak.g, krish.reddy,
	a.manzanares, alok.rathore, arpit.sysdev, cpgs

On Fri, 30 Jan 2026 17:33:50 +0530
Arpit Kumar <arpit1.kumar@samsung.com> wrote:

> On 27/01/26 03:23PM, Jonathan Cameron wrote:
> >On Tue, 16 Sep 2025 13:37:34 +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.  
> >
> >Hi Arpit.
> >
> >So I came back to this series to try and tweak it into being in a state
> >for upstreaming.  I made some fairly heavy changes (to push the
> >data down to the individual ports where to me it more logically sits)
> >but it became apparent to me during this that there is a fundamental
> >problem.  This stuff isn't static because it needs to reflect port
> >retraining and hotplug events etc.  If we want to store cached
> >data we are going to have to deal with updating it from a significant
> >number of different places. The perst one you have covered in patch 2
> >is an example but there are others.
> >
> >Now we definitely can do the generation in a neater way, but I've
> >come to the conclusion that caching it really doesn't make sense.
> >
> >Maybe we can do that for the USP, but definitely not DSPs.
> >
> >Given I did a bunch of refactoring I might as well share it even though
> >I'm currently thinking it might not be worth doing:
> >
> >From 40a8a36f22a6eb053c965231a698c8054d417049 Mon Sep 17 00:00:00 2001
> >From: Arpit Kumar <arpit1.kumar@samsung.com>
> >Date: Tue, 16 Sep 2025 13:37:35 +0530
> >Subject: [PATCH] hw/cxl: Refactored Identify Switch Device & Get Physical Port
> > State
> >
> >Store the physical port info for the USP on reset but refresh
> >it on demand for the DSPs as they are hotplug capable.
> >
> >Enables common data to be used for both of:
> >* 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>
> >Co-developed: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> >Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> >
> >---
> >Added a Co-dev as I have modified this a lot from Arpit's v4.
> >v5:
> >- Create a new header to avoid importing all of cxl_device.h into
> >the upstream port emulation.
> >- Move the call that caches state to the cxl_upstream_port reset
> >  to ensure downstream ports are in place before it is called.
> >  Also will make it available from whatever CCI.  If we ever support
> >  multiple VCS switches this will need to move an appropriate structure
> >  representing whole switch information. With only one USP that is
> >  a reasonable place to put full switch info.
> >- Downstream ports have more dynamic state (as devices can be
> >  hotplugged below them).  As such, only fill in the data at time
> >  of request.  
> 
> Hi Jonathan,
> Thanks for the update! The changes look good to me. I do concede that
> it requires more dynamic approach. This would help in future command 
> support like multiple VCS, MLD etc. I intially refrained from making changes
> to individual ports but it does logically fits there and would
> optimize the command support.

My current thinking is this major refactor is justified. There isn't
a strong reason not to do what the original code does and built it directly
in the command handler.

I'm preparing some cleanup of that though, such as using the defines
you added so it is more obvious what values are set.

The Physical Port Controls stuff can then drop in on top of that.

Once all that is in place we can reconsider if a refactor like the
one in the code below is worth doing.

We might also need a few more updates to keep track of things like flit
mode control patches.

Anyhow, I'll throw something together in the next day or two to share.

Jonathan




^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2026-01-30 12:31 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <CGME20250916080757epcas5p2db403c9648f7818b22413d90acb4fdd4@epcas5p2.samsung.com>
2025-09-16  8:07 ` [PATCH v4 0/2] FM-API Physical Switch Command Set Support Arpit Kumar
2025-09-16  8:07   ` [PATCH v4 1/2] hw/cxl: Refactored Identify Switch Device & Get Physical Port State Arpit Kumar
2025-09-17 15:55     ` Jonathan Cameron
2025-09-19 11:03       ` Arpit Kumar
2025-09-30 14:26         ` Jonathan Cameron
2025-09-16  8:07   ` [PATCH v4 2/2] hw/cxl: Add Physical Port Control (Opcode 5102h) Arpit Kumar
2025-09-17 16:05     ` Jonathan Cameron
2025-09-19 11:16       ` Arpit Kumar
2025-09-17 16:29     ` Jonathan Cameron
2025-09-19 11:21       ` Arpit Kumar
2026-01-27 15:23   ` [PATCH v4 0/2] FM-API Physical Switch Command Set Support Jonathan Cameron
2026-01-30 12:03     ` Arpit Kumar
2026-01-30 12:31       ` Jonathan Cameron

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox