Re: Issue with shared information page on Xen/ARM 4.17

[Elliott Mitchell <ehem+xen@xxxxxxx>]

On Wed, Oct 04, 2023 at 03:21:04PM -0700, Stefano Stabellini wrote:
> On Wed, 4 Oct 2023, Elliott Mitchell wrote:
> > On Wed, Oct 04, 2023 at 03:39:16PM +0200, Roger Pau Monné wrote:
> > > On Wed, Oct 04, 2023 at 02:03:43PM +0100, Julien Grall wrote:
> > > > 
> > > > On 04/10/2023 13:53, Roger Pau Monné wrote:
> > > > > 
> > > > > When using UEFI there's RAM that will always be in-use by the
> > > > > firmware, as runtime services cannot be shut down, and hence the
> > > > > firmware must already have a way to remove/reserve such region(s) on
> > > > > the memory map.
> > > > 
> > > > Can either you or Elliott confirm if EDK2 reserve the region?
> > > 
> > > I will defer to Elliott to check for arm.  I would be quite surprised
> > > if it doesn't on x86, or else we would get a myriad of bug reports
> > > about guests randomly crashing when using edk2.
> > 
> > When I had originally looked I thought there was no problem as
> > `OvmfPkg/XenPlatformPei/Xen.c`:
> > CalibrateLapicTimer()
> >     MapSharedInfoPage(SharedInfo)
> >     ...
> >     UnmapXenPage(SharedInfo)
> > 
> > Later using `find * -type f -print0 | xargs -0 grep 
> > -eXENMAPSPACE_shared_info`
> > `OvmfPkg/XenBusDxe/XenBusDxe.c`:
> >     XenGetSharedInfoPage()
> >   // using reserved page because the page is not released when Linux is
> >   // starting because of the add_to_physmap. QEMU might try to access the
> >   // page, and fail because it have no right to do so (segv).
> > 
> > Looks like this second case leaks the shared information page.
> > Originally I thought there was no problem as I'd only found the first
> > instance.  Appears this second instance is the problem.
> 
> I understand this second case is *not* unmapping the SharedInfo page,
> but is it reserving it somehow? For instance marking it as reserved in
> the EFI memory map?

Notice the "//" comment which I carefully grabbed?

  // using reserved page because the page is not released when Linux is
  // starting because of the add_to_physmap. QEMU might try to access the
  // page, and fail because it have no right to do so (segv).

So the page shouldn't be touched by anyone, but it does end up wasted.
Likely ExitBootServices() should clear the mapping.


-- 
(\___(\___(\______          --=> 8-) EHM <=--          ______/)___/)___/)
 \BS (    |         ehem+sigmsg@xxxxxxx  PGP 87145445         |    )   /
  \_CS\   |  _____  -O #include <stddisclaimer.h> O-   _____  |   /  _/
8A19\___\_|_/58D2 7E3D DDF4 7BA6 <-PGP-> 41D1 B375 37D0 8714\_|_/___/5445