Hello Jan,
> On 17 Nov 2020, at 11:12 am, Jan Beulich <jbeul...@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.si...@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
