[PATCH 1/1] PCI: vmd: Fix boot failure when trying to clean up domain before enumeration

[PATCH 1/1] PCI: vmd: Fix boot failure when trying to clean up domain before enumeration

[Adrian Huang]

From: Adrian Huang <ahuang12@lenovo.com>

Commit 6aab5622296b ("PCI: vmd: Clean up domain before enumeration")
clears PCI configuration space of VMD root ports. However, the host OS
cannot boot successfully with the following error message:

  vmd 0000:64:05.5: PCI host bridge to bus 10000:00
  ...
  vmd 0000:64:05.5: Bound to PCI domain 10000
  ...
  DMAR: VT-d detected Invalidation Queue Error: Reason f
  DMAR: VT-d detected Invalidation Time-out Error: SID ffff
  DMAR: VT-d detected Invalidation Completion Error: SID ffff
  DMAR: QI HEAD: UNKNOWN qw0 = 0x0, qw1 = 0x0
  DMAR: QI PRIOR: UNKNOWN qw0 = 0x0, qw1 = 0x0
  DMAR: Invalidation Time-out Error (ITE) cleared

The root cause is that memset_io() clears prefetchable memory base/limit
registers and prefetchable base/limit 32 bits registers sequentially. This
might enable prefetchable memory if the device disables prefetchable memory
originally. Here is an example (before memset_io()):

  PCI configuration space for 10000:00:00.0:
  86 80 30 20 06 00 10 00 04 00 04 06 00 00 01 00
  00 00 00 00 00 00 00 00 00 01 01 00 00 00 00 20
  00 00 00 00 01 00 01 00 ff ff ff ff 75 05 00 00
  00 00 00 00 40 00 00 00 00 00 00 00 00 01 02 00

So, prefetchable memory is ffffffff00000000-575000fffff, which is disabled.
Here is the quote from section 7.5.1.3.9 of PCI Express Base 6.0 spec:

  The Prefetchable Memory Limit register must be programmed to a smaller
  value than the Prefetchable Memory Base register if there is no
  prefetchable memory on the secondary side of the bridge.

When memset_io() clears prefetchable base 32 bits register, the
prefetchable memory becomes 0000000000000000-575000fffff, which is enabled.
This behavior causes that the content of PCI configuration space of VMD
root ports is 0xff after invoking memset_io() in vmd_domain_reset():

  10000:00:00.0 PCI bridge: Intel Corporation Sky Lake-E PCI Express Root Port A (rev ff) (prog-if ff)
          !!! Unknown header type 7f
  00: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
  ...
  f0: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff

  10000:00:01.0 PCI bridge: Intel Corporation Sky Lake-E PCI Express Root Port B (rev ff) (prog-if ff)
          !!! Unknown header type 7f
  00: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
  ...
  f0: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff

To fix the issue, prefetchable limit upper 32 bits register needs to be
cleared firstly. This also adheres to the implementation of
pci_setup_bridge_mmio_pref(). Please see the function for detail.

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=216644
Fixes: 6aab5622296b ("PCI: vmd: Clean up domain before enumeration")
Cc: Nirmal Patel <nirmal.patel@linux.intel.com>
Signed-off-by: Adrian Huang <ahuang12@lenovo.com>
---
 drivers/pci/controller/vmd.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/pci/controller/vmd.c b/drivers/pci/controller/vmd.c
index 769eedeb8802..e520aec55b68 100644
--- a/drivers/pci/controller/vmd.c
+++ b/drivers/pci/controller/vmd.c
@@ -526,6 +526,9 @@ static void vmd_domain_reset(struct vmd_dev *vmd)
 				     PCI_CLASS_BRIDGE_PCI))
 					continue;
 
+				/* Clear the upper 32 bits of PREF limit. */
+				memset_io(base + PCI_PREF_LIMIT_UPPER32, 0, 4);
+
 				memset_io(base + PCI_IO_BASE, 0,
 					  PCI_ROM_ADDRESS1 - PCI_IO_BASE);
 			}
