Re: [PATCH for-4.15 1/4] xen/iommu: Check if the IOMMU was initialized before tearing down

[Jan Beulich <jbeulich@xxxxxxxx>]

On 23.12.2020 15:51, Julien Grall wrote:
> On 23/12/2020 13:59, Jan Beulich wrote:
>> On 23.12.2020 14:50, Julien Grall wrote:
>>> On 23/12/2020 13:27, Jan Beulich wrote:
>>>> And finally "we assume" is in at least latent conflict with
>>>> ->platform_ops getting set only after arch_iommu_domain_init()
>>>> was called. Right now neither x86'es nor Arm's do anything
>>>> that would need undoing, but I'd still suggest to replace
>>>> "assume" here (if you want to keep the comment in the first
>>>> place) and move the assignment up a few lines (right after the
>>>> is_iommu_enabled() check there).
>>> My initial implementation of this patch moved the initialization of
>>> hd->platform_ops before arch_iommu_domain_init().
>>>
>>> However, this is going to lead to some issues with Paul's series which
>>> add an IOMMU page-table pool ([1]).
>>>
>>> The function arch_iommu_domain_init() may now fail. If we initialize
>>> hd->platform_ops earlier on, then the ->teardown() will be called before
>>> ->init().
>>>
>>> This may be an issue if ->teardown() expects some list pointer to be
>>> initialized by ->init(). I am not aware of any today, but this seems
>>> quite risky to me.
>>
>> In such a case the obvious thing is to make the teardown handler
>> check whether its init counterpart has run before. This will then
>> fit our apparently much wider goal of making cleanup functions
>> idempotent.
> 
> I will have a look. This may require another boolean which I wanted to 
> avoid.

I could imagine list_head_is_null() to become handy here.

Jan