lkml.org 
[lkml]   [2006]   [Aug]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    SubjectRe: [PATCH 1/4] x86 paravirt_ops: create no_paravirt.h for native ops
    Date

    > ===================================================================
    > --- /dev/null
    > +++ b/include/asm-i386/no_paravirt.h

    I can't say I like the name. After all that should be the normal
    case for a long time now ... native? normal? bareiron?

    Also I would prefer if you split this file up a bit - the old
    processor/system/irqflags split wasn't too bad.



    > +
    > +/*
    > + * Set IOPL bits in EFLAGS from given mask
    > + */
    > +static inline void set_iopl_mask(unsigned mask)

    This function can be completely written in C using local_save_flags()/local_restore_flags()
    Please do that. I guess it's still a good idea to keep it separated
    though because it might allow other optimizations.

    e.g. i've been thinking about special casing IF changes in save/restore flags
    to optimize CPUs which have slow pushf/popf. If you already make sure
    all non IF manipulations of flags are separated that would help.



    > +/* Stop speculative execution */
    > +static inline void sync_core(void)
    > +{
    > + unsigned int eax = 1, ebx, ecx, edx;
    > + __cpuid(&eax, &ebx, &ecx, &edx);
    > +}

    Actually I don't think this one should be para virtualized at all.
    I don't see any reason at all why a hypervisor should trap it and it
    is very time critical. I would recommend you move it back into the
    normal files without hooks.

    > +
    > +/*
    > + * Clear and set 'TS' bit respectively
    > + */

    The comment seems out of date (no set TS)


    > +#define clts() __asm__ __volatile__ ("clts")
    > +#define read_cr0() ({ \
    > + unsigned int __dummy; \
    > + __asm__ __volatile__( \

    Maybe it's just me, but can't you just drop all these __s around
    asm and volatile? They are completely useless as far I know.

    Also the assembly will be easier readable if you just keep it on a single
    line for the simple ones.

    -Andi
    -
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2006-08-07 07:33    [W:2.173 / U:0.432 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site