Re: [Xen-devel] Xen virtual IOMMU high level design doc

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 31.08.16 at 10:39, <tianyu.lan@xxxxxxxxx> wrote:
> On 2016年08月25日 19:11, Jan Beulich wrote:
>>>>> On 17.08.16 at 14:05, <tianyu.lan@xxxxxxxxx> wrote:
>>> 1 Motivation for Xen vIOMMU
>>> ============================================================================
>>> ===
>>> 1.1 Enable more than 255 vcpu support
>>> HPC virtualization requires more than 255 vcpus support in a single VM
>>> to meet parallel computing requirement. More than 255 vcpus support
>>> requires interrupt remapping capability present on vIOMMU to deliver
>>> interrupt to #vcpu >255 Otherwise Linux guest fails to boot up with >255
>>> vcpus if interrupt remapping is absent.
>> 
>> I continue to question this as a valid motivation at this point in
>> time, for the reasons Andrew has been explaining.
> 
> If we want to support Linux guest with >255 vcpus, interrupt remapping
> is necessary.

I don't understand why you keep repeating this, without adding
_why_ you think there is a demand for such guests and _what_
your plans are to eliminate Andrew's concerns.

>>> 3 Xen hypervisor
>>> ==========================================================================
>>>
>>> 3.1 New hypercall XEN_SYSCTL_viommu_op
>>> 1) Definition of "struct xen_sysctl_viommu_op" as new hypercall parameter.
>>>
>>> struct xen_sysctl_viommu_op {
>>>     u32 cmd;
>>>     u32 domid;
>>>     union {
>>>             struct {
>>>                     u32 capabilities;
>>>             } query_capabilities;
>>>             struct {
>>>                     u32 capabilities;
>>>                     u64 base_address;
>>>             } create_iommu;
>>>             struct {
>>>                     u8  bus;
>>>                     u8  devfn;
>> 
>> Please can we avoid introducing any new interfaces without segment/
>> domain value, even if for now it'll be always zero?
> 
> Sure. Will add segment field.
> 
>> 
>>>                     u64 iova;
>>>                     u64 translated_addr;
>>>                     u64 addr_mask; /* Translation page size */
>>>                     IOMMUAccessFlags permisson;             
>>>             } 2th_level_translation;
>> 
>> I suppose "translated_addr" is an output here, but for the following
>> fields this already isn't clear. Please add IN and OUT annotations for
>> clarity.
>> 
>> Also, may I suggest to name this "l2_translation"? (But there are
>> other implementation specific things to be considered here, which
>> I guess don't belong into a design doc discussion.)
> 
> How about this?
>         struct {
>           /* IN parameters. */
>           u8  segment;
>             u8  bus;
>             u8  devfn;
>             u64 iova;
>           /* Out parameters. */
>             u64 translated_addr;
>             u64 addr_mask; /* Translation page size */
>             IOMMUAccessFlags permisson;
>         } l2_translation;

"segment" clearly needs to be a 16-bit value, but apart from that
(and missing padding fields) this looks okay.

>>> 3.5 Implementation consideration
>>> Linux Intel IOMMU driver will fail to be loaded without 2th level
>>> translation support even if interrupt remapping and 1th level
>>> translation are available. This means it's needed to enable 2th level
>>> translation first before other functions.
>> 
>> Is there a reason for this? I.e. do they unconditionally need that
>> functionality?
> 
> Yes, Linux intel IOMMU driver unconditionally needs l2 translation.
> Driver checks whether there is a valid sagaw(supported Adjusted Guest
> Address Widths) during initializing IOMMU data struct and return error
> if not.

How about my first question then?

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel