From owner-cvs-src@FreeBSD.ORG Fri Oct 27 00:04:03 2006 Return-Path: X-Original-To: cvs-src@FreeBSD.org Delivered-To: cvs-src@FreeBSD.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id CD48416A40F; Fri, 27 Oct 2006 00:04:03 +0000 (UTC) (envelope-from tanimura@tanimura.dyndns.org) Received: from silver.tanimura.dyndns.org (fntkngw003003.kngw.fnt.adsl.ppp.infoweb.ne.jp [222.158.252.163]) by mx1.FreeBSD.org (Postfix) with ESMTP id C166A43D5D; Fri, 27 Oct 2006 00:04:02 +0000 (GMT) (envelope-from tanimura@tanimura.dyndns.org) Received: from silver.tanimura.dyndns.org (localhost [IPv6:::1]) by silver.tanimura.dyndns.org (8.13.7/3.7W-carrots-Keikyu-Kurihama) with ESMTP id k9R03ula032256 ; Fri, 27 Oct 2006 09:03:56 +0900 (JST) Message-Id: <200610270003.k9R03ula032256@silver.tanimura.dyndns.org> Date: Fri, 27 Oct 2006 09:03:56 +0900 From: Seigo Tanimura To: Hiroki Sato In-Reply-To: <20061027.051637.115993470.hrs@allbsd.org> References: <200610150504.k9F548ld008933@repoman.freebsd.org> <20061027.051637.115993470.hrs@allbsd.org> User-Agent: Wanderlust/2.14.0 (Africa) SEMI/1.14.6 (Maruoka) FLIM/1.14.8 (=?ISO-8859-4?Q?Shij=F2?=) APEL/10.6 MULE XEmacs/21.4 (patch 19) (Constant Variable) (i386--freebsd) MIME-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka") Content-Type: text/plain; charset=US-ASCII X-Spam-Status: No, score=5.9 required=8.0 tests=AWL,BAYES_50,NO_RECEIVED, NO_RELAYS autolearn=no version=3.1.5 X-Spam-Level: ***** X-Spam-Checker-Version: SpamAssassin 3.1.5 (2006-08-29) on silver.tanimura.dyndns.org Cc: cvs-src@FreeBSD.org, src-committers@FreeBSD.org, re@FreeBSD.org, cvs-all@FreeBSD.org, tanimura@FreeBSD.org Subject: Re: cvs commit: src/sys/pci agp.c X-BeenThere: cvs-src@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: CVS commit messages for the src tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 27 Oct 2006 00:04:03 -0000 On Fri, 27 Oct 2006 05:16:37 +0900 (JST), Hiroki Sato said: hrs> Seigo Tanimura wrote hrs> in <200610150504.k9F548ld008933@repoman.freebsd.org>: ta> tanimura 2006-10-15 05:04:07 UTC ta> ta> FreeBSD src repository ta> ta> Modified files: ta> sys/pci agp.c ta> Log: ta> Fix the wraparound of memsize >=2GB. ta> ta> Revision Changes Path ta> 1.54 +3 -2 src/sys/pci/agp.c hrs> I have doubt about this change because int memsize->u_int memsize hrs> does not solve the problem directly; memsize never occurs wraparound hrs> actually and an implicit cast to unsigned int just makes the problem hrs> invisible. The questionable code fragment in agp.c is the following: hrs> memsize = ptoa(Maxmem) >> 20; hrs> for (i = 0; i < agp_max_size; i++) { hrs> if (memsize <= agp_max[i][0]) hrs> break; hrs> } hrs> ptoa(Maxmem)>>20 will occur a wraparound problem when Maxmem>=2GB, so hrs> this part should be fixed instead. BTW, this should be a problem hrs> only on i386 since the definition of ptoa() is "#define ptoa(x) ((x) hrs> << PAGE_SHIFT)". The other platforms use a cast like "#define hrs> ptoa(x) ((unsigned long)(x) << PAGE_SHIFT)". hrs> I think it can be solved by using "ptoa((unsigned long)Maxmem)" or hrs> so, but I am not sure if this is reasonable because there are more hrs> notional types like vm_paddr_t. If "#define ptoa(x) ((vm_paddr_t)(x) hrs> << PAGE_SHIFT)" works fine on all platforms, it looks more reasonable hrs> to me, but I have a misunderstanding? ptoa() is machine-dependent; (unsigned long) should be OK. (vm_paddr_t) would mess up the namespace. What hassles me is that Maxmem is *NOT* unsigned, grrr... hrs> -- hrs> | Hiroki SATO -- Seigo Tanimura