| Message ID | 20250303232941.2641306-2-andrew.cooper3@citrix.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | xen/watchdog: Identify which domain watchdog fired | expand |
On Mon, 3 Mar 2025, Andrew Cooper wrote: > struct domain is always a page aligned allocation. Update it's type to > reflect this, so we can safely reuse the lower bits in the pointer for > auxiliary information. > > No functional change. > > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org> > --- > CC: Anthony PERARD <anthony.perard@vates.tech> > CC: Michal Orzel <michal.orzel@amd.com> > CC: Jan Beulich <jbeulich@suse.com> > CC: Julien Grall <julien@xen.org> > CC: Roger Pau Monné <roger.pau@citrix.com> > CC: Stefano Stabellini <sstabellini@kernel.org> > CC: Dario Faggioli <dfaggioli@suse.com> > CC: Juergen Gross <jgross@suse.com> > CC: George Dunlap <gwd@xenproject.org> > > v2: > * New > > Interestingly this does cause two changes in the resulting binary, both caused > by GCC electing to use a more complicated addressing mode to save one ADD > instruction. > --- > xen/include/xen/sched.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h > index 037c83fda219..8412b45178ca 100644 > --- a/xen/include/xen/sched.h > +++ b/xen/include/xen/sched.h > @@ -645,7 +645,7 @@ struct domain > unsigned int num_llc_colors; > const unsigned int *llc_colors; > #endif > -}; > +} __aligned(PAGE_SIZE); > > static inline struct page_list_head *page_to_list( > struct domain *d, const struct page_info *pg) > -- > 2.39.5 >
On 04.03.2025 00:29, Andrew Cooper wrote: > struct domain is always a page aligned allocation. Update it's type to > reflect this, so we can safely reuse the lower bits in the pointer for > auxiliary information. > > No functional change. > > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> > --- > CC: Anthony PERARD <anthony.perard@vates.tech> > CC: Michal Orzel <michal.orzel@amd.com> > CC: Jan Beulich <jbeulich@suse.com> > CC: Julien Grall <julien@xen.org> > CC: Roger Pau Monné <roger.pau@citrix.com> > CC: Stefano Stabellini <sstabellini@kernel.org> > CC: Dario Faggioli <dfaggioli@suse.com> > CC: Juergen Gross <jgross@suse.com> > CC: George Dunlap <gwd@xenproject.org> > > v2: > * New > > Interestingly this does cause two changes in the resulting binary, both caused > by GCC electing to use a more complicated addressing mode to save one ADD > instruction. That's on x86, I suppose? > --- a/xen/include/xen/sched.h > +++ b/xen/include/xen/sched.h > @@ -645,7 +645,7 @@ struct domain > unsigned int num_llc_colors; > const unsigned int *llc_colors; > #endif > -}; > +} __aligned(PAGE_SIZE); > > static inline struct page_list_head *page_to_list( > struct domain *d, const struct page_info *pg) I understand struct domain is where you need the annotation right away, but is there a reason not to do the same for struct vcpu right away? Jan
On 05/03/2025 9:23 am, Jan Beulich wrote: > On 04.03.2025 00:29, Andrew Cooper wrote: >> struct domain is always a page aligned allocation. Update it's type to >> reflect this, so we can safely reuse the lower bits in the pointer for >> auxiliary information. >> >> No functional change. >> >> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> >> --- >> CC: Anthony PERARD <anthony.perard@vates.tech> >> CC: Michal Orzel <michal.orzel@amd.com> >> CC: Jan Beulich <jbeulich@suse.com> >> CC: Julien Grall <julien@xen.org> >> CC: Roger Pau Monné <roger.pau@citrix.com> >> CC: Stefano Stabellini <sstabellini@kernel.org> >> CC: Dario Faggioli <dfaggioli@suse.com> >> CC: Juergen Gross <jgross@suse.com> >> CC: George Dunlap <gwd@xenproject.org> >> >> v2: >> * New >> >> Interestingly this does cause two changes in the resulting binary, both caused >> by GCC electing to use a more complicated addressing mode to save one ADD >> instruction. > That's on x86, I suppose? Yes. > >> --- a/xen/include/xen/sched.h >> +++ b/xen/include/xen/sched.h >> @@ -645,7 +645,7 @@ struct domain >> unsigned int num_llc_colors; >> const unsigned int *llc_colors; >> #endif >> -}; >> +} __aligned(PAGE_SIZE); >> >> static inline struct page_list_head *page_to_list( >> struct domain *d, const struct page_info *pg) > I understand struct domain is where you need the annotation right away, but is > there a reason not to do the same for struct vcpu right away? struct vcpu is more complicated. It's multi-page on ARM, and I have a strong suspicion that alignment is going to have to change away from PAGE_SIZE for architectures wanting to use larger page sizes. e.g. with 64k pagetables, I expect these will still want 4k alignment, and therefore they'll all want changing. i.e. I'm probably creating less work for someone in the future by not annotating struct vcpu. ~Andrew
On 06.03.2025 22:29, Andrew Cooper wrote: > On 05/03/2025 9:23 am, Jan Beulich wrote: >> On 04.03.2025 00:29, Andrew Cooper wrote: >>> --- a/xen/include/xen/sched.h >>> +++ b/xen/include/xen/sched.h >>> @@ -645,7 +645,7 @@ struct domain >>> unsigned int num_llc_colors; >>> const unsigned int *llc_colors; >>> #endif >>> -}; >>> +} __aligned(PAGE_SIZE); >>> >>> static inline struct page_list_head *page_to_list( >>> struct domain *d, const struct page_info *pg) >> I understand struct domain is where you need the annotation right away, but is >> there a reason not to do the same for struct vcpu right away? > > struct vcpu is more complicated. It's multi-page on ARM, and I have a > strong suspicion that alignment is going to have to change away from > PAGE_SIZE for architectures wanting to use larger page sizes. > > e.g. with 64k pagetables, I expect these will still want 4k alignment, > and therefore they'll all want changing. i.e. I'm probably creating less > work for someone in the future by not annotating struct vcpu. ~Andrew Wouldn't the same hold for struct domain? Jan
diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h index 037c83fda219..8412b45178ca 100644 --- a/xen/include/xen/sched.h +++ b/xen/include/xen/sched.h @@ -645,7 +645,7 @@ struct domain unsigned int num_llc_colors; const unsigned int *llc_colors; #endif -}; +} __aligned(PAGE_SIZE); static inline struct page_list_head *page_to_list( struct domain *d, const struct page_info *pg)
struct domain is always a page aligned allocation. Update it's type to reflect this, so we can safely reuse the lower bits in the pointer for auxiliary information. No functional change. Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> --- CC: Anthony PERARD <anthony.perard@vates.tech> CC: Michal Orzel <michal.orzel@amd.com> CC: Jan Beulich <jbeulich@suse.com> CC: Julien Grall <julien@xen.org> CC: Roger Pau Monné <roger.pau@citrix.com> CC: Stefano Stabellini <sstabellini@kernel.org> CC: Dario Faggioli <dfaggioli@suse.com> CC: Juergen Gross <jgross@suse.com> CC: George Dunlap <gwd@xenproject.org> v2: * New Interestingly this does cause two changes in the resulting binary, both caused by GCC electing to use a more complicated addressing mode to save one ADD instruction. --- xen/include/xen/sched.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)