The function returning zero is an error indication; we shouldn't try to
map MFN 0 and then treat that page as a context table.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
Whether to really have the ASSERT_UNREACHABLE() on the unmap path I'm
not quite sure about: According to my checking, the early bailing from
intel_iommu_remove_device() is what covers the case where an earlier
mapping attempt may have failed; in all other cases a mapping was
previously established. IOW while it should indeed be impossible to
hit that path, there's not really a problem if it was taken: It would
then be no different from finding a non-present context entry.

--- a/xen/drivers/passthrough/vtd/iommu.c
+++ b/xen/drivers/passthrough/vtd/iommu.c
@@ -1499,6 +1499,11 @@ int domain_context_mapping_one(
     ASSERT(pcidevs_locked());
     spin_lock(&iommu->lock);
     maddr = bus_to_context_maddr(iommu, bus);
+    if ( !maddr )
+    {
+        spin_unlock(&iommu->lock);
+        return -ENOMEM;
+    }
     context_entries = (struct context_entry *)map_vtd_domain_page(maddr);
     context = &context_entries[devfn];
     old = (lctxt = *context).full;
@@ -1853,6 +1858,12 @@ int domain_context_unmap_one(
     spin_lock(&iommu->lock);
 
     maddr = bus_to_context_maddr(iommu, bus);
+    if ( !maddr )
+    {
+        ASSERT_UNREACHABLE();
+        spin_unlock(&iommu->lock);
+        return 0;
+    }
     context_entries = (struct context_entry *)map_vtd_domain_page(maddr);
     context = &context_entries[devfn];

On 07/08/2025 3:10 pm, Jan Beulich wrote:
> The function returning zero is an error indication; we shouldn't try to
> map MFN 0 and then treat that page as a context table.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>