From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: [PATCH 4/5] VT-d: widen NUMA nodes to be allocated from Date: Thu, 5 Mar 2015 17:08:31 +0000 Message-ID: <54F88D8F.7050908@citrix.com> References: <54EF315902000078000640FF@mail.emea.novell.com> <54EF33E00200007800064148@mail.emea.novell.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============4663606522375369677==" Return-path: Received: from mail6.bemta3.messagelabs.com ([195.245.230.39]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1YTZZj-00058i-MO for xen-devel@lists.xenproject.org; Thu, 05 Mar 2015 17:28:43 +0000 In-Reply-To: <54EF33E00200007800064148@mail.emea.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich , xen-devel Cc: Yang Z Zhang , Dario Faggioli , Kevin Tian List-Id: xen-devel@lists.xenproject.org --===============4663606522375369677== Content-Type: multipart/alternative; boundary="------------040507040209000601080301" --------------040507040209000601080301 Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: quoted-printable On 26/02/15 13:55, Jan Beulich wrote: > Signed-off-by: Jan Beulich I believe that this change is incorrect. > > --- a/xen/drivers/passthrough/vtd/extern.h > +++ b/xen/drivers/passthrough/vtd/extern.h > @@ -67,7 +67,8 @@ unsigned int get_cache_line_size(void); > void cacheline_flush(char *); > void flush_all_cache(void); > =20 > -u64 alloc_pgtable_maddr(struct acpi_drhd_unit *drhd, unsigned long npa= ges); > +u64 alloc_pgtable_maddr(struct acpi_drhd_unit *, unsigned int npages, > + struct domain *); > void free_pgtable_maddr(u64 maddr); > void *map_vtd_domain_page(u64 maddr); > void unmap_vtd_domain_page(void *va); > --- a/xen/drivers/passthrough/vtd/intremap.c > +++ b/xen/drivers/passthrough/vtd/intremap.c > @@ -739,7 +739,8 @@ int enable_intremap(struct iommu *iommu, > if ( ir_ctrl->iremap_maddr =3D=3D 0 ) > { > drhd =3D iommu_to_drhd(iommu); > - ir_ctrl->iremap_maddr =3D alloc_pgtable_maddr(drhd, IREMAP_ARC= H_PAGE_NR); > + ir_ctrl->iremap_maddr =3D alloc_pgtable_maddr(drhd, IREMAP_ARC= H_PAGE_NR, > + NULL); > if ( ir_ctrl->iremap_maddr =3D=3D 0 ) > { > dprintk(XENLOG_WARNING VTDPREFIX, > --- a/xen/drivers/passthrough/vtd/iommu.c > +++ b/xen/drivers/passthrough/vtd/iommu.c > @@ -185,7 +185,8 @@ void iommu_flush_cache_page(void *addr,=20 > } > =20 > /* Allocate page table, return its machine address */ > -u64 alloc_pgtable_maddr(struct acpi_drhd_unit *drhd, unsigned long npa= ges) > +u64 alloc_pgtable_maddr(struct acpi_drhd_unit *drhd, unsigned int npag= es, > + struct domain *d) > { > struct acpi_rhsa_unit *rhsa; > struct page_info *pg, *cur_pg; > @@ -195,10 +196,10 @@ u64 alloc_pgtable_maddr(struct acpi_drhd > =20 > rhsa =3D drhd_to_rhsa(drhd); > if ( rhsa ) > - node =3D pxm_to_node(rhsa->proximity_domain); > + node =3D pxm_to_node(rhsa->proximity_domain); > =20 > - pg =3D alloc_domheap_pages(NULL, get_order_from_pages(npages), > - (node =3D=3D NUMA_NO_NODE) ? 0 : MEMF_nod= e(node)); > + pg =3D alloc_domheap_pages(d, get_order_from_pages(npages), > + MEMF_node(node) | MEMF_no_owner); > if ( !pg ) > return 0; > =20 > @@ -223,7 +224,7 @@ void free_pgtable_maddr(u64 maddr) > } > =20 > /* context entry handling */ > -static u64 bus_to_context_maddr(struct iommu *iommu, u8 bus) > +static u64 bus_to_context_maddr(struct iommu *iommu, u8 bus, struct do= main *d) > { > struct acpi_drhd_unit *drhd; > struct root_entry *root, *root_entries; > @@ -235,7 +236,7 @@ static u64 bus_to_context_maddr(struct i > if ( !root_present(*root) ) > { > drhd =3D iommu_to_drhd(iommu); > - maddr =3D alloc_pgtable_maddr(drhd, 1); > + maddr =3D alloc_pgtable_maddr(drhd, 1, d); > if ( maddr =3D=3D 0 ) > { > unmap_vtd_domain_page(root_entries); > @@ -271,7 +272,9 @@ static u64 addr_to_dma_page_maddr(struct > */ > pdev =3D pci_get_pdev_by_domain(domain, -1, -1, -1); > drhd =3D acpi_find_matched_drhd_unit(pdev); > - if ( !alloc || ((hd->arch.pgd_maddr =3D alloc_pgtable_maddr(dr= hd, 1)) =3D=3D 0) ) > + if ( !alloc || > + ((hd->arch.pgd_maddr =3D alloc_pgtable_maddr(drhd, 1, > + domain)) =3D=3D= 0) ) This allocation should be based on the proximity information of the iommu, not of the requesting domain. > goto out; > } > =20 > @@ -289,7 +292,7 @@ static u64 addr_to_dma_page_maddr(struct > =20 > pdev =3D pci_get_pdev_by_domain(domain, -1, -1, -1); > drhd =3D acpi_find_matched_drhd_unit(pdev); > - pte_maddr =3D alloc_pgtable_maddr(drhd, 1); > + pte_maddr =3D alloc_pgtable_maddr(drhd, 1, domain); As should this. I don't see a need to extend the prototype of alloc_pgtable_maddr().=20 The drhd parameter contains all relevant information concerning proximity= =2E ~Andrew > if ( !pte_maddr ) > break; > =20 > @@ -1119,7 +1122,7 @@ int __init iommu_alloc(struct acpi_drhd_ > iommu->intel->drhd =3D drhd; > drhd->iommu =3D iommu; > =20 > - if ( !(iommu->root_maddr =3D alloc_pgtable_maddr(drhd, 1)) ) > + if ( !(iommu->root_maddr =3D alloc_pgtable_maddr(drhd, 1, NULL)) )= > return -ENOMEM; > =20 > iommu->reg =3D ioremap(drhd->address, PAGE_SIZE); > @@ -1270,7 +1273,7 @@ int domain_context_mapping_one( > =20 > ASSERT(spin_is_locked(&pcidevs_lock)); > spin_lock(&iommu->lock); > - maddr =3D bus_to_context_maddr(iommu, bus); > + maddr =3D bus_to_context_maddr(iommu, bus, domain); > context_entries =3D (struct context_entry *)map_vtd_domain_page(ma= ddr); > context =3D &context_entries[devfn]; > =20 > @@ -1495,7 +1498,7 @@ int domain_context_unmap_one( > ASSERT(spin_is_locked(&pcidevs_lock)); > spin_lock(&iommu->lock); > =20 > - maddr =3D bus_to_context_maddr(iommu, bus); > + maddr =3D bus_to_context_maddr(iommu, bus, domain); > context_entries =3D (struct context_entry *)map_vtd_domain_page(ma= ddr); > context =3D &context_entries[devfn]; > =20 > --- a/xen/drivers/passthrough/vtd/qinval.c > +++ b/xen/drivers/passthrough/vtd/qinval.c > @@ -374,7 +374,8 @@ int enable_qinval(struct iommu *iommu) > if ( qi_ctrl->qinval_maddr =3D=3D 0 ) > { > drhd =3D iommu_to_drhd(iommu); > - qi_ctrl->qinval_maddr =3D alloc_pgtable_maddr(drhd, QINVAL_ARC= H_PAGE_NR); > + qi_ctrl->qinval_maddr =3D alloc_pgtable_maddr(drhd, QINVAL_ARC= H_PAGE_NR, > + NULL); > if ( qi_ctrl->qinval_maddr =3D=3D 0 ) > { > dprintk(XENLOG_WARNING VTDPREFIX, > > > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel --------------040507040209000601080301 Content-Type: text/html; charset="windows-1252" Content-Length: 7412 Content-Transfer-Encoding: quoted-printable
On 26/02/15 13:55, Jan Beulich wrote:
Signed-off-by: Jan Beulich <jbeulich@suse.com>

