Re: [Xen-devel] [PATCH RFC 1/4] xen: evtchn: make evtchn_reset() ready for soft reset
"Jan Beulich" writes:
On 03.06.15 at 15:35, wrote:
>> --- a/xen/common/event_channel.c
>> +++ b/xen/common/event_channel.c
>> @@ -274,11 +274,13 @@ static long
>> evtchn_bind_interdomain(evtchn_bind_interdomain_t *bind)
>>
>> lchn->u.interdomain.remote_dom = rd;
>> lchn->u.interdomain.remote_port = rport;
>> +lchn->u.interdomain.opened_by = current->domain;
>> lchn->state = ECS_INTERDOMAIN;
>> evtchn_port_init(ld, lchn);
>>
>> rchn->u.interdomain.remote_dom = ld;
>> rchn->u.interdomain.remote_port = lport;
>> +rchn->u.interdomain.opened_by = current->domain;
>> rchn->state = ECS_INTERDOMAIN;
>
> For one ld == current->domain. And I don't think you need to store
> domain pointers here; storing the domain ID would seem sufficient
> (with the nice benefit of not growing struct evtchn). Plus
> "opened_by" doesn't really reflect what is being done here - the
> event channels are being bound, not opened.
>
>> @@ -933,26 +935,30 @@ int evtchn_unmask(unsigned int port)
>> }
>>
>>
>> -static long evtchn_reset(evtchn_reset_t *r)
>> +void evtchn_reset(struct domain *d, bool_t soft_reset)
>> {
>> -domid_t dom = r->dom;
>> -struct domain *d;
>> -int i, rc;
>> -
>> -d = rcu_lock_domain_by_any_id(dom);
>> -if ( d == NULL )
>> -return -ESRCH;
>> -
>> -rc = xsm_evtchn_reset(XSM_TARGET, current->domain, d);
>> -if ( rc )
>> -goto out;
>> +int i;
>
> unsigned int
>
>> +struct evtchn *chn;
>
> const (and perhaps moved inside the loop below)
>
>> +/*
>> + * ECS_INTERDOMAIN channels with port number suitable for the 2-level
>> ABI
>> + * opened by other domains should remain opened as the domain doing soft
>> + * reset won't be able to reopen them.
>
> One question of course is - does this really apply to _all_ interdomain
> event channels (rather than just to the xenstore and xenconsole ones)?
The wild guess here is that if a channel was bound by someone else the
domain doing this reset operation won't probably be able (and allowed)
to bind such interdomain connectivity so we need to keep it, from the
hypervisor PoV xenstore and xenconsole channels are not special. We,
however, won't probably need that in case we go for the
toolstack-assisted approach (and as far as I understand there are no
objections against such approach for now but I'll have to prototype it
so we can make a decision.)
--
Vitaly
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH RFC 1/4] xen: evtchn: make evtchn_reset() ready for soft reset
>>> On 03.06.15 at 15:35, wrote:
> --- a/xen/common/event_channel.c
> +++ b/xen/common/event_channel.c
> @@ -274,11 +274,13 @@ static long
> evtchn_bind_interdomain(evtchn_bind_interdomain_t *bind)
>
> lchn->u.interdomain.remote_dom = rd;
> lchn->u.interdomain.remote_port = rport;
> +lchn->u.interdomain.opened_by = current->domain;
> lchn->state = ECS_INTERDOMAIN;
> evtchn_port_init(ld, lchn);
>
> rchn->u.interdomain.remote_dom = ld;
> rchn->u.interdomain.remote_port = lport;
> +rchn->u.interdomain.opened_by = current->domain;
> rchn->state = ECS_INTERDOMAIN;
For one ld == current->domain. And I don't think you need to store
domain pointers here; storing the domain ID would seem sufficient
(with the nice benefit of not growing struct evtchn). Plus
"opened_by" doesn't really reflect what is being done here - the
event channels are being bound, not opened.
> @@ -933,26 +935,30 @@ int evtchn_unmask(unsigned int port)
> }
>
>
> -static long evtchn_reset(evtchn_reset_t *r)
> +void evtchn_reset(struct domain *d, bool_t soft_reset)
> {
> -domid_t dom = r->dom;
> -struct domain *d;
> -int i, rc;
> -
> -d = rcu_lock_domain_by_any_id(dom);
> -if ( d == NULL )
> -return -ESRCH;
> -
> -rc = xsm_evtchn_reset(XSM_TARGET, current->domain, d);
> -if ( rc )
> -goto out;
> +int i;
unsigned int
> +struct evtchn *chn;
const (and perhaps moved inside the loop below)
> +/*
> + * ECS_INTERDOMAIN channels with port number suitable for the 2-level ABI
> + * opened by other domains should remain opened as the domain doing soft
> + * reset won't be able to reopen them.
One question of course is - does this really apply to _all_ interdomain
event channels (rather than just to the xenstore and xenconsole ones)?
Jan
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH RFC 1/4] xen: evtchn: make evtchn_reset() ready for soft reset
>>> On 05.06.15 at 10:58, wrote: > On Fri, 2015-06-05 at 09:52 +0100, Jan Beulich wrote: >> >>> On 04.06.15 at 17:47, wrote: >> > At 16:19 +0100 on 04 Jun (1433434780), David Vrabel wrote: >> >> On 04/06/15 15:05, Tim Deegan wrote: >> >> > At 15:35 +0200 on 03 Jun (1433345719), Vitaly Kuznetsov wrote: >> >> >> We need to close all event channel so the domain performing soft reset >> >> >> will be able to open them back. Interdomain channels are, however, >> >> >> special. We need to keep track of who opened it as in (the most common) >> >> >> case it was opened by the control domain we won't be able (and >> >> >> allowed) to >> >> >> re-establish it. >> >> > >> >> > >> >> > I'm not sure I understand -- can you give an example of what this is >> >> > avoiding? I would have thought that the kexec'ing VM needs to tear >> >> > down _all_ its connections and then restart the ones it wantrs in the >> >> > new OS. >> >> >> >> There are some that are in state ECS_UNBOUND (console, xenstore, I >> >> think) at start of day that are allocated on behalf of the domain by the >> >> toolstack. The kexec'd image will expect them in these same state. >> > >> > I see. But does that not come under "tear down all its connections >> > and then restart the ones it wants"? I.e. should the guest not reset >> > all its channels and then allocate fresh console & xenstore ones? >> >> But just like during boot the guest doesn't (and can't) created these, >> it also can't do so after kexec. The remote side needs to be set up >> for it, so it can simply bind to them. I don't think this can work without >> tool stack involvement. > > Which IIRC is pretty much the logic which lead to the more toolstack > centric approach in the other series. Are you suggesting something in > the middle? The main issue with the other series was the moving of all the memory pages of the guest; I didn't mind there being tool stack parts (and considering the split of duties between hypervisor and tool stack it would also seem suspicious if domain reset could be implemented entirely in the hypervisor). Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH RFC 1/4] xen: evtchn: make evtchn_reset() ready for soft reset
On Fri, 2015-06-05 at 09:52 +0100, Jan Beulich wrote: > >>> On 04.06.15 at 17:47, wrote: > > At 16:19 +0100 on 04 Jun (1433434780), David Vrabel wrote: > >> On 04/06/15 15:05, Tim Deegan wrote: > >> > At 15:35 +0200 on 03 Jun (1433345719), Vitaly Kuznetsov wrote: > >> >> We need to close all event channel so the domain performing soft reset > >> >> will be able to open them back. Interdomain channels are, however, > >> >> special. We need to keep track of who opened it as in (the most common) > >> >> case it was opened by the control domain we won't be able (and allowed) > >> >> to > >> >> re-establish it. > >> > > >> > > >> > I'm not sure I understand -- can you give an example of what this is > >> > avoiding? I would have thought that the kexec'ing VM needs to tear > >> > down _all_ its connections and then restart the ones it wantrs in the > >> > new OS. > >> > >> There are some that are in state ECS_UNBOUND (console, xenstore, I > >> think) at start of day that are allocated on behalf of the domain by the > >> toolstack. The kexec'd image will expect them in these same state. > > > > I see. But does that not come under "tear down all its connections > > and then restart the ones it wants"? I.e. should the guest not reset > > all its channels and then allocate fresh console & xenstore ones? > > But just like during boot the guest doesn't (and can't) created these, > it also can't do so after kexec. The remote side needs to be set up > for it, so it can simply bind to them. I don't think this can work without > tool stack involvement. Which IIRC is pretty much the logic which lead to the more toolstack centric approach in the other series. Are you suggesting something in the middle? ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH RFC 1/4] xen: evtchn: make evtchn_reset() ready for soft reset
>>> On 04.06.15 at 17:47, wrote: > At 16:19 +0100 on 04 Jun (1433434780), David Vrabel wrote: >> On 04/06/15 15:05, Tim Deegan wrote: >> > At 15:35 +0200 on 03 Jun (1433345719), Vitaly Kuznetsov wrote: >> >> We need to close all event channel so the domain performing soft reset >> >> will be able to open them back. Interdomain channels are, however, >> >> special. We need to keep track of who opened it as in (the most common) >> >> case it was opened by the control domain we won't be able (and allowed) to >> >> re-establish it. >> > >> > >> > I'm not sure I understand -- can you give an example of what this is >> > avoiding? I would have thought that the kexec'ing VM needs to tear >> > down _all_ its connections and then restart the ones it wantrs in the >> > new OS. >> >> There are some that are in state ECS_UNBOUND (console, xenstore, I >> think) at start of day that are allocated on behalf of the domain by the >> toolstack. The kexec'd image will expect them in these same state. > > I see. But does that not come under "tear down all its connections > and then restart the ones it wants"? I.e. should the guest not reset > all its channels and then allocate fresh console & xenstore ones? But just like during boot the guest doesn't (and can't) created these, it also can't do so after kexec. The remote side needs to be set up for it, so it can simply bind to them. I don't think this can work without tool stack involvement. Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH RFC 1/4] xen: evtchn: make evtchn_reset() ready for soft reset
At 16:19 +0100 on 04 Jun (1433434780), David Vrabel wrote: > On 04/06/15 15:05, Tim Deegan wrote: > > At 15:35 +0200 on 03 Jun (1433345719), Vitaly Kuznetsov wrote: > >> We need to close all event channel so the domain performing soft reset > >> will be able to open them back. Interdomain channels are, however, > >> special. We need to keep track of who opened it as in (the most common) > >> case it was opened by the control domain we won't be able (and allowed) to > >> re-establish it. > > > > > > I'm not sure I understand -- can you give an example of what this is > > avoiding? I would have thought that the kexec'ing VM needs to tear > > down _all_ its connections and then restart the ones it wantrs in the > > new OS. > > There are some that are in state ECS_UNBOUND (console, xenstore, I > think) at start of day that are allocated on behalf of the domain by the > toolstack. The kexec'd image will expect them in these same state. I see. But does that not come under "tear down all its connections and then restart the ones it wants"? I.e. should the guest not reset all its channels and then allocate fresh console & xenstore ones? Tim. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH RFC 1/4] xen: evtchn: make evtchn_reset() ready for soft reset
On 04/06/15 15:05, Tim Deegan wrote: > At 15:35 +0200 on 03 Jun (1433345719), Vitaly Kuznetsov wrote: >> We need to close all event channel so the domain performing soft reset >> will be able to open them back. Interdomain channels are, however, >> special. We need to keep track of who opened it as in (the most common) >> case it was opened by the control domain we won't be able (and allowed) to >> re-establish it. > > > I'm not sure I understand -- can you give an example of what this is > avoiding? I would have thought that the kexec'ing VM needs to tear > down _all_ its connections and then restart the ones it wantrs in the > new OS. There are some that are in state ECS_UNBOUND (console, xenstore, I think) at start of day that are allocated on behalf of the domain by the toolstack. The kexec'd image will expect them in these same state. David ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH RFC 1/4] xen: evtchn: make evtchn_reset() ready for soft reset
At 15:35 +0200 on 03 Jun (1433345719), Vitaly Kuznetsov wrote:
> We need to close all event channel so the domain performing soft reset
> will be able to open them back. Interdomain channels are, however,
> special. We need to keep track of who opened it as in (the most common)
> case it was opened by the control domain we won't be able (and allowed) to
> re-establish it.
I'm not sure I understand -- can you give an example of what this is
avoiding? I would have thought that the kexec'ing VM needs to tear
down _all_ its connections and then restart the ones it wantrs in the
new OS.
Cheers,
Tim.
> Signed-off-by: Vitaly Kuznetsov
> ---
> xen/common/event_channel.c | 52
> +++---
> xen/include/xen/event.h| 3 +++
> xen/include/xen/sched.h| 1 +
> 3 files changed, 35 insertions(+), 21 deletions(-)
>
> diff --git a/xen/common/event_channel.c b/xen/common/event_channel.c
> index fae242d..3204c74 100644
> --- a/xen/common/event_channel.c
> +++ b/xen/common/event_channel.c
> @@ -274,11 +274,13 @@ static long
> evtchn_bind_interdomain(evtchn_bind_interdomain_t *bind)
>
> lchn->u.interdomain.remote_dom = rd;
> lchn->u.interdomain.remote_port = rport;
> +lchn->u.interdomain.opened_by = current->domain;
> lchn->state = ECS_INTERDOMAIN;
> evtchn_port_init(ld, lchn);
>
> rchn->u.interdomain.remote_dom = ld;
> rchn->u.interdomain.remote_port = lport;
> +rchn->u.interdomain.opened_by = current->domain;
> rchn->state = ECS_INTERDOMAIN;
>
> /*
> @@ -933,26 +935,30 @@ int evtchn_unmask(unsigned int port)
> }
>
>
> -static long evtchn_reset(evtchn_reset_t *r)
> +void evtchn_reset(struct domain *d, bool_t soft_reset)
> {
> -domid_t dom = r->dom;
> -struct domain *d;
> -int i, rc;
> -
> -d = rcu_lock_domain_by_any_id(dom);
> -if ( d == NULL )
> -return -ESRCH;
> -
> -rc = xsm_evtchn_reset(XSM_TARGET, current->domain, d);
> -if ( rc )
> -goto out;
> +int i;
> +struct evtchn *chn;
>
> +/*
> + * ECS_INTERDOMAIN channels with port number suitable for the 2-level ABI
> + * opened by other domains should remain opened as the domain doing soft
> + * reset won't be able to reopen them.
> + * __evtchn_close() also leaves consumer_is_xen() channels open.
> + */
> for ( i = 0; port_is_valid(d, i); i++ )
> +{
> +chn = evtchn_from_port(d, i);
> +if ( !soft_reset ||
> + i >= (BITS_PER_EVTCHN_WORD(d) * BITS_PER_EVTCHN_WORD(d)) ||
> + chn->state != ECS_INTERDOMAIN ||
> + chn->u.interdomain.opened_by == d )
> (void)__evtchn_close(d, i);
> +}
>
> spin_lock(&d->event_lock);
>
> -if ( (dom == DOMID_SELF) && d->evtchn_fifo )
> +if ( (d == current->domain) && d->evtchn_fifo )
> {
> /*
> * Guest domain called EVTCHNOP_reset with DOMID_SELF, destroying
> @@ -964,13 +970,6 @@ static long evtchn_reset(evtchn_reset_t *r)
> }
>
> spin_unlock(&d->event_lock);
> -
> -rc = 0;
> -
> -out:
> -rcu_unlock_domain(d);
> -
> -return rc;
> }
>
> static long evtchn_set_priority(const struct evtchn_set_priority
> *set_priority)
> @@ -1097,9 +1096,20 @@ long do_event_channel_op(int cmd,
> XEN_GUEST_HANDLE_PARAM(void) arg)
>
> case EVTCHNOP_reset: {
> struct evtchn_reset reset;
> +struct domain *d;
> +
> if ( copy_from_guest(&reset, arg, 1) != 0 )
> return -EFAULT;
> -rc = evtchn_reset(&reset);
> +
> +d = rcu_lock_domain_by_any_id(reset.dom);
> +if ( d == NULL )
> +return -ESRCH;
> +
> +rc = xsm_evtchn_reset(XSM_TARGET, current->domain, d);
> +if ( !rc )
> +evtchn_reset(d, 0);
> +
> +rcu_unlock_domain(d);
> break;
> }
>
> diff --git a/xen/include/xen/event.h b/xen/include/xen/event.h
> index 690f865..d0479a6 100644
> --- a/xen/include/xen/event.h
> +++ b/xen/include/xen/event.h
> @@ -130,6 +130,9 @@ void evtchn_check_pollers(struct domain *d, unsigned int
> port);
>
> void evtchn_2l_init(struct domain *d);
>
> +/* Close all event channels and reset to 2-level ABI */
> +void evtchn_reset(struct domain *d, bool_t soft_reset);
> +
> /*
> * Low-level event channel port ops.
> */
> diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
> index 80c6f62..13b6b86 100644
> --- a/xen/include/xen/sched.h
> +++ b/xen/include/xen/sched.h
> @@ -98,6 +98,7 @@ struct evtchn
> struct {
> evtchn_port_t remote_port;
> struct domain *remote_dom;
> +struct domain *opened_by;
> } interdomain; /* state == ECS_INTERDOMAIN */
> struct {
> u32irq;
> --
> 1.9.3
>
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://list
[Xen-devel] [PATCH RFC 1/4] xen: evtchn: make evtchn_reset() ready for soft reset
We need to close all event channel so the domain performing soft reset
will be able to open them back. Interdomain channels are, however,
special. We need to keep track of who opened it as in (the most common)
case it was opened by the control domain we won't be able (and allowed) to
re-establish it.
Signed-off-by: Vitaly Kuznetsov
---
xen/common/event_channel.c | 52 +++---
xen/include/xen/event.h| 3 +++
xen/include/xen/sched.h| 1 +
3 files changed, 35 insertions(+), 21 deletions(-)
diff --git a/xen/common/event_channel.c b/xen/common/event_channel.c
index fae242d..3204c74 100644
--- a/xen/common/event_channel.c
+++ b/xen/common/event_channel.c
@@ -274,11 +274,13 @@ static long
evtchn_bind_interdomain(evtchn_bind_interdomain_t *bind)
lchn->u.interdomain.remote_dom = rd;
lchn->u.interdomain.remote_port = rport;
+lchn->u.interdomain.opened_by = current->domain;
lchn->state = ECS_INTERDOMAIN;
evtchn_port_init(ld, lchn);
rchn->u.interdomain.remote_dom = ld;
rchn->u.interdomain.remote_port = lport;
+rchn->u.interdomain.opened_by = current->domain;
rchn->state = ECS_INTERDOMAIN;
/*
@@ -933,26 +935,30 @@ int evtchn_unmask(unsigned int port)
}
-static long evtchn_reset(evtchn_reset_t *r)
+void evtchn_reset(struct domain *d, bool_t soft_reset)
{
-domid_t dom = r->dom;
-struct domain *d;
-int i, rc;
-
-d = rcu_lock_domain_by_any_id(dom);
-if ( d == NULL )
-return -ESRCH;
-
-rc = xsm_evtchn_reset(XSM_TARGET, current->domain, d);
-if ( rc )
-goto out;
+int i;
+struct evtchn *chn;
+/*
+ * ECS_INTERDOMAIN channels with port number suitable for the 2-level ABI
+ * opened by other domains should remain opened as the domain doing soft
+ * reset won't be able to reopen them.
+ * __evtchn_close() also leaves consumer_is_xen() channels open.
+ */
for ( i = 0; port_is_valid(d, i); i++ )
+{
+chn = evtchn_from_port(d, i);
+if ( !soft_reset ||
+ i >= (BITS_PER_EVTCHN_WORD(d) * BITS_PER_EVTCHN_WORD(d)) ||
+ chn->state != ECS_INTERDOMAIN ||
+ chn->u.interdomain.opened_by == d )
(void)__evtchn_close(d, i);
+}
spin_lock(&d->event_lock);
-if ( (dom == DOMID_SELF) && d->evtchn_fifo )
+if ( (d == current->domain) && d->evtchn_fifo )
{
/*
* Guest domain called EVTCHNOP_reset with DOMID_SELF, destroying
@@ -964,13 +970,6 @@ static long evtchn_reset(evtchn_reset_t *r)
}
spin_unlock(&d->event_lock);
-
-rc = 0;
-
-out:
-rcu_unlock_domain(d);
-
-return rc;
}
static long evtchn_set_priority(const struct evtchn_set_priority *set_priority)
@@ -1097,9 +1096,20 @@ long do_event_channel_op(int cmd,
XEN_GUEST_HANDLE_PARAM(void) arg)
case EVTCHNOP_reset: {
struct evtchn_reset reset;
+struct domain *d;
+
if ( copy_from_guest(&reset, arg, 1) != 0 )
return -EFAULT;
-rc = evtchn_reset(&reset);
+
+d = rcu_lock_domain_by_any_id(reset.dom);
+if ( d == NULL )
+return -ESRCH;
+
+rc = xsm_evtchn_reset(XSM_TARGET, current->domain, d);
+if ( !rc )
+evtchn_reset(d, 0);
+
+rcu_unlock_domain(d);
break;
}
diff --git a/xen/include/xen/event.h b/xen/include/xen/event.h
index 690f865..d0479a6 100644
--- a/xen/include/xen/event.h
+++ b/xen/include/xen/event.h
@@ -130,6 +130,9 @@ void evtchn_check_pollers(struct domain *d, unsigned int
port);
void evtchn_2l_init(struct domain *d);
+/* Close all event channels and reset to 2-level ABI */
+void evtchn_reset(struct domain *d, bool_t soft_reset);
+
/*
* Low-level event channel port ops.
*/
diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
index 80c6f62..13b6b86 100644
--- a/xen/include/xen/sched.h
+++ b/xen/include/xen/sched.h
@@ -98,6 +98,7 @@ struct evtchn
struct {
evtchn_port_t remote_port;
struct domain *remote_dom;
+struct domain *opened_by;
} interdomain; /* state == ECS_INTERDOMAIN */
struct {
u32irq;
--
1.9.3
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
