Re: [PATCH v5 12/15] VT-d: replace all-contiguous page tables by superpage mappings

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Thu, Jun 02, 2022 at 11:58:48AM +0200, Jan Beulich wrote:
> On 02.06.2022 11:35, Roger Pau Monné wrote:
> > On Fri, May 27, 2022 at 01:19:55PM +0200, Jan Beulich wrote:
> >> When a page table ends up with all contiguous entries (including all
> >> identical attributes), it can be replaced by a superpage entry at the
> >> next higher level. The page table itself can then be scheduled for
> >> freeing.
> >>
> >> The adjustment to LEVEL_MASK is merely to avoid leaving a latent trap
> >> for whenever we (and obviously hardware) start supporting 512G mappings.
> >>
> >> Note that cache sync-ing is likely more strict than necessary. This is
> >> both to be on the safe side as well as to maintain the pattern of all
> >> updates of (potentially) live tables being accompanied by a flush (if so
> >> needed).
> >>
> >> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> >> Reviewed-by: Kevin Tian <kevin.tian@xxxxxxxxx>
> > 
> > Reviewed-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> 
> Thanks.
> 
> >> ---
> >> Unlike the freeing of all-empty page tables, this causes quite a bit of
> >> back and forth for PV domains, due to their mapping/unmapping of pages
> >> when they get converted to/from being page tables. It may therefore be
> >> worth considering to delay re-coalescing a little, to avoid doing so
> >> when the superpage would otherwise get split again pretty soon. But I
> >> think this would better be the subject of a separate change anyway.
> >>
> >> Of course this could also be helped by more "aware" kernel side
> >> behavior: They could avoid immediately mapping freed page tables
> >> writable again, in anticipation of re-using that same page for another
> >> page table elsewhere.
> > 
> > Could we provide an option to select whether to use super-pages for
> > IOMMU, so that PV domains could keep the previous behavior?
> 
> Hmm, I did (a while ago) consider adding a command line option, largely
> to have something in case of problems, but here you're asking about a
> per-domain setting. Possible, sure, but I have to admit I'm always
> somewhat hesitant when it comes to changes requiring to touch the tool
> stack in non-trivial ways (required besides a separate Dom0 control).

Well, per-domain is always better IMO, but I don't want to block you
on this, so I guess a command line option would be OK.

Per-domain would IMO be helpful in this case because an admin might
wish to disable IOMMU super-pages just for PV guests, in order to
prevent the back-and-forth in that case.  We could also do so with a
command line option but it's not the most user-friendly approach.

> It's also not clear what granularity we'd want to allow control at:
> Just yes/no, or also an upper bound on the page sizes permitted, or
> even a map of (dis)allowed page sizes?

I would be fine with just yes/no.  I don't think we need to complicate
the logic, this should be a fallback in case things don't work as
expected.

> Finally, what would the behavior be for HVM guests using shared page
> tables? Should the IOMMU option there also suppress CPU-side large
> pages? Or should the IOMMU option, when not fulfillable with shared
> page tables, lead to use of separate page tables (and an error if
> shared page tables were explicitly requested)?

I think the option should error out (or be ignored?) when used with
shared page tables, there are already options to control the page
sizes for the CPU side page tables, and those should be used when
using shared page tables.

Thanks, Roger.