linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 00/12] Miscellaneous Sparse fixes
@ 2023-10-11  5:36 Benjamin Gray
  2023-10-11  5:37 ` [PATCH 01/12] powerpc/xive: Fix endian conversion size Benjamin Gray
                   ` (12 more replies)
  0 siblings, 13 replies; 16+ messages in thread
From: Benjamin Gray @ 2023-10-11  5:36 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Benjamin Gray

There are many Sparse warnings in the kernel, including the powerpc
directory. This series provides fixes for some low-hanging fruit found
when trying to triage the warnings earlier this year. It addresses about
100 warnings (many have the same root cause).

I know there's concerns about making it harder to backport things. In
general, as someone who was not around during the development of these
features, I think that it is useful make the annotations as correct as
possible. But it's no fuss if some/all of the patches are nacked for this
reason. I just figured some of it might be useful instead of continuing to
sit on it indefinitely.

Benjamin Gray (12):
  powerpc/xive: Fix endian conversion size
  powerpc/pseries: Restructure hvc_get_chars() endianness
  powerpc: Explicitly reverse bytes when checking for byte reversal
  powerpc: Use NULL instead of 0 for null pointers
  powerpc: Remove extern from function implementations
  powerpc: Annotate endianness of various variables and functions
  powerpc/kvm: Force cast endianness of KVM shared regs
  powerpc/opal: Annotate out param endianness
  powerpc/uaccess: Cast away __user annotation after verification
  powerpc: Cast away __iomem in low level IO routines
  powerpc/eeh: Remove unnecessary cast
  powerpc/fadump: Annotate endianness cast with __force

 arch/powerpc/include/asm/book3s/64/pgtable.h    |  2 +-
 arch/powerpc/include/asm/imc-pmu.h              | 16 ++++++++--------
 arch/powerpc/include/asm/kvm_ppc.h              |  8 ++++----
 arch/powerpc/include/asm/opal.h                 |  2 +-
 arch/powerpc/include/asm/uaccess.h              |  2 +-
 arch/powerpc/kernel/eeh_driver.c                |  4 ++--
 arch/powerpc/kernel/io.c                        | 12 ++++++------
 arch/powerpc/kernel/iommu.c                     |  8 ++++----
 arch/powerpc/kernel/prom_init.c                 |  2 +-
 arch/powerpc/kernel/setup_64.c                  |  2 +-
 arch/powerpc/kernel/traps.c                     |  4 ++--
 arch/powerpc/kexec/core_64.c                    |  4 ++--
 arch/powerpc/kexec/file_load_64.c               |  6 +++---
 arch/powerpc/kvm/book3s_64_vio.c                |  8 ++++----
 arch/powerpc/kvm/book3s_xive_native.c           |  2 +-
 arch/powerpc/mm/drmem.c                         |  2 +-
 arch/powerpc/net/bpf_jit_comp.c                 |  8 ++++----
 arch/powerpc/perf/hv-24x7.c                     |  2 +-
 arch/powerpc/perf/imc-pmu.c                     | 11 ++++++-----
 arch/powerpc/platforms/4xx/soc.c                |  2 +-
 arch/powerpc/platforms/powermac/feature.c       |  3 ++-
 arch/powerpc/platforms/powernv/opal-fadump.h    |  2 +-
 arch/powerpc/platforms/pseries/hotplug-memory.c |  3 ++-
 arch/powerpc/platforms/pseries/hvconsole.c      |  6 +++---
 arch/powerpc/platforms/pseries/lpar.c           |  8 ++++----
 arch/powerpc/sysdev/mpic.c                      |  2 +-
 arch/powerpc/sysdev/xive/native.c               |  2 +-
 27 files changed, 68 insertions(+), 65 deletions(-)

-- 
2.39.2

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 01/12] powerpc/xive: Fix endian conversion size
  2023-10-11  5:36 [PATCH 00/12] Miscellaneous Sparse fixes Benjamin Gray
@ 2023-10-11  5:37 ` Benjamin Gray
  2023-10-11  5:37 ` [PATCH 02/12] powerpc/pseries: Restructure hvc_get_chars() endianness Benjamin Gray
                   ` (11 subsequent siblings)
  12 siblings, 0 replies; 16+ messages in thread
From: Benjamin Gray @ 2023-10-11  5:37 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Benjamin Gray

Sparse reports a size mismatch in the endian swap. The Opal
implementation[1] passes the value as a __be64, and the receiving
variable out_qsize is a u64, so the use of be32_to_cpu() appears to be
an error.

[1]: https://github.com/open-power/skiboot/blob/80e2b1dc73/hw/xive.c#L3854

Fixes: 88ec6b93c8e7 ("powerpc/xive: add OPAL extensions for the XIVE native exploitation support")
Signed-off-by: Benjamin Gray <bgray@linux.ibm.com>
---
 arch/powerpc/sysdev/xive/native.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/sysdev/xive/native.c b/arch/powerpc/sysdev/xive/native.c
index 9f0af4d795d8..f1c0fa6ece21 100644
--- a/arch/powerpc/sysdev/xive/native.c
+++ b/arch/powerpc/sysdev/xive/native.c
@@ -802,7 +802,7 @@ int xive_native_get_queue_info(u32 vp_id, u32 prio,
 	if (out_qpage)
 		*out_qpage = be64_to_cpu(qpage);
 	if (out_qsize)
-		*out_qsize = be32_to_cpu(qsize);
+		*out_qsize = be64_to_cpu(qsize);
 	if (out_qeoi_page)
 		*out_qeoi_page = be64_to_cpu(qeoi_page);
 	if (out_escalate_irq)
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 02/12] powerpc/pseries: Restructure hvc_get_chars() endianness
  2023-10-11  5:36 [PATCH 00/12] Miscellaneous Sparse fixes Benjamin Gray
  2023-10-11  5:37 ` [PATCH 01/12] powerpc/xive: Fix endian conversion size Benjamin Gray
@ 2023-10-11  5:37 ` Benjamin Gray
  2023-10-30 13:16   ` Aneesh Kumar K.V
  2023-10-11  5:37 ` [PATCH 03/12] powerpc: Explicitly reverse bytes when checking for byte reversal Benjamin Gray
                   ` (10 subsequent siblings)
  12 siblings, 1 reply; 16+ messages in thread
From: Benjamin Gray @ 2023-10-11  5:37 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Benjamin Gray

Sparse reports an endian mismatch in hvc_get_chars().

At first it seemed like the retbuf should be __be64[], but actually
retbuf holds serialized registers returned by the hypervisor call, so
it's correctly CPU endian typed.

Instead, it is the be64_to_cpu() that's misleading. The intent is to do
a byte swap on a little endian CPU. The swap is required because we
wanted to store the register values to memory without 'swapping' bytes,
so that the high order byte of the first register is the first byte
in the result buffer.

In diagram form, on a LE CPU with the letters representing the return
string bytes:

    (register bytes) A B C D E F G H   I J K L M N O P
  (retbuf mem bytes) H G F E D C B A   P O N M L K J I
(buf/lbuf mem bytes) A B C D E F G H   I J K L M N O P

So retbuf stores the registers in CPU endian, and buf always stores in
big endian.

So replace the byte swap function with cpu_to_be64() and cast lbuf as an
array of __be64 to match the semantics closer.

Signed-off-by: Benjamin Gray <bgray@linux.ibm.com>
---
 arch/powerpc/platforms/pseries/hvconsole.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/hvconsole.c b/arch/powerpc/platforms/pseries/hvconsole.c
index 1ac52963e08b..647718a15e78 100644
--- a/arch/powerpc/platforms/pseries/hvconsole.c
+++ b/arch/powerpc/platforms/pseries/hvconsole.c
@@ -29,11 +29,11 @@ int hvc_get_chars(uint32_t vtermno, char *buf, int count)
 {
 	long ret;
 	unsigned long retbuf[PLPAR_HCALL_BUFSIZE];
-	unsigned long *lbuf = (unsigned long *)buf;
+	__be64 *lbuf = (__be64 __force *)buf;
 
 	ret = plpar_hcall(H_GET_TERM_CHAR, retbuf, vtermno);
-	lbuf[0] = be64_to_cpu(retbuf[1]);
-	lbuf[1] = be64_to_cpu(retbuf[2]);
+	lbuf[0] = cpu_to_be64(retbuf[1]);
+	lbuf[1] = cpu_to_be64(retbuf[2]);
 
 	if (ret == H_SUCCESS)
 		return retbuf[0];
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 03/12] powerpc: Explicitly reverse bytes when checking for byte reversal
  2023-10-11  5:36 [PATCH 00/12] Miscellaneous Sparse fixes Benjamin Gray
  2023-10-11  5:37 ` [PATCH 01/12] powerpc/xive: Fix endian conversion size Benjamin Gray
  2023-10-11  5:37 ` [PATCH 02/12] powerpc/pseries: Restructure hvc_get_chars() endianness Benjamin Gray
@ 2023-10-11  5:37 ` Benjamin Gray
  2023-10-11  5:37 ` [PATCH 04/12] powerpc: Use NULL instead of 0 for null pointers Benjamin Gray
                   ` (9 subsequent siblings)
  12 siblings, 0 replies; 16+ messages in thread
From: Benjamin Gray @ 2023-10-11  5:37 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Benjamin Gray

Sparse reports an invalid endian cast here. The code is written for
big endian platforms, so le32_to_cpu() acts as a byte reversal.

This file is checked by sparse on a little endian build though, so
replace the reverse function with the dedicated swab32() function to
better express the intent of the code.

Signed-off-by: Benjamin Gray <bgray@linux.ibm.com>
---
 arch/powerpc/sysdev/mpic.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/sysdev/mpic.c b/arch/powerpc/sysdev/mpic.c
index ba287abcb008..dabbdd356664 100644
--- a/arch/powerpc/sysdev/mpic.c
+++ b/arch/powerpc/sysdev/mpic.c
@@ -355,7 +355,7 @@ static void __init mpic_test_broken_ipi(struct mpic *mpic)
 	mpic_write(mpic->gregs, MPIC_INFO(GREG_IPI_VECTOR_PRI_0), MPIC_VECPRI_MASK);
 	r = mpic_read(mpic->gregs, MPIC_INFO(GREG_IPI_VECTOR_PRI_0));
 
-	if (r == le32_to_cpu(MPIC_VECPRI_MASK)) {
+	if (r == swab32(MPIC_VECPRI_MASK)) {
 		printk(KERN_INFO "mpic: Detected reversed IPI registers\n");
 		mpic->flags |= MPIC_BROKEN_IPI;
 	}
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 04/12] powerpc: Use NULL instead of 0 for null pointers
  2023-10-11  5:36 [PATCH 00/12] Miscellaneous Sparse fixes Benjamin Gray
                   ` (2 preceding siblings ...)
  2023-10-11  5:37 ` [PATCH 03/12] powerpc: Explicitly reverse bytes when checking for byte reversal Benjamin Gray
@ 2023-10-11  5:37 ` Benjamin Gray
  2023-10-11  5:37 ` [PATCH 05/12] powerpc: Remove extern from function implementations Benjamin Gray
                   ` (8 subsequent siblings)
  12 siblings, 0 replies; 16+ messages in thread
From: Benjamin Gray @ 2023-10-11  5:37 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Benjamin Gray

Sparse reports several uses of 0 for pointer arguments and comparisons.
Replace with NULL to better convey the intent. Remove entirely if a
comparison to follow the kernel style of implicit boolean conversions.

Signed-off-by: Benjamin Gray <bgray@linux.ibm.com>
---
 arch/powerpc/kernel/setup_64.c        | 2 +-
 arch/powerpc/kvm/book3s_xive_native.c | 2 +-
 arch/powerpc/net/bpf_jit_comp.c       | 8 ++++----
 arch/powerpc/perf/imc-pmu.c           | 2 +-
 arch/powerpc/platforms/4xx/soc.c      | 2 +-
 arch/powerpc/platforms/pseries/lpar.c | 8 ++++----
 6 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/arch/powerpc/kernel/setup_64.c b/arch/powerpc/kernel/setup_64.c
index 246201d0d879..2f19d5e94485 100644
--- a/arch/powerpc/kernel/setup_64.c
+++ b/arch/powerpc/kernel/setup_64.c
@@ -364,7 +364,7 @@ void __init early_setup(unsigned long dt_ptr)
 	 */
 	initialise_paca(&boot_paca, 0);
 	fixup_boot_paca(&boot_paca);
-	WARN_ON(local_paca != 0);
+	WARN_ON(local_paca);
 	setup_paca(&boot_paca); /* install the paca into registers */
 
 	/* -------- printk is now safe to use ------- */
diff --git a/arch/powerpc/kvm/book3s_xive_native.c b/arch/powerpc/kvm/book3s_xive_native.c
index 712ab91ced39..6e2ebbd8aaac 100644
--- a/arch/powerpc/kvm/book3s_xive_native.c
+++ b/arch/powerpc/kvm/book3s_xive_native.c
@@ -567,7 +567,7 @@ static int kvmppc_xive_native_set_queue_config(struct kvmppc_xive *xive,
 	u8 priority;
 	struct kvm_ppc_xive_eq kvm_eq;
 	int rc;
-	__be32 *qaddr = 0;
+	__be32 *qaddr = NULL;
 	struct page *page;
 	struct xive_q *q;
 	gfn_t gfn;
diff --git a/arch/powerpc/net/bpf_jit_comp.c b/arch/powerpc/net/bpf_jit_comp.c
index 37043dfc1add..316cadf442b2 100644
--- a/arch/powerpc/net/bpf_jit_comp.c
+++ b/arch/powerpc/net/bpf_jit_comp.c
@@ -119,7 +119,7 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *fp)
 	cgctx.stack_size = round_up(fp->aux->stack_depth, 16);
 
 	/* Scouting faux-generate pass 0 */
