On 22/10/2023 5:52 pm, Julien Grall wrote: > Hi Andrew, > > On 20/10/2023 21:26, Andrew Cooper wrote: >> When in use, the spew: >> >> (XEN) Assertion '!memchr_inv(b->ptr.buffer + MIN_BLOCK_SIZE, >> POISON_BYTE, (b->size & BLOCK_SIZE_MASK) - MIN_BLOCK_SIZE)' failed at >> common/xmalloc_tlsf.c:246 >> >> is unweidly and meaningless to non-Xen developers. Therefore: >> >> * Switch to IS_ENABLED(). There's no need for full #ifdef-ary. >> * Pull memchr_inv() out into the if(), and provide a better error >> message. >> >> Signed-off-by: Andrew Cooper <[email protected]> >> --- >> CC: George Dunlap <[email protected]> >> CC: Jan Beulich <[email protected]> >> CC: Stefano Stabellini <[email protected]> >> CC: Wei Liu <[email protected]> >> CC: Julien Grall <[email protected]> >> CC: Roger Pau Monné <[email protected]> >> >> Observations from the debugging of: >> https://github.com/Dasharo/dasharo-issues/issues/488 >> >> There's a further bug. XMEM_POOL_POISON can be enabled in release >> builds, >> where the ASSERT() gets dropped. >> >> However, upping to a BUG() can't provide any helpful message out to >> the user. >> >> I tried modifying BUG() to take an optional message, but xen/bug.h needs >> untangling substantially before that will work, and I don't have time >> right now. > > Do we actually care about the registers values? If not, we can either > use: > > printk(...); > BUG(); > > Or > > panic(...); > > This would allow us to use XMEM_POOL_POISON in prod build before BUG() > can accept string.
We care about the backtrace, so panic() on it's own isn't good enough. But printk();BUG(); should be good enough so I'll swap to that. ~Andrew
