On 11.10.2021 13:29, Michal Orzel wrote:
> On 08.10.2021 23:46, Stefano Stabellini wrote:
>> On Fri, 8 Oct 2021, Julien Grall wrote:
>>> On Fri, 8 Oct 2021, 20:07 Andrew Cooper, <andrew.coop...@citrix.com> 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.si...@arm.com>
>>> > Reviewed-by: Stefano Stabellini <sstabell...@kernel.org>
>>> > Acked-by: Christian Lindig <christian.lin...@citrix.com>
>>>
>>> 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. Also, this is a requirement for the stable
>> hypercall interface.
>>
>>
>> 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
>> finalized.
>>
> As Rahul is on leave:
> I'm ok to introduce XEN_SYSCTL_PHYSCAP_vpci. I did the same for vpmu so it's
> ok.
> However the problem I have is about setting this cap.
> On arm it is easy as we are not supporting vpci at the moment so the cap can
> be set to false.
> But how to deal with this cap in x86 code? I'm not familiar with x86 so I'm
> asking for advice.
As the sysctl is mainly from tool stacks to drive guests (DomU-s), I'd
set it to false for x86 as well. Roger - do you see any reason this
could be needed to express anything Dom0-related?
Jan
