Hi Julien
On 2023/8/23 02:01, Julien Grall wrote:
Hi Henry,
On 14/08/2023 05:25, Henry Wang wrote:
From: Penny Zheng <penny.zh...@arm.com>
Current P2M implementation is designed for MMU system only.
We move the MMU-specific codes into mmu/p2m.c, and only keep generic
codes in p2m.c, like VMID allocator, etc. We also move MMU-specific
definitions and declarations to mmu/p2m.h, such as p2m_tlb_flush_sync().
Also expose previously static functions p2m_vmid_allocator_init(),
p2m_alloc_vmid(), __p2m_set_entry() and setup_virt_paging_one()
Looking at the code, it seemsm that you need to keep expose
__p2m_set_entry() because of p2m_relinquish_mapping(). However, it is
not clear how this code is supposed to work for the MPU. So should we
instead from p2m_relinquish_mapping() to mmu/p2m.c?
p2m->root stores per-domain P2M table, which is actually an array of MPU
region(pr_t). So maybe we should relinquish mapping region by region,
instead of page by page. Nevertheless, p2m_relinquish_mapping() shall be
moved to mmu/p2m.c and we need MPU version of it.
Other functions which doesn't seem to make sense in p2m.c are:
* p2m_clear_root_pages(): AFAIU there is no concept of root in the
MPU. This also means that we possibly want to move out anything specific
to the MMU from 'struct p2m'. This could be done separately.
Current MPU implementation is re-using p2m->root to store P2M table.
Do you agree on this, or should we create a new variable, like
p2m->mpu_table, for MPU P2M table only?
How we treat p2m_clear_root_pages also decides how we destroy P2M at
domain destruction stage, current MPU flow is as follows:
```
PROGRESS(mapping):
ret = relinquish_p2m_mapping(d);
if ( ret )
return ret;
PROGRESS(p2m_root):
/*
* We are about to free the intermediate page-tables, so clear the
* root to prevent any walk to use them.
*/
p2m_clear_root_pages(&d->arch.p2m);
#ifdef CONFIG_HAS_PAGING_MEMPOOL
PROGRESS(p2m):
ret = p2m_teardown(d);
if ( ret )
return ret;
PROGRESS(p2m_pool):
ret = p2m_teardown_allocation(d);
if( ret )
return ret;
#endif
```
We guard MMU-specific intermediate page-tables destroy with the new
Kconfig CONFIG_HAS_PAGING_MEMPOOL, check
https://gitlab.com/xen-project/people/henryw/xen/-/commit/7ff6d351e65f43fc34ea694adea0e030a51b1576
for more details.
If we destroy MPU P2M table in relinquish_p2m_mapping, region by region,
we could provide empty stub for p2m_clear_root_pages, and move it to
mmu/p2m.c
* p2m_flush_vm(): This is built with MMU in mind as we can use the
page-table to track access pages. You don't have that fine granularity
in the MPU.
Understood
for futher MPU usage.
typo: futher/further/
With the code movement, global variable max_vmid is used in multiple
files instead of a single file (and will be used in MPU P2M
implementation), declare it in the header and remove the "static" of
this variable.
Add #ifdef CONFIG_HAS_MMU to p2m_write_unlock() since future MPU
work does not need p2m_tlb_flush_sync().
And there are no specific barrier required? Overall, I am not sure I
like the #ifdef rather than providing a stub helper.
If the other like the idea of the #ifdef. I think a comment on top would
be necessary to explain why there is nothing to do in the context of the
MPU.
Cheers,
