[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH 2/5] x86/p2m: collapse the two ->write_p2m_entry() hooks
On 10.11.2020 12:06, Roger Pau Monné wrote: > On Wed, Oct 28, 2020 at 10:22:58AM +0100, Jan Beulich wrote: >> @@ -1132,7 +1132,13 @@ void p2m_pt_init(struct p2m_domain *p2m) >> p2m->recalc = do_recalc; >> p2m->change_entry_type_global = p2m_pt_change_entry_type_global; >> p2m->change_entry_type_range = p2m_pt_change_entry_type_range; >> - p2m->write_p2m_entry = write_p2m_entry; >> + >> + /* Still too early to use paging_mode_hap(). */ >> + if ( hap_enabled(p2m->domain) ) >> + hap_p2m_init(p2m); >> + else if ( IS_ENABLED(CONFIG_SHADOW_PAGING) ) >> + shadow_p2m_init(p2m); > > There's already some logic in p2m_initialise that checks for > hap_enabled for EPT specific initialization. Do you think you could > move this there so that it's more contained? > > I think having the initialization condition sprinkled all over the > different functions makes the logic more complicated to follow. > > Also, should hap_p2m_init be limited to HAP and PT, as opposed to HAP > and EPT which doesn't use the helper AFAICT? It is limited to HAP and PT - we're in p2m_pt_init() here. This is also why I don't want to move it to e.g. p2m_initialise(), as that would be the wrong layer. > Maybe it would be clearer to unify shadow_write_p2m_entry with > hap_write_p2m_entry and call it p2m_pt_write_p2m_entry to match the > rest of the p2m PT helpers? This looks to go along the lines of what I'd put up as a post- commit-message remark in "x86/p2m: collapse the two ->write_p2m_entry() hooks". The nested handler is perhaps the bigger problem with such merging, plus it would feel a little like a layering violation (which is why I did put up the question instead of doing it right away). Jan
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |