Hi Ilias,
On Wed, 19 Mar 2025 at 16:22, Ilias Apalodimas
<ilias.apalodi...@linaro.org> wrote:
>
> Hi Simon,
>
> On Wed, 19 Mar 2025 at 17:04, Simon Glass <s...@chromium.org> wrote:
> >
> > Hi Ilias,
> >
> > On Wed, 19 Mar 2025 at 08:22, Ilias Apalodimas
> > <ilias.apalodi...@linaro.org> wrote:
> > >
> > > We currently set both and print both PXN and UXN bits when removing
> > > execution for pages. This happens even in the existing per platform
> > > definitions of 'struct mm_region'.
> > >
> > > That's not entirely correct though. For stage-1 translations, if a
> > > platform runs on a translation regime with a single privilege level or the
> > > the translation regime supports two privilege levels and we are not
> > > in EL1&0 with HCR_EL2.{NV, NV1} = {1, 1} only BIT54 (XN) is needed
> > > and BIT53(PXN) is reserved 0.
> > >
> > > Currently we support Non-Secure EL2, Non-secure EL2&0 and Non-secure
> > > EL1&0.
> > >
> > > We already have get_effective_el() which returns 1 if we are
> > > - Running in EL1 so we assume an EL1 translation regime but without
> > > checking HCR_EL2.{NV, NV1} != {1,1}
> > > - Running in EL2 with HCR_EL2.E2H = 1
> > >
> > > The only problem with the above is that if we are in EL1&0 and
> > > HCR_EL2.{NV1, NV} == {1, 1}, then
> > > - Bit[54] holds the PXN instead of the UXN
> > > - The Effective value of UXN is 0
> > > - Bit[53] is RES0
> > >
> > > So let's re-use that function and set PXN only when we are in
> > > and EL[2|1]&0 translation regime.
> > >
> > > Signed-off-by: Ilias Apalodimas <ilias.apalodi...@linaro.org>
> > > ---
> > > arch/arm/cpu/armv8/cache_v8.c | 28 ++++++++++++++++++++++++----
> > > 1 file changed, 24 insertions(+), 4 deletions(-)
> > >
> >
> > Perhaps add a test?
>
> Sure, I'll send one, but it obviously can't run on sandbox.
> We can run in QEMU with an without virt, but we also care for real
> hardware (which is all problematic AFAICT -- at least the ones that
> have an EL2 translation regime).
>
> Any pointers on where I should add the test? A new file under test?
I am more talking about the algorithm than the effect. You could write
something which calls part of your code (an exported function) and
then checks part of the resulting table(s) in memory.
For example, if you pass in the value of find_pte() then you could
write something which calls pgprot_set_attrs() and then does a few
spot checks on the resulting (fake) page tables, e.g. to check that
your fix holds.
With effort you could run such a test on sandbox, but it might be
easier to enable it on QEMU on ARM, bearing in mind that it would not
be messing with the real page tables, just checking the algorithm.
Probably only a few dozen lines of test code, but it would help
provide a basis for people to build on as things grow over time.
There also seems to be some code-duplication in this file.
Regards,
Simon
