Re: [PATCH v4 13/14] arm/libxl: Emulated PCI device tree node in libxl

[Rahul Singh <Rahul.Singh@xxxxxxx>]

Hi Ian,

> On 6 Oct 2021, at 12:33 pm, Ian Jackson <iwj@xxxxxxxxxxxxxx> wrote:
> 
> Rahul Singh writes ("Re: [PATCH v4 13/14] arm/libxl: Emulated PCI device tree 
> node in libxl"):
>> Hi Ian       
>>> What is wrong with putting it in
>>> libxl__arch_domain_build_info_setdefault
>>> which I think exists precisely for this kind of thing ?
>> 
>> As we have to set the arch_arm.vpci to false for x86 and ARM I
>> thought it is right to move the code to common code to avoid
>> duplication.
>> 
>> Are you suggesting to put "
>> libxl_defbool_setdefault(&b_info->arch_arm.vpci, false)ïżœin
>> libxl__arch_domain_build_info_setdefault() for x86 and ARM
>> differently.
> 
> I've gone back and reread the whole thread, which I probably should
> have done to start with....
> 
> So:
> 
>>>> #if defined(__arm__) || defined(__aarch64__)                  
>>>>   /*                                    
>>>>    * Enable VPCI support for ARM. VPCI support for DOMU guests is not    
>>>>    * supported for x86.                          
>>>>    */                                   
>>>>   if (d_config->num_pcidevs)                        
>>>>     libxl_defbool_set(&b_info->arch_arm.vpci, true);           
>>>> #endif 
> 
> I think this logic probably ought to be in libxl, not in xl.

I will move the code to "libxl_arm.c"to avoid #ifdef in common code and also  
to avoid setting the vpci for x86

diff --git a/tools/libs/light/libxl_arm.c b/tools/libs/light/libxl_arm.c
index e3140a6e00..2be208b99b 100644
--- a/tools/libs/light/libxl_arm.c
+++ b/tools/libs/light/libxl_arm.c
@@ -101,6 +101,12 @@ int libxl__arch_domain_prepare_config(libxl__gc *gc,
         return ERROR_FAIL;
     }
 
+    /* Enable VPCI support. */
+    if (d_config->num_pcidevs) {
+        config->flags |= XEN_DOMCTL_CDF_vpci;
+        libxl_defbool_set(&d_config->b_info.arch_arm.vpci, true);
+    }
+
     return 0;
 }

>  We try
> to make the libxl API "do the right thing" by default.  In this case I
> think that means to enable VPCI (i) on platforms where it's available
> (ii) if the guest has PCI passthrough devices.  Is that right ?

Yes you are right VPCI will be enabled for guest when guest has PCI passthrough 
device 
assigned and VPCI support is available.  
> 
> Sorry to ask these question now, and please forgive my ignorance:
> 
> Is VPCI inherently an ARM-specific ABI or protocol ?

As of now VPCI for DOMU guests is only implemented  for ARM.
 
>  When might an
> admin want to turn it on explicitly ?

It will be enabled dynamically when admin assign any PCI device to guest.

> 
> How does this all relate to the (non-arch-specific) "passthrough"
> option ?

VPCI will be enabled only when there is any PCI device assigned to guest 
therefore I used 
"d_config->num_pcidevs” to enable VPCI.

Regards,
Rahul

> 
> Ian.