Page MenuHomeFreeBSD

Fix endianness on FDT read in ARM GIC
ClosedPublic

Authored by zbb on Feb 20 2015, 11:31 AM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Dec 4, 4:13 AM
Unknown Object (File)
Sun, Nov 24, 11:16 AM
Unknown Object (File)
Sat, Nov 23, 1:39 PM
Unknown Object (File)
Oct 4 2024, 5:25 AM
Unknown Object (File)
Oct 4 2024, 12:53 AM
Unknown Object (File)
Oct 2 2024, 8:14 PM
Unknown Object (File)
Oct 2 2024, 3:00 PM
Unknown Object (File)
Sep 30 2024, 5:48 PM

Details

Reviewers
andrew
imp
ian
Summary

Submitted by: Jakub Palider <jpa@semihalf.com>
Obtained from: Semihalf

Diff Detail

Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

zbb retitled this revision from to Fix endianness on FDT read in ARM GIC.
zbb updated this object.
zbb edited the test plan for this revision. (Show Details)
zbb added reviewers: imp, andrew.
zbb added a subscriber: Unknown Object (MLST).

In fact, the opposite is correct. In present, the arguments passed to function are already in suitable format. So, all fdt32_to_cpu() calls should be removed. I remembered some debate with Michal about that that only consumer knows what kind of data come to him, so decoding should be done on his side entirely. However, in current tree, the data are decoded in ofw_bus_intr_to_rl().

Well, Michal pointed me that it works in currnet. So, the true is that interrupt cells are now decoded in ofw_bus_intr_to_rl(), then again in arm_fdt_map_irq(), and then again in gic_decode_fdt(). So I was wrong. However, doing cpu_to_fdt32() three times on same data is not good too.

Yes that is the case here.
We suggest to commit this patch now and think about fixing arm_fdt_map_irq() (and other intr mapping routines) later.
Is there any sense to improve intr.c now (intr_ng is going to replace it)?

This is a good patch. The spec defines that interrupts properties are in 32-bit chunks "encoded as with <encode-int>" (i.e. big-endian and 32-bits at a time), so all endian decoding should be confined to OF_getencprop() calls in ofw_bus_intr_to_rl().

ian added a reviewer: ian.
ian added a subscriber: ian.

This looks right to me (but then... I'm the one who screwed it up in the first place, so don't take my word for it).

I agree that triple-reversing the endianess is strange and we should make sure we get that fixed in intrng.

This revision is now accepted and ready to land.Feb 21 2015, 8:07 PM