From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tamas K Lengyel Subject: Re: [PATCH v5 5/9] monitor: ARM SMC events Date: Tue, 7 Jun 2016 10:06:34 -0600 Message-ID: References: <1464907946-19242-1-git-send-email-tamas@tklengyel.com> <57515292.40100@arm.com> <5751977A.6080907@arm.com> <20160604090352.GA16305@toto> <57559CC5.7070805@arm.com> <5755A6EF.9090403@arm.com> <5756901D02000078000F26DF@prv-mh.provo.novell.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============8677794330375308478==" Return-path: Received: from mail6.bemta6.messagelabs.com ([85.158.143.247]) by lists.xenproject.org with esmtp (Exim 4.84_2) (envelope-from ) id 1bAJWZ-0002Em-QO for xen-devel@lists.xenproject.org; Tue, 07 Jun 2016 16:06:39 +0000 Received: by mail-wm0-f67.google.com with SMTP id n184so25407253wmn.1 for ; Tue, 07 Jun 2016 09:06:37 -0700 (PDT) Received: from mail-wm0-f49.google.com (mail-wm0-f49.google.com. [74.125.82.49]) by smtp.gmail.com with ESMTPSA id b201sm20157042wmb.9.2016.06.07.09.06.35 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 07 Jun 2016 09:06:36 -0700 (PDT) Received: by mail-wm0-f49.google.com with SMTP id k204so75956510wmk.0 for ; Tue, 07 Jun 2016 09:06:35 -0700 (PDT) In-Reply-To: List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xen.org Sender: "Xen-devel" To: Stefano Stabellini Cc: "Edgar E. Iglesias" , Julien Grall , Steve Capper , Jan Beulich , Xen-devel List-Id: xen-devel@lists.xenproject.org --===============8677794330375308478== Content-Type: multipart/alternative; boundary=001a1130cc6e84b2ac0534b25f1e --001a1130cc6e84b2ac0534b25f1e Content-Type: text/plain; charset=UTF-8 On Jun 7, 2016 04:30, "Stefano Stabellini" wrote: > > On Tue, 7 Jun 2016, Jan Beulich wrote: > > >>> On 06.06.16 at 18:38, wrote: > > > On 06/06/16 17:14, Tamas K Lengyel wrote: > > >> On Mon, Jun 6, 2016 at 9:56 AM, Tamas K Lengyel wrote: > > >>> On Mon, Jun 6, 2016 at 9:54 AM, Julien Grall 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 --001a1130cc6e84b2ac0534b25f1e Content-Type: text/html; charset=UTF-8


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

--001a1130cc6e84b2ac0534b25f1e-- --===============8677794330375308478== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KWGVuLWRldmVs IG1haWxpbmcgbGlzdApYZW4tZGV2ZWxAbGlzdHMueGVuLm9yZwpodHRwOi8vbGlzdHMueGVuLm9y Zy94ZW4tZGV2ZWwK --===============8677794330375308478==--