Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Puplic MCTP Medium Physical Address to MCTP Endpoint D-Bus Object path #52

Open
ThuBaNguyen opened this issue Oct 2, 2024 · 7 comments

Comments

@ThuBaNguyen
Copy link
Contributor

  1. Background
  • One MCTP Endpoint can support multiple medium interfaces: I2C, USB, PCIe-VDM. The speed of the different medium interface can be different. The sensor polling, event monitoring should use the highest speed interface to increase the performance.
  • The upper application such as pldmd can support multiple MCTP-Binding interface at the same time. It can also choose of interface with highest speed. So pldmd need to know the medium type to decide which medium interface should it use to communicate with EP when there are many medium interfaces.
  1. Propose
  • Add the property name Type in au.com.codeconstruct.MCTP.Endpoint1 with data type as byte to define the medium type as Table 2 – MCTP physical medium identifiers in DSP0239 V1.3.0
  • Create the property name Address in au.com.codeconstruct.MCTP.Endpoint1 with data type as string or byte array as Table 27 – Routing Table Entry format in DSP0236 V1.3.0. Depends on the MCTP binding type the size of this Address property can different. Maybe we can add AddressSize property to identify the byte size of address.
  • As table Table 27 – Routing Table Entry format, the port number should also be public to MCTP Endpoint D-Bus object path. When the EP is bridge we need to identify a particular bus connection that the physical address for the entry is defined under that bridge.
@amboar
Copy link
Contributor

amboar commented Oct 2, 2024

By DSP0236 v1.3.3 Figure 9 this statement isn't necessarily true:

The upper application such as pldmd can support multiple MCTP-Binding interface at the same time. It can also choose of interface with highest speed. So pldmd need to know the medium type to decide which medium interface should it use to communicate with EP when there are many medium interfaces.

The one EID can be assigned to multiple ports on a device, in which case the interface selection must be done by the kernel route table.

Given that I think that we need to adjust your proposal a little. What thoughts do you have on accommodating multiple interfaces for a given EID?

@ThuBaNguyen
Copy link
Contributor Author

ThuBaNguyen commented Oct 3, 2024

The one EID can be assigned to multiple ports on a device, in which case the interface selection must be done by the kernel route table.

Given that I think that we need to adjust your proposal a little. What thoughts do you have on accommodating multiple interfaces for a given EID?

In that case, as my opinion, the Type property can be integrated to Address property.
The address type will be changed from array of byte to vector of pair {typeAddress, portNumber, array of byte}.
So the address will list the supported interfaces address struct of one EID. Each address includes the address type and portNumber and Address bytes.

@amboar
Copy link
Contributor

amboar commented Oct 4, 2024

So changing the address type isn't feasible without changing the object version. I think it's something we should try to avoid. Bear in mind that the object path is keyed by the EID, so using an array of addresses in the interface feels like it's shoe-horning the information into places its not designed to belong.

Further, if one EID is mapped across multiple ports on a device, exposing this information in userspace isn't useful anyway as it shouldn't be the responsibility of userspace to make the choice.

I think we need something more generic, possibly:

/au/com/codeconstruct/mctp1/devices/<UUID>

On this we can have one interface describing the network in which the device is participating, and another interface describing its physical ports, their properties, and the EID associated with each port. That way you can use the current known EID to look up the endpoint in mctpd, pull the UUID from the xyz.openbmc_project.Common.UUID interface, and then look it up under the devices path to find the associated interfaces. If all interfaces are mapped to one EID then its up to the kernel to make the routing choice, but if distinct ports on the device have distinct EIDs then at least userspace can choose an EID based on the transport properties.

@ThuBaNguyen
Copy link
Contributor Author

/au/com/codeconstruct/mctp1/devices/<UUID>

I think we need trims some character from UUID to make it compatible with D-Bus object path requirement. Right?


On this we can have one interface describing the network in which the device is participating, 

Do you mean the interface like below?

xyz.openbmc_project.MCTP.Network    interface -         -                                      -
.NetworkIds                         property  au         1 2 3                                 const

and another interface describing its physical ports, their properties, and the EID associated with each port.

Do you have any idea about the Physical ports interface.
I think we can use the name xyz.openbmc_project.MCTP.Ports.
What do you think about below.

xyz.openbmc_project.Common.Port     interface -         -                                      -
.EIDs                               property  ay        8 9                                    const
.Types                              property  ay        3 d                                    const
.PortNumber                         property  ay        0 3                                    const
.Address                            property  v{ay}     {I2Caddress} {PCIe Address}            const

So the device has 2 ports:

  • The port 0: EID = 8, medium type 3 (SMBus) , Portnumber (in routing entry) 0, PhyAddress {I2CAddress}
  • The port 1: EID = 9, medium type d (PCIe 5.0) , Portnumber (in routing entry) 3, PhyAddress {PCieAddress}

That way you can use the current known EID to look up the endpoint in mctpd, pull the UUID from the xyz.openbmc_project.Common.UUID interface, and then look it up under the devices path to find the associated interfaces. If all interfaces are mapped to one EID then its up to the kernel to make the routing choice, but if distinct ports on the device have distinct EIDs then at least userspace can choose an EID based on the transport properties.

@jk-ozlabs
Copy link
Member

I think we need trims some character from UUID to make it compatible with D-Bus object path requirement. Right?

There is no limit on object path length, but we would need to comply with the object name requirements, and format the UUID accordingly (mainly: no - characters)

On this we can have one interface describing the network in which the device is participating,

Do you mean the interface like below?

xyz.openbmc_project.MCTP.Network    interface -         -                                      -
.NetworkIds                         property  au         1 2 3                                 const

Probably not. We would want to describe the connectivity to a device, rather than a set of network numbers (and then EIDs/hardware addresses elsewhere). We may be able to do that with an object path reference, but can discuss with Andrew on the design there.

That way you can use the current known EID to look up the endpoint in mctpd, pull the UUID from the xyz.openbmc_project.Common.UUID interface, and then look it up under the devices path to find the associated interfaces. If all interfaces are mapped to one EID then its up to the kernel to make the routing choice, but if distinct ports on the device have distinct EIDs then at least userspace can choose an EID based on the transport properties.

I think we're missing part of the requirements here: how are you discovering those devices in the first place?

If we just have the Medium Type available on the endpoint object, you should then have all you need to choose an endpoint, no?

Overall, I think we need to make some overall design choices on whether we would "prefer" the split vs. unified EID approach here, and target our design decisions accordingly (of course, we'd still want to support both approaches regardless). I'll have a think about that and update shortly.

@amboar
Copy link
Contributor

amboar commented Oct 9, 2024

If we just have the Medium Type available on the endpoint object, you should then have all you need to choose an endpoint, no?

But it's a matter of finding the set of EIDs associated with a specific device, right? If we don't have an index object such as something addressed by the UUID, then we have to iterate all the EID objects to correlate the UUIDs to find the faster interface.

@ThuBaNguyen
Copy link
Contributor Author

I think we need trims some character from UUID to make it compatible with D-Bus object path requirement. Right?

There is no limit on object path length, but we would need to comply with the object name requirements, and format the UUID accordingly (mainly: no - characters)

On this we can have one interface describing the network in which the device is participating,

Do you mean the interface like below?

xyz.openbmc_project.MCTP.Network    interface -         -                                      -
.NetworkIds                         property  au         1 2 3                                 const

Probably not. We would want to describe the connectivity to a device, rather than a set of network numbers (and then EIDs/hardware addresses elsewhere). We may be able to do that with an object path reference, but can discuss with Andrew on the design there.

That way you can use the current known EID to look up the endpoint in mctpd, pull the UUID from the xyz.openbmc_project.Common.UUID interface, and then look it up under the devices path to find the associated interfaces. If all interfaces are mapped to one EID then its up to the kernel to make the routing choice, but if distinct ports on the device have distinct EIDs then at least userspace can choose an EID based on the transport properties.

I think we're missing part of the requirements here: how are you discovering those devices in the first place?

There are two ways the mctpd can discovery the device.

  1. As the current mctpd, when BMC is BO, we are using the MCTP D-Bus method such as .AssignEndpoint, .AssignEndpointStatic, .LearnEndpoint, and .SetupEndpoint. In this case the EID, Address is known, PortNumber is 0 in this case. Network is assigned by mctpd.
  2. In the future, when BMC is EP, the mctpd service can send GetRoutingTable to get the Routing table from BO, in each routing entry we have EID, Address, port number. Network is assigned by mctpd.

If we just have the Medium Type available on the endpoint object, you should then have all you need to choose an endpoint, no?

It is not enough because we can't map the medium type of one EID.

The other things, we need to concern is the name of endpoint (Terminus name) which is required to create the PLDM sensors. Currently, the terminus name can come from PLDM "Overall system entity AUX name PDR" in pldmd. The other solution is static configuration in mctpd.

  1. About "Overall system entity AUX name PDR", this PDR is optional, I found many device will not support this PDRs. They don't event support the Sensors name PDRs. CXL cards are example. When there are multiple card, the terminus name from PDRs can be the same, pldmd need to use the phyaddress to indexing the cards.
  2. About the static configuration, when BMC is EP it can't assign static EID for terminus. Moreover in the PCIe interface the physical address is enumerated in the boot up time. The static configuration will not work also.
    In that case, mctpd needs provide the routing table infos to D-Bus to allow the other application such as pldmd mapping the Port number, physical address to Terminus name.

Overall, I think we need to make some overall design choices on whether we would "prefer" the split vs. unified EID approach here, and target our design decisions accordingly (of course, we'd still want to support both approaches regardless). I'll have a think about that and update shortly.

Yah, please things about that. In the current design, only mctpd knows about the physical address (physical address is mandatory for one endpoint in MCTP interface). If mctpd does not share those info, the other application can get the trouble.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants