Hi Julien, > -----Original Message----- > From: Julien Grall <[email protected]> > Subject: Re: [PATCH v4] xen/arm: p2m: Populate pages for GICv2 mapping in > arch_domain_create() > > if ( p2m->root ) > > @@ -1784,6 +1800,8 @@ int p2m_init(struct domain *d) > > As Andrew pointed out the change in p2m_init() will end up leaking > either the VMID or the root table. > > Andrew's patch #1 [1] should help to solve the problem. So I would > suggest to rebase on top of it.
Of course. I appreciate Andrew very much for finding the issue. To properly solve the problem and reduce the amount of work from maintainers, I will try to respin the patch series (with #1 from Andrew and #2 from my rebased patch with commit message properly adapted). So that either we decided to pick Andrew's whole series or Andrew's #1 plus my #2, we can fetch the series directly from ML. [...] > > Other than that, the logic looks good to me. This is even knowning that > Andrew said the code is buggy. I spent some time starring at the code Thanks for your time :) > and can't figure out where the issue lies because p2m_teardown() will do > nothing when the list is empty. If it is not empty, then it is > guaranteed that the VMID and root table is allocated. So the code looks > functional but just not efficient. > > We already discussed the latter point earlier in the review and agreed > to look at improve it post 4.17. > > For the former, I am happy to be proven wrong. But this is going to > require more substantial explanations. ...having said that, definitely no problem for me to wait for a bit for the discussion continues. We can pick the most suitable series when we reach to the conclusion. Kind regards, Henry > > Cheers, > > -- > Julien Grall
