Re: [Xen-devel] [PATCH v3 4/6] xen/arm: zynqmp: eemi access control

[Julien Grall <julien.grall@xxxxxxx>]

Hi Stefano,

On 10/16/2018 03:39 AM, Stefano Stabellini wrote:
> On 15/10/2018 08:25, Julien Grall wrote:
> > Hi,
> >
> > On 10/10/2018 11:35 PM, Stefano Stabellini wrote:
> > > On Tue, 28 Aug 2018, Julien Grall wrote:
> > > > On 11/08/18 01:01, Stefano Stabellini wrote:
> > > > >     #include <xen/iocap.h>
> > > > >     #include <xen/sched.h>
> > > > >     #include <xen/types.h>
> > > > > @@ -23,6 +91,721 @@
> > > > >     #include <asm/regs.h>
> > > > >     #include <asm/platforms/xilinx-zynqmp-eemi.h>
> > > > >     +struct pm_access
> > > > > +{
> > > > > +    paddr_t addr;
> > > > It seems that the address will always page-aligned. So could we store a
> > > > frame
> > > > using mfn_t?
> > > Yes we could store just the frame. I started to make the change
> > > suggested, and all the required corresponding changes to the
> > > initializations below, for instance:
> > >
> > >     [NODE_RPU] = { MM_RPU },
> > >
> > > needs to become:
> > >
> > >     [NODE_RPU] = { _mfn(MM_RPU) },
> > >
> > > but when I tried to do that, I get a compilation error:
> > >
> > >     xilinx-zynqmp-eemi.c:106:20: error: initializer element is not constant
> > >          [NODE_RPU] = { _mfn(MM_RPU) },
> > >
> > > Unfortunately it is not possible to use mfn_t in static initializations,
> > > because it is a static inline. I could do:
> > This is a bug in GCC older than 5.0.
> >
> > >     [NODE_RPU] = { { MM_RPU } },
> > >
> > > which would work for DEBUG builds but wouldn't work for non-DEBUG
> > > builds.
> > >
> > > I'll keep paddr_t for the next version of the series.
> > What is the issue with that on non-debug build? We are using this
> > construction in another place without any compiler issue.
> I modified the code as suggested again and tried with newer GCCs (both
> 6.3.1 and 7.3.1) but I still get the same error:
>
> xilinx-zynqmp-eemi.c:105:20: error: initializer element is not constant
>       [NODE_RPU] = { _mfn(MM_RPU) },
I actually misremembered the problem. _mfn is using a static inline 
helper which is not considered as a constant.
The correct solution would be (mfn_t){ MM_RPU } but GCC 5.0 (and older) 
does not parse correctly the type cast. So we workaround by dropping the 
cast for the initializer.
On your previous e-mail you said that { { MM_RPU } } would not work for 
debug. One solution would be to introduce _mfn_initializer(...) that 
would be implemented differently for debug and non-debug.
I think such helper would be useful in other context as well.

Cheers,

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel