From nobody Wed Feb 14 17:30:46 2024 X-Original-To: freebsd-arm@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4TZlbD66n0z514xL; Wed, 14 Feb 2024 17:30:48 +0000 (UTC) (envelope-from jhb@FreeBSD.org) Received: from smtp.freebsd.org (smtp.freebsd.org [96.47.72.83]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "smtp.freebsd.org", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4TZlbD5g25z4JC0; Wed, 14 Feb 2024 17:30:48 +0000 (UTC) (envelope-from jhb@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1707931848; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=LsxTKj68riQhYjKMYsHXvMxX5HZyz+blVVNaAspRYdQ=; b=Ex/4r+jTh0fbheJXSDpf0vXJmSWa3zv0eAdtzREBVc9imqX573yYCsBPSkdtNYyFTP+y2l W8mzN6zd8JfNUivuYBYzIH3gV5TL4VAatB8VNWDzo/wr+riWLTY5V6DRNJV0mzhJdMqjqT DpnFL5+FDv+lIVZmiW6TnyBFVsRizQpxJQoxBCuuhv3VpEv8e3fh1qWHCGdJqKLSR7j31b d3fV2c9FbZvV9+BkA72QI1GkeXn7oDYZP/S3zfD4epaTnJVM4unSBlvMXSLEQX+EprhDMA xyDDq1dM824qQEO7u1BqqhBy3YmiQTn8G/UPGuvpHcw9ekpiJhWYo8C0ZtK8UA== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1707931848; a=rsa-sha256; cv=none; b=bX3SP/sCs26aosB184r6RqoAkJVaED0EQ4dlOvFqNskl9MO2Yitmz89EBtYII9NSIhttqp LkEW7MWmi3Uj/ckVj0B1kbUSj7NFTguLLMBU+CzpzkDrs4DCpXQ9LFi0Zk8OY0dvdpErGw 4aq/FAJa6SrQw5cmSqA6LDrdeR8GNt7YIMEt+0zIfTPsIHigiV+E+IfUvowFHC+yVuS/MR NvKDsdU17tm5nMX8z2qa0v+E80RWVzjwwp9P5/pxXy+WXvc6mbpWE7Zhl2JIZjffHsKpZF xcDa6sVcIIJnZ8Y8B4LqM35wyEi1lrZRujzC6qlSR4PPPndJ24t3Y0hIk6kfuw== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1707931848; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=LsxTKj68riQhYjKMYsHXvMxX5HZyz+blVVNaAspRYdQ=; b=udUTfm/tGhI+GH+nFU1mm7tEWns6FUgC4KvKRE7Khw/2wSgHKTu0+zn5vSYSIaFEw2cFJI dzpbBAybiOFrwrUeS1uPvvMJNZcqqbxDRaDoWRBM5vk4GgNPq2tzAAgAvcV7tb2fMTkYqd o2HMD4ShtNQfLGCQrn27eq3y+rP9IjYBKXE41Hg1tyo7PxTp5Xr09etxmU/n/t9Gkg/Kz5 1lhCx1UTXMPI4sxyoxzsT+aFRkL4veO2OXOsEy2YT43nnz90+atYBgrE6jJgch3cvF53ds cLc1ILUIhjsNfcyisvEHnuCoGhQL4FnJnigxxfc3t9C8OOCwIkHsVx91U0u7DA== Received: from [IPV6:2601:644:937c:5920:5974:74f9:c690:271e] (unknown [IPv6:2601:644:937c:5920:5974:74f9:c690:271e]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) (Authenticated sender: jhb) by smtp.freebsd.org (Postfix) with ESMTPSA id 4TZlbD1hB6zNJH; Wed, 14 Feb 2024 17:30:48 +0000 (UTC) (envelope-from jhb@FreeBSD.org) Message-ID: Date: Wed, 14 Feb 2024 09:30:46 -0800 List-Id: Porting FreeBSD to ARM processors List-Archive: https://lists.freebsd.org/archives/freebsd-arm List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-freebsd-arm@freebsd.org MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: Recent commits reject RPi4B booting: pcib0 vs. pcib1 "rman_manage_region: request" leads to panic Content-Language: en-US To: Warner Losh Cc: Mark Millard , FreeBSD ARM List , Current FreeBSD References: <76AB969F-5BC5-4116-8AF4-3ED2CABEBBA5.ref@yahoo.com> <76AB969F-5BC5-4116-8AF4-3ED2CABEBBA5@yahoo.com> <1F704317-FDB8-4BDA-8A67-61CF48794DFE@yahoo.com> <9AFDF067-96E4-4E67-90D2-F40DAF3F138F@yahoo.com> <4C279710-5F88-4295-B1A4-7C395F3587E5@yahoo.com> <3A145420-399D-4EBD-9FF4-18924908AB1D@yahoo.com> <1298DF9C-0F82-4567-8E81-7332A608C7FC@yahoo.com> From: John Baldwin In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit On 2/14/24 8:42 AM, Warner Losh wrote: > On Wed, Feb 14, 2024 at 9:08 AM John Baldwin wrote: > >> On 2/12/24 5:57 PM, Mark Millard wrote: >>> On Feb 12, 2024, at 16:36, Mark Millard wrote: >>> >>>> On Feb 12, 2024, at 16:10, Mark Millard wrote: >>>> >>>>> On Feb 12, 2024, at 12:00, Mark Millard wrote: >>>>> >>>>>> [Gack: I was looking at the wrong vintage of source code, predating >>>>>> your changes: wrong system used.] >>>>>> >>>>>> >>>>>> On Feb 12, 2024, at 10:41, Mark Millard wrote: >>>>>> >>>>>>> On Feb 12, 2024, at 09:32, John Baldwin wrote: >>>>>>> >>>>>>>> On 2/9/24 8:13 PM, Mark Millard wrote: >>>>>>>>> Summary: >>>>>>>>> pcib0: mem >> 0x7d500000-0x7d50930f irq 80,81 on simplebus2 >>>>>>>>> pcib0: parsing FDT for ECAM0: >>>>>>>>> pcib0: PCI addr: 0xc0000000, CPU addr: 0x600000000, Size: >> 0x40000000 >>>>>>>>> . . . >>>>>>>>> rman_manage_region: request: start >> 0x600000000, end 0x6000fffff >>>>>>>>> panic: Failed to add resource to rman >>>>>>>> >>>>>>>> Hmmm, I suspect this is due to the way that bus_translate_resource >> works which is >>>>>>>> fundamentally broken. It rewrites the start address of a resource >> in-situ instead >>>>>>>> of keeping downstream resources separate from the upstream >> resources. For example, >>>>>>>> I don't see how you could ever release a resource in this design >> without completely >>>>>>>> screwing up your rman. That is, I expect trying to detach a PCI >> device behind a >>>>>>>> translating bridge that uses the current approach should corrupt >> the allocated >>>>>>>> resource ranges in an rman long before my changes. >>>>>>>> >>>>>>>> That said, that doesn't really explain the panic. Hmm, the panic >> might be because >>>>>>>> for PCI bridge windows the driver now passes RF_ACTIVE and the >> bus_translate_resource >>>>>>>> hack only kicks in the activate_resource method of >> pci_host_generic.c. >>>>>>>> >>>>>>>>> Detail: >>>>>>>>> . . . >>>>>>>>> pcib0: mem >> 0x7d500000-0x7d50930f irq 80,81 on simplebus2 >>>>>>>>> pcib0: parsing FDT for ECAM0: >>>>>>>>> pcib0: PCI addr: 0xc0000000, CPU addr: 0x600000000, Size: >> 0x40000000 >>>>>>>> >>>>>>>> This indicates this is a translating bus. >>>>>>>> >>>>>>>>> pcib1: irq 91 at device 0.0 on pci0 >>>>>>>>> rman_manage_region: request: start 0x1, end 0x1 >>>>>>>>> pcib0: rman_reserve_resource: start=0xc0000000, end=0xc00fffff, >> count=0x100000 >>>>>>>>> rman_reserve_resource_bound: request: [0xc0000000, >> 0xc00fffff], length 0x100000, flags 102, device pcib1 >>>>>>>>> rman_reserve_resource_bound: trying 0xffffffff <0xc0000000,0xfffff> >>>>>>>>> considering [0xc0000000, 0xffffffff] >>>>>>>>> truncated region: [0xc0000000, 0xc00fffff]; size 0x100000 >> (requested 0x100000) >>>>>>>>> candidate region: [0xc0000000, 0xc00fffff], size 0x100000 >>>>>>>>> allocating from the beginning >>>>>>>>> rman_manage_region: request: start >> 0x600000000, end 0x6000fffff >>>>> >>>>> What you later typed does not match: >>>>> >>>>> 0x600000000 >>>>> 0x6000fffff >>>>> >>>>> You later typed: >>>>> >>>>> 0x60000000 >>>>> 0x600fffffff >>>>> >>>>> This seems to have lead to some confusion from using the >>>>> wrong figure(s). >>>>> >>>>>>>> The fact that we are trying to reserve the CPU addresses in the >> rman is because >>>>>>>> bus_translate_resource rewrote the start address in the resource >> after it was allocated. >>>>>>>> >>>>>>>> That said, I can't see why rman_manage_region would actually fail. >> At this point the >>>>>>>> rman is empty (this is the first call to rman_manage_region for >> "pcib1 memory window"), >>>>>>>> so only the check that should be failing are the checks against >> rm_start and >>>>>>>> rm_end. For the memory window, rm_start is always 0, and rm_end is >> always >>>>>>>> 0xffffffff, so both the old (0xc00000000 - 0xc00fffff) and new >> (0x60000000 - 0x600fffffff) >>>>>>>> ranges are within those bounds. >>>>> >>>>> No: >>>>> >>>>> 0xffffffff >>>>> >>>>> .vs (actual): >>>>> >>>>> 0x600000000 >>>>> 0x6000fffff >> >> Ok, then this explains the failure if the "raw" addresses are above 4G. I >> have >> access to an emag I'm currently using to test fixes to pci_host_generic.c >> to >> avoid corrupting struct resource objects. I'll post the diff once I've got >> something verified to work. >> >>> It looks to me like in sys/dev/pci/pci_pci.c the: >>> >>> static void >>> pcib_probe_windows(struct pcib_softc *sc) >>> { >>> . . . >>> pcib_alloc_window(sc, &sc->mem, SYS_RES_MEMORY, 0, 0xffffffff); >>> . . . >>> >>> is just inappropriately restrictive about where in the system >>> address space a PCIe can validly be mapped to on the high end. >>> That, in turn, leads to the rejection on the RPi4B now that >>> the range use is checked. >> >> No, the physical register in PCI-PCI bridges is only 32-bits. Only the >> prefetchable BAR supports 64-bit addresses. This is why the host bridge >> is doing a translation from the CPU side (0x600000000) to the PCI BAR >> addresses (0xc0000000) so that the BAR addresses are down in the 32-bit >> address range. It's also true that many PCI devices only support 32-bit >> addresses in memory BARs. 64-bit BARs are an optional extension not >> universally supported. >> >> The translation here is somewhat akin to a type of MMU where the CPU >> addresses are mapped to PCI addresses. The problem here is that the >> PCI BAR resources need to "stay" as PCI addresses since we depend on >> being able to use rman_get_start/end to get the PCI addresses of >> allocated resources, but pci_host_generic.c currently rewrites the >> addresses. >> >> Probably I should remove rman_set_start/end entirely (Warner added them >> back in 2004) as the methods don't do anything to deal with the fallout >> that the rman.rm_list linked-list is no longer sorted by address once >> some addresses get rewritten, etc. >> > > At the time, they made sense. Removing it, though may take some doing > since we use it in about 284 places in sys/dev today... Somewhat more > pervasive than I'd have thought they'd be... Eh, I only find the one that I'm now removing: % git grep -E 'rman_set_(start|end)' sys/ sys/dev/pci/pci_host_generic.c: rman_set_start(r, start); sys/dev/pci/pci_host_generic.c: rman_set_end(r, end); sys/kern/subr_rman.c:rman_set_start(struct resource *r, rman_res_t start) sys/kern/subr_rman.c:rman_set_end(struct resource *r, rman_res_t end) sys/sys/rman.h:void rman_set_end(struct resource *_r, rman_res_t _end); sys/sys/rman.h:void rman_set_start(struct resource *_r, rman_res_t _start); Also, I managed to boot the emag I have access to this morning. I had to fix a few other bugs in acpi(4) for my changes in pci_host_generic to work but will post reviews later today. -- John Baldwin