Re: [Xen-devel] [PATCH v4] xen/arm: flush icache as well when XEN_DOMCTL_cacheflush is issued

[Tamas K Lengyel <tamas.lengyel@xxxxxxxxxxxx>]

On Mon, Jan 30, 2017 at 9:01 AM, Julien Grall <julien.grall@xxxxxxx> wrote:
> Hi Stefano,
>
> On 27/01/17 23:53, Stefano Stabellini wrote:
>>
>> On Fri, 27 Jan 2017, Julien Grall wrote:
>>>
>>> On 27/01/2017 20:41, Stefano Stabellini wrote:
>>>>
>>>> On Fri, 27 Jan 2017, Tamas K Lengyel wrote:
>>>
>>> For the second instance, we have no other choice.
>>
>>
>> Most alloc_heap_pages (alloc_xenheap_pages and alloc_domheap_pages) are
>> done at domain initialization, so they would also be taken care by
>> flushing the instruction cache before the domain is running. There are
>> only very few exceptions to that, the main one being ballooning, and we
>> need another icache flush in that case. But I think we should avoid
>> doing global icache flushes every time alloc_heap_pages is called.
>
>
> The invalidation of the full icache is unfortunate but necessary in non-PIPT
> cache. For PIPT cache you could do invalidation by VA.
>
> Limiting the number of call is a nice optimization, but we need to be
> careful how to do it. Until then, a full icache invalidation (or by VA for
> PIPT) is the best solution.
>
>>>> I am also wondering about all the libxc/libxl callers, many of whom
>>>> don't need an icache flush. Ideally we would have an argument in
>>>> XEN_DOMCTL_cacheflush to specify the type of cache flush we need,
>>>> similar to GNTTABOP_cache_flush.
>>>
>>>
>>> Unless the instruction cache will be flushed before the guest is booting,
>>> all
>>> the callers of DOMCTL_cacheflush will require the invalidation.
>>
>>
>> DOMCTL_cacheflush is called several time during the domain build, it is
>> certainly better to do the icache flush once, rather than many times.
>>
>> If we decide to perform one icache flush at domain creation in Xen at
>> the right time, we still need XEN_DOMCTL_cacheflush in its current form
>> to flush the dcache.
>>
>> Also we still need a way to flush the icache to solve Tamas' problem
>> with mem_access at run time.
>>
>> As a consequence, even after introducing one icache flush in Xen at
>> domain creation, I think we still need a new "icache flush" flag in
>> XEN_DOMCTL_cacheflush: all the current callers would not use it, but
>> mem_access userspace components will be able to use it.
>
>
> Why do we need a flag? No matter the way the flag is defined (set -> icache
> invalidation vs set -> no icache invalidation), a user will likely misuse
> it. The hypervisor is the best person to decide whether the icache flush is
> needed. Aside at domain building time, 99.9% (to not say 100%) of the call
> to cacheflush will require the invalidation of the icache.
>
> Furthermore, if you implement the optimization for invalidating PIP icache
> (e.g by VA rather than full), a user will not have the possibility to
> invalidate the full icache.
>
> So I would go the same way as it has been done for tlbflush (see bbb17f6
> "move TLB-flush filtering out into populate_physmap during vm creation").
> Let Xen decides when to optimize the invalidation.
>

Giving the user the option during the domctl to choose which cache to
flush I think would be fine. I would agree that the most likely case
is that both caches should be flushed at the same time, but who knows,
having the option may be useful for someone. At least the linux
syscall has that option as well
(http://man7.org/linux/man-pages/man2/cacheflush.2.html).

In any rate, I already made the patch that implements it:
https://github.com/tklengyel/xen/compare/icache?expand=1. Let me know
if you would want me to send it or not.

Thanks,
Tamas

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel