| Message ID | 20190930142508.25102-3-alexandru.elisei@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | arm64: Add code generation test | expand |
On Mon, Sep 30, 2019 at 03:25:07PM +0100, Alexandru Elisei wrote: > The PTE_USER bit (AP[1]) in a page entry means that lower privilege levels > (EL0, on arm64, or PL0, on arm) can read and write from that memory > location [1][2]. On arm64, it also implies PXN (Privileged execute-never) > when is set [3]. Add a function to clear the bit which we can use when we > want to execute code from that page or the prevent access from lower > exception levels. > > Make it available to arm too, in case someone needs it at some point. > > [1] ARM DDI 0406C.d, Table B3-6 > [2] ARM DDI 0487E.a, table D5-28 > [3] ARM DDI 0487E.a, table D5-33 > > Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com> > --- > lib/arm/asm/mmu-api.h | 1 + > lib/arm/mmu.c | 15 +++++++++++++++ > 2 files changed, 16 insertions(+) > > diff --git a/lib/arm/asm/mmu-api.h b/lib/arm/asm/mmu-api.h > index df3ccf7bc7e0..8fe85ba31ec9 100644 > --- a/lib/arm/asm/mmu-api.h > +++ b/lib/arm/asm/mmu-api.h > @@ -22,4 +22,5 @@ extern void mmu_set_range_sect(pgd_t *pgtable, uintptr_t virt_offset, > extern void mmu_set_range_ptes(pgd_t *pgtable, uintptr_t virt_offset, > phys_addr_t phys_start, phys_addr_t phys_end, > pgprot_t prot); > +extern void mmu_clear_user(unsigned long vaddr); > #endif > diff --git a/lib/arm/mmu.c b/lib/arm/mmu.c > index 3d38c8397f5a..78db22e6af14 100644 > --- a/lib/arm/mmu.c > +++ b/lib/arm/mmu.c > @@ -217,3 +217,18 @@ unsigned long __phys_to_virt(phys_addr_t addr) > assert(!mmu_enabled() || __virt_to_phys(addr) == addr); > return addr; > } > + > +void mmu_clear_user(unsigned long vaddr) > +{ > + pgd_t *pgtable; > + pteval_t *pte; > + > + if (!mmu_enabled()) > + return; > + > + pgtable = current_thread_info()->pgtable; > + pte = get_pte(pgtable, vaddr); > + > + *pte &= ~PTE_USER; > + flush_tlb_page(vaddr); > +} > -- > 2.20.1 > This is fine, but I think you could just export get_pte() and then implement the PTE_USER clearing in the cache unit test instead. Anyway, Reviewed-by: Andrew Jones <drjones@redhat.com>
Hi, On 9/30/19 3:53 PM, Andrew Jones wrote: > On Mon, Sep 30, 2019 at 03:25:07PM +0100, Alexandru Elisei wrote: >> The PTE_USER bit (AP[1]) in a page entry means that lower privilege levels >> (EL0, on arm64, or PL0, on arm) can read and write from that memory >> location [1][2]. On arm64, it also implies PXN (Privileged execute-never) >> when is set [3]. Add a function to clear the bit which we can use when we >> want to execute code from that page or the prevent access from lower >> exception levels. >> >> Make it available to arm too, in case someone needs it at some point. >> >> [1] ARM DDI 0406C.d, Table B3-6 >> [2] ARM DDI 0487E.a, table D5-28 >> [3] ARM DDI 0487E.a, table D5-33 >> >> Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com> >> --- >> lib/arm/asm/mmu-api.h | 1 + >> lib/arm/mmu.c | 15 +++++++++++++++ >> 2 files changed, 16 insertions(+) >> >> diff --git a/lib/arm/asm/mmu-api.h b/lib/arm/asm/mmu-api.h >> index df3ccf7bc7e0..8fe85ba31ec9 100644 >> --- a/lib/arm/asm/mmu-api.h >> +++ b/lib/arm/asm/mmu-api.h >> @@ -22,4 +22,5 @@ extern void mmu_set_range_sect(pgd_t *pgtable, uintptr_t virt_offset, >> extern void mmu_set_range_ptes(pgd_t *pgtable, uintptr_t virt_offset, >> phys_addr_t phys_start, phys_addr_t phys_end, >> pgprot_t prot); >> +extern void mmu_clear_user(unsigned long vaddr); >> #endif >> diff --git a/lib/arm/mmu.c b/lib/arm/mmu.c >> index 3d38c8397f5a..78db22e6af14 100644 >> --- a/lib/arm/mmu.c >> +++ b/lib/arm/mmu.c >> @@ -217,3 +217,18 @@ unsigned long __phys_to_virt(phys_addr_t addr) >> assert(!mmu_enabled() || __virt_to_phys(addr) == addr); >> return addr; >> } >> + >> +void mmu_clear_user(unsigned long vaddr) >> +{ >> + pgd_t *pgtable; >> + pteval_t *pte; >> + >> + if (!mmu_enabled()) >> + return; >> + >> + pgtable = current_thread_info()->pgtable; >> + pte = get_pte(pgtable, vaddr); >> + >> + *pte &= ~PTE_USER; >> + flush_tlb_page(vaddr); >> +} >> -- >> 2.20.1 >> > This is fine, but I think you could just export get_pte() and then > implement the PTE_USER clearing in the cache unit test instead. Anyway, I thought about that, but I opted to make this a library function because I would like to modify it to also act on block mappings and use it in patch #4 from the EL2 series (the patch that adds the prefetch abort test), and send that change as part of the EL2 series. I am assuming that this patch set will get merged before the EL2 series. > > Reviewed-by: Andrew Jones <drjones@redhat.com>
On Mon, Sep 30, 2019 at 04:09:49PM +0100, Alexandru Elisei wrote: > Hi, > > On 9/30/19 3:53 PM, Andrew Jones wrote: > > > On Mon, Sep 30, 2019 at 03:25:07PM +0100, Alexandru Elisei wrote: > > > The PTE_USER bit (AP[1]) in a page entry means that lower privilege levels > > > (EL0, on arm64, or PL0, on arm) can read and write from that memory > > > location [1][2]. On arm64, it also implies PXN (Privileged execute-never) > > > when is set [3]. Add a function to clear the bit which we can use when we > > > want to execute code from that page or the prevent access from lower > > > exception levels. > > > > > > Make it available to arm too, in case someone needs it at some point. > > > > > > [1] ARM DDI 0406C.d, Table B3-6 > > > [2] ARM DDI 0487E.a, table D5-28 > > > [3] ARM DDI 0487E.a, table D5-33 > > > > > > Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com> > > > --- > > > lib/arm/asm/mmu-api.h | 1 + > > > lib/arm/mmu.c | 15 +++++++++++++++ > > > 2 files changed, 16 insertions(+) > > > > > > diff --git a/lib/arm/asm/mmu-api.h b/lib/arm/asm/mmu-api.h > > > index df3ccf7bc7e0..8fe85ba31ec9 100644 > > > --- a/lib/arm/asm/mmu-api.h > > > +++ b/lib/arm/asm/mmu-api.h > > > @@ -22,4 +22,5 @@ extern void mmu_set_range_sect(pgd_t *pgtable, uintptr_t virt_offset, > > > extern void mmu_set_range_ptes(pgd_t *pgtable, uintptr_t virt_offset, > > > phys_addr_t phys_start, phys_addr_t phys_end, > > > pgprot_t prot); > > > +extern void mmu_clear_user(unsigned long vaddr); > > > #endif > > > diff --git a/lib/arm/mmu.c b/lib/arm/mmu.c > > > index 3d38c8397f5a..78db22e6af14 100644 > > > --- a/lib/arm/mmu.c > > > +++ b/lib/arm/mmu.c > > > @@ -217,3 +217,18 @@ unsigned long __phys_to_virt(phys_addr_t addr) > > > assert(!mmu_enabled() || __virt_to_phys(addr) == addr); > > > return addr; > > > } > > > + > > > +void mmu_clear_user(unsigned long vaddr) > > > +{ > > > + pgd_t *pgtable; > > > + pteval_t *pte; > > > + > > > + if (!mmu_enabled()) > > > + return; > > > + > > > + pgtable = current_thread_info()->pgtable; > > > + pte = get_pte(pgtable, vaddr); > > > + > > > + *pte &= ~PTE_USER; > > > + flush_tlb_page(vaddr); > > > +} > > > -- > > > 2.20.1 > > > > > This is fine, but I think you could just export get_pte() and then > > implement the PTE_USER clearing in the cache unit test instead. Anyway, > > I thought about that, but I opted to make this a library function because I > would like to modify it to also act on block mappings and use it in patch #4 > from the EL2 series (the patch that adds the prefetch abort test), and send > that change as part of the EL2 series. I am assuming that this patch set > will get merged before the EL2 series. Yeah, I need to get back to that EL2 series. I just need to wrap up a couple more things and then I'll be able to focus on it. If you have some plans to refresh it, then feel free to do that now. When I get back to it, I'll just jump into the refreshed version. Thanks, drew
On 9/30/19 4:14 PM, Andrew Jones wrote: > On Mon, Sep 30, 2019 at 04:09:49PM +0100, Alexandru Elisei wrote: >> Hi, >> >> On 9/30/19 3:53 PM, Andrew Jones wrote: >> >>> On Mon, Sep 30, 2019 at 03:25:07PM +0100, Alexandru Elisei wrote: >>>> The PTE_USER bit (AP[1]) in a page entry means that lower privilege levels >>>> (EL0, on arm64, or PL0, on arm) can read and write from that memory >>>> location [1][2]. On arm64, it also implies PXN (Privileged execute-never) >>>> when is set [3]. Add a function to clear the bit which we can use when we >>>> want to execute code from that page or the prevent access from lower >>>> exception levels. >>>> >>>> Make it available to arm too, in case someone needs it at some point. >>>> >>>> [1] ARM DDI 0406C.d, Table B3-6 >>>> [2] ARM DDI 0487E.a, table D5-28 >>>> [3] ARM DDI 0487E.a, table D5-33 >>>> >>>> Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com> >>>> --- >>>> lib/arm/asm/mmu-api.h | 1 + >>>> lib/arm/mmu.c | 15 +++++++++++++++ >>>> 2 files changed, 16 insertions(+) >>>> >>>> diff --git a/lib/arm/asm/mmu-api.h b/lib/arm/asm/mmu-api.h >>>> index df3ccf7bc7e0..8fe85ba31ec9 100644 >>>> --- a/lib/arm/asm/mmu-api.h >>>> +++ b/lib/arm/asm/mmu-api.h >>>> @@ -22,4 +22,5 @@ extern void mmu_set_range_sect(pgd_t *pgtable, uintptr_t virt_offset, >>>> extern void mmu_set_range_ptes(pgd_t *pgtable, uintptr_t virt_offset, >>>> phys_addr_t phys_start, phys_addr_t phys_end, >>>> pgprot_t prot); >>>> +extern void mmu_clear_user(unsigned long vaddr); >>>> #endif >>>> diff --git a/lib/arm/mmu.c b/lib/arm/mmu.c >>>> index 3d38c8397f5a..78db22e6af14 100644 >>>> --- a/lib/arm/mmu.c >>>> +++ b/lib/arm/mmu.c >>>> @@ -217,3 +217,18 @@ unsigned long __phys_to_virt(phys_addr_t addr) >>>> assert(!mmu_enabled() || __virt_to_phys(addr) == addr); >>>> return addr; >>>> } >>>> + >>>> +void mmu_clear_user(unsigned long vaddr) >>>> +{ >>>> + pgd_t *pgtable; >>>> + pteval_t *pte; >>>> + >>>> + if (!mmu_enabled()) >>>> + return; >>>> + >>>> + pgtable = current_thread_info()->pgtable; >>>> + pte = get_pte(pgtable, vaddr); >>>> + >>>> + *pte &= ~PTE_USER; >>>> + flush_tlb_page(vaddr); >>>> +} >>>> -- >>>> 2.20.1 >>>> >>> This is fine, but I think you could just export get_pte() and then >>> implement the PTE_USER clearing in the cache unit test instead. Anyway, >> I thought about that, but I opted to make this a library function because I >> would like to modify it to also act on block mappings and use it in patch #4 >> from the EL2 series (the patch that adds the prefetch abort test), and send >> that change as part of the EL2 series. I am assuming that this patch set >> will get merged before the EL2 series. > Yeah, I need to get back to that EL2 series. I just need to wrap up a > couple more things and then I'll be able to focus on it. If you have > some plans to refresh it, then feel free to do that now. When I get > back to it, I'll just jump into the refreshed version. That's great, I have v2 almost ready, I'll send it tomorrow. > Thanks, > drew IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
diff --git a/lib/arm/asm/mmu-api.h b/lib/arm/asm/mmu-api.h index df3ccf7bc7e0..8fe85ba31ec9 100644 --- a/lib/arm/asm/mmu-api.h +++ b/lib/arm/asm/mmu-api.h @@ -22,4 +22,5 @@ extern void mmu_set_range_sect(pgd_t *pgtable, uintptr_t virt_offset, extern void mmu_set_range_ptes(pgd_t *pgtable, uintptr_t virt_offset, phys_addr_t phys_start, phys_addr_t phys_end, pgprot_t prot); +extern void mmu_clear_user(unsigned long vaddr); #endif diff --git a/lib/arm/mmu.c b/lib/arm/mmu.c index 3d38c8397f5a..78db22e6af14 100644 --- a/lib/arm/mmu.c +++ b/lib/arm/mmu.c @@ -217,3 +217,18 @@ unsigned long __phys_to_virt(phys_addr_t addr) assert(!mmu_enabled() || __virt_to_phys(addr) == addr); return addr; } + +void mmu_clear_user(unsigned long vaddr) +{ + pgd_t *pgtable; + pteval_t *pte; + + if (!mmu_enabled()) + return; + + pgtable = current_thread_info()->pgtable; + pte = get_pte(pgtable, vaddr); + + *pte &= ~PTE_USER; + flush_tlb_page(vaddr); +}
The PTE_USER bit (AP[1]) in a page entry means that lower privilege levels (EL0, on arm64, or PL0, on arm) can read and write from that memory location [1][2]. On arm64, it also implies PXN (Privileged execute-never) when is set [3]. Add a function to clear the bit which we can use when we want to execute code from that page or the prevent access from lower exception levels. Make it available to arm too, in case someone needs it at some point. [1] ARM DDI 0406C.d, Table B3-6 [2] ARM DDI 0487E.a, table D5-28 [3] ARM DDI 0487E.a, table D5-33 Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com> --- lib/arm/asm/mmu-api.h | 1 + lib/arm/mmu.c | 15 +++++++++++++++ 2 files changed, 16 insertions(+)