From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tamas K Lengyel Subject: Re: [PATCH] arm/monitor vm-events: Implement guest-request support Date: Fri, 19 Feb 2016 10:54:13 -0700 Message-ID: References: <1455824116-13783-1-git-send-email-czuzu@bitdefender.com> <56C73B22.1060702@bitdefender.com> <56C73D5D.3080903@citrix.com> <56C73E7F.1030204@bitdefender.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============0948605117618127081==" Return-path: In-Reply-To: List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Stefano Stabellini Cc: Keir Fraser , Ian Campbell , Razvan Cojocaru , Andrew Cooper , Xen-devel , Stefano Stabellini , Jan Beulich , Corneliu ZUZU List-Id: xen-devel@lists.xenproject.org --===============0948605117618127081== Content-Type: multipart/alternative; boundary=001a114261d8c8429d052c232b02 --001a114261d8c8429d052c232b02 Content-Type: text/plain; charset=UTF-8 On Fri, Feb 19, 2016 at 10:47 AM, Stefano Stabellini < stefano.stabellini@eu.citrix.com> wrote: > On Fri, 19 Feb 2016, Corneliu ZUZU wrote: > > On 2/19/2016 6:05 PM, Andrew Cooper wrote: > > > On 19/02/16 16:00, Stefano Stabellini wrote: > > > > On Fri, 19 Feb 2016, Corneliu ZUZU wrote: > > > > > On 2/19/2016 3:49 PM, Stefano Stabellini wrote: > > > > > > On Thu, 18 Feb 2016, Corneliu ZUZU wrote: > > > > > > > + > > > > > > > + if ( sync ) > > > > > > > + { > > > > > > > + req->flags |= VM_EVENT_FLAG_VCPU_PAUSED; > > > > > > > + vm_event_vcpu_pause(v); > > > > > > > + } > > > > > > > + > > > > > > > +#if CONFIG_X86 > > > > > > > + if ( altp2m_active(d) ) > > > > > > I would rather > > > > > > > > > > > > #define altp2m_active(d) (0) > > > > > > > > > > > > on ARM, removing the two ifdefs in this file. > > > > > Yeah, I actually wanted to get rid of that too at some point, the > > > > > question is, > > > > > what do I do with "req->altp2m_idx = vcpu_altp2m(v).p2midx"? I'm > not > > > > > familiar > > > > > w/ altp2m design, maybe someone that knows more of the internals > of that > > > > > can > > > > > give a suggestion. > > > > If you #define altp2m_active to (0), gcc will automatically avoid > the if > > > > statement. > > > You will still get the compile error from ARM's struct vcpu not having > > > altp2m information. > > > > > > ~Andrew > > > > > > > Yep. > > Yes, you are right, especially given that Xen is compiled -Wall -Werror. > > How do you plan to introduce altp2m support on ARM? Is there going to be > a struct altp2mvcpu on ARM too? It is not nice to access stuff under > v->arch from common code. Maybe we need another arch_blah function to > set altp2m_idx. > As altp2m could be implemented for ARM as well it might make sense to start introducing bits and pieces that would make it easier to do that work in the future. But I agree, accessing v->arch directly from common is not a good way to go about it. Tamas --001a114261d8c8429d052c232b02 Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable


On Fri, Feb 19, 2016 at 10:47 AM, Stefano Stabellini = <s= tefano.stabellini@eu.citrix.com> wrote:
On Fri, 19 Feb 2016, = Corneliu ZUZU wrote:
> On 2/19/2016 6:05 PM, Andrew Cooper wrote:
> > On 19/02/16 16:00, Stefano Stabellini wrote:
> > > On Fri, 19 Feb 2016, Corneliu ZUZU wrote:
> > > > On 2/19/2016 3:49 PM, Stefano Stabellini wrote:
> > > > > On Thu, 18 Feb 2016, Corneliu ZUZU wrote:
> > > > > > +
> > > > > > +=C2=A0 =C2=A0 if ( sync )
> > > > > > +=C2=A0 =C2=A0 {
> > > > > > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 req->flags |= =3D VM_EVENT_FLAG_VCPU_PAUSED;
> > > > > > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 vm_event_vcpu_pa= use(v);
> > > > > > +=C2=A0 =C2=A0 }
> > > > > > +
> > > > > > +#if CONFIG_X86
> > > > > > +=C2=A0 =C2=A0 if ( altp2m_active(d) )
> > > > > I would rather
> > > > >
> > > > > #define altp2m_active(d) (0)
> > > > >
> > > > > on ARM, removing the two ifdefs in this file.
> > > > Yeah, I actually wanted to get rid of that too at some = point, the
> > > > question is,
> > > > what do I do with "req->altp2m_idx =3D vcpu_alt= p2m(v).p2midx"? I'm not
> > > > familiar
> > > > w/ altp2m design, maybe someone that knows more of the = internals of that
> > > > can
> > > > give a suggestion.
> > > If you #define altp2m_active to (0), gcc will automatically = avoid the if
> > > statement.
> > You will still get the compile error from ARM's struct vcpu n= ot having
> > altp2m information.
> >
> > ~Andrew
> >
>
> Yep.

Yes, you are right, especially given that Xen is compiled -Wall= -Werror.

How do you plan to introduce altp2m support on ARM? Is there going to be a struct altp2mvcpu on ARM too? It is not nice to access stuff under
v->arch from common code. Maybe we need another arch_blah function to set altp2m_idx.

As altp2m could be = implemented for ARM as well it might make sense to start introducing bits a= nd pieces that would make it easier to do that work in the future. But I ag= ree, accessing v->arch directly from common is not a good way to go abou= t it.

Tamas
--001a114261d8c8429d052c232b02-- --===============0948605117618127081== 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 --===============0948605117618127081==--