* [PATCH] VT-d: fix off-by-one when handling extra RMRR ranges
@ 2026-02-13 3:17 Marek Marczykowski-Górecki
2026-02-13 8:16 ` Roger Pau Monné
0 siblings, 1 reply; 3+ messages in thread
From: Marek Marczykowski-Górecki @ 2026-02-13 3:17 UTC (permalink / raw)
To: xen-devel
Cc: Marek Marczykowski-Górecki, Jan Beulich, Andrew Cooper,
Roger Pau Monné
add_one_user_rmrr() operates on inclusive [start,end] range, which means
the end page needs to be calculated as (start + page_count - 1).
This off-by-one error resulted in one extra pages being mapped in IOMMU
context, but not marked as reserved in the memory map. This in turns
confused PVH dom0 code, resulting in the following crash:
(XEN) [ 3.934848] d0: GFN 0x5475c (0x5475c,5,3) -> (0x46a0f4,0,7) not permitted (0x20)
(XEN) [ 3.969657] domain_crash called from arch/x86/mm/p2m.c:695
(XEN) [ 3.972568] Domain 0 reported crashed by domain 32767 on cpu#0:
(XEN) [ 3.975527] Hardware Dom0 crashed: rebooting machine in 5 seconds.
(XEN) [ 8.986353] Resetting with ACPI MEMORY or I/O RESET_REG.
I checked other parts of this API and it was the only error like this.
Other places:
- iommu_get_extra_reserved_device_memory() -> reserve_e820_ram() - this
function expects exclusive range, so the code is correct
- add_one_extra_ivmd() - this operates on start address and memory
length
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
---
xen/drivers/passthrough/vtd/dmar.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/xen/drivers/passthrough/vtd/dmar.c b/xen/drivers/passthrough/vtd/dmar.c
index 91c22b833043..3da0854e6d91 100644
--- a/xen/drivers/passthrough/vtd/dmar.c
+++ b/xen/drivers/passthrough/vtd/dmar.c
@@ -1065,7 +1065,7 @@ static int __init add_user_rmrr(void)
static int __init cf_check add_one_extra_rmrr(xen_pfn_t start, xen_ulong_t nr, u32 id, void *ctxt)
{
u32 sbdf_array[] = { id };
- return add_one_user_rmrr(start, start+nr, 1, sbdf_array);
+ return add_one_user_rmrr(start, start + nr - 1, 1, sbdf_array);
}
static int __init add_extra_rmrr(void)
--
2.51.0
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] VT-d: fix off-by-one when handling extra RMRR ranges
2026-02-13 3:17 [PATCH] VT-d: fix off-by-one when handling extra RMRR ranges Marek Marczykowski-Górecki
@ 2026-02-13 8:16 ` Roger Pau Monné
2026-02-13 9:37 ` Marek Marczykowski-Górecki
0 siblings, 1 reply; 3+ messages in thread
From: Roger Pau Monné @ 2026-02-13 8:16 UTC (permalink / raw)
To: Marek Marczykowski-Górecki; +Cc: xen-devel, Jan Beulich, Andrew Cooper
On Fri, Feb 13, 2026 at 04:17:48AM +0100, Marek Marczykowski-Górecki wrote:
> add_one_user_rmrr() operates on inclusive [start,end] range, which means
> the end page needs to be calculated as (start + page_count - 1).
> This off-by-one error resulted in one extra pages being mapped in IOMMU
> context, but not marked as reserved in the memory map. This in turns
> confused PVH dom0 code, resulting in the following crash:
>
> (XEN) [ 3.934848] d0: GFN 0x5475c (0x5475c,5,3) -> (0x46a0f4,0,7) not permitted (0x20)
> (XEN) [ 3.969657] domain_crash called from arch/x86/mm/p2m.c:695
> (XEN) [ 3.972568] Domain 0 reported crashed by domain 32767 on cpu#0:
> (XEN) [ 3.975527] Hardware Dom0 crashed: rebooting machine in 5 seconds.
> (XEN) [ 8.986353] Resetting with ACPI MEMORY or I/O RESET_REG.
>
> I checked other parts of this API and it was the only error like this.
> Other places:
> - iommu_get_extra_reserved_device_memory() -> reserve_e820_ram() - this
> function expects exclusive range, so the code is correct
> - add_one_extra_ivmd() - this operates on start address and memory
> length
>
You possibly want:
Fixes: 2d9b3699136d ("IOMMU/VT-d: wire common device reserved memory API")
> Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
> ---
> xen/drivers/passthrough/vtd/dmar.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/xen/drivers/passthrough/vtd/dmar.c b/xen/drivers/passthrough/vtd/dmar.c
> index 91c22b833043..3da0854e6d91 100644
> --- a/xen/drivers/passthrough/vtd/dmar.c
> +++ b/xen/drivers/passthrough/vtd/dmar.c
> @@ -1065,7 +1065,7 @@ static int __init add_user_rmrr(void)
> static int __init cf_check add_one_extra_rmrr(xen_pfn_t start, xen_ulong_t nr, u32 id, void *ctxt)
> {
> u32 sbdf_array[] = { id };
> - return add_one_user_rmrr(start, start+nr, 1, sbdf_array);
> + return add_one_user_rmrr(start, start + nr - 1, 1, sbdf_array);
While here, would you mind if we add a newline between the sbdf_array
definition and the return?
Thanks, Roger.
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] VT-d: fix off-by-one when handling extra RMRR ranges
2026-02-13 8:16 ` Roger Pau Monné
@ 2026-02-13 9:37 ` Marek Marczykowski-Górecki
0 siblings, 0 replies; 3+ messages in thread
From: Marek Marczykowski-Górecki @ 2026-02-13 9:37 UTC (permalink / raw)
To: Roger Pau Monné; +Cc: xen-devel, Jan Beulich, Andrew Cooper
[-- Attachment #1: Type: text/plain, Size: 2480 bytes --]
On Fri, Feb 13, 2026 at 09:16:50AM +0100, Roger Pau Monné wrote:
> On Fri, Feb 13, 2026 at 04:17:48AM +0100, Marek Marczykowski-Górecki wrote:
> > add_one_user_rmrr() operates on inclusive [start,end] range, which means
> > the end page needs to be calculated as (start + page_count - 1).
> > This off-by-one error resulted in one extra pages being mapped in IOMMU
> > context, but not marked as reserved in the memory map. This in turns
> > confused PVH dom0 code, resulting in the following crash:
> >
> > (XEN) [ 3.934848] d0: GFN 0x5475c (0x5475c,5,3) -> (0x46a0f4,0,7) not permitted (0x20)
> > (XEN) [ 3.969657] domain_crash called from arch/x86/mm/p2m.c:695
> > (XEN) [ 3.972568] Domain 0 reported crashed by domain 32767 on cpu#0:
> > (XEN) [ 3.975527] Hardware Dom0 crashed: rebooting machine in 5 seconds.
> > (XEN) [ 8.986353] Resetting with ACPI MEMORY or I/O RESET_REG.
> >
> > I checked other parts of this API and it was the only error like this.
> > Other places:
> > - iommu_get_extra_reserved_device_memory() -> reserve_e820_ram() - this
> > function expects exclusive range, so the code is correct
> > - add_one_extra_ivmd() - this operates on start address and memory
> > length
> >
>
> You possibly want:
>
> Fixes: 2d9b3699136d ("IOMMU/VT-d: wire common device reserved memory API")
Yes, indeed.
> > Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
>
> Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
Thanks.
> > ---
> > xen/drivers/passthrough/vtd/dmar.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/xen/drivers/passthrough/vtd/dmar.c b/xen/drivers/passthrough/vtd/dmar.c
> > index 91c22b833043..3da0854e6d91 100644
> > --- a/xen/drivers/passthrough/vtd/dmar.c
> > +++ b/xen/drivers/passthrough/vtd/dmar.c
> > @@ -1065,7 +1065,7 @@ static int __init add_user_rmrr(void)
> > static int __init cf_check add_one_extra_rmrr(xen_pfn_t start, xen_ulong_t nr, u32 id, void *ctxt)
> > {
> > u32 sbdf_array[] = { id };
> > - return add_one_user_rmrr(start, start+nr, 1, sbdf_array);
> > + return add_one_user_rmrr(start, start + nr - 1, 1, sbdf_array);
>
> While here, would you mind if we add a newline between the sbdf_array
> definition and the return?
Yes, while at it makes sense to fix that too.
--
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] 3+ messages in thread
end of thread, other threads:[~2026-02-13 9:38 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-02-13 3:17 [PATCH] VT-d: fix off-by-one when handling extra RMRR ranges Marek Marczykowski-Górecki
2026-02-13 8:16 ` Roger Pau Monné
2026-02-13 9:37 ` 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.