On 12.11.2014 16:42, Gedare Bloom wrote:
On Wed, Nov 12, 2014 at 10:07 AM, Jan Dolezal <dolez...@fel.cvut.cz> wrote:
---
  c/src/lib/libbsp/i386/shared/irq/idt.c | 147 +++++++++++++++++++++++++--------
  c/src/lib/libcpu/i386/cpu.h            |  83 ++++++++++++++++++-
  2 files changed, 194 insertions(+), 36 deletions(-)
diff --git a/c/src/lib/libbsp/i386/shared/irq/idt.c 
b/c/src/lib/libbsp/i386/shared/irq/idt.c
index b79c60a..e6f1d57 100644
--- a/c/src/lib/libbsp/i386/shared/irq/idt.c
+++ b/c/src/lib/libbsp/i386/shared/irq/idt.c
@@ -229,50 +229,33 @@ int i386_get_idt_config (rtems_raw_irq_global_settings** 
config)
    return 1;
  }

-/*
- * Caution this function assumes the GDTR has been already set.
- */
-int i386_set_gdt_entry (unsigned short segment_selector, unsigned base,
-                       unsigned limit)
+int i386_raw_gdt_entry (unsigned short segment_selector_index, 
segment_descriptors* sd)
Break apart lines > 80 characters, or shorten the line (by shortening
variable names).
ok

  {
-    unsigned                   gdt_limit;
+    unsigned                    gdt_limit;
      unsigned short              tmp_segment = 0;
-    unsigned int                limit_adjusted;
-    segment_descriptors*       gdt_entry_tbl;
+    segment_descriptors*        gdt_entry_tbl;
+    unsigned int present;

      i386_get_info_from_GDTR (&gdt_entry_tbl, &gdt_limit);

-    if (segment_selector > limit) {
+    if (segment_selector_index >= (gdt_limit+1)/8) {
+      /* index to GDT table out of bounds */
        return 0;
      }
-    /*
-     * set up limit first
-     */
-    limit_adjusted = limit;
-    if ( limit > 4095 ) {
-      gdt_entry_tbl[segment_selector].granularity = 1;
-      limit_adjusted /= 4096;
+    if (segment_selector_index == 0) {
+      /* index 0 is not usable */
+      return 0;
      }
-    gdt_entry_tbl[segment_selector].limit_15_0  = limit_adjusted & 0xffff;
-    gdt_entry_tbl[segment_selector].limit_19_16 = (limit_adjusted >> 16) & 0xf;
-    /*
-     * set up base
-     */
-    gdt_entry_tbl[segment_selector].base_address_15_0  = base & 0xffff;
-    gdt_entry_tbl[segment_selector].base_address_23_16 = (base >> 16) & 0xff;
-    gdt_entry_tbl[segment_selector].base_address_31_24 = (base >> 24) & 0xff;
-    /*
-     * set up descriptor type (this may well becomes a parameter if needed)
-     */
-    gdt_entry_tbl[segment_selector].type               = 2;    /* Data R/W */
-    gdt_entry_tbl[segment_selector].descriptor_type    = 1;    /* Code or Data 
*/
-    gdt_entry_tbl[segment_selector].privilege          = 0;    /* ring 0 */
-    gdt_entry_tbl[segment_selector].present            = 1;    /* not present 
*/

+    /* put prepared descriptor into the GDT */
+    present = sd->present;
+    sd->present = 0;
+    gdt_entry_tbl[segment_selector_index] = *sd;
+    sd->present = present;
+    gdt_entry_tbl[segment_selector_index].present = present;
Why write the present field separately from the rest?
It was meant as a protection from using partly written descriptor. While
present bit is set to 0 and an instruction tries to load it, processor
generates exception.

Better code for this to work correctly would be:
    /* put prepared descriptor into the GDT */
    present = sd->present;
    sd->present = 0;
    gdt_entry_tbl[segment_selector_index].present = 0;
    RTEMS_COMPILER_MEMORY_BARRIER();
    gdt_entry_tbl[segment_selector_index] = *sd;
    RTEMS_COMPILER_MEMORY_BARRIER();
    gdt_entry_tbl[segment_selector_index].present = present;
    sd->present = present;

      /*
-     * Now, reload all segment registers so the limit takes effect.
+     * Now, reload all segment registers so that the possible changes takes 
effect.
       */
-
      __asm__ volatile( "movw %%ds,%0 ; movw %0,%%ds\n\t"
                    "movw %%es,%0 ; movw %0,%%es\n\t"
                    "movw %%fs,%0 ; movw %0,%%fs\n\t"
@@ -280,7 +263,101 @@ int i386_set_gdt_entry (unsigned short segment_selector, 
unsigned base,
                    "movw %%ss,%0 ; movw %0,%%ss"
                     : "=r" (tmp_segment)
                     : "0"  (tmp_segment)
-                 );
-
+                 );
      return 1;
  }
+
+inline void i386_fill_segment_desc_base(unsigned base, segment_descriptors* sd)
+{
+    sd->base_address_15_0  = base & 0xffff;
+    sd->base_address_23_16 = (base >> 16) & 0xff;
+    sd->base_address_31_24 = (base >> 24) & 0xff;
+}
+
+inline void i386_fill_segment_desc_limit(unsigned limit, segment_descriptors* 
sd)
Anytime you use inline, you should use static. If the function should
be available to other files, it should go in a header. The same goes
for any non-static function, they should have prototypes declared in a
header.
ok, I made from these non-inline functions.

+{
+    sd->granularity = 0;
+    if (limit > 65535) {
+      sd->granularity = 1;
+      limit /= 4096;
+    }
+    sd->limit_15_0  = limit & 0xffff;
+    sd->limit_19_16 = (limit >> 16) & 0xf;
+}
+
+/*
+ * Caution this function assumes the GDTR has been already set.
+ */
+int i386_set_gdt_entry (unsigned short segment_selector_index, unsigned base,
+                        unsigned limit)
+{
+    segment_descriptors         gdt_entry;
+    memset(&gdt_entry, 0, sizeof(gdt_entry));
+
+    i386_fill_segment_desc_limit(limit, &gdt_entry);
+    i386_fill_segment_desc_base(base, &gdt_entry);
+    /*
+     * set up descriptor type (this may well becomes a parameter if needed)
+     */
+    gdt_entry.type              = 2;    /* Data R/W */
+    gdt_entry.descriptor_type   = 1;    /* Code or Data */
+    gdt_entry.privilege         = 0;    /* ring 0 */
+    gdt_entry.present           = 1;    /* not present */
+
+    /*
+     * Now, reload all segment registers so the limit takes effect.
+     */
+    return i386_raw_gdt_entry(segment_selector_index, &gdt_entry);
+}
+
+unsigned short i386_next_empty_gdt_entry ()
+{
+    unsigned                    gdt_limit;
+    segment_descriptors*        gdt_entry_tbl;
+    /* initial amount of filled descriptors */
+    static unsigned short       segment_selector_index = 2;
+
+    segment_selector_index += 1;
+    i386_get_info_from_GDTR (&gdt_entry_tbl, &gdt_limit);
+    if (segment_selector_index >= (gdt_limit+1)/8) {
+      return 0;
+    }
+    return segment_selector_index;
+}
+
+unsigned short i386_cpy_gdt_entry(unsigned short segment_selector_index, 
segment_descriptors* strucToFill)
Prefer to avoid camelNotation. Also check for lines longer than 80 characters.
ok

Is there a reason that sometimes you are using unsigned, and sometimes
unsigned short? I don't like seeing the mixing of int types, and I
would prefer to see explicit-width (e.g. uint32_t and uint16_t) types
if the width is important.
It is not completely true for this patch, but in my working branch it's done:
The reason is to use the same type of variable as the function's
parameter type the variable goes into. e.g.
i386_get_info_from_GDTR have second parameter of type 'unsigned' so
gdt_limit which goes in there has is also 'unsigned' and
'segment_selector_index' which is compared with gdt_limit is also
unsigned (not in this patch).

I could use explicit-width type as you suggested and then cast it to
(unsigned) or change parameters types of function involved.

'unsigned int' is used for bit fields in segment_descriptors structure,
so I stick with this type when manipulating structure mentioned.

Any ideas are very welcomed.
+{
+    unsigned                    gdt_limit;
+    segment_descriptors*        gdt_entry_tbl;
+
+    i386_get_info_from_GDTR (&gdt_entry_tbl, &gdt_limit);
+
+    if (segment_selector_index >= (gdt_limit+1)/8) {
+      return 0;
+    }
+
+    *strucToFill = gdt_entry_tbl[segment_selector_index];
+    return segment_selector_index;
+}
+
+segment_descriptors* i386_get_gdt_entry(unsigned short segment_selector_index)
+{
+    unsigned                    gdt_limit;
+    segment_descriptors*        gdt_entry_tbl;
+
+    i386_get_info_from_GDTR (&gdt_entry_tbl, &gdt_limit);
+
+    if (segment_selector_index >= (gdt_limit+1)/8) {
+      return 0;
+    }
+    return &gdt_entry_tbl[segment_selector_index];
+}
+
+unsigned i386_limit_gdt_entry(segment_descriptors* gdt_entry)
+{
+    unsigned lim = (gdt_entry->limit_15_0 + (gdt_entry->limit_19_16<<16));
+    if (gdt_entry->granularity) {
+      return lim*4096+4095;
+    }
+    return lim;
+}
diff --git a/c/src/lib/libcpu/i386/cpu.h b/c/src/lib/libcpu/i386/cpu.h
index 4c56731..049a35c 100644
--- a/c/src/lib/libcpu/i386/cpu.h
+++ b/c/src/lib/libcpu/i386/cpu.h
@@ -269,11 +269,92 @@ extern void i386_get_info_from_GDTR 
(segment_descriptors** table,
  extern void i386_set_GDTR (segment_descriptors*,
                            unsigned limit);

+/**
+ * C callable function:
+ * Puts global descriptor @sd to the global descriptor table on index
+ * @segment_selector_index
+ *
+ * @return  0 FAILED out of GDT range or index is 0, which is not valid
+ *                   index in GDT
+ *          1 SUCCESS
+ */
+extern int i386_raw_gdt_entry ( unsigned short segment_selector_index,
+                                segment_descriptors* sd);
+
+/**
+ * C callable function
+ * fills @sd with provided @base in appropriate fields of @sd
+ *
+ * @param base 32-bit address to be set as descriptor's base
+ * @param sd descriptor being filled with @base
+ */
+extern void i386_fill_segment_desc_base(unsigned base, segment_descriptors* 
sd);
+
+/**
+ * C callable function
+ * fills @sd with provided @limit in appropriate fields of @sd
+ * also influences granularity bit
+ *
+ * @param limit 32-bit value representing number of limit bytes
+ * @param sd descriptor being filled with @limit
+ */
+extern void i386_fill_segment_desc_limit(unsigned limit,
+                                        segment_descriptors* sd);
+
  /*
   * C callable function enabling to set up one raw interrupt handler
   */
  extern int i386_set_gdt_entry (unsigned short segment_selector, unsigned base,
-                                            unsigned limit);
+                                unsigned limit);
+
+/**
+ * C callable function returns next empty descriptor in GDT.
+ *
+ * @return  0 FAILED GDT is full
+ *          <1;65535> segment_selector number as index to GDT
+ */
+extern unsigned short i386_next_empty_gdt_entry (void);
+
+/**
+ * Copies GDT entry at index @segment_selector to structure
+ * pointed to by @strucToFill
+ *
+ * @param  segment_selector index to GDT table for specifying descriptor to 
copy
+ * @return  0 FAILED segment_selector out of GDT range
+ *          <1;65535> retrieved segment_selector
+ */
+extern unsigned short i386_cpy_gdt_entry(unsigned short segment_selector,
+                                        segment_descriptors* strucToFill);
+
+/**
+ * Returns pointer to GDT table at index given by @segment_selector
+ *
+ * @param   segment_selector index to GDT table for specifying descriptor to 
get
+ * @return  NULL FAILED segment_selector out of GDT range
+ *          pointer to GDT table at @segment_selector
+ */
+extern segment_descriptors* i386_get_gdt_entry(unsigned short 
segment_selector);
+
+/**
+ * Extracts base address from GDT entry pointed to by @gdt_entry
+ *
+ * @param  gdt_entry pointer to entry from which base should be retrieved
+ * @return base address from GDT entry
+*/
+extern inline void* i386_base_gdt_entry(segment_descriptors* gdt_entry)
+{
+    return (void*)(gdt_entry->base_address_15_0 +
+            (gdt_entry->base_address_23_16<<16) +
+            (gdt_entry->base_address_31_24<<24));
+}
use static inline. There is a macro RTEMS_INLINE_ROUTINE that probably
should be used. also, using bitwise-or instead of addition seems
better for composing this field, although I don't have any
particularly compelling reason.
ok

+
+/**
+ * Extracts limit in bytes from GDT entry pointed to by @gdt_entry
+ *
+ * @param  gdt_entry pointer to entry from which limit should be retrieved
+ * @return limit value in bytes from GDT entry
+ */
+extern unsigned i386_limit_gdt_entry(segment_descriptors* gdt_entry);

  /*
   * See page 11.18 Figure 11-12.
--
1.9.1

_______________________________________________
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel
_______________________________________________
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

Reply via email to