[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v13 0/4] add per-domain IOMMU control
Hi, On 25/09/2019 17:10, Paul Durrant wrote: >> -----Original Message----- >> From: Oleksandr <olekstysh@xxxxxxxxx> >> Sent: 25 September 2019 16:50 >> To: Paul Durrant <Paul.Durrant@xxxxxxxxxx>; 'Jan Beulich' <jbeulich@xxxxxxxx> >> Cc: Petre Pircalabu <ppircalabu@xxxxxxxxxxxxxxx>; Stefano Stabellini >> <sstabellini@xxxxxxxxxx>; Wei Liu >> <wl@xxxxxxx>; KonradRzeszutek Wilk <konrad.wilk@xxxxxxxxxx>; Andrew Cooper >> <Andrew.Cooper3@xxxxxxxxxx>; David Scott <dave@xxxxxxxxxx>; Tim (Xen.org) >> <tim@xxxxxxx>; George Dunlap >> <George.Dunlap@xxxxxxxxxx>; Tamas K Lengyel <tamas@xxxxxxxxxxxxx>; Ian >> Jackson >> <Ian.Jackson@xxxxxxxxxx>; Anthony Perard <anthony.perard@xxxxxxxxxx>; >> xen-devel@xxxxxxxxxxxxxxxxxxxx; >> Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>; Roger Pau Monne >> <roger.pau@xxxxxxxxxx>; Julien Grall >> <julien.grall@xxxxxxx> >> Subject: Re: [Xen-devel] [PATCH v13 0/4] add per-domain IOMMU control >> >> >> [CC Julien] >> >> >> Hi Paul >> >> I may mistake, but looks like >> >> 80ff3d338dc93260b41ffeeebb0f852c2edef9ce iommu: tidy up >> iommu_use_hap_pt() and need_iommu_pt_sync() macros >> >> triggers ASSERT_UNREACHABLE on Arm if no IOMMU has been found (I built >> with my platform's IOMMU driver disabled: # CONFIG_IPMMU_VMSA is not set) . >> >> So, iommu_setup() calls clear_iommu_hap_pt_share() with >> iommu_hap_pt_share being set (CONFIG_IOMMU_FORCE_PT_SHARE=y) which, >> actually, triggers ASSERT. >> > > Here a minimal patch, leaving 'force pt share' in place. Does this avoid the > problem? > > ---8<--- > diff --git a/xen/common/sysctl.c b/xen/common/sysctl.c > index e8763c7fdf..f88a285e7f 100644 > --- a/xen/common/sysctl.c > +++ b/xen/common/sysctl.c > @@ -268,9 +268,11 @@ long do_sysctl(XEN_GUEST_HANDLE_PARAM(xen_sysctl_t) > u_sysctl) > pi->max_mfn = get_upper_mfn_bound(); > arch_do_physinfo(pi); > if ( iommu_enabled ) > + { > pi->capabilities |= XEN_SYSCTL_PHYSCAP_directio; > - if ( iommu_hap_pt_share ) > - pi->capabilities |= XEN_SYSCTL_PHYSCAP_iommu_hap_pt_share; > + if ( iommu_hap_pt_share ) > + pi->capabilities |= XEN_SYSCTL_PHYSCAP_iommu_hap_pt_share; > + } > > if ( copy_to_guest(u_sysctl, op, 1) ) > ret = -EFAULT; > diff --git a/xen/include/xen/iommu.h b/xen/include/xen/iommu.h > index 7c3003f3f1..6a10a24128 100644 > --- a/xen/include/xen/iommu.h > +++ b/xen/include/xen/iommu.h > @@ -68,8 +68,6 @@ static inline void clear_iommu_hap_pt_share(void) > { > #ifndef iommu_hap_pt_share > iommu_hap_pt_share = false; > -#elif iommu_hap_pt_share > - ASSERT_UNREACHABLE(); > #endif IHMO, calling this function is a mistake on platform only supporting shared page-table so the ASSERT() should be kept here. This raises the question why the function is actually called from common code. iommu_hap_enabled() should technically not be used in any code if the IOMMU is not enabled/present. So what are you trying to prevent here? Cheers, -- Julien Grall _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxxxxxxxxx https://lists.xenproject.org/mailman/listinfo/xen-devel
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |