Re: [coreboot] [commit] r6626 - trunk/src/mainboard/asrock/e350m1
Stefan Reinauer wrote: ]> + // all cores: set pstate 0 (1600 MHz) early to save a few ms of boot ]time ]> + __writemsr (0xc0010062, 0); ]> + ] ]why not use writemsr instead of __writemsr? Hello Stefan, The wrmsr function is certainly OK. I am in the habit us using the Intel/MS functions because they are built into the Microsoft compiler and therefore portable between all UEFI vendor code bases and any other code that uses the MS complier. But portability with UEFI is not the reason I still sometimes use these function types. Truthfully, the coreboot wrmsr function seems awkward to me. With the Intel/MS __writemsr function, any 64-bit value can be written with a single statement. Coreboot code using wrmsr is often taking 4 statements to do the same thing. While there is probably little difference in generated code, using the coreboot wrmsr function does require more source code typing. I also find it less readable. For example, // set bit 46 of NB_CFG_MSR using wrmsr function: msr_t msr; msr = rdmsr(NB_CFG_MSR); msr.hi |= (1 << (46 - 32)); wrmsr(NB_CFG_MSR, msr); // Making msr_t a union that adds a uint64_t would allow: msr_t msr; msr = rdmsr(NB_CFG_MSR); msr.all |= 1 << 46; wrmsr(NB_CFG_MSR, msr); // The Intel/MS style uint64_t prototypes allow: __writemsr (__readmsr (NB_CFG_MSR) | 1 << 46); A question I have wanted to ask for a while is why coreboot doesn't use 64 bit integers in situations like this? Thanks, Scott -- coreboot mailing list: coreboot@coreboot.org http://www.coreboot.org/mailman/listinfo/coreboot
Re: [coreboot] [commit] r6626 - trunk/src/mainboard/asrock/e350m1
* repository service [110604 17:44]: > Author: stuge > Date: Sat Jun 4 17:44:54 2011 > New Revision: 6626 > URL: https://tracker.coreboot.org/trac/coreboot/changeset/6626 > > Log: > Port persimmon r6583 to e350m1: pstate 0 early > > Switch processor cores to pstate 0 early to reduce boot time. > > Signed-off-by: Marshall Buschman > Acked-by: Peter Stuge > > Modified: >trunk/src/mainboard/asrock/e350m1/romstage.c > > Modified: trunk/src/mainboard/asrock/e350m1/romstage.c > == > --- trunk/src/mainboard/asrock/e350m1/romstage.c Sat Jun 4 17:44:31 > 2011(r6625) > +++ trunk/src/mainboard/asrock/e350m1/romstage.c Sat Jun 4 17:44:54 > 2011(r6626) > @@ -47,6 +47,9 @@ >u32 val; >u8 reg8; > > + // all cores: set pstate 0 (1600 MHz) early to save a few ms of boot time > + __writemsr (0xc0010062, 0); > + why not use writemsr instead of __writemsr? >// early enable of SPI 33 MHz fast mode read >if (boot_cpu()) > { > > -- > coreboot mailing list: coreboot@coreboot.org > http://www.coreboot.org/mailman/listinfo/coreboot > -- coreboot mailing list: coreboot@coreboot.org http://www.coreboot.org/mailman/listinfo/coreboot
[coreboot] [commit] r6626 - trunk/src/mainboard/asrock/e350m1
Author: stuge Date: Sat Jun 4 17:44:54 2011 New Revision: 6626 URL: https://tracker.coreboot.org/trac/coreboot/changeset/6626 Log: Port persimmon r6583 to e350m1: pstate 0 early Switch processor cores to pstate 0 early to reduce boot time. Signed-off-by: Marshall Buschman Acked-by: Peter Stuge Modified: trunk/src/mainboard/asrock/e350m1/romstage.c Modified: trunk/src/mainboard/asrock/e350m1/romstage.c == --- trunk/src/mainboard/asrock/e350m1/romstage.cSat Jun 4 17:44:31 2011(r6625) +++ trunk/src/mainboard/asrock/e350m1/romstage.cSat Jun 4 17:44:54 2011(r6626) @@ -47,6 +47,9 @@ u32 val; u8 reg8; + // all cores: set pstate 0 (1600 MHz) early to save a few ms of boot time + __writemsr (0xc0010062, 0); + // early enable of SPI 33 MHz fast mode read if (boot_cpu()) { -- coreboot mailing list: coreboot@coreboot.org http://www.coreboot.org/mailman/listinfo/coreboot