From owner-cvs-src@FreeBSD.ORG Tue Nov 30 19:36:50 2004 Return-Path: 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 2EF1A16A4E7; Tue, 30 Nov 2004 19:36:50 +0000 (GMT) Received: from www.cryptography.com (li-22.members.linode.com [64.5.53.22]) by mx1.FreeBSD.org (Postfix) with ESMTP id D5FAF43D7D; Tue, 30 Nov 2004 19:36:49 +0000 (GMT) (envelope-from nate@root.org) Received: from [10.0.0.34] (adsl-67-119-74-222.dsl.sntc01.pacbell.net [67.119.74.222]) by www.cryptography.com (8.12.8/8.12.8) with ESMTP id iAUJakC4009644 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NOT); Tue, 30 Nov 2004 11:36:47 -0800 Message-ID: <41ACCBC4.8030201@root.org> Date: Tue, 30 Nov 2004 11:36:36 -0800 From: Nate Lawson User-Agent: Mozilla Thunderbird 0.9 (Windows/20041103) X-Accept-Language: en-us, en MIME-Version: 1.0 To: John Baldwin References: <200411300655.iAU6th5L066950@repoman.freebsd.org> <200411301253.26797.jhb@FreeBSD.org> In-Reply-To: <200411301253.26797.jhb@FreeBSD.org> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit cc: cvs-src@FreeBSD.org cc: src-committers@FreeBSD.org cc: cvs-all@FreeBSD.org Subject: Re: cvs commit: src/sys/dev/acpica acpi_pci_link.c X-BeenThere: cvs-src@freebsd.org X-Mailman-Version: 2.1.1 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: Tue, 30 Nov 2004 19:36:50 -0000 John Baldwin wrote: > On Tuesday 30 November 2004 01:55 am, Nate Lawson wrote: > >>njl 2004-11-30 06:55:43 UTC >> >> FreeBSD src repository >> >> Modified files: >> sys/dev/acpica acpi_pci_link.c >> Log: >> Make sure the link array is big enough to hold both _CRS and _PRS >> resource lists. It used to be sized based only on _CRS, hence _PRS could >> perform an out-of-bounds access if it was larger (i.e., when there are >> dependent functions). Add asserts to detect this case. Note, this is >> only a temporary fix and I believe _PRS and _CRS should have separate >> arrays. >> >> Also, fix a typo where the wrong irq was being check for the APIC case. >> >> Submitted by: tegge > > > The real fix is not separate arrays per se but real dependent function > support. I agree and that's what the XXX comment says in my commit. However, they may need to be different sizes if _CRS gives us an irq resource and _PRS gives an extended irq resource, not just for DPFs. This could occur if the link was boot-configured in PIC mode and then we went to APIC mode. I think it's best to handle each separately for this case. > We need a resource management concept of resource sets, so that > each set of dependent resources with a dependent start/end pair are > considered a set, and a driver can query 1) how many sets there are and 2) > set the current set of resources. For now, we should just skip dependent functions (both start and end). For PCI irq routing, I've never seen them used for anything. I've only seen them used for multiple resources in the case of _PRS for ACPI devices like lpt and sio, like this for sio: Name (_PRS, ResourceTemplate () { StartDependentFn (0x00, 0x00) { IO (Decode16, 0x03F8, 0x03F8, 0x01, 0x08) IRQNoFlags () {4} } StartDependentFn (0x01, 0x00) { IO (Decode16, 0x02F8, 0x02F8, 0x01, 0x08) IRQNoFlags () {3} } etc. > Trying to use more resources than are in > _CRS is probably a bug and might break _SRS on these machines. Hmm, also, the > code assumes when doing _SRS that the link indices are the resource indices, > so you just broke _SRS for any link devices that have a non-IRQ resource. :( I see. Sorry, I was concerned about data corruption and so didn't go through a longer review/testing process. We should only set irq or extended irq resources in _SRS, no dependent functions. In every _SRS function I've examined (for both ACPI devices and irq links), they examine the resource passed to them at fixed offsets so using a DPF would screw this up. > What I think we should do for now is to only include the first resource set as > the current resources for an ACPI device that has multiple resource sets > instead of just including all the resources. I'll try to work on this. Yes, I think you're saying the same thing as I said above, which is to have a check for only one DPF set (if any) and fail if there is more than one. We should handle this for both _PRS and _CRS (consuming DPFs) and never generate them for _SRS. -- Nate