Re: [Xen-devel] [PATCH v13 0/4] add per-domain IOMMU control

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: Oleksandr <olekstysh@xxxxxxxxx>
> Sent: 25 September 2019 19:07
> 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
> 
> 
> 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
> >>   }
> >> ---8<---
> >>
> >>    Paul
> >
> > Thank you for the patch, but I need some time to check it (now I have
> > some troubles with starting guest VM). I will inform you once I check.
> 
> 
> With the patch applied, the issue I have faced (during Xen boot) is gone
> away. But, I haven't analyzed your "per-domain IOMMU control series" yet
> to have opinion regarding your patch itself.
> 
> 
> I noticed the following:
> 
> When iommu_enabled = false (my case, when IOMMU driver is disable), I
> can't create guest VM if "dtdev" property is present and contains DMA
> masters in the domain config:
> 
> Parsing config from /xt/dom.cfg/domd.cfg
> ERROR: passthrough not supported on this platform
> 
> Even if I add passthrough = "disabled", it still denies:
> 
> Parsing config from /xt/dom.cfg/domd.cfg
> ERROR: passthrough disabled but devices are specified
> 
> Looks like, correct behavior...
> 

Yes, that all seems correct. Passthrough should not be done without an IOMMU.

  Paul

> 
> --
> Regards,
> 
> Oleksandr Tyshchenko

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel