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).
> {
> - 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?
> /*
> - * 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.
> +{
> + 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.
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.
> +{
> + 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.
> +
> +/**
> + * 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
