[PATCH v4 32/56] KVM: arm/arm64: vgic-new: Add SGIPENDR register handlers

[christoffer.dall@linaro.org (Christoffer Dall)]

On Thu, May 19, 2016 at 3:25 PM, Andre Przywara <andre.przywara@arm.com> wrote:
> Hi,
>
> On 18/05/16 14:50, Christoffer Dall wrote:
>> On Wed, May 18, 2016 at 02:31:18PM +0100, Andre Przywara wrote:
>>> Hi,
>>>
>>> On 18/05/16 14:14, Christoffer Dall wrote:
>>>> On Mon, May 16, 2016 at 10:53:20AM +0100, Andre Przywara wrote:
>>>>> As this register is v2 specific, its implementation lives entirely
>>>>> in vgic-mmio-v2.c.
>>>>> This register allows setting the source mask of an IPI.
>>>>>
>>>>> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
>>>>> Reviewed-by: Christoffer Dall <christoffer.dall@linaro.org>
>>>>> ---
>>>>> Changelog RFC..v1:
>>>>> - remove IRQ lock from read handler
>>>>> - update pending bit on setting the first / clearing the last bit
>>>>> - queue virtual IRQ if necessary
>>>>>
>>>>> Changelog v1 .. v2:
>>>>> - adapt to new MMIO framework
>>>>>
>>>>> Changelog v3 .. v4:
>>>>> - specify accessor width
>>>>>
>>>>>  virt/kvm/arm/vgic/vgic-mmio-v2.c | 62 ++++++++++++++++++++++++++++++++++++++--
>>>>>  1 file changed, 60 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/virt/kvm/arm/vgic/vgic-mmio-v2.c b/virt/kvm/arm/vgic/vgic-mmio-v2.c
>>>>> index c884e9b..3925d4c 100644
>>>>> --- a/virt/kvm/arm/vgic/vgic-mmio-v2.c
>>>>> +++ b/virt/kvm/arm/vgic/vgic-mmio-v2.c
>>>>> @@ -146,6 +146,64 @@ static void vgic_mmio_write_target(struct kvm_vcpu *vcpu,
>>>>>    }
>>>>>  }
>>>>>
>>>>> +static unsigned long vgic_mmio_read_sgipend(struct kvm_vcpu *vcpu,
>>>>> +                                      gpa_t addr, unsigned int len)
>>>>> +{
>>>>> +  u32 intid = addr & 0x0f;
>>>>
>>>> is there a reason why we cannot use the magic macro here?
>>>
>>> I wasn't sure about this, because it's not covering all 1024 interrupts,
>>> but just SGIs, so it's always fixed to 16 interrupts ? 8 bits. The
>>> default mask would be too big in this case.
>>> I guess it would work anyway because this region is limited to 16 bytes
>>> in our description, so we could use this here anyway to make it more
>>> aligned with the other handlers, maybe adding a comment about the
>>> difference?
>>>
>> meh, either way works for me actually, whatever you prefer.
>
> So for the records: we can't use the macro here, since it relies on
> masking and thus a certain alignment, which the SGIPENDR registers do
> not have (being mapped to 0xF10 and 0xF20, respectively). So the
> calculated intid would be much higher than the number of actually
> allocated interrupts - with all the consequences ;-)
>
Ah, yeah, didn't see that one coming.  Sorry for making you spend half
a day to give me an answer to a seemingly innocent question.

-Christoffer