1. Patchew
  2. Xen
  3. View series

[PATCH] VT-d: fix off-by-one when handling extra RMRR ranges

Marek Marczykowski-Górecki posted 1 patch 1 month, 4 weeks ago
Patches applied successfully (tree, apply log)
git fetch https://gitlab.com/xen-project/patchew/xen tags/patchew/20260213031801.1744145-1-marmarek@invisiblethingslab.com
xen/drivers/passthrough/vtd/dmar.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[PATCH] VT-d: fix off-by-one when handling extra RMRR ranges
Posted by Marek Marczykowski-Górecki 1 month, 4 weeks ago 
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
Re: [PATCH] VT-d: fix off-by-one when handling extra RMRR ranges
Posted by Roger Pau Monné 1 month, 4 weeks ago 
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.
Re: [PATCH] VT-d: fix off-by-one when handling extra RMRR ranges
Posted by Marek Marczykowski-Górecki 1 month, 4 weeks ago 
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

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.