* [PATCH 3/3] powerpc/mm: print hash info in a helper
From: Christophe Leroy @ 2019-03-20 8:06 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <67ed3a31a16b9ece870ec55139e83c86d4d9ce60.1553069097.git.christophe.leroy@c-s.fr>
Reduce #ifdef mess by defining a helper to print
hash info at startup.
In the meantime, remove the display of hash table address
to reduce leak of non necessary information.
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
arch/powerpc/kernel/setup-common.c | 19 +------------------
arch/powerpc/mm/hash_utils_64.c | 8 ++++++++
arch/powerpc/mm/mmu_decl.h | 5 ++++-
arch/powerpc/mm/ppc_mmu_32.c | 9 ++++++++-
4 files changed, 21 insertions(+), 20 deletions(-)
diff --git a/arch/powerpc/kernel/setup-common.c b/arch/powerpc/kernel/setup-common.c
index 2e5dfb6e0823..f24a74f7912d 100644
--- a/arch/powerpc/kernel/setup-common.c
+++ b/arch/powerpc/kernel/setup-common.c
@@ -799,12 +799,6 @@ void arch_setup_pdev_archdata(struct platform_device *pdev)
static __init void print_system_info(void)
{
pr_info("-----------------------------------------------------\n");
-#ifdef CONFIG_PPC_BOOK3S_64
- pr_info("ppc64_pft_size = 0x%llx\n", ppc64_pft_size);
-#endif
-#ifdef CONFIG_PPC_BOOK3S_32
- pr_info("Hash_size = 0x%lx\n", Hash_size);
-#endif
pr_info("phys_mem_size = 0x%llx\n",
(unsigned long long)memblock_phys_mem_size());
@@ -826,18 +820,7 @@ static __init void print_system_info(void)
pr_info("firmware_features = 0x%016lx\n", powerpc_firmware_features);
#endif
-#ifdef CONFIG_PPC_BOOK3S_64
- if (htab_address)
- pr_info("htab_address = 0x%p\n", htab_address);
- if (htab_hash_mask)
- pr_info("htab_hash_mask = 0x%lx\n", htab_hash_mask);
-#endif
-#ifdef CONFIG_PPC_BOOK3S_32
- if (Hash)
- pr_info("Hash = 0x%p\n", Hash);
- if (Hash_mask)
- pr_info("Hash_mask = 0x%lx\n", Hash_mask);
-#endif
+ print_system_hash_info();
if (PHYSICAL_START > 0)
pr_info("physical_start = 0x%llx\n",
diff --git a/arch/powerpc/mm/hash_utils_64.c b/arch/powerpc/mm/hash_utils_64.c
index 0a4f939a8161..017380b890bb 100644
--- a/arch/powerpc/mm/hash_utils_64.c
+++ b/arch/powerpc/mm/hash_utils_64.c
@@ -1909,3 +1909,11 @@ static int __init hash64_debugfs(void)
}
machine_device_initcall(pseries, hash64_debugfs);
#endif /* CONFIG_DEBUG_FS */
+
+void __init print_system_hash_info(void)
+{
+ pr_info("ppc64_pft_size = 0x%llx\n", ppc64_pft_size);
+
+ if (htab_hash_mask)
+ pr_info("htab_hash_mask = 0x%lx\n", htab_hash_mask);
+}
diff --git a/arch/powerpc/mm/mmu_decl.h b/arch/powerpc/mm/mmu_decl.h
index f7f1374ba3ee..dc617ade83ab 100644
--- a/arch/powerpc/mm/mmu_decl.h
+++ b/arch/powerpc/mm/mmu_decl.h
@@ -83,6 +83,8 @@ static inline void _tlbivax_bcast(unsigned long address, unsigned int pid,
}
#endif
+static inline void print_system_hash_info(void) {}
+
#else /* CONFIG_PPC_MMU_NOHASH */
extern void hash_preload(struct mm_struct *mm, unsigned long ea,
@@ -92,6 +94,8 @@ extern void hash_preload(struct mm_struct *mm, unsigned long ea,
extern void _tlbie(unsigned long address);
extern void _tlbia(void);
+void print_system_hash_info(void);
+
#endif /* CONFIG_PPC_MMU_NOHASH */
#ifdef CONFIG_PPC32
@@ -105,7 +109,6 @@ extern unsigned int rtas_data, rtas_size;
struct hash_pte;
extern struct hash_pte *Hash;
-extern unsigned long Hash_size, Hash_mask;
#endif /* CONFIG_PPC32 */
diff --git a/arch/powerpc/mm/ppc_mmu_32.c b/arch/powerpc/mm/ppc_mmu_32.c
index 088f14d57cce..864096489b6d 100644
--- a/arch/powerpc/mm/ppc_mmu_32.c
+++ b/arch/powerpc/mm/ppc_mmu_32.c
@@ -37,7 +37,7 @@
#include "mmu_decl.h"
struct hash_pte *Hash;
-unsigned long Hash_size, Hash_mask;
+static unsigned long Hash_size, Hash_mask;
unsigned long _SDR1;
struct ppc_bat BATS[8][2]; /* 8 pairs of IBAT, DBAT */
@@ -392,3 +392,10 @@ void setup_initial_memory_limit(phys_addr_t first_memblock_base,
else /* Anything else has 256M mapped */
memblock_set_current_limit(min_t(u64, first_memblock_size, 0x10000000));
}
+
+void __init print_system_hash_info(void)
+{
+ pr_info("Hash_size = 0x%lx\n", Hash_size);
+ if (Hash_mask)
+ pr_info("Hash_mask = 0x%lx\n", Hash_mask);
+}
--
2.13.3
^ permalink raw reply related
* [PATCH 2/3] powerpc/32s: don't try to print hash table address.
From: Christophe Leroy @ 2019-03-20 8:06 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <67ed3a31a16b9ece870ec55139e83c86d4d9ce60.1553069097.git.christophe.leroy@c-s.fr>
Due to %p, (ptrval) is printed in lieu of the hash table address.
showing the hash table address isn't an operationnal need so just
don't print it.
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
arch/powerpc/mm/ppc_mmu_32.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/mm/ppc_mmu_32.c b/arch/powerpc/mm/ppc_mmu_32.c
index 302409fcfbc7..088f14d57cce 100644
--- a/arch/powerpc/mm/ppc_mmu_32.c
+++ b/arch/powerpc/mm/ppc_mmu_32.c
@@ -345,8 +345,8 @@ void __init MMU_init_hw(void)
__func__, Hash_size, Hash_size);
_SDR1 = __pa(Hash) | SDR1_LOW_BITS;
- printk("Total memory = %lldMB; using %ldkB for hash table (at %p)\n",
- (unsigned long long)(total_memory >> 20), Hash_size >> 10, Hash);
+ pr_info("Total memory = %lldMB; using %ldkB for hash table\n",
+ (unsigned long long)(total_memory >> 20), Hash_size >> 10);
/*
--
2.13.3
^ permalink raw reply related
* [PATCH 1/3] powerpc/32s: drop Hash_end
From: Christophe Leroy @ 2019-03-20 8:06 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
Cc: linuxppc-dev, linux-kernel
Hash_end has never been used, drop it.
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
arch/powerpc/mm/mmu_decl.h | 2 +-
arch/powerpc/mm/ppc_mmu_32.c | 4 +---
2 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/mm/mmu_decl.h b/arch/powerpc/mm/mmu_decl.h
index 74ff61dabcb1..f7f1374ba3ee 100644
--- a/arch/powerpc/mm/mmu_decl.h
+++ b/arch/powerpc/mm/mmu_decl.h
@@ -104,7 +104,7 @@ extern int __map_without_bats;
extern unsigned int rtas_data, rtas_size;
struct hash_pte;
-extern struct hash_pte *Hash, *Hash_end;
+extern struct hash_pte *Hash;
extern unsigned long Hash_size, Hash_mask;
#endif /* CONFIG_PPC32 */
diff --git a/arch/powerpc/mm/ppc_mmu_32.c b/arch/powerpc/mm/ppc_mmu_32.c
index f29d2f118b44..302409fcfbc7 100644
--- a/arch/powerpc/mm/ppc_mmu_32.c
+++ b/arch/powerpc/mm/ppc_mmu_32.c
@@ -36,7 +36,7 @@
#include "mmu_decl.h"
-struct hash_pte *Hash, *Hash_end;
+struct hash_pte *Hash;
unsigned long Hash_size, Hash_mask;
unsigned long _SDR1;
@@ -345,8 +345,6 @@ void __init MMU_init_hw(void)
__func__, Hash_size, Hash_size);
_SDR1 = __pa(Hash) | SDR1_LOW_BITS;
- Hash_end = (struct hash_pte *) ((unsigned long)Hash + Hash_size);
-
printk("Total memory = %lldMB; using %ldkB for hash table (at %p)\n",
(unsigned long long)(total_memory >> 20), Hash_size >> 10, Hash);
--
2.13.3
^ permalink raw reply related
* Re: [PATCH v3 06/17] KVM: PPC: Book3S HV: XIVE: add controls for the EQ configuration
From: Cédric Le Goater @ 2019-03-20 6:44 UTC (permalink / raw)
To: David Gibson; +Cc: linuxppc-dev, Paul Mackerras, kvm, kvm-ppc
In-Reply-To: <20190320034429.GF31018@umbus.fritz.box>
On 3/20/19 4:44 AM, David Gibson wrote:
> On Tue, Mar 19, 2019 at 04:47:20PM +0100, Cédric Le Goater wrote:
>> On 3/19/19 5:54 AM, David Gibson wrote:
>>> On Mon, Mar 18, 2019 at 03:12:10PM +0100, Cédric Le Goater wrote:
>>>> On 3/18/19 4:23 AM, David Gibson wrote:
>>>>> On Fri, Mar 15, 2019 at 01:05:58PM +0100, Cédric Le Goater wrote:
>>>>>> These controls will be used by the H_INT_SET_QUEUE_CONFIG and
>>>>>> H_INT_GET_QUEUE_CONFIG hcalls from QEMU to configure the underlying
>>>>>> Event Queue in the XIVE IC. They will also be used to restore the
>>>>>> configuration of the XIVE EQs and to capture the internal run-time
>>>>>> state of the EQs. Both 'get' and 'set' rely on an OPAL call to access
>>>>>> the EQ toggle bit and EQ index which are updated by the XIVE IC when
>>>>>> event notifications are enqueued in the EQ.
>>>>>>
>>>>>> The value of the guest physical address of the event queue is saved in
>>>>>> the XIVE internal xive_q structure for later use. That is when
>>>>>> migration needs to mark the EQ pages dirty to capture a consistent
>>>>>> memory state of the VM.
>>>>>>
>>>>>> To be noted that H_INT_SET_QUEUE_CONFIG does not require the extra
>>>>>> OPAL call setting the EQ toggle bit and EQ index to configure the EQ,
>>>>>> but restoring the EQ state will.
>>>>>>
>>>>>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>>>>>> ---
>>>>>>
>>>>>> Changes since v2 :
>>>>>>
>>>>>> - fixed comments on the KVM device attribute definitions
>>>>>> - fixed check on supported EQ size to restrict to 64K pages
>>>>>> - checked kvm_eq.flags that need to be zero
>>>>>> - removed the OPAL call when EQ qtoggle bit and index are zero.
>>>>>>
>>>>>> arch/powerpc/include/asm/xive.h | 2 +
>>>>>> arch/powerpc/include/uapi/asm/kvm.h | 21 ++
>>>>>> arch/powerpc/kvm/book3s_xive.h | 2 +
>>>>>> arch/powerpc/kvm/book3s_xive.c | 15 +-
>>>>>> arch/powerpc/kvm/book3s_xive_native.c | 232 +++++++++++++++++++++
>>>>>> Documentation/virtual/kvm/devices/xive.txt | 31 +++
>>>>>> 6 files changed, 297 insertions(+), 6 deletions(-)
>>>>>>
>>>>>> diff --git a/arch/powerpc/include/asm/xive.h b/arch/powerpc/include/asm/xive.h
>>>>>> index b579a943407b..46891f321606 100644
>>>>>> --- a/arch/powerpc/include/asm/xive.h
>>>>>> +++ b/arch/powerpc/include/asm/xive.h
>>>>>> @@ -73,6 +73,8 @@ struct xive_q {
>>>>>> u32 esc_irq;
>>>>>> atomic_t count;
>>>>>> atomic_t pending_count;
>>>>>> + u64 guest_qpage;
>>>>>> + u32 guest_qsize;
>>>>>> };
>>>>>>
>>>>>> /* Global enable flags for the XIVE support */
>>>>>> diff --git a/arch/powerpc/include/uapi/asm/kvm.h b/arch/powerpc/include/uapi/asm/kvm.h
>>>>>> index 12bb01baf0ae..1cd728c87d7c 100644
>>>>>> --- a/arch/powerpc/include/uapi/asm/kvm.h
>>>>>> +++ b/arch/powerpc/include/uapi/asm/kvm.h
>>>>>> @@ -679,6 +679,7 @@ struct kvm_ppc_cpu_char {
>>>>>> #define KVM_DEV_XIVE_GRP_CTRL 1
>>>>>> #define KVM_DEV_XIVE_GRP_SOURCE 2 /* 64-bit source identifier */
>>>>>> #define KVM_DEV_XIVE_GRP_SOURCE_CONFIG 3 /* 64-bit source identifier */
>>>>>> +#define KVM_DEV_XIVE_GRP_EQ_CONFIG 4 /* 64-bit EQ identifier */
>>>>>>
>>>>>> /* Layout of 64-bit XIVE source attribute values */
>>>>>> #define KVM_XIVE_LEVEL_SENSITIVE (1ULL << 0)
>>>>>> @@ -694,4 +695,24 @@ struct kvm_ppc_cpu_char {
>>>>>> #define KVM_XIVE_SOURCE_EISN_SHIFT 33
>>>>>> #define KVM_XIVE_SOURCE_EISN_MASK 0xfffffffe00000000ULL
>>>>>>
>>>>>> +/* Layout of 64-bit EQ identifier */
>>>>>> +#define KVM_XIVE_EQ_PRIORITY_SHIFT 0
>>>>>> +#define KVM_XIVE_EQ_PRIORITY_MASK 0x7
>>>>>> +#define KVM_XIVE_EQ_SERVER_SHIFT 3
>>>>>> +#define KVM_XIVE_EQ_SERVER_MASK 0xfffffff8ULL
>>>>>> +
>>>>>> +/* Layout of EQ configuration values (64 bytes) */
>>>>>> +struct kvm_ppc_xive_eq {
>>>>>> + __u32 flags;
>>>>>> + __u32 qsize;
>>>>>> + __u64 qpage;
>>>>>> + __u32 qtoggle;
>>>>>> + __u32 qindex;
>>>>>> + __u8 pad[40];
>>>>>> +};
>>>>>> +
>>>>>> +#define KVM_XIVE_EQ_FLAG_ENABLED 0x00000001
>>>>>> +#define KVM_XIVE_EQ_FLAG_ALWAYS_NOTIFY 0x00000002
>>>>>> +#define KVM_XIVE_EQ_FLAG_ESCALATE 0x00000004
>>>>>> +
>>>>>> #endif /* __LINUX_KVM_POWERPC_H */
>>>>>> diff --git a/arch/powerpc/kvm/book3s_xive.h b/arch/powerpc/kvm/book3s_xive.h
>>>>>> index ae26fe653d98..622f594d93e1 100644
>>>>>> --- a/arch/powerpc/kvm/book3s_xive.h
>>>>>> +++ b/arch/powerpc/kvm/book3s_xive.h
>>>>>> @@ -272,6 +272,8 @@ struct kvmppc_xive_src_block *kvmppc_xive_create_src_block(
>>>>>> struct kvmppc_xive *xive, int irq);
>>>>>> void kvmppc_xive_free_sources(struct kvmppc_xive_src_block *sb);
>>>>>> int kvmppc_xive_select_target(struct kvm *kvm, u32 *server, u8 prio);
>>>>>> +int kvmppc_xive_attach_escalation(struct kvm_vcpu *vcpu, u8 prio,
>>>>>> + bool single_escalation);
>>>>>>
>>>>>> #endif /* CONFIG_KVM_XICS */
>>>>>> #endif /* _KVM_PPC_BOOK3S_XICS_H */
>>>>>> diff --git a/arch/powerpc/kvm/book3s_xive.c b/arch/powerpc/kvm/book3s_xive.c
>>>>>> index e09f3addffe5..c1b7aa7dbc28 100644
>>>>>> --- a/arch/powerpc/kvm/book3s_xive.c
>>>>>> +++ b/arch/powerpc/kvm/book3s_xive.c
>>>>>> @@ -166,7 +166,8 @@ static irqreturn_t xive_esc_irq(int irq, void *data)
>>>>>> return IRQ_HANDLED;
>>>>>> }
>>>>>>
>>>>>> -static int xive_attach_escalation(struct kvm_vcpu *vcpu, u8 prio)
>>>>>> +int kvmppc_xive_attach_escalation(struct kvm_vcpu *vcpu, u8 prio,
>>>>>> + bool single_escalation)
>>>>>> {
>>>>>> struct kvmppc_xive_vcpu *xc = vcpu->arch.xive_vcpu;
>>>>>> struct xive_q *q = &xc->queues[prio];
>>>>>> @@ -185,7 +186,7 @@ static int xive_attach_escalation(struct kvm_vcpu *vcpu, u8 prio)
>>>>>> return -EIO;
>>>>>> }
>>>>>>
>>>>>> - if (xc->xive->single_escalation)
>>>>>> + if (single_escalation)
>>>>>> name = kasprintf(GFP_KERNEL, "kvm-%d-%d",
>>>>>> vcpu->kvm->arch.lpid, xc->server_num);
>>>>>> else
>>>>>> @@ -217,7 +218,7 @@ static int xive_attach_escalation(struct kvm_vcpu *vcpu, u8 prio)
>>>>>> * interrupt, thus leaving it effectively masked after
>>>>>> * it fires once.
>>>>>> */
>>>>>> - if (xc->xive->single_escalation) {
>>>>>> + if (single_escalation) {
>>>>>> struct irq_data *d = irq_get_irq_data(xc->esc_virq[prio]);
>>>>>> struct xive_irq_data *xd = irq_data_get_irq_handler_data(d);
>>>>>>
>>>>>> @@ -291,7 +292,8 @@ static int xive_check_provisioning(struct kvm *kvm, u8 prio)
>>>>>> continue;
>>>>>> rc = xive_provision_queue(vcpu, prio);
>>>>>> if (rc == 0 && !xive->single_escalation)
>>>>>> - xive_attach_escalation(vcpu, prio);
>>>>>> + kvmppc_xive_attach_escalation(vcpu, prio,
>>>>>> + xive->single_escalation);
>>>>>> if (rc)
>>>>>> return rc;
>>>>>> }
>>>>>> @@ -1214,7 +1216,8 @@ int kvmppc_xive_connect_vcpu(struct kvm_device *dev,
>>>>>> if (xive->qmap & (1 << i)) {
>>>>>> r = xive_provision_queue(vcpu, i);
>>>>>> if (r == 0 && !xive->single_escalation)
>>>>>> - xive_attach_escalation(vcpu, i);
>>>>>> + kvmppc_xive_attach_escalation(
>>>>>> + vcpu, i, xive->single_escalation);
>>>>>> if (r)
>>>>>> goto bail;
>>>>>> } else {
>>>>>> @@ -1229,7 +1232,7 @@ int kvmppc_xive_connect_vcpu(struct kvm_device *dev,
>>>>>> }
>>>>>>
>>>>>> /* If not done above, attach priority 0 escalation */
>>>>>> - r = xive_attach_escalation(vcpu, 0);
>>>>>> + r = kvmppc_xive_attach_escalation(vcpu, 0, xive->single_escalation);
>>>>>> if (r)
>>>>>> goto bail;
>>>>>>
>>>>>> diff --git a/arch/powerpc/kvm/book3s_xive_native.c b/arch/powerpc/kvm/book3s_xive_native.c
>>>>>> index b841d339f674..42e824658a30 100644
>>>>>> --- a/arch/powerpc/kvm/book3s_xive_native.c
>>>>>> +++ b/arch/powerpc/kvm/book3s_xive_native.c
>>>>>> @@ -340,6 +340,226 @@ static int kvmppc_xive_native_set_source_config(struct kvmppc_xive *xive,
>>>>>> priority, masked, eisn);
>>>>>> }
>>>>>>
>>>>>> +static int xive_native_validate_queue_size(u32 qsize)
>>>>>> +{
>>>>>> + /*
>>>>>> + * We only support 64K pages for the moment. This is also
>>>>>> + * advertised in the DT property "ibm,xive-eq-sizes"
>>>>>
>>>>> IIUC, that won't work properly if you had a guest using 4kiB pages.
>>>>
>>>>> That's fine, but do we have somewhere that checks for that case and
>>>>> throws a suitable error?
>>>>
>>>> Not in the device.
>>>>
>>>> So, we should check the current page_size of the guest ? Is there a way
>>>> to do that simply from KVM ?
>>>
>>> Not really. But I think I know where to make the necessary test, see
>>> comment below..
>>>
>>>>>> + */
>>>>>> + switch (qsize) {
>>>>>> + case 0: /* EQ reset */
>>>>>> + case 16:
>>>>>> + return 0;
>>>>>> + case 12:
>>>>>> + case 21:
>>>>>> + case 24:
>>>>>> + default:
>>>>>> + return -EINVAL;
>>>>>> + }
>>>>>> +}
>>>>>> +
>>>>>> +static int kvmppc_xive_native_set_queue_config(struct kvmppc_xive *xive,
>>>>>> + long eq_idx, u64 addr)
>>>>>> +{
>>>>>> + struct kvm *kvm = xive->kvm;
>>>>>> + struct kvm_vcpu *vcpu;
>>>>>> + struct kvmppc_xive_vcpu *xc;
>>>>>> + void __user *ubufp = (u64 __user *) addr;
>>>>>
>>>>> Nit: that should be (void __user *) on the right, shouldn't it?
>>>>
>>>> yes.
>>>>
>>>>>
>>>>>> + u32 server;
>>>>>> + u8 priority;
>>>>>> + struct kvm_ppc_xive_eq kvm_eq;
>>>>>> + int rc;
>>>>>> + __be32 *qaddr = 0;
>>>>>> + struct page *page;
>>>>>> + struct xive_q *q;
>>>>>> +
>>>>>> + /*
>>>>>> + * Demangle priority/server tuple from the EQ identifier
>>>>>> + */
>>>>>> + priority = (eq_idx & KVM_XIVE_EQ_PRIORITY_MASK) >>
>>>>>> + KVM_XIVE_EQ_PRIORITY_SHIFT;
>>>>>> + server = (eq_idx & KVM_XIVE_EQ_SERVER_MASK) >>
>>>>>> + KVM_XIVE_EQ_SERVER_SHIFT;
>>>>>> +
>>>>>> + if (copy_from_user(&kvm_eq, ubufp, sizeof(kvm_eq)))
>>>>>> + return -EFAULT;
>>>>>> +
>>>>>> + vcpu = kvmppc_xive_find_server(kvm, server);
>>>>>> + if (!vcpu) {
>>>>>> + pr_err("Can't find server %d\n", server);
>>>>>> + return -ENOENT;
>>>>>> + }
>>>>>> + xc = vcpu->arch.xive_vcpu;
>>>>>> +
>>>>>> + if (priority != xive_prio_from_guest(priority)) {
>>>>>> + pr_err("Trying to restore invalid queue %d for VCPU %d\n",
>>>>>> + priority, server);
>>>>>> + return -EINVAL;
>>>>>> + }
>>>>>> + q = &xc->queues[priority];
>>>>>> +
>>>>>> + pr_devel("%s VCPU %d priority %d fl:%x sz:%d addr:%llx g:%d idx:%d\n",
>>>>>> + __func__, server, priority, kvm_eq.flags,
>>>>>> + kvm_eq.qsize, kvm_eq.qpage, kvm_eq.qtoggle, kvm_eq.qindex);
>>>>>> +
>>>>>> + /*
>>>>>> + * We can not tune the EQ configuration from user space. All
>>>>>> + * is done in OPAL.
>>>>>> + */
>>>>>> + if (kvm_eq.flags != 0) {
>>>>>> + pr_err("invalid flags %d\n", kvm_eq.flags);
>>>>>> + return -EINVAL;
>>>>>> + }
>>>>>> +
>>>>>> + rc = xive_native_validate_queue_size(kvm_eq.qsize);
>>>>>> + if (rc) {
>>>>>> + pr_err("invalid queue size %d\n", kvm_eq.qsize);
>>>>>> + return rc;
>>>>>> + }
>>>>>> +
>>>>>> + /* reset queue and disable queueing */
>>>>>> + if (!kvm_eq.qsize) {
>>>>>> + q->guest_qpage = 0;
>>>>>> + q->guest_qsize = 0;
>>>>>> +
>>>>>> + rc = xive_native_configure_queue(xc->vp_id, q, priority,
>>>>>> + NULL, 0, true);
>>>>>> + if (rc) {
>>>>>> + pr_err("Failed to reset queue %d for VCPU %d: %d\n",
>>>>>> + priority, xc->server_num, rc);
>>>>>> + return rc;
>>>>>> + }
>>>>>> +
>>>>>> + if (q->qpage) {
>>>>>> + put_page(virt_to_page(q->qpage));
>>>>>> + q->qpage = NULL;
>>>>>> + }
>>>>>> +
>>>>>> + return 0;
>>>>>> + }
>>>>>> +
>>>>>> +
>>>>>> + page = gfn_to_page(kvm, gpa_to_gfn(kvm_eq.qpage));
>>>>>> + if (is_error_page(page)) {
>>>>>> + pr_warn("Couldn't get guest page for %llx!\n", kvm_eq.qpage);
>>>>>> + return -EINVAL;
>>>>>> + }
>>>>>
>>>>> Yeah.. for the case of a 4kiB page host (these days weird, but not
>>>>> actually prohibited, AFAIK) you need to check that the qsize selected
>>>>> actually fits within the page.
>>>>
>>>> Ah yes. sure.
>>>
>>> I think the pagesize test belongs here. Rather than thinking about
>>> the pagesize of the guest overall, you can check that this specific
>>> page (possibly compound) is large enough to take the requested queue
>>> size.
>>
>> OK. It think kvm_host_page_size() is what we need. It returns the page
>> size of the underlying VMA of the memblock holding the gfn. So I am going
>> to add :
>
> Yes, that sounds good.
>
>> + page_size = kvm_host_page_size(kvm, gfn);
>> + if (1ull << kvm_eq.qshift > page_size) {
>> + pr_warn("Incompatible host page size %lx!\n", page_size);
>> + return -EINVAL;
>> + }
>> +
>>
>> Also I am renaming 'qsize' in 'qshift' and renaming 'qpage' to 'qaddr'.
>>
>>> That should be enough to protect the host - it ensures that userspace
>>> owns a suitable contiguous chunk of memory for the XIVE to write the
>>> queue into.
>>>
>>> It's possible there are weirder edge cases with a large page that's
>>> not fully mapped into the guest - if necessary we can add tests for
>>> that on the qemu side.
>>>
>>> Oh.. it occurs to me that we might need to pin the queue page to make
>>> sure it doesn't get swapped out or page-migrated while the XIVE holds
>>> a pointer to it
>>
>> That is what gfn_to_page() ends up doing by calling hva_to_pfn().
>
> Ah, ok.
>
>>>>>> + /*
>>>>>> + * Return some information on the EQ configuration in
>>>>>> + * OPAL. This is purely informative for now as we can't really
>>>>>> + * tune the EQ configuration from user space.
>>>>>> + */
>>>>>> + kvm_eq.flags = 0;
>>>>>> + if (qflags & OPAL_XIVE_EQ_ENABLED)
>>>>>> + kvm_eq.flags |= KVM_XIVE_EQ_FLAG_ENABLED;
>>>>>> + if (qflags & OPAL_XIVE_EQ_ALWAYS_NOTIFY)
>>>>>> + kvm_eq.flags |= KVM_XIVE_EQ_FLAG_ALWAYS_NOTIFY;
>>>>>> + if (qflags & OPAL_XIVE_EQ_ESCALATE)
>>>>>> + kvm_eq.flags |= KVM_XIVE_EQ_FLAG_ESCALATE;
>>>>>
>>>>> If there's not really anything it can do about it, does it make sense
>>>>> to even expose this info to userspace?
>>>>
>>>> Hmm, good question.
>>>>
>>>> - KVM_XIVE_EQ_FLAG_ENABLED
>>>> may be uselessly obvious.
>>>
>>> What's it controlled by?
>>
>> OPAL only. It's equivalent to the VALID bit in the XIVE END structure.
>> We can drop this one.
>
> Ok.
>
>>>> - KVM_XIVE_EQ_FLAG_ALWAYS_NOTIFY
>>>> means we do not use the END ESBs to coalesce the events at the END
>>>> level. This flag is reflected by the XIVE_EQ_ALWAYS_NOTIFY option
>>>> in the sPAPR specs. We don't support the END ESBs but we might one
>>>> day.
>>>
>>> Since the guest isn't currently permitted to set this, it should never
>>> be set here either, no?
>>
>> The OS does not support the END ESBs so this flag is always set in the
>> Linux XIVE driver. END ESBs are supported in the emulated device but not
>> in KVM/OPAL.
>
> Ok, it's reasonable to keep this then.
>
>>>> - KVM_XIVE_EQ_FLAG_ESCALATE
>>>> means the EQ is an escalation. QEMU doesn't really care for now
>>>> but it's an important information I think.
>>>
>>> Likewise this one, yes?
>>
>> That is an hypervisor information on the nature of the EQ, so it is
>> for QEMU and not the guest. I am not sure it is important today as
>> we don't support escalation in QEMU. May be drop ?
>
> Yes, I think drop it. We can add something later if we have a
> concrete use case for it.
>
>>>> I tried not to add too many of the END flags, only the relevant ones which
>>>> could have an impact in the future modeling.
>>>>
>>>> I think KVM_XIVE_EQ_FLAG_ALWAYS_NOTIFY is important. I was setting it from
>>>> QEMU in the hcall but as OPAL does the same blindly I removed it in
>>>> v3.
>>>
>>> So, I might have misinterpreted this a bit the first time around. Am
>>> I correct in thinking that these bits all correspond to defined
>>> options in the PAPR hcall
>>
>> Yes for KVM_XIVE_EQ_FLAG_ALWAYS_NOTIFY which is the only flag defined
>> in sPAPR.
>>
>>> - but that for now we don't allow guests to
>>> set them (because we haven't implemented support so far).
>>
>> It's a bit more complex.
>>
>> KVM_XIVE_EQ_FLAG_ALWAYS_NOTIFY is a "required" flag as the OS does not
>> support END ESBs. END ESBs are not supported in KVM/OPAL but they are
>> in the QEMU device.
>
> So, the (current) guest needs this behaviour, but I'm guessing PAPR
> doesn't require it. Is this behaviour configurable in the PAPR interface?
It is part of the sPAPR specs. The H_INT_SET_QUEUE_CONFIG hcall has a
"Unconditional Notify (n) flag" to force notification without using the
coalescing mechanism provided by the XIVE END ESBs. As the XIVE driver
in Linux doesn't use the END ESBS, this flag is always set at the spapr
level.
We have a similar flag at the PowerNV level, and for the same reason
that on sPAPR, the flag is always set: OPAL_XIVE_EQ_ALWAYS_NOTIFY in
xive_native_configure_queue()
I think we should propagate at the KVM level and possibly, one day, add
a parameter to xive_native_configure_queue() to reflect this setting
from KVM. This is not required today as we don't support "conditional
notification".
I will add some comments in this area.
C.
>> I think it would be good for consistency to set KVM_XIVE_EQ_FLAG_ALWAYS_NOTIFY
>> when calling kvmppc_xive_native_get_queue_config() from QEMU, as I did
>> in v2. But as OPAL forces the same behavior without any flag, it is not
>> really needed at the KVM level ...
>>
>> Please tell me.
>
^ permalink raw reply
* Re: [PATCH] compiler: allow all arches to enable CONFIG_OPTIMIZE_INLINING
From: Masahiro Yamada @ 2019-03-20 6:40 UTC (permalink / raw)
To: Andrew Morton, linux-arch
Cc: linux-s390, Dave Hansen, Arnd Bergmann, X86 ML, linux-mips,
Linux Kernel Mailing List, Paul Burton, Ingo Molnar, linux-mtd,
linuxppc-dev, linux-arm-kernel
In-Reply-To: <1553062828-27798-1-git-send-email-yamada.masahiro@socionext.com>
On Wed, Mar 20, 2019 at 3:21 PM Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
>
> Commit 60a3cdd06394 ("x86: add optimized inlining") introduced
> CONFIG_OPTIMIZE_INLINING, but it has been available only for x86.
>
> The idea is obviously arch-agnostic although we need some code fixups.
> This commit moves the config entry from arch/x86/Kconfig.debug to
> lib/Kconfig.debug so that all architectures (except MIPS for now) can
> benefit from it.
>
> At this moment, I added "depends on !MIPS" because fixing 0day bot reports
> for MIPS was complex to me.
BTW, I got the following error if I enabled CONFIG_OPTIMIZE_INLINING for MIPS.
It is unclear to me how to fix it.
That's why I ended up with "depends on !MIPS".
MODPOST vmlinux.o
arch/mips/mm/sc-mips.o: In function `mips_sc_prefetch_enable.part.2':
sc-mips.c:(.text+0x98): undefined reference to `mips_gcr_base'
sc-mips.c:(.text+0x9c): undefined reference to `mips_gcr_base'
sc-mips.c:(.text+0xbc): undefined reference to `mips_gcr_base'
sc-mips.c:(.text+0xc8): undefined reference to `mips_gcr_base'
sc-mips.c:(.text+0xdc): undefined reference to `mips_gcr_base'
arch/mips/mm/sc-mips.o:sc-mips.c:(.text.unlikely+0x44): more undefined
references to `mips_gcr_base'
Perhaps, MIPS folks may know how to fix it.
> I tested this patch on my arm/arm64 boards.
>
> This can make a huge difference in kernel image size especially when
> CONFIG_OPTIMIZE_FOR_SIZE is enabled.
>
> For example, I got 3.5% smaller arm64 kernel image for v5.1-rc1.
>
> dec file
> 18983424 arch/arm64/boot/Image.before
> 18321920 arch/arm64/boot/Image.after
>
> This also slightly improves the "Kernel hacking" Kconfig menu.
> Commit e61aca5158a8 ("Merge branch 'kconfig-diet' from Dave Hansen')
> mentioned this config option would be a good fit in the "compiler option"
> menu. I did so.
>
> I fixed up some files to avoid build warnings/errors.
>
> [1] arch/arm64/include/asm/cpufeature.h
>
> In file included from ././include/linux/compiler_types.h:68,
> from <command-line>:
> ./arch/arm64/include/asm/jump_label.h: In function 'cpus_have_const_cap':
> ./include/linux/compiler-gcc.h:120:38: warning: asm operand 0 probably doesn't match constraints
> #define asm_volatile_goto(x...) do { asm goto(x); asm (""); } while (0)
> ^~~
> ./arch/arm64/include/asm/jump_label.h:32:2: note: in expansion of macro 'asm_volatile_goto'
> asm_volatile_goto(
> ^~~~~~~~~~~~~~~~~
> ./include/linux/compiler-gcc.h:120:38: error: impossible constraint in 'asm'
> #define asm_volatile_goto(x...) do { asm goto(x); asm (""); } while (0)
> ^~~
> ./arch/arm64/include/asm/jump_label.h:32:2: note: in expansion of macro 'asm_volatile_goto'
> asm_volatile_goto(
> ^~~~~~~~~~~~~~~~~
>
> [2] arch/mips/kernel/cpu-bugs64.c
>
> arch/mips/kernel/cpu-bugs64.c: In function 'mult_sh_align_mod.constprop':
> arch/mips/kernel/cpu-bugs64.c:33:2: error: asm operand 1 probably doesn't match constraints [-Werror]
> asm volatile(
> ^~~
> arch/mips/kernel/cpu-bugs64.c:33:2: error: asm operand 1 probably doesn't match constraints [-Werror]
> asm volatile(
> ^~~
> arch/mips/kernel/cpu-bugs64.c:33:2: error: impossible constraint in 'asm'
> asm volatile(
> ^~~
> arch/mips/kernel/cpu-bugs64.c:33:2: error: impossible constraint in 'asm'
> asm volatile(
> ^~~
>
> [3] arch/powerpc/mm/tlb-radix.c
>
> arch/powerpc/mm/tlb-radix.c: In function '__radix__flush_tlb_range_psize':
> arch/powerpc/mm/tlb-radix.c:104:2: error: asm operand 3 probably doesn't match constraints [-Werror]
> asm volatile(PPC_TLBIEL(%0, %4, %3, %2, %1)
> ^~~
> arch/powerpc/mm/tlb-radix.c:104:2: error: impossible constraint in 'asm'
> CC arch/powerpc/perf/hv-gpci.o
>
> [4] arch/s390/include/asm/cpacf.h
>
> In file included from arch/s390/crypto/des_s390.c:19:
> ./arch/s390/include/asm/cpacf.h: In function 'cpacf_query':
> ./arch/s390/include/asm/cpacf.h:170:2: warning: asm operand 3 probably doesn't match constraints
> asm volatile(
> ^~~
> ./arch/s390/include/asm/cpacf.h:170:2: error: impossible constraint in 'asm'
>
> [5] arch/powerpc/kernel/prom_init.c
>
> WARNING: vmlinux.o(.text.unlikely+0x20): Section mismatch in reference from the function .prom_getprop() to the function .init.text:.call_prom()
> The function .prom_getprop() references
> the function __init .call_prom().
> This is often because .prom_getprop lacks a __init
> annotation or the annotation of .call_prom is wrong.
>
> WARNING: vmlinux.o(.text.unlikely+0x3c): Section mismatch in reference from the function .prom_getproplen() to the function .init.text:.call_prom()
> The function .prom_getproplen() references
> the function __init .call_prom().
> This is often because .prom_getproplen lacks a __init
> annotation or the annotation of .call_prom is wrong.
>
> [6] drivers/mtd/nand/raw/vf610_nfc.c
>
> drivers/mtd/nand/raw/vf610_nfc.c: In function ‘vf610_nfc_cmd’:
> drivers/mtd/nand/raw/vf610_nfc.c:455:3: warning: ‘offset’ may be used uninitialized in this function [-Wmaybe-uninitialized]
> vf610_nfc_rd_from_sram(instr->ctx.data.buf.in + offset,
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> nfc->regs + NFC_MAIN_AREA(0) + offset,
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> trfr_sz, !nfc->data_access);
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> [7] arch/arm/kernel/smp.c
>
> arch/arm/kernel/smp.c: In function ‘raise_nmi’:
> arch/arm/kernel/smp.c:522:2: warning: array subscript is above array bounds [-Warray-bounds]
> trace_ipi_raise_rcuidle(target, ipi_types[ipinr]);
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> The fixup is not included in this. The patch is available in ML:
>
> http://lists.infradead.org/pipermail/linux-arm-kernel/2016-February/409393.html
>
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---
>
> arch/arm64/include/asm/cpufeature.h | 4 ++--
> arch/mips/kernel/cpu-bugs64.c | 4 ++--
> arch/powerpc/kernel/prom_init.c | 6 +++---
> arch/powerpc/mm/tlb-radix.c | 2 +-
> arch/s390/include/asm/cpacf.h | 2 +-
> arch/x86/Kconfig | 3 ---
> arch/x86/Kconfig.debug | 14 --------------
> drivers/mtd/nand/raw/vf610_nfc.c | 2 +-
> include/linux/compiler_types.h | 3 +--
> lib/Kconfig.debug | 15 +++++++++++++++
> 10 files changed, 26 insertions(+), 29 deletions(-)
>
> diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
> index e505e1f..77d1aa5 100644
> --- a/arch/arm64/include/asm/cpufeature.h
> +++ b/arch/arm64/include/asm/cpufeature.h
> @@ -406,7 +406,7 @@ static inline bool cpu_have_feature(unsigned int num)
> }
>
> /* System capability check for constant caps */
> -static inline bool __cpus_have_const_cap(int num)
> +static __always_inline bool __cpus_have_const_cap(int num)
> {
> if (num >= ARM64_NCAPS)
> return false;
> @@ -420,7 +420,7 @@ static inline bool cpus_have_cap(unsigned int num)
> return test_bit(num, cpu_hwcaps);
> }
>
> -static inline bool cpus_have_const_cap(int num)
> +static __always_inline bool cpus_have_const_cap(int num)
> {
> if (static_branch_likely(&arm64_const_caps_ready))
> return __cpus_have_const_cap(num);
> diff --git a/arch/mips/kernel/cpu-bugs64.c b/arch/mips/kernel/cpu-bugs64.c
> index bada74a..c04b97a 100644
> --- a/arch/mips/kernel/cpu-bugs64.c
> +++ b/arch/mips/kernel/cpu-bugs64.c
> @@ -42,8 +42,8 @@ static inline void align_mod(const int align, const int mod)
> : "n"(align), "n"(mod));
> }
>
> -static inline void mult_sh_align_mod(long *v1, long *v2, long *w,
> - const int align, const int mod)
> +static __always_inline void mult_sh_align_mod(long *v1, long *v2, long *w,
> + const int align, const int mod)
> {
> unsigned long flags;
> int m1, m2;
> diff --git a/arch/powerpc/kernel/prom_init.c b/arch/powerpc/kernel/prom_init.c
> index f33ff41..241fe6b 100644
> --- a/arch/powerpc/kernel/prom_init.c
> +++ b/arch/powerpc/kernel/prom_init.c
> @@ -501,14 +501,14 @@ static int __init prom_next_node(phandle *nodep)
> }
> }
>
> -static inline int prom_getprop(phandle node, const char *pname,
> - void *value, size_t valuelen)
> +static inline int __init prom_getprop(phandle node, const char *pname,
> + void *value, size_t valuelen)
> {
> return call_prom("getprop", 4, 1, node, ADDR(pname),
> (u32)(unsigned long) value, (u32) valuelen);
> }
>
> -static inline int prom_getproplen(phandle node, const char *pname)
> +static inline int __init prom_getproplen(phandle node, const char *pname)
> {
> return call_prom("getproplen", 2, 1, node, ADDR(pname));
> }
> diff --git a/arch/powerpc/mm/tlb-radix.c b/arch/powerpc/mm/tlb-radix.c
> index 6a23b9e..a2b2848 100644
> --- a/arch/powerpc/mm/tlb-radix.c
> +++ b/arch/powerpc/mm/tlb-radix.c
> @@ -928,7 +928,7 @@ void radix__tlb_flush(struct mmu_gather *tlb)
> tlb->need_flush_all = 0;
> }
>
> -static inline void __radix__flush_tlb_range_psize(struct mm_struct *mm,
> +static __always_inline void __radix__flush_tlb_range_psize(struct mm_struct *mm,
> unsigned long start, unsigned long end,
> int psize, bool also_pwc)
> {
> diff --git a/arch/s390/include/asm/cpacf.h b/arch/s390/include/asm/cpacf.h
> index 3cc52e3..f316de4 100644
> --- a/arch/s390/include/asm/cpacf.h
> +++ b/arch/s390/include/asm/cpacf.h
> @@ -202,7 +202,7 @@ static inline int __cpacf_check_opcode(unsigned int opcode)
> }
> }
>
> -static inline int cpacf_query(unsigned int opcode, cpacf_mask_t *mask)
> +static __always_inline int cpacf_query(unsigned int opcode, cpacf_mask_t *mask)
> {
> if (__cpacf_check_opcode(opcode)) {
> __cpacf_query(opcode, mask);
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index c1f9b3c..1a3e2b5 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -310,9 +310,6 @@ config ZONE_DMA32
> config AUDIT_ARCH
> def_bool y if X86_64
>
> -config ARCH_SUPPORTS_OPTIMIZED_INLINING
> - def_bool y
> -
> config ARCH_SUPPORTS_DEBUG_PAGEALLOC
> def_bool y
>
> diff --git a/arch/x86/Kconfig.debug b/arch/x86/Kconfig.debug
> index 15d0fbe..f730680 100644
> --- a/arch/x86/Kconfig.debug
> +++ b/arch/x86/Kconfig.debug
> @@ -266,20 +266,6 @@ config CPA_DEBUG
> ---help---
> Do change_page_attr() self-tests every 30 seconds.
>
> -config OPTIMIZE_INLINING
> - bool "Allow gcc to uninline functions marked 'inline'"
> - ---help---
> - This option determines if the kernel forces gcc to inline the functions
> - developers have marked 'inline'. Doing so takes away freedom from gcc to
> - do what it thinks is best, which is desirable for the gcc 3.x series of
> - compilers. The gcc 4.x series have a rewritten inlining algorithm and
> - enabling this option will generate a smaller kernel there. Hopefully
> - this algorithm is so good that allowing gcc 4.x and above to make the
> - decision will become the default in the future. Until then this option
> - is there to test gcc for this.
> -
> - If unsure, say N.
> -
> config DEBUG_ENTRY
> bool "Debug low-level entry code"
> depends on DEBUG_KERNEL
> diff --git a/drivers/mtd/nand/raw/vf610_nfc.c b/drivers/mtd/nand/raw/vf610_nfc.c
> index a662ca1..19792d7 100644
> --- a/drivers/mtd/nand/raw/vf610_nfc.c
> +++ b/drivers/mtd/nand/raw/vf610_nfc.c
> @@ -364,7 +364,7 @@ static int vf610_nfc_cmd(struct nand_chip *chip,
> {
> const struct nand_op_instr *instr;
> struct vf610_nfc *nfc = chip_to_nfc(chip);
> - int op_id = -1, trfr_sz = 0, offset;
> + int op_id = -1, trfr_sz = 0, offset = 0;
> u32 col = 0, row = 0, cmd1 = 0, cmd2 = 0, code = 0;
> bool force8bit = false;
>
> diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h
> index ba814f1..19e58b9 100644
> --- a/include/linux/compiler_types.h
> +++ b/include/linux/compiler_types.h
> @@ -140,8 +140,7 @@ struct ftrace_likely_data {
> * Do not use __always_inline here, since currently it expands to inline again
> * (which would break users of __always_inline).
> */
> -#if !defined(CONFIG_ARCH_SUPPORTS_OPTIMIZED_INLINING) || \
> - !defined(CONFIG_OPTIMIZE_INLINING)
> +#if !defined(CONFIG_OPTIMIZE_INLINING)
> #define inline inline __attribute__((__always_inline__)) __gnu_inline \
> __maybe_unused notrace
> #else
> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> index 0d9e817..20f3dfc 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -310,6 +310,21 @@ config HEADERS_CHECK
> exported to $(INSTALL_HDR_PATH) (usually 'usr/include' in
> your build tree), to make sure they're suitable.
>
> +config OPTIMIZE_INLINING
> + bool "Allow compiler to uninline functions marked 'inline'"
> + depends on !MIPS # TODO: look into MIPS code
> + help
> + This option determines if the kernel forces gcc to inline the functions
> + developers have marked 'inline'. Doing so takes away freedom from gcc to
> + do what it thinks is best, which is desirable for the gcc 3.x series of
> + compilers. The gcc 4.x series have a rewritten inlining algorithm and
> + enabling this option will generate a smaller kernel there. Hopefully
> + this algorithm is so good that allowing gcc 4.x and above to make the
> + decision will become the default in the future. Until then this option
> + is there to test gcc for this.
> +
> + If unsure, say N.
> +
> config DEBUG_SECTION_MISMATCH
> bool "Enable full Section mismatch analysis"
> help
> --
> 2.7.4
>
>
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/
--
Best Regards
Masahiro Yamada
^ permalink raw reply
* Re: [PATCH 5/8] powerpc/eeh: Add eeh_show_enabled()
From: Oliver @ 2019-03-20 6:24 UTC (permalink / raw)
To: Alexey Kardashevskiy; +Cc: Sam Bobroff, linuxppc-dev
In-Reply-To: <26b7c0de-4192-eae5-f8aa-94073b3e768e@ozlabs.ru>
On Wed, Mar 20, 2019 at 5:06 PM Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
>
>
>
> On 20/03/2019 13:58, Sam Bobroff wrote:
> > Move the EEH enabled message into it's own function so that future
> > work can call it from multiple places.
> >
> > Signed-off-by: Sam Bobroff <sbobroff@linux.ibm.com>
> > ---
> > arch/powerpc/include/asm/eeh.h | 3 +++
> > arch/powerpc/kernel/eeh.c | 16 +++++++++++-----
> > 2 files changed, 14 insertions(+), 5 deletions(-)
> >
> > diff --git a/arch/powerpc/include/asm/eeh.h b/arch/powerpc/include/asm/eeh.h
> > index fe4cf7208890..e217ccda55d0 100644
> > --- a/arch/powerpc/include/asm/eeh.h
> > +++ b/arch/powerpc/include/asm/eeh.h
> > @@ -289,6 +289,7 @@ 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);
> > void eeh_probe_devices(void);
> > int __init eeh_ops_register(struct eeh_ops *ops);
> > int __exit eeh_ops_unregister(const char *name);
> > @@ -338,6 +339,8 @@ static inline bool eeh_enabled(void)
> > return false;
> > }
> >
> > +static inline void eeh_show_enabled(void) { }
> > +
> > static inline bool eeh_phb_enabled(void)
> > {
> > return false;
> > diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
> > index b14d89547895..3dcff29cb9b3 100644
> > --- a/arch/powerpc/kernel/eeh.c
> > +++ b/arch/powerpc/kernel/eeh.c
> > @@ -163,6 +163,16 @@ static int __init eeh_setup(char *str)
> > }
> > __setup("eeh=", eeh_setup);
> >
> > +void eeh_show_enabled(void)
> > +{
> > + if (eeh_has_flag(EEH_FORCE_DISABLED))
> > + pr_info("EEH: PCI Enhanced I/O Error Handling DISABLED (by eeh=off)\n");
> > + else if (eeh_enabled())
>
>
> I'd make it eeh_has_flag(EEH_ENABLED) for clarity.
>
>
> > + pr_info("EEH: PCI Enhanced I/O Error Handling ENABLED (capable adapter found)\n");
> > + else
> > + pr_info("EEH: PCI Enhanced I/O Error Handling DISABLED (no capable adapter found)\n");
> > +}
> > +
> > /*
> > * This routine captures assorted PCI configuration space data
> > * for the indicated PCI device, and puts them into a buffer
> > @@ -1166,11 +1176,7 @@ void eeh_probe_devices(void)
> > pdn = hose->pci_data;
> > traverse_pci_dn(pdn, eeh_ops->probe, NULL);
> > }
> > - if (eeh_enabled())
> > - pr_info("EEH: PCI Enhanced I/O Error Handling Enabled\n");
> > - else
> > - pr_info("EEH: No capable adapters found\n");
> > -
> > + eeh_show_enabled();
>
>
> This line moves later in the series so I'd just merge this patch into
> 8/8 to reduce number of lines moving withing the patchset.
>
> In general the whole point of the EEH_ENABLED flag is fading away. Its
> meaning now is that "at least somewhere in the box for at least one
> device with enabled EEH" which does not seem extremely useful as we have
> a pci_dev or pe pretty much everywhere we look at eeh_enabled() and
> pdev->dev.archdata.edev can tell if eeh is enabled for a device.
> Although I am pretty sure this is in your list already :)
The other function is to disable attempting to detect EEH errors when
we get 0xFFs from an MMIO load, but I don't think anyone ever disables
it.
> > }
> >
> > /**
> >
>
> --
> Alexey
^ permalink raw reply
* [PATCH] compiler: allow all arches to enable CONFIG_OPTIMIZE_INLINING
From: Masahiro Yamada @ 2019-03-20 6:20 UTC (permalink / raw)
To: Andrew Morton, linux-arch
Cc: linux-s390, Dave Hansen, Arnd Bergmann, x86, linux-mips,
linux-kernel, Masahiro Yamada, Paul Burton, Ingo Molnar,
linux-mtd, linuxppc-dev, linux-arm-kernel
Commit 60a3cdd06394 ("x86: add optimized inlining") introduced
CONFIG_OPTIMIZE_INLINING, but it has been available only for x86.
The idea is obviously arch-agnostic although we need some code fixups.
This commit moves the config entry from arch/x86/Kconfig.debug to
lib/Kconfig.debug so that all architectures (except MIPS for now) can
benefit from it.
At this moment, I added "depends on !MIPS" because fixing 0day bot reports
for MIPS was complex to me.
I tested this patch on my arm/arm64 boards.
This can make a huge difference in kernel image size especially when
CONFIG_OPTIMIZE_FOR_SIZE is enabled.
For example, I got 3.5% smaller arm64 kernel image for v5.1-rc1.
dec file
18983424 arch/arm64/boot/Image.before
18321920 arch/arm64/boot/Image.after
This also slightly improves the "Kernel hacking" Kconfig menu.
Commit e61aca5158a8 ("Merge branch 'kconfig-diet' from Dave Hansen')
mentioned this config option would be a good fit in the "compiler option"
menu. I did so.
I fixed up some files to avoid build warnings/errors.
[1] arch/arm64/include/asm/cpufeature.h
In file included from ././include/linux/compiler_types.h:68,
from <command-line>:
./arch/arm64/include/asm/jump_label.h: In function 'cpus_have_const_cap':
./include/linux/compiler-gcc.h:120:38: warning: asm operand 0 probably doesn't match constraints
#define asm_volatile_goto(x...) do { asm goto(x); asm (""); } while (0)
^~~
./arch/arm64/include/asm/jump_label.h:32:2: note: in expansion of macro 'asm_volatile_goto'
asm_volatile_goto(
^~~~~~~~~~~~~~~~~
./include/linux/compiler-gcc.h:120:38: error: impossible constraint in 'asm'
#define asm_volatile_goto(x...) do { asm goto(x); asm (""); } while (0)
^~~
./arch/arm64/include/asm/jump_label.h:32:2: note: in expansion of macro 'asm_volatile_goto'
asm_volatile_goto(
^~~~~~~~~~~~~~~~~
[2] arch/mips/kernel/cpu-bugs64.c
arch/mips/kernel/cpu-bugs64.c: In function 'mult_sh_align_mod.constprop':
arch/mips/kernel/cpu-bugs64.c:33:2: error: asm operand 1 probably doesn't match constraints [-Werror]
asm volatile(
^~~
arch/mips/kernel/cpu-bugs64.c:33:2: error: asm operand 1 probably doesn't match constraints [-Werror]
asm volatile(
^~~
arch/mips/kernel/cpu-bugs64.c:33:2: error: impossible constraint in 'asm'
asm volatile(
^~~
arch/mips/kernel/cpu-bugs64.c:33:2: error: impossible constraint in 'asm'
asm volatile(
^~~
[3] arch/powerpc/mm/tlb-radix.c
arch/powerpc/mm/tlb-radix.c: In function '__radix__flush_tlb_range_psize':
arch/powerpc/mm/tlb-radix.c:104:2: error: asm operand 3 probably doesn't match constraints [-Werror]
asm volatile(PPC_TLBIEL(%0, %4, %3, %2, %1)
^~~
arch/powerpc/mm/tlb-radix.c:104:2: error: impossible constraint in 'asm'
CC arch/powerpc/perf/hv-gpci.o
[4] arch/s390/include/asm/cpacf.h
In file included from arch/s390/crypto/des_s390.c:19:
./arch/s390/include/asm/cpacf.h: In function 'cpacf_query':
./arch/s390/include/asm/cpacf.h:170:2: warning: asm operand 3 probably doesn't match constraints
asm volatile(
^~~
./arch/s390/include/asm/cpacf.h:170:2: error: impossible constraint in 'asm'
[5] arch/powerpc/kernel/prom_init.c
WARNING: vmlinux.o(.text.unlikely+0x20): Section mismatch in reference from the function .prom_getprop() to the function .init.text:.call_prom()
The function .prom_getprop() references
the function __init .call_prom().
This is often because .prom_getprop lacks a __init
annotation or the annotation of .call_prom is wrong.
WARNING: vmlinux.o(.text.unlikely+0x3c): Section mismatch in reference from the function .prom_getproplen() to the function .init.text:.call_prom()
The function .prom_getproplen() references
the function __init .call_prom().
This is often because .prom_getproplen lacks a __init
annotation or the annotation of .call_prom is wrong.
[6] drivers/mtd/nand/raw/vf610_nfc.c
drivers/mtd/nand/raw/vf610_nfc.c: In function ‘vf610_nfc_cmd’:
drivers/mtd/nand/raw/vf610_nfc.c:455:3: warning: ‘offset’ may be used uninitialized in this function [-Wmaybe-uninitialized]
vf610_nfc_rd_from_sram(instr->ctx.data.buf.in + offset,
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
nfc->regs + NFC_MAIN_AREA(0) + offset,
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
trfr_sz, !nfc->data_access);
~~~~~~~~~~~~~~~~~~~~~~~~~~~
[7] arch/arm/kernel/smp.c
arch/arm/kernel/smp.c: In function ‘raise_nmi’:
arch/arm/kernel/smp.c:522:2: warning: array subscript is above array bounds [-Warray-bounds]
trace_ipi_raise_rcuidle(target, ipi_types[ipinr]);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
The fixup is not included in this. The patch is available in ML:
http://lists.infradead.org/pipermail/linux-arm-kernel/2016-February/409393.html
Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---
arch/arm64/include/asm/cpufeature.h | 4 ++--
arch/mips/kernel/cpu-bugs64.c | 4 ++--
arch/powerpc/kernel/prom_init.c | 6 +++---
arch/powerpc/mm/tlb-radix.c | 2 +-
arch/s390/include/asm/cpacf.h | 2 +-
arch/x86/Kconfig | 3 ---
arch/x86/Kconfig.debug | 14 --------------
drivers/mtd/nand/raw/vf610_nfc.c | 2 +-
include/linux/compiler_types.h | 3 +--
lib/Kconfig.debug | 15 +++++++++++++++
10 files changed, 26 insertions(+), 29 deletions(-)
diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
index e505e1f..77d1aa5 100644
--- a/arch/arm64/include/asm/cpufeature.h
+++ b/arch/arm64/include/asm/cpufeature.h
@@ -406,7 +406,7 @@ static inline bool cpu_have_feature(unsigned int num)
}
/* System capability check for constant caps */
-static inline bool __cpus_have_const_cap(int num)
+static __always_inline bool __cpus_have_const_cap(int num)
{
if (num >= ARM64_NCAPS)
return false;
@@ -420,7 +420,7 @@ static inline bool cpus_have_cap(unsigned int num)
return test_bit(num, cpu_hwcaps);
}
-static inline bool cpus_have_const_cap(int num)
+static __always_inline bool cpus_have_const_cap(int num)
{
if (static_branch_likely(&arm64_const_caps_ready))
return __cpus_have_const_cap(num);
diff --git a/arch/mips/kernel/cpu-bugs64.c b/arch/mips/kernel/cpu-bugs64.c
index bada74a..c04b97a 100644
--- a/arch/mips/kernel/cpu-bugs64.c
+++ b/arch/mips/kernel/cpu-bugs64.c
@@ -42,8 +42,8 @@ static inline void align_mod(const int align, const int mod)
: "n"(align), "n"(mod));
}
-static inline void mult_sh_align_mod(long *v1, long *v2, long *w,
- const int align, const int mod)
+static __always_inline void mult_sh_align_mod(long *v1, long *v2, long *w,
+ const int align, const int mod)
{
unsigned long flags;
int m1, m2;
diff --git a/arch/powerpc/kernel/prom_init.c b/arch/powerpc/kernel/prom_init.c
index f33ff41..241fe6b 100644
--- a/arch/powerpc/kernel/prom_init.c
+++ b/arch/powerpc/kernel/prom_init.c
@@ -501,14 +501,14 @@ static int __init prom_next_node(phandle *nodep)
}
}
-static inline int prom_getprop(phandle node, const char *pname,
- void *value, size_t valuelen)
+static inline int __init prom_getprop(phandle node, const char *pname,
+ void *value, size_t valuelen)
{
return call_prom("getprop", 4, 1, node, ADDR(pname),
(u32)(unsigned long) value, (u32) valuelen);
}
-static inline int prom_getproplen(phandle node, const char *pname)
+static inline int __init prom_getproplen(phandle node, const char *pname)
{
return call_prom("getproplen", 2, 1, node, ADDR(pname));
}
diff --git a/arch/powerpc/mm/tlb-radix.c b/arch/powerpc/mm/tlb-radix.c
index 6a23b9e..a2b2848 100644
--- a/arch/powerpc/mm/tlb-radix.c
+++ b/arch/powerpc/mm/tlb-radix.c
@@ -928,7 +928,7 @@ void radix__tlb_flush(struct mmu_gather *tlb)
tlb->need_flush_all = 0;
}
-static inline void __radix__flush_tlb_range_psize(struct mm_struct *mm,
+static __always_inline void __radix__flush_tlb_range_psize(struct mm_struct *mm,
unsigned long start, unsigned long end,
int psize, bool also_pwc)
{
diff --git a/arch/s390/include/asm/cpacf.h b/arch/s390/include/asm/cpacf.h
index 3cc52e3..f316de4 100644
--- a/arch/s390/include/asm/cpacf.h
+++ b/arch/s390/include/asm/cpacf.h
@@ -202,7 +202,7 @@ static inline int __cpacf_check_opcode(unsigned int opcode)
}
}
-static inline int cpacf_query(unsigned int opcode, cpacf_mask_t *mask)
+static __always_inline int cpacf_query(unsigned int opcode, cpacf_mask_t *mask)
{
if (__cpacf_check_opcode(opcode)) {
__cpacf_query(opcode, mask);
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index c1f9b3c..1a3e2b5 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -310,9 +310,6 @@ config ZONE_DMA32
config AUDIT_ARCH
def_bool y if X86_64
-config ARCH_SUPPORTS_OPTIMIZED_INLINING
- def_bool y
-
config ARCH_SUPPORTS_DEBUG_PAGEALLOC
def_bool y
diff --git a/arch/x86/Kconfig.debug b/arch/x86/Kconfig.debug
index 15d0fbe..f730680 100644
--- a/arch/x86/Kconfig.debug
+++ b/arch/x86/Kconfig.debug
@@ -266,20 +266,6 @@ config CPA_DEBUG
---help---
Do change_page_attr() self-tests every 30 seconds.
-config OPTIMIZE_INLINING
- bool "Allow gcc to uninline functions marked 'inline'"
- ---help---
- This option determines if the kernel forces gcc to inline the functions
- developers have marked 'inline'. Doing so takes away freedom from gcc to
- do what it thinks is best, which is desirable for the gcc 3.x series of
- compilers. The gcc 4.x series have a rewritten inlining algorithm and
- enabling this option will generate a smaller kernel there. Hopefully
- this algorithm is so good that allowing gcc 4.x and above to make the
- decision will become the default in the future. Until then this option
- is there to test gcc for this.
-
- If unsure, say N.
-
config DEBUG_ENTRY
bool "Debug low-level entry code"
depends on DEBUG_KERNEL
diff --git a/drivers/mtd/nand/raw/vf610_nfc.c b/drivers/mtd/nand/raw/vf610_nfc.c
index a662ca1..19792d7 100644
--- a/drivers/mtd/nand/raw/vf610_nfc.c
+++ b/drivers/mtd/nand/raw/vf610_nfc.c
@@ -364,7 +364,7 @@ static int vf610_nfc_cmd(struct nand_chip *chip,
{
const struct nand_op_instr *instr;
struct vf610_nfc *nfc = chip_to_nfc(chip);
- int op_id = -1, trfr_sz = 0, offset;
+ int op_id = -1, trfr_sz = 0, offset = 0;
u32 col = 0, row = 0, cmd1 = 0, cmd2 = 0, code = 0;
bool force8bit = false;
diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h
index ba814f1..19e58b9 100644
--- a/include/linux/compiler_types.h
+++ b/include/linux/compiler_types.h
@@ -140,8 +140,7 @@ struct ftrace_likely_data {
* Do not use __always_inline here, since currently it expands to inline again
* (which would break users of __always_inline).
*/
-#if !defined(CONFIG_ARCH_SUPPORTS_OPTIMIZED_INLINING) || \
- !defined(CONFIG_OPTIMIZE_INLINING)
+#if !defined(CONFIG_OPTIMIZE_INLINING)
#define inline inline __attribute__((__always_inline__)) __gnu_inline \
__maybe_unused notrace
#else
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 0d9e817..20f3dfc 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -310,6 +310,21 @@ config HEADERS_CHECK
exported to $(INSTALL_HDR_PATH) (usually 'usr/include' in
your build tree), to make sure they're suitable.
+config OPTIMIZE_INLINING
+ bool "Allow compiler to uninline functions marked 'inline'"
+ depends on !MIPS # TODO: look into MIPS code
+ help
+ This option determines if the kernel forces gcc to inline the functions
+ developers have marked 'inline'. Doing so takes away freedom from gcc to
+ do what it thinks is best, which is desirable for the gcc 3.x series of
+ compilers. The gcc 4.x series have a rewritten inlining algorithm and
+ enabling this option will generate a smaller kernel there. Hopefully
+ this algorithm is so good that allowing gcc 4.x and above to make the
+ decision will become the default in the future. Until then this option
+ is there to test gcc for this.
+
+ If unsure, say N.
+
config DEBUG_SECTION_MISMATCH
bool "Enable full Section mismatch analysis"
help
--
2.7.4
^ permalink raw reply related
* Re: [PATCH 8/8] powerpc/eeh: Remove eeh_probe_devices() and eeh_addr_cache_build()
From: Alexey Kardashevskiy @ 2019-03-20 6:05 UTC (permalink / raw)
To: Sam Bobroff, linuxppc-dev
In-Reply-To: <f43eac2b2adf92848afc3304cabd770c9bc976ce.1553050609.git.sbobroff@linux.ibm.com>
On 20/03/2019 13:58, Sam Bobroff wrote:
> Now that EEH support for all devices (on PowerNV and pSeries) is
> provided by the pcibios bus add device hooks, eeh_probe_devices() and
> eeh_addr_cache_build() are redundant and can be removed.
>
> Note that previously on pSeries, useless EEH sysfs files were created
> for some devices that did not have EEH support and this change
> prevents them from being created.
>
> Signed-off-by: Sam Bobroff <sbobroff@linux.ibm.com>
> ---
> arch/powerpc/include/asm/eeh.h | 6 ----
> arch/powerpc/kernel/eeh.c | 13 --------
> arch/powerpc/kernel/eeh_cache.c | 32 --------------------
> arch/powerpc/platforms/powernv/eeh-powernv.c | 5 ++-
> arch/powerpc/platforms/pseries/pci.c | 3 +-
> 5 files changed, 3 insertions(+), 56 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/eeh.h b/arch/powerpc/include/asm/eeh.h
> index 791b9e6fcc45..f1eca1757cbc 100644
> --- a/arch/powerpc/include/asm/eeh.h
> +++ b/arch/powerpc/include/asm/eeh.h
> @@ -290,13 +290,11 @@ 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);
> -void eeh_probe_devices(void);
> int __init eeh_ops_register(struct eeh_ops *ops);
> int __exit eeh_ops_unregister(const char *name);
> int eeh_check_failure(const volatile void __iomem *token);
> int eeh_dev_check_failure(struct eeh_dev *edev);
> void eeh_addr_cache_init(void);
> -void eeh_addr_cache_build(void);
> void eeh_add_device_early(struct pci_dn *);
> void eeh_add_device_tree_early(struct pci_dn *);
> void eeh_add_device_late(struct pci_dev *);
> @@ -347,8 +345,6 @@ static inline bool eeh_phb_enabled(void)
> return false;
> }
>
> -static inline void eeh_probe_devices(void) { }
> -
> static inline void *eeh_dev_init(struct pci_dn *pdn, void *data)
> {
> return NULL;
> @@ -365,8 +361,6 @@ static inline int eeh_check_failure(const volatile void __iomem *token)
>
> static inline void eeh_addr_cache_init(void) { }
>
> -static inline void eeh_addr_cache_build(void) { }
> -
> static inline void eeh_add_device_early(struct pci_dn *pdn) { }
>
> static inline void eeh_add_device_tree_early(struct pci_dn *pdn) { }
> diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
> index 217e14bb1fb6..cd2abbe41497 100644
> --- a/arch/powerpc/kernel/eeh.c
> +++ b/arch/powerpc/kernel/eeh.c
> @@ -1166,19 +1166,6 @@ static struct notifier_block eeh_reboot_nb = {
> .notifier_call = eeh_reboot_notifier,
> };
>
> -void eeh_probe_devices(void)
> -{
> - struct pci_controller *hose, *tmp;
> - struct pci_dn *pdn;
> -
> - /* Enable EEH for all adapters */
> - list_for_each_entry_safe(hose, tmp, &hose_list, list_node) {
> - pdn = hose->pci_data;
> - traverse_pci_dn(pdn, eeh_ops->probe, NULL);
> - }
> - eeh_show_enabled();
> -}
> -
> /**
> * eeh_init - EEH initialization
> *
> diff --git a/arch/powerpc/kernel/eeh_cache.c b/arch/powerpc/kernel/eeh_cache.c
> index f93dd5cf6a39..c40078d036af 100644
> --- a/arch/powerpc/kernel/eeh_cache.c
> +++ b/arch/powerpc/kernel/eeh_cache.c
> @@ -278,38 +278,6 @@ void eeh_addr_cache_init(void)
> spin_lock_init(&pci_io_addr_cache_root.piar_lock);
> }
>
> -/**
> - * eeh_addr_cache_build - Build a cache of I/O addresses
> - *
> - * Build a cache of pci i/o addresses. This cache will be used to
> - * find the pci device that corresponds to a given address.
> - * This routine scans all pci busses to build the cache.
> - * Must be run late in boot process, after the pci controllers
> - * have been scanned for devices (after all device resources are known).
> - */
> -void eeh_addr_cache_build(void)
> -{
> - struct pci_dn *pdn;
> - struct eeh_dev *edev;
> - struct pci_dev *dev = NULL;
> -
> - for_each_pci_dev(dev) {
> - pdn = pci_get_pdn_by_devfn(dev->bus, dev->devfn);
> - if (!pdn)
> - continue;
> -
> - edev = pdn_to_eeh_dev(pdn);
> - if (!edev)
> - continue;
> -
> - dev->dev.archdata.edev = edev;
> - edev->pdev = dev;
> -
> - eeh_addr_cache_insert_dev(dev);
> - eeh_sysfs_add_device(dev);
> - }
> -}
> -
> static int eeh_addr_cache_show(struct seq_file *s, void *v)
> {
> struct pci_io_addr_range *piar;
> diff --git a/arch/powerpc/platforms/powernv/eeh-powernv.c b/arch/powerpc/platforms/powernv/eeh-powernv.c
> index 81b0923cc55f..6a08f4fab255 100644
> --- a/arch/powerpc/platforms/powernv/eeh-powernv.c
> +++ b/arch/powerpc/platforms/powernv/eeh-powernv.c
> @@ -240,9 +240,7 @@ int pnv_eeh_post_init(void)
> struct pnv_phb *phb;
> int ret = 0;
>
> - /* Probe devices & build address cache */
> - eeh_probe_devices();
> - eeh_addr_cache_build();
> + eeh_show_enabled();
>
> /* Register OPAL event notifier */
> eeh_event_irq = opal_event_request(ilog2(OPAL_EVENT_PCI_ERROR));
> @@ -360,6 +358,7 @@ static int pnv_eeh_find_ecap(struct pci_dn *pdn, int cap)
> return 0;
> }
>
> +
Unrelated new line. Otherwise
Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> /**
> * pnv_eeh_probe - Do probe on PCI device
> * @pdn: PCI device node
> diff --git a/arch/powerpc/platforms/pseries/pci.c b/arch/powerpc/platforms/pseries/pci.c
> index 7be80882c08d..7c781b04fb15 100644
> --- a/arch/powerpc/platforms/pseries/pci.c
> +++ b/arch/powerpc/platforms/pseries/pci.c
> @@ -242,8 +242,7 @@ void __init pSeries_final_fixup(void)
>
> pSeries_request_regions();
>
> - eeh_probe_devices();
> - eeh_addr_cache_build();
> + eeh_show_enabled();
> #ifdef CONFIG_EEH
> if (eeh_enabled())
> eeh_add_flag(EEH_PHB_ENABLED);
>
--
Alexey
^ permalink raw reply
* Re: [PATCH kernel RFC 2/2] vfio-pci-nvlink2: Implement interconnect isolation
From: David Gibson @ 2019-03-20 4:38 UTC (permalink / raw)
To: Alex Williamson
Cc: Jose Ricardo Ziviani, Alexey Kardashevskiy,
Daniel Henrique Barboza, kvm-ppc, Piotr Jaroszynski,
Leonardo Augusto Guimarães Garcia, linuxppc-dev
In-Reply-To: <20190319103619.6534c7df@x1.home>
[-- Attachment #1: Type: text/plain, Size: 4220 bytes --]
On Tue, Mar 19, 2019 at 10:36:19AM -0600, Alex Williamson wrote:
> On Fri, 15 Mar 2019 19:18:35 +1100
> Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
>
> > The NVIDIA V100 SXM2 GPUs are connected to the CPU via PCIe links and
> > (on POWER9) NVLinks. In addition to that, GPUs themselves have direct
> > peer to peer NVLinks in groups of 2 to 4 GPUs. At the moment the POWERNV
> > platform puts all interconnected GPUs to the same IOMMU group.
> >
> > However the user may want to pass individual GPUs to the userspace so
> > in order to do so we need to put them into separate IOMMU groups and
> > cut off the interconnects.
> >
> > Thankfully V100 GPUs implement an interface to do by programming link
> > disabling mask to BAR0 of a GPU. Once a link is disabled in a GPU using
> > this interface, it cannot be re-enabled until the secondary bus reset is
> > issued to the GPU.
> >
> > This defines a reset_done() handler for V100 NVlink2 device which
> > determines what links need to be disabled. This relies on presence
> > of the new "ibm,nvlink-peers" device tree property of a GPU telling which
> > PCI peers it is connected to (which includes NVLink bridges or peer GPUs).
> >
> > This does not change the existing behaviour and instead adds
> > a new "isolate_nvlink" kernel parameter to allow such isolation.
> >
> > The alternative approaches would be:
> >
> > 1. do this in the system firmware (skiboot) but for that we would need
> > to tell skiboot via an additional OPAL call whether or not we want this
> > isolation - skiboot is unaware of IOMMU groups.
> >
> > 2. do this in the secondary bus reset handler in the POWERNV platform -
> > the problem with that is at that point the device is not enabled, i.e.
> > config space is not restored so we need to enable the device (i.e. MMIO
> > bit in CMD register + program valid address to BAR0) in order to disable
> > links and then perhaps undo all this initialization to bring the device
> > back to the state where pci_try_reset_function() expects it to be.
>
> The trouble seems to be that this approach only maintains the isolation
> exposed by the IOMMU group when vfio-pci is the active driver for the
> device. IOMMU groups can be used by any driver and the IOMMU core is
> incorporating groups in various ways.
I don't think that reasoning is quite right. An IOMMU group doesn't
necessarily represent devices which *are* isolated, just devices which
*can be* isolated. There are plenty of instances when we don't need
to isolate devices in different IOMMU groups: passing both groups to
the same guest or userspace VFIO driver for example, or indeed when
both groups are owned by regular host kernel drivers.
In at least some of those cases we also don't want to isolate the
devices when we don't have to, usually for performance reasons.
> So, if there's a device specific
> way to configure the isolation reported in the group, which requires
> some sort of active management against things like secondary bus
> resets, then I think we need to manage it above the attached endpoint
> driver.
The problem is that above the endpoint driver, we don't actually have
enough information about what should be isolated. For VFIO we want to
isolate things if they're in different containers, for most regular
host kernel drivers we don't need to isolate at all (although we might
as well when it doesn't have a cost). The host side nVidia GPGPU
drivers also won't want to isolate the (host owned) NVLink devices
from each other, since they'll want to use the fast interconnects
> Ideally I'd see this as a set of PCI quirks so that we might
> leverage it beyond POWER platforms. I'm not sure how we get past the
> reliance on device tree properties that we won't have on other
> platforms though, if only NVIDIA could at least open a spec addressing
> the discovery and configuration of NVLink registers on their
> devices :-\ Thanks,
Yeah, that'd be nice :/.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH 1/8] powerpc/64: Adjust order in pcibios_init()
From: Alexey Kardashevskiy @ 2019-03-20 6:03 UTC (permalink / raw)
To: Sam Bobroff, linuxppc-dev
In-Reply-To: <fcdb48612cb4cb777ed33a617b932b054170b074.1553050609.git.sbobroff@linux.ibm.com>
On 20/03/2019 13:58, Sam Bobroff wrote:
> The pcibios_init() function for 64 bit PowerPC currently calls
> pci_bus_add_devices() before pcibios_resource_survey(), which seems
> incorrect because it adds devices and attempts to bind their drivers
> before allocating their resources (although no problems seem to be
> apparent).
>
> So move the call to pci_bus_add_devices() to after
> pcibios_resource_survey(), while extracting call to the
> pcibios_fixup() hook so that it remains in the same location.
>
> This will also allow the ppc_md.pcibios_bus_add_device() hooks to
> perform actions that depend on PCI resources, both during rescanning
> (where this is already the case) and at boot time, to support future
> work.
>
> Signed-off-by: Sam Bobroff <sbobroff@linux.ibm.com>
Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> ---
> arch/powerpc/kernel/pci-common.c | 4 ----
> arch/powerpc/kernel/pci_32.c | 4 ++++
> arch/powerpc/kernel/pci_64.c | 12 +++++++++---
> 3 files changed, 13 insertions(+), 7 deletions(-)
>
> diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c
> index ff4b7539cbdf..3146eb73e3b3 100644
> --- a/arch/powerpc/kernel/pci-common.c
> +++ b/arch/powerpc/kernel/pci-common.c
> @@ -1383,10 +1383,6 @@ void __init pcibios_resource_survey(void)
> pr_debug("PCI: Assigning unassigned resources...\n");
> pci_assign_unassigned_resources();
> }
> -
> - /* Call machine dependent fixup */
> - if (ppc_md.pcibios_fixup)
> - ppc_md.pcibios_fixup();
> }
>
> /* This is used by the PCI hotplug driver to allocate resource
> diff --git a/arch/powerpc/kernel/pci_32.c b/arch/powerpc/kernel/pci_32.c
> index d3f04f2d8249..40aaa1a6e193 100644
> --- a/arch/powerpc/kernel/pci_32.c
> +++ b/arch/powerpc/kernel/pci_32.c
> @@ -259,6 +259,10 @@ static int __init pcibios_init(void)
> /* Call common code to handle resource allocation */
> pcibios_resource_survey();
>
> + /* Call machine dependent fixup */
> + if (ppc_md.pcibios_fixup)
> + ppc_md.pcibios_fixup();
> +
> /* Call machine dependent post-init code */
> if (ppc_md.pcibios_after_init)
> ppc_md.pcibios_after_init();
> diff --git a/arch/powerpc/kernel/pci_64.c b/arch/powerpc/kernel/pci_64.c
> index 9d8c10d55407..6f16f30031d7 100644
> --- a/arch/powerpc/kernel/pci_64.c
> +++ b/arch/powerpc/kernel/pci_64.c
> @@ -58,14 +58,20 @@ static int __init pcibios_init(void)
> pci_add_flags(PCI_ENABLE_PROC_DOMAINS | PCI_COMPAT_DOMAIN_0);
>
> /* Scan all of the recorded PCI controllers. */
> - list_for_each_entry_safe(hose, tmp, &hose_list, list_node) {
> + list_for_each_entry_safe(hose, tmp, &hose_list, list_node)
> pcibios_scan_phb(hose);
> - pci_bus_add_devices(hose->bus);
> - }
>
> /* Call common code to handle resource allocation */
> pcibios_resource_survey();
>
> + /* Add devices. */
> + list_for_each_entry_safe(hose, tmp, &hose_list, list_node)
> + pci_bus_add_devices(hose->bus);
> +
> + /* Call machine dependent fixup */
> + if (ppc_md.pcibios_fixup)
> + ppc_md.pcibios_fixup();
> +
> printk(KERN_DEBUG "PCI: Probing PCI hardware done\n");
>
> return 0;
>
--
Alexey
^ permalink raw reply
* Re: [PATCH 2/8] powerpc/eeh: Clear stale EEH_DEV_NO_HANDLER flag
From: Alexey Kardashevskiy @ 2019-03-20 6:02 UTC (permalink / raw)
To: Sam Bobroff, linuxppc-dev
In-Reply-To: <28b287eb3939b1941bd46b2ed9a6981a577568c4.1553050609.git.sbobroff@linux.ibm.com>
On 20/03/2019 13:58, Sam Bobroff wrote:
> The EEH_DEV_NO_HANDLER flag is used by the EEH system to prevent the
> use of driver callbacks in drivers that have been bound part way
> through the recovery process. This is necessary to prevent later stage
> handlers from being called when the earlier stage handlers haven't,
> which can be confusing for drivers.
The flag is used from eeh_pe_report()->eeh_pe_report_edev which is
called many times from eeh_handle_normal_event() (and you clear the flag
here unconditionally) and once from eeh_handle_special_event() - so this
is actually the only case now when the flag matters. Is my understanding
correct? Also is not clearing the flag correct in that case? I do not
quite understand eeh_handle_normal_event vs. eeh_handle_special_event
business though.
>
> However, the flag is set for all devices that are added after boot
> time and only cleared at the end of the EEH recovery process. This
> results in hot plugged devices erroneously having the flag set during
> the first recovery after they are added (causing their driver's
> handlers to be incorrectly ignored).
>
> To remedy this, clear the flag at the beginning of recovery
> processing. The flag is still cleared at the end of recovery
> processing, although it is no longer really necessary.
Then may be remove that redundant clearing?
>
> Signed-off-by: Sam Bobroff <sbobroff@linux.ibm.com>
> ---
> arch/powerpc/kernel/eeh_driver.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/arch/powerpc/kernel/eeh_driver.c b/arch/powerpc/kernel/eeh_driver.c
> index 6f3ee30565dd..4c34b9901f15 100644
> --- a/arch/powerpc/kernel/eeh_driver.c
> +++ b/arch/powerpc/kernel/eeh_driver.c
> @@ -819,6 +819,10 @@ void eeh_handle_normal_event(struct eeh_pe *pe)
> result = PCI_ERS_RESULT_DISCONNECT;
> }
>
> + eeh_for_each_pe(pe, tmp_pe)
> + eeh_pe_for_each_dev(tmp_pe, edev, tmp)
> + edev->mode &= ~EEH_DEV_NO_HANDLER;
> +
> /* Walk the various device drivers attached to this slot through
> * a reset sequence, giving each an opportunity to do what it needs
> * to accomplish the reset. Each child gets a report of the
>
--
Alexey
^ permalink raw reply
* Re: [PATCH 3/8] powerpc/eeh: Convert PNV_PHB_FLAG_EEH to global flag
From: Alexey Kardashevskiy @ 2019-03-20 6:02 UTC (permalink / raw)
To: Sam Bobroff, linuxppc-dev
In-Reply-To: <17f0dc9c30a139f19dceefd09689d34c3ad01a17.1553050609.git.sbobroff@linux.ibm.com>
On 20/03/2019 13:58, Sam Bobroff wrote:
> The PHB flag, PNV_PHB_FLAG_EEH, is set (on PowerNV) individually on
> each PHB once the EEH subsystem is ready. It is the only use of the
> flags member of the phb struct.
Then why to keep pnv_phb::flags?
> However there is no need to store this separately on each PHB, so
> convert it to a global flag. For symmetry, the flag is now also set
> for pSeries; although it is currently unused it may be useful in the
> future.
Just using eeh_enabled() instead of (phb->flags & PNV_PHB_FLAG_EEH)
seems easier and cleaner; also pseries does not use it so there is no
point defining it there either.
>
> Signed-off-by: Sam Bobroff <sbobroff@linux.ibm.com>
> ---
> arch/powerpc/include/asm/eeh.h | 11 +++++++++++
> arch/powerpc/platforms/powernv/eeh-powernv.c | 14 +++-----------
> arch/powerpc/platforms/powernv/pci.c | 7 +++----
> arch/powerpc/platforms/powernv/pci.h | 2 --
> arch/powerpc/platforms/pseries/pci.c | 4 ++++
> 5 files changed, 21 insertions(+), 17 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/eeh.h b/arch/powerpc/include/asm/eeh.h
> index 3613a56281f2..fe4cf7208890 100644
> --- a/arch/powerpc/include/asm/eeh.h
> +++ b/arch/powerpc/include/asm/eeh.h
> @@ -43,6 +43,7 @@ struct pci_dn;
> #define EEH_VALID_PE_ZERO 0x10 /* PE#0 is valid */
> #define EEH_ENABLE_IO_FOR_LOG 0x20 /* Enable IO for log */
> #define EEH_EARLY_DUMP_LOG 0x40 /* Dump log immediately */
> +#define EEH_PHB_ENABLED 0x80 /* PHB recovery uses EEH */
>
> /*
> * Delay for PE reset, all in ms
> @@ -245,6 +246,11 @@ static inline bool eeh_enabled(void)
> return eeh_has_flag(EEH_ENABLED) && !eeh_has_flag(EEH_FORCE_DISABLED);
> }
>
> +static inline bool eeh_phb_enabled(void)
> +{
> + return eeh_has_flag(EEH_PHB_ENABLED);
> +}
> +
> static inline void eeh_serialize_lock(unsigned long *flags)
> {
> raw_spin_lock_irqsave(&confirm_error_lock, *flags);
> @@ -332,6 +338,11 @@ static inline bool eeh_enabled(void)
> return false;
> }
>
> +static inline bool eeh_phb_enabled(void)
> +{
> + return false;
> +}
> +
> static inline void eeh_probe_devices(void) { }
>
> static inline void *eeh_dev_init(struct pci_dn *pdn, void *data)
> diff --git a/arch/powerpc/platforms/powernv/eeh-powernv.c b/arch/powerpc/platforms/powernv/eeh-powernv.c
> index 6fc1a463b796..f0a95f663810 100644
> --- a/arch/powerpc/platforms/powernv/eeh-powernv.c
> +++ b/arch/powerpc/platforms/powernv/eeh-powernv.c
> @@ -264,22 +264,14 @@ int pnv_eeh_post_init(void)
> return ret;
> }
>
> - if (!eeh_enabled())
> + if (eeh_enabled())
> + eeh_add_flag(EEH_PHB_ENABLED);
> + else
> disable_irq(eeh_event_irq);
>
> list_for_each_entry(hose, &hose_list, list_node) {
> phb = hose->private_data;
>
> - /*
> - * If EEH is enabled, we're going to rely on that.
> - * Otherwise, we restore to conventional mechanism
> - * to clear frozen PE during PCI config access.
> - */
> - if (eeh_enabled())
> - phb->flags |= PNV_PHB_FLAG_EEH;
> - else
> - phb->flags &= ~PNV_PHB_FLAG_EEH;
> -
> /* Create debugfs entries */
> #ifdef CONFIG_DEBUG_FS
> if (phb->has_dbgfs || !phb->dbgfs)
> diff --git a/arch/powerpc/platforms/powernv/pci.c b/arch/powerpc/platforms/powernv/pci.c
> index 307181fd8a17..d2b50f3bf6b1 100644
> --- a/arch/powerpc/platforms/powernv/pci.c
> +++ b/arch/powerpc/platforms/powernv/pci.c
> @@ -717,10 +717,9 @@ int pnv_pci_cfg_write(struct pci_dn *pdn,
> static bool pnv_pci_cfg_check(struct pci_dn *pdn)
> {
> struct eeh_dev *edev = NULL;
> - struct pnv_phb *phb = pdn->phb->private_data;
>
> /* EEH not enabled ? */
> - if (!(phb->flags & PNV_PHB_FLAG_EEH))
> + if (!eeh_phb_enabled())
> return true;
>
> /* PE reset or device removed ? */
> @@ -761,7 +760,7 @@ static int pnv_pci_read_config(struct pci_bus *bus,
>
> ret = pnv_pci_cfg_read(pdn, where, size, val);
> phb = pdn->phb->private_data;
> - if (phb->flags & PNV_PHB_FLAG_EEH && pdn->edev) {
> + if (eeh_phb_enabled() && pdn->edev) {
> if (*val == EEH_IO_ERROR_VALUE(size) &&
> eeh_dev_check_failure(pdn->edev))
> return PCIBIOS_DEVICE_NOT_FOUND;
> @@ -789,7 +788,7 @@ static int pnv_pci_write_config(struct pci_bus *bus,
>
> ret = pnv_pci_cfg_write(pdn, where, size, val);
> phb = pdn->phb->private_data;
> - if (!(phb->flags & PNV_PHB_FLAG_EEH))
> + if (!eeh_phb_enabled())
> pnv_pci_config_check_eeh(pdn);
>
> return ret;
> diff --git a/arch/powerpc/platforms/powernv/pci.h b/arch/powerpc/platforms/powernv/pci.h
> index 8e36da379252..eb0add61397b 100644
> --- a/arch/powerpc/platforms/powernv/pci.h
> +++ b/arch/powerpc/platforms/powernv/pci.h
> @@ -85,8 +85,6 @@ struct pnv_ioda_pe {
> struct list_head list;
> };
>
> -#define PNV_PHB_FLAG_EEH (1 << 0)
> -
> struct pnv_phb {
> struct pci_controller *hose;
> enum pnv_phb_type type;
> diff --git a/arch/powerpc/platforms/pseries/pci.c b/arch/powerpc/platforms/pseries/pci.c
> index 37a77e57893e..7be80882c08d 100644
> --- a/arch/powerpc/platforms/pseries/pci.c
> +++ b/arch/powerpc/platforms/pseries/pci.c
> @@ -244,6 +244,10 @@ void __init pSeries_final_fixup(void)
>
> eeh_probe_devices();
> eeh_addr_cache_build();
> +#ifdef CONFIG_EEH
> + if (eeh_enabled())
> + eeh_add_flag(EEH_PHB_ENABLED);
> +#endif
>
> #ifdef CONFIG_PCI_IOV
> ppc_md.pcibios_sriov_enable = pseries_pcibios_sriov_enable;
>
--
Alexey
^ permalink raw reply
* Re: [PATCH 4/8] powerpc/eeh: Improve debug messages around device addition
From: Alexey Kardashevskiy @ 2019-03-20 6:02 UTC (permalink / raw)
To: Sam Bobroff, linuxppc-dev
In-Reply-To: <7ae025867a92715bb6fe253834211e2745f53787.1553050609.git.sbobroff@linux.ibm.com>
On 20/03/2019 13:58, Sam Bobroff wrote:
> Also remove useless comment.
Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>
> Signed-off-by: Sam Bobroff <sbobroff@linux.ibm.com>
> ---
> arch/powerpc/kernel/eeh.c | 2 +-
> arch/powerpc/platforms/powernv/eeh-powernv.c | 14 ++++++++----
> arch/powerpc/platforms/pseries/eeh_pseries.c | 23 +++++++++++++++-----
> 3 files changed, 28 insertions(+), 11 deletions(-)
>
> diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
> index 8d3c36a1f194..b14d89547895 100644
> --- a/arch/powerpc/kernel/eeh.c
> +++ b/arch/powerpc/kernel/eeh.c
> @@ -1291,7 +1291,7 @@ void eeh_add_device_late(struct pci_dev *dev)
> pdn = pci_get_pdn_by_devfn(dev->bus, dev->devfn);
> edev = pdn_to_eeh_dev(pdn);
> if (edev->pdev == dev) {
> - pr_debug("EEH: Already referenced !\n");
> + pr_debug("EEH: Device %s already referenced!\n", pci_name(dev));
> return;
> }
>
> diff --git a/arch/powerpc/platforms/powernv/eeh-powernv.c b/arch/powerpc/platforms/powernv/eeh-powernv.c
> index f0a95f663810..51c5b6bb9b0e 100644
> --- a/arch/powerpc/platforms/powernv/eeh-powernv.c
> +++ b/arch/powerpc/platforms/powernv/eeh-powernv.c
> @@ -50,10 +50,7 @@ void pnv_pcibios_bus_add_device(struct pci_dev *pdev)
> if (!pdev->is_virtfn)
> return;
>
> - /*
> - * The following operations will fail if VF's sysfs files
> - * aren't created or its resources aren't finalized.
> - */
> + pr_debug("%s: EEH: Setting up device %s.\n", __func__, pci_name(pdev));
> eeh_add_device_early(pdn);
> eeh_add_device_late(pdev);
> eeh_sysfs_add_device(pdev);
> @@ -389,6 +386,10 @@ static void *pnv_eeh_probe(struct pci_dn *pdn, void *data)
> int ret;
> int config_addr = (pdn->busno << 8) | (pdn->devfn);
>
> + pr_debug("%s: probing %04x:%02x:%02x.%01x\n",
> + __func__, hose->global_number, pdn->busno,
> + PCI_SLOT(pdn->devfn), PCI_FUNC(pdn->devfn));
> +
> /*
> * When probing the root bridge, which doesn't have any
> * subordinate PCI devices. We don't have OF node for
> @@ -483,6 +484,11 @@ static void *pnv_eeh_probe(struct pci_dn *pdn, void *data)
> /* Save memory bars */
> eeh_save_bars(edev);
>
> + pr_debug("%s: EEH enabled on %02x:%02x.%01x PHB#%x-PE#%x\n",
> + __func__, pdn->busno, PCI_SLOT(pdn->devfn),
> + PCI_FUNC(pdn->devfn), edev->pe->phb->global_number,
> + edev->pe->addr);
> +
> return NULL;
> }
>
> diff --git a/arch/powerpc/platforms/pseries/eeh_pseries.c b/arch/powerpc/platforms/pseries/eeh_pseries.c
> index 7aa50258dd42..ae06878fbdea 100644
> --- a/arch/powerpc/platforms/pseries/eeh_pseries.c
> +++ b/arch/powerpc/platforms/pseries/eeh_pseries.c
> @@ -65,6 +65,8 @@ void pseries_pcibios_bus_add_device(struct pci_dev *pdev)
> if (!pdev->is_virtfn)
> return;
>
> + pr_debug("%s: EEH: Setting up device %s.\n", __func__, pci_name(pdev));
> +
> pdn->device_id = pdev->device;
> pdn->vendor_id = pdev->vendor;
> pdn->class_code = pdev->class;
> @@ -251,6 +253,10 @@ static void *pseries_eeh_probe(struct pci_dn *pdn, void *data)
> int enable = 0;
> int ret;
>
> + pr_debug("%s: probing %04x:%02x:%02x.%01x\n",
> + __func__, pdn->phb->global_number, pdn->busno,
> + PCI_SLOT(pdn->devfn), PCI_FUNC(pdn->devfn));
> +
> /* Retrieve OF node and eeh device */
> edev = pdn_to_eeh_dev(pdn);
> if (!edev || edev->pe)
> @@ -294,7 +300,12 @@ static void *pseries_eeh_probe(struct pci_dn *pdn, void *data)
>
> /* Enable EEH on the device */
> ret = eeh_ops->set_option(&pe, EEH_OPT_ENABLE);
> - if (!ret) {
> + if (ret) {
> + pr_debug("%s: EEH failed to enable on %02x:%02x.%01x PHB#%x-PE#%x (code %d)\n",
> + __func__, pdn->busno, PCI_SLOT(pdn->devfn),
> + PCI_FUNC(pdn->devfn), pe.phb->global_number,
> + pe.addr, ret);
> + } else {
> /* Retrieve PE address */
> edev->pe_config_addr = eeh_ops->get_pe_addr(&pe);
> pe.addr = edev->pe_config_addr;
> @@ -310,11 +321,6 @@ static void *pseries_eeh_probe(struct pci_dn *pdn, void *data)
> if (enable) {
> eeh_add_flag(EEH_ENABLED);
> eeh_add_to_parent_pe(edev);
> -
> - pr_debug("%s: EEH enabled on %02x:%02x.%01x PHB#%x-PE#%x\n",
> - __func__, pdn->busno, PCI_SLOT(pdn->devfn),
> - PCI_FUNC(pdn->devfn), pe.phb->global_number,
> - pe.addr);
> } else if (pdn->parent && pdn_to_eeh_dev(pdn->parent) &&
> (pdn_to_eeh_dev(pdn->parent))->pe) {
> /* This device doesn't support EEH, but it may have an
> @@ -323,6 +329,11 @@ static void *pseries_eeh_probe(struct pci_dn *pdn, void *data)
> edev->pe_config_addr = pdn_to_eeh_dev(pdn->parent)->pe_config_addr;
> eeh_add_to_parent_pe(edev);
> }
> + pr_debug("%s: EEH %s on %02x:%02x.%01x PHB#%x-PE#%x (code %d)\n",
> + __func__, (enable ? "enabled" : "unsupported"),
> + pdn->busno, PCI_SLOT(pdn->devfn),
> + PCI_FUNC(pdn->devfn), pe.phb->global_number,
> + pe.addr, ret);
> }
>
> /* Save memory bars */
>
--
Alexey
^ permalink raw reply
* Re: [PATCH 5/8] powerpc/eeh: Add eeh_show_enabled()
From: Alexey Kardashevskiy @ 2019-03-20 6:02 UTC (permalink / raw)
To: Sam Bobroff, linuxppc-dev
In-Reply-To: <ad8cf11e3151fd39c6a9deedc0098d50274c75e4.1553050609.git.sbobroff@linux.ibm.com>
On 20/03/2019 13:58, Sam Bobroff wrote:
> Move the EEH enabled message into it's own function so that future
> work can call it from multiple places.
>
> Signed-off-by: Sam Bobroff <sbobroff@linux.ibm.com>
> ---
> arch/powerpc/include/asm/eeh.h | 3 +++
> arch/powerpc/kernel/eeh.c | 16 +++++++++++-----
> 2 files changed, 14 insertions(+), 5 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/eeh.h b/arch/powerpc/include/asm/eeh.h
> index fe4cf7208890..e217ccda55d0 100644
> --- a/arch/powerpc/include/asm/eeh.h
> +++ b/arch/powerpc/include/asm/eeh.h
> @@ -289,6 +289,7 @@ 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);
> void eeh_probe_devices(void);
> int __init eeh_ops_register(struct eeh_ops *ops);
> int __exit eeh_ops_unregister(const char *name);
> @@ -338,6 +339,8 @@ static inline bool eeh_enabled(void)
> return false;
> }
>
> +static inline void eeh_show_enabled(void) { }
> +
> static inline bool eeh_phb_enabled(void)
> {
> return false;
> diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
> index b14d89547895..3dcff29cb9b3 100644
> --- a/arch/powerpc/kernel/eeh.c
> +++ b/arch/powerpc/kernel/eeh.c
> @@ -163,6 +163,16 @@ static int __init eeh_setup(char *str)
> }
> __setup("eeh=", eeh_setup);
>
> +void eeh_show_enabled(void)
> +{
> + if (eeh_has_flag(EEH_FORCE_DISABLED))
> + pr_info("EEH: PCI Enhanced I/O Error Handling DISABLED (by eeh=off)\n");
> + else if (eeh_enabled())
I'd make it eeh_has_flag(EEH_ENABLED) for clarity.
> + pr_info("EEH: PCI Enhanced I/O Error Handling ENABLED (capable adapter found)\n");
> + else
> + pr_info("EEH: PCI Enhanced I/O Error Handling DISABLED (no capable adapter found)\n");
> +}
> +
> /*
> * This routine captures assorted PCI configuration space data
> * for the indicated PCI device, and puts them into a buffer
> @@ -1166,11 +1176,7 @@ void eeh_probe_devices(void)
> pdn = hose->pci_data;
> traverse_pci_dn(pdn, eeh_ops->probe, NULL);
> }
> - if (eeh_enabled())
> - pr_info("EEH: PCI Enhanced I/O Error Handling Enabled\n");
> - else
> - pr_info("EEH: No capable adapters found\n");
> -
> + eeh_show_enabled();
This line moves later in the series so I'd just merge this patch into
8/8 to reduce number of lines moving withing the patchset.
In general the whole point of the EEH_ENABLED flag is fading away. Its
meaning now is that "at least somewhere in the box for at least one
device with enabled EEH" which does not seem extremely useful as we have
a pci_dev or pe pretty much everywhere we look at eeh_enabled() and
pdev->dev.archdata.edev can tell if eeh is enabled for a device.
Although I am pretty sure this is in your list already :)
> }
>
> /**
>
--
Alexey
^ permalink raw reply
* Re: [PATCH 6/8] powerpc/eeh: Initialize EEH address cache earlier
From: Alexey Kardashevskiy @ 2019-03-20 6:02 UTC (permalink / raw)
To: Sam Bobroff, linuxppc-dev
In-Reply-To: <0ca5b600a69f73323b269e4bfdaec1a53cdb4282.1553050609.git.sbobroff@linux.ibm.com>
On 20/03/2019 13:58, Sam Bobroff wrote:
> The EEH address cache is currently initialized and populated by a
> single function: eeh_addr_cache_build(). While the initial population
> of the cache can only be done once resources are allocated,
> initialization (just setting up a spinlock) could be done much
> earlier.
>
> So move the initialization step into a separate function and call it
> from a core_initcall (rather than a subsys initcall).
>
> This will allow future work to make use of the cache during boot time
> PCI scanning.
>
> Signed-off-by: Sam Bobroff <sbobroff@linux.ibm.com>
Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> ---
> arch/powerpc/include/asm/eeh.h | 3 +++
> arch/powerpc/kernel/eeh.c | 2 ++
> arch/powerpc/kernel/eeh_cache.c | 13 +++++++++++--
> 3 files changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/eeh.h b/arch/powerpc/include/asm/eeh.h
> index e217ccda55d0..791b9e6fcc45 100644
> --- a/arch/powerpc/include/asm/eeh.h
> +++ b/arch/powerpc/include/asm/eeh.h
> @@ -295,6 +295,7 @@ int __init eeh_ops_register(struct eeh_ops *ops);
> int __exit eeh_ops_unregister(const char *name);
> int eeh_check_failure(const volatile void __iomem *token);
> int eeh_dev_check_failure(struct eeh_dev *edev);
> +void eeh_addr_cache_init(void);
> void eeh_addr_cache_build(void);
> void eeh_add_device_early(struct pci_dn *);
> void eeh_add_device_tree_early(struct pci_dn *);
> @@ -362,6 +363,8 @@ static inline int eeh_check_failure(const volatile void __iomem *token)
>
> #define eeh_dev_check_failure(x) (0)
>
> +static inline void eeh_addr_cache_init(void) { }
> +
> static inline void eeh_addr_cache_build(void) { }
>
> static inline void eeh_add_device_early(struct pci_dn *pdn) { }
> diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
> index 3dcff29cb9b3..7a406d58d2c0 100644
> --- a/arch/powerpc/kernel/eeh.c
> +++ b/arch/powerpc/kernel/eeh.c
> @@ -1219,6 +1219,8 @@ static int eeh_init(void)
> list_for_each_entry_safe(hose, tmp, &hose_list, list_node)
> eeh_dev_phb_init_dynamic(hose);
>
> + eeh_addr_cache_init();
> +
> /* Initialize EEH event */
> return eeh_event_init();
> }
> diff --git a/arch/powerpc/kernel/eeh_cache.c b/arch/powerpc/kernel/eeh_cache.c
> index 9c68f0837385..f93dd5cf6a39 100644
> --- a/arch/powerpc/kernel/eeh_cache.c
> +++ b/arch/powerpc/kernel/eeh_cache.c
> @@ -267,6 +267,17 @@ void eeh_addr_cache_rmv_dev(struct pci_dev *dev)
> spin_unlock_irqrestore(&pci_io_addr_cache_root.piar_lock, flags);
> }
>
> +/**
> + * eeh_addr_cache_init - Initialize a cache of I/O addresses
> + *
> + * Initialize a cache of pci i/o addresses. This cache will be used to
> + * find the pci device that corresponds to a given address.
> + */
> +void eeh_addr_cache_init(void)
> +{
> + spin_lock_init(&pci_io_addr_cache_root.piar_lock);
> +}
> +
> /**
> * eeh_addr_cache_build - Build a cache of I/O addresses
> *
> @@ -282,8 +293,6 @@ void eeh_addr_cache_build(void)
> struct eeh_dev *edev;
> struct pci_dev *dev = NULL;
>
> - spin_lock_init(&pci_io_addr_cache_root.piar_lock);
> -
> for_each_pci_dev(dev) {
> pdn = pci_get_pdn_by_devfn(dev->bus, dev->devfn);
> if (!pdn)
>
--
Alexey
^ permalink raw reply
* Re: [PATCH 7/8] powerpc/eeh: EEH for pSeries hot plug
From: Alexey Kardashevskiy @ 2019-03-20 6:02 UTC (permalink / raw)
To: Sam Bobroff, linuxppc-dev
In-Reply-To: <ec3ab4f1b484d37ff0864b439612c01e7ee42948.1553050609.git.sbobroff@linux.ibm.com>
On 20/03/2019 13:58, Sam Bobroff wrote:
> On PowerNV and pSeries, devices currently acquire EEH support from
> several different places: Boot-time devices from eeh_probe_devices()
> and eeh_addr_cache_build(), Virtual Function devices from the pcibios
> bus add device hooks and hot plugged devices from pci_hp_add_devices()
> (with other platforms using other methods as well). Unfortunately,
> pSeries machines currently discover hot plugged devices using
> pci_rescan_bus(), not pci_hp_add_devices(), and so those devices do
> not receive EEH support.
>
> Rather than adding another case for pci_rescan_bus(), this change
> widens the scope of the pcibios bus add device hooks so that they can
> handle all devices. As a side effect this also supports devices
> discovered after manually rescanning via /sys/bus/pci/rescan.
>
> Note that on PowerNV, this change allows the EEH subsystem to become
> enabled after boot as long as it has not been forced off, which was
> not previously possible (it was already possible on pSeries).
>
> Signed-off-by: Sam Bobroff <sbobroff@linux.ibm.com>
> ---
> arch/powerpc/kernel/eeh.c | 2 +-
> arch/powerpc/kernel/of_platform.c | 3 +-
> arch/powerpc/platforms/powernv/eeh-powernv.c | 8 ++-
> arch/powerpc/platforms/pseries/eeh_pseries.c | 54 ++++++++++----------
> 4 files changed, 35 insertions(+), 32 deletions(-)
>
> diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
> index 7a406d58d2c0..217e14bb1fb6 100644
> --- a/arch/powerpc/kernel/eeh.c
> +++ b/arch/powerpc/kernel/eeh.c
> @@ -1291,7 +1291,7 @@ void eeh_add_device_late(struct pci_dev *dev)
> struct pci_dn *pdn;
> struct eeh_dev *edev;
>
> - if (!dev || !eeh_enabled())
> + if (!dev)
> return;
>
> pr_debug("EEH: Adding device %s\n", pci_name(dev));
> diff --git a/arch/powerpc/kernel/of_platform.c b/arch/powerpc/kernel/of_platform.c
> index becaec990140..d5818e9c4069 100644
> --- a/arch/powerpc/kernel/of_platform.c
> +++ b/arch/powerpc/kernel/of_platform.c
> @@ -86,7 +86,8 @@ static int of_pci_phb_probe(struct platform_device *dev)
> pcibios_claim_one_bus(phb->bus);
>
> /* Finish EEH setup */
> - eeh_add_device_tree_late(phb->bus);
> + if (!eeh_has_flag(EEH_FORCE_DISABLED))
> + eeh_add_device_tree_late(phb->bus);
>
> /* Add probed PCI devices to the device model */
> pci_bus_add_devices(phb->bus);
> diff --git a/arch/powerpc/platforms/powernv/eeh-powernv.c b/arch/powerpc/platforms/powernv/eeh-powernv.c
> index 51c5b6bb9b0e..81b0923cc55f 100644
> --- a/arch/powerpc/platforms/powernv/eeh-powernv.c
> +++ b/arch/powerpc/platforms/powernv/eeh-powernv.c
> @@ -47,7 +47,7 @@ void pnv_pcibios_bus_add_device(struct pci_dev *pdev)
> {
> struct pci_dn *pdn = pci_get_pdn(pdev);
>
> - if (!pdev->is_virtfn)
> + if (eeh_has_flag(EEH_FORCE_DISABLED))
> return;
>
> pr_debug("%s: EEH: Setting up device %s.\n", __func__, pci_name(pdev));
> @@ -479,7 +479,11 @@ static void *pnv_eeh_probe(struct pci_dn *pdn, void *data)
> * Enable EEH explicitly so that we will do EEH check
> * while accessing I/O stuff
> */
> - eeh_add_flag(EEH_ENABLED);
> + if (!eeh_has_flag(EEH_ENABLED)) {
> + enable_irq(eeh_event_irq);
> + eeh_add_flag(EEH_PHB_ENABLED);
Except that I do not think we need EEH_PHB_ENABLED (commented elsewhere),
Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> + eeh_add_flag(EEH_ENABLED);
> + }
>
> /* Save memory bars */
> eeh_save_bars(edev);
> diff --git a/arch/powerpc/platforms/pseries/eeh_pseries.c b/arch/powerpc/platforms/pseries/eeh_pseries.c
> index ae06878fbdea..e68c79164974 100644
> --- a/arch/powerpc/platforms/pseries/eeh_pseries.c
> +++ b/arch/powerpc/platforms/pseries/eeh_pseries.c
> @@ -55,44 +55,44 @@ static int ibm_get_config_addr_info;
> static int ibm_get_config_addr_info2;
> static int ibm_configure_pe;
>
> -#ifdef CONFIG_PCI_IOV
> void pseries_pcibios_bus_add_device(struct pci_dev *pdev)
> {
> struct pci_dn *pdn = pci_get_pdn(pdev);
> - struct pci_dn *physfn_pdn;
> - struct eeh_dev *edev;
>
> - if (!pdev->is_virtfn)
> + if (eeh_has_flag(EEH_FORCE_DISABLED))
> return;
>
> pr_debug("%s: EEH: Setting up device %s.\n", __func__, pci_name(pdev));
> +#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;
> - /*
> - * Last allow unfreeze return code used for retrieval
> - * by user space in eeh-sysfs to show the last command
> - * 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];
> - edev = pdn_to_eeh_dev(pdn);
> -
> - /*
> - * The following operations will fail if VF's sysfs files
> - * aren't created or its resources aren't finalized.
> - */
> + pdn->device_id = pdev->device;
> + pdn->vendor_id = pdev->vendor;
> + pdn->class_code = pdev->class;
> + /*
> + * Last allow unfreeze return code used for retrieval
> + * by user space in eeh-sysfs to show the last command
> + * 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
> eeh_add_device_early(pdn);
> eeh_add_device_late(pdev);
> - edev->pe_config_addr = (pdn->busno << 16) | (pdn->devfn << 8);
> - eeh_rmv_from_parent_pe(edev); /* Remove as it is adding to bus pe */
> - eeh_add_to_parent_pe(edev); /* Add as VF PE type */
> - eeh_sysfs_add_device(pdev);
> +#ifdef CONFIG_PCI_IOV
> + if (pdev->is_virtfn) {
> + struct eeh_dev *edev = pdn_to_eeh_dev(pdn);
>
> -}
> + edev->pe_config_addr = (pdn->busno << 16) | (pdn->devfn << 8);
> + eeh_rmv_from_parent_pe(edev); /* Remove as it is adding to bus pe */
> + eeh_add_to_parent_pe(edev); /* Add as VF PE type */
> + }
> #endif
> + eeh_sysfs_add_device(pdev);
> +}
>
> /*
> * Buffer for reporting slot-error-detail rtas calls. Its here
> @@ -159,10 +159,8 @@ static int pseries_eeh_init(void)
> /* Set EEH probe mode */
> eeh_add_flag(EEH_PROBE_MODE_DEVTREE | EEH_ENABLE_IO_FOR_LOG);
>
> -#ifdef CONFIG_PCI_IOV
> /* Set EEH machine dependent code */
> ppc_md.pcibios_bus_add_device = pseries_pcibios_bus_add_device;
> -#endif
>
> return 0;
> }
>
--
Alexey
^ permalink raw reply
* [PATCH v3 4/5] ocxl: Remove superfluous 'extern' from headers
From: Alastair D'Silva @ 2019-03-20 5:34 UTC (permalink / raw)
To: alastair
Cc: Arnd Bergmann, Greg Kroah-Hartman, linux-kernel, Andrew Donnellan,
Frederic Barrat, linuxppc-dev
In-Reply-To: <20190320053448.2098-1-alastair@au1.ibm.com>
From: Alastair D'Silva <alastair@d-silva.org>
The 'extern' keyword adds no value here.
Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
---
drivers/misc/ocxl/ocxl_internal.h | 54 +++++++++++++++----------------
include/misc/ocxl.h | 36 ++++++++++-----------
2 files changed, 44 insertions(+), 46 deletions(-)
diff --git a/drivers/misc/ocxl/ocxl_internal.h b/drivers/misc/ocxl/ocxl_internal.h
index a32f2151029f..321b29e77f45 100644
--- a/drivers/misc/ocxl/ocxl_internal.h
+++ b/drivers/misc/ocxl/ocxl_internal.h
@@ -16,7 +16,6 @@
extern struct pci_driver ocxl_pci_driver;
-
struct ocxl_fn {
struct device dev;
int bar_used[3];
@@ -92,41 +91,40 @@ struct ocxl_process_element {
__be32 software_state;
};
+struct ocxl_afu *ocxl_afu_get(struct ocxl_afu *afu);
+void ocxl_afu_put(struct ocxl_afu *afu);
-extern struct ocxl_afu *ocxl_afu_get(struct ocxl_afu *afu);
-extern void ocxl_afu_put(struct ocxl_afu *afu);
-
-extern int ocxl_create_cdev(struct ocxl_afu *afu);
-extern void ocxl_destroy_cdev(struct ocxl_afu *afu);
-extern int ocxl_register_afu(struct ocxl_afu *afu);
-extern void ocxl_unregister_afu(struct ocxl_afu *afu);
+int ocxl_create_cdev(struct ocxl_afu *afu);
+void ocxl_destroy_cdev(struct ocxl_afu *afu);
+int ocxl_register_afu(struct ocxl_afu *afu);
+void ocxl_unregister_afu(struct ocxl_afu *afu);
-extern int ocxl_file_init(void);
-extern void ocxl_file_exit(void);
+int ocxl_file_init(void);
+void ocxl_file_exit(void);
-extern int ocxl_pasid_afu_alloc(struct ocxl_fn *fn, u32 size);
-extern void ocxl_pasid_afu_free(struct ocxl_fn *fn, u32 start, u32 size);
-extern int ocxl_actag_afu_alloc(struct ocxl_fn *fn, u32 size);
-extern void ocxl_actag_afu_free(struct ocxl_fn *fn, u32 start, u32 size);
+int ocxl_pasid_afu_alloc(struct ocxl_fn *fn, u32 size);
+void ocxl_pasid_afu_free(struct ocxl_fn *fn, u32 start, u32 size);
+int ocxl_actag_afu_alloc(struct ocxl_fn *fn, u32 size);
+void ocxl_actag_afu_free(struct ocxl_fn *fn, u32 start, u32 size);
-extern struct ocxl_context *ocxl_context_alloc(void);
-extern int ocxl_context_init(struct ocxl_context *ctx, struct ocxl_afu *afu,
+struct ocxl_context *ocxl_context_alloc(void);
+int ocxl_context_init(struct ocxl_context *ctx, struct ocxl_afu *afu,
struct address_space *mapping);
-extern int ocxl_context_attach(struct ocxl_context *ctx, u64 amr);
-extern int ocxl_context_mmap(struct ocxl_context *ctx,
+int ocxl_context_attach(struct ocxl_context *ctx, u64 amr);
+int ocxl_context_mmap(struct ocxl_context *ctx,
struct vm_area_struct *vma);
-extern int ocxl_context_detach(struct ocxl_context *ctx);
-extern void ocxl_context_detach_all(struct ocxl_afu *afu);
-extern void ocxl_context_free(struct ocxl_context *ctx);
+int ocxl_context_detach(struct ocxl_context *ctx);
+void ocxl_context_detach_all(struct ocxl_afu *afu);
+void ocxl_context_free(struct ocxl_context *ctx);
-extern int ocxl_sysfs_add_afu(struct ocxl_afu *afu);
-extern void ocxl_sysfs_remove_afu(struct ocxl_afu *afu);
+int ocxl_sysfs_add_afu(struct ocxl_afu *afu);
+void ocxl_sysfs_remove_afu(struct ocxl_afu *afu);
-extern int ocxl_afu_irq_alloc(struct ocxl_context *ctx, u64 *irq_offset);
-extern int ocxl_afu_irq_free(struct ocxl_context *ctx, u64 irq_offset);
-extern void ocxl_afu_irq_free_all(struct ocxl_context *ctx);
-extern int ocxl_afu_irq_set_fd(struct ocxl_context *ctx, u64 irq_offset,
+int ocxl_afu_irq_alloc(struct ocxl_context *ctx, u64 *irq_offset);
+int ocxl_afu_irq_free(struct ocxl_context *ctx, u64 irq_offset);
+void ocxl_afu_irq_free_all(struct ocxl_context *ctx);
+int ocxl_afu_irq_set_fd(struct ocxl_context *ctx, u64 irq_offset,
int eventfd);
-extern u64 ocxl_afu_irq_get_addr(struct ocxl_context *ctx, u64 irq_offset);
+u64 ocxl_afu_irq_get_addr(struct ocxl_context *ctx, u64 irq_offset);
#endif /* _OCXL_INTERNAL_H_ */
diff --git a/include/misc/ocxl.h b/include/misc/ocxl.h
index 9ff6ddc28e22..4544573cc93c 100644
--- a/include/misc/ocxl.h
+++ b/include/misc/ocxl.h
@@ -53,7 +53,7 @@ struct ocxl_fn_config {
* Read the configuration space of a function and fill in a
* ocxl_fn_config structure with all the function details
*/
-extern int ocxl_config_read_function(struct pci_dev *dev,
+int ocxl_config_read_function(struct pci_dev *dev,
struct ocxl_fn_config *fn);
/*
@@ -62,14 +62,14 @@ extern int ocxl_config_read_function(struct pci_dev *dev,
* AFU indexes can be sparse, so a driver should check all indexes up
* to the maximum found in the function description
*/
-extern int ocxl_config_check_afu_index(struct pci_dev *dev,
+int ocxl_config_check_afu_index(struct pci_dev *dev,
struct ocxl_fn_config *fn, int afu_idx);
/*
* Read the configuration space of a function for the AFU specified by
* the index 'afu_idx'. Fills in a ocxl_afu_config structure
*/
-extern int ocxl_config_read_afu(struct pci_dev *dev,
+int ocxl_config_read_afu(struct pci_dev *dev,
struct ocxl_fn_config *fn,
struct ocxl_afu_config *afu,
u8 afu_idx);
@@ -77,7 +77,7 @@ extern int ocxl_config_read_afu(struct pci_dev *dev,
/*
* Get the max PASID value that can be used by the function
*/
-extern int ocxl_config_get_pasid_info(struct pci_dev *dev, int *count);
+int ocxl_config_get_pasid_info(struct pci_dev *dev, int *count);
/*
* Tell an AFU, by writing in the configuration space, the PASIDs that
@@ -87,7 +87,7 @@ extern int ocxl_config_get_pasid_info(struct pci_dev *dev, int *count);
* 'afu_control_offset' is the offset of the AFU control DVSEC which
* can be found in the function configuration
*/
-extern void ocxl_config_set_afu_pasid(struct pci_dev *dev,
+void ocxl_config_set_afu_pasid(struct pci_dev *dev,
int afu_control_offset,
int pasid_base, u32 pasid_count_log);
@@ -98,7 +98,7 @@ extern void ocxl_config_set_afu_pasid(struct pci_dev *dev,
* 'supported' is the total number of actags desired by all the AFUs
* of the function.
*/
-extern int ocxl_config_get_actag_info(struct pci_dev *dev,
+int ocxl_config_get_actag_info(struct pci_dev *dev,
u16 *base, u16 *enabled, u16 *supported);
/*
@@ -108,7 +108,7 @@ extern int ocxl_config_get_actag_info(struct pci_dev *dev,
* 'func_offset' is the offset of the Function DVSEC that can found in
* the function configuration
*/
-extern void ocxl_config_set_actag(struct pci_dev *dev, int func_offset,
+void ocxl_config_set_actag(struct pci_dev *dev, int func_offset,
u32 actag_base, u32 actag_count);
/*
@@ -118,7 +118,7 @@ extern void ocxl_config_set_actag(struct pci_dev *dev, int func_offset,
* 'afu_control_offset' is the offset of the AFU control DVSEC for the
* desired AFU. It can be found in the AFU configuration
*/
-extern void ocxl_config_set_afu_actag(struct pci_dev *dev,
+void ocxl_config_set_afu_actag(struct pci_dev *dev,
int afu_control_offset,
int actag_base, int actag_count);
@@ -128,7 +128,7 @@ extern void ocxl_config_set_afu_actag(struct pci_dev *dev,
* 'afu_control_offset' is the offset of the AFU control DVSEC for the
* desired AFU. It can be found in the AFU configuration
*/
-extern void ocxl_config_set_afu_state(struct pci_dev *dev,
+void ocxl_config_set_afu_state(struct pci_dev *dev,
int afu_control_offset, int enable);
/*
@@ -139,7 +139,7 @@ extern void ocxl_config_set_afu_state(struct pci_dev *dev,
* between the host and device, and set the Transaction Layer on both
* accordingly.
*/
-extern int ocxl_config_set_TL(struct pci_dev *dev, int tl_dvsec);
+int ocxl_config_set_TL(struct pci_dev *dev, int tl_dvsec);
/*
* Request an AFU to terminate a PASID.
@@ -152,7 +152,7 @@ extern int ocxl_config_set_TL(struct pci_dev *dev, int tl_dvsec);
* 'afu_control_offset' is the offset of the AFU control DVSEC for the
* desired AFU. It can be found in the AFU configuration
*/
-extern int ocxl_config_terminate_pasid(struct pci_dev *dev,
+int ocxl_config_terminate_pasid(struct pci_dev *dev,
int afu_control_offset, int pasid);
/*
@@ -165,13 +165,13 @@ extern int ocxl_config_terminate_pasid(struct pci_dev *dev,
* Returns a 'link handle' that should be used for further calls for
* the link
*/
-extern int ocxl_link_setup(struct pci_dev *dev, int PE_mask,
+int ocxl_link_setup(struct pci_dev *dev, int PE_mask,
void **link_handle);
/*
* Remove the association between the function and its link.
*/
-extern void ocxl_link_release(struct pci_dev *dev, void *link_handle);
+void ocxl_link_release(struct pci_dev *dev, void *link_handle);
/*
* Add a Process Element to the Shared Process Area for a link.
@@ -183,7 +183,7 @@ extern void ocxl_link_release(struct pci_dev *dev, void *link_handle);
* 'xsl_err_data' is an argument passed to the above callback, if
* defined
*/
-extern int ocxl_link_add_pe(void *link_handle, int pasid, u32 pidr, u32 tidr,
+int ocxl_link_add_pe(void *link_handle, int pasid, u32 pidr, u32 tidr,
u64 amr, struct mm_struct *mm,
void (*xsl_err_cb)(void *data, u64 addr, u64 dsisr),
void *xsl_err_data);
@@ -195,12 +195,12 @@ extern int ocxl_link_add_pe(void *link_handle, int pasid, u32 pidr, u32 tidr,
* pasid: the PASID for the AFU context
* tid: the new thread id for the process element
*/
-extern int ocxl_link_update_pe(void *link_handle, int pasid, __u16 tid);
+int ocxl_link_update_pe(void *link_handle, int pasid, __u16 tid);
/*
* Remove a Process Element from the Shared Process Area for a link
*/
-extern int ocxl_link_remove_pe(void *link_handle, int pasid);
+int ocxl_link_remove_pe(void *link_handle, int pasid);
/*
* Allocate an AFU interrupt associated to the link.
@@ -212,12 +212,12 @@ extern int ocxl_link_remove_pe(void *link_handle, int pasid);
* interrupt. It is an MMIO address which needs to be remapped (one
* page).
*/
-extern int ocxl_link_irq_alloc(void *link_handle, int *hw_irq,
+int ocxl_link_irq_alloc(void *link_handle, int *hw_irq,
u64 *obj_handle);
/*
* Free a previously allocated AFU interrupt
*/
-extern void ocxl_link_free_irq(void *link_handle, int hw_irq);
+void ocxl_link_free_irq(void *link_handle, int hw_irq);
#endif /* _MISC_OCXL_H_ */
--
2.20.1
^ permalink raw reply related
* [PATCH v3 0/5] ocxl: OpenCAPI Cleanup
From: Alastair D'Silva @ 2019-03-20 5:34 UTC (permalink / raw)
To: alastair
Cc: Arnd Bergmann, Greg Kroah-Hartman, linux-kernel, Andrew Donnellan,
Frederic Barrat, linuxppc-dev
In-Reply-To: <20190313040702.14276-1-alastair@au1.ibm.com>
From: Alastair D'Silva <alastair@d-silva.org>
Some minor cleanups for the OpenCAPI driver as a prerequisite
for an ocxl driver refactoring to allow the driver core to
be utilised by external drivers.
Changelog:
V3:
- Add missed header in 'ocxl: Remove some unused exported symbols'.
This addresses the introduced sparse warnings
V2:
- remove intermediate assignment of 'link' var in
'Rename struct link to ocxl_link'
- Don't shift definition of ocxl_context_attach in
'Remove some unused exported symbols'
Alastair D'Silva (5):
ocxl: Rename struct link to ocxl_link
ocxl: Clean up printf formats
ocxl: read_pasid never returns an error, so make it void
ocxl: Remove superfluous 'extern' from headers
ocxl: Remove some unused exported symbols
drivers/misc/ocxl/config.c | 17 ++-----
drivers/misc/ocxl/context.c | 2 +-
drivers/misc/ocxl/file.c | 5 +-
drivers/misc/ocxl/link.c | 36 ++++++-------
drivers/misc/ocxl/ocxl_internal.h | 85 +++++++++++++++++++------------
drivers/misc/ocxl/trace.h | 10 ++--
include/misc/ocxl.h | 53 ++++++-------------
7 files changed, 99 insertions(+), 109 deletions(-)
--
2.20.1
^ permalink raw reply
* [PATCH v3 5/5] ocxl: Remove some unused exported symbols
From: Alastair D'Silva @ 2019-03-20 5:34 UTC (permalink / raw)
To: alastair
Cc: Arnd Bergmann, Greg Kroah-Hartman, linux-kernel, Andrew Donnellan,
Frederic Barrat, linuxppc-dev
In-Reply-To: <20190320053448.2098-1-alastair@au1.ibm.com>
From: Alastair D'Silva <alastair@d-silva.org>
Remove some unused exported symbols.
Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
---
drivers/misc/ocxl/config.c | 2 --
drivers/misc/ocxl/ocxl_internal.h | 23 +++++++++++++++++++++++
include/misc/ocxl.h | 23 -----------------------
3 files changed, 23 insertions(+), 25 deletions(-)
diff --git a/drivers/misc/ocxl/config.c b/drivers/misc/ocxl/config.c
index 026ac2ac4f9c..c90c2e4875bf 100644
--- a/drivers/misc/ocxl/config.c
+++ b/drivers/misc/ocxl/config.c
@@ -299,7 +299,6 @@ int ocxl_config_check_afu_index(struct pci_dev *dev,
}
return 1;
}
-EXPORT_SYMBOL_GPL(ocxl_config_check_afu_index);
static int read_afu_name(struct pci_dev *dev, struct ocxl_fn_config *fn,
struct ocxl_afu_config *afu)
@@ -535,7 +534,6 @@ int ocxl_config_get_pasid_info(struct pci_dev *dev, int *count)
{
return pnv_ocxl_get_pasid_count(dev, count);
}
-EXPORT_SYMBOL_GPL(ocxl_config_get_pasid_info);
void ocxl_config_set_afu_pasid(struct pci_dev *dev, int pos, int pasid_base,
u32 pasid_count_log)
diff --git a/drivers/misc/ocxl/ocxl_internal.h b/drivers/misc/ocxl/ocxl_internal.h
index 321b29e77f45..06fd98c989c8 100644
--- a/drivers/misc/ocxl/ocxl_internal.h
+++ b/drivers/misc/ocxl/ocxl_internal.h
@@ -107,6 +107,29 @@ void ocxl_pasid_afu_free(struct ocxl_fn *fn, u32 start, u32 size);
int ocxl_actag_afu_alloc(struct ocxl_fn *fn, u32 size);
void ocxl_actag_afu_free(struct ocxl_fn *fn, u32 start, u32 size);
+/*
+ * Get the max PASID value that can be used by the function
+ */
+int ocxl_config_get_pasid_info(struct pci_dev *dev, int *count);
+
+/*
+ * Check if an AFU index is valid for the given function.
+ *
+ * AFU indexes can be sparse, so a driver should check all indexes up
+ * to the maximum found in the function description
+ */
+int ocxl_config_check_afu_index(struct pci_dev *dev,
+ struct ocxl_fn_config *fn, int afu_idx);
+
+/**
+ * Update values within a Process Element
+ *
+ * link_handle: the link handle associated with the process element
+ * pasid: the PASID for the AFU context
+ * tid: the new thread id for the process element
+ */
+int ocxl_link_update_pe(void *link_handle, int pasid, __u16 tid);
+
struct ocxl_context *ocxl_context_alloc(void);
int ocxl_context_init(struct ocxl_context *ctx, struct ocxl_afu *afu,
struct address_space *mapping);
diff --git a/include/misc/ocxl.h b/include/misc/ocxl.h
index 4544573cc93c..9530d3be1b30 100644
--- a/include/misc/ocxl.h
+++ b/include/misc/ocxl.h
@@ -56,15 +56,6 @@ struct ocxl_fn_config {
int ocxl_config_read_function(struct pci_dev *dev,
struct ocxl_fn_config *fn);
-/*
- * Check if an AFU index is valid for the given function.
- *
- * AFU indexes can be sparse, so a driver should check all indexes up
- * to the maximum found in the function description
- */
-int ocxl_config_check_afu_index(struct pci_dev *dev,
- struct ocxl_fn_config *fn, int afu_idx);
-
/*
* Read the configuration space of a function for the AFU specified by
* the index 'afu_idx'. Fills in a ocxl_afu_config structure
@@ -74,11 +65,6 @@ int ocxl_config_read_afu(struct pci_dev *dev,
struct ocxl_afu_config *afu,
u8 afu_idx);
-/*
- * Get the max PASID value that can be used by the function
- */
-int ocxl_config_get_pasid_info(struct pci_dev *dev, int *count);
-
/*
* Tell an AFU, by writing in the configuration space, the PASIDs that
* it can use. Range starts at 'pasid_base' and its size is a multiple
@@ -188,15 +174,6 @@ int ocxl_link_add_pe(void *link_handle, int pasid, u32 pidr, u32 tidr,
void (*xsl_err_cb)(void *data, u64 addr, u64 dsisr),
void *xsl_err_data);
-/**
- * Update values within a Process Element
- *
- * link_handle: the link handle associated with the process element
- * pasid: the PASID for the AFU context
- * tid: the new thread id for the process element
- */
-int ocxl_link_update_pe(void *link_handle, int pasid, __u16 tid);
-
/*
* Remove a Process Element from the Shared Process Area for a link
*/
--
2.20.1
^ permalink raw reply related
* [PATCH v3 3/5] ocxl: read_pasid never returns an error, so make it void
From: Alastair D'Silva @ 2019-03-20 5:34 UTC (permalink / raw)
To: alastair
Cc: Arnd Bergmann, Greg Kroah-Hartman, Greg Kurz, linux-kernel,
Andrew Donnellan, Frederic Barrat, linuxppc-dev
In-Reply-To: <20190320053448.2098-1-alastair@au1.ibm.com>
From: Alastair D'Silva <alastair@d-silva.org>
No need for a return value in read_pasid as it only returns 0.
Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
Reviewed-by: Greg Kurz <groug@kaod.org>
---
drivers/misc/ocxl/config.c | 9 ++-------
1 file changed, 2 insertions(+), 7 deletions(-)
diff --git a/drivers/misc/ocxl/config.c b/drivers/misc/ocxl/config.c
index 0ee7856b033d..026ac2ac4f9c 100644
--- a/drivers/misc/ocxl/config.c
+++ b/drivers/misc/ocxl/config.c
@@ -68,7 +68,7 @@ static int find_dvsec_afu_ctrl(struct pci_dev *dev, u8 afu_idx)
return 0;
}
-static int read_pasid(struct pci_dev *dev, struct ocxl_fn_config *fn)
+static void read_pasid(struct pci_dev *dev, struct ocxl_fn_config *fn)
{
u16 val;
int pos;
@@ -89,7 +89,6 @@ static int read_pasid(struct pci_dev *dev, struct ocxl_fn_config *fn)
out:
dev_dbg(&dev->dev, "PASID capability:\n");
dev_dbg(&dev->dev, " Max PASID log = %d\n", fn->max_pasid_log);
- return 0;
}
static int read_dvsec_tl(struct pci_dev *dev, struct ocxl_fn_config *fn)
@@ -205,11 +204,7 @@ int ocxl_config_read_function(struct pci_dev *dev, struct ocxl_fn_config *fn)
{
int rc;
- rc = read_pasid(dev, fn);
- if (rc) {
- dev_err(&dev->dev, "Invalid PASID configuration: %d\n", rc);
- return -ENODEV;
- }
+ read_pasid(dev, fn);
rc = read_dvsec_tl(dev, fn);
if (rc) {
--
2.20.1
^ permalink raw reply related
* [PATCH v3 2/5] ocxl: Clean up printf formats
From: Alastair D'Silva @ 2019-03-20 5:34 UTC (permalink / raw)
To: alastair
Cc: Arnd Bergmann, Greg Kroah-Hartman, linux-kernel, Andrew Donnellan,
Frederic Barrat, linuxppc-dev
In-Reply-To: <20190320053448.2098-1-alastair@au1.ibm.com>
From: Alastair D'Silva <alastair@d-silva.org>
Use %# instead of using a literal '0x'
Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
---
drivers/misc/ocxl/config.c | 6 +++---
drivers/misc/ocxl/context.c | 2 +-
drivers/misc/ocxl/trace.h | 10 +++++-----
3 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/misc/ocxl/config.c b/drivers/misc/ocxl/config.c
index 8f2c5d8bd2ee..0ee7856b033d 100644
--- a/drivers/misc/ocxl/config.c
+++ b/drivers/misc/ocxl/config.c
@@ -178,9 +178,9 @@ static int read_dvsec_vendor(struct pci_dev *dev)
pci_read_config_dword(dev, pos + OCXL_DVSEC_VENDOR_DLX_VERS, &dlx);
dev_dbg(&dev->dev, "Vendor specific DVSEC:\n");
- dev_dbg(&dev->dev, " CFG version = 0x%x\n", cfg);
- dev_dbg(&dev->dev, " TLX version = 0x%x\n", tlx);
- dev_dbg(&dev->dev, " DLX version = 0x%x\n", dlx);
+ dev_dbg(&dev->dev, " CFG version = %#x\n", cfg);
+ dev_dbg(&dev->dev, " TLX version = %#x\n", tlx);
+ dev_dbg(&dev->dev, " DLX version = %#x\n", dlx);
return 0;
}
diff --git a/drivers/misc/ocxl/context.c b/drivers/misc/ocxl/context.c
index c10a940e3b38..3498a0199bde 100644
--- a/drivers/misc/ocxl/context.c
+++ b/drivers/misc/ocxl/context.c
@@ -134,7 +134,7 @@ static vm_fault_t ocxl_mmap_fault(struct vm_fault *vmf)
vm_fault_t ret;
offset = vmf->pgoff << PAGE_SHIFT;
- pr_debug("%s: pasid %d address 0x%lx offset 0x%llx\n", __func__,
+ pr_debug("%s: pasid %d address %#lx offset %#llx\n", __func__,
ctx->pasid, vmf->address, offset);
if (offset < ctx->afu->irq_base_offset)
diff --git a/drivers/misc/ocxl/trace.h b/drivers/misc/ocxl/trace.h
index bcb7ff330c1e..68bf2f173a1a 100644
--- a/drivers/misc/ocxl/trace.h
+++ b/drivers/misc/ocxl/trace.h
@@ -28,7 +28,7 @@ DECLARE_EVENT_CLASS(ocxl_context,
__entry->tidr = tidr;
),
- TP_printk("linux pid=%d spa=0x%p pasid=0x%x pidr=0x%x tidr=0x%x",
+ TP_printk("linux pid=%d spa=%p pasid=%#x pidr=%#x tidr=%#x",
__entry->pid,
__entry->spa,
__entry->pasid,
@@ -61,7 +61,7 @@ TRACE_EVENT(ocxl_terminate_pasid,
__entry->rc = rc;
),
- TP_printk("pasid=0x%x rc=%d",
+ TP_printk("pasid=%#x rc=%d",
__entry->pasid,
__entry->rc
)
@@ -87,7 +87,7 @@ DECLARE_EVENT_CLASS(ocxl_fault_handler,
__entry->tfc = tfc;
),
- TP_printk("spa=%p pe=0x%llx dsisr=0x%llx dar=0x%llx tfc=0x%llx",
+ TP_printk("spa=%p pe=%#llx dsisr=%#llx dar=%#llx tfc=%#llx",
__entry->spa,
__entry->pe,
__entry->dsisr,
@@ -127,7 +127,7 @@ TRACE_EVENT(ocxl_afu_irq_alloc,
__entry->irq_offset = irq_offset;
),
- TP_printk("pasid=0x%x irq_id=%d virq=%u hw_irq=%d irq_offset=0x%llx",
+ TP_printk("pasid=%#x irq_id=%d virq=%u hw_irq=%d irq_offset=%#llx",
__entry->pasid,
__entry->irq_id,
__entry->virq,
@@ -150,7 +150,7 @@ TRACE_EVENT(ocxl_afu_irq_free,
__entry->irq_id = irq_id;
),
- TP_printk("pasid=0x%x irq_id=%d",
+ TP_printk("pasid=%#x irq_id=%d",
__entry->pasid,
__entry->irq_id
)
--
2.20.1
^ permalink raw reply related
* [PATCH v3 1/5] ocxl: Rename struct link to ocxl_link
From: Alastair D'Silva @ 2019-03-20 5:34 UTC (permalink / raw)
To: alastair
Cc: Arnd Bergmann, Greg Kroah-Hartman, Greg Kurz, linux-kernel,
Andrew Donnellan, Frederic Barrat, linuxppc-dev
In-Reply-To: <20190320053448.2098-1-alastair@au1.ibm.com>
From: Alastair D'Silva <alastair@d-silva.org>
The term 'link' is ambiguous (especially when the struct is used for a
list), so rename it for clarity.
Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
Reviewed-by: Greg Kurz <groug@kaod.org>
---
drivers/misc/ocxl/file.c | 5 ++---
drivers/misc/ocxl/link.c | 36 ++++++++++++++++++------------------
2 files changed, 20 insertions(+), 21 deletions(-)
diff --git a/drivers/misc/ocxl/file.c b/drivers/misc/ocxl/file.c
index e6a607488f8a..009e09b7ded5 100644
--- a/drivers/misc/ocxl/file.c
+++ b/drivers/misc/ocxl/file.c
@@ -151,10 +151,9 @@ static long afu_ioctl_enable_p9_wait(struct ocxl_context *ctx,
mutex_unlock(&ctx->status_mutex);
if (status == ATTACHED) {
- int rc;
- struct link *link = ctx->afu->fn->link;
+ int rc = ocxl_link_update_pe(ctx->afu->fn->link,
+ ctx->pasid, ctx->tidr);
- rc = ocxl_link_update_pe(link, ctx->pasid, ctx->tidr);
if (rc)
return rc;
}
diff --git a/drivers/misc/ocxl/link.c b/drivers/misc/ocxl/link.c
index d50b861d7e57..8d2690a1a9de 100644
--- a/drivers/misc/ocxl/link.c
+++ b/drivers/misc/ocxl/link.c
@@ -76,7 +76,7 @@ struct spa {
* limited number of opencapi slots on a system and lookup is only
* done when the device is probed
*/
-struct link {
+struct ocxl_link {
struct list_head list;
struct kref ref;
int domain;
@@ -179,7 +179,7 @@ static void xsl_fault_handler_bh(struct work_struct *fault_work)
static irqreturn_t xsl_fault_handler(int irq, void *data)
{
- struct link *link = (struct link *) data;
+ struct ocxl_link *link = (struct ocxl_link *) data;
struct spa *spa = link->spa;
u64 dsisr, dar, pe_handle;
struct pe_data *pe_data;
@@ -256,7 +256,7 @@ static int map_irq_registers(struct pci_dev *dev, struct spa *spa)
&spa->reg_tfc, &spa->reg_pe_handle);
}
-static int setup_xsl_irq(struct pci_dev *dev, struct link *link)
+static int setup_xsl_irq(struct pci_dev *dev, struct ocxl_link *link)
{
struct spa *spa = link->spa;
int rc;
@@ -311,7 +311,7 @@ static int setup_xsl_irq(struct pci_dev *dev, struct link *link)
return rc;
}
-static void release_xsl_irq(struct link *link)
+static void release_xsl_irq(struct ocxl_link *link)
{
struct spa *spa = link->spa;
@@ -323,7 +323,7 @@ static void release_xsl_irq(struct link *link)
unmap_irq_registers(spa);
}
-static int alloc_spa(struct pci_dev *dev, struct link *link)
+static int alloc_spa(struct pci_dev *dev, struct ocxl_link *link)
{
struct spa *spa;
@@ -350,7 +350,7 @@ static int alloc_spa(struct pci_dev *dev, struct link *link)
return 0;
}
-static void free_spa(struct link *link)
+static void free_spa(struct ocxl_link *link)
{
struct spa *spa = link->spa;
@@ -364,12 +364,12 @@ static void free_spa(struct link *link)
}
}
-static int alloc_link(struct pci_dev *dev, int PE_mask, struct link **out_link)
+static int alloc_link(struct pci_dev *dev, int PE_mask, struct ocxl_link **out_link)
{
- struct link *link;
+ struct ocxl_link *link;
int rc;
- link = kzalloc(sizeof(struct link), GFP_KERNEL);
+ link = kzalloc(sizeof(struct ocxl_link), GFP_KERNEL);
if (!link)
return -ENOMEM;
@@ -405,7 +405,7 @@ static int alloc_link(struct pci_dev *dev, int PE_mask, struct link **out_link)
return rc;
}
-static void free_link(struct link *link)
+static void free_link(struct ocxl_link *link)
{
release_xsl_irq(link);
free_spa(link);
@@ -415,7 +415,7 @@ static void free_link(struct link *link)
int ocxl_link_setup(struct pci_dev *dev, int PE_mask, void **link_handle)
{
int rc = 0;
- struct link *link;
+ struct ocxl_link *link;
mutex_lock(&links_list_lock);
list_for_each_entry(link, &links_list, list) {
@@ -442,7 +442,7 @@ EXPORT_SYMBOL_GPL(ocxl_link_setup);
static void release_xsl(struct kref *ref)
{
- struct link *link = container_of(ref, struct link, ref);
+ struct ocxl_link *link = container_of(ref, struct ocxl_link, ref);
list_del(&link->list);
/* call platform code before releasing data */
@@ -452,7 +452,7 @@ static void release_xsl(struct kref *ref)
void ocxl_link_release(struct pci_dev *dev, void *link_handle)
{
- struct link *link = (struct link *) link_handle;
+ struct ocxl_link *link = (struct ocxl_link *) link_handle;
mutex_lock(&links_list_lock);
kref_put(&link->ref, release_xsl);
@@ -488,7 +488,7 @@ int ocxl_link_add_pe(void *link_handle, int pasid, u32 pidr, u32 tidr,
void (*xsl_err_cb)(void *data, u64 addr, u64 dsisr),
void *xsl_err_data)
{
- struct link *link = (struct link *) link_handle;
+ struct ocxl_link *link = (struct ocxl_link *) link_handle;
struct spa *spa = link->spa;
struct ocxl_process_element *pe;
int pe_handle, rc = 0;
@@ -558,7 +558,7 @@ EXPORT_SYMBOL_GPL(ocxl_link_add_pe);
int ocxl_link_update_pe(void *link_handle, int pasid, __u16 tid)
{
- struct link *link = (struct link *) link_handle;
+ struct ocxl_link *link = (struct ocxl_link *) link_handle;
struct spa *spa = link->spa;
struct ocxl_process_element *pe;
int pe_handle, rc;
@@ -594,7 +594,7 @@ int ocxl_link_update_pe(void *link_handle, int pasid, __u16 tid)
int ocxl_link_remove_pe(void *link_handle, int pasid)
{
- struct link *link = (struct link *) link_handle;
+ struct ocxl_link *link = (struct ocxl_link *) link_handle;
struct spa *spa = link->spa;
struct ocxl_process_element *pe;
struct pe_data *pe_data;
@@ -666,7 +666,7 @@ EXPORT_SYMBOL_GPL(ocxl_link_remove_pe);
int ocxl_link_irq_alloc(void *link_handle, int *hw_irq, u64 *trigger_addr)
{
- struct link *link = (struct link *) link_handle;
+ struct ocxl_link *link = (struct ocxl_link *) link_handle;
int rc, irq;
u64 addr;
@@ -687,7 +687,7 @@ EXPORT_SYMBOL_GPL(ocxl_link_irq_alloc);
void ocxl_link_free_irq(void *link_handle, int hw_irq)
{
- struct link *link = (struct link *) link_handle;
+ struct ocxl_link *link = (struct ocxl_link *) link_handle;
pnv_ocxl_free_xive_irq(hw_irq);
atomic_inc(&link->irq_available);
--
2.20.1
^ permalink raw reply related
* [PATCH v2 7/7] ocxl: Provide global MMIO accessors for external drivers
From: Alastair D'Silva @ 2019-03-20 5:08 UTC (permalink / raw)
To: alastair
Cc: Arnd Bergmann, Greg Kroah-Hartman, Greg Kurz, linux-kernel,
Andrew Donnellan, Frederic Barrat, linuxppc-dev
In-Reply-To: <20190320050901.310-1-alastair@au1.ibm.com>
From: Alastair D'Silva <alastair@d-silva.org>
External drivers that communicate via OpenCAPI will need to make
MMIO calls to interact with the devices.
Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
Reviewed-by: Greg Kurz <groug@kaod.org>
---
drivers/misc/ocxl/Makefile | 2 +-
drivers/misc/ocxl/mmio.c | 234 +++++++++++++++++++++++++++++++++++++
include/misc/ocxl.h | 110 +++++++++++++++++
3 files changed, 345 insertions(+), 1 deletion(-)
create mode 100644 drivers/misc/ocxl/mmio.c
diff --git a/drivers/misc/ocxl/Makefile b/drivers/misc/ocxl/Makefile
index bc4e39bfda7b..d07d1bb8e8d4 100644
--- a/drivers/misc/ocxl/Makefile
+++ b/drivers/misc/ocxl/Makefile
@@ -1,7 +1,7 @@
# SPDX-License-Identifier: GPL-2.0+
ccflags-$(CONFIG_PPC_WERROR) += -Werror
-ocxl-y += main.o pci.o config.o file.o pasid.o
+ocxl-y += main.o pci.o config.o file.o pasid.o mmio.o
ocxl-y += link.o context.o afu_irq.o sysfs.o trace.o
ocxl-y += core.o
obj-$(CONFIG_OCXL) += ocxl.o
diff --git a/drivers/misc/ocxl/mmio.c b/drivers/misc/ocxl/mmio.c
new file mode 100644
index 000000000000..7f6ebae1c6c7
--- /dev/null
+++ b/drivers/misc/ocxl/mmio.c
@@ -0,0 +1,234 @@
+// SPDX-License-Identifier: GPL-2.0+
+// Copyright 2017 IBM Corp.
+#include <linux/sched/mm.h>
+#include "trace.h"
+#include "ocxl_internal.h"
+
+int ocxl_global_mmio_read32(struct ocxl_afu *afu, size_t offset,
+ enum ocxl_endian endian, u32 *val)
+{
+ if (offset > afu->config.global_mmio_size - 4)
+ return -EINVAL;
+
+#ifdef __BIG_ENDIAN__
+ if (endian == OCXL_HOST_ENDIAN)
+ endian = OCXL_BIG_ENDIAN;
+#endif
+
+ switch (endian) {
+ case OCXL_BIG_ENDIAN:
+ *val = readl_be((char *)afu->global_mmio_ptr + offset);
+ break;
+
+ default:
+ *val = readl((char *)afu->global_mmio_ptr + offset);
+ break;
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(ocxl_global_mmio_read32);
+
+int ocxl_global_mmio_read64(struct ocxl_afu *afu, size_t offset,
+ enum ocxl_endian endian, u64 *val)
+{
+ if (offset > afu->config.global_mmio_size - 8)
+ return -EINVAL;
+
+#ifdef __BIG_ENDIAN__
+ if (endian == OCXL_HOST_ENDIAN)
+ endian = OCXL_BIG_ENDIAN;
+#endif
+
+ switch (endian) {
+ case OCXL_BIG_ENDIAN:
+ *val = readq_be((char *)afu->global_mmio_ptr + offset);
+ break;
+
+ default:
+ *val = readq((char *)afu->global_mmio_ptr + offset);
+ break;
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(ocxl_global_mmio_read64);
+
+int ocxl_global_mmio_write32(struct ocxl_afu *afu, size_t offset,
+ enum ocxl_endian endian, u32 val)
+{
+ if (offset > afu->config.global_mmio_size - 4)
+ return -EINVAL;
+
+#ifdef __BIG_ENDIAN__
+ if (endian == OCXL_HOST_ENDIAN)
+ endian = OCXL_BIG_ENDIAN;
+#endif
+
+ switch (endian) {
+ case OCXL_BIG_ENDIAN:
+ writel_be(val, (char *)afu->global_mmio_ptr + offset);
+ break;
+
+ default:
+ writel(val, (char *)afu->global_mmio_ptr + offset);
+ break;
+ }
+
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(ocxl_global_mmio_write32);
+
+int ocxl_global_mmio_write64(struct ocxl_afu *afu, size_t offset,
+ enum ocxl_endian endian, u64 val)
+{
+ if (offset > afu->config.global_mmio_size - 8)
+ return -EINVAL;
+
+#ifdef __BIG_ENDIAN__
+ if (endian == OCXL_HOST_ENDIAN)
+ endian = OCXL_BIG_ENDIAN;
+#endif
+
+ switch (endian) {
+ case OCXL_BIG_ENDIAN:
+ writeq_be(val, (char *)afu->global_mmio_ptr + offset);
+ break;
+
+ default:
+ writeq(val, (char *)afu->global_mmio_ptr + offset);
+ break;
+ }
+
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(ocxl_global_mmio_write64);
+
+int ocxl_global_mmio_set32(struct ocxl_afu *afu, size_t offset,
+ enum ocxl_endian endian, u32 mask)
+{
+ u32 tmp;
+
+ if (offset > afu->config.global_mmio_size - 4)
+ return -EINVAL;
+
+#ifdef __BIG_ENDIAN__
+ if (endian == OCXL_HOST_ENDIAN)
+ endian = OCXL_BIG_ENDIAN;
+#endif
+
+ switch (endian) {
+ case OCXL_BIG_ENDIAN:
+ tmp = readl_be((char *)afu->global_mmio_ptr + offset);
+ tmp |= mask;
+ writel_be(tmp, (char *)afu->global_mmio_ptr + offset);
+ break;
+
+ default:
+ tmp = readl((char *)afu->global_mmio_ptr + offset);
+ tmp |= mask;
+ writel(tmp, (char *)afu->global_mmio_ptr + offset);
+ break;
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(ocxl_global_mmio_set32);
+
+int ocxl_global_mmio_set64(struct ocxl_afu *afu, size_t offset,
+ enum ocxl_endian endian, u64 mask)
+{
+ u64 tmp;
+
+ if (offset > afu->config.global_mmio_size - 8)
+ return -EINVAL;
+
+#ifdef __BIG_ENDIAN__
+ if (endian == OCXL_HOST_ENDIAN)
+ endian = OCXL_BIG_ENDIAN;
+#endif
+
+ switch (endian) {
+ case OCXL_BIG_ENDIAN:
+ tmp = readq_be((char *)afu->global_mmio_ptr + offset);
+ tmp |= mask;
+ writeq_be(tmp, (char *)afu->global_mmio_ptr + offset);
+ break;
+
+ default:
+ tmp = readq((char *)afu->global_mmio_ptr + offset);
+ tmp |= mask;
+ writeq(tmp, (char *)afu->global_mmio_ptr + offset);
+ break;
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(ocxl_global_mmio_set64);
+
+int ocxl_global_mmio_clear32(struct ocxl_afu *afu, size_t offset,
+ enum ocxl_endian endian, u32 mask)
+{
+ u32 tmp;
+
+ if (offset > afu->config.global_mmio_size - 4)
+ return -EINVAL;
+
+#ifdef __BIG_ENDIAN__
+ if (endian == OCXL_HOST_ENDIAN)
+ endian = OCXL_BIG_ENDIAN;
+#endif
+
+ switch (endian) {
+ case OCXL_BIG_ENDIAN:
+ tmp = readl_be((char *)afu->global_mmio_ptr + offset);
+ tmp &= ~mask;
+ writel_be(tmp, (char *)afu->global_mmio_ptr + offset);
+ break;
+
+ default:
+ tmp = readl((char *)afu->global_mmio_ptr + offset);
+ tmp &= ~mask;
+ writel(tmp, (char *)afu->global_mmio_ptr + offset);
+ break;
+ }
+
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(ocxl_global_mmio_clear32);
+
+int ocxl_global_mmio_clear64(struct ocxl_afu *afu, size_t offset,
+ enum ocxl_endian endian, u64 mask)
+{
+ u64 tmp;
+
+ if (offset > afu->config.global_mmio_size - 8)
+ return -EINVAL;
+
+#ifdef __BIG_ENDIAN__
+ if (endian == OCXL_HOST_ENDIAN)
+ endian = OCXL_BIG_ENDIAN;
+#endif
+
+ switch (endian) {
+ case OCXL_BIG_ENDIAN:
+ tmp = readq_be((char *)afu->global_mmio_ptr + offset);
+ tmp &= ~mask;
+ writeq_be(tmp, (char *)afu->global_mmio_ptr + offset);
+ break;
+
+ default:
+ tmp = readq((char *)afu->global_mmio_ptr + offset);
+ tmp &= ~mask;
+ writeq(tmp, (char *)afu->global_mmio_ptr + offset);
+ break;
+ }
+
+ writeq(tmp, (char *)afu->global_mmio_ptr + offset);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(ocxl_global_mmio_clear64);
diff --git a/include/misc/ocxl.h b/include/misc/ocxl.h
index 790b25987d9a..45efee29d654 100644
--- a/include/misc/ocxl.h
+++ b/include/misc/ocxl.h
@@ -45,6 +45,12 @@ struct ocxl_fn_config {
s8 max_afu_index;
};
+enum ocxl_endian {
+ OCXL_BIG_ENDIAN = 0, /**< AFU data is big-endian */
+ OCXL_LITTLE_ENDIAN = 1, /**< AFU data is little-endian */
+ OCXL_HOST_ENDIAN = 2, /**< AFU data is the same endianness as the host */
+};
+
// These are opaque outside the ocxl driver
struct ocxl_afu;
struct ocxl_fn;
@@ -238,6 +244,110 @@ void ocxl_afu_set_private(struct ocxl_afu *afu, void *private,
*/
void *ocxl_afu_get_private(struct ocxl_afu *dev);
+// Global MMIO
+/**
+ * Read a 32 bit value from global MMIO
+ *
+ * @afu: The AFU
+ * @offset: The Offset from the start of MMIO
+ * @endian: the endianness that the MMIO data is in
+ * @val: returns the value
+ *
+ * Returns 0 for success, negative on error
+ */
+int ocxl_global_mmio_read32(struct ocxl_afu *afu, size_t offset,
+ enum ocxl_endian endian, u32 *val);
+
+/**
+ * Read a 64 bit value from global MMIO
+ *
+ * @afu: The AFU
+ * @offset: The Offset from the start of MMIO
+ * @endian: the endianness that the MMIO data is in
+ * @val: returns the value
+ *
+ * Returns 0 for success, negative on error
+ */
+int ocxl_global_mmio_read64(struct ocxl_afu *afu, size_t offset,
+ enum ocxl_endian endian, u64 *val);
+
+/**
+ * Write a 32 bit value to global MMIO
+ *
+ * @afu: The AFU
+ * @offset: The Offset from the start of MMIO
+ * @endian: the endianness that the MMIO data is in
+ * @val: The value to write
+ *
+ * Returns 0 for success, negative on error
+ */
+int ocxl_global_mmio_write32(struct ocxl_afu *afu, size_t offset,
+ enum ocxl_endian endian, u32 val);
+
+/**
+ * Write a 64 bit value to global MMIO
+ *
+ * @afu: The AFU
+ * @offset: The Offset from the start of MMIO
+ * @endian: the endianness that the MMIO data is in
+ * @val: The value to write
+ *
+ * Returns 0 for success, negative on error
+ */
+int ocxl_global_mmio_write64(struct ocxl_afu *afu, size_t offset,
+ enum ocxl_endian endian, u64 val);
+
+/**
+ * Set bits in a 32 bit global MMIO register
+ *
+ * @afu: The AFU
+ * @offset: The Offset from the start of MMIO
+ * @endian: the endianness that the MMIO data is in
+ * @mask: a mask of the bits to set
+ *
+ * Returns 0 for success, negative on error
+ */
+int ocxl_global_mmio_set32(struct ocxl_afu *afu, size_t offset,
+ enum ocxl_endian endian, u32 mask);
+
+/**
+ * Set bits in a 64 bit global MMIO register
+ *
+ * @afu: The AFU
+ * @offset: The Offset from the start of MMIO
+ * @endian: the endianness that the MMIO data is in
+ * @mask: a mask of the bits to set
+ *
+ * Returns 0 for success, negative on error
+ */
+int ocxl_global_mmio_set64(struct ocxl_afu *afu, size_t offset,
+ enum ocxl_endian endian, u64 mask);
+
+/**
+ * Set bits in a 32 bit global MMIO register
+ *
+ * @afu: The AFU
+ * @offset: The Offset from the start of MMIO
+ * @endian: the endianness that the MMIO data is in
+ * @mask: a mask of the bits to set
+ *
+ * Returns 0 for success, negative on error
+ */
+int ocxl_global_mmio_clear32(struct ocxl_afu *afu, size_t offset,
+ enum ocxl_endian endian, u32 mask);
+
+/**
+ * Set bits in a 64 bit global MMIO register
+ *
+ * @afu: The AFU
+ * @offset: The Offset from the start of MMIO
+ * @endian: the endianness that the MMIO data is in
+ * @mask: a mask of the bits to set
+ *
+ * Returns 0 for success, negative on error
+ */
+int ocxl_global_mmio_clear64(struct ocxl_afu *afu, size_t offset,
+ enum ocxl_endian endian, u64 mask);
// Functions left here are for compatibility with the cxlflash driver
--
2.20.1
^ permalink raw reply related
* [PATCH v2 6/7] ocxl: move event_fd handling to frontend
From: Alastair D'Silva @ 2019-03-20 5:08 UTC (permalink / raw)
To: alastair
Cc: Arnd Bergmann, Greg Kroah-Hartman, linux-kernel, Andrew Donnellan,
Frederic Barrat, linuxppc-dev
In-Reply-To: <20190320050901.310-1-alastair@au1.ibm.com>
From: Alastair D'Silva <alastair@d-silva.org>
Event_fd is only used in the driver frontend, so it does not
need to exist in the backend code. Relocate it to the frontend
and provide an opaque mechanism for consumers instead.
Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
---
drivers/misc/ocxl/afu_irq.c | 69 +++++++++++++++++--------------
drivers/misc/ocxl/file.c | 22 +++++++++-
drivers/misc/ocxl/ocxl_internal.h | 5 ---
include/misc/ocxl.h | 46 +++++++++++++++++++++
4 files changed, 104 insertions(+), 38 deletions(-)
diff --git a/drivers/misc/ocxl/afu_irq.c b/drivers/misc/ocxl/afu_irq.c
index 2d410cd6f817..f23cd585e737 100644
--- a/drivers/misc/ocxl/afu_irq.c
+++ b/drivers/misc/ocxl/afu_irq.c
@@ -1,7 +1,7 @@
// SPDX-License-Identifier: GPL-2.0+
// Copyright 2017 IBM Corp.
#include <linux/interrupt.h>
-#include <linux/eventfd.h>
+#include <asm/pnv-ocxl.h>
#include "ocxl_internal.h"
#include "trace.h"
@@ -11,7 +11,9 @@ struct afu_irq {
unsigned int virq;
char *name;
u64 trigger_page;
- struct eventfd_ctx *ev_ctx;
+ irqreturn_t (*handler)(void *private);
+ void (*free_private)(void *private);
+ void *private;
};
int ocxl_irq_offset_to_id(struct ocxl_context *ctx, u64 offset)
@@ -24,14 +26,42 @@ u64 ocxl_irq_id_to_offset(struct ocxl_context *ctx, int irq_id)
return ctx->afu->irq_base_offset + (irq_id << PAGE_SHIFT);
}
+int ocxl_irq_set_handler(struct ocxl_context *ctx, int irq_id,
+ irqreturn_t (*handler)(void *private),
+ void (*free_private)(void *private),
+ void *private)
+{
+ struct afu_irq *irq;
+ int rc;
+
+ mutex_lock(&ctx->irq_lock);
+ irq = idr_find(&ctx->irq_idr, irq_id);
+ if (!irq) {
+ rc = -EINVAL;
+ goto unlock;
+ }
+
+ irq->handler = handler;
+ irq->private = private;
+
+ rc = 0;
+ goto unlock;
+
+unlock:
+ mutex_unlock(&ctx->irq_lock);
+ return rc;
+}
+
static irqreturn_t afu_irq_handler(int virq, void *data)
{
struct afu_irq *irq = (struct afu_irq *) data;
trace_ocxl_afu_irq_receive(virq);
- if (irq->ev_ctx)
- eventfd_signal(irq->ev_ctx, 1);
- return IRQ_HANDLED;
+
+ if (irq->handler)
+ return irq->handler(irq->private);
+
+ return IRQ_HANDLED; // Just drop it on the ground
}
static int setup_afu_irq(struct ocxl_context *ctx, struct afu_irq *irq)
@@ -126,8 +156,8 @@ static void afu_irq_free(struct afu_irq *irq, struct ocxl_context *ctx)
ocxl_irq_id_to_offset(ctx, irq->id),
1 << PAGE_SHIFT, 1);
release_afu_irq(irq);
- if (irq->ev_ctx)
- eventfd_ctx_put(irq->ev_ctx);
+ if (irq->free_private)
+ irq->free_private(irq->private);
ocxl_link_free_irq(ctx->afu->fn->link, irq->hw_irq);
kfree(irq);
}
@@ -160,31 +190,6 @@ void ocxl_afu_irq_free_all(struct ocxl_context *ctx)
mutex_unlock(&ctx->irq_lock);
}
-int ocxl_afu_irq_set_fd(struct ocxl_context *ctx, int irq_id, int eventfd)
-{
- struct afu_irq *irq;
- struct eventfd_ctx *ev_ctx;
- int rc = 0;
-
- mutex_lock(&ctx->irq_lock);
- irq = idr_find(&ctx->irq_idr, irq_id);
- if (!irq) {
- rc = -EINVAL;
- goto unlock;
- }
-
- ev_ctx = eventfd_ctx_fdget(eventfd);
- if (IS_ERR(ev_ctx)) {
- rc = -EINVAL;
- goto unlock;
- }
-
- irq->ev_ctx = ev_ctx;
-unlock:
- mutex_unlock(&ctx->irq_lock);
- return rc;
-}
-
u64 ocxl_afu_irq_get_addr(struct ocxl_context *ctx, int irq_id)
{
struct afu_irq *irq;
diff --git a/drivers/misc/ocxl/file.c b/drivers/misc/ocxl/file.c
index d13297336253..42214b0c956a 100644
--- a/drivers/misc/ocxl/file.c
+++ b/drivers/misc/ocxl/file.c
@@ -3,6 +3,7 @@
#include <linux/fs.h>
#include <linux/poll.h>
#include <linux/sched/signal.h>
+#include <linux/eventfd.h>
#include <linux/uaccess.h>
#include <uapi/misc/ocxl.h>
#include <asm/reg.h>
@@ -191,11 +192,27 @@ static long afu_ioctl_get_features(struct ocxl_context *ctx,
x == OCXL_IOCTL_GET_FEATURES ? "GET_FEATURES" : \
"UNKNOWN")
+static irqreturn_t irq_handler(void *private)
+{
+ struct eventfd_ctx *ev_ctx = private;
+
+ eventfd_signal(ev_ctx, 1);
+ return IRQ_HANDLED;
+}
+
+static void irq_free(void *private)
+{
+ struct eventfd_ctx *ev_ctx = private;
+
+ eventfd_ctx_put(ev_ctx);
+}
+
static long afu_ioctl(struct file *file, unsigned int cmd,
unsigned long args)
{
struct ocxl_context *ctx = file->private_data;
struct ocxl_ioctl_irq_fd irq_fd;
+ struct eventfd_ctx *ev_ctx;
int irq_id;
u64 irq_offset;
long rc;
@@ -247,7 +264,10 @@ static long afu_ioctl(struct file *file, unsigned int cmd,
if (irq_fd.reserved)
return -EINVAL;
irq_id = ocxl_irq_offset_to_id(ctx, irq_fd.irq_offset);
- rc = ocxl_afu_irq_set_fd(ctx, irq_id, irq_fd.eventfd);
+ ev_ctx = eventfd_ctx_fdget(irq_fd.eventfd);
+ if (!ev_ctx)
+ return -EFAULT;
+ rc = ocxl_irq_set_handler(ctx, irq_id, irq_handler, irq_free, ev_ctx);
break;
case OCXL_IOCTL_GET_METADATA:
diff --git a/drivers/misc/ocxl/ocxl_internal.h b/drivers/misc/ocxl/ocxl_internal.h
index 871155b464da..f3775223f4b1 100644
--- a/drivers/misc/ocxl/ocxl_internal.h
+++ b/drivers/misc/ocxl/ocxl_internal.h
@@ -142,12 +142,7 @@ void ocxl_sysfs_unregister_afu(struct ocxl_afu *afu);
int ocxl_irq_offset_to_id(struct ocxl_context *ctx, u64 offset);
u64 ocxl_irq_id_to_offset(struct ocxl_context *ctx, int irq_id);
-int ocxl_afu_irq_alloc(struct ocxl_context *ctx, int *irq_id);
-int ocxl_afu_irq_free(struct ocxl_context *ctx, int irq_id);
void ocxl_afu_irq_free_all(struct ocxl_context *ctx);
-int ocxl_afu_irq_set_fd(struct ocxl_context *ctx, int irq_id,
- int eventfd);
-u64 ocxl_afu_irq_get_addr(struct ocxl_context *ctx, int irq_id);
/**
* Free an AFU
diff --git a/include/misc/ocxl.h b/include/misc/ocxl.h
index a5e94da6c8db..790b25987d9a 100644
--- a/include/misc/ocxl.h
+++ b/include/misc/ocxl.h
@@ -161,6 +161,52 @@ int ocxl_context_attach(struct ocxl_context *ctx, u64 amr,
*/
int ocxl_context_detach(struct ocxl_context *ctx);
+// AFU IRQs
+
+/**
+ * Allocate an IRQ associated with an AFU context
+ * @ctx: the AFU context
+ * @irq_id: out, the IRQ ID
+ *
+ * Returns 0 on success, negative on failure
+ */
+extern int ocxl_afu_irq_alloc(struct ocxl_context *ctx, int *irq_id);
+
+/**
+ * Frees an IRQ associated with an AFU context
+ * @ctx: the AFU context
+ * @irq_id: the IRQ ID
+ *
+ * Returns 0 on success, negative on failure
+ */
+extern int ocxl_afu_irq_free(struct ocxl_context *ctx, int irq_id);
+
+/**
+ * Gets the address of the trigger page for an IRQ
+ * This can then be provided to an AFU which will write to that
+ * page to trigger the IRQ.
+ * @ctx: The AFU context that the IRQ is associated with
+ * @irq_id: The IRQ ID
+ *
+ * returns the trigger page address, or 0 if the IRQ is not valid
+ */
+extern u64 ocxl_afu_irq_get_addr(struct ocxl_context *ctx, int irq_id);
+
+/**
+ * Provide a callback to be called when an IRQ is triggered
+ * @ctx: The AFU context that the IRQ is associated with
+ * @irq_id: The IRQ ID
+ * @handler: the callback to be called when the IRQ is triggered
+ * @free_private: the callback to be called when the IRQ is freed
+ * @private: Private data to be passed to the callbacks
+ *
+ * Returns 0 on success, negative on failure
+ */
+int ocxl_irq_set_handler(struct ocxl_context *ctx, int irq_id,
+ irqreturn_t (*handler)(void *private),
+ void (*free_private)(void *private),
+ void *private);
+
// AFU Metadata
/**
--
2.20.1
^ permalink raw reply related
* [PATCH v2 5/7] ocxl: afu_irq only deals with IRQ IDs, not offsets
From: Alastair D'Silva @ 2019-03-20 5:08 UTC (permalink / raw)
To: alastair
Cc: Arnd Bergmann, Greg Kroah-Hartman, linux-kernel, Andrew Donnellan,
Frederic Barrat, linuxppc-dev
In-Reply-To: <20190320050901.310-1-alastair@au1.ibm.com>
From: Alastair D'Silva <alastair@d-silva.org>
The use of offsets is required only in the frontend, so alter
the IRQ API to only work with IRQ IDs in the backend.
Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
---
drivers/misc/ocxl/afu_irq.c | 34 +++++++++++++++----------------
drivers/misc/ocxl/context.c | 7 +++++--
drivers/misc/ocxl/file.c | 13 +++++++-----
drivers/misc/ocxl/ocxl_internal.h | 10 +++++----
drivers/misc/ocxl/trace.h | 12 ++++-------
5 files changed, 39 insertions(+), 37 deletions(-)
diff --git a/drivers/misc/ocxl/afu_irq.c b/drivers/misc/ocxl/afu_irq.c
index 11ab996657a2..2d410cd6f817 100644
--- a/drivers/misc/ocxl/afu_irq.c
+++ b/drivers/misc/ocxl/afu_irq.c
@@ -14,14 +14,14 @@ struct afu_irq {
struct eventfd_ctx *ev_ctx;
};
-static int irq_offset_to_id(struct ocxl_context *ctx, u64 offset)
+int ocxl_irq_offset_to_id(struct ocxl_context *ctx, u64 offset)
{
return (offset - ctx->afu->irq_base_offset) >> PAGE_SHIFT;
}
-static u64 irq_id_to_offset(struct ocxl_context *ctx, int id)
+u64 ocxl_irq_id_to_offset(struct ocxl_context *ctx, int irq_id)
{
- return ctx->afu->irq_base_offset + (id << PAGE_SHIFT);
+ return ctx->afu->irq_base_offset + (irq_id << PAGE_SHIFT);
}
static irqreturn_t afu_irq_handler(int virq, void *data)
@@ -69,7 +69,7 @@ static void release_afu_irq(struct afu_irq *irq)
kfree(irq->name);
}
-int ocxl_afu_irq_alloc(struct ocxl_context *ctx, u64 *irq_offset)
+int ocxl_afu_irq_alloc(struct ocxl_context *ctx, int *irq_id)
{
struct afu_irq *irq;
int rc;
@@ -101,11 +101,11 @@ int ocxl_afu_irq_alloc(struct ocxl_context *ctx, u64 *irq_offset)
if (rc)
goto err_alloc;
- *irq_offset = irq_id_to_offset(ctx, irq->id);
-
- trace_ocxl_afu_irq_alloc(ctx->pasid, irq->id, irq->virq, irq->hw_irq,
- *irq_offset);
+ trace_ocxl_afu_irq_alloc(ctx->pasid, irq->id, irq->virq, irq->hw_irq);
mutex_unlock(&ctx->irq_lock);
+
+ *irq_id = irq->id;
+
return 0;
err_alloc:
@@ -123,7 +123,7 @@ static void afu_irq_free(struct afu_irq *irq, struct ocxl_context *ctx)
trace_ocxl_afu_irq_free(ctx->pasid, irq->id);
if (ctx->mapping)
unmap_mapping_range(ctx->mapping,
- irq_id_to_offset(ctx, irq->id),
+ ocxl_irq_id_to_offset(ctx, irq->id),
1 << PAGE_SHIFT, 1);
release_afu_irq(irq);
if (irq->ev_ctx)
@@ -132,14 +132,13 @@ static void afu_irq_free(struct afu_irq *irq, struct ocxl_context *ctx)
kfree(irq);
}
-int ocxl_afu_irq_free(struct ocxl_context *ctx, u64 irq_offset)
+int ocxl_afu_irq_free(struct ocxl_context *ctx, int irq_id)
{
struct afu_irq *irq;
- int id = irq_offset_to_id(ctx, irq_offset);
mutex_lock(&ctx->irq_lock);
- irq = idr_find(&ctx->irq_idr, id);
+ irq = idr_find(&ctx->irq_idr, irq_id);
if (!irq) {
mutex_unlock(&ctx->irq_lock);
return -EINVAL;
@@ -161,14 +160,14 @@ void ocxl_afu_irq_free_all(struct ocxl_context *ctx)
mutex_unlock(&ctx->irq_lock);
}
-int ocxl_afu_irq_set_fd(struct ocxl_context *ctx, u64 irq_offset, int eventfd)
+int ocxl_afu_irq_set_fd(struct ocxl_context *ctx, int irq_id, int eventfd)
{
struct afu_irq *irq;
struct eventfd_ctx *ev_ctx;
- int rc = 0, id = irq_offset_to_id(ctx, irq_offset);
+ int rc = 0;
mutex_lock(&ctx->irq_lock);
- irq = idr_find(&ctx->irq_idr, id);
+ irq = idr_find(&ctx->irq_idr, irq_id);
if (!irq) {
rc = -EINVAL;
goto unlock;
@@ -186,14 +185,13 @@ int ocxl_afu_irq_set_fd(struct ocxl_context *ctx, u64 irq_offset, int eventfd)
return rc;
}
-u64 ocxl_afu_irq_get_addr(struct ocxl_context *ctx, u64 irq_offset)
+u64 ocxl_afu_irq_get_addr(struct ocxl_context *ctx, int irq_id)
{
struct afu_irq *irq;
- int id = irq_offset_to_id(ctx, irq_offset);
u64 addr = 0;
mutex_lock(&ctx->irq_lock);
- irq = idr_find(&ctx->irq_idr, id);
+ irq = idr_find(&ctx->irq_idr, irq_id);
if (irq)
addr = irq->trigger_page;
mutex_unlock(&ctx->irq_lock);
diff --git a/drivers/misc/ocxl/context.c b/drivers/misc/ocxl/context.c
index 9a37e9632cd9..c04887591837 100644
--- a/drivers/misc/ocxl/context.c
+++ b/drivers/misc/ocxl/context.c
@@ -93,8 +93,9 @@ static vm_fault_t map_afu_irq(struct vm_area_struct *vma, unsigned long address,
u64 offset, struct ocxl_context *ctx)
{
u64 trigger_addr;
+ int irq_id = ocxl_irq_offset_to_id(ctx, offset);
- trigger_addr = ocxl_afu_irq_get_addr(ctx, offset);
+ trigger_addr = ocxl_afu_irq_get_addr(ctx, irq_id);
if (!trigger_addr)
return VM_FAULT_SIGBUS;
@@ -154,12 +155,14 @@ static const struct vm_operations_struct ocxl_vmops = {
static int check_mmap_afu_irq(struct ocxl_context *ctx,
struct vm_area_struct *vma)
{
+ int irq_id = ocxl_irq_offset_to_id(ctx, vma->vm_pgoff << PAGE_SHIFT);
+
/* only one page */
if (vma_pages(vma) != 1)
return -EINVAL;
/* check offset validty */
- if (!ocxl_afu_irq_get_addr(ctx, vma->vm_pgoff << PAGE_SHIFT))
+ if (!ocxl_afu_irq_get_addr(ctx, irq_id))
return -EINVAL;
/*
diff --git a/drivers/misc/ocxl/file.c b/drivers/misc/ocxl/file.c
index 665422c6c8a0..d13297336253 100644
--- a/drivers/misc/ocxl/file.c
+++ b/drivers/misc/ocxl/file.c
@@ -196,6 +196,7 @@ static long afu_ioctl(struct file *file, unsigned int cmd,
{
struct ocxl_context *ctx = file->private_data;
struct ocxl_ioctl_irq_fd irq_fd;
+ int irq_id;
u64 irq_offset;
long rc;
bool closed;
@@ -217,12 +218,13 @@ static long afu_ioctl(struct file *file, unsigned int cmd,
break;
case OCXL_IOCTL_IRQ_ALLOC:
- rc = ocxl_afu_irq_alloc(ctx, &irq_offset);
+ rc = ocxl_afu_irq_alloc(ctx, &irq_id);
if (!rc) {
+ irq_offset = ocxl_irq_id_to_offset(ctx, irq_id);
rc = copy_to_user((u64 __user *) args, &irq_offset,
sizeof(irq_offset));
if (rc) {
- ocxl_afu_irq_free(ctx, irq_offset);
+ ocxl_afu_irq_free(ctx, irq_id);
return -EFAULT;
}
}
@@ -233,7 +235,8 @@ static long afu_ioctl(struct file *file, unsigned int cmd,
sizeof(irq_offset));
if (rc)
return -EFAULT;
- rc = ocxl_afu_irq_free(ctx, irq_offset);
+ irq_id = ocxl_irq_offset_to_id(ctx, irq_offset);
+ rc = ocxl_afu_irq_free(ctx, irq_id);
break;
case OCXL_IOCTL_IRQ_SET_FD:
@@ -243,8 +246,8 @@ static long afu_ioctl(struct file *file, unsigned int cmd,
return -EFAULT;
if (irq_fd.reserved)
return -EINVAL;
- rc = ocxl_afu_irq_set_fd(ctx, irq_fd.irq_offset,
- irq_fd.eventfd);
+ irq_id = ocxl_irq_offset_to_id(ctx, irq_fd.irq_offset);
+ rc = ocxl_afu_irq_set_fd(ctx, irq_id, irq_fd.eventfd);
break;
case OCXL_IOCTL_GET_METADATA:
diff --git a/drivers/misc/ocxl/ocxl_internal.h b/drivers/misc/ocxl/ocxl_internal.h
index 4fc7e9597ede..871155b464da 100644
--- a/drivers/misc/ocxl/ocxl_internal.h
+++ b/drivers/misc/ocxl/ocxl_internal.h
@@ -140,12 +140,14 @@ void ocxl_context_detach_all(struct ocxl_afu *afu);
int ocxl_sysfs_register_afu(struct ocxl_afu *afu);
void ocxl_sysfs_unregister_afu(struct ocxl_afu *afu);
-int ocxl_afu_irq_alloc(struct ocxl_context *ctx, u64 *irq_offset);
-int ocxl_afu_irq_free(struct ocxl_context *ctx, u64 irq_offset);
+int ocxl_irq_offset_to_id(struct ocxl_context *ctx, u64 offset);
+u64 ocxl_irq_id_to_offset(struct ocxl_context *ctx, int irq_id);
+int ocxl_afu_irq_alloc(struct ocxl_context *ctx, int *irq_id);
+int ocxl_afu_irq_free(struct ocxl_context *ctx, int irq_id);
void ocxl_afu_irq_free_all(struct ocxl_context *ctx);
-int ocxl_afu_irq_set_fd(struct ocxl_context *ctx, u64 irq_offset,
+int ocxl_afu_irq_set_fd(struct ocxl_context *ctx, int irq_id,
int eventfd);
-u64 ocxl_afu_irq_get_addr(struct ocxl_context *ctx, u64 irq_offset);
+u64 ocxl_afu_irq_get_addr(struct ocxl_context *ctx, int irq_id);
/**
* Free an AFU
diff --git a/drivers/misc/ocxl/trace.h b/drivers/misc/ocxl/trace.h
index 68bf2f173a1a..a081e644f5f3 100644
--- a/drivers/misc/ocxl/trace.h
+++ b/drivers/misc/ocxl/trace.h
@@ -107,16 +107,14 @@ DEFINE_EVENT(ocxl_fault_handler, ocxl_fault_ack,
);
TRACE_EVENT(ocxl_afu_irq_alloc,
- TP_PROTO(int pasid, int irq_id, unsigned int virq, int hw_irq,
- u64 irq_offset),
- TP_ARGS(pasid, irq_id, virq, hw_irq, irq_offset),
+ TP_PROTO(int pasid, int irq_id, unsigned int virq, int hw_irq),
+ TP_ARGS(pasid, irq_id, virq, hw_irq),
TP_STRUCT__entry(
__field(int, pasid)
__field(int, irq_id)
__field(unsigned int, virq)
__field(int, hw_irq)
- __field(u64, irq_offset)
),
TP_fast_assign(
@@ -124,15 +122,13 @@ TRACE_EVENT(ocxl_afu_irq_alloc,
__entry->irq_id = irq_id;
__entry->virq = virq;
__entry->hw_irq = hw_irq;
- __entry->irq_offset = irq_offset;
),
- TP_printk("pasid=%#x irq_id=%d virq=%u hw_irq=%d irq_offset=%#llx",
+ TP_printk("pasid=%#x irq_id=%d virq=%u hw_irq=%d",
__entry->pasid,
__entry->irq_id,
__entry->virq,
- __entry->hw_irq,
- __entry->irq_offset
+ __entry->hw_irq
)
);
--
2.20.1
^ 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