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

[Jan Beulich <jbeulich@xxxxxxxx>]

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."

> --- 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?

Jan