Re: [PATCH v4 1/2] hw/cxl: Refactored Identify Switch Device & Get Physical Port State

[Jonathan Cameron <jonathan.cameron@huawei.com>]

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