Re: [PATCH 3 of 4] Nested p2m: clarify logic in p2m_get_nestedp2m()

[Christoph Egger <Christoph.Egger@amd.com>]

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