Christophe Leroy <christophe.le...@csgroup.eu> writes:
> arch/powerpc/mm/ptdump/hashpagetable.c:264:29: warning: restricted __be64
> degrades to integer
> arch/powerpc/mm/ptdump/hashpagetable.c:265:49: warning: restricted __be64
> degrades to integer
> arch/powerpc/mm/ptdump/hashpagetable.c:267:36: warning: incorrect type in
> assignment (different base types)
> arch/powerpc/mm/ptdump/hashpagetable.c:267:36: expected unsigned long
> long [usertype]
> arch/powerpc/mm/ptdump/hashpagetable.c:267:36: got restricted __be64
> [usertype] v
> arch/powerpc/mm/ptdump/hashpagetable.c:268:36: warning: incorrect type in
> assignment (different base types)
> arch/powerpc/mm/ptdump/hashpagetable.c:268:36: expected unsigned long
> long [usertype]
> arch/powerpc/mm/ptdump/hashpagetable.c:268:36: got restricted __be64
> [usertype] r
>
> struct hash_pte fields have type __be64. Convert them to
> regular long before using them.
Your patch changes one side of the comparison but not the other, which
implies the code doesn't work at the moment, ie. it should never be
matching.
But it does work at the moment, so there must be something else going on.
> diff --git a/arch/powerpc/mm/ptdump/hashpagetable.c
> b/arch/powerpc/mm/ptdump/hashpagetable.c
> index c7f824d294b2..bf60ab1bedb9 100644
> --- a/arch/powerpc/mm/ptdump/hashpagetable.c
> +++ b/arch/powerpc/mm/ptdump/hashpagetable.c
> @@ -261,11 +261,11 @@ static int pseries_find(unsigned long ea, int psize,
> bool primary, u64 *v, u64 *
Expanding the context a little:
for (i = 0; i < HPTES_PER_GROUP; i += 4, hpte_group += 4) {
lpar_rc = plpar_pte_read_4(0, hpte_group, (void *)ptes);
> if (lpar_rc)
> continue;
> for (j = 0; j < 4; j++) {
> - if (HPTE_V_COMPARE(ptes[j].v, want_v) &&
> - (ptes[j].v & HPTE_V_VALID)) {
> + if (HPTE_V_COMPARE(be64_to_cpu(ptes[j].v), want_v) &&
> + (be64_to_cpu(ptes[j].v) & HPTE_V_VALID)) {
> /* HPTE matches */
> - *v = ptes[j].v;
> - *r = ptes[j].r;
> + *v = be64_to_cpu(ptes[j].v);
> + *r = be64_to_cpu(ptes[j].r);
> return 0;
> }
> }
Turns out the values returned from plpar_pte_read_4() are already in CPU
endian.
We pass an on-stack buffer to plpar_pte_read_4():
plpar_pte_read_4(0, hpte_group, (void *)ptes);
Which makes it look like the hypercall is writing to memory (our
buffer), so we'd expect the values to need an endian swap.
But plpar_pte_read_4() writes into that buffer from another on-stack
buffer:
static inline long plpar_pte_read_4(unsigned long flags, unsigned long ptex,
unsigned long *ptes)
{
long rc;
unsigned long retbuf[PLPAR_HCALL9_BUFSIZE];
rc = plpar_hcall9(H_READ, retbuf, flags | H_READ_4, ptex);
memcpy(ptes, retbuf, 8*sizeof(unsigned long));
return rc;
}
And the values in that stack buffer are actually returned from the
hypervisor in registers, r4-r11, and written into retbuf by the asm
wrapper:
_GLOBAL_TOC(plpar_hcall9)
HMT_MEDIUM
mfcr r0
stw r0,8(r1)
HCALL_BRANCH(plpar_hcall9_trace)
std r4,STK_PARAM(R4)(r1) /* Save ret buffer */ <- this
is retbuf
mr r4,r5
mr r5,r6
mr r6,r7
mr r7,r8
mr r8,r9
mr r9,r10
ld r10,STK_PARAM(R11)(r1) /* put arg7 in R10 */
ld r11,STK_PARAM(R12)(r1) /* put arg8 in R11 */
ld r12,STK_PARAM(R13)(r1) /* put arg9 in R12 */
HVSC /* invoke the hypervisor */
mr r0,r12
ld r12,STK_PARAM(R4)(r1) <- reload retbuf into r12
std r4, 0(r12)
std r5, 8(r12)
std r6, 16(r12)
std r7, 24(r12)
std r8, 32(r12)
std r9, 40(r12)
std r10,48(r12)
std r11,56(r12)
std r0, 64(r12)
Although the values are BE in memory in the actual HPT, they're read by
the hypervisor which does the byte swap for us, and then when the
hypervisor returns they're returned in the registers. So there's no
extra byte swap needed.
Possibly we should move struct hash_pte into hash_native.c, which is
where it's almost exclusively used, and is used to point to actual HPTEs
in memory.
But for now I think the patch below is a minimal fix for this sparse
warning, it's what other callers of plpar_pte_read_4() are doing.
cheers
diff --git a/arch/powerpc/mm/ptdump/hashpagetable.c
b/arch/powerpc/mm/ptdump/hashpagetable.c
index c7f824d294b2..9a601587836b 100644
--- a/arch/powerpc/mm/ptdump/hashpagetable.c
+++ b/arch/powerpc/mm/ptdump/hashpagetable.c
@@ -238,7 +238,10 @@ static int native_find(unsigned long ea, int psize, bool
primary, u64 *v, u64
static int pseries_find(unsigned long ea, int psize, bool primary, u64 *v, u64
*r)
{
- struct hash_pte ptes[4];
+ struct {
+ unsigned long v;
+ unsigned long r;
+ } ptes[4];
unsigned long vsid, vpn, hash, hpte_group, want_v;
int i, j, ssize = mmu_kernel_ssize;
long lpar_rc = 0;
