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

["Tian, Kevin" <kevin.tian@xxxxxxxxx>]

> From: Malcolm Crossley [mailto:malcolm.crossley@xxxxxxxxxx]
> Sent: Wednesday, November 25, 2015 11:59 PM
> 
> On 25/11/15 15:38, Jan Beulich wrote:
> >>>> On 25.11.15 at 16:13, <andrew.cooper3@xxxxxxxxxx> wrote:
> >> On 25/11/15 10:49, Jan Beulich wrote:
> >>>>>> On 25.11.15 at 11:28, <andrew.cooper3@xxxxxxxxxx> wrote:
> >>>> On 24/11/15 17:41, Jan Beulich wrote:
> >>>>>>>> On 24.11.15 at 18:17, <Anshul Makkar anshul.makkar@xxxxxxxxxx> 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
> 

I'm OK to use processor model here, though ideally Jan is right. :-)

Thanks
Kevin

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