From owner-freebsd-net@FreeBSD.ORG Wed Oct 20 19:21:05 2010 Return-Path: Delivered-To: net@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 3B3F2106566B; Wed, 20 Oct 2010 19:21:05 +0000 (UTC) (envelope-from onemda@gmail.com) Received: from mail-ww0-f50.google.com (mail-ww0-f50.google.com [74.125.82.50]) by mx1.freebsd.org (Postfix) with ESMTP id 97FFF8FC15; Wed, 20 Oct 2010 19:21:04 +0000 (UTC) Received: by wwb13 with SMTP id 13so3168373wwb.31 for ; Wed, 20 Oct 2010 12:21:03 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:mime-version:received:received:in-reply-to :references:date:message-id:subject:from:to:cc:content-type; bh=h8avQu1E+LZG1cmMaDOC2WY+kRCXBBpuOKu0k/rLk4g=; b=fMGj4UPJcSvjXH929AaXQB/FyrwDpQh7TLQdy6FgENqte5kQHlfo36TpsOMLMXEzdn s6thJqMkvijPyMwrRAtbZg2hFOxkOSIpY8jNZ+MPip0Ap42pAgVSKCLi9Pjts3NMS5Na U0EkJvlxInDdcnQbohs2wv+SlBPyVFX9OQKdU= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type; b=aQsrJ9MIqdf7ieIazSDsnU2heCuRgjp4TDp1L4vkoUAJK8q/8eiAA3hfzxFb7glZhK GFlCiKBj/i7RMwUBG0Fkox2mOJlMWmAVIRBCpK3dm36LsAkVcYnYp/d3balUqxR1qb2B a/ric5b371cLzlI5TnVzRNmfhW7TYjvwv0qkQ= MIME-Version: 1.0 Received: by 10.227.147.79 with SMTP id k15mr8195311wbv.128.1287602463075; Wed, 20 Oct 2010 12:21:03 -0700 (PDT) Received: by 10.216.183.146 with HTTP; Wed, 20 Oct 2010 12:21:03 -0700 (PDT) In-Reply-To: References: Date: Wed, 20 Oct 2010 21:21:03 +0200 Message-ID: From: Paul B Mahol To: Bernhard Schmidt Content-Type: text/plain; charset=ISO-8859-1 Cc: net@freebsd.org Subject: Re: ndis: patch for review X-BeenThere: freebsd-net@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Networking and TCP/IP with FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 20 Oct 2010 19:21:05 -0000 On 10/20/10, Bernhard Schmidt wrote: > 9, 2010 at 23:53, Paul B Mahol wrote: >> Hi, >> >> First: we should pin curthread on CPU before we check on which CPU is >> curthread. >> >> Second: instead of sti & cli use critical sections when saving %fs >> register. >> >> Third: I do not know what happens if we get preempted while windows >> code were running, >> so leave critical section only _after_ executing windows code. > > > Do you have a test case for that? If so, can you post it? Unfortunately I do not have test case for third problem. But not using sched_pin or/and critical sections correctly cause NTOS: timer %p fired even though it was canceled in my environment - causing watchdog on ndisX. And attempt to unload miniport driver after that will cause panic because something got corrupted. Theoretically when executing windows code there is small chance for our thread to be preempted and than it will actually cause %fs corruption on CPU we were running, and reading comments it appears FreeBSD use %fs for PCPU. > >> diff --git a/sys/compat/ndis/kern_windrv.c b/sys/compat/ndis/kern_windrv.c >> index f231863..ba29fd2 100644 >> --- a/sys/compat/ndis/kern_windrv.c >> +++ b/sys/compat/ndis/kern_windrv.c >> @@ -613,8 +613,6 @@ struct gdt { >> extern uint16_t x86_getfs(void); >> extern void x86_setfs(uint16_t); >> extern void *x86_gettid(void); >> -extern void x86_critical_enter(void); >> -extern void x86_critical_exit(void); >> extern void x86_getldt(struct gdt *, uint16_t *); >> extern void x86_setldt(struct gdt *, uint16_t); >> >> @@ -668,8 +666,10 @@ extern void x86_setldt(struct gdt *, uint16_t); >> void >> ctxsw_utow(void) >> { >> - struct tid *t; >> + struct tid *t; >> >> + sched_pin(); >> + critical_enter(); >> t = &my_tids[curthread->td_oncpu]; >> >> /* >> @@ -685,12 +685,9 @@ ctxsw_utow(void) >> if (t->tid_self != t) >> x86_newldt(NULL); >> >> - x86_critical_enter(); >> t->tid_oldfs = x86_getfs(); >> t->tid_cpu = curthread->td_oncpu; >> - sched_pin(); >> x86_setfs(SEL_TO_FS(t->tid_selector)); >> - x86_critical_exit(); >> >> /* Now entering Windows land, population: you. */ >> } >> @@ -706,11 +703,10 @@ ctxsw_wtou(void) >> { >> struct tid *t; >> >> - x86_critical_enter(); >> t = x86_gettid(); >> x86_setfs(t->tid_oldfs); >> + critical_exit(); >> sched_unpin(); >> - x86_critical_exit(); >> >> /* Welcome back to UNIX land, we missed you. */ >> >> diff --git a/sys/compat/ndis/winx32_wrap.S b/sys/compat/ndis/winx32_wrap.S >> index c051504..fa4aa87 100644 >> --- a/sys/compat/ndis/winx32_wrap.S >> +++ b/sys/compat/ndis/winx32_wrap.S >> @@ -375,11 +375,3 @@ ENTRY(x86_setfs) >> ENTRY(x86_gettid) >> mov %fs:12,%eax >> ret >> - >> -ENTRY(x86_critical_enter) >> - cli >> - ret >> - >> -ENTRY(x86_critical_exit) >> - sti >> - ret >> _______________________________________________ >> freebsd-net@freebsd.org mailing list >> http://lists.freebsd.org/mailman/listinfo/freebsd-net >> To unsubscribe, send any mail to "freebsd-net-unsubscribe@freebsd.org" >> > > > > -- > Bernhard >