Re: [Xen-devel] [PATCH v1 08/10] iommu: Split iommu_hwdom_init() into arch specific parts

[Julien Grall <julien.grall@xxxxxxx>]

On 05/17/2017 07:51 PM, Oleksandr Tyshchenko wrote:
> Hi, all.
Hi Oleksandr,

> On Wed, May 17, 2017 at 7:01 PM, Jan Beulich <JBeulich@xxxxxxxx> wrote:
> > > > > On 17.05.17 at 17:45, <olekstysh@xxxxxxxxx> wrote:
> > > On Mon, May 15, 2017 at 3:43 PM, Jan Beulich <JBeulich@xxxxxxxx> wrote:
> > > > > > > On 15.05.17 at 13:45, <julien.grall@xxxxxxx> wrote:
> > > > > On 05/15/2017 09:19 AM, Jan Beulich wrote:
> > > > > > > > > On 15.05.17 at 09:42, <julien.grall@xxxxxxx> wrote:
> > > > > > > On 15/05/2017 08:20, Jan Beulich wrote:
> > > > > > > > With this I think there's quite a bit of justification needed to keep
> > > > > > > > going without M2P on ARM.
> > > > > > > As said in a previous thread, I made a quick calculation, ARM32 supports
> > > > > > > up 40-bit PA and IPA (e.g guest address), which means 28-bits of
> > > > > > > MFN/GFN. The GFN would have to be stored in a 32-bit for alignment, so
> > > > > > > we would need 2^28 * 4 = 1GiB of virtual address space and potentially
> > > > > > > physical memory. We don't have 1GB of VA space free on 32-bit right now.
> > > > > > How come? You don't share address spaces with guests.
> > > > > Below the layout for ARM32:
> > > > >
> > > > >
> > > > >   *   0  -  12M   <COMMON>
> > > > >   *
> > > > >   *  32M - 128M   Frametable: 24 bytes per page for 16GB of RAM
> > > > >   * 256M -   1G   VMAP: ioremap and early_ioremap use this virtual address
> > > > >   *                    space
> > > > >   *
> > > > >   *   1G -   2G   Xenheap: always-mapped memory
> > > > >   *   2G -   4G   Domheap: on-demand-mapped
> > > > Since Domheap hardly covers all memory, the obvious thing would
> > > > seem to be to take part of that region, just like on x86 we also
> > > > had to reduce the direct mapping area in order to support systems
> > > > with more than 5Tb.
> > > >
> > > > > > > ARM64 currently supports up to 48-bit PA and 48-bit IPA, which means
> > > > > > > 36-bits of MFN/GFN. The GFN would have to be stored in 64-bit for
> > > > > > > alignment, so we would need 2^36 * 8 = 512GiB of virtual address space
> > > > > > > and potentially physical memory. While virtual address space is not a
> > > > > > > problem, the memory is a problem for embedded platform. We want Xen to
> > > > > > > be as lean as possible.
> > > > > > You don't need to allocate all 512Gb, the table can be as sparse as
> > > > > > present memory permits.
> > > > > I am aware about that... The main point is reducing the footprint of
> > > > > Xen. Assuming you have a 8GB board, you would have to use 16MB for the M2P.
> > > > >
> > > > > Likely this will increase the footprint of Xen ARM. For what benefits?
> > > > > Avoiding to store few byte in a non-generic way when we need it...
> > > > But that's the point: Generic code becomes more and more clumsy
> > > > if non-generic mechanisms need to be introduced. Quite a few we've
> > > > had the discussion of saving a few Mb here or there, and I've almost
> > > > always been the one to ask for not wasting memory even if we have
> > > > plenty, so I'm all with you on that aspect. Nevertheless there is a
> > > > point where the trade-off between memory overhead and generic
> > > > (i.e. easier to maintain) code crosses a boundary, and I'm simply
> > > > wondering whether we aren't at that point.
> > > Is the lack of M2P support on ARM a blocker for this patch to be accepted?
> > Well, if the ARM maintainers insist on baking their own thing every
> > time we'd use the M2P if it was there, I think I can't reasonably
> > block this patch. Otoh I'd prefer to not see the non-x86-specific
> > code move to x86/, so perhaps the whole patch wants
> > re-structuring using either a manifest definition in the ARM headers
> > or e.g. CONFIG_M2P (which x86 would select, but ARM wouldn't).
> Jan, I am afraid but I didn't get what you meant here:
> "manifest definition in the ARM headers"
I think he meant to have either a define in the header mentioning the 
absence/presence of M2P. But my preference would be using the Kconfig here.
> Julien, Stefano what do you think in general?
My point stands here. The M2P sounds a real waste of memory for a 
limited benefit. There are only a few place in common code that care 
about that and the only one where would potentially really need it (e.g 
iommu_hwdom_init()) can be replaced by allocating page table in advance 
(for justification see my answer on patch #6).
I will take an action to replace the few mfn_to_gmfn by wrapper and 
proper implementation for ARM.
So Jan suggestion for CONFIG_M2P (or maybe CONFIG_HAS_M2P) would be a 
good solution.
Cheers,

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel