On 18/11/2022 11:01, Tudor Cretu wrote:
On 11-11-2022 10:12, Kevin Brodsky wrote:
On 02/11/2022 12:08, Tudor Cretu wrote:
Don't exclude PTE_READ_CAPS and PTE_WRITE_CAPS when calling pte_modify.
I would be good to explain why, as this is just saying what the diff is. In fact, I am not entirely sure I understand why we need this now (and we didn't need it before).
Indeed, I was not clear about this patch. I didn't find any issue that requires this patch now, it's just a fix that I have found while looking at changes need for MTE. pte_modify is used in change_pte_range, which by backtracking a bit, it looks to be used in a fair number of contexts.
Right I suspected this was to align with MTE. AFAICT change_pte_range() is ultimately only used by mprotect() and related operations. Having looked a little bit more, I have a better idea of what happens now: without this change, a change in VM_*_CAPS during mprotect() would simply be ignored (because the value of PTE_*_CAPS is always preserved as-is).
Here is my concern: in this series, VM_*_CAPS still cannot be changed by mprotect(), specifically the value of newflags calculated in do_mprotect_pkey() will always have its VM_*_CAPS fields taken directly from vma->vm_flags. So it seems to me a bit inconsistent to allow updating PTE_*_CAPS while VM_*_CAPS still cannot be changed (that would require updating arch_calc_vm_prot_bits() and probably other bits too).
It remains the case that this patch is perfectly fine, I would just prefer to get it in at the same time as allowing changes to VM_*_CAPS.
Kevin
I think PTE_LOAD_CAPS and PTE_STORE_CAPS should remain part of the pteval if such a function is applied.
I don't have a strong opinion on merging this patch though, so if you think it's not useful, I'm happy to drop it in v2.
Thanks, Tudor
Kevin
Signed-off-by: Tudor Cretu tudor.cretu@arm.com
arch/arm64/include/asm/pgtable.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h index dff2b483ea50..642f561e55a9 100644 --- a/arch/arm64/include/asm/pgtable.h +++ b/arch/arm64/include/asm/pgtable.h @@ -761,7 +761,7 @@ static inline pte_t pte_modify(pte_t pte, pgprot_t newprot) */ const pteval_t mask = PTE_USER | PTE_PXN | PTE_UXN | PTE_RDONLY | PTE_PROT_NONE | PTE_VALID | PTE_WRITE | PTE_GP | - PTE_ATTRINDX_MASK; + PTE_ATTRINDX_MASK | PTE_LOAD_CAPS | PTE_STORE_CAPS; /* preserve the hardware dirty information */ if (pte_hw_dirty(pte)) pte = pte_mkdirty(pte);