Re: [PATCH 2/2] powerpc/pseries: Check if EEH is enabled on DDW mechanism code
On 02/04/2016 03:30 AM, Gavin Shan wrote: On Wed, Feb 03, 2016 at 10:26:36AM -0200, Guilherme G. Piccoli wrote: On 02/02/2016 09:48 PM, Gavin Shan wrote: Gavin, thanks very much for the clarification. So, we can interchange edev->config_addr with pdn->pci_ext_config_space ? This would solve the issue with DDW being enabled when EEH is not. I hit the issue on PowerVM (PHyp). I wasn't able to perform hotplug in qemu that time I was testing this - I can re-test on qemu. Can we use pci_dn config address in qemu guest too? On PowerKVM, QEMU can't recognize (1). pHyp is expected to support both of them. Please have a try with (1) when eeh_enabled() returns false. The address (1) can be retrieved from pci_dn as below: (pdn->busno << 8) | (pdn->devfn) Thanks, Gavin Gavin, thanks very much for your advice. And sorry for my huge delay in replying this... I tested the pci_dn's busno/devfn solution in both PHyp and qemu and it worked fine - DDW was enabled with success. I just sent v2 patches to the list - your review is much appreciated. Thanks, Guilherme ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 2/2] powerpc/pseries: Check if EEH is enabled on DDW mechanism code
On 02/02/2016 09:48 PM, Gavin Shan wrote:
On Tue, Jan 19, 2016 at 06:18:20PM -0200, Guilherme G. Piccoli wrote:
- /* only attempt to use a new window if 64-bit DMA is requested */
- if (!disable_ddw && dma_mask == DMA_BIT_MASK(64)) {
+ /* We should check if EEH is enabled here, since DDW mechanism has
+* an intrinsic dependency of EEH config addr information. Also, we
+* only attempt to use a new window if 64-bit DMA is requested */
+ if (eeh_enabled() && !disable_ddw && dma_mask == DMA_BIT_MASK(64)) {
dn = pci_device_to_OF_node(pdev);
dev_dbg(dev, "node is %s\n", dn->full_name);
There are two types of addresses: (1) PCI config address (2) PE config address.
(1) is used to indentify one PCI device which is included in the PE. (2) is the
PCI config address of PE's primary bus in pHyp. Both of them can be used to
identify
the PE. It means the (1) PCI config address, which is retrieved from pci_dn,
can be
passed to hypervisor. Then we don't have to disable DDW when EEH is disabled.
Guilherme, did you hit the crash on pHyp or PowerKVM?
Gavin, thanks very much for the clarification. So, we can interchange
edev->config_addr with pdn->pci_ext_config_space ?
This would solve the issue with DDW being enabled when EEH is not.
I hit the issue on PowerVM (PHyp). I wasn't able to perform hotplug in
qemu that time I was testing this - I can re-test on qemu. Can we use
pci_dn config address in qemu guest too?
Cheers,
Guilherme
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 2/2] powerpc/pseries: Check if EEH is enabled on DDW mechanism code
On Wed, Feb 03, 2016 at 10:26:36AM -0200, Guilherme G. Piccoli wrote:
>On 02/02/2016 09:48 PM, Gavin Shan wrote:
>>On Tue, Jan 19, 2016 at 06:18:20PM -0200, Guilherme G. Piccoli wrote:
>>>-/* only attempt to use a new window if 64-bit DMA is requested */
>>>-if (!disable_ddw && dma_mask == DMA_BIT_MASK(64)) {
>>>+/* We should check if EEH is enabled here, since DDW mechanism has
>>>+ * an intrinsic dependency of EEH config addr information. Also, we
>>>+ * only attempt to use a new window if 64-bit DMA is requested */
>>>+if (eeh_enabled() && !disable_ddw && dma_mask == DMA_BIT_MASK(64)) {
>>> dn = pci_device_to_OF_node(pdev);
>>> dev_dbg(dev, "node is %s\n", dn->full_name);
>>>
>>
>>There are two types of addresses: (1) PCI config address (2) PE config
>>address.
>>(1) is used to indentify one PCI device which is included in the PE. (2) is
>>the
>>PCI config address of PE's primary bus in pHyp. Both of them can be used to
>>identify
>>the PE. It means the (1) PCI config address, which is retrieved from pci_dn,
>>can be
>>passed to hypervisor. Then we don't have to disable DDW when EEH is disabled.
>>
>>Guilherme, did you hit the crash on pHyp or PowerKVM?
>
>Gavin, thanks very much for the clarification. So, we can interchange
>edev->config_addr with pdn->pci_ext_config_space ?
>This would solve the issue with DDW being enabled when EEH is not.
>
>I hit the issue on PowerVM (PHyp). I wasn't able to perform hotplug in qemu
>that time I was testing this - I can re-test on qemu. Can we use pci_dn
>config address in qemu guest too?
>
On PowerKVM, QEMU can't recognize (1). pHyp is expected to support both
of them. Please have a try with (1) when eeh_enabled() returns false.
The address (1) can be retrieved from pci_dn as below:
(pdn->busno << 8) | (pdn->devfn)
Thanks,
Gavin
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 2/2] powerpc/pseries: Check if EEH is enabled on DDW mechanism code
On Tue, Jan 19, 2016 at 06:18:20PM -0200, Guilherme G. Piccoli wrote:
>The Dynamic DMA Window (DDW) mechanism relies on EEH to obtain the
>configuration address of devices. For example, the functions query_ddw()
>and create_ddw() make use of eeh_dev struct. So, the dependency is
>intrinsic - DDW mechanism will fail if EEH is not enabled.
>
>Despite this dependency, no check for EEH availability is performed in DDW
>code. This patch adds a check based on eeh_enabled() function, so if EEH is
>not enabled before eeh_dev struct use, DDW will fallback to default iommu
>mechanism and won't fail.
>
>One use case for this patch is when we disable EEH globally via kernel
>command-line ("eeh=off") - without the patch, a device probe can hit a kernel
>oops because EEH is disabled but DDW will try to use it.
>
>Signed-off-by: Guilherme G. Piccoli
>---
> arch/powerpc/platforms/pseries/iommu.c | 6 --
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
>diff --git a/arch/powerpc/platforms/pseries/iommu.c
>b/arch/powerpc/platforms/pseries/iommu.c
>index bd98ce2..1ff55cc 100644
>--- a/arch/powerpc/platforms/pseries/iommu.c
>+++ b/arch/powerpc/platforms/pseries/iommu.c
>@@ -1224,8 +1224,10 @@ static int dma_set_mask_pSeriesLP(struct device *dev,
>u64 dma_mask)
>
> pdev = to_pci_dev(dev);
>
>- /* only attempt to use a new window if 64-bit DMA is requested */
>- if (!disable_ddw && dma_mask == DMA_BIT_MASK(64)) {
>+ /* We should check if EEH is enabled here, since DDW mechanism has
>+ * an intrinsic dependency of EEH config addr information. Also, we
>+ * only attempt to use a new window if 64-bit DMA is requested */
>+ if (eeh_enabled() && !disable_ddw && dma_mask == DMA_BIT_MASK(64)) {
> dn = pci_device_to_OF_node(pdev);
> dev_dbg(dev, "node is %s\n", dn->full_name);
>
There are two types of addresses: (1) PCI config address (2) PE config address.
(1) is used to indentify one PCI device which is included in the PE. (2) is the
PCI config address of PE's primary bus in pHyp. Both of them can be used to
identify
the PE. It means the (1) PCI config address, which is retrieved from pci_dn,
can be
passed to hypervisor. Then we don't have to disable DDW when EEH is disabled.
Guilherme, did you hit the crash on pHyp or PowerKVM?
Thanks,
Gavin
>--
>2.1.0
>
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 2/2] powerpc/pseries: Check if EEH is enabled on DDW mechanism code
The Dynamic DMA Window (DDW) mechanism relies on EEH to obtain the
configuration address of devices. For example, the functions query_ddw()
and create_ddw() make use of eeh_dev struct. So, the dependency is
intrinsic - DDW mechanism will fail if EEH is not enabled.
Despite this dependency, no check for EEH availability is performed in DDW
code. This patch adds a check based on eeh_enabled() function, so if EEH is
not enabled before eeh_dev struct use, DDW will fallback to default iommu
mechanism and won't fail.
One use case for this patch is when we disable EEH globally via kernel
command-line ("eeh=off") - without the patch, a device probe can hit a kernel
oops because EEH is disabled but DDW will try to use it.
Signed-off-by: Guilherme G. Piccoli
---
arch/powerpc/platforms/pseries/iommu.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/platforms/pseries/iommu.c
b/arch/powerpc/platforms/pseries/iommu.c
index bd98ce2..1ff55cc 100644
--- a/arch/powerpc/platforms/pseries/iommu.c
+++ b/arch/powerpc/platforms/pseries/iommu.c
@@ -1224,8 +1224,10 @@ static int dma_set_mask_pSeriesLP(struct device *dev,
u64 dma_mask)
pdev = to_pci_dev(dev);
- /* only attempt to use a new window if 64-bit DMA is requested */
- if (!disable_ddw && dma_mask == DMA_BIT_MASK(64)) {
+ /* We should check if EEH is enabled here, since DDW mechanism has
+* an intrinsic dependency of EEH config addr information. Also, we
+* only attempt to use a new window if 64-bit DMA is requested */
+ if (eeh_enabled() && !disable_ddw && dma_mask == DMA_BIT_MASK(64)) {
dn = pci_device_to_OF_node(pdev);
dev_dbg(dev, "node is %s\n", dn->full_name);
--
2.1.0
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
