Re: [RFC 5/6] Virtual 8086 mode support for kernel mode drivers.

Jan Klötzke <[email protected]> Tue, 29 Apr 2008 23:25:41 +0200
Newsgroups gmane.os.openbeos.kernel.devel
Message-ID <[email protected]>
Am Dienstag, 29. April 2008 schrieb Axel Dörfler:
> Hi Jan,
>
> I've only got some minor comments (just looking through your changes):
>
> Jan Klötzke <[email protected]> wrote:
> > diff --git a/headers/private/kernel/arch/x86/arch_cpu.h b/headers/
> > private/kernel/arch/x86/arch_cpu.h
>
> [...]
>
> > +struct vm86_iframe {
>
> Maybe just inherit from the standard iframe instead?
> (would only force vm86.c to be a .cpp file, but do as you prefer)

While I like the idea I would have to move the definition of the vm86_iframe 
out of arch_cpu.h I guess (made a quick compile test). I'll leave it for the 
moment.

> > --- a/src/system/kernel/arch/x86/arch_interrupts.S
> > +++ b/src/system/kernel/arch/x86/arch_interrupts.S
> > @@ -246,10 +246,10 @@ int_bottom:
> >
> >  	movl	%esp, %ebp		// frame pointer is the iframe
> >
> > +	testl	$0x20000, IFRAME_flags(%ebp)	// VM86 mode
> > +	jnz		int_bottom_vm86
> >  	cmp		$USER_CODE_SEG, IFRAME_cs(%ebp)
> >  	je		int_bottom_user
> > -	testl	$0x20000, IFRAME_flags(%ebp)	// VM86 mode
> > -	jnz		int_bottom_user
>
> This looks like its a patch against a local version, and not the one in
> the repository!

Oops. I'll fix it...

> > +status_t
> > +vm86_prepare(struct vm86_state *state, unsigned int ram_size)
>
> According to our coding style, this would need to be "ramSize" instead.

Ok, I think I deviated on other locations too. I'll have 2nd look at it.

Thanks,
/Jan

-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference 
Don't miss this year's exciting event. There's still time to save $100. 
Use priority code J8TL2D2. 
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone