| Message ID | 20220117132437.60286-1-abhishek@linux.ibm.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Use volatile pointer for constant address asignment | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/github-Docker_builds_and_checks | success | Successfully ran 9 jobs. |
Excerpts from Abhishek Singh Tomar's message of January 17, 2022 11:24 pm: > use volatile pointer to avoid optimization introduced with gcc-11 on constant address asignment to pointer. > Resolves : the warray ofset warning during compilation. Hm. What exactly is the warning, and what is the optimization that's causing breakage? Thanks, Nick > > Signed-off-by: Abhishek Singh Tomar <abhishek@linux.ibm.com> > --- > core/console.c | 2 +- > core/fast-reboot.c | 6 ++++-- > core/init.c | 6 +++++- > core/opal-dump.c | 7 ++++--- > hdata/spira.c | 2 +- > hdata/spira.h | 2 +- > hw/fsp/fsp.c | 2 +- > 7 files changed, 17 insertions(+), 10 deletions(-) > > diff --git a/core/console.c b/core/console.c > index 2a150902..229bc84a 100644 > --- a/core/console.c > +++ b/core/console.c > @@ -15,7 +15,7 @@ > #include <processor.h> > #include <cpu.h> > > -static char *con_buf = (char *)INMEM_CON_START; > +static char *volatile con_buf = (char *)INMEM_CON_START; > static size_t con_in; > static size_t con_out; > static bool con_wrapped; > diff --git a/core/fast-reboot.c b/core/fast-reboot.c > index fedfa88c..3e3db68e 100644 > --- a/core/fast-reboot.c > +++ b/core/fast-reboot.c > @@ -307,6 +307,8 @@ void __noreturn fast_reboot_entry(void); > void __noreturn fast_reboot_entry(void) > { > struct cpu_thread *cpu = this_cpu(); > + void *volatile kerneal_load_base_addr = KERNEL_LOAD_BASE; > + void *volatile initramfs_load_base_addr = INITRAMFS_LOAD_BASE; > > if (proc_gen == proc_gen_p8) { > /* We reset our ICP first ! Otherwise we might get stray > @@ -425,8 +427,8 @@ void __noreturn fast_reboot_entry(void) > * Mambo may have embedded payload here, so don't clear > * it at all. > */ > - memset(KERNEL_LOAD_BASE, 0, KERNEL_LOAD_SIZE); > - memset(INITRAMFS_LOAD_BASE, 0, INITRAMFS_LOAD_SIZE); > + memset(kerneal_load_base_addr, 0, KERNEL_LOAD_SIZE); > + memset(initramfs_load_base_addr, 0, INITRAMFS_LOAD_SIZE); > } > > /* Start preloading kernel and ramdisk */ > diff --git a/core/init.c b/core/init.c > index b4d33518..00f667e1 100644 > --- a/core/init.c > +++ b/core/init.c > @@ -869,7 +869,11 @@ void copy_exception_vectors(void) > * this is the boot flag used by CPUs still potentially entering > * skiboot. > */ > - memcpy((void *)0x100, (void *)(SKIBOOT_BASE + 0x100), > + void *volatile exception_vectors_start_addr = (void*)(SKIBOOT_BASE + 0x100); > + void *volatile dst = (void*)0x100; > + > + > + memcpy(dst,exception_vectors_start_addr, > EXCEPTION_VECTORS_END - 0x100); > sync_icache(); > } > diff --git a/core/opal-dump.c b/core/opal-dump.c > index 4f54a3ef..4943fedd 100644 > --- a/core/opal-dump.c > +++ b/core/opal-dump.c > @@ -301,6 +301,7 @@ static int64_t opal_mpipl_update(enum opal_mpipl_ops ops, > u64 src, u64 dest, u64 size) > { > int rc; > + void *volatile mdrt_table_base_addr =(void*) MDRT_TABLE_BASE; > > switch (ops) { > case OPAL_MPIPL_ADD_RANGE: > @@ -330,7 +331,7 @@ static int64_t opal_mpipl_update(enum opal_mpipl_ops ops, > free(opal_mpipl_cpu_data); > opal_mpipl_cpu_data = NULL; > /* Clear MDRT table */ > - memset((void *)MDRT_TABLE_BASE, 0, MDRT_TABLE_SIZE); > + memset(mdrt_table_base_addr, 0, MDRT_TABLE_SIZE); > /* Set MDRT count to max allocated count */ > ntuple_mdrt->act_cnt = cpu_to_be16(MDRT_TABLE_SIZE / sizeof(struct mdrt_table)); > rc = OPAL_SUCCESS; > @@ -529,8 +530,8 @@ bool is_mpipl_enabled(void) > > void opal_mpipl_init(void) > { > - void *mdst_base = (void *)MDST_TABLE_BASE; > - void *mddt_base = (void *)MDDT_TABLE_BASE; > + void *volatile mdst_base = (void *)MDST_TABLE_BASE; > + void *volatile mddt_base = (void *)MDDT_TABLE_BASE; > struct dt_node *dump_node; > > dump_node = dt_find_by_path(opal_node, "dump"); > diff --git a/hdata/spira.c b/hdata/spira.c > index 1a351462..3bbec351 100644 > --- a/hdata/spira.c > +++ b/hdata/spira.c > @@ -236,7 +236,7 @@ __section(".spirah.data") struct spirah spirah = { > }; > > /* The service processor SPIRA-S structure */ > -struct spiras *spiras; > +struct spiras *volatile spiras; > > /* Overridden for testing. */ > #ifndef spira_check_ptr > diff --git a/hdata/spira.h b/hdata/spira.h > index 8def23bd..6df3df56 100644 > --- a/hdata/spira.h > +++ b/hdata/spira.h > @@ -150,7 +150,7 @@ struct spiras { > u8 reserved[0x180]; > } __packed __align(0x100); > > -extern struct spiras *spiras; > +extern struct spiras *volatile spiras; > > > /* This macro can be used to check the validity of a pointer returned > diff --git a/hw/fsp/fsp.c b/hw/fsp/fsp.c > index 2c5f9d71..259e83a3 100644 > --- a/hw/fsp/fsp.c > +++ b/hw/fsp/fsp.c > @@ -91,7 +91,7 @@ static enum ipl_state ipl_state = ipl_initial; > static struct fsp *first_fsp; > static struct fsp *active_fsp; > static u16 fsp_curseq = 0x8000; > -static __be64 *fsp_tce_table; > +static __be64 *volatile fsp_tce_table; > > #define FSP_INBOUND_SIZE 0x00100000UL > static void *fsp_inbound_buf = NULL; > -- > 2.31.1 > > _______________________________________________ > Skiboot mailing list > Skiboot@lists.ozlabs.org > https://lists.ozlabs.org/listinfo/skiboot >
Hello Nick I have sent the V2 for this patch is available at https://lists.ozlabs.org/pipermail/skiboot/2022-January/018143.html. I had not mentioned V2 on that. kindly review that patch as V2. The warnings we encounter are as below: /build/libc/include/string.h:34:16: warning: '__builtin_memset' offset [0, 2097151] is out of the bounds [0, 0] [-Warray-bounds] 34 | #define memset __builtin_memset hw/fsp/fsp.c:1855:9: note: in expansion of macro 'memset' 1855 | memset(fsp_tce_table, 0, PSI_TCE_TABLE_SIZE); gcc optimization is that it is checking is memory allocated for that specific pointer. In our case, we are using a constant address for memset,memcpy etc. We are not allocating memory as in the application program to that specific pointers. so when gcc tries to map it with access it results in warning. Regards Abhishek On Wed, Jan 19, 2022 at 02:21:26PM +1000, Nicholas Piggin wrote: > Excerpts from Abhishek Singh Tomar's message of January 17, 2022 11:24 pm: > > use volatile pointer to avoid optimization introduced with gcc-11 on constant address asignment to pointer. > > Resolves : the warray ofset warning during compilation. > > Hm. What exactly is the warning, and what is the optimization that's > causing breakage? > > Thanks, > Nick > > > > > Signed-off-by: Abhishek Singh Tomar <abhishek@linux.ibm.com> > > --- > > core/console.c | 2 +- > > core/fast-reboot.c | 6 ++++-- > > core/init.c | 6 +++++- > > core/opal-dump.c | 7 ++++--- > > hdata/spira.c | 2 +- > > hdata/spira.h | 2 +- > > hw/fsp/fsp.c | 2 +- > > 7 files changed, 17 insertions(+), 10 deletions(-) > > > > diff --git a/core/console.c b/core/console.c > > index 2a150902..229bc84a 100644 > > --- a/core/console.c > > +++ b/core/console.c > > @@ -15,7 +15,7 @@ > > #include <processor.h> > > #include <cpu.h> > > > > -static char *con_buf = (char *)INMEM_CON_START; > > +static char *volatile con_buf = (char *)INMEM_CON_START; > > static size_t con_in; > > static size_t con_out; > > static bool con_wrapped; > > diff --git a/core/fast-reboot.c b/core/fast-reboot.c > > index fedfa88c..3e3db68e 100644 > > --- a/core/fast-reboot.c > > +++ b/core/fast-reboot.c > > @@ -307,6 +307,8 @@ void __noreturn fast_reboot_entry(void); > > void __noreturn fast_reboot_entry(void) > > { > > struct cpu_thread *cpu = this_cpu(); > > + void *volatile kerneal_load_base_addr = KERNEL_LOAD_BASE; > > + void *volatile initramfs_load_base_addr = INITRAMFS_LOAD_BASE; > > > > if (proc_gen == proc_gen_p8) { > > /* We reset our ICP first ! Otherwise we might get stray > > @@ -425,8 +427,8 @@ void __noreturn fast_reboot_entry(void) > > * Mambo may have embedded payload here, so don't clear > > * it at all. > > */ > > - memset(KERNEL_LOAD_BASE, 0, KERNEL_LOAD_SIZE); > > - memset(INITRAMFS_LOAD_BASE, 0, INITRAMFS_LOAD_SIZE); > > + memset(kerneal_load_base_addr, 0, KERNEL_LOAD_SIZE); > > + memset(initramfs_load_base_addr, 0, INITRAMFS_LOAD_SIZE); > > } > > > > /* Start preloading kernel and ramdisk */ > > diff --git a/core/init.c b/core/init.c > > index b4d33518..00f667e1 100644 > > --- a/core/init.c > > +++ b/core/init.c > > @@ -869,7 +869,11 @@ void copy_exception_vectors(void) > > * this is the boot flag used by CPUs still potentially entering > > * skiboot. > > */ > > - memcpy((void *)0x100, (void *)(SKIBOOT_BASE + 0x100), > > + void *volatile exception_vectors_start_addr = (void*)(SKIBOOT_BASE + 0x100); > > + void *volatile dst = (void*)0x100; > > + > > + > > + memcpy(dst,exception_vectors_start_addr, > > EXCEPTION_VECTORS_END - 0x100); > > sync_icache(); > > } > > diff --git a/core/opal-dump.c b/core/opal-dump.c > > index 4f54a3ef..4943fedd 100644 > > --- a/core/opal-dump.c > > +++ b/core/opal-dump.c > > @@ -301,6 +301,7 @@ static int64_t opal_mpipl_update(enum opal_mpipl_ops ops, > > u64 src, u64 dest, u64 size) > > { > > int rc; > > + void *volatile mdrt_table_base_addr =(void*) MDRT_TABLE_BASE; > > > > switch (ops) { > > case OPAL_MPIPL_ADD_RANGE: > > @@ -330,7 +331,7 @@ static int64_t opal_mpipl_update(enum opal_mpipl_ops ops, > > free(opal_mpipl_cpu_data); > > opal_mpipl_cpu_data = NULL; > > /* Clear MDRT table */ > > - memset((void *)MDRT_TABLE_BASE, 0, MDRT_TABLE_SIZE); > > + memset(mdrt_table_base_addr, 0, MDRT_TABLE_SIZE); > > /* Set MDRT count to max allocated count */ > > ntuple_mdrt->act_cnt = cpu_to_be16(MDRT_TABLE_SIZE / sizeof(struct mdrt_table)); > > rc = OPAL_SUCCESS; > > @@ -529,8 +530,8 @@ bool is_mpipl_enabled(void) > > > > void opal_mpipl_init(void) > > { > > - void *mdst_base = (void *)MDST_TABLE_BASE; > > - void *mddt_base = (void *)MDDT_TABLE_BASE; > > + void *volatile mdst_base = (void *)MDST_TABLE_BASE; > > + void *volatile mddt_base = (void *)MDDT_TABLE_BASE; > > struct dt_node *dump_node; > > > > dump_node = dt_find_by_path(opal_node, "dump"); > > diff --git a/hdata/spira.c b/hdata/spira.c > > index 1a351462..3bbec351 100644 > > --- a/hdata/spira.c > > +++ b/hdata/spira.c > > @@ -236,7 +236,7 @@ __section(".spirah.data") struct spirah spirah = { > > }; > > > > /* The service processor SPIRA-S structure */ > > -struct spiras *spiras; > > +struct spiras *volatile spiras; > > > > /* Overridden for testing. */ > > #ifndef spira_check_ptr > > diff --git a/hdata/spira.h b/hdata/spira.h > > index 8def23bd..6df3df56 100644 > > --- a/hdata/spira.h > > +++ b/hdata/spira.h > > @@ -150,7 +150,7 @@ struct spiras { > > u8 reserved[0x180]; > > } __packed __align(0x100); > > > > -extern struct spiras *spiras; > > +extern struct spiras *volatile spiras; > > > > > > /* This macro can be used to check the validity of a pointer returned > > diff --git a/hw/fsp/fsp.c b/hw/fsp/fsp.c > > index 2c5f9d71..259e83a3 100644 > > --- a/hw/fsp/fsp.c > > +++ b/hw/fsp/fsp.c > > @@ -91,7 +91,7 @@ static enum ipl_state ipl_state = ipl_initial; > > static struct fsp *first_fsp; > > static struct fsp *active_fsp; > > static u16 fsp_curseq = 0x8000; > > -static __be64 *fsp_tce_table; > > +static __be64 *volatile fsp_tce_table; > > > > #define FSP_INBOUND_SIZE 0x00100000UL > > static void *fsp_inbound_buf = NULL; > > -- > > 2.31.1 > > > > _______________________________________________ > > Skiboot mailing list > > Skiboot@lists.ozlabs.org > > https://lists.ozlabs.org/listinfo/skiboot > >
diff --git a/core/console.c b/core/console.c index 2a150902..229bc84a 100644 --- a/core/console.c +++ b/core/console.c @@ -15,7 +15,7 @@ #include <processor.h> #include <cpu.h> -static char *con_buf = (char *)INMEM_CON_START; +static char *volatile con_buf = (char *)INMEM_CON_START; static size_t con_in; static size_t con_out; static bool con_wrapped; diff --git a/core/fast-reboot.c b/core/fast-reboot.c index fedfa88c..3e3db68e 100644 --- a/core/fast-reboot.c +++ b/core/fast-reboot.c @@ -307,6 +307,8 @@ void __noreturn fast_reboot_entry(void); void __noreturn fast_reboot_entry(void) { struct cpu_thread *cpu = this_cpu(); + void *volatile kerneal_load_base_addr = KERNEL_LOAD_BASE; + void *volatile initramfs_load_base_addr = INITRAMFS_LOAD_BASE; if (proc_gen == proc_gen_p8) { /* We reset our ICP first ! Otherwise we might get stray @@ -425,8 +427,8 @@ void __noreturn fast_reboot_entry(void) * Mambo may have embedded payload here, so don't clear * it at all. */ - memset(KERNEL_LOAD_BASE, 0, KERNEL_LOAD_SIZE); - memset(INITRAMFS_LOAD_BASE, 0, INITRAMFS_LOAD_SIZE); + memset(kerneal_load_base_addr, 0, KERNEL_LOAD_SIZE); + memset(initramfs_load_base_addr, 0, INITRAMFS_LOAD_SIZE); } /* Start preloading kernel and ramdisk */ diff --git a/core/init.c b/core/init.c index b4d33518..00f667e1 100644 --- a/core/init.c +++ b/core/init.c @@ -869,7 +869,11 @@ void copy_exception_vectors(void) * this is the boot flag used by CPUs still potentially entering * skiboot. */ - memcpy((void *)0x100, (void *)(SKIBOOT_BASE + 0x100), + void *volatile exception_vectors_start_addr = (void*)(SKIBOOT_BASE + 0x100); + void *volatile dst = (void*)0x100; + + + memcpy(dst,exception_vectors_start_addr, EXCEPTION_VECTORS_END - 0x100); sync_icache(); } diff --git a/core/opal-dump.c b/core/opal-dump.c index 4f54a3ef..4943fedd 100644 --- a/core/opal-dump.c +++ b/core/opal-dump.c @@ -301,6 +301,7 @@ static int64_t opal_mpipl_update(enum opal_mpipl_ops ops, u64 src, u64 dest, u64 size) { int rc; + void *volatile mdrt_table_base_addr =(void*) MDRT_TABLE_BASE; switch (ops) { case OPAL_MPIPL_ADD_RANGE: @@ -330,7 +331,7 @@ static int64_t opal_mpipl_update(enum opal_mpipl_ops ops, free(opal_mpipl_cpu_data); opal_mpipl_cpu_data = NULL; /* Clear MDRT table */ - memset((void *)MDRT_TABLE_BASE, 0, MDRT_TABLE_SIZE); + memset(mdrt_table_base_addr, 0, MDRT_TABLE_SIZE); /* Set MDRT count to max allocated count */ ntuple_mdrt->act_cnt = cpu_to_be16(MDRT_TABLE_SIZE / sizeof(struct mdrt_table)); rc = OPAL_SUCCESS; @@ -529,8 +530,8 @@ bool is_mpipl_enabled(void) void opal_mpipl_init(void) { - void *mdst_base = (void *)MDST_TABLE_BASE; - void *mddt_base = (void *)MDDT_TABLE_BASE; + void *volatile mdst_base = (void *)MDST_TABLE_BASE; + void *volatile mddt_base = (void *)MDDT_TABLE_BASE; struct dt_node *dump_node; dump_node = dt_find_by_path(opal_node, "dump"); diff --git a/hdata/spira.c b/hdata/spira.c index 1a351462..3bbec351 100644 --- a/hdata/spira.c +++ b/hdata/spira.c @@ -236,7 +236,7 @@ __section(".spirah.data") struct spirah spirah = { }; /* The service processor SPIRA-S structure */ -struct spiras *spiras; +struct spiras *volatile spiras; /* Overridden for testing. */ #ifndef spira_check_ptr diff --git a/hdata/spira.h b/hdata/spira.h index 8def23bd..6df3df56 100644 --- a/hdata/spira.h +++ b/hdata/spira.h @@ -150,7 +150,7 @@ struct spiras { u8 reserved[0x180]; } __packed __align(0x100); -extern struct spiras *spiras; +extern struct spiras *volatile spiras; /* This macro can be used to check the validity of a pointer returned diff --git a/hw/fsp/fsp.c b/hw/fsp/fsp.c index 2c5f9d71..259e83a3 100644 --- a/hw/fsp/fsp.c +++ b/hw/fsp/fsp.c @@ -91,7 +91,7 @@ static enum ipl_state ipl_state = ipl_initial; static struct fsp *first_fsp; static struct fsp *active_fsp; static u16 fsp_curseq = 0x8000; -static __be64 *fsp_tce_table; +static __be64 *volatile fsp_tce_table; #define FSP_INBOUND_SIZE 0x00100000UL static void *fsp_inbound_buf = NULL;
use volatile pointer to avoid optimization introduced with gcc-11 on constant address asignment to pointer. Resolves : the warray ofset warning during compilation. Signed-off-by: Abhishek Singh Tomar <abhishek@linux.ibm.com> --- core/console.c | 2 +- core/fast-reboot.c | 6 ++++-- core/init.c | 6 +++++- core/opal-dump.c | 7 ++++--- hdata/spira.c | 2 +- hdata/spira.h | 2 +- hw/fsp/fsp.c | 2 +- 7 files changed, 17 insertions(+), 10 deletions(-)