On 2/19/2016 8:42 PM, Tamas K Lengyel wrote:


On Fri, Feb 19, 2016 at 11:33 AM, Corneliu ZUZU <cz...@bitdefender.com <mailto:cz...@bitdefender.com>> wrote:

    On 2/19/2016 8:27 PM, Tamas K Lengyel wrote:


    On Fri, Feb 19, 2016 at 11:11 AM, Corneliu ZUZU
    <cz...@bitdefender.com <mailto:cz...@bitdefender.com>> wrote:

        On 2/19/2016 7:54 PM, Tamas K Lengyel wrote:


        On Fri, Feb 19, 2016 at 10:47 AM, Stefano Stabellini
        <stefano.stabell...@eu.citrix.com
        <mailto:stefano.stabell...@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

        I am not at all familiar w/ altp2m at the moment, but I'll
        try to look into it.
        Since that doesn't relate so much with the code motion of
        this changeset and it might not be that straightforward to
        implement, would it be ok to leave the #ifdef CONFIG_X86
        there for now and remove it in a separate patch?


    We are trying to avoid having to do ifdefs where-ever possible.
    So in this case too introducing arch-specific function(s) that
    are empty for ARM would be more appropriate.

    Tamas


    I understand that, I was merely asking if it would be okay to do
    it in another patch, because it didn't seem that straightforward.
    More concretely, are you suggesting to:
    * do the "#define altp2m_active(d) (0)" as Stefano suggested


That is easy enough to implement so go with that.

    * incorporate "vcpu_altp2m(v).p2midx" into an arch_foo function


Implement an arch-specific function for this, say p2m_get_vcpu_altp2m_idx(v) which would just return 0 on ARM for now.

Tamas


Got it.

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

Reply via email to