Re: [PATCH v5 5/9] monitor: ARM SMC events

From: Stefano Stabellini <sstabellini@kernel.org>
To: Jan Beulich <JBeulich@suse.com>
Cc: Tamas K Lengyel <tamas@tklengyel.com>,
	Steve Capper <Steve.Capper@arm.com>,
	Julien Grall <julien.grall@arm.com>,
	Stefano Stabellini <sstabellini@kernel.org>,
	Xen-devel <xen-devel@lists.xenproject.org>,
	"Edgar E. Iglesias" <edgar.iglesias@gmail.com>
Subject: Re: [PATCH v5 5/9] monitor: ARM SMC events
Date: Tue, 7 Jun 2016 11:30:40 +0100 (BST)	[thread overview]
Message-ID: <alpine.DEB.2.10.1606071121400.14126@sstabellini-ThinkPad-X260> (raw)
In-Reply-To: <5756901D02000078000F26DF@prv-mh.provo.novell.com>

On Tue, 7 Jun 2016, Jan Beulich wrote:
> >>> On 06.06.16 at 18:38, <julien.grall@arm.com> wrote:
> > On 06/06/16 17:14, Tamas K Lengyel wrote:
> >> On Mon, Jun 6, 2016 at 9:56 AM, Tamas K Lengyel <tamas@tklengyel.com> wrote:
> >>> On Mon, Jun 6, 2016 at 9:54 AM, Julien Grall <julien.grall@arm.com> wrote:
> >> So either way, I don't see a technical reason why Xen should silently
> >> swallow any SMC trap if the vm_event user specifically asked them to
> >> be forwarded. Other then it being odd that some ARM chips have varying
> >> behavior regarding a subset of SMC instructions, it should not affect
> >> when the vm_event user gets the events. If the user requests that it
> >> wants to get notified any time an SMC is trapped to the VMM, it
> >> should, regardless of whether that makes sense for "us". Depending on
> >> the use-case of the user, indeed it may need extra information if it
> >> wants to do emulation. If that need arises, the interface can easily
> >> be extended to accommodate that usecase. We can also add a comment
> >> saying that the forwarded events may also include ones with failed
> >> condition checks depending on the CPU implementation. Also, it would
> >> also be possible in the future to add a monitor configuration bit
> >> where the user can specify if it wants the failed condition check SMCs
> >> ignored by default or not. At this time however I want to start simple
> >> and just forward all events, adding more bits and pieces only as
> >> needed.
> > 
> > We disagree on what is a "starting simple". It easier to relax than 
> > restricting a behavior later one.
> > 
> > Even if we decide to add a bit to ignore some SMC in a later version of 
> > Xen, the introspection app will need to carry the burden mentioned in 
> > lengthly way on the previous mails because they may want to support 
> > older version of Xen.
> 
> FWIW, I'm with Julien here given the information available so far
> on this thread. Some of the basic problem is that the original
> patch (and namely its modification to the public header) doesn't
> really make clear what's intended: To intercept all SMC instruction
> uses (aiui that's impossible on some hardware) or to intercept all
> privileged calls resulting from their use (in which case instances
> with the condition being false wouldn't count).

Right. I think that the first thing to do would be to write down in the
public header file what is the intended behavior. Given the scope for
confusion, this is necessary regardless of the chosen behavior.

> What you, Tamas, want to get to seems to be some middle
> ground, which I don't see what use it would be to the consumer.

I think that forwarding SMC events only for unconditional SMCs and SMCs
which succeeded the conditional check would make for a better interface.
This would be my preference.

If you really want to forward SMC events for SMCs which failed the
conditional check, then please add to the SMC event struct all the
necessary information so that the monitoring application can quickly
find out whether the conditional check succeeded or failed without
jumping through hoops.

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