vardhan has quit [Remote host closed the connection]
cbmuser has quit [Ping timeout: 248 seconds]
vardhan has joined #u-boot
vardhan has quit [Max SendQ exceeded]
vardhan has joined #u-boot
mmu_man has joined #u-boot
<sjg1>
apalos: OK great, thanks. We could use the 'manual run' thing too, like with my lab (which I need to bring back to life now that the integration has landed)
cbmuser has joined #u-boot
mmu_man has quit [Ping timeout: 260 seconds]
mmu_man has joined #u-boot
monstr has quit [Remote host closed the connection]
<sjg1>
Tartarus: think the patchwork patch is correct. This one should give a warning
<Tartarus>
sjg1: The patchwork one is wrong because we're putting map_to_sysmem in generic code and shouldn't be
<Tartarus>
So I want to put that call in the sandbox function
<sjg1>
Tartarus: Your code fragment here is a nop, so won't work
<Tartarus>
sjg1: wdym?
<sjg1>
flush_cache() takes an address. If you want to pass it a pointer, you need to map_to_sysmem() it first
<Tartarus>
I was unclear perhaps, that's the sandbox function
<Tartarus>
And that passes all of the tests on amd64 again.
<sjg1>
Tartarus: Think of map_sysmem() as a way to convert a ulong to a void *; map_to_sysmem() goes the other way
<sjg1>
Tartarus: I'm surprised it passes the tests, but then the failure never happened on x86_64, so perhaps it is a bit random?
<sjg1>
Tartarus: It depends on where U-Boot is loaded in memory on that particular machine
<Tartarus>
brb
joeskb7 has quit [Ping timeout: 260 seconds]
<Tartarus>
So, for amd64, I can make the test fail as part of trying to rework that patch
<Tartarus>
And, I don't think we're doing anything wrong in the rest of the code, it's just sandbox that's confusing things
<Tartarus>
Since all of these are nops for everyone else
davlefou has quit [Ping timeout: 252 seconds]
<sjg1>
Well any type you are casting from ulong to void *, or vice versa, sandbox may break
<Tartarus>
Changing "(ulong)efi_reloc" to "(phys_addr_t)(uintptr_t)efi_reloc" isn't a useful change I think
<Tartarus>
We need to work on getting the sandbox'ism out of the generic code whenever possible is the thing
<Tartarus>
And from checking around, I believe this is an easy example of that
<Tartarus>
And, er, sorry,, wrong example above
ikarso has joined #u-boot
<Tartarus>
No, that was right, nm
redbrain has joined #u-boot
joeskb7 has joined #u-boot
warpme has joined #u-boot
warpme has quit [Client Quit]
davlefou has joined #u-boot
redbrain has quit [Read error: Connection reset by peer]
redbrain has joined #u-boot
<Tartarus>
sjg1: I think perhaps the longer explanation is that yes, flush_cache takes an address. And sandbox is special in that we can't "just" cast is as appropriate. But if you want to add a flush_cache_ptr() or whatever, so that we get things consistent for everyone, OK. But the first mis-step here was doing it via "map_to_sysmem" which does not say "generic change" but "sandbox change". Like, drivers/net/hifemac.c::hisi_femac_send() could
<Tartarus>
be doing flush_cache(map_to_sysmem(packet), ...) and not ulong addr = (ulong)packet;\nflush_cache(addr, ...) but that would be silly since it's basically the same thing. So I'm pretty sure changing sandbox's flush_cache to match how everyone else works is the right step for now, and some future cleanup would be reworking how we handle cache flushes, and in a way that doesn't leave us open to people introducing more code that wouldn't
<Tartarus>
work on sandbox because sandbox is the case where things are different than everyone else.
<sjg1>
Tartarus: It is exactly the 'generic' code where sandbox-isms are needed!
<sjg1>
Tartarus: Arch-specific code doesn't need to worry about it
<Tartarus>
No, that's the problem. Sandbox needs to be able to work with generic code as much as possible.
<Tartarus>
Because otherwise people will keep getting it wrong
<sjg1>
Well we are not on the same page here, sorry. I think I understand what you want, but you canna change the laws of physics
<sjg1>
But the nice thing is, if we get it wrong, it only matters if there is a sandbox test which relies on it. So things get corrected over time
<sjg1>
Also, as you have pointed out, there is no need to add mapmem to code which won't ever be used on sandbox
<Tartarus>
It would be better to not have failures to start with tho, when we can avoid them
<Tartarus>
Because then I don't have to point people at how to build and use sandbox too
<Tartarus>
Or have them cycle through Azure trying to test things
<Tartarus>
But this cache thing is one case where we can easily get sandbox to act like a regular platform, not a special platform
redbrain has quit [Read error: Connection reset by peer]
<sjg1>
'make qcheck' should be enough for most people?
<sjg1>
I don't see how this cache thing can be done differently, unless you are suggesting using pointers, which I don't like
<sjg1>
If people are writing generic code, they should add a sandbox test, which will normally show up any problems
<sjg1>
You could consider that we just started running sandbox on a completely different architecture, and we only hit a few small problems
redbrain has joined #u-boot
<sjg1>
I'd actually call that a pretty astounding success
<sjg1>
I was certainly pretty surprised at how well everything worked
<Tartarus>
Oh right, "make qcheck". No, that runs the event_dump test which fails for me, I think that's why I always forget it exists
<Tartarus>
But also, sigh, there's a headdesk-worthy mistake in one of my buildman wrappers, it didn't blow up the build when it should have blown up the build
rvalue has quit [Read error: Connection reset by peer]