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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 04.12.2020 10:29, Julien Grall wrote:
> 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?

I've added a paragraph.

>>>> --- /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?

Hmm, yes, why not. I guess I'd slightly prefer the latter.

Jan