Re: [PATCH 1/4] ACE documentation

[Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>]

On Sunday 06 May 2007, Wink Saville wrote:
> +Atomic Code Execution (ACE) allows code to execute as if it was
> +surrounded by spin_lock_irqsave and spin_unlock_irqrestore without
> +requiring actual access to the processor flags register.

I guess you mean spin_{un,}lock_irq here, right? The save/restore
part had me confused for quite a while when trying to understand
what you are doing.

> Thus code 
> +executing within the ACE area can also be executed from user space or
> +kernel space. This is accomplished by using spin locks when executing
> +within the ACE area and changes to arch/x86_64/kernel/entry.S such that
> +when an interrupt occurs while executing code in the ACE area that code
> +will be completed before the interrupt is dispatched.

I don't understand how you can write to the spinlock when coming from
user space. If the page is writable, how do you make sure the user can't
write malicious code or data into it?

> +The modifications to entry.S starts at the label ace_common plus the
> +macro HANDLE_ACE immediately following. HANDLE_ACE invokes ace_common
> +at the beginning of each interrupt, if it finds that code in the ACE
> +area was interrupted it causes the ACE code to resume execution prior
> +to handling the interrupt.

You add eight instruction to every interrupt entry, have you measured
whether that makes a difference in timing?

> +The code for the ACE area is in the file arch/x86_64/kernel/ace.S. This
> +file contains all of the code that maybe executed by the kernel or
> +user-space. To be able to share this code with user space the code is
> +also compiled by user space, but this is only to generate the jump
> +table. Other techniques are possible, but this is the current
> +implementation and can be seen in the directory test/ace.

Can't you put this into the vdso? Calling into the right place sounds
like a problem that is already solved.

> +The ACE area is initialized by calling
> +drivers/ace/ace_device.c/ace_init() from init/main.c/start_kernel().

I don't understand what you even want the character device for,
shouldn't the memory just always be there?

> +4. ACE area API Reference
> +
> +ACE supports the following API:
> +
> +uint64_t ace_inc_counter(void)
> +
> +  Increment the counter
> +
> +
> +uint64_t ace_inc_timer_counter(void)
> +
> +  Increment the timer counter
> +
> +
> +void ace_get_counters(uint64_t *counter, uint64_t *timer_counter)
> +
> +  Return the counter and timer counter

None of these functions sound like you need such a complex infrastructure
to do them, setting single values can be done with atomic operations from
user space. Is there anything more complex that you plan to add as the
next step?

	Arnd <><

-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/