Re: [PATCH 2/2] mm: split out mfn_t / gfn_t / pfn_t definitions and helpers

[Julien Grall <julien@xxxxxxx>]

Hi Jan,

On 03/12/2020 09:39, Jan Beulich wrote:
> On 02.12.2020 18:35, Julien Grall wrote:
> > On 02/12/2020 14:50, Jan Beulich wrote:
> > > xen/mm.h has heavy dependencies, while in a number of cases only these
> > > type definitions are needed. This separation then also allows pulling in
> > > these definitions when including xen/mm.h would cause cyclic
> > > dependencies.
> > >
> > > Replace xen/mm.h inclusion where possible in include/xen/. (In
> > > xen/iommu.h also take the opportunity and correct the few remaining
> > > sorting issues.)
> > >
> > > Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> > >
> > > --- a/xen/arch/x86/acpi/power.c
> > > +++ b/xen/arch/x86/acpi/power.c
> > > @@ -10,7 +10,6 @@
> > >     * Slimmed with Xen specific support.
> > >     */
> > >    
> > > -#include <asm/io.h>
> > This seems to be unrelated of this work.
> Well spotted, but the answer really is "yes and no". My first
> attempt at fixing build issues from this and similar asm/io.h
> inclusions was to remove such unnecessary ones. But this didn't
> work out - I had to fix the header instead. If you think this
> extra cleanup really does any harm here, I can drop it. But I'd
> prefer to keep it.
I am fine with keeping it here. Can you mention it in the commit message?

> > > --- /dev/null
> > > +++ b/xen/include/xen/frame-num.h
> > It would feel more natural to me if the file is named mm-types.h.
> Indeed I was first meaning to use this name (not the least
> because I don't particularly like the one chosen, but I also
> couldn't think of a better one). However, then things like
> struct page_info would imo also belong there (more precisely in
> asm/mm-types.h to be included from here), which is specifically
> something I want to avoid. Yes, eventually we may (I'm inclined
> to even say "will") want such a header, but I still want to
> keep these even more fundamental types in a separate one.
> Otherwise we'll again end up with files including mm-types.h
> just because of needing e.g. gfn_t for a function declaration.
> (Note that the same isn't the case for struct page_info, which
> can simply be forward declared.)
Thanks for the explanation. AFAICT, this file will mostly contain 
typesafe for MM. So how about naming it to mm-typesafe.h? Or maybe 
mm-frame.h?
Cheers,

--
Julien Grall