-- 
2.31.1

Re: [PATCH 1/1] PCI: vmd: Fix boot failure when trying to clean up domain before enumeration

[Jonathan Derrick]

Reviewed-by: Jon Derrick <jonathan.derrick@linux.dev>

On 1/9/2023 5:51 AM, Adrian Huang wrote:
> From: Adrian Huang <ahuang12@lenovo.com>
> 
> Commit 6aab5622296b ("PCI: vmd: Clean up domain before enumeration")
> clears PCI configuration space of VMD root ports. However, the host OS
> cannot boot successfully with the following error message:
> 
>   vmd 0000:64:05.5: PCI host bridge to bus 10000:00
>   ...
>   vmd 0000:64:05.5: Bound to PCI domain 10000
>   ...
>   DMAR: VT-d detected Invalidation Queue Error: Reason f
>   DMAR: VT-d detected Invalidation Time-out Error: SID ffff
>   DMAR: VT-d detected Invalidation Completion Error: SID ffff
>   DMAR: QI HEAD: UNKNOWN qw0 = 0x0, qw1 = 0x0
>   DMAR: QI PRIOR: UNKNOWN qw0 = 0x0, qw1 = 0x0
>   DMAR: Invalidation Time-out Error (ITE) cleared
> 
> The root cause is that memset_io() clears prefetchable memory base/limit
> registers and prefetchable base/limit 32 bits registers sequentially. This
> might enable prefetchable memory if the device disables prefetchable memory
> originally. Here is an example (before memset_io()):
> 
>   PCI configuration space for 10000:00:00.0:
>   86 80 30 20 06 00 10 00 04 00 04 06 00 00 01 00
>   00 00 00 00 00 00 00 00 00 01 01 00 00 00 00 20
>   00 00 00 00 01 00 01 00 ff ff ff ff 75 05 00 00
>   00 00 00 00 40 00 00 00 00 00 00 00 00 01 02 00
> 
> So, prefetchable memory is ffffffff00000000-575000fffff, which is disabled.
> Here is the quote from section 7.5.1.3.9 of PCI Express Base 6.0 spec:
> 
>   The Prefetchable Memory Limit register must be programmed to a smaller
>   value than the Prefetchable Memory Base register if there is no
>   prefetchable memory on the secondary side of the bridge.
> 
> When memset_io() clears prefetchable base 32 bits register, the
> prefetchable memory becomes 0000000000000000-575000fffff, which is enabled.
> This behavior causes that the content of PCI configuration space of VMD
> root ports is 0xff after invoking memset_io() in vmd_domain_reset():
> 
>   10000:00:00.0 PCI bridge: Intel Corporation Sky Lake-E PCI Express Root Port A (rev ff) (prog-if ff)
>           !!! Unknown header type 7f
>   00: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
>   ...
>   f0: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
> 
>   10000:00:01.0 PCI bridge: Intel Corporation Sky Lake-E PCI Express Root Port B (rev ff) (prog-if ff)
>           !!! Unknown header type 7f
>   00: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
>   ...
>   f0: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
> 
> To fix the issue, prefetchable limit upper 32 bits register needs to be
> cleared firstly. This also adheres to the implementation of
> pci_setup_bridge_mmio_pref(). Please see the function for detail.
> 
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=216644
> Fixes: 6aab5622296b ("PCI: vmd: Clean up domain before enumeration")
> Cc: Nirmal Patel <nirmal.patel@linux.intel.com>
> Signed-off-by: Adrian Huang <ahuang12@lenovo.com>
> ---
>  drivers/pci/controller/vmd.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/pci/controller/vmd.c b/drivers/pci/controller/vmd.c
> index 769eedeb8802..e520aec55b68 100644
> --- a/drivers/pci/controller/vmd.c
> +++ b/drivers/pci/controller/vmd.c
> @@ -526,6 +526,9 @@ static void vmd_domain_reset(struct vmd_dev *vmd)
>  				     PCI_CLASS_BRIDGE_PCI))
>  					continue;
>  
> +				/* Clear the upper 32 bits of PREF limit. */
> +				memset_io(base + PCI_PREF_LIMIT_UPPER32, 0, 4);
> +
>  				memset_io(base + PCI_IO_BASE, 0,
>  					  PCI_ROM_ADDRESS1 - PCI_IO_BASE);
>  			}

Re: [PATCH 1/1] PCI: vmd: Fix boot failure when trying to clean up domain before enumeration

[Bjorn Helgaas]

Can we make the subject line any more specific about what this patch
does?  Apparently this is really about avoiding accidental enablement
of the window because the base & limit can't be updated atomically?

On Mon, Jan 09, 2023 at 08:51:48PM +0800, Adrian Huang wrote:
> From: Adrian Huang <ahuang12@lenovo.com>
> 
> Commit 6aab5622296b ("PCI: vmd: Clean up domain before enumeration")
> clears PCI configuration space of VMD root ports. However, the host OS
> cannot boot successfully with the following error message:
> 
>   vmd 0000:64:05.5: PCI host bridge to bus 10000:00
>   ...
>   vmd 0000:64:05.5: Bound to PCI domain 10000
>   ...
>   DMAR: VT-d detected Invalidation Queue Error: Reason f
>   DMAR: VT-d detected Invalidation Time-out Error: SID ffff
>   DMAR: VT-d detected Invalidation Completion Error: SID ffff
>   DMAR: QI HEAD: UNKNOWN qw0 = 0x0, qw1 = 0x0
>   DMAR: QI PRIOR: UNKNOWN qw0 = 0x0, qw1 = 0x0
>   DMAR: Invalidation Time-out Error (ITE) cleared
> 
> The root cause is that memset_io() clears prefetchable memory base/limit
> registers and prefetchable base/limit 32 bits registers sequentially. This
> might enable prefetchable memory if the device disables prefetchable memory
> originally. Here is an example (before memset_io()):
> 
>   PCI configuration space for 10000:00:00.0:
>   86 80 30 20 06 00 10 00 04 00 04 06 00 00 01 00
>   00 00 00 00 00 00 00 00 00 01 01 00 00 00 00 20
>   00 00 00 00 01 00 01 00 ff ff ff ff 75 05 00 00
>   00 00 00 00 40 00 00 00 00 00 00 00 00 01 02 00
> 
> So, prefetchable memory is ffffffff00000000-575000fffff, which is disabled.
> Here is the quote from section 7.5.1.3.9 of PCI Express Base 6.0 spec:
> 
>   The Prefetchable Memory Limit register must be programmed to a smaller
>   value than the Prefetchable Memory Base register if there is no
>   prefetchable memory on the secondary side of the bridge.
> 
> When memset_io() clears prefetchable base 32 bits register, the
> prefetchable memory becomes 0000000000000000-575000fffff, which is enabled.

  #define PCI_IO_BASE             0x1c
  #define PCI_PREF_LIMIT_UPPER32  0x2c
  #define PCI_ROM_ADDRESS1        0x38

  memset_io(base + PCI_IO_BASE, 0, PCI_ROM_ADDRESS1 - PCI_IO_BASE);

The memset does clear PCI_PREF_LIMIT_UPPER32 already, but I think
you're saying that PCI_PREF_MEMORY_BASE, PCI_PREF_MEMORY_LIMIT, and
PCI_PREF_BASE_UPPER32 are cleared first, so there is a time when the
prefetchable base is zero and the limit is non-zero, so the window is
enabled.

I would expect that to be a transient thing that you wouldn't be
likely to trip over, but you seem to see it consistently.

> This behavior causes that the content of PCI configuration space of VMD
> root ports is 0xff after invoking memset_io() in vmd_domain_reset():

Well, it doesn't actually change the content of config space, does it?
I assume these config accesses get routed the wrong place because the
window is enabled, and some PCI error like Unsupported Request is
getting turned into ~0?

I don't understand why this seems to be a stable state, though.

Seems sort of unexpected to me that config space would be in a
*prefetchable* window, but I guess that must be a VMD thing.

>   10000:00:00.0 PCI bridge: Intel Corporation Sky Lake-E PCI Express Root Port A (rev ff) (prog-if ff)
>           !!! Unknown header type 7f
>   00: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
>   ...
>   f0: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
> 
>   10000:00:01.0 PCI bridge: Intel Corporation Sky Lake-E PCI Express Root Port B (rev ff) (prog-if ff)
>           !!! Unknown header type 7f
>   00: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
>   ...
>   f0: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
> 
> To fix the issue, prefetchable limit upper 32 bits register needs to be
> cleared firstly. This also adheres to the implementation of
> pci_setup_bridge_mmio_pref(). Please see the function for detail.
> 
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=216644
> Fixes: 6aab5622296b ("PCI: vmd: Clean up domain before enumeration")
> Cc: Nirmal Patel <nirmal.patel@linux.intel.com>
> Signed-off-by: Adrian Huang <ahuang12@lenovo.com>
> ---
>  drivers/pci/controller/vmd.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/pci/controller/vmd.c b/drivers/pci/controller/vmd.c
> index 769eedeb8802..e520aec55b68 100644
> --- a/drivers/pci/controller/vmd.c
> +++ b/drivers/pci/controller/vmd.c
> @@ -526,6 +526,9 @@ static void vmd_domain_reset(struct vmd_dev *vmd)
>  				     PCI_CLASS_BRIDGE_PCI))
>  					continue;
>  
> +				/* Clear the upper 32 bits of PREF limit. */
> +				memset_io(base + PCI_PREF_LIMIT_UPPER32, 0, 4);
> +
>  				memset_io(base + PCI_IO_BASE, 0,
>  					  PCI_ROM_ADDRESS1 - PCI_IO_BASE);

Re: [PATCH 1/1] PCI: vmd: Fix boot failure when trying to clean up domain before enumeration

[Huang Adrian]

On Tue, Jan 10, 2023 at 5:46 AM Bjorn Helgaas <helgaas@kernel.org> wrote:
>
> Can we make the subject line any more specific about what this patch
> does?  Apparently this is really about avoiding accidental enablement
> of the window because the base & limit can't be updated atomically?

Sure, I will rephrase the subject.

>   #define PCI_IO_BASE             0x1c
>   #define PCI_PREF_LIMIT_UPPER32  0x2c
>   #define PCI_ROM_ADDRESS1        0x38
>
>   memset_io(base + PCI_IO_BASE, 0, PCI_ROM_ADDRESS1 - PCI_IO_BASE);
>
> The memset does clear PCI_PREF_LIMIT_UPPER32 already, but I think
> you're saying that PCI_PREF_MEMORY_BASE, PCI_PREF_MEMORY_LIMIT, and
> PCI_PREF_BASE_UPPER32 are cleared first, so there is a time when the
> prefetchable base is zero and the limit is non-zero, so the window is
> enabled.

Yes, your understanding is correct.

> I would expect that to be a transient thing that you wouldn't be
> likely to trip over, but you seem to see it consistently.
>
> > This behavior causes that the content of PCI configuration space of VMD
> > root ports is 0xff after invoking memset_io() in vmd_domain_reset():
>
> Well, it doesn't actually change the content of config space, does it?
> I assume these config accesses get routed the wrong place because the
> window is enabled, and some PCI error like Unsupported Request is
> getting turned into ~0?

Yes, I think so.
I'll rephrase the commit message accordingly.

-- Adrian