Re: [Xen-devel] [PATCH v7 4/6] mm: introduce a helper to get the memory type of a page

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Wed, Aug 22, 2018 at 10:45:34AM +0100, Wei Liu wrote:
> On Wed, Aug 22, 2018 at 09:51:58AM +0200, Roger Pau Monne wrote:
> > Returns all the memory types applicable to a page.
> > 
> > This function is unimplemented for ARM.
> > 
> > Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> > Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
> > ---
> > Changes since v5:
> >  - Return all types that apply to a page, since the types themselves
> >    are flags that can be or'ed together.
> > ---
> > Cc: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> > Cc: Julien Grall <julien.grall@xxxxxxx>
> > Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> > Cc: George Dunlap <George.Dunlap@xxxxxxxxxxxxx>
> > Cc: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
> > Cc: Jan Beulich <jbeulich@xxxxxxxx>
> > Cc: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
> > Cc: Tim Deegan <tim@xxxxxxx>
> > Cc: Wei Liu <wei.liu2@xxxxxxxxxx>
> > ---
> >  xen/arch/x86/mm.c    | 42 ++++++++++++++++++++++++++++++++++++++++++
> >  xen/include/xen/mm.h |  3 +++
> >  2 files changed, 45 insertions(+)
> > 
> > diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
> > index 8ac4412554..fcf4bb0afc 100644
> > --- a/xen/arch/x86/mm.c
> > +++ b/xen/arch/x86/mm.c
> > @@ -430,6 +430,48 @@ int page_is_ram_type(unsigned long mfn, unsigned long 
> > mem_type)
> >      return 0;
> >  }
> >  
> > +unsigned int page_get_ram_type(unsigned long mfn)
> 
> It would be better if you can use type-safe mfn_t here.
> 
> 
> > +{
> > +    uint64_t last = 0, maddr = pfn_to_paddr(mfn);
> 
> And then
> 
>     maddr = mfn_to_maddr(mfn);
> 
> The rest looks good to me.

OK, I haven't done it before because the only caller already uses
unsigned long to deal with mfns, and the function itself will also
end up translating the mfn into unsigned long, so it seemed to me
quite pointless to add this extra translation that doesn't seem to
benefit either the caller or the function itself, but will change it
now.

Thanks, Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel