Am 03.11.2010 23:13, Marcelo Tosatti wrote:
> On Wed, Nov 03, 2010 at 09:11:13AM +0100, Jan Kiszka wrote:
>> From: Jan Kiszka <jan.kiszka@xxxxxxxxxxx>
>>
>> This improves the IRQ forwarding for assigned devices: By using the
>> kernel's threaded IRQ scheme, we can get rid of the latency-prone work
>> queue and simplify the code in the same run.
>>
>> Moreover, we no longer have to hold assigned_dev_lock while raising the
>> guest IRQ, which can be a lenghty operation as we may have to iterate
>> over all VCPUs. The lock is now only used for synchronizing masking vs.
>> unmasking of INTx-type IRQs, thus is renames to intx_lock.
>>
>> Signed-off-by: Jan Kiszka <jan.kiszka@xxxxxxxxxxx>
>> ---
>> include/linux/kvm_host.h | 12 +----
>> virt/kvm/assigned-dev.c | 106 ++++++++++++++-------------------------------
>> 2 files changed, 35 insertions(+), 83 deletions(-)
>>
>> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
>> index bcf71c7..eaacb5d 100644
>> --- a/include/linux/kvm_host.h
>> +++ b/include/linux/kvm_host.h
>> @@ -457,16 +457,8 @@ struct kvm_irq_ack_notifier {
>> void (*irq_acked)(struct kvm_irq_ack_notifier *kian);
>> };
>>
>> -#define KVM_ASSIGNED_MSIX_PENDING 0x1
>> -struct kvm_guest_msix_entry {
>> - u32 vector;
>> - u16 entry;
>> - u16 flags;
>> -};
>> -
>> struct kvm_assigned_dev_kernel {
>> struct kvm_irq_ack_notifier ack_notifier;
>> - struct work_struct interrupt_work;
>> struct list_head list;
>> int assigned_dev_id;
>> int host_segnr;
>> @@ -477,13 +469,13 @@ struct kvm_assigned_dev_kernel {
>> bool host_irq_disabled;
>> struct msix_entry *host_msix_entries;
>> int guest_irq;
>> - struct kvm_guest_msix_entry *guest_msix_entries;
>> + struct msix_entry *guest_msix_entries;
>> unsigned long irq_requested_type;
>> int irq_source_id;
>> int flags;
>> struct pci_dev *dev;
>> struct kvm *kvm;
>> - spinlock_t assigned_dev_lock;
>> + spinlock_t intx_lock;
>> };
>>
>> struct kvm_irq_mask_notifier {
>> diff --git a/virt/kvm/assigned-dev.c b/virt/kvm/assigned-dev.c
>> index ecc4419..d0b07ea 100644
>> --- a/virt/kvm/assigned-dev.c
>> +++ b/virt/kvm/assigned-dev.c
>> @@ -55,58 +55,31 @@ static int find_index_from_host_irq(struct kvm_assigned_dev_kernel
>> return index;
>> }
>>
>> -static void kvm_assigned_dev_interrupt_work_handler(struct work_struct *work)
>> +static irqreturn_t kvm_assigned_dev_thread(int irq, void *dev_id)
>> {
>> - struct kvm_assigned_dev_kernel *assigned_dev;
>> - int i;
>> + struct kvm_assigned_dev_kernel *assigned_dev = dev_id;
>> + u32 vector;
>> + int index;
>>
>> - assigned_dev = container_of(work, struct kvm_assigned_dev_kernel,
>> - interrupt_work);
>> + if (assigned_dev->irq_requested_type & KVM_DEV_IRQ_HOST_INTX) {
>> + spin_lock(&assigned_dev->intx_lock);
>> + disable_irq_nosync(irq);
>> + assigned_dev->host_irq_disabled = true;
>> + spin_unlock(&assigned_dev->intx_lock);
>> + }
>
> Shouldnt this happen in the hardirq handler? Otherwise host will receive
> interrupts between ->eoi and this point.
>
Yeah, there is indeed still a bug. I thought the kernel would
automatically keep the line masked until the threaded handler returned,
but that's only the case if we set IRQF_ONESHOT. Will fix in v4.
Thanks,
Jan
Attachment:
signature.asc
Description: OpenPGP digital signature
