Hi
We spend more time debating whether to keep AGESA in the master branch than actually reviewing code to maintains it.
Here are some patches series I would like to be tested & reviewed:
- Agesa was never properly linked and relied on default linker behavior to append unmatched data. Here is the fix: https://review.coreboot.org/q/topic:AGESA_DATA - Use MRC cache for non volatile data https://review.coreboot.org/q/topic:AGESA_MRC_CACHE - Use CLFLUSH to make sure code hits DRAM and incidently avoid inconsistent MTRRs (bonus is compressed postcar stage): https://review.coreboot.org/q/topic:compress_postcar
Kind regards Arthur
Arthur, you are not making an argument that any vendor should release their source code as opensource. I agree that all of this code should be reviewed, but if we complain about code quality and lack of testing for open sourced code, but don't for closed source, that's an argument against any company opening their codebases.
Instead of phrasing it this way, maybe say something like, "Thanks AMD for releasing this, now community, let's get together and review and improve things."
Just my opinion, and I'm intentionally replying off list. But I'll say that I'm going to fight *very* hard to keep the AGESA codebases in coreboot for as long as there are people testing it. Doing otherwise is again, a disincentive to companies for opening their sourcecode.
Take care. Martin
May 17, 2022, 08:47 by arthur@aheymans.xyz:
Hi We spend more time debating whether to keep AGESA in the master branch than actually reviewing code to maintains it. Here are some patches series I would like to be tested & reviewed: Agesa was never properly linked and relied on default linker behavior to append unmatched data. Here is the fix: > https://review.coreboot.org/q/topic:AGESA_DATA Use MRC cache for non volatile data > https://review.coreboot.org/q/topic:AGESA_MRC_CACHE Use CLFLUSH to make sure code hits DRAM and incidently avoid inconsistent MTRRs (bonus is compressed postcar stage): > https://review.coreboot.org/q/topic:compress_postcar Kind regards Arthur
Hi
I have patches that improve those platforms and wrote code that should make some of the AGESA platforms easier to transition to newer soon to be mandated codepaths and I did so with past codepath mandates with both code and review.
My message about moving code from the master branch was more or less a tongue in cheek, I'm sorry that it was perceived differently. I'm not advocating for removal, just referring to past discussions. It is however a problem to get code changes tested. The (admittedly cynical) joke was just to get some attention to patches I would like to get reviewed as that work tends to be related to general codebase improvements.
Instead of phrasing it this way, maybe say something like, "Thanks AMD
for releasing this, now community, let's get together and review and improve things."
It's likely that AMD does not care about the AGESA platforms supported in coreboot, since they don't produce that hardware anymore? Just making assumptions here. Anyway, improving that code is my intention given that I write patches for it ;-)
Arthur
On Tue, May 17, 2022 at 7:40 PM Martin Roth gaumless@tutanota.com wrote:
Arthur, you are not making an argument that any vendor should release their source code as opensource. I agree that all of this code should be reviewed, but if we complain about code quality and lack of testing for open sourced code, but don't for closed source, that's an argument against any company opening their codebases.
Instead of phrasing it this way, maybe say something like, "Thanks AMD for releasing this, now community, let's get together and review and improve things."
Just my opinion, and I'm intentionally replying off list. But I'll say that I'm going to fight *very* hard to keep the AGESA codebases in coreboot for as long as there are people testing it. Doing otherwise is again, a disincentive to companies for opening their sourcecode.
Take care. Martin
May 17, 2022, 08:47 by arthur@aheymans.xyz:
Hi We spend more time debating whether to keep AGESA in the master branch
than actually reviewing code to maintains it.
Here are some patches series I would like to be tested & reviewed: Agesa was never properly linked and relied on default linker behavior to
append unmatched data. Here is the fix: > https://review.coreboot.org/q/topic:AGESA_DATA
Use MRC cache for non volatile data >
https://review.coreboot.org/q/topic:AGESA_MRC_CACHE
Use CLFLUSH to make sure code hits DRAM and incidently avoid
inconsistent MTRRs (bonus is compressed postcar stage): > https://review.coreboot.org/q/topic:compress_postcar
Kind regards Arthur
Sorry, I guess I'm just sensitive about the topic of removing code, since, as you say, it's been talked about a lot.
I obviously do appreciate your work on it.
Take care. Martin
May 17, 2022, 12:17 by arthur@aheymans.xyz:
Hi I have patches that improve those platforms and wrote code that should make some of the AGESA platforms easier to transition to newer soon to be mandated codepaths and I did so with past codepath mandates with both code and review.
My message about moving code from the master branch was more or less a tongue in cheek, I'm sorry that it was perceived differently. I'm not advocating for removal, just referring to past discussions. It is however a problem to get code changes tested. The (admittedly cynical) joke was just to get some attention to patches I would like to get reviewed as that work tends to be related to general codebase improvements.
Instead of phrasing it this way, maybe say something like, "Thanks AMD for releasing this, now community, let's get together and review and improve things."
It's likely that AMD does not care about the AGESA platforms supported in coreboot, since they don't produce that hardware anymore? Just making assumptions here. Anyway, improving that code is my intention given that I write patches for it ;-)
Arthur
On Tue, May 17, 2022 at 7:40 PM Martin Roth <> gaumless@tutanota.com> > wrote:
Arthur, you are not making an argument that any vendor should release their source code as opensource. I agree that all of this code should be reviewed, but if we complain about code quality and lack of testing for open sourced code, but don't for closed source, that's an argument against any company opening their codebases.
Instead of phrasing it this way, maybe say something like, "Thanks AMD for releasing this, now community, let's get together and review and improve things."
Just my opinion, and I'm intentionally replying off list. But I'll say that I'm going to fight *very* hard to keep the AGESA codebases in coreboot for as long as there are people testing it. Doing otherwise is again, a disincentive to companies for opening their sourcecode.
Take care. Martin
May 17, 2022, 08:47 by >> arthur@aheymans.xyz>> :
Hi We spend more time debating whether to keep AGESA in the master branch than actually reviewing code to maintains it. Here are some patches series I would like to be tested & reviewed: Agesa was never properly linked and relied on default linker behavior to append unmatched data. Here is the fix: > >> https://review.coreboot.org/q/topic:AGESA_DATA Use MRC cache for non volatile data > >> https://review.coreboot.org/q/topic:AGESA_MRC_CACHE Use CLFLUSH to make sure code hits DRAM and incidently avoid inconsistent MTRRs (bonus is compressed postcar stage): > >> https://review.coreboot.org/q/topic:compress_postcar Kind regards Arthur
Hi Arthur,
On 17.05.2022 16:47, Arthur Heymans wrote:
Hi
We spend more time debating whether to keep AGESA in the master branch than actually reviewing code to maintains it.
Here are some patches series I would like to be tested & reviewed:
- Agesa was never properly linked and relied on default linker behavior to append unmatched data. Here is the fix: https://review.coreboot.org/q/topic:AGESA_DATA https://review.coreboot.org/q/topic:AGESA_DATA
- Use MRC cache for non volatile data https://review.coreboot.org/q/topic:AGESA_MRC_CACHE https://review.coreboot.org/q/topic:AGESA_MRC_CACHE
- Use CLFLUSH to make sure code hits DRAM and incidently avoid inconsistent MTRRs (bonus is compressed postcar stage): https://review.coreboot.org/q/topic:compress_postcar https://review.coreboot.org/q/topic:compress_postcar
Thank you for your efforts to make this code better. We (3mdeb) will take a look at these patches in the upcoming days and provide feedback and review.
Kind regards Arthur
coreboot mailing list -- coreboot@coreboot.org To unsubscribe send an email to coreboot-leave@coreboot.org
Best regards,
-
Arthur Heymans -
Martin Roth -
Michał Żygowski