RE: [v5 15/19] KVM: eventfd: add irq bypass consumer management

[Wu, Feng]

> -----Original Message-----
> From: Paolo Bonzini [mailto:pbonzini@xxxxxxxxxx]
> Sent: Monday, July 13, 2015 9:47 PM
> To: Eric Auger; Wu, Feng; kvm@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx
> Cc: alex.williamson@xxxxxxxxxx; joro@xxxxxxxxxx
> Subject: Re: [v5 15/19] KVM: eventfd: add irq bypass consumer management
> 
>  13/07/2015 15:16, Eric Auger wrote:
> >> >
> >> > +	irqfd->consumer.token = (void *)irqfd->eventfd;
> >> > +	kvm_arch_irq_consumer_init(&irqfd->consumer);
> > what if the architecture does not implement kvm_arch_irq_consumer_init?
> >
> > Also you are using here this single function kvm_arch_irq_consumer_init
> > to do some irq bypass manager settings + attaching your
> > irqfd->arch_update cb which does not really relate to IRQ bypass
> > manager. I think I preferred the approach where start/top/add/del were
> > exposed separately ([RFC v2 5/6] KVM: introduce kvm_arch functions for
> > IRQ bypass).
> >
> > Why not adding another kvm_arch_irq_routing_update then, not necessarily
> > linked to irq bypass manager.
> 
> Yes, I also preferred the dummy kvm_arch_* functions to this approach
> with an init function.  You'd have to add dummy init functions anyway
> for non-ARM, non-x86 architectures.

I think dummy kvm_arch_* is okay for me. However, my point is that currently
'add_producer ' and 'del_producer ' are mandatory, other callbacks are optional.
In patch "[RFC v2 5/6] KVM: introduce kvm_arch functions for IRQ bypass " and
"[RFC v2 6/6] KVM: eventfd: add irq bypass consumer management ", it
provides all the callbacks, which means we need to implement dummy arch
specific functions no matter it is necessary. In that case, seems it is pointless
to make some of the callbacks optional. Anyway, if you guys are fine with the
dummy approach, I am good! :)

Thanks,
Feng

> 
> Paolo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/