[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v1 4/4] xen/pci: solve compilation error when memory paging is not enabled.



On 29.10.2020 17:58, Rahul Singh wrote:
>> On 28 Oct 2020, at 3:13 pm, Rahul Singh <Rahul.Singh@xxxxxxx> wrote:
>>> On 28 Oct 2020, at 11:56 am, Jan Beulich <jbeulich@xxxxxxxx> wrote:
>>> On 26.10.2020 18:17, Rahul Singh wrote:
>>>> --- a/xen/drivers/passthrough/pci.c
>>>> +++ b/xen/drivers/passthrough/pci.c
>>>> @@ -1419,13 +1419,15 @@ 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 
>>>> +#if defined(CONFIG_HAS_MEM_PAGING) || defined(CONFIG_MEM_SHARING)
>>>> +    /* 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) )
>>>>        return -EXDEV;
>>>> +#endif
>>>
>>> Besides this also disabling mem-sharing and log-dirty related
>>> logic, I don't think the change is correct: Each item being
>>> checked needs individually disabling depending on its associated
>>> CONFIG_*. For this, perhaps you want to introduce something
>>> like mem_paging_enabled(d), to avoid the need for #ifdef here?
>>>
>>
>> Ok I will fix that in next version. 
> 
> I just check and found out that mem-sharing , men-paging and log-dirty is x86 
> specific and not implemented for ARM.
> Is that will be ok if I move above code to x86 specific directory and 
> introduce new function arch_pcidev_is_assignable() that will test if pcidev 
> is assignable or not ?

As an immediate workaround - perhaps (long term the individual
pieces should still be individually checked here, as they're
not inherently x86-specific). Since there's no device property
involved here, the suggested name looks misleading. Perhaps
arch_iommu_usable(d)?

Jan



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.