RE: [PATCH v2] xen/privcmd: add IOCTL_PRIVCMD_MMAP_RESOURCE
> -Original Message- > From: Boris Ostrovsky [mailto:boris.ostrov...@oracle.com] > Sent: 05 April 2018 23:34 > To: Paul Durrant <paul.durr...@citrix.com>; x...@kernel.org; xen- > de...@lists.xenproject.org; linux-kernel@vger.kernel.org > Cc: Juergen Gross <jgr...@suse.com>; Thomas Gleixner > <t...@linutronix.de>; Ingo Molnar <mi...@redhat.com> > Subject: Re: [PATCH v2] xen/privcmd: add > IOCTL_PRIVCMD_MMAP_RESOURCE > > On 04/05/2018 11:42 AM, Paul Durrant wrote: > > My recent Xen patch series introduces a new HYPERVISOR_memory_op to > > support direct priv-mapping of certain guest resources (such as ioreq > > pages, used by emulators) by a tools domain, rather than having to access > > such resources via the guest P2M. > > > > This patch adds the necessary infrastructure to the privcmd driver and > > Xen MMU code to support direct resource mapping. > > > > NOTE: The adjustment in the MMU code is partially cosmetic. Xen will now > > allow a PV tools domain to map guest pages either by GFN or MFN, thus > > the term 'mfn' has been swapped for 'pfn' in the lower layers of the > > remap code. > > > > Signed-off-by: Paul Durrant <paul.durr...@citrix.com> > > --- > > Cc: Boris Ostrovsky <boris.ostrov...@oracle.com> > > Cc: Juergen Gross <jgr...@suse.com> > > Cc: Thomas Gleixner <t...@linutronix.de> > > Cc: Ingo Molnar <mi...@redhat.com> > > > > v2: > > - Fix bug when mapping multiple pages of a resource > > > Only a few nits below. > > > --- > > arch/x86/xen/mmu.c | 50 +++- > > drivers/xen/privcmd.c | 130 > + > > include/uapi/xen/privcmd.h | 11 > > include/xen/interface/memory.h | 66 + > > include/xen/interface/xen.h| 7 ++- > > include/xen/xen-ops.h | 24 +++- > > 6 files changed, 270 insertions(+), 18 deletions(-) > > > > diff --git a/arch/x86/xen/mmu.c b/arch/x86/xen/mmu.c > > index d33e7dbe3129..8453d7be415c 100644 > > --- a/arch/x86/xen/mmu.c > > +++ b/arch/x86/xen/mmu.c > > @@ -65,37 +65,42 @@ static void xen_flush_tlb_all(void) > > #define REMAP_BATCH_SIZE 16 > > > > struct remap_data { > > - xen_pfn_t *mfn; > > + xen_pfn_t *pfn; > > bool contiguous; > > + bool no_translate; > > pgprot_t prot; > > struct mmu_update *mmu_update; > > }; > > > > -static int remap_area_mfn_pte_fn(pte_t *ptep, pgtable_t token, > > +static int remap_area_pfn_pte_fn(pte_t *ptep, pgtable_t token, > > unsigned long addr, void *data) > > { > > struct remap_data *rmd = data; > > - pte_t pte = pte_mkspecial(mfn_pte(*rmd->mfn, rmd->prot)); > > + pte_t pte = pte_mkspecial(mfn_pte(*rmd->pfn, rmd->prot)); > > > > /* If we have a contiguous range, just update the mfn itself, > >else update pointer to be "next mfn". */ > > This probably also needs to be updated (and while at it, comment style > fixed) > Ok. > > if (rmd->contiguous) > > - (*rmd->mfn)++; > > + (*rmd->pfn)++; > > else > > - rmd->mfn++; > > + rmd->pfn++; > > > > - rmd->mmu_update->ptr = virt_to_machine(ptep).maddr | > MMU_NORMAL_PT_UPDATE; > > + rmd->mmu_update->ptr = virt_to_machine(ptep).maddr; > > + rmd->mmu_update->ptr |= rmd->no_translate ? > > + MMU_PT_UPDATE_NO_TRANSLATE : > > + MMU_NORMAL_PT_UPDATE; > > rmd->mmu_update->val = pte_val_ma(pte); > > rmd->mmu_update++; > > > > return 0; > > } > > > > -static int do_remap_gfn(struct vm_area_struct *vma, > > +static int do_remap_pfn(struct vm_area_struct *vma, > > unsigned long addr, > > - xen_pfn_t *gfn, int nr, > > + xen_pfn_t *pfn, int nr, > > int *err_ptr, pgprot_t prot, > > - unsigned domid, > > + unsigned int domid, > > + bool no_translate, > > struct page **pages) > > { > > int err = 0; > > @@ -106,11 +111,12 @@ static int do_remap_gfn(struct vm_area_struct > *vma, > > > > BUG_ON(!((vma->vm_flags & (VM_PFNMAP | VM_IO)) == > (VM_PFNMAP | VM_IO))); > > > > - rmd.mfn = gfn; > > + rmd.pfn = pfn; > &g
RE: [PATCH v2] xen/privcmd: add IOCTL_PRIVCMD_MMAP_RESOURCE
> -Original Message- > From: Boris Ostrovsky [mailto:boris.ostrov...@oracle.com] > Sent: 05 April 2018 23:34 > To: Paul Durrant ; x...@kernel.org; xen- > de...@lists.xenproject.org; linux-kernel@vger.kernel.org > Cc: Juergen Gross ; Thomas Gleixner > ; Ingo Molnar > Subject: Re: [PATCH v2] xen/privcmd: add > IOCTL_PRIVCMD_MMAP_RESOURCE > > On 04/05/2018 11:42 AM, Paul Durrant wrote: > > My recent Xen patch series introduces a new HYPERVISOR_memory_op to > > support direct priv-mapping of certain guest resources (such as ioreq > > pages, used by emulators) by a tools domain, rather than having to access > > such resources via the guest P2M. > > > > This patch adds the necessary infrastructure to the privcmd driver and > > Xen MMU code to support direct resource mapping. > > > > NOTE: The adjustment in the MMU code is partially cosmetic. Xen will now > > allow a PV tools domain to map guest pages either by GFN or MFN, thus > > the term 'mfn' has been swapped for 'pfn' in the lower layers of the > > remap code. > > > > Signed-off-by: Paul Durrant > > --- > > Cc: Boris Ostrovsky > > Cc: Juergen Gross > > Cc: Thomas Gleixner > > Cc: Ingo Molnar > > > > v2: > > - Fix bug when mapping multiple pages of a resource > > > Only a few nits below. > > > --- > > arch/x86/xen/mmu.c | 50 +++- > > drivers/xen/privcmd.c | 130 > + > > include/uapi/xen/privcmd.h | 11 > > include/xen/interface/memory.h | 66 + > > include/xen/interface/xen.h| 7 ++- > > include/xen/xen-ops.h | 24 +++- > > 6 files changed, 270 insertions(+), 18 deletions(-) > > > > diff --git a/arch/x86/xen/mmu.c b/arch/x86/xen/mmu.c > > index d33e7dbe3129..8453d7be415c 100644 > > --- a/arch/x86/xen/mmu.c > > +++ b/arch/x86/xen/mmu.c > > @@ -65,37 +65,42 @@ static void xen_flush_tlb_all(void) > > #define REMAP_BATCH_SIZE 16 > > > > struct remap_data { > > - xen_pfn_t *mfn; > > + xen_pfn_t *pfn; > > bool contiguous; > > + bool no_translate; > > pgprot_t prot; > > struct mmu_update *mmu_update; > > }; > > > > -static int remap_area_mfn_pte_fn(pte_t *ptep, pgtable_t token, > > +static int remap_area_pfn_pte_fn(pte_t *ptep, pgtable_t token, > > unsigned long addr, void *data) > > { > > struct remap_data *rmd = data; > > - pte_t pte = pte_mkspecial(mfn_pte(*rmd->mfn, rmd->prot)); > > + pte_t pte = pte_mkspecial(mfn_pte(*rmd->pfn, rmd->prot)); > > > > /* If we have a contiguous range, just update the mfn itself, > >else update pointer to be "next mfn". */ > > This probably also needs to be updated (and while at it, comment style > fixed) > Ok. > > if (rmd->contiguous) > > - (*rmd->mfn)++; > > + (*rmd->pfn)++; > > else > > - rmd->mfn++; > > + rmd->pfn++; > > > > - rmd->mmu_update->ptr = virt_to_machine(ptep).maddr | > MMU_NORMAL_PT_UPDATE; > > + rmd->mmu_update->ptr = virt_to_machine(ptep).maddr; > > + rmd->mmu_update->ptr |= rmd->no_translate ? > > + MMU_PT_UPDATE_NO_TRANSLATE : > > + MMU_NORMAL_PT_UPDATE; > > rmd->mmu_update->val = pte_val_ma(pte); > > rmd->mmu_update++; > > > > return 0; > > } > > > > -static int do_remap_gfn(struct vm_area_struct *vma, > > +static int do_remap_pfn(struct vm_area_struct *vma, > > unsigned long addr, > > - xen_pfn_t *gfn, int nr, > > + xen_pfn_t *pfn, int nr, > > int *err_ptr, pgprot_t prot, > > - unsigned domid, > > + unsigned int domid, > > + bool no_translate, > > struct page **pages) > > { > > int err = 0; > > @@ -106,11 +111,12 @@ static int do_remap_gfn(struct vm_area_struct > *vma, > > > > BUG_ON(!((vma->vm_flags & (VM_PFNMAP | VM_IO)) == > (VM_PFNMAP | VM_IO))); > > > > - rmd.mfn = gfn; > > + rmd.pfn = pfn; > > rmd.prot = prot; > > /* We use the err_ptr to indicate if there we are doing a contiguous > > * mapping or a discontigious mapping. */ > > Style. > I'm not modifying this comment but I'll fix it. > &g
Re: [PATCH v2] xen/privcmd: add IOCTL_PRIVCMD_MMAP_RESOURCE
Hi Paul, I love your patch! Yet something to improve: [auto build test ERROR on xen-tip/linux-next] [also build test ERROR on v4.16 next-20180405] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Paul-Durrant/xen-privcmd-add-IOCTL_PRIVCMD_MMAP_RESOURCE/20180406-121749 base: https://git.kernel.org/pub/scm/linux/kernel/git/xen/tip.git linux-next config: arm64-defconfig (attached as .config) compiler: aarch64-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0 reproduce: wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree make.cross ARCH=arm64 All errors (new ones prefixed by >>): drivers/xen/privcmd.o: In function `privcmd_ioctl': >> privcmd.c:(.text+0x12ac): undefined reference to `xen_remap_domain_mfn_array' privcmd.c:(.text+0x12ac): relocation truncated to fit: R_AARCH64_CALL26 against undefined symbol `xen_remap_domain_mfn_array' --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: application/gzip
Re: [PATCH v2] xen/privcmd: add IOCTL_PRIVCMD_MMAP_RESOURCE
Hi Paul, I love your patch! Yet something to improve: [auto build test ERROR on xen-tip/linux-next] [also build test ERROR on v4.16 next-20180405] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Paul-Durrant/xen-privcmd-add-IOCTL_PRIVCMD_MMAP_RESOURCE/20180406-121749 base: https://git.kernel.org/pub/scm/linux/kernel/git/xen/tip.git linux-next config: arm64-defconfig (attached as .config) compiler: aarch64-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0 reproduce: wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree make.cross ARCH=arm64 All errors (new ones prefixed by >>): drivers/xen/privcmd.o: In function `privcmd_ioctl': >> privcmd.c:(.text+0x12ac): undefined reference to `xen_remap_domain_mfn_array' privcmd.c:(.text+0x12ac): relocation truncated to fit: R_AARCH64_CALL26 against undefined symbol `xen_remap_domain_mfn_array' --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: application/gzip
Re: [PATCH v2] xen/privcmd: add IOCTL_PRIVCMD_MMAP_RESOURCE
On 04/05/2018 11:42 AM, Paul Durrant wrote: > My recent Xen patch series introduces a new HYPERVISOR_memory_op to > support direct priv-mapping of certain guest resources (such as ioreq > pages, used by emulators) by a tools domain, rather than having to access > such resources via the guest P2M. > > This patch adds the necessary infrastructure to the privcmd driver and > Xen MMU code to support direct resource mapping. > > NOTE: The adjustment in the MMU code is partially cosmetic. Xen will now > allow a PV tools domain to map guest pages either by GFN or MFN, thus > the term 'mfn' has been swapped for 'pfn' in the lower layers of the > remap code. > > Signed-off-by: Paul Durrant> --- > Cc: Boris Ostrovsky > Cc: Juergen Gross > Cc: Thomas Gleixner > Cc: Ingo Molnar > > v2: > - Fix bug when mapping multiple pages of a resource Only a few nits below. > --- > arch/x86/xen/mmu.c | 50 +++- > drivers/xen/privcmd.c | 130 > + > include/uapi/xen/privcmd.h | 11 > include/xen/interface/memory.h | 66 + > include/xen/interface/xen.h| 7 ++- > include/xen/xen-ops.h | 24 +++- > 6 files changed, 270 insertions(+), 18 deletions(-) > > diff --git a/arch/x86/xen/mmu.c b/arch/x86/xen/mmu.c > index d33e7dbe3129..8453d7be415c 100644 > --- a/arch/x86/xen/mmu.c > +++ b/arch/x86/xen/mmu.c > @@ -65,37 +65,42 @@ static void xen_flush_tlb_all(void) > #define REMAP_BATCH_SIZE 16 > > struct remap_data { > - xen_pfn_t *mfn; > + xen_pfn_t *pfn; > bool contiguous; > + bool no_translate; > pgprot_t prot; > struct mmu_update *mmu_update; > }; > > -static int remap_area_mfn_pte_fn(pte_t *ptep, pgtable_t token, > +static int remap_area_pfn_pte_fn(pte_t *ptep, pgtable_t token, >unsigned long addr, void *data) > { > struct remap_data *rmd = data; > - pte_t pte = pte_mkspecial(mfn_pte(*rmd->mfn, rmd->prot)); > + pte_t pte = pte_mkspecial(mfn_pte(*rmd->pfn, rmd->prot)); > > /* If we have a contiguous range, just update the mfn itself, > else update pointer to be "next mfn". */ This probably also needs to be updated (and while at it, comment style fixed) > if (rmd->contiguous) > - (*rmd->mfn)++; > + (*rmd->pfn)++; > else > - rmd->mfn++; > + rmd->pfn++; > > - rmd->mmu_update->ptr = virt_to_machine(ptep).maddr | > MMU_NORMAL_PT_UPDATE; > + rmd->mmu_update->ptr = virt_to_machine(ptep).maddr; > + rmd->mmu_update->ptr |= rmd->no_translate ? > + MMU_PT_UPDATE_NO_TRANSLATE : > + MMU_NORMAL_PT_UPDATE; > rmd->mmu_update->val = pte_val_ma(pte); > rmd->mmu_update++; > > return 0; > } > > -static int do_remap_gfn(struct vm_area_struct *vma, > +static int do_remap_pfn(struct vm_area_struct *vma, > unsigned long addr, > - xen_pfn_t *gfn, int nr, > + xen_pfn_t *pfn, int nr, > int *err_ptr, pgprot_t prot, > - unsigned domid, > + unsigned int domid, > + bool no_translate, > struct page **pages) > { > int err = 0; > @@ -106,11 +111,12 @@ static int do_remap_gfn(struct vm_area_struct *vma, > > BUG_ON(!((vma->vm_flags & (VM_PFNMAP | VM_IO)) == (VM_PFNMAP | VM_IO))); > > - rmd.mfn = gfn; > + rmd.pfn = pfn; > rmd.prot = prot; > /* We use the err_ptr to indicate if there we are doing a contiguous >* mapping or a discontigious mapping. */ Style. > rmd.contiguous = !err_ptr; > + rmd.no_translate = no_translate; > > while (nr) { > int index = 0; > @@ -121,7 +127,7 @@ static int do_remap_gfn(struct vm_area_struct *vma, > > rmd.mmu_update = mmu_update; > err = apply_to_page_range(vma->vm_mm, addr, range, > - remap_area_mfn_pte_fn, ); > + remap_area_pfn_pte_fn, ); > if (err) > goto out; > > @@ -175,7 +181,8 @@ int xen_remap_domain_gfn_range(struct vm_area_struct *vma, > if (xen_feature(XENFEAT_auto_translated_physmap)) > return -EOPNOTSUPP; > > - return do_remap_gfn(vma, addr, , nr, NULL, prot, domid, pages); > + return do_remap_pfn(vma, addr, , nr, NULL, prot, domid, false, > + pages); > } > EXPORT_SYMBOL_GPL(xen_remap_domain_gfn_range); > > @@ -183,7 +190,7 @@ int xen_remap_domain_gfn_array(struct vm_area_struct *vma, > unsigned long addr, > xen_pfn_t *gfn, int nr, >
Re: [PATCH v2] xen/privcmd: add IOCTL_PRIVCMD_MMAP_RESOURCE
On 04/05/2018 11:42 AM, Paul Durrant wrote: > My recent Xen patch series introduces a new HYPERVISOR_memory_op to > support direct priv-mapping of certain guest resources (such as ioreq > pages, used by emulators) by a tools domain, rather than having to access > such resources via the guest P2M. > > This patch adds the necessary infrastructure to the privcmd driver and > Xen MMU code to support direct resource mapping. > > NOTE: The adjustment in the MMU code is partially cosmetic. Xen will now > allow a PV tools domain to map guest pages either by GFN or MFN, thus > the term 'mfn' has been swapped for 'pfn' in the lower layers of the > remap code. > > Signed-off-by: Paul Durrant > --- > Cc: Boris Ostrovsky > Cc: Juergen Gross > Cc: Thomas Gleixner > Cc: Ingo Molnar > > v2: > - Fix bug when mapping multiple pages of a resource Only a few nits below. > --- > arch/x86/xen/mmu.c | 50 +++- > drivers/xen/privcmd.c | 130 > + > include/uapi/xen/privcmd.h | 11 > include/xen/interface/memory.h | 66 + > include/xen/interface/xen.h| 7 ++- > include/xen/xen-ops.h | 24 +++- > 6 files changed, 270 insertions(+), 18 deletions(-) > > diff --git a/arch/x86/xen/mmu.c b/arch/x86/xen/mmu.c > index d33e7dbe3129..8453d7be415c 100644 > --- a/arch/x86/xen/mmu.c > +++ b/arch/x86/xen/mmu.c > @@ -65,37 +65,42 @@ static void xen_flush_tlb_all(void) > #define REMAP_BATCH_SIZE 16 > > struct remap_data { > - xen_pfn_t *mfn; > + xen_pfn_t *pfn; > bool contiguous; > + bool no_translate; > pgprot_t prot; > struct mmu_update *mmu_update; > }; > > -static int remap_area_mfn_pte_fn(pte_t *ptep, pgtable_t token, > +static int remap_area_pfn_pte_fn(pte_t *ptep, pgtable_t token, >unsigned long addr, void *data) > { > struct remap_data *rmd = data; > - pte_t pte = pte_mkspecial(mfn_pte(*rmd->mfn, rmd->prot)); > + pte_t pte = pte_mkspecial(mfn_pte(*rmd->pfn, rmd->prot)); > > /* If we have a contiguous range, just update the mfn itself, > else update pointer to be "next mfn". */ This probably also needs to be updated (and while at it, comment style fixed) > if (rmd->contiguous) > - (*rmd->mfn)++; > + (*rmd->pfn)++; > else > - rmd->mfn++; > + rmd->pfn++; > > - rmd->mmu_update->ptr = virt_to_machine(ptep).maddr | > MMU_NORMAL_PT_UPDATE; > + rmd->mmu_update->ptr = virt_to_machine(ptep).maddr; > + rmd->mmu_update->ptr |= rmd->no_translate ? > + MMU_PT_UPDATE_NO_TRANSLATE : > + MMU_NORMAL_PT_UPDATE; > rmd->mmu_update->val = pte_val_ma(pte); > rmd->mmu_update++; > > return 0; > } > > -static int do_remap_gfn(struct vm_area_struct *vma, > +static int do_remap_pfn(struct vm_area_struct *vma, > unsigned long addr, > - xen_pfn_t *gfn, int nr, > + xen_pfn_t *pfn, int nr, > int *err_ptr, pgprot_t prot, > - unsigned domid, > + unsigned int domid, > + bool no_translate, > struct page **pages) > { > int err = 0; > @@ -106,11 +111,12 @@ static int do_remap_gfn(struct vm_area_struct *vma, > > BUG_ON(!((vma->vm_flags & (VM_PFNMAP | VM_IO)) == (VM_PFNMAP | VM_IO))); > > - rmd.mfn = gfn; > + rmd.pfn = pfn; > rmd.prot = prot; > /* We use the err_ptr to indicate if there we are doing a contiguous >* mapping or a discontigious mapping. */ Style. > rmd.contiguous = !err_ptr; > + rmd.no_translate = no_translate; > > while (nr) { > int index = 0; > @@ -121,7 +127,7 @@ static int do_remap_gfn(struct vm_area_struct *vma, > > rmd.mmu_update = mmu_update; > err = apply_to_page_range(vma->vm_mm, addr, range, > - remap_area_mfn_pte_fn, ); > + remap_area_pfn_pte_fn, ); > if (err) > goto out; > > @@ -175,7 +181,8 @@ int xen_remap_domain_gfn_range(struct vm_area_struct *vma, > if (xen_feature(XENFEAT_auto_translated_physmap)) > return -EOPNOTSUPP; > > - return do_remap_gfn(vma, addr, , nr, NULL, prot, domid, pages); > + return do_remap_pfn(vma, addr, , nr, NULL, prot, domid, false, > + pages); > } > EXPORT_SYMBOL_GPL(xen_remap_domain_gfn_range); > > @@ -183,7 +190,7 @@ int xen_remap_domain_gfn_array(struct vm_area_struct *vma, > unsigned long addr, > xen_pfn_t *gfn, int nr, > int *err_ptr, pgprot_t prot, > -unsigned domid, struct page **pages) > +