[PATCH] powerpc: Only make kernel text pages of linear mapping executable
Commit bc033b63bbfeb6c4b4eb0a1d083c650e4a0d2af8 (powerpc/mm: Fix
attribute confusion with htab_bolt_mapping()) moved the check for
whether we should make pages of the linear mapping executable from
htab_bolt_mapping into its callers, including htab_initialize.
A side-effect of this is that the decision is now made once for
each contiguous section in the LMB array rather than for each page
individually. This can often mean that the whole of the linear
mapping ends up being executable.
This reverts to the previous behaviour, where individual pages are
checked for being part of the kernel text or not, by moving the check
back down into htab_bolt_mapping.
Signed-off-by: Paul Mackerras [EMAIL PROTECTED]
---
diff --git a/arch/powerpc/mm/hash_utils_64.c b/arch/powerpc/mm/hash_utils_64.c
index 14be408..1fe7ac6 100644
--- a/arch/powerpc/mm/hash_utils_64.c
+++ b/arch/powerpc/mm/hash_utils_64.c
@@ -191,12 +191,17 @@ int htab_bolt_mapping(unsigned long vstart, unsigned long
vend,
unsigned long hash, hpteg;
unsigned long vsid = get_kernel_vsid(vaddr, ssize);
unsigned long va = hpt_va(vaddr, vsid, ssize);
+ unsigned long tprot = prot;
+
+ /* Only kernel text is executable */
+ if (!in_kernel_text(vaddr))
+ tprot |= HPTE_R_N;
hash = hpt_hash(va, shift, ssize);
hpteg = ((hash htab_hash_mask) * HPTES_PER_GROUP);
BUG_ON(!ppc_md.hpte_insert);
- ret = ppc_md.hpte_insert(hpteg, va, paddr, prot,
+ ret = ppc_md.hpte_insert(hpteg, va, paddr, tprot,
HPTE_V_BOLTED, psize, ssize);
if (ret 0)
@@ -584,7 +589,7 @@ void __init htab_initialize(void)
{
unsigned long table;
unsigned long pteg_count;
- unsigned long prot, tprot;
+ unsigned long prot;
unsigned long base = 0, size = 0, limit;
int i;
@@ -660,10 +665,9 @@ void __init htab_initialize(void)
for (i=0; i lmb.memory.cnt; i++) {
base = (unsigned long)__va(lmb.memory.region[i].base);
size = lmb.memory.region[i].size;
- tprot = prot | (in_kernel_text(base) ? _PAGE_EXEC : 0);
DBG(creating mapping for region: %lx..%lx (prot: %x)\n,
- base, size, tprot);
+ base, size, prot);
#ifdef CONFIG_U3_DART
/* Do not map the DART space. Fortunately, it will be aligned
@@ -680,21 +684,21 @@ void __init htab_initialize(void)
unsigned long dart_table_end = dart_tablebase + 16 * MB;
if (base != dart_tablebase)
BUG_ON(htab_bolt_mapping(base, dart_tablebase,
- __pa(base), tprot,
+ __pa(base), prot,
mmu_linear_psize,
mmu_kernel_ssize));
if ((base + size) dart_table_end)
BUG_ON(htab_bolt_mapping(dart_tablebase+16*MB,
base + size,
__pa(dart_table_end),
-tprot,
+prot,
mmu_linear_psize,
mmu_kernel_ssize));
continue;
}
#endif /* CONFIG_U3_DART */
BUG_ON(htab_bolt_mapping(base, base + size, __pa(base),
- tprot, mmu_linear_psize, mmu_kernel_ssize));
+ prot, mmu_linear_psize, mmu_kernel_ssize));
}
/*
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] powerpc: Only make kernel text pages of linear mapping executable
On Thu, 2008-08-28 at 16:38 +1000, Paul Mackerras wrote: Commit bc033b63bbfeb6c4b4eb0a1d083c650e4a0d2af8 (powerpc/mm: Fix attribute confusion with htab_bolt_mapping()) moved the check for whether we should make pages of the linear mapping executable from htab_bolt_mapping into its callers, including htab_initialize. A side-effect of this is that the decision is now made once for each contiguous section in the LMB array rather than for each page individually. This can often mean that the whole of the linear mapping ends up being executable. This reverts to the previous behaviour, where individual pages are checked for being part of the kernel text or not, by moving the check back down into htab_bolt_mapping. Signed-off-by: Paul Mackerras [EMAIL PROTECTED] Acked-by: Benjamin Herrenschmidt [EMAIL PROTECTED] ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH 2/4] Add cpufreq driver for the IBM PowerPC 750GX
On Wednesday 27 August 2008, Kevin Diggs wrote: Arnd Bergmann wrote: Module parameter names should be short, so just minmax would be a good name, but better put the module_param() line right after that. If it's a bool type, I would just leave out the initialization. Ok. But leaving out the initialization will make me itch. Should I also replace override_min_core with mincore (or min_core)? And override_max_core with maxcore (or max_core)? Leaving out the initializations makes me ... uneasy. It's ok to leave them out if they are 0? Yes, that's how global and static variables are defined in C. Only automatic variables have undefined content. I think the module_exit() function should leave the frequency in a well-defined state, so the easiest way to get there is probably to delete the timer, and then manually set the frequency. I really don't follow you here? If I let the timer fire then the cpu (and the cpufreq sub-system) will be left in a well-defined state. I don't understand why you want me to delete the timer and then basically do manually what it was going to do anyway. There are two calls to cpufreq_notify_transition(). One just before the modify_PLL() call, with CPUFREQ_PRECHANGE as an argument, and one in the pll_switch_cb() routine, with CPUFREQ_POSTCHANGE as an argument. I would need to make sure that these are matched up. Even without the HRTimer stuff being used the timer fires in less than 4 ms (@ 250 HZ). So I can't see the user actually trying to interrupt a frequency change. With HRTimers it is 100 us. Can we please, please leave this part as is? I'm still not convinced that it's actually correct if you call complete() from the other places as well. You have three locations in your code where you call complete() but only one for INIT_COMPLETION. The part that I don't understand (and therefore don't expect other readers to understand) is how the driver guarantees that only one complete() will be called on the completion variable after it has been initialized. What I meant with the well-defined state is that after unloading the module, the CPU frequency should be the same as before loading the module, typically the maximum frequency, but not the one that was set last. Arnd ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: checkpatch nits ...
On Wed, Aug 27, 2008 at 09:49:44AM +0200, Wolfram Sang wrote: On Sat, Aug 23, 2008 at 10:57:21AM +0200, Arnd Bergmann wrote: On Saturday 23 August 2008, Kevin Diggs wrote: WARNING: externs should be avoided in .c files #1137: FILE: powerpc/kernel/cpu/pll_if.c:369: + __asm__ __volatile__ ( ??? I don't know what this is? The entire block is: __asm__ __volatile__ ( addi %0,%3,-1\n andc %1,%3,%0\n cntlzw %1,%1\n subfic %1,%1,31\n cntlzw %0,%2\n: =r(cntlz), =r(cnttz): r(tmp), b(cnttz) ); It's a bug in checkpatch, your code is correct (although I would write asm volatile, not __asm__ __volatile__, and add \t after each \n). Can you explain why you need that inline assembly? All you do in there are arithmetic operations, so you should be able to express that using C, or at least using the helpers we already have. Checkpatch thinks that what you wrote is a declaration for a function named __volatile__ returning a variable of type __asm__, and complains that this declaration belongs into a header file. I wonder if checkpatch-maintainers read this list, so I put Andy Whitcroft to CC. Thanks for the Cc:, even if you do read the lists you can easily miss something of direct interest. I actually think I saw a previous patch flow past showing this issue and there appears to be a specific exception for __asm__ never being a type. Cirtainly this fragment does not throw the error with the latest checkpatch in my tree. That fix appears to have gone in in 0.20. Could you check which version you have, the version number appears in the usage message when checkpatch is invoked without parameters. Also give the version at the URL below a go, it represents the current development copy. http://www.kernel.org/pub/linux/kernel/people/apw/checkpatch/checkpatch.pl-testing Thanks for the report. -apw ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH 0/5] Relocatable 64-bit kernel using linker PIE support
On Wed, 2008-08-13 at 11:27 +1000, Paul Mackerras wrote: The following series of patches implement support for a relocatable kernel by building it as a position-independent executable (PIE). When the linker is given the -pie flag, it creates an executable that contains dynamic relocations which can be used to relocate the image at boot time for any desired base address. This patch series adds a CONFIG_RELOCATABLE config option for 64-bit which links the kernel with -pie and arranges to process the relocations in early boot. With the first 4 patches applied, a relocatable kernel will still copy itself down to real address 0. The last patch changes things so that a relocatable kernel will run wherever it was loaded. This last patch is pretty much just a proof of concept since it doesn't do anything to ensure appropriate alignment of the base address (the base address needs to be 16kB aligned). We probably want to work out whether we are a kdump kernel and run in-place if so, or copy down to 0 if not. Is this mature enough for us to consider putting it in Fedora? We'd _love_ to stop building a separate kdump kernel for ppc64... -- dwmw2 ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH 0/5] Relocatable 64-bit kernel using linker PIE support
On Aug 28, 2008, at 7:12 AM, David Woodhouse wrote: On Wed, 2008-08-13 at 11:27 +1000, Paul Mackerras wrote: The following series of patches implement support for a relocatable kernel by building it as a position-independent executable (PIE). When the linker is given the -pie flag, it creates an executable that contains dynamic relocations which can be used to relocate the image at boot time for any desired base address. This patch series adds a CONFIG_RELOCATABLE config option for 64-bit which links the kernel with -pie and arranges to process the relocations in early boot. With the first 4 patches applied, a relocatable kernel will still copy itself down to real address 0. The last patch changes things so that a relocatable kernel will run wherever it was loaded. This last patch is pretty much just a proof of concept since it doesn't do anything to ensure appropriate alignment of the base address (the base address needs to be 16kB aligned). We probably want to work out whether we are a kdump kernel and run in-place if so, or copy down to 0 if not. Is this mature enough for us to consider putting it in Fedora? We'd _love_ to stop building a separate kdump kernel for ppc64... Also, can we get this on ppc32 (head_32.S)? - k ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH 0/5] Relocatable 64-bit kernel using linker PIE support
On Thu, 28 Aug 2008, David Woodhouse wrote: On Wed, 2008-08-13 at 11:27 +1000, Paul Mackerras wrote: The following series of patches implement support for a relocatable kernel by building it as a position-independent executable (PIE). When the linker is given the -pie flag, it creates an executable that contains dynamic relocations which can be used to relocate the image at boot time for any desired base address. This patch series adds a CONFIG_RELOCATABLE config option for 64-bit which links the kernel with -pie and arranges to process the relocations in early boot. With the first 4 patches applied, a relocatable kernel will still copy itself down to real address 0. The last patch changes things so that a relocatable kernel will run wherever it was loaded. This last patch is pretty much just a proof of concept since it doesn't do anything to ensure appropriate alignment of the base address (the base address needs to be 16kB aligned). We probably want to work out whether we are a kdump kernel and run in-place if so, or copy down to 0 if not. Is this mature enough for us to consider putting it in Fedora? We'd _love_ to stop building a separate kdump kernel for ppc64... I tried CONFIG_RELOCATABLE=y on PS3 a while ago, and the resulting kernel locked up during early boot. With kind regards, Geert Uytterhoeven Software Architect Sony Techsoft Centre Europe The Corporate Village · Da Vincilaan 7-D1 · B-1935 Zaventem · Belgium Phone:+32 (0)2 700 8453 Fax: +32 (0)2 700 8622 E-mail: [EMAIL PROTECTED] Internet: http://www.sony-europe.com/ A division of Sony Europe (Belgium) N.V. VAT BE 0413.825.160 · RPR Brussels Fortis · BIC GEBABEBB · IBAN BE41293037680010___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] ppc4xx_pci: necessary fixes for 4GB RAM size
On Fri, 22 Aug 2008 11:43:35 +0400
Ilya Yanok [EMAIL PROTECTED] wrote:
1. total_memory should be phys_addr_t not unsigned long
2. is_power_of_2() works with u32 so I just inlined (size (size-1)) != 0
instead.
Also this patch fixes default initialization: res-end should be 0x7fff
not 0x8000.
Signed-off-by: Ilya Yanok [EMAIL PROTECTED]
Ben, any comments here? Looks right to me.
josh
---
arch/powerpc/sysdev/ppc4xx_pci.c | 11 ++-
1 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/sysdev/ppc4xx_pci.c
b/arch/powerpc/sysdev/ppc4xx_pci.c
index e1c7df9..645b2c9 100644
--- a/arch/powerpc/sysdev/ppc4xx_pci.c
+++ b/arch/powerpc/sysdev/ppc4xx_pci.c
@@ -36,7 +36,7 @@
static int dma_offset_set;
/* Move that to a useable header */
-extern unsigned long total_memory;
+extern phys_addr_t total_memory;
#define U64_TO_U32_LOW(val) ((u32)((val) 0xULL))
#define U64_TO_U32_HIGH(val) ((u32)((val) 32))
@@ -105,7 +105,8 @@ static int __init ppc4xx_parse_dma_ranges(struct
pci_controller *hose,
/* Default */
res-start = 0;
- res-end = size = 0x8000;
+ size = 0x8000;
+ res-end = size - 1;
res-flags = IORESOURCE_MEM | IORESOURCE_PREFETCH;
/* Get dma-ranges property */
@@ -167,13 +168,13 @@ static int __init ppc4xx_parse_dma_ranges(struct
pci_controller *hose,
*/
if (size total_memory) {
printk(KERN_ERR %s: dma-ranges too small
-(size=%llx total_memory=%lx)\n,
-hose-dn-full_name, size, total_memory);
+(size=%llx total_memory=%llx)\n,
+hose-dn-full_name, size, (u64)total_memory);
return -ENXIO;
}
/* Check we are a power of 2 size and that base is a multiple of size*/
- if (!is_power_of_2(size) ||
+ if ((size (size - 1)) != 0 ||
(res-start (size - 1)) != 0) {
printk(KERN_ERR %s: dma-ranges unaligned\n,
hose-dn-full_name);
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] ppc4xx_pci: necessary fixes for 4GB RAM size
On Thursday 28 August 2008, Josh Boyer wrote: 1. total_memory should be phys_addr_t not unsigned long 2. is_power_of_2() works with u32 so I just inlined (size (size-1)) != 0 instead. Also this patch fixes default initialization: res-end should be 0x7fff not 0x8000. Signed-off-by: Ilya Yanok [EMAIL PROTECTED] Ben, any comments here? Looks right to me. Looks good to me too. So: Acked-by: Stefan Roese [EMAIL PROTECTED] Best regards, Stefan ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [BUG] linux-next: Tree for August 26 - Badness at kernel/notifier.c:25
On Thu, 2008-08-28 at 00:38 +1000, Stephen Rothwell wrote: Hi Arjan, On Thu, 28 Aug 2008 00:33:08 +1000 Stephen Rothwell [EMAIL PROTECTED] wrote: The original reported trace was during setup_system which is very early in the boot. But, of course, that version didn't have the necessary extra dereference of the function address ... And the later debug patch did not check the address at register time, only at notify time. The later trace also looks to be early in the boot. It's isa_bridge_notify(), which is neither within _[se]text nor _[se]inittext, so the core_kernel_text() function disavows it. Where are __devinit functions supposed to end up? $ egrep _[es]init\|_[es]text\|isa_bridge_notify System.map c000 T _stext c045d000 T _etext c0463ca8 t .isa_bridge_notify c063a000 T _sinittext c067c3bc T _einittext c071fd80 d isa_bridge_notify -- David WoodhouseOpen Source Technology Centre [EMAIL PROTECTED] Intel Corporation ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [BUG] linux-next: Tree for August 26 - Badness at kernel/notifier.c:25
On Thu, 2008-08-28 at 15:23 +0100, David Woodhouse wrote: On Thu, 2008-08-28 at 00:38 +1000, Stephen Rothwell wrote: Hi Arjan, On Thu, 28 Aug 2008 00:33:08 +1000 Stephen Rothwell [EMAIL PROTECTED] wrote: The original reported trace was during setup_system which is very early in the boot. But, of course, that version didn't have the necessary extra dereference of the function address ... And the later debug patch did not check the address at register time, only at notify time. The later trace also looks to be early in the boot. It's isa_bridge_notify(), which is neither within _[se]text nor _[se]inittext, so the core_kernel_text() function disavows it. Where are __devinit functions supposed to end up? The TEXT_TEXT macro defined in asm-generic/vmlinux.lds.h should get this right... but we don't use it. Is there any particular reason for that, or should we Signed-off-by: David Woodhouse [EMAIL PROTECTED] --- linux-2.6.26.ppc64/arch/powerpc/kernel/vmlinux.lds.S~ 2008-07-13 22:51:29.0 +0100 +++ linux-2.6.26.ppc64/arch/powerpc/kernel/vmlinux.lds.S2008-08-28 15:39:14.0 +0100 @@ -35,10 +35,11 @@ SECTIONS ALIGN_FUNCTION(); *(.text.head) _text = .; - *(.text .fixup .text.init.refok .exit.text.refok) + TEXT_TEXT SCHED_TEXT LOCK_TEXT KPROBES_TEXT + *(.fixup) #ifdef CONFIG_PPC32 *(.got1) -- David WoodhouseOpen Source Technology Centre [EMAIL PROTECTED] Intel Corporation ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: Why does one stw fail with address translation disabled in PPC405EP?
Hi, all:
Well, as described before, the problem happens at 0xc434
InstructionAccess+52: stw r12,64(r11). At this moment,
address translation is disabled and physical addresses are used, but r11
contains 0x03072da0 which is a physical address out of the range of 0x0
and 0x01ff. I check backward and see that value of r11 is from r1 by
tophys(r11,r1). r1 should hold the stack pointer, whose value is in the
form of 0xc3xx when the problem happens.
In this project, we make use of the concept of domain for high level
OSes above XtratuM kernel. Here Linux is the root domain and the problem
happens when we tries to load and transfer to a testing domain which
just prints some sentences, and 0x10a0 is the entry point of the
testing domain, so we need to execute this address. We define a struct
domain_t for the domain and the stack pointer is achieved by:
d-sstack_st = vmalloc (DEFAULT_SSTACK_SIZE); /* DEFAULT_SSTACK_SIZE
is 0x1000 */
if (!d-sstack_st) {
destroy_domain (d);
id = -OUT_OF_MEMORY;
goto exit_load_domain;
}
d-sstack = (unsigned long *)((unsigned long)d-sstack_st +
DEFAULT_SSTACK_SIZE); /* sstack is the new domain's stack pointer that
will be moved to r1 */
After doing this, sstack will get the value of 0xc3xx.
Shouln't I use vmalloc here? Or are there any other solution?
Thanks in advance for any advice!
Best Wishes
Zhou Rui
2008-08-28
在 2008-08-25一的 21:16 +0200，Zhou Rui写道：
Hi,
I think maybe you have known this project named XtratuM
(http://www.xtratum.org). I'm porting it from x86 to PPC405. The
implementation on PPC440 has been basically finished
(ftp://dslab.lzu.edu.cn/pub/xtratum/xtratum-ppc/snapshots/xtratum-ppc-20071205.tar.bz2)
and I know there was discussion about it in this mail list before. XtratuM
is an ADEOS based nano kernel. It aims for realtime and is designed to
provide virtual timer, virtual interrupt and memory space sperations for
domains. Each domain is loaded by a userspace program (instead of the root
domain as a kernel module) and the loader will load the domain's (ELF
staticly excutable) PT_LOAD section into memory, and then raise a properly
system call (passing the structurized loaded data as arguments) to load the
domain via load_domain_sys() of XtratuM, and at the last step of loading the
domain, xtratum will jump to the entry code of the new domain(asm wrappered
start() routine) and then everything should be fine. 0x10a0 is the entry
point of the test domain, and that is why I need to start execution from it.
I think I can say something of my analysis so far for the cause of my
problem. Thanks for the mention of memory size. Once the kernel module
of XtratuM is loaded, the symbols of it are placed to virtual addresses
like 0xc3xx. Because in normal state, address translation is enabled
(MSR[IR, DR] = [1, 1]), these addresses are okay. However, when loading
the domain, because the entry point 0x10a0 is not in TLB and it
should be reloaded, Data TLB Miss Exception arises and DTLBMiss is
called. The exception clears MSR[IR, DR], so address translation is
disabled and physical address should be used at this moment. If we want
something at the virtual address of 0xc3xx, we must access the
physical addresses like 0x03xx. Nevertheless, the limitation of 32MB
memory makes the valid physical address range from 0x0 to 0x1ff.
Therefore, during the exception handling, the addresses out of range
should not be accessed, but the instructions cannot know the memory
limitation in advance and tries to do something in addresses such as
0x03072da0 based on the address translation mechanism, which leads to
machine check.
I haved tried to append mem=32M to kernel command line but no help. I
think it is because when loading the kernel in normal state, address
translation is enabled and the virtual addresses are okay. Kernel cannot
foresee that there is going to be a TLB miss exception and the illegal
physical addresses like 0x03xx may be accessed.
So any ideas for this problem are welcome.
Thank you very much for taking care.
Best Wishes
Zhou Rui
2008-08-25
在 2008-08-24日的 20:55 +0200，Wolfgang Denk写道：
Dear Zhou Rui,
In message [EMAIL PROTECTED] you wrote:
I am running a kernel module which will execute a user space
application. The entry point of the application is 0x10a0. At the
That should be the first clue that you are doing it wrong. Don't do
stuff like that in modules...
Oh, but our project needs a function like that ...
You should really think about this. Why do you think you need this?
What exactly are you trying to do? [Probably there are better
approaches to solve your problem...]
It is physical address at this moment. Address translation is disabled
automatically (MSR[IR, DR] = [0, 0]) because of TLB Miss Exception and
Instrunction Storage Exception.
Hm..
Re: [PATCH v2] POWERPC: Allow 32-bit pgtable code to support 36-bit physical
On Aug 27, 2008, at 6:43 PM, Scott Wood wrote: Becky Bruce wrote: #if _PAGE_HASHPTE != 0 +#ifndef CONFIG_PTE_64BIT pte_update(ptep, ~_PAGE_HASHPTE, pte_val(pte) ~_PAGE_HASHPTE); #else + /* +* We have to do the write of the 64b pte as 2 stores. This +* code assumes that the entry we're storing to is currently +* not valid and that all callers have the page table lock. +* Having the entry be not valid protects readers who might read +* between the first and second stores. +*/ + unsigned int tmp; + + __asm__ __volatile__(\ +1: lwarx %0,0,%4\n\ + rlwimi %L2,%0,0,30,30\n\ + stw %2,0(%3)\n\ + eieio\n\ + stwcx. %L2,0,%4\n\ + bne-1b + : =r (tmp), =m (*ptep) + : r (pte), r (ptep), r ((unsigned long)(ptep) + 4), m (*ptep) + : cc); +#endif /* CONFIG_PTE_64BIT */ Could the stw to the same reservation granule as the stwcx cancel the reservation on some implementations? P Not on the same processor. lus, if you're assuming that the entry is currently invalid and all callers have the page table lock, do we need the lwarx/stwcx at all? At the least, it should check PTE_ATOMIC_UPDATES. I'm pretty sure I went through this in great detail at one point and concluded that I did in fact need the lwarx/stwcx. IIRC, it has to do with other non-set_pte_at writers not necessarily holding the page table lock. FYI, the existing 32-bit PTE code is doing atomic updates as well. About PTE_ATOMIC_UPDATES, I didn't add that in because hashed page table implementations require atomic updates. Adding it in would create another clause in that code, because I would still need to order the operations with a 64-bit PTE and I can't call pte_update as it only changes the low word of the pte. I wasn't feeling too keen on adding untested pagetable code into the kernel :) I can add it if the peanut gallery wants it, but I'll be marking it with a big fat BUYER BEWARE. %2 should be +r, not r. diff --git a/arch/powerpc/platforms/Kconfig.cputype b/arch/powerpc/ platforms/Kconfig.cputype index 7f65127..ca5b58b 100644 --- a/arch/powerpc/platforms/Kconfig.cputype +++ b/arch/powerpc/platforms/Kconfig.cputype @@ -128,18 +128,22 @@ config FSL_EMB_PERFMON config PTE_64BIT bool - depends on 44x || E500 + depends on 44x || E500 || 6xx default y if 44x - default y if E500 PHYS_64BIT + default y if PHYS_64BIT How is this different from PHYS_64BIT? One is the width of the PTE and one is the width of a physical address. It's entirely plausible to have a 64-bit PTE because you have a bunch of status bits, and only have 32-bit physical addressing. That's why there are 2 options. config PHYS_64BIT - bool 'Large physical address support' if E500 - depends on 44x || E500 + bool 'Large physical address support' if E500 || 6xx Maybe if !44x, or have 44x select this, rather than listing all arches where it's optional. Not sure exactly what you're suggesting here + depends on 44x || E500 || 6xx select RESOURCES_64BIT default y if 44x ---help--- This option enables kernel support for larger than 32-bit physical - addresses. This features is not be available on all e500 cores. + addresses. This features is not be available on all cores. This features is not be? Heh, I didn't type that :) But I can fix it. Thanks, B ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH v2] POWERPC: Allow 32-bit pgtable code to support 36-bit physical
Becky Bruce wrote: I'm pretty sure I went through this in great detail at one point and concluded that I did in fact need the lwarx/stwcx. IIRC, it has to do with other non-set_pte_at writers not necessarily holding the page table lock. FYI, the existing 32-bit PTE code is doing atomic updates as well. But will those updates happen if there isn't already a valid PTE? About PTE_ATOMIC_UPDATES, I didn't add that in because hashed page table implementations require atomic updates. Right, I misread it and thought it was being used for non-hashed implementations as well. What happens if you enable 64-bit PTEs on a 603-ish CPU? The kconfig seems to allow it. Adding it in would create another clause in that code, because I would still need to order the operations with a 64-bit PTE and I can't call pte_update as it only changes the low word of the pte. I wasn't feeling too keen on adding untested pagetable code into the kernel :) Wimp. :-) I can add it if the peanut gallery wants it, but I'll be marking it with a big fat BUYER BEWARE. No, there's little point if nothing selects it (or is planned to in the near future). diff --git a/arch/powerpc/platforms/Kconfig.cputype b/arch/powerpc/platforms/Kconfig.cputype index 7f65127..ca5b58b 100644 --- a/arch/powerpc/platforms/Kconfig.cputype +++ b/arch/powerpc/platforms/Kconfig.cputype @@ -128,18 +128,22 @@ config FSL_EMB_PERFMON config PTE_64BIT bool -depends on 44x || E500 +depends on 44x || E500 || 6xx default y if 44x -default y if E500 PHYS_64BIT +default y if PHYS_64BIT How is this different from PHYS_64BIT? One is the width of the PTE and one is the width of a physical address. It's entirely plausible to have a 64-bit PTE because you have a bunch of status bits, and only have 32-bit physical addressing. That's why there are 2 options. Right, I just didn't see anything that actually selects it independently of PHYS_64BIT. Is this something that's expected to actually happen in the future? The default y if 44x line is redundant with the default y if PHYS_64BIT. config PHYS_64BIT -bool 'Large physical address support' if E500 -depends on 44x || E500 +bool 'Large physical address support' if E500 || 6xx Maybe if !44x, or have 44x select this, rather than listing all arches where it's optional. Not sure exactly what you're suggesting here It just seems simpler to not conditionalize the bool, but instead have CONFIG_44x do select PHYS_64BIT. I'd rather avoid another list of platforms accumulating in a kconfig dependency. +depends on 44x || E500 || 6xx select RESOURCES_64BIT default y if 44x ---help--- This option enables kernel support for larger than 32-bit physical - addresses. This features is not be available on all e500 cores. + addresses. This features is not be available on all cores. This features is not be? Heh, I didn't type that :) But I can fix it. You didn't type it, but you touched it. Tag, you're it. :-) -Scott ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH 2/4] Add cpufreq driver for the IBM PowerPC 750GX
Arnd Bergmann wrote: On Wednesday 27 August 2008, Kevin Diggs wrote: Arnd Bergmann wrote: I think the module_exit() function should leave the frequency in a well-defined state, so the easiest way to get there is probably to delete the timer, and then manually set the frequency. I really don't follow you here? If I let the timer fire then the cpu (and the cpufreq sub-system) will be left in a well-defined state. I don't understand why you want me to delete the timer and then basically do manually what it was going to do anyway. There are two calls to cpufreq_notify_transition(). One just before the modify_PLL() call, with CPUFREQ_PRECHANGE as an argument, and one in the pll_switch_cb() routine, with CPUFREQ_POSTCHANGE as an argument. I would need to make sure that these are matched up. Even without the HRTimer stuff being used the timer fires in less than 4 ms (@ 250 HZ). So I can't see the user actually trying to interrupt a frequency change. With HRTimers it is 100 us. Can we please, please leave this part as is? I'm still not convinced that it's actually correct if you call complete() from the other places as well. You have three locations in your code where you call complete() but only one for INIT_COMPLETION. The part that I don't understand (and therefore don't expect other readers to understand) is how the driver guarantees that only one complete() will be called on the completion variable after it has been initialized. What I meant with the well-defined state is that after unloading the module, the CPU frequency should be the same as before loading the module, typically the maximum frequency, but not the one that was set last. As you point out, there are three calls to complete(). The first is in the switch callback, in the idle_pll_off disabled branch. This callback is triggered from the pll switch routine in pll_if. So that means the call to _modify_PLL() in _target worked. So the complete() call in _target will NOT be called. The complete() call in the lock callback is only called in the idle_pll_off enabled branch. As just mentioned, the second complete() call in the lock callback is only called when idle_pll_off is enabled. The final complete() call is in the unlock exit path in _target(). This is an error path, where for one reason or another, there was no successful call to _modify_PLL(). Thus there will be triggering of either callback. There is another initialization of the completion: the DECLARE_COMPLETION(). I think I will deadlock if I get unloaded before _target() is ever called. This can happen. I may add a test_bit(...changing_pll_bit) condition on the wait_for_completion() call. Arnd Thanks for taking the time to review and for the comments! kevin ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] usb: add Freescale QE/CPM USB peripheral controller driver
On Thu, Aug 28, 2008 at 05:43:33PM +0800, Li Yang wrote: +config USB_GADGET_FSL_QE + boolean Freescale QE/CPM USB Device Controller + depends on FSL_SOC (QUICC_ENGINE || CPM) + help +Some of Freescale PowerPC processors have a Full Speed +QE/CPM2 USB controller, which support device mode with 4 +programmable endpoints. This driver supports the +controller in the MPC8360 and MPC8272, and should work with +controllers having QE or CPM2, given minor tweaks. + +Say y to link the driver statically, or m to build a +dynamically linked module called fsl_qe_udc and force all +gadget drivers to also be dynamically linked. How can you say m to something that is not a tristate? -Scott ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: RFC: Could cpm2_clk_setup and cpm2_set_pin be exported ?
On Thursday 28 August 2008, Scott Wood wrote: On Thu, Aug 28, 2008 at 05:57:13PM +0200, Laurent Pinchart wrote: I'm facing a situation where I need to call cpm2_clk_setup and cpm2_set_pin from a device driver compiled as a module. Before submitting a patch to export both functions, I'd like to make sure there isn't a cleaner way to implement the desired functionality without calling functions that are supposed to be used by board setup code. Have you looked at using the GPIO API? Yes, but the GPIO API doesn't support dedicated pin usage. Basically all I can do is configure a pin as a general purpose input or output, and set its level when configured as an output. The GPIO API doesn't provide any way to access the PAR and SOR registers. Beside, the GPIO API won't help configuring clock routing. Best regards, -- Laurent Pinchart CSE Semaphore Belgium Chaussee de Bruxelles, 732A B-1410 Waterloo Belgium T +32 (2) 387 42 59 F +32 (2) 387 42 75 signature.asc Description: This is a digitally signed message part. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [BUG] linux-next: Tree for August 26 - Badness at kernel/notifier.c:25
David Woodhouse dwmw2 at infradead.org Fri Aug 29 00:55:07 EST 2008 On Thu, 2008-08-28 at 15:23 +0100, David Woodhouse wrote: On Thu, 2008-08-28 at 00:38 +1000, Stephen Rothwell wrote: Hi Arjan, On Thu, 28 Aug 2008 00:33:08 +1000 Stephen Rothwell sfr at canb.auug.org.au wrote: The original reported trace was during setup_system which is very early in the boot. But, of course, that version didn't have the necessary extra dereference of the function address ... And the later debug patch did not check the address at register time, only at notify time. The later trace also looks to be early in the boot. It's isa_bridge_notify(), which is neither within _[se]text nor _[se]inittext, so the core_kernel_text() function disavows it. Where are __devinit functions supposed to end up? The TEXT_TEXT macro defined in asm-generic/vmlinux.lds.h should get this right... but we don't use it. Is there any particular reason for that, or should we gitk -- arch/powerpc/kernel/vmlinux.S e95c91821fa56b489d7beb74103a419466c5ec10 [POWERPC] Fix link errors for allyesconfig An allyesconfig build creates a .text section that is so big that the .text.init.refok and .fixup sections are too far away for the relocations to be fixed up correctly. This patch fixes that by linking all the relevent text sections for each file together. Suggested by Paul Mackerras. Signed-off-by: Stephen Rothwell [EMAIL PROTECTED] Signed-off-by: Paul Mackerras [EMAIL PROTECTED] Although I think its really fac23fe4be23259a8eaa9bad822f5b14dd07d15c powerpc: Introduce infrastructure for feature sections with alternatives that causes the problems. If the problem is only reaching the branch-out-of-fixup-section, then we could create a macro that caculates the branch as if it were already at the destination address (using something like b (target-fixup_start)-(current-alternative_start) and then removing the code that determines the branch target goes beyond the feature section. Just a concept, have't tried it yet and don't know if there are other problems with .text.init.refok. Or we fix our defintion and put a comment next to TEXT_TEXT that we don't use it for future editors. Signed-off-by: David Woodhouse David.Woodhouse at intel.com --- linux-2.6.26.ppc64/arch/powerpc/kernel/vmlinux.lds.S~ 2008-07-13 22:51:29.0 +0100 +++ linux-2.6.26.ppc64/arch/powerpc/kernel/vmlinux.lds.S 2008-08-28 15:39:14.0 +0100 @@ -35,10 +35,11 @@ SECTIONS ALIGN_FUNCTION(); *(.text.head) _text = .; - *(.text .fixup .text.init.refok .exit.text.refok) + TEXT_TEXT SCHED_TEXT LOCK_TEXT KPROBES_TEXT + *(.fixup) #ifdef CONFIG_PPC32 *(.got1) -- David WoodhouseOpen Source Technology Centre David.Woodhouse at intel.com Intel Corporation ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] usb: add Freescale QE/CPM USB peripheral controller driver
On Thu, 28 Aug 2008, Arnd Bergmann wrote: Not addressing this driver in particular, but the USB gadget layer in general: This is a horrible interface, since every gadget driver exports the same symbols, you can never build a kernel that includes more than one gadget driver. Even if the drivers are all built as modules, simply loading one of them prevents loading another one. This was done deliberately. The relevant standards state that a USB device can have no more than one peripheral interface. Now, I don't claim to fully support this decision. But there is a reason behind it; it's not just a case of bad design. Alan Stern ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: RFC: Could cpm2_clk_setup and cpm2_set_pin be exported ?
On Thu, Aug 28, 2008 at 05:57:13PM +0200, Laurent Pinchart wrote: I'm facing a situation where I need to call cpm2_clk_setup and cpm2_set_pin from a device driver compiled as a module. Before submitting a patch to export both functions, I'd like to make sure there isn't a cleaner way to implement the desired functionality without calling functions that are supposed to be used by board setup code. Have you looked at using the GPIO API? -Scott ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: RFC: Could cpm2_clk_setup and cpm2_set_pin be exported ?
Laurent Pinchart wrote: On Thursday 28 August 2008, Scott Wood wrote: On Thu, Aug 28, 2008 at 05:57:13PM +0200, Laurent Pinchart wrote: I'm facing a situation where I need to call cpm2_clk_setup and cpm2_set_pin from a device driver compiled as a module. Before submitting a patch to export both functions, I'd like to make sure there isn't a cleaner way to implement the desired functionality without calling functions that are supposed to be used by board setup code. Have you looked at using the GPIO API? Yes, but the GPIO API doesn't support dedicated pin usage. Basically all I can do is configure a pin as a general purpose input or output, and set its level when configured as an output. The GPIO API doesn't provide any way to access the PAR and SOR registers. OK, wasn't sure what it was that you needed to set at runtime. Are you actually switching between dedicated functions dynamically? Why do you need to do this? Beside, the GPIO API won't help configuring clock routing. Why does the clock routing need to change dynamically? If it turns out this really does need to happen, we can add some locks and export the functions, but I'd like to hear more about the use case first. -Scott ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [RFC][USB] powerpc: Workaround for the PPC440EPX USBH_23 errata
В Wed, 27 Aug 2008 10:36:20 -0400 (EDT)
Alan Stern [EMAIL PROTECTED] пишет:
On Wed, 27 Aug 2008, Vitaly Bordug wrote:
A published errata for ppc440epx states, that when running Linux
with both EHCI and OHCI modules loaded, the EHCI module experiences
a fatal error when a high-speed device is connected to the USB2.0,
and functions normally if OHCI module is not loaded.
Quote from original descriprion:
The 440EPx USB 2.0 Host controller is an EHCI compliant
controller. In USB 2.0 Host controllers, each EHCI controller has
one or more companion controllers, which may be OHCI or UHCI. An
USB 2.0 Host controller will contain one or more ports. For each
port, only one of the controllers is connected at any one time. In
the 440EPx, there is only one OHCI companion controller, and only
one USB 2.0 Host port. All ports on an USB 2.0 controller default
to the companion controller. If you load only an ohci driver, it
will have control of the ports and any deviceplugged in will
operate, although high speed devices will be forced to operate at
full speed. When an ehci driver is loaded, it explicitly takes
control of the ports. If there is a device connected, and / or
every time there is a new device connected, the ehci driver
determines if the device is high speed or not. If it is high
speed, the driver retains control of the port. If it is not, the
driver explicitly gives the companion controller control of the
port.
This doesn't explain why the fatal error occurs.
On certain 44x set of SoCs, only one controller is able to function,
e.g. technically they are mutually exclusive.
There used to be recommendation to use only hi-speed or full-speed
devices with specific conditions, when respective module was unloaded.
Later, it was observed that ohci suspend is enough to keep things
going, and it was turned into workaround, as explained below.
The is a software workaround that uses
Initial version of the software workaround was posted to
linux-usb-devel:
http://www.mail-archive.com/[EMAIL PROTECTED]/msg54019.html
and later available from amcc.com:
http://www.amcc.com/Embedded/Downloads/download.html?cat=1family=15ins=2
The patch below is generally based on the latter, but reworked to
powerpc/of_device USB drivers, and uses a few devicetree inquiries
to get rid of all the hardcoded defines and CONFIG_* stuff, in
favor to defining specific quirk. The latter required to add more
accurate description into compatible field of USB node for
'sequioia' board.
I have some doubts about parts of this patch.
What stands about noted gotchas, I agree and will fix them, thanks for
looking through the patch. I agree this doesn't look pleasant, but
unfortunately is the only way to say use USB keyboard, and hi-speed
memory stick at the same time.
--- a/drivers/usb/host/ehci-ppc-of.c
+++ b/drivers/usb/host/ehci-ppc-of.c
@@ -107,16 +107,17 @@ ehci_hcd_ppc_of_probe(struct of_device *op,
const struct of_device_id *match) {
struct device_node *dn = op-node;
struct usb_hcd *hcd;
- struct ehci_hcd *ehci;
+ struct ehci_hcd *ehci = NULL;
struct resource res;
int irq;
int rv;
+ struct device_node *np;
+
if (usb_disabled())
return -ENODEV;
dev_dbg(op-dev, initializing PPC-OF USB Controller\n);
-
rv = of_address_to_resource(dn, 0, res);
if (rv)
return rv;
@@ -125,6 +126,7 @@ ehci_hcd_ppc_of_probe(struct of_device *op,
const struct of_device_id *match) if (!hcd)
return -ENOMEM;
+
Is there any reason for these apparently gratuitous whitespace
changes?
hcd-rsrc_start = res.start;
hcd-rsrc_len = res.end - res.start + 1;
@@ -172,6 +174,21 @@ ehci_hcd_ppc_of_probe(struct of_device *op,
const struct of_device_id *match) rv ? NOT : );
}
+ np = of_find_compatible_node(NULL, NULL,
ibm,usb-ohci-440epx);
+ if (np != NULL) {
+ /* claim we really affected by usb23 erratum */
+ ehci-has_amcc_usb23 = 1;
+ if (!of_address_to_resource(np, 0, res))
+ ehci-ohci_hcctrl_reg = ioremap(res.start +
+ OHCI_HCCTRL_OFFSET,
OHCI_HCCTRL_LEN);
+ else
+ pr_debug(__FILE__ : no ohci offset in
fdt\n);
+ if (!ehci-ohci_hcctrl_reg) {
+ pr_debug(__FILE__ : ioremap for ohci
hcctrl failed\n);
+ return -ENOMEM;
This is a memory leak.
--- a/drivers/usb/host/ehci.h
+++ b/drivers/usb/host/ehci.h
@@ -809,6 +819,20 @@ static inline u32 hc32_to_cpup (const struct
ehci_hcd *ehci, const __hc32 *x) le32_to_cpup((__force __le32 *)x);
}
+static inline void set_ohci_hcfs(struct ehci_hcd *ehci, int
operational) +{
+ u32 hc_control;
+
+ hc_control = (readl_be(ehci-ohci_hcctrl_reg)
~OHCI_CTRL_HCFS);
+ if
Re: [RFC][USB] powerpc: Workaround for the PPC440EPX USBH_23 errata
On Thu, 28 Aug 2008, Vitaly Bordug wrote: This doesn't explain why the fatal error occurs. On certain 44x set of SoCs, only one controller is able to function, e.g. technically they are mutually exclusive. There used to be recommendation to use only hi-speed or full-speed devices with specific conditions, when respective module was unloaded. Later, it was observed that ohci suspend is enough to keep things going, and it was turned into workaround, as explained below. Okay, good. _This_ is what you should have put in the patch description, instead of all that other stuff. I have some doubts about parts of this patch. What stands about noted gotchas, I agree and will fix them, thanks for looking through the patch. I agree this doesn't look pleasant, but unfortunately is the only way to say use USB keyboard, and hi-speed memory stick at the same time. Your original post mentioned that the 440EPx has only one USB 2.0 host port. Then how can you use a keyboard and memory stick at the same time? You'd have to plug them into a hub -- in which case only one controller would be needed, the one driving the hub. The patch would be unnecessary. I doubt this will interact properly with usbcore and the rest of ohci-hcd. They do not expect the root hub to be suspended unless they know about it. I need to reemphasize, that upper is not normal, but unfortunately the only way to have both full and hi-speed usb live in peace with 44xEPX family. Quirks are not going to be executed on other chip anyway, and on chip in question, it was tested and works stable enough. I can add an explicit warning, that workaround is being utilised, to make things clear if it will be considered appropriate. Do these systems support CONFIG_PM? If they do then your patch shouldn't be needed, because then ohci-hcd will automatically suspend the OHCI root hub 1 second after the last full/low-speed device is unplugged. You could reduce that 1 second value if you wanted to decrease the probability of conflicts. Alan Stern ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] usb: add Freescale QE/CPM USB peripheral controller driver
On Thu, 28 Aug 2008, Scott Wood wrote: Alan Stern wrote: This was done deliberately. The relevant standards state that a USB device can have no more than one peripheral interface. Does building a kernel image that can run on different hardware without rebuilding also violate the relevant standards? No. That isn't what Arnd was concerned about. He noted that even if you did build multiple modules, only one of them could be loaded at any time. And who's to say that there aren't multiple USB devices on a single board, that just happen to share a CPU and memory? :-) That's why I don't fully support this decision. But I wanted to point out that there _was_ a conscious decision, as opposed to bad programming through sheer carelessness. Alan Stern ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH v2] POWERPC: Allow 32-bit pgtable code to support 36-bit physical
Becky Bruce wrote: On Aug 28, 2008, at 11:07 AM, Scott Wood wrote: Becky Bruce wrote: I'm pretty sure I went through this in great detail at one point and concluded that I did in fact need the lwarx/stwcx. IIRC, it has to do with other non-set_pte_at writers not necessarily holding the page table lock. FYI, the existing 32-bit PTE code is doing atomic updates as well. But will those updates happen if there isn't already a valid PTE? I understand what you're saying, I've been here before :) However, I was never able to convince myself that it's safe without the lwarx/stwcx. There's hashing code that wanks around with the HASHPTE bit doing a RMW without holding any lock (other than lwarx/stwcx-ing the PTE itself). OK. I was concerned not just about efficiency, but of the safety of the stw write if there were other modifications going on (even if the set_pte_at stwcx fails, the other updater could have lwarxed an succesfully stwcxed after the stw and ended up with a mixed PTE), but it may not be an issue depending on the nature of the updates. About PTE_ATOMIC_UPDATES, I didn't add that in because hashed page table implementations require atomic updates. Right, I misread it and thought it was being used for non-hashed implementations as well. What happens if you enable 64-bit PTEs on a 603-ish CPU? The kconfig seems to allow it. Don't do that :) That's why the help is there in the Kconfig. People will do it anyway -- and there's multiplatform to consider. Otherwise, I have to list out every 74xx part that supports 36-bit physical addressing. In any event, nothing interesting will happen other than that you'll waste some space. The kernel boots fine with a non-36b physical u-boot and small amounts of RAM. My concern was the generic code trying to use 64-bit PTEs, and the 603 TLB miss handlers continuing to assume that the PTEs are 32-bit, and loading the wrong thing. Wasted space alone is an acceptable consequence of turning on things you don't need. :-) I'm still not sure where you're going with this - I can remove 44x from the conditional part, but we still have to deal with e500 and 6xx. You still need it in depends (in the absence of a PHYS_64BIT_CAPABLE or some such), but not bool '...' if. It's not a big deal, just a pet peeve. In which case you're now setting this in different places for difft plats, making it potentially harder to read. Unless you're suggesting allowing the selection of PHYS_64BIT on any platform No, unless the code for all platforms makes it safe to do so. -Scott ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [RFC][USB] powerpc: Workaround for the PPC440EPX USBH_23 errata
Alan Stern wrote: Your original post mentioned that the 440EPx has only one USB 2.0 host port. Then how can you use a keyboard and memory stick at the same time? You'd have to plug them into a hub -- in which case only one controller would be needed, the one driving the hub. The patch would be unnecessary. I have one of these processors on a Sequoia board. What happens is that if you build the kernel with both EHCI and OHCI support, then plug in a modern USB memory stick, it initially tries EHCI, the driver fails, and the whole thing falls back to OHCI. So you wind up running at 12 Mbps. The only way to make high speed work is to turn off the OHCI driver, and then you cannot support slow devices with that kernel. So, hile you cannot plug two devices in at one time, you can plug in different speed devices at different times, and my understanding is that this patch will let that work seamlessly. I doubt this will interact properly with usbcore and the rest of ohci-hcd. They do not expect the root hub to be suspended unless they know about it. I need to reemphasize, that upper is not normal, but unfortunately the only way to have both full and hi-speed usb live in peace with 44xEPX family. Quirks are not going to be executed on other chip anyway, and on chip in question, it was tested and works stable enough. I can add an explicit warning, that workaround is being utilised, to make things clear if it will be considered appropriate. Do these systems support CONFIG_PM? If they do then your patch shouldn't be needed, because then ohci-hcd will automatically suspend the OHCI root hub 1 second after the last full/low-speed device is unplugged. You could reduce that 1 second value if you wanted to decrease the probability of conflicts. Alan Stern ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH v2] POWERPC: Allow 32-bit pgtable code to support 36-bit physical
Great, so *you* got my email, and I did not. I love our mailserver! On Aug 28, 2008, at 3:28 PM, Scott Wood wrote: Becky Bruce wrote: On Aug 28, 2008, at 11:07 AM, Scott Wood wrote: Becky Bruce wrote: I'm pretty sure I went through this in great detail at one point and concluded that I did in fact need the lwarx/stwcx. IIRC, it has to do with other non-set_pte_at writers not necessarily holding the page table lock. FYI, the existing 32-bit PTE code is doing atomic updates as well. But will those updates happen if there isn't already a valid PTE? I understand what you're saying, I've been here before :) However, I was never able to convince myself that it's safe without the lwarx/ stwcx. There's hashing code that wanks around with the HASHPTE bit doing a RMW without holding any lock (other than lwarx/stwcx-ing the PTE itself). OK. I was concerned not just about efficiency, but of the safety of the stw write if there were other modifications going on (even if the set_pte_at stwcx fails, the other updater could have lwarxed an succesfully stwcxed after the stw and ended up with a mixed PTE), but it may not be an issue depending on the nature of the updates. It shouldn't be an issue - set_pte_at() is the only writer of the high bits of the PTE, the pte is invalid upon entry to set_pte_at(), and none of the potential interfering writers should be turning on the valid bit. About PTE_ATOMIC_UPDATES, I didn't add that in because hashed page table implementations require atomic updates. Right, I misread it and thought it was being used for non-hashed implementations as well. What happens if you enable 64-bit PTEs on a 603-ish CPU? The kconfig seems to allow it. Don't do that :) That's why the help is there in the Kconfig. People will do it anyway -- and there's multiplatform to consider. Otherwise, I have to list out every 74xx part that supports 36-bit physical addressing. In any event, nothing interesting will happen other than that you'll waste some space. The kernel boots fine with a non-36b physical u-boot and small amounts of RAM. My concern was the generic code trying to use 64-bit PTEs, and the 603 TLB miss handlers continuing to assume that the PTEs are 32-bit, and loading the wrong thing. Wasted space alone is an acceptable consequence of turning on things you don't need. :-) Actually, I'm lying to you - I forgot about the old parts with DTLB/ ITLB handlers. That code will actually break, and I'd rather not hack it up to pointlessly accomodate large PTEs. I do need to fix this the Kconfig, even though it's going to be gross. Thanks for pointing this out. I'm still not sure where you're going with this - I can remove 44x from the conditional part, but we still have to deal with e500 and 6xx. You still need it in depends (in the absence of a PHYS_64BIT_CAPABLE or some such), but not bool '...' if. It's not a big deal, just a pet peeve. I'll look at making it less peevy :) In which case you're now setting this in different places for difft plats, making it potentially harder to read. Unless you're suggesting allowing the selection of PHYS_64BIT on any platform No, unless the code for all platforms makes it safe to do so. Thanks! -Becky ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [RFC][USB] powerpc: Workaround for the PPC440EPX USBH_23 errata
On Thu, 28 Aug 2008, Steven A. Falco wrote: Alan Stern wrote: Your original post mentioned that the 440EPx has only one USB 2.0 host port. Then how can you use a keyboard and memory stick at the same time? You'd have to plug them into a hub -- in which case only one controller would be needed, the one driving the hub. The patch would be unnecessary. I have one of these processors on a Sequoia board. What happens is that if you build the kernel with both EHCI and OHCI support, then plug in a modern USB memory stick, it initially tries EHCI, the driver fails, and the whole thing falls back to OHCI. So you wind up running at 12 Mbps. The only way to make high speed work is to turn off the OHCI driver, and then you cannot support slow devices with that kernel. So, hile you cannot plug two devices in at one time, you can plug in different speed devices at different times, and my understanding is that this patch will let that work seamlessly. Is there some reason why it doesn't work already? All the patch does is suspend the OHCI root hub when you plug in the memory stick -- but the root hub should already be suspended. Unless the memory stick is already plugged in when the kernel boots. In which case the root hub won't be suspended -- it won't suspend until 1 second after ohci-hcd initializes the controller. Is that the scenario you're worried about? Alan Stern ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] usb: add Freescale QE/CPM USB peripheral controller driver
On Thursday 28 August 2008, Alan Stern wrote: On Thu, 28 Aug 2008, Scott Wood wrote: Alan Stern wrote: This was done deliberately. The relevant standards state that a USB device can have no more than one peripheral interface. Does building a kernel image that can run on different hardware without rebuilding also violate the relevant standards? No. That isn't what Arnd was concerned about. He noted that even if you did build multiple modules, only one of them could be loaded at any time. Well, actually it was exactly what I was concerned about ;-) The way I understand the code, it is layered into the hardware specific part and the protocol specific part, which are connected through the interfaces I pointed out. The standard requires that there can only be one protocol handler per physical interface, which is a reasonable limitation. However, what the Linux implementation actually enforces is that there can only be one hardware specific driver built or loaded into the kernel, which just looks like an arbitrary restriction that does not actually help. If the gadget hardware drivers were registering the device with a gadget_bus_type, you could still enforce the only one protocol rule by binding every protocol to every device in that bus type. Arnd ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH v2] POWERPC: Allow 32-bit pgtable code to support 36-bit physical
On Aug 28, 2008, at 11:07 AM, Scott Wood wrote: Becky Bruce wrote: I'm pretty sure I went through this in great detail at one point and concluded that I did in fact need the lwarx/stwcx. IIRC, it has to do with other non-set_pte_at writers not necessarily holding the page table lock. FYI, the existing 32-bit PTE code is doing atomic updates as well. But will those updates happen if there isn't already a valid PTE? I understand what you're saying, I've been here before :) However, I was never able to convince myself that it's safe without the lwarx/ stwcx. There's hashing code that wanks around with the HASHPTE bit doing a RMW without holding any lock (other than lwarx/stwcx-ing the PTE itself). And there's definitely code that's fairly far removed from the last time you checked that an entry was valid. I've CC'd Ben on this mail - perhaps he can shed some light. If we don't need the atomics, I'm happy to yank them. Now, it *does* seem like set_pte_at could be optimized for the non-SMP case I'll have to chew on that one a bit. About PTE_ATOMIC_UPDATES, I didn't add that in because hashed page table implementations require atomic updates. Right, I misread it and thought it was being used for non-hashed implementations as well. What happens if you enable 64-bit PTEs on a 603-ish CPU? The kconfig seems to allow it. Don't do that :) That's why the help is there in the Kconfig. Otherwise, I have to list out every 74xx part that supports 36-bit physical addressing. In any event, nothing interesting will happen other than that you'll waste some space. The kernel boots fine with a non-36b physical u-boot and small amounts of RAM. Adding it in would create another clause in that code, because I would still need to order the operations with a 64-bit PTE and I can't call pte_update as it only changes the low word of the pte. I wasn't feeling too keen on adding untested pagetable code into the kernel :) Wimp. :-) Pansy :) I can add it if the peanut gallery wants it, but I'll be marking it with a big fat BUYER BEWARE. No, there's little point if nothing selects it (or is planned to in the near future). Good :) diff --git a/arch/powerpc/platforms/Kconfig.cputype b/arch/ powerpc/platforms/Kconfig.cputype index 7f65127..ca5b58b 100644 --- a/arch/powerpc/platforms/Kconfig.cputype +++ b/arch/powerpc/platforms/Kconfig.cputype @@ -128,18 +128,22 @@ config FSL_EMB_PERFMON config PTE_64BIT bool -depends on 44x || E500 +depends on 44x || E500 || 6xx default y if 44x -default y if E500 PHYS_64BIT +default y if PHYS_64BIT How is this different from PHYS_64BIT? One is the width of the PTE and one is the width of a physical address. It's entirely plausible to have a 64-bit PTE because you have a bunch of status bits, and only have 32-bit physical addressing. That's why there are 2 options. Right, I just didn't see anything that actually selects it independently of PHYS_64BIT. Is this something that's expected to actually happen in the future? There's been talk of making the PTEs always 64-bit even on 32-bit platforms. So it's entirely plausible. The default y if 44x line is redundant with the default y if PHYS_64BIT. You're right, I'll clean that up. config PHYS_64BIT -bool 'Large physical address support' if E500 -depends on 44x || E500 +bool 'Large physical address support' if E500 || 6xx Maybe if !44x, or have 44x select this, rather than listing all arches where it's optional. Not sure exactly what you're suggesting here It just seems simpler to not conditionalize the bool, but instead have CONFIG_44x do select PHYS_64BIT. I'd rather avoid another list of platforms accumulating in a kconfig dependency. I'm still not sure where you're going with this - I can remove 44x from the conditional part, but we still have to deal with e500 and 6xx. In which case you're now setting this in different places for difft plats, making it potentially harder to read. Unless you're suggesting allowing the selection of PHYS_64BIT on any platform (in which case your comment about what happens if I select this on 603 doesn't make much sense). +depends on 44x || E500 || 6xx select RESOURCES_64BIT default y if 44x ---help--- This option enables kernel support for larger than 32-bit physical - addresses. This features is not be available on all e500 cores. + addresses. This features is not be available on all cores. This features is not be? Heh, I didn't type that :) But I can fix it. You didn't type it, but you touched it. Tag, you're it. :-) It's already fixed in my tree :) -B ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] usb: add Freescale QE/CPM USB peripheral controller driver
Alan Stern wrote: This was done deliberately. The relevant standards state that a USB device can have no more than one peripheral interface. Does building a kernel image that can run on different hardware without rebuilding also violate the relevant standards? And who's to say that there aren't multiple USB devices on a single board, that just happen to share a CPU and memory? :-) -Scott ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH v2] POWERPC: Allow 32-bit pgtable code to support 36-bit physical
I understand what you're saying, I've been here before :) However, I was never able to convince myself that it's safe without the lwarx/ stwcx. There's hashing code that wanks around with the HASHPTE bit doing a RMW without holding any lock (other than lwarx/stwcx-ing the PTE itself). And there's definitely code that's fairly far removed from the last time you checked that an entry was valid. I've CC'd Ben on this mail - perhaps he can shed some light. If we don't need the atomics, I'm happy to yank them. Now, it *does* seem like set_pte_at could be optimized for the non-SMP case I'll have to chew on that one a bit. I haven't read the whole discussion not reviewed the patches yet, so I'm just answering to the above sentence before I head off for the week-end (and yes, Becky, I _DO_ have reviewing your stuff high on my todo list, but I've been really swamped those last 2 weeks). So all generic code always accesses PTEs with the PTE lock held (that lock can be in various places ... typically for us it's one per PTE page). 44x and FSL BookE no longer write to the PTEs without that lock anymore and thus don't require atomic access in set_pte and friends. Hash based platforms still do because of -one- thing : the hashing code proper which writes back using lwarx/stwcx. to update _PAGE_ACCESSED, _PAGE_HASHPTE and _PAGE_DIRTY. The hash code does take a global lock to avoid double-hashing of the same page but this isn't something we should use elsewhere. So on hash based platforms, updates of the PTEs are expected to be done atomically. Now if you extend the size of the PTE, hopefully those atomic updates are still necessary but only for the -low- part of the PTE that contains those bits. For the non-SMP case, I think it should be possible to optimize it. The only thing that can happen at interrupt time is hashing of kernel or vmalloc/ioremap pages, which shouldn't compete with set_pte on those pages, so there would be no access races there, but I may be missing something as it's the morning and I about just woke up :-) Cheers, Ben. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [RFC][USB] powerpc: Workaround for the PPC440EPX USBH_23 errata
On Thu, 2008-08-28 at 17:33 -0400, Alan Stern wrote: Is there some reason why it doesn't work already? All the patch does is suspend the OHCI root hub when you plug in the memory stick -- but the root hub should already be suspended. Unless the memory stick is already plugged in when the kernel boots. In which case the root hub won't be suspended -- it won't suspend until 1 second after ohci-hcd initializes the controller. Is that the scenario you're worried about? I suppose some embedded platforms don't use CONFIG_PM, is this still a requirement for autosuspend ? Or do that happen always on an empty port nowadays ? Cheers, Ben. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH 1/2] leds: make the default trigger name const
The default_trigger fields of struct gpio_led and thus struct led_classdev
are pretty much always assigned from a string literal, which means the
string can't be modified. Which is fine, since there is no reason to
modify the string and in fact it never is.
But they should be marked const to prevent such code from being added, to
prevent warnings if -Wwrite-strings is used and when assigned from a
constant string other than a string literal (which produces a warning under
current kernel compiler flags), and for general good coding practices.
Signed-off-by: Trent Piepho [EMAIL PROTECTED]
---
Reposting this again. It still applies to powerpc-next as of Aug 28 and I
don't think anyone had any problems with it.
include/linux/leds.h |4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/linux/leds.h b/include/linux/leds.h
index d41ccb5..d3a73f5 100644
--- a/include/linux/leds.h
+++ b/include/linux/leds.h
@@ -123,7 +123,7 @@ extern void ledtrig_ide_activity(void);
*/
struct led_info {
const char *name;
- char*default_trigger;
+ const char *default_trigger;
int flags;
};
@@ -135,7 +135,7 @@ struct led_platform_data {
/* For the leds-gpio driver */
struct gpio_led {
const char *name;
- char *default_trigger;
+ const char *default_trigger;
unsignedgpio;
u8 active_low;
};
--
1.5.4.3
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
libfdt: Fix bugs in fdt_get_path()
The current implementation of fdt_get_path() has a couple of bugs,
fixed by this patch.
First, contrary to its documentation, on success it returns the length
of the node's path, rather than 0. The testcase is correspondingly
wrong, and the patch fixes this as well.
Second, in some circumstances, it will return -FDT_ERR_BADOFFSET
instead of -FDT_ERR_NOSPACE when given insufficient buffer space.
Specifically this happens when there is insufficient space even to
hold the path's second last component. This behaviour is corrected,
and the testcase updated to check it.
Signed-off-by: David Gibson [EMAIL PROTECTED]
Index: dtc/tests/get_path.c
===
--- dtc.orig/tests/get_path.c 2008-08-29 14:11:09.0 +1000
+++ dtc/tests/get_path.c2008-08-29 14:11:11.0 +1000
@@ -43,6 +43,8 @@ static void check_path_buf(void *fdt, co
memset(buf, POISON, sizeof(buf)); /* poison the buffer */
len = fdt_get_path(fdt, offset, buf, buflen);
+ verbose_printf(get_path() %s - %d - %s\n, path, offset, buf);
+
if (buflen = pathlen) {
if (len != -FDT_ERR_NOSPACE)
FAIL(fdt_get_path([%d bytes]) returns %d with
@@ -51,9 +53,9 @@ static void check_path_buf(void *fdt, co
if (len 0)
FAIL(fdt_get_path([%d bytes]): %s, buflen,
fdt_strerror(len));
- if (len != pathlen)
- FAIL(fdt_get_path([%d bytes]) reports length %d
-instead of %d, buflen, len, pathlen);
+ if (len != 0)
+ FAIL(fdt_get_path([%d bytes]) returns %d
+instead of 0, buflen, len);
if (strcmp(buf, path) != 0)
FAIL(fdt_get_path([%d bytes]) returns \%s\
instead of \%s\, buflen, buf, path);
@@ -70,6 +72,8 @@ static void check_path(void *fdt, const
check_path_buf(fdt, path, pathlen, 1024);
check_path_buf(fdt, path, pathlen, pathlen+1);
check_path_buf(fdt, path, pathlen, pathlen);
+ check_path_buf(fdt, path, pathlen, 0);
+ check_path_buf(fdt, path, pathlen, 2);
}
int main(int argc, char *argv[])
Index: dtc/libfdt/fdt_ro.c
===
--- dtc.orig/libfdt/fdt_ro.c2008-08-29 14:11:11.0 +1000
+++ dtc/libfdt/fdt_ro.c 2008-08-29 14:11:11.0 +1000
@@ -328,9 +328,6 @@ int fdt_get_path(const void *fdt, int no
for (offset = 0, depth = 0;
(offset = 0) (offset = nodeoffset);
offset = fdt_next_node(fdt, offset, depth)) {
- if (pdepth depth)
- continue; /* overflowed buffer */
-
while (pdepth depth) {
do {
p--;
@@ -338,14 +335,16 @@ int fdt_get_path(const void *fdt, int no
pdepth--;
}
- name = fdt_get_name(fdt, offset, namelen);
- if (!name)
- return namelen;
- if ((p + namelen + 1) = buflen) {
- memcpy(buf + p, name, namelen);
- p += namelen;
- buf[p++] = '/';
- pdepth++;
+ if (pdepth = depth) {
+ name = fdt_get_name(fdt, offset, namelen);
+ if (!name)
+ return namelen;
+ if ((p + namelen + 1) = buflen) {
+ memcpy(buf + p, name, namelen);
+ p += namelen;
+ buf[p++] = '/';
+ pdepth++;
+ }
}
if (offset == nodeoffset) {
@@ -355,7 +354,7 @@ int fdt_get_path(const void *fdt, int no
if (p 1) /* special case so that root path is /,
not */
p--;
buf[p] = '\0';
- return p;
+ return 0;
}
}
--
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
