Michael S. Tsirkin wrote:
> On Mon, Oct 26, 2009 at 12:22:03PM -0400, Gregory Haskins wrote:
>> Certain GSI's support lockless injecton, but we have no way to detect
>> which ones at the GSI level. Knowledge of this attribute will be
>> useful later in the series so that we can optimize irqfd injection
>> paths for cases where we know the code will not sleep. Therefore,
>> we provide an API to query a specific GSI.
>>
>> Signed-off-by: Gregory Haskins <ghask...@novell.com>
>> ---
>>
>> include/linux/kvm_host.h | 2 ++
>> virt/kvm/irq_comm.c | 35 ++++++++++++++++++++++++++++++++++-
>> 2 files changed, 36 insertions(+), 1 deletions(-)
>>
>> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
>> index 1fe135d..01151a6 100644
>> --- a/include/linux/kvm_host.h
>> +++ b/include/linux/kvm_host.h
>> @@ -119,6 +119,7 @@ struct kvm_memory_slot {
>> struct kvm_kernel_irq_routing_entry {
>> u32 gsi;
>> u32 type;
>> + bool lockless;
>
> So lockless is the same as type == MSI from below?
Yep, today anyway.
> If the idea is to make it extensible for the future,
> let's just add an inline function, we don't need a field for this.
>
This makes sense.
>> int (*set)(struct kvm_kernel_irq_routing_entry *e,
>> struct kvm *kvm, int irq_source_id, int level);
>> union {
>> @@ -420,6 +421,7 @@ void kvm_get_intr_delivery_bitmask(struct kvm_ioapic
>> *ioapic,
>> unsigned long *deliver_bitmask);
>> #endif
>> int kvm_set_irq(struct kvm *kvm, int irq_source_id, u32 irq, int level);
>> +int kvm_irq_check_lockless(struct kvm *kvm, u32 irq);
>> void kvm_notify_acked_irq(struct kvm *kvm, unsigned irqchip, unsigned pin);
>> void kvm_register_irq_ack_notifier(struct kvm *kvm,
>> struct kvm_irq_ack_notifier *kian);
>> diff --git a/virt/kvm/irq_comm.c b/virt/kvm/irq_comm.c
>> index db2553f..a7fd487 100644
>> --- a/virt/kvm/irq_comm.c
>> +++ b/virt/kvm/irq_comm.c
>> @@ -173,6 +173,35 @@ int kvm_set_irq(struct kvm *kvm, int irq_source_id, u32
>> irq, int level)
>> return ret;
>> }
>>
>> +int kvm_irq_check_lockless(struct kvm *kvm, u32 irq)
>> +{
>> + struct kvm_kernel_irq_routing_entry *e;
>> + struct kvm_irq_routing_table *irq_rt;
>> + struct hlist_node *n;
>> + int ret = -ENOENT;
>> + int idx;
>> +
>> + idx = srcu_read_lock(&kvm->irq_routing.srcu);
>> + irq_rt = rcu_dereference(kvm->irq_routing.table);
>> + if (irq < irq_rt->nr_rt_entries)
>> + hlist_for_each_entry(e, n, &irq_rt->map[irq], link) {
>> + if (!e->lockless) {
>> + /*
>> + * all destinations need to be lockless to
>> + * declare that the GSI as a whole is also
>> + * lockless
>> + */
>> + ret = 0;
>> + break;
>> + }
>> +
>> + ret = 1;
>> + }
>> + srcu_read_unlock(&kvm->irq_routing.srcu, idx);
>> +
>> + return ret;
>> +}
>> +
>> void kvm_notify_acked_irq(struct kvm *kvm, unsigned irqchip, unsigned pin)
>> {
>> struct kvm_irq_ack_notifier *kian;
>> @@ -310,18 +339,22 @@ static int setup_routing_entry(struct
>> kvm_irq_routing_table *rt,
>> int delta;
>> struct kvm_kernel_irq_routing_entry *ei;
>> struct hlist_node *n;
>> + bool lockless = ue->type == KVM_IRQ_ROUTING_MSI;
>>
>> /*
>> * Do not allow GSI to be mapped to the same irqchip more than once.
>> * Allow only one to one mapping between GSI and MSI.
>> + * Do not allow mixed lockless vs locked variants to coexist.
>
> Userspace has no idea which entries are lockless and which are not:
> this is an implementation detail - so it might not be able to avoid
> illegal combinations.
> Since this is called on an ioctl, can the rule be formulated in a way
> that makes sense for userspace?
>
I'm not sure.
>> */
>> hlist_for_each_entry(ei, n, &rt->map[ue->gsi], link)
>> if (ei->type == KVM_IRQ_ROUTING_MSI ||
>> - ue->u.irqchip.irqchip == ei->irqchip.irqchip)
>> + ue->u.irqchip.irqchip == ei->irqchip.irqchip ||
>> + ei->lockless != lockless)
>
> So this check seems like it does nothing, because lockless is same as
> MSI, and MSI is always 1:1? Intentional?
>
Yeah, it was more to guard-against/document the dependency, in case the
1:1 with MSI ever changes in the future.
Kind Regards,
-Greg
signature.asc
Description: OpenPGP digital signature
