Re: Error during update_runstate_area with KPTI activated

[Hongyan Xia <hx242@xxxxxxx>]

On Fri, 2020-05-15 at 11:08 +0100, Julien Grall wrote:
> Hi,
> 
> On 15/05/2020 10:57, Roger Pau Monné wrote:
> > On Fri, May 15, 2020 at 10:23:16AM +0100, Julien Grall wrote:
> > > [CAUTION - EXTERNAL EMAIL] DO NOT reply, click links, or open
> > > attachments unless you have verified the sender and know the
> > > content is safe.
> > > 
> > > 
> > > On 15/05/2020 10:10, Roger Pau Monné wrote:
> > > > On Fri, May 15, 2020 at 09:53:54AM +0100, Julien Grall wrote:
...

> > > > 
> > > > Could we map/unmap the runtime area on domain switch at a per-
> > > > cpu
> > > > based linear space area? There's no reason to have all the
> > > > runtime
> > > > areas mapped all the time, you just care about the one from the
> > > > running vcpu.
> > > 
> > > AFAICT, this is only used during context switching. This is a bit
> > > surprising
> > > because I would expect it to be updated when the vCPU is running.
> > > 
> > > So maybe we could just use map_domain_page() and take care
> > > manually about
> > > cross-page boundary.
> > > 
> > > > 
> > > > Maybe the overhead of that mapping and unmapping would be
> > > > too high? But seeing that we are aiming at a secret-free Xen we
> > > > would
> > > > have to eventually go that route anyway.
> > > 
> > > The overhead is likely to be higher with the existing code as you
> > > have to
> > > walk the guest page-tables and the p2m everytime in order to
> > > translate the
> > > guest virtual address to a host physical address.
> > 
> > Maybe I'm getting confused, but you actually want to avoid the
> > guest
> > page table walk, as the guest might be running with user-space page
> > tables that don't have the linear address of the runtime area
> > mapped,
> > and hence you would like to do the walk only once (at hypercall
> > registration time) and keep a reference to the page(s)?
> 
> That's right.
> 
> > 
> > I assumed the whole point of this was to avoid doing the page table
> > walk when you need to update the runstate info area.
> 
> Sorry I wasn't clear. I was trying to answer to your question about
> the 
> overhead.
> 
> The overhead with SH and the existing runstate implementation is
> going 
> to be quite high because you would need to map/unmap each table
> during 
> your walk. By removing the walk, you would now have only one
> map/unmap 
> for the update which I think is acceptable.
> 
> So the change discussed in this thread is also going to be
> beneficial 
> for SH even if we keep a map/unmap in the process.

For every hypercall, trap, context switch... one or two maps and unmaps
is definitely fine, showing almost no impact in real-world performance.
The most impact I found in the direct map removal work is definitely
GVA->GFN->MFN walk for traps and hypercalls. HVM + EPT walk can take up
to 20 maps and unmaps which degrades hypercall and emmulated MMIO
performance by up to 60%. It would be really nice if some paths can
just take GFN or just register the MFN. So I would definitely welcome a
change to use the physcial address.

Hongyan