On Fri, 28 Jul 2023 14:39:15 +0100 Jonathan Cameron via Linaro-open-discussions linaro-open-discussions@op-lists.linaro.org wrote:
On Fri, 28 Jul 2023 11:56:44 +0100 James Morse via Linaro-open-discussions linaro-open-discussions@op-lists.linaro.org wrote:
Hi Jonathan,
On 7/27/23 11:36, Jonathan Cameron wrote:
Given you mentioned rebasing MPAM :)
It's the same thing I do every night ....
:)
One trivial bug that might still be hanging around. When adding resources parsed from the MPAM ACPI table I think you are adding one byte too much for IORESOURCE_MEM https://git.kernel.org/pub/scm/linux/kernel/git/morse/linux.git/tree/drivers... should be a -1 similar to IORT handling. https://elixir.bootlin.com/linux/latest/source/drivers/acpi/arm64/iort.c#L15...
Thanks, fixed.
I still haven't gotten to the bottom of the difference between proximity-domain numbers and numa-nodes that Hesham reported.
I'll poke that one later... Too much fun to be had adding features first and generating piles of JSON so I can review the state as I poke it.
As a side note, I'm emulating MPAM on QEMU so I can poke the corners of the code beyond the real implementations we have access to and in a fashion I can talk about more freely.
Awesome! Something else to test with is always good.
Speaking of which... I got some unexpected write values in the bandwidth bitmap registers. I had a small bit map - 16 entries and got 0x1ffff written wd is coming from CPBM_WD which (unlike many register fields in the spec) is not offset by 1.
Bug looks to be an off by one.
https://git.kernel.org/pub/scm/linux/kernel/git/morse/linux.git/tree/drivers...
static void mpam_reset_msc_bitmap(struct mpam_msc *msc, u16 reg, u16 wd) { u32 bm = ~0; int i;
lockdep_assert_held(&msc->part_sel_lock);
/* * Write all but the last full-32bit-word. * The number of portions is 1<->wd, start at 1 instead of 0. */ for (i = 1; i < wd / 32; i++, reg += sizeof(bm))
for (i = 1; i < (wd + 31) /32; i++, reg += sizeof(bm))
DIV_ROUND_UP()
as you want to round up rather than down here.
Testing with a width of 48 we want this to run once and the below to pick up last 16 bits.
__mpam_write_reg(msc, reg, bm);/* and the last partial 32bit word */ bm = GENMASK(wd % 32, 0);
wd % 32 - 1
I think.
if (bm) __mpam_write_reg(msc, reg, bm); }
Other than the above, mostly seems to be working fine beyond a bug in the upstream PPTT parsing code that doesn't affect what you have implemented so far.
There is an infinite loop / deadlock hiding somewhere though in one of the error paths though that I haven't chased down yet. Maybe I'll be lucky and you've fixed it already :)
Oooeer, that sounds new .
There were some false-positive lockdep warnings that I've fixed/replaced-with-comments.
Otherwise the response to the error interrupt isn't fantastic at the moment, resctrl mostly manages to tear itself down, but there is evidently a reference counting problem somewhere as a few of the files live on. I can't find a way of triggering that code on x86, so its not too surprising. I haven't tested that for a while...
I was being lazy on interrupts - will wire that up at some point. If I get really bored I might even wire up the PPC channel interfaces but don't hold your breath on that one. By coincidence I wired up PCC in QEMU for something else last week so it wouldn't be too tricky - it's just not that high on my list unless you'll find it 'really useful'.
I hope everyone else is reading along ;) There will be a test on Thursday.
Jonathan
Thanks,
James