| Message ID | 20220801211240.597859-10-quic_eberman@quicinc.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Drivers for gunyah hypervisor | expand |
On 02/08/2022 00:12, Elliot Berman wrote: > Gunyah resource manager exposes a concrete functionalities which > complicate a single resource manager driver. Use auxiliary bus > to help split high level functions for the resource manager and keep the > primary resource manager driver focused on the RPC with RM itself. > Delegate Resource Manager's console functionality to the auxiliary bus. > > Signed-off-by: Elliot Berman <quic_eberman@quicinc.com> > --- > drivers/virt/gunyah/rsc_mgr.c | 61 ++++++++++++++++++++++++++++++++++- > 1 file changed, 60 insertions(+), 1 deletion(-) > > diff --git a/drivers/virt/gunyah/rsc_mgr.c b/drivers/virt/gunyah/rsc_mgr.c > index b8268ee02fab..44b22cef7d44 100644 > --- a/drivers/virt/gunyah/rsc_mgr.c > +++ b/drivers/virt/gunyah/rsc_mgr.c > @@ -91,6 +91,11 @@ struct gh_rm_notif_complete { > struct work_struct work; > }; > > +struct gh_rsc_mgr_adev { > + struct auxiliary_device adev; > + struct list_head list; > +}; > + > struct gh_rsc_mgr { > struct task_struct *recv_task; > struct gunyah_device *msgq_tx, *msgq_rx; > @@ -99,6 +104,13 @@ struct gh_rsc_mgr { > struct mutex call_idr_lock; > > struct mutex send_lock; > + > + struct list_head adevs; > +}; > + > +/* List of auxiliary devices which resource manager creates */ > +static const char * const adev_names[] = { > + "console", > }; Which other auxilliary devices do you expect at this moment? > > static struct gh_rsc_mgr *__rsc_mgr; > @@ -516,6 +528,14 @@ int gh_rm_unregister_notifier(struct notifier_block *nb) > } > EXPORT_SYMBOL_GPL(gh_rm_unregister_notifier); > > +static void gh_rm_adev_release(struct device *dev) > +{ > + struct gh_rsc_mgr_adev *rm_adev = container_of(dev, struct gh_rsc_mgr_adev, adev.dev); > + > + list_del(&rm_adev->list); is there a race for the rsc_mgr->list? Can multiple release functions be called in parallel? Rather than having a list, it would be easier to have an array of devices. Less race conditions, simpler code. Or just add gh_rsc_msg->console_adev and use it directly without any additional bells and whistles. > + kfree(rm_adev); > +} > + > static struct gunyah_device *gh_msgq_platform_probe_direction(struct platform_device *pdev, > u8 gh_type, int idx) > {
On 8/2/2022 1:38 AM, Dmitry Baryshkov wrote: > On 02/08/2022 00:12, Elliot Berman wrote: >> Gunyah resource manager exposes a concrete functionalities which >> complicate a single resource manager driver. Use auxiliary bus >> to help split high level functions for the resource manager and keep the >> primary resource manager driver focused on the RPC with RM itself. >> Delegate Resource Manager's console functionality to the auxiliary bus. >> >> Signed-off-by: Elliot Berman <quic_eberman@quicinc.com> >> --- >> drivers/virt/gunyah/rsc_mgr.c | 61 ++++++++++++++++++++++++++++++++++- >> 1 file changed, 60 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/virt/gunyah/rsc_mgr.c >> b/drivers/virt/gunyah/rsc_mgr.c >> index b8268ee02fab..44b22cef7d44 100644 >> --- a/drivers/virt/gunyah/rsc_mgr.c >> +++ b/drivers/virt/gunyah/rsc_mgr.c >> @@ -91,6 +91,11 @@ struct gh_rm_notif_complete { >> struct work_struct work; >> }; >> +struct gh_rsc_mgr_adev { >> + struct auxiliary_device adev; >> + struct list_head list; >> +}; >> + >> struct gh_rsc_mgr { >> struct task_struct *recv_task; >> struct gunyah_device *msgq_tx, *msgq_rx; >> @@ -99,6 +104,13 @@ struct gh_rsc_mgr { >> struct mutex call_idr_lock; >> struct mutex send_lock; >> + >> + struct list_head adevs; >> +}; >> + >> +/* List of auxiliary devices which resource manager creates */ >> +static const char * const adev_names[] = { >> + "console", >> }; > > Which other auxilliary devices do you expect at this moment? > Only foresee a VM loader auxiliary device. >> static struct gh_rsc_mgr *__rsc_mgr; >> @@ -516,6 +528,14 @@ int gh_rm_unregister_notifier(struct >> notifier_block *nb) >> } >> EXPORT_SYMBOL_GPL(gh_rm_unregister_notifier); >> +static void gh_rm_adev_release(struct device *dev) >> +{ >> + struct gh_rsc_mgr_adev *rm_adev = container_of(dev, struct >> gh_rsc_mgr_adev, adev.dev); >> + >> + list_del(&rm_adev->list); > > is there a race for the rsc_mgr->list? Can multiple release functions be > called in parallel? > Rather than having a list, it would be easier to have an array of > devices. Less race conditions, simpler code. > > Or just add gh_rsc_msg->console_adev and use it directly without any > additional bells and whistles. > I like this approach, it is much simpler. >> + kfree(rm_adev); >> +} >> + >> static struct gunyah_device *gh_msgq_platform_probe_direction(struct >> platform_device *pdev, >> u8 gh_type, int idx) >> {
diff --git a/drivers/virt/gunyah/rsc_mgr.c b/drivers/virt/gunyah/rsc_mgr.c index b8268ee02fab..44b22cef7d44 100644 --- a/drivers/virt/gunyah/rsc_mgr.c +++ b/drivers/virt/gunyah/rsc_mgr.c @@ -91,6 +91,11 @@ struct gh_rm_notif_complete { struct work_struct work; }; +struct gh_rsc_mgr_adev { + struct auxiliary_device adev; + struct list_head list; +}; + struct gh_rsc_mgr { struct task_struct *recv_task; struct gunyah_device *msgq_tx, *msgq_rx; @@ -99,6 +104,13 @@ struct gh_rsc_mgr { struct mutex call_idr_lock; struct mutex send_lock; + + struct list_head adevs; +}; + +/* List of auxiliary devices which resource manager creates */ +static const char * const adev_names[] = { + "console", }; static struct gh_rsc_mgr *__rsc_mgr; @@ -516,6 +528,14 @@ int gh_rm_unregister_notifier(struct notifier_block *nb) } EXPORT_SYMBOL_GPL(gh_rm_unregister_notifier); +static void gh_rm_adev_release(struct device *dev) +{ + struct gh_rsc_mgr_adev *rm_adev = container_of(dev, struct gh_rsc_mgr_adev, adev.dev); + + list_del(&rm_adev->list); + kfree(rm_adev); +} + static struct gunyah_device *gh_msgq_platform_probe_direction(struct platform_device *pdev, u8 gh_type, int idx) { @@ -550,7 +570,9 @@ static struct gunyah_device *gh_msgq_platform_probe_direction(struct platform_de static int gh_rm_drv_probe(struct platform_device *pdev) { struct gh_rsc_mgr *rsc_mgr; - int ret; + struct gh_rsc_mgr_adev *rm_adev; + struct list_head *l, *n; + int ret, i; rsc_mgr = devm_kzalloc(&pdev->dev, sizeof(*rsc_mgr), GFP_KERNEL); if (!rsc_mgr) @@ -560,6 +582,7 @@ static int gh_rm_drv_probe(struct platform_device *pdev) mutex_init(&rsc_mgr->call_idr_lock); idr_init(&rsc_mgr->call_idr); mutex_init(&rsc_mgr->send_lock); + INIT_LIST_HEAD(&rsc_mgr->adevs); rsc_mgr->msgq_tx = gh_msgq_platform_probe_direction(pdev, GUNYAH_DEVICE_TYPE_MSGQ_TX, 0); if (IS_ERR(rsc_mgr->msgq_tx)) @@ -576,10 +599,38 @@ static int gh_rm_drv_probe(struct platform_device *pdev) goto err_msgq; } + for (i = 0; i < ARRAY_SIZE(adev_names); i++) { + rm_adev = kzalloc(sizeof(*rm_adev), GFP_KERNEL); + + rm_adev->adev.dev.parent = &pdev->dev; + rm_adev->adev.dev.release = gh_rm_adev_release; + rm_adev->adev.name = adev_names[i]; + ret = auxiliary_device_init(&rm_adev->adev); + if (ret) { + kfree(rm_adev); + goto err_adevs; + } + + list_add(&rm_adev->list, &rsc_mgr->adevs); + + ret = auxiliary_device_add(&rm_adev->adev); + if (ret) { + auxiliary_device_uninit(&rm_adev->adev); + goto err_adevs; + } + } + __rsc_mgr = rsc_mgr; return 0; +err_adevs: + list_for_each_safe(l, n, &rsc_mgr->adevs) { + rm_adev = container_of(l, struct gh_rsc_mgr_adev, list); + auxiliary_device_delete(&rm_adev->adev); + auxiliary_device_uninit(&rm_adev->adev); + } + err_msgq: gunyah_device_remove(rsc_mgr->msgq_rx); err_msgq_tx: @@ -590,6 +641,14 @@ static int gh_rm_drv_probe(struct platform_device *pdev) static int gh_rm_drv_remove(struct platform_device *pdev) { struct gh_rsc_mgr *rsc_mgr = platform_get_drvdata(pdev); + struct gh_rsc_mgr_adev *rm_adev; + struct list_head *l, *n; + + list_for_each_safe(l, n, &rsc_mgr->adevs) { + rm_adev = container_of(l, struct gh_rsc_mgr_adev, list); + auxiliary_device_delete(&rm_adev->adev); + auxiliary_device_uninit(&rm_adev->adev); + } gunyah_device_remove(rsc_mgr->msgq_tx); gunyah_device_remove(rsc_mgr->msgq_rx);
Gunyah resource manager exposes a concrete functionalities which complicate a single resource manager driver. Use auxiliary bus to help split high level functions for the resource manager and keep the primary resource manager driver focused on the RPC with RM itself. Delegate Resource Manager's console functionality to the auxiliary bus. Signed-off-by: Elliot Berman <quic_eberman@quicinc.com> --- drivers/virt/gunyah/rsc_mgr.c | 61 ++++++++++++++++++++++++++++++++++- 1 file changed, 60 insertions(+), 1 deletion(-)