| Message ID | efa0c2578a6aabb642b8f38257cf96a983944301.1605527997.git.rahul.singh@arm.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | xen/arm: Make PCI passthrough code non-x86 specific | expand |
On 16.11.2020 13:25, Rahul Singh wrote: > If mem-sharing, mem-paging, or log-dirty functionality is not enabled > for non-x86 architecture when HAS_PCI is enabled, the compiler will > throw an error. > > Move code to x86 specific directory to fix compilation error. Perhaps rather "file" than "directory"? > Also, modify the code to use likely() in place of unlikley() for each > condition to make code more optimized. > > No functional change. > > Signed-off-by: Rahul Singh <rahul.singh@arm.com> In principle I'm okay with this now, but there continue to be a few nits: > --- a/xen/drivers/passthrough/x86/iommu.c > +++ b/xen/drivers/passthrough/x86/iommu.c > @@ -23,6 +23,7 @@ > #include <asm/hvm/io.h> > #include <asm/io_apic.h> > #include <asm/setup.h> > +#include <xen/vm_event.h> Please insert this alongside the other "#include <xen/...>" higher up. > @@ -315,6 +316,17 @@ int iommu_update_ire_from_msi( > ? iommu_call(&iommu_ops, update_ire_from_msi, msi_desc, msg) : 0; > } > > +bool arch_iommu_use_permitted(const struct domain *d) > +{ > + /* > + * Prevent device assign if mem paging, mem sharing or log-dirty > + * have been enabled for this domain. > + */ > + return d == dom_io || > + (likely(!mem_sharing_enabled(d)) && > + likely(!vm_event_check_ring(d->vm_event_paging)) && > + likely(!p2m_get_hostp2m(d)->global_logdirty)); > +} > /* > * Local variables: > * mode: C Please don't alter stylistic aspects like this trailing comment being preceded by a blank line. > --- a/xen/include/xen/iommu.h > +++ b/xen/include/xen/iommu.h > @@ -381,6 +381,8 @@ DECLARE_PER_CPU(bool_t, iommu_dont_flush_iotlb); > extern struct spinlock iommu_pt_cleanup_lock; > extern struct page_list_head iommu_pt_cleanup_list; > > +bool arch_iommu_use_permitted(const struct domain *d); Just FTR - this way you effectively preclude an arch from making this a trivial static inline in one of its headers. Jan
Hello Jan, > On 17 Nov 2020, at 11:12 am, Jan Beulich <jbeulich@suse.com> wrote: > > On 16.11.2020 13:25, Rahul Singh wrote: >> If mem-sharing, mem-paging, or log-dirty functionality is not enabled >> for non-x86 architecture when HAS_PCI is enabled, the compiler will >> throw an error. >> >> Move code to x86 specific directory to fix compilation error. > > Perhaps rather "file" than "directory”? Ok. > >> Also, modify the code to use likely() in place of unlikley() for each >> condition to make code more optimized. >> >> No functional change. >> >> Signed-off-by: Rahul Singh <rahul.singh@arm.com> > > In principle I'm okay with this now, but there continue to be a few > nits: Thanks for reviewing the code I will fix all comments and will share the next version. > >> --- a/xen/drivers/passthrough/x86/iommu.c >> +++ b/xen/drivers/passthrough/x86/iommu.c >> @@ -23,6 +23,7 @@ >> #include <asm/hvm/io.h> >> #include <asm/io_apic.h> >> #include <asm/setup.h> >> +#include <xen/vm_event.h> > > Please insert this alongside the other "#include <xen/...>" higher up. Ok. > >> @@ -315,6 +316,17 @@ int iommu_update_ire_from_msi( >> ? iommu_call(&iommu_ops, update_ire_from_msi, msi_desc, msg) : 0; >> } >> >> +bool arch_iommu_use_permitted(const struct domain *d) >> +{ >> + /* >> + * Prevent device assign if mem paging, mem sharing or log-dirty >> + * have been enabled for this domain. >> + */ >> + return d == dom_io || >> + (likely(!mem_sharing_enabled(d)) && >> + likely(!vm_event_check_ring(d->vm_event_paging)) && >> + likely(!p2m_get_hostp2m(d)->global_logdirty)); >> +} >> /* >> * Local variables: >> * mode: C > > Please don't alter stylistic aspects like this trailing comment > being preceded by a blank line. Ok. > >> --- a/xen/include/xen/iommu.h >> +++ b/xen/include/xen/iommu.h >> @@ -381,6 +381,8 @@ DECLARE_PER_CPU(bool_t, iommu_dont_flush_iotlb); >> extern struct spinlock iommu_pt_cleanup_lock; >> extern struct page_list_head iommu_pt_cleanup_list; >> >> +bool arch_iommu_use_permitted(const struct domain *d); > > Just FTR - this way you effectively preclude an arch from > making this a trivial static inline in one of its headers. > > Jan
diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c index e8a28df126..804b24a0e0 100644 --- a/xen/drivers/passthrough/pci.c +++ b/xen/drivers/passthrough/pci.c @@ -20,7 +20,6 @@ #include <xen/iommu.h> #include <xen/irq.h> #include <xen/param.h> -#include <xen/vm_event.h> #include <xen/delay.h> #include <xen/keyhandler.h> #include <xen/event.h> @@ -1411,12 +1410,7 @@ static int assign_device(struct domain *d, u16 seg, u8 bus, u8 devfn, u32 flag) if ( !is_iommu_enabled(d) ) return 0; - /* Prevent device assign if mem paging or mem sharing have been - * enabled for this domain */ - if ( d != dom_io && - unlikely(mem_sharing_enabled(d) || - vm_event_check_ring(d->vm_event_paging) || - p2m_get_hostp2m(d)->global_logdirty) ) + if( !arch_iommu_use_permitted(d) ) return -EXDEV; /* device_assigned() should already have cleared the device for assignment */ diff --git a/xen/drivers/passthrough/x86/iommu.c b/xen/drivers/passthrough/x86/iommu.c index 875e67b53b..26f57b7e88 100644 --- a/xen/drivers/passthrough/x86/iommu.c +++ b/xen/drivers/passthrough/x86/iommu.c @@ -23,6 +23,7 @@ #include <asm/hvm/io.h> #include <asm/io_apic.h> #include <asm/setup.h> +#include <xen/vm_event.h> const struct iommu_init_ops *__initdata iommu_init_ops; struct iommu_ops __read_mostly iommu_ops; @@ -315,6 +316,17 @@ int iommu_update_ire_from_msi( ? iommu_call(&iommu_ops, update_ire_from_msi, msi_desc, msg) : 0; } +bool arch_iommu_use_permitted(const struct domain *d) +{ + /* + * Prevent device assign if mem paging, mem sharing or log-dirty + * have been enabled for this domain. + */ + return d == dom_io || + (likely(!mem_sharing_enabled(d)) && + likely(!vm_event_check_ring(d->vm_event_paging)) && + likely(!p2m_get_hostp2m(d)->global_logdirty)); +} /* * Local variables: * mode: C diff --git a/xen/include/xen/iommu.h b/xen/include/xen/iommu.h index 191021870f..056eaa09fc 100644 --- a/xen/include/xen/iommu.h +++ b/xen/include/xen/iommu.h @@ -381,6 +381,8 @@ DECLARE_PER_CPU(bool_t, iommu_dont_flush_iotlb); extern struct spinlock iommu_pt_cleanup_lock; extern struct page_list_head iommu_pt_cleanup_list; +bool arch_iommu_use_permitted(const struct domain *d); + #endif /* _IOMMU_H_ */ /*
If mem-sharing, mem-paging, or log-dirty functionality is not enabled for non-x86 architecture when HAS_PCI is enabled, the compiler will throw an error. Move code to x86 specific directory to fix compilation error. Also, modify the code to use likely() in place of unlikley() for each condition to make code more optimized. No functional change. Signed-off-by: Rahul Singh <rahul.singh@arm.com> --- Changes in v3: - rename arch_iommu_usable() to arch_iommu_use_permitted() - fixed comments. --- xen/drivers/passthrough/pci.c | 8 +------- xen/drivers/passthrough/x86/iommu.c | 12 ++++++++++++ xen/include/xen/iommu.h | 2 ++ 3 files changed, 15 insertions(+), 7 deletions(-)