Re: [PATCH] Proof of concept interrupt wrapping

[Robert Millan <rmh@aybabtu.com>]

On Thu, Dec 31, 2009 at 01:28:30PM +0100, Vladimir 'φ-coder/phcoder' Serbinenko wrote:
> Hello. We were discussing with Robert how to move BIOS interrupt
> routines out of kernel. There are following possibilities:
> 1) Have a .lowmem section in every concerned module which will always be
> placed in low memory. Currently in experimental.
> Advantages:
>   a) moving functions to modules is straightforward
>   b) functions grow neither in size nor in complexity
> Disadvantages:
>   c) needs lowmem allocators in core
> 2) Make every function needing bios interrupts setup its own trampoline.
> Due to complexity of trampolines it's not a real option
> 3) Have an universal routine grub_interrupt (int intno, struct
> grub_cpu_interrupt_regs *regs) which will be used by C routine to do the
> interrupt calls. This would move the complexity from asm to C.
> Advantages:
> a) simplicity in core
> b) complexity moved to a more readable language
> c)  we can also rename grub_interrupt to grub_interrupt_real and make
> grub_interrupt dprintf registers before and after the call. This would
> make debugging BIOS quirks easier.
> Disadvantages:
> a) Moving functions needs effort
> b) C functions are probably bigger but it may be offset by possibility
> of inlining functions
> c) repeadetly changing from/to real mode is an overhead when executing
> multiple interrupts in series. Fortunately this condition is rare in our
> codebase and is only on non performance-critical parts like halting.
> d) Some functions aren't covered by this. At least grub_pxe_call is in
> this case. But we can use method 2 for them

We could diminish #1.c with ifdef GRUB_MACHINE_PCBIOS, but it's still ugly.
I like #3 a lot more.

As for C being bigger than asm, it's argueable, taking into account that
we have regparm, function alignment hacks, and gcc size optimizations :-)

In any case #3 looks a lot cleaner.

Some comments about the patch:

> +struct grub_cpu_int_registers
> +{
> +  grub_uint16_t bx;
> +  grub_uint16_t es;
> +  grub_uint16_t cx;
> +  grub_uint16_t ax;
> +  grub_uint16_t dx;
> +  grub_uint16_t ds;
> +  grub_uint16_t di;
> +  grub_uint16_t flags;
> +  grub_uint16_t si;
> +};

This structure is named in generic way, but its member names are CPU-specific.
Is it useful to make this generic?  In practice, it will be impossible for
CPU-independant code to use it.

Besides, it's biased towards BIOS as the only i386 way.  If, say, some new
i386 firmware provides an interrupt-based callback interface that we're
compelled to support (and YES I really hope this won't happen...), it
wouldn't use i8086 mode at all.

I think it would be better to admit that this is a pure ad-hoc kludge.  It's
not beautiful, but it's *much* better than what we have now.

-- 
Robert Millan

  "Be the change you want to see in the world" -- Gandhi