On 25/05/2026 14:53, Ilias Apalodimas wrote:
> Hi Casey,
>
> [...]
>
>>
>> return NULL;
>> }
>>
>> +/* Checks if the current PTE is an aligned subset of the region */
>> static bool is_aligned(u64 addr, u64 size, u64 align)
>> {
>> - return !(addr & (align - 1)) && !(size & (align - 1));
>> + return !(addr & (align - 1)) && size >= align;
>> }
>>
>> /* Use flag to indicate if attrs has more than d-cache attributes */
>> static u64 set_one_region(u64 start, u64 size, u64 attrs, bool flag, int
>> level)
>> @@ -1003,11 +1004,16 @@ static u64 set_one_region(u64 start, u64 size, u64
>> attrs, bool flag, int level)
>> int levelshift = level2shift(level);
>> u64 levelsize = 1ULL << levelshift;
>> u64 *pte = find_pte(start, level);
>>
>> - /* Can we can just modify the current level block PTE? */
>> + /* Can we can just modify the current level block/page? */
>> if (is_aligned(start, size, levelsize)) {
>> - if (flag) {
>> + if (attrs == PTE_TYPE_FAULT) {
>> + if (pte_type(pte) == PTE_TYPE_TABLE && level < 3)
>> + *pte = 0;
>> + else
>> + *pte &= ~(PMD_ATTRMASK | PTE_TYPE_MASK |
>> PTE_BLOCK_INNER_SHARE);
>
> Looks correct now, but the umapped memory appears as 'Device-nGnRnE'.
> Shouldn't we preserve whatever was there?
Maybe you could make a case for preserving the old values to know if
this region was mapped at some point, but I think that might just be
more confusing since we aren't generally interested in the history of
the TLB more so the current mappings. If you're explicitly debugging MMU
code then it still probably wouldn't be the most useful way to go about it.
>
> Thanks
> /Ilias
>> + } else if (flag) {
>> *pte &= ~PMD_ATTRMASK;
>> *pte |= attrs & PMD_ATTRMASK;
>> } else {
>> *pte &= ~PMD_ATTRINDX_MASK;
>> @@ -1107,8 +1113,12 @@ void mmu_change_region_attr(phys_addr_t addr, size_t
>> size, u64 attrs)
>> flush_dcache_range(gd->arch.tlb_addr,
>> gd->arch.tlb_addr + gd->arch.tlb_size);
>> __asm_invalidate_tlb_all();
>>
>> + /* If we were unmapping a region then we have nothing to make and
>> can return. */
>> + if (attrs == PTE_TYPE_FAULT)
>> + return;
>> +
>> mmu_change_region_attr_nobreak(addr, size, attrs);
>> }
>>
>> int pgprot_set_attrs(phys_addr_t addr, size_t size, enum pgprot_attrs perm)
>>
>> --
>> 2.53.0
>>
--
// Casey (she/her)
