Re: [PATCH v6 4/7] xen/arm: configure dom0less domain for enabling xenstore after boot

[Bertrand Marquis <Bertrand.Marquis@xxxxxxx>]

Hi Julien,

> On 11 May 2022, at 10:10, Julien Grall <julien@xxxxxxx> wrote:
> 
> Hi Bertrand,
> 
> On 11/05/2022 09:46, Bertrand Marquis wrote:
>>> On 11 May 2022, at 09:38, Julien Grall <julien@xxxxxxx> wrote:
>>> 
>>> Hi Bertrand,
>>> 
>>> On 11/05/2022 08:46, Bertrand Marquis wrote:
>>>>> On 10 May 2022, at 17:35, Julien Grall <julien@xxxxxxx> wrote:
>>>>> 
>>>>> Hi Rahul,
>>>>> 
>>>>> On 10/05/2022 17:30, Rahul Singh wrote:
>>>>>>> + rc = evtchn_alloc_unbound(&alloc);
>>>>>>> + if ( rc )
>>>>>>> + {
>>>>>>> + printk("Failed allocating event channel for domain\n");
>>>>>>> + return rc;
>>>>>>> + }
>>>>>>> +
>>>>>>> + d->arch.hvm.params[HVM_PARAM_STORE_EVTCHN] = alloc.port;
>>>>>>> +
>>>>>>> + return 0;
>>>>>>> +}
>>>>>>> +
>>>>>>> static int __init construct_domU(struct domain *d,
>>>>>>> const struct dt_device_node *node)
>>>>>>> {
>>>>>>> @@ -3214,6 +3243,14 @@ static int __init construct_domU(struct domain 
>>>>>>> *d,
>>>>>>> if ( rc < 0 )
>>>>>>> return rc;
>>>>>>> 
>>>>>>> + if ( kinfo.dom0less_enhanced )
>>>>>> I think we need to do something like this to fix the error.
>>>>>> if ( hardware_domain && kinfo.dom0less_enhanced )
>>>>>> {
>>>>>> }
>>>>> 
>>>>> Is there any use case to use "dom0less_enhanced" without dom0 (or a 
>>>>> domain servicing Xenstored)?
>>>>> 
>>>> Just being curious here but would it even be possible to have non dom0 
>>>> domain servicing xenstored ?
>>> 
>>> You can build Xenstored against mini-os and configure the init script to 
>>> launch xenstored as a domain.
>> So dom0 is not mandatory or should mini-os be started as Dom0 for this to 
>> work ?
> 
> In order to allocate the event channel, you need to know the ID of the domain 
> where Xenstored will run. Stefano's patch is relying on Xenstored to be run 
> in Domain 0.
> 
> This would need to be updated if we want to run it in a separate domain.

Ok then Dom0 is mandatory at the moment, I am ok with that.

> 
>>> 
>>>>> If not, then I would consider to forbid this case and return an error.
>>>> One way or an other we need to solve the crash but if it is forbidden we 
>>>> must prevent coming to this step earlier as it means the configuration is 
>>>> wrong.
>>> 
>>> I think this should be checked when parsing the configuration.
>> If dom0 is mandatory yes, we should still make sure that this code cannot be 
>> reached so an ASSERT would be nice here at least in case someone tries to 
>> activate this code without dom0 (which might happen when we will push the 
>> serie for static event channels).
> 
> I am fine with an ASSERT().
> 
> Are you saying that dom0less_enhanced will be set to true for the static 
> event channel series?
> 
> If yes, then I think dom0less_enhanced will need to be an enum so we know 
> what part of Xen is exposed.

No it won’t, we just need some of the changes done but without setting 
dom0less_enhanced.
@Rahul: can you confirm.

Cheers
Bertrand

> 
> Cheers,
> 
> -- 
> Julien Grall