Re: [PATCH v2 7/7] xen/arm: introduce new xen,enhanced property value

[Julien Grall <julien@xxxxxxx>]

On 24/08/2022 15:42, Rahul Singh wrote:
> > On 24 Aug 2022, at 1:59 pm, Julien Grall <julien@xxxxxxx> wrote:
> >
> >
> >
> > On 24/08/2022 13:15, Rahul Singh wrote:
> > > Hi Julien,
> > Hi Rahul,
> >
> > > Please let me know your view on this.
> > > diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> > > index bfe7bc6b36..a1e23eee59 100644
> > > --- a/xen/arch/arm/domain_build.c
> > > +++ b/xen/arch/arm/domain_build.c
> > > @@ -3562,12 +3562,7 @@ static int __init construct_domU(struct domain *d,
> > >     if ( rc == -EILSEQ ||
> > >       rc == -ENODATA ||
> > >       (rc == 0 && !strcmp(dom0less_enhanced, “enabled”)) )
> > > -  {
> > > -    if ( hardware_domain )
> > >         kinfo.dom0less_enhanced = true;
> > > -    else
> > > -      panic(“Tried to use xen,enhanced without dom0\n”);
> > > -  }
> > You can't use "xen,enhanced" without dom0. In fact, you will end up to dereference NULL 
> > in alloc_xenstore_evtchn(). That's because "xen,enhanced" means the domain will be able 
> > to use Xenstored.
> >
> > Now if you want to support your feature without a dom0. Then I think we want to introduce 
> > an option which would be the same as "xen,enhanced" but doesn't expose 
> > Xenstored.
> If we modify the patch as below we can use the "xen,enhanced" for domUs without 
> dom0.
> I tested the patch and its works fine. Do you see any issue with this approach?
Yes. For two reasons:
 1) It is muddying the meaning of "xen,enhanced". In particular a user 
may not realize that Xenstore is not available if dom0 is not present.
 2) It would be more complicated to handle the case where Xenstored 
lives in a non-dom0 domain. I am not aware of anyone wanting this on Arm 
yet, but I don't want to close the door.
So if you want to support create "xen,xen" without all the rest. Then I 
think we need a different property value. I don't have a good suggestion 
for the name.
Cheers,

--
Julien Grall