[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH 0/2 for-4.12] Introduce runstate area registration with phys address



Hi Juergen,

On 3/8/19 6:28 AM, Juergen Gross wrote:
On 07/03/2019 19:00, Julien Grall wrote:
Hi Roger,

On 07/03/2019 17:15, Roger Pau Monné wrote:
On Thu, Mar 07, 2019 at 04:36:59PM +0000, Julien Grall wrote:
Hi Roger,

Thank you for the answer.

On 07/03/2019 16:16, Roger Pau Monné wrote:
On Thu, Mar 07, 2019 at 03:17:54PM +0000, Julien Grall wrote:
Hi Andrii,

On 07/03/2019 14:34, Andrii Anisov wrote:
On 07.03.19 16:02, Julien Grall wrote:
     - IMHO, this implementation is simpler and cleaner than what I
have for runstate mapping on access.

Did you implement it using access_guest_memory_by_ipa?
Not exactly, access_guest_memory_by_ipa() has no implementation
for x86.
But it is made around that code.

For the HVM, the equivalent function is hvm_copy_to_guest_phys. I
don't know
what would be the interface for PV. Roger, any idea?

For PV I think you will have to use get_page_from_gfn, check the
permissions, map it, write and unmap it. The same flow would also work
for HVM, so I'm not sure if there's much point in using
hvm_copy_to_guest_phys. Or you can implement a generic
copy_to_guest_phys helper that works for both PV and HVM.

Note that for translated guests you will have to walk the HAP page
tables for each vCPU for each context switch, which I think will be
expensive in terms of performance (I might be wrong however, since I
have no proof of this).

AFAICT, we already walk the page-table with the current
implementation. So
this should be no different here, except we will not need to walk the
guest-PT here. No?

Yes, current implementation is even worse because it walks both the
guest page tables and the HAP page tables in the HVM case. It would be
interesting IMO if we could avoid walking any of those page tables.

I see you have concerns about permanently mapping the runstate area,
so I'm not going to oppose, albeit even with only 1G of VA space you
can map plenty of runstate areas, and taking into account this is
32bit hardware I'm not sure you will ever have that many vCPUs that
you will run out of VA space to map runstate areas.

Actually the vmap is only 768MB. The vmap is at the moment used for
mapping:
     - MMIO devices (through ioremap)
         - event channel pages

As the runstate is far smaller than a page, this sounds like a waste of
memory for a benefits that haven't not yet been shown. Indeed, number
provided by Andrii either show worst performance or similar one.

But TBH, I am not expecting that a really clear performance improvement
on Arm as there are a lot to do in the context switch.


That being said, if the implementation turns out to be more
complicated because of this permanent mapping, walking the guest HAP
page tables is certainly no worse than what's done ATM.

To be honest I am not fully against always mapping the runstate in Xen.
But I need data to show this is worth it. So far, the performance
promised are not there and the implementation is not foolproof yet.

If we want to keep the runstate mapped permanently, then we need to add
either a lock or a refcounting. So the page does not disappear during
context switch if we happen to update the runstate concurrently (via the
hypercall).

This may increase the complexity of the implementation (not sure by how
much thought).

Another solution is to prevent the runstate to be updated. But I think
we will just add a bit more burden in the guest OS.

Not sure about other systems, but current Linux kernel registers the
runstate area for other cpus only if those are not up. So there is no
way the runstate of that foreign vcpu could be updated during
registering it. Of course this would need to be tested (e.g. -EBUSY
for registering runstate of an active foreign vcpu).

I was under the impression that xen_vcpu_restore() (see arch/x86/xen/enlighten.c) may update the runstate while the vCPU is up (at least on pre-Xen 4.5). Did I miss anything?

Cheers,

--
Julien Grall

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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.