From: "Jan Beulich" <JBeulich@novell.com>
To: "xen-devel@lists.xensource.com" <xen-devel@lists.xensource.com>
Subject: [PATCH] VT-d: fix and improve print_vtd_entries()
Date: Thu, 16 Dec 2010 15:44:25 +0000 [thread overview]
Message-ID: <4D0A41E902000078000286D3@vpn.id2.novell.com> (raw)
[-- Attachment #1: Type: text/plain, Size: 3892 bytes --]
Fix leaking of mapped domain pages (root_entry and ctxt_entry when
falling out of the level traversing loop). Do this by re-arranging
things slightly so that a mapping is retained only as long as it
really is needed.
Fix the failure to use map_domain_page() in the level traversing loop
of the function.
Add a mssing return statement in one of the error paths.
Also I wonder whether not being able to call print_vtd_entries() from
iommu_page_fault_do_one() in ix86 is still correct, now that
map_domain_page() is IRQ safe.
Signed-off-by: Jan Beulich <jbeulich@novell.com>
--- a/xen/drivers/passthrough/vtd/utils.c
+++ b/xen/drivers/passthrough/vtd/utils.c
@@ -103,7 +103,7 @@ void print_vtd_entries(struct iommu *iom
struct context_entry *ctxt_entry;
struct root_entry *root_entry;
struct dma_pte pte;
- u64 *l;
+ u64 *l, val;
u32 l_index, level;
printk("print_vtd_entries: iommu = %p bdf = %x:%x.%x gmfn = %"PRIx64"\n",
@@ -116,6 +116,11 @@ void print_vtd_entries(struct iommu *iom
}
root_entry = (struct root_entry *)map_vtd_domain_page(iommu->root_maddr);
+ if ( root_entry == NULL )
+ {
+ printk(" root_entry == NULL\n");
+ return;
+ }
printk(" root_entry = %p\n", root_entry);
printk(" root_entry[%x] = %"PRIx64"\n", bus, root_entry[bus].val);
@@ -126,61 +131,57 @@ void print_vtd_entries(struct iommu *iom
return;
}
- ctxt_entry =
- (struct context_entry *)map_vtd_domain_page(root_entry[bus].val);
+ val = root_entry[bus].val;
+ unmap_vtd_domain_page(root_entry);
+ ctxt_entry = map_vtd_domain_page(val);
if ( ctxt_entry == NULL )
{
- unmap_vtd_domain_page(root_entry);
printk(" ctxt_entry == NULL\n");
return;
}
printk(" context = %p\n", ctxt_entry);
+ val = ctxt_entry[devfn].lo;
printk(" context[%x] = %"PRIx64"_%"PRIx64"\n",
- devfn, ctxt_entry[devfn].hi, ctxt_entry[devfn].lo);
+ devfn, ctxt_entry[devfn].hi, val);
if ( !context_present(ctxt_entry[devfn]) )
{
unmap_vtd_domain_page(ctxt_entry);
- unmap_vtd_domain_page(root_entry);
printk(" ctxt_entry[%x] not present\n", devfn);
return;
}
level = agaw_to_level(context_address_width(ctxt_entry[devfn]));
+ unmap_vtd_domain_page(ctxt_entry);
if ( level != VTD_PAGE_TABLE_LEVEL_3 &&
level != VTD_PAGE_TABLE_LEVEL_4)
{
- unmap_vtd_domain_page(ctxt_entry);
- unmap_vtd_domain_page(root_entry);
printk("Unsupported VTD page table level (%d)!\n", level);
+ return;
}
- l = maddr_to_virt(ctxt_entry[devfn].lo);
do
{
- l = (u64*)(((unsigned long)l >> PAGE_SHIFT_4K) << PAGE_SHIFT_4K);
+ l = map_vtd_domain_page(val);
printk(" l%d = %p\n", level, l);
if ( l == NULL )
{
- unmap_vtd_domain_page(ctxt_entry);
- unmap_vtd_domain_page(root_entry);
printk(" l%d == NULL\n", level);
break;
}
l_index = get_level_index(gmfn, level);
printk(" l%d_index = %x\n", level, l_index);
- printk(" l%d[%x] = %"PRIx64"\n", level, l_index, l[l_index]);
- pte.val = l[l_index];
+ val = l[l_index];
+ unmap_vtd_domain_page(l);
+ printk(" l%d[%x] = %"PRIx64"\n", level, l_index, val);
+
+ pte.val = val;
if ( !dma_pte_present(pte) )
{
- unmap_vtd_domain_page(ctxt_entry);
- unmap_vtd_domain_page(root_entry);
printk(" l%d[%x] not present\n", level, l_index);
break;
}
-
- l = maddr_to_virt(l[l_index]);
} while ( --level );
}
[-- Attachment #2: vtd-print-entries.patch --]
[-- Type: text/plain, Size: 3886 bytes --]
Fix leaking of mapped domain pages (root_entry and ctxt_entry when
falling out of the level traversing loop). Do this by re-arranging
things slightly so that a mapping is retained only as long as it
really is needed.
Fix the failure to use map_domain_page() in the level traversing loop
of the function.
Add a mssing return statement in one of the error paths.
Also I wonder whether not being able to call print_vtd_entries() from
iommu_page_fault_do_one() in ix86 is still correct, now that
map_domain_page() is IRQ safe.
Signed-off-by: Jan Beulich <jbeulich@novell.com>
--- a/xen/drivers/passthrough/vtd/utils.c
+++ b/xen/drivers/passthrough/vtd/utils.c
@@ -103,7 +103,7 @@ void print_vtd_entries(struct iommu *iom
struct context_entry *ctxt_entry;
struct root_entry *root_entry;
struct dma_pte pte;
- u64 *l;
+ u64 *l, val;
u32 l_index, level;
printk("print_vtd_entries: iommu = %p bdf = %x:%x.%x gmfn = %"PRIx64"\n",
@@ -116,6 +116,11 @@ void print_vtd_entries(struct iommu *iom
}
root_entry = (struct root_entry *)map_vtd_domain_page(iommu->root_maddr);
+ if ( root_entry == NULL )
+ {
+ printk(" root_entry == NULL\n");
+ return;
+ }
printk(" root_entry = %p\n", root_entry);
printk(" root_entry[%x] = %"PRIx64"\n", bus, root_entry[bus].val);
@@ -126,61 +131,57 @@ void print_vtd_entries(struct iommu *iom
return;
}
- ctxt_entry =
- (struct context_entry *)map_vtd_domain_page(root_entry[bus].val);
+ val = root_entry[bus].val;
+ unmap_vtd_domain_page(root_entry);
+ ctxt_entry = map_vtd_domain_page(val);
if ( ctxt_entry == NULL )
{
- unmap_vtd_domain_page(root_entry);
printk(" ctxt_entry == NULL\n");
return;
}
printk(" context = %p\n", ctxt_entry);
+ val = ctxt_entry[devfn].lo;
printk(" context[%x] = %"PRIx64"_%"PRIx64"\n",
- devfn, ctxt_entry[devfn].hi, ctxt_entry[devfn].lo);
+ devfn, ctxt_entry[devfn].hi, val);
if ( !context_present(ctxt_entry[devfn]) )
{
unmap_vtd_domain_page(ctxt_entry);
- unmap_vtd_domain_page(root_entry);
printk(" ctxt_entry[%x] not present\n", devfn);
return;
}
level = agaw_to_level(context_address_width(ctxt_entry[devfn]));
+ unmap_vtd_domain_page(ctxt_entry);
if ( level != VTD_PAGE_TABLE_LEVEL_3 &&
level != VTD_PAGE_TABLE_LEVEL_4)
{
- unmap_vtd_domain_page(ctxt_entry);
- unmap_vtd_domain_page(root_entry);
printk("Unsupported VTD page table level (%d)!\n", level);
+ return;
}
- l = maddr_to_virt(ctxt_entry[devfn].lo);
do
{
- l = (u64*)(((unsigned long)l >> PAGE_SHIFT_4K) << PAGE_SHIFT_4K);
+ l = map_vtd_domain_page(val);
printk(" l%d = %p\n", level, l);
if ( l == NULL )
{
- unmap_vtd_domain_page(ctxt_entry);
- unmap_vtd_domain_page(root_entry);
printk(" l%d == NULL\n", level);
break;
}
l_index = get_level_index(gmfn, level);
printk(" l%d_index = %x\n", level, l_index);
- printk(" l%d[%x] = %"PRIx64"\n", level, l_index, l[l_index]);
- pte.val = l[l_index];
+ val = l[l_index];
+ unmap_vtd_domain_page(l);
+ printk(" l%d[%x] = %"PRIx64"\n", level, l_index, val);
+
+ pte.val = val;
if ( !dma_pte_present(pte) )
{
- unmap_vtd_domain_page(ctxt_entry);
- unmap_vtd_domain_page(root_entry);
printk(" l%d[%x] not present\n", level, l_index);
break;
}
-
- l = maddr_to_virt(l[l_index]);
} while ( --level );
}
[-- Attachment #3: Type: text/plain, Size: 138 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
reply other threads:[~2010-12-16 15:44 UTC|newest]
Thread overview: [no followups] expand[flat|nested] mbox.gz Atom feed
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4D0A41E902000078000286D3@vpn.id2.novell.com \
--to=jbeulich@novell.com \
--cc=xen-devel@lists.xensource.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).