-	if (bpf_jit_build_body(fp, 0, &cgctx, addrs, 0, false)) {
+	if (bpf_jit_build_body(fp, NULL, &cgctx, addrs, 0, false)) {
 		/* We hit something illegal or unsupported. */
 		fp = org_fp;
 		goto out_addrs;
@@ -134,7 +134,7 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *fp)
 	 */
 	if (cgctx.seen & SEEN_TAILCALL || !is_offset_in_branch_range((long)cgctx.idx * 4)) {
 		cgctx.idx = 0;
-		if (bpf_jit_build_body(fp, 0, &cgctx, addrs, 0, false)) {
+		if (bpf_jit_build_body(fp, NULL, &cgctx, addrs, 0, false)) {
 			fp = org_fp;
 			goto out_addrs;
 		}
@@ -146,9 +146,9 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *fp)
 	 * update ctgtx.idx as it pretends to output instructions, then we can
 	 * calculate total size from idx.
 	 */
-	bpf_jit_build_prologue(0, &cgctx);
+	bpf_jit_build_prologue(NULL, &cgctx);
 	addrs[fp->len] = cgctx.idx * 4;
-	bpf_jit_build_epilogue(0, &cgctx);
+	bpf_jit_build_epilogue(NULL, &cgctx);
 
 	fixup_len = fp->aux->num_exentries * BPF_FIXUP_LEN * 4;
 	extable_len = fp->aux->num_exentries * sizeof(struct exception_table_entry);
diff --git a/arch/powerpc/perf/imc-pmu.c b/arch/powerpc/perf/imc-pmu.c
index 9d229ef7f86e..eba2baabccb0 100644
--- a/arch/powerpc/perf/imc-pmu.c
+++ b/arch/powerpc/perf/imc-pmu.c
@@ -544,7 +544,7 @@ static int nest_imc_event_init(struct perf_event *event)
 			break;
 		}
 		pcni++;
-	} while (pcni->vbase != 0);
+	} while (pcni->vbase);
 
 	if (!flag)
 		return -ENODEV;
diff --git a/arch/powerpc/platforms/4xx/soc.c b/arch/powerpc/platforms/4xx/soc.c
index b2d940437a66..5412e6b21e10 100644
--- a/arch/powerpc/platforms/4xx/soc.c
+++ b/arch/powerpc/platforms/4xx/soc.c
@@ -112,7 +112,7 @@ static int __init ppc4xx_l2c_probe(void)
 	}
 
 	/* Install error handler */
