[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

Hi Stefano,

On 10/16/2018 03:39 AM, Stefano Stabellini wrote:
On 15/10/2018 08:25, Julien Grall wrote:

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
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

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.


Julien Grall

Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.