From: Stefano Stabellini <sstabellini@kernel.org>
To: Henry Wang <xin.wang2@amd.com>
Cc: xen-devel@lists.xenproject.org, linux-kernel@vger.kernel.org,
Juergen Gross <jgross@suse.com>,
Stefano Stabellini <sstabellini@kernel.org>,
Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>,
Michal Orzel <michal.orzel@amd.com>
Subject: Re: [PATCH v2] drivers/xen: Improve the late XenStore init protocol
Date: Thu, 16 May 2024 18:21:55 -0700 (PDT) [thread overview]
Message-ID: <alpine.DEB.2.22.394.2405161821240.2544314@ubuntu-linux-20-04-desktop> (raw)
In-Reply-To: <20240517011516.1451087-1-xin.wang2@amd.com>
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
>
prev parent reply other threads:[~2024-05-17 1:21 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-05-17 1:15 [PATCH v2] drivers/xen: Improve the late XenStore init protocol Henry Wang
2024-05-17 1:21 ` Stefano Stabellini [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=alpine.DEB.2.22.394.2405161821240.2544314@ubuntu-linux-20-04-desktop \
--to=sstabellini@kernel.org \
--cc=jgross@suse.com \
--cc=linux-kernel@vger.kernel.org \
--cc=michal.orzel@amd.com \
--cc=oleksandr_tyshchenko@epam.com \
--cc=xen-devel@lists.xenproject.org \
--cc=xin.wang2@amd.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.