Re: [PATCH] iommu/quirk: disable shared EPT for Sandybridge and earlier processors.

[Malcolm Crossley <malcolm.crossley@citrix.com>]

On 25/11/15 15:38, Jan Beulich wrote:
>>>> On 25.11.15 at 16:13, <andrew.cooper3@citrix.com> wrote:
>> On 25/11/15 10:49, Jan Beulich wrote:
>>>>>> On 25.11.15 at 11:28, <andrew.cooper3@citrix.com> wrote:
>>>> On 24/11/15 17:41, Jan Beulich wrote:
>>>>>>>> On 24.11.15 at 18:17, <Anshul Makkar anshul.makkar@citrix.com> wrote:
>>>>>> --- a/xen/drivers/passthrough/vtd/quirks.c
>>>>>> +++ b/xen/drivers/passthrough/vtd/quirks.c
>>>>>> @@ -320,6 +320,20 @@ void __init platform_quirks_init(void)
>>>>>>      /* Tylersburg interrupt remap quirk */
>>>>>>      if ( iommu_intremap )
>>>>>>          tylersburg_intremap_quirk();
>>>>>> +
>>>>>> +    /*
>>>>>> +     * Disable shared EPT ("sharept") on Sandybridge and older processors
>>>>>> +     * by default.
>>>>>> +     * SandyBridge has no huge page support for IOTLB which leads to 
>> fallback
>>>>>> +     * on 4k pages and leads to performance degradation.
>>>>>> +     *
>>>>>> +     * Shared EPT ("sharept") will be disabled only if user has not
>>>>>> +     * provided explicit choice on the command line thus iommu_hap_pt_share 
>> is
>>>>>> +     * at its initialized value of -1.
>>>>>> +     */
>>>>>> +    if ( (boot_cpu_data.x86 == 0x06 && (boot_cpu_data.x86_model <= 0x2F ||
>>>>>> +          boot_cpu_data.x86_model == 0x36)) && (iommu_hap_pt_share == -1) )
>>>>>> +        iommu_hap_pt_share = 0;
>>>>> If we really want to do this, then I think we should key this on
>>>>> EPT but not VT-d having 2M support, instead of on CPU models.
>>>> This check is already performed by vtd_ept_page_compatible()
>>> Yeah, I realized there would be such a check on the way home.
>>>
>>>> The problem is that SandyBridge IOMMUs advertise 2M support and do
>>>> function with it, but cannot cache 2MB translations in the IOTLBs.
>>>>
>>>> As a result, attempting to use 2M translations causes substantially
>>>> worse performance than 4K translations.
>>> So commit message and comment should make this more explicit,
>>> to avoid the impression "IOTLB" isn't just the relatively common
>>> mis-naming of "IOMMU".
>>>
>>> Plus I guess the sharing won't need suppressing if !opt_hap_2mb?
>>>
>>> Further the model based check is relatively broad, and includes
>>> Atoms (0x36 actually is one), which can't be considered "Sandybridge
>>> or older" imo.
>>>
>>> And finally I'm not fully convinced using CPU model info to deduce
>>> chipset behavior is entirely correct (albeit perhaps in practice it'll
>>> be fine except maybe when running Xen itself virtualized).
>>
>> What else would you suggest? I can't think of any better identifying
>> information.
> 
> Chipset IDs / revisions?

In this case the IOMMU is integrated into the Sandybridge-EP processor itself.
Unfortunately there's no register to query the IOTLB configuration of the IOMMU
and so we're stuck identifying the via the processor model number itself.

Malcolm

> 
> Jan
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
>