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 <[email protected]>
Signed-off-by: Michal Orzel <[email protected]>
---
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
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 <[email protected]>
> Signed-off-by: Michal Orzel <[email protected]>
Reviewed-by: Stefano Stabellini <[email protected]>
> ---
> 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
>