* [PATCH] PCI: vmd: Fix domain reset operation
@ 2023-05-30 21:47 Nirmal Patel
2023-06-20 17:14 ` Patel, Nirmal
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Nirmal Patel @ 2023-05-30 21:47 UTC (permalink / raw)
To: linux-pci; +Cc: Nirmal Patel
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.
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
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] PCI: vmd: Fix domain reset operation
2023-05-30 21:47 [PATCH] PCI: vmd: Fix domain reset operation Nirmal Patel
@ 2023-06-20 17:14 ` Patel, Nirmal
2023-06-20 17:36 ` Lorenzo Pieralisi
2023-06-20 17:58 ` Bjorn Helgaas
2023-06-20 18:07 ` Bjorn Helgaas
2 siblings, 1 reply; 7+ messages in thread
From: Patel, Nirmal @ 2023-06-20 17:14 UTC (permalink / raw)
To: Lorenzo Pieralisi; +Cc: linux-pci@vger.kernel.org
On 5/30/2023 2:47 PM, 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.
>
> 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);
> }
> }
> }
Gentle reminder!
Thanks.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] PCI: vmd: Fix domain reset operation
2023-06-20 17:14 ` Patel, Nirmal
@ 2023-06-20 17:36 ` Lorenzo Pieralisi
0 siblings, 0 replies; 7+ messages in thread
From: Lorenzo Pieralisi @ 2023-06-20 17:36 UTC (permalink / raw)
To: Patel, Nirmal; +Cc: Lorenzo Pieralisi, linux-pci@vger.kernel.org
On Tue, Jun 20, 2023 at 10:14:10AM -0700, Patel, Nirmal wrote:
> On 5/30/2023 2:47 PM, 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
I don't get why the top 32 bits aren't cleared. The patch is
fine to me.
Lorenzo
> > prefetchable memory is enabled accidentally.
> >
> > Implementing correct operation by writing a value to Prefetchable
> > Base Memory larger than the value of Prefetchable Memory Limit.
> >
> > 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);
> > }
> > }
> > }
>
> Gentle reminder!
> Thanks.
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] PCI: vmd: Fix domain reset operation
2023-05-30 21:47 [PATCH] PCI: vmd: Fix domain reset operation Nirmal Patel
2023-06-20 17:14 ` Patel, Nirmal
@ 2023-06-20 17:58 ` Bjorn Helgaas
2023-06-20 18:07 ` Bjorn Helgaas
2 siblings, 0 replies; 7+ messages in thread
From: Bjorn Helgaas @ 2023-06-20 17:58 UTC (permalink / raw)
To: Nirmal Patel; +Cc: linux-pci
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
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] PCI: vmd: Fix domain reset operation
2023-05-30 21:47 [PATCH] PCI: vmd: Fix domain reset operation Nirmal Patel
2023-06-20 17:14 ` Patel, Nirmal
2023-06-20 17:58 ` Bjorn Helgaas
@ 2023-06-20 18:07 ` Bjorn Helgaas
2023-06-21 23:02 ` Patel, Nirmal
2023-07-05 17:39 ` Patel, Nirmal
2 siblings, 2 replies; 7+ messages in thread
From: Bjorn Helgaas @ 2023-06-20 18:07 UTC (permalink / raw)
To: Nirmal Patel; +Cc: linux-pci
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.
Oh, I forgot: better to use imperative mood here instead of present
participle ("-ing" form), e.g., "Write ... so that ..."
This should probably use the same form as pci_disable_bridge_window(),
since I think it's doing the same thing.
> 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
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] PCI: vmd: Fix domain reset operation
2023-06-20 18:07 ` Bjorn Helgaas
@ 2023-06-21 23:02 ` Patel, Nirmal
2023-07-05 17:39 ` Patel, Nirmal
1 sibling, 0 replies; 7+ messages in thread
From: Patel, Nirmal @ 2023-06-21 23:02 UTC (permalink / raw)
To: Bjorn Helgaas; +Cc: linux-pci
On 6/20/2023 11:07 AM, Bjorn Helgaas wrote:
> 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.
> Oh, I forgot: better to use imperative mood here instead of present
> participle ("-ing" form), e.g., "Write ... so that ..."
>
> This should probably use the same form as pci_disable_bridge_window(),
> since I think it's doing the same thing.
I will make the adjustment.
>
>> 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
>>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] PCI: vmd: Fix domain reset operation
2023-06-20 18:07 ` Bjorn Helgaas
2023-06-21 23:02 ` Patel, Nirmal
@ 2023-07-05 17:39 ` Patel, Nirmal
1 sibling, 0 replies; 7+ messages in thread
From: Patel, Nirmal @ 2023-07-05 17:39 UTC (permalink / raw)
To: Bjorn Helgaas; +Cc: linux-pci
On 6/20/2023 11:07 AM, Bjorn Helgaas wrote:
> 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.
> Oh, I forgot: better to use imperative mood here instead of present
> participle ("-ing" form), e.g., "Write ... so that ..."
>
> This should probably use the same form as pci_disable_bridge_window(),
> since I think it's doing the same thing.
will do that. Thanks.
>
>> 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
>>
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2023-07-05 17:39 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-05-30 21:47 [PATCH] PCI: vmd: Fix domain reset operation Nirmal Patel
2023-06-20 17:14 ` Patel, Nirmal
2023-06-20 17:36 ` Lorenzo Pieralisi
2023-06-20 17:58 ` Bjorn Helgaas
2023-06-20 18:07 ` Bjorn Helgaas
2023-06-21 23:02 ` Patel, Nirmal
2023-07-05 17:39 ` Patel, Nirmal
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox