Update the kernel patch to this tag from Aspeed Kernel Repo - kernel-6.12 v00.07.02

[chander-nexthop]

Have generated the kernel patch to be applied from this tag - AspeedTech-BMC/linux@7fa04c4

[mssonicbld]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[chander-nexthop]

The patch file seems to have deleted IRQ_PENSANDO. Will investigate and fix!

[mssonicbld]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[paulmenzel]

Please document, where the Aspeed kernel repository is. The diff is too big to render in the GitHub Web interface, so no review is possible. No idea how to handle it.

I hope the Aspeed repository has small commits. It’d might be good to add this commit series instead of one big patch file.

Lastly, what is Aspeed’s plan on getting this upstream?

[chander-nexthop]

Please document, where the Aspeed kernel repository is. The diff is too big to render in the GitHub Web interface, so no review is possible. No idea how to handle it.

I hope the Aspeed repository has small commits. It’d might be good to add this commit series instead of one big patch file.

Lastly, what is Aspeed’s plan on getting this upstream?

The link to aspeed repository is given in the first (top most) comment. You need to download the diff to review. Aspeed folks do not give the list of commit id's. IIUC, They don't have plans to support sonic kernel and hence this large diff.

[paulmenzel]

The link to aspeed repository is given in the first (top most) comment.

Thank you. I missed that. Please add it to the patch file, as the merge/pull request description is not copied to the git repository to my knowledge.

You need to download the diff to review.

Understood. I do not have time to review it.

Aspeed folks do not give the list of commit id's. IIUC, They don't have plans to support sonic kernel and hence this large diff.

First, please add more information then on what revision the Aspeed kernel is based. Besides that, who is going to maintain the patch, when SONiC needs to update the Linux kernel and the diff does not apply. Who is going to pay for this maintenance hell?

Let’s see what the maintainers say, but in my opinion, the current approach does not work, and Aspeed and Nexthop(?) need to do the work to bring this upstream and makes this digestible.

[chander-nexthop]

The link to aspeed repository is given in the first (top most) comment.

Thank you. I missed that. Please add it to the patch file, as the merge/pull request description is not copied to the git repository to my knowledge.

You need to download the diff to review.

Understood. I do not have time to review it.

Aspeed folks do not give the list of commit id's. IIUC, They don't have plans to support sonic kernel and hence this large diff.

First, please add more information then on what revision the Aspeed kernel is based. Besides that, who is going to maintain the patch, when SONiC needs to update the Linux kernel and the diff does not apply. Who is going to pay for this maintenance hell?

Let’s see what the maintainers say, but in my opinion, the current approach does not work, and Aspeed and Nexthop(?) need to do the work to bring this upstream and makes this digestible.

This patch replaces a earlier merge that is already there in the SONIC kernel and hence the brevity in the description/commit message. Aspeed is a dominant vendor for BMC SoC's and many vendors are looking to add BMC's to their switch designs, especially the liquid cooled ones.

From Nexthop side we are still working with aspeed on this. The SONIC Working Group for BMC debated this for some time and decided to take this middle ground to allow this patch for the time till aspeed merges their kernel drivers to the main repository. The main considerations were

1. The patch is almost all new aspeed code and diffs are mainly for Makefile and KConfig files that enable options
2. Not having support for SONiC implies all switch vendors will be forced to add support for a different OS, with its own build system, tools and software just for this component, to their workflow.
3. It would be a ideal thing to have a community driven approach to BMC enabled platform bringup and management as switches adopt BMC's, rather than leaving this unaddressed. Solving this problem later, after each vendor adopts their own software workflow, is a worser problem to have than this temporary pain.

[paulmenzel]

Sorry, I looked at merge/pull request #522 and obviously, it wasn’t properly reviewed. Where can I find the community decision? Where is it documented?

I strongly oppose this direction. If vendors want this SONiC support, they should pressure Aspeed to do the right thing and invest into more upstreaming and easy ways to backport stuff.

This cannot be reviewed, and you did not put in the effort to make review easy by documented how you created the diff, and what changed between the version. How can the project make sure, that no backdoor is slipped in?

[saiarcot895]

@paulmenzel I agree that it's not great that all Aspeed changes are in one giant patch. However, there's not much of a better way in the short term:

• Trying to have individual patches for each change they made is going to result in too many files, because their repo consists of many small changes for multiple chips.
• Trying to logically group those changes requires support from Aspeed, which might be better spent upstreaming their changes (discussed later)
• Moving this to be out-of-tree kernel modules isn't easy, because there are some changes to existing kernel code (including struct changes), and uses header files not exported for out-of-tree kernel modules.

Additionally, I agree that it's very difficult to impossible to make sure that there are no backdoors. However, what I can make sure is that the changes that are coming in don't affect other kernel modules/code, that the code being changed is only aspeed code. Additionally, because this is only on arm64, this has the small advantage that this code (at least the portions that are not in common header files or other shared parts of the kernel) will be compiled only for boards using the Aspeed chip. (This is a bit of a side effect for SONiC in that unlike amd64 which has everything necessary enabled in a single kernel build, there are multiple different kernels being compiled for both armhf and arm64.)

All of the above led to where we are today: a single giant patch adding support for this chip.

As for upstreaming their code, I do see commits going in from Aspeed that includes some of the changes present in this patch, such as this, this, this, and this. However, all of those are going to be in kernel versions newer than whatever is going to be used for Debian Trixie, and none of those would be backported to the 6.12 kernel, so it is an improvement, but it is going to take time, and there is still a lot of code that still needs to be upstreamed.