On 27.01.20 15:58, Ralf Ramsauer wrote:


On 27/01/2020 15:45, Jan Kiszka wrote:
On 27.01.20 15:22, Ralf Ramsauer wrote:


On 27/01/2020 14:39, Jan Kiszka wrote:
On 27.01.20 13:15, Jan Kiszka wrote:
On 27.01.20 12:40, Ralf Ramsauer wrote:


On 27/01/2020 07:37, Jan Kiszka wrote:
From: Jan Kiszka <[email protected]>

Better pad than rely on both sides using the same compiler logic.

Ack. But shouldn't we then, in turn, use __attribute__((unpacked)) to
avoid that the compiler accidentally does some other unintended
alignment / reordering?

Do you mean "packed"? It shouldn't be needed at this stage, but it
shouldn't harm as well.



Signed-off-by: Jan Kiszka <[email protected]>
---
    include/arch/arm-common/asm/jailhouse_hypercall.h | 1 +
    1 file changed, 1 insertion(+)

diff --git a/include/arch/arm-common/asm/jailhouse_hypercall.h
b/include/arch/arm-common/asm/jailhouse_hypercall.h
index 83cec97b..aeab2816 100644
--- a/include/arch/arm-common/asm/jailhouse_hypercall.h
+++ b/include/arch/arm-common/asm/jailhouse_hypercall.h
@@ -38,6 +38,7 @@

    #define COMM_REGION_COMMON_PLATFORM_INFO    \
        __u8 gic_version;            \
+    __u8 padding[7];            \
        __u64 gicd_base;            \
        __u64 gicc_base;            \
        __u64 gicr_base;            \

BTW: It's really hard to trace how the structures are being defined.

Instead of creating the structure in arch-specific parts for each
architecture, I think it would be nicer to introduce the structure for
all architectures, and then include arch-specific parts.

...as anonymous sub-structs - possibly.

Not that easy: The anonymous struct thing does not work, and the doxygen
documentation would have to be refactored as well.

Ack, just realised that as well.

Have a look at the attachment. If you agree on this idea, I'll make a
proper patch out of it.

Not truly convinced as this pulls arch-specific stuff into the generic
header while we have asm headers in the loop. To avoid that, either use
non-anonymous arch structs or defines again (I was considering the latter).

I already tried the latter one, and looks like that's not possible: We
need to have the structure in place before we include arch-specific
headers, as the routines in the header already make use of them. Forward
declaration isn't sufficient.

Ah, now I see: jailhouse_send_msg_to_cell/reply_from_cell is defined in
the arch header and needs jailhouse_comm_region ready.


I'd go for the non-anonymous arch structs then, but still leave them in
the generic header.

That won't change the picture. My idea was to have the struct composed
of another one, defined in that arch header. But that won't fly. Unless
we split headers. Doesn't look like that would add much value.

I've sent some alternative cleanup, though.

Jan

--
You received this message because you are subscribed to the Google Groups 
"Jailhouse" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
To view this discussion on the web visit 
https://groups.google.com/d/msgid/jailhouse-dev/1173c9e8-4f39-bd9d-6b8b-cb4c3ac263ed%40web.de.

Reply via email to