Re: [PATCH 52/59] KVM: arm64: nv: vgic: Allow userland to set VGIC maintenance IRQ

Julien Thierry
2019-07-04T09:05:04Z

On 04/07/2019 10:01, Andre Przywara wrote:
> On Thu, 4 Jul 2019 08:38:20 +0100
> Julien Thierry <julien.thierry@arm.com> wrote:
>
>> On 21/06/2019 10:38, Marc Zyngier wrote:
>>> From: Andre Przywara <andre.przywara@arm.com>
>>>
>>> The VGIC maintenance IRQ signals various conditions about the LRs, when
>>> the GIC's virtualization extension is used.
>>> So far we didn't need it, but nested virtualization needs to know about
>>> this interrupt, so add a userland interface to setup the IRQ number.
>>> The architecture mandates that it must be a PPI, on top of that this code
>>> only exports a per-device option, so the PPI is the same on all VCPUs.
>>>
>>> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
>>> [added some bits of documentation]
>>> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
>>> ---
>>> .../virtual/kvm/devices/arm-vgic-v3.txt | 9 ++++++++
>>> arch/arm/include/uapi/asm/kvm.h | 1 +
>>> arch/arm64/include/uapi/asm/kvm.h | 1 +
>>> include/kvm/arm_vgic.h | 3 +++
>>> virt/kvm/arm/vgic/vgic-kvm-device.c | 22 +++++++++++++++++++
>>> 5 files changed, 36 insertions(+)
>>>
>>> diff --git a/Documentation/virtual/kvm/devices/arm-vgic-v3.txt b/Documentation/virtual/kvm/devices/arm-vgic-v3.txt
>>> index ff290b43c8e5..c70e8f2e0c9c 100644
>>> --- a/Documentation/virtual/kvm/devices/arm-vgic-v3.txt
>>> +++ b/Documentation/virtual/kvm/devices/arm-vgic-v3.txt
>>> @@ -249,3 +249,12 @@ Groups:
>>> Errors:
>>> -EINVAL: vINTID is not multiple of 32 or
>>> info field is not VGIC_LEVEL_INFO_LINE_LEVEL
>>> +
>>> + KVM_DEV_ARM_VGIC_GRP_MAINT_IRQ
>>> + The attr field of kvm_device_attr encodes the following values:
>>> + bits: | 31 .... 5 | 4 .... 0 |
>>> + values: | RES0 | vINTID |
>>> +
>>> + The vINTID specifies which interrupt is generated when the vGIC
>>> + must generate a maintenance interrupt. This must be a PPI.
>>> +
>>
>> Something seems off. The documentation suggests that the value of the
>> attribute will be between 0-15 (and other values will be masked down to
>> a value between 0 and 15).
>
> Where does that happen? The mask is [4:0], so 5 bits, that should be enough for PPIs as well.
> We could add a line to the documentation to stress that this is an interrupt ID as seen by the virtual GIC, if that helps.
>

You're right, I misread the length of the vINTID field.

Nevermind then!

Thanks,

--
Julien Thierry