On Wed, Mar 27, 2024 at 9:19 PM Conor Dooley <co...@kernel.org> wrote: > > Christoph linked here on his submission to Linux of a fix for this, so I > am reviving this to leave a couple comments :) > > On Thu, Feb 15, 2024 at 02:24:02PM +1000, Alistair Francis wrote: > > On Mon, Feb 5, 2024 at 6:37 PM Christoph Müllner > > <christoph.muell...@vrull.eu> wrote: > > > On Mon, Feb 5, 2024 at 3:42 AM Alistair Francis <alistai...@gmail.com> > > > wrote: > > > > On Sun, Feb 4, 2024 at 3:44 PM LIU Zhiwei > > > > <zhiwei_...@linux.alibaba.com> wrote: > > > > > > ppn = (pte & (target_ulong)PTE_PPN_MASK) >> > > > > > PTE_PPN_SHIFT; > > > > > > > > Unfortunately we won't be able to take this upstream. This is core > > > > QEMU RISC-V code that is now being changed against the spec. I think > > > > adding the CSR is fine, but we can't take this core change. > > > > > > > > A fix that works for everyone should be supporting the th_mxstatus > > > > CSR, but don't support setting the TH_MXSTATUS_MAEE bit. That way > > > > guests can detect that the bit isn't set and not use the reserved bits > > > > in the PTE. From my understanding the extra PTE bits are related to > > > > cache control in the hardware, which we don't need here > > > > > > Sounds good! Let me recap the overall plan: > > > * QEMU does not emulate MAEE, but signals that MAEE is not available > > > by setting TH_MXSTATUS_MAEE to 0. > > > > Yep! > > > > > * Consequence: The c906 emulation does not enable any page-base memory > > > attribute mechanism. > > > > Exactly > > > > > * OpenSBI tests the TH_MXSTATUS_MAEE bit (M-mode only) and provides > > > that information to user-space (e.g. DTB). > > > > To the kernel, but yep! > > > > > * The current Linux errata code will be enhanced to not assume MAEE > > > for each core with T-Head vendor ID, but also query the MAEE bit and > > > ensure it is set. > > > > I feel like it should already do that :) > > It doesn't quite do this right now. It only makes the assumption for > CPUs where marchid and mvendorid are zero. The c908, and I think Guo Ren > confirmed it will be the case going forward, sets these to non-zero > values. We should have always required a dt property be set, rather than > using m*id, but we can't go back on that for these devices. Going > forward, if there are more CPUs that want to use this e.g. C908 in MAEE > mode (it can do svpbmt too) I'm gonna require it is explicitly set in > DT.
A DT node that we don't set also works fine for us Alistair