On 10.12.2021 16:06, Roger Pau Monné wrote: > On Fri, Sep 24, 2021 at 11:52:14AM +0200, Jan Beulich wrote: >> --- >> I'm not fully sure about allowing 512G mappings: The scheduling-for- >> freeing of intermediate page tables can take quite a while when >> replacing a tree of 4k mappings by a single 512G one. Plus (or otoh) >> there's no present code path via which 512G chunks of memory could be >> allocated (and hence mapped) anyway. > > I would limit to 1G, which is what we support for CPU page tables > also.
I'm not sure I buy comparing with CPU side support when not sharing page tables. Not the least with PV in mind. >> @@ -288,10 +289,31 @@ static int iommu_pde_from_dfn(struct dom >> return 0; >> } >> >> +static void queue_free_pt(struct domain *d, mfn_t mfn, unsigned int >> next_level) > > Nit: should the last parameter be named level rather than next_level? > AFAICT it's the level of the mfn parameter. Yeah, might make sense. > Should we also assert that level (or next_level) is always != 0 for > extra safety? As said elsewhere - if this wasn't a static helper, I'd agree. But all call sites have respective conditionals around the call. If anything I'd move those checks into the function (but only if you think that would improve things, as to me having them at the call sites is more logical). Jan
