Skip to contentSkip to conversation

Few questions about porting Linux to SMP86xx boards

DISCUSSION Oleg Kolosov 5 participants 12 messages Feb 1, 2015 – Feb 5, 2015 MIPS

lore
Share & tools
lore
#0 lore raw

Hello MIPS gurus!

I'm adding support for Sigma Designs SMP8652/SMP8654 (Tango3 family, MIPS 24kf CPU) to newer kernel. I've selectively adapted patches from 2.6.32.15 (the latest officially available for us) to the latest mips 3.18 stable branch and things seem to work (it boots, runs simple test programs), but there are few questions which I was not able to resolve yet with my limited experience:

1. They (Sigma Designs) have overridden __fast_iob which is identical to the default one except for one line:

    : "m" (*(int *)CKSEG1)

is replaced with:

    : "m" (*(int *)(CKSEG1+CPU_REMAP_SPACE))

where CPU_REMAP_SPACE=0x4000000 is a compile time constant for Tango3 and also called KERNEL_START_ADDRESS in Makefiles. The same value is also written to ebase:

ebase = KSEG0ADDR(CPU_REMAP_SPACE);
write_c0_ebase(ebase);

in traps.c:per_cpu_trap_init()

while writing ebase is really necessary for the kernel to boot, I've not found any negative effects of not applying __fast_iob patch. What is it supposed to do?

2. In io.h they have added explicit __sync() to the end of pfx##write##bwlq and pfx##out##bwlq##p. Is this really necessary? I've not yet found any adverse effects of not doing so. Maybe this was a workaround for some old kernel issue which was fixed since then?

3. In c-r4k.c:r4k_cache_init() they assign:

flush_icache_page = r4k_flush_icache_page;

where:

static void r4k_flush_icache_page(struct vm_area_struct *vma,
struct page *page)

{
    r4k_flush_icache_range((unsigned long)page_address(page),
        (unsigned long)page_address(page) + PAGE_SIZE);
}

thus overriding default empty flush_icache_page.

By digging the archives I've found some talks about removing flush_icache_page. Various sources says it should not be necessary. Maybe this is board-specific workaround?

--------------

I would really appreciate some explanations on what these changes supposed to solve or pointers to some background info to better understand what I'm actually doing. The main concern is that not properly applying the changes might break something subtly, and, on the contrary, applying everything might conflict with some fixes in newer kernel and break something subtly - the sadness.

Thanks a lot!

Signature · 2 lines
Regards, Oleg
Art System
#1 lore raw
Quoted 22 lines · Oleg Kolosov

Hello MIPS gurus!

I'm adding support for Sigma Designs SMP8652/SMP8654 (Tango3 family, MIPS 24kf CPU) to newer kernel. I've selectively adapted patches from 2.6.32.15 (the latest officially available for us) to the latest mips 3.18 stable branch and things seem to work (it boots, runs simple test programs), but there are few questions which I was not able to resolve yet with my limited experience:

1. They (Sigma Designs) have overridden __fast_iob which is identical to the default one except for one line:

    : "m" (*(int *)CKSEG1)

is replaced with:

    : "m" (*(int *)(CKSEG1+CPU_REMAP_SPACE))

where CPU_REMAP_SPACE=0x4000000 is a compile time constant for Tango3 and also called KERNEL_START_ADDRESS in Makefiles. The same value is also written to ebase:

Quoted 8 lines

ebase = KSEG0ADDR(CPU_REMAP_SPACE);
write_c0_ebase(ebase);

in traps.c:per_cpu_trap_init()

while writing ebase is really necessary for the kernel to boot, I've not found any negative effects of not applying __fast_iob patch. What is it supposed to do?

I do not have any direct experience with these SoCs, but you might want to look at the memory map to try to figure this one out. i.e. if __fast_iob() normally performs an uncached dummy read from the first word of physical memory, does the address need to be adjusted by 64MB on the Sigma chips because system memory (or the memory allocated to the Linux application processor) starts at PA 0x0400_0000 instead of 0x0000_0000?

That theory would also explain why the exception vectors were adjusted by the same offset.

BTW, you can override ebase from the platform code, as was done in arch/mips/kernel/smp-bmips.c. It probably isn't necessary to change the common per_cpu_trap_init() code (but it may have been necessary way back in 2.6.32).

Quoted 4 lines

2. In io.h they have added explicit __sync() to the end of pfx##write##bwlq and pfx##out##bwlq##p. Is this really necessary? I've not yet found any adverse effects of not doing so. Maybe this was a workaround for some old kernel issue which was fixed since then?

Adding a barrier in writel(), as was done on ARM, might have something to do with the SoC's busing or peripherals. Sometimes there are chip bugs that cause MMIO transaction ordering to break in unexpected ways. Or it could be there to compensate for missing barriers or bad assumptions in a driver somewhere.

For #2 and #3, it is likely that somebody at Sigma could find a bug report or changelog explaining why it was added. In my experience these sorts of changes are usually made to work around subtle problems discovered in testing or production. Figuring out the exact problem that inspired the patch can be difficult without insider knowledge, unless you happened to run across the same failure.

Quoted 4 lines

3. In c-r4k.c:r4k_cache_init() they assign:

flush_icache_page = r4k_flush_icache_page;

Quoted 8 lines

where:

static void r4k_flush_icache_page(struct vm_area_struct *vma,
struct page *page)

{
    r4k_flush_icache_range((unsigned long)page_address(page),
        (unsigned long)page_address(page) + PAGE_SIZE);
}

Hmm, this might not play nice with HIGHMEM. If it does, it would be helpful to include a comment explaining why.

#2 lore raw
Quoted 40 lines · Kevin Cernekee
Quoted 22 lines · Oleg Kolosov

Hello MIPS gurus!

I'm adding support for Sigma Designs SMP8652/SMP8654 (Tango3 family, MIPS 24kf CPU) to newer kernel. I've selectively adapted patches from 2.6.32.15 (the latest officially available for us) to the latest mips 3.18 stable branch and things seem to work (it boots, runs simple test programs), but there are few questions which I was not able to resolve yet with my limited experience:

1. They (Sigma Designs) have overridden __fast_iob which is identical to the default one except for one line:

    : "m" (*(int *)CKSEG1)

is replaced with:

    : "m" (*(int *)(CKSEG1+CPU_REMAP_SPACE))

where CPU_REMAP_SPACE=0x4000000 is a compile time constant for Tango3 and also called KERNEL_START_ADDRESS in Makefiles. The same value is also written to ebase:

Quoted 8 lines

ebase = KSEG0ADDR(CPU_REMAP_SPACE);
write_c0_ebase(ebase);

in traps.c:per_cpu_trap_init()

while writing ebase is really necessary for the kernel to boot, I've not found any negative effects of not applying __fast_iob patch. What is it supposed to do?

I do not have any direct experience with these SoCs, but you might want to look at the memory map to try to figure this one out. i.e. if __fast_iob() normally performs an uncached dummy read from the first word of physical memory, does the address need to be adjusted by 64MB on the Sigma chips because system memory (or the memory allocated to the Linux application processor) starts at PA 0x0400_0000 instead of 0x0000_0000?

Quoted 2 lines

That theory would also explain why the exception vectors were adjusted by the same offset.

The 86xx has two DRAM controllers mapped with 1GB windows at 0x8000_0000 and 0xc000_0000, and also with 256MB windows at 0x1000_0000 and 0x2000_0000. To complicate matters, CPU physical addresses starting at 0x04000000 are subjected to a set of remapping registers translating 6 blocks of 64MB to an arbitrary (64MB-aligned) bus address (not that these addresses overlap with the low mappings of the DRAM controllers). The obvious way to support this would be to simply set these registers to an identity mapping and use highmem for anything that doesn't fit the low windows. Obviously, they didn't do that.

Quoted 4 lines

BTW, you can override ebase from the platform code, as was done in arch/mips/kernel/smp-bmips.c. It probably isn't necessary to change the common per_cpu_trap_init() code (but it may have been necessary way back in 2.6.32).

Most of the hacks they've done to generic code are actually completely unnecessary, if not outright wrong.

