[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH v5 11/13] xen/arm: mmu: move MMU specific P2M code to mmu/p2m.{c,h}
On 23/08/2023 04:47, Penny Zheng wrote: Hi Julien Hi Penny, On 2023/8/23 02:01, Julien Grall wrote:Hi Henry, On 14/08/2023 05:25, Henry Wang wrote:From: Penny Zheng <penny.zheng@xxxxxxx> 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? I have looked at the mpu_v5 tree to check how you use p2m->root. The common pattern is: table = (pr_t *)page_to_virt(p2m->root);AFAICT the "root" is always mapped in your case. So this is a bit inneficient to have to convert from a page to virt every single time you need to modify the P2M. Therefore it would be better to introduce something more specific to the MPU. Rather than introduce a field in the structure p2m, it would be better if we introduce a structure that would be defined in {mmu,mpu}/p2m.h, would include there specific fields and be embedded in struct p2m. 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/7ff6d351e65f43fc34ea694adea0e030a51b1576for more details. If we destroy MPU P2M table in relinquish_p2m_mapping, region by region, It would seem to be better to handle it region by region. Note that you will still need to handle preemption and that may happen in the middle of the region (in particular if they are big). Cheers, -- Julien Grall
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |