XHCI is 0xDEAD on FreeBSD about rpi4 HOT 37 CLOSED

Yes, indeed.

Just posted a less terrible patch that roughly does the right thing (well, the thing NetBSD does): https://reviews.freebsd.org/D25201

from rpi4.

That’s awesome. I am happy to hear you folks sorted it out :-).

from rpi4.

If you want to report an issue with RPi4 firmware releases, please report them in the RPi4 firmware repository.

Also, you haven't indicated whether XHCI works with firmware 1.0 or 1.1. What's the last version of the firmware that worked?

from rpi4.

Haven't tried previous versions before.. Same behavior on 1.1.

from rpi4.

Okay, so this isn't a regression.

You do understand that this firmware is very experimental and almost no single OS is expect to work fully at this stage, right? There's a big note indicating so in the README of the project where you can download the firmware as well as in the README of the firmware archive itself (which is the same file)...

from rpi4.

Of course. I'm not asking for you to fix everything, I'm just wondering if you have anything that might help with debugging, if anyone has seen this behavior on any other OS, something like that?

from rpi4.

Can you jump onto the Discord channel? We're actively discussing this (with one of your FreeBSD colleagues)

Either _INI is not being run for the device or your accessing garbage.

Good:

Bad:

from rpi4.

Any thoughts on this?

from rpi4.

From the FreeBSD folks... otherwise I'm just going to close this off a "not a firmware bug"

from rpi4.

Does reading 0x600000000 specifically without doing _INI also result in this?

I have tested with ACPI logging, I think I did see the "xHCI enable" being logged. I am suspicious about the ACPI0004 wrapping, maybe there's something not supported yet that is causing a bug, but the memory address is 0x600000000 in the dmesg…

from rpi4.

I’ve been hacking a BSD PCI-e driver for the Pi4 for a while. I see 0xdeadc0de whenever I try to access certain memory regions mapped to the PCI-e controller when the controller is not fully initialized. I can’t shed a lot of light on it but I definitely see this value a lot.

from rpi4.

Have you tried writing an e-mail to [email protected] ?

from rpi4.

Have you tried writing an e-mail to [email protected] ?

Hi @hselasky, thanks for coming here as an xhci-driver-specialist,
of course we discussed that in [email protected] ,
it's even in the Wiki.
https://wiki.freebsd.org/arm/Raspberry%20Pi#RPI4
https://wiki.freebsd.org/arm/Raspberry%20Pi#RPI4_UEFI ,
So we have misaligned access in kernel space...
@myfreeweb and me didn't know where exactly in the "xhci-inheritance-chain "
we should align.

