Re: [Xen-devel] [PATCH v4 3/5] xen/arm: vgic: Optimize the way to store the target vCPU in the rank

[Julien Grall <julien.grall@xxxxxxxxxx>]

On 23/10/15 10:34, Ian Campbell wrote:
> On Thu, 2015-10-22 at 18:15 +0100, Julien Grall wrote:
>> Hi Ian,
>>
>> On 22/10/15 17:17, Ian Campbell wrote:
>>> On Mon, 2015-10-12 at 15:22 +0100, Julien Grall wrote:
>>>> [...]
>>>>          /* Only migrate the vIRQ if the target vCPU has changed */
>>>>          if ( new_target != old_target )
>>>>          {
>>>> +            unsigned int virq = rank->index * NR_INTERRUPT_PER_RANK
>>>> + offset;
>>>
>>> FWIW this was the value of offset before it was shifted + masked, I
>>> think.
>>> Could you not just save it up top and remember it?
>>
>> In fact, the virq is already correctly set before the loop (see patch
>> #2):
>>
>> virq = rank->index * NR_INTERRUPT_PER_RANK + offset;
>>
>> The variable is incremented in the for loop. So I just forgot to drop
>> this line when I did the split.
>>
>> Not that it's not possible to use directly offset because for byte
>> access it will point to the byte modified and not the base address of
>> the register.
>>
>> Though, I could use a mask, but I find this solution clearer.
> 
> But per the above what is actually going to happen is you drop this change?

As said, the introduction of virq within this patch is a mistake.
Patch #2 already set virq before the loop:

offset &= INTERRUPT_RANK_MASK;
offset &= ~(NR_TARGET_PER_REG - 1);

virq = rank->index * NR_INTERRUPT_PER_RANK + offset;

for ( i = 0; i < NR_TARGET_PER_REG; i++, offset++, virq++ )


-- 
Julien Grall

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