On 03.08.2023 01:02, Shawn Anastasio wrote: > A few files treewide depend on defininitions in headers that they > don't include. This works when arch headers end up including the > required headers by chance, but broke on ppc64 with only minimal/stub > arch headers. > > Signed-off-by: Shawn Anastasio <[email protected]>
I'm okay with the changes in principle, but I'd like to ask a question nevertheless, perhaps also for other REST maintainers (whom you should have Cc-ed, btw) to chime in. > --- a/xen/common/memory.c > +++ b/xen/common/memory.c > @@ -28,6 +28,7 @@ > #include <asm/current.h> > #include <asm/hardirq.h> > #include <asm/p2m.h> > +#include <asm/page.h> > #include <public/memory.h> > #include <xsm/xsm.h> I realize there are several asm/*.h being included here already. Yet generally I think common .c files would better not include any of them directly; only xen/*.h ones should (and even there one might see possible restrictions on what's "legitimate"). Do you recall what it was that's needed from asm/page.h here ... > --- a/xen/common/xmalloc_tlsf.c > +++ b/xen/common/xmalloc_tlsf.c > @@ -27,6 +27,7 @@ > #include <xen/mm.h> > #include <xen/pfn.h> > #include <asm/time.h> > +#include <asm/page.h> ... and here? > --- a/xen/include/xen/domain.h > +++ b/xen/include/xen/domain.h > @@ -4,6 +4,7 @@ > > #include <xen/types.h> > > +#include <public/domctl.h> > #include <public/xen.h> While following our sorting guidelines, this still looks a little odd. We typically would include public/xen.h first, but then almost all other public headers include it anyway. So I'm inclined to suggest to replace (rather than amend) the existing #include here. Then again I wonder why this include is needed. xen/domain.h is effectively included everywhere, yet I would have hoped public/domctl.h isn't. Jan
