Re: [Xen-devel] [PATCHv6 2/3] xen: unify foreign GFN map/unmap for auto-xlated physmap guests

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Fri, 6 Mar 2015, David Vrabel wrote:
> On 06/03/15 17:51, Stefano Stabellini wrote:
> > On Fri, 6 Mar 2015, David Vrabel wrote:
> >> Auto-translated physmap guests (arm, arm64 and x86 PVHVM/PVH) map and
> >> unmap foreign GFNs using the same method (updating the physmap).
> >> Unify the two arm and x86 implementations into one commont one.
> >>
> >> Note that on arm and arm64, the correct error code will be returned
> >> (instead of always -EFAULT) and map/unmap failure warnings are no
> >> longer printed.  These changes are required if the foreign domain is
> >> paging (-ENOENT failures are expected and must be propagated up to the
> >> caller).
> >>
> [...]
> >> --- a/drivers/xen/Kconfig
> >> +++ b/drivers/xen/Kconfig
> >> @@ -253,4 +253,10 @@ config XEN_EFI
> >>    def_bool y
> >>    depends on X86_64 && EFI
> >>  
> >> +config XEN_AUTO_XLATE
> >> +  def_bool y
> >> +  depends on ARM || ARM64 || XEN_PVHVM
> >> +  help
> >> +    Support for auto-translated physmap guests.
> >> +
> >>  endmenu
> > 
> > I think the dependency chain is inverted: in order to enable XEN on ARM
> > and ARM64 or to enable XEN_PVHVM, one needs XEN_AUTO_XLATE.
> > I think that config XEN should select XEN_AUTO_XLATE on ARM and ARM64
> > and config XEN_PVHVM should select XEN_AUTO_XLATE on x86.
> 
> This is a non-visible option that is automatically enabled if required,
> so I'm not sure what you mean here.

OK


> >> +int xen_xlate_unmap_gfn_range(struct vm_area_struct *vma,
> >> +                        int nr, struct page **pages)
> >> +{
> >> +  int i;
> >> +
> >> +  for (i = 0; i < nr; i++) {
> >> +          struct xen_remove_from_physmap xrp;
> >> +          unsigned long pfn;
> >> +
> >> +          pfn = page_to_pfn(pages[i]);
> >> +
> >> +          xrp.domid = DOMID_SELF;
> >> +          xrp.gpfn = pfn;
> >> +          (void)HYPERVISOR_memory_op(XENMEM_remove_from_physmap, &xrp);
> > 
> > Why drop the warning we had before?
> 
> Because map failures (and hence) unmap failures are expected and would
> result in log spam (particularly with an x86 HVM guest).

Ah right, the explanation is already in the commit message. Good.


> >> --- a/include/xen/xen-ops.h
> >> +++ b/include/xen/xen-ops.h
> >> @@ -34,6 +34,16 @@ int xen_remap_domain_mfn_range(struct vm_area_struct 
> >> *vma,
> >>                           struct page **pages);
> >>  int xen_unmap_domain_mfn_range(struct vm_area_struct *vma,
> >>                           int numpgs, struct page **pages);
> >> +#ifdef CONFIG_XEN_AUTO_XLATE
> >> +int xen_xlate_remap_gfn_range(struct vm_area_struct *vma,
> >> +                        unsigned long addr,
> >> +                        xen_pfn_t gfn, int nr,
> >> +                        pgprot_t prot,
> >> +                        unsigned domid, 
> >> +                        struct page **pages);
> >> +int xen_xlate_unmap_gfn_range(struct vm_area_struct *vma,
> >> +                        int nr, struct page **pages);
> >> +#endif
> > 
> > I don't think we actually need the #ifdef in the header file?
> 
> It serves as useful documentation if nothing else.

git serves as useful documentation

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