| Message ID | 20240517011516.1451087-1-xin.wang2@amd.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | a3607581cd49c17128a486a526a36a97bafcb2bb |
| Headers | show |
| Series | [v2] drivers/xen: Improve the late XenStore init protocol | expand |
On Fri, 17 May 2024, Henry Wang wrote: > Currently, the late XenStore init protocol is only triggered properly > for the case that HVM_PARAM_STORE_PFN is ~0ULL (invalid). For the > case that XenStore interface is allocated but not ready (the connection > status is not XENSTORE_CONNECTED), Linux should also wait until the > XenStore is set up properly. > > Introduce a macro to describe the XenStore interface is ready, use > it in xenbus_probe_initcall() to select the code path of doing the > late XenStore init protocol or not. Since now we have more than one > condition for XenStore late init, rework the check in xenbus_probe() > for the free_irq(). > > Take the opportunity to enhance the check of the allocated XenStore > interface can be properly mapped, and return error early if the > memremap() fails. > > Fixes: 5b3353949e89 ("xen: add support for initializing xenstore later as HVM domain") > Signed-off-by: Henry Wang <xin.wang2@amd.com> > Signed-off-by: Michal Orzel <michal.orzel@amd.com> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org> > --- > v2: > - Use -EINVAL for the memremap() check. (Stefano) > - Add Fixes: tag. (Stefano) > - Rework the condition for free_irq() in xenbus_probe(). (Stefano) > --- > drivers/xen/xenbus/xenbus_probe.c | 36 ++++++++++++++++++++----------- > 1 file changed, 23 insertions(+), 13 deletions(-) > > diff --git a/drivers/xen/xenbus/xenbus_probe.c b/drivers/xen/xenbus/xenbus_probe.c > index 3205e5d724c8..1a9ded0cddcb 100644 > --- a/drivers/xen/xenbus/xenbus_probe.c > +++ b/drivers/xen/xenbus/xenbus_probe.c > @@ -65,13 +65,17 @@ > #include "xenbus.h" > > > -static int xs_init_irq; > +static int xs_init_irq = -1; > int xen_store_evtchn; > EXPORT_SYMBOL_GPL(xen_store_evtchn); > > struct xenstore_domain_interface *xen_store_interface; > EXPORT_SYMBOL_GPL(xen_store_interface); > > +#define XS_INTERFACE_READY \ > + ((xen_store_interface != NULL) && \ > + (xen_store_interface->connection == XENSTORE_CONNECTED)) > + > enum xenstore_init xen_store_domain_type; > EXPORT_SYMBOL_GPL(xen_store_domain_type); > > @@ -751,19 +755,19 @@ static void xenbus_probe(void) > { > xenstored_ready = 1; > > - if (!xen_store_interface) { > + if (!xen_store_interface) > xen_store_interface = memremap(xen_store_gfn << XEN_PAGE_SHIFT, > XEN_PAGE_SIZE, MEMREMAP_WB); > - /* > - * Now it is safe to free the IRQ used for xenstore late > - * initialization. No need to unbind: it is about to be > - * bound again from xb_init_comms. Note that calling > - * unbind_from_irqhandler now would result in xen_evtchn_close() > - * being called and the event channel not being enabled again > - * afterwards, resulting in missed event notifications. > - */ > + /* > + * Now it is safe to free the IRQ used for xenstore late > + * initialization. No need to unbind: it is about to be > + * bound again from xb_init_comms. Note that calling > + * unbind_from_irqhandler now would result in xen_evtchn_close() > + * being called and the event channel not being enabled again > + * afterwards, resulting in missed event notifications. > + */ > + if (xs_init_irq >= 0) > free_irq(xs_init_irq, &xb_waitq); > - } > > /* > * In the HVM case, xenbus_init() deferred its call to > @@ -822,7 +826,7 @@ static int __init xenbus_probe_initcall(void) > if (xen_store_domain_type == XS_PV || > (xen_store_domain_type == XS_HVM && > !xs_hvm_defer_init_for_callback() && > - xen_store_interface != NULL)) > + XS_INTERFACE_READY)) > xenbus_probe(); > > /* > @@ -831,7 +835,7 @@ static int __init xenbus_probe_initcall(void) > * started, then probe. It will be triggered when communication > * starts happening, by waiting on xb_waitq. > */ > - if (xen_store_domain_type == XS_LOCAL || xen_store_interface == NULL) { > + if (xen_store_domain_type == XS_LOCAL || !XS_INTERFACE_READY) { > struct task_struct *probe_task; > > probe_task = kthread_run(xenbus_probe_thread, NULL, > @@ -1014,6 +1018,12 @@ static int __init xenbus_init(void) > xen_store_interface = > memremap(xen_store_gfn << XEN_PAGE_SHIFT, > XEN_PAGE_SIZE, MEMREMAP_WB); > + if (!xen_store_interface) { > + pr_err("%s: cannot map HVM_PARAM_STORE_PFN=%llx\n", > + __func__, v); > + err = -EINVAL; > + goto out_error; > + } > if (xen_store_interface->connection != XENSTORE_CONNECTED) > wait = true; > } > -- > 2.34.1 >
diff --git a/drivers/xen/xenbus/xenbus_probe.c b/drivers/xen/xenbus/xenbus_probe.c index 3205e5d724c8..1a9ded0cddcb 100644 --- a/drivers/xen/xenbus/xenbus_probe.c +++ b/drivers/xen/xenbus/xenbus_probe.c @@ -65,13 +65,17 @@ #include "xenbus.h" -static int xs_init_irq; +static int xs_init_irq = -1; int xen_store_evtchn; EXPORT_SYMBOL_GPL(xen_store_evtchn); struct xenstore_domain_interface *xen_store_interface; EXPORT_SYMBOL_GPL(xen_store_interface); +#define XS_INTERFACE_READY \ + ((xen_store_interface != NULL) && \ + (xen_store_interface->connection == XENSTORE_CONNECTED)) + enum xenstore_init xen_store_domain_type; EXPORT_SYMBOL_GPL(xen_store_domain_type); @@ -751,19 +755,19 @@ static void xenbus_probe(void) { xenstored_ready = 1; - if (!xen_store_interface) { + if (!xen_store_interface) xen_store_interface = memremap(xen_store_gfn << XEN_PAGE_SHIFT, XEN_PAGE_SIZE, MEMREMAP_WB); - /* - * Now it is safe to free the IRQ used for xenstore late - * initialization. No need to unbind: it is about to be - * bound again from xb_init_comms. Note that calling - * unbind_from_irqhandler now would result in xen_evtchn_close() - * being called and the event channel not being enabled again - * afterwards, resulting in missed event notifications. - */ + /* + * Now it is safe to free the IRQ used for xenstore late + * initialization. No need to unbind: it is about to be + * bound again from xb_init_comms. Note that calling + * unbind_from_irqhandler now would result in xen_evtchn_close() + * being called and the event channel not being enabled again + * afterwards, resulting in missed event notifications. + */ + if (xs_init_irq >= 0) free_irq(xs_init_irq, &xb_waitq); - } /* * In the HVM case, xenbus_init() deferred its call to @@ -822,7 +826,7 @@ static int __init xenbus_probe_initcall(void) if (xen_store_domain_type == XS_PV || (xen_store_domain_type == XS_HVM && !xs_hvm_defer_init_for_callback() && - xen_store_interface != NULL)) + XS_INTERFACE_READY)) xenbus_probe(); /* @@ -831,7 +835,7 @@ static int __init xenbus_probe_initcall(void) * started, then probe. It will be triggered when communication * starts happening, by waiting on xb_waitq. */ - if (xen_store_domain_type == XS_LOCAL || xen_store_interface == NULL) { + if (xen_store_domain_type == XS_LOCAL || !XS_INTERFACE_READY) { struct task_struct *probe_task; probe_task = kthread_run(xenbus_probe_thread, NULL, @@ -1014,6 +1018,12 @@ static int __init xenbus_init(void) xen_store_interface = memremap(xen_store_gfn << XEN_PAGE_SHIFT, XEN_PAGE_SIZE, MEMREMAP_WB); + if (!xen_store_interface) { + pr_err("%s: cannot map HVM_PARAM_STORE_PFN=%llx\n", + __func__, v); + err = -EINVAL; + goto out_error; + } if (xen_store_interface->connection != XENSTORE_CONNECTED) wait = true; }
Currently, the late XenStore init protocol is only triggered properly for the case that HVM_PARAM_STORE_PFN is ~0ULL (invalid). For the case that XenStore interface is allocated but not ready (the connection status is not XENSTORE_CONNECTED), Linux should also wait until the XenStore is set up properly. Introduce a macro to describe the XenStore interface is ready, use it in xenbus_probe_initcall() to select the code path of doing the late XenStore init protocol or not. Since now we have more than one condition for XenStore late init, rework the check in xenbus_probe() for the free_irq(). Take the opportunity to enhance the check of the allocated XenStore interface can be properly mapped, and return error early if the memremap() fails. Fixes: 5b3353949e89 ("xen: add support for initializing xenstore later as HVM domain") Signed-off-by: Henry Wang <xin.wang2@amd.com> Signed-off-by: Michal Orzel <michal.orzel@amd.com> --- v2: - Use -EINVAL for the memremap() check. (Stefano) - Add Fixes: tag. (Stefano) - Rework the condition for free_irq() in xenbus_probe(). (Stefano) --- drivers/xen/xenbus/xenbus_probe.c | 36 ++++++++++++++++++++----------- 1 file changed, 23 insertions(+), 13 deletions(-)