Re: [PATCH v3 15/52] xen: make VMAP only support in MMU system

[Julien Grall <julien@xxxxxxx>]

Hi,

On 28/06/2023 09:41, Penny Zheng wrote:
> On 2023/6/28 15:59, Julien Grall wrote:
> > Hi,
> >
> > On 28/06/2023 06:38, Penny Zheng wrote:
> > > On 2023/6/26 14:00, Jan Beulich wrote:
> > > > On 26.06.2023 05:34, Penny Zheng wrote:
> > > > > --- a/xen/common/vmap.c
> > > > > +++ b/xen/common/vmap.c
> > > > > @@ -331,4 +331,11 @@ void vfree(void *va)
> > > > >       while ( (pg = page_list_remove_head(&pg_list)) != NULL )
> > > > >           free_domheap_page(pg);
> > > > >   }
> > > > > +
> > > > > +void iounmap(void __iomem *va)
> > > > > +{
> > > > > +    unsigned long addr = (unsigned long)(void __force *)va;
> > > > > +
> > > > > +    vunmap((void *)(addr & PAGE_MASK));
> > > > > +}
> > > > Why does this move here?
> > > ioremap/iounmap is using vmap, at least in ARM. So for this more
> > > generic interface, I was intending to implement it on MPU system.
> > > In commit "[PATCH v3 19/52] xen/arm: switch to use ioremap_xxx in 
> > > common file", I'm trying to replace all direct vmap interface with 
> > > ioremap_xxx in common files.
> > While the implementation of ioremap() is based on vmap(), the intended 
> > usage is not the same. ioremap() is for MMIO regions while vmap() is 
> > for RAM.
> > So I don't think this is correct to replace vmap() with ioremap().
> Sure, understood now.
> So then the current usage of ioremap_xxx in xen/arch/arm/kernel.c, like
> ```
> kernel_zimage_load()
> ...
> kernel = ioremap_wc(paddr, len);
> ...
> ```
> ioremap_wc() is used for remapping and then copying guest kernel image 
> to guest memory. Since ioremap_xxx is for MMIO regions, maybe we shall 
> provide a new pair of interfaces for RAM, like ramremap_xxx, to replace 
> the ioremap_xxx here?
I always wondered why we were using ioremap_wc() there. But I have never 
made up my mind for this one. We would need to look at the history to 
understand why it was used.
Anyway, if we want to replace it, then I think this would be a plain 
vmap() rather inventing a new API.
Cheers,

--
Julien Grall