Re: [BUG] x86/efi: MMRs no longer properly mapped after switch to isolated page table

[Alex Thorlton]

On Sat, Apr 30, 2016 at 11:12:09PM +0100, Matt Fleming wrote:
> On Fri, 29 Apr, at 10:41:19AM, Alex Thorlton wrote:
> > 
> > You can see here that we've made it past the MMR read in uv_system_init,
> > but we die inside of our first EFI callback.  In this example, it looks
> > like we're using the kernel page table at the time of the failure, and I
> > believe that the failing address is somewhere in our EFI runtime code:
> > 
> > [    0.803396] efi: ATHORLTON EFI md dump:
> > [    0.803396]         type: 5
> > [    0.803396]         pad: 0
> > [    0.803396]         phys_addr: 6a0a6000
> > [    0.803396]         virt_addr: 0
> > [    0.803396]         num_pages: 184
> > [    0.803396]         attribute: 800000000000000f
> > 
> > So it looks like we're trying to read from EFI runtime space while using
> > the kernel page table, which fails, presumably because the space is also
> > not mapped into the kernel page table.  While I understand *why* it
> > fails, and why the address isn't mapped, I don't fully know how we
> > should handle fixing it.
> 
> How come you're not using the new EFI page tables while making EFI
> runtime calls?

That is a good question :)  Until now, I wasn't aware that we were doing
it this way.  As Boris pointed out, we need to look into this and get it
corrected.  I've started playing with it, but haven't quite got things
figured out yet.  I'll explaing in my response to Boris.

> Who owns the MMR space and what is it used for? Do both the kernel and
> the firmware need access to it? My SGI UV knowledge is zero, so I'm
> happy to be educated! I can't think of any analogous memory regions on
> x86 where the EFI services require the kernel to map them, other than
> the EFI regions themselves.

We have MMRs that get used for a ton of different purposes.  I'm only
familiar with the details of a few of them, but they provide a bunch of
information about various bits of SGI-specific hardware (i.e. the hub
and the BAU) and I think there are also some that allow you to control
that hardware.

This is more Mike Travis's department - he might be able to paint a
better picture.

> Runtime EFI regions should be opaque, isolated and self-contained.
> This is why there are two phases of execution for firmware; before and
> after ExitBootServices(). Once the kernel takes control after
> ExitBootServices() firmware can no longer provide timer services, for
> example, and doesn't care where the kernel maps the LAPIC because it
> never tries to access it.
> 
> The fact that the UV firmware does care where the MMR space is mapped
> makes me suspect that there's a lack of isolation.

The way it has been described to me is that there are MMRs that we need
access to directly from the kernel, as well as during some of our
runtime EFI callbacks, so we need them mapped in both the EFI page
tables, and the kernel page tables (glossing over the fact that we have
not been properly utilizing said EFI page tables).

It looks like mapping in the registers explicitly gets us where we need
to be, as far as the MMRs go.  I'm still working on sorting out the
problems with our callbacks, but I think that's sort of a separate
issue.

If you think we're violating EFI rules by accessing these registers from
both sides of the fence, please let me know.  I'd like to make sure that
we get everything behaving the way it should be!

- Alex