| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Fri, 27 Mar 2009 11:25:30 +0100 (CET)
From: Thomas Gleixner <tglx@...utronix.de>
To: liqin.chen@...plusct.com
cc: linux-arch@...r.kernel.org, linux-kernel@...r.kernel.org,
torvalds@...ux-foundation.org
Subject: Re: [PATCH 9/13] score - New architecure port to SunplusCT S+CORE
processor
On Fri, 27 Mar 2009, liqin.chen@...plusct.com wrote:
> +
> +#include <linux/init.h>
> +#include <linux/interrupt.h>
> +#include <linux/irq.h>
> +#include <linux/kernel_stat.h>
> +#include <linux/proc_fs.h>
> +#include <linux/seq_file.h>
> +
> +extern void interrupt_exception_vector(void);
Move this to a header file please.
> +/*
> + * handles all normal device IRQs
> + */
> +asmlinkage void do_IRQ(int irq)
> +{
> + irq_enter();
> + generic_handle_irq(irq);
> + irq_exit();
> +}
> +
> +/*
> + * on-CPU PIC operations
> + */
> +void ack_bad_irq(unsigned int irq)
> +{
> + printk("unexpected IRQ # %d\n", irq);
> +}
> +
> +static void score_mask(unsigned int irq_nr)
> +{
> + unsigned int irq_source = 63 - irq_nr;
> +
> + if (irq_source < 32)
> + rIMASK_L |= 1 << irq_source;
> + else
> + rIMASK_H |= 1 << (irq_source - 32);
> +}
> +
> +static void score_unmask(unsigned int irq_nr)
> +{
> + unsigned int irq_source = 63 - irq_nr;
> +
> + if (irq_source < 32)
> + rIMASK_L &= ~(1 << irq_source);
> + else
> + rIMASK_H &= ~(1 << (irq_source - 32));
> +}
> +
> +struct irq_chip score_irq_chip = {
> + .name = "Score7-level",
> + .mask = score_mask,
> + .mask_ack = score_mask,
> + .unmask = score_unmask,
> + .end = score_mask,
.end is not needed.
> +};
> +
> +/*
> + * initialise the interrupt system
> + */
> +void __init init_IRQ(void)
> +{
> + int index;
> + unsigned long target_addr;
> +
> + for (index = 0; index < NR_IRQS; ++index)
> + set_irq_chip_and_handler(index, &score_irq_chip,
> + handle_level_irq);
> +
> + for (target_addr = IRQ_VECTOR_BASE_ADDR;
> + target_addr <= IRQ_VECTOR_END_ADDR;
> + target_addr += 0x10)
> + memcpy((void*)target_addr, interrupt_exception_vector,
> 0x10);
Magic number 0x10 ?
> +
> + rIMASK_L=0xffffffff;
> + rIMASK_H=0xffffffff;
lower case variable names please. What's this magic 0xfffffff
initialization ?
> +
> + __asm__ __volatile__(
> + "mtcr %0,cr3\n\t"
> + :: "r" (EXCEPTION_VECTOR_BASE_ADDR | 1));
Please explain what such asm constructs are doing either by adding a
comment or by moving it into an inline function with a self
explanatory function name.
> +
> +void *module_alloc(unsigned long size)
> +{
> + if(size != 0)
missing blank: if (). Please run your patches through checkpatch.pl
> + return vmalloc(size);
> + else
> + return 0;
return NULL;
Please simplify to:
return size ? vmalloc(size) : NULL;
> +
> +void module_arch_cleanup(struct module *mod)
> +{}
either
void func(arg) { }
or
void func(arg)
{
}
Also those empty functions can be implemented as static inline. That
avoids the call to the empty function and gets optimized out by the
compiler.
> +
> +/* If or when software machine-restart is implemented, add code here. */
> +void machine_restart(char *command)
> +{}
See above.
> +/* If or when software machine-halt is implemented, add code here. */
> +void machine_halt(void)
> +{}
> +
> +/* If or when software machine-power-off is implemented, add code here.
> */
> +void machine_power_off(void)
> +{}
> +
> +/*
> + * The idle thread. There's no useful work to be
> + * done, so just try to conserve power and have a
> + * low exit latency (ie sit in a loop waiting for
> + * somebody to say that they'd like to reschedule)
> + */
> +void __noreturn cpu_idle(void)
> +{
> + /* endless idle loop with no priority at all */
> + while (1) {
> + while (!need_resched()) {
> + if (cpu_wait)
> + (*cpu_wait)();
Please initialize cpu_wait with a default function and just call
cpu_wait();
> + }
> + preempt_enable_no_resched();
> + schedule();
> + preempt_disable();
> + }
> +}
> +
> +asmlinkage void ret_from_fork(void);
Declarations in header files please
> +void start_thread(struct pt_regs *regs, unsigned long pc, unsigned long
> sp)
> +{
> + unsigned long status;
> +
> + /* New thread loses kernel privileges. */
> + status = regs->cp0_psr & ~(KU_MASK);
> + status |= KU_USER;
> + regs->cp0_psr = status;
> + regs->cp0_epc = pc;
> + regs->regs[0] = sp;
> +}
> +
> +void exit_thread(void)
> +{}
> +
> +/*
> + * When a process does an "exec", machine state like FPU and debug
> + * registers need to be reset. This is a hook function for that.
> + * Currently we don't have any such state to reset, so this is empty.
> + */
> +void flush_thread(void)
> +{}
> +
> +/*
> + * set up the kernel stack and exception frames for a new process
> + */
> +int copy_thread(int nr, unsigned long clone_flags, unsigned long usp,
> + unsigned long stk_sz, struct task_struct *p, struct pt_regs *regs)
> +{
> + struct thread_info *ti = task_thread_info(p);
> + struct pt_regs *childregs = task_pt_regs(p);
> +
> + p->set_child_tid = p->clear_child_tid = NULL;
> +
> + *childregs = *regs;
> + childregs->regs[7] = 0; /* Clear error flag */
> + childregs->regs[4] = 0; /* Child gets zero as
> return value */
> + regs->regs[4] = p->pid;
> +
> + if (childregs->cp0_psr & 0x8) /* test kernel fork or
> user fork */
> + childregs->regs[0] = usp; /* user fork */
> + else {
> + childregs->regs[28] = (unsigned long) ti; /* kernel fork
> */
> + childregs->regs[0] = (unsigned long) childregs;
> + }
> + p->thread.reg0 = (unsigned long) childregs;
> + p->thread.reg3 = (unsigned long) ret_from_fork;
> + p->thread.cp0_psr = 0;
> +
> + return 0;
> +}
> +
> +/* Fill in the fpu structure for a core dump. */
> +int dump_fpu(struct pt_regs *regs, elf_fpregset_t *r)
> +{
> + return 1;
> +}
> +
> +void elf_dump_regs(elf_greg_t *gp, struct pt_regs *regs)
> +{}
> +
> +int dump_task_regs(struct task_struct *tsk, elf_gregset_t *regs)
> +{
> + return 1;
> +}
> +
> +static void __noreturn kernel_thread_helper(void *unused0, int (*fn)(void
> *), void *arg, void *unused1)
> +{
> + do_exit(fn(arg));
> +}
> +
> +/*
> + * Create a kernel thread.
> + */
> +long kernel_thread(int (*fn)(void *), void *arg, unsigned long flags)
> +{
> + struct pt_regs regs;
> +
> + memset(®s, 0, sizeof(regs));
> +
> + regs.regs[6] = (unsigned long) arg;
> + regs.regs[5] = (unsigned long) fn;
> + regs.cp0_epc = (unsigned long) kernel_thread_helper;
> + regs.cp0_psr = (regs.cp0_psr & ~(0x1|0x4|0x8)) | (regs.cp0_psr &
> 0x3) <<2;
> +
> + return do_fork(flags | CLONE_VM | CLONE_UNTRACED, 0, ®s, 0,
> NULL, NULL);
> +}
> +
> +unsigned long thread_saved_pc(struct task_struct *tsk)
> +{
> + return task_pt_regs(tsk)->cp0_epc;
> +}
> +
> +unsigned long get_wchan(struct task_struct *task)
> +{
> + unsigned long pc = 0;
> +
> + if (!task || task == current || task->state == TASK_RUNNING)
> + goto out;
No goto needed. A simple return 0 is sufficient.
> + if (!task_stack_page(task))
> + goto out;
Ditto
> + pc = task_pt_regs(task)->cp0_epc;
Then this becomes
return task_pt_regs....;
> +out:
> + return pc;
> +}
> +
Thanks,
tglx
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists