Re: [Xen-devel] [PATCH] xen/x86: p2m: Don't initialize slot 0 of the P2M

[Julien Grall <julien@xxxxxxx>]

On 03/02/2020 17:37, George Dunlap wrote:
> On 2/3/20 5:22 PM, Julien Grall wrote:
> > Hi,
> >
> > On 03/02/2020 17:10, George Dunlap wrote:
> > > On 2/3/20 4:58 PM, Julien Grall wrote:
> > > > From: Julien Grall <jgrall@xxxxxxxxxx>
> > > >
> > > > It is not entirely clear why the slot 0 of each p2m should be populated
> > > > with empty page-tables. The commit introducing it 759af8e3800 "[HVM]
> > > > Fix 64-bit HVM domain creation." does not contain meaningful
> > > > explanation except that it was necessary for shadow.
> > > Tim, any ideas here?
> > >
> > > > As we don't seem to have a good explanation why this is there, drop the
> > > > code completely.
> > > >
> > > > This was tested by successfully booting a HVM with shadow enabled.
> > > >
> > > > Signed-off-by: Julien Grall <jgrall@xxxxxxxxxx>
> > > >
> > > > ---
> > > >
> > > > I don't know for sure if this is going to break a setup. I have tried
> > > > HVM guest with hap={0, 1} without any trouble. I am happy to try more
> > > > setup if you have any in mind.
> > > >
> > > > If this break a setup, then please describe the setup and I will send a
> > > > documentation patch instead.
> > > This is a somewhat risky strategy.  Other than code clean-up, is there
> > > any advantage to removing this code at the moment?
> > If Tim doesn't have an explanation, then we have two solutions:
> >     1) Checkin the code and see if that breaks
> >     2) Keep code we have no clue why it is there
> It is probably early enough in the dev cycle to do this.
>
> Also, it's not clear to me what kind of bug the code you're deleting
> would fix.  If you read a not-present entry, you should get INVALID_MFN
> anyway.  Unless you were calling p2m_get_entry_query(), which I'm pretty
> sure hadn't been introduced at this point.
I can't find this function you mention in staging. Was it removed recently?

The code is allocating all page-tables for _gfn(0). I would not expect 
the common code to care whether a table is allocated or not. So this 
would suggest that an internal implementation (of the shadow?) is 
relying on this.
However, I can't find anything obvious suggesting that is necessary. If 
there was anything, I would expect to happen during domain creation, as 
neither Xen nor a guest could rely on this (there are way to make those 
pages disappear with the MEMORY op hypercall).
Cheers,

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel