[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v5 07/11] xen/domctl: Introduce XEN_DOMCTL_CDF_vpci flag

Hi Stefano,

On 08/10/2021 22:46, Stefano Stabellini wrote:
On Fri, 8 Oct 2021, Julien Grall wrote:
Hi Andrew,

On Fri, 8 Oct 2021, 20:07 Andrew Cooper, <andrew.cooper3@xxxxxxxxxx> wrote:
       On 06/10/2021 18:40, Rahul Singh wrote:
       > Introduce XEN_DOMCTL_CDF_vpci flag to enable VPCI support in XEN.
       > Reject the use of this new flag for x86 as VPCI is not supported for
       > DOMU guests for x86.
       > Signed-off-by: Rahul Singh <rahul.singh@xxxxxxx>
       > Reviewed-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>
       > Acked-by: Christian Lindig <christian.lindig@xxxxxxxxxx>

       I'm afraid this logic is broken.

       There's no matching feature to indicate to the toolstack whether
       XEN_DOMCTL_CDF_vpci will be accepted or not.  The usual way of doing
       this is with a physinfo_cap field.

I am slightly puzzled by this. I am assuming you are referring to 
XENVER_get_features which AFAICT is a stable interface. So why should we
use it to describe the presence of an unstable interface?

       This flag needs using in Patch 10 to reject attempts to create a VM with
       devices when passthrough support is unavailable.

May I ask why we can't rely on the hypervisor to reject the flag when suitable?

       Ian, for the 4.16 release, this series either needs completing with the
       additional flag implemented, or this patch needs reverting to avoid us
       shipping a broken interface.

I fail to see how the interface would be broken... Would you mind to describe a 
bit more what could go wrong with this interface?

After chatting with Andrew on IRC, this is my understanding.

Today if pci=[] is specified in the VM config file then
XEN_DOMCTL_CDF_vpci is added. If Xen doesn't support it, Xen returns
an error but libxl/xl won't be able to tell exactly why it fails. So xl
will end up printing a generic VM creation failure. Andrew would like to
see something like the following in libxl:

if ( PCI_devices && !cap.vcpi )
     error("Sorry - PCI not supported")

So that the user gets a clear informative error back rather than a
generic VM creation failure.

I can understand this request. However...

Also, this is a requirement for the stable
hypercall interface.

... leaving aside the fact that domctl is clearly not stable today, the flag would be rejected on hypervisor not supporting vPCI. So I don't really see how this is a requirement for the stable hypercall interface.

Would you mind providing more details?

I think that's fine and we can implement this request easily by adding
XEN_SYSCTL_PHYSCAP_vpci. Rahul or Bertrand, are you guys happy with
doing that? Otherwise I could take it on.

As a side note, given that PCI passthrough support is actually not yet
complete on ARM, we could even just do the following in xl/libxl:

if ( PCI_devices )
     error("Sorry - PCI not supported")

or temporarily remove XEN_DOMCTL_CDF_vpci until PCI passthrough gets


Julien Grall



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.