Re: [PATCH v2 kvmtool 03/10] irq: add irqfd helpers

[Jean-Philippe Brucker <jean-philippe.brucker@arm.com>]

On 31/07/17 18:55, Punit Agrawal wrote:
> Jean-Philippe Brucker <jean-philippe.brucker@arm.com> writes:
> 
>> Add helpers to add and remove IRQFD routing for both irqchips and MSIs.
>> We have to make a special case of IRQ lines on ARM where the
>> initialisation order goes like this:
>>
>>  (1) Devices reserve their IRQ lines
>>  (2) VGIC is setup with VGIC_CTRL_INIT (in a late_init call)
>>  (3) MSIs are reserved lazily, when the guest needs them
>>
>> Since we cannot setup IRQFD before (2), store the IRQFD routing for IRQ
>> lines temporarily until we're ready to submit them.
>>
>> Signed-off-by: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>
>> ---
>>  arm/gic.c                    | 74 +++++++++++++++++++++++++++++++++++++++++++-
>>  arm/include/arm-common/gic.h |  6 ++++
>>  hw/pci-shmem.c               |  8 +----
>>  include/kvm/irq.h            | 17 ++++++++++
>>  irq.c                        | 24 ++++++++++++++
>>  virtio/net.c                 |  9 ++----
>>  virtio/scsi.c                | 10 ++----
>>  7 files changed, 126 insertions(+), 22 deletions(-)
>>
> 
> [...]
> 
> 
>> diff --git a/arm/include/arm-common/gic.h b/arm/include/arm-common/gic.h
>> index 433dd237..0c279aca 100644
>> --- a/arm/include/arm-common/gic.h
>> +++ b/arm/include/arm-common/gic.h
>> @@ -33,4 +33,10 @@ int gic__alloc_irqnum(void);
>>  int gic__create(struct kvm *kvm, enum irqchip_type type);
>>  void gic__generate_fdt_nodes(void *fdt, enum irqchip_type type);
>>  
>> +int gic__add_irqfd(struct kvm *kvm, unsigned int gsi, int trigger_fd,
>> +		   int resample_fd);
>> +void gic__del_irqfd(struct kvm *kvm, unsigned int gsi, int trigger_fd);
>> +#define irq__add_irqfd gic__add_irqfd
>> +#define irq__del_irqfd gic__del_irqfd
>> +
>>  #endif /* ARM_COMMON__GIC_H */
>> diff --git a/hw/pci-shmem.c b/hw/pci-shmem.c
>> index 512b5b06..107043e9 100644
>> --- a/hw/pci-shmem.c
>> +++ b/hw/pci-shmem.c
>> @@ -127,7 +127,6 @@ static void callback_mmio_msix(struct kvm_cpu *vcpu, u64 addr, u8 *data, u32 len
>>  int pci_shmem__get_local_irqfd(struct kvm *kvm)
>>  {
>>  	int fd, gsi, r;
>> -	struct kvm_irqfd irqfd;
>>  
>>  	if (local_fd == 0) {
>>  		fd = eventfd(0, 0);
>> @@ -143,12 +142,7 @@ int pci_shmem__get_local_irqfd(struct kvm *kvm)
>>  			gsi = pci_shmem_pci_device.irq_line;
>>  		}
>>  
>> -		irqfd = (struct kvm_irqfd) {
>> -			.fd = fd,
>> -			.gsi = gsi,
>> -		};
>> -
>> -		r = ioctl(kvm->vm_fd, KVM_IRQFD, &irqfd);
>> +		r = irq__add_irqfd(kvm, gsi, fd, -1);
>>  		if (r < 0)
>>  			return r;
>>  
> 
> Not something you've introduced but I couldn't find any users of
> pci_shmem__get_local_irqfd(). Could this function be dropped - either as
> a pre-patch or separately?

Heh, indeed. The situation is the same for pci_shmem__add/remove_client. I
suppose it's an interface for another device that has never been added.

Cleaning up pci_shmem is another task altogether. It's not used and there
is no driver upstream. It should probably be removed altogether [1], but I
have enough patches in the pipe right now :)

Thanks,
Jean

[1] https://lists.cs.columbia.edu/pipermail/kvmarm/2015-June/015270.html