<Tartarus>
xypron: But also, why isn't that covered by the lmb?
<Tartarus>
We shouldn't be able to overwrite ourselves at run time
<xypron>
Tartarus: lmb is a sillyness. It does not keep track of memory allocations
<xypron>
Tartarus: lmb should be replaced by proper memory management.
<Tartarus>
malloc area shouldn't be overwritten
<xypron>
Tartarus: who spoke about malloc?
<xypron>
Think of efi_allocate_pages()
<Tartarus>
ok
<xypron>
But why do we relocate initrd at all?
<Tartarus>
Why is that not covered by the existing mechanisms to ensure that we don't overwrite it?
<Tartarus>
To ensure it's not overwritten by the kernel
<Tartarus>
And we need to ensure that you can't overwrite stuff like EFI pages with arbitrary things, for security reasons too
<Tartarus>
Hence that whole series that made it harder to overwrite the running u-boot when loading the kernel/etc
<xypron>
Tartarus: Why should U-Boot take care that the kernel does not overwrite initrd? That is Linux' job.
<Tartarus>
xypron: No, it's our job, especially since Linux can't
<Tartarus>
It's less of an issue with arm64
<Tartarus>
And I think riscv copied that logic in too
<Tartarus>
But historically, oh my, no
<Tartarus>
So many engineering years lost debugging "Oh, the kernel BSS overwrote ..."
<Tartarus>
Which is why disabling initrd relocation in board environment files is allowed, but discouraged
<Tartarus>
But you absolutely cannot disable fdt relocation by default
<xypron>
Tartarus: We pass a kernel and and an initrd that don't overlap. If Linux messes that up, it is a Linux bug.
<Tartarus>
That's what lead us down "oh, libfdt will now do unaligned address safe loads for every access"
<xypron>
Why don't you want to disable fdt relocation?
<xypron>
Who needs it?
<Tartarus>
xypron: Alignment
<xypron>
Our fdt_addr_r values are aligned?
<Tartarus>
There's tons of ways we get non-8-byte aligned device trees
<Tartarus>
fdt_addr_r doesn't matter if it's loaded elsewhere
<Tartarus>
xypron: Anyhow, the big problem is why are those EFI pages not protected by the usual mechanisms to ensure they can't be overwritten with arbitrary data, as that's a security problem these days.
<xypron>
Tartarus: does lmb read the EFI memory map?
<vagrantc>
xypron: i can confirm that setenv initrd_high 0xffffffffffffffff; setenv fdt_high 0xffffffffffffffff fixies it on the odroid-c2!
<vagrantc>
or ... at least ... works around the issue
<Tartarus>
vagrantc: Please do not push that out however in debs, that will lead to other problems.
<vagrantc>
glad to finally have some leads on this issue ... been nagging at me a while
<Tartarus>
xypron: Yes, I suspect the efi subsystem should be calling lmb_reserve somewhere along the way somewhere
<xypron>
Tartarus: on every fs_read() we call fs_read_lmb_check() and recalculate memory reservation. This shows that lmb is of no use.
<xypron>
We should eliminate lmb and use a proper memory management.
<Tartarus>
I mean, I'm not gonna argue lmb is the best way to solve this for forever
<Tartarus>
but there's a problem today and it's both a functional thing and a security thing once someone else notices
<Tartarus>
Then we'll get someone getting a CVE assigned
<Tartarus>
And "everyone" will be wanting to know when we can have this fixed
<Tartarus>
afk dinner
<xypron>
The could we have in lib/efi_loader/efi_memory.c manages memory on 4KiB page granularity. Wouldn't such a page management be enough to replace LMB?
vagrantc has quit [Quit: leaving]
ldevulder has quit [Read error: Connection reset by peer]
ladis has quit [Remote host closed the connection]
ladis has joined #u-boot
teejay has joined #u-boot
<qschulz>
FYI, next is broken for me on Ringneck PX30, bisect returns 0478dac62a9a ("kbuild: Remove uncmd_spl logic")
<qschulz>
(testing my patch series for ringneck support on top of next)
<qschulz>
no serial output at all
<qschulz>
yeah just reverting it fixes my issues
<qschulz>
I guess it might have something to do with TPL_DM not being enabled for my board and the changes made to ns16550 files
jagan has quit [Quit: Connection closed]
___nick___ has joined #u-boot
gsz has joined #u-boot
ikarso has quit [Quit: Connection closed for inactivity]
prabhakarlad has quit [Quit: Client closed]
<xypron>
Tartarus: concerning lmb: my patch set passes Gitlab and I am able to run the UEFI SCT with it. What I don't know is if any board might not boot with current address values due to memory conflicts.
<qschulz>
before the patch, CONFIG_IS_ENABLED(DM_SERIAL) was defined(CONFIG_DM_SERIAL)
<qschulz>
which was set in TPL (though not TPL_DM_SERIAL, but that's the whole point of the bug)
<qschulz>
then it meant it wouldn't match this condition
<qschulz>
which means, the ns16550 struct is made of UART_REG of unsigned char only (8b) instead of u32
<qschulz>
this is essential because of DEBUG_UART_SHIFT that is set to 2 for me
<qschulz>
which means, I need a struct with unsigned char so that the DBUG_UART code can look at the right offset and only read an u8
<qschulz>
Tartarus: ^ since it's the commit you pushed that broke it :)
<qschulz>
sjg1: ^ since I think you are rather familiar with the driver
<qschulz>
(ns16550)
<marex>
the ns16550 is seriously scary code, the ifdeffery is awful
<qschulz>
and you have two drivers :)
<qschulz>
and one of them actually works with DM, with and without OF (I think?) and DEBUG_UART
<Tartarus>
It's all a bit extra fun, yes
<Tartarus>
There's some code for the powerpc SPL case
<Tartarus>
(and powerpc is SPL->TPL->U-Boot)
* marex
rolls eyes
<marex>
Tartarus: it should be TPL->SPL->U-Boot btw ;-)
<Tartarus>
qschulz: You don't want a shift of 4?
<marex>
Tartarus: its just that , PPC has it reversed
<Tartarus>
marex: Well, they came first so everyone else is wrong ;)
ladis has quit [Ping timeout: 252 seconds]
<Tartarus>
But given the extremely limited cycles anyone has for dealing with PowerPC, oh well
<qschulz>
Tartarus: I just want my console to work :)
<marex>
Tartarus: we have kabel and Pali ;-)
<Tartarus>
qschulz: So, in short, yes, there were a few places in the code playing some very tricky games about what is or isn't enabled for the stage of the build we're on
<qschulz>
yeah the logic was pretty much wrong, but it ended up working somehow :/
<Tartarus>
And that serial stuff gave me a good long headache to get apparently only mostly right
<qschulz>
the diff of the Kconfig is correct I believe
<qschulz>
because this option is used in drivers/serial/ns16550.c too, which is guarded by SYS_NS16550 option (not SYS_NS16550_SERIAL, which is for another driver0
<Tartarus>
qschulz: And then which case if that if/else do you end up in?
<Tartarus>
(and element is being stupid about DMs right now :( )
<qschulz>
Tartarus: I need SYS_NS16550_REG_SIZE to be 1 or -1, the sign does not matter
<qschulz>
so it can enter either, does not matter
<qschulz>
(because then you get an array of unsigned char of size 0, so that's fine)
apritzel has quit [Ping timeout: 252 seconds]
<Tartarus>
qschulz: OK, and it really is correct that you're setting CONFIG_SYS_NS16550_MEM32 ?
<Forty-Bot>
marex: and all Pali has done lately is complain that his stuff rots on the mailing list for months :)
<Forty-Bot>
which I imagine is quite demoralizing
Gravis has joined #u-boot
Gravis_ has joined #u-boot
Gravis has quit [Ping timeout: 265 seconds]
<marex>
Forty-Bot: if he fails to CC maintainers, too bad
<Forty-Bot>
is that what's been happening?
<marex>
Forty-Bot: sometimes
<marex>
Forty-Bot: I think its partly fault of get maintainer script in u-boot , which isn't as good as get maintainer script in Linux
<Forty-Bot>
it isn't?
<marex>
Forty-Bot: I think sometimes it doesnt return all the maintainers
<Forty-Bot>
hm
<marex>
still, complaining is only half of the way forward ... finding a solution for the problem is the second part, which seems to be not happening
matthias_bgg has quit [Read error: Connection reset by peer]
<marex>
and really, there is a solution, become a maintainer ;-)
matthias_bgg has joined #u-boot
<Forty-Bot>
yeah, he seems like the person who cares most about ppc and has hardware to test it
<marex>
Forty-Bot: well yeah, because the first Turris was PPC
<marex>
Forty-Bot: the next one was some marvell arm32
<Forty-Bot>
I discovered this week that someone made an amiga "upgrade" which is a PPC computer in an ATX form factor
<Forty-Bot>
pretty neat
<Forty-Bot>
still has at least 2 users, because one of them found a bug in my code >.>
<marex>
amiga wasnt ppc ? or was that m68k ?
<Forty-Bot>
apparenty it's used by AmigaOS 4
<Forty-Bot>
but after commodore went under
<marex>
The Motorola 68000 series of microprocessors is used in all Amiga models from Commodore. -- wiki
<milkylainen>
Amiga after m68k just wasn't as fun anymore. :\
<marex>
right
* milkylainen
fondly remembers his A4000/040.
redbrain has quit [Ping timeout: 272 seconds]
redbrain has joined #u-boot
<sjg1>
qschulz: Tartarus There is NS16550_DYNAMIC which increases code size but avoids the #ifdefs
<sjg1>
It would be possible to remove the code size penalty by having platforms define which options they need to support, and using IS_ENABLED() in serial_out_dynamic() and serial_in_dynamic()
<rfs613>
milkylainen, marex: i still have mine... removed the RTC battery years ago (leaking)...
<rfs613>
also I need to replace the microswitch on the left mouse button... it's a pain to use it as-is
* rfs613
should probaby just get rid of it...
<rfs613>
i doubt my kids will enjoy Lemmings or the Boing demo, the way that I did...
<Tartarus>
marex: I think the big problem with get_maintainers.pl is (a) waiting on Pali or someone else to post the patch that updates our defaults on how much to care about git history and (b) a ton of files without anyone listed
<marex>
Tartarus: the (b) could be inferred from git history, so the blocker is really (a)
<Tartarus>
marex: The complaint is that git history is too random (Pali keeps getting tagged for minor changes he made)
<milkylainen>
rfs613: you have a 4000/040? Nice.
<milkylainen>
rfs613: I sold mine a couple of years after I got it.
<rfs613>
milkylainen: yep. Also had an a500 but got rid of that years ago.
<Forty-Bot>
yeah, git history really needs to be looked at case-by-case
<Forty-Bot>
sometimes a 1-line change is some style thing which was done to 5 drivers
<milkylainen>
rfs613 :)
<Forty-Bot>
and sometimes it's a bug fix which indicates that you have the hardware and use it (and presumably would like it to keep working)
<Tartarus>
Yeah, which is why a real MAINTAINERS entry is good, and checkpatch whines if you lack one
<Tartarus>
Just been ignoring it for forever, should make progress on changing that
<marex>
Tartarus: I get flak in Linux for not using get maintainer recently, sigh
<marex>
Tartarus: seems in Linux it just CCs ton of peoples and that's fine by everyone
<marex>
I also get CCed on ton of things i don't really care about, so shrug