Date: Fri, 27 Oct 2006 09:03:56 +0900 From: Seigo Tanimura <tanimura@tanimura.dyndns.org> To: Hiroki Sato <hrs@FreeBSD.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 Message-ID: <200610270003.k9R03ula032256@silver.tanimura.dyndns.org> In-Reply-To: <20061027.051637.115993470.hrs@allbsd.org> References: <200610150504.k9F548ld008933@repoman.freebsd.org> <20061027.051637.115993470.hrs@allbsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Fri, 27 Oct 2006 05:16:37 +0900 (JST),
Hiroki Sato <hrs@FreeBSD.org> said:
hrs> Seigo Tanimura <tanimura@FreeBSD.org> 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 <tanimura@tanimura.dyndns.org> <tanimura@FreeBSD.org>
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200610270003.k9R03ula032256>