-	if (request_irq(irq, l2c_error_handler, 0, "L2C", 0) < 0) {
+	if (request_irq(irq, l2c_error_handler, 0, "L2C", NULL) < 0) {
 		printk(KERN_ERR "Cannot install L2C error handler"
 		       ", cache is not enabled\n");
 		of_node_put(np);
diff --git a/arch/powerpc/platforms/pseries/lpar.c b/arch/powerpc/platforms/pseries/lpar.c
index f2cb62148f36..b4e86b7ea584 100644
--- a/arch/powerpc/platforms/pseries/lpar.c
+++ b/arch/powerpc/platforms/pseries/lpar.c
@@ -192,9 +192,9 @@ static void free_dtl_buffers(unsigned long *time_limit)
 			continue;
 		kmem_cache_free(dtl_cache, pp->dispatch_log);
 		pp->dtl_ridx = 0;
-		pp->dispatch_log = 0;
-		pp->dispatch_log_end = 0;
-		pp->dtl_curr = 0;
+		pp->dispatch_log = NULL;
+		pp->dispatch_log_end = NULL;
+		pp->dtl_curr = NULL;
 
 		if (time_limit && time_after(jiffies, *time_limit)) {
 			cond_resched();
@@ -223,7 +223,7 @@ static void destroy_cpu_associativity(void)
 {
 	kfree(vcpu_associativity);
 	kfree(pcpu_associativity);
-	vcpu_associativity = pcpu_associativity = 0;
+	vcpu_associativity = pcpu_associativity = NULL;
 }
 
 static __be32 *__get_cpu_associativity(int cpu, __be32 *cpu_assoc, int flag)
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 05/12] powerpc: Remove extern from function implementations
  2023-10-11  5:36 [PATCH 00/12] Miscellaneous Sparse fixes Benjamin Gray
                   ` (3 preceding siblings ...)
  2023-10-11  5:37 ` [PATCH 04/12] powerpc: Use NULL instead of 0 for null pointers Benjamin Gray
@ 2023-10-11  5:37 ` Benjamin Gray
  2023-10-11  5:37 ` [PATCH 06/12] powerpc: Annotate endianness of various variables and functions Benjamin Gray
                   ` (7 subsequent siblings)
  12 siblings, 0 replies; 16+ messages in thread
From: Benjamin Gray @ 2023-10-11  5:37 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Benjamin Gray

Sparse reports several function implementations annotated with extern.
This is clearly incorrect, likely just copied from an actual extern
declaration in another file.

Fix the sparse warnings by removing extern.

Signed-off-by: Benjamin Gray <bgray@linux.ibm.com>
---
 arch/powerpc/kernel/iommu.c      | 8 ++++----
 arch/powerpc/kernel/traps.c      | 4 ++--
 arch/powerpc/kvm/book3s_64_vio.c | 8 ++++----
 3 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/arch/powerpc/kernel/iommu.c b/arch/powerpc/kernel/iommu.c
index 14251bc5219e..3e28579f7c62 100644
--- a/arch/powerpc/kernel/iommu.c
+++ b/arch/powerpc/kernel/iommu.c
@@ -1074,10 +1074,10 @@ int iommu_tce_check_gpa(unsigned long page_shift, unsigned long gpa)
 }
 EXPORT_SYMBOL_GPL(iommu_tce_check_gpa);
 
-extern long iommu_tce_xchg_no_kill(struct mm_struct *mm,
-		struct iommu_table *tbl,
-		unsigned long entry, unsigned long *hpa,
-		enum dma_data_direction *direction)
+long iommu_tce_xchg_no_kill(struct mm_struct *mm,
+			    struct iommu_table *tbl,
+			    unsigned long entry, unsigned long *hpa,
+			    enum dma_data_direction *direction)
 {
 	long ret;
 	unsigned long size = 0;
diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
index 64ff37721fd0..12d128238cfe 100644
--- a/arch/powerpc/kernel/traps.c
+++ b/arch/powerpc/kernel/traps.c
@@ -157,7 +157,7 @@ static int die_owner = -1;
 static unsigned int die_nest_count;
 static int die_counter;
 
-extern void panic_flush_kmsg_start(void)
+void panic_flush_kmsg_start(void)
 {
 	/*
 	 * These are mostly taken from kernel/panic.c, but tries to do
@@ -170,7 +170,7 @@ extern void panic_flush_kmsg_start(void)
 	bust_spinlocks(1);
 }
 
-extern void panic_flush_kmsg_end(void)
+void panic_flush_kmsg_end(void)
 {
 	kmsg_dump(KMSG_DUMP_PANIC);
 	bust_spinlocks(0);
diff --git a/arch/powerpc/kvm/book3s_64_vio.c b/arch/powerpc/kvm/book3s_64_vio.c
index 93b695b289e9..15200d766fc5 100644
--- a/arch/powerpc/kvm/book3s_64_vio.c
+++ b/arch/powerpc/kvm/book3s_64_vio.c
@@ -77,8 +77,8 @@ static void kvm_spapr_tce_liobn_put(struct kref *kref)
 	call_rcu(&stit->rcu, kvm_spapr_tce_iommu_table_free);
 }
 
-extern void kvm_spapr_tce_release_iommu_group(struct kvm *kvm,
-		struct iommu_group *grp)
+void kvm_spapr_tce_release_iommu_group(struct kvm *kvm,
+				       struct iommu_group *grp)
 {
 	int i;
 	struct kvmppc_spapr_tce_table *stt;
@@ -105,8 +105,8 @@ extern void kvm_spapr_tce_release_iommu_group(struct kvm *kvm,
 	rcu_read_unlock();
 }
 
-extern long kvm_spapr_tce_attach_iommu_group(struct kvm *kvm, int tablefd,
-		struct iommu_group *grp)
+long kvm_spapr_tce_attach_iommu_group(struct kvm *kvm, int tablefd,
+				      struct iommu_group *grp)
 {
 	struct kvmppc_spapr_tce_table *stt = NULL;
 	bool found = false;
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 06/12] powerpc: Annotate endianness of various variables and functions
  2023-10-11  5:36 [PATCH 00/12] Miscellaneous Sparse fixes Benjamin Gray
                   ` (4 preceding siblings ...)
  2023-10-11  5:37 ` [PATCH 05/12] powerpc: Remove extern from function implementations Benjamin Gray
@ 2023-10-11  5:37 ` Benjamin Gray
  2023-10-11  5:37 ` [PATCH 07/12] powerpc/kvm: Force cast endianness of KVM shared regs Benjamin Gray
                   ` (6 subsequent siblings)
  12 siblings, 0 replies; 16+ messages in thread
From: Benjamin Gray @ 2023-10-11  5:37 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Benjamin Gray

Sparse reports several endianness warnings on variables and functions
that are consistently treated as big endian. There are no
multi-endianness shenanigans going on here so fix these low hanging
fruit up in one patch.

All changes are just type annotations; no endianness switching
operations are introduced by this patch.

Signed-off-by: Benjamin Gray <bgray@linux.ibm.com>
---
 arch/powerpc/include/asm/book3s/64/pgtable.h    |  2 +-
 arch/powerpc/include/asm/imc-pmu.h              | 16 ++++++++--------
 arch/powerpc/kernel/prom_init.c                 |  2 +-
 arch/powerpc/kexec/core_64.c                    |  4 ++--
 arch/powerpc/kexec/file_load_64.c               |  6 +++---
 arch/powerpc/mm/drmem.c                         |  2 +-
 arch/powerpc/perf/hv-24x7.c                     |  2 +-
 arch/powerpc/perf/imc-pmu.c                     |  9 +++++----
 arch/powerpc/platforms/powermac/feature.c       |  3 ++-
 arch/powerpc/platforms/pseries/hotplug-memory.c |  3 ++-
 10 files changed, 26 insertions(+), 23 deletions(-)

diff --git a/arch/powerpc/include/asm/book3s/64/pgtable.h b/arch/powerpc/include/asm/book3s/64/pgtable.h
index 5c497c862d75..7b0d0f9d343a 100644
--- a/arch/powerpc/include/asm/book3s/64/pgtable.h
+++ b/arch/powerpc/include/asm/book3s/64/pgtable.h
@@ -647,7 +647,7 @@ static inline pte_t pte_mkuser(pte_t pte)
  */
 static inline int pte_devmap(pte_t pte)
 {
-	u64 mask = cpu_to_be64(_PAGE_DEVMAP | _PAGE_PTE);
+	__be64 mask = cpu_to_be64(_PAGE_DEVMAP | _PAGE_PTE);
 
 	return (pte_raw(pte) & mask) == mask;
 }
diff --git a/arch/powerpc/include/asm/imc-pmu.h b/arch/powerpc/include/asm/imc-pmu.h
index 699a88584ae1..a656635df386 100644
--- a/arch/powerpc/include/asm/imc-pmu.h
+++ b/arch/powerpc/include/asm/imc-pmu.h
@@ -74,14 +74,14 @@ struct imc_events {
  * The following is the data structure to hold trace imc data.
  */
 struct trace_imc_data {
-	u64 tb1;
-	u64 ip;
-	u64 val;
-	u64 cpmc1;
-	u64 cpmc2;
-	u64 cpmc3;
-	u64 cpmc4;
-	u64 tb2;
+	__be64 tb1;
+	__be64 ip;
+	__be64 val;
+	__be64 cpmc1;
+	__be64 cpmc2;
+	__be64 cpmc3;
+	__be64 cpmc4;
+	__be64 tb2;
 };
 
 /* Event attribute array index */
diff --git a/arch/powerpc/kernel/prom_init.c b/arch/powerpc/kernel/prom_init.c
index d464ba412084..e67effdba85c 100644
--- a/arch/powerpc/kernel/prom_init.c
+++ b/arch/powerpc/kernel/prom_init.c
@@ -947,7 +947,7 @@ struct option_vector7 {
 } __packed;
 
 struct ibm_arch_vec {
-	struct { u32 mask, val; } pvrs[14];
+	struct { __be32 mask, val; } pvrs[14];
 
 	u8 num_vectors;
 
diff --git a/arch/powerpc/kexec/core_64.c b/arch/powerpc/kexec/core_64.c
index a79e28c91e2b..0bee7ca9a77c 100644
--- a/arch/powerpc/kexec/core_64.c
+++ b/arch/powerpc/kexec/core_64.c
@@ -379,8 +379,8 @@ void default_machine_kexec(struct kimage *image)
 
 #ifdef CONFIG_PPC_64S_HASH_MMU
 /* Values we need to export to the second kernel via the device tree. */
-static unsigned long htab_base;
-static unsigned long htab_size;
+static __be64 htab_base;
+static __be64 htab_size;
 
 static struct property htab_base_prop = {
 	.name = "linux,htab-base",
diff --git a/arch/powerpc/kexec/file_load_64.c b/arch/powerpc/kexec/file_load_64.c
index 09187aca3d1f..961a6dd67365 100644
--- a/arch/powerpc/kexec/file_load_64.c
+++ b/arch/powerpc/kexec/file_load_64.c
@@ -32,7 +32,7 @@
 #include <asm/plpks.h>
 
 struct umem_info {
-	u64 *buf;		/* data buffer for usable-memory property */
+	__be64 *buf;		/* data buffer for usable-memory property */
 	u32 size;		/* size allocated for the data buffer */
 	u32 max_entries;	/* maximum no. of entries */
 	u32 idx;		/* index of current entry */
@@ -443,10 +443,10 @@ static int locate_mem_hole_bottom_up_ppc64(struct kexec_buf *kbuf,
  *
  * Returns buffer on success, NULL on error.
  */
-static u64 *check_realloc_usable_mem(struct umem_info *um_info, int cnt)
+static __be64 *check_realloc_usable_mem(struct umem_info *um_info, int cnt)
 {
 	u32 new_size;
-	u64 *tbuf;
+	__be64 *tbuf;
 
 	if ((um_info->idx + cnt) <= um_info->max_entries)
 		return um_info->buf;
diff --git a/arch/powerpc/mm/drmem.c b/arch/powerpc/mm/drmem.c
index 2369d1bf2411..fde7790277f7 100644
--- a/arch/powerpc/mm/drmem.c
+++ b/arch/powerpc/mm/drmem.c
@@ -67,7 +67,7 @@ static int drmem_update_dt_v1(struct device_node *memory,
 	struct property *new_prop;
 	struct of_drconf_cell_v1 *dr_cell;
 	struct drmem_lmb *lmb;
-	u32 *p;
+	__be32 *p;
 
 	new_prop = clone_property(prop, prop->length);
 	if (!new_prop)
diff --git a/arch/powerpc/perf/hv-24x7.c b/arch/powerpc/perf/hv-24x7.c
index 3449be7c0d51..057ec2e3451d 100644
--- a/arch/powerpc/perf/hv-24x7.c
+++ b/arch/powerpc/perf/hv-24x7.c
@@ -1338,7 +1338,7 @@ static int get_count_from_result(struct perf_event *event,
 	for (i = count = 0, element_data = res->elements + data_offset;
 	     i < num_elements;
 	     i++, element_data += data_size + data_offset)
-		count += be64_to_cpu(*((u64 *) element_data));
+		count += be64_to_cpu(*((__be64 *)element_data));
 
 	*countp = count;
 
diff --git a/arch/powerpc/perf/imc-pmu.c b/arch/powerpc/perf/imc-pmu.c
index eba2baabccb0..2016aee27037 100644
--- a/arch/powerpc/perf/imc-pmu.c
+++ b/arch/powerpc/perf/imc-pmu.c
@@ -1025,16 +1025,16 @@ static bool is_thread_imc_pmu(struct perf_event *event)
 	return false;
 }
 
-static u64 * get_event_base_addr(struct perf_event *event)
+static __be64 *get_event_base_addr(struct perf_event *event)
 {
 	u64 addr;
 
 	if (is_thread_imc_pmu(event)) {
 		addr = (u64)per_cpu(thread_imc_mem, smp_processor_id());
-		return (u64 *)(addr + (event->attr.config & IMC_EVENT_OFFSET_MASK));
+		return (__be64 *)(addr + (event->attr.config & IMC_EVENT_OFFSET_MASK));
 	}
 
-	return (u64 *)event->hw.event_base;
+	return (__be64 *)event->hw.event_base;
 }
 
 static void thread_imc_pmu_start_txn(struct pmu *pmu,
@@ -1058,7 +1058,8 @@ static int thread_imc_pmu_commit_txn(struct pmu *pmu)
 
 static u64 imc_read_counter(struct perf_event *event)
 {
-	u64 *addr, data;
+	__be64 *addr;
+	u64 data;
 
 	/*
 	 * In-Memory Collection (IMC) counters are free flowing counters.
diff --git a/arch/powerpc/platforms/powermac/feature.c b/arch/powerpc/platforms/powermac/feature.c
index ae62d432db8b..81c9fbae88b1 100644
--- a/arch/powerpc/platforms/powermac/feature.c
+++ b/arch/powerpc/platforms/powermac/feature.c
@@ -2614,7 +2614,8 @@ static void __init probe_one_macio(const char *name, const char *compat, int typ
 	struct device_node*	node;
 	int			i;
 	volatile u32 __iomem	*base;
-	const u32		*addrp, *revp;
+	const __be32		*addrp;
+	const u32		*revp;
 	phys_addr_t		addr;
 	u64			size;
 
diff --git a/arch/powerpc/platforms/pseries/hotplug-memory.c b/arch/powerpc/platforms/pseries/hotplug-memory.c
index aa4042dcd6d4..a43bfb01720a 100644
--- a/arch/powerpc/platforms/pseries/hotplug-memory.c
+++ b/arch/powerpc/platforms/pseries/hotplug-memory.c
@@ -55,7 +55,8 @@ static bool find_aa_index(struct device_node *dr_node,
 			 struct property *ala_prop,
 			 const u32 *lmb_assoc, u32 *aa_index)
 {
-	u32 *assoc_arrays, new_prop_size;
+	__be32 *assoc_arrays;
+	u32 new_prop_size;
 	struct property *new_prop;
 	int aa_arrays, aa_array_entries, aa_array_sz;
 	int i, index;
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 07/12] powerpc/kvm: Force cast endianness of KVM shared regs
  2023-10-11  5:36 [PATCH 00/12] Miscellaneous Sparse fixes Benjamin Gray
                   ` (5 preceding siblings ...)
  2023-10-11  5:37 ` [PATCH 06/12] powerpc: Annotate endianness of various variables and functions Benjamin Gray
@ 2023-10-11  5:37 ` Benjamin Gray
  2023-10-11  5:37 ` [PATCH 08/12] powerpc/opal: Annotate out param endianness Benjamin Gray
                   ` (5 subsequent siblings)
  12 siblings, 0 replies; 16+ messages in thread
From: Benjamin Gray @ 2023-10-11  5:37 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Benjamin Gray

Sparse reports endianness mismatches in the KVM shared regs getter and
setter helpers.

This code has dynamic endianness behind a safe interface, so a force is
warranted here to tell sparse this is OK.

Signed-off-by: Benjamin Gray <bgray@linux.ibm.com>
---
 arch/powerpc/include/asm/kvm_ppc.h | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/powerpc/include/asm/kvm_ppc.h b/arch/powerpc/include/asm/kvm_ppc.h
index b4da8514af43..b547bcf1a995 100644
--- a/arch/powerpc/include/asm/kvm_ppc.h
+++ b/arch/powerpc/include/asm/kvm_ppc.h
@@ -943,18 +943,18 @@ static inline void kvmppc_set_##reg(struct kvm_vcpu *vcpu, ulong val)	\
 static inline u##size kvmppc_get_##reg(struct kvm_vcpu *vcpu)		\
 {									\
 	if (kvmppc_shared_big_endian(vcpu))				\
-	       return be##size##_to_cpu(vcpu->arch.shared->reg);	\
+		return be##size##_to_cpu((__be##size __force)vcpu->arch.shared->reg);	\
 	else								\
-	       return le##size##_to_cpu(vcpu->arch.shared->reg);	\
+		return le##size##_to_cpu((__le##size __force)vcpu->arch.shared->reg);	\
 }									\
 
 #define SHARED_WRAPPER_SET(reg, size)					\
 static inline void kvmppc_set_##reg(struct kvm_vcpu *vcpu, u##size val)	\
 {									\
 	if (kvmppc_shared_big_endian(vcpu))				\
-	       vcpu->arch.shared->reg = cpu_to_be##size(val);		\
+		vcpu->arch.shared->reg = (u##size __force)cpu_to_be##size(val);	\
 	else								\
-	       vcpu->arch.shared->reg = cpu_to_le##size(val);		\
+		vcpu->arch.shared->reg = (u##size __force)cpu_to_le##size(val);	\
 }									\
 
 #define SHARED_WRAPPER(reg, size)					\
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 08/12] powerpc/opal: Annotate out param endianness
  2023-10-11  5:36 [PATCH 00/12] Miscellaneous Sparse fixes Benjamin Gray
                   ` (6 preceding siblings ...)
  2023-10-11  5:37 ` [PATCH 07/12] powerpc/kvm: Force cast endianness of KVM shared regs Benjamin Gray
@ 2023-10-11  5:37 ` Benjamin Gray
  2023-10-11  5:37 ` [PATCH 09/12] powerpc/uaccess: Cast away __user annotation after verification Benjamin Gray
                   ` (4 subsequent siblings)
  12 siblings, 0 replies; 16+ messages in thread
From: Benjamin Gray @ 2023-10-11  5:37 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Benjamin Gray

Sparse reports an endian mismatch with args to opal_int_get_xirr().
Checking the skiboot source[1] shows the function takes a __be32*
(as expected), so update the function declaration to reflect this.

[1]: https://github.com/open-power/skiboot/blob/80e2b1dc73/hw/xive.c#L3479

Signed-off-by: Benjamin Gray <bgray@linux.ibm.com>
---
 arch/powerpc/include/asm/opal.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/include/asm/opal.h b/arch/powerpc/include/asm/opal.h
index a9b31cc258fc..b66b0c615f4f 100644
--- a/arch/powerpc/include/asm/opal.h
+++ b/arch/powerpc/include/asm/opal.h
@@ -227,7 +227,7 @@ int64_t opal_pci_set_power_state(uint64_t async_token, uint64_t id,
 				 uint64_t data);
 int64_t opal_pci_poll2(uint64_t id, uint64_t data);
 
-int64_t opal_int_get_xirr(uint32_t *out_xirr, bool just_poll);
+int64_t opal_int_get_xirr(__be32 *out_xirr, bool just_poll);
 int64_t opal_int_set_cppr(uint8_t cppr);
 int64_t opal_int_eoi(uint32_t xirr);
 int64_t opal_int_set_mfrr(uint32_t cpu, uint8_t mfrr);
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 09/12] powerpc/uaccess: Cast away __user annotation after verification
  2023-10-11  5:36 [PATCH 00/12] Miscellaneous Sparse fixes Benjamin Gray
                   ` (7 preceding siblings ...)
  2023-10-11  5:37 ` [PATCH 08/12] powerpc/opal: Annotate out param endianness Benjamin Gray
@ 2023-10-11  5:37 ` Benjamin Gray
  2023-10-11  5:37 ` [PATCH 10/12] powerpc: Cast away __iomem in low level IO routines Benjamin Gray
                   ` (3 subsequent siblings)
  12 siblings, 0 replies; 16+ messages in thread
From: Benjamin Gray @ 2023-10-11  5:37 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Benjamin Gray

Sparse reports dereference of a __user pointer. copy_mc_to_user() takes
a __user pointer, verifies it, then calls the generic copy routine
copy_mc_generic().

As we have verified the pointer, cast out the __user annotation when
passing to copy_mc_generic().

Signed-off-by: Benjamin Gray <bgray@linux.ibm.com>
---
 arch/powerpc/include/asm/uaccess.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/include/asm/uaccess.h b/arch/powerpc/include/asm/uaccess.h
index fb725ec77926..f1f9890f50d3 100644
--- a/arch/powerpc/include/asm/uaccess.h
+++ b/arch/powerpc/include/asm/uaccess.h
@@ -374,7 +374,7 @@ copy_mc_to_user(void __user *to, const void *from, unsigned long n)
 	if (check_copy_size(from, n, true)) {
 		if (access_ok(to, n)) {
 			allow_write_to_user(to, n);
-			n = copy_mc_generic((void *)to, from, n);
+			n = copy_mc_generic((void __force *)to, from, n);
 			prevent_write_to_user(to, n);
 		}
 	}
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 10/12] powerpc: Cast away __iomem in low level IO routines
  2023-10-11  5:36 [PATCH 00/12] Miscellaneous Sparse fixes Benjamin Gray
                   ` (8 preceding siblings ...)
  2023-10-11  5:37 ` [PATCH 09/12] powerpc/uaccess: Cast away __user annotation after verification Benjamin Gray
@ 2023-10-11  5:37 ` Benjamin Gray
  2023-10-11  5:37 ` [PATCH 11/12] powerpc/eeh: Remove unnecessary cast Benjamin Gray
                   ` (2 subsequent siblings)
  12 siblings, 0 replies; 16+ messages in thread
From: Benjamin Gray @ 2023-10-11  5:37 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Benjamin Gray

Sparse reports dereferencing an __iomem pointer. These routines
are clearly low level handlers for IO memory, so force cast away
the __iomem annotation to tell sparse the dereferences are safe.

Signed-off-by: Benjamin Gray <bgray@linux.ibm.com>
---
 arch/powerpc/kernel/io.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/arch/powerpc/kernel/io.c b/arch/powerpc/kernel/io.c
index 2f29b7d432de..6af535905984 100644
--- a/arch/powerpc/kernel/io.c
+++ b/arch/powerpc/kernel/io.c
@@ -33,7 +33,7 @@ void _insb(const volatile u8 __iomem *port, void *buf, long count)
 		return;
 	asm volatile("sync");
 	do {
-		tmp = *port;
+		tmp = *(const volatile u8 __force *)port;
 		eieio();
 		*tbuf++ = tmp;
 	} while (--count != 0);
@@ -49,7 +49,7 @@ void _outsb(volatile u8 __iomem *port, const void *buf, long count)
 		return;
 	asm volatile("sync");
 	do {
-		*port = *tbuf++;
+		*(volatile u8 __force *)port = *tbuf++;
 	} while (--count != 0);
 	asm volatile("sync");
 }
@@ -64,7 +64,7 @@ void _insw_ns(const volatile u16 __iomem *port, void *buf, long count)
 		return;
 	asm volatile("sync");
 	do {
-		tmp = *port;
+		tmp = *(const volatile u16 __force *)port;
 		eieio();
 		*tbuf++ = tmp;
 	} while (--count != 0);
@@ -80,7 +80,7 @@ void _outsw_ns(volatile u16 __iomem *port, const void *buf, long count)
 		return;
 	asm volatile("sync");
 	do {
-		*port = *tbuf++;
+		*(volatile u16 __force *)port = *tbuf++;
 	} while (--count != 0);
 	asm volatile("sync");
 }
@@ -95,7 +95,7 @@ void _insl_ns(const volatile u32 __iomem *port, void *buf, long count)
 		return;
 	asm volatile("sync");
 	do {
-		tmp = *port;
+		tmp = *(const volatile u32 __force *)port;
 		eieio();
 		*tbuf++ = tmp;
 	} while (--count != 0);
@@ -111,7 +111,7 @@ void _outsl_ns(volatile u32 __iomem *port, const void *buf, long count)
 		return;
 	asm volatile("sync");
 	do {
-		*port = *tbuf++;
+		*(volatile u32 __force *)port = *tbuf++;
 	} while (--count != 0);
 	asm volatile("sync");
 }
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 11/12] powerpc/eeh: Remove unnecessary cast
  2023-10-11  5:36 [PATCH 00/12] Miscellaneous Sparse fixes Benjamin Gray
                   ` (9 preceding siblings ...)
  2023-10-11  5:37 ` [PATCH 10/12] powerpc: Cast away __iomem in low level IO routines Benjamin Gray
@ 2023-10-11  5:37 ` Benjamin Gray
  2023-10-11  5:37 ` [PATCH 12/12] powerpc/fadump: Annotate endianness cast with __force Benjamin Gray
  2023-10-27  9:59 ` (subset) [PATCH 00/12] Miscellaneous Sparse fixes Michael Ellerman
  12 siblings, 0 replies; 16+ messages in thread
From: Benjamin Gray @ 2023-10-11  5:37 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Benjamin Gray

Sparse reports a warning when casting to an int. There is no need to
cast in the first place, so drop them.

Signed-off-by: Benjamin Gray <bgray@linux.ibm.com>
---
 arch/powerpc/kernel/eeh_driver.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/kernel/eeh_driver.c b/arch/powerpc/kernel/eeh_driver.c
index 438568a472d0..48773d2d9be3 100644
--- a/arch/powerpc/kernel/eeh_driver.c
+++ b/arch/powerpc/kernel/eeh_driver.c
@@ -39,7 +39,7 @@ static int eeh_result_priority(enum pci_ers_result result)
 	case PCI_ERS_RESULT_NEED_RESET:
 		return 6;
 	default:
-		WARN_ONCE(1, "Unknown pci_ers_result value: %d\n", (int)result);
+		WARN_ONCE(1, "Unknown pci_ers_result value: %d\n", result);
 		return 0;
 	}
 };
@@ -60,7 +60,7 @@ static const char *pci_ers_result_name(enum pci_ers_result result)
 	case PCI_ERS_RESULT_NO_AER_DRIVER:
 		return "no AER driver";
 	default:
-		WARN_ONCE(1, "Unknown result type: %d\n", (int)result);
+		WARN_ONCE(1, "Unknown result type: %d\n", result);
 		return "unknown";
 	}
 };
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 12/12] powerpc/fadump: Annotate endianness cast with __force
  2023-10-11  5:36 [PATCH 00/12] Miscellaneous Sparse fixes Benjamin Gray
                   ` (10 preceding siblings ...)
  2023-10-11  5:37 ` [PATCH 11/12] powerpc/eeh: Remove unnecessary cast Benjamin Gray
@ 2023-10-11  5:37 ` Benjamin Gray
  2023-10-27  9:59 ` (subset) [PATCH 00/12] Miscellaneous Sparse fixes Michael Ellerman
  12 siblings, 0 replies; 16+ messages in thread
From: Benjamin Gray @ 2023-10-11  5:37 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Benjamin Gray

Sparse reports an endianness error with the else case of

  val = (cpu_endian ? be64_to_cpu(reg_entry->reg_val) :
         (u64)(reg_entry->reg_val));

This is a safe operation because the code is explicitly working with
dynamic endianness, so add the __force annotation to tell Sparse to
ignore it.

Signed-off-by: Benjamin Gray <bgray@linux.ibm.com>
---
 arch/powerpc/platforms/powernv/opal-fadump.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/platforms/powernv/opal-fadump.h b/arch/powerpc/platforms/powernv/opal-fadump.h
index 3f715efb0aa6..5eeb794b5eb1 100644
--- a/arch/powerpc/platforms/powernv/opal-fadump.h
+++ b/arch/powerpc/platforms/powernv/opal-fadump.h
@@ -135,7 +135,7 @@ static inline void opal_fadump_read_regs(char *bufp, unsigned int regs_cnt,
 	for (i = 0; i < regs_cnt; i++, bufp += reg_entry_size) {
 		reg_entry = (struct hdat_fadump_reg_entry *)bufp;
 		val = (cpu_endian ? be64_to_cpu(reg_entry->reg_val) :
-		       (u64)(reg_entry->reg_val));
+		       (u64 __force)(reg_entry->reg_val));
 		opal_fadump_set_regval_regnum(regs,
 					      be32_to_cpu(reg_entry->reg_type),
 					      be32_to_cpu(reg_entry->reg_num),
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: (subset) [PATCH 00/12] Miscellaneous Sparse fixes
  2023-10-11  5:36 [PATCH 00/12] Miscellaneous Sparse fixes Benjamin Gray
                   ` (11 preceding siblings ...)
  2023-10-11  5:37 ` [PATCH 12/12] powerpc/fadump: Annotate endianness cast with __force Benjamin Gray
@ 2023-10-27  9:59 ` Michael Ellerman
  12 siblings, 0 replies; 16+ messages in thread
From: Michael Ellerman @ 2023-10-27  9:59 UTC (permalink / raw)
  To: linuxppc-dev, Benjamin Gray

On Wed, 11 Oct 2023 16:36:59 +1100, Benjamin Gray wrote:
> There are many Sparse warnings in the kernel, including the powerpc
> directory. This series provides fixes for some low-hanging fruit found
> when trying to triage the warnings earlier this year. It addresses about
> 100 warnings (many have the same root cause).
> 
> I know there's concerns about making it harder to backport things. In
> general, as someone who was not around during the development of these
> features, I think that it is useful make the annotations as correct as
> possible. But it's no fuss if some/all of the patches are nacked for this
> reason. I just figured some of it might be useful instead of continuing to
> sit on it indefinitely.
> 
> [...]

Patch 1, and 3-12 applied to powerpc/next.

[01/12] powerpc/xive: Fix endian conversion size
        https://git.kernel.org/powerpc/c/ff7a60ab1e065257a0e467c13b519f4debcd7fcf
[03/12] powerpc: Explicitly reverse bytes when checking for byte reversal
        https://git.kernel.org/powerpc/c/340a60e3725b9a229eaf03a9b3f8538f22f6ac16
[04/12] powerpc: Use NULL instead of 0 for null pointers
        https://git.kernel.org/powerpc/c/ddfb7d9db843fd4fbdff81fb8a8743f865c3dd96
[05/12] powerpc: Remove extern from function implementations
        https://git.kernel.org/powerpc/c/419d5d112c2e1e78beda9c3299f71c35141d8dba
[06/12] powerpc: Annotate endianness of various variables and functions
        https://git.kernel.org/powerpc/c/2b4a6cc9a1a7cf6958c8b11f94e61c8e81b60b88
[07/12] powerpc/kvm: Force cast endianness of KVM shared regs
        https://git.kernel.org/powerpc/c/b7bce570430e42229fb63f775fcbb10f38b83c71
[08/12] powerpc/opal: Annotate out param endianness
        https://git.kernel.org/powerpc/c/8577dd00a6ba335bc359313599d6100522a1931c
[09/12] powerpc/uaccess: Cast away __user annotation after verification
        https://git.kernel.org/powerpc/c/c6519c6df0722e432f330afbc7c00d16d5be5c58
[10/12] powerpc: Cast away __iomem in low level IO routines
        https://git.kernel.org/powerpc/c/2c4ce3e65b1a543123ffcec4b021ad6ebd4e4e4e
[11/12] powerpc/eeh: Remove unnecessary cast
        https://git.kernel.org/powerpc/c/82f635243f209a85d3deb9f64439c3ea84cd4ecb
[12/12] powerpc/fadump: Annotate endianness cast with __force
        https://git.kernel.org/powerpc/c/b574b817cc7bfcc87bbc92b4b19525442401ae5e

cheers

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 02/12] powerpc/pseries: Restructure hvc_get_chars() endianness
  2023-10-11  5:37 ` [PATCH 02/12] powerpc/pseries: Restructure hvc_get_chars() endianness Benjamin Gray
@ 2023-10-30 13:16   ` Aneesh Kumar K.V
  2024-02-20  7:10     ` Christophe Leroy
  0 siblings, 1 reply; 16+ messages in thread
From: Aneesh Kumar K.V @ 2023-10-30 13:16 UTC (permalink / raw)
  To: Benjamin Gray, linuxppc-dev; +Cc: Benjamin Gray

Benjamin Gray <bgray@linux.ibm.com> writes:

> Sparse reports an endian mismatch in hvc_get_chars().
>
> At first it seemed like the retbuf should be __be64[], but actually
> retbuf holds serialized registers returned by the hypervisor call, so
> it's correctly CPU endian typed.
>
> Instead, it is the be64_to_cpu() that's misleading. The intent is to do
> a byte swap on a little endian CPU. The swap is required because we
> wanted to store the register values to memory without 'swapping' bytes,
> so that the high order byte of the first register is the first byte
> in the result buffer.
>
> In diagram form, on a LE CPU with the letters representing the return
> string bytes:
>
>     (register bytes) A B C D E F G H   I J K L M N O P
>   (retbuf mem bytes) H G F E D C B A   P O N M L K J I
> (buf/lbuf mem bytes) A B C D E F G H   I J K L M N O P
>
> So retbuf stores the registers in CPU endian, and buf always stores in
> big endian.
>
> So replace the byte swap function with cpu_to_be64() and cast lbuf as an
> array of __be64 to match the semantics closer.
>
> Signed-off-by: Benjamin Gray <bgray@linux.ibm.com>
> ---
>  arch/powerpc/platforms/pseries/hvconsole.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/arch/powerpc/platforms/pseries/hvconsole.c b/arch/powerpc/platforms/pseries/hvconsole.c
> index 1ac52963e08b..647718a15e78 100644
> --- a/arch/powerpc/platforms/pseries/hvconsole.c
> +++ b/arch/powerpc/platforms/pseries/hvconsole.c
> @@ -29,11 +29,11 @@ int hvc_get_chars(uint32_t vtermno, char *buf, int count)
>  {
>  	long ret;
>  	unsigned long retbuf[PLPAR_HCALL_BUFSIZE];
> -	unsigned long *lbuf = (unsigned long *)buf;
> +	__be64 *lbuf = (__be64 __force *)buf;
>  
>  	ret = plpar_hcall(H_GET_TERM_CHAR, retbuf, vtermno);
> -	lbuf[0] = be64_to_cpu(retbuf[1]);
> -	lbuf[1] = be64_to_cpu(retbuf[2]);
> +	lbuf[0] = cpu_to_be64(retbuf[1]);
> +	lbuf[1] = cpu_to_be64(retbuf[2]);
>  
>  	if (ret == H_SUCCESS)
>  		return retbuf[0];
>

There is no functionality change in this patch. It is clarifying the
details that it expect the buf to have the big-endian format and retbuf
contains native endian format.

Not sure why this was not picked.

Reviewed-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 02/12] powerpc/pseries: Restructure hvc_get_chars() endianness
  2023-10-30 13:16   ` Aneesh Kumar K.V
@ 2024-02-20  7:10     ` Christophe Leroy
  0 siblings, 0 replies; 16+ messages in thread
From: Christophe Leroy @ 2024-02-20  7:10 UTC (permalink / raw)
  To: Michael Ellerman; +Cc: Aneesh Kumar K.V, linuxppc-dev, Benjamin Gray

Michael ? Any reason for not picking that one ?

Le 30/10/2023 à 14:16, Aneesh Kumar K.V a écrit :
> Benjamin Gray <bgray@linux.ibm.com> writes:
> 
>> Sparse reports an endian mismatch in hvc_get_chars().
>>
>> At first it seemed like the retbuf should be __be64[], but actually
>> retbuf holds serialized registers returned by the hypervisor call, so
>> it's correctly CPU endian typed.
>>
>> Instead, it is the be64_to_cpu() that's misleading. The intent is to do
>> a byte swap on a little endian CPU. The swap is required because we
>> wanted to store the register values to memory without 'swapping' bytes,
>> so that the high order byte of the first register is the first byte
>> in the result buffer.
>>
>> In diagram form, on a LE CPU with the letters representing the return
>> string bytes:
>>
>>      (register bytes) A B C D E F G H   I J K L M N O P
>>    (retbuf mem bytes) H G F E D C B A   P O N M L K J I
>> (buf/lbuf mem bytes) A B C D E F G H   I J K L M N O P
>>
>> So retbuf stores the registers in CPU endian, and buf always stores in
>> big endian.
>>
>> So replace the byte swap function with cpu_to_be64() and cast lbuf as an
>> array of __be64 to match the semantics closer.
>>
>> Signed-off-by: Benjamin Gray <bgray@linux.ibm.com>
>> ---
>>   arch/powerpc/platforms/pseries/hvconsole.c | 6 +++---
>>   1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/powerpc/platforms/pseries/hvconsole.c b/arch/powerpc/platforms/pseries/hvconsole.c
>> index 1ac52963e08b..647718a15e78 100644
>> --- a/arch/powerpc/platforms/pseries/hvconsole.c
>> +++ b/arch/powerpc/platforms/pseries/hvconsole.c
>> @@ -29,11 +29,11 @@ int hvc_get_chars(uint32_t vtermno, char *buf, int count)
>>   {
>>   	long ret;
>>   	unsigned long retbuf[PLPAR_HCALL_BUFSIZE];
>> -	unsigned long *lbuf = (unsigned long *)buf;
>> +	__be64 *lbuf = (__be64 __force *)buf;
>>   
>>   	ret = plpar_hcall(H_GET_TERM_CHAR, retbuf, vtermno);
>> -	lbuf[0] = be64_to_cpu(retbuf[1]);
>> -	lbuf[1] = be64_to_cpu(retbuf[2]);
>> +	lbuf[0] = cpu_to_be64(retbuf[1]);
>> +	lbuf[1] = cpu_to_be64(retbuf[2]);
>>   
>>   	if (ret == H_SUCCESS)
>>   		return retbuf[0];
>>
> 
> There is no functionality change in this patch. It is clarifying the
> details that it expect the buf to have the big-endian format and retbuf
> contains native endian format.
> 
> Not sure why this was not picked.
> 
> Reviewed-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2024-02-20  7:12 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-10-11  5:36 [PATCH 00/12] Miscellaneous Sparse fixes Benjamin Gray
2023-10-11  5:37 ` [PATCH 01/12] powerpc/xive: Fix endian conversion size Benjamin Gray
2023-10-11  5:37 ` [PATCH 02/12] powerpc/pseries: Restructure hvc_get_chars() endianness Benjamin Gray
2023-10-30 13:16   ` Aneesh Kumar K.V
2024-02-20  7:10     ` Christophe Leroy
2023-10-11  5:37 ` [PATCH 03/12] powerpc: Explicitly reverse bytes when checking for byte reversal Benjamin Gray
2023-10-11  5:37 ` [PATCH 04/12] powerpc: Use NULL instead of 0 for null pointers Benjamin Gray
2023-10-11  5:37 ` [PATCH 05/12] powerpc: Remove extern from function implementations Benjamin Gray
2023-10-11  5:37 ` [PATCH 06/12] powerpc: Annotate endianness of various variables and functions Benjamin Gray
2023-10-11  5:37 ` [PATCH 07/12] powerpc/kvm: Force cast endianness of KVM shared regs Benjamin Gray
2023-10-11  5:37 ` [PATCH 08/12] powerpc/opal: Annotate out param endianness Benjamin Gray
2023-10-11  5:37 ` [PATCH 09/12] powerpc/uaccess: Cast away __user annotation after verification Benjamin Gray
2023-10-11  5:37 ` [PATCH 10/12] powerpc: Cast away __iomem in low level IO routines Benjamin Gray
2023-10-11  5:37 ` [PATCH 11/12] powerpc/eeh: Remove unnecessary cast Benjamin Gray
2023-10-11  5:37 ` [PATCH 12/12] powerpc/fadump: Annotate endianness cast with __force Benjamin Gray
2023-10-27  9:59 ` (subset) [PATCH 00/12] Miscellaneous Sparse fixes Michael Ellerman

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).