Re: [PATCH v3 3/4] xen/arm: mm: Rename xenheap_* variable to directmap_*

[Julien Grall <julien@xxxxxxx>]

On 07/09/2022 12:09, Henry Wang wrote:
> Hi Julien,
Hi Henry,

> > -----Original Message-----
> > From: Julien Grall <julien@xxxxxxx>
> > > > > @@ -609,7 +609,7 @@ void __init setup_xenheap_mappings(unsigned
> > > > long base_mfn,
> > > >
> > > > I think the function also want to be renamed to match the code below.
> > > Hmmm, renaming the name to "setup_directmap_mappings" would
> > > somehow lead me to think of we are getting rid of the name "xenheap"
> > > completely in the code, which seems a little bit scary to me...
> > >
> > > But I just checked there is a comment
> > > "/* Set up the xenheap: up to 1GB of contiguous, always-mapped
> > memory."
> > > above the function and the declaration so I guess we are fine?
> > We are not getting rid of "xenheap". In fact the common code will
> > continue to use the concept.
> Ack.
>
> > What we make clear is this function is not only here to map the xenheap
> > but other memory (e.g. static domain memory on arm64).
> In that case I think the comment in function declaration (attached below)
> ```
> /* Set up the xenheap: up to 1GB of contiguous, always-mapped memory.
>   * Base must be 32MB aligned and size a multiple of 32MB. */
> extern void setup_xenheap_mappings(unsigned long base_mfn, unsigned long 
> nr_mfns);
> ```
> would also need changes, as I think it only refers to the Arm32.
>
> How about
> /*
>   * For Arm32, set up the xenheap: up to 1GB of contiguous,
>   * always-mapped memory. Base must be 32MB aligned and size
>   * a multiple of 32MB.
>   * For Arm64, set up the directmap area of memory.
One remark. I would say: "map the region in the directmap area"

Cheers,

--
Julien Grall