Re: [PATCH v2 20/25] arm/altp2m: Add altp2m paging mechanism.

[Sergej Proskurin <proskurin@sec.in.tum.de>]

Hi Julien,


On 08/11/2016 10:47 AM, Julien Grall wrote:
>
>
> On 10/08/2016 11:32, Sergej Proskurin wrote:
>> Hi Julien,
>
> Hello Sergej,
>
>>>>> [...]
>>>>>
>>>>>>      switch ( fsc )
>>>>>>      {
>>>>>> +    case FSC_FLT_TRANS:
>>>>>> +    {
>>>>>> +        if ( altp2m_active(d) )
>>>>>> +        {
>>>>>> +            const struct npfec npfec = {
>>>>>> +                .insn_fetch = 1,
>>>>>> +                .gla_valid = 1,
>>>>>> +                .kind = hsr.iabt.s1ptw ? npfec_kind_in_gpt :
>>>>>> npfec_kind_with_gla
>>>>>> +            };
>>>>>> +
>>>>>> +            /*
>>>>>> +             * Copy the entire page of the failing instruction
>>>>>> into the
>>>>>> +             * currently active altp2m view.
>>>>>> +             */
>>>>>> +            if ( altp2m_lazy_copy(v, gpa, gva, npfec, &p2m) )
>>>>>> +                return;
>>>>>
>>>>> I forgot to mention that I think there is a race condition here. If
>>>>> multiple vCPU (let say A and B) use the same altp2m, they may fault
>>>>> here.
>>>>>
>>>>> If vCPU A already fixed the fault, this function will return false
>>>>> and
>>>>> continue. So this will lead to inject an instruction abort to the
>>>>> guest.
>>>>>
>>
>> I have solved this issue as well:
>>
>> In altp2m_lazy_copy, we check whether the faulting address is already
>> mapped in the current altp2m view. The only reason why the current
>> altp2m should have a valid entry for the apparently faulting address is
>> that it was previously (almost simultaneously) mapped by another vcpu.
>> That is, if the mapping for the faulting address is valid in the altp2m,
>> we return true and hence let the guest retry (without injecting an
>> instruction/data abort exception) to access the address in question.
>
> I am afraid that your description does not match the implementation of
> altp2m_lazy_copy in this version of the patch series.
>
> If you find a valid entry in the altp2m, you will return 0 (i.e
> false). This will lead to inject an abort into the guest.

I was describing the way I have solved it in the new patch. I apologize
if I did not make that clear enough.

Best regards,
~Sergej

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