On 07/03/2019 13:01, Andrii Anisov wrote:
Dear Julien,
Hello,
On 05.03.19 15:56, Julien Grall wrote:
I had some concern about this solution in [1] that have not been addressed nor
even mentioned in the series.
You missed the link.
I was referring to your [1].
So I assume you say about you preferences to not have runstate area mapped
because of consuming vmap space for arm64. Also, along that thread you mentioned
you that guest might change gva mapping, what is irrelevant to registration with
physical address.
My reasons to have that runstate mapped are following:
- Introducing the new interface we are not burden with legacy, so in charge to
impose requirements. In this case - to have runstate area not crossing a page
boundary
- Global mapping used here does not consume vmap space on arm64. It seems to
me x86 guys are ok with mapping as well, at least Roger suggested it from the
beginning. So it should be ok for them as well.
You left arm32 out of your equations here...
- In case domain is mapping runstate with physical address, it can not change
the mapping.
This is not entirely correct. The domain can not change the mapping under our
feet, but it can still change via the hypercall. There are nothing preventing
that with current hypercall and the one your propose.
AFAICT, the vCPU operation can happen from a different vCPU. So now you need to
add some kind of synchronization to avoid the page disappearing under your feet
if the mapping is replaced from another vCPU. The risk is to write to a page the
has been allocated for another purpose and corrupt it.
I am not entirely sure we want to deny the call been done twice as this may have
an impact on the way the OS is written. So the only way would be to introduce
either reference counting or lock (if the former is not possible).
This is not an issue with the current code because we only store the guest
virtual address. The worst thing that can happen is we are writing to the wrong
guest virtual address, but at that point this is a guest problem. Although we
may want to use ACCESS_ONCE() in both load and store to ensure atomicity.
- I suppose, normally, runstate area would be registered once per vcpu. While
it is updated each context switch, I do estimate it more optimal to have it
mapped once on registration. This reduces two mappings per context switch in the
worst case (one for prev vcpu, one for the next).
Well the number you showed in the other thread didn't show any improvement at
all... So please explain why we should call map_domain_page_global() here and
using more vmap on arm32.
- 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?
But I don't think the implementation you suggest will be that simpler once you
deal with the problem above.
So, taking in account all above, I really do not see, how runstate area
registered with physical address differs from vcpu_info and does not deserve to
be permanently mapped.
As I said to the other thread, we should switch all the hypercalls to use
physical address. However, I am not convinced that keep the runstate mapped in
Xen is a good idea.
Cheers,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
