On Mon, Jul 12, 2010 at 08:47:47PM +0900, Isaku Yamahata wrote:
pam register offset is north bridge specific. So determine the offset based on found north bridge.
Is it really just the offset that is north bridge specific? I thought the entire process was very north bridge specific.
If so, I don't think it makes sense to pass back the pam0 register - instead the north bridge specific code should do the necessary work (using helper functions if possible).
I have the same concern with part 3 and 4 of this series.
-Kevin
On Mon, Jul 12, 2010 at 08:59:14PM -0400, Kevin O'Connor wrote:
On Mon, Jul 12, 2010 at 08:47:47PM +0900, Isaku Yamahata wrote:
pam register offset is north bridge specific. So determine the offset based on found north bridge.
Is it really just the offset that is north bridge specific? I thought the entire process was very north bridge specific.
If so, I don't think it makes sense to pass back the pam0 register - instead the north bridge specific code should do the necessary work (using helper functions if possible).
I have the same concern with part 3 and 4 of this series.
I440fx and Q35 (all Intel chipset?) are similar in registers which seabios programs, so I choice to abstract it at register offset level. I don't expect that other vendor's chipset support is wanted.
If you want more high level abstract, I'll respin the patch set.
Isaku Yamahata wrote:
I choice to abstract it at register offset level.
..
If you want more high level abstract, I'll respin the patch set.
If abstracted on a higher level then it will quite likely duplicate a lot of infrastructure which is already in coreboot.
//Peter
On Wed, Jul 14, 2010 at 02:03:02AM +0200, Peter Stuge wrote:
Isaku Yamahata wrote:
I choice to abstract it at register offset level.
..
If you want more high level abstract, I'll respin the patch set.
If abstracted on a higher level then it will quite likely duplicate a lot of infrastructure which is already in coreboot.
Hi Peter,
Yes - there is overlap between SeaBIOS' hardware setup and coreboot, and these patches increase that overlap.
It would be possible to extend the hardware setup in coreboot, and then champion a coreboot+seabios solution for qemu. To do so would require supporting all the qemu specific stuff in coreboot - it's the stuff in: src/acpi-dsdt.dsl, src/acpi.c, src/dev-i440fx.c, src/mptable.c, src/mtrr.c, src/paravirt.c, src/pciinit.c, src/shadow.c src/smbios.c, src/smm.c, src/smp.c. Coreboot and this code do many similar things, but coreboot isn't currently a replacement. For example, coreboot would need to handle the fw_cfg and cmos reading to get config settings. Qemu isn't just one machine, it's a whole class of machines. Coreboot would need to be able to support them dynamically.
If there's a desire to do this, I wont object. However, someone will need to step forward and start enhancing coreboot. In the meantime, I wont hold up on patches to SeaBIOS.
-Kevin
On Tue, Jul 13, 2010 at 06:45:00PM +0900, Isaku Yamahata wrote:
On Mon, Jul 12, 2010 at 08:59:14PM -0400, Kevin O'Connor wrote:
On Mon, Jul 12, 2010 at 08:47:47PM +0900, Isaku Yamahata wrote:
pam register offset is north bridge specific. So determine the offset based on found north bridge.
Is it really just the offset that is north bridge specific? I thought the entire process was very north bridge specific.
If so, I don't think it makes sense to pass back the pam0 register - instead the north bridge specific code should do the necessary work (using helper functions if possible).
I have the same concern with part 3 and 4 of this series.
I440fx and Q35 (all Intel chipset?) are similar in registers which seabios programs, so I choice to abstract it at register offset level. I don't expect that other vendor's chipset support is wanted.
Although it isn't currently used, the memory locking support is useful on real machines too. I'd prefer a solution that would work on both qemu and real machines.
It's minor for part 2 of the series, but I found part 3/4 to be hard to follow due to the way the flow of code jumps between machine specific code in dev-i440fx.c and the smm code in smm.c.
If you want more high level abstract, I'll respin the patch set.
I've been meaning to look through the full series of changes in your repo, but have not yet had a chance to do so. I hope to get to that in the next few days. Sorry for the delay.
-Kevin
-
Isaku Yamahata -
Kevin O'Connor -
Peter Stuge