EPT is of no interest when !HVM. While I'm observing gcc11 to fully
eliminate the function, older gcc's DCE looks to not be as good. Aid the
compiler in eliminating the accesses of opt_hap_{2mb,1gb}, which
otherwise cause undefined symbol errors when linking.

While there adjust types.

Fixes: c479415610f0 ("x86/P2M: p2m.c is HVM-only")
Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/drivers/passthrough/vtd/iommu.c
+++ b/xen/drivers/passthrough/vtd/iommu.c
@@ -2155,14 +2155,17 @@ static int cf_check intel_iommu_lookup_p
     return 0;
 }
 
-static int __init vtd_ept_page_compatible(struct vtd_iommu *iommu)
+static bool __init vtd_ept_page_compatible(const struct vtd_iommu *iommu)
 {
-    u64 ept_cap, vtd_cap = iommu->cap;
+    uint64_t ept_cap, vtd_cap = iommu->cap;
+
+    if ( !IS_ENABLED(CONFIG_HVM) )
+        return false;
 
     /* EPT is not initialised yet, so we must check the capability in
      * the MSR explicitly rather than use cpu_has_vmx_ept_*() */
     if ( rdmsr_safe(MSR_IA32_VMX_EPT_VPID_CAP, ept_cap) != 0 ) 
-        return 0;
+        return false;
 
     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);

On 22.04.2022 11:58, Jan Beulich wrote:
> EPT is of no interest when !HVM. While I'm observing gcc11 to fully
> eliminate the function, older gcc's DCE looks to not be as good. Aid the
> compiler in eliminating the accesses of opt_hap_{2mb,1gb}, which
> otherwise cause undefined symbol errors when linking.
> 
> While there adjust types.
> 
> Fixes: c479415610f0 ("x86/P2M: p2m.c is HVM-only")
> Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

I guess I'll put this in (as being simple enough) if I don't hear
anything back by the end of the week.

Jan

> --- a/xen/drivers/passthrough/vtd/iommu.c
> +++ b/xen/drivers/passthrough/vtd/iommu.c
> @@ -2155,14 +2155,17 @@ static int cf_check intel_iommu_lookup_p
>      return 0;
>  }
>  
> -static int __init vtd_ept_page_compatible(struct vtd_iommu *iommu)
> +static bool __init vtd_ept_page_compatible(const struct vtd_iommu *iommu)
>  {
> -    u64 ept_cap, vtd_cap = iommu->cap;
> +    uint64_t ept_cap, vtd_cap = iommu->cap;
> +
> +    if ( !IS_ENABLED(CONFIG_HVM) )
> +        return false;
>  
>      /* EPT is not initialised yet, so we must check the capability in
>       * the MSR explicitly rather than use cpu_has_vmx_ept_*() */
>      if ( rdmsr_safe(MSR_IA32_VMX_EPT_VPID_CAP, ept_cap) != 0 ) 
> -        return 0;
> +        return false;
>  
>      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);
> 
>

> From: Jan Beulich <jbeulich@suse.com>
> Sent: Thursday, May 19, 2022 8:20 PM
> 
> On 22.04.2022 11:58, Jan Beulich wrote:
> > EPT is of no interest when !HVM. While I'm observing gcc11 to fully
> > eliminate the function, older gcc's DCE looks to not be as good. Aid the
> > compiler in eliminating the accesses of opt_hap_{2mb,1gb}, which
> > otherwise cause undefined symbol errors when linking.
> >
> > While there adjust types.
> >
> > Fixes: c479415610f0 ("x86/P2M: p2m.c is HVM-only")
> > Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
> > Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> I guess I'll put this in (as being simple enough) if I don't hear
> anything back by the end of the week.

Reviewed-by: Kevin Tian <kevin.tian@intel.com>

> 
> Jan
> 
> > --- a/xen/drivers/passthrough/vtd/iommu.c
> > +++ b/xen/drivers/passthrough/vtd/iommu.c
> > @@ -2155,14 +2155,17 @@ static int cf_check intel_iommu_lookup_p
> >      return 0;
> >  }
> >
> > -static int __init vtd_ept_page_compatible(struct vtd_iommu *iommu)
> > +static bool __init vtd_ept_page_compatible(const struct vtd_iommu
> *iommu)
> >  {
> > -    u64 ept_cap, vtd_cap = iommu->cap;
> > +    uint64_t ept_cap, vtd_cap = iommu->cap;
> > +
> > +    if ( !IS_ENABLED(CONFIG_HVM) )
> > +        return false;
> >
> >      /* EPT is not initialised yet, so we must check the capability in
> >       * the MSR explicitly rather than use cpu_has_vmx_ept_*() */
> >      if ( rdmsr_safe(MSR_IA32_VMX_EPT_VPID_CAP, ept_cap) != 0 )
> > -        return 0;
> > +        return false;
> >
> >      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);
> >
> >

On 22/04/2022 10:58, Jan Beulich wrote:
> EPT is of no interest when !HVM. While I'm observing gcc11 to fully
> eliminate the function, older gcc's DCE looks to not be as good. Aid the
> compiler in eliminating the accesses of opt_hap_{2mb,1gb}, which
> otherwise cause undefined symbol errors when linking.

I've just reproduced it on GCC 11, using CONFIG_UBSAN as well.

>
> While there adjust types.
>
> Fixes: c479415610f0 ("x86/P2M: p2m.c is HVM-only")
> Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>, preferably with
the commit message tweaked.