Re: [PATCH v3] PCI/ERR: Use pcie_aer_is_native() to judge whether OS owns AER
On 7/26/22 9:02 PM, Sathyanarayanan Kuppuswamy wrote:
On 7/26/22 8:53 PM, Zhuo Chen wrote:
Use pcie_aer_is_native() in place of "host->native_aer ||
pcie_ports_native" to judge whether OS has native control of AER
in pcie_do_recovery().
Replace "dev->aer_cap && (pcie_ports_native || host->native_aer)" in
get_port_device_capability() with pcie_aer_is_native(), which has no
functional changes.
Signed-off-by: Zhuo Chen
---
Patch looks better now. It looks like following two changes
can also be replaced with pcie_aer_is_native() check.
drivers/pci/pcie/aer.c:1407:if ((host->native_aer || pcie_ports_native) &&
aer) {
drivers/pci/pcie/aer.c:1426:if ((host->native_aer || pcie_ports_native) &&
aer) {
Good advice. But I wonder is there a scenario that dev->rcec ("root") is
NULL meanwhile dev->aer_cap is not NULL? If so, replace
"(host->native_aer || pcie_ports_native) && aer" with
pcie_aer_is_native() will change original function.
Changelog:
v3:
- Simplify why we use pcie_aer_is_native().
- Revert modification of pci_aer_clear_nonfatal_status() and comments.
v2:
- Add details and note in commit log.
---
drivers/pci/pcie/err.c | 3 +--
drivers/pci/pcie/portdrv_core.c | 3 +--
2 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/pci/pcie/err.c b/drivers/pci/pcie/err.c
index 0c5a143025af..121a53338e44 100644
--- a/drivers/pci/pcie/err.c
+++ b/drivers/pci/pcie/err.c
@@ -184,7 +184,6 @@ pci_ers_result_t pcie_do_recovery(struct pci_dev *dev,
int type = pci_pcie_type(dev);
struct pci_dev *bridge;
pci_ers_result_t status = PCI_ERS_RESULT_CAN_RECOVER;
- struct pci_host_bridge *host = pci_find_host_bridge(dev->bus);
/*
* If the error was detected by a Root Port, Downstream Port, RCEC,
@@ -243,7 +242,7 @@ pci_ers_result_t pcie_do_recovery(struct pci_dev *dev,
* it is responsible for clearing this status. In that case, the
* signaling device may not even be visible to the OS.
*/
- if (host->native_aer || pcie_ports_native) {
+ if (pcie_aer_is_native(dev)) {
pcie_clear_device_status(dev);
pci_aer_clear_nonfatal_status(dev);
}
diff --git a/drivers/pci/pcie/portdrv_core.c b/drivers/pci/pcie/portdrv_core.c
index 604feeb84ee4..98c18f4a01b2 100644
--- a/drivers/pci/pcie/portdrv_core.c
+++ b/drivers/pci/pcie/portdrv_core.c
@@ -221,8 +221,7 @@ static int get_port_device_capability(struct pci_dev *dev)
}
#ifdef CONFIG_PCIEAER
- if (dev->aer_cap && pci_aer_available() &&
- (pcie_ports_native || host->native_aer)) {
+ if (pcie_aer_is_native(dev) && pci_aer_available()) {
services |= PCIE_PORT_SERVICE_AER;
/*
Thanks,
Zhuo Chen
[powerpc:next] BUILD SUCCESS da4ef6d652fcefc0617ecd32f23154a28eef5e70
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next branch HEAD: da4ef6d652fcefc0617ecd32f23154a28eef5e70 powerpc/64e: Fix build failure with GCC 12 (unrecognized opcode: `wrteei') elapsed time: 726m configs tested: 107 configs skipped: 2 The following configs have been built successfully. More configs may be tested in the coming days. gcc tested configs: arm64allyesconfig arm defconfig arm allyesconfig powerpc randconfig-c003-20220724 i386 randconfig-c001 arcnsimosci_defconfig s390 allmodconfig sh rsk7264_defconfig m68km5272c3_defconfig ia64 gensparse_defconfig alphaalldefconfig arc nsimosci_hs_defconfig sh j2_defconfig powerpc iss476-smp_defconfig powerpc ps3_defconfig armkeystone_defconfig arm eseries_pxa_defconfig powerpc storcenter_defconfig sh se7705_defconfig m68k m5275evb_defconfig armshmobile_defconfig powerpc mpc837x_rdb_defconfig mips mpc30x_defconfig m68k apollo_defconfig nios2 10m50_defconfig powerpc eiger_defconfig sparc64 alldefconfig arm sama5_defconfig riscvnommu_virt_defconfig riscv rv32_defconfig riscvnommu_k210_defconfig riscv allnoconfig i386 debian-10.3-kselftests i386 debian-10.3 loongarch defconfig loongarch allnoconfig i386 debian-10.3-kvm i386debian-10.3-kunit i386 debian-10.3-func x86_64randconfig-c001 arm randconfig-c002-20220724 ia64 allmodconfig csky allnoconfig alpha allnoconfig arc allnoconfig m68k allyesconfig m68k allmodconfig arc allyesconfig alphaallyesconfig powerpc allnoconfig mips allyesconfig powerpc allmodconfig sh allmodconfig i386 allyesconfig i386defconfig x86_64randconfig-a002 x86_64randconfig-a004 x86_64randconfig-a006 i386 randconfig-a001 i386 randconfig-a005 i386 randconfig-a003 x86_64randconfig-a011 x86_64randconfig-a013 x86_64randconfig-a015 i386 randconfig-a012 i386 randconfig-a014 i386 randconfig-a016 s390 randconfig-r044-20220724 arc randconfig-r043-20220724 riscvrandconfig-r042-20220724 s390 randconfig-r044-20220726 riscvrandconfig-r042-20220726 arc randconfig-r043-20220726 x86_64rhel-8.3-kselftests um x86_64_defconfig um i386_defconfig x86_64 defconfig x86_64 allyesconfig x86_64 rhel-8.3 x86_64 rhel-8.3-kvm x86_64 rhel-8.3-func x86_64 rhel-8.3-syz x86_64 rhel-8.3-kunit clang tested configs: mips randconfig-c004-20220726 x86_64randconfig-c007 s390 randconfig-c005-20220726 powerpc randconfig-c003-20220726 i386 randconfig-c001 riscvrandconfig-c006-20220726 arm randconfig-c002-20220726 mips rs90_defconfig x86_64randconfig-k001 x86_64randconfig-a001 x86_64randconfig-a003 x86_64randconfig-a005 i386 randconfig-a002 i386 randconfig-a006 i386 randconfig-a004 x86_64randconfig-a012 x86_64
Re: [PATCH v3] PCI/ERR: Use pcie_aer_is_native() to judge whether OS owns AER
On 7/26/22 8:53 PM, Zhuo Chen wrote:
> Use pcie_aer_is_native() in place of "host->native_aer ||
> pcie_ports_native" to judge whether OS has native control of AER
> in pcie_do_recovery().
>
> Replace "dev->aer_cap && (pcie_ports_native || host->native_aer)" in
> get_port_device_capability() with pcie_aer_is_native(), which has no
> functional changes.
>
> Signed-off-by: Zhuo Chen
> ---
Patch looks better now. It looks like following two changes
can also be replaced with pcie_aer_is_native() check.
drivers/pci/pcie/aer.c:1407:if ((host->native_aer || pcie_ports_native) &&
aer) {
drivers/pci/pcie/aer.c:1426:if ((host->native_aer || pcie_ports_native) &&
aer) {
> Changelog:
> v3:
> - Simplify why we use pcie_aer_is_native().
> - Revert modification of pci_aer_clear_nonfatal_status() and comments.
> v2:
> - Add details and note in commit log.
> ---
> drivers/pci/pcie/err.c | 3 +--
> drivers/pci/pcie/portdrv_core.c | 3 +--
> 2 files changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/pci/pcie/err.c b/drivers/pci/pcie/err.c
> index 0c5a143025af..121a53338e44 100644
> --- a/drivers/pci/pcie/err.c
> +++ b/drivers/pci/pcie/err.c
> @@ -184,7 +184,6 @@ pci_ers_result_t pcie_do_recovery(struct pci_dev *dev,
> int type = pci_pcie_type(dev);
> struct pci_dev *bridge;
> pci_ers_result_t status = PCI_ERS_RESULT_CAN_RECOVER;
> - struct pci_host_bridge *host = pci_find_host_bridge(dev->bus);
>
> /*
>* If the error was detected by a Root Port, Downstream Port, RCEC,
> @@ -243,7 +242,7 @@ pci_ers_result_t pcie_do_recovery(struct pci_dev *dev,
>* it is responsible for clearing this status. In that case, the
>* signaling device may not even be visible to the OS.
>*/
> - if (host->native_aer || pcie_ports_native) {
> + if (pcie_aer_is_native(dev)) {
> pcie_clear_device_status(dev);
> pci_aer_clear_nonfatal_status(dev);
> }
> diff --git a/drivers/pci/pcie/portdrv_core.c b/drivers/pci/pcie/portdrv_core.c
> index 604feeb84ee4..98c18f4a01b2 100644
> --- a/drivers/pci/pcie/portdrv_core.c
> +++ b/drivers/pci/pcie/portdrv_core.c
> @@ -221,8 +221,7 @@ static int get_port_device_capability(struct pci_dev *dev)
> }
>
> #ifdef CONFIG_PCIEAER
> - if (dev->aer_cap && pci_aer_available() &&
> - (pcie_ports_native || host->native_aer)) {
> + if (pcie_aer_is_native(dev) && pci_aer_available()) {
> services |= PCIE_PORT_SERVICE_AER;
>
> /*
--
Sathyanarayanan Kuppuswamy
Linux Kernel Developer
Re: [PATCH v5 0/2] powerpc rng cleanups
"Jason A. Donenfeld" writes: > On Mon, Jul 18, 2022 at 3:09 PM Jason A. Donenfeld wrote: >> On Tue, Jul 12, 2022 at 01:24:46AM +0200, Jason A. Donenfeld wrote: >> > These are two small cleanups for -next. This v5 rebases on the latest >> > git master, as some whitespace was added that made v4 no longer apply. >> > >> > Jason A. Donenfeld (2): >> > powerpc/powernv: rename remaining rng powernv_ functions to pnv_ >> > powerpc/kvm: don't crash on missing rng, and use darn >> > >> > arch/powerpc/include/asm/archrandom.h | 7 +-- >> > arch/powerpc/kvm/book3s_hv_builtin.c | 7 +-- >> > arch/powerpc/platforms/powernv/rng.c | 66 ++- >> > drivers/char/hw_random/powernv-rng.c | 2 +- >> > 4 files changed, 30 insertions(+), 52 deletions(-) >> >> I think v5 has reached a completion point. Could you queue these up in >> some PPC tree for 5.20? > > Just paging again. Do you think you could queue these up for 5.20? > This trivial series is over a month old now. It can't claim to fix a guest-triggerable hypervisor crash and also be "trivial" :) But yes I plan to queue it for v5.20. cheers
[PATCH v3] PCI/ERR: Use pcie_aer_is_native() to judge whether OS owns AER
Use pcie_aer_is_native() in place of "host->native_aer ||
pcie_ports_native" to judge whether OS has native control of AER
in pcie_do_recovery().
Replace "dev->aer_cap && (pcie_ports_native || host->native_aer)" in
get_port_device_capability() with pcie_aer_is_native(), which has no
functional changes.
Signed-off-by: Zhuo Chen
---
Changelog:
v3:
- Simplify why we use pcie_aer_is_native().
- Revert modification of pci_aer_clear_nonfatal_status() and comments.
v2:
- Add details and note in commit log.
---
drivers/pci/pcie/err.c | 3 +--
drivers/pci/pcie/portdrv_core.c | 3 +--
2 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/pci/pcie/err.c b/drivers/pci/pcie/err.c
index 0c5a143025af..121a53338e44 100644
--- a/drivers/pci/pcie/err.c
+++ b/drivers/pci/pcie/err.c
@@ -184,7 +184,6 @@ pci_ers_result_t pcie_do_recovery(struct pci_dev *dev,
int type = pci_pcie_type(dev);
struct pci_dev *bridge;
pci_ers_result_t status = PCI_ERS_RESULT_CAN_RECOVER;
- struct pci_host_bridge *host = pci_find_host_bridge(dev->bus);
/*
* If the error was detected by a Root Port, Downstream Port, RCEC,
@@ -243,7 +242,7 @@ pci_ers_result_t pcie_do_recovery(struct pci_dev *dev,
* it is responsible for clearing this status. In that case, the
* signaling device may not even be visible to the OS.
*/
- if (host->native_aer || pcie_ports_native) {
+ if (pcie_aer_is_native(dev)) {
pcie_clear_device_status(dev);
pci_aer_clear_nonfatal_status(dev);
}
diff --git a/drivers/pci/pcie/portdrv_core.c b/drivers/pci/pcie/portdrv_core.c
index 604feeb84ee4..98c18f4a01b2 100644
--- a/drivers/pci/pcie/portdrv_core.c
+++ b/drivers/pci/pcie/portdrv_core.c
@@ -221,8 +221,7 @@ static int get_port_device_capability(struct pci_dev *dev)
}
#ifdef CONFIG_PCIEAER
- if (dev->aer_cap && pci_aer_available() &&
- (pcie_ports_native || host->native_aer)) {
+ if (pcie_aer_is_native(dev) && pci_aer_available()) {
services |= PCIE_PORT_SERVICE_AER;
/*
--
2.30.1 (Apple Git-130)
Re: [PATCH v5 0/2] powerpc rng cleanups
Hi Michael, On Mon, Jul 18, 2022 at 3:09 PM Jason A. Donenfeld wrote: > > Hey again, > > On Tue, Jul 12, 2022 at 01:24:46AM +0200, Jason A. Donenfeld wrote: > > These are two small cleanups for -next. This v5 rebases on the latest > > git master, as some whitespace was added that made v4 no longer apply. > > > > Jason A. Donenfeld (2): > > powerpc/powernv: rename remaining rng powernv_ functions to pnv_ > > powerpc/kvm: don't crash on missing rng, and use darn > > > > arch/powerpc/include/asm/archrandom.h | 7 +-- > > arch/powerpc/kvm/book3s_hv_builtin.c | 7 +-- > > arch/powerpc/platforms/powernv/rng.c | 66 ++- > > drivers/char/hw_random/powernv-rng.c | 2 +- > > 4 files changed, 30 insertions(+), 52 deletions(-) > > I think v5 has reached a completion point. Could you queue these up in > some PPC tree for 5.20? Just paging again. Do you think you could queue these up for 5.20? This trivial series is over a month old now. Thanks, Jason
[PATCH 3/3] PCI/DPC: Disable DPC service on suspend when IRQ is shared with PME
PCIe service that shares IRQ with PME may cause spurious wakeup on
system suspend.
Since AER is conditionally disabled in previous patch, also apply the
same condition to disable DPC which depends on AER to work.
PCIe Base Spec 5.0, section 5.2 "Link State Power Management" states
that TLP and DLLP transmission is disabled for a Link in L2/L3 Ready
(D3hot), L2 (D3cold with aux power) and L3 (D3cold), so we don't lose
much here to disable DPC during system suspend.
This is very similar to previous attempts to suspend AER and DPC [1],
but with a different reason.
[1]
https://lore.kernel.org/linux-pci/20220408153159.106741-1-kai.heng.f...@canonical.com/
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=216295
Signed-off-by: Kai-Heng Feng
---
drivers/pci/pcie/dpc.c | 52 +-
1 file changed, 41 insertions(+), 11 deletions(-)
diff --git a/drivers/pci/pcie/dpc.c b/drivers/pci/pcie/dpc.c
index 3e9afee02e8d1..542f282c43f75 100644
--- a/drivers/pci/pcie/dpc.c
+++ b/drivers/pci/pcie/dpc.c
@@ -343,13 +343,33 @@ void pci_dpc_init(struct pci_dev *pdev)
}
}
+static void dpc_enable(struct pcie_device *dev)
+{
+ struct pci_dev *pdev = dev->port;
+ u16 ctl;
+
+ pci_read_config_word(pdev, pdev->dpc_cap + PCI_EXP_DPC_CTL, );
+ ctl = (ctl & 0xfff4) | PCI_EXP_DPC_CTL_EN_FATAL |
PCI_EXP_DPC_CTL_INT_EN;
+ pci_write_config_word(pdev, pdev->dpc_cap + PCI_EXP_DPC_CTL, ctl);
+}
+
+static void dpc_disable(struct pcie_device *dev)
+{
+ struct pci_dev *pdev = dev->port;
+ u16 ctl;
+
+ pci_read_config_word(pdev, pdev->dpc_cap + PCI_EXP_DPC_CTL, );
+ ctl &= ~(PCI_EXP_DPC_CTL_EN_FATAL | PCI_EXP_DPC_CTL_INT_EN);
+ pci_write_config_word(pdev, pdev->dpc_cap + PCI_EXP_DPC_CTL, ctl);
+}
+
#define FLAG(x, y) (((x) & (y)) ? '+' : '-')
static int dpc_probe(struct pcie_device *dev)
{
struct pci_dev *pdev = dev->port;
struct device *device = >device;
int status;
- u16 ctl, cap;
+ u16 cap;
if (!pcie_aer_is_native(pdev) && !pcie_ports_dpc_native)
return -ENOTSUPP;
@@ -364,10 +384,7 @@ static int dpc_probe(struct pcie_device *dev)
}
pci_read_config_word(pdev, pdev->dpc_cap + PCI_EXP_DPC_CAP, );
- pci_read_config_word(pdev, pdev->dpc_cap + PCI_EXP_DPC_CTL, );
-
- ctl = (ctl & 0xfff4) | PCI_EXP_DPC_CTL_EN_FATAL |
PCI_EXP_DPC_CTL_INT_EN;
- pci_write_config_word(pdev, pdev->dpc_cap + PCI_EXP_DPC_CTL, ctl);
+ dpc_enable(dev);
pci_info(pdev, "enabled with IRQ %d\n", dev->irq);
pci_info(pdev, "error containment capabilities: Int Msg #%d, RPExt%c
PoisonedTLP%c SwTrigger%c RP PIO Log %d, DL_ActiveErr%c\n",
@@ -380,14 +397,25 @@ static int dpc_probe(struct pcie_device *dev)
return status;
}
-static void dpc_remove(struct pcie_device *dev)
+static int dpc_suspend(struct pcie_device *dev)
{
- struct pci_dev *pdev = dev->port;
- u16 ctl;
+ if (dev->shared_pme_irq)
+ dpc_disable(dev);
- pci_read_config_word(pdev, pdev->dpc_cap + PCI_EXP_DPC_CTL, );
- ctl &= ~(PCI_EXP_DPC_CTL_EN_FATAL | PCI_EXP_DPC_CTL_INT_EN);
- pci_write_config_word(pdev, pdev->dpc_cap + PCI_EXP_DPC_CTL, ctl);
+ return 0;
+}
+
+static int dpc_resume(struct pcie_device *dev)
+{
+ if (dev->shared_pme_irq)
+ dpc_enable(dev);
+
+ return 0;
+}
+
+static void dpc_remove(struct pcie_device *dev)
+{
+ dpc_disable(dev);
}
static struct pcie_port_service_driver dpcdriver = {
@@ -395,6 +423,8 @@ static struct pcie_port_service_driver dpcdriver = {
.port_type = PCIE_ANY_PORT,
.service= PCIE_PORT_SERVICE_DPC,
.probe = dpc_probe,
+ .suspend= dpc_suspend,
+ .resume = dpc_resume,
.remove = dpc_remove,
};
--
2.36.1
[PATCH 2/3] PCI/AER: Disable AER service on suspend when IRQ is shared with PME
PCIe service that shares IRQ with PME may cause spurious wakeup on
system suspend.
PCIe Base Spec 5.0, section 5.2 "Link State Power Management" states
that TLP and DLLP transmission is disabled for a Link in L2/L3 Ready
(D3hot), L2 (D3cold with aux power) and L3 (D3cold), so we don't lose
much here to disable AER during system suspend.
This is very similar to previous attempts to suspend AER and DPC [1],
but with a different reason.
[1]
https://lore.kernel.org/linux-pci/20220408153159.106741-1-kai.heng.f...@canonical.com/
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=216295
Signed-off-by: Kai-Heng Feng
---
drivers/pci/pcie/aer.c | 23 ++-
1 file changed, 22 insertions(+), 1 deletion(-)
diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
index 7952e5efd6cf3..60cc373754af2 100644
--- a/drivers/pci/pcie/aer.c
+++ b/drivers/pci/pcie/aer.c
@@ -1372,6 +1372,26 @@ static int aer_probe(struct pcie_device *dev)
return 0;
}
+static int aer_suspend(struct pcie_device *dev)
+{
+ struct aer_rpc *rpc = get_service_data(dev);
+
+ if (dev->shared_pme_irq)
+ aer_disable_rootport(rpc);
+
+ return 0;
+}
+
+static int aer_resume(struct pcie_device *dev)
+{
+ struct aer_rpc *rpc = get_service_data(dev);
+
+ if (dev->shared_pme_irq)
+ aer_enable_rootport(rpc);
+
+ return 0;
+}
+
/**
* aer_root_reset - reset Root Port hierarchy, RCEC, or RCiEP
* @dev: pointer to Root Port, RCEC, or RCiEP
@@ -1441,8 +1461,9 @@ static struct pcie_port_service_driver aerdriver = {
.name = "aer",
.port_type = PCIE_ANY_PORT,
.service= PCIE_PORT_SERVICE_AER,
-
.probe = aer_probe,
+ .suspend= aer_suspend,
+ .resume = aer_resume,
.remove = aer_remove,
};
--
2.36.1
Re: [PATCH v2 09/10] drm/ofdrm: Add per-model device function
Hello Michal,
On 7/26/22 16:40, Michal Suchánek wrote:
> Hello,
>
> On Tue, Jul 26, 2022 at 03:38:37PM +0200, Javier Martinez Canillas wrote:
>> On 7/20/22 16:27, Thomas Zimmermann wrote:
>>> Add a per-model device-function structure in preparation of adding
>>> color-management support. Detection of the individual models has been
>>> taken from fbdev's offb.
>>>
>>> Signed-off-by: Thomas Zimmermann
>>> ---
>>
>> Reviewed-by: Javier Martinez Canillas
>>
>> [...]
>>
>>> +static bool is_avivo(__be32 vendor, __be32 device)
>>> +{
>>> + /* This will match most R5xx */
>>> + return (vendor == 0x1002) &&
>>> + ((device >= 0x7100 && device < 0x7800) || (device >= 0x9400));
>>> +}
>>
>> Maybe add some constant macros to not have these magic numbers ?
>
> This is based on the existing fbdev implementation's magic numbers:
>
> drivers/video/fbdev/offb.c: ((*did >= 0x7100 && *did <
> 0x7800) ||
>
Ah, I see. Then we might have to go with the magic numbers...
> Of course, it would be great if somebody knowledgeable could clarify
> those.
>
Indeed.
--
Best regards,
Javier Martinez Canillas
Linux Engineering
Red Hat
Re: [PATCH v2] PCI/ERR: Use pcie_aer_is_native() to judge whether OS owns AER
On 7/25/22 7:05 PM, Zhuo Chen wrote:
> The AER status of the device that reported the error rather than
> the first downstream port is cleared after commit 7d7cbeaba5b7
> ("PCI/ERR: Clear status of the reporting device"). So "a bridge
> may not exist" which commit aa344bc8b727 ("PCI/ERR: Clear AER
> status only when we control AER") referring to is no longer
> existent, and we just use pcie_aer_is_native() in stead of
> "host->native_aer || pcie_ports_native".
IMO, above history is not required to justify using pcie_aer_is_native()
in place of "host->native_aer || pcie_ports_native".
>
> pci_aer_clear_nonfatal_status() already has pcie_aer_is_native(),
> so we move pci_aer_clear_nonfatal_status() out of
> pcie_aer_is_native().
Moving it outside (pcie_aer_is_native()) does not optimize the
code. So I think it is better to leave it inside.
>
> Replace statements that judge whether OS owns AER in
> get_port_device_capability() with pcie_aer_is_native(), which has
> no functional changes.
>
> Signed-off-by: Zhuo Chen
> ---
> v2:
> - Add details and note in commit log
> ---
> drivers/pci/pcie/err.c | 12 ++--
> drivers/pci/pcie/portdrv_core.c | 3 +--
> 2 files changed, 3 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/pci/pcie/err.c b/drivers/pci/pcie/err.c
> index 0c5a143025af..28339c741555 100644
> --- a/drivers/pci/pcie/err.c
> +++ b/drivers/pci/pcie/err.c
> @@ -184,7 +184,6 @@ pci_ers_result_t pcie_do_recovery(struct pci_dev *dev,
> int type = pci_pcie_type(dev);
> struct pci_dev *bridge;
> pci_ers_result_t status = PCI_ERS_RESULT_CAN_RECOVER;
> - struct pci_host_bridge *host = pci_find_host_bridge(dev->bus);
>
> /*
>* If the error was detected by a Root Port, Downstream Port, RCEC,
> @@ -237,16 +236,9 @@ pci_ers_result_t pcie_do_recovery(struct pci_dev *dev,
> pci_dbg(bridge, "broadcast resume message\n");
> pci_walk_bridge(bridge, report_resume, );
>
> - /*
> - * If we have native control of AER, clear error status in the device
> - * that detected the error. If the platform retained control of AER,
> - * it is responsible for clearing this status. In that case, the
> - * signaling device may not even be visible to the OS.
> - */
The above comment is still applicable. So I think you don't need to remove it.
> - if (host->native_aer || pcie_ports_native) {
> + if (pcie_aer_is_native(dev))
> pcie_clear_device_status(dev);
> - pci_aer_clear_nonfatal_status(dev);
> - }
> + pci_aer_clear_nonfatal_status(dev);
> pci_info(bridge, "device recovery successful\n");
> return status;
>
> diff --git a/drivers/pci/pcie/portdrv_core.c b/drivers/pci/pcie/portdrv_core.c
> index 604feeb84ee4..98c18f4a01b2 100644
> --- a/drivers/pci/pcie/portdrv_core.c
> +++ b/drivers/pci/pcie/portdrv_core.c
> @@ -221,8 +221,7 @@ static int get_port_device_capability(struct pci_dev *dev)
> }
>
> #ifdef CONFIG_PCIEAER
> - if (dev->aer_cap && pci_aer_available() &&
> - (pcie_ports_native || host->native_aer)) {
> + if (pcie_aer_is_native(dev) && pci_aer_available()) {
> services |= PCIE_PORT_SERVICE_AER;
>
> /*
--
Sathyanarayanan Kuppuswamy
Linux Kernel Developer
Re: [PATCH 1/2] powerpc: drop dependency on in archrandom.h
On Tue, Jul 26, 2022 at 04:57:38PM +1000, Michael Ellerman wrote: > Yury Norov writes: > > On Mon, Jul 25, 2022 at 09:28:12AM +0200, Andy Shevchenko wrote: > >> On Sun, Jul 24, 2022 at 12:19 AM Yury Norov wrote: > >> > > >> > archrandom.h includes to refer ppc_md. This causes > >> > circular header dependency, if generic nodemask.h includes random.h: > >> > > >> > In file included from include/linux/cred.h:16, > >> > from include/linux/seq_file.h:13, > >> > from arch/powerpc/include/asm/machdep.h:6, > >> > from arch/powerpc/include/asm/archrandom.h:5, > >> > from include/linux/random.h:109, > >> > from include/linux/nodemask.h:97, > >> > from include/linux/list_lru.h:12, > >> > from include/linux/fs.h:13, > >> > from include/linux/compat.h:17, > >> > from arch/powerpc/kernel/asm-offsets.c:12: > >> > include/linux/sched.h:1203:9: error: unknown type name 'nodemask_t' > >> > 1203 | nodemask_t mems_allowed; > >> > | ^~ > >> > > >> > Fix it by removing dependency from archrandom.h > >> > >> ... > >> > >> > EXPORT_SYMBOL_GPL(pm_power_off); > >> > >> ^^^ (Note this and read below) > >> > >> ... > >> > >> > +EXPORT_SYMBOL(arch_get_random_seed_long); > >> > >> It can't be like this. Brief browsing of the callees shows that. > > > > Is my understanding correct that you're suggesting to make it GPL? > > > > ppc_md is exported with EXPORT_SYMBOL(), and the function is in header, > > so it's available for non-GPL code now. I don't want to change it. > > That's true, your change maintains the status quo. > > But I think we actually don't need it exported to modules, I think it's > a private detail of the RNG <-> architecture interface, not something > that modules should be calling. > > So I think it's OK to drop the EXPORT_SYMBOL, either in this patch or a > subsequent one if you don't want to rebase. OK, changed.
Re: [PATCH v2 09/10] drm/ofdrm: Add per-model device function
Hello,
On Tue, Jul 26, 2022 at 03:38:37PM +0200, Javier Martinez Canillas wrote:
> On 7/20/22 16:27, Thomas Zimmermann wrote:
> > Add a per-model device-function structure in preparation of adding
> > color-management support. Detection of the individual models has been
> > taken from fbdev's offb.
> >
> > Signed-off-by: Thomas Zimmermann
> > ---
>
> Reviewed-by: Javier Martinez Canillas
>
> [...]
>
> > +static bool is_avivo(__be32 vendor, __be32 device)
> > +{
> > + /* This will match most R5xx */
> > + return (vendor == 0x1002) &&
> > + ((device >= 0x7100 && device < 0x7800) || (device >= 0x9400));
> > +}
>
> Maybe add some constant macros to not have these magic numbers ?
This is based on the existing fbdev implementation's magic numbers:
drivers/video/fbdev/offb.c: ((*did >= 0x7100 && *did < 0x7800)
||
Of course, it would be great if somebody knowledgeable could clarify
those.
Thanks
Michal
Re: Regression: Linux v5.15+ does not boot on Freescale P2020
Le 26/07/2022 à 10:34, Pali Rohár a écrit : > On Monday 25 July 2022 16:54:16 Segher Boessenkool wrote: >> On Mon, Jul 25, 2022 at 10:10:09PM +0200, Pali Rohár wrote: >>> On Monday 25 July 2022 16:20:49 Christophe Leroy wrote: >>> Now I did again clean test with same Debian 10 cross compiler. >>> >>> $ git clone >>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git && cd >>> linux >>> $ git checkout v5.15 >>> $ make mpc85xx_smp_defconfig ARCH=powerpc >>> CROSS_COMPILE=powerpc-linux-gnuspe- >>> $ make vmlinux ARCH=powerpc CROSS_COMPILE=powerpc-linux-gnuspe- >>> $ cp -a vmlinux vmlinux.v5.15 >>> $ git revert 9401f4e46cf6965e23738f70e149172344a01eef >>> $ make vmlinux ARCH=powerpc CROSS_COMPILE=powerpc-linux-gnuspe- >>> $ cp -a vmlinux vmlinux.revert >>> $ powerpc-linux-gnuspe-objdump -d vmlinux.revert > vmlinux.revert.dump >>> $ powerpc-linux-gnuspe-objdump -d vmlinux.v5.15 > vmlinux.v5.15.dump >>> $ diff -Naurp vmlinux.v5.15.dump vmlinux.revert.dump >>> >>> And there are: >>> >>> -c000c304: 7d 20 f8 29 lwarx r9,0,r31,1 >>> +c000c304: 7d 20 f8 28 lwarx r9,0,r31 >>> >>> I guess it must be reproducible this issue as I'm using regular >>> toolchain from distribution. >> >> The kernel had >> >> #define PPC_RAW_LWARX(t, a, b, eh) (0x7c28 | ___PPC_RT(t) | >> ___PPC_RA(a) | ___PPC_RB(b) | __PPC_EH(eh)) >> >> and >> >> #define PPC_LWARX(t, a, b, eh) stringify_in_c(.long PPC_RAW_LWARX(t, a, b, >> eh)) >> >> and >> >> #ifdef CONFIG_PPC64 >> #define __PPC_EH(eh)(((eh) & 0x1) << 0) >> #else >> #define __PPC_EH(eh)0 >> #endif >> >> but Christophe's 9401f4e46cf6 changed >> >> -"1:" PPC_LWARX(%0,0,%2,1) "\n\ >> +"1:lwarx %0,0,%2,1\n\ >> >> no longer checking CONFIG_PPC64. That appears to be the bug. > > Nice catch! > > Now I have tried to apply following change on master (without reverting > anything) > > diff --git a/arch/powerpc/include/asm/simple_spinlock.h > b/arch/powerpc/include/asm/simple_spinlock.h > index 7ae6aeef8464..72d3657fd2f7 100644 > --- a/arch/powerpc/include/asm/simple_spinlock.h > +++ b/arch/powerpc/include/asm/simple_spinlock.h > @@ -51,7 +51,7 @@ static inline unsigned long > __arch_spin_trylock(arch_spinlock_t *lock) > > token = LOCK_TOKEN; > __asm__ __volatile__( > -"1: lwarx %0,0,%2,1\n\ > +"1: lwarx %0,0,%2,0\n\ > cmpwi 0,%0,0\n\ > bne-2f\n\ > stwcx. %1,0,%2\n\ > @@ -158,7 +158,7 @@ static inline long __arch_read_trylock(arch_rwlock_t *rw) > long tmp; > > __asm__ __volatile__( > -"1: lwarx %0,0,%1,1\n" > +"1: lwarx %0,0,%1,0\n" > __DO_SIGN_EXTEND > " addic. %0,%0,1\n\ > ble-2f\n" > @@ -182,7 +182,7 @@ static inline long __arch_write_trylock(arch_rwlock_t *rw) > > token = WRLOCK_TOKEN; > __asm__ __volatile__( > -"1: lwarx %0,0,%2,1\n\ > +"1: lwarx %0,0,%2,0\n\ > cmpwi 0,%0,0\n\ > bne-2f\n" > " stwcx. %1,0,%2\n\ > > and with this change, objdump showed exactly same result as if I revert > that problematic commit on top of master branch. > > I guess that simple_spinlock.h should be fixed to pass 1 to lwarx for > CONFIG_PPC64 and 0 otherwise. > > Christophe, are you going to look at it? > Yes I will, but next week at the earliest. Christophe
Re: Regression: Linux v5.15+ does not boot on Freescale P2020
On Tue, Jul 26, 2022 at 04:01:00PM +0200, Pali Rohár wrote: > On Tuesday 26 July 2022 08:44:05 Segher Boessenkool wrote: > > And the architecture says > > Programming Note > > Warning: On some processors that comply with versions of the > > architecture that precede Version 2.00 > > But e500v2 is 2.03 and not older than 2.00... Yes. And it does not implement reserved fields in instructions (*any* reserved fields in instructions, apparently!) correctly at all. > > e500v1/v2 based chips will treat any reserved field being set in an > > opcode as illegal. > > > > while the architecture says > > > > Reserved fields in instructions are ignored by the processor. > > > > Whoops :-) We need fixes for processor implementation bugs all the > > time of course, but this is a massive *design* bug. > > I looked also in e500v2 and P2020 errata documents there is nothing > mentioned about eh flag. But it looks like a bug. The bug is if it does this for any reserved field (and it apparently does it for all even). > > Also people using an SMP kernel on older cores should see the problem, > > no? > > Probably yes. > > But most people on these machines are using stable LTS kernels and do > not upgrade too often. Yeah. > So you need to wait longer time to see people starting reporting such > bugs. Need to wait at least when v4.14 and v4.19 LTS versions stops > receiving updates. v4.19 is used in Debian 10 (oldstable) and v5.4 is > used by current OpenWRT. Both distributions are still supported, so > users have not migrated to new v5.15 problematic kernel yet. That's not a reasonable timeline for kernel development of course. We see the same thing with the compiler... Although GCC has a much slower release cadence (one new major version every year), it often takes two or three or more years before we get bug reports that something was broken. If stuff isn't tested, we cannot really support it at all. Segher
Re: Regression: Linux v5.15+ does not boot on Freescale P2020
On Tuesday 26 July 2022 08:44:05 Segher Boessenkool wrote:
> On Tue, Jul 26, 2022 at 11:02:59AM +0200, Arnd Bergmann wrote:
> > On Tue, Jul 26, 2022 at 10:34 AM Pali Rohár wrote:
> > > On Monday 25 July 2022 16:54:16 Segher Boessenkool wrote:
> > > > The EH field in larx insns is new since ISA 2.05, and some ISA 1.x cpu
> > > > implementations actually raise an illegal insn exception on EH=1. It
> > > > appears P2020 is one of those.
> > >
> > > P2020 has e500 cores. e500 cores uses ISA 2.03. So this may be reason.
> > > But in official Freescale/NXP documentation for e500 is documented that
> > > lwarx supports also eh=1. Maybe it is not really supported.
> > > https://www.nxp.com/files-static/32bit/doc/ref_manual/EREF_RM.pdf (page
> > > 562)
>
> (page 6-186)
>
> > > At least there is NOTE:
> > > Some older processors may treat EH=1 as an illegal instruction.
>
> And the architecture says
> Programming Note
> Warning: On some processors that comply with versions of the
> architecture that precede Version 2.00
But e500v2 is 2.03 and not older than 2.00...
> executing a Load And Reserve
> instruction in which EH = 1 will cause the illegal instruction error
> handler to be invoked.
>
> > In commit d6ccb1f55ddf ("powerpc/85xx: Make sure lwarx hint isn't set on
> > ppc32")
> > this was clarified to affect (all?) e500v1/v2,
>
> e500v1/v2 based chips will treat any reserved field being set in an
> opcode as illegal.
>
> while the architecture says
>
> Reserved fields in instructions are ignored by the processor.
>
> Whoops :-) We need fixes for processor implementation bugs all the
> time of course, but this is a massive *design* bug.
I looked also in e500v2 and P2020 errata documents there is nothing
mentioned about eh flag. But it looks like a bug.
> I'm surprised this
> CPU still works as well as it does!
>
> Even the venerable PEM (last updated in 1997) shows the EH field as
> reserved, always treated as 0.
>
> > this one apparently
> > fixed it before,
> > but Christophe's commit effectively reverted that change.
> >
> > I think only the simple_spinlock.h file actually uses EH=1
>
> That's right afaics.
>
> > and this is not
> > included in non-SMP kernels, so presumably the only affected machines were
> > the rare dual-core e500v2 ones (p2020, MPC8572, bsc9132), which would
> > explain why nobody noticed for the past 9 months.
>
> Also people using an SMP kernel on older cores should see the problem,
> no?
Probably yes.
But most people on these machines are using stable LTS kernels and do
not upgrade too often.
So you need to wait longer time to see people starting reporting such
bugs. Need to wait at least when v4.14 and v4.19 LTS versions stops
receiving updates. v4.19 is used in Debian 10 (oldstable) and v5.4 is
used by current OpenWRT. Both distributions are still supported, so
users have not migrated to new v5.15 problematic kernel yet.
> Or is that patched out? Or does this use case never happen :-)
>
>
> Segher
Re: [PATCH v2 10/10] drm/ofdrm: Support color management
On 7/20/22 16:27, Thomas Zimmermann wrote:
> Support the CRTC's color-management property and implement each model's
> palette support.
>
> The OF hardware has different methods of setting the palette. The
> respective code has been taken from fbdev's offb and refactored into
> per-model device functions. The device functions integrate this
> functionality into the overall modesetting.
>
> As palette handling is a CRTC property that depends on the primary
> plane's color format, the plane's atomic_check helper now updates the
> format field in ofdrm's custom CRTC state. The CRTC's atomic_flush
> helper updates the palette for the format as needed.
>
> Signed-off-by: Thomas Zimmermann
> ---
Reviewed-by: Javier Martinez Canillas
[...]
> +static void __iomem *ofdrm_mach64_cmap_ioremap(struct ofdrm_device *odev,
> +struct device_node *of_node,
> +u64 fb_base)
> +{
> + struct drm_device *dev = >dev;
> + u64 address;
> + void __iomem *cmap_base;
> +
> + address = fb_base & 0xff00ul;
> + address += 0x7ff000;
> +
It would be good to know where these addresses are coming from. Maybe some
constant macros or a comment ? Same for the other places where addresses
and offsets are used.
[...]
> static struct ofdrm_crtc_state *to_ofdrm_crtc_state(struct drm_crtc_state
> *base)
> @@ -376,10 +735,12 @@ static int
> ofdrm_primary_plane_helper_atomic_check(struct drm_plane *plane,
> struct drm_atomic_state
> *new_state)
> {
> struct drm_plane_state *new_plane_state =
> drm_atomic_get_new_plane_state(new_state, plane);
> + struct drm_framebuffer *new_fb = new_plane_state->fb;
> struct drm_crtc_state *new_crtc_state;
> + struct ofdrm_crtc_state *new_ofdrm_crtc_state;
> int ret;
>
> - if (!new_plane_state->fb)
> + if (!new_fb)
> return 0;
>
> new_crtc_state = drm_atomic_get_new_crtc_state(new_state,
> new_plane_state->crtc);
> @@ -391,6 +752,14 @@ static int
> ofdrm_primary_plane_helper_atomic_check(struct drm_plane *plane,
> if (ret)
> return ret;
>
> + if (!new_plane_state->visible)
> + return 0;
> +
> + new_crtc_state = drm_atomic_get_new_crtc_state(new_state,
> new_plane_state->crtc);
> +
> + new_ofdrm_crtc_state = to_ofdrm_crtc_state(new_crtc_state);
> + new_ofdrm_crtc_state->format = new_fb->format;
> +
Ah, I understand now why you didn't factor out the .atomic_check callbacks
for the two drivers in a fwfb helper. Maybe you can also add a comment to
mention that this updates the format so the CRTC palette can be applied in
the .atomic_flush callback ?
--
Best regards,
Javier Martinez Canillas
Linux Engineering
Red Hat
Re: Regression: Linux v5.15+ does not boot on Freescale P2020
On Tue, Jul 26, 2022 at 11:02:59AM +0200, Arnd Bergmann wrote:
> On Tue, Jul 26, 2022 at 10:34 AM Pali Rohár wrote:
> > On Monday 25 July 2022 16:54:16 Segher Boessenkool wrote:
> > > The EH field in larx insns is new since ISA 2.05, and some ISA 1.x cpu
> > > implementations actually raise an illegal insn exception on EH=1. It
> > > appears P2020 is one of those.
> >
> > P2020 has e500 cores. e500 cores uses ISA 2.03. So this may be reason.
> > But in official Freescale/NXP documentation for e500 is documented that
> > lwarx supports also eh=1. Maybe it is not really supported.
> > https://www.nxp.com/files-static/32bit/doc/ref_manual/EREF_RM.pdf (page 562)
(page 6-186)
> > At least there is NOTE:
> > Some older processors may treat EH=1 as an illegal instruction.
And the architecture says
Programming Note
Warning: On some processors that comply with versions of the
architecture that precede Version 2.00, executing a Load And Reserve
instruction in which EH = 1 will cause the illegal instruction error
handler to be invoked.
> In commit d6ccb1f55ddf ("powerpc/85xx: Make sure lwarx hint isn't set on
> ppc32")
> this was clarified to affect (all?) e500v1/v2,
e500v1/v2 based chips will treat any reserved field being set in an
opcode as illegal.
while the architecture says
Reserved fields in instructions are ignored by the processor.
Whoops :-) We need fixes for processor implementation bugs all the
time of course, but this is a massive *design* bug. I'm surprised this
CPU still works as well as it does!
Even the venerable PEM (last updated in 1997) shows the EH field as
reserved, always treated as 0.
> this one apparently
> fixed it before,
> but Christophe's commit effectively reverted that change.
>
> I think only the simple_spinlock.h file actually uses EH=1
That's right afaics.
> and this is not
> included in non-SMP kernels, so presumably the only affected machines were
> the rare dual-core e500v2 ones (p2020, MPC8572, bsc9132), which would
> explain why nobody noticed for the past 9 months.
Also people using an SMP kernel on older cores should see the problem,
no? Or is that patched out? Or does this use case never happen :-)
Segher
Re: [PATCH v2 09/10] drm/ofdrm: Add per-model device function
On 7/20/22 16:27, Thomas Zimmermann wrote:
> Add a per-model device-function structure in preparation of adding
> color-management support. Detection of the individual models has been
> taken from fbdev's offb.
>
> Signed-off-by: Thomas Zimmermann
> ---
Reviewed-by: Javier Martinez Canillas
[...]
> +static bool is_avivo(__be32 vendor, __be32 device)
> +{
> + /* This will match most R5xx */
> + return (vendor == 0x1002) &&
> +((device >= 0x7100 && device < 0x7800) || (device >= 0x9400));
> +}
Maybe add some constant macros to not have these magic numbers ?
--
Best regards,
Javier Martinez Canillas
Linux Engineering
Red Hat
Re: [PATCH v2 08/10] drm/ofdrm: Add CRTC state
On 7/20/22 16:27, Thomas Zimmermann wrote:
> Add a dedicated CRTC state to ofdrm to later store information for
> palette updates.
>
> Signed-off-by: Thomas Zimmermann
> ---
> drivers/gpu/drm/tiny/ofdrm.c | 62 ++--
>
Reviewed-by: Javier Martinez Canillas
[...]
> +static void ofdrm_crtc_reset(struct drm_crtc *crtc)
> +{
> + struct ofdrm_crtc_state *ofdrm_crtc_state;
> +
> + if (crtc->state) {
> + ofdrm_crtc_state_destroy(to_ofdrm_crtc_state(crtc->state));
> + crtc->state = NULL; /* must be set to NULL here */
> + }
> +
> + ofdrm_crtc_state = kzalloc(sizeof(*ofdrm_crtc_state), GFP_KERNEL);
> + if (!ofdrm_crtc_state)
> + return;
> + __drm_atomic_helper_crtc_reset(crtc, _crtc_state->base);
> +}
> +
IMO this function is hard to read, I would instead write it as following:
static void ofdrm_crtc_reset(struct drm_crtc *crtc)
{
struct ofdrm_crtc_state *ofdrm_crtc_state =
kzalloc(sizeof(*ofdrm_crtc_state), GFP_KERNEL);
if (!ofdrm_crtc_state)
return;
if (crtc->state) {
ofdrm_crtc_state_destroy(to_ofdrm_crtc_state(crtc->state));
crtc->state = NULL; /* must be set to NULL here */
}
__drm_atomic_helper_crtc_reset(crtc, _crtc_state->base);
}
Also with that form I think that the crtc->state = NULL could just be dropped ?
--
Best regards,
Javier Martinez Canillas
Linux Engineering
Red Hat
Re: [PATCH v2 07/10] drm/ofdrm: Add ofdrm for Open Firmware framebuffers
Hello Thomas,
On 7/20/22 16:27, Thomas Zimmermann wrote:
> Open Firmware provides basic display output via the 'display' node.
> DT platform code already provides a device that represents the node's
> framebuffer. Add a DRM driver for the device. The display mode and
> color format is pre-initialized by the system's firmware. Runtime
> modesetting via DRM is not possible. The display is useful during
> early boot stages or as error fallback.
>
I'm not familiar with OF display but the driver looks good to me.
Reviewed-by: Javier Martinez Canillas
I just have a few questions below.
[...]
> +static int ofdrm_primary_plane_helper_atomic_check(struct drm_plane *plane,
> +struct drm_atomic_state
> *new_state)
> +{
> + struct drm_plane_state *new_plane_state =
> drm_atomic_get_new_plane_state(new_state, plane);
> + struct drm_crtc_state *new_crtc_state;
> + int ret;
> +
> + if (!new_plane_state->fb)
> + return 0;
> +
> + new_crtc_state = drm_atomic_get_new_crtc_state(new_state,
> new_plane_state->crtc);
> +
> + ret = drm_atomic_helper_check_plane_state(new_plane_state,
> new_crtc_state,
> + DRM_PLANE_HELPER_NO_SCALING,
> + DRM_PLANE_HELPER_NO_SCALING,
> + false, false);
> + if (ret)
> + return ret;
> +
> + return 0;
> +}
This seems to be exactly the same check than used in the simpledrm driver.
Maybe could be moved to the fwfb helper library too ?
[...]
> +
> +static void ofdrm_crtc_helper_atomic_disable(struct drm_crtc *crtc,
> + struct drm_atomic_state *old_state)
> +{
> + /*
> + * Always enabled; disabling clears the screen in the
> + * primary plane's atomic_disable function.
> + */
> +}
> +
Same comment than for simpledrm, are these no-op helpers really needed ?
[...]
> +static const struct of_device_id ofdrm_of_match_display[] = {
> + { .compatible = "display", },
> + { },
> +};
> +MODULE_DEVICE_TABLE(of, ofdrm_of_match_display);
> +
I don't see a binding for this in Documentation/devicetree/bindings/display.
Do we need one or it's that only required for FDT and not Open Firmware DT ?
--
Best regards,
Javier Martinez Canillas
Linux Engineering
Red Hat
Re: [PATCH v3 2/2] powerpc/64s: Make POWER10 and later use pause_short in cpu_relax loops
Nicholas Piggin writes: > We want to move away from using SMT prioroty updates for cpu_relax, and > use a 'wait' instruction which is similar to x86. As well as being a > much better fit for what everybody else uses and tests with, priority > nops are stateful which is nasty (interrupts have to consider they might > be taken at a different priority), and they're expensive to execute, > similar to a mtSPR which can effect other threads in the pipe. > > Signed-off-by: Nicholas Piggin > --- > Unfortunately qemu TCG does not emulate pause_short properly and will > cause hangs. That _really_ sucks for testing, being able to use qemu is a huge benefit. I can boot test multiple kernels per minute using qemu, vs multiple minutes per kernel using real hardware. > I have a patch for it but not merged yet. But if we tune > qspinlock code it would be best to do it with this patch. What's the urgency on this patch? Can we wait for the qemu change to land? I guess Qemu 8 is not due until next year? :/ cheers
[powerpc:fixes-test] BUILD SUCCESS c653c591789b3acfa4bf6ae45d5af4f330e50a91
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git fixes-test branch HEAD: c653c591789b3acfa4bf6ae45d5af4f330e50a91 drm/amdgpu: Re-enable DCN for 64-bit powerpc elapsed time: 727m configs tested: 117 configs skipped: 4 The following configs have been built successfully. More configs may be tested in the coming days. gcc tested configs: arm64allyesconfig arm defconfig arm allyesconfig powerpc randconfig-c003-20220724 i386 randconfig-c001 sh sh7724_generic_defconfig arm s3c6400_defconfig m68k m5249evb_defconfig arm multi_v4t_defconfig mips loongson1b_defconfig m68k multi_defconfig xtensa iss_defconfig arc axs103_smp_defconfig m68kmac_defconfig shshmin_defconfig pariscgeneric-32bit_defconfig mips rt305x_defconfig loongarch loongson3_defconfig alpha defconfig loongarch defconfig powerpc mpc85xx_cds_defconfig arm ezx_defconfig mips allmodconfig sh espt_defconfig m68kdefconfig mips gcw0_defconfig mips tb0226_defconfig sh sh7710voipgw_defconfig sparc allnoconfig powerpcadder875_defconfig sh polaris_defconfig m68k atari_defconfig mips decstation_r4k_defconfig powerpc tqm8xx_defconfig powerpc tqm8555_defconfig mips fuloong2e_defconfig armoxnas_v6_defconfig sparc defconfig m68k amcore_defconfig nios2 10m50_defconfig s390defconfig s390 allmodconfig arc defconfig s390 allyesconfig arm64 defconfig ia64 allyesconfig arm allmodconfig ia64defconfig riscvnommu_virt_defconfig riscv rv32_defconfig riscvnommu_k210_defconfig riscv allnoconfig i386 debian-10.3-kselftests i386 debian-10.3 loongarch allnoconfig ia64 allmodconfig csky allnoconfig alpha allnoconfig arc allnoconfig m68k allyesconfig m68k allmodconfig arc allyesconfig alphaallyesconfig powerpc allnoconfig mips allyesconfig powerpc allmodconfig sh allmodconfig i386 allyesconfig i386defconfig x86_64randconfig-a006 x86_64randconfig-a004 x86_64randconfig-a002 i386 randconfig-a003 i386 randconfig-a001 i386 randconfig-a005 x86_64randconfig-a011 x86_64randconfig-a013 x86_64randconfig-a015 i386 randconfig-a012 i386 randconfig-a014 i386 randconfig-a016 s390 randconfig-r044-20220724 riscvrandconfig-r042-20220724 arc randconfig-r043-20220724 s390 randconfig-r044-20220726 riscvrandconfig-r042-20220726 arc randconfig-r043-20220726 x86_64rhel-8.3-kselftests um x86_64_defconfig um i386_defconfig x86_64 defconfig x86_64 allyesconfig x86_64 rhel-8.3 x86_64 rhel-8.3-kvm x86_64 rhel-8.3-func x86_64 rhel-8.3-syz x86_64 rhel-8.3-kunit clang tested configs: arm moxart_defconfig powerpc walnut_defconfig powerpc mpc832x_rdb_defconfig
Re: [PATCH 5/5] powerpc/pci: Add config option for using all 256 PCI buses
On Tuesday 26 July 2022 21:02:22 Michael Ellerman wrote: > Pali Rohár writes: > > On Wednesday 06 July 2022 12:43:08 Pali Rohár wrote: > >> By default on PPC32 are PCI bus numbers unique across all PCI domains. > >> So system could have only 256 PCI buses independently of available > >> PCI domains. > >> > >> This is due to filling DT property pci-OF-bus-map which does not reflect > >> multi-domain setup. > >> > >> On all powerpc platforms except chrp and powermac there is no DT property > >> pci-OF-bus-map anymore and therefore it is possible on non-chrp/powermac > >> platforms to avoid this limitation of maximal number of 256 PCI buses in > >> system even on multi-domain setup. > >> > >> But avoiding this limitation would mean that all PCI and PCIe devices would > >> be present on completely different BDF addresses as every PCI domain starts > >> numbering PCI bueses from zero (instead of the last bus number of previous > >> enumerated PCI domain). Such change could break existing software which > >> expects fixed PCI bus numbers. > >> > >> So add a new config option CONFIG_PPC_PCI_BUS_NUM_DOMAIN_DEPENDENT which > >> enables this change. By default it is disabled. It cause that initial value > >> of hose->first_busno is zero. > >> > >> Signed-off-by: Pali Rohár > >> --- > >> arch/powerpc/Kconfig | 11 +++ > >> arch/powerpc/kernel/pci_32.c | 6 ++ > >> 2 files changed, 17 insertions(+) > >> > >> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig > >> index be68c1f02b79..f66084bc1dfe 100644 > >> --- a/arch/powerpc/Kconfig > >> +++ b/arch/powerpc/Kconfig > >> @@ -370,6 +370,17 @@ config PPC_DCR > >>depends on PPC_DCR_NATIVE || PPC_DCR_MMIO > >>default y > >> > >> +config PPC_PCI_BUS_NUM_DOMAIN_DEPENDENT > >> + depends on PPC32 > >> + depends on !PPC_PMAC && !PPC_CHRP > >> + bool "Assign PCI bus numbers from zero individually for each PCI domain" > >> + help > >> +By default on PPC32 were PCI bus numbers unique across all PCI > >> domains. > >> +So system could have only 256 PCI buses independently of available > >> +PCI domains. When this option is enabled then PCI bus numbers are > >> +PCI domain dependent and each PCI controller on own domain can have > >> +256 PCI buses, like it is on other Linux architectures. > >> + > > > > What do you think, would it be possible to set default value of this > > option to enabled? > > My preference would be to not have the option at all, just make it the > default behaviour. Every new CONFIG option adds more combinations that > need testing, or more likely don't get well tested. > > But I don't have a good feel for what could break if we turn it on, so > honestly I don't really know. > > Also I do most of my 32-bit testing on pmacs, which are not affected by > the change. It is because this change is incompatible with deprecated pci-OF-bus-map which pmac uses. I do not have any pmac box for testing or development, so I let this part as is. If one day pci-OF-bus-map would be possible to disable on pmac then this pci bus number change can be enabled also for pmac. > So I'll probably take the series as-is, and then we can do some more > widespread testing and possibly flip the default to enabled, and then > maybe remove the option entirely in future. > > cheers I have tested it on P2020 board with 3 PCIe devices, each on own bus where each bus is connected to different PCIe controller / domain and it works correctly. Every PCIe device is on bus 1 bus but on different domains.
Re: [PATCH 5/5] powerpc/pci: Add config option for using all 256 PCI buses
Pali Rohár writes: > On Wednesday 06 July 2022 12:43:08 Pali Rohár wrote: >> By default on PPC32 are PCI bus numbers unique across all PCI domains. >> So system could have only 256 PCI buses independently of available >> PCI domains. >> >> This is due to filling DT property pci-OF-bus-map which does not reflect >> multi-domain setup. >> >> On all powerpc platforms except chrp and powermac there is no DT property >> pci-OF-bus-map anymore and therefore it is possible on non-chrp/powermac >> platforms to avoid this limitation of maximal number of 256 PCI buses in >> system even on multi-domain setup. >> >> But avoiding this limitation would mean that all PCI and PCIe devices would >> be present on completely different BDF addresses as every PCI domain starts >> numbering PCI bueses from zero (instead of the last bus number of previous >> enumerated PCI domain). Such change could break existing software which >> expects fixed PCI bus numbers. >> >> So add a new config option CONFIG_PPC_PCI_BUS_NUM_DOMAIN_DEPENDENT which >> enables this change. By default it is disabled. It cause that initial value >> of hose->first_busno is zero. >> >> Signed-off-by: Pali Rohár >> --- >> arch/powerpc/Kconfig | 11 +++ >> arch/powerpc/kernel/pci_32.c | 6 ++ >> 2 files changed, 17 insertions(+) >> >> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig >> index be68c1f02b79..f66084bc1dfe 100644 >> --- a/arch/powerpc/Kconfig >> +++ b/arch/powerpc/Kconfig >> @@ -370,6 +370,17 @@ config PPC_DCR >> depends on PPC_DCR_NATIVE || PPC_DCR_MMIO >> default y >> >> +config PPC_PCI_BUS_NUM_DOMAIN_DEPENDENT >> +depends on PPC32 >> +depends on !PPC_PMAC && !PPC_CHRP >> +bool "Assign PCI bus numbers from zero individually for each PCI domain" >> +help >> + By default on PPC32 were PCI bus numbers unique across all PCI >> domains. >> + So system could have only 256 PCI buses independently of available >> + PCI domains. When this option is enabled then PCI bus numbers are >> + PCI domain dependent and each PCI controller on own domain can have >> + 256 PCI buses, like it is on other Linux architectures. >> + > > What do you think, would it be possible to set default value of this > option to enabled? My preference would be to not have the option at all, just make it the default behaviour. Every new CONFIG option adds more combinations that need testing, or more likely don't get well tested. But I don't have a good feel for what could break if we turn it on, so honestly I don't really know. Also I do most of my 32-bit testing on pmacs, which are not affected by the change. So I'll probably take the series as-is, and then we can do some more widespread testing and possibly flip the default to enabled, and then maybe remove the option entirely in future. cheers
Re: [PATCH v5 4/4] pseries/mobility: set NMI watchdog factor during an LPM
Laurent Dufour writes: > Le 13/07/2022 à 22:17, Randy Dunlap a écrit : >> Hi Laurent, >> >> On 7/13/22 08:47, Laurent Dufour wrote: >>> diff --git a/Documentation/admin-guide/sysctl/kernel.rst >>> b/Documentation/admin-guide/sysctl/kernel.rst >>> index ddccd1077462..d73faa619c15 100644 >>> --- a/Documentation/admin-guide/sysctl/kernel.rst >>> +++ b/Documentation/admin-guide/sysctl/kernel.rst >>> @@ -592,6 +592,18 @@ to the guest kernel command line (see >>> Documentation/admin-guide/kernel-parameters.rst). >>> >>> >>> +nmi_wd_lpm_factor (PPC only) >>> + >>> + >>> +Factor apply to the NMI watchdog timeout (only when ``nmi_watchdog`` is >> >>Factor to apply to > > Thanks, Randy. > > Michael, could you fix that when applying the series? Yes, I did. cheers
Re: [PATCH 14/36] cpuidle: Fix rcu_idle_*() usage
On Wed, Jun 08, 2022 at 04:27:37PM +0200, Peter Zijlstra wrote:
> The whole disable-RCU, enable-IRQS dance is very intricate since
> changing IRQ state is traced, which depends on RCU.
>
> Add two helpers for the cpuidle case that mirror the entry code.
>
> Signed-off-by: Peter Zijlstra (Intel)
Reviewed-by: Gautham R. Shenoy
> ---
> arch/arm/mach-imx/cpuidle-imx6q.c|4 +--
> arch/arm/mach-imx/cpuidle-imx6sx.c |4 +--
> arch/arm/mach-omap2/cpuidle34xx.c|4 +--
> arch/arm/mach-omap2/cpuidle44xx.c|8 +++---
> drivers/acpi/processor_idle.c| 18 --
> drivers/cpuidle/cpuidle-big_little.c |4 +--
> drivers/cpuidle/cpuidle-mvebu-v7.c |4 +--
> drivers/cpuidle/cpuidle-psci.c |4 +--
> drivers/cpuidle/cpuidle-riscv-sbi.c |4 +--
> drivers/cpuidle/cpuidle-tegra.c |8 +++---
> drivers/cpuidle/cpuidle.c| 11
> include/linux/cpuidle.h | 37 +---
> kernel/sched/idle.c | 45
> ++-
> kernel/time/tick-broadcast.c |6 +++-
> 14 files changed, 90 insertions(+), 71 deletions(-)
>
> --- a/arch/arm/mach-imx/cpuidle-imx6q.c
> +++ b/arch/arm/mach-imx/cpuidle-imx6q.c
> @@ -24,9 +24,9 @@ static int imx6q_enter_wait(struct cpuid
> imx6_set_lpm(WAIT_UNCLOCKED);
> raw_spin_unlock(_lock);
>
> - rcu_idle_enter();
> + cpuidle_rcu_enter();
> cpu_do_idle();
> - rcu_idle_exit();
> + cpuidle_rcu_exit();
>
> raw_spin_lock(_lock);
> if (num_idle_cpus-- == num_online_cpus())
> --- a/arch/arm/mach-imx/cpuidle-imx6sx.c
> +++ b/arch/arm/mach-imx/cpuidle-imx6sx.c
> @@ -47,9 +47,9 @@ static int imx6sx_enter_wait(struct cpui
> cpu_pm_enter();
> cpu_cluster_pm_enter();
>
> - rcu_idle_enter();
> + cpuidle_rcu_enter();
> cpu_suspend(0, imx6sx_idle_finish);
> - rcu_idle_exit();
> + cpuidle_rcu_exit();
>
> cpu_cluster_pm_exit();
> cpu_pm_exit();
> --- a/arch/arm/mach-omap2/cpuidle34xx.c
> +++ b/arch/arm/mach-omap2/cpuidle34xx.c
> @@ -133,9 +133,9 @@ static int omap3_enter_idle(struct cpuid
> }
>
> /* Execute ARM wfi */
> - rcu_idle_enter();
> + cpuidle_rcu_enter();
> omap_sram_idle();
> - rcu_idle_exit();
> + cpuidle_rcu_exit();
>
> /*
>* Call idle CPU PM enter notifier chain to restore
> --- a/arch/arm/mach-omap2/cpuidle44xx.c
> +++ b/arch/arm/mach-omap2/cpuidle44xx.c
> @@ -105,9 +105,9 @@ static int omap_enter_idle_smp(struct cp
> }
> raw_spin_unlock_irqrestore(_lock, flag);
>
> - rcu_idle_enter();
> + cpuidle_rcu_enter();
> omap4_enter_lowpower(dev->cpu, cx->cpu_state);
> - rcu_idle_exit();
> + cpuidle_rcu_exit();
>
> raw_spin_lock_irqsave(_lock, flag);
> if (cx->mpu_state_vote == num_online_cpus())
> @@ -186,10 +186,10 @@ static int omap_enter_idle_coupled(struc
> }
> }
>
> - rcu_idle_enter();
> + cpuidle_rcu_enter();
> omap4_enter_lowpower(dev->cpu, cx->cpu_state);
> cpu_done[dev->cpu] = true;
> - rcu_idle_exit();
> + cpuidle_rcu_exit();
>
> /* Wakeup CPU1 only if it is not offlined */
> if (dev->cpu == 0 && cpumask_test_cpu(1, cpu_online_mask)) {
> --- a/drivers/acpi/processor_idle.c
> +++ b/drivers/acpi/processor_idle.c
> @@ -607,7 +607,7 @@ static DEFINE_RAW_SPINLOCK(c3_lock);
> * @cx: Target state context
> * @index: index of target state
> */
> -static int acpi_idle_enter_bm(struct cpuidle_driver *drv,
> +static noinstr int acpi_idle_enter_bm(struct cpuidle_driver *drv,
> struct acpi_processor *pr,
> struct acpi_processor_cx *cx,
> int index)
> @@ -626,6 +626,8 @@ static int acpi_idle_enter_bm(struct cpu
>*/
> bool dis_bm = pr->flags.bm_control;
>
> + instrumentation_begin();
> +
> /* If we can skip BM, demote to a safe state. */
> if (!cx->bm_sts_skip && acpi_idle_bm_check()) {
> dis_bm = false;
> @@ -647,11 +649,11 @@ static int acpi_idle_enter_bm(struct cpu
> raw_spin_unlock(_lock);
> }
>
> - rcu_idle_enter();
> + cpuidle_rcu_enter();
>
> acpi_idle_do_entry(cx);
>
> - rcu_idle_exit();
> + cpuidle_rcu_exit();
>
> /* Re-enable bus master arbitration */
> if (dis_bm) {
> @@ -661,11 +663,13 @@ static int acpi_idle_enter_bm(struct cpu
> raw_spin_unlock(_lock);
> }
>
> + instrumentation_end();
> +
> return index;
> }
>
> -static int acpi_idle_enter(struct cpuidle_device *dev,
> -struct cpuidle_driver *drv, int index)
> +static noinstr int acpi_idle_enter(struct cpuidle_device *dev,
> +struct cpuidle_driver
Re: Regression: Linux v5.15+ does not boot on Freescale P2020
On Tue, Jul 26, 2022 at 10:34 AM Pali Rohár wrote:
> On Monday 25 July 2022 16:54:16 Segher Boessenkool wrote:
> > On Mon, Jul 25, 2022 at 10:10:09PM +0200, Pali Rohár wrote:
> > > On Monday 25 July 2022 16:20:49 Christophe Leroy wrote:
> > > Now I did again clean test with same Debian 10 cross compiler.
> > >
> > > $ git clone
> > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git && cd
> > > linux
> > > $ git checkout v5.15
> > > $ make mpc85xx_smp_defconfig ARCH=powerpc
> > > CROSS_COMPILE=powerpc-linux-gnuspe-
> > > $ make vmlinux ARCH=powerpc CROSS_COMPILE=powerpc-linux-gnuspe-
> > > $ cp -a vmlinux vmlinux.v5.15
> > > $ git revert 9401f4e46cf6965e23738f70e149172344a01eef
> > > $ make vmlinux ARCH=powerpc CROSS_COMPILE=powerpc-linux-gnuspe-
> > > $ cp -a vmlinux vmlinux.revert
> > > $ powerpc-linux-gnuspe-objdump -d vmlinux.revert > vmlinux.revert.dump
> > > $ powerpc-linux-gnuspe-objdump -d vmlinux.v5.15 > vmlinux.v5.15.dump
> > > $ diff -Naurp vmlinux.v5.15.dump vmlinux.revert.dump
> > >
> > > And there are:
> > >
> > > -c000c304: 7d 20 f8 29 lwarx r9,0,r31,1
> > > +c000c304: 7d 20 f8 28 lwarx r9,0,r31
> > >
> > > I guess it must be reproducible this issue as I'm using regular
> > > toolchain from distribution.
> >
>
> > The EH field in larx insns is new since ISA 2.05, and some ISA 1.x cpu
> > implementations actually raise an illegal insn exception on EH=1. It
> > appears P2020 is one of those.
> >
>
> P2020 has e500 cores. e500 cores uses ISA 2.03. So this may be reason.
> But in official Freescale/NXP documentation for e500 is documented that
> lwarx supports also eh=1. Maybe it is not really supported.
> https://www.nxp.com/files-static/32bit/doc/ref_manual/EREF_RM.pdf (page 562)
> At least there is NOTE:
> Some older processors may treat EH=1 as an illegal instruction.
In commit d6ccb1f55ddf ("powerpc/85xx: Make sure lwarx hint isn't set on ppc32")
this was clarified to affect (all?) e500v1/v2, this one apparently
fixed it before,
but Christophe's commit effectively reverted that change.
I think only the simple_spinlock.h file actually uses EH=1 and this is not
included in non-SMP kernels, so presumably the only affected machines were
the rare dual-core e500v2 ones (p2020, MPC8572, bsc9132), which would
explain why nobody noticed for the past 9 months.
Arnd
Re: Regression: Linux v5.15+ does not boot on Freescale P2020
On Monday 25 July 2022 16:54:16 Segher Boessenkool wrote: > On Mon, Jul 25, 2022 at 10:10:09PM +0200, Pali Rohár wrote: > > On Monday 25 July 2022 16:20:49 Christophe Leroy wrote: > > Now I did again clean test with same Debian 10 cross compiler. > > > > $ git clone > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git && cd > > linux > > $ git checkout v5.15 > > $ make mpc85xx_smp_defconfig ARCH=powerpc > > CROSS_COMPILE=powerpc-linux-gnuspe- > > $ make vmlinux ARCH=powerpc CROSS_COMPILE=powerpc-linux-gnuspe- > > $ cp -a vmlinux vmlinux.v5.15 > > $ git revert 9401f4e46cf6965e23738f70e149172344a01eef > > $ make vmlinux ARCH=powerpc CROSS_COMPILE=powerpc-linux-gnuspe- > > $ cp -a vmlinux vmlinux.revert > > $ powerpc-linux-gnuspe-objdump -d vmlinux.revert > vmlinux.revert.dump > > $ powerpc-linux-gnuspe-objdump -d vmlinux.v5.15 > vmlinux.v5.15.dump > > $ diff -Naurp vmlinux.v5.15.dump vmlinux.revert.dump > > > > And there are: > > > > -c000c304: 7d 20 f8 29 lwarx r9,0,r31,1 > > +c000c304: 7d 20 f8 28 lwarx r9,0,r31 > > > > I guess it must be reproducible this issue as I'm using regular > > toolchain from distribution. > > The kernel had > > #define PPC_RAW_LWARX(t, a, b, eh) (0x7c28 | ___PPC_RT(t) | > ___PPC_RA(a) | ___PPC_RB(b) | __PPC_EH(eh)) > > and > > #define PPC_LWARX(t, a, b, eh) stringify_in_c(.long PPC_RAW_LWARX(t, a, b, > eh)) > > and > > #ifdef CONFIG_PPC64 > #define __PPC_EH(eh)(((eh) & 0x1) << 0) > #else > #define __PPC_EH(eh)0 > #endif > > but Christophe's 9401f4e46cf6 changed > > -"1:" PPC_LWARX(%0,0,%2,1) "\n\ > +"1:lwarx %0,0,%2,1\n\ > > no longer checking CONFIG_PPC64. That appears to be the bug. Nice catch! Now I have tried to apply following change on master (without reverting anything) diff --git a/arch/powerpc/include/asm/simple_spinlock.h b/arch/powerpc/include/asm/simple_spinlock.h index 7ae6aeef8464..72d3657fd2f7 100644 --- a/arch/powerpc/include/asm/simple_spinlock.h +++ b/arch/powerpc/include/asm/simple_spinlock.h @@ -51,7 +51,7 @@ static inline unsigned long __arch_spin_trylock(arch_spinlock_t *lock) token = LOCK_TOKEN; __asm__ __volatile__( -"1:lwarx %0,0,%2,1\n\ +"1:lwarx %0,0,%2,0\n\ cmpwi 0,%0,0\n\ bne-2f\n\ stwcx. %1,0,%2\n\ @@ -158,7 +158,7 @@ static inline long __arch_read_trylock(arch_rwlock_t *rw) long tmp; __asm__ __volatile__( -"1:lwarx %0,0,%1,1\n" +"1:lwarx %0,0,%1,0\n" __DO_SIGN_EXTEND " addic. %0,%0,1\n\ ble-2f\n" @@ -182,7 +182,7 @@ static inline long __arch_write_trylock(arch_rwlock_t *rw) token = WRLOCK_TOKEN; __asm__ __volatile__( -"1:lwarx %0,0,%2,1\n\ +"1:lwarx %0,0,%2,0\n\ cmpwi 0,%0,0\n\ bne-2f\n" " stwcx. %1,0,%2\n\ and with this change, objdump showed exactly same result as if I revert that problematic commit on top of master branch. I guess that simple_spinlock.h should be fixed to pass 1 to lwarx for CONFIG_PPC64 and 0 otherwise. Christophe, are you going to look at it? > The EH field in larx insns is new since ISA 2.05, and some ISA 1.x cpu > implementations actually raise an illegal insn exception on EH=1. It > appears P2020 is one of those. > > > Segher P2020 has e500 cores. e500 cores uses ISA 2.03. So this may be reason. But in official Freescale/NXP documentation for e500 is documented that lwarx supports also eh=1. Maybe it is not really supported. https://www.nxp.com/files-static/32bit/doc/ref_manual/EREF_RM.pdf (page 562) At least there is NOTE: Some older processors may treat EH=1 as an illegal instruction.
Re: [PATCH v3 4/4] watchdog/pseries-wdt: initial support for H_WATCHDOG-based watchdog timers
Guenter Roeck writes: > On Mon, Jul 25, 2022 at 11:11:41AM -0500, Scott Cheloha wrote: >> On Wed, Jul 13, 2022 at 01:50:14PM -0700, Guenter Roeck wrote: >> > On 7/13/22 13:23, Scott Cheloha wrote: >> > > PAPR v2.12 defines a new hypercall, H_WATCHDOG. The hypercall permits >> > > guest control of one or more virtual watchdog timers. The timers have >> > > millisecond granularity. The guest is terminated when a timer >> > > expires. >> > > >> > > This patch adds a watchdog driver for these timers, "pseries-wdt". >> > > >> > > pseries_wdt_probe() currently assumes the existence of only one >> > > platform device and always assigns it watchdogNumber 1. If we ever >> > > expose more than one timer to userspace we will need to devise a way >> > > to assign a distinct watchdogNumber to each platform device at device >> > > registration time. >> > > >> > > Signed-off-by: Scott Cheloha >> > >> > Acked-by: Guenter Roeck >> >> Guenter, Michael, >> >> Which tree is taking this series? powerpc. It's been in next-test/next for about a week. It's in linux-next today. > The series includes non-watchdog changes, so my expectation was that some > other tree would take it. Yep, thanks for the ack. cheers
Re: [PATCH 1/2] powerpc: drop dependency on in archrandom.h
Yury Norov writes: > On Mon, Jul 25, 2022 at 09:28:12AM +0200, Andy Shevchenko wrote: >> On Sun, Jul 24, 2022 at 12:19 AM Yury Norov wrote: >> > >> > archrandom.h includes to refer ppc_md. This causes >> > circular header dependency, if generic nodemask.h includes random.h: >> > >> > In file included from include/linux/cred.h:16, >> > from include/linux/seq_file.h:13, >> > from arch/powerpc/include/asm/machdep.h:6, >> > from arch/powerpc/include/asm/archrandom.h:5, >> > from include/linux/random.h:109, >> > from include/linux/nodemask.h:97, >> > from include/linux/list_lru.h:12, >> > from include/linux/fs.h:13, >> > from include/linux/compat.h:17, >> > from arch/powerpc/kernel/asm-offsets.c:12: >> > include/linux/sched.h:1203:9: error: unknown type name 'nodemask_t' >> > 1203 | nodemask_t mems_allowed; >> > | ^~ >> > >> > Fix it by removing dependency from archrandom.h >> >> ... >> >> > EXPORT_SYMBOL_GPL(pm_power_off); >> >> ^^^ (Note this and read below) >> >> ... >> >> > +EXPORT_SYMBOL(arch_get_random_seed_long); >> >> It can't be like this. Brief browsing of the callees shows that. > > Is my understanding correct that you're suggesting to make it GPL? > > ppc_md is exported with EXPORT_SYMBOL(), and the function is in header, > so it's available for non-GPL code now. I don't want to change it. That's true, your change maintains the status quo. But I think we actually don't need it exported to modules, I think it's a private detail of the RNG <-> architecture interface, not something that modules should be calling. So I think it's OK to drop the EXPORT_SYMBOL, either in this patch or a subsequent one if you don't want to rebase. cheers
Re: [PATCH 1/2] powerpc: drop dependency on in archrandom.h
On Tue, Jul 26, 2022 at 8:13 AM Andy Shevchenko
wrote:
>
> On Tue, Jul 26, 2022 at 1:35 AM Yury Norov wrote:
> > On Mon, Jul 25, 2022 at 11:39:39PM +0200, Andy Shevchenko wrote:
> > > On Mon, Jul 25, 2022 at 6:19 PM Yury Norov wrote:
> > > > On Mon, Jul 25, 2022 at 09:28:12AM +0200, Andy Shevchenko wrote:
> > > > > On Sun, Jul 24, 2022 at 12:19 AM Yury Norov
> > > > > wrote:
>
> ...
>
> > > > > > EXPORT_SYMBOL_GPL(pm_power_off);
> > > > >
> > > > > ^^^ (Note this and read below)
> > > > >
> > > > > ...
> > > > >
> > > > > > +EXPORT_SYMBOL(arch_get_random_seed_long);
> > > > >
> > > > > It can't be like this. Brief browsing of the callees shows that.
> > > >
> > > > Is my understanding correct that you're suggesting to make it GPL?
> > > >
> > > > ppc_md is exported with EXPORT_SYMBOL(), and the function is in header,
> > > > so it's available for non-GPL code now. I don't want to change it.
> > >
> > > The symbols your function calls are GPL. As far as I understand (not a
> > > lawyer!) it logically one may not call GPL and pretend to be non-GPL.
> >
> > Can you explain what you mean in details?
> >
> > The function is:
> > static inline bool __must_check arch_get_random_seed_long(unsigned
> > long *v)
> > {
> >if (ppc_md.get_random_seed)
> >return ppc_md.get_random_seed(v);
> >
> >return false;
> > }
> >
> > ppc_md is non-GPL:
> > 77 /* The main machine-dep calls structure
> > 78 */
> > 79 struct machdep_calls ppc_md;
> > 80 EXPORT_SYMBOL(ppc_md);
>
> What a mess...
>
> > And get_random_seed is initialized in in
> > arch/powerpc/platforms/powernv/rng.c
> > with different functions that are static and not exported at all.
To be clear, their license is defined in the file: "GPL-2.0-or-later".
But again, not a lawyer, just using my common sense.
> > I don't understand where arch_get_random_seed_long calls GPL...
>
> The ->get_random_seed() (aka "callees" in my previous mail) are all
> GPL (maybe I missed one out of five which is non-GPL, but then it's
> even more of a mess).
--
With Best Regards,
Andy Shevchenko
Re: [PATCH 1/2] powerpc: drop dependency on in archrandom.h
On Tue, Jul 26, 2022 at 1:35 AM Yury Norov wrote:
> On Mon, Jul 25, 2022 at 11:39:39PM +0200, Andy Shevchenko wrote:
> > On Mon, Jul 25, 2022 at 6:19 PM Yury Norov wrote:
> > > On Mon, Jul 25, 2022 at 09:28:12AM +0200, Andy Shevchenko wrote:
> > > > On Sun, Jul 24, 2022 at 12:19 AM Yury Norov
> > > > wrote:
...
> > > > > EXPORT_SYMBOL_GPL(pm_power_off);
> > > >
> > > > ^^^ (Note this and read below)
> > > >
> > > > ...
> > > >
> > > > > +EXPORT_SYMBOL(arch_get_random_seed_long);
> > > >
> > > > It can't be like this. Brief browsing of the callees shows that.
> > >
> > > Is my understanding correct that you're suggesting to make it GPL?
> > >
> > > ppc_md is exported with EXPORT_SYMBOL(), and the function is in header,
> > > so it's available for non-GPL code now. I don't want to change it.
> >
> > The symbols your function calls are GPL. As far as I understand (not a
> > lawyer!) it logically one may not call GPL and pretend to be non-GPL.
>
> Can you explain what you mean in details?
>
> The function is:
> static inline bool __must_check arch_get_random_seed_long(unsigned
> long *v)
> {
>if (ppc_md.get_random_seed)
>return ppc_md.get_random_seed(v);
>
>return false;
> }
>
> ppc_md is non-GPL:
> 77 /* The main machine-dep calls structure
> 78 */
> 79 struct machdep_calls ppc_md;
> 80 EXPORT_SYMBOL(ppc_md);
What a mess...
> And get_random_seed is initialized in in arch/powerpc/platforms/powernv/rng.c
> with different functions that are static and not exported at all.
>
> I don't understand where arch_get_random_seed_long calls GPL...
The ->get_random_seed() (aka "callees" in my previous mail) are all
GPL (maybe I missed one out of five which is non-GPL, but then it's
even more of a mess).
--
With Best Regards,
Andy Shevchenko
