Re: [PATCH] x86/vtd: fix EPT page table sharing check

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Fri, Apr 17, 2020 at 12:57:16PM +0200, Jan Beulich wrote:
> On 17.04.2020 12:26, Roger Pau Monne wrote:
> > The EPT page tables can be shared with the IOMMU as long as the page
> > sizes supported by EPT are also supported by the IOMMU.
> > 
> > Current code checks that both the IOMMU and EPT support the same page
> > sizes, but this is not strictly required, the IOMMU supporting more
> > page sizes than EPT is fine and shouldn't block page sharing.
> 
> Meaning the check isn't wrong, just too strict. Hence maybe
> "relax" instead of "fix" in the subject?
> 
> Also "... page table sharing."

Sure.

> > --- a/xen/drivers/passthrough/vtd/iommu.c
> > +++ b/xen/drivers/passthrough/vtd/iommu.c
> > @@ -1914,8 +1914,10 @@ static int __init vtd_ept_page_compatible(struct 
> > vtd_iommu *iommu)
> >      if ( rdmsr_safe(MSR_IA32_VMX_EPT_VPID_CAP, ept_cap) != 0 ) 
> >          return 0;
> >  
> > -    return (ept_has_2mb(ept_cap) && opt_hap_2mb) == cap_sps_2mb(vtd_cap) &&
> > -           (ept_has_1gb(ept_cap) && opt_hap_1gb) == cap_sps_1gb(vtd_cap);
> > +    return ((ept_has_2mb(ept_cap) && opt_hap_2mb) ? cap_sps_2mb(vtd_cap)
> > +                                                  : true) &&
> > +           ((ept_has_1gb(ept_cap) && opt_hap_1gb) ? cap_sps_1gb(vtd_cap)
> > +                                                  : true);
> 
> I have to admit that I always find it odd to have "true" or "false"
> as the 2nd or 3rd operand of a ternary. How about simply changing
> == to <= in the original expression?

I find it weird to use <= to compare booleans, but I can change it.
Let me post a new version with the adjustments to the commit message
and this requested change.

Thanks, Roger.