Re: [RFC v7 7/7] KVM: arm: enable KVM_SIGNAL_MSI and MSI routing

["Radim Krčmář" <rkrcmar@redhat.com>]

2016-07-21 23:10+0200, Auger Eric:
> On 21/07/2016 18:33, Radim Krčmář wrote:
>> 2016-07-18 13:25+0000, Eric Auger:
>>> If the ITS modality is not available, let's simply support MSI
>>> injection by transforming the MSI.data into an SPI ID.
>>>
>>> This becomes possible to use KVM_SIGNAL_MSI ioctl and MSI
>>> routing for arm too.
>>>
>>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>>>
>>> ---
>>> diff --git a/virt/kvm/arm/vgic/vgic-irqfd.c b/virt/kvm/arm/vgic/vgic-irqfd.c
>>> +static int vgic_v2m_inject_msi(struct kvm *kvm, struct kvm_msi *msi)
>>> +{
>>> +	if (msi->flags & KVM_MSI_VALID_DEVID)
>>> +		return -EINVAL;
>>> +	if (!vgic_valid_spi(kvm, msi->data))
>>> +		return -EINVAL;
>>> +
>>> +	return kvm_vgic_inject_irq(kvm, 0, msi->data, 1);
>> 
>> Hm, this isn't very MSI related ...
>> 
>> arm already has KVM_IRQ_LINE/kvm_vm_ioctl_irq_line with
>> KVM_ARM_IRQ_TYPE_SPI that does
>>   kvm_vgic_inject_irq(kvm, 0, irq_num, level)
>> 
>> Is that interface lacking?
> 
> You mean KVM_SIGNAL_MSI? Well at QEMU level, for ARM/ARM64 is doesn't.

No, I meant KVM_IRQ_LINE, the one that is used to deliver SPI today.
Or isn't it?

> For kvm-tools I guess, Andre manages without.
> 
> My first feeling was it is part of the KVM API and we can implement it
> easily for GICv2M, as we do for GICv3 ITS . This can avoid a user app to
> do what QEMU implements as "kvm_gsi_direct_mapping" and manage the
> translation into the semantic of the ARM GSI.

I think that reusing KVM_SIGNAL_MSI and KVM_IRQ_ROUTING_MSI for SPI is
unfortunate.

SPI only uses msi.data, which makes remaining fields in the msi struct
arbitrary and [5/7] defined KVM_IRQ_ROUTING_IRQCHIP for SPI, so two
route types now do the same, but only sometimes (without ITS), which
makes the situation even less understandable ...

Delivering SPI as KVM_IRQ_ROUTING_IRQCHIP seems more sensible and if we
wanted ad-hoc delivery of KVM_IRQ_ROUTING_IRQCHIP, then I would prefer a
new interface to two different meanings for KVM_SIGNAL_MSI:
KVM_SIGNAL_MSI was created because we didn't have anything that could
inject an interrupt without setting up a route with KVM_SET_GSI_ROUTING
and we are still missing a generic interface to do that.

> But Well, if you prefer we do not implement it for GICv2M, since
> considered as far fetched I can remove this patch.

I do, thanks.  Documentation in [6/7] was ahead and needs changing then.
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm