[RFC] xen/x86: allow overlaps with non-RAM regions

Commit Message

Stefano Stabellini April 4, 2025, 1:01 a.m. UTC

On one Sapphire AMD x86 board, I see this:


(XEN) [0000003943ca6ff2]  [00000000f0000000, 00000000f7ffffff] (reserved)
(XEN) [00000039460886d9]  [00000000fd000000, 00000000ffffffff] (reserved)
[...]
(XEN) [    4.612235] 0000:02:00.0: not mapping BAR [fea00, fea03] invalid position


Linux boots fine on this platform but Linux as Dom0 on Xen does not.
This is because the pci_check_bar->is_memory_hole check fails due to the
MMIO region overlapping with the EFI reserved region.

While I think ideally this should not happen, as you can imagine users
are never happy when Linux baremetal boots fine, and Linux on Xen does
not.

This patch fixes the boot issue by relaxing the is_memory_hole check.

Signed-off-by: Stefano Stabellini <[e-mail]>

Comments

Jan Beulich April 4, 2025, 8:07 a.m. UTC | #1

On 04.04.2025 03:01, Stefano Stabellini wrote:
> On one Sapphire AMD x86 board, I see this:
> 
> 
> (XEN) [0000003943ca6ff2]  [00000000f0000000, 00000000f7ffffff] (reserved)
> (XEN) [00000039460886d9]  [00000000fd000000, 00000000ffffffff] (reserved)
> [...]
> (XEN) [    4.612235] 0000:02:00.0: not mapping BAR [fea00, fea03] invalid position

I, too, see something like this on an SPR system. That's a firmware issue,
which hence first of all should be dealt with at the firmware side.

> Linux boots fine on this platform but Linux as Dom0 on Xen does not.
> This is because the pci_check_bar->is_memory_hole check fails due to the
> MMIO region overlapping with the EFI reserved region.

And then what's the actual, observable problem? On my system I haven't
noticed anything going wrong yet, albeit the affected device is also left
without a driver.

Also aiui you strictly mean PVH Dom0 here?

> --- a/xen/arch/x86/mm.c
> +++ b/xen/arch/x86/mm.c
> @@ -797,6 +797,9 @@ bool is_memory_hole(mfn_t start, mfn_t end)
>          if ( !entry->size )
>              continue;
>  
> +        if ( entry->type > 1 )
> +            continue;

I'm sorry to ask, but what's a literal 1 here? I'm pretty convinced we
would want to still object to overlaps with unusable ranges, for example.
Yet by open-coding what E820_RAM expands to you completely hide what this
check is about. Yes, this is an RFC, but even there such context is
important.

Furthermore my general take here is: We shouldn't simply silence issues
arising from firmware doing odd things. My take here is still the same
as the position I took when I still was maintainer of the EFI code in
Xen: We shouldn't by default work around such issues, when doing so may
negatively affect systems not exposing such odd behavior.

Finally a Misra-related observation while looking at this: Isn't
is_memory_hole() unreachable code in a !HVM configuration?

Jan

Roger Pau Monné April 4, 2025, 10:28 a.m. UTC | #2

On Thu, Apr 03, 2025 at 06:01:42PM -0700, Stefano Stabellini wrote:
> On one Sapphire AMD x86 board, I see this:
> 
> 
> (XEN) [0000003943ca6ff2]  [00000000f0000000, 00000000f7ffffff] (reserved)
> (XEN) [00000039460886d9]  [00000000fd000000, 00000000ffffffff] (reserved)
> [...]
> (XEN) [    4.612235] 0000:02:00.0: not mapping BAR [fea00, fea03] invalid position
> 
> 
> Linux boots fine on this platform but Linux as Dom0 on Xen does not.
> This is because the pci_check_bar->is_memory_hole check fails due to the
> MMIO region overlapping with the EFI reserved region.

That's weird.  (Partially) the reason to not attempt to map such BAR
is that it should already be mapped, because at dom0 creation time all
reserved regions are added to the p2m (see arch_iommu_hwdom_init()).
If that's not the case we should figure out why this reserved region
is not added to dom0 p2m as part of arch_iommu_hwdom_init().

Can you paste the dom0 build output when booted with `iommu=verbose
dom0=pvh,verbose`?  Does using `dom0=pvh,verbose,pf-fixup` solve the
boot issue? (and can you paste the output if it does)

The issue with allowing BARs to modify p2m reserved regions is that if
memory decoding is disabled for the PCI device, the BAR will be
unmapped from the p2m, thus creating a hole in the p2m for a reserved
region, which would be undesirable IMO.

Thanks, Roger.

Patch

diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index b294497a14..afb54d6f0f 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -797,6 +797,9 @@  bool is_memory_hole(mfn_t start, mfn_t end)
         if ( !entry->size )
             continue;
 
+        if ( entry->type > 1 )
+            continue;
+
         /* Do not allow overlaps with any memory range. */
         if ( s <= PFN_DOWN(entry->addr + entry->size - 1) &&
              PFN_DOWN(entry->addr) <= e )