[OpenBIOS] [PATCH] ppc: fix reuse of MMU translation slots

[Mark Cave-Ayland]

On 13/02/16 20:09, Alexander Graf wrote:

> On 02/12/2016 12:38 PM, Mark Cave-Ayland wrote:
>> On 10/02/16 23:25, Alyssa Milburn wrote:
>>
>>> The current code always picks slot 0 in the reuse case. (This doesn't
>>> seem
>>> to matter in the cases I've tested, though.)
>>>
>>> Signed-off-by: Alyssa Milburn <fuzzie at fuzzie.org>
>>> ---
>>>   arch/ppc/qemu/ofmem.c | 16 ++++++++++------
>>>   1 file changed, 10 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/arch/ppc/qemu/ofmem.c b/arch/ppc/qemu/ofmem.c
>>> index 6f346a3..c9fe154 100644
>>> --- a/arch/ppc/qemu/ofmem.c
>>> +++ b/arch/ppc/qemu/ofmem.c
>>> @@ -359,9 +359,11 @@ hash_page_64(unsigned long ea, phys_addr_t phys,
>>> ucell mode)
>>>               found = 1;
>>>         /* otherwise use a free slot */
>>> -    for (i = 0; !found && i < 8; i++)
>>> -        if (!pp[i].v)
>>> -            found = 1;
>>> +    if (!found) {
>>> +        for (i = 0; !found && i < 8; i++)
>>> +            if (!pp[i].v)
>>> +                found = 1;
>>> +    }
>>>         /* out of slots, just evict one */
>>>       if (!found)
>>> @@ -414,9 +416,11 @@ hash_page_32(unsigned long ea, phys_addr_t phys,
>>> ucell mode)
>>>               found = 1;
>>>         /* otherwise use a free slot */
>>> -    for (i = 0; !found && i < 8; i++)
>>> -        if (!pp[i].v)
>>> -            found = 1;
>>> +    if (!found) {
>>> +        for (i = 0; !found && i < 8; i++)
>>> +            if (!pp[i].v)
>>> +                found = 1;
>>> +    }
>>>         /* out of slots, just evict one */
>>>       if (!found)
>>>
>> Hmmm yes I think you're right there is a bug here. Assuming that we
>> locate the index of an old translation, then i should be set to the slot
>> number to replace. But even though found has been set, the subsequent
>> for() will always reset i to zero, even though it exits without
>> iterating.
>>
>> In theory this could mean that we have multiple entries in the hash
>> table for the same translation which can't be a good thing.
> 
> Yes, on real hardware machine checks can result from this.
> 
>>
>> Alex - does this look like the right solution to you?
> 
> Looks reasonable to me.

Thanks! Would you like to commit for a change? I get bored of seeing my
own name in the commit logs all the time ;)


ATB,

Mark.