February 2015 - coreboot - mail.coreboot.org

Re: [coreboot] Unifying IO accessor macros
by Alexandru Gagniuc Feb. 18, 2015

Feb. 18, 2015

On Tuesday, February 17, 2015 03:12:35 PM Julius Werner wrote: > I'd like to propose an API change/cleanup for a long-standing problem > with architecture-independent code that people have hit and privately > discussed/cursed multiple times already, but no one really had time to > make the big change/fix yet. (Some earlier discussion among Chromium > people about this is available at http://crbug.com/451388 ) > > For doing MMIO accesses to registers, x86 code has traditionally used > a function with a signature like this: > > void write32(unsigned long addr, u32 value); // equivalents for > read32(), write8(), etc. > > While ARM (and based on that, ARM64) has been brought up with both of these: > > void write32(u32 value, void *addr); > void writel(u32 value, void *addr); > > There's two problems with this: first, the order of arguments (value > and address) is switched around between the two architectures, meaning > it is currently impossible to write any architecture-independent MMIO > code (which blocks important features like generalizing > src/lib/uart8250mem.c). Second, the x86 version is not type safe (it > takes addresses as integers instead of pointers), making it extremely > easy to accidentally swap address and value (especially due to all > this architecture confusion about which order is correct). Changing > the addresses to a pointer type will generate obvious compiler errors > for most of those screw-ups, which I think is a good thing that x86 > could benefit from as well. The x86 was recently fixed to take in void *. The order of arguments was discussed before, and the agreement is to use write*(addr, val); This has the advantage of being consistent with other accessor functions, such as PCI, and follows the logical C ordering of the assignment operator. destination = value; > > I want to unify all architectures under a single, type safe version of > these macros that will remove the confusion and allow writing > architecture-independent MMIO code in the future. For this, I plan to: > > 1. Switch all existing uses of write32() (and friends) in ARM(64) code > to writel(). Please don't. Always use functions that indicate bitness. > 2. Remove write32() from all architectures but x86. See above point. > 3. Add writel(u32 value, void *addr) which is compatible with other > architectures to x86. What is writel? Is it 32 bits? 64 bits? This C70's mentality has to go away. We've worked pretty hard to go into the 21st century, with stdint types, and other "duh! This just makes sense" changes. > 4. Pronounce write32(unsigned long addr, u32 value) deprecated for x86 > and slowly phase it out over time (at least from common/recent code). > Already done. It's void * for the addr arg... just where we want it. And it indicates bitness, just like we want it. > After this we'll be converged on writel(u32 value, void *addr) as the Please don't use AT&T-centric syntax. It's inconsistent with every other accessor in coreboot. > one and only way to do it across all architectures going forward. I > know that there are probably people who prefer the name write32() > aesthetically, or who have a strong preference for the It's part of the critical system guidelines to use types that indicate bitness. > first-address-then-value parameter order. I personally don't really Again, this matches the logical order of the C programming language. Maybe we want to switch coreboot to assembly? JK, JK. > like the way writel() looks myself, but at the end of the day it > doesn't make a huge difference. I think this is the best solution But it does. You see, bitness is not obviuous from the likes of long, long long, int short, word, dword, etc. It's also compiler dependent, just adding to the overall confusion. Then what do you do for write64()... writell()? That's easily mistakeable for writel(). > because it has strong practical advantages and will allow us to solve > the issue with the least amount of changes: most ARM(64) code already > uses writel(), libpayload uses writel() across all architectures, and All it takes is a trivial semantic patch or short sed line to switch this. This is hardly any effort at all when compared to the efforts of integrating code from other codebases. > it's the standard for U-Boot and a majority of the Linux kernel which We got it right. And consistent. Using codebases that are either as old as linux, or got it wrong, like uboot, is really not a fair comparison. > are popular import sources. Also, the amount of existing x86 code is > probably too large to ever change (and appropriately test) it all... > so it's useful to pick a solution that can leave the old untyped > write32() in place on x86 as a deprecated fallback. Huh? It's almost trivial to do with a semantic patch. > > Since the changes required for this would mostly affect ARM(64) SoCs > and boards, which are currently much more developed (and in flux) in > the Chromium tree, I'm planning to only change this there for now and > wait until it gets merged into coreboot mainline in the right order as > part of the current upstreaming process. I still wanted to float the > idea here first to give everyone a chance to chime in before it's all > said and done. You're free to continue to provide writel() and friends as a fallback for ARM code picked from other codebases, but please consider that it's wrong. Alex

2 1

Unifying IO accessor macros
by Julius Werner Feb. 17, 2015

Feb. 17, 2015

I'd like to propose an API change/cleanup for a long-standing problem with architecture-independent code that people have hit and privately discussed/cursed multiple times already, but no one really had time to make the big change/fix yet. (Some earlier discussion among Chromium people about this is available at http://crbug.com/451388 ) For doing MMIO accesses to registers, x86 code has traditionally used a function with a signature like this: void write32(unsigned long addr, u32 value); // equivalents for read32(), write8(), etc. While ARM (and based on that, ARM64) has been brought up with both of these: void write32(u32 value, void *addr); void writel(u32 value, void *addr); There's two problems with this: first, the order of arguments (value and address) is switched around between the two architectures, meaning it is currently impossible to write any architecture-independent MMIO code (which blocks important features like generalizing src/lib/uart8250mem.c). Second, the x86 version is not type safe (it takes addresses as integers instead of pointers), making it extremely easy to accidentally swap address and value (especially due to all this architecture confusion about which order is correct). Changing the addresses to a pointer type will generate obvious compiler errors for most of those screw-ups, which I think is a good thing that x86 could benefit from as well. I want to unify all architectures under a single, type safe version of these macros that will remove the confusion and allow writing architecture-independent MMIO code in the future. For this, I plan to: 1. Switch all existing uses of write32() (and friends) in ARM(64) code to writel(). 2. Remove write32() from all architectures but x86. 3. Add writel(u32 value, void *addr) which is compatible with other architectures to x86. 4. Pronounce write32(unsigned long addr, u32 value) deprecated for x86 and slowly phase it out over time (at least from common/recent code). After this we'll be converged on writel(u32 value, void *addr) as the one and only way to do it across all architectures going forward. I know that there are probably people who prefer the name write32() aesthetically, or who have a strong preference for the first-address-then-value parameter order. I personally don't really like the way writel() looks myself, but at the end of the day it doesn't make a huge difference. I think this is the best solution because it has strong practical advantages and will allow us to solve the issue with the least amount of changes: most ARM(64) code already uses writel(), libpayload uses writel() across all architectures, and it's the standard for U-Boot and a majority of the Linux kernel which are popular import sources. Also, the amount of existing x86 code is probably too large to ever change (and appropriately test) it all... so it's useful to pick a solution that can leave the old untyped write32() in place on x86 as a deprecated fallback. Since the changes required for this would mostly affect ARM(64) SoCs and boards, which are currently much more developed (and in flux) in the Chromium tree, I'm planning to only change this there for now and wait until it gets merged into coreboot mainline in the right order as part of the current upstreaming process. I still wanted to float the idea here first to give everyone a chance to chime in before it's all said and done.

1 0

New Defects reported by Coverity Scan for coreboot
by scan-admin＠coverity.com Feb. 17, 2015

Feb. 17, 2015

Hi, Please find the latest report on new defect(s) introduced to coreboot found with Coverity Scan. 8 new defect(s) introduced to coreboot found with Coverity Scan. 521 defect(s), reported by Coverity Scan earlier, were marked fixed in the recent build analyzed by Coverity Scan. New defect(s) Reported-by: Coverity Scan Showing 8 of 8 defect(s) ** CID 1270744: Control flow issues (NO_EFFECT) /src/drivers/pc80/mc146818rtc.c: 270 in set_cmos_value() ________________________________________________________________________________________________________ *** CID 1270744: Control flow issues (NO_EFFECT) /src/drivers/pc80/mc146818rtc.c: 270 in set_cmos_value() 264 } else { /* more that one byte so transfer the whole bytes */ 265 if (byte_bit || length % 8) 266 return CB_ERR_ARG; 267 268 for (i = 0; length; i++, length -= 8, byte++) 269 cmos_write(ret[i], byte); >>> CID 1270744: Control flow issues (NO_EFFECT) >>> This greater-than-or-equal-to-zero comparison of an unsigned value is always true. "byte >= 0UL". 270 if (byte >= LB_CKS_RANGE_START && 271 byte <= LB_CKS_RANGE_END) 272 chksum_update_needed = 1; 273 } 274 275 if (chksum_update_needed) { ________________________________________________________________________________________________________ To view the defects in Coverity Scan visit, https://scan.coverity.com/projects/1016?tab=overview To manage Coverity Scan email notifications for "coreboot(a)coreboot.org", click https://scan.coverity.com/subscriptions/edit?email=coreboot%40coreboot.org&… .

1 0

Re: [coreboot] Bay trail and SATA legacy IDE mode
by Alexandru Gagniuc Feb. 17, 2015

Feb. 17, 2015

On Monday, February 09, 2015 07:05:04 AM Stojsavljevic, Zoran wrote: > But still... I have some limited knowledge of Coreboot from 15 months ago, > and I am trying to understand if Coreboot x86 code has some influence on > the SATA IDE mode. From what I am reading... There is a possibility! Check src/soc/intel/baytrail/sata.c on how the registers are configured based on devicetree.cb if (!config->sata_ahci) { /* Set legacy or native decoding mode */ if (config->ide_legacy_combined) { > The silicon is BYT SoC, ATOM. The boot time to OS bootloader is around 300 > ms. > My best understanding how this enumeration works for SATA is that all SATA > are put in default AHCI mode. But some people (As Berth) must have SATA > interfaces in IDE mode. register "sata_ahci" = "0" register "ide_legacy_combined" = "1" Alex

2 1

[RFH] ACPI testers needed!
by Timothy Pearson Feb. 14, 2015

Feb. 14, 2015

All, I have recently updated the coreboot ACPI autogeneration code to generate valid processor _PSS objects when more than 10 cores are installed in one system. Unfortunately this required a rather large update to multiple boards in order to change the _PSS object name from CPUn to CPnn. We would like some feedback to make sure this update didn't break anything before committing. The patch is available at: http://review.coreboot.org/8422 Thanks! -- Timothy Pearson Raptor Engineering +1 (415) 727-8645 http://www.raptorengineeringinc.com

3 3

Re: [coreboot] GSOC 2015 Preperations
by Alexander Couzens Feb. 14, 2015

Feb. 14, 2015

Hi Marc, I would like to attend as a student this year. I'm open for most of the ideas at the page. Here are some of my ideas. - porting new mainboards (hp micro n54L and x121e atm) - porting new arm platforms - refactoring amd code - create an opensource firmware for some Thinkpad (H8S based) - building a basic testing system (looking for a x60 or other supported_board) - add usb debug support to SerialICE Best, lynxis On Sat, 14 Feb 2015 00:05:28 +0000 Marc Jones <marcj303(a)gmail.com> wrote: > Hi Everyone, > > It is time for coreboot to apply for GSOC 2015. We could use your help. We > need student project ideas, mentors, and qualified students! > > Please update the wiki page with project ideas. > http://www.coreboot.org/Project_Ideas > > Let me know if you would like to be a student mentor this summer. You need > to signup with Google and then I can add you to the project. > > Start informing prospective GSOC students about coreboot. Please be helpful > to new faces in IRC and direct them to me or Patrick if they have questions. > > Regards, > Marc -- Alexander Couzens mail: lynxis(a)fe80.eu jabber: lynxis(a)jabber.ccc.de mobile: +4915123277221

4 3

GSOC 2015 Preperations
by Marc Jones Feb. 14, 2015

Feb. 14, 2015

Hi Everyone, It is time for coreboot to apply for GSOC 2015. We could use your help. We need student project ideas, mentors, and qualified students! Please update the wiki page with project ideas. http://www.coreboot.org/Project_Ideas Let me know if you would like to be a student mentor this summer. You need to signup with Google and then I can add you to the project. Start informing prospective GSOC students about coreboot. Please be helpful to new faces in IRC and direct them to me or Patrick if they have questions. Regards, Marc

1 0

Re: [coreboot] dell sputnik
by Peter Stuge Feb. 12, 2015

Feb. 12, 2015

benny windolph wrote: > Its absolutely fine if the team wont do it. I am not a developer, i > would rather donate or something. But its okay. Donations are difficult, but still much appreciated! The scarce resource is (as always?) development time, unfortunately neither money nor hardware. > Thanks anyway and have a nice day Thanks - the same to you! //Peter

1 0

Re: [coreboot] dell sputnik
by Peter Stuge Feb. 12, 2015

Feb. 12, 2015

benny windolph wrote: > Thanks for the clarification, but is it possible that the coreboot-team > will put this device on its agenda to bring coreboot to the dell sputnik? It's not likely. If you care about something in open source you have to do the work. There's nothing wrong with asking someone else to do it, but if they don't then that's fine too. //Peter

2 1

dell sputnik
by Benny Feb. 12, 2015

Feb. 12, 2015

Hey everyone, is there a chance coreboot comes to the Dell Sputnik (XPS13, L322X1Y1)? Thanks, Benny

4 3