Re: [Xen-devel] [PATCH v3] xen: introduce VCPUOP_register_runstate_phys_memory_area hypercall

From: "Jan Beulich" <JBeulich@suse.com>
To: "Julien Grall" <julien.grall@arm.com>
Cc: Tim Deegan <tim@xen.org>,
	Stefano Stabellini <sstabellini@kernel.org>,
	Wei Liu <wei.liu2@citrix.com>,
	Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>,
	George Dunlap <George.Dunlap@eu.citrix.com>,
	Andrew Cooper <andrew.cooper3@citrix.com>,
	Ian Jackson <Ian.Jackson@eu.citrix.com>,
	andrii.anisov@gmail.com,
	xen-devel <xen-devel@lists.xenproject.org>,
	"andrii_anisov@epam.com" <andrii_anisov@epam.com>,
	Roger Pau Monne <roger.pau@citrix.com>
Subject: Re: [Xen-devel] [PATCH v3] xen: introduce VCPUOP_register_runstate_phys_memory_area hypercall
Date: Tue, 11 Jun 2019 03:10:06 -0600	[thread overview]
Message-ID: <5CFF6FEE0200007800236E11@prv1-mh.provo.novell.com> (raw)
In-Reply-To: <7a225ac4-f1e0-7cf8-b697-ea1f985f2dc8@arm.com>

>>> On 10.06.19 at 13:44, <julien.grall@arm.com> wrote:
> Hi Jan,
> 
> On 07/06/2019 15:23, Jan Beulich wrote:
>>>>> On 24.05.19 at 20:12, <andrii.anisov@gmail.com> wrote:
>>> From: Andrii Anisov <andrii_anisov@epam.com>
>>>
>>> Existing interface to register runstate are with its virtual address
>>> is prone to issues which became more obvious with KPTI enablement in
>>> guests. The nature of those issues is the fact that the guest could
>>> be interrupted by the hypervisor at any time, and there is no guarantee
>>> to have the registered virtual address translated with the currently
>>> available guest's page tables. Before the KPTI such a situation was
>>> possible in case the guest is caught in the middle of PT processing
>>> (e.g. superpage shattering). With the KPTI this happens also when the
>>> guest runs userspace, so has a pretty high probability.
>> 
>> Except when there's no need for KPTI in the guest in the first place,
>> as is the case for x86-64 PV guests. I think this is worthwhile clarifying.
> 
> I am not sure what is your point here. At least on Arm, using virtual address is 
> not safe at all (whether KPTI is used or not). A guest can genuinely decides to 
> shatter the mapping where the virtual address is. On Arm, this require to use 
> the break-before-make sequence. It means the translation VA -> PA may fail is 
> you happen to do it while the guest is using the sequence.
> 
> Some of the intermittent issues I have seen on the Arndale in the past [1] might 
> be related to using virtual address. I am not 100% sure because even if the 
> debug, the error does not make sense. But this is the most plausible reason for 
> the failure.

All fine, but Arm-specific. The point of my comment was to ask to call
out that there is one environment (x86-64 PV) where this KPTI
discussion is entirely inapplicable.

>>> @@ -35,8 +37,16 @@ arch_compat_vcpu_op(
>>>                !compat_handle_okay(area.addr.h, 1) )
>>>               break;
>>>   
>>> +        while( xchg(&v->runstate_in_use, 1) == 0);
>> 
>> At the very least such loops want a cpu_relax() in their bodies.
>> But this being on a hypercall path - are there theoretical guarantees
>> that a guest can't abuse this to lock up a CPU?
> Hmmm, I suggested this but it looks like a guest may call the hypercall multiple 
> time from different vCPU. So this could be a way to delay work on the CPU.
> 
> I wanted to make the context switch mostly lockless and therefore avoiding to 
> introduce a spinlock.

Well, constructs like the above are trying to mimic a spinlock
without actually using a spinlock. There are extremely rare
situation in which this may indeed be warranted, but here it
falls in the common "makes things worse overall" bucket, I
think. To not unduly penalize the actual update paths, I think
using a r/w lock would be appropriate here.

Jan

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