Quoted 11 lines
Quoted 4 lines

2. In io.h they have added explicit __sync() to the end of pfx##write##bwlq and pfx##out##bwlq##p. Is this really necessary? I've not yet found any adverse effects of not doing so. Maybe this was a workaround for some old kernel issue which was fixed since then?

Adding a barrier in writel(), as was done on ARM, might have something to do with the SoC's busing or peripherals. Sometimes there are chip bugs that cause MMIO transaction ordering to break in unexpected ways. Or it could be there to compensate for missing barriers or bad assumptions in a driver somewhere.

Quoted 6 lines

For #2 and #3, it is likely that somebody at Sigma could find a bug report or changelog explaining why it was added. In my experience these sorts of changes are usually made to work around subtle problems discovered in testing or production. Figuring out the exact problem that inspired the patch can be difficult without insider knowledge, unless you happened to run across the same failure.

I suspect the Sigma patches were produced by randomly prodding a kernel with a stick until it started working.

Signature · 2 lines
Måns Rullgård
[email protected]
#3 lore raw
Quoted 18 lines · Måns Rullgård
Quoted 13 lines · Kevin Cernekee
Quoted 3 lines · Oleg Kolosov

1. They (Sigma Designs) have overridden __fast_iob which is identical to the default one except for one line:
...

I do not have any direct experience with these SoCs, but you might want to look at the memory map to try to figure this one out. i.e. if __fast_iob() normally performs an uncached dummy read from the first word of physical memory, does the address need to be adjusted by 64MB on the Sigma chips because system memory (or the memory allocated to the Linux application processor) starts at PA 0x0400_0000 instead of 0x0000_0000?

Quoted 2 lines

That theory would also explain why the exception vectors were adjusted by the same offset.

Quoted 10 lines

The 86xx has two DRAM controllers mapped with 1GB windows at 0x8000_0000 and 0xc000_0000, and also with 256MB windows at 0x1000_0000 and 0x2000_0000. To complicate matters, CPU physical addresses starting at 0x04000000 are subjected to a set of remapping registers translating 6 blocks of 64MB to an arbitrary (64MB-aligned) bus address (not that these addresses overlap with the low mappings of the DRAM controllers). The obvious way to support this would be to simply set these registers to an identity mapping and use highmem for anything that doesn't fit the low windows. Obviously, they didn't do that.

Thanks for the explanations! This is really useful.

Quoted 8 lines
Quoted 4 lines

BTW, you can override ebase from the platform code, as was done in arch/mips/kernel/smp-bmips.c. It probably isn't necessary to change the common per_cpu_trap_init() code (but it may have been necessary way back in 2.6.32).

Most of the hacks they've done to generic code are actually completely unnecessary, if not outright wrong.

That was my suspicion as well. It is reassuring to have a confirmation from someone more knowledgeable. Thanks!

Quoted 21 lines
Quoted 11 lines
Quoted 4 lines

2. In io.h they have added explicit __sync() to the end of pfx##write##bwlq and pfx##out##bwlq##p. Is this really necessary? I've not yet found any adverse effects of not doing so. Maybe this was a workaround for some old kernel issue which was fixed since then?

Adding a barrier in writel(), as was done on ARM, might have something to do with the SoC's busing or peripherals. Sometimes there are chip bugs that cause MMIO transaction ordering to break in unexpected ways. Or it could be there to compensate for missing barriers or bad assumptions in a driver somewhere.

Quoted 6 lines

For #2 and #3, it is likely that somebody at Sigma could find a bug report or changelog explaining why it was added. In my experience these sorts of changes are usually made to work around subtle problems discovered in testing or production. Figuring out the exact problem that inspired the patch can be difficult without insider knowledge, unless you happened to run across the same failure.

I suspect the Sigma patches were produced by randomly prodding a kernel with a stick until it started working.

Signature · 2 lines
Regards, Oleg
Art System
#4 lore raw
Quoted 26 lines · Oleg Kolosov
Quoted 15 lines
Quoted 12 lines
Quoted 3 lines

1. They (Sigma Designs) have overridden __fast_iob which is identical to the default one except for one line:
...

I do not have any direct experience with these SoCs, but you might want to look at the memory map to try to figure this one out. i.e. if __fast_iob() normally performs an uncached dummy read from the first word of physical memory, does the address need to be adjusted by 64MB on the Sigma chips because system memory (or the memory allocated to the Linux application processor) starts at PA 0x0400_0000 instead of 0x0000_0000?

Quoted 2 lines

That theory would also explain why the exception vectors were adjusted by the same offset.

Quoted 10 lines

The 86xx has two DRAM controllers mapped with 1GB windows at 0x8000_0000 and 0xc000_0000, and also with 256MB windows at 0x1000_0000 and 0x2000_0000. To complicate matters, CPU physical addresses starting at 0x04000000 are subjected to a set of remapping registers translating 6 blocks of 64MB to an arbitrary (64MB-aligned) bus address (not that these addresses overlap with the low mappings of the DRAM controllers). The obvious way to support this would be to simply set these registers to an identity mapping and use highmem for anything that doesn't fit the low windows. Obviously, they didn't do that.

Quoted 1 line

Thanks for the explanations! This is really useful.

For the record -- the exact address `__fast_iob' reads from does not
really matter, all it has to guarantee is no side effects on read access. Using the base of KSEG1 was therefore a natural choice for legacy MIPS processors that set the architecture back at the time this code was added, as the presence of exception vectors there guaranteed this area of the address space behaved like RAM so the same location did for any system.

With the introduction of revision 2 of the MIPS architecture the CP0
EBase register was added and consequently there is no longer a guarantee that exception vectors reside at the base of KSEG1. Using the value read from CP0.EBase to determine a usable address might therefore be a better idea, although the current revision of the MIPS architecture specification that includes segmentation control makes it a bit complicated. Using a dummy page mapped uncached instead might work the best.

Maciej

#5 lore raw
Quoted 7 lines · Maciej W. Rozycki

For the record -- the exact address `__fast_iob' reads from does not
really matter, all it has to guarantee is no side effects on read access. Using the base of KSEG1 was therefore a natural choice for legacy MIPS processors that set the architecture back at the time this code was added, as the presence of exception vectors there guaranteed this area of the address space behaved like RAM so the same location did for any system.

Quoted 7 lines

With the introduction of revision 2 of the MIPS architecture the CP0
EBase register was added and consequently there is no longer a guarantee that exception vectors reside at the base of KSEG1. Using the value read from CP0.EBase to determine a usable address might therefore be a better idea, although the current revision of the MIPS architecture specification that includes segmentation control makes it a bit complicated. Using a dummy page mapped uncached instead might work the best.

Would something like this work, assuming __fast_iob() doesn't get called before mem_init()?

CKSEG1ADDR((void *)empty_zero_page)

It is currently a GPL export, so maybe that would need to change to allow non-GPL drivers to use iob(). But that's still easier than allocating another dummy page.

#6 lore raw
Quoted 16 lines · Kevin Cernekee
Quoted 7 lines · Maciej W. Rozycki

For the record -- the exact address `__fast_iob' reads from does not
really matter, all it has to guarantee is no side effects on read access. Using the base of KSEG1 was therefore a natural choice for legacy MIPS processors that set the architecture back at the time this code was added, as the presence of exception vectors there guaranteed this area of the address space behaved like RAM so the same location did for any system.

Quoted 7 lines

With the introduction of revision 2 of the MIPS architecture the CP0
EBase register was added and consequently there is no longer a guarantee that exception vectors reside at the base of KSEG1. Using the value read from CP0.EBase to determine a usable address might therefore be a better idea, although the current revision of the MIPS architecture specification that includes segmentation control makes it a bit complicated. Using a dummy page mapped uncached instead might work the best.

Quoted 8 lines

Would something like this work, assuming __fast_iob() doesn't get called before mem_init()?

CKSEG1ADDR((void *)empty_zero_page)

It is currently a GPL export, so maybe that would need to change to allow non-GPL drivers to use iob(). But that's still easier than allocating another dummy page.

The 86xx has a 64k remappable block at CPU physical address zero, so one option would be to simply point this at some actual memory and leave the macro alone. There doesn't seem to be anything useful in that bus address range anyway. Reading returns zeros, and writes have no apparent effect. Maybe it's even safe to do a dummy read from there in iob().

Signature · 2 lines
Måns Rullgård
[email protected]
#7 lore raw
Quoted 26 lines · Måns Rullgård
Quoted 16 lines · Kevin Cernekee
Quoted 7 lines · Maciej W. Rozycki

For the record -- the exact address `__fast_iob' reads from does not
really matter, all it has to guarantee is no side effects on read access. Using the base of KSEG1 was therefore a natural choice for legacy MIPS processors that set the architecture back at the time this code was added, as the presence of exception vectors there guaranteed this area of the address space behaved like RAM so the same location did for any system.

Quoted 7 lines

With the introduction of revision 2 of the MIPS architecture the CP0
EBase register was added and consequently there is no longer a guarantee that exception vectors reside at the base of KSEG1. Using the value read from CP0.EBase to determine a usable address might therefore be a better idea, although the current revision of the MIPS architecture specification that includes segmentation control makes it a bit complicated. Using a dummy page mapped uncached instead might work the best.

Quoted 8 lines

Would something like this work, assuming __fast_iob() doesn't get called before mem_init()?

CKSEG1ADDR((void *)empty_zero_page)

It is currently a GPL export, so maybe that would need to change to allow non-GPL drivers to use iob(). But that's still easier than allocating another dummy page.

Quoted 6 lines

The 86xx has a 64k remappable block at CPU physical address zero, so one option would be to simply point this at some actual memory and leave the macro alone. There doesn't seem to be anything useful in that bus address range anyway. Reading returns zeros, and writes have no apparent effect. Maybe it's even safe to do a dummy read from there in iob().

IIRC, one of the special operating modes on BCM7435 allows partitioning the hardware in a way that prohibits accesses to PA 0. Not sure how widely it is used, however. I've also seen other embedded MIPS systems that don't have RAM at PA 0, but they didn't run Linux.

So there are two paths forward:

1) Make SMP86xx behave like other currently-supported CPUs, i.e. use the remap registers to configure the chip so that uncached reads from PA 0 do something sensible. This sounds like the easiest fix.

2) Agree to support memory configurations where PA 0 doesn't map to RAM, changing __fast_iob (and maybe other code) accordingly.

#8 lore raw
Quoted 8 lines · Kevin Cernekee
Quoted 8 lines
Quoted 8 lines
Quoted 7 lines

With the introduction of revision 2 of the MIPS architecture the CP0
EBase register was added and consequently there is no longer a guarantee that exception vectors reside at the base of KSEG1. Using the value read from CP0.EBase to determine a usable address might therefore be a better idea, although the current revision of the MIPS architecture specification that includes segmentation control makes it a bit complicated. Using a dummy page mapped uncached instead might work the best.

Quoted 8 lines
Quoted 8 lines
Quoted 8 lines

Would something like this work, assuming __fast_iob() doesn't get called before mem_init()?

CKSEG1ADDR((void *)empty_zero_page)

It is currently a GPL export, so maybe that would need to change to allow non-GPL drivers to use iob(). But that's still easier than allocating another dummy page.

You only need a mapping, not a separate page. Maybe you can use `vmap'
to alias a page from the kernel text -- no memory wasted then.

And the point of segmentation is KSEG1 may not be uncached anymore, it
may not be unmapped even. So a virtual mapping is really the proper and only solution. And this situation is another reason to avoid using CKSEG1ADDR and friends where possible.

But this is more of a heads-up from me rather than a request for
implementation as we don't support segmentation at this point.

Quoted 8 lines

So there are two paths forward:

1) Make SMP86xx behave like other currently-supported CPUs, i.e. use the remap registers to configure the chip so that uncached reads from PA 0 do something sensible. This sounds like the easiest fix.

2) Agree to support memory configurations where PA 0 doesn't map to RAM, changing __fast_iob (and maybe other code) accordingly.

I suspect the latter is bound to happen sooner or later anyway. However
we handle setting CP0.EBase ourselves and we can use it to our advantage. So knowing that our CP0.EBase points to somewhere within KSEG0 we can make an example r2 `__fast_iob' implementation look like:

#define CKSEG1EBASE ((read_c0_ebase() | CKSEG1 | 0xfff) - 0xfff)

#define __fast_iob()				\
	__asm__ __volatile__(			\
		".set	push\n\t"		\
		".set	noreorder\n\t"		\
		"lw	$0,%a0\n\t"		\
		"nop\n\t"			\
		".set	pop"			\
		: /* no output */		\
		: "p" (CKSEG1EBASE)		\
		: "memory")

which at 6 instructions produced is I think the best you can get without using an auxiliary variable rather than CP0.EBase and then handcoding the whole address calculation in assembly or using indirection.

Maciej

#9 lore raw
In reply to Oleg Kolosov
Quoted 2 lines · Oleg Kolosov

Hello MIPS gurus!

Hello.

Quoted 7 lines

I'm adding support for Sigma Designs SMP8652/SMP8654 (Tango3 family, MIPS 24kf CPU) to newer kernel. I've selectively adapted patches from 2.6.32.15 (the latest officially available for us) to the latest mips 3.18 stable branch and things seem to work (it boots, runs simple test programs), but there are few questions which I was not able to resolve yet with my limited experience:

It is good to hear somebody is working with that hardware. I have uploaded all the Sigma source that we were given along with their root file system images. A lot is for the 8910, but there is stuff in there for the 86xx family as well.

Steve

http://www.linux-mips.org/pub/linux/mips/people/sjhill/Sigma/

#10 lore raw
Quoted 12 lines · Steven J. Hill
Quoted 2 lines · Oleg Kolosov

Hello MIPS gurus!

Hello.

Quoted 7 lines

I'm adding support for Sigma Designs SMP8652/SMP8654 (Tango3 family, MIPS 24kf CPU) to newer kernel. I've selectively adapted patches from 2.6.32.15 (the latest officially available for us) to the latest mips 3.18 stable branch and things seem to work (it boots, runs simple test programs), but there are few questions which I was not able to resolve yet with my limited experience:

Quoted 8 lines

It is good to hear somebody is working with that hardware. I have uploaded all the Sigma source that we were given along with their root file system images. A lot is for the 8910, but there is stuff in there for the 86xx family as well.

Steve

http://www.linux-mips.org/pub/linux/mips/people/sjhill/Sigma/

I have a bunch of cleaned/rewritten 86xx drivers for 3.19 here:
https://github.com/mansr/linux-tangox

Signature · 2 lines
Måns Rullgård
[email protected]
#11 lore raw
Quoted 14 lines · Måns Rullgård
Quoted 12 lines · Steven J. Hill
Quoted 2 lines · Oleg Kolosov

Hello MIPS gurus!

Hello.

Quoted 7 lines

I'm adding support for Sigma Designs SMP8652/SMP8654 (Tango3 family, MIPS 24kf CPU) to newer kernel. I've selectively adapted patches from 2.6.32.15 (the latest officially available for us) to the latest mips 3.18 stable branch and things seem to work (it boots, runs simple test programs), but there are few questions which I was not able to resolve yet with my limited experience:

Quoted 8 lines
Quoted 8 lines

It is good to hear somebody is working with that hardware. I have uploaded all the Sigma source that we were given along with their root file system images. A lot is for the 8910, but there is stuff in there for the 86xx family as well.

Steve

http://www.linux-mips.org/pub/linux/mips/people/sjhill/Sigma/

Thanks a lot! I've been curious if there are some improvements. Unfortunately, all the same workarounds faithfully merged. But still, looks like there might be some useful bits - like cpu feature overrides.

Quoted 4 lines

I have a bunch of cleaned/rewritten 86xx drivers for 3.19 here:
https://github.com/mansr/linux-tangox

Wow! You are my hero! Even DT bindings and I2C driver - dreams came true. Nice and clean - without all those horrible ifdefs and gbus_write's sprinkled everywhere. After so much struggle - it is like revelation. I will thoroughly study your solution.

Signature · 2 lines
Regards, Oleg
Art System