I believe that this change is incorrect.


--- a/xen/drivers/passthrough/vtd/extern.h
+++ b/xen/drivers/passthrough/vtd/extern.h
@@ -67,7 +67,8 @@ unsigned int get_cache_line_size(void);
 void cacheline_flush(char *);
 void flush_all_cache(void);
 
-u64 alloc_pgtable_maddr(struct acpi_drhd_unit *drhd, unsigned long npages);
+u64 alloc_pgtable_maddr(struct acpi_drhd_unit *, unsigned int npages,
+                        struct domain *);
 void free_pgtable_maddr(u64 maddr);
 void *map_vtd_domain_page(u64 maddr);
 void unmap_vtd_domain_page(void *va);
--- a/xen/drivers/passthrough/vtd/intremap.c
+++ b/xen/drivers/passthrough/vtd/intremap.c
@@ -739,7 +739,8 @@ int enable_intremap(struct iommu *iommu,
     if ( ir_ctrl->iremap_maddr =3D=3D 0 )
     {
         drhd =3D iommu_to_drhd(iommu);
-        ir_ctrl->iremap_maddr =3D alloc_pgtable_maddr(drhd, IREMAP_ARCH_PAGE_NR);
+        ir_ctrl->iremap_maddr =3D alloc_pgtable_maddr(drhd, IREMAP_ARCH_PAGE_NR,
+                                                    NULL);
         if ( ir_ctrl->iremap_maddr =3D=3D 0 )
         {
             dprintk(XENLOG_WARNING VTDPREFIX,
--- a/xen/drivers/passthrough/vtd/iommu.c
+++ b/xen/drivers/passthrough/vtd/iommu.c
@@ -185,7 +185,8 @@ void iommu_flush_cache_page(void *addr, 
 }
 
 /* Allocate page table, return its machine address */
-u64 alloc_pgtable_maddr(struct acpi_drhd_unit *drhd, unsigned long npages)
+u64 alloc_pgtable_maddr(struct acpi_drhd_unit *drhd, unsigned int npages,
+                        struct domain *d)
 {
     struct acpi_rhsa_unit *rhsa;
     struct page_info *pg, *cur_pg;
@@ -195,10 +196,10 @@ u64 alloc_pgtable_maddr(struct acpi_drhd
 
     rhsa =3D drhd_to_rhsa(drhd);
     if ( rhsa )
-        node =3D  pxm_to_node(rhsa->proximity_domain);
+        node =3D pxm_to_node(rhsa->proximity_domain);
 
-    pg =3D alloc_domheap_pages(NULL, get_order_from_pages(npages),
-                             (node =3D=3D NUMA_NO_NODE) =3F 0 : MEMF_node(node));
+    pg =3D alloc_domheap_pages(d, get_order_from_pages(npages),
+                             MEMF_node(node) | MEMF_no_owner);
     if ( !pg )
         return 0;
 
@@ -223,7 +224,7 @@ void free_pgtable_maddr(u64 maddr)
 }
 
 /* context entry handling */
-static u64 bus_to_context_maddr(struct iommu *iommu, u8 bus)
+static u64 bus_to_context_maddr(struct iommu *iommu, u8 bus, struct domain *d)
 {
     struct acpi_drhd_unit *drhd;
     struct root_entry *root, *root_entries;
@@ -235,7 +236,7 @@ static u64 bus_to_context_maddr(struct i
     if ( !root_present(*root) )
     {
         drhd =3D iommu_to_drhd(iommu);
-        maddr =3D alloc_pgtable_maddr(drhd, 1);
+        maddr =3D alloc_pgtable_maddr(drhd, 1, d);
         if ( maddr =3D=3D 0 )
         {
             unmap_vtd_domain_page(root_entries);
@@ -271,7 +272,9 @@ static u64 addr_to_dma_page_maddr(struct
          */
         pdev =3D pci_get_pdev_by_domain(domain, -1, -1, -1);
         drhd =3D acpi_find_matched_drhd_unit(pdev);
-        if ( !alloc || ((hd->arch.pgd_maddr =3D alloc_pgtable_maddr(drhd, 1)) =3D=3D 0) )
+        if ( !alloc ||
+             ((hd->arch.pgd_maddr =3D alloc_pgtable_maddr(drhd, 1,
+                                                        domain)) =3D=3D 0) )

This allocation should be based on the proximity information of the iommu, not of the requesting domain.

             goto out;
     }
 
@@ -289,7 +292,7 @@ static u64 addr_to_dma_page_maddr(struct
 
             pdev =3D pci_get_pdev_by_domain(domain, -1, -1, -1);
             drhd =3D acpi_find_matched_drhd_unit(pdev);
-            pte_maddr =3D alloc_pgtable_maddr(drhd, 1);
+            pte_maddr =3D alloc_pgtable_maddr(drhd, 1, domain);

As should this.

I don't see a need to extend the prototype of alloc_pgtable_maddr().=A0 The drhd parameter contains all relevant information concerning proximity.

~Andrew

             if ( !pte_maddr )
                 break;
 
@@ -1119,7 +1122,7 @@ int __init iommu_alloc(struct acpi_drhd_
     iommu->intel->drhd =3D drhd;
     drhd->iommu =3D iommu;
 
-    if ( !(iommu->root_maddr =3D alloc_pgtable_maddr(drhd, 1)) )
+    if ( !(iommu->root_maddr =3D alloc_pgtable_maddr(drhd, 1, NULL)) )
         return -ENOMEM;
 
     iommu->reg =3D ioremap(drhd->address, PAGE_SIZE);
@@ -1270,7 +1273,7 @@ int domain_context_mapping_one(
 
     ASSERT(spin_is_locked(&pcidevs_lock));
     spin_lock(&iommu->lock);
-    maddr =3D bus_to_context_maddr(iommu, bus);
+    maddr =3D bus_to_context_maddr(iommu, bus, domain);
     context_entries =3D (struct context_entry *)map_vtd_domain_page(maddr);
     context =3D &context_entries[devfn];
 
@@ -1495,7 +1498,7 @@ int domain_context_unmap_one(
     ASSERT(spin_is_locked(&pcidevs_lock));
     spin_lock(&iommu->lock);
 
-    maddr =3D bus_to_context_maddr(iommu, bus);
+    maddr =3D bus_to_context_maddr(iommu, bus, domain);
     context_entries =3D (struct context_entry *)map_vtd_domain_page(maddr);
     context =3D &context_entries[devfn];
 
--- a/xen/drivers/passthrough/vtd/qinval.c
+++ b/xen/drivers/passthrough/vtd/qinval.c
@@ -374,7 +374,8 @@ int enable_qinval(struct iommu *iommu)
     if ( qi_ctrl->qinval_maddr =3D=3D 0 )
     {
         drhd =3D iommu_to_drhd(iommu);
-        qi_ctrl->qinval_maddr =3D alloc_pgtable_maddr(drhd, QINVAL_ARCH_PAGE_NR);
+        qi_ctrl->qinval_maddr =3D alloc_pgtable_maddr(drhd, QINVAL_ARCH_PAGE_NR,
+                                                    NULL);
         if ( qi_ctrl->qinval_maddr =3D=3D 0 )
         {
             dprintk(XENLOG_WARNING VTDPREFIX,





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

--------------040507040209000601080301-- --===============4663606522375369677== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel --===============4663606522375369677==--