From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tamas K Lengyel Subject: Re: [PATCH for-4.5 v8 04/19] xen: Relocate p2m_mem_access_resume to mem_access common Date: Tue, 23 Sep 2014 16:04:15 +0200 Message-ID: References: <1411478070-13836-1-git-send-email-tklengyel@sec.in.tum.de> <1411478070-13836-5-git-send-email-tklengyel@sec.in.tum.de> <542191B40200007800037B8D@mail.emea.novell.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============6600155089005222834==" Return-path: In-Reply-To: <542191B40200007800037B8D@mail.emea.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich Cc: Ian Campbell , Tim Deegan , Julien Grall , Ian Jackson , "xen-devel@lists.xen.org" , Stefano Stabellini , Andres Lagar-Cavilla , Daniel De Graaf , Tamas K Lengyel List-Id: xen-devel@lists.xenproject.org --===============6600155089005222834== Content-Type: multipart/alternative; boundary=001a11333838eb45650503bc0abd --001a11333838eb45650503bc0abd Content-Type: text/plain; charset=ISO-8859-1 On Tue, Sep 23, 2014 at 3:28 PM, Jan Beulich wrote: > >>> On 23.09.14 at 15:14, wrote: > > Relocate p2m_mem_access_resume to common and abstract the new > > p2m_mem_event_emulate_check into the p2m layer to. > > > > Signed-off-by: Tamas K Lengyel > > Acked-by: Tim Deegan > > --- > > v8: Abstract p2m_mem_event_emulate_check. > > This is a pretty big change compared to the size of this patch, so > retaining the ack wasn't really appropriate. That's even more so > because you didn't even retain previous behavior: > I was debating about it and I had it as a separate patch first. If required, should be no problem and I can resend the series before the feature freeze. > > > +void p2m_mem_event_emulate_check(struct domain *d, const > mem_event_response_t *rsp) > > +{ > > + /* Mark vcpu for skipping one instruction upon rescheduling. */ > > + if ( rsp->flags & MEM_EVENT_FLAG_EMULATE ) > > + { > > + struct vcpu *v = current; > > Compare this with ... > > > -void p2m_mem_access_resume(struct domain *d) > > -{ > > - mem_event_response_t rsp; > > - > > - /* Pull all responses off the ring */ > > - while( mem_event_get_response(d, &d->mem_event->access, &rsp) ) > > - { > > - struct vcpu *v; > > - > > - if ( rsp.flags & MEM_EVENT_FLAG_DUMMY ) > > - continue; > > - > > - /* Validate the vcpu_id in the response. */ > > - if ( (rsp.vcpu_id >= d->max_vcpus) || !d->vcpu[rsp.vcpu_id] ) > > - continue; > > - > > - v = d->vcpu[rsp.vcpu_id]; > > ... the original code. I.e. you should pass v instead of d into > p2m_mem_event_emulate_check(). > Ah good point, but I think passing d is better (as it is also required further down), I just have to make v = d->vcpu[rsp.vcpu_id] instead of current. Tamas > > Jan > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel > --001a11333838eb45650503bc0abd Content-Type: text/html; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable


On Tue, Sep 23, 2014 at 3:28 PM, Jan Beulich <JBeulich@suse.com>= ; wrote:
>>> On 23.09.14 at 15:14, <tklengyel@sec.in.tum.de> wrote:
> Relocate p2m_mem_access_resume to common and abstract the new
> p2m_mem_event_emulate_check into the p2m layer to.
>
> Signed-off-by: Tamas K Lengyel <tklengyel@sec.in.tum.de>
> Acked-by: Tim Deegan <tim@xen.org>
> ---
> v8: Abstract p2m_mem_event_emulate_check.

This is a pretty big change compared to the size of this patch, so retaining the ack wasn't really appropriate. That's even more so because you didn't even retain previous behavior:

> +void p2m_mem_event_emulate_check(struct domain *d, const mem_event_re= sponse_t *rsp)
> +{
> +=A0 =A0 /* Mark vcpu for skipping one instruction upon rescheduling. = */
> +=A0 =A0 if ( rsp->flags & MEM_EVENT_FLAG_EMULATE )
> +=A0 =A0 {
> +=A0 =A0 =A0 =A0 struct vcpu *v =3D current;

Compare this with ...

> -void p2m_mem_access_resume(struct domain *d)
> -{
> -=A0 =A0 mem_event_response_t rsp;
> -
> -=A0 =A0 /* Pull all responses off the ring */
> -=A0 =A0 while( mem_event_get_response(d, &d->mem_event->acc= ess, &rsp) )
> -=A0 =A0 {
> -=A0 =A0 =A0 =A0 struct vcpu *v;
> -
> -=A0 =A0 =A0 =A0 if ( rsp.flags & MEM_EVENT_FLAG_DUMMY )
> -=A0 =A0 =A0 =A0 =A0 =A0 continue;
> -
> -=A0 =A0 =A0 =A0 /* Validate the vcpu_id in the response. */
> -=A0 =A0 =A0 =A0 if ( (rsp.vcpu_id >=3D d->max_vcpus) || !d->= vcpu[rsp.vcpu_id] )
> -=A0 =A0 =A0 =A0 =A0 =A0 continue;
> -
> -=A0 =A0 =A0 =A0 v =3D d->vcpu[rsp.vcpu_id];

... the original code. I.e. you should pass v instead of d into
p2m_mem_event_emulate_check().


Jan


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

--001a11333838eb45650503bc0abd-- --===============6600155089005222834== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel --===============6600155089005222834==--