Re: [for-4.15][RESEND PATCH v4 1/2] xen/x86: iommu: Ignore IOMMU mapping requests when a domain is dying

[Julien Grall <julien@xxxxxxx>]

Hi Jan,

On 25/02/2021 13:18, Jan Beulich wrote:
> On 25.02.2021 12:56, Julien Grall wrote:
> > On 24/02/2021 14:07, Jan Beulich wrote:
> > > On 24.02.2021 10:43, Julien Grall wrote:
> > > > --- a/xen/drivers/passthrough/x86/iommu.c
> > > > +++ b/xen/drivers/passthrough/x86/iommu.c
> > > > @@ -267,6 +267,12 @@ int iommu_free_pgtables(struct domain *d)
> > > >        struct page_info *pg;
> > > >        unsigned int done = 0;
> > > >    
> > > > +    if ( !is_iommu_enabled(d) )
> > > > +        return 0;
> > > Why is this addition needed? Hitting a not yet initialize spin lock
> > > is - afaict - no worse than a not yet initialized list, so it would
> > > seem to me that this can't be the reason. No other reason looks to
> > > be called out by the description.
> > struct domain_iommu will be initially zeroed as it is part of struct domain.
> >
> > For the list, we are so far fine because page_list_remove_head()
> > tolerates NULL. If we were using the normal list operations (e.g.
> > list_del), then this code would have segfaulted.
> And so we do, in the CONFIG_BIGMEM case. May I suggest then to split
> this out as a prereq patch, or add wording to the description
> mentioning this additional effect?
You are correct, I can crash the hypervisor when enabling 
CONFIG_BIGMEM=y and not using the IOMMU. I will move this chunk in a 
separate patch.
Cheers,

--
Julien Grall