From mboxrd@z Thu Jan 1 00:00:00 1970 From: Christoph Egger Subject: Re: [PATCH 3 of 4] Nested p2m: clarify logic in p2m_get_nestedp2m() Date: Fri, 24 Jun 2011 16:53:36 +0200 Message-ID: <4E04A4F0.4090803@amd.com> References: <4E049E64.9080908@amd.com> <20110624143726.GI9784@whitby.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset="ISO-8859-1"; format=flowed Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20110624143726.GI9784@whitby.uk.xensource.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: Tim Deegan Cc: "xen-devel@lists.xensource.com" List-Id: xen-devel@lists.xenproject.org On 06/24/11 16:37, Tim Deegan wrote: > At 15:25 +0100 on 24 Jun (1308929140), Christoph Egger wrote: >>> diff -r dcb8ae5e3eaf -r b265371addbb xen/arch/x86/mm/p2m.c >>> --- a/xen/arch/x86/mm/p2m.c Wed Jun 22 17:04:08 2011 +0100 >>> +++ b/xen/arch/x86/mm/p2m.c Wed Jun 22 17:04:08 2011 +0100 >>> @@ -1131,11 +1131,9 @@ p2m_get_nestedp2m(struct vcpu *v, uint64 >>> >>> d = v->domain; >>> nestedp2m_lock(d); >>> - for (i = 0; i< MAX_NESTEDP2M; i++) { >>> - p2m = d->arch.nested_p2m[i]; >>> - if ((p2m->cr3 != cr3&& p2m->cr3 != CR3_EADDR) || (p2m != nv->nv_p2m)) >>> - continue; >>> - >>> + p2m = nv->nv_p2m; >>> + if ( p2m&& (p2m->cr3 == cr3 || p2m->cr3 == CR3_EADDR) ) >>> + { >>> nv->nv_flushp2m = 0; >>> p2m_getlru_nestedp2m(d, p2m); >>> nv->nv_p2m = p2m; >>> >> >> >> Ok, thanks. >> >> In p2m_get_nestedp2m() replace this code hunk >> >> for (i = 0; i< MAX_NESTEDP2M; i++) { >> p2m = p2m_getlru_nestedp2m(d, NULL); >> p2m_flush_locked(p2m); >> } >> >> with >> >> p2m = p2m_getlru_nestedp2m(d, NULL); >> p2m_flush_locked(p2m); > > That seems like an improvement. I'll put it into my queue. > > More generally, I think that you need to figure out exactly what > behaviour you want from this function. For example in the current code > there's no way that two vcpus with the same ncr3 value can share a > nested-p2m. Is that deliberate? By 'current code' do you mean with or w/o this patch ? It is deliberate that two vcpus with the same ncr3 share a nested-p2m. But fixing the p2m locking problem in upstream tree has a higher priority right now and we can work on that after the p2m locking issue is fixed upstream. Christoph -- ---to satisfy European Law for business letters: Advanced Micro Devices GmbH Einsteinring 24, 85689 Dornach b. Muenchen Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen Registergericht Muenchen, HRB Nr. 43632