Skip to content

HLD: ELSFP Memory map and paging support#2207

Open
abhi-nexthop wants to merge 6 commits intosonic-net:masterfrom
nexthop-ai:elsfp-cmis-memory-map
Open

HLD: ELSFP Memory map and paging support#2207
abhi-nexthop wants to merge 6 commits intosonic-net:masterfrom
nexthop-ai:elsfp-cmis-memory-map

Conversation

@abhi-nexthop
Copy link
Copy Markdown

@abhi-nexthop abhi-nexthop commented Feb 6, 2026

This HLD refactors CMIS Memory Map into page classes and defines an ELSFP Memory Map along with its respective pages. The refactor also avoids any API level changes for any classes that may consume the CMIS map

Implementation PRs:
PR 1: Memory Map Refactor into pages
nexthop-ai/sonic-platform-common#1

PR 2: ELSFP pages and Memory Map
nexthop-ai/sonic-platform-common#2

@linux-foundation-easycla
Copy link
Copy Markdown

linux-foundation-easycla Bot commented Feb 6, 2026

CLA Signed

The committers listed above are authorized under a signed CLA.

Comment thread doc/layer1/elsfp-and-paging-support.md
Comment thread doc/layer1/elsfp-and-paging-support.md
Copy link
Copy Markdown
Contributor

@prgeor prgeor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Junchao-Mellanox is there any impact to caching of eeprom ?

@prgeor
Copy link
Copy Markdown
Contributor

prgeor commented Feb 13, 2026

@abhi-nexthop please fix the DCO checker. The commit needs to be signed-off

abhi-nexthop and others added 5 commits February 13, 2026 17:36
Signed-off-by: Abhi Singh <abhi@nexthop.ai>
Co-authored-by: Brian Gallagher <bgallagher@nexthop.ai>
Signed-off-by: Abhi Singh <abhi@nexthop.ai>
Signed-off-by: Abhi Singh <abhi@nexthop.ai>
Signed-off-by: Abhi Singh <abhi@nexthop.ai>
Signed-off-by: Abhi Singh <abhi@nexthop.ai>
@mssonicbld
Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines
Copy link
Copy Markdown

No pipelines are associated with this pull request.

@prgeor prgeor requested a review from eddyk-nvidia March 23, 2026 14:52
| A0-AF | CDB EPL extended payload segments |


While, SONiC registers these paged registers, it does not explicitly sort them in individual pages. As the CMIS spec grows and new specs that branch off CMIS are defined (such as ELSFP and C-CMIS) , more registers may be added and the memory map will continue to grow. Furthermore, currently, other memory classes such as the CCmisMemMap inherit the CMIS memory map and add their own registers. However, not all pages in the CMIS memory map are used by every device. For example, if an ELSFPMemoryMap object derives from the CMIS memory map, it will contain registers from pages that do not comply to the ELSFP spec. Therefore, there is a need to explicitly define pages and provide the ability to pick a subset of CMIS pages for a memory map.
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please explain the benefit of splitting it into separate memory maps instead of simply extending CmisMemMap to support ELSFP using inheritance?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The ELSFP memory map is not a superset of the common CMIS memory map. Pages like 10h for lane/datapath control do not exist in the ELSFP memory map for instance, since an ELSFP is a resource module with no datapath. Extending the CmisMemMap via inheritance would end up defining a memory map for the ELSFP that contains these pages erroneously.

Screenshot 2026-04-09 at 12 35 33 PM

The rationale behind factoring pages out into their own classes is that it allows for easier composition of higher-level memory maps (ELSFP/CMIS/C-CMIS) that share pages but are not strict subset or superset extensions of each other.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we look at the ELSFP CMIS specification itself, it states the following:

Image Image

I agree that there may be a few pages that are not used, but we can simply choose not to use them when calling the CmisApi functions.
As I see it, MemMap should represent only the layout and should not contain functionality. Because of that, I think it is perfectly acceptable for some pages to remain unused as a result of inheritance.

I understand that in separate mode there may be a need for two CMIS interfaces. However, I still believe the right approach would be to have two memory maps: one for the common CMIS layout, and another for ELSFP CMIS that also inherits from CMIS for the ELSFP-specific information.

Also, in joint mode we use both CMIS and ELSFP CMIS through the same interface, so this approach does not fit joint mode well. With this design, we would end up duplicating the EEPROM layout.

Copy link
Copy Markdown

@aditya-nexthop aditya-nexthop Apr 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that there may be a few pages that are not used, but we can simply choose not to use them when calling the CmisApi functions.

Why not do it properly that reflects the actual pages implemented? We already have classes like CCmisMemMap, CmisFlatMemMap that try to portray the correct pages and fields available.

MemMap should represent only the layout and should not contain functionality

Please can you explain how the MemMap is containing functionality here?

Also, in joint mode we use both CMIS and ELSFP CMIS through the same interface, so this approach does not fit joint mode well. With this design, we would end up duplicating the EEPROM layout.

I think your comment might be implementation specific. Please can you explain how the EEPROM layout is duplicated in this proposal?

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My point is not that ELSFP uses all CMIS pages, but that it should still be modeled as an extension of the CMIS memory map, similar to how CCmisMemMap extends CmisMemMap.

I do not see inheriting unused pages as a problem by itself, since inheritance here is mainly about preserving the common structure, while the implementation can still use only the relevant subset.

Regarding duplication, what I meant is that in joint mode, if both CMIS and ELSFP are represented separately over the same interface, the lower/common pages could end up being defined twice. That said, as you noted, this is likely implementation-specific, and from our side I expect we will use a single memmap anyway.

Because of that, I still think it would be better to keep a consistent approach across both joint and separate modes, and model ELSFP through inheritance from the common CMIS map rather than through a selective-page definition.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps #2211 will help explain how these separate memory maps will be used.

Even though there are two separate memory maps, and their corresponding separate CmisApis for them, the CompositeSfp concept bundles them together to ensure that they appear to xcvrd as a single device.

In joint mode, ELSFP memory pages may be mapped at different vendor pages (00L at B0h, or some other vendor-custom page) but what remains the same is the content of the page - which is based on ELSFP-CMIS.
Our design offers the flexibility of defining a standards based ELSFP memory map that can be flexibly remapped as per requirements, allowing use of the same ElsfpCmisApi to access ELSFP information, no matter where a given ELSFP page is mapped in a combined/joint-mode memory map.

I think our design offers a consistent approach for both separate mode and joint mode.

For the sake of discussion, if ELSFP memory map were to extend CmisMemMap, it would also have Page 10h and 11h. Would your proposal be to use this ElsfpMemMap for joint mode and have:

  • Pages 00L, 00h, 01h, 02h, 10h, 11h reflect OE information,
  • then 1Ah and 1Bh show ELSFP information and
  • the vendor custom pages B0h-B3h show ELSFP information again?

This approach works just because ELSFP 1Ah and 1Bh are luckily mapped to the standard assigned pages for ELSFP, but a different vendor that maps ELSFP information at a different set of vendor custom pages will be unable to use the custom-defined non 1Ah/1Bh ELSFP register definitions without duplicating code.

Also, the ELSFP memory map used in such a way would not truly reflect an ELSFP and would instead be a mix of OE and ELSFP information.

.
.

class Device1MemMap(CmisMemMap):
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this part be handled within each vendor’s platform implementation?
I believe we would like to keep the creation of the API object and the MemMap object consistent with the existing create_xcvr_api function, rather than requiring a vendor-specific override for each implementation.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This simply enables easy remapping in vendor specific solutions. Its a benefit of the refactor that this HLD brings along with addition of ELS memory classes

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this approach may not fully align with the requirement to reuse the existing framework. It may be helpful to also consider the proposed HLD in PR #2291 for the DOM vendor-specific extension mechanism. With that approach, we can continue leveraging the existing common framework while also enabling vendor-specific functionality through the same framework.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The linked HLD only adds support for DOM vendor specific extensions and can coexist with this proposal.
It does not seem to allow for the ability to read static and identification information from the ELSFP memory map space that is not a part of DOM/VDM/CDB - things like serial number, vendor information, advertisements etc.

This is possible by having the ELSFP be its own memory map, and the vendor can set up the composite memory maps however they like. Any custom, non-standard DOM registers can still use the vendor specific extensions so that those registers are polled automatically alongside DOM.

In our view, an ELSFP is a standalone device, with its own memory map and not just a small set of vendor-specific register extensions. In joint mode, some or all pages of the ELSFP may be mapped to custom/vendor-specific pages. Our proposal allows for an easy way of accomplishing that without having to re-define all the ELSFP registers in a vendor-custom space.


### Functional Requirements

1. Support paging in CMIS memory maps
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please clarify the motivation behind this change? Why not keep the existing support? SONiC already supports different page IDs, so I don’t see the need to redesign the entire memory layout.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The idea is to cleanly separate the pages of the Memory map rather than having them as a massive dump. This happens to work well with the idea that each device does not support every CMIS page, for example ELSFP CMIS explicitly mentions that page 6 and 7h are not supported

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, the redesign is strictly internal. The existing memory maps - CmisFlatMemMap, CmisMemMap, CCmisMemMap are functionally identical with no required changes to their users. This also allows for easier composition of strictly applicable pages into future mem maps (Elsfp as an example) and makes them more maintainable by separating out fields into their respective pages rather than them being in one monolithic CmisMemMap class.

Comment on lines +100 to +103
### Non-Functional Requirements

1. Existing maps should remain functionally identical and cause no API changes
2. Easy remapping of pages to another page
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please clarify why we shouldn’t add all the non-vendor-specific fields to the existing map and use only the relevant ones from the API object?

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same reason there exist CmisFlatMemMap and CCmisMemMap. We tried to be true to the actually implemented pages, just like these memory maps applying to their respective module types.

bgallagher-nexthop pushed a commit to nexthop-ai/sonic-platform-common that referenced this pull request Apr 10, 2026
<!-- Provide a general summary of your changes in the Title above -->

#### Description
<!--
     Describe your changes in detail
-->

Creates ELSFP classes based on the ELSFP HLD here:
sonic-net/SONiC#2207
Depends on submitting CMIS refactor first:
nexthop-ai/private-sonic-platform-common#65

#### Motivation and Context
<!--
     Why is this change required? What problem does it solve?
     If this pull request closes/resolves an open Issue, make sure you
include the text "fixes #xxxx", "closes #xxxx" or "resolves #xxxx" here
-->

#### How Has This Been Tested?
<!--
     Please describe in detail how you tested your changes.
Include details of your testing environment, and the tests you ran to
     see how your change affects other areas of the code, etc.
-->

#### Which release branch to backport/forwardport

- [ ] 202505
- [ ] 202511

#### Additional Information (Optional)

---------

Signed-off-by: Abhi Singh <abhi@nexthop.ai>
@abhi-nexthop
Copy link
Copy Markdown
Author

Our Implementation PRs can currently be found here (will be moved to sonic-net once parent PR is merged upstream):
PR 1: Memory Map Refactor into pages
nexthop-ai/sonic-platform-common#1

PR 2: ELSFP pages and Memory Map
nexthop-ai/sonic-platform-common#2

Signed-off-by: Abhi Singh <abhi@nexthop.ai>
@mssonicbld
Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines
Copy link
Copy Markdown

No pipelines are associated with this pull request.

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

Successfully merging this pull request may close these issues.

8 participants