* Re: [PATCH 5/5] powerpc sstep: Add tests for Prefixed Add Immediate
From: Jordan Niethe @ 2020-07-22 1:26 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Alistair Popple, Balamuruhan S
In-Reply-To: <20200525025923.19843-5-jniethe5@gmail.com>
On Mon, May 25, 2020 at 1:00 PM Jordan Niethe <jniethe5@gmail.com> wrote:
>
> Use the existing support for testing compute type instructions to test
> Prefixed Add Immediate (paddi). The R bit of the paddi instruction
> controls whether current instruction address is used. Add test cases for
> when R=1 and for R=0. paddi has a 34 bit immediate field formed by
> concatenating si0 and si1. Add tests for the extreme values of this
> field.
>
> Skip the paddi tests if ISA v3.1 is unsupported.
>
> Some of these test cases were added by Balamuruhan S.
>
> Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
> ---
> arch/powerpc/lib/test_emulate_step.c | 127 ++++++++++++++++++
> .../lib/test_emulate_step_exec_instr.S | 1 +
> 2 files changed, 128 insertions(+)
>
> diff --git a/arch/powerpc/lib/test_emulate_step.c b/arch/powerpc/lib/test_emulate_step.c
> index 579b5db80674..33a72b7d2764 100644
> --- a/arch/powerpc/lib/test_emulate_step.c
> +++ b/arch/powerpc/lib/test_emulate_step.c
> @@ -105,6 +105,13 @@
> ___PPC_RA(a) | ___PPC_RB(b))
> #define TEST_ADDC_DOT(t, a, b) ppc_inst(PPC_INST_ADDC | ___PPC_RT(t) | \
> ___PPC_RA(a) | ___PPC_RB(b) | 0x1)
> +#define TEST_PADDI(t, a, i, pr) ppc_inst_prefix(PPC_PREFIX_MLS | __PPC_PRFX_R(pr) | \
> + IMM_H(i), \
> + PPC_INST_ADDI | \
> + ___PPC_RT(t) | ___PPC_RA(a) | \
> + IMM_L(i))
> +
> +
>
> #define MAX_SUBTESTS 16
>
> @@ -699,6 +706,11 @@ struct compute_test {
> } subtests[MAX_SUBTESTS + 1];
> };
>
> +/* Extreme values for si0||si1 (the MLS:D-form 34 bit immediate field) */
> +#define SI_MIN BIT(33)
> +#define SI_MAX (BIT(33) - 1)
> +#define SI_UMAX (BIT(34) - 1)
> +
> static struct compute_test compute_tests[] = {
> {
> .mnemonic = "nop",
> @@ -1071,6 +1083,121 @@ static struct compute_test compute_tests[] = {
> }
> }
> }
> + },
> + {
> + .mnemonic = "paddi",
> + .cpu_feature = CPU_FTR_ARCH_31,
> + .subtests = {
> + {
> + .descr = "RA = LONG_MIN, SI = SI_MIN, R = 0",
> + .instr = TEST_PADDI(21, 22, SI_MIN, 0),
> + .regs = {
> + .gpr[21] = 0,
> + .gpr[22] = LONG_MIN,
> + }
> + },
> + {
> + .descr = "RA = LONG_MIN, SI = SI_MAX, R = 0",
> + .instr = TEST_PADDI(21, 22, SI_MAX, 0),
> + .regs = {
> + .gpr[21] = 0,
> + .gpr[22] = LONG_MIN,
> + }
> + },
> + {
> + .descr = "RA = LONG_MAX, SI = SI_MAX, R = 0",
> + .instr = TEST_PADDI(21, 22, SI_MAX, 0),
> + .regs = {
> + .gpr[21] = 0,
> + .gpr[22] = LONG_MAX,
> + }
> + },
> + {
> + .descr = "RA = ULONG_MAX, SI = SI_UMAX, R = 0",
> + .instr = TEST_PADDI(21, 22, SI_UMAX, 0),
> + .regs = {
> + .gpr[21] = 0,
> + .gpr[22] = ULONG_MAX,
> + }
> + },
> + {
> + .descr = "RA = ULONG_MAX, SI = 0x1, R = 0",
> + .instr = TEST_PADDI(21, 22, 0x1, 0),
> + .regs = {
> + .gpr[21] = 0,
> + .gpr[22] = ULONG_MAX,
> + }
> + },
> + {
> + .descr = "RA = INT_MIN, SI = SI_MIN, R = 0",
> + .instr = TEST_PADDI(21, 22, SI_MIN, 0),
> + .regs = {
> + .gpr[21] = 0,
> + .gpr[22] = INT_MIN,
> + }
> + },
> + {
> + .descr = "RA = INT_MIN, SI = SI_MAX, R = 0",
> + .instr = TEST_PADDI(21, 22, SI_MAX, 0),
> + .regs = {
> + .gpr[21] = 0,
> + .gpr[22] = INT_MIN,
> + }
> + },
> + {
> + .descr = "RA = INT_MAX, SI = SI_MAX, R = 0",
> + .instr = TEST_PADDI(21, 22, SI_MAX, 0),
> + .regs = {
> + .gpr[21] = 0,
> + .gpr[22] = INT_MAX,
> + }
> + },
> + {
> + .descr = "RA = UINT_MAX, SI = 0x1, R = 0",
> + .instr = TEST_PADDI(21, 22, 0x1, 0),
> + .regs = {
> + .gpr[21] = 0,
> + .gpr[22] = UINT_MAX,
> + }
> + },
> + {
> + .descr = "RA = UINT_MAX, SI = SI_MAX, R = 0",
> + .instr = TEST_PADDI(21, 22, SI_MAX, 0),
> + .regs = {
> + .gpr[21] = 0,
> + .gpr[22] = UINT_MAX,
> + }
> + },
> + {
> + .descr = "RA is r0, SI = SI_MIN, R = 0",
> + .instr = TEST_PADDI(21, 0, SI_MIN, 0),
> + .regs = {
> + .gpr[21] = 0x0,
> + }
> + },
> + {
> + .descr = "RA = 0, SI = SI_MIN, R = 0",
> + .instr = TEST_PADDI(21, 22, SI_MIN, 0),
> + .regs = {
> + .gpr[21] = 0x0,
> + .gpr[22] = 0x0,
> + }
> + },
> + {
> + .descr = "RA is r0, SI = 0, R = 1",
> + .instr = TEST_PADDI(21, 0, 0, 1),
> + .regs = {
> + .gpr[21] = 0,
> + }
> + },
> + {
> + .descr = "RA is r0, SI = SI_MIN, R = 1",
> + .instr = TEST_PADDI(21, 0, SI_MIN, 1),
> + .regs = {
> + .gpr[21] = 0,
> + }
> + }
> + }
> }
> };
>
> diff --git a/arch/powerpc/lib/test_emulate_step_exec_instr.S b/arch/powerpc/lib/test_emulate_step_exec_instr.S
> index 1580f34f4f4f..aef53ee77a43 100644
> --- a/arch/powerpc/lib/test_emulate_step_exec_instr.S
> +++ b/arch/powerpc/lib/test_emulate_step_exec_instr.S
> @@ -81,6 +81,7 @@ _GLOBAL(exec_instr)
>
> /* Placeholder for the test instruction */
> 1: nop
> + nop
> patch_site 1b patch__exec_instr
>
> /*
> --
> 2.17.1
>
Because of the alignment requirements of prefixed instructions, the
noops to be patched need to be aligned.
mpe, want me to send a new version?
--- a/arch/powerpc/lib/test_emulate_step_exec_instr.S
+++ b/arch/powerpc/lib/test_emulate_step_exec_instr.S
@@ -80,6 +80,7 @@ _GLOBAL(exec_instr)
REST_NVGPRS(r31)
/* Placeholder for the test instruction */
+.align 6
1: nop
nop
patch_site 1b patch__exec_instr
^ permalink raw reply
* Re: [PATCH v4 5/7] powerpc/iommu: Move iommu_table cleaning routine to iommu_table_clean
From: Alexey Kardashevskiy @ 2020-07-22 1:28 UTC (permalink / raw)
To: Leonardo Bras, Michael Ellerman, Benjamin Herrenschmidt,
Paul Mackerras, Joel Stanley, Christophe Leroy,
Thiago Jung Bauermann, Ram Pai, Brian King
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <0f4c2d84d0958e98e7ada53c25750fe548cadf0b.camel@gmail.com>
On 22/07/2020 08:13, Leonardo Bras wrote:
> On Tue, 2020-07-21 at 14:59 +1000, Alexey Kardashevskiy wrote:
>>
>> On 16/07/2020 17:16, Leonardo Bras wrote:
>>> Move the part of iommu_table_free() that does struct iommu_table cleaning
>>> into iommu_table_clean, so we can invoke it separately.
>>>
>>> This new function is useful for cleaning struct iommu_table before
>>> initializing it again with a new DMA window, without having it freed and
>>> allocated again.
>>>
>>> Signed-off-by: Leonardo Bras <leobras.c@gmail.com>
>>> ---
>>> arch/powerpc/kernel/iommu.c | 30 ++++++++++++++++++------------
>>> 1 file changed, 18 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/arch/powerpc/kernel/iommu.c b/arch/powerpc/kernel/iommu.c
>>> index 9704f3f76e63..c3242253a4e7 100644
>>> --- a/arch/powerpc/kernel/iommu.c
>>> +++ b/arch/powerpc/kernel/iommu.c
>>> @@ -735,21 +735,10 @@ struct iommu_table *iommu_init_table(struct iommu_table *tbl, int nid,
>>> return tbl;
>>> }
>>>
>>> -static void iommu_table_free(struct kref *kref)
>>> +static void iommu_table_clean(struct iommu_table *tbl)
>>
>> iommu_table_free() + iommu_init_table() + set_iommu_table_base() should
>> work too, why new helper?
>
> iommu_table_free() also frees the tbl, which would need allocate it
> again (new address) and to fill it up again, unnecessarily.
It is a new table in fact, everything is new there. You are only saving
kfree+kzalloc which does not seem a huge win.
Also, iommu_table_update() simply assumes 64bit window by passing
res_start=res_end=0 to iommu_init_table() which is not horribly robust
either. Yeah, I know, iommu_init_table() is always called with zeroes in
pseries but this is somewhat ok as those tables are from the device tree
and those windows don't overlap with 32bit MMIO but under KVM they will
(well, if we hack QEMU to advertise a single window).
I suggest removing iommu_pseries_table_update() from 6/7 and do
iommu_table_free() + iommu_init_table() + set_iommu_table_base() with a
WARN_ON(pdev->dev.archdata.dma_offset>=SZ_4G), may be even do this all
in enable_ddw() where we know for sure if it is 1:1 mapping or just a
big window.
Out of curiosity - what page sizes does pHyp advertise in "query"?
> I think it's a better approach to only change what is needed.
>
>> There is also iommu_table_clear() which does a different thing so you
>> need a better name.
>
> I agree.
> I had not noticed this other function before sending the patchset. What
> would be a better name though? __iommu_table_free()?
>
>> Second, iommu_table_free
>> use and it would be ok as we would only see this when hot-unplugging a
>> PE because we always kept the default window.
>> Btw you must be seeing these warnings now every time you create DDW with
>> these patches as at least the first page is reserved, do not you?
>
> It does not print a warning.
> I noticed other warnings,
And what are these?
> but not this one from iommu_table_free():
> /* verify that table contains no entries */
> if (!bitmap_empty(tbl->it_ma
> p, tbl->it_size))
> pr_warn("%s: Unexpected TCEs\n", __func__);
>
> Before that, iommu_table_release_pages(tbl) is supposed to clear the
> bitmap, so this only tests for a tce that is created in this short period.
iommu_table_release_pages() only clears reserved pages - page 0 (just a
protection against NULL DMA pointers) and 32bit MMIO (these should not
be set for 64bit window). The "%s: Unexpected TCEs\n" is what checks for
actual mapped TCEs.
>> Since we are replacing a table for a device which is still in the
>> system, we should not try messing with its DMA if it already has
>> mappings so the warning should become an error preventing DDW. It is
>> rather hard to trigger in practice but I could hack a driver to ask for
>> 32bit DMA mask first, map few pages and then ask for 64bit DMA mask, it
>> is not illegal, I think. So this needs a new helper - "bool
>> iommu_table_in_use(tbl)" - to use in enable_ddw(). Or I am overthinking
>> this?... Thanks,
>
> As of today, there seems to be nothing like that happening in the
> driver I am testing.
> I spoke to Brian King on slack, and he mentioned that at the point DDW
> is created there should be no allocations in place.
Correct, there should not be. But it is also not a huge effort to fall
back if there are.
> But I suppose some driver could try to do this.
>
> Maybe a better approach would be removing the mapping only if the
> default window is removed (at the end of enable_ddw, as an else to
> resetting the default DMA window), and having a way to add more
> mappings to those pools. But this last part doesn't look so simple, and
> it would be better to understand if it's necessary investing work in
> this.
>
> What do you think?
Add iommu_table_in_use(tbl) and fail DDW if that says "yes".
--
Alexey
^ permalink raw reply
* [PATCHv3 1/2] powerpc/pseries: group lmb operation and memblock's
From: Pingfan Liu @ 2020-07-22 1:52 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Nathan Lynch, kexec, Hari Bathini, Pingfan Liu
This patch prepares for the incoming patch which swaps the order of KOBJ_
uevent and dt's updating.
It has no functional effect, just groups lmb operation and memblock's in
order to insert dt updating operation easily, and makes it easier to
review.
Signed-off-by: Pingfan Liu <kernelfans@gmail.com>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Hari Bathini <hbathini@linux.ibm.com>
Cc: Nathan Lynch <nathanl@linux.ibm.com>
To: linuxppc-dev@lists.ozlabs.org
Cc: kexec@lists.infradead.org
---
v2 -> v3: rebase onto ppc next-test branch
---
arch/powerpc/platforms/pseries/hotplug-memory.c | 26 ++++++++++++++++---------
1 file changed, 17 insertions(+), 9 deletions(-)
diff --git a/arch/powerpc/platforms/pseries/hotplug-memory.c b/arch/powerpc/platforms/pseries/hotplug-memory.c
index 5d545b7..1a3ac3b 100644
--- a/arch/powerpc/platforms/pseries/hotplug-memory.c
+++ b/arch/powerpc/platforms/pseries/hotplug-memory.c
@@ -355,7 +355,8 @@ static int dlpar_add_lmb(struct drmem_lmb *);
static int dlpar_remove_lmb(struct drmem_lmb *lmb)
{
unsigned long block_sz;
- int rc;
+ phys_addr_t base_addr;
+ int rc, nid;
if (!lmb_is_removable(lmb))
return -EINVAL;
@@ -364,17 +365,19 @@ static int dlpar_remove_lmb(struct drmem_lmb *lmb)
if (rc)
return rc;
+ base_addr = lmb->base_addr;
+ nid = lmb->nid;
block_sz = pseries_memory_block_size();
- __remove_memory(lmb->nid, lmb->base_addr, block_sz);
-
- /* Update memory regions for memory remove */
- memblock_remove(lmb->base_addr, block_sz);
-
invalidate_lmb_associativity_index(lmb);
lmb_clear_nid(lmb);
lmb->flags &= ~DRCONF_MEM_ASSIGNED;
+ __remove_memory(nid, base_addr, block_sz);
+
+ /* Update memory regions for memory remove */
+ memblock_remove(base_addr, block_sz);
+
return 0;
}
@@ -603,6 +606,8 @@ static int dlpar_add_lmb(struct drmem_lmb *lmb)
}
lmb_set_nid(lmb);
+ lmb->flags |= DRCONF_MEM_ASSIGNED;
+
block_sz = memory_block_size_bytes();
/* Add the memory */
@@ -614,11 +619,14 @@ static int dlpar_add_lmb(struct drmem_lmb *lmb)
rc = dlpar_online_lmb(lmb);
if (rc) {
- __remove_memory(lmb->nid, lmb->base_addr, block_sz);
+ int nid = lmb->nid;
+ phys_addr_t base_addr = lmb->base_addr;
+
invalidate_lmb_associativity_index(lmb);
lmb_clear_nid(lmb);
- } else {
- lmb->flags |= DRCONF_MEM_ASSIGNED;
+ lmb->flags &= ~DRCONF_MEM_ASSIGNED;
+
+ __remove_memory(nid, base_addr, block_sz);
}
return rc;
--
2.7.5
^ permalink raw reply related
* [PATCHv3 2/2] powerpc/pseries: update device tree before ejecting hotplug uevents
From: Pingfan Liu @ 2020-07-22 1:52 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Nathan Lynch, kexec, Hari Bathini, Pingfan Liu
In-Reply-To: <1595382730-10565-1-git-send-email-kernelfans@gmail.com>
A bug is observed on pseries by taking the following steps on rhel:
-1. drmgr -c mem -r -q 5
-2. echo c > /proc/sysrq-trigger
And then, the failure looks like:
kdump: saving to /sysroot//var/crash/127.0.0.1-2020-01-16-02:06:14/
kdump: saving vmcore-dmesg.txt
kdump: saving vmcore-dmesg.txt complete
kdump: saving vmcore
Checking for memory holes : [ 0.0 %] / Checking for memory holes : [100.0 %] | Excluding unnecessary pages : [100.0 %] \ Copying data : [ 0.3 %] - eta: 38s[ 44.337636] hash-mmu: mm: Hashing failure ! EA=0x7fffba400000 access=0x8000000000000004 current=makedumpfile
[ 44.337663] hash-mmu: trap=0x300 vsid=0x13a109c ssize=1 base psize=2 psize 2 pte=0xc000000050000504
[ 44.337677] hash-mmu: mm: Hashing failure ! EA=0x7fffba400000 access=0x8000000000000004 current=makedumpfile
[ 44.337692] hash-mmu: trap=0x300 vsid=0x13a109c ssize=1 base psize=2 psize 2 pte=0xc000000050000504
[ 44.337708] makedumpfile[469]: unhandled signal 7 at 00007fffba400000 nip 00007fffbbc4d7fc lr 000000011356ca3c code 2
[ 44.338548] Core dump to |/bin/false pipe failed
/lib/kdump-lib-initramfs.sh: line 98: 469 Bus error $CORE_COLLECTOR /proc/vmcore $_mp/$KDUMP_PATH/$HOST_IP-$DATEDIR/vmcore-incomplete
kdump: saving vmcore failed
* Root cause *
After analyzing, it turns out that in the current implementation,
when hot-removing lmb, the KOBJ_REMOVE event ejects before the dt updating as
the code __remove_memory() comes before drmem_update_dt().
So in kdump kernel, when read_from_oldmem() resorts to
pSeries_lpar_hpte_insert() to install hpte, but fails with -2 due to
non-exist pfn. And finally, low_hash_fault() raise SIGBUS to process, as it
can be observed "Bus error"
From a viewpoint of listener and publisher, the publisher notifies the
listener before data is ready. This introduces a problem where udev
launches kexec-tools (due to KOBJ_REMOVE) and loads a stale dt before
updating. And in capture kernel, makedumpfile will access the memory based
on the stale dt info, and hit a SIGBUS error due to an un-existed lmb.
* Fix *
In order to fix this issue, update dt before __remove_memory(), and
accordingly the same rule in hot-add path.
This will introduce extra dt updating payload for each involved lmb when hotplug.
But it should be fine since drmem_update_dt() is memory based operation and
hotplug is not a hot path.
Signed-off-by: Pingfan Liu <kernelfans@gmail.com>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Hari Bathini <hbathini@linux.ibm.com>
Cc: Nathan Lynch <nathanl@linux.ibm.com>
To: linuxppc-dev@lists.ozlabs.org
Cc: kexec@lists.infradead.org
---
v2 -> v3: rebase onto ppc next-test branch
---
arch/powerpc/platforms/pseries/hotplug-memory.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/platforms/pseries/hotplug-memory.c b/arch/powerpc/platforms/pseries/hotplug-memory.c
index 1a3ac3b..def8cb3f 100644
--- a/arch/powerpc/platforms/pseries/hotplug-memory.c
+++ b/arch/powerpc/platforms/pseries/hotplug-memory.c
@@ -372,6 +372,7 @@ static int dlpar_remove_lmb(struct drmem_lmb *lmb)
invalidate_lmb_associativity_index(lmb);
lmb_clear_nid(lmb);
lmb->flags &= ~DRCONF_MEM_ASSIGNED;
+ drmem_update_dt();
__remove_memory(nid, base_addr, block_sz);
@@ -607,6 +608,7 @@ static int dlpar_add_lmb(struct drmem_lmb *lmb)
lmb_set_nid(lmb);
lmb->flags |= DRCONF_MEM_ASSIGNED;
+ drmem_update_dt();
block_sz = memory_block_size_bytes();
@@ -625,6 +627,7 @@ static int dlpar_add_lmb(struct drmem_lmb *lmb)
invalidate_lmb_associativity_index(lmb);
lmb_clear_nid(lmb);
lmb->flags &= ~DRCONF_MEM_ASSIGNED;
+ drmem_update_dt();
__remove_memory(nid, base_addr, block_sz);
}
@@ -877,9 +880,6 @@ int dlpar_memory(struct pseries_hp_errorlog *hp_elog)
break;
}
- if (!rc)
- rc = drmem_update_dt();
-
unlock_device_hotplug();
return rc;
}
--
2.7.5
^ permalink raw reply related
* Re: [RFC PATCH] powerpc/pseries/svm: capture instruction faulting on MMIO access, in sprg0 register
From: Michael Ellerman @ 2020-07-22 2:06 UTC (permalink / raw)
To: Ram Pai, kvm-ppc, linuxppc-dev
Cc: sukadev, aik, linuxram, bharata, sathnaga, ldufour, bauerman,
david
In-Reply-To: <1594888333-9370-1-git-send-email-linuxram@us.ibm.com>
Ram Pai <linuxram@us.ibm.com> writes:
> An instruction accessing a mmio address, generates a HDSI fault. This fault is
> appropriately handled by the Hypervisor. However in the case of secureVMs, the
> fault is delivered to the ultravisor.
>
> Unfortunately the Ultravisor has no correct-way to fetch the faulting
> instruction. The PEF architecture does not allow Ultravisor to enable MMU
> translation. Walking the two level page table to read the instruction can race
> with other vcpus modifying the SVM's process scoped page table.
You're trying to read the guest's kernel text IIUC, that mapping should
be stable. Possibly permissions on it could change over time, but the
virtual -> real mapping should not.
> This problem can be correctly solved with some help from the kernel.
>
> Capture the faulting instruction in SPRG0 register, before executing the
> faulting instruction. This enables the ultravisor to easily procure the
> faulting instruction and emulate it.
This is not something I'm going to merge. Sorry.
cheers
^ permalink raw reply
* Re: [PATCH v5 1/4] riscv: Move kernel mapping to vmalloc zone
From: Benjamin Herrenschmidt @ 2020-07-22 2:21 UTC (permalink / raw)
To: Palmer Dabbelt
Cc: aou, alex, linux-mm, Anup Patel, linux-kernel, Atish Patra,
paulus, zong.li, Paul Walmsley, linux-riscv, linuxppc-dev
In-Reply-To: <mhng-cd9a74ea-2edf-47e4-aade-b090f1a069f1@palmerdabbelt-glaptop1>
On Tue, 2020-07-21 at 16:48 -0700, Palmer Dabbelt wrote:
> > Why ? Branch distance limits ? You can't use trampolines ?
>
> Nothing fundamental, it's just that we don't have a large code model in the C
> compiler. As a result all the global symbols are resolved as 32-bit
> PC-relative accesses. We could fix this with a fast large code model, but then
> the kernel would need to relax global symbol references in modules and we don't
> even do that for the simple code models we have now. FWIW, some of the
> proposed large code models are essentially just split-PLT/GOT and therefor
> don't require relaxation, but at that point we're essentially PIC until we
> have more that 2GiB of kernel text -- and even then, we keep all the
> performance issues.
My memory might be out of date but I *think* we do it on powerpc
without going to a large code model, but just having the in-kernel
linker insert trampolines.
Cheers,
Ben.
^ permalink raw reply
* Re: [RFC PATCH] powerpc/pseries/svm: capture instruction faulting on MMIO access, in sprg0 register
From: Benjamin Herrenschmidt @ 2020-07-22 2:23 UTC (permalink / raw)
To: Michael Ellerman, Ram Pai, kvm-ppc, linuxppc-dev
Cc: sukadev, aik, bharata, sathnaga, ldufour, bauerman, david
In-Reply-To: <875zags3qp.fsf@mpe.ellerman.id.au>
On Wed, 2020-07-22 at 12:06 +1000, Michael Ellerman wrote:
> Ram Pai <linuxram@us.ibm.com> writes:
> > An instruction accessing a mmio address, generates a HDSI fault. This fault is
> > appropriately handled by the Hypervisor. However in the case of secureVMs, the
> > fault is delivered to the ultravisor.
> >
> > Unfortunately the Ultravisor has no correct-way to fetch the faulting
> > instruction. The PEF architecture does not allow Ultravisor to enable MMU
> > translation. Walking the two level page table to read the instruction can race
> > with other vcpus modifying the SVM's process scoped page table.
>
> You're trying to read the guest's kernel text IIUC, that mapping should
> be stable. Possibly permissions on it could change over time, but the
> virtual -> real mapping should not.
This will of course no longer work if userspace tries to access MMIO
but as long as you are ok limiting MMIO usage to the kernel, that's one
way.
iirc MMIO emulation in KVM on powerpc already has that race because of
HW TLB inval broadcast and it hasn't hurt anybody ... so far.
> > This problem can be correctly solved with some help from the kernel.
> >
> > Capture the faulting instruction in SPRG0 register, before executing the
> > faulting instruction. This enables the ultravisor to easily procure the
> > faulting instruction and emulate it.
>
> This is not something I'm going to merge. Sorry.
Another approach is to have the guest explicitly switch to using UV
calls for MMIO.
Cheers,
Ben.
^ permalink raw reply
* [PATCH] powerpc/40x: Fix assembler warning about r0
From: Michael Ellerman @ 2020-07-22 2:24 UTC (permalink / raw)
To: linuxppc-dev
The assembler says:
arch/powerpc/kernel/head_40x.S:623: Warning: invalid register expression
It's objecting to the use of r0 as the RA argument. That's because
when RA = 0 the literal value 0 is used, rather than the content of
r0, making the use of r0 in the source potentially confusing.
Fix it to use a literal 0, the generated code is identical.
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
arch/powerpc/kernel/head_40x.S | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/kernel/head_40x.S b/arch/powerpc/kernel/head_40x.S
index 926bfa73586a..5b282d9965a5 100644
--- a/arch/powerpc/kernel/head_40x.S
+++ b/arch/powerpc/kernel/head_40x.S
@@ -620,7 +620,7 @@ _ENTRY(saved_ksp_limit)
ori r6, r6, swapper_pg_dir@l
lis r5, abatron_pteptrs@h
ori r5, r5, abatron_pteptrs@l
- stw r5, 0xf0(r0) /* Must match your Abatron config file */
+ stw r5, 0xf0(0) /* Must match your Abatron config file */
tophys(r5,r5)
stw r6, 0(r5)
--
2.25.1
^ permalink raw reply related
* Re: [PATCH 1/2] powerpc/numa: Limit possible nodes to within num_possible_nodes
From: Nathan Lynch @ 2020-07-22 3:14 UTC (permalink / raw)
To: Srikar Dronamraju, Michael Ellerman
Cc: Tyrel Datwyler, linuxppc-dev, Srikar Dronamraju, Michael Ellerman
In-Reply-To: <20200715120534.3673-1-srikar@linux.vnet.ibm.com>
Srikar Dronamraju <srikar@linux.vnet.ibm.com> writes:
> MAX_NUMNODES is a theoretical maximum number of nodes thats is supported
> by the kernel. Device tree properties exposes the number of possible
> nodes on the current platform. The kernel would detected this and would
> use it for most of its resource allocations. If the platform now
> increases the nodes to over what was already exposed, then it may lead
> to inconsistencies. Hence limit it to the already exposed nodes.
>
> Suggested-by: Nathan Lynch <nathanl@linux.ibm.com>
> Cc: linuxppc-dev <linuxppc-dev@lists.ozlabs.org>
> Cc: Michael Ellerman <michaele@au1.ibm.com>
> Cc: Nathan Lynch <nathanl@linux.ibm.com>
> Cc: Tyrel Datwyler <tyreld@linux.ibm.com>
> Signed-off-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
> ---
> arch/powerpc/mm/numa.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c
> index 8ec7ff05ae47..a2c5fe0d0cad 100644
> --- a/arch/powerpc/mm/numa.c
> +++ b/arch/powerpc/mm/numa.c
> @@ -221,7 +221,7 @@ static void initialize_distance_lookup_table(int nid,
> }
> }
>
> -/* Returns nid in the range [0..MAX_NUMNODES-1], or -1 if no useful numa
> +/* Returns nid in the range [0..num_possible_nodes()-1], or -1 if no useful numa
> * info is found.
> */
> static int associativity_to_nid(const __be32 *associativity)
> @@ -235,7 +235,7 @@ static int associativity_to_nid(const __be32 *associativity)
> nid = of_read_number(&associativity[min_common_depth], 1);
>
> /* POWER4 LPAR uses 0xffff as invalid node */
> - if (nid == 0xffff || nid >= MAX_NUMNODES)
> + if (nid == 0xffff || nid >= num_possible_nodes())
> nid = NUMA_NO_NODE;
>
> if (nid > 0 &&
> @@ -448,7 +448,7 @@ static int of_drconf_to_nid_single(struct drmem_lmb *lmb)
> index = lmb->aa_index * aa.array_sz + min_common_depth - 1;
> nid = of_read_number(&aa.arrays[index], 1);
>
> - if (nid == 0xffff || nid >= MAX_NUMNODES)
> + if (nid == 0xffff || nid >= num_possible_nodes())
> nid = default_nid;
>
> if (nid > 0) {
Yes, looks fine to me.
Reviewed-by: Nathan Lynch <nathanl@linux.ibm.com>
^ permalink raw reply
* Re: [v3 02/15] KVM: PPC: Book3S HV: Cleanup updates for kvm vcpu MMCR
From: Athira Rajeev @ 2020-07-22 2:09 UTC (permalink / raw)
To: Paul Mackerras
Cc: ego, Michael Neuling, maddy, kvm, kvm-ppc, svaidyan, acme, jolsa,
linuxppc-dev
In-Reply-To: <20200721035420.GA3819606@thinks.paulus.ozlabs.org>
[-- Attachment #1: Type: text/plain, Size: 3591 bytes --]
> On 21-Jul-2020, at 9:24 AM, Paul Mackerras <paulus@ozlabs.org> wrote:
>
> On Fri, Jul 17, 2020 at 10:38:14AM -0400, Athira Rajeev wrote:
>> Currently `kvm_vcpu_arch` stores all Monitor Mode Control registers
>> in a flat array in order: mmcr0, mmcr1, mmcra, mmcr2, mmcrs
>> Split this to give mmcra and mmcrs its own entries in vcpu and
>> use a flat array for mmcr0 to mmcr2. This patch implements this
>> cleanup to make code easier to read.
>
> Changing the way KVM stores these values internally is fine, but
> changing the user ABI is not. This part:
>
>> diff --git a/arch/powerpc/include/uapi/asm/kvm.h b/arch/powerpc/include/uapi/asm/kvm.h
>> index 264e266..e55d847 100644
>> --- a/arch/powerpc/include/uapi/asm/kvm.h
>> +++ b/arch/powerpc/include/uapi/asm/kvm.h
>> @@ -510,8 +510,8 @@ struct kvm_ppc_cpu_char {
>>
>> #define KVM_REG_PPC_MMCR0 (KVM_REG_PPC | KVM_REG_SIZE_U64 | 0x10)
>> #define KVM_REG_PPC_MMCR1 (KVM_REG_PPC | KVM_REG_SIZE_U64 | 0x11)
>> -#define KVM_REG_PPC_MMCRA (KVM_REG_PPC | KVM_REG_SIZE_U64 | 0x12)
>> -#define KVM_REG_PPC_MMCR2 (KVM_REG_PPC | KVM_REG_SIZE_U64 | 0x13)
>> +#define KVM_REG_PPC_MMCR2 (KVM_REG_PPC | KVM_REG_SIZE_U64 | 0x12)
>> +#define KVM_REG_PPC_MMCRA (KVM_REG_PPC | KVM_REG_SIZE_U64 | 0x13)
>
> means that existing userspace programs that used to work would now be
> broken. That is not acceptable (breaking the user ABI is only ever
> acceptable with a very compelling reason). So NAK to this part of the
> patch.
Hi Paul
Thanks for checking the patch. I understood your point on user ABI breakage that this particular change can cause.
I will retain original KVM_REG_PPC_MMCRA and KVM_REG_PPC_MMCR2 order in `kvm.h`
And with that, additionally I will need below change ( on top of current patch ) for my clean up updates for kvm cpu MMCR to work,
Because now mmcra and mmcrs will have its own entries in vcpu and is not part of the mmcr[] array
Please suggest if this looks good
Thanks
Athira
—
diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index 3f90eee261fc..b10bb404f0d5 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -1679,10 +1679,13 @@ static int kvmppc_get_one_reg_hv(struct kvm_vcpu *vcpu, u64 id,
case KVM_REG_PPC_UAMOR:
*val = get_reg_val(id, vcpu->arch.uamor);
break;
- case KVM_REG_PPC_MMCR0 ... KVM_REG_PPC_MMCR2:
+ case KVM_REG_PPC_MMCR0 ... KVM_REG_PPC_MMCR1:
i = id - KVM_REG_PPC_MMCR0;
*val = get_reg_val(id, vcpu->arch.mmcr[i]);
break;
+ case KVM_REG_PPC_MMCR2:
+ *val = get_reg_val(id, vcpu->arch.mmcr[2]);
+ break;
case KVM_REG_PPC_MMCRA:
*val = get_reg_val(id, vcpu->arch.mmcra);
break;
@@ -1906,10 +1909,13 @@ static int kvmppc_set_one_reg_hv(struct kvm_vcpu *vcpu, u64 id,
case KVM_REG_PPC_UAMOR:
vcpu->arch.uamor = set_reg_val(id, *val);
break;
- case KVM_REG_PPC_MMCR0 ... KVM_REG_PPC_MMCR2:
+ case KVM_REG_PPC_MMCR0 ... KVM_REG_PPC_MMCR1:
i = id - KVM_REG_PPC_MMCR0;
vcpu->arch.mmcr[i] = set_reg_val(id, *val);
break;
+ case KVM_REG_PPC_MMCR2:
+ vcpu->arch.mmcr[2] = set_reg_val(id, *val);
+ break;
case KVM_REG_PPC_MMCRA:
vcpu->arch.mmcra = set_reg_val(id, *val);
break;
—
>
> Regards,
> Paul.
[-- Attachment #2: Type: text/html, Size: 11048 bytes --]
^ permalink raw reply related
* Re: [v3 01/15] powerpc/perf: Update cpu_hw_event to use `struct` for storing MMCR registers
From: Athira Rajeev @ 2020-07-22 2:15 UTC (permalink / raw)
To: Jordan Niethe
Cc: Gautham R Shenoy, Michael Neuling, maddy, kvm, kvm-ppc, svaidyan,
acme, jolsa, linuxppc-dev
In-Reply-To: <CACzsE9rnE_z-uEw2tyvdFDVzkOeBKJoZiTHD9fSRm9d8fVBr5w@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 25536 bytes --]
> On 21-Jul-2020, at 9:12 AM, Jordan Niethe <jniethe5@gmail.com> wrote:
>
> On Sat, Jul 18, 2020 at 12:48 AM Athira Rajeev
> <atrajeev@linux.vnet.ibm.com <mailto:atrajeev@linux.vnet.ibm.com>> wrote:
>>
>> core-book3s currently uses array to store the MMCR registers as part
>> of per-cpu `cpu_hw_events`. This patch does a clean up to use `struct`
>> to store mmcr regs instead of array. This will make code easier to read
>> and reduces chance of any subtle bug that may come in the future, say
>> when new registers are added. Patch updates all relevant code that was
>> using MMCR array ( cpuhw->mmcr[x]) to use newly introduced `struct`.
>> This includes the PMU driver code for supported platforms (power5
>> to power9) and ISA macros for counter support functions.
>>
>> Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
>> ---
>> arch/powerpc/include/asm/perf_event_server.h | 10 ++++--
>> arch/powerpc/perf/core-book3s.c | 53 +++++++++++++---------------
>> arch/powerpc/perf/isa207-common.c | 20 +++++------
>> arch/powerpc/perf/isa207-common.h | 4 +--
>> arch/powerpc/perf/mpc7450-pmu.c | 21 +++++++----
>> arch/powerpc/perf/power5+-pmu.c | 17 ++++-----
>> arch/powerpc/perf/power5-pmu.c | 17 ++++-----
>> arch/powerpc/perf/power6-pmu.c | 16 ++++-----
>> arch/powerpc/perf/power7-pmu.c | 17 ++++-----
>> arch/powerpc/perf/ppc970-pmu.c | 24 ++++++-------
>> 10 files changed, 105 insertions(+), 94 deletions(-)
>>
>> diff --git a/arch/powerpc/include/asm/perf_event_server.h b/arch/powerpc/include/asm/perf_event_server.h
>> index 3e9703f..f9a3668 100644
>> --- a/arch/powerpc/include/asm/perf_event_server.h
>> +++ b/arch/powerpc/include/asm/perf_event_server.h
>> @@ -17,6 +17,12 @@
>>
>> struct perf_event;
>>
>> +struct mmcr_regs {
>> + unsigned long mmcr0;
>> + unsigned long mmcr1;
>> + unsigned long mmcr2;
>> + unsigned long mmcra;
>> +};
>> /*
>> * This struct provides the constants and functions needed to
>> * describe the PMU on a particular POWER-family CPU.
>> @@ -28,7 +34,7 @@ struct power_pmu {
>> unsigned long add_fields;
>> unsigned long test_adder;
>> int (*compute_mmcr)(u64 events[], int n_ev,
>> - unsigned int hwc[], unsigned long mmcr[],
>> + unsigned int hwc[], struct mmcr_regs *mmcr,
>> struct perf_event *pevents[]);
>> int (*get_constraint)(u64 event_id, unsigned long *mskp,
>> unsigned long *valp);
>> @@ -41,7 +47,7 @@ struct power_pmu {
>> unsigned long group_constraint_val;
>> u64 (*bhrb_filter_map)(u64 branch_sample_type);
>> void (*config_bhrb)(u64 pmu_bhrb_filter);
>> - void (*disable_pmc)(unsigned int pmc, unsigned long mmcr[]);
>> + void (*disable_pmc)(unsigned int pmc, struct mmcr_regs *mmcr);
>> int (*limited_pmc_event)(u64 event_id);
>> u32 flags;
>> const struct attribute_group **attr_groups;
>> diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
>> index cd6a742..18b1b6a 100644
>> --- a/arch/powerpc/perf/core-book3s.c
>> +++ b/arch/powerpc/perf/core-book3s.c
>> @@ -37,12 +37,7 @@ struct cpu_hw_events {
>> struct perf_event *event[MAX_HWEVENTS];
>> u64 events[MAX_HWEVENTS];
>> unsigned int flags[MAX_HWEVENTS];
>> - /*
>> - * The order of the MMCR array is:
>> - * - 64-bit, MMCR0, MMCR1, MMCRA, MMCR2
>> - * - 32-bit, MMCR0, MMCR1, MMCR2
>> - */
>> - unsigned long mmcr[4];
>> + struct mmcr_regs mmcr;
>> struct perf_event *limited_counter[MAX_LIMITED_HWCOUNTERS];
>> u8 limited_hwidx[MAX_LIMITED_HWCOUNTERS];
>> u64 alternatives[MAX_HWEVENTS][MAX_EVENT_ALTERNATIVES];
>> @@ -121,7 +116,7 @@ static void ebb_event_add(struct perf_event *event) { }
>> static void ebb_switch_out(unsigned long mmcr0) { }
>> static unsigned long ebb_switch_in(bool ebb, struct cpu_hw_events *cpuhw)
>> {
>> - return cpuhw->mmcr[0];
>> + return cpuhw->mmcr.mmcr0;
>> }
>>
>> static inline void power_pmu_bhrb_enable(struct perf_event *event) {}
>> @@ -590,7 +585,7 @@ static void ebb_switch_out(unsigned long mmcr0)
>>
>> static unsigned long ebb_switch_in(bool ebb, struct cpu_hw_events *cpuhw)
>> {
>> - unsigned long mmcr0 = cpuhw->mmcr[0];
>> + unsigned long mmcr0 = cpuhw->mmcr.mmcr0;
>>
>> if (!ebb)
>> goto out;
>> @@ -624,7 +619,7 @@ static unsigned long ebb_switch_in(bool ebb, struct cpu_hw_events *cpuhw)
>> * unfreeze counters, it should not set exclude_xxx in its events and
>> * instead manage the MMCR2 entirely by itself.
>> */
>> - mtspr(SPRN_MMCR2, cpuhw->mmcr[3] | current->thread.mmcr2);
>> + mtspr(SPRN_MMCR2, cpuhw->mmcr.mmcr2 | current->thread.mmcr2);
>> out:
>> return mmcr0;
>> }
>> @@ -1232,9 +1227,9 @@ static void power_pmu_disable(struct pmu *pmu)
>> /*
>> * Disable instruction sampling if it was enabled
>> */
>> - if (cpuhw->mmcr[2] & MMCRA_SAMPLE_ENABLE) {
>> + if (cpuhw->mmcr.mmcra & MMCRA_SAMPLE_ENABLE) {
>> mtspr(SPRN_MMCRA,
>> - cpuhw->mmcr[2] & ~MMCRA_SAMPLE_ENABLE);
>> + cpuhw->mmcr.mmcra & ~MMCRA_SAMPLE_ENABLE);
>> mb();
>> isync();
>> }
>> @@ -1308,18 +1303,18 @@ static void power_pmu_enable(struct pmu *pmu)
>> * (possibly updated for removal of events).
>> */
>> if (!cpuhw->n_added) {
>> - mtspr(SPRN_MMCRA, cpuhw->mmcr[2] & ~MMCRA_SAMPLE_ENABLE);
>> - mtspr(SPRN_MMCR1, cpuhw->mmcr[1]);
>> + mtspr(SPRN_MMCRA, cpuhw->mmcr.mmcra & ~MMCRA_SAMPLE_ENABLE);
>> + mtspr(SPRN_MMCR1, cpuhw->mmcr.mmcr1);
>> goto out_enable;
>> }
>>
>> /*
>> * Clear all MMCR settings and recompute them for the new set of events.
>> */
>> - memset(cpuhw->mmcr, 0, sizeof(cpuhw->mmcr));
>> + memset(&cpuhw->mmcr, 0, sizeof(cpuhw->mmcr));
>>
>> if (ppmu->compute_mmcr(cpuhw->events, cpuhw->n_events, hwc_index,
>> - cpuhw->mmcr, cpuhw->event)) {
>> + &cpuhw->mmcr, cpuhw->event)) {
>> /* shouldn't ever get here */
>> printk(KERN_ERR "oops compute_mmcr failed\n");
>> goto out;
>> @@ -1333,11 +1328,11 @@ static void power_pmu_enable(struct pmu *pmu)
>> */
>> event = cpuhw->event[0];
>> if (event->attr.exclude_user)
>> - cpuhw->mmcr[0] |= MMCR0_FCP;
>> + cpuhw->mmcr.mmcr0 |= MMCR0_FCP;
>> if (event->attr.exclude_kernel)
>> - cpuhw->mmcr[0] |= freeze_events_kernel;
>> + cpuhw->mmcr.mmcr0 |= freeze_events_kernel;
>> if (event->attr.exclude_hv)
>> - cpuhw->mmcr[0] |= MMCR0_FCHV;
>> + cpuhw->mmcr.mmcr0 |= MMCR0_FCHV;
>> }
>>
>> /*
>> @@ -1346,12 +1341,12 @@ static void power_pmu_enable(struct pmu *pmu)
>> * Then unfreeze the events.
>> */
>> ppc_set_pmu_inuse(1);
>> - mtspr(SPRN_MMCRA, cpuhw->mmcr[2] & ~MMCRA_SAMPLE_ENABLE);
>> - mtspr(SPRN_MMCR1, cpuhw->mmcr[1]);
>> - mtspr(SPRN_MMCR0, (cpuhw->mmcr[0] & ~(MMCR0_PMC1CE | MMCR0_PMCjCE))
>> + mtspr(SPRN_MMCRA, cpuhw->mmcr.mmcra & ~MMCRA_SAMPLE_ENABLE);
>> + mtspr(SPRN_MMCR1, cpuhw->mmcr.mmcr1);
>> + mtspr(SPRN_MMCR0, (cpuhw->mmcr.mmcr0 & ~(MMCR0_PMC1CE | MMCR0_PMCjCE))
>> | MMCR0_FC);
>> if (ppmu->flags & PPMU_ARCH_207S)
>> - mtspr(SPRN_MMCR2, cpuhw->mmcr[3]);
>> + mtspr(SPRN_MMCR2, cpuhw->mmcr.mmcr2);
>>
>> /*
>> * Read off any pre-existing events that need to move
>> @@ -1402,7 +1397,7 @@ static void power_pmu_enable(struct pmu *pmu)
>> perf_event_update_userpage(event);
>> }
>> cpuhw->n_limited = n_lim;
>> - cpuhw->mmcr[0] |= MMCR0_PMXE | MMCR0_FCECE;
>> + cpuhw->mmcr.mmcr0 |= MMCR0_PMXE | MMCR0_FCECE;
>>
>> out_enable:
>> pmao_restore_workaround(ebb);
>> @@ -1418,9 +1413,9 @@ static void power_pmu_enable(struct pmu *pmu)
>> /*
>> * Enable instruction sampling if necessary
>> */
>> - if (cpuhw->mmcr[2] & MMCRA_SAMPLE_ENABLE) {
>> + if (cpuhw->mmcr.mmcra & MMCRA_SAMPLE_ENABLE) {
>> mb();
>> - mtspr(SPRN_MMCRA, cpuhw->mmcr[2]);
>> + mtspr(SPRN_MMCRA, cpuhw->mmcr.mmcra);
>> }
>>
>> out:
>> @@ -1550,7 +1545,7 @@ static void power_pmu_del(struct perf_event *event, int ef_flags)
>> cpuhw->flags[i-1] = cpuhw->flags[i];
>> }
>> --cpuhw->n_events;
>> - ppmu->disable_pmc(event->hw.idx - 1, cpuhw->mmcr);
>> + ppmu->disable_pmc(event->hw.idx - 1, &cpuhw->mmcr);
>> if (event->hw.idx) {
>> write_pmc(event->hw.idx, 0);
>> event->hw.idx = 0;
>> @@ -1571,7 +1566,7 @@ static void power_pmu_del(struct perf_event *event, int ef_flags)
>> }
>> if (cpuhw->n_events == 0) {
>> /* disable exceptions if no events are running */
>> - cpuhw->mmcr[0] &= ~(MMCR0_PMXE | MMCR0_FCECE);
>> + cpuhw->mmcr.mmcr0 &= ~(MMCR0_PMXE | MMCR0_FCECE);
>> }
>>
>> if (has_branch_stack(event))
>> @@ -2240,7 +2235,7 @@ static void __perf_event_interrupt(struct pt_regs *regs)
>> * XXX might want to use MSR.PM to keep the events frozen until
>> * we get back out of this interrupt.
>> */
>> - write_mmcr0(cpuhw, cpuhw->mmcr[0]);
>> + write_mmcr0(cpuhw, cpuhw->mmcr.mmcr0);
>>
>> if (nmi)
>> nmi_exit();
>> @@ -2262,7 +2257,7 @@ static int power_pmu_prepare_cpu(unsigned int cpu)
>>
>> if (ppmu) {
>> memset(cpuhw, 0, sizeof(*cpuhw));
>> - cpuhw->mmcr[0] = MMCR0_FC;
>> + cpuhw->mmcr.mmcr0 = MMCR0_FC;
>> }
>> return 0;
>> }
>> diff --git a/arch/powerpc/perf/isa207-common.c b/arch/powerpc/perf/isa207-common.c
>> index 4c86da5..2fe63f2 100644
>> --- a/arch/powerpc/perf/isa207-common.c
>> +++ b/arch/powerpc/perf/isa207-common.c
>> @@ -363,7 +363,7 @@ int isa207_get_constraint(u64 event, unsigned long *maskp, unsigned long *valp)
>> }
>>
>> int isa207_compute_mmcr(u64 event[], int n_ev,
>> - unsigned int hwc[], unsigned long mmcr[],
>> + unsigned int hwc[], struct mmcr_regs *mmcr,
>> struct perf_event *pevents[])
>> {
>> unsigned long mmcra, mmcr1, mmcr2, unit, combine, psel, cache, val;
>> @@ -464,30 +464,30 @@ int isa207_compute_mmcr(u64 event[], int n_ev,
>> }
>>
>> /* Return MMCRx values */
>> - mmcr[0] = 0;
>> + mmcr->mmcr0 = 0;
>>
>> /* pmc_inuse is 1-based */
>> if (pmc_inuse & 2)
>> - mmcr[0] = MMCR0_PMC1CE;
>> + mmcr->mmcr0 = MMCR0_PMC1CE;
>>
>> if (pmc_inuse & 0x7c)
>> - mmcr[0] |= MMCR0_PMCjCE;
>> + mmcr->mmcr0 |= MMCR0_PMCjCE;
>>
>> /* If we're not using PMC 5 or 6, freeze them */
>> if (!(pmc_inuse & 0x60))
>> - mmcr[0] |= MMCR0_FC56;
>> + mmcr->mmcr0 |= MMCR0_FC56;
>>
>> - mmcr[1] = mmcr1;
>> - mmcr[2] = mmcra;
>> - mmcr[3] = mmcr2;
>> + mmcr->mmcr1 = mmcr1;
>> + mmcr->mmcra = mmcra;
>> + mmcr->mmcr2 = mmcr2;
>>
>> return 0;
>> }
>>
>> -void isa207_disable_pmc(unsigned int pmc, unsigned long mmcr[])
>> +void isa207_disable_pmc(unsigned int pmc, struct mmcr_regs *mmcr)
>> {
>> if (pmc <= 3)
>> - mmcr[1] &= ~(0xffUL << MMCR1_PMCSEL_SHIFT(pmc + 1));
>> + mmcr->mmcr1 &= ~(0xffUL << MMCR1_PMCSEL_SHIFT(pmc + 1));
>> }
>>
>> static int find_alternative(u64 event, const unsigned int ev_alt[][MAX_ALT], int size)
>> diff --git a/arch/powerpc/perf/isa207-common.h b/arch/powerpc/perf/isa207-common.h
>> index 63fd4f3..df968fd 100644
>> --- a/arch/powerpc/perf/isa207-common.h
>> +++ b/arch/powerpc/perf/isa207-common.h
>> @@ -217,9 +217,9 @@
>>
>> int isa207_get_constraint(u64 event, unsigned long *maskp, unsigned long *valp);
>> int isa207_compute_mmcr(u64 event[], int n_ev,
>> - unsigned int hwc[], unsigned long mmcr[],
>> + unsigned int hwc[], struct mmcr_regs *mmcr,
>> struct perf_event *pevents[]);
>> -void isa207_disable_pmc(unsigned int pmc, unsigned long mmcr[]);
>> +void isa207_disable_pmc(unsigned int pmc, struct mmcr_regs *mmcr);
>> int isa207_get_alternatives(u64 event, u64 alt[], int size, unsigned int flags,
>> const unsigned int ev_alt[][MAX_ALT]);
>> void isa207_get_mem_data_src(union perf_mem_data_src *dsrc, u32 flags,
>> diff --git a/arch/powerpc/perf/mpc7450-pmu.c b/arch/powerpc/perf/mpc7450-pmu.c
>> index 4d5ef92..826de25 100644
>> --- a/arch/powerpc/perf/mpc7450-pmu.c
>> +++ b/arch/powerpc/perf/mpc7450-pmu.c
>> @@ -257,7 +257,7 @@ static int mpc7450_get_alternatives(u64 event, unsigned int flags, u64 alt[])
>> * Compute MMCR0/1/2 values for a set of events.
>> */
>> static int mpc7450_compute_mmcr(u64 event[], int n_ev, unsigned int hwc[],
>> - unsigned long mmcr[],
>> + struct mmcr_regs *mmcr,
>> struct perf_event *pevents[])
>> {
>> u8 event_index[N_CLASSES][N_COUNTER];
>> @@ -321,9 +321,16 @@ static int mpc7450_compute_mmcr(u64 event[], int n_ev, unsigned int hwc[],
>> mmcr0 |= MMCR0_PMCnCE;
>>
>> /* Return MMCRx values */
>> - mmcr[0] = mmcr0;
>> - mmcr[1] = mmcr1;
>> - mmcr[2] = mmcr2;
>> + mmcr->mmcr0 = mmcr0;
>> + mmcr->mmcr1 = mmcr1;
>> + mmcr->mmcr2 = mmcr2;
> Will this mmcr->mmcr2 be used anywere, or will it always use mmcr->mmcra?
Hi Jordan
We will be actually using mmcr->mmcra in the core-book3s for mpc7450-pmu.c
I have still assigned mmcr->mmcr2 so that it will work for any future changes if made in corebook
>> + /*
>> + * 32-bit doesn't have an MMCRA and uses SPRN_MMCR2 to define
>> + * SPRN_MMCRA. So assign mmcra of cpu_hw_events with `mmcr2`
>> + * value to ensure that any write to this SPRN_MMCRA will
>> + * use mmcr2 value.
>> + */
> Something like this comment would probably be useful to near the struct mmcr.
Sure, I will add this information
Thanks for your suggestion
Athira
>> + mmcr->mmcra = mmcr2;
>> return 0;
>> }
>>
>> @@ -331,12 +338,12 @@ static int mpc7450_compute_mmcr(u64 event[], int n_ev, unsigned int hwc[],
>> * Disable counting by a PMC.
>> * Note that the pmc argument is 0-based here, not 1-based.
>> */
>> -static void mpc7450_disable_pmc(unsigned int pmc, unsigned long mmcr[])
>> +static void mpc7450_disable_pmc(unsigned int pmc, struct mmcr_regs *mmcr)
>> {
>> if (pmc <= 1)
>> - mmcr[0] &= ~(pmcsel_mask[pmc] << pmcsel_shift[pmc]);
>> + mmcr->mmcr0 &= ~(pmcsel_mask[pmc] << pmcsel_shift[pmc]);
>> else
>> - mmcr[1] &= ~(pmcsel_mask[pmc] << pmcsel_shift[pmc]);
>> + mmcr->mmcr1 &= ~(pmcsel_mask[pmc] << pmcsel_shift[pmc]);
>> }
>>
>> static int mpc7450_generic_events[] = {
>> diff --git a/arch/powerpc/perf/power5+-pmu.c b/arch/powerpc/perf/power5+-pmu.c
>> index f857454..5f0821e 100644
>> --- a/arch/powerpc/perf/power5+-pmu.c
>> +++ b/arch/powerpc/perf/power5+-pmu.c
>> @@ -448,7 +448,8 @@ static int power5p_marked_instr_event(u64 event)
>> }
>>
>> static int power5p_compute_mmcr(u64 event[], int n_ev,
>> - unsigned int hwc[], unsigned long mmcr[], struct perf_event *pevents[])
>> + unsigned int hwc[], struct mmcr_regs *mmcr,
>> + struct perf_event *pevents[])
>> {
>> unsigned long mmcr1 = 0;
>> unsigned long mmcra = 0;
>> @@ -586,20 +587,20 @@ static int power5p_compute_mmcr(u64 event[], int n_ev,
>> }
>>
>> /* Return MMCRx values */
>> - mmcr[0] = 0;
>> + mmcr->mmcr0 = 0;
>> if (pmc_inuse & 1)
>> - mmcr[0] = MMCR0_PMC1CE;
>> + mmcr->mmcr0 = MMCR0_PMC1CE;
>> if (pmc_inuse & 0x3e)
>> - mmcr[0] |= MMCR0_PMCjCE;
>> - mmcr[1] = mmcr1;
>> - mmcr[2] = mmcra;
>> + mmcr->mmcr0 |= MMCR0_PMCjCE;
>> + mmcr->mmcr1 = mmcr1;
>> + mmcr->mmcra = mmcra;
>> return 0;
>> }
>>
>> -static void power5p_disable_pmc(unsigned int pmc, unsigned long mmcr[])
>> +static void power5p_disable_pmc(unsigned int pmc, struct mmcr_regs *mmcr)
>> {
>> if (pmc <= 3)
>> - mmcr[1] &= ~(0x7fUL << MMCR1_PMCSEL_SH(pmc));
>> + mmcr->mmcr1 &= ~(0x7fUL << MMCR1_PMCSEL_SH(pmc));
>> }
>>
>> static int power5p_generic_events[] = {
>> diff --git a/arch/powerpc/perf/power5-pmu.c b/arch/powerpc/perf/power5-pmu.c
>> index da52eca..426021d 100644
>> --- a/arch/powerpc/perf/power5-pmu.c
>> +++ b/arch/powerpc/perf/power5-pmu.c
>> @@ -379,7 +379,8 @@ static int power5_marked_instr_event(u64 event)
>> }
>>
>> static int power5_compute_mmcr(u64 event[], int n_ev,
>> - unsigned int hwc[], unsigned long mmcr[], struct perf_event *pevents[])
>> + unsigned int hwc[], struct mmcr_regs *mmcr,
>> + struct perf_event *pevents[])
>> {
>> unsigned long mmcr1 = 0;
>> unsigned long mmcra = MMCRA_SDAR_DCACHE_MISS | MMCRA_SDAR_ERAT_MISS;
>> @@ -528,20 +529,20 @@ static int power5_compute_mmcr(u64 event[], int n_ev,
>> }
>>
>> /* Return MMCRx values */
>> - mmcr[0] = 0;
>> + mmcr->mmcr0 = 0;
>> if (pmc_inuse & 1)
>> - mmcr[0] = MMCR0_PMC1CE;
>> + mmcr->mmcr0 = MMCR0_PMC1CE;
>> if (pmc_inuse & 0x3e)
>> - mmcr[0] |= MMCR0_PMCjCE;
>> - mmcr[1] = mmcr1;
>> - mmcr[2] = mmcra;
>> + mmcr->mmcr0 |= MMCR0_PMCjCE;
>> + mmcr->mmcr1 = mmcr1;
>> + mmcr->mmcra = mmcra;
>> return 0;
>> }
>>
>> -static void power5_disable_pmc(unsigned int pmc, unsigned long mmcr[])
>> +static void power5_disable_pmc(unsigned int pmc, struct mmcr_regs *mmcr)
>> {
>> if (pmc <= 3)
>> - mmcr[1] &= ~(0x7fUL << MMCR1_PMCSEL_SH(pmc));
>> + mmcr->mmcr1 &= ~(0x7fUL << MMCR1_PMCSEL_SH(pmc));
>> }
>>
>> static int power5_generic_events[] = {
>> diff --git a/arch/powerpc/perf/power6-pmu.c b/arch/powerpc/perf/power6-pmu.c
>> index 3929cac..e343a51 100644
>> --- a/arch/powerpc/perf/power6-pmu.c
>> +++ b/arch/powerpc/perf/power6-pmu.c
>> @@ -171,7 +171,7 @@ static int power6_marked_instr_event(u64 event)
>> * Assign PMC numbers and compute MMCR1 value for a set of events
>> */
>> static int p6_compute_mmcr(u64 event[], int n_ev,
>> - unsigned int hwc[], unsigned long mmcr[], struct perf_event *pevents[])
>> + unsigned int hwc[], struct mmcr_regs *mmcr, struct perf_event *pevents[])
>> {
>> unsigned long mmcr1 = 0;
>> unsigned long mmcra = MMCRA_SDAR_DCACHE_MISS | MMCRA_SDAR_ERAT_MISS;
>> @@ -243,13 +243,13 @@ static int p6_compute_mmcr(u64 event[], int n_ev,
>> if (pmc < 4)
>> mmcr1 |= (unsigned long)psel << MMCR1_PMCSEL_SH(pmc);
>> }
>> - mmcr[0] = 0;
>> + mmcr->mmcr0 = 0;
>> if (pmc_inuse & 1)
>> - mmcr[0] = MMCR0_PMC1CE;
>> + mmcr->mmcr0 = MMCR0_PMC1CE;
>> if (pmc_inuse & 0xe)
>> - mmcr[0] |= MMCR0_PMCjCE;
>> - mmcr[1] = mmcr1;
>> - mmcr[2] = mmcra;
>> + mmcr->mmcr0 |= MMCR0_PMCjCE;
>> + mmcr->mmcr1 = mmcr1;
>> + mmcr->mmcra = mmcra;
>> return 0;
>> }
>>
>> @@ -457,11 +457,11 @@ static int p6_get_alternatives(u64 event, unsigned int flags, u64 alt[])
>> return nalt;
>> }
>>
>> -static void p6_disable_pmc(unsigned int pmc, unsigned long mmcr[])
>> +static void p6_disable_pmc(unsigned int pmc, struct mmcr_regs *mmcr)
>> {
>> /* Set PMCxSEL to 0 to disable PMCx */
>> if (pmc <= 3)
>> - mmcr[1] &= ~(0xffUL << MMCR1_PMCSEL_SH(pmc));
>> + mmcr->mmcr1 &= ~(0xffUL << MMCR1_PMCSEL_SH(pmc));
>> }
>>
>> static int power6_generic_events[] = {
>> diff --git a/arch/powerpc/perf/power7-pmu.c b/arch/powerpc/perf/power7-pmu.c
>> index a137813..3152336 100644
>> --- a/arch/powerpc/perf/power7-pmu.c
>> +++ b/arch/powerpc/perf/power7-pmu.c
>> @@ -242,7 +242,8 @@ static int power7_marked_instr_event(u64 event)
>> }
>>
>> static int power7_compute_mmcr(u64 event[], int n_ev,
>> - unsigned int hwc[], unsigned long mmcr[], struct perf_event *pevents[])
>> + unsigned int hwc[], struct mmcr_regs *mmcr,
>> + struct perf_event *pevents[])
>> {
>> unsigned long mmcr1 = 0;
>> unsigned long mmcra = MMCRA_SDAR_DCACHE_MISS | MMCRA_SDAR_ERAT_MISS;
>> @@ -298,20 +299,20 @@ static int power7_compute_mmcr(u64 event[], int n_ev,
>> }
>>
>> /* Return MMCRx values */
>> - mmcr[0] = 0;
>> + mmcr->mmcr0 = 0;
>> if (pmc_inuse & 1)
>> - mmcr[0] = MMCR0_PMC1CE;
>> + mmcr->mmcr0 = MMCR0_PMC1CE;
>> if (pmc_inuse & 0x3e)
>> - mmcr[0] |= MMCR0_PMCjCE;
>> - mmcr[1] = mmcr1;
>> - mmcr[2] = mmcra;
>> + mmcr->mmcr0 |= MMCR0_PMCjCE;
>> + mmcr->mmcr1 = mmcr1;
>> + mmcr->mmcra = mmcra;
>> return 0;
>> }
>>
>> -static void power7_disable_pmc(unsigned int pmc, unsigned long mmcr[])
>> +static void power7_disable_pmc(unsigned int pmc, struct mmcr_regs *mmcr)
>> {
>> if (pmc <= 3)
>> - mmcr[1] &= ~(0xffUL << MMCR1_PMCSEL_SH(pmc));
>> + mmcr->mmcr1 &= ~(0xffUL << MMCR1_PMCSEL_SH(pmc));
>> }
>>
>> static int power7_generic_events[] = {
>> diff --git a/arch/powerpc/perf/ppc970-pmu.c b/arch/powerpc/perf/ppc970-pmu.c
>> index 4035d93..89a90ab 100644
>> --- a/arch/powerpc/perf/ppc970-pmu.c
>> +++ b/arch/powerpc/perf/ppc970-pmu.c
>> @@ -253,7 +253,8 @@ static int p970_get_alternatives(u64 event, unsigned int flags, u64 alt[])
>> }
>>
>> static int p970_compute_mmcr(u64 event[], int n_ev,
>> - unsigned int hwc[], unsigned long mmcr[], struct perf_event *pevents[])
>> + unsigned int hwc[], struct mmcr_regs *mmcr,
>> + struct perf_event *pevents[])
>> {
>> unsigned long mmcr0 = 0, mmcr1 = 0, mmcra = 0;
>> unsigned int pmc, unit, byte, psel;
>> @@ -393,27 +394,26 @@ static int p970_compute_mmcr(u64 event[], int n_ev,
>> mmcra |= 0x2000; /* mark only one IOP per PPC instruction */
>>
>> /* Return MMCRx values */
>> - mmcr[0] = mmcr0;
>> - mmcr[1] = mmcr1;
>> - mmcr[2] = mmcra;
>> + mmcr->mmcr0 = mmcr0;
>> + mmcr->mmcr1 = mmcr1;
>> + mmcr->mmcra = mmcra;
>> return 0;
>> }
>>
>> -static void p970_disable_pmc(unsigned int pmc, unsigned long mmcr[])
>> +static void p970_disable_pmc(unsigned int pmc, struct mmcr_regs *mmcr)
>> {
>> - int shift, i;
>> + int shift;
>>
>> + /*
>> + * Setting the PMCxSEL field to 0x08 disables PMC x.
>> + */
>> if (pmc <= 1) {
>> shift = MMCR0_PMC1SEL_SH - 7 * pmc;
>> - i = 0;
>> + mmcr->mmcr0 = (mmcr->mmcr0 & ~(0x1fUL << shift)) | (0x08UL << shift);
>> } else {
>> shift = MMCR1_PMC3SEL_SH - 5 * (pmc - 2);
>> - i = 1;
>> + mmcr->mmcr1 = (mmcr->mmcr1 & ~(0x1fUL << shift)) | (0x08UL << shift);
>> }
>> - /*
>> - * Setting the PMCxSEL field to 0x08 disables PMC x.
>> - */
>> - mmcr[i] = (mmcr[i] & ~(0x1fUL << shift)) | (0x08UL << shift);
>> }
>>
>> static int ppc970_generic_events[] = {
>> --
>> 1.8.3.1
[-- Attachment #2: Type: text/html, Size: 56934 bytes --]
^ permalink raw reply
* Re: [PATCH 2/2] powerpc/numa: Remove a redundant variable
From: Nathan Lynch @ 2020-07-22 3:28 UTC (permalink / raw)
To: Srikar Dronamraju; +Cc: Tyrel Datwyler, linuxppc-dev
In-Reply-To: <20200715120534.3673-2-srikar@linux.vnet.ibm.com>
Srikar Dronamraju <srikar@linux.vnet.ibm.com> writes:
> In of_drconf_to_nid_single() default_nid always refers to NUMA_NO_NODE.
> Hence replace it with NUMA_NO_NODE.
>
> No functional changes.
>
...
> diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c
> index a2c5fe0d0cad..b066d89c2975 100644
> --- a/arch/powerpc/mm/numa.c
> +++ b/arch/powerpc/mm/numa.c
> @@ -432,16 +432,15 @@ static int of_get_assoc_arrays(struct assoc_arrays *aa)
> static int of_drconf_to_nid_single(struct drmem_lmb *lmb)
> {
> struct assoc_arrays aa = { .arrays = NULL };
> - int default_nid = NUMA_NO_NODE;
> - int nid = default_nid;
> + int nid = NUMA_NO_NODE;
> int rc, index;
>
> if ((min_common_depth < 0) || !numa_enabled)
> - return default_nid;
> + return NUMA_NO_NODE;
>
> rc = of_get_assoc_arrays(&aa);
> if (rc)
> - return default_nid;
> + return NUMA_NO_NODE;
>
> if (min_common_depth <= aa.array_sz &&
> !(lmb->flags & DRCONF_MEM_AI_INVALID) && lmb->aa_index < aa.n_arrays) {
> @@ -449,7 +448,7 @@ static int of_drconf_to_nid_single(struct drmem_lmb *lmb)
> nid = of_read_number(&aa.arrays[index], 1);
>
> if (nid == 0xffff || nid >= num_possible_nodes())
> - nid = default_nid;
> + nid = NUMA_NO_NODE;
>
> if (nid > 0) {
> index = lmb->aa_index * aa.array_sz;
Doesn't seem like an improvement to me, sorry. Admittedly it's a small
point, but a local variable named "default_nid" communicates that it's a
default or fallback value. That information is lost with this change.
^ permalink raw reply
* Re: [v3 04/15] powerpc/perf: Add support for ISA3.1 PMU SPRs
From: Jordan Niethe @ 2020-07-22 4:18 UTC (permalink / raw)
To: Athira Rajeev
Cc: Gautham R Shenoy, mikey, maddy, kvm, kvm-ppc, svaidyan, acme,
jolsa, linuxppc-dev
In-Reply-To: <1594996707-3727-5-git-send-email-atrajeev@linux.vnet.ibm.com>
On Sat, Jul 18, 2020 at 1:02 AM Athira Rajeev
<atrajeev@linux.vnet.ibm.com> wrote:
>
> From: Madhavan Srinivasan <maddy@linux.ibm.com>
>
> PowerISA v3.1 includes new performance monitoring unit(PMU)
> special purpose registers (SPRs). They are
>
> Monitor Mode Control Register 3 (MMCR3)
> Sampled Instruction Event Register 2 (SIER2)
> Sampled Instruction Event Register 3 (SIER3)
>
> MMCR3 is added for further sampling related configuration
> control. SIER2/SIER3 are added to provide additional
> information about the sampled instruction.
>
> Patch adds new PPMU flag called "PPMU_ARCH_310S" to support
> handling of these new SPRs, updates the struct thread_struct
> to include these new SPRs, include MMCR3 in struct mmcr_regs.
> This is needed to support programming of MMCR3 SPR during
> event_[enable/disable]. Patch also adds the sysfs support
> for the MMCR3 SPR along with SPRN_ macros for these new pmu sprs.
>
> Signed-off-by: Madhavan Srinivasan <maddy@linux.ibm.com>
> ---
> arch/powerpc/include/asm/perf_event_server.h | 2 ++
> arch/powerpc/include/asm/processor.h | 4 ++++
> arch/powerpc/include/asm/reg.h | 6 ++++++
> arch/powerpc/kernel/sysfs.c | 8 ++++++++
> arch/powerpc/perf/core-book3s.c | 29 ++++++++++++++++++++++++++++
> 5 files changed, 49 insertions(+)
>
> diff --git a/arch/powerpc/include/asm/perf_event_server.h b/arch/powerpc/include/asm/perf_event_server.h
> index 14b8dc1..832450a 100644
> --- a/arch/powerpc/include/asm/perf_event_server.h
> +++ b/arch/powerpc/include/asm/perf_event_server.h
> @@ -22,6 +22,7 @@ struct mmcr_regs {
> unsigned long mmcr1;
> unsigned long mmcr2;
> unsigned long mmcra;
> + unsigned long mmcr3;
> };
> /*
> * This struct provides the constants and functions needed to
> @@ -75,6 +76,7 @@ struct power_pmu {
> #define PPMU_HAS_SIER 0x00000040 /* Has SIER */
> #define PPMU_ARCH_207S 0x00000080 /* PMC is architecture v2.07S */
> #define PPMU_NO_SIAR 0x00000100 /* Do not use SIAR */
> +#define PPMU_ARCH_310S 0x00000200 /* Has MMCR3, SIER2 and SIER3 */
We elsewhere have CPU_FTR_ARCH_31, so should this be PPMU_ARCH_31S to
be consistent.
>
> /*
> * Values for flags to get_alternatives()
> diff --git a/arch/powerpc/include/asm/processor.h b/arch/powerpc/include/asm/processor.h
> index 52a6783..a466e94 100644
> --- a/arch/powerpc/include/asm/processor.h
> +++ b/arch/powerpc/include/asm/processor.h
> @@ -272,6 +272,10 @@ struct thread_struct {
> unsigned mmcr0;
>
> unsigned used_ebb;
> + unsigned long mmcr3;
> + unsigned long sier2;
> + unsigned long sier3;
> +
> #endif
> };
>
> diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h
> index 88e6c78..21a1b2d 100644
> --- a/arch/powerpc/include/asm/reg.h
> +++ b/arch/powerpc/include/asm/reg.h
> @@ -876,7 +876,9 @@
> #define MMCR0_FCHV 0x00000001UL /* freeze conditions in hypervisor mode */
> #define SPRN_MMCR1 798
> #define SPRN_MMCR2 785
> +#define SPRN_MMCR3 754
> #define SPRN_UMMCR2 769
> +#define SPRN_UMMCR3 738
> #define SPRN_MMCRA 0x312
> #define MMCRA_SDSYNC 0x80000000UL /* SDAR synced with SIAR */
> #define MMCRA_SDAR_DCACHE_MISS 0x40000000UL
> @@ -918,6 +920,10 @@
> #define SIER_SIHV 0x1000000 /* Sampled MSR_HV */
> #define SIER_SIAR_VALID 0x0400000 /* SIAR contents valid */
> #define SIER_SDAR_VALID 0x0200000 /* SDAR contents valid */
> +#define SPRN_SIER2 752
> +#define SPRN_SIER3 753
> +#define SPRN_USIER2 736
> +#define SPRN_USIER3 737
> #define SPRN_SIAR 796
> #define SPRN_SDAR 797
> #define SPRN_TACR 888
> diff --git a/arch/powerpc/kernel/sysfs.c b/arch/powerpc/kernel/sysfs.c
> index 571b325..46b4ebc 100644
> --- a/arch/powerpc/kernel/sysfs.c
> +++ b/arch/powerpc/kernel/sysfs.c
> @@ -622,8 +622,10 @@ void ppc_enable_pmcs(void)
> SYSFS_PMCSETUP(pmc8, SPRN_PMC8);
>
> SYSFS_PMCSETUP(mmcra, SPRN_MMCRA);
> +SYSFS_PMCSETUP(mmcr3, SPRN_MMCR3);
>
> static DEVICE_ATTR(mmcra, 0600, show_mmcra, store_mmcra);
> +static DEVICE_ATTR(mmcr3, 0600, show_mmcr3, store_mmcr3);
> #endif /* HAS_PPC_PMC56 */
>
>
> @@ -886,6 +888,9 @@ static int register_cpu_online(unsigned int cpu)
> #ifdef CONFIG_PMU_SYSFS
> if (cpu_has_feature(CPU_FTR_MMCRA))
> device_create_file(s, &dev_attr_mmcra);
> +
> + if (cpu_has_feature(CPU_FTR_ARCH_31))
> + device_create_file(s, &dev_attr_mmcr3);
> #endif /* CONFIG_PMU_SYSFS */
>
> if (cpu_has_feature(CPU_FTR_PURR)) {
> @@ -980,6 +985,9 @@ static int unregister_cpu_online(unsigned int cpu)
> #ifdef CONFIG_PMU_SYSFS
> if (cpu_has_feature(CPU_FTR_MMCRA))
> device_remove_file(s, &dev_attr_mmcra);
> +
> + if (cpu_has_feature(CPU_FTR_ARCH_31))
> + device_remove_file(s, &dev_attr_mmcr3);
> #endif /* CONFIG_PMU_SYSFS */
>
> if (cpu_has_feature(CPU_FTR_PURR)) {
> diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
> index f4d07b5..ca32fc0 100644
> --- a/arch/powerpc/perf/core-book3s.c
> +++ b/arch/powerpc/perf/core-book3s.c
> @@ -72,6 +72,11 @@ struct cpu_hw_events {
> /*
> * 32-bit doesn't have MMCRA but does have an MMCR2,
> * and a few other names are different.
> + * Also 32-bit doesn't have MMCR3, SIER2 and SIER3.
> + * Define them as zero knowing that any code path accessing
> + * these registers (via mtspr/mfspr) are done under ppmu flag
> + * check for PPMU_ARCH_310S and we will not enter that code path
> + * for 32-bit.
> */
> #ifdef CONFIG_PPC32
>
> @@ -85,6 +90,9 @@ struct cpu_hw_events {
> #define MMCR0_PMCC_U6 0
>
> #define SPRN_MMCRA SPRN_MMCR2
> +#define SPRN_MMCR3 0
> +#define SPRN_SIER2 0
> +#define SPRN_SIER3 0
> #define MMCRA_SAMPLE_ENABLE 0
>
> static inline unsigned long perf_ip_adjust(struct pt_regs *regs)
> @@ -581,6 +589,11 @@ static void ebb_switch_out(unsigned long mmcr0)
> current->thread.sdar = mfspr(SPRN_SDAR);
> current->thread.mmcr0 = mmcr0 & MMCR0_USER_MASK;
> current->thread.mmcr2 = mfspr(SPRN_MMCR2) & MMCR2_USER_MASK;
> + if (ppmu->flags & PPMU_ARCH_310S) {
> + current->thread.mmcr3 = mfspr(SPRN_MMCR3);
Like MMCR0_USER_MASK and MMCR2_USER_MASK do we need a MMCR3_USER_MASK
here, or is there no need?
> + current->thread.sier2 = mfspr(SPRN_SIER2);
> + current->thread.sier3 = mfspr(SPRN_SIER3);
> + }
> }
>
> static unsigned long ebb_switch_in(bool ebb, struct cpu_hw_events *cpuhw)
> @@ -620,6 +633,12 @@ static unsigned long ebb_switch_in(bool ebb, struct cpu_hw_events *cpuhw)
> * instead manage the MMCR2 entirely by itself.
> */
> mtspr(SPRN_MMCR2, cpuhw->mmcr.mmcr2 | current->thread.mmcr2);
> +
> + if (ppmu->flags & PPMU_ARCH_310S) {
> + mtspr(SPRN_MMCR3, current->thread.mmcr3);
> + mtspr(SPRN_SIER2, current->thread.sier2);
> + mtspr(SPRN_SIER3, current->thread.sier3);
> + }
> out:
> return mmcr0;
> }
> @@ -840,6 +859,11 @@ void perf_event_print_debug(void)
> pr_info("EBBRR: %016lx BESCR: %016lx\n",
> mfspr(SPRN_EBBRR), mfspr(SPRN_BESCR));
> }
> +
> + if (ppmu->flags & PPMU_ARCH_310S) {
> + pr_info("MMCR3: %016lx SIER2: %016lx SIER3: %016lx\n",
> + mfspr(SPRN_MMCR3), mfspr(SPRN_SIER2), mfspr(SPRN_SIER3));
> + }
> #endif
> pr_info("SIAR: %016lx SDAR: %016lx SIER: %016lx\n",
> mfspr(SPRN_SIAR), sdar, sier);
> @@ -1305,6 +1329,8 @@ static void power_pmu_enable(struct pmu *pmu)
> if (!cpuhw->n_added) {
> mtspr(SPRN_MMCRA, cpuhw->mmcr.mmcra & ~MMCRA_SAMPLE_ENABLE);
> mtspr(SPRN_MMCR1, cpuhw->mmcr.mmcr1);
> + if (ppmu->flags & PPMU_ARCH_310S)
> + mtspr(SPRN_MMCR3, cpuhw->mmcr.mmcr3);
> goto out_enable;
> }
>
> @@ -1348,6 +1374,9 @@ static void power_pmu_enable(struct pmu *pmu)
> if (ppmu->flags & PPMU_ARCH_207S)
> mtspr(SPRN_MMCR2, cpuhw->mmcr.mmcr2);
>
> + if (ppmu->flags & PPMU_ARCH_310S)
> + mtspr(SPRN_MMCR3, cpuhw->mmcr.mmcr3);
> +
> /*
> * Read off any pre-existing events that need to move
> * to another PMC.
> --
> 1.8.3.1
>
^ permalink raw reply
* OF: Can't handle multiple dma-ranges with different offsets
From: Chris Packham @ 2020-07-22 4:19 UTC (permalink / raw)
To: robh+dt@kernel.org, frowand.list@gmail.com, mpe@ellerman.id.au,
benh@kernel.crashing.org, paulus@samba.org,
christophe.leroy@c-s.fr
Cc: devicetree@vger.kernel.org, linuxppc-dev@lists.ozlabs.org,
linux-kernel@vger.kernel.org
Hi,
I've just fired up linux kernel v5.7 on a p2040 based system and I'm
getting the following new warning
OF: Can't handle multiple dma-ranges with different offsets on
node(/pcie@ffe202000)
OF: Can't handle multiple dma-ranges with different offsets on
node(/pcie@ffe202000)
The warning itself was added in commit 9d55bebd9816 ("of/address:
Support multiple 'dma-ranges' entries") but I gather it's pointing out
something about the dts. My boards dts is based heavily on p2041rdb.dts
and the relevant pci2 section is identical (reproduced below for reference).
pci2: pcie@ffe202000 {
reg = <0xf 0xfe202000 0 0x1000>;
ranges = <0x02000000 0 0xe0000000 0xc 0x40000000 0 0x20000000
0x01000000 0 0x00000000 0xf 0xf8020000 0 0x00010000>;
pcie@0 {
ranges = <0x02000000 0 0xe0000000
0x02000000 0 0xe0000000
0 0x20000000
0x01000000 0 0x00000000
0x01000000 0 0x00000000
0 0x00010000>;
};
};
I haven't noticed any ill effect (aside from the scary message). I'm not
sure if there's something missing in the dts or in the code that checks
the ranges. Any guidance would be appreciated.
Thanks,
Chris
^ permalink raw reply
* Re: [PATCH v4 07/12] ppc64/kexec_file: add support to relocate purgatory
From: Michael Ellerman @ 2020-07-22 4:25 UTC (permalink / raw)
To: Hari Bathini, Andrew Morton
Cc: kernel test robot, Pingfan Liu, Kexec-ml, Nayna Jain,
Petr Tesarik, Mahesh J Salgaonkar, Mimi Zohar, lkml, linuxppc-dev,
Sourabh Jain, Vivek Goyal, Dave Young, Thiago Jung Bauermann,
Eric Biederman
In-Reply-To: <159524956457.20855.12480643681198700190.stgit@hbathini.in.ibm.com>
Hari Bathini <hbathini@linux.ibm.com> writes:
> Right now purgatory implementation is only minimal. But if purgatory
> code is to be enhanced to copy memory to the backup region and verify
> sha256 digest, relocations may have to be applied to the purgatory.
> So, add support to relocate purgatory in kexec_file_load system call
> by setting up TOC pointer and applying RELA relocations as needed.
>
> Reported-by: kernel test robot <lkp@intel.com>
> [lkp: In v1, 'struct mem_sym' was declared in parameter list]
> Signed-off-by: Hari Bathini <hbathini@linux.ibm.com>
> ---
>
> * Michael, can you share your opinion on the below:
> - https://lore.kernel.org/patchwork/patch/1272027/
> - My intention in cover note.
It seems like a lot of complexity for little benefit.
AFAICS your final purgatory_64.c is only 36 lines, and all it does is a
single (open coded) memcpy().
It seems like we could write that in not many more lines of assembler
and avoid all this code.
What am I missing?
cheers
^ permalink raw reply
* [PATCH v2 01/14] powerpc/eeh: Remove eeh_dev_phb_init_dynamic()
From: Oliver O'Halloran @ 2020-07-22 4:26 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Oliver O'Halloran
This function is a one line wrapper around eeh_phb_pe_create() and despite
the name it doesn't create any eeh_dev structures. Replace it with direct
calls to eeh_phb_pe_create() since that does what it says on the tin
and removes a layer of indirection.
Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
---
v2: Added sub prototype of eeh_phb_pe_create() for the !CONFIG_EEH case.
---
arch/powerpc/include/asm/eeh.h | 3 ++-
arch/powerpc/kernel/eeh.c | 2 +-
arch/powerpc/kernel/eeh_dev.c | 13 -------------
arch/powerpc/kernel/of_platform.c | 4 ++--
arch/powerpc/platforms/pseries/pci_dlpar.c | 2 +-
5 files changed, 6 insertions(+), 18 deletions(-)
diff --git a/arch/powerpc/include/asm/eeh.h b/arch/powerpc/include/asm/eeh.h
index 964a54292b36..64487b88c569 100644
--- a/arch/powerpc/include/asm/eeh.h
+++ b/arch/powerpc/include/asm/eeh.h
@@ -294,7 +294,6 @@ const char *eeh_pe_loc_get(struct eeh_pe *pe);
struct pci_bus *eeh_pe_bus_get(struct eeh_pe *pe);
struct eeh_dev *eeh_dev_init(struct pci_dn *pdn);
-void eeh_dev_phb_init_dynamic(struct pci_controller *phb);
void eeh_show_enabled(void);
int __init eeh_ops_register(struct eeh_ops *ops);
int __exit eeh_ops_unregister(const char *name);
@@ -370,6 +369,8 @@ void pseries_eeh_init_edev_recursive(struct pci_dn *pdn);
#else
static inline void pseries_eeh_add_device_early(struct pci_dn *pdn) { }
static inline void pseries_eeh_add_device_tree_early(struct pci_dn *pdn) { }
+
+static inline int eeh_phb_pe_create(struct pci_controller *phb) { return 0; }
#endif
#ifdef CONFIG_PPC64
diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
index d407981dec76..859f76020256 100644
--- a/arch/powerpc/kernel/eeh.c
+++ b/arch/powerpc/kernel/eeh.c
@@ -1096,7 +1096,7 @@ static int eeh_init(void)
/* Initialize PHB PEs */
list_for_each_entry_safe(hose, tmp, &hose_list, list_node)
- eeh_dev_phb_init_dynamic(hose);
+ eeh_phb_pe_create(hose);
eeh_addr_cache_init();
diff --git a/arch/powerpc/kernel/eeh_dev.c b/arch/powerpc/kernel/eeh_dev.c
index 7370185c7a05..8e159a12f10c 100644
--- a/arch/powerpc/kernel/eeh_dev.c
+++ b/arch/powerpc/kernel/eeh_dev.c
@@ -52,16 +52,3 @@ struct eeh_dev *eeh_dev_init(struct pci_dn *pdn)
return edev;
}
-
-/**
- * eeh_dev_phb_init_dynamic - Create EEH devices for devices included in PHB
- * @phb: PHB
- *
- * Scan the PHB OF node and its child association, then create the
- * EEH devices accordingly
- */
-void eeh_dev_phb_init_dynamic(struct pci_controller *phb)
-{
- /* EEH PE for PHB */
- eeh_phb_pe_create(phb);
-}
diff --git a/arch/powerpc/kernel/of_platform.c b/arch/powerpc/kernel/of_platform.c
index 71a3f97dc988..f89376ff633e 100644
--- a/arch/powerpc/kernel/of_platform.c
+++ b/arch/powerpc/kernel/of_platform.c
@@ -62,8 +62,8 @@ static int of_pci_phb_probe(struct platform_device *dev)
/* Init pci_dn data structures */
pci_devs_phb_init_dynamic(phb);
- /* Create EEH PEs for the PHB */
- eeh_dev_phb_init_dynamic(phb);
+ /* Create EEH PE for the PHB */
+ eeh_phb_pe_create(phb);
/* Scan the bus */
pcibios_scan_phb(phb);
diff --git a/arch/powerpc/platforms/pseries/pci_dlpar.c b/arch/powerpc/platforms/pseries/pci_dlpar.c
index b3a38f5a6b68..f9ae17e8a0f4 100644
--- a/arch/powerpc/platforms/pseries/pci_dlpar.c
+++ b/arch/powerpc/platforms/pseries/pci_dlpar.c
@@ -34,7 +34,7 @@ struct pci_controller *init_phb_dynamic(struct device_node *dn)
pci_devs_phb_init_dynamic(phb);
/* Create EEH devices for the PHB */
- eeh_dev_phb_init_dynamic(phb);
+ eeh_phb_pe_create(phb);
if (dn->child)
pseries_eeh_init_edev_recursive(PCI_DN(dn));
--
2.26.2
^ permalink raw reply related
* [PATCH v2 02/14] powerpc/eeh: Remove eeh_dev.c
From: Oliver O'Halloran @ 2020-07-22 4:26 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Oliver O'Halloran
In-Reply-To: <20200722042628.1425880-1-oohall@gmail.com>
The only thing in this file is eeh_dev_init() which is allocates and
initialises an eeh_dev based on a pci_dn. This is only ever called from
pci_dn.c so move it into there and remove the file.
Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
---
v2: no change
---
arch/powerpc/include/asm/eeh.h | 6 ----
arch/powerpc/kernel/Makefile | 2 +-
arch/powerpc/kernel/eeh_dev.c | 54 ----------------------------------
arch/powerpc/kernel/pci_dn.c | 20 +++++++++++++
4 files changed, 21 insertions(+), 61 deletions(-)
delete mode 100644 arch/powerpc/kernel/eeh_dev.c
diff --git a/arch/powerpc/include/asm/eeh.h b/arch/powerpc/include/asm/eeh.h
index 64487b88c569..8537dd334094 100644
--- a/arch/powerpc/include/asm/eeh.h
+++ b/arch/powerpc/include/asm/eeh.h
@@ -293,7 +293,6 @@ void eeh_pe_restore_bars(struct eeh_pe *pe);
const char *eeh_pe_loc_get(struct eeh_pe *pe);
struct pci_bus *eeh_pe_bus_get(struct eeh_pe *pe);
-struct eeh_dev *eeh_dev_init(struct pci_dn *pdn);
void eeh_show_enabled(void);
int __init eeh_ops_register(struct eeh_ops *ops);
int __exit eeh_ops_unregister(const char *name);
@@ -339,11 +338,6 @@ static inline bool eeh_enabled(void)
static inline void eeh_show_enabled(void) { }
-static inline void *eeh_dev_init(struct pci_dn *pdn, void *data)
-{
- return NULL;
-}
-
static inline void eeh_dev_phb_init_dynamic(struct pci_controller *phb) { }
static inline int eeh_check_failure(const volatile void __iomem *token)
diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
index 244542ae2a91..c5211bdcf1b6 100644
--- a/arch/powerpc/kernel/Makefile
+++ b/arch/powerpc/kernel/Makefile
@@ -71,7 +71,7 @@ obj-$(CONFIG_PPC_RTAS_DAEMON) += rtasd.o
obj-$(CONFIG_RTAS_FLASH) += rtas_flash.o
obj-$(CONFIG_RTAS_PROC) += rtas-proc.o
obj-$(CONFIG_PPC_DT_CPU_FTRS) += dt_cpu_ftrs.o
-obj-$(CONFIG_EEH) += eeh.o eeh_pe.o eeh_dev.o eeh_cache.o \
+obj-$(CONFIG_EEH) += eeh.o eeh_pe.o eeh_cache.o \
eeh_driver.o eeh_event.o eeh_sysfs.o
obj-$(CONFIG_GENERIC_TBSYNC) += smp-tbsync.o
obj-$(CONFIG_CRASH_DUMP) += crash_dump.o
diff --git a/arch/powerpc/kernel/eeh_dev.c b/arch/powerpc/kernel/eeh_dev.c
deleted file mode 100644
index 8e159a12f10c..000000000000
--- a/arch/powerpc/kernel/eeh_dev.c
+++ /dev/null
@@ -1,54 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0-or-later
-/*
- * The file intends to implement dynamic creation of EEH device, which will
- * be bound with OF node and PCI device simutaneously. The EEH devices would
- * be foundamental information for EEH core components to work proerly. Besides,
- * We have to support multiple situations where dynamic creation of EEH device
- * is required:
- *
- * 1) Before PCI emunation starts, we need create EEH devices according to the
- * PCI sensitive OF nodes.
- * 2) When PCI emunation is done, we need do the binding between PCI device and
- * the associated EEH device.
- * 3) DR (Dynamic Reconfiguration) would create PCI sensitive OF node. EEH device
- * will be created while PCI sensitive OF node is detected from DR.
- * 4) PCI hotplug needs redoing the binding between PCI device and EEH device. If
- * PHB is newly inserted, we also need create EEH devices accordingly.
- *
- * Copyright Benjamin Herrenschmidt & Gavin Shan, IBM Corporation 2012.
- */
-
-#include <linux/export.h>
-#include <linux/gfp.h>
-#include <linux/init.h>
-#include <linux/kernel.h>
-#include <linux/pci.h>
-#include <linux/string.h>
-
-#include <asm/pci-bridge.h>
-#include <asm/ppc-pci.h>
-
-/**
- * eeh_dev_init - Create EEH device according to OF node
- * @pdn: PCI device node
- *
- * It will create EEH device according to the given OF node. The function
- * might be called by PCI emunation, DR, PHB hotplug.
- */
-struct eeh_dev *eeh_dev_init(struct pci_dn *pdn)
-{
- struct eeh_dev *edev;
-
- /* Allocate EEH device */
- edev = kzalloc(sizeof(*edev), GFP_KERNEL);
- if (!edev)
- return NULL;
-
- /* Associate EEH device with OF node */
- pdn->edev = edev;
- edev->pdn = pdn;
- edev->bdfn = (pdn->busno << 8) | pdn->devfn;
- edev->controller = pdn->phb;
-
- return edev;
-}
diff --git a/arch/powerpc/kernel/pci_dn.c b/arch/powerpc/kernel/pci_dn.c
index 4e654df55969..f790a8d06f50 100644
--- a/arch/powerpc/kernel/pci_dn.c
+++ b/arch/powerpc/kernel/pci_dn.c
@@ -124,6 +124,26 @@ struct pci_dn *pci_get_pdn(struct pci_dev *pdev)
return NULL;
}
+#ifdef CONFIG_EEH
+static struct eeh_dev *eeh_dev_init(struct pci_dn *pdn)
+{
+ struct eeh_dev *edev;
+
+ /* Allocate EEH device */
+ edev = kzalloc(sizeof(*edev), GFP_KERNEL);
+ if (!edev)
+ return NULL;
+
+ /* Associate EEH device with OF node */
+ pdn->edev = edev;
+ edev->pdn = pdn;
+ edev->bdfn = (pdn->busno << 8) | pdn->devfn;
+ edev->controller = pdn->phb;
+
+ return edev;
+}
+#endif /* CONFIG_EEH */
+
#ifdef CONFIG_PCI_IOV
static struct pci_dn *add_one_sriov_vf_pdn(struct pci_dn *parent,
int vf_index,
--
2.26.2
^ permalink raw reply related
* [PATCH v2 03/14] powerpc/eeh: Move vf_index out of pci_dn and into eeh_dev
From: Oliver O'Halloran @ 2020-07-22 4:26 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Alexey Kardashevskiy, Oliver O'Halloran
In-Reply-To: <20200722042628.1425880-1-oohall@gmail.com>
Drivers that do not support the PCI error handling callbacks are handled by
tearing down the device and re-probing them. If the device being removed is
a virtual function then we need to know the VF index so it can be removed
using the pci_iov_{add|remove}_virtfn() API.
Currently this is handled by looking up the pci_dn, and using the vf_index
that was stashed there when the pci_dn for the VF was created in
pcibios_sriov_enable(). We would like to eliminate the use of pci_dn
outside of pseries though so we need to provide the generic EEH code with
some other way to find the vf_index.
The easiest thing to do here is move the vf_index field out of pci_dn and
into eeh_dev. Currently pci_dn and eeh_dev are allocated and initialized
together so this is a fairly minimal change in preparation for splitting
pci_dn and eeh_dev in the future.
Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
v2: Commit message fixup
---
arch/powerpc/include/asm/eeh.h | 3 +++
arch/powerpc/include/asm/pci-bridge.h | 1 -
arch/powerpc/kernel/eeh_driver.c | 6 ++----
arch/powerpc/kernel/pci_dn.c | 7 ++++---
4 files changed, 9 insertions(+), 8 deletions(-)
diff --git a/arch/powerpc/include/asm/eeh.h b/arch/powerpc/include/asm/eeh.h
index 8537dd334094..9c5ba535f6a1 100644
--- a/arch/powerpc/include/asm/eeh.h
+++ b/arch/powerpc/include/asm/eeh.h
@@ -148,7 +148,10 @@ struct eeh_dev {
struct pci_dn *pdn; /* Associated PCI device node */
struct pci_dev *pdev; /* Associated PCI device */
bool in_error; /* Error flag for edev */
+
+ /* VF specific properties */
struct pci_dev *physfn; /* Associated SRIOV PF */
+ int vf_index; /* Index of this VF */
};
/* "fmt" must be a simple literal string */
diff --git a/arch/powerpc/include/asm/pci-bridge.h b/arch/powerpc/include/asm/pci-bridge.h
index b92e81b256e5..d2a2a14e56f9 100644
--- a/arch/powerpc/include/asm/pci-bridge.h
+++ b/arch/powerpc/include/asm/pci-bridge.h
@@ -202,7 +202,6 @@ struct pci_dn {
#define IODA_INVALID_PE 0xFFFFFFFF
unsigned int pe_number;
#ifdef CONFIG_PCI_IOV
- int vf_index; /* VF index in the PF */
u16 vfs_expanded; /* number of VFs IOV BAR expanded */
u16 num_vfs; /* number of VFs enabled*/
unsigned int *pe_num_map; /* PE# for the first VF PE or array */
diff --git a/arch/powerpc/kernel/eeh_driver.c b/arch/powerpc/kernel/eeh_driver.c
index 7b048cee767c..b70b9273f45a 100644
--- a/arch/powerpc/kernel/eeh_driver.c
+++ b/arch/powerpc/kernel/eeh_driver.c
@@ -477,7 +477,7 @@ static void *eeh_add_virt_device(struct eeh_dev *edev)
}
#ifdef CONFIG_PCI_IOV
- pci_iov_add_virtfn(edev->physfn, eeh_dev_to_pdn(edev)->vf_index);
+ pci_iov_add_virtfn(edev->physfn, edev->vf_index);
#endif
return NULL;
}
@@ -521,9 +521,7 @@ static void eeh_rmv_device(struct eeh_dev *edev, void *userdata)
if (edev->physfn) {
#ifdef CONFIG_PCI_IOV
- struct pci_dn *pdn = eeh_dev_to_pdn(edev);
-
- pci_iov_remove_virtfn(edev->physfn, pdn->vf_index);
+ pci_iov_remove_virtfn(edev->physfn, edev->vf_index);
edev->pdev = NULL;
#endif
if (rmv_data)
diff --git a/arch/powerpc/kernel/pci_dn.c b/arch/powerpc/kernel/pci_dn.c
index f790a8d06f50..bf11ac8427ac 100644
--- a/arch/powerpc/kernel/pci_dn.c
+++ b/arch/powerpc/kernel/pci_dn.c
@@ -146,7 +146,6 @@ static struct eeh_dev *eeh_dev_init(struct pci_dn *pdn)
#ifdef CONFIG_PCI_IOV
static struct pci_dn *add_one_sriov_vf_pdn(struct pci_dn *parent,
- int vf_index,
int busno, int devfn)
{
struct pci_dn *pdn;
@@ -163,7 +162,6 @@ static struct pci_dn *add_one_sriov_vf_pdn(struct pci_dn *parent,
pdn->parent = parent;
pdn->busno = busno;
pdn->devfn = devfn;
- pdn->vf_index = vf_index;
pdn->pe_number = IODA_INVALID_PE;
INIT_LIST_HEAD(&pdn->child_list);
INIT_LIST_HEAD(&pdn->list);
@@ -194,7 +192,7 @@ struct pci_dn *add_sriov_vf_pdns(struct pci_dev *pdev)
for (i = 0; i < pci_sriov_get_totalvfs(pdev); i++) {
struct eeh_dev *edev __maybe_unused;
- pdn = add_one_sriov_vf_pdn(parent, i,
+ pdn = add_one_sriov_vf_pdn(parent,
pci_iov_virtfn_bus(pdev, i),
pci_iov_virtfn_devfn(pdev, i));
if (!pdn) {
@@ -207,7 +205,10 @@ struct pci_dn *add_sriov_vf_pdns(struct pci_dev *pdev)
/* Create the EEH device for the VF */
edev = eeh_dev_init(pdn);
BUG_ON(!edev);
+
+ /* FIXME: these should probably be populated by the EEH probe */
edev->physfn = pdev;
+ edev->vf_index = i;
#endif /* CONFIG_EEH */
}
return pci_get_pdn(pdev);
--
2.26.2
^ permalink raw reply related
* [PATCH v2 04/14] powerpc/pseries: Stop using pdn->pe_number
From: Oliver O'Halloran @ 2020-07-22 4:26 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Alexey Kardashevskiy, Oliver O'Halloran
In-Reply-To: <20200722042628.1425880-1-oohall@gmail.com>
The pci_dn->pe_number field is mainly used to track the IODA PE number of a
device on PowerNV. At some point it grew a user in the pseries SR-IOV
support which muddies the waters a bit, so remove it.
Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
v2: no change
---
arch/powerpc/platforms/pseries/eeh_pseries.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/arch/powerpc/platforms/pseries/eeh_pseries.c b/arch/powerpc/platforms/pseries/eeh_pseries.c
index ace117f99d94..18a2522b9b5e 100644
--- a/arch/powerpc/platforms/pseries/eeh_pseries.c
+++ b/arch/powerpc/platforms/pseries/eeh_pseries.c
@@ -52,8 +52,6 @@ void pseries_pcibios_bus_add_device(struct pci_dev *pdev)
dev_dbg(&pdev->dev, "EEH: Setting up device\n");
#ifdef CONFIG_PCI_IOV
if (pdev->is_virtfn) {
- struct pci_dn *physfn_pdn;
-
pdn->device_id = pdev->device;
pdn->vendor_id = pdev->vendor;
pdn->class_code = pdev->class;
@@ -63,8 +61,6 @@ void pseries_pcibios_bus_add_device(struct pci_dev *pdev)
* completion from platform.
*/
pdn->last_allow_rc = 0;
- physfn_pdn = pci_get_pdn(pdev->physfn);
- pdn->pe_number = physfn_pdn->pe_num_map[pdn->vf_index];
}
#endif
pseries_eeh_init_edev(pdn);
@@ -772,8 +768,8 @@ int pseries_send_allow_unfreeze(struct pci_dn *pdn,
static int pseries_call_allow_unfreeze(struct eeh_dev *edev)
{
+ int cur_vfs = 0, rc = 0, vf_index, bus, devfn, vf_pe_num;
struct pci_dn *pdn, *tmp, *parent, *physfn_pdn;
- int cur_vfs = 0, rc = 0, vf_index, bus, devfn;
u16 *vf_pe_array;
vf_pe_array = kzalloc(RTAS_DATA_BUF_SIZE, GFP_KERNEL);
@@ -806,8 +802,10 @@ static int pseries_call_allow_unfreeze(struct eeh_dev *edev)
}
} else {
pdn = pci_get_pdn(edev->pdev);
- vf_pe_array[0] = cpu_to_be16(pdn->pe_number);
physfn_pdn = pci_get_pdn(edev->physfn);
+
+ vf_pe_num = physfn_pdn->pe_num_map[edev->vf_index];
+ vf_pe_array[0] = cpu_to_be16(vf_pe_num);
rc = pseries_send_allow_unfreeze(physfn_pdn,
vf_pe_array, 1);
pdn->last_allow_rc = rc;
--
2.26.2
^ permalink raw reply related
* [PATCH v2 05/14] powerpc/eeh: Kill off eeh_ops->get_pe_addr()
From: Oliver O'Halloran @ 2020-07-22 4:26 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Alexey Kardashevskiy, Oliver O'Halloran
In-Reply-To: <20200722042628.1425880-1-oohall@gmail.com>
This is used in precisely one place which is in pseries specific platform
code. There's no need to have the callback in eeh_ops since the platform
chooses the EEH PE addresses anyway. The PowerNV implementation has always
been a stub too so remove it.
Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
v2: Made "buid" in pseries_eeh_get_pe_addr() an unsigned long to match
the pci_controller type.
---
arch/powerpc/include/asm/eeh.h | 1 -
arch/powerpc/platforms/powernv/eeh-powernv.c | 13 ------------
arch/powerpc/platforms/pseries/eeh_pseries.c | 22 ++++++++++----------
3 files changed, 11 insertions(+), 25 deletions(-)
diff --git a/arch/powerpc/include/asm/eeh.h b/arch/powerpc/include/asm/eeh.h
index 9c5ba535f6a1..d619012281bd 100644
--- a/arch/powerpc/include/asm/eeh.h
+++ b/arch/powerpc/include/asm/eeh.h
@@ -220,7 +220,6 @@ struct eeh_ops {
int (*init)(void);
struct eeh_dev *(*probe)(struct pci_dev *pdev);
int (*set_option)(struct eeh_pe *pe, int option);
- int (*get_pe_addr)(struct eeh_pe *pe);
int (*get_state)(struct eeh_pe *pe, int *delay);
int (*reset)(struct eeh_pe *pe, int option);
int (*get_log)(struct eeh_pe *pe, int severity, char *drv_log, unsigned long len);
diff --git a/arch/powerpc/platforms/powernv/eeh-powernv.c b/arch/powerpc/platforms/powernv/eeh-powernv.c
index 79409e005fcd..bcd0515d8f79 100644
--- a/arch/powerpc/platforms/powernv/eeh-powernv.c
+++ b/arch/powerpc/platforms/powernv/eeh-powernv.c
@@ -535,18 +535,6 @@ static int pnv_eeh_set_option(struct eeh_pe *pe, int option)
return 0;
}
-/**
- * pnv_eeh_get_pe_addr - Retrieve PE address
- * @pe: EEH PE
- *
- * Retrieve the PE address according to the given tranditional
- * PCI BDF (Bus/Device/Function) address.
- */
-static int pnv_eeh_get_pe_addr(struct eeh_pe *pe)
-{
- return pe->addr;
-}
-
static void pnv_eeh_get_phb_diag(struct eeh_pe *pe)
{
struct pnv_phb *phb = pe->phb->private_data;
@@ -1670,7 +1658,6 @@ static struct eeh_ops pnv_eeh_ops = {
.init = pnv_eeh_init,
.probe = pnv_eeh_probe,
.set_option = pnv_eeh_set_option,
- .get_pe_addr = pnv_eeh_get_pe_addr,
.get_state = pnv_eeh_get_state,
.reset = pnv_eeh_reset,
.get_log = pnv_eeh_get_log,
diff --git a/arch/powerpc/platforms/pseries/eeh_pseries.c b/arch/powerpc/platforms/pseries/eeh_pseries.c
index 18a2522b9b5e..bcc72b9a5309 100644
--- a/arch/powerpc/platforms/pseries/eeh_pseries.c
+++ b/arch/powerpc/platforms/pseries/eeh_pseries.c
@@ -32,6 +32,8 @@
#include <asm/ppc-pci.h>
#include <asm/rtas.h>
+static int pseries_eeh_get_pe_addr(struct pci_dn *pdn);
+
/* RTAS tokens */
static int ibm_set_eeh_option;
static int ibm_set_slot_reset;
@@ -301,7 +303,7 @@ void pseries_eeh_init_edev(struct pci_dn *pdn)
eeh_edev_dbg(edev, "EEH failed to enable on device (code %d)\n", ret);
} else {
/* Retrieve PE address */
- edev->pe_config_addr = eeh_ops->get_pe_addr(&pe);
+ edev->pe_config_addr = pseries_eeh_get_pe_addr(pdn);
pe.addr = edev->pe_config_addr;
/* Some older systems (Power4) allow the ibm,set-eeh-option
@@ -431,8 +433,10 @@ static int pseries_eeh_set_option(struct eeh_pe *pe, int option)
* It's notable that zero'ed return value means invalid PE config
* address.
*/
-static int pseries_eeh_get_pe_addr(struct eeh_pe *pe)
+static int pseries_eeh_get_pe_addr(struct pci_dn *pdn)
{
+ int config_addr = rtas_config_addr(pdn->busno, pdn->devfn, 0);
+ unsigned long buid = pdn->phb->buid;
int ret = 0;
int rets[3];
@@ -443,18 +447,16 @@ static int pseries_eeh_get_pe_addr(struct eeh_pe *pe)
* meaningless.
*/
ret = rtas_call(ibm_get_config_addr_info2, 4, 2, rets,
- pe->config_addr, BUID_HI(pe->phb->buid),
- BUID_LO(pe->phb->buid), 1);
+ config_addr, BUID_HI(buid), BUID_LO(buid), 1);
if (ret || (rets[0] == 0))
return 0;
/* Retrieve the associated PE config address */
ret = rtas_call(ibm_get_config_addr_info2, 4, 2, rets,
- pe->config_addr, BUID_HI(pe->phb->buid),
- BUID_LO(pe->phb->buid), 0);
+ config_addr, BUID_HI(buid), BUID_LO(buid), 0);
if (ret) {
pr_warn("%s: Failed to get address for PHB#%x-PE#%x\n",
- __func__, pe->phb->global_number, pe->config_addr);
+ __func__, pdn->phb->global_number, config_addr);
return 0;
}
@@ -463,11 +465,10 @@ static int pseries_eeh_get_pe_addr(struct eeh_pe *pe)
if (ibm_get_config_addr_info != RTAS_UNKNOWN_SERVICE) {
ret = rtas_call(ibm_get_config_addr_info, 4, 2, rets,
- pe->config_addr, BUID_HI(pe->phb->buid),
- BUID_LO(pe->phb->buid), 0);
+ config_addr, BUID_HI(buid), BUID_LO(buid), 0);
if (ret) {
pr_warn("%s: Failed to get address for PHB#%x-PE#%x\n",
- __func__, pe->phb->global_number, pe->config_addr);
+ __func__, pdn->phb->global_number, config_addr);
return 0;
}
@@ -839,7 +840,6 @@ static struct eeh_ops pseries_eeh_ops = {
.init = pseries_eeh_init,
.probe = pseries_eeh_probe,
.set_option = pseries_eeh_set_option,
- .get_pe_addr = pseries_eeh_get_pe_addr,
.get_state = pseries_eeh_get_state,
.reset = pseries_eeh_reset,
.get_log = pseries_eeh_get_log,
--
2.26.2
^ permalink raw reply related
* [PATCH v2 06/14] powerpc/eeh: Remove VF config space restoration
From: Oliver O'Halloran @ 2020-07-22 4:26 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Alexey Kardashevskiy, Oliver O'Halloran
In-Reply-To: <20200722042628.1425880-1-oohall@gmail.com>
There's a bunch of strange things about this code. First up is that none of
the fields being written to are functional for a VF. The SR-IOV
specification lists then as "Reserved, but OS should preserve" so writing
new values to them doesn't do anything and is clearly wrong from a
correctness perspective.
However, since VFs are designed to be managed by the OS there is an
argument to be made that we should be saving and restoring some parts of
config space. We already sort of do that by saving the first 64 bytes of
config space in the eeh_dev (see eeh_dev->config_space[]). This is
inadequate since it doesn't even consider saving and restoring the PCI
capability structures. However, this is a problem with EEH in general and
that needs to be fixed for non-VF devices too.
There's no real reason to keep around this around so delete it.
Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
v2: no changes
---
arch/powerpc/include/asm/eeh.h | 1 -
arch/powerpc/kernel/eeh.c | 59 --------------------
arch/powerpc/platforms/powernv/eeh-powernv.c | 20 ++-----
arch/powerpc/platforms/pseries/eeh_pseries.c | 26 +--------
4 files changed, 7 insertions(+), 99 deletions(-)
diff --git a/arch/powerpc/include/asm/eeh.h b/arch/powerpc/include/asm/eeh.h
index d619012281bd..c7d5a234bb51 100644
--- a/arch/powerpc/include/asm/eeh.h
+++ b/arch/powerpc/include/asm/eeh.h
@@ -314,7 +314,6 @@ int eeh_pe_reset(struct eeh_pe *pe, int option, bool include_passed);
int eeh_pe_configure(struct eeh_pe *pe);
int eeh_pe_inject_err(struct eeh_pe *pe, int type, int func,
unsigned long addr, unsigned long mask);
-int eeh_restore_vf_config(struct pci_dn *pdn);
/**
* EEH_POSSIBLE_ERROR() -- test for possible MMIO failure.
diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
index 859f76020256..a4df6f6de0bd 100644
--- a/arch/powerpc/kernel/eeh.c
+++ b/arch/powerpc/kernel/eeh.c
@@ -742,65 +742,6 @@ static void eeh_restore_dev_state(struct eeh_dev *edev, void *userdata)
pci_restore_state(pdev);
}
-int eeh_restore_vf_config(struct pci_dn *pdn)
-{
- struct eeh_dev *edev = pdn_to_eeh_dev(pdn);
- u32 devctl, cmd, cap2, aer_capctl;
- int old_mps;
-
- if (edev->pcie_cap) {
- /* Restore MPS */
- old_mps = (ffs(pdn->mps) - 8) << 5;
- eeh_ops->read_config(pdn, edev->pcie_cap + PCI_EXP_DEVCTL,
- 2, &devctl);
- devctl &= ~PCI_EXP_DEVCTL_PAYLOAD;
- devctl |= old_mps;
- eeh_ops->write_config(pdn, edev->pcie_cap + PCI_EXP_DEVCTL,
- 2, devctl);
-
- /* Disable Completion Timeout if possible */
- eeh_ops->read_config(pdn, edev->pcie_cap + PCI_EXP_DEVCAP2,
- 4, &cap2);
- if (cap2 & PCI_EXP_DEVCAP2_COMP_TMOUT_DIS) {
- eeh_ops->read_config(pdn,
- edev->pcie_cap + PCI_EXP_DEVCTL2,
- 4, &cap2);
- cap2 |= PCI_EXP_DEVCTL2_COMP_TMOUT_DIS;
- eeh_ops->write_config(pdn,
- edev->pcie_cap + PCI_EXP_DEVCTL2,
- 4, cap2);
- }
- }
-
- /* Enable SERR and parity checking */
- eeh_ops->read_config(pdn, PCI_COMMAND, 2, &cmd);
- cmd |= (PCI_COMMAND_PARITY | PCI_COMMAND_SERR);
- eeh_ops->write_config(pdn, PCI_COMMAND, 2, cmd);
-
- /* Enable report various errors */
- if (edev->pcie_cap) {
- eeh_ops->read_config(pdn, edev->pcie_cap + PCI_EXP_DEVCTL,
- 2, &devctl);
- devctl &= ~PCI_EXP_DEVCTL_CERE;
- devctl |= (PCI_EXP_DEVCTL_NFERE |
- PCI_EXP_DEVCTL_FERE |
- PCI_EXP_DEVCTL_URRE);
- eeh_ops->write_config(pdn, edev->pcie_cap + PCI_EXP_DEVCTL,
- 2, devctl);
- }
-
- /* Enable ECRC generation and check */
- if (edev->pcie_cap && edev->aer_cap) {
- eeh_ops->read_config(pdn, edev->aer_cap + PCI_ERR_CAP,
- 4, &aer_capctl);
- aer_capctl |= (PCI_ERR_CAP_ECRC_GENE | PCI_ERR_CAP_ECRC_CHKE);
- eeh_ops->write_config(pdn, edev->aer_cap + PCI_ERR_CAP,
- 4, aer_capctl);
- }
-
- return 0;
-}
-
/**
* pcibios_set_pcie_reset_state - Set PCI-E reset state
* @dev: pci device struct
diff --git a/arch/powerpc/platforms/powernv/eeh-powernv.c b/arch/powerpc/platforms/powernv/eeh-powernv.c
index bcd0515d8f79..8f3a7611efc1 100644
--- a/arch/powerpc/platforms/powernv/eeh-powernv.c
+++ b/arch/powerpc/platforms/powernv/eeh-powernv.c
@@ -1629,20 +1629,12 @@ static int pnv_eeh_restore_config(struct pci_dn *pdn)
if (!edev)
return -EEXIST;
- /*
- * We have to restore the PCI config space after reset since the
- * firmware can't see SRIOV VFs.
- *
- * FIXME: The MPS, error routing rules, timeout setting are worthy
- * to be exported by firmware in extendible way.
- */
- if (edev->physfn) {
- ret = eeh_restore_vf_config(pdn);
- } else {
- phb = pdn->phb->private_data;
- ret = opal_pci_reinit(phb->opal_id,
- OPAL_REINIT_PCI_DEV, config_addr);
- }
+ if (edev->physfn)
+ return 0;
+
+ phb = edev->controller->private_data;
+ ret = opal_pci_reinit(phb->opal_id,
+ OPAL_REINIT_PCI_DEV, edev->bdfn);
if (ret) {
pr_warn("%s: Can't reinit PCI dev 0x%x (%lld)\n",
diff --git a/arch/powerpc/platforms/pseries/eeh_pseries.c b/arch/powerpc/platforms/pseries/eeh_pseries.c
index bcc72b9a5309..39cdf447dfa6 100644
--- a/arch/powerpc/platforms/pseries/eeh_pseries.c
+++ b/arch/powerpc/platforms/pseries/eeh_pseries.c
@@ -718,30 +718,6 @@ static int pseries_eeh_write_config(struct pci_dn *pdn, int where, int size, u32
return rtas_write_config(pdn, where, size, val);
}
-static int pseries_eeh_restore_config(struct pci_dn *pdn)
-{
- struct eeh_dev *edev = pdn_to_eeh_dev(pdn);
- s64 ret = 0;
-
- if (!edev)
- return -EEXIST;
-
- /*
- * FIXME: The MPS, error routing rules, timeout setting are worthy
- * to be exported by firmware in extendible way.
- */
- if (edev->physfn)
- ret = eeh_restore_vf_config(pdn);
-
- if (ret) {
- pr_warn("%s: Can't reinit PCI dev 0x%x (%lld)\n",
- __func__, edev->pe_config_addr, ret);
- return -EIO;
- }
-
- return ret;
-}
-
#ifdef CONFIG_PCI_IOV
int pseries_send_allow_unfreeze(struct pci_dn *pdn,
u16 *vf_pe_array, int cur_vfs)
@@ -848,7 +824,7 @@ static struct eeh_ops pseries_eeh_ops = {
.read_config = pseries_eeh_read_config,
.write_config = pseries_eeh_write_config,
.next_error = NULL,
- .restore_config = pseries_eeh_restore_config,
+ .restore_config = NULL, /* NB: configure_bridge() does this */
#ifdef CONFIG_PCI_IOV
.notify_resume = pseries_notify_resume
#endif
--
2.26.2
^ permalink raw reply related
* [PATCH v2 07/14] powerpc/eeh: Pass eeh_dev to eeh_ops->restore_config()
From: Oliver O'Halloran @ 2020-07-22 4:26 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Oliver O'Halloran
In-Reply-To: <20200722042628.1425880-1-oohall@gmail.com>
Mechanical conversion of the eeh_ops interfaces to use eeh_dev to reference
a specific device rather than pci_dn. No functional changes.
Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
---
v2: no changes
---
arch/powerpc/include/asm/eeh.h | 2 +-
arch/powerpc/kernel/eeh.c | 5 ++---
arch/powerpc/kernel/eeh_pe.c | 6 ++----
arch/powerpc/platforms/powernv/eeh-powernv.c | 6 ++----
4 files changed, 7 insertions(+), 12 deletions(-)
diff --git a/arch/powerpc/include/asm/eeh.h b/arch/powerpc/include/asm/eeh.h
index c7d5a234bb51..70a686d731f6 100644
--- a/arch/powerpc/include/asm/eeh.h
+++ b/arch/powerpc/include/asm/eeh.h
@@ -229,7 +229,7 @@ struct eeh_ops {
int (*read_config)(struct pci_dn *pdn, int where, int size, u32 *val);
int (*write_config)(struct pci_dn *pdn, int where, int size, u32 val);
int (*next_error)(struct eeh_pe **pe);
- int (*restore_config)(struct pci_dn *pdn);
+ int (*restore_config)(struct eeh_dev *edev);
int (*notify_resume)(struct pci_dn *pdn);
};
diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
index a4df6f6de0bd..1cef0f4bb2d5 100644
--- a/arch/powerpc/kernel/eeh.c
+++ b/arch/powerpc/kernel/eeh.c
@@ -726,7 +726,6 @@ static void eeh_disable_and_save_dev_state(struct eeh_dev *edev,
static void eeh_restore_dev_state(struct eeh_dev *edev, void *userdata)
{
- struct pci_dn *pdn = eeh_dev_to_pdn(edev);
struct pci_dev *pdev = eeh_dev_to_pci_dev(edev);
struct pci_dev *dev = userdata;
@@ -734,8 +733,8 @@ static void eeh_restore_dev_state(struct eeh_dev *edev, void *userdata)
return;
/* Apply customization from firmware */
- if (pdn && eeh_ops->restore_config)
- eeh_ops->restore_config(pdn);
+ if (eeh_ops->restore_config)
+ eeh_ops->restore_config(edev);
/* The caller should restore state for the specified device */
if (pdev != dev)
diff --git a/arch/powerpc/kernel/eeh_pe.c b/arch/powerpc/kernel/eeh_pe.c
index 177852e39a25..d71493f66917 100644
--- a/arch/powerpc/kernel/eeh_pe.c
+++ b/arch/powerpc/kernel/eeh_pe.c
@@ -843,16 +843,14 @@ static void eeh_restore_device_bars(struct eeh_dev *edev)
*/
static void eeh_restore_one_device_bars(struct eeh_dev *edev, void *flag)
{
- struct pci_dn *pdn = eeh_dev_to_pdn(edev);
-
/* Do special restore for bridges */
if (edev->mode & EEH_DEV_BRIDGE)
eeh_restore_bridge_bars(edev);
else
eeh_restore_device_bars(edev);
- if (eeh_ops->restore_config && pdn)
- eeh_ops->restore_config(pdn);
+ if (eeh_ops->restore_config)
+ eeh_ops->restore_config(edev);
}
/**
diff --git a/arch/powerpc/platforms/powernv/eeh-powernv.c b/arch/powerpc/platforms/powernv/eeh-powernv.c
index 8f3a7611efc1..a41e67f674e6 100644
--- a/arch/powerpc/platforms/powernv/eeh-powernv.c
+++ b/arch/powerpc/platforms/powernv/eeh-powernv.c
@@ -1619,12 +1619,10 @@ static int pnv_eeh_next_error(struct eeh_pe **pe)
return ret;
}
-static int pnv_eeh_restore_config(struct pci_dn *pdn)
+static int pnv_eeh_restore_config(struct eeh_dev *edev)
{
- struct eeh_dev *edev = pdn_to_eeh_dev(pdn);
struct pnv_phb *phb;
s64 ret = 0;
- int config_addr = (pdn->busno << 8) | (pdn->devfn);
if (!edev)
return -EEXIST;
@@ -1638,7 +1636,7 @@ static int pnv_eeh_restore_config(struct pci_dn *pdn)
if (ret) {
pr_warn("%s: Can't reinit PCI dev 0x%x (%lld)\n",
- __func__, config_addr, ret);
+ __func__, edev->bdfn, ret);
return -EIO;
}
--
2.26.2
^ permalink raw reply related
* [PATCH v2 08/14] powerpc/eeh: Pass eeh_dev to eeh_ops->resume_notify()
From: Oliver O'Halloran @ 2020-07-22 4:26 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Oliver O'Halloran
In-Reply-To: <20200722042628.1425880-1-oohall@gmail.com>
Mechanical conversion of the eeh_ops interfaces to use eeh_dev to reference
a specific device rather than pci_dn. No functional changes.
Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
---
v2: no changes
---
arch/powerpc/include/asm/eeh.h | 2 +-
arch/powerpc/kernel/eeh_driver.c | 4 ++--
arch/powerpc/kernel/eeh_sysfs.c | 2 +-
arch/powerpc/platforms/pseries/eeh_pseries.c | 4 +---
4 files changed, 5 insertions(+), 7 deletions(-)
diff --git a/arch/powerpc/include/asm/eeh.h b/arch/powerpc/include/asm/eeh.h
index 70a686d731f6..8045cede0df9 100644
--- a/arch/powerpc/include/asm/eeh.h
+++ b/arch/powerpc/include/asm/eeh.h
@@ -230,7 +230,7 @@ struct eeh_ops {
int (*write_config)(struct pci_dn *pdn, int where, int size, u32 val);
int (*next_error)(struct eeh_pe **pe);
int (*restore_config)(struct eeh_dev *edev);
- int (*notify_resume)(struct pci_dn *pdn);
+ int (*notify_resume)(struct eeh_dev *edev);
};
extern int eeh_subsystem_flags;
diff --git a/arch/powerpc/kernel/eeh_driver.c b/arch/powerpc/kernel/eeh_driver.c
index b70b9273f45a..b84d3cb2532e 100644
--- a/arch/powerpc/kernel/eeh_driver.c
+++ b/arch/powerpc/kernel/eeh_driver.c
@@ -425,8 +425,8 @@ static enum pci_ers_result eeh_report_resume(struct eeh_dev *edev,
pci_uevent_ers(edev->pdev, PCI_ERS_RESULT_RECOVERED);
#ifdef CONFIG_PCI_IOV
- if (eeh_ops->notify_resume && eeh_dev_to_pdn(edev))
- eeh_ops->notify_resume(eeh_dev_to_pdn(edev));
+ if (eeh_ops->notify_resume)
+ eeh_ops->notify_resume(edev);
#endif
return PCI_ERS_RESULT_NONE;
}
diff --git a/arch/powerpc/kernel/eeh_sysfs.c b/arch/powerpc/kernel/eeh_sysfs.c
index 4fb0f1e1017a..429620da73ba 100644
--- a/arch/powerpc/kernel/eeh_sysfs.c
+++ b/arch/powerpc/kernel/eeh_sysfs.c
@@ -99,7 +99,7 @@ static ssize_t eeh_notify_resume_store(struct device *dev,
if (!edev || !edev->pe || !eeh_ops->notify_resume)
return -ENODEV;
- if (eeh_ops->notify_resume(pci_get_pdn(pdev)))
+ if (eeh_ops->notify_resume(edev))
return -EIO;
return count;
diff --git a/arch/powerpc/platforms/pseries/eeh_pseries.c b/arch/powerpc/platforms/pseries/eeh_pseries.c
index 39cdf447dfa6..426227ddee32 100644
--- a/arch/powerpc/platforms/pseries/eeh_pseries.c
+++ b/arch/powerpc/platforms/pseries/eeh_pseries.c
@@ -793,10 +793,8 @@ static int pseries_call_allow_unfreeze(struct eeh_dev *edev)
return rc;
}
-static int pseries_notify_resume(struct pci_dn *pdn)
+static int pseries_notify_resume(struct eeh_dev *edev)
{
- struct eeh_dev *edev = pdn_to_eeh_dev(pdn);
-
if (!edev)
return -EEXIST;
--
2.26.2
^ permalink raw reply related
* [PATCH v2 09/14] powerpc/eeh: Pass eeh_dev to eeh_ops->{read|write}_config()
From: Oliver O'Halloran @ 2020-07-22 4:26 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Oliver O'Halloran
In-Reply-To: <20200722042628.1425880-1-oohall@gmail.com>
Mechanical conversion of the eeh_ops interfaces to use eeh_dev to reference
a specific device rather than pci_dn. No functional changes.
Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
---
v2: no change
---
arch/powerpc/include/asm/eeh.h | 4 +-
arch/powerpc/kernel/eeh.c | 22 +++++----
arch/powerpc/kernel/eeh_pe.c | 47 +++++++++-----------
arch/powerpc/platforms/powernv/eeh-powernv.c | 43 ++++++++++--------
arch/powerpc/platforms/pseries/eeh_pseries.c | 16 ++++---
5 files changed, 68 insertions(+), 64 deletions(-)
diff --git a/arch/powerpc/include/asm/eeh.h b/arch/powerpc/include/asm/eeh.h
index 8045cede0df9..d16d5b59dd22 100644
--- a/arch/powerpc/include/asm/eeh.h
+++ b/arch/powerpc/include/asm/eeh.h
@@ -226,8 +226,8 @@ struct eeh_ops {
int (*configure_bridge)(struct eeh_pe *pe);
int (*err_inject)(struct eeh_pe *pe, int type, int func,
unsigned long addr, unsigned long mask);
- int (*read_config)(struct pci_dn *pdn, int where, int size, u32 *val);
- int (*write_config)(struct pci_dn *pdn, int where, int size, u32 val);
+ int (*read_config)(struct eeh_dev *edev, int where, int size, u32 *val);
+ int (*write_config)(struct eeh_dev *edev, int where, int size, u32 val);
int (*next_error)(struct eeh_pe **pe);
int (*restore_config)(struct eeh_dev *edev);
int (*notify_resume)(struct eeh_dev *edev);
diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
index 1cef0f4bb2d5..1a12c8bdf61e 100644
--- a/arch/powerpc/kernel/eeh.c
+++ b/arch/powerpc/kernel/eeh.c
@@ -185,21 +185,21 @@ static size_t eeh_dump_dev_log(struct eeh_dev *edev, char *buf, size_t len)
pdn->phb->global_number, pdn->busno,
PCI_SLOT(pdn->devfn), PCI_FUNC(pdn->devfn));
- eeh_ops->read_config(pdn, PCI_VENDOR_ID, 4, &cfg);
+ eeh_ops->read_config(edev, PCI_VENDOR_ID, 4, &cfg);
n += scnprintf(buf+n, len-n, "dev/vend:%08x\n", cfg);
pr_warn("EEH: PCI device/vendor: %08x\n", cfg);
- eeh_ops->read_config(pdn, PCI_COMMAND, 4, &cfg);
+ eeh_ops->read_config(edev, PCI_COMMAND, 4, &cfg);
n += scnprintf(buf+n, len-n, "cmd/stat:%x\n", cfg);
pr_warn("EEH: PCI cmd/status register: %08x\n", cfg);
/* Gather bridge-specific registers */
if (edev->mode & EEH_DEV_BRIDGE) {
- eeh_ops->read_config(pdn, PCI_SEC_STATUS, 2, &cfg);
+ eeh_ops->read_config(edev, PCI_SEC_STATUS, 2, &cfg);
n += scnprintf(buf+n, len-n, "sec stat:%x\n", cfg);
pr_warn("EEH: Bridge secondary status: %04x\n", cfg);
- eeh_ops->read_config(pdn, PCI_BRIDGE_CONTROL, 2, &cfg);
+ eeh_ops->read_config(edev, PCI_BRIDGE_CONTROL, 2, &cfg);
n += scnprintf(buf+n, len-n, "brdg ctl:%x\n", cfg);
pr_warn("EEH: Bridge control: %04x\n", cfg);
}
@@ -207,11 +207,11 @@ static size_t eeh_dump_dev_log(struct eeh_dev *edev, char *buf, size_t len)
/* Dump out the PCI-X command and status regs */
cap = edev->pcix_cap;
if (cap) {
- eeh_ops->read_config(pdn, cap, 4, &cfg);
+ eeh_ops->read_config(edev, cap, 4, &cfg);
n += scnprintf(buf+n, len-n, "pcix-cmd:%x\n", cfg);
pr_warn("EEH: PCI-X cmd: %08x\n", cfg);
- eeh_ops->read_config(pdn, cap+4, 4, &cfg);
+ eeh_ops->read_config(edev, cap+4, 4, &cfg);
n += scnprintf(buf+n, len-n, "pcix-stat:%x\n", cfg);
pr_warn("EEH: PCI-X status: %08x\n", cfg);
}
@@ -223,7 +223,7 @@ static size_t eeh_dump_dev_log(struct eeh_dev *edev, char *buf, size_t len)
pr_warn("EEH: PCI-E capabilities and status follow:\n");
for (i=0; i<=8; i++) {
- eeh_ops->read_config(pdn, cap+4*i, 4, &cfg);
+ eeh_ops->read_config(edev, cap+4*i, 4, &cfg);
n += scnprintf(buf+n, len-n, "%02x:%x\n", 4*i, cfg);
if ((i % 4) == 0) {
@@ -250,7 +250,7 @@ static size_t eeh_dump_dev_log(struct eeh_dev *edev, char *buf, size_t len)
pr_warn("EEH: PCI-E AER capability register set follows:\n");
for (i=0; i<=13; i++) {
- eeh_ops->read_config(pdn, cap+4*i, 4, &cfg);
+ eeh_ops->read_config(edev, cap+4*i, 4, &cfg);
n += scnprintf(buf+n, len-n, "%02x:%x\n", 4*i, cfg);
if ((i % 4) == 0) {
@@ -917,15 +917,13 @@ int eeh_pe_reset_full(struct eeh_pe *pe, bool include_passed)
*/
void eeh_save_bars(struct eeh_dev *edev)
{
- struct pci_dn *pdn;
int i;
- pdn = eeh_dev_to_pdn(edev);
- if (!pdn)
+ if (!edev)
return;
for (i = 0; i < 16; i++)
- eeh_ops->read_config(pdn, i * 4, 4, &edev->config_space[i]);
+ eeh_ops->read_config(edev, i * 4, 4, &edev->config_space[i]);
/*
* For PCI bridges including root port, we need enable bus
diff --git a/arch/powerpc/kernel/eeh_pe.c b/arch/powerpc/kernel/eeh_pe.c
index d71493f66917..f20fb0ee6aec 100644
--- a/arch/powerpc/kernel/eeh_pe.c
+++ b/arch/powerpc/kernel/eeh_pe.c
@@ -698,7 +698,6 @@ void eeh_pe_state_clear(struct eeh_pe *root, int state, bool include_passed)
*/
static void eeh_bridge_check_link(struct eeh_dev *edev)
{
- struct pci_dn *pdn = eeh_dev_to_pdn(edev);
int cap;
uint32_t val;
int timeout = 0;
@@ -714,32 +713,32 @@ static void eeh_bridge_check_link(struct eeh_dev *edev)
/* Check slot status */
cap = edev->pcie_cap;
- eeh_ops->read_config(pdn, cap + PCI_EXP_SLTSTA, 2, &val);
+ eeh_ops->read_config(edev, cap + PCI_EXP_SLTSTA, 2, &val);
if (!(val & PCI_EXP_SLTSTA_PDS)) {
eeh_edev_dbg(edev, "No card in the slot (0x%04x) !\n", val);
return;
}
/* Check power status if we have the capability */
- eeh_ops->read_config(pdn, cap + PCI_EXP_SLTCAP, 2, &val);
+ eeh_ops->read_config(edev, cap + PCI_EXP_SLTCAP, 2, &val);
if (val & PCI_EXP_SLTCAP_PCP) {
- eeh_ops->read_config(pdn, cap + PCI_EXP_SLTCTL, 2, &val);
+ eeh_ops->read_config(edev, cap + PCI_EXP_SLTCTL, 2, &val);
if (val & PCI_EXP_SLTCTL_PCC) {
eeh_edev_dbg(edev, "In power-off state, power it on ...\n");
val &= ~(PCI_EXP_SLTCTL_PCC | PCI_EXP_SLTCTL_PIC);
val |= (0x0100 & PCI_EXP_SLTCTL_PIC);
- eeh_ops->write_config(pdn, cap + PCI_EXP_SLTCTL, 2, val);
+ eeh_ops->write_config(edev, cap + PCI_EXP_SLTCTL, 2, val);
msleep(2 * 1000);
}
}
/* Enable link */
- eeh_ops->read_config(pdn, cap + PCI_EXP_LNKCTL, 2, &val);
+ eeh_ops->read_config(edev, cap + PCI_EXP_LNKCTL, 2, &val);
val &= ~PCI_EXP_LNKCTL_LD;
- eeh_ops->write_config(pdn, cap + PCI_EXP_LNKCTL, 2, val);
+ eeh_ops->write_config(edev, cap + PCI_EXP_LNKCTL, 2, val);
/* Check link */
- eeh_ops->read_config(pdn, cap + PCI_EXP_LNKCAP, 4, &val);
+ eeh_ops->read_config(edev, cap + PCI_EXP_LNKCAP, 4, &val);
if (!(val & PCI_EXP_LNKCAP_DLLLARC)) {
eeh_edev_dbg(edev, "No link reporting capability (0x%08x) \n", val);
msleep(1000);
@@ -752,7 +751,7 @@ static void eeh_bridge_check_link(struct eeh_dev *edev)
msleep(20);
timeout += 20;
- eeh_ops->read_config(pdn, cap + PCI_EXP_LNKSTA, 2, &val);
+ eeh_ops->read_config(edev, cap + PCI_EXP_LNKSTA, 2, &val);
if (val & PCI_EXP_LNKSTA_DLLLA)
break;
}
@@ -769,7 +768,6 @@ static void eeh_bridge_check_link(struct eeh_dev *edev)
static void eeh_restore_bridge_bars(struct eeh_dev *edev)
{
- struct pci_dn *pdn = eeh_dev_to_pdn(edev);
int i;
/*
@@ -777,20 +775,20 @@ static void eeh_restore_bridge_bars(struct eeh_dev *edev)
* Bus numbers and windows: 0x18 - 0x30
*/
for (i = 4; i < 13; i++)
- eeh_ops->write_config(pdn, i*4, 4, edev->config_space[i]);
+ eeh_ops->write_config(edev, i*4, 4, edev->config_space[i]);
/* Rom: 0x38 */
- eeh_ops->write_config(pdn, 14*4, 4, edev->config_space[14]);
+ eeh_ops->write_config(edev, 14*4, 4, edev->config_space[14]);
/* Cache line & Latency timer: 0xC 0xD */
- eeh_ops->write_config(pdn, PCI_CACHE_LINE_SIZE, 1,
+ eeh_ops->write_config(edev, PCI_CACHE_LINE_SIZE, 1,
SAVED_BYTE(PCI_CACHE_LINE_SIZE));
- eeh_ops->write_config(pdn, PCI_LATENCY_TIMER, 1,
- SAVED_BYTE(PCI_LATENCY_TIMER));
+ eeh_ops->write_config(edev, PCI_LATENCY_TIMER, 1,
+ SAVED_BYTE(PCI_LATENCY_TIMER));
/* Max latency, min grant, interrupt ping and line: 0x3C */
- eeh_ops->write_config(pdn, 15*4, 4, edev->config_space[15]);
+ eeh_ops->write_config(edev, 15*4, 4, edev->config_space[15]);
/* PCI Command: 0x4 */
- eeh_ops->write_config(pdn, PCI_COMMAND, 4, edev->config_space[1] |
+ eeh_ops->write_config(edev, PCI_COMMAND, 4, edev->config_space[1] |
PCI_COMMAND_MEMORY | PCI_COMMAND_MASTER);
/* Check the PCIe link is ready */
@@ -799,28 +797,27 @@ static void eeh_restore_bridge_bars(struct eeh_dev *edev)
static void eeh_restore_device_bars(struct eeh_dev *edev)
{
- struct pci_dn *pdn = eeh_dev_to_pdn(edev);
int i;
u32 cmd;
for (i = 4; i < 10; i++)
- eeh_ops->write_config(pdn, i*4, 4, edev->config_space[i]);
+ eeh_ops->write_config(edev, i*4, 4, edev->config_space[i]);
/* 12 == Expansion ROM Address */
- eeh_ops->write_config(pdn, 12*4, 4, edev->config_space[12]);
+ eeh_ops->write_config(edev, 12*4, 4, edev->config_space[12]);
- eeh_ops->write_config(pdn, PCI_CACHE_LINE_SIZE, 1,
+ eeh_ops->write_config(edev, PCI_CACHE_LINE_SIZE, 1,
SAVED_BYTE(PCI_CACHE_LINE_SIZE));
- eeh_ops->write_config(pdn, PCI_LATENCY_TIMER, 1,
+ eeh_ops->write_config(edev, PCI_LATENCY_TIMER, 1,
SAVED_BYTE(PCI_LATENCY_TIMER));
/* max latency, min grant, interrupt pin and line */
- eeh_ops->write_config(pdn, 15*4, 4, edev->config_space[15]);
+ eeh_ops->write_config(edev, 15*4, 4, edev->config_space[15]);
/*
* Restore PERR & SERR bits, some devices require it,
* don't touch the other command bits
*/
- eeh_ops->read_config(pdn, PCI_COMMAND, 4, &cmd);
+ eeh_ops->read_config(edev, PCI_COMMAND, 4, &cmd);
if (edev->config_space[1] & PCI_COMMAND_PARITY)
cmd |= PCI_COMMAND_PARITY;
else
@@ -829,7 +826,7 @@ static void eeh_restore_device_bars(struct eeh_dev *edev)
cmd |= PCI_COMMAND_SERR;
else
cmd &= ~PCI_COMMAND_SERR;
- eeh_ops->write_config(pdn, PCI_COMMAND, 4, cmd);
+ eeh_ops->write_config(edev, PCI_COMMAND, 4, cmd);
}
/**
diff --git a/arch/powerpc/platforms/powernv/eeh-powernv.c b/arch/powerpc/platforms/powernv/eeh-powernv.c
index a41e67f674e6..c9f2f454d053 100644
--- a/arch/powerpc/platforms/powernv/eeh-powernv.c
+++ b/arch/powerpc/platforms/powernv/eeh-powernv.c
@@ -838,32 +838,32 @@ static int __pnv_eeh_bridge_reset(struct pci_dev *dev, int option)
case EEH_RESET_HOT:
/* Don't report linkDown event */
if (aer) {
- eeh_ops->read_config(pdn, aer + PCI_ERR_UNCOR_MASK,
+ eeh_ops->read_config(edev, aer + PCI_ERR_UNCOR_MASK,
4, &ctrl);
ctrl |= PCI_ERR_UNC_SURPDN;
- eeh_ops->write_config(pdn, aer + PCI_ERR_UNCOR_MASK,
+ eeh_ops->write_config(edev, aer + PCI_ERR_UNCOR_MASK,
4, ctrl);
}
- eeh_ops->read_config(pdn, PCI_BRIDGE_CONTROL, 2, &ctrl);
+ eeh_ops->read_config(edev, PCI_BRIDGE_CONTROL, 2, &ctrl);
ctrl |= PCI_BRIDGE_CTL_BUS_RESET;
- eeh_ops->write_config(pdn, PCI_BRIDGE_CONTROL, 2, ctrl);
+ eeh_ops->write_config(edev, PCI_BRIDGE_CONTROL, 2, ctrl);
msleep(EEH_PE_RST_HOLD_TIME);
break;
case EEH_RESET_DEACTIVATE:
- eeh_ops->read_config(pdn, PCI_BRIDGE_CONTROL, 2, &ctrl);
+ eeh_ops->read_config(edev, PCI_BRIDGE_CONTROL, 2, &ctrl);
ctrl &= ~PCI_BRIDGE_CTL_BUS_RESET;
- eeh_ops->write_config(pdn, PCI_BRIDGE_CONTROL, 2, ctrl);
+ eeh_ops->write_config(edev, PCI_BRIDGE_CONTROL, 2, ctrl);
msleep(EEH_PE_RST_SETTLE_TIME);
/* Continue reporting linkDown event */
if (aer) {
- eeh_ops->read_config(pdn, aer + PCI_ERR_UNCOR_MASK,
+ eeh_ops->read_config(edev, aer + PCI_ERR_UNCOR_MASK,
4, &ctrl);
ctrl &= ~PCI_ERR_UNC_SURPDN;
- eeh_ops->write_config(pdn, aer + PCI_ERR_UNCOR_MASK,
+ eeh_ops->write_config(edev, aer + PCI_ERR_UNCOR_MASK,
4, ctrl);
}
@@ -932,11 +932,12 @@ void pnv_pci_reset_secondary_bus(struct pci_dev *dev)
static void pnv_eeh_wait_for_pending(struct pci_dn *pdn, const char *type,
int pos, u16 mask)
{
+ struct eeh_dev *edev = pdn->edev;
int i, status = 0;
/* Wait for Transaction Pending bit to be cleared */
for (i = 0; i < 4; i++) {
- eeh_ops->read_config(pdn, pos, 2, &status);
+ eeh_ops->read_config(edev, pos, 2, &status);
if (!(status & mask))
return;
@@ -957,7 +958,7 @@ static int pnv_eeh_do_flr(struct pci_dn *pdn, int option)
if (WARN_ON(!edev->pcie_cap))
return -ENOTTY;
- eeh_ops->read_config(pdn, edev->pcie_cap + PCI_EXP_DEVCAP, 4, ®);
+ eeh_ops->read_config(edev, edev->pcie_cap + PCI_EXP_DEVCAP, 4, ®);
if (!(reg & PCI_EXP_DEVCAP_FLR))
return -ENOTTY;
@@ -967,18 +968,18 @@ static int pnv_eeh_do_flr(struct pci_dn *pdn, int option)
pnv_eeh_wait_for_pending(pdn, "",
edev->pcie_cap + PCI_EXP_DEVSTA,
PCI_EXP_DEVSTA_TRPND);
- eeh_ops->read_config(pdn, edev->pcie_cap + PCI_EXP_DEVCTL,
+ eeh_ops->read_config(edev, edev->pcie_cap + PCI_EXP_DEVCTL,
4, ®);
reg |= PCI_EXP_DEVCTL_BCR_FLR;
- eeh_ops->write_config(pdn, edev->pcie_cap + PCI_EXP_DEVCTL,
+ eeh_ops->write_config(edev, edev->pcie_cap + PCI_EXP_DEVCTL,
4, reg);
msleep(EEH_PE_RST_HOLD_TIME);
break;
case EEH_RESET_DEACTIVATE:
- eeh_ops->read_config(pdn, edev->pcie_cap + PCI_EXP_DEVCTL,
+ eeh_ops->read_config(edev, edev->pcie_cap + PCI_EXP_DEVCTL,
4, ®);
reg &= ~PCI_EXP_DEVCTL_BCR_FLR;
- eeh_ops->write_config(pdn, edev->pcie_cap + PCI_EXP_DEVCTL,
+ eeh_ops->write_config(edev, edev->pcie_cap + PCI_EXP_DEVCTL,
4, reg);
msleep(EEH_PE_RST_SETTLE_TIME);
break;
@@ -995,7 +996,7 @@ static int pnv_eeh_do_af_flr(struct pci_dn *pdn, int option)
if (WARN_ON(!edev->af_cap))
return -ENOTTY;
- eeh_ops->read_config(pdn, edev->af_cap + PCI_AF_CAP, 1, &cap);
+ eeh_ops->read_config(edev, edev->af_cap + PCI_AF_CAP, 1, &cap);
if (!(cap & PCI_AF_CAP_TP) || !(cap & PCI_AF_CAP_FLR))
return -ENOTTY;
@@ -1010,12 +1011,12 @@ static int pnv_eeh_do_af_flr(struct pci_dn *pdn, int option)
pnv_eeh_wait_for_pending(pdn, "AF",
edev->af_cap + PCI_AF_CTRL,
PCI_AF_STATUS_TP << 8);
- eeh_ops->write_config(pdn, edev->af_cap + PCI_AF_CTRL,
+ eeh_ops->write_config(edev, edev->af_cap + PCI_AF_CTRL,
1, PCI_AF_CTRL_FLR);
msleep(EEH_PE_RST_HOLD_TIME);
break;
case EEH_RESET_DEACTIVATE:
- eeh_ops->write_config(pdn, edev->af_cap + PCI_AF_CTRL, 1, 0);
+ eeh_ops->write_config(edev, edev->af_cap + PCI_AF_CTRL, 1, 0);
msleep(EEH_PE_RST_SETTLE_TIME);
break;
}
@@ -1249,9 +1250,11 @@ static inline bool pnv_eeh_cfg_blocked(struct pci_dn *pdn)
return false;
}
-static int pnv_eeh_read_config(struct pci_dn *pdn,
+static int pnv_eeh_read_config(struct eeh_dev *edev,
int where, int size, u32 *val)
{
+ struct pci_dn *pdn = eeh_dev_to_pdn(edev);
+
if (!pdn)
return PCIBIOS_DEVICE_NOT_FOUND;
@@ -1263,9 +1266,11 @@ static int pnv_eeh_read_config(struct pci_dn *pdn,
return pnv_pci_cfg_read(pdn, where, size, val);
}
-static int pnv_eeh_write_config(struct pci_dn *pdn,
+static int pnv_eeh_write_config(struct eeh_dev *edev,
int where, int size, u32 val)
{
+ struct pci_dn *pdn = eeh_dev_to_pdn(edev);
+
if (!pdn)
return PCIBIOS_DEVICE_NOT_FOUND;
diff --git a/arch/powerpc/platforms/pseries/eeh_pseries.c b/arch/powerpc/platforms/pseries/eeh_pseries.c
index 426227ddee32..b981332db873 100644
--- a/arch/powerpc/platforms/pseries/eeh_pseries.c
+++ b/arch/powerpc/platforms/pseries/eeh_pseries.c
@@ -692,29 +692,33 @@ static int pseries_eeh_configure_bridge(struct eeh_pe *pe)
/**
* pseries_eeh_read_config - Read PCI config space
- * @pdn: PCI device node
- * @where: PCI address
+ * @edev: EEH device handle
+ * @where: PCI config space offset
* @size: size to read
* @val: return value
*
* Read config space from the speicifed device
*/
-static int pseries_eeh_read_config(struct pci_dn *pdn, int where, int size, u32 *val)
+static int pseries_eeh_read_config(struct eeh_dev *edev, int where, int size, u32 *val)
{
+ struct pci_dn *pdn = eeh_dev_to_pdn(edev);
+
return rtas_read_config(pdn, where, size, val);
}
/**
* pseries_eeh_write_config - Write PCI config space
- * @pdn: PCI device node
- * @where: PCI address
+ * @edev: EEH device handle
+ * @where: PCI config space offset
* @size: size to write
* @val: value to be written
*
* Write config space to the specified device
*/
-static int pseries_eeh_write_config(struct pci_dn *pdn, int where, int size, u32 val)
+static int pseries_eeh_write_config(struct eeh_dev *edev, int where, int size, u32 val)
{
+ struct pci_dn *pdn = eeh_dev_to_pdn(edev);
+
return rtas_write_config(pdn, where, size, val);
}
--
2.26.2
^ permalink raw reply related
* [PATCH v2 10/14] powerpc/eeh: Remove spurious use of pci_dn in eeh_dump_dev_log
From: Oliver O'Halloran @ 2020-07-22 4:26 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Oliver O'Halloran
In-Reply-To: <20200722042628.1425880-1-oohall@gmail.com>
Retrieve the domain, bus, device, and function numbers from the edev.
Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
---
v2: no change
---
arch/powerpc/kernel/eeh.c | 14 ++++----------
1 file changed, 4 insertions(+), 10 deletions(-)
diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
index 1a12c8bdf61e..f203ffc5c57d 100644
--- a/arch/powerpc/kernel/eeh.c
+++ b/arch/powerpc/kernel/eeh.c
@@ -167,23 +167,17 @@ void eeh_show_enabled(void)
*/
static size_t eeh_dump_dev_log(struct eeh_dev *edev, char *buf, size_t len)
{
- struct pci_dn *pdn = eeh_dev_to_pdn(edev);
u32 cfg;
int cap, i;
int n = 0, l = 0;
char buffer[128];
- if (!pdn) {
- pr_warn("EEH: Note: No error log for absent device.\n");
- return 0;
- }
-
n += scnprintf(buf+n, len-n, "%04x:%02x:%02x.%01x\n",
- pdn->phb->global_number, pdn->busno,
- PCI_SLOT(pdn->devfn), PCI_FUNC(pdn->devfn));
+ edev->pe->phb->global_number, edev->bdfn >> 8,
+ PCI_SLOT(edev->bdfn), PCI_FUNC(edev->bdfn));
pr_warn("EEH: of node=%04x:%02x:%02x.%01x\n",
- pdn->phb->global_number, pdn->busno,
- PCI_SLOT(pdn->devfn), PCI_FUNC(pdn->devfn));
+ edev->pe->phb->global_number, edev->bdfn >> 8,
+ PCI_SLOT(edev->bdfn), PCI_FUNC(edev->bdfn));
eeh_ops->read_config(edev, PCI_VENDOR_ID, 4, &cfg);
n += scnprintf(buf+n, len-n, "dev/vend:%08x\n", cfg);
--
2.26.2
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox