On 2020/7/28 下午5:18, Zhu, Lingshan wrote:
* status to 0.
@@ -167,6 +220,15 @@ static long vhost_vdpa_set_status(struct
vhost_vdpa *v, u8 __user *statusp)
if (status != 0 && (ops->get_status(vdpa) & ~status) != 0)
return -EINVAL;
+ /* vq irq is not expected to be changed once DRIVER_OK is set */
So this basically limit the usage of get_vq_irq() in the context
vhost_vdpa_set_status() and other vDPA bus drivers' set_status(). If
this is true, there's even no need to introduce any new config ops
but just let set_status() to return the irqs used for the device. Or
if we want this to be more generic, we need vpda's own irq manager
(which should be similar to irq bypass manager). That is:
I think there is no need for a driver to free / re-request its irqs after DRIVER_OK though it can do so. If a driver changed its irq of a vq after DRIVER_OK, the vq is still operational but will lose irq offloading that is reasonable.
If we want set_status() return irqs, we need to record the irqs somewhere in vdpa_device,
Why, we can simply pass an array to the driver I think?
void (*set_status)(struct vdpa_device *vdev, u8 status, int *irqs);
as we discussed in a previous thread, this may need initialize and cleanup works, so a new ops
with proper comments (don't say they could not change irq, but highlight if irq changes, irq offloading will not work till next DRIVER_OK) could be more elegant.
However if we really need to change irq after DRIVER_OK, I think maybe we still need vDPA vq irq allocate / free helpers, then the helpers can not be used in probe() as we discussed before, it is a step back to V3 series.
Still, it's not about whether driver may change irq after DRIVER_OK but
implication of the assumption. If one bus ops must be called in another
ops, it's better to just implement them in one ops.
- bus driver can register itself as consumer
- vDPA device driver can register itself as producer
- matching via queue index
IMHO, is it too heavy for this feature,
Do you mean LOCs? We can:
1) refactor irq bypass manager
2) invent it our own (a much simplified version compared to bypass manager)
3) enforcing them via vDPA bus
Each of the above should be not a lot of coding. I think method 3 is
partially done in your previous series but in an implicit manner:
- bus driver that has alloc_irq/free_irq implemented could be implicitly
treated as consumer registering
- every vDPA device driver could be treated as producer
- vdpa_devm_alloc_irq() could be treated as producer registering
- alloc_irq/free_irq is the add_producer/del_procuer
We probably just lack some synchronization with driver probe/remove.
and how can they match if two individual adapters both have vq idx = 1.
The matching is per vDPA device.
Thanks
Thanks!
- deal with registering/unregistering of consumer/producer
So there's no need to care when or where the vDPA device driver to
allocate the irq, and we don't need to care at which context the vDPA
bus driver can use the irq. Either side may get notified when the
other side is gone (though we only care about the gone of producer
probably).
Any thought on this?
Thanks
