diff mbox series

[v2,25/29] tools/xenstored: map stubdom interface

Message ID 20231110160804.29021-26-jgross@suse.com (mailing list archive)
State Superseded
Headers show
Series tools: enable xenstore-stubdom to use 9pfs | expand

Commit Message

Jürgen Groß Nov. 10, 2023, 4:08 p.m. UTC
When running as stubdom, map the stubdom's Xenstore ring page in order
to support using the 9pfs frontend.

Signed-off-by: Juergen Gross <jgross@suse.com>
---
 tools/xenstored/core.c   |  2 ++
 tools/xenstored/domain.c | 27 ++++++++++++++++++++++++++-
 tools/xenstored/domain.h |  1 +
 3 files changed, 29 insertions(+), 1 deletion(-)

Comments

Julien Grall Nov. 13, 2023, 10:04 p.m. UTC | #1
Hi Juergen,

On 10/11/2023 16:08, Juergen Gross wrote:
> When running as stubdom, map the stubdom's Xenstore ring page in order
> to support using the 9pfs frontend.
> 
> Signed-off-by: Juergen Gross <jgross@suse.com>
> ---
>   tools/xenstored/core.c   |  2 ++
>   tools/xenstored/domain.c | 27 ++++++++++++++++++++++++++-
>   tools/xenstored/domain.h |  1 +
>   3 files changed, 29 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/xenstored/core.c b/tools/xenstored/core.c
> index b9ec50b34c..4a9d874f17 100644
> --- a/tools/xenstored/core.c
> +++ b/tools/xenstored/core.c
> @@ -2991,6 +2991,8 @@ int main(int argc, char *argv[])
>   		lu_read_state();
>   #endif
>   
> +	stubdom_init();
> +
>   	check_store();
>   
>   	/* Get ready to listen to the tools. */
> diff --git a/tools/xenstored/domain.c b/tools/xenstored/domain.c
> index fa17f68618..162b87b460 100644
> --- a/tools/xenstored/domain.c
> +++ b/tools/xenstored/domain.c
> @@ -37,6 +37,10 @@
>   #include <xenctrl.h>
>   #include <xen/grant_table.h>
>   
> +#ifdef __MINIOS__
> +#include <mini-os/xenbus.h>
> +#endif
> +
>   static xc_interface **xc_handle;
>   xengnttab_handle **xgt_handle;
>   static evtchn_port_t virq_port;
> @@ -500,6 +504,11 @@ static void *map_interface(domid_t domid)
>   	if (domid == xenbus_master_domid())
>   		return xenbus_map();
>   
> +#ifdef __MINIOS__
> +	if (domid == stub_domid)
> +		return xenstore_buf;
> +#endif
> +
>   	return xengnttab_map_grant_ref(*xgt_handle, domid,
>   				       GNTTAB_RESERVED_XENSTORE,
>   				       PROT_READ|PROT_WRITE);
> @@ -509,7 +518,7 @@ static void unmap_interface(domid_t domid, void *interface)
>   {
>   	if (domid == xenbus_master_domid())
>   		unmap_xenbus(interface);
> -	else
> +	else if (domid != stub_domid)
>   		xengnttab_unmap(*xgt_handle, interface, 1);
>   }
>   
> @@ -1214,6 +1223,22 @@ void dom0_init(void)
>   	xenevtchn_notify(xce_handle, dom0->port);
>   }
>   
> +void stubdom_init(void)
> +{
> +#ifdef __MINIOS__
> +	struct domain *stubdom;
> +
> +	if (stub_domid < 0)
> +		return;
> +
> +	stubdom = introduce_domain(NULL, stub_domid, xenbus_evtchn, false);
> +	if (!stubdom)
> +		barf_perror("Failed to initialize stubdom");
> +
> +	xenevtchn_notify(xce_handle, stubdom->port);

If I compare to introduce_domain(), it is not entirely clear to me why 
the notification is done unconditionally here. Can you clarify?

> +#endif
> +}
> +
>   static unsigned int domhash_fn(const void *k)
>   {
>   	return *(const unsigned int *)k;
> diff --git a/tools/xenstored/domain.h b/tools/xenstored/domain.h
> index 6c00540311..49c60c56bf 100644
> --- a/tools/xenstored/domain.h
> +++ b/tools/xenstored/domain.h
> @@ -85,6 +85,7 @@ int do_reset_watches(const void *ctx, struct connection *conn,
>   void domain_static_init(void);
>   void domain_init(int evtfd);
>   void dom0_init(void);
> +void stubdom_init(void);
>   void domain_deinit(void);
>   void ignore_connection(struct connection *conn, unsigned int err);
>   

Cheers,
Jürgen Groß Nov. 14, 2023, 6:33 a.m. UTC | #2
On 13.11.23 23:04, Julien Grall wrote:
> Hi Juergen,
> 
> On 10/11/2023 16:08, Juergen Gross wrote:
>> When running as stubdom, map the stubdom's Xenstore ring page in order
>> to support using the 9pfs frontend.
>>
>> Signed-off-by: Juergen Gross <jgross@suse.com>
>> ---
>>   tools/xenstored/core.c   |  2 ++
>>   tools/xenstored/domain.c | 27 ++++++++++++++++++++++++++-
>>   tools/xenstored/domain.h |  1 +
>>   3 files changed, 29 insertions(+), 1 deletion(-)
>>
>> diff --git a/tools/xenstored/core.c b/tools/xenstored/core.c
>> index b9ec50b34c..4a9d874f17 100644
>> --- a/tools/xenstored/core.c
>> +++ b/tools/xenstored/core.c
>> @@ -2991,6 +2991,8 @@ int main(int argc, char *argv[])
>>           lu_read_state();
>>   #endif
>> +    stubdom_init();
>> +
>>       check_store();
>>       /* Get ready to listen to the tools. */
>> diff --git a/tools/xenstored/domain.c b/tools/xenstored/domain.c
>> index fa17f68618..162b87b460 100644
>> --- a/tools/xenstored/domain.c
>> +++ b/tools/xenstored/domain.c
>> @@ -37,6 +37,10 @@
>>   #include <xenctrl.h>
>>   #include <xen/grant_table.h>
>> +#ifdef __MINIOS__
>> +#include <mini-os/xenbus.h>
>> +#endif
>> +
>>   static xc_interface **xc_handle;
>>   xengnttab_handle **xgt_handle;
>>   static evtchn_port_t virq_port;
>> @@ -500,6 +504,11 @@ static void *map_interface(domid_t domid)
>>       if (domid == xenbus_master_domid())
>>           return xenbus_map();
>> +#ifdef __MINIOS__
>> +    if (domid == stub_domid)
>> +        return xenstore_buf;
>> +#endif
>> +
>>       return xengnttab_map_grant_ref(*xgt_handle, domid,
>>                          GNTTAB_RESERVED_XENSTORE,
>>                          PROT_READ|PROT_WRITE);
>> @@ -509,7 +518,7 @@ static void unmap_interface(domid_t domid, void *interface)
>>   {
>>       if (domid == xenbus_master_domid())
>>           unmap_xenbus(interface);
>> -    else
>> +    else if (domid != stub_domid)
>>           xengnttab_unmap(*xgt_handle, interface, 1);
>>   }
>> @@ -1214,6 +1223,22 @@ void dom0_init(void)
>>       xenevtchn_notify(xce_handle, dom0->port);
>>   }
>> +void stubdom_init(void)
>> +{
>> +#ifdef __MINIOS__
>> +    struct domain *stubdom;
>> +
>> +    if (stub_domid < 0)
>> +        return;
>> +
>> +    stubdom = introduce_domain(NULL, stub_domid, xenbus_evtchn, false);
>> +    if (!stubdom)
>> +        barf_perror("Failed to initialize stubdom");
>> +
>> +    xenevtchn_notify(xce_handle, stubdom->port);
> 
> If I compare to introduce_domain(), it is not entirely clear to me why the 
> notification is done unconditionally here. Can you clarify?

There is no reason to do it conditionally, as we can be sure the event channel
is existing and the ring page is accessible.


Juergen
Julien Grall Nov. 14, 2023, 8:56 a.m. UTC | #3
Hi,

On 14/11/2023 06:33, Juergen Gross wrote:
> On 13.11.23 23:04, Julien Grall wrote:
>> Hi Juergen,
>>
>> On 10/11/2023 16:08, Juergen Gross wrote:
>>> When running as stubdom, map the stubdom's Xenstore ring page in order
>>> to support using the 9pfs frontend.
>>>
>>> Signed-off-by: Juergen Gross <jgross@suse.com>
>>> ---
>>>   tools/xenstored/core.c   |  2 ++
>>>   tools/xenstored/domain.c | 27 ++++++++++++++++++++++++++-
>>>   tools/xenstored/domain.h |  1 +
>>>   3 files changed, 29 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/tools/xenstored/core.c b/tools/xenstored/core.c
>>> index b9ec50b34c..4a9d874f17 100644
>>> --- a/tools/xenstored/core.c
>>> +++ b/tools/xenstored/core.c
>>> @@ -2991,6 +2991,8 @@ int main(int argc, char *argv[])
>>>           lu_read_state();
>>>   #endif
>>> +    stubdom_init();
>>> +
>>>       check_store();
>>>       /* Get ready to listen to the tools. */
>>> diff --git a/tools/xenstored/domain.c b/tools/xenstored/domain.c
>>> index fa17f68618..162b87b460 100644
>>> --- a/tools/xenstored/domain.c
>>> +++ b/tools/xenstored/domain.c
>>> @@ -37,6 +37,10 @@
>>>   #include <xenctrl.h>
>>>   #include <xen/grant_table.h>
>>> +#ifdef __MINIOS__
>>> +#include <mini-os/xenbus.h>
>>> +#endif
>>> +
>>>   static xc_interface **xc_handle;
>>>   xengnttab_handle **xgt_handle;
>>>   static evtchn_port_t virq_port;
>>> @@ -500,6 +504,11 @@ static void *map_interface(domid_t domid)
>>>       if (domid == xenbus_master_domid())
>>>           return xenbus_map();
>>> +#ifdef __MINIOS__
>>> +    if (domid == stub_domid)
>>> +        return xenstore_buf;
>>> +#endif
>>> +
>>>       return xengnttab_map_grant_ref(*xgt_handle, domid,
>>>                          GNTTAB_RESERVED_XENSTORE,
>>>                          PROT_READ|PROT_WRITE);
>>> @@ -509,7 +518,7 @@ static void unmap_interface(domid_t domid, void 
>>> *interface)
>>>   {
>>>       if (domid == xenbus_master_domid())
>>>           unmap_xenbus(interface);
>>> -    else
>>> +    else if (domid != stub_domid)
>>>           xengnttab_unmap(*xgt_handle, interface, 1);
>>>   }
>>> @@ -1214,6 +1223,22 @@ void dom0_init(void)
>>>       xenevtchn_notify(xce_handle, dom0->port);
>>>   }
>>> +void stubdom_init(void)
>>> +{
>>> +#ifdef __MINIOS__
>>> +    struct domain *stubdom;
>>> +
>>> +    if (stub_domid < 0)
>>> +        return;
>>> +
>>> +    stubdom = introduce_domain(NULL, stub_domid, xenbus_evtchn, false);
>>> +    if (!stubdom)
>>> +        barf_perror("Failed to initialize stubdom");
>>> +
>>> +    xenevtchn_notify(xce_handle, stubdom->port);
>>
>> If I compare to introduce_domain(), it is not entirely clear to me why 
>> the notification is done unconditionally here. Can you clarify?
> 
> There is no reason to do it conditionally, as we can be sure the event 
> channel
> is existing and the ring page is accessible.

Hmmm... But there is a second part in the condition:

domain->interface->connection == XENSTORE_RECONNECT

Why isn't it necessary here? What I am looking for particularly is some 
in-code comment (or at least in the commit message) because this is not 
100% clear why we are diverging.

Cheers,
Jürgen Groß Nov. 14, 2023, 9:12 a.m. UTC | #4
On 14.11.23 09:56, Julien Grall wrote:
> Hi,
> 
> On 14/11/2023 06:33, Juergen Gross wrote:
>> On 13.11.23 23:04, Julien Grall wrote:
>>> Hi Juergen,
>>>
>>> On 10/11/2023 16:08, Juergen Gross wrote:
>>>> When running as stubdom, map the stubdom's Xenstore ring page in order
>>>> to support using the 9pfs frontend.
>>>>
>>>> Signed-off-by: Juergen Gross <jgross@suse.com>
>>>> ---
>>>>   tools/xenstored/core.c   |  2 ++
>>>>   tools/xenstored/domain.c | 27 ++++++++++++++++++++++++++-
>>>>   tools/xenstored/domain.h |  1 +
>>>>   3 files changed, 29 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/tools/xenstored/core.c b/tools/xenstored/core.c
>>>> index b9ec50b34c..4a9d874f17 100644
>>>> --- a/tools/xenstored/core.c
>>>> +++ b/tools/xenstored/core.c
>>>> @@ -2991,6 +2991,8 @@ int main(int argc, char *argv[])
>>>>           lu_read_state();
>>>>   #endif
>>>> +    stubdom_init();
>>>> +
>>>>       check_store();
>>>>       /* Get ready to listen to the tools. */
>>>> diff --git a/tools/xenstored/domain.c b/tools/xenstored/domain.c
>>>> index fa17f68618..162b87b460 100644
>>>> --- a/tools/xenstored/domain.c
>>>> +++ b/tools/xenstored/domain.c
>>>> @@ -37,6 +37,10 @@
>>>>   #include <xenctrl.h>
>>>>   #include <xen/grant_table.h>
>>>> +#ifdef __MINIOS__
>>>> +#include <mini-os/xenbus.h>
>>>> +#endif
>>>> +
>>>>   static xc_interface **xc_handle;
>>>>   xengnttab_handle **xgt_handle;
>>>>   static evtchn_port_t virq_port;
>>>> @@ -500,6 +504,11 @@ static void *map_interface(domid_t domid)
>>>>       if (domid == xenbus_master_domid())
>>>>           return xenbus_map();
>>>> +#ifdef __MINIOS__
>>>> +    if (domid == stub_domid)
>>>> +        return xenstore_buf;
>>>> +#endif
>>>> +
>>>>       return xengnttab_map_grant_ref(*xgt_handle, domid,
>>>>                          GNTTAB_RESERVED_XENSTORE,
>>>>                          PROT_READ|PROT_WRITE);
>>>> @@ -509,7 +518,7 @@ static void unmap_interface(domid_t domid, void *interface)
>>>>   {
>>>>       if (domid == xenbus_master_domid())
>>>>           unmap_xenbus(interface);
>>>> -    else
>>>> +    else if (domid != stub_domid)
>>>>           xengnttab_unmap(*xgt_handle, interface, 1);
>>>>   }
>>>> @@ -1214,6 +1223,22 @@ void dom0_init(void)
>>>>       xenevtchn_notify(xce_handle, dom0->port);
>>>>   }
>>>> +void stubdom_init(void)
>>>> +{
>>>> +#ifdef __MINIOS__
>>>> +    struct domain *stubdom;
>>>> +
>>>> +    if (stub_domid < 0)
>>>> +        return;
>>>> +
>>>> +    stubdom = introduce_domain(NULL, stub_domid, xenbus_evtchn, false);
>>>> +    if (!stubdom)
>>>> +        barf_perror("Failed to initialize stubdom");
>>>> +
>>>> +    xenevtchn_notify(xce_handle, stubdom->port);
>>>
>>> If I compare to introduce_domain(), it is not entirely clear to me why the 
>>> notification is done unconditionally here. Can you clarify?
>>
>> There is no reason to do it conditionally, as we can be sure the event channel
>> is existing and the ring page is accessible.
> 
> Hmmm... But there is a second part in the condition:
> 
> domain->interface->connection == XENSTORE_RECONNECT
> 
> Why isn't it necessary here? What I am looking for particularly is some in-code 
> comment (or at least in the commit message) because this is not 100% clear why 
> we are diverging.

The test of XENSTORE_RECONNECT is done for domUs started before or together with
dom0 in order to give them a signal that they can start to use Xenstore.

Here we are initializing our own connection, so there is no need to test whether
the other end is waiting for us. We know there is no possible problem sending
the event, so we can just do it. The main instruction guarded by the test of
XENSTORE_RECONNECT is the setting of XENSTORE_CONNECTED, which then needs the
event to be sent to signal that change in the connection state.

In the end we are _not_ diverging. You should just compare the code to the more
comparable dom0_init() code. There the event is being sent unconditionally, too.


Juergen
Julien Grall Nov. 14, 2023, 8:48 p.m. UTC | #5
Hi,

On 14/11/2023 09:12, Juergen Gross wrote:
> On 14.11.23 09:56, Julien Grall wrote:
>> Hi,
>>
>> On 14/11/2023 06:33, Juergen Gross wrote:
>>> On 13.11.23 23:04, Julien Grall wrote:
>>>> Hi Juergen,
>>>>
>>>> On 10/11/2023 16:08, Juergen Gross wrote:
>>>>> When running as stubdom, map the stubdom's Xenstore ring page in order
>>>>> to support using the 9pfs frontend.
>>>>>
>>>>> Signed-off-by: Juergen Gross <jgross@suse.com>
>>>>> ---
>>>>>   tools/xenstored/core.c   |  2 ++
>>>>>   tools/xenstored/domain.c | 27 ++++++++++++++++++++++++++-
>>>>>   tools/xenstored/domain.h |  1 +
>>>>>   3 files changed, 29 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/tools/xenstored/core.c b/tools/xenstored/core.c
>>>>> index b9ec50b34c..4a9d874f17 100644
>>>>> --- a/tools/xenstored/core.c
>>>>> +++ b/tools/xenstored/core.c
>>>>> @@ -2991,6 +2991,8 @@ int main(int argc, char *argv[])
>>>>>           lu_read_state();
>>>>>   #endif
>>>>> +    stubdom_init();
>>>>> +
>>>>>       check_store();
>>>>>       /* Get ready to listen to the tools. */
>>>>> diff --git a/tools/xenstored/domain.c b/tools/xenstored/domain.c
>>>>> index fa17f68618..162b87b460 100644
>>>>> --- a/tools/xenstored/domain.c
>>>>> +++ b/tools/xenstored/domain.c
>>>>> @@ -37,6 +37,10 @@
>>>>>   #include <xenctrl.h>
>>>>>   #include <xen/grant_table.h>
>>>>> +#ifdef __MINIOS__
>>>>> +#include <mini-os/xenbus.h>
>>>>> +#endif
>>>>> +
>>>>>   static xc_interface **xc_handle;
>>>>>   xengnttab_handle **xgt_handle;
>>>>>   static evtchn_port_t virq_port;
>>>>> @@ -500,6 +504,11 @@ static void *map_interface(domid_t domid)
>>>>>       if (domid == xenbus_master_domid())
>>>>>           return xenbus_map();
>>>>> +#ifdef __MINIOS__
>>>>> +    if (domid == stub_domid)
>>>>> +        return xenstore_buf;
>>>>> +#endif
>>>>> +
>>>>>       return xengnttab_map_grant_ref(*xgt_handle, domid,
>>>>>                          GNTTAB_RESERVED_XENSTORE,
>>>>>                          PROT_READ|PROT_WRITE);
>>>>> @@ -509,7 +518,7 @@ static void unmap_interface(domid_t domid, void 
>>>>> *interface)
>>>>>   {
>>>>>       if (domid == xenbus_master_domid())
>>>>>           unmap_xenbus(interface);
>>>>> -    else
>>>>> +    else if (domid != stub_domid)
>>>>>           xengnttab_unmap(*xgt_handle, interface, 1);
>>>>>   }
>>>>> @@ -1214,6 +1223,22 @@ void dom0_init(void)
>>>>>       xenevtchn_notify(xce_handle, dom0->port);
>>>>>   }
>>>>> +void stubdom_init(void)
>>>>> +{
>>>>> +#ifdef __MINIOS__
>>>>> +    struct domain *stubdom;
>>>>> +
>>>>> +    if (stub_domid < 0)
>>>>> +        return;
>>>>> +
>>>>> +    stubdom = introduce_domain(NULL, stub_domid, xenbus_evtchn, 
>>>>> false);
>>>>> +    if (!stubdom)
>>>>> +        barf_perror("Failed to initialize stubdom");
>>>>> +
>>>>> +    xenevtchn_notify(xce_handle, stubdom->port);
>>>>
>>>> If I compare to introduce_domain(), it is not entirely clear to me 
>>>> why the notification is done unconditionally here. Can you clarify?
>>>
>>> There is no reason to do it conditionally, as we can be sure the 
>>> event channel
>>> is existing and the ring page is accessible.
>>
>> Hmmm... But there is a second part in the condition:
>>
>> domain->interface->connection == XENSTORE_RECONNECT
>>
>> Why isn't it necessary here? What I am looking for particularly is 
>> some in-code comment (or at least in the commit message) because this 
>> is not 100% clear why we are diverging.
> 
> The test of XENSTORE_RECONNECT is done for domUs started before or 
> together with
> dom0 in order to give them a signal that they can start to use Xenstore.
> 
> Here we are initializing our own connection, so there is no need to test 
> whether
> the other end is waiting for us. We know there is no possible problem 
> sending
> the event, so we can just do it. The main instruction guarded by the 
> test of
> XENSTORE_RECONNECT is the setting of XENSTORE_CONNECTED, which then 
> needs the
> event to be sent to signal that change in the connection state.
> 
> In the end we are _not_ diverging. You should just compare the code to 
> the more
> comparable dom0_init() code. There the event is being sent 
> unconditionally, too.

Ok. Can this be documented somewhere?

Cheers,
Jürgen Groß Nov. 15, 2023, 6:07 a.m. UTC | #6
On 14.11.23 21:48, Julien Grall wrote:
> Hi,
> 
> On 14/11/2023 09:12, Juergen Gross wrote:
>> On 14.11.23 09:56, Julien Grall wrote:
>>> Hi,
>>>
>>> On 14/11/2023 06:33, Juergen Gross wrote:
>>>> On 13.11.23 23:04, Julien Grall wrote:
>>>>> Hi Juergen,
>>>>>
>>>>> On 10/11/2023 16:08, Juergen Gross wrote:
>>>>>> When running as stubdom, map the stubdom's Xenstore ring page in order
>>>>>> to support using the 9pfs frontend.
>>>>>>
>>>>>> Signed-off-by: Juergen Gross <jgross@suse.com>
>>>>>> ---
>>>>>>   tools/xenstored/core.c   |  2 ++
>>>>>>   tools/xenstored/domain.c | 27 ++++++++++++++++++++++++++-
>>>>>>   tools/xenstored/domain.h |  1 +
>>>>>>   3 files changed, 29 insertions(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/tools/xenstored/core.c b/tools/xenstored/core.c
>>>>>> index b9ec50b34c..4a9d874f17 100644
>>>>>> --- a/tools/xenstored/core.c
>>>>>> +++ b/tools/xenstored/core.c
>>>>>> @@ -2991,6 +2991,8 @@ int main(int argc, char *argv[])
>>>>>>           lu_read_state();
>>>>>>   #endif
>>>>>> +    stubdom_init();
>>>>>> +
>>>>>>       check_store();
>>>>>>       /* Get ready to listen to the tools. */
>>>>>> diff --git a/tools/xenstored/domain.c b/tools/xenstored/domain.c
>>>>>> index fa17f68618..162b87b460 100644
>>>>>> --- a/tools/xenstored/domain.c
>>>>>> +++ b/tools/xenstored/domain.c
>>>>>> @@ -37,6 +37,10 @@
>>>>>>   #include <xenctrl.h>
>>>>>>   #include <xen/grant_table.h>
>>>>>> +#ifdef __MINIOS__
>>>>>> +#include <mini-os/xenbus.h>
>>>>>> +#endif
>>>>>> +
>>>>>>   static xc_interface **xc_handle;
>>>>>>   xengnttab_handle **xgt_handle;
>>>>>>   static evtchn_port_t virq_port;
>>>>>> @@ -500,6 +504,11 @@ static void *map_interface(domid_t domid)
>>>>>>       if (domid == xenbus_master_domid())
>>>>>>           return xenbus_map();
>>>>>> +#ifdef __MINIOS__
>>>>>> +    if (domid == stub_domid)
>>>>>> +        return xenstore_buf;
>>>>>> +#endif
>>>>>> +
>>>>>>       return xengnttab_map_grant_ref(*xgt_handle, domid,
>>>>>>                          GNTTAB_RESERVED_XENSTORE,
>>>>>>                          PROT_READ|PROT_WRITE);
>>>>>> @@ -509,7 +518,7 @@ static void unmap_interface(domid_t domid, void 
>>>>>> *interface)
>>>>>>   {
>>>>>>       if (domid == xenbus_master_domid())
>>>>>>           unmap_xenbus(interface);
>>>>>> -    else
>>>>>> +    else if (domid != stub_domid)
>>>>>>           xengnttab_unmap(*xgt_handle, interface, 1);
>>>>>>   }
>>>>>> @@ -1214,6 +1223,22 @@ void dom0_init(void)
>>>>>>       xenevtchn_notify(xce_handle, dom0->port);
>>>>>>   }
>>>>>> +void stubdom_init(void)
>>>>>> +{
>>>>>> +#ifdef __MINIOS__
>>>>>> +    struct domain *stubdom;
>>>>>> +
>>>>>> +    if (stub_domid < 0)
>>>>>> +        return;
>>>>>> +
>>>>>> +    stubdom = introduce_domain(NULL, stub_domid, xenbus_evtchn, false);
>>>>>> +    if (!stubdom)
>>>>>> +        barf_perror("Failed to initialize stubdom");
>>>>>> +
>>>>>> +    xenevtchn_notify(xce_handle, stubdom->port);
>>>>>
>>>>> If I compare to introduce_domain(), it is not entirely clear to me why the 
>>>>> notification is done unconditionally here. Can you clarify?
>>>>
>>>> There is no reason to do it conditionally, as we can be sure the event channel
>>>> is existing and the ring page is accessible.
>>>
>>> Hmmm... But there is a second part in the condition:
>>>
>>> domain->interface->connection == XENSTORE_RECONNECT
>>>
>>> Why isn't it necessary here? What I am looking for particularly is some 
>>> in-code comment (or at least in the commit message) because this is not 100% 
>>> clear why we are diverging.
>>
>> The test of XENSTORE_RECONNECT is done for domUs started before or together with
>> dom0 in order to give them a signal that they can start to use Xenstore.
>>
>> Here we are initializing our own connection, so there is no need to test whether
>> the other end is waiting for us. We know there is no possible problem sending
>> the event, so we can just do it. The main instruction guarded by the test of
>> XENSTORE_RECONNECT is the setting of XENSTORE_CONNECTED, which then needs the
>> event to be sent to signal that change in the connection state.
>>
>> In the end we are _not_ diverging. You should just compare the code to the more
>> comparable dom0_init() code. There the event is being sent unconditionally, too.
> 
> Ok. Can this be documented somewhere?

I'll add something to the commit message.


Juergen
diff mbox series

Patch

diff --git a/tools/xenstored/core.c b/tools/xenstored/core.c
index b9ec50b34c..4a9d874f17 100644
--- a/tools/xenstored/core.c
+++ b/tools/xenstored/core.c
@@ -2991,6 +2991,8 @@  int main(int argc, char *argv[])
 		lu_read_state();
 #endif
 
+	stubdom_init();
+
 	check_store();
 
 	/* Get ready to listen to the tools. */
diff --git a/tools/xenstored/domain.c b/tools/xenstored/domain.c
index fa17f68618..162b87b460 100644
--- a/tools/xenstored/domain.c
+++ b/tools/xenstored/domain.c
@@ -37,6 +37,10 @@ 
 #include <xenctrl.h>
 #include <xen/grant_table.h>
 
+#ifdef __MINIOS__
+#include <mini-os/xenbus.h>
+#endif
+
 static xc_interface **xc_handle;
 xengnttab_handle **xgt_handle;
 static evtchn_port_t virq_port;
@@ -500,6 +504,11 @@  static void *map_interface(domid_t domid)
 	if (domid == xenbus_master_domid())
 		return xenbus_map();
 
+#ifdef __MINIOS__
+	if (domid == stub_domid)
+		return xenstore_buf;
+#endif
+
 	return xengnttab_map_grant_ref(*xgt_handle, domid,
 				       GNTTAB_RESERVED_XENSTORE,
 				       PROT_READ|PROT_WRITE);
@@ -509,7 +518,7 @@  static void unmap_interface(domid_t domid, void *interface)
 {
 	if (domid == xenbus_master_domid())
 		unmap_xenbus(interface);
-	else
+	else if (domid != stub_domid)
 		xengnttab_unmap(*xgt_handle, interface, 1);
 }
 
@@ -1214,6 +1223,22 @@  void dom0_init(void)
 	xenevtchn_notify(xce_handle, dom0->port);
 }
 
+void stubdom_init(void)
+{
+#ifdef __MINIOS__
+	struct domain *stubdom;
+
+	if (stub_domid < 0)
+		return;
+
+	stubdom = introduce_domain(NULL, stub_domid, xenbus_evtchn, false);
+	if (!stubdom)
+		barf_perror("Failed to initialize stubdom");
+
+	xenevtchn_notify(xce_handle, stubdom->port);
+#endif
+}
+
 static unsigned int domhash_fn(const void *k)
 {
 	return *(const unsigned int *)k;
diff --git a/tools/xenstored/domain.h b/tools/xenstored/domain.h
index 6c00540311..49c60c56bf 100644
--- a/tools/xenstored/domain.h
+++ b/tools/xenstored/domain.h
@@ -85,6 +85,7 @@  int do_reset_watches(const void *ctx, struct connection *conn,
 void domain_static_init(void);
 void domain_init(int evtfd);
 void dom0_init(void);
+void stubdom_init(void);
 void domain_deinit(void);
 void ignore_connection(struct connection *conn, unsigned int err);