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
