| Message ID | 20240201170159.66330-2-roger.pau@citrix.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | iommu/x86: fixes/improvements for unity range checks | expand |
On Thursday, February 1st, 2024 at 18:01, Roger Pau Monne <roger.pau@citrix.com> wrote: > The current code that parses the IVMD blocks is relaxed with regard to the > restriction that such unity regions should always fall into memory ranges > marked as reserved in the memory map. > > However the type checks for the IVMD addresses are inverted, and as a result > IVMD ranges falling into RAM areas are accepted. Note that having such ranges > in the first place is a firmware bug, as IVMD should always fall into reserved > ranges. > > Fixes: ed6c77ebf0c1 ('AMD/IOMMU: check / convert IVMD ranges for being / to be reserved') > Signed-off-by: Roger Pau Monné roger.pau@citrix.com > > --- > Cc: oxjo@proton.me > --- > xen/drivers/passthrough/amd/iommu_acpi.c | 11 ++++++++--- > 1 file changed, 8 insertions(+), 3 deletions(-) > > diff --git a/xen/drivers/passthrough/amd/iommu_acpi.c b/xen/drivers/passthrough/amd/iommu_acpi.c > index 2e3b83014beb..ca70f4f3ae2c 100644 > --- a/xen/drivers/passthrough/amd/iommu_acpi.c > +++ b/xen/drivers/passthrough/amd/iommu_acpi.c > @@ -426,9 +426,14 @@ static int __init parse_ivmd_block(const struct acpi_ivrs_memory ivmd_block) > return -EIO; > } > > - / Types which won't be handed out are considered good enough. / > - if ( !(type & (RAM_TYPE_RESERVED | RAM_TYPE_ACPI | > - RAM_TYPE_UNUSABLE)) ) > + / > + * Types which aren't RAM are considered good enough. > + * Note that a page being partially RESERVED, ACPI or UNUSABLE will > + * force Xen into assuming the whole page as having that type in > + * practice. > + */ > + if ( type & (RAM_TYPE_RESERVED | RAM_TYPE_ACPI | > + RAM_TYPE_UNUSABLE) ) > continue; > > AMD_IOMMU_ERROR("IVMD: page at %lx can't be converted\n", addr); I tested the patch and it resolves the issue. It eliminates the boot IVMD error message. AMD-Vi is enabled and pci passthrough works. Tested-by: oxjo <oxjo@proton.me>
On 01.02.2024 18:01, Roger Pau Monne wrote: > The current code that parses the IVMD blocks is relaxed with regard to the > restriction that such unity regions should always fall into memory ranges > marked as reserved in the memory map. > > However the type checks for the IVMD addresses are inverted, and as a result > IVMD ranges falling into RAM areas are accepted. Note that having such ranges > in the first place is a firmware bug, as IVMD should always fall into reserved > ranges. > > Fixes: ed6c77ebf0c1 ('AMD/IOMMU: check / convert IVMD ranges for being / to be reserved') > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> Reviewed-by: Jan Beulich <jbeulich@suse.com>
On Thu, Feb 01, 2024 at 06:01:56PM +0100, Roger Pau Monne wrote: > The current code that parses the IVMD blocks is relaxed with regard to the > restriction that such unity regions should always fall into memory ranges > marked as reserved in the memory map. > > However the type checks for the IVMD addresses are inverted, and as a result > IVMD ranges falling into RAM areas are accepted. Note that having such ranges > in the first place is a firmware bug, as IVMD should always fall into reserved > ranges. > > Fixes: ed6c77ebf0c1 ('AMD/IOMMU: check / convert IVMD ranges for being / to be reserved') > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> FYI Xen 4.17.3 with this patch (but not others in the series) causes panic on boot on Framework 14 AMD laptop: (XEN) [0000000044570000, 000000005077efff] (usable) ... (XEN) AMD-Vi: Warning: IVMD: [4f83f000,4f880000) is not (entirely) in reserved memory (XEN) AMD-Vi: Error: IVMD: page at 4f83f000 can't be converted ... (XEN) Xen BUG at drivers/passthrough/amd/iommu_init.c:1386 Full boot log at https://github.com/QubesOS/qubes-issues/issues/9030 4.17.2 worked fine. I'll try the whole series (and the follow up one), but I don't expect any difference. This looks to be related to XHCI console, which does use the same API to allow device to DMA into relevant buffers even when the rest of XHCI is used by dom0 (or even other domain if 'share=yes' is used): https://gitlab.com/xen-project/xen/-/blob/staging/xen/drivers/char/xhci-dbc.c?ref_type=heads#L1421-1424
On 11.03.2024 03:18, Marek Marczykowski-Górecki wrote: > On Thu, Feb 01, 2024 at 06:01:56PM +0100, Roger Pau Monne wrote: >> The current code that parses the IVMD blocks is relaxed with regard to the >> restriction that such unity regions should always fall into memory ranges >> marked as reserved in the memory map. >> >> However the type checks for the IVMD addresses are inverted, and as a result >> IVMD ranges falling into RAM areas are accepted. Note that having such ranges >> in the first place is a firmware bug, as IVMD should always fall into reserved >> ranges. >> >> Fixes: ed6c77ebf0c1 ('AMD/IOMMU: check / convert IVMD ranges for being / to be reserved') >> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> > > FYI Xen 4.17.3 with this patch (but not others in the series) causes > panic on boot on Framework 14 AMD laptop: > > (XEN) [0000000044570000, 000000005077efff] (usable) > ... > (XEN) AMD-Vi: Warning: IVMD: [4f83f000,4f880000) is not (entirely) in reserved memory > (XEN) AMD-Vi: Error: IVMD: page at 4f83f000 can't be converted > ... > (XEN) Xen BUG at drivers/passthrough/amd/iommu_init.c:1386 > > Full boot log at https://github.com/QubesOS/qubes-issues/issues/9030 > 4.17.2 worked fine. > I'll try the whole series (and the follow up one), but I don't expect > any difference. > > This looks to be related to XHCI console, which does use the same API to > allow device to DMA into relevant buffers even when the rest of XHCI is > used by dom0 (or even other domain if 'share=yes' is used): > https://gitlab.com/xen-project/xen/-/blob/staging/xen/drivers/char/xhci-dbc.c?ref_type=heads#L1421-1424 Hmm, yes, this is what we get for allowing such (ab)use. I guess we need to have iommu_add_extra_reserved_device_memory() actually convert the region in question, to satisfy the later check. Care to make a patch? Jan
diff --git a/xen/drivers/passthrough/amd/iommu_acpi.c b/xen/drivers/passthrough/amd/iommu_acpi.c index 2e3b83014beb..ca70f4f3ae2c 100644 --- a/xen/drivers/passthrough/amd/iommu_acpi.c +++ b/xen/drivers/passthrough/amd/iommu_acpi.c @@ -426,9 +426,14 @@ static int __init parse_ivmd_block(const struct acpi_ivrs_memory *ivmd_block) return -EIO; } - /* Types which won't be handed out are considered good enough. */ - if ( !(type & (RAM_TYPE_RESERVED | RAM_TYPE_ACPI | - RAM_TYPE_UNUSABLE)) ) + /* + * Types which aren't RAM are considered good enough. + * Note that a page being partially RESERVED, ACPI or UNUSABLE will + * force Xen into assuming the whole page as having that type in + * practice. + */ + if ( type & (RAM_TYPE_RESERVED | RAM_TYPE_ACPI | + RAM_TYPE_UNUSABLE) ) continue; AMD_IOMMU_ERROR("IVMD: page at %lx can't be converted\n", addr);
The current code that parses the IVMD blocks is relaxed with regard to the restriction that such unity regions should always fall into memory ranges marked as reserved in the memory map. However the type checks for the IVMD addresses are inverted, and as a result IVMD ranges falling into RAM areas are accepted. Note that having such ranges in the first place is a firmware bug, as IVMD should always fall into reserved ranges. Fixes: ed6c77ebf0c1 ('AMD/IOMMU: check / convert IVMD ranges for being / to be reserved') Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> --- Cc: oxjo@proton.me --- xen/drivers/passthrough/amd/iommu_acpi.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-)