grub-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] Proof of concept interrupt wrapping


From: Vladimir 'φ-coder/phcoder' Serbinenko
Subject: Re: [PATCH] Proof of concept interrupt wrapping
Date: Sun, 17 Jan 2010 21:19:47 +0100
User-agent: Mozilla-Thunderbird 2.0.0.22 (X11/20091109)

Robert Millan wrote:
> 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.
>   
I changed it to _bios_
> 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.
>
>   
I added 32-bit register support since it's needed for APM. Can wrapper
be merged to mainline or is it better for it to be in a separate branch
and be merged with first bunch of BIOS code going out of kernel?


-- 
Regards
Vladimir 'φ-coder/phcoder' Serbinenko

=== modified file 'conf/i386-pc.rmk'
--- conf/i386-pc.rmk    2010-01-15 20:11:51 +0000
+++ conf/i386-pc.rmk    2010-01-17 20:16:27 +0000
@@ -64,7 +64,7 @@
        partition.h msdos_partition.h reader.h symbol.h term.h time.h types.h \
        machine/biosdisk.h machine/boot.h machine/console.h machine/init.h \
        machine/memory.h machine/loader.h machine/vga.h machine/vbe.h \
-       machine/kernel.h machine/pxe.h i386/pit.h list.h handler.h command.h 
i18n.h
+       machine/kernel.h machine/pxe.h i386/pit.h list.h handler.h command.h 
i18n.h machine/int.h
 kernel_img_CFLAGS = $(COMMON_CFLAGS)  $(TARGET_IMG_CFLAGS)
 kernel_img_ASFLAGS = $(COMMON_ASFLAGS)
 kernel_img_LDFLAGS = $(COMMON_LDFLAGS) 
$(TARGET_IMG_LDFLAGS)$(GRUB_KERNEL_MACHINE_LINK_ADDR) $(COMMON_CFLAGS)

=== modified file 'disk/i386/pc/biosdisk.c'
--- disk/i386/pc/biosdisk.c     2010-01-04 23:30:27 +0000
+++ disk/i386/pc/biosdisk.c     2010-01-17 20:16:27 +0000
@@ -19,6 +19,7 @@
 #include <grub/machine/biosdisk.h>
 #include <grub/machine/memory.h>
 #include <grub/machine/kernel.h>
+#include <grub/machine/int.h>
 #include <grub/disk.h>
 #include <grub/dl.h>
 #include <grub/mm.h>
@@ -28,6 +29,59 @@
 #include <grub/term.h>
 
 static int cd_drive = 0;
+static int grub_biosdisk_rw_int13_extensions (int ah, int drive, void *dap);
+
+static int grub_biosdisk_get_num_floppies (void)
+{
+  struct grub_bios_int_registers regs;
+  int drive;
+
+  /* reset the disk system first */
+  regs.eax = 0;
+  regs.edx = 0;
+  regs.flags = GRUB_CPU_INT_FLAGS_DEFAULT;
+
+  grub_bios_interrupt (0x13, &regs);
+
+  for (drive = 0; drive < 2; drive++)
+    {
+      regs.flags = GRUB_CPU_INT_FLAGS_DEFAULT | GRUB_CPU_INT_FLAGS_CARRY;
+      regs.edx = drive;
+
+      /* call GET DISK TYPE */
+      regs.eax = 0x1500;
+      grub_bios_interrupt (0x13, &regs);
+      if (regs.flags & GRUB_CPU_INT_FLAGS_CARRY)
+       break;
+
+      /* check if this drive exists */
+      if (!(regs.eax & 0x300))
+       break;
+    }
+
+  return drive;
+}
+
+/*
+ *   Call IBM/MS INT13 Extensions (int 13 %ah=AH) for DRIVE. DAP
+ *   is passed for disk address packet. If an error occurs, return
+ *   non-zero, otherwise zero.
+ */
+
+static int 
+grub_biosdisk_rw_int13_extensions (int ah, int drive, void *dap)
+{
+  struct grub_bios_int_registers regs;
+  regs.eax = ah << 8;
+  /* compute the address of disk_address_packet */
+  regs.ds = (((grub_addr_t) dap) & 0xffff0000) >> 4;
+  regs.esi = (((grub_addr_t) dap) & 0xffff);
+  regs.edx = drive;
+  regs.flags = GRUB_CPU_INT_FLAGS_DEFAULT;
+
+  grub_bios_interrupt (0x13, &regs);
+  return (regs.eax >> 8) & 0xff;
+}
 
 static int
 grub_biosdisk_get_drive (const char *name)

=== modified file 'include/grub/i386/pc/biosdisk.h'
--- include/grub/i386/pc/biosdisk.h     2009-06-10 21:04:23 +0000
+++ include/grub/i386/pc/biosdisk.h     2009-12-31 11:49:34 +0000
@@ -106,7 +106,6 @@
   grub_uint64_t block;
 } __attribute__ ((packed));
 
-int EXPORT_FUNC(grub_biosdisk_rw_int13_extensions) (int ah, int drive, void 
*dap);
 int EXPORT_FUNC(grub_biosdisk_rw_standard) (int ah, int drive, int coff, int 
hoff,
                               int soff, int nsec, int segment);
 int EXPORT_FUNC(grub_biosdisk_check_int13_extensions) (int drive);
@@ -118,7 +117,6 @@
                                         unsigned long *cylinders,
                                         unsigned long *heads,
                                         unsigned long *sectors);
-int EXPORT_FUNC(grub_biosdisk_get_num_floppies) (void);
 
 void grub_biosdisk_init (void);
 void grub_biosdisk_fini (void);

=== added file 'include/grub/i386/pc/int.h'
--- include/grub/i386/pc/int.h  1970-01-01 00:00:00 +0000
+++ include/grub/i386/pc/int.h  2010-01-17 19:23:55 +0000
@@ -0,0 +1,52 @@
+/*
+ *  GRUB  --  GRand Unified Bootloader
+ *  Copyright (C) 2009  Free Software Foundation, Inc.
+ *
+ *  GRUB is free software: you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation, either version 3 of the License, or
+ *  (at your option) any later version.
+ *
+ *  GRUB is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with GRUB.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef GRUB_INTERRUPT_MACHINE_HEADER
+#define GRUB_INTERRUPT_MACHINE_HEADER  1
+
+#include <grub/symbol.h>
+
+struct grub_bios_int_registers
+{
+  grub_uint32_t eax;
+  grub_uint16_t es;
+  grub_uint16_t ds;
+  grub_uint16_t flags;
+  grub_uint16_t dummy;
+  grub_uint32_t ebx;
+  grub_uint32_t ecx;
+  grub_uint32_t edi;
+  grub_uint32_t esi;
+  grub_uint32_t edx;
+};
+
+#define  GRUB_CPU_INT_FLAGS_CARRY     0x1
+#define  GRUB_CPU_INT_FLAGS_PARITY    0x4
+#define  GRUB_CPU_INT_FLAGS_ADJUST    0x10
+#define  GRUB_CPU_INT_FLAGS_ZERO      0x40
+#define  GRUB_CPU_INT_FLAGS_SIGN      0x80
+#define  GRUB_CPU_INT_FLAGS_TRAP      0x100
+#define  GRUB_CPU_INT_FLAGS_INTERRUPT 0x200
+#define  GRUB_CPU_INT_FLAGS_DIRECTION 0x400
+#define  GRUB_CPU_INT_FLAGS_OVERFLOW  0x800
+#define  GRUB_CPU_INT_FLAGS_DEFAULT   GRUB_CPU_INT_FLAGS_INTERRUPT
+
+void EXPORT_FUNC (grub_bios_interrupt) (grub_uint8_t intno,
+                                       struct grub_bios_int_registers *regs);
+
+#endif

=== modified file 'kern/i386/pc/startup.S'
--- kern/i386/pc/startup.S      2010-01-03 22:05:07 +0000
+++ kern/i386/pc/startup.S      2010-01-17 20:16:27 +0000
@@ -567,44 +567,6 @@
 #include "../loader.S"
 
 /*
- *   int grub_biosdisk_rw_int13_extensions (int ah, int drive, void *dap)
- *
- *   Call IBM/MS INT13 Extensions (int 13 %ah=AH) for DRIVE. DAP
- *   is passed for disk address packet. If an error occurs, return
- *   non-zero, otherwise zero.
- */
-
-FUNCTION(grub_biosdisk_rw_int13_extensions)
-       pushl   %ebp
-       pushl   %esi
-
-       /* compute the address of disk_address_packet */
-       movw    %cx, %si
-       xorw    %cx, %cx
-       shrl    $4, %ecx        /* save the segment to cx */
-
-       /* ah */
-       movb    %al, %dh
-       /* enter real mode */
-       call    prot_to_real
-
-       .code16
-       movb    %dh, %ah
-       movw    %cx, %ds
-       int     $0x13           /* do the operation */
-       movb    %ah, %dl        /* save return value */
-       /* back to protected mode */
-       DATA32  call    real_to_prot
-       .code32
-
-       movb    %dl, %al        /* return value in %eax */
-
-       popl    %esi
-       popl    %ebp
-
-       ret
-
-/*
  *   int grub_biosdisk_rw_standard (int ah, int drive, int coff, int hoff,
  *                                  int soff, int nsec, int segment)
  *
@@ -862,43 +824,6 @@
 
 
 /*
- * int grub_biosdisk_get_num_floppies (void)
- */
-FUNCTION(grub_biosdisk_get_num_floppies)
-       pushl   %ebp
-
-       xorl    %edx, %edx
-       call    prot_to_real
-
-       .code16
-       /* reset the disk system first */
-       int     $0x13
-1:
-       stc
-
-       /* call GET DISK TYPE */
-       movb    $0x15, %ah
-       int     $0x13
-
-       jc      2f
-
-       /* check if this drive exists */
-       testb   $0x3, %ah
-       jz      2f
-
-       incb    %dl
-       cmpb    $2, %dl
-       jne     1b
-2:
-       DATA32  call    real_to_prot
-       .code32
-
-       movl    %edx, %eax
-       popl    %ebp
-       ret
-
-
-/*
  *
  * grub_get_memsize(i) :  return the memory size in KB. i == 0 for conventional
  *             memory, i == 1 for extended memory
@@ -2142,3 +2067,97 @@
        popl    %esi
        popl    %ebp
        ret
+
+FUNCTION(grub_bios_interrupt)
+       pushl    %ebp
+       pushl    %ecx
+       pushl    %eax
+       pushl    %edx
+       
+       movb     %al, intno
+       movl     (%edx), %eax
+       movl     %eax, LOCAL(bios_register_eax)
+       movw     4(%edx), %ax
+       movw     %ax, LOCAL(bios_register_es)
+       movw     6(%edx), %ax
+       movw     %ax, LOCAL(bios_register_ds)
+       movw     8(%edx), %ax
+       movw     %ax, LOCAL(bios_register_flags)
+
+       movl    12(%edx), %ebx
+       movl    16(%edx), %ecx
+       movl    20(%edx), %edi
+       movl    24(%edx), %esi
+       movl    28(%edx), %edx
+
+       call    prot_to_real
+       .code16
+
+       mov     %ds, %ax
+       push    %ax
+
+       /* movw imm16, %ax*/
+       .byte   0xb8
+LOCAL(bios_register_es):
+       .short  0
+       movw    %ax, %es
+       /* movw imm16, %ax*/
+       .byte   0xb8
+LOCAL(bios_register_ds):
+       .short  0
+       movw    %ax, %ds
+
+       /* movw imm16, %ax*/
+       .byte   0xb8
+LOCAL(bios_register_flags):
+       .short  0
+       push    %ax
+       popf
+
+       /* movl imm32, %eax*/
+       .byte   0x66, 0xb8
+LOCAL(bios_register_eax):
+       .long   0
+       
+       /* int imm8.  */
+       .byte   0xcd
+intno: 
+       .byte   0
+
+       movl    %eax, %cs:LOCAL(bios_register_eax)
+       movw    %ds, %ax
+       movw    %ax, %cs:LOCAL(bios_register_ds)
+       pop     %ax
+       mov     %ax, %ds
+       pushf
+       pop     %ax
+       movw    %ax, LOCAL(bios_register_flags)
+       mov     %es, %ax
+       movw    %ax, LOCAL(bios_register_es)
+
+       DATA32  call    real_to_prot
+       .code32
+
+       popl    %eax
+
+       movl    %ebx, 12(%eax)
+       movl    %ecx, 16(%eax)
+       movl    %edi, 20(%eax)
+       movl    %esi, 24(%eax)
+       movl    %edx, 28(%eax)
+
+       movl     %eax, %edx
+
+       movl     LOCAL(bios_register_eax), %eax
+       movl     %eax, (%edx)
+       movw     LOCAL(bios_register_es), %ax
+       movw     %ax, 4(%edx)
+       movw     LOCAL(bios_register_ds), %ax
+       movw     %ax, 6(%edx)
+       movw     LOCAL(bios_register_flags), %ax
+       movw     %ax, 8(%edx)
+
+       popl    %eax
+       popl    %ecx
+       popl    %ebp
+       ret

Attachment: signature.asc
Description: OpenPGP digital signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]