On 3/23/2017 5:00 PM, Jan Beulich wrote:
On 23.03.17 at 04:23, <yu.c.zh...@linux.intel.com> wrote:
On 3/22/2017 10:29 PM, Jan Beulich wrote:
On 21.03.17 at 03:52, <yu.c.zh...@linux.intel.com> wrote:
--- a/xen/arch/x86/hvm/ioreq.c
+++ b/xen/arch/x86/hvm/ioreq.c
@@ -949,6 +949,14 @@ int hvm_map_mem_type_to_ioreq_server(struct domain *d,
ioservid_t id,
spin_unlock_recursive(&d->arch.hvm_domain.ioreq_server.lock);
+ if ( rc == 0 && flags == 0 )
+ {
+ struct p2m_domain *p2m = p2m_get_hostp2m(d);
+
+ if ( read_atomic(&p2m->ioreq.entry_count) )
+ p2m_change_entry_type_global(d, p2m_ioreq_server, p2m_ram_rw);
+ }
If you do this after dropping the lock, don't you risk a race with
another server mapping the type to itself?
I believe it's OK. Remaining p2m_ioreq_server entries still needs to be
cleaned anyway.
Are you refusing a new server mapping the type before being
done with the cleanup?
No. I meant even a new server is mapped, we can still sweep the p2m
table later asynchronously.
But this reminds me other point - will a dm op be interrupted by another
one, or should it?
Since we have patch 5/5 which sweep the p2m table right after the unmap
happens, maybe
we should refuse any mapping requirement if there's remaining
p2m_ioreq_server entries.
--- a/xen/arch/x86/mm/p2m-ept.c
+++ b/xen/arch/x86/mm/p2m-ept.c
@@ -544,6 +544,12 @@ static int resolve_misconfig(struct p2m_domain *p2m,
unsigned long gfn)
e.ipat = ipat;
if ( e.recalc && p2m_is_changeable(e.sa_p2mt) )
{
+ if ( e.sa_p2mt == p2m_ioreq_server )
+ {
+ p2m->ioreq.entry_count--;
+ ASSERT(p2m->ioreq.entry_count >= 0);
If you did the ASSERT() first (using > 0), you wouldn't need the
type be a signed one, doubling the valid value range (even if
right now the full 64 bits can't be used anyway, but it would be
one less thing to worry about once we get 6-level page tables).
Well, entry_count counts only for 4K pages, so even if the guest
physical address
width is extended up to 64 bit in the future, entry_count will not
exceed 2^52(
2^64/2^12).
Oh, true. Still I'd prefer if you used an unsigned type for a count
when that's easily possible.
Got it. :)
Yu
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
