* [PATCH v3 1/2] IOMMU: store name for extra reserved device memory
@ 2024-03-27 2:53 Marek Marczykowski-Górecki
2024-03-27 2:53 ` [PATCH v3 2/2] drivers/char: mark extra reserved device memory in memory map Marek Marczykowski-Górecki
2024-03-27 2:57 ` [PATCH v3 1/2] IOMMU: store name for extra reserved device memory Marek Marczykowski-Górecki
0 siblings, 2 replies; 7+ messages in thread
From: Marek Marczykowski-Górecki @ 2024-03-27 2:53 UTC (permalink / raw
To: xen-devel
Cc: Marek Marczykowski-Górecki, Jan Beulich, Andrew Cooper,
George Dunlap, Julien Grall, Stefano Stabellini, Wei Liu,
Paul Durrant, Roger Pau Monné
It will be useful for error reporting in a subsequent patch.
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
---
New in v2
---
xen/drivers/char/xhci-dbc.c | 3 ++-
xen/drivers/passthrough/iommu.c | 5 ++++-
xen/include/xen/iommu.h | 3 ++-
3 files changed, 8 insertions(+), 3 deletions(-)
diff --git a/xen/drivers/char/xhci-dbc.c b/xen/drivers/char/xhci-dbc.c
index 3bf389be7d0b..8e2037f1a5f7 100644
--- a/xen/drivers/char/xhci-dbc.c
+++ b/xen/drivers/char/xhci-dbc.c
@@ -1421,7 +1421,8 @@ void __init xhci_dbc_uart_init(void)
iommu_add_extra_reserved_device_memory(
PFN_DOWN(virt_to_maddr(&dbc_dma_bufs)),
PFN_UP(sizeof(dbc_dma_bufs)),
- uart->dbc.sbdf);
+ uart->dbc.sbdf,
+ "XHCI console");
serial_register_uart(SERHND_XHCI, &dbc_uart_driver, &dbc_uart);
}
}
diff --git a/xen/drivers/passthrough/iommu.c b/xen/drivers/passthrough/iommu.c
index 996c31be1284..03587c0cd680 100644
--- a/xen/drivers/passthrough/iommu.c
+++ b/xen/drivers/passthrough/iommu.c
@@ -682,6 +682,7 @@ struct extra_reserved_range {
unsigned long start;
unsigned long nr;
pci_sbdf_t sbdf;
+ const char *name;
};
static unsigned int __initdata nr_extra_reserved_ranges;
static struct extra_reserved_range __initdata
@@ -689,7 +690,8 @@ static struct extra_reserved_range __initdata
int __init iommu_add_extra_reserved_device_memory(unsigned long start,
unsigned long nr,
- pci_sbdf_t sbdf)
+ pci_sbdf_t sbdf,
+ const char *name)
{
unsigned int idx;
@@ -700,6 +702,7 @@ int __init iommu_add_extra_reserved_device_memory(unsigned long start,
extra_reserved_ranges[idx].start = start;
extra_reserved_ranges[idx].nr = nr;
extra_reserved_ranges[idx].sbdf = sbdf;
+ extra_reserved_ranges[idx].name = name;
return 0;
}
diff --git a/xen/include/xen/iommu.h b/xen/include/xen/iommu.h
index 9621459c63ee..b7829dff4588 100644
--- a/xen/include/xen/iommu.h
+++ b/xen/include/xen/iommu.h
@@ -329,7 +329,8 @@ struct iommu_ops {
*/
extern int iommu_add_extra_reserved_device_memory(unsigned long start,
unsigned long nr,
- pci_sbdf_t sbdf);
+ pci_sbdf_t sbdf,
+ const char *name);
/*
* To be called by specific IOMMU driver during initialization,
* to fetch ranges registered with iommu_add_extra_reserved_device_memory().
--
2.43.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v3 2/2] drivers/char: mark extra reserved device memory in memory map
2024-03-27 2:53 [PATCH v3 1/2] IOMMU: store name for extra reserved device memory Marek Marczykowski-Górecki
@ 2024-03-27 2:53 ` Marek Marczykowski-Górecki
2024-04-03 7:10 ` Jan Beulich
2024-03-27 2:57 ` [PATCH v3 1/2] IOMMU: store name for extra reserved device memory Marek Marczykowski-Górecki
1 sibling, 1 reply; 7+ messages in thread
From: Marek Marczykowski-Górecki @ 2024-03-27 2:53 UTC (permalink / raw
To: xen-devel
Cc: Marek Marczykowski-Górecki, Jan Beulich, Paul Durrant,
Roger Pau Monné
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
Changes in v3:
- code style, typo
---
xen/drivers/passthrough/iommu.c | 17 +++++++++++++++++
xen/include/xen/iommu.h | 5 ++++-
2 files changed, 21 insertions(+), 1 deletion(-)
diff --git a/xen/drivers/passthrough/iommu.c b/xen/drivers/passthrough/iommu.c
index 03587c0cd680..ba18136c461c 100644
--- a/xen/drivers/passthrough/iommu.c
+++ b/xen/drivers/passthrough/iommu.c
@@ -22,6 +22,10 @@
#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 +719,19 @@ int __init iommu_get_extra_reserved_device_memory(iommu_grdm_t *func,
for ( idx = 0; idx < nr_extra_reserved_ranges; idx++ )
{
+#ifdef CONFIG_X86
+ paddr_t start = pfn_to_paddr(extra_reserved_ranges[idx].start);
+ paddr_t end = pfn_to_paddr(extra_reserved_ranges[idx].start +
+ extra_reserved_ranges[idx].nr);
+
+ if ( !reserve_e820_ram(&e820, start, end) )
+ {
+ printk(XENLOG_ERR "Failed to reserve [%"PRIx64"-%"PRIx64") for %s, "
+ "skipping IOMMU mapping for it, some functionality may be broken\n",
+ start, end, 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..1f56a6cf456a 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 "reserved" in the
+ * memory map.
*/
extern int iommu_get_extra_reserved_device_memory(iommu_grdm_t *func,
void *ctxt);
--
2.43.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v3 1/2] IOMMU: store name for extra reserved device memory
2024-03-27 2:53 [PATCH v3 1/2] IOMMU: store name for extra reserved device memory Marek Marczykowski-Górecki
2024-03-27 2:53 ` [PATCH v3 2/2] drivers/char: mark extra reserved device memory in memory map Marek Marczykowski-Górecki
@ 2024-03-27 2:57 ` Marek Marczykowski-Górecki
1 sibling, 0 replies; 7+ messages in thread
From: Marek Marczykowski-Górecki @ 2024-03-27 2:57 UTC (permalink / raw
To: xen-devel
Cc: Jan Beulich, Andrew Cooper, George Dunlap, Julien Grall,
Stefano Stabellini, Wei Liu, Paul Durrant, Roger Pau Monné
[-- Attachment #1: Type: text/plain, Size: 3266 bytes --]
On Wed, Mar 27, 2024 at 03:53:10AM +0100, Marek Marczykowski-Górecki wrote:
> It will be useful for error reporting in a subsequent patch.
>
> Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
> Acked-by: Jan Beulich <jbeulich@suse.com>
This one is already applied, sorry for re-send.
> ---
> New in v2
> ---
> xen/drivers/char/xhci-dbc.c | 3 ++-
> xen/drivers/passthrough/iommu.c | 5 ++++-
> xen/include/xen/iommu.h | 3 ++-
> 3 files changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/xen/drivers/char/xhci-dbc.c b/xen/drivers/char/xhci-dbc.c
> index 3bf389be7d0b..8e2037f1a5f7 100644
> --- a/xen/drivers/char/xhci-dbc.c
> +++ b/xen/drivers/char/xhci-dbc.c
> @@ -1421,7 +1421,8 @@ void __init xhci_dbc_uart_init(void)
> iommu_add_extra_reserved_device_memory(
> PFN_DOWN(virt_to_maddr(&dbc_dma_bufs)),
> PFN_UP(sizeof(dbc_dma_bufs)),
> - uart->dbc.sbdf);
> + uart->dbc.sbdf,
> + "XHCI console");
> serial_register_uart(SERHND_XHCI, &dbc_uart_driver, &dbc_uart);
> }
> }
> diff --git a/xen/drivers/passthrough/iommu.c b/xen/drivers/passthrough/iommu.c
> index 996c31be1284..03587c0cd680 100644
> --- a/xen/drivers/passthrough/iommu.c
> +++ b/xen/drivers/passthrough/iommu.c
> @@ -682,6 +682,7 @@ struct extra_reserved_range {
> unsigned long start;
> unsigned long nr;
> pci_sbdf_t sbdf;
> + const char *name;
> };
> static unsigned int __initdata nr_extra_reserved_ranges;
> static struct extra_reserved_range __initdata
> @@ -689,7 +690,8 @@ static struct extra_reserved_range __initdata
>
> int __init iommu_add_extra_reserved_device_memory(unsigned long start,
> unsigned long nr,
> - pci_sbdf_t sbdf)
> + pci_sbdf_t sbdf,
> + const char *name)
> {
> unsigned int idx;
>
> @@ -700,6 +702,7 @@ int __init iommu_add_extra_reserved_device_memory(unsigned long start,
> extra_reserved_ranges[idx].start = start;
> extra_reserved_ranges[idx].nr = nr;
> extra_reserved_ranges[idx].sbdf = sbdf;
> + extra_reserved_ranges[idx].name = name;
>
> return 0;
> }
> diff --git a/xen/include/xen/iommu.h b/xen/include/xen/iommu.h
> index 9621459c63ee..b7829dff4588 100644
> --- a/xen/include/xen/iommu.h
> +++ b/xen/include/xen/iommu.h
> @@ -329,7 +329,8 @@ struct iommu_ops {
> */
> extern int iommu_add_extra_reserved_device_memory(unsigned long start,
> unsigned long nr,
> - pci_sbdf_t sbdf);
> + pci_sbdf_t sbdf,
> + const char *name);
> /*
> * To be called by specific IOMMU driver during initialization,
> * to fetch ranges registered with iommu_add_extra_reserved_device_memory().
> --
> 2.43.0
>
--
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v3 2/2] drivers/char: mark extra reserved device memory in memory map
2024-03-27 2:53 ` [PATCH v3 2/2] drivers/char: mark extra reserved device memory in memory map Marek Marczykowski-Górecki
@ 2024-04-03 7:10 ` Jan Beulich
2024-04-14 0:32 ` Marek Marczykowski-Górecki
0 siblings, 1 reply; 7+ messages in thread
From: Jan Beulich @ 2024-04-03 7:10 UTC (permalink / raw
To: Marek Marczykowski-Górecki
Cc: Paul Durrant, Roger Pau Monné, xen-devel
On 27.03.2024 03:53, Marek Marczykowski-Górecki wrote:
> 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>
Acked-by: Jan Beulich <jbeulich@suse.com>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v3 2/2] drivers/char: mark extra reserved device memory in memory map
2024-04-03 7:10 ` Jan Beulich
@ 2024-04-14 0:32 ` Marek Marczykowski-Górecki
2024-04-17 14:17 ` Jan Beulich
0 siblings, 1 reply; 7+ messages in thread
From: Marek Marczykowski-Górecki @ 2024-04-14 0:32 UTC (permalink / raw
To: Jan Beulich; +Cc: Paul Durrant, Roger Pau Monné, xen-devel
[-- Attachment #1: Type: text/plain, Size: 1587 bytes --]
On Wed, Apr 03, 2024 at 09:10:40AM +0200, Jan Beulich wrote:
> On 27.03.2024 03:53, Marek Marczykowski-Górecki wrote:
> > 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>
>
> Acked-by: Jan Beulich <jbeulich@suse.com>
Hi,
Is any ack missing here, or has it just fallen through the cracks?
--
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v3 2/2] drivers/char: mark extra reserved device memory in memory map
2024-04-14 0:32 ` Marek Marczykowski-Górecki
@ 2024-04-17 14:17 ` Jan Beulich
2024-04-17 14:25 ` Marek Marczykowski-Górecki
0 siblings, 1 reply; 7+ messages in thread
From: Jan Beulich @ 2024-04-17 14:17 UTC (permalink / raw
To: Marek Marczykowski-Górecki
Cc: Paul Durrant, Roger Pau Monné, xen-devel
On 14.04.2024 02:32, Marek Marczykowski-Górecki wrote:
> On Wed, Apr 03, 2024 at 09:10:40AM +0200, Jan Beulich wrote:
>> On 27.03.2024 03:53, Marek Marczykowski-Górecki wrote:
>>> 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>
>>
>> Acked-by: Jan Beulich <jbeulich@suse.com>
>
> Is any ack missing here, or has it just fallen through the cracks?
??? (commit dd5101a6169f89b9e3f3b72f0b0fcdb38db2fb35)
Jan
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v3 2/2] drivers/char: mark extra reserved device memory in memory map
2024-04-17 14:17 ` Jan Beulich
@ 2024-04-17 14:25 ` Marek Marczykowski-Górecki
0 siblings, 0 replies; 7+ messages in thread
From: Marek Marczykowski-Górecki @ 2024-04-17 14:25 UTC (permalink / raw
To: Jan Beulich; +Cc: Paul Durrant, Roger Pau Monné, xen-devel
[-- Attachment #1: Type: text/plain, Size: 1877 bytes --]
On Wed, Apr 17, 2024 at 04:17:48PM +0200, Jan Beulich wrote:
> On 14.04.2024 02:32, Marek Marczykowski-Górecki wrote:
> > On Wed, Apr 03, 2024 at 09:10:40AM +0200, Jan Beulich wrote:
> >> On 27.03.2024 03:53, Marek Marczykowski-Górecki wrote:
> >>> 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>
> >>
> >> Acked-by: Jan Beulich <jbeulich@suse.com>
> >
> > Is any ack missing here, or has it just fallen through the cracks?
>
> ??? (commit dd5101a6169f89b9e3f3b72f0b0fcdb38db2fb35)
Oh, sorry, somehow I missed it. All good then, thanks.
--
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2024-04-17 14:25 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-03-27 2:53 [PATCH v3 1/2] IOMMU: store name for extra reserved device memory Marek Marczykowski-Górecki
2024-03-27 2:53 ` [PATCH v3 2/2] drivers/char: mark extra reserved device memory in memory map Marek Marczykowski-Górecki
2024-04-03 7:10 ` Jan Beulich
2024-04-14 0:32 ` Marek Marczykowski-Górecki
2024-04-17 14:17 ` Jan Beulich
2024-04-17 14:25 ` Marek Marczykowski-Górecki
2024-03-27 2:57 ` [PATCH v3 1/2] IOMMU: store name for extra reserved device memory Marek Marczykowski-Górecki
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.