All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Marek Marczykowski-Górecki" <marmarek@invisiblethingslab.com>
To: xen-devel@lists.xenproject.org
Cc: "Marek Marczykowski-Górecki" <marmarek@invisiblethingslab.com>,
	"Jan Beulich" <jbeulich@suse.com>, "Paul Durrant" <paul@xen.org>,
	"Roger Pau Monné" <roger.pau@citrix.com>
Subject: [PATCH v2 2/2] drivers/char: mark extra reserved device memory in memory map
Date: Tue, 12 Mar 2024 17:25:15 +0100	[thread overview]
Message-ID: <20240312162541.384793-2-marmarek@invisiblethingslab.com> (raw)
In-Reply-To: <20240312162541.384793-1-marmarek@invisiblethingslab.com>

The IOMMU driver checks if RMRR/IVMD are marked as reserved in memory
map. This should be true for addresses coming from the firmware, but
when extra pages used by Xen itself are included in the mapping, those
are taken from usable RAM used. Mark those pages as reserved too.

Not marking the pages as reserved didn't caused issues before due to
another a bug in IOMMU driver code, that was fixed in 83afa3135830
("amd-vi: fix IVMD memory type checks").

Failing to reserve memory will lead to panic in IOMMU setup code. And
not including the page in IOMMU mapping will lead to broken console (due
to IOMMU faults). The pages chosen by the XHCI console driver should
still be usable by the CPU though, and the console code already can deal
with too slow console by dropping characters (and console not printing
anything is a special case of "slow"). When reserving fails print an error
message showing which pages failed and who requested them. This should
be enough hint to find why XHCI console doesn't work.

Fixes: 3a1a7b809ffa "drivers/char: mark DMA buffers as reserved for the XHCI"
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
---
Alternative error handling could be a panic, but with this version I
think it can be avoided. And not panicing gives a better chance to
actually see the error message (from the hopefully started dom0),
especially as the affected driver is the console one.

The reserve_e820_ram() is x86-specific. Is there some equivalent API for
ARM, or maybe even some abstract one? That said, I have no way to test
XHCI console on ARM, I don't know if such hardware even exists...

Changes in v2:
- move reserving to iommu_get_extra_reserved_device_memory() to cover
  all users of iommu_add_extra_reserved_device_memory()
- change error handling to not panic, as in this code layout it can skip
  sending the pages to the IOMMU driver
---
 xen/drivers/passthrough/iommu.c | 19 +++++++++++++++++++
 xen/include/xen/iommu.h         |  5 ++++-
 2 files changed, 23 insertions(+), 1 deletion(-)

diff --git a/xen/drivers/passthrough/iommu.c b/xen/drivers/passthrough/iommu.c
index 03587c0cd680..a311a37a2a03 100644
--- a/xen/drivers/passthrough/iommu.c
+++ b/xen/drivers/passthrough/iommu.c
@@ -21,6 +21,9 @@
 #include <xen/softirq.h>
 #include <xen/keyhandler.h>
 #include <xsm/xsm.h>
+#ifdef CONFIG_X86
+#include <asm/e820.h>
+#endif
 
 unsigned int __read_mostly iommu_dev_iotlb_timeout = 1000;
 integer_param("iommu_dev_iotlb_timeout", iommu_dev_iotlb_timeout);
@@ -715,6 +718,22 @@ int __init iommu_get_extra_reserved_device_memory(iommu_grdm_t *func,
 
     for ( idx = 0; idx < nr_extra_reserved_ranges; idx++ )
     {
+#ifdef CONFIG_X86
+        if ( !reserve_e820_ram(
+                &e820,
+                pfn_to_paddr(extra_reserved_ranges[idx].start),
+                pfn_to_paddr(extra_reserved_ranges[idx].start +
+                             extra_reserved_ranges[idx].nr)) )
+        {
+            printk(XENLOG_ERR "Failed to reserve [%"PRIx64"-%"PRIx64") for %s, "
+                   "skipping IOMMU mapping for it, some functionality may be broken\n",
+                   pfn_to_paddr(extra_reserved_ranges[idx].start),
+                   pfn_to_paddr(extra_reserved_ranges[idx].start +
+                                extra_reserved_ranges[idx].nr),
+                   extra_reserved_ranges[idx].name);
+            continue;
+        }
+#endif
         ret = func(extra_reserved_ranges[idx].start,
                    extra_reserved_ranges[idx].nr,
                    extra_reserved_ranges[idx].sbdf.sbdf,
diff --git a/xen/include/xen/iommu.h b/xen/include/xen/iommu.h
index b7829dff4588..875eaeb90167 100644
--- a/xen/include/xen/iommu.h
+++ b/xen/include/xen/iommu.h
@@ -324,7 +324,8 @@ struct iommu_ops {
 };
 
 /*
- * To be called by Xen internally, to register extra RMRR/IVMD ranges.
+ * To be called by Xen internally, to register extra RMRR/IVMD ranges for RAM
+ * pages.
  * Needs to be called before IOMMU initialization.
  */
 extern int iommu_add_extra_reserved_device_memory(unsigned long start,
@@ -334,6 +335,8 @@ extern int iommu_add_extra_reserved_device_memory(unsigned long start,
 /*
  * To be called by specific IOMMU driver during initialization,
  * to fetch ranges registered with iommu_add_extra_reserved_device_memory().
+ * This has a side effect of marking requested ranges as "reserverd" in the
+ * memory map.
  */
 extern int iommu_get_extra_reserved_device_memory(iommu_grdm_t *func,
                                                   void *ctxt);
-- 
2.43.0



  reply	other threads:[~2024-03-12 16:26 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-03-12 16:25 [PATCH v2 1/2] IOMMU: store name for extra reserved device memory Marek Marczykowski-Górecki
2024-03-12 16:25 ` Marek Marczykowski-Górecki [this message]
2024-03-18 13:48   ` [PATCH v2 2/2] drivers/char: mark extra reserved device memory in memory map Jan Beulich
2024-03-27  2:33     ` Marek Marczykowski-Górecki
2024-03-18 13:40 ` [PATCH v2 1/2] IOMMU: store name for extra reserved device memory Jan Beulich
2024-03-18 15:52   ` Roger Pau Monné
2024-03-27  2:46     ` Marek Marczykowski-Górecki

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240312162541.384793-2-marmarek@invisiblethingslab.com \
    --to=marmarek@invisiblethingslab.com \
    --cc=jbeulich@suse.com \
    --cc=paul@xen.org \
    --cc=roger.pau@citrix.com \
    --cc=xen-devel@lists.xenproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.