Re: [PATCH 5/6] x86/pv: map and unmap page tables in mark_pv_pt_pages_rdonly

[Hongyan Xia <hx242@xxxxxxx>]

On Tue, 2020-04-28 at 16:59 +0100, Hongyan Xia wrote:
> On Tue, 2020-04-28 at 16:55 +0100, Hongyan Xia wrote:
> > On Tue, 2020-04-28 at 17:33 +0200, Jan Beulich wrote:
> > > On 17.04.2020 11:52, Hongyan Xia wrote:
> > > > --- a/xen/arch/x86/pv/dom0_build.c
> > > > +++ b/xen/arch/x86/pv/dom0_build.c
> > > > @@ -50,17 +50,17 @@ static __init void
> > > > mark_pv_pt_pages_rdonly(struct domain *d,
> > > >      unsigned long count;
> > > >      struct page_info *page;
> > > >      l4_pgentry_t *pl4e;
> > > > -    l3_pgentry_t *pl3e;
> > > > -    l2_pgentry_t *pl2e;
> > > > -    l1_pgentry_t *pl1e;
> > > > +    l3_pgentry_t *pl3e, *l3t;
> > > > +    l2_pgentry_t *pl2e, *l2t;
> > > > +    l1_pgentry_t *pl1e, *l1t;
> > > 
> > > I don't quite see why the new local variables get introduced:
> > > unmap_domain_page(), iirc, is quite fine with a non-page-
> > > aligned argument.
> > 
> > You are right, although in this function, where plXe points to may
> > not
> > be the page we want to unmap. When plXe becomes aligned and points
> > to
> > a
> > new page, we actually want to unmap the page before it increments
> > to
> > an
> > aligned value.
> 
> Hmm, we can just unmap(plXe - 1) if my logic is correct, and save 3
> local variables.

Sorry for monologuing, but I still prefer separating plXe and lXt
because it makes it clear what we are unmapping. Unmapping plXe - 1 is
a bit hackish.

But if people do not have a problem with plXe - 1, I will post a new
revision for that.

Hongyan