All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Roger Pau Monné" <roger.pau@citrix.com>
To: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>,
	Willi Junga <xenproject@ymy.be>,
	xen-devel@lists.xenproject.org
Subject: Re: [PATCH] iommu/amd-vi: do not error if device referenced in IVMD is not behind any IOMMU
Date: Wed, 9 Oct 2024 10:03:22 +0200	[thread overview]
Message-ID: <ZwY4ym2Gnlx4tytP@macbook.local> (raw)
In-Reply-To: <d6489e43-2cfe-4ad7-824e-a3212367dbca@suse.com>

On Tue, Oct 08, 2024 at 04:01:28PM +0200, Jan Beulich wrote:
> On 08.10.2024 12:47, Roger Pau Monne wrote:
> > IVMD table contains restrictions about memory which must be mandatory assigned
> > to devices (and which permissions it should use), or memory that should be
> > never accessible to devices.
> > 
> > Some hardware however contains ranges in IVMD that reference devices outside of
> > the IVHD tables (in other words, devices not behind any IOMMU).  Such mismatch
> > will cause Xen to fail in register_range_for_device(), ultimately leading to
> > the IOMMU being disabled, and Xen crashing as x2APIC support might be already
> > enabled and relying on the IOMMU functionality.
> 
> I find it hard to believe that on x86 systems with IOMMUs some devices would
> be left uncovered by any IOMMU. Is it possible that IVHD is flawed there? In
> which case we might rightfully refuse to boot? (Can you share e.g. that
> "iommu=debug" output that results from parsing the tables on that system?)

I'm afraid I don't have any of such systems to test myself, however I
have the contents of IVRS:

  ACPI Table Header
------------------------------------------------------------------
Signature          : IVRS
Length             : 0x000001F8
Revision           : 0x02
Checksum           : 0x06
OEM ID             : AMD  
OEM Table ID       : AmdTable
OEM Revision       : 0x00000001
Creator ID         : AMD 
Creator Revision   : 0x00000001
IVinfo             : 0x00203043
	  IVHD
	----------------------------------------------------------------
	Type                  : 0x10
	Flags                 : 0xB0
	Length                : 0x0044
	IOMMU Device ID       : 0x0002
	Capability Offset     : 0x0040
	IOMMU Base Address    : 0x00000000FD200000
	Segment Group         : 0x0000
	IOMMU Info            : 0x0000
	IOMMU Feature Info    : 0x80048F6E
		  Range
		--------------------------------------------------
		Type                  : 0x03
		Start of Range        : 0x0003
		End of Range          : 0xFFFE
		DTE Setting           : 0x00
		  Alias Range
		--------------------------------------------------
		Type                  : 0x43
		Start of Range        : 0xFF00
		End of Range          : 0xFFFF
		DTE Setting           : 0x00
		Source Device ID      : 0x00A5
		  Special Device
		--------------------------------------------------
		Type                  : 0x48
		Device ID             : 0x0000
		DTE Setting           : 0x00
		Source Device ID      : 0x00A0
		Handle                : 0x00
		Variety               : HPET
		  Special Device
		--------------------------------------------------
		Type                  : 0x48
		Device ID             : 0x0000
		DTE Setting           : 0xD7
		Source Device ID      : 0x00A0
		Handle                : 0x21
		Variety               : IOAPIC
		  Special Device
		--------------------------------------------------
		Type                  : 0x48
		Device ID             : 0x0000
		DTE Setting           : 0x00
		Source Device ID      : 0x0001
		Handle                : 0x22
		Variety               : IOAPIC
	  IVHD
	----------------------------------------------------------------
	Type                  : 0x11
	Flags                 : 0x30
	Length                : 0x0054
	IOMMU Device ID       : 0x0002
	Capability Offset     : 0x0040
	IOMMU Base Address    : 0x00000000FD200000
	Segment Group         : 0x0000
	IOMMU Info            : 0x0000
	IOMMU Feature Info    : 0x00048000
		  Range
		--------------------------------------------------
		Type                  : 0x03
		Start of Range        : 0x0003
		End of Range          : 0xFFFE
		DTE Setting           : 0x00
		  Alias Range
		--------------------------------------------------
		Type                  : 0x43
		Start of Range        : 0xFF00
		End of Range          : 0xFFFF
		DTE Setting           : 0x00
		Source Device ID      : 0x00A5
		  Special Device
		--------------------------------------------------
		Type                  : 0x48
		Device ID             : 0x0000
		DTE Setting           : 0x00
		Source Device ID      : 0x00A0
		Handle                : 0x00
		Variety               : HPET
		  Special Device
		--------------------------------------------------
		Type                  : 0x48
		Device ID             : 0x0000
		DTE Setting           : 0xD7
		Source Device ID      : 0x00A0
		Handle                : 0x21
		Variety               : IOAPIC
		  Special Device
		--------------------------------------------------
		Type                  : 0x48
		Device ID             : 0x0000
		DTE Setting           : 0x00
		Source Device ID      : 0x0001
		Handle                : 0x22
		Variety               : IOAPIC
	  IVMD
	----------------------------------------------------------------
	Type                                 : 0x22
	Flags                                : 0x08
	Length                               : 0x0020
	DeviceID                             : 0x0000
	AuxiliaryData                        : 0x0FFF
	Reserved                             : 0x0000000000000000
	IVMD Start Address                   : 0x0000000096191000
	IVMD Memory Block Length             : 0x0000000000000022
	  IVMD
	----------------------------------------------------------------
	Type                                 : 0x22
	Flags                                : 0x08
	Length                               : 0x0020
	DeviceID                             : 0x0000
	AuxiliaryData                        : 0x0FFF
	Reserved                             : 0x0000000000000000
	IVMD Start Address                   : 0x0000000097D9E000
	IVMD Memory Block Length             : 0x0000000000000022
	  IVMD
	----------------------------------------------------------------
	Type                                 : 0x22
	Flags                                : 0x08
	Length                               : 0x0020
	DeviceID                             : 0x0000
	AuxiliaryData                        : 0x0FFF
	Reserved                             : 0x0000000000000000
	IVMD Start Address                   : 0x0000000097D9D000
	IVMD Memory Block Length             : 0x0000000000000022
	  IVHD
	----------------------------------------------------------------
	Type                  : 0x40
	Flags                 : 0x30
	Length                : 0x00D0
	IOMMU Device ID       : 0x0002
	Capability Offset     : 0x0040
	IOMMU Base Address    : 0x00000000FD200000
	Segment Group         : 0x0000
	IOMMU Info            : 0x0000
	IOMMU Feature Info    : 0x00048000
		  Range
		--------------------------------------------------
		Type                  : 0x03
		Start of Range        : 0x0003
		End of Range          : 0xFFFE
		DTE Setting           : 0x00
		  Alias Range
		--------------------------------------------------
		Type                  : 0x43
		Start of Range        : 0xFF00
		End of Range          : 0xFFFF
		DTE Setting           : 0x00
		Source Device ID      : 0x00A5
		  Special Device
		--------------------------------------------------
		Type                  : 0x48
		Device ID             : 0x0000
		DTE Setting           : 0x00
		Source Device ID      : 0x00A0
		Handle                : 0x00
		Variety               : HPET
		  Special Device
		--------------------------------------------------
		Type                  : 0x48
		Device ID             : 0x0000
		DTE Setting           : 0xD7
		Source Device ID      : 0x00A0
		Handle                : 0x21
		Variety               : IOAPIC
		  Special Device
		--------------------------------------------------
		Type                  : 0x48
		Device ID             : 0x0000
		DTE Setting           : 0x00
		Source Device ID      : 0x0001
		Handle                : 0x22
		Variety               : IOAPIC
		  Variable Length ACPI HID Device
		--------------------------------------------------
		Type                  : 0xF0
		Device ID             : 0x00A5
		DTE Setting           : 0x40
		Hardware ID           : AMDI0020
		Extended DTE Setting  : 
		Unique ID Format      : 2
		Unique ID Length      : 9
		Unique ID             : \_SB.FUR0
		  Variable Length ACPI HID Device
		--------------------------------------------------
		Type                  : 0xF0
		Device ID             : 0x00A5
		DTE Setting           : 0x40
		Hardware ID           : AMDI0020
		Extended DTE Setting  : 
		Unique ID Format      : 2
		Unique ID Length      : 9
		Unique ID             : \_SB.FUR1
		  Variable Length ACPI HID Device
		--------------------------------------------------
		Type                  : 0xF0
		Device ID             : 0x00A5
		DTE Setting           : 0x40
		Hardware ID           : AMDI0020
		Extended DTE Setting  : 
		Unique ID Format      : 2
		Unique ID Length      : 9
		Unique ID             : \_SB.FUR2
		  Variable Length ACPI HID Device
		--------------------------------------------------
		Type                  : 0xF0
		Device ID             : 0x00A5
		DTE Setting           : 0x40
		Hardware ID           : AMDI0020
		Extended DTE Setting  : 
		Unique ID Format      : 2
		Unique ID Length      : 9
		Unique ID             : \_SB.FUR3

FWIW, I've checked on one of the AMD server systems we have on the
lab, and the IVHD entries are fairly similar to the ones here, as
neither the PCI Host Bridge, nor the IOMMU are covered by any IVHD
block.  That system however doesn't have any IVMD blocks.

> > Relax IVMD parsing: allow IVMD blocks to reference devices not assigned to any
> > IOMMU.  It's impossible for Xen to fulfill the requirement in the IVMD block if
> > the device is not behind any IOMMU, but it's no worse than booting without
> > IOMMU support, and thus not parsing ACPI IVRS in the first place.
> > 
> > Reported-by: Willi Junga <xenproject@ymy.be>
> > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> > ---
> >  xen/drivers/passthrough/amd/iommu_acpi.c | 5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> > 
> > diff --git a/xen/drivers/passthrough/amd/iommu_acpi.c b/xen/drivers/passthrough/amd/iommu_acpi.c
> > index 3f5508eba049..c416120326c9 100644
> > --- a/xen/drivers/passthrough/amd/iommu_acpi.c
> > +++ b/xen/drivers/passthrough/amd/iommu_acpi.c
> > @@ -248,8 +248,9 @@ static int __init register_range_for_device(
> >      iommu = find_iommu_for_device(seg, bdf);
> >      if ( !iommu )
> >      {
> > -        AMD_IOMMU_ERROR("IVMD: no IOMMU for Dev_Id %#x\n", bdf);
> > -        return -ENODEV;
> > +        AMD_IOMMU_WARN("IVMD: no IOMMU for device %pp - ignoring constrain\n",
> 
> I'm not a native speaker, but "constrain" to me can only be a verb (with
> "constraint" being the noun). IOW as worded I'm afraid I can't make sense
> of the message.

Indeed, sorry for the typo.

> > +                       &PCI_SBDF(seg, bdf));
> > +        return 0;
> >      }
> >      req = ivrs_mappings[bdf].dte_requestor_id;
> >  
> 
> Down from here in parse_ivmd_device_iommu() is somewhat similar code.
> Wouldn't that need adjusting similarly then? Or else shouldn't the
> adjustment above be accompanied by a comment clarifying that the
> behavior is just because of observations on certain hardware?

Hm, I think that one is bogus and should be removed, according to my
copy of the AMD-Vi spec (48882—Rev 3.08-PUB—Oct 2023), the IVMD type
can only be:

20h=all peripherals
21h=specified peripheral
22h=peripheral range

So type 23h (ACPI_IVRS_TYPE_MEMORY_IOMMU) is not a valid type for the
IVMD blocks.

Thanks, Roger.


  reply	other threads:[~2024-10-09  8:03 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-10-08 10:47 [PATCH] iommu/amd-vi: do not error if device referenced in IVMD is not behind any IOMMU Roger Pau Monne
2024-10-08 14:01 ` Jan Beulich
2024-10-09  8:03   ` Roger Pau Monné [this message]
2024-10-09 10:52     ` Jan Beulich
2024-10-09 11:13       ` Roger Pau Monné
2024-10-09 11:28         ` Jan Beulich
2024-10-09 11:47           ` Roger Pau Monné
2024-10-09 12:09             ` Jan Beulich
2024-10-09 12:28               ` Teddy Astie
2024-10-09 12:46                 ` Jan Beulich
2024-10-09 13:37               ` Roger Pau Monné
2024-10-10 12:45                 ` Marek Marczykowski-Górecki
2024-10-15  6:38                 ` Jan Beulich

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ZwY4ym2Gnlx4tytP@macbook.local \
    --to=roger.pau@citrix.com \
    --cc=andrew.cooper3@citrix.com \
    --cc=jbeulich@suse.com \
    --cc=xen-devel@lists.xenproject.org \
    --cc=xenproject@ymy.be \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.