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

[Julien Grall <julien.grall@xxxxxxx>]

Hi Jan,

On 15/05/2017 08:20, Jan Beulich wrote:
> > > > On 12.05.17 at 17:34, <JBeulich@xxxxxxxx> wrote:
> > > > > On 12.05.17 at 17:25, <olekstysh@xxxxxxxxx> wrote:
> > > On Fri, May 12, 2017 at 5:41 PM, Jan Beulich <JBeulich@xxxxxxxx> wrote:
> > > > > > > On 10.05.17 at 16:03, <olekstysh@xxxxxxxxx> wrote:
> > > > > The "retrieving mapping" code has never executed since
> > > > > iommu_use_hap_pt(d) always returned true on ARM so far. But, with
> > > > > introducing the non-shared IOMMU patch series we can no longer keep
> > > > > this code as is due to the lack of M2P support.
> > > > >
> > > > > In order to retain the current behavior for x86 this code was completely
> > > > > moved to x86 specific part.
> > > > > For ARM we just need to populate IOMMU page table if need_iommu flag
> > > > > is already set and the IOMMU is non-shared.
> > > > >
> > > > > So, the logic on ARM was changed a bit, but no functional change for x86.
> > > > >
> > > > > Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@xxxxxxxx>
> > > > > CC: Jan Beulich <jbeulich@xxxxxxxx>
> > > > > CC: Julien Grall <julien.grall@xxxxxxx>
> > > > >
> > > > > ---
> > > > >    Changes in V1:
> > > > >       - Clarify patch description.
> > > > My prior objection stands: You don't make clear why architecture-
> > > > agnostic code needs to be moved into an architecture-specific file.
> > > Is the following description more cleaner?
> > >
> > > Although this code looks like architecture-agnostic code, there is
> > > only one thing that prevents it
> > > to be *completely* architecture-agnostic -> mfn_to_gmfn helper.
> > > As we don't have an M2P on ARM, it always returns incoming mfn.
> > > And if domain is not direct mapped we will get into the trouble using that.
> > >
> > > We didn't care about this code before because it has never been executed
> > > (iommu_use_hap_pt(d) always returned true on ARM so far).
> > > But, with introducing the non-shared IOMMU patch series we can no longer keep
> > > this code as is since it will be executed for non-shared IOMMU.
> > > So, move this code to x86-specific file in order not to expose a possible bug.
> > Yes, this helps.
> Having thought about this some more, what's still missing is a
> clear explanation why this new need of a non-stub mfn_to_gmfn()
> isn't finally enough of a reason to introduce an M2P on ARM. We
> currently have several uses already which ARM fakes one way or
> another:
> - gnttab_shared_gmfn()
This does not use mfn_to_gmfn on ARM.

> - gnttab_status_gmfn()
gnttab_status_gmfn() returns 0 so far. I have to look at this one.

> - memory_exchange()
Memory exchange does not work on ARM today and will require more work 
than that. When I looked at the code a couple of years ago, it was 
possible to drop the call to mfn_to_gmfn().
> - getdomaininfo()
We could rework to store the gmfn in arch_domain.

> 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.
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.
So the M2P is not a solution on ARM. A better approach is to drop those 
calls from common code and replace by something different (as we did for 
gnttab_shared_mfn).
Cheers,

--
Julien Grall

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