Re: [PATCH] PCI: vmd: Fix domain reset operation

[Bjorn Helgaas <helgaas@kernel.org>]

On Tue, May 30, 2023 at 02:47:06PM -0700, Nirmal Patel wrote:
> During domain reset process we are accidentally enabling
> the prefetchable memory by writing 0x0 to Prefetchable Memory
> Base and Prefetchable Memory Limit registers. As a result certain
> platforms failed to boot up.
> 
> 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 clearing Prefetchable Memory Base, Prefetchable Memory
> Limit and Prefetchable Base Upper 32 bits, the prefetchable
> memory range becomes 0x0-0x575000fffff. As a result the
> prefetchable memory is enabled accidentally.
> 
> Implementing correct operation by writing a value to Prefetchable
> Base Memory larger than the value of Prefetchable Memory Limit.

Can you make the subject line more descriptive than "Fix X"?

There are a zillion ways X might be broken, so it's useful to have a
hint about which one it is.

Actually, vmd_domain_reset() is sort of weirdly named, because I don't
see any *reset* there.  It looks like it's actually disabling all
bridge windows (not just the prefetchable one, as the log suggests).

The fact that this makes a difference suggests that the bridges had
PCI_COMMAND_MEMORY set, which sounds like something you may not have
wanted.

> Signed-off-by: Nirmal Patel <nirmal.patel@linux.intel.com>
> ---
>  drivers/pci/controller/vmd.c | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pci/controller/vmd.c b/drivers/pci/controller/vmd.c
> index 769eedeb8802..f3eb740e3028 100644
> --- a/drivers/pci/controller/vmd.c
> +++ b/drivers/pci/controller/vmd.c
> @@ -526,8 +526,18 @@ static void vmd_domain_reset(struct vmd_dev *vmd)
>  				     PCI_CLASS_BRIDGE_PCI))
>  					continue;
>  
> -				memset_io(base + PCI_IO_BASE, 0,
> -					  PCI_ROM_ADDRESS1 - PCI_IO_BASE);
> +				writel(0, base + PCI_IO_BASE);
> +				writew(0xFFF0, base + PCI_MEMORY_BASE);
> +				writew(0, base + PCI_MEMORY_LIMIT);
> +
> +				writew(0xFFF1, base + PCI_PREF_MEMORY_BASE);
> +				writew(0, base + PCI_PREF_MEMORY_LIMIT);
> +
> +				writel(0xFFFFFFFF, base + PCI_PREF_BASE_UPPER32);
> +				writel(0, base + PCI_PREF_LIMIT_UPPER32);
> +
> +				writel(0, base + PCI_IO_BASE_UPPER16);
> +				writeb(0, base + PCI_CAPABILITY_LIST);
>  			}
>  		}
>  	}
> -- 
> 2.27.0
>