On Fri, Feb 19, 2016 at 11:11 AM, Corneliu ZUZU <czuzu@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.stabellini@eu.citrix.com>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
>
>
> 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