Correct padding length for RISC-V PCPU data.

[jhb]

Summary

There was an additional 7 bytes of compiler-inserted padding at the
end of the structure visible via 'ptype /o' in gdb.

Obtained from: CheriBSD

Test Plan

• built a kernel and compared ptype /o before/after

[jhb]

Before:

(gdb) ptype /o struct pcpu
/* offset    |  size */  type = struct pcpu {
/*    0      |     8 */    struct thread *pc_curthread;
/*    8      |     8 */    struct thread *pc_idlethread;
/*   16      |     8 */    struct thread *pc_fpcurthread;
/*   24      |     8 */    struct thread *pc_deadthread;
/*   32      |     8 */    struct pcb *pc_curpcb;
/*   40      |    16 */    void *pc_sched;
/*   48      |     8 */    uint64_t pc_switchtime;
/*   56      |     4 */    int pc_switchticks;
/*   60      |     4 */    u_int pc_cpuid;
/*   64      |     8 */    struct {
/*   64      |     8 */        struct pcpu *stqe_next;

                               /* total size (bytes):    8 */
                           } pc_allcpu;
/*   72      |     8 */    struct lock_list_entry *pc_spinlocks;
/*   80      |    40 */    long pc_cp_time[5];
/*  120      |     8 */    struct device *pc_device;
/*  128      |    16 */    void *pc_netisr;
/*  136      |     4 */    int pc_unused1;
/*  140      |     4 */    int pc_domain;
/*  144      |    16 */    struct rm_queue {
/*  144      |     8 */        struct rm_queue * volatile rmq_next;
--Type <RET> for more, q to quit, c to continue without paging--
/*  152      |     8 */        struct rm_queue * volatile rmq_prev;

                               /* total size (bytes):   16 */
                           } pc_rm_queue;
/*  160      |     8 */    uintptr_t pc_dynamic;
/*  168      |     8 */    uint64_t pc_early_dummy_counter;
/*  176      |     8 */    uintptr_t pc_zpcpu_offset;
/*  184      |     8 */    struct pmap *pc_curpmap;
/*  192      |     4 */    uint32_t pc_pending_ipis;
/*  196      |     4 */    uint32_t pc_hart;
/*  200      |    49 */    char __pad[49];
/* XXX  7-byte padding */

                           /* total size (bytes):  256 */
                         }

after:

(gdb) ptype /o struct pcpu
/* offset    |  size */  type = struct pcpu {
/*    0      |     8 */    struct thread *pc_curthread;
/*    8      |     8 */    struct thread *pc_idlethread;
/*   16      |     8 */    struct thread *pc_fpcurthread;
/*   24      |     8 */    struct thread *pc_deadthread;
/*   32      |     8 */    struct pcb *pc_curpcb;
/*   40      |     8 */    void *pc_sched;
/*   48      |     8 */    uint64_t pc_switchtime;
/*   56      |     4 */    int pc_switchticks;
/*   60      |     4 */    u_int pc_cpuid;
/*   64      |     8 */    struct {
/*   64      |     8 */        struct pcpu *stqe_next;

                               /* total size (bytes):    8 */
                           } pc_allcpu;
/*   72      |     8 */    struct lock_list_entry *pc_spinlocks;
/*   80      |    40 */    long pc_cp_time[5];
/*  120      |     8 */    struct device *pc_device;
/*  128      |     8 */    void *pc_netisr;
/*  136      |     4 */    int pc_unused1;
/*  140      |     4 */    int pc_domain;
/*  144      |    16 */    struct rm_queue {
/*  144      |     8 */        struct rm_queue * volatile rmq_next;
--Type <RET> for more, q to quit, c to continue without paging--
/*  152      |     8 */        struct rm_queue * volatile rmq_prev;

                               /* total size (bytes):   16 */
                           } pc_rm_queue;
/*  160      |     8 */    uintptr_t pc_dynamic;
/*  168      |     8 */    uint64_t pc_early_dummy_counter;
/*  176      |     8 */    uintptr_t pc_zpcpu_offset;
/*  184      |     8 */    struct pmap *pc_curpmap;
/*  192      |     4 */    uint32_t pc_pending_ipis;
/*  196      |     4 */    uint32_t pc_hart;
/*  200      |    56 */    char __pad[56];

                           /* total size (bytes):  256 */
                         }

[jrtc27]

1. Can we please properly document where this magic value comes from?

2. Can we make pcpu_aux.h also assert that offsetof(__pad) + sizeof(__pad) == sizeof(struct pcpu) so all the padding is accounted for?

[jrtc27]

sys/riscv/include/pcpu.h

51 ↗

(On Diff #75084)

i.e. something like this is probably enough for explaining why this exists and where the value comes from

[mhorne]

Nice find. The patch seems fine as is, but @jrtc27's suggestions are worth implementing.

sys/riscv/include/pcpu.h

51 ↗

(On Diff #75084)

Seems that the alignment is to CACHE_LINE_SIZE, not page boundary.

[jrtc27]

sys/riscv/include/pcpu.h

51 ↗

(On Diff #75084)

Ah, it's not even that:

sys/riscv/include/pcpu_aux.h:_Static_assert(PAGE_SIZE % sizeof(struct pcpu) == 0, "fix pcpu size");

It's checking that they are a _factor_ of PAGE_SIZE, presumably so we can guarantee that only one page needs to be mapped for each PCPU (i.e. not just that it fits in a page, but also that it never straddles two pages)?

(except for amd64/i386, which do sizeof(struct pcpu) == UMA_PCPU_ALLOC_SIZE, where the latter is just defined to PAGE_SIZE in sys/sys/pcpu.h)

[jhb]

• Add a comment and static assertion for pad size.

[mhorne]

sys/riscv/include/pcpu_aux.h

50 ↗

(On Diff #75403)

Neat, I hadn't considered that you could get the size of a struct member that way.