[PATCH 3/6] i386: global descriptor table manipulation functions
Jan Dolezal
dolezj21 at fel.cvut.cz
Tue Nov 18 15:46:50 UTC 2014
On 12.11.2014 16:42, Gedare Bloom wrote:
> On Wed, Nov 12, 2014 at 10:07 AM, Jan Dolezal <dolezj21 at 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 at rtems.org
>> http://lists.rtems.org/mailman/listinfo/devel
More information about the devel
mailing list