On Tue, 2006-05-16 at 01:10 -0700, Zachary Amsden wrote: > Rusty Russell wrote: > > I'd expect (1) higher-level additions to paravirt_ops, and (2) binary > > patching similar to the VMI patches for interrupt operations. ... > > +#define IRET jmp *paravirt_ops+PARAVIRT_iret > > +#define CLI pushl %eax; pushl %ecx; pushl %edx; call *paravirt_ops+PARAVIRT_irq_disable; popl %edx; popl %ecx; popl %eax > > +#define STI pushl %eax; pushl %ecx; pushl %edx; call *paravirt_ops+PARAVIRT_irq_enable; popl %edx; popl %ecx; popl %eax > > +#define STI_SYSEXIT jmp *paravirt_ops+PARAVIRT_irq_enable_sysexit > > +#define GET_CR0 pushl %ecx; pushl %edx; call *paravirt_ops+PARAVIRT_read_cr0; popl %edx; popl %ecx > > +#define RDMSR pushl %ecx; pushl %edx; pushl %ecx; call *paravirt_ops+PARAVIRT_read_msr; addl $4, %esp; popl %edx; popl %ecx > > +#define WRMSR pushl %eax; pushl %ecx; pushl %edx; pushl %eax; pushl %ecx; call *paravirt_ops+PARAVIRT_write_msr; addl $8, %esp; popl %edx; popl %ecx > > +#define CPUID pushl %edx; pushl %ecx; pushl %ebx; pushl %eax; leal $16(%esp), %eax; pushl %eax; leal $16(%esp), %eax; pushl %eax; leal $16(%esp), %eax; pushl %eax; leal $16(%esp), %eax; pushl %eax; call *paravirt_ops+PARAVIRT_cpuid; addl $16; popl %eax; popl %ebx; popl %ecx; popl %edx > > + > > I'm pretty uncomfortable with this bit - the extra push / pops here are > really redundant if you implement the CLI/STI in assembly. And it is > pretty clear that they have to be for performance. That said, I'm ok > with this as a transitory step to building a working interface first, > and getting performance optimal second. Indeed, but I think the answer is going to be that any serious implementation will binary patch these, so the fact that the pre-patched code is scarily naive is less of an issue. Meanwhile, it makes implementing them trivial. It's also tempting to say "CLI clobbers %eax" or something, so we can guarantee a free reg for paravirt to use. But that means deeper changes to the existing asm, which is something I'm studiously avoiding. > Also, RDMSR / WRMSR need not be defined for assembly code - this was > legacy from my original patch. Ah, thanks, I'll cut them out! Rusty. -- ccontrol: http://ccontrol.ozlabs.org
