On Jun 7, 2016 04:30, "Stefano Stabellini" <sstabellini@kernel.org> wrote:
>
> 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.

Ack. As I said I have no use for conditional SMCs at all so this is beyond
what I am looking for. From my perspective it is just easier to forward
every trap.

So as for doing the actual filtering, Julien mentioned he is going to add a
patch for that. I'll wait for that and then rebase on top.

Thanks,
Tamas