On Thu, Oct 31, 2024 at 6:00 AM Kevin Brodsky <kevin.brodsky@xxxxxxx> wrote: > > On 30/10/2024 22:59, Jeff Xu wrote: > > Apologize in advance that I'm unfamiliar with ARM's POR, up to review > > this patch series, so I might ask silly questions or based on wrong > > understanding. > > That's no problem, your input is very welcome! There is no fundamental > difference between POR and PKRU AFAIK - the encoding is different, but > the principle is the same. The main thing to keep in mind is that POE > (the arm64 extension) allows restricting execution in addition to > read/write. > > > It seems that the patch has the same logic as Aruna Ramakrishna > > proposed for X86, is this correct ? > > Yes, patch 1 aims at aligning arm64 with x86 (same behaviour). Going > forward I think we should try and keep the arm64 and x86 handling of > pkeys as consistent as possible. > > > In the latest version of x86 change [1], I have a comment if we want > > to consider adding a new flag SS_PKEYALTSTACK (see SS_AUTODISARM as an > > example) in sigaltstack, and restrict this mechanism (overwriting > > PKRU/POR_EL0 and sigframe) to sigaltstack() with SS_PKEYALTSTACK. > > There is a subtle difference if we do that, i.e. the existing > > signaling handling user might not care or do not use PKEY/POE, and > > overwriting PKRU/POR_EL0 and sigframe every time will add extra CPU > > time on the signaling delivery, which could be real-time sensitive. > > From a purely functional perspective, resetting POR to allow access to > all pkeys before writing the signal frame should be safe in any context, > and allows keeping the handling simple (no conditional code). The > performance aspect is a fair point though, as we are adding an ISB > (synchronisation barrier) on the signal delivery path if POE is supported. > Yes. The functional level is the same. Having worked on a read-time system a bit in the past, I'm aware that signaling handling paths are real-time sensitive. > > Since I raised this comment on X86, I think raising it for ARM for > > discussion would be ok, > > it might make sense to have consistent API experience for arm/x86 here. > > And indeed this is what I think is most important at this point. > Considering that Aruna's series resets PKRU unconditionally (sigaltstack > or not) and has already been pulled into mainline during 6.12-rc1 [2], I > still believe that patch 1 is doing the right thing, i.e. aligning arm64 > with x86. If the concern with performance is confirmed, and there is an > agreement to reset POR/PKRU less eagerly on both architectures, this > could potentially be revisited. > Oh, I didn't know it was already in main. My information is out-dated. It does feel a little rushed because my comment on the performance perspective isn't addressed/responded. -Jeff > - Kevin > > [2] > https://lore.kernel.org/lkml/172656199227.2471820.13578261908219597067.tglx@xen13/ > > > [1] https://lore.kernel.org/lkml/CABi2SkWjF2Sicrr71=a6H8XJyf9q9L_Nd5FPp0CJ2mvB46Rrrg@xxxxxxxxxxxxxx/ >