@kettenis of OpenBSD fixed the QWord-thing in
https://github.com/tianocore/edk2-platforms/blob/master/Platform/RaspberryPi/AcpiTables/Xhci.asl#L118
via :
https://github.com/openbsd/src/blob/master/sys/dev/acpi/dsdt.c
( we don't have that acpica-thing in src afaik)

(OpenBSD)
xhci_acpi_parse_resources(int crsidx, union acpi_resource *crs, void *arg)
{
	struct xhci_acpi_softc *sc = arg;
	int type = AML_CRSTYPE(crs);

	switch (type) {
	case LR_MEM32FIXED:
		/* XHCI registers are specified by the first resource. */
		if (sc->sc_size == 0) {
			sc->sc_addr = crs->lr_m32fixed._bas;
			sc->sc_size = crs->lr_m32fixed._len;
		}
		break;
	// RPI4
	case LR_QWORD:
		/* XHCI registers are specified by the first resource. */
		if (sc->sc_size == 0) {
			sc->sc_addr = crs->lr_qword._min;
			sc->sc_size = crs->lr_qword._len;
		}
		break;
	case LR_EXTIRQ:
		sc->sc_irq = crs->lr_extirq.irq[0];
		sc->sc_irq_flags = crs->lr_extirq.flags;
		break;
	}

	return 0;
}

@RobCrowston has written :
https://reviews.freebsd.org/D25068
-- @zxombie : https://reviews.freebsd.org/D25147

pcie itself, as @andreiw andreiw told us, is not exposed in rpi4UEFi-dev ,
so it belongs to generic_xhci_acpi-driver(which is a 'quite empty' file for now )..
this should only be an assessment of what needs to be done, HPS certainly knows more.
Robert and me have both RPI4 4GB & 8GB-model available for tests, if needed,
Greg at least 4GB ( afaik).
So if you(or Andrew) have code for the possibly simple bugfix to test, please let us know,
thanks!

from rpi4.

All PCI access in XHCI is done using bus_space_XXX functions. If anything is not properly aligned, it should be fixed there?

I think we should transform the ACPI information into regular bus_space compatible memory resources. I unfortunately don't have the full overview how ARM64 and ACPI works.

--HPS

from rpi4.

I think we should transform the ACPI information into regular bus_space compatible memory resources

That's already done automatically by the ACPI code — how else do you think generic_xhci_acpi would be practically empty? :)

the QWord-thing

I was initially skeptical that anything would be wrong with that since — well.. the driver attached and had a memory resource listed with the correct address in the dmesg log, so what could be wrong, everything should be handled fine by generic ACPI code, right? QWordMemory does work fine for PCIe buses after all..

But the resource has indeed been Memory32Fixed on all platforms I know (Ampere eMAG, Marvell Armada8k, NXP LX2160A),
so QWord is the only difference.

Maybe the wrong thing is getting set here or something: sc->sc_io_size = rman_get_size(sc->sc_io_res)

from rpi4.

Thanks, HPS!
.. not really inside of my level of expertise...
HPS and one of @bsdimp , @pgiffuni , @bukinr , @zxombie perhaps ?

.. new release : https://github.com/pftf/RPi4/releases

from rpi4.

Usually you get deadcode and deadbeef when the I/O range is not enabled. May some ACPI method invokation be missing? Also with embedded ARM you need to enable clocks. Do we have a full dump of the ACPI tables?

from rpi4.

Usually you get deadcode and deadbeef when the I/O range is not enabled

Yeah, that's why I was kinda skeptical about alignment.. but I'll investigate how the resource is set up.
If OpenBSD had to modify their stuff specifically for this, maybe there's something to it.

May some ACPI method invokation be missing?

XHCI's _INI does print its debug thing when ACPI_DEBUG is enabled. One of my theories is that its write doesn't actually go through, but idk how likely that is. (maybe @RobCrowston can check using JTAG?)

Do we have a full dump of the ACPI tables?

We even have a source. Been linked above, but again: https://github.com/tianocore/edk2-platforms/tree/master/Platform/RaspberryPi/AcpiTables

from rpi4.

so, QWordMemory in ACPI tables is actually AML_RESOURCE_ADDRESS64 at runtime. Looking at dev/acpica/acpi_resource.c it does seem like granularity/alignment is lost with these Address resources..

(weird, for resources that are not ACPI_ADDRESS_FIXED, it would call set_memoryrange with alignment but that's not implemented)

(well, our bus_space_ does not seem to support alignment at all. and anyway the openbsd thing above is not about alignment!)

from rpi4.

good catch ...
last author of dev/acpica/acpi_resource.c : @emaste , he will do it together with @hselasky , right ? :-)

from rpi4.

oh, haha, I just realized that the reason @klaus4 was talking about alignment so much is the

panic: Misaligned access from kernel space!

caused by accessing 0xDEAD offsets. I've added a check if (XREAD2(sc, capa, XHCI_HCIVERSION) == 0xdead) that doesn't let me get to that :)

Yeah, there are no alignment issues here, and the OpenBSD commit has nothing to do with alignment, seems like their generic ACPI resource support is not great and they had to hack this into XHCI-specific code.

I guess the problem is with the _INI write after all.

from rpi4.

panic: Misaligned access from kernel space!

yep, that's not the trigger itself, just where it panics in xhci0
and logically therefore it can't attach irq (as far as I remember the details).
and yes: @andreiw told us it's the _INI write

.. OpenBSD commit has nothing to do with alignment, seems like their generic ACPI resource > >support is not great ...

Ha Ha :-)
... but great enough support(to the opposite of ours) to boot in ACPI-mode, so let`s please come up with a solution after all :-)

from rpi4.

This looks like the _INI execution (gBcm27xxTokenSpaceGuid.PcdBcm27xxPciRegBase|0xfd500000 + PCIE_EXT_CFG_DATA 0x8000):

  exfldio-0698 ExFieldDatumIo        : Value Written 0000000000000006, Width 1
  exfldio-0705 ExFieldDatumIo        : ----Exit- AE_OK
  exfldio-0812 ExWriteWithUpdateRule : ----Exit- AE_OK
  exfldio-0736 ExWriteWithUpdateRule : ----Entry 000000FF
  exfldio-0798 ExWriteWithUpdateRule : Mask 00000000000000FF, DatumOffset 1, Width 1, Value 0000000000000000, MergedValue 0000000000000000
  exfldio-0520 ExFieldDatumIo        : ----Entry 00000001
  exfldio-0368 ExAccessRegion        : ----Entry
  exfldio-0209 ExSetupRegion         : ----Entry 00000001
  exfldio-0334 ExSetupRegion         : ----Exit- AE_OK
  exfldio-0399 ExAccessRegion        : [WRITE] Region [SystemMemory:0], Width 1, ByteBase 4, Offset 1 at 00000000FD508005
 evregion-0274 EvAddressSpaceDispatch: ----Entry
 nsobject-0457 NsGetSecondaryObject  : ----Entry 0xfffffd0000bf8080
 nsobject-0468 NsGetSecondaryObject  : ----Exit- 0xfffffd0000bf9f00
 evregion-0400 EvAddressSpaceDispatch: Handler 0xfffffd0000bd7000 (@0xffff00000007900c) Address 00000000FD508005 [SystemMemory]
 exregion-0199 ExSystemMemorySpaceHan: ----Entry
 exregion-0323 ExSystemMemorySpaceHan: System-Memory (width 8) R/W 1 Address=00000000FD508005

Significant pause after that last message.

Modifying ACPICA to do 16-bit writes (as if it wasn't AnyAcc but 16-bit access) doesn't help.

from rpi4.

folks you're aware that you can't do 64-bit reads and writes, right? it's a horrible Pi quirk for the PCIe

from rpi4.

let me know if you want this to be exposed via DT property or something (for the ACPI XHCI node)

from rpi4.

I'm not sure anything is trying 64-bit reads?

In the debugger, reads of any width are dead (though idk maybe it reads 64-bit always.. well the xhci driver shouldn't, it uses bus_space_read_{1,2,4}):

db> x/bx 0xffff00004037c000,16
0xffff00004037c000:     ad  de  ad  de  ad  de  ad  de  ad  de  ad  de  ad  de
0xffff00004037c00e:     ad  de  ad  de  ad  de  ad  de
db> x/hx 0xffff00004037c000,16
0xffff00004037c000:     dead    dead    dead    dead    dead    dead    dead
0xffff00004037c00e:     dead    dead    dead    dead    dead    dead    dead
0xffff00004037c01c:     dead    dead    dead    dead    dead    dead    dead
0xffff00004037c02a:     dead
db> x/wx 0xffff00004037c000,16
0xffff00004037c000:     deaddead        deaddead        deaddead        deaddead
0xffff00004037c010:     deaddead        deaddead        deaddead        deaddead
0xffff00004037c020:     deaddead        deaddead        deaddead        deaddead
0xffff00004037c030:     deaddead        deaddead        deaddead        deaddead
0xffff00004037c040:     deaddead        deaddead        deaddead        deaddead
0xffff00004037c050:     deaddead        deaddead
db> x/qx 0xffff00004037c000,16
0xffff00004037c000:     deaddead        deaddead        deaddead        deaddead
0xffff00004037c010:     deaddead        deaddead        deaddead        deaddead
0xffff00004037c020:     deaddead        deaddead        deaddead        deaddead
0xffff00004037c030:     deaddead        deaddead        deaddead        deaddead
0xffff00004037c040:     deaddead        deaddead        deaddead        deaddead
0xffff00004037c050:     deaddead        deaddead

DT property or something (for the ACPI XHCI node)

We still don't have a helper for reading those :D

from rpi4.

Added logging of virtual address to the _INI executor:

 exregion-0323 ExSystemMemorySpaceHan: System-Memory (width 16) R/W 1 Address=00000000FD508004 LogicalAddress=FFFF00005A9FE004

Now I can see the PCIe config memory, I think:

db> x/hx 0xFFFF00005A9FE004,8
0xffff00005a9fe004:     fc67    0       143b    200     0       0       0
0xffff00005a9fe012:     0       0       0       0       0       0       0

hmm, fc67 and 143b look like vendor/device IDs, but they're not tightly packed 16 bit values like in the description:

Field (PCFG, AnyAcc, NoLock, Preserve) {
                VNID, 16, // Vendor ID
                DVID, 16, // Device ID
                CMND, 16, // Command register
                STAT, 16, // Status register
}

from rpi4.

hmm those values don't look like the venid:devid to me :-/.

from rpi4.

oh okay I thought I saw 143b in the UEFI Shell device list but maybe I'm just imagining things.

Maybe this is no longer mapped by the time I'm in the debugger. Or maybe the mapping is not right on arm64. This _INI write is not supposed to take a whole second, right?

from rpi4.

to be fair I am looking at the 8gb device...

from rpi4.

That 143b does seem familiar somehow, though

from rpi4.

oh okay I thought I saw 143b in the UEFI Shell device list but maybe I'm just imagining things.

Maybe this is no longer mapped by the time I'm in the debugger. Or maybe the mapping is not right on arm64. This _INI write is not supposed to take a whole second, right?

No, the delay would imply you're off accessing something completely random, not the correct registers. I've seen that (delay on accessing non-existent register) before.

from rpi4.

(yeah the PCI IDs are the same on 4gb here)

oh, the memory with contents starting with the fc67 is not the beginning of the config space, it's already offset by the register we're writing to.

whoops looks like our pmap_mapbios (used by ACPI memory mapping) on arm64 does not map as device memory! that's not good..

from rpi4.

oh, the memory with contents starting with the fc67 is not the beginning of the config space, it's already offset by the register we're writing to.

I know far less about it than you ... maybe it has to do with the comment from @RobCrowston here : https://reviews.freebsd.org/D25147 ?

from rpi4.

whoops looks like our pmap_mapbios (used by ACPI memory mapping) on arm64 does not map as device memory! that's not good..

oh yeah. Well that was really, really stupid.

Terrible hack patch:

diff --git i/sys/arm64/arm64/pmap.c w/sys/arm64/arm64/pmap.c
index 4ddadb233567..f8677b6207fd 100644
--- i/sys/arm64/arm64/pmap.c
+++ w/sys/arm64/arm64/pmap.c
@@ -5445,6 +5445,33 @@ pmap_mapbios(vm_paddr_t pa, vm_size_t size)
        return ((void *)(va + offset));
 }

+void *pmap_mapacpidev(vm_paddr_t pa, vm_size_t size);
+void *pmap_mapacpidev(vm_paddr_t pa, vm_size_t size)
+{
+       vm_offset_t va, offset;
+       pd_entry_t *pde;
+       int lvl;
+
+       offset = pa & PAGE_MASK;
+       size = round_page(offset + size);
+
+       va = kva_alloc(size);
+       if (va == 0)
+               panic("%s: Couldn't allocate KVA", __func__);
+
+       pde = pmap_pde(kernel_pmap, va, &lvl);
+       KASSERT(lvl == 2, ("pmap_mapbios: Invalid level %d", lvl));
+
+       /* L3 table is linked */
+       va = trunc_page(va);
+       pa = trunc_page(pa);
+       pmap_kenter(va, size, pa, VM_MEMATTR_DEVICE);
+
+       printf("pmap_mapacpidev %lx -> %lx\n", va, pa);
+
+       return ((void *)(va + offset));
+}
+
 void
 pmap_unmapbios(vm_offset_t va, vm_size_t size)
 {
diff --git i/sys/dev/acpica/Osd/OsdMemory.c w/sys/dev/acpica/Osd/OsdMemory.c
index b806642a61fc..385b2aeb6594 100644
--- i/sys/dev/acpica/Osd/OsdMemory.c
+++ w/sys/dev/acpica/Osd/OsdMemory.c
@@ -57,6 +57,11 @@ AcpiOsFree(void *Memory)
 void *
 AcpiOsMapMemory(ACPI_PHYSICAL_ADDRESS PhysicalAddress, ACPI_SIZE Length)
 {
+       if (PhysicalAddress >= 0xfd500000 && PhysicalAddress <= 0xfff00000) {
+               extern void * pmap_mapacpidev(vm_paddr_t pa, vm_size_t size);
+               return (pmap_mapacpidev((vm_offset_t)PhysicalAddress, Length));
+       }
+
     return (pmap_mapbios((vm_offset_t)PhysicalAddress, Length));
 }

Now, to figure out a good way to make AcpiOsMapMemory map stuff as device memory.. (no, naively using VM_MEMATTR_DEVICE everywhere in pmap_mapbios doesn't work at all).

from rpi4.

Oh, looking at NetBSD code: we're supposed to use the EFI memory map to figure these attributes out.

from rpi4.

did you want to tell us with the close button that the machine starts up with your patch in acpi mode?
If so, congratulations!

from rpi4.