* [PATCH 0/4] misc safety certification fixes @ 2018-10-15 9:55 Stefano Stabellini 2018-10-15 9:56 ` [PATCH 1/4] xen/arm: initialize target Stefano Stabellini ` (3 more replies) 0 siblings, 4 replies; 19+ messages in thread From: Stefano Stabellini @ 2018-10-15 9:55 UTC (permalink / raw) To: xen-devel; +Cc: andrew.cooper3, julien.grall, sstabellini, JBeulich Hi all, This short patch series fixes a few issues discovered by the safety certifications code scanner. The first two patches address simple variable initializations concerns. The third patch introduces a new macro that is used throughout the code in patch #4 to access _stext, _etext pointers and friends. Jan, Andrew, as you know my goal is to fix arm and common code. I think it would be best to do the same for the x86 code as well, but let me know if you think otherwise. Cheers, Stefano Stefano Stabellini (4): xen/arm: initialize target xen/arm: initialize access xen: introduce __symbol xen: use __symbol everywhere xen/arch/arm/alternative.c | 6 +-- xen/arch/arm/arm32/livepatch.c | 2 +- xen/arch/arm/arm64/livepatch.c | 2 +- xen/arch/arm/domain_build.c | 2 +- xen/arch/arm/livepatch.c | 6 +-- xen/arch/arm/mem_access.c | 1 + xen/arch/arm/mm.c | 9 +++-- xen/arch/arm/setup.c | 8 ++-- xen/arch/arm/vgic-v2.c | 2 +- xen/arch/arm/vgic-v3.c | 2 +- xen/arch/x86/setup.c | 79 +++++++++++++++++++------------------ xen/arch/x86/tboot.c | 12 +++--- xen/arch/x86/x86_64/machine_kexec.c | 4 +- xen/include/asm-arm/grant_table.h | 3 +- xen/include/asm-arm/mm.h | 10 ++++- xen/include/asm-x86/mm.h | 4 +- xen/include/asm-x86/page.h | 6 +++ xen/include/xen/kernel.h | 8 ++-- 18 files changed, 93 insertions(+), 73 deletions(-) _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 1/4] xen/arm: initialize target 2018-10-15 9:55 [PATCH 0/4] misc safety certification fixes Stefano Stabellini @ 2018-10-15 9:56 ` Stefano Stabellini 2018-10-15 13:07 ` Julien Grall 2018-10-15 9:56 ` [PATCH 2/4] xen/arm: initialize access Stefano Stabellini ` (2 subsequent siblings) 3 siblings, 1 reply; 19+ messages in thread From: Stefano Stabellini @ 2018-10-15 9:56 UTC (permalink / raw) To: xen-devel; +Cc: Stefano Stabellini, julien.grall, sstabellini Initialize variable target before passing it as a parameter. Signed-off-by: Stefano Stabellini <stefanos@xilinx.com> --- xen/arch/arm/vgic-v2.c | 2 +- xen/arch/arm/vgic-v3.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/xen/arch/arm/vgic-v2.c b/xen/arch/arm/vgic-v2.c index f6c11f1..0099fcf 100644 --- a/xen/arch/arm/vgic-v2.c +++ b/xen/arch/arm/vgic-v2.c @@ -379,6 +379,7 @@ static bool vgic_v2_to_sgi(struct vcpu *v, register_t sgir) enum gic_sgi_mode sgi_mode; struct sgi_target target; + sgi_target_init(&target); irqmode = (sgir & GICD_SGI_TARGET_LIST_MASK) >> GICD_SGI_TARGET_LIST_SHIFT; virq = (sgir & GICD_SGI_INTID_MASK); @@ -386,7 +387,6 @@ static bool vgic_v2_to_sgi(struct vcpu *v, register_t sgir) switch ( irqmode ) { case GICD_SGI_TARGET_LIST_VAL: - sgi_target_init(&target); target.list = (sgir & GICD_SGI_TARGET_MASK) >> GICD_SGI_TARGET_SHIFT; sgi_mode = SGI_TARGET_LIST; break; diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c index efe824c..c14bcd8 100644 --- a/xen/arch/arm/vgic-v3.c +++ b/xen/arch/arm/vgic-v3.c @@ -1474,6 +1474,7 @@ static bool vgic_v3_to_sgi(struct vcpu *v, register_t sgir) enum gic_sgi_mode sgi_mode; struct sgi_target target; + sgi_target_init(&target); irqmode = (sgir >> ICH_SGI_IRQMODE_SHIFT) & ICH_SGI_IRQMODE_MASK; virq = (sgir >> ICH_SGI_IRQ_SHIFT ) & ICH_SGI_IRQ_MASK; @@ -1481,7 +1482,6 @@ static bool vgic_v3_to_sgi(struct vcpu *v, register_t sgir) switch ( irqmode ) { case ICH_SGI_TARGET_LIST: - sgi_target_init(&target); /* We assume that only AFF1 is used in ICC_SGI1R_EL1. */ target.aff1 = (sgir >> ICH_SGI_AFFINITY_LEVEL(1)) & ICH_SGI_AFFx_MASK; target.list = sgir & ICH_SGI_TARGETLIST_MASK; -- 1.9.1 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH 1/4] xen/arm: initialize target 2018-10-15 9:56 ` [PATCH 1/4] xen/arm: initialize target Stefano Stabellini @ 2018-10-15 13:07 ` Julien Grall 0 siblings, 0 replies; 19+ messages in thread From: Julien Grall @ 2018-10-15 13:07 UTC (permalink / raw) To: Stefano Stabellini, xen-devel; +Cc: Stefano Stabellini, nd Hi, On 15/10/2018 10:56, Stefano Stabellini wrote: > Initialize variable target before passing it as a parameter. As such the current code is not wrong. So please explain why you need this. Cheers, -- Julien Grall _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 2/4] xen/arm: initialize access 2018-10-15 9:55 [PATCH 0/4] misc safety certification fixes Stefano Stabellini 2018-10-15 9:56 ` [PATCH 1/4] xen/arm: initialize target Stefano Stabellini @ 2018-10-15 9:56 ` Stefano Stabellini 2018-10-15 13:08 ` Julien Grall 2018-10-15 15:15 ` Tamas K Lengyel 2018-10-15 9:56 ` [PATCH 3/4] xen: introduce __symbol Stefano Stabellini 2018-10-15 9:56 ` [PATCH 4/4] xen: use __symbol everywhere Stefano Stabellini 3 siblings, 2 replies; 19+ messages in thread From: Stefano Stabellini @ 2018-10-15 9:56 UTC (permalink / raw) To: xen-devel; +Cc: Stefano Stabellini, julien.grall, sstabellini Initialize variable *access before returning it back to the caller. Signed-off-by: Stefano Stabellini <stefanos@xilinx.com> --- xen/arch/arm/mem_access.c | 1 + 1 file changed, 1 insertion(+) diff --git a/xen/arch/arm/mem_access.c b/xen/arch/arm/mem_access.c index ba4ec78..10ab308 100644 --- a/xen/arch/arm/mem_access.c +++ b/xen/arch/arm/mem_access.c @@ -47,6 +47,7 @@ static int __p2m_get_mem_access(struct domain *d, gfn_t gfn, }; ASSERT(p2m_is_locked(p2m)); + *access = XENMEM_access_n; /* If no setting was ever set, just return rwx. */ if ( !p2m->mem_access_enabled ) -- 1.9.1 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH 2/4] xen/arm: initialize access 2018-10-15 9:56 ` [PATCH 2/4] xen/arm: initialize access Stefano Stabellini @ 2018-10-15 13:08 ` Julien Grall 2018-10-15 15:15 ` Tamas K Lengyel 1 sibling, 0 replies; 19+ messages in thread From: Julien Grall @ 2018-10-15 13:08 UTC (permalink / raw) To: Stefano Stabellini, xen-devel Cc: Stefano Stabellini, nd, Tamas K Lengyel, Razvan Cojocaru Hi, Please use scripts/get_maintainers.pl to CC relevant maintainers. In this case you need to add Ravzan and Tamas. On 15/10/2018 10:56, Stefano Stabellini wrote: > Initialize variable *access before returning it back to the caller. Same as the previous patch, why do you need this? Cheers, > > Signed-off-by: Stefano Stabellini <stefanos@xilinx.com> > --- > xen/arch/arm/mem_access.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/xen/arch/arm/mem_access.c b/xen/arch/arm/mem_access.c > index ba4ec78..10ab308 100644 > --- a/xen/arch/arm/mem_access.c > +++ b/xen/arch/arm/mem_access.c > @@ -47,6 +47,7 @@ static int __p2m_get_mem_access(struct domain *d, gfn_t gfn, > }; > > ASSERT(p2m_is_locked(p2m)); > + *access = XENMEM_access_n; > > /* If no setting was ever set, just return rwx. */ > if ( !p2m->mem_access_enabled ) > -- Julien Grall _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 2/4] xen/arm: initialize access 2018-10-15 9:56 ` [PATCH 2/4] xen/arm: initialize access Stefano Stabellini 2018-10-15 13:08 ` Julien Grall @ 2018-10-15 15:15 ` Tamas K Lengyel 2018-10-16 1:14 ` Stefano Stabellini 1 sibling, 1 reply; 19+ messages in thread From: Tamas K Lengyel @ 2018-10-15 15:15 UTC (permalink / raw) To: Stefano Stabellini; +Cc: Xen-devel, Julien Grall, stefanos On Mon, Oct 15, 2018 at 3:57 AM Stefano Stabellini <sstabellini@kernel.org> wrote: > > Initialize variable *access before returning it back to the caller. > > Signed-off-by: Stefano Stabellini <stefanos@xilinx.com> > --- > xen/arch/arm/mem_access.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/xen/arch/arm/mem_access.c b/xen/arch/arm/mem_access.c > index ba4ec78..10ab308 100644 > --- a/xen/arch/arm/mem_access.c > +++ b/xen/arch/arm/mem_access.c > @@ -47,6 +47,7 @@ static int __p2m_get_mem_access(struct domain *d, gfn_t gfn, > }; > > ASSERT(p2m_is_locked(p2m)); > + *access = XENMEM_access_n; Why XENMEM_access_n and why set this at all here? Tamas _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 2/4] xen/arm: initialize access 2018-10-15 15:15 ` Tamas K Lengyel @ 2018-10-16 1:14 ` Stefano Stabellini 2018-10-16 15:24 ` Tamas K Lengyel 0 siblings, 1 reply; 19+ messages in thread From: Stefano Stabellini @ 2018-10-16 1:14 UTC (permalink / raw) To: Tamas K Lengyel; +Cc: Xen-devel, Julien Grall, Stefano Stabellini, stefanos On Mon, 15 Oct 2018, Tamas K Lengyel wrote: > On Mon, Oct 15, 2018 at 3:57 AM Stefano Stabellini > <sstabellini@kernel.org> wrote: > > > > Initialize variable *access before returning it back to the caller. > > > > Signed-off-by: Stefano Stabellini <stefanos@xilinx.com> > > --- > > xen/arch/arm/mem_access.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/xen/arch/arm/mem_access.c b/xen/arch/arm/mem_access.c > > index ba4ec78..10ab308 100644 > > --- a/xen/arch/arm/mem_access.c > > +++ b/xen/arch/arm/mem_access.c > > @@ -47,6 +47,7 @@ static int __p2m_get_mem_access(struct domain *d, gfn_t gfn, > > }; > > > > ASSERT(p2m_is_locked(p2m)); > > + *access = XENMEM_access_n; > > Why XENMEM_access_n and why set this at all here? Hi Tamas, Yes, I missed an explanation. Initializing variables before passing them as parameter or as a return value to a function is a safety certification requirement. Also, it makes the code a bit nicer. In the specific case of this function, *access is initialized before returning in all cases but the return -ESRCH case. I thought the nicer way to make sure *access is always initialized, making the code more robust, would be to initialize *access at the beginning of the function with a restrictive value. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 2/4] xen/arm: initialize access 2018-10-16 1:14 ` Stefano Stabellini @ 2018-10-16 15:24 ` Tamas K Lengyel 2018-10-17 13:53 ` Stefano Stabellini 0 siblings, 1 reply; 19+ messages in thread From: Tamas K Lengyel @ 2018-10-16 15:24 UTC (permalink / raw) To: Stefano Stabellini; +Cc: Xen-devel, Julien Grall, stefanos On Mon, Oct 15, 2018 at 7:14 PM Stefano Stabellini <sstabellini@kernel.org> wrote: > > On Mon, 15 Oct 2018, Tamas K Lengyel wrote: > > On Mon, Oct 15, 2018 at 3:57 AM Stefano Stabellini > > <sstabellini@kernel.org> wrote: > > > > > > Initialize variable *access before returning it back to the caller. > > > > > > Signed-off-by: Stefano Stabellini <stefanos@xilinx.com> > > > --- > > > xen/arch/arm/mem_access.c | 1 + > > > 1 file changed, 1 insertion(+) > > > > > > diff --git a/xen/arch/arm/mem_access.c b/xen/arch/arm/mem_access.c > > > index ba4ec78..10ab308 100644 > > > --- a/xen/arch/arm/mem_access.c > > > +++ b/xen/arch/arm/mem_access.c > > > @@ -47,6 +47,7 @@ static int __p2m_get_mem_access(struct domain *d, gfn_t gfn, > > > }; > > > > > > ASSERT(p2m_is_locked(p2m)); > > > + *access = XENMEM_access_n; > > > > Why XENMEM_access_n and why set this at all here? > > Hi Tamas, > > Yes, I missed an explanation. Initializing variables before passing them > as parameter or as a return value to a function is a safety > certification requirement. Also, it makes the code a bit nicer. > > In the specific case of this function, *access is initialized before > returning in all cases but the return -ESRCH case. I thought the nicer > way to make sure *access is always initialized, making the code more > robust, would be to initialize *access at the beginning of the function > with a restrictive value. Got it, thanks, Please use p2m->default_access for this instead to be consistent with similar code at other spots. Tamas _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 2/4] xen/arm: initialize access 2018-10-16 15:24 ` Tamas K Lengyel @ 2018-10-17 13:53 ` Stefano Stabellini 0 siblings, 0 replies; 19+ messages in thread From: Stefano Stabellini @ 2018-10-17 13:53 UTC (permalink / raw) To: Tamas K Lengyel; +Cc: Xen-devel, Julien Grall, Stefano Stabellini, stefanos On Tue, 16 Oct 2018, Tamas K Lengyel wrote: > On Mon, Oct 15, 2018 at 7:14 PM Stefano Stabellini > <sstabellini@kernel.org> wrote: > > > > On Mon, 15 Oct 2018, Tamas K Lengyel wrote: > > > On Mon, Oct 15, 2018 at 3:57 AM Stefano Stabellini > > > <sstabellini@kernel.org> wrote: > > > > > > > > Initialize variable *access before returning it back to the caller. > > > > > > > > Signed-off-by: Stefano Stabellini <stefanos@xilinx.com> > > > > --- > > > > xen/arch/arm/mem_access.c | 1 + > > > > 1 file changed, 1 insertion(+) > > > > > > > > diff --git a/xen/arch/arm/mem_access.c b/xen/arch/arm/mem_access.c > > > > index ba4ec78..10ab308 100644 > > > > --- a/xen/arch/arm/mem_access.c > > > > +++ b/xen/arch/arm/mem_access.c > > > > @@ -47,6 +47,7 @@ static int __p2m_get_mem_access(struct domain *d, gfn_t gfn, > > > > }; > > > > > > > > ASSERT(p2m_is_locked(p2m)); > > > > + *access = XENMEM_access_n; > > > > > > Why XENMEM_access_n and why set this at all here? > > > > Hi Tamas, > > > > Yes, I missed an explanation. Initializing variables before passing them > > as parameter or as a return value to a function is a safety > > certification requirement. Also, it makes the code a bit nicer. > > > > In the specific case of this function, *access is initialized before > > returning in all cases but the return -ESRCH case. I thought the nicer > > way to make sure *access is always initialized, making the code more > > robust, would be to initialize *access at the beginning of the function > > with a restrictive value. > > Got it, thanks, Please use p2m->default_access for this instead to be > consistent with similar code at other spots. OK, no prob _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 3/4] xen: introduce __symbol 2018-10-15 9:55 [PATCH 0/4] misc safety certification fixes Stefano Stabellini 2018-10-15 9:56 ` [PATCH 1/4] xen/arm: initialize target Stefano Stabellini 2018-10-15 9:56 ` [PATCH 2/4] xen/arm: initialize access Stefano Stabellini @ 2018-10-15 9:56 ` Stefano Stabellini 2018-10-15 12:20 ` Andrew Cooper ` (2 more replies) 2018-10-15 9:56 ` [PATCH 4/4] xen: use __symbol everywhere Stefano Stabellini 3 siblings, 3 replies; 19+ messages in thread From: Stefano Stabellini @ 2018-10-15 9:56 UTC (permalink / raw) To: xen-devel; +Cc: Stefano Stabellini, julien.grall, sstabellini Introduce a macro, __symbol, which is a simple wrapper around RELOC_HIDE to be used everywhere symbols such as _stext and _etext are used in the code. RELOC_HIDE is needed when accessing symbols such as _stext and _etext because the C standard forbids comparisons between pointers pointing to different objects. _stext, _etext, etc. are all pointers to different objects from ANCI C point of view. To work around potential C compiler issues (which have actually been found, see the comment on top of RELOC_HIDE in Linux), and to help with certifications, let's introduce some syntactic sugar to be used in following patches. Signed-off-by: Stefano Stabellini <stefanos@xilinx.com> --- xen/include/asm-arm/mm.h | 6 ++++++ xen/include/asm-x86/page.h | 6 ++++++ 2 files changed, 12 insertions(+) diff --git a/xen/include/asm-arm/mm.h b/xen/include/asm-arm/mm.h index 940b74b..02ce05a 100644 --- a/xen/include/asm-arm/mm.h +++ b/xen/include/asm-arm/mm.h @@ -284,6 +284,12 @@ static inline uint64_t gvirt_to_maddr(vaddr_t va, paddr_t *pa, #define __mfn_to_virt(mfn) (maddr_to_virt((paddr_t)(mfn) << PAGE_SHIFT)) /* + * Use RELOC_HIDE with symbols such as _stext and _etext to avoid errors + * on comparing pointers to different objects + */ +#define __symbol(x) ((char *)RELOC_HIDE((unsigned long)(x), 0)) + +/* * We define non-underscored wrappers for above conversion functions. * These are overriden in various source files while underscored version * remain intact. diff --git a/xen/include/asm-x86/page.h b/xen/include/asm-x86/page.h index c1e9293..3f1592b 100644 --- a/xen/include/asm-x86/page.h +++ b/xen/include/asm-x86/page.h @@ -274,6 +274,12 @@ void copy_page_sse2(void *, const void *); #define vmap_to_mfn(va) _mfn(l1e_get_pfn(*virt_to_xen_l1e((unsigned long)(va)))) #define vmap_to_page(va) mfn_to_page(vmap_to_mfn(va)) +/* + * Use RELOC_HIDE with symbols such as _stext and _etext to avoid errors + * on comparing pointers to different objects + */ +#define __symbol(x) ((char *)RELOC_HIDE((unsigned long)(x), 0)) + #endif /* !defined(__ASSEMBLY__) */ /* Where to find each level of the linear mapping */ -- 1.9.1 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH 3/4] xen: introduce __symbol 2018-10-15 9:56 ` [PATCH 3/4] xen: introduce __symbol Stefano Stabellini @ 2018-10-15 12:20 ` Andrew Cooper 2018-10-15 13:20 ` Julien Grall 2018-10-25 10:37 ` Jan Beulich 2 siblings, 0 replies; 19+ messages in thread From: Andrew Cooper @ 2018-10-15 12:20 UTC (permalink / raw) To: Stefano Stabellini, xen-devel; +Cc: Stefano Stabellini, julien.grall On 15/10/18 10:56, Stefano Stabellini wrote: > Introduce a macro, __symbol, which is a simple wrapper around RELOC_HIDE > to be used everywhere symbols such as _stext and _etext are used in the > code. > > RELOC_HIDE is needed when accessing symbols such as _stext and _etext > because the C standard forbids comparisons between pointers pointing to > different objects. _stext, _etext, etc. are all pointers to different > objects from ANCI C point of view. > > To work around potential C compiler issues (which have actually > been found, see the comment on top of RELOC_HIDE in Linux), and to help > with certifications, let's introduce some syntactic sugar to be used in > following patches. > > Signed-off-by: Stefano Stabellini <stefanos@xilinx.com> > --- > xen/include/asm-arm/mm.h | 6 ++++++ > xen/include/asm-x86/page.h | 6 ++++++ > 2 files changed, 12 insertions(+) > > diff --git a/xen/include/asm-arm/mm.h b/xen/include/asm-arm/mm.h > index 940b74b..02ce05a 100644 > --- a/xen/include/asm-arm/mm.h > +++ b/xen/include/asm-arm/mm.h > @@ -284,6 +284,12 @@ static inline uint64_t gvirt_to_maddr(vaddr_t va, paddr_t *pa, > #define __mfn_to_virt(mfn) (maddr_to_virt((paddr_t)(mfn) << PAGE_SHIFT)) > > /* > + * Use RELOC_HIDE with symbols such as _stext and _etext to avoid errors > + * on comparing pointers to different objects > + */ > +#define __symbol(x) ((char *)RELOC_HIDE((unsigned long)(x), 0)) Casting to char * isn't correct. It breaks pointer arithmetic for some of the uses you didn't convert in the following patch, and the unsigned long cast isn't necessary either. Dropping both of them will keep everything working correctly, as RELOC_HIDE() already has the correct return type. ~Andrew _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 3/4] xen: introduce __symbol 2018-10-15 9:56 ` [PATCH 3/4] xen: introduce __symbol Stefano Stabellini 2018-10-15 12:20 ` Andrew Cooper @ 2018-10-15 13:20 ` Julien Grall 2018-10-16 1:34 ` Stefano Stabellini 2018-10-25 10:37 ` Jan Beulich 2 siblings, 1 reply; 19+ messages in thread From: Julien Grall @ 2018-10-15 13:20 UTC (permalink / raw) To: Stefano Stabellini, xen-devel Cc: Stefano Stabellini, nd, Jan Beulich, Andrew Cooper Hi, Please use scripts/get_maintainers.pl to CC relevant maintainers. On 15/10/2018 10:56, Stefano Stabellini wrote: > Introduce a macro, __symbol, which is a simple wrapper around RELOC_HIDE > to be used everywhere symbols such as _stext and _etext are used in the > code. > > RELOC_HIDE is needed when accessing symbols such as _stext and _etext > because the C standard forbids comparisons between pointers pointing to > different objects. _stext, _etext, etc. are all pointers to different > objects from ANCI C point of view. > > To work around potential C compiler issues (which have actually > been found, see the comment on top of RELOC_HIDE in Linux), and to help > with certifications, let's introduce some syntactic sugar to be used in > following patches. > > Signed-off-by: Stefano Stabellini <stefanos@xilinx.com> > --- > xen/include/asm-arm/mm.h | 6 ++++++ > xen/include/asm-x86/page.h | 6 ++++++ > 2 files changed, 12 insertions(+) > > diff --git a/xen/include/asm-arm/mm.h b/xen/include/asm-arm/mm.h > index 940b74b..02ce05a 100644 > --- a/xen/include/asm-arm/mm.h > +++ b/xen/include/asm-arm/mm.h > @@ -284,6 +284,12 @@ static inline uint64_t gvirt_to_maddr(vaddr_t va, paddr_t *pa, > #define __mfn_to_virt(mfn) (maddr_to_virt((paddr_t)(mfn) << PAGE_SHIFT)) > > /* > + * Use RELOC_HIDE with symbols such as _stext and _etext to avoid errors > + * on comparing pointers to different objects > + */ > +#define __symbol(x) ((char *)RELOC_HIDE((unsigned long)(x), 0)) There are no different between arm and x86. A better place would be xen/compiler.h (or something common). But, after this patch, there are even more chance the compiler will consider that 2 _symbol(...) will come from different objects. So how is this meant to help here? Cheers, -- Julien Grall _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 3/4] xen: introduce __symbol 2018-10-15 13:20 ` Julien Grall @ 2018-10-16 1:34 ` Stefano Stabellini 2018-10-16 10:06 ` Andrew Cooper 0 siblings, 1 reply; 19+ messages in thread From: Stefano Stabellini @ 2018-10-16 1:34 UTC (permalink / raw) To: Julien Grall Cc: Stefano Stabellini, Stefano Stabellini, Andrew Cooper, Jan Beulich, xen-devel, nd On Mon, 15 Oct 2018, Julien Grall wrote: > Hi, > > Please use scripts/get_maintainers.pl to CC relevant maintainers. > > On 15/10/2018 10:56, Stefano Stabellini wrote: > > Introduce a macro, __symbol, which is a simple wrapper around RELOC_HIDE > > to be used everywhere symbols such as _stext and _etext are used in the > > code. > > > > RELOC_HIDE is needed when accessing symbols such as _stext and _etext > > because the C standard forbids comparisons between pointers pointing to > > different objects. _stext, _etext, etc. are all pointers to different > > objects from ANCI C point of view. > > > > To work around potential C compiler issues (which have actually > > been found, see the comment on top of RELOC_HIDE in Linux), and to help > > with certifications, let's introduce some syntactic sugar to be used in > > following patches. > > > > Signed-off-by: Stefano Stabellini <stefanos@xilinx.com> > > --- > > xen/include/asm-arm/mm.h | 6 ++++++ > > xen/include/asm-x86/page.h | 6 ++++++ > > 2 files changed, 12 insertions(+) > > > > diff --git a/xen/include/asm-arm/mm.h b/xen/include/asm-arm/mm.h > > index 940b74b..02ce05a 100644 > > --- a/xen/include/asm-arm/mm.h > > +++ b/xen/include/asm-arm/mm.h > > @@ -284,6 +284,12 @@ static inline uint64_t gvirt_to_maddr(vaddr_t va, > > paddr_t *pa, > > #define __mfn_to_virt(mfn) (maddr_to_virt((paddr_t)(mfn) << PAGE_SHIFT)) > > /* > > + * Use RELOC_HIDE with symbols such as _stext and _etext to avoid errors > > + * on comparing pointers to different objects > > + */ > > +#define __symbol(x) ((char *)RELOC_HIDE((unsigned long)(x), 0)) > > There are no different between arm and x86. A better place would be > xen/compiler.h (or something common). OK > But, after this patch, there are even more chance the compiler will consider > that 2 _symbol(...) will come from different objects. So how is this meant to > help here? I think the mistake was to cast the return to char*. If I remove the cast, as Andrew suggested, then any comparison would be a comparison between unsigned long, that should be accepted and safe. However, the parameter to RELOC_HIDE has to be casted to unsigned long, because most often we pass char*: /local/repos/xen-upstream/xen/include/xen/compiler.h:100:5: error: cast specifies array type (typeof(ptr)) (__ptr + (off)); }) So I think the __symbol macro should be: +#define __symbol(x) (RELOC_HIDE((unsigned long)(x), 0)) _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 3/4] xen: introduce __symbol 2018-10-16 1:34 ` Stefano Stabellini @ 2018-10-16 10:06 ` Andrew Cooper 2018-10-16 13:01 ` Stefano Stabellini 0 siblings, 1 reply; 19+ messages in thread From: Andrew Cooper @ 2018-10-16 10:06 UTC (permalink / raw) To: Stefano Stabellini, Julien Grall Cc: xen-devel, nd, Jan Beulich, Stefano Stabellini On 16/10/18 02:34, Stefano Stabellini wrote: > On Mon, 15 Oct 2018, Julien Grall wrote: >> Hi, >> >> Please use scripts/get_maintainers.pl to CC relevant maintainers. >> >> On 15/10/2018 10:56, Stefano Stabellini wrote: >>> Introduce a macro, __symbol, which is a simple wrapper around RELOC_HIDE >>> to be used everywhere symbols such as _stext and _etext are used in the >>> code. >>> >>> RELOC_HIDE is needed when accessing symbols such as _stext and _etext >>> because the C standard forbids comparisons between pointers pointing to >>> different objects. _stext, _etext, etc. are all pointers to different >>> objects from ANCI C point of view. >>> >>> To work around potential C compiler issues (which have actually >>> been found, see the comment on top of RELOC_HIDE in Linux), and to help >>> with certifications, let's introduce some syntactic sugar to be used in >>> following patches. >>> >>> Signed-off-by: Stefano Stabellini <stefanos@xilinx.com> >>> --- >>> xen/include/asm-arm/mm.h | 6 ++++++ >>> xen/include/asm-x86/page.h | 6 ++++++ >>> 2 files changed, 12 insertions(+) >>> >>> diff --git a/xen/include/asm-arm/mm.h b/xen/include/asm-arm/mm.h >>> index 940b74b..02ce05a 100644 >>> --- a/xen/include/asm-arm/mm.h >>> +++ b/xen/include/asm-arm/mm.h >>> @@ -284,6 +284,12 @@ static inline uint64_t gvirt_to_maddr(vaddr_t va, >>> paddr_t *pa, >>> #define __mfn_to_virt(mfn) (maddr_to_virt((paddr_t)(mfn) << PAGE_SHIFT)) >>> /* >>> + * Use RELOC_HIDE with symbols such as _stext and _etext to avoid errors >>> + * on comparing pointers to different objects >>> + */ >>> +#define __symbol(x) ((char *)RELOC_HIDE((unsigned long)(x), 0)) >> There are no different between arm and x86. A better place would be >> xen/compiler.h (or something common). > OK > > >> But, after this patch, there are even more chance the compiler will consider >> that 2 _symbol(...) will come from different objects. So how is this meant to >> help here? > I think the mistake was to cast the return to char*. If I remove the > cast, as Andrew suggested, then any comparison would be a comparison > between unsigned long, that should be accepted and safe. > > However, the parameter to RELOC_HIDE has to be casted to unsigned long, > because most often we pass char*: > > /local/repos/xen-upstream/xen/include/xen/compiler.h:100:5: error: cast specifies array type > (typeof(ptr)) (__ptr + (off)); }) > > So I think the __symbol macro should be: > > +#define __symbol(x) (RELOC_HIDE((unsigned long)(x), 0)) > I don't see why the unsigned long cast is needed. What is wrong by having "char *ptr + 0" which is legal pointer arithmetic, and has well defined behaviour? If it is necessary, then you need a (typeof(x)) cast again on the outside, because the unsigned long cast causes RELOC_HIDE's typeof to be wrong overall. ~Andrew _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 3/4] xen: introduce __symbol 2018-10-16 10:06 ` Andrew Cooper @ 2018-10-16 13:01 ` Stefano Stabellini 0 siblings, 0 replies; 19+ messages in thread From: Stefano Stabellini @ 2018-10-16 13:01 UTC (permalink / raw) To: Andrew Cooper Cc: Stefano Stabellini, Stefano Stabellini, Julien Grall, Jan Beulich, xen-devel, nd [-- Attachment #1: Type: TEXT/PLAIN, Size: 3988 bytes --] On Tue, 16 Oct 2018, Andrew Cooper wrote: > On 16/10/18 02:34, Stefano Stabellini wrote: > > On Mon, 15 Oct 2018, Julien Grall wrote: > >> Hi, > >> > >> Please use scripts/get_maintainers.pl to CC relevant maintainers. > >> > >> On 15/10/2018 10:56, Stefano Stabellini wrote: > >>> Introduce a macro, __symbol, which is a simple wrapper around RELOC_HIDE > >>> to be used everywhere symbols such as _stext and _etext are used in the > >>> code. > >>> > >>> RELOC_HIDE is needed when accessing symbols such as _stext and _etext > >>> because the C standard forbids comparisons between pointers pointing to > >>> different objects. _stext, _etext, etc. are all pointers to different > >>> objects from ANCI C point of view. > >>> > >>> To work around potential C compiler issues (which have actually > >>> been found, see the comment on top of RELOC_HIDE in Linux), and to help > >>> with certifications, let's introduce some syntactic sugar to be used in > >>> following patches. > >>> > >>> Signed-off-by: Stefano Stabellini <stefanos@xilinx.com> > >>> --- > >>> xen/include/asm-arm/mm.h | 6 ++++++ > >>> xen/include/asm-x86/page.h | 6 ++++++ > >>> 2 files changed, 12 insertions(+) > >>> > >>> diff --git a/xen/include/asm-arm/mm.h b/xen/include/asm-arm/mm.h > >>> index 940b74b..02ce05a 100644 > >>> --- a/xen/include/asm-arm/mm.h > >>> +++ b/xen/include/asm-arm/mm.h > >>> @@ -284,6 +284,12 @@ static inline uint64_t gvirt_to_maddr(vaddr_t va, > >>> paddr_t *pa, > >>> #define __mfn_to_virt(mfn) (maddr_to_virt((paddr_t)(mfn) << PAGE_SHIFT)) > >>> /* > >>> + * Use RELOC_HIDE with symbols such as _stext and _etext to avoid errors > >>> + * on comparing pointers to different objects > >>> + */ > >>> +#define __symbol(x) ((char *)RELOC_HIDE((unsigned long)(x), 0)) > >> There are no different between arm and x86. A better place would be > >> xen/compiler.h (or something common). > > OK > > > > > >> But, after this patch, there are even more chance the compiler will consider > >> that 2 _symbol(...) will come from different objects. So how is this meant to > >> help here? > > I think the mistake was to cast the return to char*. If I remove the > > cast, as Andrew suggested, then any comparison would be a comparison > > between unsigned long, that should be accepted and safe. > > > > However, the parameter to RELOC_HIDE has to be casted to unsigned long, > > because most often we pass char*: > > > > /local/repos/xen-upstream/xen/include/xen/compiler.h:100:5: error: cast specifies array type > > (typeof(ptr)) (__ptr + (off)); }) > > > > So I think the __symbol macro should be: > > > > +#define __symbol(x) (RELOC_HIDE((unsigned long)(x), 0)) > > > > I don't see why the unsigned long cast is needed. What is wrong by > having "char *ptr + 0" which is legal pointer arithmetic, and has well > defined behaviour? The issue is that _stext is defined as char _stext[], so __symbol(_stext) wouldn't work because _stext is an array type. As I pasted above the error is "cast specifies array type". I guess it would have to be __symbol(&_stext[0]) everywhere. Also, I don't think it is a good idea for __symbol to return a pointer, because then we are still left with the original issue of comparing pointers of different sizes. We just moved the issue from the original pointers to the ones returned by RELOC_HIDE. > If it is necessary, then you need a (typeof(x)) cast again on the > outside, because the unsigned long cast causes RELOC_HIDE's typeof to be > wrong overall. I am not sure I follow you here, do you mean: ((typeof(x)) RELOC_HIDE((unsigned long)(x), 0) If so, I think it is best to have RELOC_HIDE return unsigned long (no outside cast), hence removing all issues of comparing pointers to different objects. Then, I'll adjust the few call sites that need changes because of the char* -> unsigned long conversion. I have already modified patch #4 following this strategy and the modifications required were very few. [-- Attachment #2: Type: text/plain, Size: 157 bytes --] _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 3/4] xen: introduce __symbol 2018-10-15 9:56 ` [PATCH 3/4] xen: introduce __symbol Stefano Stabellini 2018-10-15 12:20 ` Andrew Cooper 2018-10-15 13:20 ` Julien Grall @ 2018-10-25 10:37 ` Jan Beulich 2 siblings, 0 replies; 19+ messages in thread From: Jan Beulich @ 2018-10-25 10:37 UTC (permalink / raw) To: Stefano Stabellini; +Cc: xen-devel, Julien Grall, Stefano Stabellini >>> On 15.10.18 at 11:56, <sstabellini@kernel.org> wrote: > Introduce a macro, __symbol, Irrespective of all other remarks already made, and irrespective of me not being convinced that we really need to wrap _text, _stext, and alike - no new name space violations please. I.e. no leading underscores in header files, and no double leading underscores in .c ones. Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 4/4] xen: use __symbol everywhere 2018-10-15 9:55 [PATCH 0/4] misc safety certification fixes Stefano Stabellini ` (2 preceding siblings ...) 2018-10-15 9:56 ` [PATCH 3/4] xen: introduce __symbol Stefano Stabellini @ 2018-10-15 9:56 ` Stefano Stabellini 2018-10-25 10:39 ` Jan Beulich 3 siblings, 1 reply; 19+ messages in thread From: Stefano Stabellini @ 2018-10-15 9:56 UTC (permalink / raw) To: xen-devel; +Cc: Stefano Stabellini, julien.grall, sstabellini Use __symbol everywhere _stext, _etext, etc. are used. Technically, it is only required when comparing pointers, but use it everywhere to avoid confusion. Signed-off-by: Stefano Stabellini <stefanos@xilinx.com> --- xen/arch/arm/alternative.c | 6 +-- xen/arch/arm/arm32/livepatch.c | 2 +- xen/arch/arm/arm64/livepatch.c | 2 +- xen/arch/arm/domain_build.c | 2 +- xen/arch/arm/livepatch.c | 6 +-- xen/arch/arm/mm.c | 9 +++-- xen/arch/arm/setup.c | 8 ++-- xen/arch/x86/setup.c | 79 +++++++++++++++++++------------------ xen/arch/x86/tboot.c | 12 +++--- xen/arch/x86/x86_64/machine_kexec.c | 4 +- xen/include/asm-arm/grant_table.h | 3 +- xen/include/asm-arm/mm.h | 4 +- xen/include/asm-x86/mm.h | 4 +- xen/include/xen/kernel.h | 8 ++-- 14 files changed, 78 insertions(+), 71 deletions(-) diff --git a/xen/arch/arm/alternative.c b/xen/arch/arm/alternative.c index 52ed7ed..305b337 100644 --- a/xen/arch/arm/alternative.c +++ b/xen/arch/arm/alternative.c @@ -187,8 +187,8 @@ static int __apply_alternatives_multi_stop(void *unused) { int ret; struct alt_region region; - mfn_t xen_mfn = virt_to_mfn(_start); - paddr_t xen_size = _end - _start; + mfn_t xen_mfn = virt_to_mfn(__symbol(_start)); + paddr_t xen_size = __symbol(_end) - __symbol(_start); unsigned int xen_order = get_order_from_bytes(xen_size); void *xenmap; @@ -206,7 +206,7 @@ static int __apply_alternatives_multi_stop(void *unused) region.begin = __alt_instructions; region.end = __alt_instructions_end; - ret = __apply_alternatives(®ion, xenmap - (void *)_start); + ret = __apply_alternatives(®ion, xenmap - (void *)__symbol(_start)); /* The patching is not expected to fail during boot. */ BUG_ON(ret != 0); diff --git a/xen/arch/arm/arm32/livepatch.c b/xen/arch/arm/arm32/livepatch.c index 41378a5..e71764c 100644 --- a/xen/arch/arm/arm32/livepatch.c +++ b/xen/arch/arm/arm32/livepatch.c @@ -56,7 +56,7 @@ void arch_livepatch_apply(struct livepatch_func *func) else insn = 0xe1a00000; /* mov r0, r0 */ - new_ptr = func->old_addr - (void *)_start + vmap_of_xen_text; + new_ptr = func->old_addr - (void *)__symbol(_start) + vmap_of_xen_text; len = len / sizeof(uint32_t); /* PATCH! */ diff --git a/xen/arch/arm/arm64/livepatch.c b/xen/arch/arm/arm64/livepatch.c index 2247b92..4a31026 100644 --- a/xen/arch/arm/arm64/livepatch.c +++ b/xen/arch/arm/arm64/livepatch.c @@ -43,7 +43,7 @@ void arch_livepatch_apply(struct livepatch_func *func) /* Verified in livepatch_verify_distance. */ ASSERT(insn != AARCH64_BREAK_FAULT); - new_ptr = func->old_addr - (void *)_start + vmap_of_xen_text; + new_ptr = func->old_addr - (void *)__symbol(_start) + vmap_of_xen_text; len = len / sizeof(uint32_t); /* PATCH! */ diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c index f552154..40968d0 100644 --- a/xen/arch/arm/domain_build.c +++ b/xen/arch/arm/domain_build.c @@ -2090,7 +2090,7 @@ static void __init find_gnttab_region(struct domain *d, * Only use the text section as it's always present and will contain * enough space for a large grant table */ - kinfo->gnttab_start = __pa(_stext); + kinfo->gnttab_start = __pa(__symbol(_stext)); kinfo->gnttab_size = gnttab_dom0_frames() << PAGE_SHIFT; #ifdef CONFIG_ARM_32 diff --git a/xen/arch/arm/livepatch.c b/xen/arch/arm/livepatch.c index 279d52c..006dff8 100644 --- a/xen/arch/arm/livepatch.c +++ b/xen/arch/arm/livepatch.c @@ -26,8 +26,8 @@ int arch_livepatch_quiesce(void) if ( vmap_of_xen_text ) return -EINVAL; - text_mfn = virt_to_mfn(_start); - text_order = get_order_from_bytes(_end - _start); + text_mfn = virt_to_mfn(__symbol(_start)); + text_order = get_order_from_bytes(__symbol(_end) - __symbol(_start)); /* * The text section is read-only. So re-map Xen to be able to patch @@ -78,7 +78,7 @@ void arch_livepatch_revert(const struct livepatch_func *func) uint32_t *new_ptr; unsigned int len; - new_ptr = func->old_addr - (void *)_start + vmap_of_xen_text; + new_ptr = func->old_addr - (void *)__symbol(_start) + vmap_of_xen_text; len = livepatch_insn_len(func); memcpy(new_ptr, func->opaque, len); diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c index 7a06a33..210b25e 100644 --- a/xen/arch/arm/mm.c +++ b/xen/arch/arm/mm.c @@ -620,7 +620,7 @@ void __init setup_pagetables(unsigned long boot_phys_offset, paddr_t xen_paddr) int i; /* Calculate virt-to-phys offset for the new location */ - phys_offset = xen_paddr - (unsigned long) _start; + phys_offset = xen_paddr - (unsigned long) __symbol(_start); #ifdef CONFIG_ARM_64 p = (void *) xen_pgtable; @@ -681,7 +681,8 @@ void __init setup_pagetables(unsigned long boot_phys_offset, paddr_t xen_paddr) ttbr = (uintptr_t) cpu0_pgtable + phys_offset; #endif - relocate_xen(ttbr, _start, (void*)dest_va, _end - _start); + relocate_xen(ttbr, __symbol(_start), (void*)dest_va, + __symbol(_end) - __symbol(_start)); /* Clear the copy of the boot pagetables. Each secondary CPU * rebuilds these itself (see head.S) */ @@ -1100,8 +1101,8 @@ static void set_pte_flags_on_range(const char *p, unsigned long l, enum mg mg) ASSERT(!((unsigned long) p & ~PAGE_MASK)); ASSERT(!(l & ~PAGE_MASK)); - for ( i = (p - _start) / PAGE_SIZE; - i < (p + l - _start) / PAGE_SIZE; + for ( i = (p - __symbol(_start)) / PAGE_SIZE; + i < (p + l - __symbol(_start)) / PAGE_SIZE; i++ ) { pte = xen_xenmap[i]; diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c index ea2495a..9d0b915 100644 --- a/xen/arch/arm/setup.c +++ b/xen/arch/arm/setup.c @@ -394,7 +394,8 @@ static paddr_t __init get_xen_paddr(void) paddr_t paddr = 0; int i; - min_size = (_end - _start + (XEN_PADDR_ALIGN-1)) & ~(XEN_PADDR_ALIGN-1); + min_size = (__symbol(_end) - __symbol(_start) + + (XEN_PADDR_ALIGN-1)) & ~(XEN_PADDR_ALIGN-1); /* Find the highest bank with enough space. */ for ( i = 0; i < mi->nr_banks; i++ ) @@ -727,8 +728,9 @@ void __init start_xen(unsigned long boot_phys_offset, /* Register Xen's load address as a boot module. */ xen_bootmodule = add_boot_module(BOOTMOD_XEN, - (paddr_t)(uintptr_t)(_start + boot_phys_offset), - (paddr_t)(uintptr_t)(_end - _start + 1), NULL); + (paddr_t)(uintptr_t)(__symbol(_start) + boot_phys_offset), + (paddr_t)(uintptr_t)(__symbol(_end) - + __symbol(_start) + 1), NULL); BUG_ON(!xen_bootmodule); xen_paddr = get_xen_paddr(); diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c index 93b79c7..0a7d6c0 100644 --- a/xen/arch/x86/setup.c +++ b/xen/arch/x86/setup.c @@ -578,13 +578,13 @@ static void __init kexec_reserve_area(struct e820map *e820) static inline bool using_2M_mapping(void) { - return !l1_table_offset((unsigned long)__2M_text_end) && - !l1_table_offset((unsigned long)__2M_rodata_start) && - !l1_table_offset((unsigned long)__2M_rodata_end) && - !l1_table_offset((unsigned long)__2M_init_start) && - !l1_table_offset((unsigned long)__2M_init_end) && - !l1_table_offset((unsigned long)__2M_rwdata_start) && - !l1_table_offset((unsigned long)__2M_rwdata_end); + return !l1_table_offset((unsigned long)__symbol(__2M_text_end)) && + !l1_table_offset((unsigned long)__symbol(__2M_rodata_start)) && + !l1_table_offset((unsigned long)__symbol(__2M_rodata_end)) && + !l1_table_offset((unsigned long)__symbol(__2M_init_start)) && + !l1_table_offset((unsigned long)__symbol(__2M_init_end)) && + !l1_table_offset((unsigned long)__symbol(__2M_rwdata_start)) && + !l1_table_offset((unsigned long)__symbol(__2M_rwdata_end)); } static void noinline init_done(void) @@ -606,13 +606,13 @@ static void noinline init_done(void) /* Destroy Xen's mappings, and reuse the pages. */ if ( using_2M_mapping() ) { - start = (unsigned long)&__2M_init_start, - end = (unsigned long)&__2M_init_end; + start = (unsigned long)__symbol(&__2M_init_start), + end = (unsigned long)__symbol(&__2M_init_end); } else { - start = (unsigned long)&__init_begin; - end = (unsigned long)&__init_end; + start = (unsigned long)__symbol(&__init_begin); + end = (unsigned long)__symbol(&__init_end); } destroy_xen_mappings(start, end); @@ -966,8 +966,8 @@ void __init noreturn __start_xen(unsigned long mbi_p) * This needs to remain in sync with xen_in_range() and the * respective reserve_e820_ram() invocation below. */ - mod[mbi->mods_count].mod_start = virt_to_mfn(_stext); - mod[mbi->mods_count].mod_end = __2M_rwdata_end - _stext; + mod[mbi->mods_count].mod_start = virt_to_mfn(__symbol(_stext)); + mod[mbi->mods_count].mod_end = __symbol(__2M_rwdata_end) - __symbol(_stext); } modules_headroom = bzimage_headroom(bootstrap_map(mod), mod->mod_end); @@ -1018,7 +1018,7 @@ void __init noreturn __start_xen(unsigned long mbi_p) 1UL << (PAGE_SHIFT + 32)) ) e = min(HYPERVISOR_VIRT_END - DIRECTMAP_VIRT_START, 1UL << (PAGE_SHIFT + 32)); -#define reloc_size ((__pa(__2M_rwdata_end) + mask) & ~mask) +#define reloc_size ((__pa(__symbol(__2M_rwdata_end)) + mask) & ~mask) /* Is the region suitable for relocating Xen? */ if ( !xen_phys_start && e <= limit ) { @@ -1034,7 +1034,8 @@ void __init noreturn __start_xen(unsigned long mbi_p) * Is the region size greater than zero and does it begin * at or above the end of current Xen image placement? */ - if ( (end > s) && (end - reloc_size + XEN_IMG_OFFSET >= __pa(_end)) ) + if ( (end > s) && (end - reloc_size + XEN_IMG_OFFSET >= + __pa(__symbol(_end))) ) { l4_pgentry_t *pl4e; l3_pgentry_t *pl3e; @@ -1062,7 +1063,8 @@ void __init noreturn __start_xen(unsigned long mbi_p) * data until after we have switched to the relocated pagetables! */ barrier(); - move_memory(e + XEN_IMG_OFFSET, XEN_IMG_OFFSET, _end - _start, 1); + move_memory(e + XEN_IMG_OFFSET, XEN_IMG_OFFSET, + __symbol(_end) - __symbol(_start), 1); /* Walk initial pagetables, relocating page directory entries. */ pl4e = __va(__pa(idle_pg_table)); @@ -1103,8 +1105,8 @@ void __init noreturn __start_xen(unsigned long mbi_p) * is contained in this PTE. */ BUG_ON(using_2M_mapping() && - l2_table_offset((unsigned long)_erodata) == - l2_table_offset((unsigned long)_stext)); + l2_table_offset((unsigned long)__symbol(_erodata)) == + l2_table_offset((unsigned long)__symbol(_stext))); *pl2e++ = l2e_from_pfn(xen_phys_start >> PAGE_SHIFT, PAGE_HYPERVISOR_RX | _PAGE_PSE); for ( i = 1; i < L2_PAGETABLE_ENTRIES; i++, pl2e++ ) @@ -1122,22 +1124,22 @@ void __init noreturn __start_xen(unsigned long mbi_p) continue; } - if ( i < l2_table_offset((unsigned long)&__2M_text_end) ) + if ( i < l2_table_offset((unsigned long)__symbol(&__2M_text_end)) ) { flags = PAGE_HYPERVISOR_RX | _PAGE_PSE; } - else if ( i >= l2_table_offset((unsigned long)&__2M_rodata_start) && - i < l2_table_offset((unsigned long)&__2M_rodata_end) ) + else if ( i >= l2_table_offset((unsigned long)__symbol(&__2M_rodata_start)) && + i < l2_table_offset((unsigned long)__symbol(&__2M_rodata_end)) ) { flags = PAGE_HYPERVISOR_RO | _PAGE_PSE; } - else if ( i >= l2_table_offset((unsigned long)&__2M_init_start) && - i < l2_table_offset((unsigned long)&__2M_init_end) ) + else if ( i >= l2_table_offset((unsigned long)__symbol(&__2M_init_start)) && + i < l2_table_offset((unsigned long)__symbol(&__2M_init_end)) ) { flags = PAGE_HYPERVISOR_RWX | _PAGE_PSE; } - else if ( (i >= l2_table_offset((unsigned long)&__2M_rwdata_start) && - i < l2_table_offset((unsigned long)&__2M_rwdata_end)) ) + else if ( (i >= l2_table_offset((unsigned long)__symbol(&__2M_rwdata_start)) && + i < l2_table_offset((unsigned long)__symbol(&__2M_rwdata_end))) ) { flags = PAGE_HYPERVISOR_RW | _PAGE_PSE; } @@ -1234,7 +1236,7 @@ void __init noreturn __start_xen(unsigned long mbi_p) panic("Not enough memory to relocate Xen\n"); /* This needs to remain in sync with xen_in_range(). */ - reserve_e820_ram(&boot_e820, __pa(_stext), __pa(__2M_rwdata_end)); + reserve_e820_ram(&boot_e820, __pa(__symbol(_stext)), __pa(__symbol(__2M_rwdata_end))); /* Late kexec reservation (dynamic start address). */ kexec_reserve_area(&boot_e820); @@ -1377,7 +1379,8 @@ void __init noreturn __start_xen(unsigned long mbi_p) } #endif - xen_virt_end = ((unsigned long)_end + (1UL << L2_PAGETABLE_SHIFT) - 1) & + xen_virt_end = ((unsigned long)__symbol(_end) + + (1UL << L2_PAGETABLE_SHIFT) - 1) & ~((1UL << L2_PAGETABLE_SHIFT) - 1); destroy_xen_mappings(xen_virt_end, XEN_VIRT_START + BOOTSTRAP_MAP_BASE); @@ -1390,22 +1393,22 @@ void __init noreturn __start_xen(unsigned long mbi_p) { /* Mark .text as RX (avoiding the first 2M superpage). */ modify_xen_mappings(XEN_VIRT_START + MB(2), - (unsigned long)&__2M_text_end, + (unsigned long)__symbol(&__2M_text_end), PAGE_HYPERVISOR_RX); /* Mark .rodata as RO. */ - modify_xen_mappings((unsigned long)&__2M_rodata_start, - (unsigned long)&__2M_rodata_end, + modify_xen_mappings((unsigned long)__symbol(&__2M_rodata_start), + (unsigned long)__symbol(&__2M_rodata_end), PAGE_HYPERVISOR_RO); /* Mark .data and .bss as RW. */ - modify_xen_mappings((unsigned long)&__2M_rwdata_start, - (unsigned long)&__2M_rwdata_end, + modify_xen_mappings((unsigned long)__symbol(&__2M_rwdata_start), + (unsigned long)__symbol(&__2M_rwdata_end), PAGE_HYPERVISOR_RW); /* Drop the remaining mappings in the shattered superpage. */ - destroy_xen_mappings((unsigned long)&__2M_rwdata_end, - ROUNDUP((unsigned long)&__2M_rwdata_end, MB(2))); + destroy_xen_mappings((unsigned long)__symbol(&__2M_rwdata_end), + ROUNDUP((unsigned long)__symbol(&__2M_rwdata_end), MB(2))); } nr_pages = 0; @@ -1860,11 +1863,11 @@ int __hwdom_init xen_in_range(unsigned long mfn) */ /* hypervisor .text + .rodata */ - xen_regions[region_ro].s = __pa(&_stext); - xen_regions[region_ro].e = __pa(&__2M_rodata_end); + xen_regions[region_ro].s = __pa(__symbol(&_stext)); + xen_regions[region_ro].e = __pa(__symbol(&__2M_rodata_end)); /* hypervisor .data + .bss */ - xen_regions[region_rw].s = __pa(&__2M_rwdata_start); - xen_regions[region_rw].e = __pa(&__2M_rwdata_end); + xen_regions[region_rw].s = __pa(__symbol(&__2M_rwdata_start)); + xen_regions[region_rw].e = __pa(__symbol(&__2M_rwdata_end)); } start = (paddr_t)mfn << PAGE_SHIFT; diff --git a/xen/arch/x86/tboot.c b/xen/arch/x86/tboot.c index f3fdee4..032bf8e 100644 --- a/xen/arch/x86/tboot.c +++ b/xen/arch/x86/tboot.c @@ -373,13 +373,13 @@ void tboot_shutdown(uint32_t shutdown_type) g_tboot_shared->mac_regions[0].size = bootsym_phys(trampoline_end) - bootsym_phys(trampoline_start); /* hypervisor .text + .rodata */ - g_tboot_shared->mac_regions[1].start = (uint64_t)__pa(&_stext); - g_tboot_shared->mac_regions[1].size = __pa(&__2M_rodata_end) - - __pa(&_stext); + g_tboot_shared->mac_regions[1].start = (uint64_t)__pa(__symbol(&_stext)); + g_tboot_shared->mac_regions[1].size = __pa(__symbol(&__2M_rodata_end)) - + __pa(__symbol(&_stext)); /* hypervisor .data + .bss */ - g_tboot_shared->mac_regions[2].start = (uint64_t)__pa(&__2M_rwdata_start); - g_tboot_shared->mac_regions[2].size = __pa(&__2M_rwdata_end) - - __pa(&__2M_rwdata_start); + g_tboot_shared->mac_regions[2].start = (uint64_t)__pa(__symbol(&__2M_rwdata_start)); + g_tboot_shared->mac_regions[2].size = __pa(__symbol(&__2M_rwdata_end)) - + __pa(__symbol(&__2M_rwdata_start)); /* * MAC domains and other Xen memory diff --git a/xen/arch/x86/x86_64/machine_kexec.c b/xen/arch/x86/x86_64/machine_kexec.c index f4a005c..7840761 100644 --- a/xen/arch/x86/x86_64/machine_kexec.c +++ b/xen/arch/x86/x86_64/machine_kexec.c @@ -13,8 +13,8 @@ int machine_kexec_get_xen(xen_kexec_range_t *range) { - range->start = virt_to_maddr(_start); - range->size = virt_to_maddr(_end) - (unsigned long)range->start; + range->start = virt_to_maddr(__symbol(_start)); + range->size = virt_to_maddr(__symbol(_end)) - (unsigned long)range->start; return 0; } diff --git a/xen/include/asm-arm/grant_table.h b/xen/include/asm-arm/grant_table.h index 37415b7..865a073 100644 --- a/xen/include/asm-arm/grant_table.h +++ b/xen/include/asm-arm/grant_table.h @@ -31,7 +31,8 @@ void gnttab_mark_dirty(struct domain *d, mfn_t mfn); * enough space for a large grant table */ #define gnttab_dom0_frames() \ - min_t(unsigned int, opt_max_grant_frames, PFN_DOWN(_etext - _stext)) + min_t(unsigned int, opt_max_grant_frames, \ + PFN_DOWN(__symbol(_etext) - __symbol(_stext))) #define gnttab_init_arch(gt) \ ({ \ diff --git a/xen/include/asm-arm/mm.h b/xen/include/asm-arm/mm.h index 02ce05a..fd8e439 100644 --- a/xen/include/asm-arm/mm.h +++ b/xen/include/asm-arm/mm.h @@ -151,8 +151,8 @@ extern vaddr_t xenheap_virt_start; #endif #define is_xen_fixed_mfn(mfn) \ - ((pfn_to_paddr(mfn) >= virt_to_maddr(&_start)) && \ - (pfn_to_paddr(mfn) <= virt_to_maddr(&_end))) + ((pfn_to_paddr(mfn) >= virt_to_maddr(__symbol(&_start))) && \ + (pfn_to_paddr(mfn) <= virt_to_maddr(__symbol(&_end)))) #define page_get_owner(_p) (_p)->v.inuse.domain #define page_set_owner(_p,_d) ((_p)->v.inuse.domain = (_d)) diff --git a/xen/include/asm-x86/mm.h b/xen/include/asm-x86/mm.h index 6e45651..b49d00c 100644 --- a/xen/include/asm-x86/mm.h +++ b/xen/include/asm-x86/mm.h @@ -273,8 +273,8 @@ struct page_info #define is_xen_heap_mfn(mfn) \ (__mfn_valid(mfn) && is_xen_heap_page(mfn_to_page(_mfn(mfn)))) #define is_xen_fixed_mfn(mfn) \ - ((((mfn) << PAGE_SHIFT) >= __pa(&_stext)) && \ - (((mfn) << PAGE_SHIFT) <= __pa(&__2M_rwdata_end))) + ((((mfn) << PAGE_SHIFT) >= __pa(__symbol(&_stext))) && \ + (((mfn) << PAGE_SHIFT) <= __pa(__symbol(&__2M_rwdata_end)))) #define PRtype_info "016lx"/* should only be used for printk's */ diff --git a/xen/include/xen/kernel.h b/xen/include/xen/kernel.h index 548b64d..0d62300 100644 --- a/xen/include/xen/kernel.h +++ b/xen/include/xen/kernel.h @@ -68,25 +68,25 @@ extern char _start[], _end[], start[]; #define is_kernel(p) ({ \ char *__p = (char *)(unsigned long)(p); \ - (__p >= _start) && (__p < _end); \ + (__p >= __symbol(_start)) && (__p < __symbol(_end)); \ }) extern char _stext[], _etext[]; #define is_kernel_text(p) ({ \ char *__p = (char *)(unsigned long)(p); \ - (__p >= _stext) && (__p < _etext); \ + (__p >= __symbol(_stext)) && (__p < __symbol(_etext)); \ }) extern const char _srodata[], _erodata[]; #define is_kernel_rodata(p) ({ \ const char *__p = (const char *)(unsigned long)(p); \ - (__p >= _srodata) && (__p < _erodata); \ + (__p >= __symbol(_srodata)) && (__p < __symbol(_erodata)); \ }) extern char _sinittext[], _einittext[]; #define is_kernel_inittext(p) ({ \ char *__p = (char *)(unsigned long)(p); \ - (__p >= _sinittext) && (__p < _einittext); \ + (__p >= __symbol(_sinittext)) && (__p < __symbol(_einittext)); \ }) extern enum system_state { -- 1.9.1 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH 4/4] xen: use __symbol everywhere 2018-10-15 9:56 ` [PATCH 4/4] xen: use __symbol everywhere Stefano Stabellini @ 2018-10-25 10:39 ` Jan Beulich 2018-11-06 21:47 ` Stefano Stabellini 0 siblings, 1 reply; 19+ messages in thread From: Jan Beulich @ 2018-10-25 10:39 UTC (permalink / raw) To: Stefano Stabellini; +Cc: xen-devel, Julien Grall, Stefano Stabellini >>> On 15.10.18 at 11:56, <sstabellini@kernel.org> wrote: > Use __symbol everywhere _stext, _etext, etc. are used. Technically, it > is only required when comparing pointers, but use it everywhere to avoid > confusion. While the description slightly limits the scope, I'm not sure that's what you mean. What about e.g. __{start,end}_vpci_array? If you say "everywhere" in the title, then please change everything. Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 4/4] xen: use __symbol everywhere 2018-10-25 10:39 ` Jan Beulich @ 2018-11-06 21:47 ` Stefano Stabellini 0 siblings, 0 replies; 19+ messages in thread From: Stefano Stabellini @ 2018-11-06 21:47 UTC (permalink / raw) To: Jan Beulich Cc: xen-devel, Julien Grall, Stefano Stabellini, Stefano Stabellini On Thu, 25 Oct 2018, Jan Beulich wrote: > >>> On 15.10.18 at 11:56, <sstabellini@kernel.org> wrote: > > Use __symbol everywhere _stext, _etext, etc. are used. Technically, it > > is only required when comparing pointers, but use it everywhere to avoid > > confusion. > > While the description slightly limits the scope, I'm not sure that's what > you mean. What about e.g. __{start,end}_vpci_array? If you say > "everywhere" in the title, then please change everything. I forgot about that one, I'll fix it _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2018-11-06 21:47 UTC | newest] Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-10-15 9:55 [PATCH 0/4] misc safety certification fixes Stefano Stabellini 2018-10-15 9:56 ` [PATCH 1/4] xen/arm: initialize target Stefano Stabellini 2018-10-15 13:07 ` Julien Grall 2018-10-15 9:56 ` [PATCH 2/4] xen/arm: initialize access Stefano Stabellini 2018-10-15 13:08 ` Julien Grall 2018-10-15 15:15 ` Tamas K Lengyel 2018-10-16 1:14 ` Stefano Stabellini 2018-10-16 15:24 ` Tamas K Lengyel 2018-10-17 13:53 ` Stefano Stabellini 2018-10-15 9:56 ` [PATCH 3/4] xen: introduce __symbol Stefano Stabellini 2018-10-15 12:20 ` Andrew Cooper 2018-10-15 13:20 ` Julien Grall 2018-10-16 1:34 ` Stefano Stabellini 2018-10-16 10:06 ` Andrew Cooper 2018-10-16 13:01 ` Stefano Stabellini 2018-10-25 10:37 ` Jan Beulich 2018-10-15 9:56 ` [PATCH 4/4] xen: use __symbol everywhere Stefano Stabellini 2018-10-25 10:39 ` Jan Beulich 2018-11-06 21:47 ` Stefano Stabellini
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.