Are my inline assembly functions correct?

[mrjbom]

I decided to use Intel syntax instead of AT&T, so I had to rewrite inline assembly functions a bit.

Code: Select all

inline void outb(uint16_t port, uint8_t byte)
{
    asm volatile ("outb %0, %1" : : "d"(port), "a"(byte) : "memory");
}

inline uint8_t inb(uint16_t port)
{
    uint8_t ret = 0;
    asm volatile ("inb %0, %1" : "=a"(ret) : "d"(port) : "memory");
    return ret;
}

inline void io_wait(void)
{
    outb(0x80, 0);
}

inline bool are_interrupts_enabled(void)
{
    uint32_t flags = 0;
    asm volatile ("pushf\n\t"
                  "pop %0"
                  : "=rm"(flags) : : "memory" );
    return flags & (1 << 9);
}

inline uint32_t save_irqdisable(void)
{
    uint32_t flags = 0;
    asm volatile ("pushf\n\t"
                  "cli\n\t"
                  "pop %0"
                  : "=r"(flags) : : "memory");
    return flags;
}

inline void irqrestore(uint32_t flags)
{
    asm volatile ("push %0\n\t"
                  "popf"
                   : : "r"(flags) : "memory", "cc");
}

inline uint32_t read_cr3(void)
{
    uint32_t cr3 = 0;
    asm volatile ("mov %0, cr3" : "=rm"(cr3) : : "memory");
    return cr3;
}

I would like to make sure that they are written correctly, are there any errors here?

[MichaelPetch]

It looks reasonable if the target is 32-bit code.

[mrjbom]

It looks reasonable if the target is 32-bit code.

Yes, target is i386

[Octocontrabass]

Code: Select all

    asm volatile ("outb %0, %1" : : "d"(port), "a"(byte) : "memory");

GCC might accept "outb" but in Intel syntax the mnemonic is "out" with no suffix. You could replace the "d" constraint with "dN".

Code: Select all

    asm volatile ("inb %0, %1" : "=a"(ret) : "d"(port) : "memory");

GCC might accept "inb" but in Intel syntax the mnemonic is "in" with no suffix. You could replace the "d" constraint with "dN".

Code: Select all

inline bool are_interrupts_enabled(void)

If you always call this function before disabling interrupts, it doesn't need to be a separate function, you can check the saved EFLAGS value after disabling interrupts.

Code: Select all

    asm volatile ("pushf\n\t"
                  "pop %0"
                  : "=rm"(flags) : : "memory" );

You could use "g" for the constraint instead of "rm". You don't need a "memory" clobber here.

Code: Select all

    asm volatile ("pushf\n\t"
                  "cli\n\t"
                  "pop %0"
                  : "=r"(flags) : : "memory");

The "r" constraint is more limited than necessary. You could use "g" or "rm" instead.

Code: Select all

    asm volatile ("push %0\n\t"
                  "popf"
                   : : "r"(flags) : "memory", "cc");

The "r" constraint is more limited than necessary. You could use "g" or "irm" instead.

Code: Select all

    asm volatile ("mov %0, cr3" : "=rm"(cr3) : : "memory");

The "rm" constraint is invalid, you need to use "r" instead. You shouldn't need a "memory" clobber here.

[mrjbom]

Code: Select all

    asm volatile ("pushf\n\t"
                  "pop %0"
                  : "=rm"(flags) : : "memory" );

You could use "g" for the constraint instead of "rm". You don't need a "memory" clobber here.[/code]

But why don't I need a "memory" clobber here?
Memory operations also take place here.

[MichaelPetch]

Code: Select all

    asm volatile ("pushf\n\t"
                  "pop %0"
                  : "=rm"(flags) : : "memory" );

You could use "g" for the constraint instead of "rm". You don't need a "memory" clobber here.[/code]

But why don't I need a "memory" clobber here?
Memory operations also take place here.

Because The "memory" clobber tells the compiler that the assembly code performs memory reads or writes to items other than those listed in the input and output operands. In the case of pushing and then popping from the stack - memory is written and then read but not in a way that alters any of the existing data objects (variables) known to the compiler. Putting a "memory" clobber here may generate more inefficient code but the code will still work.

I will admit thought I did miss the potential error here:

Code: Select all

asm volatile ("mov %0, cr3" : "=rm"(cr3) : : "memory");

You can't use a memory operand here because mov m32, cr3 is not a valid encoding on the x86. Reading from CR3 doesn't alter memory from the perspective of the compiler so a clobber isn't needed, but if you had written to CR3 then a memory clobber would have been needed.

Note: in the case of in and out instructions you need a memory clobber because the mere act of reading or writing a port MAY alter what appears in memory from the perspective of the compiler especially if dealing with memory mapped IO. Since you don't know the side effects of accessing any given port you need to err on the side of caution and use a memory clobber.

[mrjbom]

You can't use a memory operand here because mov m32, cr3 is not a valid encoding on the x86. Reading from CR3 doesn't alter memory from the perspective of the compiler so a clobber isn't needed, but if you had written to CR3 then a memory clobber would have been needed.

Note: in the case of in and out instructions you need a memory clobber because the mere act of reading or writing a port MAY alter what appears in memory from the perspective of the compiler especially if dealing with memory mapped IO. Since you don't know the side effects of accessing any given port you need to err on the side of caution and use a memory clobber.

In general, is the "memory" clobber needed if there have been changes in memory or registers after the assembly code, right?

[nullplan]

In general, is the "memory" clobber needed if there have been changes in memory or registers after the assembly code, right?

The memory clobber is needed for two things:
For one, it is needed if some memory known to the compiler has changed. This can be stack variables or static memory. Generally something that is not a direct operand of the asm statement. This can be useful if you are in an asm goto statement and need to output some data. This use is pretty rare, though. For example, let's say you want to read esp.

Code: Select all

asm("movl %%esp, %0" : "=rm"(sp));

This is enough. No clobber needed. Even if the compiler allocates sp in memory for some reason, the instruction only writes to a direct operand, which the compiler already knows about.

For two, the clobber is needed if the statement depends on or changes some global state that is hard to quantify and impossible to express in C. For example, there is the compiler barrier

Code: Select all

asm("" ::: "memory");

This prevents the compiler from reordering memory accesses across it. Which can be useful in synchronization primitives. In most architectures, you want a memory barrier for this, though. Notice that x86, while having strong memory ordering by default, still allows loads to be reordered above independent stores. The only way to prevent that is to execute a locked instruction.

Another example would fenv access:

Code: Select all

uint16_t cw = ...; asm("fldcw %0" :: "m"(cw) : "memory");

This tells the compiler that statements afterwards depend on this one, so they cannot be reordered. Which is true: The x87 control word is input to all x87 calculations later on. But there is no way to tell the compiler that it is only floating-point calculations that depend on this, so the memory clobber is the best available thing.

In your case, the pushf modifies memory, yes, but only memory the compiler cannot touch (namely memory below the stack pointer). If you were on x86_64 with red zone, this would be different, but on i386, that memory is always off limits to the compiler.