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

[Marek Marczykowski-Górecki <marmarek@xxxxxxxxxxxxxxxxxxxxxx>]

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@xxxxxxxxxxxxxxxxxxxxxx>
> 
> Reviewed-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>

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: signature.asc
Description: PGP signature