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

[Rahul Singh]

Hi Julien,

> On 24 Aug 2022, at 1:59 pm, Julien Grall <jul...@xen.org> 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?


diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index cffd508af2..870846b742 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -3568,12 +3568,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");
-    }
 
     if ( vcpu_create(d, 0) == NULL )
         return -ENOMEM;
@@ -3613,9 +3608,8 @@ static int __init construct_domU(struct domain *d,
     if ( rc < 0 )
         return rc;
 
-    if ( kinfo.dom0less_enhanced )
+    if ( kinfo.dom0less_enhanced && hardware_domain )
     {
-        ASSERT(hardware_domain);
         rc = alloc_xenstore_evtchn(d);
         if ( rc < 0 )
             return rc;
 

Regards,
Rahul